libxl: suspend: Async evtchn wait

When negotiating guest suspend via the evtchn ("fast") protocol,
abolish synchronous wait for domain suspend.

If the guest supports the event channel suspend protocol, we used to
sit in a loop in xc_await_suspend waiting (perhaps indefinitely) for
it to suspend.

Instead, use the new libxl event channel event facility.  When we see
that the event is signaled, we look at the domain to see if it has
suspended.  (In this patch we do not yet set a timeout; that will come
next.)

So the suspend operation no longer blocks with the libxl ctx lock
held, and instead returns to the event loop.  Additionally, domains
which signal the event channel themselves, or undergo other state
changes, will be handled more correctly.

We end up making a few more hypercalls.

Also, if we encounter errors setting up the suspend event channel
(which should not happen), abort the operation rather than falling
back to the xenstore protocol.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v3: Improve commit message.

diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 28855a11011ca5d32f2a540240799e349483dd98..87c994cb6bc3c9d61acc40b2fb0dddc06c9914a0 100644 (file)
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1031,8 +1031,12 @@ static void domain_suspend_common_guest_suspended(libxl__egc *egc,
 
 static void domain_suspend_common_pvcontrol_suspending(libxl__egc *egc,
       libxl__xswait_state *xswa, int rc, const char *state);
+static void domain_suspend_common_wait_guest_evtchn(libxl__egc *egc,
+        libxl__ev_evtchn *evev);
 static void suspend_common_wait_guest_watch(libxl__egc *egc,
       libxl__ev_xswatch *xsw, const char *watch_path, const char *event_path);
+static void suspend_common_wait_guest_check(libxl__egc *egc,
+        libxl__domain_suspend_state *dss);
 static void suspend_common_wait_guest_timeout(libxl__egc *egc,
       libxl__ev_time *ev, const struct timeval *requested_abs);
 
@@ -1054,7 +1058,7 @@ static void domain_suspend_callback_common(libxl__egc *egc,
 {
     STATE_AO_GC(dss->ao);
     unsigned long hvm_s_state = 0, hvm_pvdrv = 0;
-    int ret;
+    int ret, rc;
 
     /* Convenience aliases */
     const uint32_t domid = dss->domid;
@@ -1064,21 +1068,19 @@ static void domain_suspend_callback_common(libxl__egc *egc,
         xc_get_hvm_param(CTX->xch, domid, HVM_PARAM_ACPI_S_STATE, &hvm_s_state);
     }
 
-    if ((hvm_s_state == 0) && (dss->suspend_eventchn >= 0)) {
+    if ((hvm_s_state == 0) && (dss->guest_evtchn.port >= 0)) {
         LOG(DEBUG, "issuing %s suspend request via event channel",
             dss->hvm ? "PVHVM" : "PV");
-        ret = xc_evtchn_notify(dss->xce, dss->suspend_eventchn);
+        ret = xc_evtchn_notify(CTX->xce, dss->guest_evtchn.port);
         if (ret < 0) {
             LOG(ERROR, "xc_evtchn_notify failed ret=%d", ret);
             goto err;
         }
-        ret = xc_await_suspend(CTX->xch, dss->xce, dss->suspend_eventchn);
-        if (ret < 0) {
-            LOG(ERROR, "xc_await_suspend failed ret=%d", ret);
-            goto err;
-        }
-        dss->guest_responded = 1;
-        domain_suspend_common_guest_suspended(egc, dss);
+
+        dss->guest_evtchn.callback = domain_suspend_common_wait_guest_evtchn;
+        rc = libxl__ev_evtchn_wait(gc, &dss->guest_evtchn);
+        if (rc) goto err;
+
         return;
     }
 
@@ -1114,6 +1116,19 @@ static void domain_suspend_callback_common(libxl__egc *egc,
     domain_suspend_common_failed(egc, dss);
 }
 
+static void domain_suspend_common_wait_guest_evtchn(libxl__egc *egc,
+        libxl__ev_evtchn *evev)
+{
+    libxl__domain_suspend_state *dss = CONTAINER_OF(evev, *dss, guest_evtchn);
+    STATE_AO_GC(dss->ao);
+    /* If we should be done waiting, suspend_common_wait_guest_check
+     * will end up calling domain_suspend_common_guest_suspended or
+     * domain_suspend_common_failed, both of which cancel the evtchn
+     * wait.  So re-enable it now. */
+    libxl__ev_evtchn_wait(gc, &dss->guest_evtchn);
+    suspend_common_wait_guest_check(egc, dss);
+}
+
 static void domain_suspend_common_pvcontrol_suspending(libxl__egc *egc,
       libxl__xswait_state *xswa, int rc, const char *state)
 {
@@ -1204,8 +1219,13 @@ static void domain_suspend_common_wait_guest(libxl__egc *egc,
 static void suspend_common_wait_guest_watch(libxl__egc *egc,
       libxl__ev_xswatch *xsw, const char *watch_path, const char *event_path)
 {
-    libxl__domain_suspend_state *dss =
-        CONTAINER_OF(xsw, *dss, guest_watch);
+    libxl__domain_suspend_state *dss = CONTAINER_OF(xsw, *dss, guest_watch);
+    suspend_common_wait_guest_check(egc, dss);
+}
+
+static void suspend_common_wait_guest_check(libxl__egc *egc,
+        libxl__domain_suspend_state *dss)
+{
     STATE_AO_GC(dss->ao);
     xc_domaininfo_t info;
     int ret;
@@ -1261,6 +1281,7 @@ static void domain_suspend_common_guest_suspended(libxl__egc *egc,
     STATE_AO_GC(dss->ao);
     int ret;
 
+    libxl__ev_evtchn_cancel(gc, &dss->guest_evtchn);
     libxl__ev_xswatch_deregister(gc, &dss->guest_watch);
     libxl__ev_time_deregister(gc, &dss->guest_timeout);
 
@@ -1287,6 +1308,7 @@ static void domain_suspend_common_done(libxl__egc *egc,
 {
     EGC_GC;
     assert(!libxl__xswait_inuse(&dss->pvcontrol));
+    libxl__ev_evtchn_cancel(gc, &dss->guest_evtchn);
     libxl__ev_xswatch_deregister(gc, &dss->guest_watch);
     libxl__ev_time_deregister(gc, &dss->guest_timeout);
     dss->callback_common_done(egc, dss, ok);
@@ -1475,6 +1497,7 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss)
 
     logdirty_init(&dss->logdirty);
     libxl__xswait_init(&dss->pvcontrol);
+    libxl__ev_evtchn_init(&dss->guest_evtchn);
     libxl__ev_xswatch_init(&dss->guest_watch);
     libxl__ev_time_init(&dss->guest_timeout);
 
@@ -1503,7 +1526,7 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss)
           | (debug ? XCFLAGS_DEBUG : 0)
           | (dss->hvm ? XCFLAGS_HVM : 0);
 
-    dss->suspend_eventchn = -1;
+    dss->guest_evtchn.port = -1;
     dss->guest_evtchn_lockfd = -1;
     dss->guest_responded = 0;
     dss->dm_savefile = libxl__device_model_savefile(gc, domid);
@@ -1514,20 +1537,17 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss)
             dss->xcflags |= XCFLAGS_CHECKPOINT_COMPRESS;
     }
 
-    dss->xce = xc_evtchn_open(NULL, 0);
-    if (dss->xce == NULL)
-        goto out;
-    else
-    {
-        port = xs_suspend_evtchn_port(dss->domid);
+    port = xs_suspend_evtchn_port(dss->domid);
 
-        if (port >= 0) {
-            dss->suspend_eventchn =
-                xc_suspend_evtchn_init_exclusive(CTX->xch, dss->xce,
+    if (port >= 0) {
+        dss->guest_evtchn.port =
+            xc_suspend_evtchn_init_exclusive(CTX->xch, CTX->xce,
                                   dss->domid, port, &dss->guest_evtchn_lockfd);
 
-            if (dss->suspend_eventchn < 0)
-                LOG(WARN, "Suspend event channel initialization failed");
+        if (dss->guest_evtchn.port < 0) {
+            LOG(WARN, "Suspend event channel initialization failed");
+            rc = ERROR_FAIL;
+            goto out;
         }
     }
 
@@ -1686,11 +1706,11 @@ static void domain_suspend_done(libxl__egc *egc,
     /* Convenience aliases */
     const uint32_t domid = dss->domid;
 
-    if (dss->suspend_eventchn > 0)
-        xc_suspend_evtchn_release(CTX->xch, dss->xce, domid,
-                           dss->suspend_eventchn, &dss->guest_evtchn_lockfd);
-    if (dss->xce != NULL)
-        xc_evtchn_close(dss->xce);
+    libxl__ev_evtchn_cancel(gc, &dss->guest_evtchn);
+
+    if (dss->guest_evtchn.port > 0)
+        xc_suspend_evtchn_release(CTX->xch, CTX->xce, domid,
+                           dss->guest_evtchn.port, &dss->guest_evtchn_lockfd);
 
     dss->callback(egc, dss, rc);
 }

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index a67ea3c20a7e717c73d08b37b96e50b199b49921..69d2ba8b734c5bd481a676c710ee2975c21ba777 100644 (file)
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2438,8 +2438,7 @@ struct libxl__domain_suspend_state {
     int debug;
     const libxl_domain_remus_info *remus;
     /* private */
-    xc_evtchn *xce; /* event channel handle */
-    int suspend_eventchn;
+    libxl__ev_evtchn guest_evtchn;
     int guest_evtchn_lockfd;
     int hvm;
     int xcflags;