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

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* zhanghailiang (zhang.zhanghailiang@huawei.com) 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 <zhang.zhanghailiang@huawei.com>
> ---
>  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.

Dave

>  
>  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 / dgilbert@redhat.com / Manchester, UK