Re: [Qemu-devel] [PATCH COLO-Frame v7 04/34] colo-comm/migration: skip colo info section for special cases

[zhanghailiang]

On 2015/7/18 1:07, Dr. David Alan Gilbert wrote:
> * zhanghailiang (address@hidden) wrote:
> > For older machine types, we skip the colo info section when do
> > migration, in this way, we can migrate successfully between older mainchine and
> > the new one.
> >
> > We also skip this section if colo is not enabled (i.e.
> > migrate_set_capability colo on), so that, It not break compatibility with 
> > migration
> > however the --enable-colo/disable-colo on the source/destination;
> >
> > Signed-off-by: zhanghailiang <address@hidden>
> > ---
> >   hw/i386/pc_piix.c             |  1 +
> >   hw/i386/pc_q35.c              |  1 +
> >   hw/ppc/spapr.c                |  1 +
> >   include/migration/migration.h |  1 +
> >   migration/colo-comm.c         | 13 +++++++++++++
> >   5 files changed, 17 insertions(+)
> >
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index 8167b12..926b0d8 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -313,6 +313,7 @@ static void pc_compat_2_3(MachineState *machine)
> >       }
> >       global_state_set_optional();
> >       savevm_skip_configuration();
> > +    savevm_skip_colo_state();
> >   }
> >
> >   static void pc_compat_2_2(MachineState *machine)
> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > index 974aead..b5c6c85 100644
> > --- a/hw/i386/pc_q35.c
> > +++ b/hw/i386/pc_q35.c
> > @@ -296,6 +296,7 @@ static void pc_compat_2_3(MachineState *machine)
> >       }
> >       global_state_set_optional();
> >       savevm_skip_configuration();
> > +    savevm_skip_colo_state();
> >   }
> >
> >   static void pc_compat_2_2(MachineState *machine)
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index a6f1947..568de93 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1880,6 +1880,7 @@ static void spapr_compat_2_3(Object *obj)
> >   {
> >       savevm_skip_section_footers();
> >       global_state_set_optional();
> > +    savevm_skip_colo_state();
> >   }
> >
> >   static void spapr_compat_2_2(Object *obj)
> > diff --git a/include/migration/migration.h b/include/migration/migration.h
> > index 5c797d4..1b23517 100644
> > --- a/include/migration/migration.h
> > +++ b/include/migration/migration.h
> > @@ -203,4 +203,5 @@ void savevm_skip_section_footers(void);
> >   void register_global_state(void);
> >   void global_state_set_optional(void);
> >   void savevm_skip_configuration(void);
> > +void savevm_skip_colo_state(void);
> >   #endif
> > diff --git a/migration/colo-comm.c b/migration/colo-comm.c
> > index 0a93672..3c8e361 100644
> > --- a/migration/colo-comm.c
> > +++ b/migration/colo-comm.c
> > @@ -21,6 +21,11 @@ typedef struct {
> >
> >   static COLOInfo colo_info;
> >
> > +void savevm_skip_colo_state(void)
> > +{
> > +    colo_info.skip = true;
> > +}
> > +
> >   static void colo_info_pre_save(void *opaque)
> >   {
> >       COLOInfo *s = opaque;
> > @@ -32,12 +37,20 @@ static void colo_info_pre_save(void *opaque)
> >       }
> >   }
> >
> > +static bool colo_info_need(void *opaque)
> > +{
> > +    if (migrate_enable_colo() && !colo_info.skip) {
> > +        return true;
> > +     }
> > +    return false;
> > +}
>
> That will work, but I think (untested) this can just be:
>
> +static bool colo_info_need(void *opaque)
> +{
> +    return migrate_enable_colo();
> +}
>
> and then you can get rid of the skip stuff (and merge it
> back to the previous patch).
>
> Old qemu's will never sent the section so we're safe.
> New qemu's with that flag unset won't send the section, so
>    they're still migration compatible on the machine type.
> New qemu's with the flag set will use it.

Yes, you are right, the 'skip stuff' is redundant, will fix it in next version, 
thanks.

> >   static const VMStateDescription colo_state = {
> >        .name = "COLOState",
> >        .version_id = 1,
> >        .minimum_version_id = 1,
> >        .pre_save = colo_info_pre_save,
> > +     .needed = colo_info_need,
> >        .fields = (VMStateField[]) {
> >            VMSTATE_UINT32(colo_requested, COLOInfo),
> >            VMSTATE_END_OF_LIST()
> > --
> > 1.7.12.4
> --
> Dr. David Alan Gilbert / address@hidden / Manchester, UK
>
> .