xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Ian Jackson <ian.jackson@eu.citrix.com>
To: xen-devel@lists.xensource.com
Cc: Shriram Rajagopalan <rshriram@cs.ubc.ca>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Ian Jackson <Ian.Jackson@eu.citrix.com>,
	Ian Campbell <ian.campbell@citrix.com>,
	Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Subject: [PATCH 19/23] libxl: suspend: Async xenstore pvcontrol wait
Date: Tue, 17 Dec 2013 18:35:33 +0000	[thread overview]
Message-ID: <1387305337-15355-20-git-send-email-ian.jackson@eu.citrix.com> (raw)
In-Reply-To: <1387305337-15355-1-git-send-email-ian.jackson@eu.citrix.com>

When negotiating guest suspend via the xenstore pvcontrol protocol
(ie when the guest does NOT support the evtchn fast suspend protocol):

Replace the use of loops and usleep with a call to libxl__xswait.

Also, replace the xenstore transaction loop with one using
libxl__xs_transaction_start et al.

There is not intended to be any semantic change, other than to make
the algorithm properly asynchronous.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl_dom.c      |   93 ++++++++++++++++++++++++++----------------
 tools/libxl/libxl_internal.h |    1 +
 2 files changed, 59 insertions(+), 35 deletions(-)

diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 8aceba9..dde7e33 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1028,6 +1028,8 @@ 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 domain_suspend_common_failed(libxl__egc *egc,
                                          libxl__domain_suspend_state *dss);
 static void domain_suspend_common_done(libxl__egc *egc,
@@ -1047,10 +1049,6 @@ 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;
-    char *state = "suspend";
-    int watchdog;
-    xs_transaction_t t;
-    int rc;
 
     /* Convenience aliases */
     const uint32_t domid = dss->domid;
@@ -1096,59 +1094,81 @@ static void domain_suspend_callback_common(libxl__egc *egc,
 
     libxl__domain_pvcontrol_write(gc, XBT_NULL, domid, "suspend");
 
-    LOG(DEBUG, "wait for the guest to acknowledge suspend request");
-    watchdog = 60;
-    while (!domain_suspend_pvcontrol_acked(state) && watchdog > 0) {
-        usleep(100000);
+    dss->pvcontrol.path = libxl__domain_pvcontrol_xspath(gc, domid);
+    if (!dss->pvcontrol.path) goto err;
 
-        state = libxl__domain_pvcontrol_read(gc, XBT_NULL, domid);
+    dss->pvcontrol.ao = ao;
+    dss->pvcontrol.what = "guest acknowledgement of suspend request";
+    dss->pvcontrol.timeout_ms = 60 * 1000;
+    dss->pvcontrol.callback = domain_suspend_common_pvcontrol_suspending;
+    libxl__xswait_start(gc, &dss->pvcontrol);
+    return;
 
-        watchdog--;
-    }
+ err:
+    domain_suspend_common_failed(egc, dss);
+}
 
-    /*
-     * Guest appears to not be responding. Cancel the suspend
-     * request.
-     *
-     * We re-read the suspend node and clear it within a
-     * transaction in order to handle the case where we race
-     * against the guest catching up and acknowledging the request
-     * at the last minute.
-     */
-    if (!domain_suspend_pvcontrol_acked(state)) {
-        LOG(ERROR, "guest didn't acknowledge suspend, cancelling request");
+static void domain_suspend_common_pvcontrol_suspending(libxl__egc *egc,
+      libxl__xswait_state *xswa, int rc, const char *state)
+{
+    libxl__domain_suspend_state *dss = CONTAINER_OF(xswa, *dss, pvcontrol);
+    STATE_AO_GC(dss->ao);
+    xs_transaction_t t = 0;
+
+    if (!rc && !domain_suspend_pvcontrol_acked(state))
+        /* keep waiting */
+        return;
+
+    libxl__xswait_stop(gc, &dss->pvcontrol);
+
+    if (rc == ERROR_TIMEDOUT) {
+        /*
+         * Guest appears to not be responding. Cancel the suspend
+         * request.
+         *
+         * We re-read the suspend node and clear it within a
+         * transaction in order to handle the case where we race
+         * against the guest catching up and acknowledging the request
+         * at the last minute.
+         */
         for (;;) {
             rc = libxl__xs_transaction_start(gc, &t);
             if (rc) goto err;
 
-            state = libxl__domain_pvcontrol_read(gc, t, domid);
+            rc = libxl__xs_read_checked(gc, t, xswa->path, &state);
+            if (rc) goto err;
+
+            if (domain_suspend_pvcontrol_acked(state))
+                break;
 
-            if (!domain_suspend_pvcontrol_acked(state))
-                libxl__domain_pvcontrol_write(gc, t, domid, "");
+            rc = libxl__xs_write_checked(gc, t, xswa->path, "");
+            if (rc) goto err;
 
             rc = libxl__xs_transaction_commit(gc, &t);
-            if (!rc) break;
+            if (!rc) {
+                LOG(ERROR,
+                    "guest didn't acknowledge suspend, cancelling request");
+                goto err;
+            }
             if (rc<0) goto err;
         }
-    }
-
-    /*
-     * Final check for guest acknowledgement. The guest may have
-     * acknowledged while we were cancelling the request in which
-     * case we lost the race while cancelling and should continue.
-     */
-    if (!domain_suspend_pvcontrol_acked(state)) {
-        LOG(ERROR, "guest didn't acknowledge suspend, request cancelled");
+    } else if (rc) {
+        /* some error in xswait's read of xenstore, already logged */
         goto err;
     }
 
+    assert(domain_suspend_pvcontrol_acked(state));
     LOG(DEBUG, "guest acknowledged suspend request");
+
+    libxl__xs_transaction_abort(gc, &t);
     dss->guest_responded = 1;
     domain_suspend_common_wait_guest(egc,dss);
     return;
 
  err:
+    libxl__xs_transaction_abort(gc, &t);
     domain_suspend_common_failed(egc, dss);
+    return;
 }
 
 static void domain_suspend_common_wait_guest(libxl__egc *egc,
@@ -1215,6 +1235,8 @@ static void domain_suspend_common_done(libxl__egc *egc,
                                        libxl__domain_suspend_state *dss,
                                        bool ok)
 {
+    EGC_GC;
+    assert(!libxl__xswait_inuse(&dss->pvcontrol));
     dss->callback_common_done(egc, dss, ok);
 }
 
@@ -1400,6 +1422,7 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss)
         &dss->shs.callbacks.save.a;
 
     logdirty_init(&dss->logdirty);
+    libxl__xswait_init(&dss->pvcontrol);
 
     switch (type) {
     case LIBXL_DOMAIN_TYPE_HVM: {
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 93e0d4e..c4a509b 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2410,6 +2410,7 @@ struct libxl__domain_suspend_state {
     int hvm;
     int xcflags;
     int guest_responded;
+    libxl__xswait_state pvcontrol;
     const char *dm_savefile;
     int interval; /* checkpoint interval (for Remus) */
     libxl__save_helper_state shs;
-- 
1.7.10.4

  parent reply	other threads:[~2013-12-17 18:35 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-17 18:35 (no subject) Ian Jackson
2013-12-17 18:35 ` [PATCH 01/23] xen: Document XEN_DOMCTL_subscribe Ian Jackson
2013-12-17 18:35 ` [PATCH 02/23] xen: Document that EVTCHNOP_bind_interdomain signals Ian Jackson
2013-12-17 18:35 ` [PATCH 03/23] docs: Document event-channel-based suspend protocol Ian Jackson
2013-12-17 18:35 ` [PATCH 04/23] libxc: Document xenctrl.h event channel calls Ian Jackson
2013-12-17 18:35 ` [PATCH 05/23] libxl: init: Provide a gc later in libxl_ctx_alloc Ian Jackson
2013-12-19 12:51   ` Ian Campbell
2013-12-19 17:26     ` Ian Jackson
2013-12-17 18:35 ` [PATCH 06/23] libxl: init: libxl__poller_init and _get take gc Ian Jackson
2013-12-19 13:00   ` Ian Campbell
2013-12-19 17:27     ` Ian Jackson
2013-12-17 18:35 ` [PATCH 07/23] libxl: events: const-correct *_inuse, *_isregistered Ian Jackson
2013-12-19 13:01   ` Ian Campbell
2013-12-17 18:35 ` [PATCH 08/23] libxl: events: Provide libxl__xswait_* Ian Jackson
2013-12-19 13:05   ` Ian Campbell
2013-12-19 17:30     ` Ian Jackson
2013-12-17 18:35 ` [PATCH 09/23] libxl: events: Use libxl__xswait_* in spawn code Ian Jackson
2013-12-19 13:33   ` Ian Campbell
2013-12-17 18:35 ` [PATCH 10/23] libxl: events: Provide libxl__ev_evtchn* Ian Jackson
2013-12-19 13:43   ` Ian Campbell
2013-12-19 17:47     ` Ian Jackson
2013-12-19 17:51       ` Ian Campbell
2013-12-20 11:52         ` Ian Jackson
2013-12-17 18:35 ` [PATCH 11/23] libxc: suspend: Rename, improve xc_suspend_evtchn_init Ian Jackson
2014-03-13 16:05   ` Ian Campbell
2013-12-17 18:35 ` [PATCH 12/23] libxc: suspend: Fix suspend event channel locking Ian Jackson
2013-12-17 18:35 ` [PATCH 13/23] libxl: suspend: Async libxl__domain_suspend_callback Ian Jackson
2013-12-17 18:35 ` [PATCH 14/23] libxl: suspend: Async domain_suspend_callback_common Ian Jackson
2013-12-17 18:35 ` [PATCH 15/23] libxl: suspend: Reorg domain_suspend_callback_common Ian Jackson
2013-12-17 18:35 ` [PATCH 16/23] libxl: suspend: New libxl__domain_pvcontrol_xspath Ian Jackson
2013-12-17 18:35 ` [PATCH 17/23] libxl: suspend: New domain_suspend_pvcontrol_acked Ian Jackson
2013-12-17 18:35 ` [PATCH 18/23] libxl: suspend: domain_suspend_callback_common xs errs Ian Jackson
2013-12-17 18:35 ` Ian Jackson [this message]
2013-12-17 18:35 ` [PATCH 20/23] libxl: suspend: Abolish usleeps in domain suspend wait Ian Jackson
2013-12-17 18:35 ` [PATCH 21/23] libxl: suspend: Fix suspend wait corner cases Ian Jackson
2013-12-17 18:35 ` [PATCH 22/23] libxl: suspend: Async evtchn wait Ian Jackson
2013-12-17 18:35 ` [PATCH 23/23] libxl: suspend: Apply guest timeout in evtchn case Ian Jackson
2013-12-18 11:19 ` (no subject) George Dunlap
2013-12-18 13:35   ` Ian Campbell
2014-01-07 13:55     ` Ian Campbell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1387305337-15355-20-git-send-email-ian.jackson@eu.citrix.com \
    --to=ian.jackson@eu.citrix.com \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=ian.campbell@citrix.com \
    --cc=rshriram@cs.ubc.ca \
    --cc=stefano.stabellini@eu.citrix.com \
    --cc=xen-devel@lists.xensource.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).