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

[zhanghailiang <zhang.zhanghailiang@huawei.com>]

On 2015/7/18 1:07, Dr. David Alan Gilbert wrote:
> * 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.
>

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