Re: [PATCH v2 COLOPre 05/13] tools/libxl: introduce a new API libxl__domain_restore() to load qemu state

[Ian Campbell <ian.campbell@citrix.com>]

On Thu, 2015-06-11 at 10:09 +0800, Yang Hongyang wrote:
> 
> On 06/10/2015 11:35 PM, Ian Campbell wrote:
> > On Mon, 2015-06-08 at 11:43 +0800, Yang Hongyang wrote:
> >> Secondary vm is running in colo mode. So we will do
> >> the following things again and again:
> >> 1. suspend both primay vm and secondary vm
> >> 2. sync the state
> >> 3. resume both primary vm and secondary vm
> >> We will send qemu's state each time in step2, and
> >> slave's qemu should read it each time before resuming
> >> secondary vm. Introduce a new API libxl__domain_restore()
> >> to do it. This API should be called before resuming
> >> secondary vm.
> >
> > Is this a preexisting qemu interface or one to be added?
> 
> We added the qemu interface "xen-load-devices-state",
> it's not in qemu upstream yet.

OK, please mention this dependency in the commit text since we will want
to be sure the interface is going to be accepted in this form by QEMU
upstream before we start using it. Please also CC the QEMU maintainers
on this patch in the future (by adding Cc: below the S-o-b if you don't
want to spam them the whole series), I've added them here now.

In particular "devices" seems odd to me, perhaps
"xen-load-device-state"?

> 
> >
> >>
> >> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> >> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> >> ---
> >>   tools/libxl/libxl_dom_save.c | 47 ++++++++++++++++++++++++++++++++++++++++++++
> >>   tools/libxl/libxl_internal.h |  4 ++++
> >>   tools/libxl/libxl_qmp.c      | 10 ++++++++++
> >>   3 files changed, 61 insertions(+)
> >>
> >> diff --git a/tools/libxl/libxl_dom_save.c b/tools/libxl/libxl_dom_save.c
> >> index 74a6bae..f9627f8 100644
> >> --- a/tools/libxl/libxl_dom_save.c
> >> +++ b/tools/libxl/libxl_dom_save.c
> >> @@ -663,6 +663,53 @@ int libxl__toolstack_restore(uint32_t domid, const uint8_t *buf,
> >>       return 0;
> >>   }
> >>
> >> +int libxl__domain_restore(libxl__gc *gc, uint32_t domid)
> >> +{
> >> +    int rc = 0;
> >> +
> >> +    libxl_domain_type type = libxl__domain_type(gc, domid);
> >> +    if (type != LIBXL_DOMAIN_TYPE_HVM) {
> >> +        rc = ERROR_FAIL;
> >> +        goto out;
> >> +    }
> >> +
> >> +    rc = libxl__domain_restore_device_model(gc, domid);
> >> +    if (rc)
> >> +        LOG(ERROR, "failed to restore device mode for domain %u:%d",
> >> +            domid, rc);
> >> +out:
> >> +    return rc;
> >> +}
> >> +
> >> +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.
> >> +         */
> >> +        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 1905195..20364c6 100644
> >> --- a/tools/libxl/libxl_internal.h
> >> +++ b/tools/libxl/libxl_internal.h
> >> @@ -1022,6 +1022,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,
> >> @@ -1039,6 +1040,7 @@ _hidden int libxl__userdata_store(libxl__gc *gc, uint32_t domid,
> >>                                     const char *userdata_userid,
> >>                                     const uint8_t *data, int datalen);
> >>
> >> +_hidden int libxl__domain_restore(libxl__gc *gc, uint32_t domid);
> >>   _hidden int libxl__domain_resume(libxl__gc *gc, uint32_t domid,
> >>                                    int suspend_cancel);
> >>   _hidden int libxl__domain_s3_resume(libxl__gc *gc, int domid);
> >> @@ -1651,6 +1653,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 9aa7e2e..a6f1a21 100644
> >> --- a/tools/libxl/libxl_qmp.c
> >> +++ b/tools/libxl/libxl_qmp.c
> >> @@ -892,6 +892,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)
> >>   {
> >
> >
> > .
> >
>