WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-changelog

[Xen-changelog] [xen-unstable] libxl/xl: improve behaviour when guest fa

To: xen-changelog@xxxxxxxxxxxxxxxxxxx
Subject: [Xen-changelog] [xen-unstable] libxl/xl: improve behaviour when guest fails to suspend itself.
From: Xen patchbot-unstable <patchbot@xxxxxxx>
Date: Tue, 15 Feb 2011 00:35:18 -0800
Delivery-date: Tue, 15 Feb 2011 00:35:58 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-changelog-request@lists.xensource.com?subject=help>
List-id: BK change log <xen-changelog.lists.xensource.com>
List-post: <mailto:xen-changelog@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-changelog>, <mailto:xen-changelog-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-changelog>, <mailto:xen-changelog-request@lists.xensource.com?subject=unsubscribe>
Reply-to: xen-devel@xxxxxxxxxxxxxxxxxxx
Sender: xen-changelog-bounces@xxxxxxxxxxxxxxxxxxx
# HG changeset patch
# User Ian Campbell <ian.campbell@xxxxxxxxxx>
# Date 1297447052 0
# Node ID 6868f7f3ab3f954d4a8ae68e92da1dec631c9001
# Parent  c4b843d0b5f4c873f61ba88849217019d7f7f885
libxl/xl: improve behaviour when guest fails to suspend itself.

The PV suspend protocol requires guest co-operating whereby the guest
must respond to a suspend request written to the xenstore control node
by clearing the node and then making a suspend hypercall.

Currently when a guest fails to do this libxl times out and returns
a generic failure code to the caller.

In response to this failure xl attempts to resume the guest. However
if the guest has not responded to the suspend request then the is no
guarantee that the guest has made the suspend hypercall (in fact it is
quite unlikely). Since the resume process attempts to modify the
return value of the hypercall (to indicate a cancelled suspend) this
results in the guest eax/rax register being corrupted!

To fix this change libxl to do the following:
   * Wait for the guest to acknowledge the suspend request.
     - on timeout cancel the suspend request.
       - if cancellation is successful then return a new error code to
         indicate that the guest is not responding.
       - if the cancel does not succeed then we raced with the guest
         which actually did acknowledge at the last minute, so
         continue.
   * Wait for the guest to suspend.
     - on timeout return the standard error code as before
   * Guest successfully suspended, return success.

Lastly in xl do not attempt to resume a guest if it has not responded
to the suspend request.

Tested by live migration of PVops kernels which either ignore the
suspend request, have already crashed and those which suspend/resume
correctly. In the first two cases the source domain is left alone (and
continues to function in the first case) and in the third the
migration is successful.

Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
Acked-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Committed-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
---
 tools/libxl/libxl.h      |    1 
 tools/libxl/libxl_dom.c  |   82 ++++++++++++++++++++++++++++++++++++++++-------
 tools/libxl/xl_cmdimpl.c |   11 +++++-
 3 files changed, 81 insertions(+), 13 deletions(-)

diff -r c4b843d0b5f4 -r 6868f7f3ab3f tools/libxl/libxl.h
--- a/tools/libxl/libxl.h       Fri Feb 11 17:56:24 2011 +0000
+++ b/tools/libxl/libxl.h       Fri Feb 11 17:57:32 2011 +0000
@@ -239,6 +239,7 @@ enum {
     ERROR_NOMEM = -5,
     ERROR_INVAL = -6,
     ERROR_BADFAIL = -7,
+    ERROR_GUEST_TIMEDOUT = -8,
 };
 
 #define LIBXL_VERSION 0
diff -r c4b843d0b5f4 -r 6868f7f3ab3f tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c   Fri Feb 11 17:56:24 2011 +0000
+++ b/tools/libxl/libxl_dom.c   Fri Feb 11 17:57:32 2011 +0000
@@ -320,6 +320,7 @@ struct suspendinfo {
     int domid;
     int hvm;
     unsigned int flags;
+    int guest_responded;
 };
 
 static int libxl__domain_suspend_common_switch_qemu_logdirty(int domid, 
unsigned int enable, void *data)
@@ -347,8 +348,9 @@ static int libxl__domain_suspend_common_
     unsigned long s_state = 0;
     int ret;
     char *path, *state = "suspend";
-    int watchdog = 60;
+    int watchdog;
     libxl_ctx *ctx = libxl__gc_owner(si->gc);
+    xs_transaction_t t;
 
     if (si->hvm)
         xc_get_hvm_param(ctx->xch, si->domid, HVM_PARAM_ACPI_S_STATE, 
&s_state);
@@ -363,6 +365,7 @@ static int libxl__domain_suspend_common_
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "xc_await_suspend failed 
ret=%d", ret);
             return 0;
         }
+        si->guest_responded = 1;
         return 1;
     }
     path = libxl__sprintf(si->gc, "%s/control/shutdown", 
libxl__xs_get_dompath(si->gc, si->domid));
@@ -376,8 +379,56 @@ static int libxl__domain_suspend_common_
             xc_domain_shutdown(ctx->xch, si->domid, SHUTDOWN_suspend);
         }
     }
+
+    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "wait for the guest to acknowledge 
suspend request");
+    watchdog = 60;
+    while (!strcmp(state, "suspend") && watchdog > 0) {
+        usleep(100000);
+
+        state = libxl__xs_read(si->gc, XBT_NULL, path);
+
+        watchdog--;
+    }
+
+    /*
+     * 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 (!strcmp(state, "suspend")) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "guest didn't acknowledge suspend, 
cancelling request");
+    retry_transaction:
+        t = xs_transaction_start(ctx->xsh);
+
+        state = libxl__xs_read(si->gc, t, path);
+
+        if (!strcmp(state, "suspend"))
+            libxl__xs_write(si->gc, t, path, "");
+
+        if (!xs_transaction_end(ctx->xsh, t, 0))
+            if (errno == EAGAIN)
+                goto retry_transaction;
+
+    }
+
+    /*
+     * 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 (!strcmp(state, "suspend")) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "guest didn't acknowledge suspend, 
request cancelled");
+        return 0;
+    }
+
+    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "guest acknowledged suspend request");
+    si->guest_responded = 1;
+
     LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "wait for the guest to suspend");
-    while (!strcmp(state, "suspend") && watchdog > 0) {
+    watchdog = 60;
+    while (watchdog > 0) {
         xc_domaininfo_t info;
 
         usleep(100000);
@@ -386,17 +437,17 @@ static int libxl__domain_suspend_common_
             int shutdown_reason;
 
             shutdown_reason = (info.flags >> XEN_DOMINF_shutdownshift) & 
XEN_DOMINF_shutdownmask;
-            if (shutdown_reason == SHUTDOWN_suspend)
+            if (shutdown_reason == SHUTDOWN_suspend) {
+                LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "guest has suspended");
                 return 1;
-        }
-        state = libxl__xs_read(si->gc, XBT_NULL, path);
+            }
+        }
+
         watchdog--;
     }
-    if (!strcmp(state, "suspend")) {
-        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "guest didn't suspend in time");
-        libxl__xs_write(si->gc, XBT_NULL, path, "");
-    }
-    return 1;
+
+    LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "guest did not suspend");
+    return 0;
 }
 
 int libxl__domain_suspend_common(libxl_ctx *ctx, uint32_t domid, int fd,
@@ -418,6 +469,7 @@ int libxl__domain_suspend_common(libxl_c
     si.hvm = hvm;
     si.gc = &gc;
     si.suspend_eventchn = -1;
+    si.guest_responded = 0;
 
     si.xce = xc_evtchn_open(NULL, 0);
     if (si.xce == NULL)
@@ -441,8 +493,14 @@ int libxl__domain_suspend_common(libxl_c
 
     rc = xc_domain_save(ctx->xch, fd, domid, 0, 0, flags, &callbacks, hvm);
     if ( rc ) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "saving domain");
-        rc = ERROR_FAIL;
+        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "saving domain: %s",
+                         si.guest_responded ?
+                         "domain responded to suspend request" :
+                         "domain did not respond to suspend request");
+        if ( !si.guest_responded )
+            rc = ERROR_GUEST_TIMEDOUT;
+        else
+            rc = ERROR_FAIL;
     }
 
     if (si.suspend_eventchn > 0)
diff -r c4b843d0b5f4 -r 6868f7f3ab3f tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c  Fri Feb 11 17:56:24 2011 +0000
+++ b/tools/libxl/xl_cmdimpl.c  Fri Feb 11 17:57:32 2011 +0000
@@ -2583,7 +2583,10 @@ static void migrate_domain(const char *d
     if (rc) {
         fprintf(stderr, "migration sender: libxl_domain_suspend failed"
                 " (rc=%d)\n", rc);
-        goto failed_resume;
+        if (rc == ERROR_GUEST_TIMEDOUT)
+            goto failed_suspend;
+        else
+            goto failed_resume;
     }
 
     //fprintf(stderr, "migration sender: Transfer complete.\n");
@@ -2661,6 +2664,12 @@ static void migrate_domain(const char *d
     libxl_domain_destroy(&ctx, domid, 1); /* bang! */
     fprintf(stderr, "Migration successful.\n");
     exit(0);
+
+ failed_suspend:
+    close(send_fd);
+    migration_child_report(child, recv_fd);
+    fprintf(stderr, "Migration failed, failed to suspend at sender.\n");
+    exit(-ERROR_FAIL);
 
  failed_resume:
     close(send_fd);

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog

<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-changelog] [xen-unstable] libxl/xl: improve behaviour when guest fails to suspend itself., Xen patchbot-unstable <=