Re: [PATCH v4 --for 4.6 COLOPre 14/25] tools/libxl: introduce libxl__domain_restore_device_model to load qemu state

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

On 07/15/2015 08:45 PM, Ian Campbell wrote:
> On Wed, 2015-07-15 at 15:45 +0800, Yang Hongyang wrote:
>> In normal migration, the qemu state was passed to qemu as a parameter.
>> With COLO, Secondary vm is running. So we will do the following steps
>> at every checkpoint:
>> 1. suspend both primay vm and secondary vm
>
> "primary"
>
>> 2. sync the state
>> 3. resume both primary vm and secondary vm
>> Primary will send qemu's state in step2, and
>> Secondary's qemu should read it and restore the state before it
>> is resumed. We can not pass the state to qemu as a parameter because
>> Secondary QEMU already started at this point, so we introduce
>> libxl__domain_restore_device_model() to do it.
>> This API should be called before resuming secondary vm.
>>
>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> Cc: Anthony Perard <anthony.perard@citrix.com>
>> ---
>>   tools/libxl/libxl_dom_save.c | 29 +++++++++++++++++++++++++++++
>>   tools/libxl/libxl_internal.h |  3 +++
>>   tools/libxl/libxl_qmp.c      | 10 ++++++++++
>>   3 files changed, 42 insertions(+)
>>
>> diff --git a/tools/libxl/libxl_dom_save.c b/tools/libxl/libxl_dom_save.c
>> index f89f5d4..0926b71 100644
>> --- a/tools/libxl/libxl_dom_save.c
>> +++ b/tools/libxl/libxl_dom_save.c
>> @@ -675,6 +675,35 @@ out:
>>       return ret;
>>   }
>>
>> +int libxl__domain_restore_device_model(libxl__gc *gc, uint32_t domid)
>> +{
>> +    char *state_file;
>> +    int rc;
>> +
>> +    switch (libxl__device_model_version_running(gc, domid)) {
>> +    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
>> +        /* not supported now */
>> +        rc = ERROR_INVAL;
>> +        break;
>> +    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
>> +        /*
>> +         * This function may be called too many times for the same gc,
>> +         * so we use NOGC, and free the memory before return to avoid
>> +         * OOM.
>> +         */
>
> It occurs to me that domid shouldn't change for the duration of a COLO
> run, right?

right!

>
> Thus I think the path can be allocated once at start of day and not per
> iteration, and can be stored in suspend_state (or similar) and passed in
> here. Hence no complexity like a nested ao is needed.

Good idea, thank you!

>
>> +        state_file = libxl__sprintf(NOGC,
>> +                                    XC_DEVICE_MODEL_RESTORE_FILE".%d",
>> +                                    domid);
>> +        rc = libxl__qmp_restore(gc, domid, state_file);
>> +        free(state_file);
>> +        break;
>> +    default:
>> +        rc = ERROR_INVAL;
>> +    }
>> +
>> +    return rc;
>> +}
>> +
>>   /*
>>    * Local variables:
>>    * mode: C
>> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
>> index 0eb5f41..fb777c1 100644
>> --- a/tools/libxl/libxl_internal.h
>> +++ b/tools/libxl/libxl_internal.h
>> @@ -1074,6 +1074,7 @@ _hidden int libxl__domain_rename(libxl__gc *gc, uint32_t domid,
>>
>>   _hidden int libxl__toolstack_restore(uint32_t domid, const uint8_t *buf,
>>                                        uint32_t size, void *data);
>> +_hidden int libxl__domain_restore_device_model(libxl__gc *gc, uint32_t domid);
>>   _hidden int libxl__domain_resume_device_model(libxl__gc *gc, uint32_t domid);
>>
>>   _hidden const char *libxl__userdata_path(libxl__gc *gc, uint32_t domid,
>> @@ -1702,6 +1703,8 @@ _hidden int libxl__qmp_stop(libxl__gc *gc, int domid);
>>   _hidden int libxl__qmp_resume(libxl__gc *gc, int domid);
>>   /* Save current QEMU state into fd. */
>>   _hidden int libxl__qmp_save(libxl__gc *gc, int domid, const char *filename);
>> +/* Load current QEMU state from fd. */
>> +_hidden int libxl__qmp_restore(libxl__gc *gc, int domid, const char *filename);
>>   /* Set dirty bitmap logging status */
>>   _hidden int libxl__qmp_set_global_dirty_log(libxl__gc *gc, int domid, bool enable);
>>   _hidden int libxl__qmp_insert_cdrom(libxl__gc *gc, int domid, const libxl_device_disk *disk);
>> diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
>> index 6484f5e..080cb9f 100644
>> --- a/tools/libxl/libxl_qmp.c
>> +++ b/tools/libxl/libxl_qmp.c
>> @@ -904,6 +904,16 @@ int libxl__qmp_save(libxl__gc *gc, int domid, const char *filename)
>>                              NULL, NULL);
>>   }
>>
>> +int libxl__qmp_restore(libxl__gc *gc, int domid, const char *state_file)
>> +{
>> +    libxl__json_object *args = NULL;
>> +
>> +    qmp_parameters_add_string(gc, &args, "filename", state_file);
>> +
>> +    return qmp_run_command(gc, domid, "xen-load-devices-state", args,
>> +                           NULL, NULL);
>> +}
>> +
>>   static int qmp_change(libxl__gc *gc, libxl__qmp_handler *qmp,
>>                         char *device, char *target, char *arg)
>>   {
>
>
> .
>

-- 
Thanks,
Yang.