[PATCH v5 15/15] libxl: Re-implement domain_suspend_device_model using libxl__ev_qmp

[Anthony PERARD <anthony.perard@citrix.com>]

The re-implementation is done because we want to be able to send the
file description that QEMU can use to save its state. When QEMU is
restricted, it would not be able to write to a path.

This replace both libxl__qmp_stop() and libxl__qmp_save().

qmp_qemu_check_version() was only used by libxl__qmp_save(), so it is
replace by a version using libxl__ev_qmp instead.

Coding style fixed in libxl__domain_suspend_device_model() for the
return value.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---

Notes:
    v5:
        rename goto 'out' label to 'error', as it is use only for errors.
        re-add/keep the comment about the "live" parameter in dm_state_fd_ready
        use libxl__remove_file instead of plain unlink
    
    v4:
        This patch replace the patch "libxl_qmp: Have QEMU save its state to a file
        descriptor" from previous version of the serie.
        It uses libxl__ev_qmp instead.

 tools/libxl/libxl_dom_suspend.c |  16 ++--
 tools/libxl/libxl_internal.h    |   9 +-
 tools/libxl/libxl_qmp.c         | 153 +++++++++++++++++++++++++-------
 3 files changed, 130 insertions(+), 48 deletions(-)

diff --git a/tools/libxl/libxl_dom_suspend.c b/tools/libxl/libxl_dom_suspend.c
index 51c432a00a..0f4e1be115 100644
--- a/tools/libxl/libxl_dom_suspend.c
+++ b/tools/libxl/libxl_dom_suspend.c
@@ -34,6 +34,7 @@ int libxl__domain_suspend_init(libxl__egc *egc,
     libxl__ev_evtchn_init(&dsps->guest_evtchn);
     libxl__ev_xswatch_init(&dsps->guest_watch);
     libxl__ev_time_init(&dsps->guest_timeout);
+    libxl__ev_qmp_init(&dsps->qmp);
 
     if (type == LIBXL_DOMAIN_TYPE_INVALID) goto out;
     dsps->type = type;
@@ -72,7 +73,7 @@ int libxl__domain_suspend_device_model(libxl__egc *egc,
                                        libxl__domain_suspend_state *dsps)
 {
     STATE_AO_GC(dsps->ao);
-    int ret = 0;
+    int rc;
     uint32_t const domid = dsps->domid;
     const char *const filename = dsps->dm_savefile;
 
@@ -81,22 +82,18 @@ int libxl__domain_suspend_device_model(libxl__egc *egc,
         LOGD(DEBUG, domid, "Saving device model state to %s", filename);
         libxl__qemu_traditional_cmd(gc, domid, "save");
         libxl__wait_for_device_model_deprecated(gc, domid, "paused", NULL, NULL, NULL);
+        dsps->callback_device_model_done(egc, dsps, 0);
+        rc = 0;
         break;
     }
     case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
-        if (libxl__qmp_stop(gc, domid))
-            return ERROR_FAIL;
-        /* Save DM state into filename */
-        ret = libxl__qmp_save(gc, domid, filename, dsps->live);
-        if (ret)
-            unlink(filename);
+        rc = libxl__qmp_suspend_save(gc, dsps);
         break;
     default:
         return ERROR_INVAL;
     }
 
-    dsps->callback_device_model_done(egc, dsps, ret);
-    return ret;
+    return rc;
 }
 
 static void domain_suspend_common_wait_guest(libxl__egc *egc,
@@ -402,6 +399,7 @@ static void domain_suspend_common_done(libxl__egc *egc,
     libxl__ev_evtchn_cancel(gc, &dsps->guest_evtchn);
     libxl__ev_xswatch_deregister(gc, &dsps->guest_watch);
     libxl__ev_time_deregister(gc, &dsps->guest_timeout);
+    libxl__ev_qmp_dispose(gc, &dsps->qmp);
     dsps->callback_common_done(egc, dsps, rc);
 }
 
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 7f0ad75160..f2162f7e92 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1941,13 +1941,8 @@ _hidden int libxl__qmp_pci_del(libxl__gc *gc, int domid,
                                libxl_device_pci *pcidev);
 /* Resume hvm domain */
 _hidden int libxl__qmp_system_wakeup(libxl__gc *gc, int domid);
-/* Suspend QEMU. */
-_hidden int libxl__qmp_stop(libxl__gc *gc, int domid);
 /* Resume QEMU. */
 _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,
-                            bool live);
 /* Load current QEMU state from file. */
 _hidden int libxl__qmp_restore(libxl__gc *gc, int domid, const char *filename);
 /* Set dirty bitmap logging status */
@@ -3405,6 +3400,7 @@ struct libxl__domain_suspend_state {
     libxl__xswait_state pvcontrol;
     libxl__ev_xswatch guest_watch;
     libxl__ev_time guest_timeout;
+    libxl__ev_qmp qmp;
 
     const char *dm_savefile;
     void (*callback_device_model_done)(libxl__egc*,
@@ -3416,6 +3412,9 @@ int libxl__domain_suspend_init(libxl__egc *egc,
                                libxl__domain_suspend_state *dsps,
                                libxl_domain_type type);
 
+_hidden int libxl__qmp_suspend_save(libxl__gc *gc,
+                                    libxl__domain_suspend_state *dsps);
+
 struct libxl__domain_save_state {
     /* set by caller of libxl__domain_save */
     libxl__ao *ao;
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 77380a869c..e2c3fca085 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -398,13 +398,14 @@ static int qmp_handle_response(libxl__gc *gc, libxl__qmp_handler *qmp,
     return 0;
 }
 
-static bool qmp_qemu_check_version(libxl__qmp_handler *qmp, int major,
-                                   int minor, int micro)
+static bool qmp_ev_qemu_check_version(libxl__ev_qmp *ev, int major,
+                                      int minor, int micro)
 {
-    return qmp->version.major > major ||
-        (qmp->version.major == major &&
-            (qmp->version.minor > minor ||
-             (qmp->version.minor == minor && qmp->version.micro >= micro)));
+    return ev->qemu_version.major > major ||
+        (ev->qemu_version.major == major &&
+            (ev->qemu_version.minor > minor ||
+             (ev->qemu_version.minor == minor &&
+              ev->qemu_version.micro >= micro)));
 }
 
 /*
@@ -1013,29 +1014,6 @@ int libxl__qmp_system_wakeup(libxl__gc *gc, int domid)
     return qmp_run_command(gc, domid, "system_wakeup", NULL, NULL, NULL);
 }
 
-int libxl__qmp_save(libxl__gc *gc, int domid, const char *filename, bool live)
-{
-    libxl__json_object *args = NULL;
-    libxl__qmp_handler *qmp = NULL;
-    int rc;
-
-    qmp = libxl__qmp_initialize(gc, domid);
-    if (!qmp)
-        return ERROR_FAIL;
-
-    qmp_parameters_add_string(gc, &args, "filename", (char *)filename);
-
-    /* live parameter was added to QEMU 2.11. It signal QEMU that the save
-     * operation is for a live migration rather that for taking a snapshot. */
-    if (qmp_qemu_check_version(qmp, 2, 11, 0))
-        qmp_parameters_add_bool(gc, &args, "live", live);
-
-    rc = qmp_synchronous_send(qmp, "xen-save-devices-state", args,
-                              NULL, NULL, qmp->timeout);
-    libxl__qmp_close(qmp);
-    return rc;
-}
-
 int libxl__qmp_restore(libxl__gc *gc, int domid, const char *state_file)
 {
     libxl__json_object *args = NULL;
@@ -1064,11 +1042,6 @@ static int qmp_change(libxl__gc *gc, libxl__qmp_handler *qmp,
     return rc;
 }
 
-int libxl__qmp_stop(libxl__gc *gc, int domid)
-{
-    return qmp_run_command(gc, domid, "stop", NULL, NULL, NULL);
-}
-
 int libxl__qmp_resume(libxl__gc *gc, int domid)
 {
     return qmp_run_command(gc, domid, "cont", NULL, NULL, NULL);
@@ -1308,6 +1281,118 @@ int libxl__qmp_initializations(libxl__gc *gc, uint32_t domid,
     return ret;
 }
 
+
+/*
+ * Function using libxl__ev_qmp
+ */
+
+static void dm_stopped(libxl__egc *egc, libxl__ev_qmp *ev,
+                       const libxl__json_object *response, int rc);
+static void dm_state_fd_ready(libxl__egc *egc, libxl__ev_qmp *ev,
+                              const libxl__json_object *response, int rc);
+static void dm_state_saved(libxl__egc *egc, libxl__ev_qmp *ev,
+                           const libxl__json_object *response, int rc);
+int libxl__qmp_suspend_save(libxl__gc *gc, libxl__domain_suspend_state *dsps)
+{
+    libxl__ev_qmp *ev = &dsps->qmp;
+    uint32_t const domid = dsps->domid;
+
+    ev->domid = domid;
+    ev->callback = dm_stopped;
+    ev->cfd = NULL;
+
+    return libxl__ev_qmp_send(gc, ev, "stop", NULL);
+}
+
+static void dm_stopped(libxl__egc *egc, libxl__ev_qmp *ev,
+                       const libxl__json_object *response, int rc)
+{
+    EGC_GC;
+    libxl__domain_suspend_state *dsps = CONTAINER_OF(ev, *dsps, qmp);
+    const char *const filename = dsps->dm_savefile;
+    uint32_t const domid = ev->domid;
+
+    if (rc)
+        goto error;
+
+    libxl__carefd_begin();
+    ev->cfd = libxl__carefd_opened(CTX,
+                                 open(filename, O_WRONLY | O_CREAT, 0600));
+    if (!ev->cfd) {
+        LOGED(ERROR, domid, "Failed to open file %s for QEMU", filename);
+        rc = ERROR_FAIL;
+        goto error;
+    }
+
+    ev->callback = dm_state_fd_ready;
+    rc = libxl__ev_qmp_send(gc, ev, "add-fd", NULL);
+    if (rc)
+        goto error;
+
+    return;
+error:
+    if (ev->cfd) {
+        libxl__carefd_close(ev->cfd);
+        unlink(filename);
+        ev->cfd = NULL;
+    }
+    dsps->callback_device_model_done(egc, dsps, rc);
+}
+
+static void dm_state_fd_ready(libxl__egc *egc, libxl__ev_qmp *ev,
+                              const libxl__json_object *response, int rc)
+{
+    EGC_GC;
+    libxl__domain_suspend_state *dsps = CONTAINER_OF(ev, *dsps, qmp);
+    libxl__json_object *args = NULL;
+    const libxl__json_object *o;
+    int fdset;
+
+    libxl__carefd_close(ev->cfd);
+    ev->cfd = NULL;
+
+    if (rc)
+        goto error;
+
+    o = libxl__json_map_get("fdset-id", response, JSON_INTEGER);
+    if (!o) {
+        rc = ERROR_FAIL;
+        goto error;
+    }
+    fdset = libxl__json_object_get_integer(o);
+
+    ev->callback = dm_state_saved;
+
+    /* live parameter was added to QEMU 2.11. It signal QEMU that the save
+     * operation is for a live migration rather that for taking a snapshot. */
+    if (qmp_ev_qemu_check_version(ev, 2, 11, 0))
+        qmp_parameters_add_bool(gc, &args, "live", dsps->live);
+    QMP_PARAMETERS_SPRINTF(&args, "filename", "/dev/fdset/%d", fdset);
+    rc = libxl__ev_qmp_send(gc, ev, "xen-save-devices-state", args);
+    if (rc)
+        goto error;
+
+    return;
+error:
+    if (rc)
+        libxl__remove_file(gc, dsps->dm_savefile);
+    dsps->callback_device_model_done(egc, dsps, rc);
+}
+
+static void dm_state_saved(libxl__egc *egc, libxl__ev_qmp *ev,
+                           const libxl__json_object *response, int rc)
+{
+    EGC_GC;
+    libxl__domain_suspend_state *dsps = CONTAINER_OF(ev, *dsps, qmp);
+
+    if (rc)
+        unlink(dsps->dm_savefile);
+
+    libxl__ev_qmp_dispose(gc, ev);
+    dsps->callback_device_model_done(egc, dsps, rc);
+}
+
+
 /* ------------ Implementation of libxl__ev_qmp ---------------- */
 
 /* hard coded message ID used for capability negotiation */
-- 
Anthony PERARD


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel