All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.