Re: [PATCH v6 COLO 05/15] send store mfn and console mfn to xl before resuming secondary vm

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

On 06/08/2015 08:16 PM, Andrew Cooper wrote:
> On 08/06/15 04:45, Yang Hongyang wrote:
>> From: Wen Congyang <wency@cn.fujitsu.com>
>>
>> We will call libxl__xc_domain_restore_done() to rebuild secondary vm. But
>> we need store mfn and console mfn when rebuilding secondary vm. So make
>> restore_results is a function pointers in callbacks struct and struct
>> {save,restore}_callbacks, and use this callback to send store mfn and
>> console mfn to xl.
>>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>> CC: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>>   tools/libxc/include/xenguest.h     | 8 ++++++++
>>   tools/libxc/xc_sr_restore.c        | 8 ++++++--
>>   tools/libxl/libxl_colo_restore.c   | 5 -----
>>   tools/libxl/libxl_create.c         | 1 +
>>   tools/libxl/libxl_save_msgs_gen.pl | 2 +-
>>   5 files changed, 16 insertions(+), 8 deletions(-)
>>
>> diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h
>> index d5902a6..50096b9 100644
>> --- a/tools/libxc/include/xenguest.h
>> +++ b/tools/libxc/include/xenguest.h
>> @@ -130,6 +130,14 @@ struct restore_callbacks {
>>       /* Enable qemu-dm logging dirty pages to xen */
>>       int (*switch_qemu_logdirty)(int domid, unsigned enable, void *data); /* HVM only */
>>
>> +    /*
>> +     * callback to send store mfn and console mfn to xl
>> +     * if we want to resume vm before xc_domain_save()
>> +     * exits.
>> +     */
>> +    void (*restore_results)(unsigned long store_mfn, unsigned long console_mfn,
>> +                            void *data);
>> +
>>       /* callback to restore toolstack specific data */
>>       int (*toolstack_restore)(uint32_t domid, const uint8_t *buf,
>>               uint32_t size, void* data);
>> diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c
>> index 982a70e..5e2efd8 100644
>> --- a/tools/libxc/xc_sr_restore.c
>> +++ b/tools/libxc/xc_sr_restore.c
>> @@ -524,7 +524,10 @@ static int handle_checkpoint(struct xc_sr_context *ctx)
>>           if ( rc )
>>               goto err;
>>
>> -        /* TODO: call restore_results */
>> +        /* call restore_results */
>
> I would drop this comment.  It is entirely redundant now.

will do, thanks.

>
> Otherwise, looks good.
>
> ~Andrew
>
>> +        ctx->restore.callbacks->restore_results(ctx->restore.xenstore_gfn,
>> +                                                ctx->restore.console_gfn,
>> +                                                ctx->restore.callbacks->data);
>>
>>           /* Resume secondary vm */
>>           ret = ctx->restore.callbacks->postcopy(ctx->restore.callbacks->data);
>> @@ -793,7 +796,8 @@ int xc_domain_restore2(xc_interface *xch, int io_fd, uint32_t dom,
>>           /* this is COLO restore */
>>           assert(callbacks->suspend &&
>>                  callbacks->checkpoint &&
>> -               callbacks->postcopy);
>> +               callbacks->postcopy &&
>> +               callbacks->restore_results);
>>       }
>>
>>       IPRINTF("In experimental %s", __func__);
>> diff --git a/tools/libxl/libxl_colo_restore.c b/tools/libxl/libxl_colo_restore.c
>> index 6c39758..c613c15 100644
>> --- a/tools/libxl/libxl_colo_restore.c
>> +++ b/tools/libxl/libxl_colo_restore.c
>> @@ -153,11 +153,6 @@ static void colo_resume_vm(libxl__egc *egc,
>>           return;
>>       }
>>
>> -    /*
>> -     * TODO: get store mfn and console mfn
>> -     *  We should call the callback restore_results in
>> -     *  xc_domain_restore() before resuming the guest.
>> -     */
>>       libxl__xc_domain_restore_done(egc, dcs, 0, 0, 0);
>>
>>       return;
>> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
>> index 1548b70..6e307f3 100644
>> --- a/tools/libxl/libxl_create.c
>> +++ b/tools/libxl/libxl_create.c
>> @@ -1157,6 +1157,7 @@ static void domcreate_bootloader_done(libxl__egc *egc,
>>           rc = ERROR_INVAL;
>>           goto out;
>>       }
>> +    callbacks->restore_results = libxl__srm_callout_callback_restore_results;
>>
>>       if (checkpointed_stream == LIBXL_CHECKPOINTED_STREAM_COLO) {
>>           crs->ao = ao;
>> diff --git a/tools/libxl/libxl_save_msgs_gen.pl b/tools/libxl/libxl_save_msgs_gen.pl
>> index fbb2d67..2ecd25d 100755
>> --- a/tools/libxl/libxl_save_msgs_gen.pl
>> +++ b/tools/libxl/libxl_save_msgs_gen.pl
>> @@ -32,7 +32,7 @@ our @msgs = (
>>       #                toolstack_save          done entirely `by hand'
>>       [  7, 'rcxW',   "toolstack_restore",     [qw(uint32_t domid
>>                                                   BLOCK tsdata)] ],
>> -    [  8, 'r',      "restore_results",       ['unsigned long', 'store_mfn',
>> +    [  8, 'rcx',    "restore_results",       ['unsigned long', 'store_mfn',
>>                                                 'unsigned long', 'console_mfn'] ],
>>       [  9, 'srW',    "complete",              [qw(int retval
>>                                                    int errnoval)] ],
>
> .
>

-- 
Thanks,
Yang.