# HG changeset patch
# User Ian Campbell <ian.campbell@xxxxxxxxxx>
# Date 1297185819 0
# Node ID d631a4996cbc69a7fa8489f28d4a3313db12e77a
# Parent a46b91cd8202726aecd9ddefd8e75faff48144d6
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>
diff -r a46b91cd8202 -r d631a4996cbc tools/libxl/libxl.h
--- a/tools/libxl/libxl.h Tue Feb 08 16:26:07 2011 +0000
+++ b/tools/libxl/libxl.h Tue Feb 08 17:23:39 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 a46b91cd8202 -r d631a4996cbc tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c Tue Feb 08 16:26:07 2011 +0000
+++ b/tools/libxl/libxl_dom.c Tue Feb 08 17:23:39 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 a46b91cd8202 -r d631a4996cbc tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Tue Feb 08 16:26:07 2011 +0000
+++ b/tools/libxl/xl_cmdimpl.c Tue Feb 08 17:23:39 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-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|