Re: [Xen-devel] [PATCH 16/19] libxl: suspend: Abolish usleeps in domain suspend wait

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ian Campbell writes ("Re: [PATCH 16/19] libxl: suspend: Abolish usleeps in 
domain suspend wait"):
> On Fri, 2014-03-14 at 17:41 +0000, Ian Jackson wrote:
> > Special-case paths starting with '@' in libxl__xswait.  Attempting to
> > read these from xenstore gives EINVAL.  Callers waiting for (say)
> > @releaseDomain will be checking for some condition which can be
> > observed other than by looking at xenstore.
> > 
> > Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> 
> Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

Thanks.

> (including if you fold it into the original xswait patch)

I think it's marginally clearer separately and there's no point
folding it in.

I've done the update to the patch which will use this ("libxl:
suspend: Abolish usleeps in domain suspend wait") and it builds but I
want to test it a bit before I formally resend.

But for your edification, it's below.

Ian.

From d337d7cf831360b0e7ed8f672db6b2559d063271 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Date: Fri, 6 Dec 2013 16:12:44 +0000
Subject: [PATCH 18/21] libxl: suspend: Abolish usleeps in domain suspend wait

Replace the use of a loop with usleep().

Instead, use an xswait xenstore watch with timeout.  (xenstore fires
watches on @releaseDomain when a domain shuts down.)

The logic which checks for the state of the domain is unchanged, and
not ideal, but we will leave that for the next patch.

There is not intended to be any semantic change, other than to make
the algorithm properly asynchronous and the consequential waiting be
on xenstore, rather than polling.

Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
CC: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
CC: Ian Campbell <ian.campbell@xxxxxxxxxx>

---
v3: Use an xswait instead of separate watch and timeout.
    Improve commit message.
    Remove some trailing whitespace
---
 tools/libxl/libxl_dom.c |   74 +++++++++++++++++++++++++++++++----------------
 1 file changed, 49 insertions(+), 25 deletions(-)

diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 66c84d2..7b78c88 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1028,8 +1028,12 @@ static void domain_suspend_common_wait_guest(libxl__egc 
*egc,
                                              libxl__domain_suspend_state *dss);
 static void domain_suspend_common_guest_suspended(libxl__egc *egc,
                                          libxl__domain_suspend_state *dss);
+
 static void domain_suspend_common_pvcontrol_suspending(libxl__egc *egc,
       libxl__xswait_state *xswa, int rc, const char *state);
+static void suspend_common_wait_guest_watch(libxl__egc *egc,
+      libxl__xswait_state *xswa, int rc, const char *data_dummy);
+
 static void domain_suspend_common_failed(libxl__egc *egc,
                                          libxl__domain_suspend_state *dss);
 static void domain_suspend_common_done(libxl__egc *egc,
@@ -1175,37 +1179,55 @@ static void domain_suspend_common_wait_guest(libxl__egc 
*egc,
                                              libxl__domain_suspend_state *dss)
 {
     STATE_AO_GC(dss->ao);
+    int rc;
+
+    LOG(DEBUG, "wait for the guest to suspend");
+
+    dss->guest_wait.ao = ao;
+    dss->guest_wait.what = "guest to suspend";
+    dss->guest_wait.path = "@releaseDomain";
+    dss->guest_wait.timeout_ms = 60*1000;
+    dss->guest_wait.callback = suspend_common_wait_guest_watch;
+    rc = libxl__xswait_start(gc, &dss->guest_wait);
+    if (rc) goto err;
+    return;
+
+ err:
+    domain_suspend_common_failed(egc, dss);
+}
+
+static void suspend_common_wait_guest_watch(libxl__egc *egc,
+      libxl__xswait_state *xswa, int rc, const char *data_dummy)
+{
+    libxl__domain_suspend_state *dss = CONTAINER_OF(xswa, *dss, guest_wait);
+    STATE_AO_GC(dss->ao);
+    xc_domaininfo_t info;
     int ret;
-    int watchdog;
+
+    if (rc) {
+        if (rc == ERROR_TIMEDOUT)
+            LOG(ERROR, "guest did not suspend, timed out");
+        domain_suspend_common_failed(egc, dss);
+        return;
+    }
 
     /* Convenience aliases */
     const uint32_t domid = dss->domid;
 
-    LOG(DEBUG, "wait for the guest to suspend");
-    watchdog = 60;
-    while (watchdog > 0) {
-        xc_domaininfo_t info;
-
-        usleep(100000);
-        ret = xc_domain_getinfolist(CTX->xch, domid, 1, &info);
-        if (ret == 1 && info.domain == domid &&
-            (info.flags & XEN_DOMINF_shutdown)) {
-            int shutdown_reason;
-
-            shutdown_reason = (info.flags >> XEN_DOMINF_shutdownshift)
-                & XEN_DOMINF_shutdownmask;
-            if (shutdown_reason == SHUTDOWN_suspend) {
-                LOG(DEBUG, "guest has suspended");
-                domain_suspend_common_guest_suspended(egc, dss);
-                return;
-            }
+    ret = xc_domain_getinfolist(CTX->xch, domid, 1, &info);
+    if (ret == 1 && info.domain == domid &&
+        (info.flags & XEN_DOMINF_shutdown)) {
+        int shutdown_reason;
+
+        shutdown_reason = (info.flags >> XEN_DOMINF_shutdownshift)
+            & XEN_DOMINF_shutdownmask;
+        if (shutdown_reason == SHUTDOWN_suspend) {
+            LOG(DEBUG, "guest has suspended");
+            domain_suspend_common_guest_suspended(egc, dss);
+            return;
         }
-
-        watchdog--;
     }
-
-    LOG(ERROR, "guest did not suspend");
-    domain_suspend_common_failed(egc, dss);
+    /* otherwise, keep waiting */
 }
 
 static void domain_suspend_common_guest_suspended(libxl__egc *egc,
@@ -1214,6 +1236,8 @@ static void 
domain_suspend_common_guest_suspended(libxl__egc *egc,
     STATE_AO_GC(dss->ao);
     int ret;
 
+    libxl__xswait_stop(gc, &dss->guest_wait);
+
     if (dss->hvm) {
         ret = libxl__domain_suspend_device_model(gc, dss);
         if (ret) {
@@ -1236,7 +1260,7 @@ static void domain_suspend_common_done(libxl__egc *egc,
                                        bool ok)
 {
     EGC_GC;
-    assert(!libxl__xswait_inuse(&dss->guest_wait));
+    libxl__xswait_stop(gc,&dss->guest_wait);
     dss->callback_common_done(egc, dss, ok);
 }
 
-- 
1.7.10.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel