[PATCH v7 14/14] 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:
    v7:
        use libxl__remove_file instead of unlink.
        comment that libxl__qmp_suspend_save can callback synchronously
    
    v6:
        extract open call from libxl__carefd_opened to respect coding style
        libxl__qmp_suspend_save now always report success/error
            via dsps->callback_device_model_done
    
    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 |  19 +---
 tools/libxl/libxl_internal.h    |  11 ++-
 tools/libxl/libxl_qmp.c         | 160 +++++++++++++++++++++++++-------
 3 files changed, 139 insertions(+), 51 deletions(-)

diff --git a/tools/libxl/libxl_dom_suspend.c b/tools/libxl/libxl_dom_suspend.c
index f8ff5cf0c5..d1af3a6573 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,6 @@ void libxl__domain_suspend_device_model(libxl__egc *egc,
                                        libxl__domain_suspend_state *dsps)
 {
     STATE_AO_GC(dsps->ao);
-    int ret = 0;
     int rc = 0;
     uint32_t const domid = dsps->domid;
     const char *const filename = dsps->dm_savefile;
@@ -85,19 +85,9 @@ void libxl__domain_suspend_device_model(libxl__egc *egc,
         break;
     }
     case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
-        ret = libxl__qmp_stop(gc, domid);
-        if (ret) {
-            rc = ERROR_FAIL;
-            goto out;
-        }
-        /* Save DM state into filename */
-        ret = libxl__qmp_save(gc, domid, filename, dsps->live);
-        if (ret) {
-            rc = ERROR_FAIL;
-            unlink(filename);
-            goto out;
-        }
-        break;
+        /* calls dsps->callback_device_model_done when done */
+        libxl__qmp_suspend_save(egc, dsps); /* must be last */
+        return;
     default:
         rc = ERROR_INVAL;
         goto out;
@@ -406,6 +396,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 9dcc474b5b..f766359fee 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1951,13 +1951,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 */
@@ -3421,6 +3416,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*,
@@ -3432,6 +3428,11 @@ int libxl__domain_suspend_init(libxl__egc *egc,
                                libxl__domain_suspend_state *dsps,
                                libxl_domain_type type);
 
+/* calls dsps->callback_device_model_done when done
+ * may synchronously calls this callback */
+_hidden void libxl__qmp_suspend_save(libxl__egc *egc,
+                                     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 6954a909e5..32331d67e5 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -405,12 +405,12 @@ static int qmp_handle_response(libxl__gc *gc, libxl__qmp_handler *qmp,
  *   = 0  if qemu's version == asked version
  *   > 0  if qemu's version >  asked version
  */
-static int qmp_qemu_compare_version(libxl__qmp_handler *qmp, int major,
-                                    int minor, int micro)
+static int qmp_ev_qemu_compare_version(libxl__ev_qmp *ev, int major,
+                                       int minor, int micro)
 {
 #define CHECK_VERSION(level) do { \
-    if (qmp->version.level > (level)) return +1; \
-    if (qmp->version.level < (level)) return -1; \
+    if (ev->qemu_version.level > (level)) return +1; \
+    if (ev->qemu_version.level < (level)) return -1; \
 } while (0)
 
     CHECK_VERSION(major);
@@ -1019,29 +1019,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_compare_version(qmp, 2, 11, 0) >= 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;
@@ -1070,11 +1047,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);
@@ -1314,6 +1286,130 @@ int libxl__qmp_initializations(libxl__gc *gc, uint32_t domid,
     return ret;
 }
 
+
+/*
+ * Functions 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);
+
+/* calls dsps->callback_device_model_done when done */
+void libxl__qmp_suspend_save(libxl__egc *egc,
+                             libxl__domain_suspend_state *dsps)
+{
+    EGC_GC;
+    int rc;
+    libxl__ev_qmp *ev = &dsps->qmp;
+
+    ev->ao = dsps->ao;
+    ev->domid = dsps->domid;
+    ev->callback = dm_stopped;
+    ev->payload_fd = -1;
+
+    rc = libxl__ev_qmp_send(gc, ev, "stop", NULL);
+    if (rc)
+        goto error;
+
+    return;
+
+error:
+    dsps->callback_device_model_done(egc, dsps, rc);
+}
+
+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;
+
+    if (rc)
+        goto error;
+
+    ev->payload_fd = open(filename, O_WRONLY | O_CREAT, 0600);
+    if (ev->payload_fd < 0) {
+        LOGED(ERROR, ev->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->payload_fd >= 0) {
+        close(ev->payload_fd);
+        libxl__remove_file(gc, filename);
+        ev->payload_fd = -1;
+    }
+    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;
+    int fdset;
+    const libxl__json_object *o;
+    libxl__json_object *args = NULL;
+    libxl__domain_suspend_state *dsps = CONTAINER_OF(ev, *dsps, qmp);
+
+    close(ev->payload_fd);
+    ev->payload_fd = -1;
+
+    if (rc)
+        goto error;
+
+    o = libxl__json_map_get("fdset-id", response, JSON_INTEGER);
+    if (!o) {
+        rc = ERROR_QEMU_API;
+        goto error;
+    }
+    fdset = libxl__json_object_get_integer(o);
+
+    ev->callback = dm_state_saved;
+
+    /* The `live` parameter was added to QEMU 2.11. It signals QEMU that
+     * the save operation is for a live migration rather than for taking a
+     * snapshot. */
+    if (qmp_ev_qemu_compare_version(ev, 2, 11, 0) >= 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:
+    assert(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)
+        libxl__remove_file(gc, dsps->dm_savefile);
+
+    dsps->callback_device_model_done(egc, dsps, rc);
+}
+
+
 /* ------------ Implementation of libxl__ev_qmp ---------------- */
 
 /*
-- 
Anthony PERARD


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