Re: [Qemu-devel] [PATCH v9 03/10] Add save_block_hdr function

[Anthony Liguori]

On 04/18/2012 12:40 PM, Juan Quintela wrote:
> Anthony Liguori<address@hidden>  wrote:
> > On 04/11/2012 01:49 PM, Orit Wasserman wrote:
> > > Signed-off-by: Orit Wasserman<address@hidden>
> > > Signed-off-by: Benoit Hudzia<address@hidden>
> > > Signed-off-by: Petter Svard<address@hidden>
> > > Signed-off-by: Aidan Shribman<address@hidden>
> > > ---
> > >    arch_init.c |   26 ++++++++++++++------------
> > >    1 files changed, 14 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/arch_init.c b/arch_init.c
> > > index 2e534f1..47b9fef 100644
> > > --- a/arch_init.c
> > > +++ b/arch_init.c
> > > @@ -347,6 +347,18 @@ void cache_resize(int64_t new_size)
> > >        g_free(old_page_cache);
> > >    }
> > >
> > > +static void save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t offset,
> > > +        int cont, int flag)
> > > +{
> > > +        qemu_put_be64(f, offset | cont | flag);
> > > +        if (!cont) {
> > > +                qemu_put_byte(f, strlen(block->idstr));
> > > +                qemu_put_buffer(f, (uint8_t *)block->idstr,
> > > +                                strlen(block->idstr));
> > > +        }
> > > +
> > > +}
> >
> >
> > Any time we're changing protocols/formats, we need to document it.  We
> > need a docs/xbzrle.txt (and it should be before this patch).
>
> Agreed.
>
> > It's not clear to me how we preserve compatibility here either.
> > You're potentially passing garbage to an older QEMU.
>
> I think not.  Only if you are migrating with the -x option.  And then,
> you get what you asked for.
>
> > I thought I had previously asked for a monitor command to negotiate
> > extensions for migration?
>
> I think it is in another patch.

So I think we also need:

{ 'command': 'set-migration-capabilities',
    'data': { 'enable': ['MigrationCapability'] }

Then a management tool just needs to:

    caps_from_src = query-migration-capabilities(src_qmp_session)
    caps_from_dst = query-migration-capabilities(dst_qmp_session)

    common_caps = intersection(caps_from_src, caps_from_dst)

    set-migration-capabilities(src_qmp_session, common_caps);
    set-migration-capabilities(dst_qmp_session, common_caps);

Then libvirt doesn't special code to handle XBRLE or whatever the next new 
migration protocol feature is.  With the current patches, libvirt would need to 
create a table of:

   migration_features = {'xblre': '-x'}

Which would change every time we add a feature.  That seems pretty undesirable 
to me.

Regards,

Anthony Liguori

> Later, Juan