Re: [PATCH v4 --for 4.6 COLOPre 13/25] migration/save: pass checkpointed_stream from libxl to libxc

[Yang Hongyang <yanghy@cn.fujitsu.com>]

On 07/15/2015 08:38 PM, Ian Campbell wrote:
> On Wed, 2015-07-15 at 15:45 +0800, Yang Hongyang wrote:
>> Pass checkpointed_stream from libxl to libxc.
>> It won't affact legacy migration because legacy migration
>> won't use this param.
>>
>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>> CC: Ian Campbell <Ian.Campbell@citrix.com>
>> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>> CC: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>>   tools/libxc/include/xenguest.h   |  9 ++++++---
>>   tools/libxc/xc_domain_save.c     |  6 ++++--
>>   tools/libxc/xc_nomigrate.c       |  3 ++-
>>   tools/libxc/xc_sr_common.h       |  2 +-
>>   tools/libxc/xc_sr_save.c         |  5 +++--
>>   tools/libxl/libxl.c              |  2 ++
>>   tools/libxl/libxl_dom_save.c     | 11 ++++++++---
>>   tools/libxl/libxl_internal.h     |  1 +
>>   tools/libxl/libxl_save_callout.c |  2 +-
>>   tools/libxl/libxl_save_helper.c  |  3 ++-
>>   10 files changed, 30 insertions(+), 14 deletions(-)
>>
>> diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h
>> index e95af54..6e24b6c 100644
>> --- a/tools/libxc/include/xenguest.h
>> +++ b/tools/libxc/include/xenguest.h
>> @@ -30,7 +30,6 @@
>>   #define XCFLAGS_HVM       (1 << 2)
>>   #define XCFLAGS_STDVGA    (1 << 3)
>>   #define XCFLAGS_CHECKPOINT_COMPRESS    (1 << 4)
>> -#define XCFLAGS_CHECKPOINTED    (1 << 5)
>>
>>   #define X86_64_B_SIZE   64
>>   #define X86_32_B_SIZE   32
>> @@ -85,16 +84,20 @@ struct save_callbacks {
>>    * @parm xch a handle to an open hypervisor interface
>>    * @parm fd the file descriptor to save a domain to
>>    * @parm dom the id of the domain
>> + * @parm checkpointed_stream non-zero if the far end of the stream is using
>> + *       checkpointing
>
> Do (or will) specific non-zero values have any meaning to the libxc
> layer? i.e. does it have any knowledge of COLO vs. Remus as the libxl
> enum added in the last patch does?

Yes, libxc side should be aware of the type of checkpointed_stream (Remus
or COLO).

I think it is better to document the non-zero values here?
for example:
      * @parm checkpointed_stream non-zero if the far end of the stream is using
      *                           checkpointing
      *                           0 no checkpointed stream
      *                           1 Remus
      *                           2 COLO

>
> If (as I hope) the answer is no then this should be a boolean and the
> libxl code which propagates the enum into this field ought to use some
> appropriate condition (!= ..._NONE most likely).
>
> Ian.
>
> .
>

-- 
Thanks,
Yang.