Re: [RFC PATCH 1/3] remus: add a bool var to indicate checkpointed stream

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

Hi Andrew,

   Thanks for the comments!

On 07/09/2014 05:45 PM, Andrew Cooper wrote:
> On 09/07/14 08:47, Yang Hongyang wrote:
>> add a bool variable to migration context to indicate checkpointed stream
>>
>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>> ---
>>   tools/libxc/saverestore/common.h | 1 +
>>   tools/libxc/saverestore/save.c   | 7 +++++++
>>   tools/libxc/xenguest.h           | 1 +
>>   tools/libxl/libxl_dom.c          | 1 +
>>   4 files changed, 10 insertions(+)
>>
>> diff --git a/tools/libxc/saverestore/common.h b/tools/libxc/saverestore/common.h
>> index 4840d3f..24ba95b 100644
>> --- a/tools/libxc/saverestore/common.h
>> +++ b/tools/libxc/saverestore/common.h
>> @@ -152,6 +152,7 @@ struct xc_sr_context
>>       int fd;
>>
>>       xc_dominfo_t dominfo;
>> +    bool checkpointed;
>
> This appears to only be used on the save side, so should live in the
> .save union.  Also, a comment indicating that this is a remus option.

I also consider to put this in the .save union, but I assume it will
also be used in restore code... So does ctx.firsttime
There's a checkpointed_stream parameter in xc_domain_restore()...

>
>>
>>       union
>>       {
>> diff --git a/tools/libxc/saverestore/save.c b/tools/libxc/saverestore/save.c
>> index 653dc8e..d2fa8a6 100644
>> --- a/tools/libxc/saverestore/save.c
>> +++ b/tools/libxc/saverestore/save.c
>> @@ -652,6 +652,13 @@ int xc_domain_save2(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_ite
>>       ctx.save.callbacks = callbacks;
>>       ctx.save.live  = !!(flags & XCFLAGS_LIVE);
>>       ctx.save.debug = !!(flags & XCFLAGS_DEBUG);
>> +    ctx.checkpointed = !!(flags & XCFLAGS_CHECKPOINTED);
>> +
>> +    if ( ctx.checkpointed ) {
>
> For better or for worse, I have been using the Xen style, as libxc is a
> mix of the two.  For consistency with the rest of the migration v2 code,
> this brace should be on the next line.

Get it!

>
> ~Andrew
>
>> +        /* This is a checkpointed save, we need these callbacks */
>> +        assert(ctx.save.callbacks->checkpoint);
>> +        assert(ctx.save.callbacks->postcopy);
>> +    }
>>
>>       IPRINTF("In experimental %s", __func__);
>>       DPRINTF("fd %d, dom %"PRIu32", max_iters %"PRIu32", max_factor %"PRIu32
>> diff --git a/tools/libxc/xenguest.h b/tools/libxc/xenguest.h
>> index 55755cf..72be3bf 100644
>> --- a/tools/libxc/xenguest.h
>> +++ b/tools/libxc/xenguest.h
>> @@ -28,6 +28,7 @@
>>   #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
>> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
>> index 83eb29a..4a3fd73 100644
>> --- a/tools/libxl/libxl_dom.c
>> +++ b/tools/libxl/libxl_dom.c
>> @@ -1573,6 +1573,7 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss)
>>
>>       if (r_info != NULL) {
>>           dss->interval = r_info->interval;
>> +        dss->xcflags |= XCFLAGS_CHECKPOINTED;
>>           if (r_info->compression)
>>               dss->xcflags |= XCFLAGS_CHECKPOINT_COMPRESS;
>>       }
>
> .
>

-- 
Thanks,
Yang.