[Xen-devel] [PATCH v8 13/17] libxl: Change libxl__domain_suspend_device_model() to be async

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

This create an extra step for the two call sites of the function.

libxl__domain_suspend_device_model() in this patch gets an extra error
variable (there is ret and rc), but ret goes away in the next patch.

Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
Acked-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>

---
libxl_domain_soft_reset() haven't been tested, as it doesn't appear to
possible to call the function from xl.
---

Notes:
    v8:
        Acked
    
    v6:
        fix multiple way to report errors,
        libxl__domain_suspend_device_model will now only report via
        callbacks, and return void
        add rc in libxl__domain_suspend_device_model (ret isn't a proper rc
        as libxl__qmp_save don't return one)

 tools/libxl/libxl_create.c      | 35 +++++++++++++++++++++-----------
 tools/libxl/libxl_dom_suspend.c | 36 ++++++++++++++++++++-------------
 tools/libxl/libxl_internal.h    |  7 +++++--
 3 files changed, 50 insertions(+), 28 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 89f4766a27..a4e74a5cd2 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1774,6 +1774,9 @@ static void domain_soft_reset_cb(libxl__egc *egc,
     domcreate_complete(egc, &cdcs->dcs, rc);
 }
 
+static void soft_reset_dm_suspended(libxl__egc *egc,
+                                    libxl__domain_suspend_state *dsps,
+                                    int rc);
 static int do_domain_soft_reset(libxl_ctx *ctx,
                                 libxl_domain_config *d_config,
                                 uint32_t domid_soft_reset,
@@ -1856,11 +1859,24 @@ static int do_domain_soft_reset(libxl_ctx *ctx,
         goto out;
     }
 
-    rc = libxl__domain_suspend_device_model(gc, &dss->dsps);
-    if (rc) {
-        LOGD(ERROR, domid_soft_reset, "failed to suspend device model.");
-        goto out;
-    }
+    dss->dsps.ao = ao;
+    dss->dsps.callback_device_model_done = soft_reset_dm_suspended;
+    libxl__domain_suspend_device_model(egc, &dss->dsps); /* must be last */
+
+    return AO_INPROGRESS;
+
+ out:
+    return AO_CREATE_FAIL(rc);
+}
+
+static void soft_reset_dm_suspended(libxl__egc *egc,
+                                    libxl__domain_suspend_state *dsps,
+                                    int rc)
+{
+    STATE_AO_GC(dsps->ao);
+    libxl__domain_soft_reset_state *srs =
+        CONTAINER_OF(dsps, *srs, dss.dsps);
+    libxl__app_domain_create_state *cdcs = &srs->cdcs;
 
     /*
      * Ask all backends to disconnect by removing the domain from
@@ -1868,18 +1884,13 @@ static int do_domain_soft_reset(libxl_ctx *ctx,
      * xenstore again with probably different store/console/...
      * channels.
      */
-    xs_release_domain(ctx->xsh, cdcs->dcs.domid_soft_reset);
+    xs_release_domain(CTX->xsh, cdcs->dcs.domid_soft_reset);
 
     srs->dds.ao = ao;
-    srs->dds.domid = domid_soft_reset;
+    srs->dds.domid = cdcs->dcs.domid_soft_reset;
     srs->dds.callback = domain_soft_reset_cb;
     srs->dds.soft_reset = true;
     libxl__domain_destroy(egc, &srs->dds);
-
-    return AO_INPROGRESS;
-
- out:
-    return AO_CREATE_FAIL(rc);
 }
 
 static void domain_create_cb(libxl__egc *egc,
diff --git a/tools/libxl/libxl_dom_suspend.c b/tools/libxl/libxl_dom_suspend.c
index 1e904bae8a..f8ff5cf0c5 100644
--- a/tools/libxl/libxl_dom_suspend.c
+++ b/tools/libxl/libxl_dom_suspend.c
@@ -68,10 +68,12 @@ int libxl__domain_suspend_init(libxl__egc *egc,
 
 /*----- callbacks, called by xc_domain_save -----*/
 
-int libxl__domain_suspend_device_model(libxl__gc *gc,
+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;
 
@@ -83,18 +85,29 @@ int libxl__domain_suspend_device_model(libxl__gc *gc,
         break;
     }
     case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
-        if (libxl__qmp_stop(gc, domid))
-            return ERROR_FAIL;
+        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)
+        if (ret) {
+            rc = ERROR_FAIL;
             unlink(filename);
+            goto out;
+        }
         break;
     default:
-        return ERROR_INVAL;
+        rc = ERROR_INVAL;
+        goto out;
     }
 
-    return ret;
+out:
+    if (rc)
+        LOGD(ERROR, dsps->domid,
+             "failed to suspend device model, rc=%d", rc);
+    dsps->callback_device_model_done(egc, dsps, rc); /* must be last */
 }
 
 static void domain_suspend_common_wait_guest(libxl__egc *egc,
@@ -371,20 +384,15 @@ static void 
domain_suspend_common_guest_suspended(libxl__egc *egc,
                                          libxl__domain_suspend_state *dsps)
 {
     STATE_AO_GC(dsps->ao);
-    int rc;
 
     libxl__ev_evtchn_cancel(gc, &dsps->guest_evtchn);
     libxl__ev_xswatch_deregister(gc, &dsps->guest_watch);
     libxl__ev_time_deregister(gc, &dsps->guest_timeout);
 
     if (dsps->type == LIBXL_DOMAIN_TYPE_HVM) {
-        rc = libxl__domain_suspend_device_model(gc, dsps);
-        if (rc) {
-            LOGD(ERROR, dsps->domid,
-                 "libxl__domain_suspend_device_model failed ret=%d", rc);
-            domain_suspend_common_done(egc, dsps, rc);
-            return;
-        }
+        dsps->callback_device_model_done = domain_suspend_common_done;
+        libxl__domain_suspend_device_model(egc, dsps); /* must be last */
+        return;
     }
     domain_suspend_common_done(egc, dsps, 0);
 }
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 62492e13d7..5a18937562 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3435,6 +3435,8 @@ struct libxl__domain_suspend_state {
     libxl__ev_time guest_timeout;
 
     const char *dm_savefile;
+    void (*callback_device_model_done)(libxl__egc*,
+                              struct libxl__domain_suspend_state*, int rc);
     void (*callback_common_done)(libxl__egc*,
                                  struct libxl__domain_suspend_state*, int ok);
 };
@@ -4076,8 +4078,9 @@ static inline bool libxl__save_helper_inuse(const 
libxl__save_helper_state *shs)
     return libxl__ev_child_inuse(&shs->child);
 }
 
-/* Each time the dm needs to be saved, we must call suspend and then save */
-_hidden int libxl__domain_suspend_device_model(libxl__gc *gc,
+/* Each time the dm needs to be saved, we must call suspend and then save
+ * calls dsps->callback_device_model_done when done */
+_hidden void libxl__domain_suspend_device_model(libxl__egc *egc,
                                            libxl__domain_suspend_state *dsps);
 
 _hidden const char *libxl__device_model_savefile(libxl__gc *gc, uint32_t 
domid);
-- 
Anthony PERARD


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