[PATCH 0 of 6 V3] libxl: refactor suspend/resume code

[PATCH 0 of 6 V3] libxl: refactor suspend/resume code

[rshriram]

This patch series refactors the suspend/resume code to minimize
Remus specific code in libxl. There are a couple of trivial bug
fixes too.

These patches depend on Stefano's "v4 libxl: save/resto​re qemu physmap"


Changes in V3:

1. rebase patches based on Stefano's patches
   use qmp_save instead of qmp_migrate
2. check if qemu moves to "running" state after resuming the device model
3. Moved comments on the co-operative suspend to libxl.h


Changes in V2:
1. migrate code is refactored as save_config , create child,
   do_preamble instead of coaelscing them all into one single
   function.
2. More documentation for suspend_cancel parameter in domain_resume
3. Minor nits

shriram
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 1 of 6 V3] libxl: helper function to send commands to traditional qemu

[rshriram]

# HG changeset patch
# User Shriram Rajagopalan <rshriram@cs.ubc.ca>
# Date 1328251592 28800
# Node ID 340dd6a3f0dab2fcba83a68dea072e9d9af20182
# Parent  4612cca7fb7c06e83a205510f6c2e43d5f246582
libxl: helper function to send commands to traditional qemu

Introduce a helper function to send commands to traditional
qemu. qemu_pci_add_xenstore, qemu_pci_remove_xenstore,
libxl__domain_save_device_model and libxl_domain_unpause have
been refactored to use this function.

Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

diff -r 4612cca7fb7c -r 340dd6a3f0da tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Thu Feb 02 22:46:17 2012 -0800
+++ b/tools/libxl/libxl.c	Thu Feb 02 22:46:32 2012 -0800
@@ -517,7 +517,7 @@ int libxl_domain_unpause(libxl_ctx *ctx,
         path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/state", domid);
         state = libxl__xs_read(gc, XBT_NULL, path);
         if (state != NULL && !strcmp(state, "paused")) {
-            libxl__xs_write(gc, XBT_NULL, libxl__sprintf(gc, "/local/domain/0/device-model/%d/command", domid), "continue");
+            libxl__qemu_traditional_cmd(gc, domid, "continue");
             libxl__wait_for_device_model(gc, domid, "running",
                                          NULL, NULL, NULL);
         }
diff -r 4612cca7fb7c -r 340dd6a3f0da tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Thu Feb 02 22:46:17 2012 -0800
+++ b/tools/libxl/libxl_dom.c	Thu Feb 02 22:46:32 2012 -0800
@@ -409,6 +409,15 @@ static int libxl__toolstack_restore(uint
     return 0;
 }
 
+int libxl__qemu_traditional_cmd(libxl__gc *gc, uint32_t domid,
+                                const char *cmd)
+{
+    char *path = NULL;
+    path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/command",
+                          domid);
+    return libxl__xs_write(gc, XBT_NULL, path, "%s", cmd);
+}
+
 int libxl__domain_restore_common(libxl__gc *gc, uint32_t domid,
                                  libxl_domain_build_info *info,
                                  libxl__domain_build_state *state,
@@ -761,12 +770,9 @@ int libxl__domain_save_device_model(libx
 
     switch (libxl__device_model_version_running(gc, domid)) {
     case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL: {
-        char *path = NULL;
         LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
                    "Saving device model state to %s", filename);
-        path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/command",
-                              domid);
-        libxl__xs_write(gc, XBT_NULL, path, "save");
+        libxl__qemu_traditional_cmd(gc, domid, "save");
         libxl__wait_for_device_model(gc, domid, "paused", NULL, NULL, NULL);
         break;
     }
diff -r 4612cca7fb7c -r 340dd6a3f0da tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Thu Feb 02 22:46:17 2012 -0800
+++ b/tools/libxl/libxl_internal.h	Thu Feb 02 22:46:32 2012 -0800
@@ -263,6 +263,8 @@ _hidden int libxl__build_hvm(libxl__gc *
               libxl_device_model_info *dm_info,
               libxl__domain_build_state *state);
 
+_hidden int libxl__qemu_traditional_cmd(libxl__gc *gc, uint32_t domid,
+                                        const char *cmd);
 _hidden int libxl__domain_rename(libxl__gc *gc, uint32_t domid,
                                  const char *old_name, const char *new_name,
                                  xs_transaction_t trans);
diff -r 4612cca7fb7c -r 340dd6a3f0da tools/libxl/libxl_pci.c
--- a/tools/libxl/libxl_pci.c	Thu Feb 02 22:46:17 2012 -0800
+++ b/tools/libxl/libxl_pci.c	Thu Feb 02 22:46:32 2012 -0800
@@ -602,9 +602,8 @@ static int qemu_pci_add_xenstore(libxl__
         libxl__xs_write(gc, XBT_NULL, path, PCI_BDF, pcidev->domain,
                         pcidev->bus, pcidev->dev, pcidev->func);
     }
-    path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/command",
-                          domid);
-    xs_write(ctx->xsh, XBT_NULL, path, "pci-ins", strlen("pci-ins"));
+
+    libxl__qemu_traditional_cmd(gc, domid, "pci-ins");
     rc = libxl__wait_for_device_model(gc, domid, NULL, NULL,
                                       pci_ins_check, state);
     path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/parameter",
@@ -857,12 +856,11 @@ static int qemu_pci_remove_xenstore(libx
     path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/parameter", domid);
     libxl__xs_write(gc, XBT_NULL, path, PCI_BDF, pcidev->domain,
                     pcidev->bus, pcidev->dev, pcidev->func);
-    path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/command", domid);
 
     /* Remove all functions at once atomically by only signalling
      * device-model for function 0 */
     if ( !force && (pcidev->vdevfn & 0x7) == 0 ) {
-        xs_write(ctx->xsh, XBT_NULL, path, "pci-rem", strlen("pci-rem"));
+        libxl__qemu_traditional_cmd(gc, domid, "pci-rem");
         if (libxl__wait_for_device_model(gc, domid, "pci-removed",
                                          NULL, NULL, NULL) < 0) {
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Device Model didn't respond in time");

[PATCH 2 of 6 V3] libxl: bugfix: create_domain() return to caller if !daemonize

[rshriram]

# HG changeset patch
# User Shriram Rajagopalan <rshriram@cs.ubc.ca>
# Date 1328251593 28800
# Node ID 636da26c40d37b84a93b6a6c3881b2fccc768aa2
# Parent  340dd6a3f0dab2fcba83a68dea072e9d9af20182
libxl: bugfix: create_domain() return to caller if !daemonize

Currently the create_domain function does not honor
the daemonize flag properly. It exits irrespective of
the value of the flag. This patch fixes the issue.

Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

diff -r 340dd6a3f0da -r 636da26c40d3 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Thu Feb 02 22:46:32 2012 -0800
+++ b/tools/libxl/xl_cmdimpl.c	Thu Feb 02 22:46:33 2012 -0800
@@ -1814,7 +1814,7 @@ waitpid_out:
      * If we have daemonized then do not return to the caller -- this has
      * already happened in the parent.
      */
-    if ( !need_daemon )
+    if ( daemonize && !need_daemon )
         exit(ret);
 
     return ret;

[PATCH 3 of 6 V3] libxl: QMP stop/resume & refactor QEMU suspend/resume/save

[rshriram]

# HG changeset patch
# User Shriram Rajagopalan <rshriram@cs.ubc.ca>
# Date 1328251593 28800
# Node ID 329b3c94c618addb1e802cebc7fe23b12b432398
# Parent  636da26c40d37b84a93b6a6c3881b2fccc768aa2
libxl: QMP stop/resume & refactor QEMU suspend/resume/save

Implement QMP stop and resume functionality and split
device model save into 3 parts:
 suspend_dm(domid)
 save_dm(domid, fd)
 resume_dm(domid)

Integrate Device model suspend into suspend_common_callback

Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>

diff -r 636da26c40d3 -r 329b3c94c618 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Thu Feb 02 22:46:33 2012 -0800
+++ b/tools/libxl/libxl_dom.c	Thu Feb 02 22:46:33 2012 -0800
@@ -488,6 +488,54 @@ static int libxl__domain_suspend_common_
     return rc ? 0 : 1;
 }
 
+int libxl__domain_suspend_device_model(libxl__gc *gc, uint32_t domid)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    int ret = 0;
+    const char *filename = libxl__device_model_savefile(gc, domid);
+
+    switch (libxl__device_model_version_running(gc, domid)) {
+    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL: {
+        LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
+                   "Saving device model state to %s", filename);
+        libxl__qemu_traditional_cmd(gc, domid, "save");
+        libxl__wait_for_device_model(gc, domid, "paused", NULL, NULL, NULL);
+        break;
+    }
+    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+        if (libxl__qmp_stop(gc, domid))
+            return ERROR_FAIL;
+        /* Save DM state into filename */
+        ret = libxl__qmp_save(gc, domid, filename);
+        if (ret)
+            unlink(filename);
+        break;
+    default:
+        return ERROR_INVAL;
+    }
+
+    return ret;
+}
+
+int libxl__domain_resume_device_model(libxl__gc *gc, uint32_t domid)
+{
+
+    switch (libxl__device_model_version_running(gc, domid)) {
+    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL: {
+        libxl__qemu_traditional_cmd(gc, domid, "continue");
+        libxl__wait_for_device_model(gc, domid, "running", NULL, NULL, NULL);
+        break;
+    }
+    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+        if (libxl__qmp_resume(gc, domid))
+            return ERROR_FAIL;
+    default:
+        return ERROR_INVAL;
+    }
+
+    return 0;
+}
+
 static int libxl__domain_suspend_common_callback(void *data)
 {
     struct suspendinfo *si = data;
@@ -517,7 +565,7 @@ static int libxl__domain_suspend_common_
             return 0;
         }
         si->guest_responded = 1;
-        return 1;
+        goto guest_suspended;
     }
 
     if (si->hvm && (!hvm_pvdrv || hvm_s_state)) {
@@ -595,7 +643,7 @@ static int libxl__domain_suspend_common_
             shutdown_reason = (info.flags >> XEN_DOMINF_shutdownshift) & XEN_DOMINF_shutdownmask;
             if (shutdown_reason == SHUTDOWN_suspend) {
                 LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "guest has suspended");
-                return 1;
+                goto guest_suspended;
             }
         }
 
@@ -604,6 +652,17 @@ static int libxl__domain_suspend_common_
 
     LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "guest did not suspend");
     return 0;
+
+ guest_suspended:
+    if (si->hvm) {
+        ret = libxl__domain_suspend_device_model(si->gc, si->domid);
+        if (ret) {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                       "libxl__domain_suspend_device_model failed ret=%d", ret);
+            return 0;
+        }
+    }
+    return 1;
 }
 
 static int libxl__toolstack_save(uint32_t domid, uint8_t **buf,
@@ -768,23 +827,6 @@ int libxl__domain_save_device_model(libx
     struct stat st;
     uint32_t qemu_state_len;
 
-    switch (libxl__device_model_version_running(gc, domid)) {
-    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL: {
-        LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
-                   "Saving device model state to %s", filename);
-        libxl__qemu_traditional_cmd(gc, domid, "save");
-        libxl__wait_for_device_model(gc, domid, "paused", NULL, NULL, NULL);
-        break;
-    }
-    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
-        ret = libxl__qmp_save(gc, domid, (char *)filename);
-        if (ret)
-            goto out;
-        break;
-    default:
-        return ERROR_INVAL;
-    }
-
     if (stat(filename, &st) < 0)
     {
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Unable to stat qemu save file\n");
diff -r 636da26c40d3 -r 329b3c94c618 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Thu Feb 02 22:46:33 2012 -0800
+++ b/tools/libxl/libxl_internal.h	Thu Feb 02 22:46:33 2012 -0800
@@ -277,6 +277,8 @@ _hidden int libxl__domain_suspend_common
                                          libxl_domain_type type,
                                          int live, int debug);
 _hidden const char *libxl__device_model_savefile(libxl__gc *gc, uint32_t domid);
+_hidden int libxl__domain_suspend_device_model(libxl__gc *gc, uint32_t domid);
+_hidden int libxl__domain_resume_device_model(libxl__gc *gc, uint32_t domid);
 _hidden int libxl__domain_save_device_model(libxl__gc *gc, uint32_t domid, int fd);
 _hidden void libxl__userdata_destroyall(libxl__gc *gc, uint32_t domid);
 
@@ -620,6 +622,10 @@ _hidden int libxl__qmp_query_serial(libx
 _hidden int libxl__qmp_pci_add(libxl__gc *gc, int d, libxl_device_pci *pcidev);
 _hidden int libxl__qmp_pci_del(libxl__gc *gc, int domid,
                                libxl_device_pci *pcidev);
+/* Suspend QEMU. */
+_hidden int libxl__qmp_stop(libxl__gc *gc, int domid);
+/* Resume QEMU. */
+_hidden int libxl__qmp_resume(libxl__gc *gc, int domid);
 /* Save current QEMU state into fd. */
 _hidden int libxl__qmp_save(libxl__gc *gc, int domid, const char *filename);
 /* close and free the QMP handler */
diff -r 636da26c40d3 -r 329b3c94c618 tools/libxl/libxl_qmp.c
--- a/tools/libxl/libxl_qmp.c	Thu Feb 02 22:46:33 2012 -0800
+++ b/tools/libxl/libxl_qmp.c	Thu Feb 02 22:46:33 2012 -0800
@@ -802,6 +802,38 @@ out:
     return rc;
 }
 
+int libxl__qmp_stop(libxl__gc *gc, int domid)
+{
+    libxl__qmp_handler *qmp = NULL;
+    int rc = 0;
+
+    qmp = libxl__qmp_initialize(libxl__gc_owner(gc), domid);
+    if (!qmp)
+        return ERROR_FAIL;
+
+    rc = qmp_synchronous_send(qmp, "stop", NULL,
+                              NULL, NULL, qmp->timeout);
+
+    libxl__qmp_close(qmp);
+    return rc;
+}
+
+int libxl__qmp_resume(libxl__gc *gc, int domid)
+{
+    libxl__qmp_handler *qmp = NULL;
+    int rc = 0;
+
+    qmp = libxl__qmp_initialize(libxl__gc_owner(gc), domid);
+    if (!qmp)
+        return ERROR_FAIL;
+
+    rc = qmp_synchronous_send(qmp, "cont", NULL,
+                              NULL, NULL, qmp->timeout);
+
+    libxl__qmp_close(qmp);
+    return rc;
+}
+
 int libxl__qmp_initializations(libxl_ctx *ctx, uint32_t domid)
 {
     libxl__qmp_handler *qmp = NULL;

[PATCH 4 of 6 V3] libxl: support suspend_cancel in domain_resume

[rshriram]

# HG changeset patch
# User Shriram Rajagopalan <rshriram@cs.ubc.ca>
# Date 1328251593 28800
# Node ID f853c88f0230a2e9d2e1006a9cd220c4cd27e74d
# Parent  329b3c94c618addb1e802cebc7fe23b12b432398
libxl: support suspend_cancel in domain_resume

Add an extra parameter to libxl_domain_resume indicating
if the caller wishes to use the SUSPEND_CANCEL style
resume instead of the normal resume.

Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>

diff -r 329b3c94c618 -r f853c88f0230 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Thu Feb 02 22:46:33 2012 -0800
+++ b/tools/libxl/libxl.c	Thu Feb 02 22:46:33 2012 -0800
@@ -229,24 +229,29 @@ int libxl_domain_rename(libxl_ctx *ctx, 
     return rc;
 }
 
-int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid)
+int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid, int suspend_cancel)
 {
     GC_INIT(ctx);
     int rc = 0;
 
-    if (LIBXL__DOMAIN_IS_TYPE(gc,  domid, HVM)) {
-        LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Called domain_resume on "
-                "non-cooperative hvm domain %u", domid);
-        rc = ERROR_NI;
-        goto out;
-    }
-    if (xc_domain_resume(ctx->xch, domid, 0)) {
+    if (xc_domain_resume(ctx->xch, domid, suspend_cancel)) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
                         "xc_domain_resume failed for domain %u",
                         domid);
         rc = ERROR_FAIL;
         goto out;
     }
+
+    if (LIBXL__DOMAIN_IS_TYPE(gc,  domid, HVM)) {
+        rc = libxl__domain_resume_device_model(gc, domid);
+        if (rc) {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                       "failed to resume device model for domain %u:%d",
+                       domid, rc);
+            goto out;
+        }
+    }
+
     if (!xs_resume_domain(ctx->xsh, domid)) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
                         "xs_resume_domain failed for domain %u",
diff -r 329b3c94c618 -r f853c88f0230 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Thu Feb 02 22:46:33 2012 -0800
+++ b/tools/libxl/libxl.h	Thu Feb 02 22:46:33 2012 -0800
@@ -268,7 +268,12 @@ int libxl_domain_create_restore(libxl_ct
 void libxl_domain_config_dispose(libxl_domain_config *d_config);
 int libxl_domain_suspend(libxl_ctx *ctx, libxl_domain_suspend_info *info,
                           uint32_t domid, int fd);
-int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid);
+
+/* @param suspend_cancel [from xenctrl.h:xc_domain_resume( @param fast )]
+ *   If this parameter is true, use co-operative resume. The guest
+ *   must support this.
+ */
+int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid, int suspend_cancel);
 int libxl_domain_shutdown(libxl_ctx *ctx, uint32_t domid);
 int libxl_domain_reboot(libxl_ctx *ctx, uint32_t domid);
 int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid);
diff -r 329b3c94c618 -r f853c88f0230 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Thu Feb 02 22:46:33 2012 -0800
+++ b/tools/libxl/xl_cmdimpl.c	Thu Feb 02 22:46:33 2012 -0800
@@ -2751,7 +2751,7 @@ static void migrate_domain(const char *d
         if (common_domname) {
             libxl_domain_rename(ctx, domid, away_domname, common_domname);
         }
-        rc = libxl_domain_resume(ctx, domid);
+        rc = libxl_domain_resume(ctx, domid, 1);
         if (!rc) fprintf(stderr, "migration sender: Resumed OK.\n");
 
         fprintf(stderr, "Migration failed due to problems at target.\n");
@@ -2773,7 +2773,7 @@ static void migrate_domain(const char *d
     close(send_fd);
     migration_child_report(child, recv_fd);
     fprintf(stderr, "Migration failed, resuming at sender.\n");
-    libxl_domain_resume(ctx, domid);
+    libxl_domain_resume(ctx, domid, 1);
     exit(-ERROR_FAIL);
 
  failed_badly:

[PATCH 5 of 6 V3] libxl: refactor migrate_domain and generalize migrate_receive

[rshriram]

# HG changeset patch
# User Shriram Rajagopalan <rshriram@cs.ubc.ca>
# Date 1328251593 28800
# Node ID 62c4fd2fe9bbc2c283e3998d852317a48e9f9770
# Parent  f853c88f0230a2e9d2e1006a9cd220c4cd27e74d
libxl: refactor migrate_domain and generalize migrate_receive

Refactor some tasks like establishing the migration channel,
initial migration protocol exchange into separate functions,
to facilitate re-use, when remus support is introduced. Also,
make migrate_receive generic (instead of resorting to stdin and
stdout as the file descriptors for communication).

Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>

diff -r f853c88f0230 -r 62c4fd2fe9bb tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Thu Feb 02 22:46:33 2012 -0800
+++ b/tools/libxl/xl_cmdimpl.c	Thu Feb 02 22:46:33 2012 -0800
@@ -2531,6 +2531,43 @@ static int save_domain(const char *p, co
     exit(0);
 }
 
+static pid_t create_migration_child(const char *rune, int *send_fd,
+                                        int *recv_fd)
+{
+    int sendpipe[2], recvpipe[2];
+    pid_t child = -1;
+
+    if (!rune || !send_fd || !recv_fd)
+        return -1;
+
+    MUST( libxl_pipe(ctx, sendpipe) );
+    MUST( libxl_pipe(ctx, recvpipe) );
+
+    child = libxl_fork(ctx);
+    if (child==-1) exit(1);
+
+    if (!child) {
+        dup2(sendpipe[0], 0);
+        dup2(recvpipe[1], 1);
+        close(sendpipe[0]); close(sendpipe[1]);
+        close(recvpipe[0]); close(recvpipe[1]);
+        execlp("sh","sh","-c",rune,(char*)0);
+        perror("failed to exec sh");
+        exit(-1);
+    }
+
+    close(sendpipe[0]);
+    close(recvpipe[1]);
+    *send_fd = sendpipe[1];
+    *recv_fd = recvpipe[0];
+
+    /* if receiver dies, we get an error and can clean up
+       rather than just dying */
+    signal(SIGPIPE, SIG_IGN);
+
+    return child;
+}
+
 static int migrate_read_fixedmessage(int fd, const void *msg, int msgsz,
                                      const char *what, const char *rune) {
     char buf[msgsz];
@@ -2616,53 +2653,17 @@ static void migration_child_report(pid_t
     migration_child = 0;
 }
 
-static void migrate_domain(const char *domain_spec, const char *rune,
-                           const char *override_config_file)
+static void migrate_do_preamble(int send_fd, int recv_fd, pid_t child,
+                                uint8_t *config_data, int config_len,
+                                const char *rune)
 {
-    pid_t child = -1;
-    int rc;
-    int sendpipe[2], recvpipe[2];
-    int send_fd, recv_fd;
-    libxl_domain_suspend_info suspinfo;
-    char *away_domname;
-    char rc_buf;
-    uint8_t *config_data;
-    int config_len;
-
-    save_domain_core_begin(domain_spec, override_config_file,
-                           &config_data, &config_len);
-
-    if (!config_len) {
-        fprintf(stderr, "No config file stored for running domain and "
-                "none supplied - cannot migrate.\n");
+    int rc = 0;
+
+    if (send_fd < 0 || recv_fd < 0) {
+        fprintf(stderr, "migrate_do_preamble: invalid file descriptors\n");
         exit(1);
     }
 
-    MUST( libxl_pipe(ctx, sendpipe) );
-    MUST( libxl_pipe(ctx, recvpipe) );
-
-    child = libxl_fork(ctx);
-    if (child==-1) exit(1);
-
-    if (!child) {
-        dup2(sendpipe[0], 0);
-        dup2(recvpipe[1], 1);
-        close(sendpipe[0]); close(sendpipe[1]);
-        close(recvpipe[0]); close(recvpipe[1]);
-        execlp("sh","sh","-c",rune,(char*)0);
-        perror("failed to exec sh");
-        exit(-1);
-    }
-
-    close(sendpipe[0]);
-    close(recvpipe[1]);
-    send_fd = sendpipe[1];
-    recv_fd = recvpipe[0];
-
-    signal(SIGPIPE, SIG_IGN);
-    /* if receiver dies, we get an error and can clean up
-       rather than just dying */
-
     rc = migrate_read_fixedmessage(recv_fd, migrate_receiver_banner,
                                    sizeof(migrate_receiver_banner)-1,
                                    "banner", rune);
@@ -2675,6 +2676,34 @@ static void migrate_domain(const char *d
     save_domain_core_writeconfig(send_fd, "migration stream",
                                  config_data, config_len);
 
+}
+
+static void migrate_domain(const char *domain_spec, const char *rune,
+                           const char *override_config_file)
+{
+    pid_t child = -1;
+    int rc;
+    int send_fd = -1, recv_fd = -1;
+    libxl_domain_suspend_info suspinfo;
+    char *away_domname;
+    char rc_buf;
+    uint8_t *config_data;
+    int config_len;
+
+    save_domain_core_begin(domain_spec, override_config_file,
+                           &config_data, &config_len);
+
+    if (!config_len) {
+        fprintf(stderr, "No config file stored for running domain and "
+                "none supplied - cannot migrate.\n");
+        exit(1);
+    }
+
+    child = create_migration_child(rune, &send_fd, &recv_fd);
+
+    migrate_do_preamble(send_fd, recv_fd, child, config_data, config_len,
+                        rune);
+
     xtl_stdiostream_adjust_flags(logger, XTL_STDIOSTREAM_HIDE_PROGRESS, 0);
 
     memset(&suspinfo, 0, sizeof(suspinfo));
@@ -2798,7 +2827,8 @@ static void core_dump_domain(const char 
     if (rc) { fprintf(stderr,"core dump failed (rc=%d)\n",rc);exit(-1); }
 }
 
-static void migrate_receive(int debug, int daemonize, int monitor)
+static void migrate_receive(int debug, int daemonize, int monitor,
+                            int send_fd, int recv_fd)
 {
     int rc, rc2;
     char rc_buf;
@@ -2810,7 +2840,7 @@ static void migrate_receive(int debug, i
 
     fprintf(stderr, "migration target: Ready to receive domain.\n");
 
-    CHK_ERRNO( libxl_write_exactly(ctx, 1,
+    CHK_ERRNO( libxl_write_exactly(ctx, send_fd,
                                    migrate_receiver_banner,
                                    sizeof(migrate_receiver_banner)-1,
                                    "migration ack stream",
@@ -2822,7 +2852,7 @@ static void migrate_receive(int debug, i
     dom_info.monitor = monitor;
     dom_info.paused = 1;
     dom_info.restore_file = "incoming migration stream";
-    dom_info.migrate_fd = 0; /* stdin */
+    dom_info.migrate_fd = recv_fd;
     dom_info.migration_domname_r = &migration_domname;
     dom_info.no_incr_generationid = 1;
 
@@ -2836,13 +2866,13 @@ static void migrate_receive(int debug, i
     fprintf(stderr, "migration target: Transfer complete,"
             " requesting permission to start domain.\n");
 
-    rc = libxl_write_exactly(ctx, 1,
+    rc = libxl_write_exactly(ctx, send_fd,
                              migrate_receiver_ready,
                              sizeof(migrate_receiver_ready),
                              "migration ack stream", "ready message");
     if (rc) exit(-rc);
 
-    rc = migrate_read_fixedmessage(0, migrate_permission_to_go,
+    rc = migrate_read_fixedmessage(recv_fd, migrate_permission_to_go,
                                    sizeof(migrate_permission_to_go),
                                    "GO message", 0);
     if (rc) goto perhaps_destroy_notify_rc;
@@ -2861,7 +2891,7 @@ static void migrate_receive(int debug, i
     rc = 0;
 
  perhaps_destroy_notify_rc:
-    rc2 = libxl_write_exactly(ctx, 1,
+    rc2 = libxl_write_exactly(ctx, send_fd,
                               migrate_report, sizeof(migrate_report),
                               "migration ack stream",
                               "success/failure report");
@@ -2869,7 +2899,7 @@ static void migrate_receive(int debug, i
 
     rc_buf = -rc;
     assert(!!rc_buf == !!rc);
-    rc2 = libxl_write_exactly(ctx, 1, &rc_buf, 1,
+    rc2 = libxl_write_exactly(ctx, send_fd, &rc_buf, 1,
                               "migration ack stream",
                               "success/failure code");
     if (rc2) exit(-ERROR_BADFAIL);
@@ -2887,7 +2917,7 @@ static void migrate_receive(int debug, i
         fprintf(stderr, "migration target: Cleanup OK, granting sender"
                 " permission to resume.\n");
 
-        rc2 = libxl_write_exactly(ctx, 1,
+        rc2 = libxl_write_exactly(ctx, send_fd,
                                   migrate_permission_to_go,
                                   sizeof(migrate_permission_to_go),
                                   "migration ack stream",
@@ -2983,7 +3013,9 @@ int main_migrate_receive(int argc, char 
         help("migrate-receive");
         return 2;
     }
-    migrate_receive(debug, daemonize, monitor);
+    migrate_receive(debug, daemonize, monitor,
+                    STDOUT_FILENO, STDIN_FILENO);
+
     return 0;
 }

[PATCH 6 of 6 V3] libxl: resume instead of unpause on xl save -c

[rshriram]

# HG changeset patch
# User Shriram Rajagopalan <rshriram@cs.ubc.ca>
# Date 1328251593 28800
# Node ID c7abecc14cceb18140335ebe20faad826282cd1f
# Parent  62c4fd2fe9bbc2c283e3998d852317a48e9f9770
libxl: resume instead of unpause on xl save -c

The guest is "suspended" via libxl_domain_suspend when taking a snapshot.
So call libxl_domain_resume instead of libxl_domain_unpause, when taking
a checkpoint of the domain (using xl save -c).

Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>

diff -r 62c4fd2fe9bb -r c7abecc14cce tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Thu Feb 02 22:46:33 2012 -0800
+++ b/tools/libxl/xl_cmdimpl.c	Thu Feb 02 22:46:33 2012 -0800
@@ -2524,7 +2524,7 @@ static int save_domain(const char *p, co
     close(fd);
 
     if (checkpoint)
-        libxl_domain_unpause(ctx, domid);
+        libxl_domain_resume(ctx, domid, 1);
     else
         libxl_domain_destroy(ctx, domid);

Re: [PATCH 3 of 6 V3] libxl: QMP stop/resume & refactor QEMU suspend/resume/save

[Ian Campbell]

On Fri, 2012-02-03 at 06:49 +0000, rshriram@cs.ubc.ca wrote:
> # HG changeset patch
> # User Shriram Rajagopalan <rshriram@cs.ubc.ca>
> # Date 1328251593 28800
> # Node ID 329b3c94c618addb1e802cebc7fe23b12b432398
> # Parent  636da26c40d37b84a93b6a6c3881b2fccc768aa2
> libxl: QMP stop/resume & refactor QEMU suspend/resume/save
> 
> Implement QMP stop and resume functionality and split
> device model save into 3 parts:
>  suspend_dm(domid)
>  save_dm(domid, fd)
>  resume_dm(domid)
> 
> Integrate Device model suspend into suspend_common_callback
> 
> Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> 
> diff -r 636da26c40d3 -r 329b3c94c618 tools/libxl/libxl_dom.c
> --- a/tools/libxl/libxl_dom.c	Thu Feb 02 22:46:33 2012 -0800
> +++ b/tools/libxl/libxl_dom.c	Thu Feb 02 22:46:33 2012 -0800
> @@ -488,6 +488,54 @@ static int libxl__domain_suspend_common_
>      return rc ? 0 : 1;
>  }
>  
> +int libxl__domain_suspend_device_model(libxl__gc *gc, uint32_t domid)
> +{
> +    libxl_ctx *ctx = libxl__gc_owner(gc);
> +    int ret = 0;
> +    const char *filename = libxl__device_model_savefile(gc, domid);
> +
> +    switch (libxl__device_model_version_running(gc, domid)) {
> +    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL: {
> +        LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
> +                   "Saving device model state to %s", filename);
> +        libxl__qemu_traditional_cmd(gc, domid, "save");
> +        libxl__wait_for_device_model(gc, domid, "paused", NULL, NULL, NULL);
> +        break;
> +    }
> +    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
> +        if (libxl__qmp_stop(gc, domid))
> +            return ERROR_FAIL;
> +        /* Save DM state into filename */
> +        ret = libxl__qmp_save(gc, domid, filename);
> +        if (ret)
> +            unlink(filename);
> +        break;
> +    default:
> +        return ERROR_INVAL;
> +    }
> +
> +    return ret;
> +}
> +
> +int libxl__domain_resume_device_model(libxl__gc *gc, uint32_t domid)
> +{
> +
> +    switch (libxl__device_model_version_running(gc, domid)) {
> +    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL: {
> +        libxl__qemu_traditional_cmd(gc, domid, "continue");
> +        libxl__wait_for_device_model(gc, domid, "running", NULL, NULL, NULL);
> +        break;
> +    }
> +    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
> +        if (libxl__qmp_resume(gc, domid))
> +            return ERROR_FAIL;
> +    default:
> +        return ERROR_INVAL;
> +    }
> +
> +    return 0;
> +}
> +
>  static int libxl__domain_suspend_common_callback(void *data)
>  {
>      struct suspendinfo *si = data;
> @@ -517,7 +565,7 @@ static int libxl__domain_suspend_common_
>              return 0;
>          }
>          si->guest_responded = 1;
> -        return 1;
> +        goto guest_suspended;
>      }
>  
>      if (si->hvm && (!hvm_pvdrv || hvm_s_state)) {
> @@ -595,7 +643,7 @@ static int libxl__domain_suspend_common_
>              shutdown_reason = (info.flags >> XEN_DOMINF_shutdownshift) & XEN_DOMINF_shutdownmask;
>              if (shutdown_reason == SHUTDOWN_suspend) {
>                  LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "guest has suspended");
> -                return 1;
> +                goto guest_suspended;
>              }
>          }
>  
> @@ -604,6 +652,17 @@ static int libxl__domain_suspend_common_
>  
>      LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "guest did not suspend");
>      return 0;
> +
> + guest_suspended:
> +    if (si->hvm) {
> +        ret = libxl__domain_suspend_device_model(si->gc, si->domid);
> +        if (ret) {
> +            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> +                       "libxl__domain_suspend_device_model failed ret=%d", ret);
> +            return 0;
> +        }
> +    }
> +    return 1;
>  }
>  
>  static int libxl__toolstack_save(uint32_t domid, uint8_t **buf,
> @@ -768,23 +827,6 @@ int libxl__domain_save_device_model(libx
>      struct stat st;
>      uint32_t qemu_state_len;
>  
> -    switch (libxl__device_model_version_running(gc, domid)) {
> -    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL: {
> -        LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
> -                   "Saving device model state to %s", filename);
> -        libxl__qemu_traditional_cmd(gc, domid, "save");
> -        libxl__wait_for_device_model(gc, domid, "paused", NULL, NULL, NULL);
> -        break;
> -    }
> -    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
> -        ret = libxl__qmp_save(gc, domid, (char *)filename);
> -        if (ret)
> -            goto out;
> -        break;
> -    default:
> -        return ERROR_INVAL;
> -    }
> -
>      if (stat(filename, &st) < 0)
>      {
>          LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Unable to stat qemu save file\n");
> diff -r 636da26c40d3 -r 329b3c94c618 tools/libxl/libxl_internal.h
> --- a/tools/libxl/libxl_internal.h	Thu Feb 02 22:46:33 2012 -0800
> +++ b/tools/libxl/libxl_internal.h	Thu Feb 02 22:46:33 2012 -0800
> @@ -277,6 +277,8 @@ _hidden int libxl__domain_suspend_common
>                                           libxl_domain_type type,
>                                           int live, int debug);
>  _hidden const char *libxl__device_model_savefile(libxl__gc *gc, uint32_t domid);
> +_hidden int libxl__domain_suspend_device_model(libxl__gc *gc, uint32_t domid);
> +_hidden int libxl__domain_resume_device_model(libxl__gc *gc, uint32_t domid);
>  _hidden int libxl__domain_save_device_model(libxl__gc *gc, uint32_t domid, int fd);
>  _hidden void libxl__userdata_destroyall(libxl__gc *gc, uint32_t domid);
>  
> @@ -620,6 +622,10 @@ _hidden int libxl__qmp_query_serial(libx
>  _hidden int libxl__qmp_pci_add(libxl__gc *gc, int d, libxl_device_pci *pcidev);
>  _hidden int libxl__qmp_pci_del(libxl__gc *gc, int domid,
>                                 libxl_device_pci *pcidev);
> +/* Suspend QEMU. */
> +_hidden int libxl__qmp_stop(libxl__gc *gc, int domid);
> +/* Resume QEMU. */
> +_hidden int libxl__qmp_resume(libxl__gc *gc, int domid);
>  /* Save current QEMU state into fd. */
>  _hidden int libxl__qmp_save(libxl__gc *gc, int domid, const char *filename);
>  /* close and free the QMP handler */
> diff -r 636da26c40d3 -r 329b3c94c618 tools/libxl/libxl_qmp.c
> --- a/tools/libxl/libxl_qmp.c	Thu Feb 02 22:46:33 2012 -0800
> +++ b/tools/libxl/libxl_qmp.c	Thu Feb 02 22:46:33 2012 -0800
> @@ -802,6 +802,38 @@ out:
>      return rc;
>  }
>  
> +int libxl__qmp_stop(libxl__gc *gc, int domid)
> +{
> +    libxl__qmp_handler *qmp = NULL;
> +    int rc = 0;
> +
> +    qmp = libxl__qmp_initialize(libxl__gc_owner(gc), domid);
> +    if (!qmp)
> +        return ERROR_FAIL;
> +
> +    rc = qmp_synchronous_send(qmp, "stop", NULL,
> +                              NULL, NULL, qmp->timeout);
> +
> +    libxl__qmp_close(qmp);
> +    return rc;
> +}
> +
> +int libxl__qmp_resume(libxl__gc *gc, int domid)
> +{
> +    libxl__qmp_handler *qmp = NULL;
> +    int rc = 0;
> +
> +    qmp = libxl__qmp_initialize(libxl__gc_owner(gc), domid);
> +    if (!qmp)
> +        return ERROR_FAIL;
> +
> +    rc = qmp_synchronous_send(qmp, "cont", NULL,
> +                              NULL, NULL, qmp->timeout);
> +
> +    libxl__qmp_close(qmp);
> +    return rc;
> +}
> +
>  int libxl__qmp_initializations(libxl_ctx *ctx, uint32_t domid)
>  {
>      libxl__qmp_handler *qmp = NULL;

Re: [PATCH 4 of 6 V3] libxl: support suspend_cancel in domain_resume

[Ian Campbell]

On Fri, 2012-02-03 at 06:50 +0000, rshriram@cs.ubc.ca wrote:
> # HG changeset patch
> # User Shriram Rajagopalan <rshriram@cs.ubc.ca>
> # Date 1328251593 28800
> # Node ID f853c88f0230a2e9d2e1006a9cd220c4cd27e74d
> # Parent  329b3c94c618addb1e802cebc7fe23b12b432398
> libxl: support suspend_cancel in domain_resume
> 
> Add an extra parameter to libxl_domain_resume indicating
> if the caller wishes to use the SUSPEND_CANCEL style
> resume instead of the normal resume.
> 
> Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
> 
> diff -r 329b3c94c618 -r f853c88f0230 tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c	Thu Feb 02 22:46:33 2012 -0800
> +++ b/tools/libxl/libxl.c	Thu Feb 02 22:46:33 2012 -0800
> @@ -229,24 +229,29 @@ int libxl_domain_rename(libxl_ctx *ctx, 
>      return rc;
>  }
>  
> -int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid)
> +int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid, int suspend_cancel)
>  {
>      GC_INIT(ctx);
>      int rc = 0;
>  
> -    if (LIBXL__DOMAIN_IS_TYPE(gc,  domid, HVM)) {
> -        LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Called domain_resume on "
> -                "non-cooperative hvm domain %u", domid);
> -        rc = ERROR_NI;
> -        goto out;
> -    }
> -    if (xc_domain_resume(ctx->xch, domid, 0)) {
> +    if (xc_domain_resume(ctx->xch, domid, suspend_cancel)) {
>          LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
>                          "xc_domain_resume failed for domain %u",
>                          domid);
>          rc = ERROR_FAIL;
>          goto out;
>      }
> +
> +    if (LIBXL__DOMAIN_IS_TYPE(gc,  domid, HVM)) {
> +        rc = libxl__domain_resume_device_model(gc, domid);
> +        if (rc) {
> +            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> +                       "failed to resume device model for domain %u:%d",
> +                       domid, rc);
> +            goto out;
> +        }
> +    }
> +
>      if (!xs_resume_domain(ctx->xsh, domid)) {
>          LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
>                          "xs_resume_domain failed for domain %u",
> diff -r 329b3c94c618 -r f853c88f0230 tools/libxl/libxl.h
> --- a/tools/libxl/libxl.h	Thu Feb 02 22:46:33 2012 -0800
> +++ b/tools/libxl/libxl.h	Thu Feb 02 22:46:33 2012 -0800
> @@ -268,7 +268,12 @@ int libxl_domain_create_restore(libxl_ct
>  void libxl_domain_config_dispose(libxl_domain_config *d_config);
>  int libxl_domain_suspend(libxl_ctx *ctx, libxl_domain_suspend_info *info,
>                            uint32_t domid, int fd);
> -int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid);
> +
> +/* @param suspend_cancel [from xenctrl.h:xc_domain_resume( @param fast )]
> + *   If this parameter is true, use co-operative resume. The guest
> + *   must support this.
> + */
> +int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid, int suspend_cancel);
>  int libxl_domain_shutdown(libxl_ctx *ctx, uint32_t domid);
>  int libxl_domain_reboot(libxl_ctx *ctx, uint32_t domid);
>  int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid);
> diff -r 329b3c94c618 -r f853c88f0230 tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c	Thu Feb 02 22:46:33 2012 -0800
> +++ b/tools/libxl/xl_cmdimpl.c	Thu Feb 02 22:46:33 2012 -0800
> @@ -2751,7 +2751,7 @@ static void migrate_domain(const char *d
>          if (common_domname) {
>              libxl_domain_rename(ctx, domid, away_domname, common_domname);
>          }
> -        rc = libxl_domain_resume(ctx, domid);
> +        rc = libxl_domain_resume(ctx, domid, 1);

Previously this code would have been equivalent to passing 0 not 1. It
may be ok to change this but it should be separate. However I'm a bit
dubious about changing it without adding some code to detect if the
guest supports it. (I know libxl_domain_resume is currently broken for
the non-cooperative suspend case but we shouldn't paper over that)

Ian.

Re: [PATCH 5 of 6 V3] libxl: refactor migrate_domain and generalize migrate_receive

[Ian Campbell]

On Fri, 2012-02-03 at 06:50 +0000, rshriram@cs.ubc.ca wrote:
> # HG changeset patch
> # User Shriram Rajagopalan <rshriram@cs.ubc.ca>
> # Date 1328251593 28800
> # Node ID 62c4fd2fe9bbc2c283e3998d852317a48e9f9770
> # Parent  f853c88f0230a2e9d2e1006a9cd220c4cd27e74d
> libxl: refactor migrate_domain and generalize migrate_receive
> 
> Refactor some tasks like establishing the migration channel,
> initial migration protocol exchange into separate functions,
> to facilitate re-use, when remus support is introduced. Also,
> make migrate_receive generic (instead of resorting to stdin and
> stdout as the file descriptors for communication).
> 
> Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> 
> diff -r f853c88f0230 -r 62c4fd2fe9bb tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c	Thu Feb 02 22:46:33 2012 -0800
> +++ b/tools/libxl/xl_cmdimpl.c	Thu Feb 02 22:46:33 2012 -0800
> @@ -2531,6 +2531,43 @@ static int save_domain(const char *p, co
>      exit(0);
>  }
>  
> +static pid_t create_migration_child(const char *rune, int *send_fd,
> +                                        int *recv_fd)
> +{
> +    int sendpipe[2], recvpipe[2];
> +    pid_t child = -1;
> +
> +    if (!rune || !send_fd || !recv_fd)
> +        return -1;
> +
> +    MUST( libxl_pipe(ctx, sendpipe) );
> +    MUST( libxl_pipe(ctx, recvpipe) );
> +
> +    child = libxl_fork(ctx);
> +    if (child==-1) exit(1);
> +
> +    if (!child) {
> +        dup2(sendpipe[0], 0);
> +        dup2(recvpipe[1], 1);
> +        close(sendpipe[0]); close(sendpipe[1]);
> +        close(recvpipe[0]); close(recvpipe[1]);
> +        execlp("sh","sh","-c",rune,(char*)0);
> +        perror("failed to exec sh");
> +        exit(-1);
> +    }
> +
> +    close(sendpipe[0]);
> +    close(recvpipe[1]);
> +    *send_fd = sendpipe[1];
> +    *recv_fd = recvpipe[0];
> +
> +    /* if receiver dies, we get an error and can clean up
> +       rather than just dying */
> +    signal(SIGPIPE, SIG_IGN);
> +
> +    return child;
> +}
> +
>  static int migrate_read_fixedmessage(int fd, const void *msg, int msgsz,
>                                       const char *what, const char *rune) {
>      char buf[msgsz];
> @@ -2616,53 +2653,17 @@ static void migration_child_report(pid_t
>      migration_child = 0;
>  }
>  
> -static void migrate_domain(const char *domain_spec, const char *rune,
> -                           const char *override_config_file)
> +static void migrate_do_preamble(int send_fd, int recv_fd, pid_t child,
> +                                uint8_t *config_data, int config_len,
> +                                const char *rune)
>  {
> -    pid_t child = -1;
> -    int rc;
> -    int sendpipe[2], recvpipe[2];
> -    int send_fd, recv_fd;
> -    libxl_domain_suspend_info suspinfo;
> -    char *away_domname;
> -    char rc_buf;
> -    uint8_t *config_data;
> -    int config_len;
> -
> -    save_domain_core_begin(domain_spec, override_config_file,
> -                           &config_data, &config_len);
> -
> -    if (!config_len) {
> -        fprintf(stderr, "No config file stored for running domain and "
> -                "none supplied - cannot migrate.\n");
> +    int rc = 0;
> +
> +    if (send_fd < 0 || recv_fd < 0) {
> +        fprintf(stderr, "migrate_do_preamble: invalid file descriptors\n");
>          exit(1);
>      }
>  
> -    MUST( libxl_pipe(ctx, sendpipe) );
> -    MUST( libxl_pipe(ctx, recvpipe) );
> -
> -    child = libxl_fork(ctx);
> -    if (child==-1) exit(1);
> -
> -    if (!child) {
> -        dup2(sendpipe[0], 0);
> -        dup2(recvpipe[1], 1);
> -        close(sendpipe[0]); close(sendpipe[1]);
> -        close(recvpipe[0]); close(recvpipe[1]);
> -        execlp("sh","sh","-c",rune,(char*)0);
> -        perror("failed to exec sh");
> -        exit(-1);
> -    }
> -
> -    close(sendpipe[0]);
> -    close(recvpipe[1]);
> -    send_fd = sendpipe[1];
> -    recv_fd = recvpipe[0];
> -
> -    signal(SIGPIPE, SIG_IGN);
> -    /* if receiver dies, we get an error and can clean up
> -       rather than just dying */
> -
>      rc = migrate_read_fixedmessage(recv_fd, migrate_receiver_banner,
>                                     sizeof(migrate_receiver_banner)-1,
>                                     "banner", rune);
> @@ -2675,6 +2676,34 @@ static void migrate_domain(const char *d
>      save_domain_core_writeconfig(send_fd, "migration stream",
>                                   config_data, config_len);
>  
> +}
> +
> +static void migrate_domain(const char *domain_spec, const char *rune,
> +                           const char *override_config_file)
> +{
> +    pid_t child = -1;
> +    int rc;
> +    int send_fd = -1, recv_fd = -1;
> +    libxl_domain_suspend_info suspinfo;
> +    char *away_domname;
> +    char rc_buf;
> +    uint8_t *config_data;
> +    int config_len;
> +
> +    save_domain_core_begin(domain_spec, override_config_file,
> +                           &config_data, &config_len);
> +
> +    if (!config_len) {
> +        fprintf(stderr, "No config file stored for running domain and "
> +                "none supplied - cannot migrate.\n");
> +        exit(1);
> +    }
> +
> +    child = create_migration_child(rune, &send_fd, &recv_fd);
> +
> +    migrate_do_preamble(send_fd, recv_fd, child, config_data, config_len,
> +                        rune);
> +
>      xtl_stdiostream_adjust_flags(logger, XTL_STDIOSTREAM_HIDE_PROGRESS, 0);
>  
>      memset(&suspinfo, 0, sizeof(suspinfo));
> @@ -2798,7 +2827,8 @@ static void core_dump_domain(const char 
>      if (rc) { fprintf(stderr,"core dump failed (rc=%d)\n",rc);exit(-1); }
>  }
>  
> -static void migrate_receive(int debug, int daemonize, int monitor)
> +static void migrate_receive(int debug, int daemonize, int monitor,
> +                            int send_fd, int recv_fd)
>  {
>      int rc, rc2;
>      char rc_buf;
> @@ -2810,7 +2840,7 @@ static void migrate_receive(int debug, i
>  
>      fprintf(stderr, "migration target: Ready to receive domain.\n");
>  
> -    CHK_ERRNO( libxl_write_exactly(ctx, 1,
> +    CHK_ERRNO( libxl_write_exactly(ctx, send_fd,
>                                     migrate_receiver_banner,
>                                     sizeof(migrate_receiver_banner)-1,
>                                     "migration ack stream",
> @@ -2822,7 +2852,7 @@ static void migrate_receive(int debug, i
>      dom_info.monitor = monitor;
>      dom_info.paused = 1;
>      dom_info.restore_file = "incoming migration stream";
> -    dom_info.migrate_fd = 0; /* stdin */
> +    dom_info.migrate_fd = recv_fd;
>      dom_info.migration_domname_r = &migration_domname;
>      dom_info.no_incr_generationid = 1;
>  
> @@ -2836,13 +2866,13 @@ static void migrate_receive(int debug, i
>      fprintf(stderr, "migration target: Transfer complete,"
>              " requesting permission to start domain.\n");
>  
> -    rc = libxl_write_exactly(ctx, 1,
> +    rc = libxl_write_exactly(ctx, send_fd,
>                               migrate_receiver_ready,
>                               sizeof(migrate_receiver_ready),
>                               "migration ack stream", "ready message");
>      if (rc) exit(-rc);
>  
> -    rc = migrate_read_fixedmessage(0, migrate_permission_to_go,
> +    rc = migrate_read_fixedmessage(recv_fd, migrate_permission_to_go,
>                                     sizeof(migrate_permission_to_go),
>                                     "GO message", 0);
>      if (rc) goto perhaps_destroy_notify_rc;
> @@ -2861,7 +2891,7 @@ static void migrate_receive(int debug, i
>      rc = 0;
>  
>   perhaps_destroy_notify_rc:
> -    rc2 = libxl_write_exactly(ctx, 1,
> +    rc2 = libxl_write_exactly(ctx, send_fd,
>                                migrate_report, sizeof(migrate_report),
>                                "migration ack stream",
>                                "success/failure report");
> @@ -2869,7 +2899,7 @@ static void migrate_receive(int debug, i
>  
>      rc_buf = -rc;
>      assert(!!rc_buf == !!rc);
> -    rc2 = libxl_write_exactly(ctx, 1, &rc_buf, 1,
> +    rc2 = libxl_write_exactly(ctx, send_fd, &rc_buf, 1,
>                                "migration ack stream",
>                                "success/failure code");
>      if (rc2) exit(-ERROR_BADFAIL);
> @@ -2887,7 +2917,7 @@ static void migrate_receive(int debug, i
>          fprintf(stderr, "migration target: Cleanup OK, granting sender"
>                  " permission to resume.\n");
>  
> -        rc2 = libxl_write_exactly(ctx, 1,
> +        rc2 = libxl_write_exactly(ctx, send_fd,
>                                    migrate_permission_to_go,
>                                    sizeof(migrate_permission_to_go),
>                                    "migration ack stream",
> @@ -2983,7 +3013,9 @@ int main_migrate_receive(int argc, char 
>          help("migrate-receive");
>          return 2;
>      }
> -    migrate_receive(debug, daemonize, monitor);
> +    migrate_receive(debug, daemonize, monitor,
> +                    STDOUT_FILENO, STDIN_FILENO);
> +
>      return 0;
>  }
>  

Re: [PATCH 6 of 6 V3] libxl: resume instead of unpause on xl save -c

[Ian Campbell]

On Fri, 2012-02-03 at 06:50 +0000, rshriram@cs.ubc.ca wrote:
> # HG changeset patch
> # User Shriram Rajagopalan <rshriram@cs.ubc.ca>
> # Date 1328251593 28800
> # Node ID c7abecc14cceb18140335ebe20faad826282cd1f
> # Parent  62c4fd2fe9bbc2c283e3998d852317a48e9f9770
> libxl: resume instead of unpause on xl save -c
> 
> The guest is "suspended" via libxl_domain_suspend when taking a snapshot.
> So call libxl_domain_resume instead of libxl_domain_unpause, when taking
> a checkpoint of the domain (using xl save -c).
> 
> Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>

Ideally we would confirm that the guest supported checkpointing before
even attempting a "save -c" but lets assume for now that the user has
confirmed by out-of-band means that this is the case.

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> 
> diff -r 62c4fd2fe9bb -r c7abecc14cce tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c	Thu Feb 02 22:46:33 2012 -0800
> +++ b/tools/libxl/xl_cmdimpl.c	Thu Feb 02 22:46:33 2012 -0800
> @@ -2524,7 +2524,7 @@ static int save_domain(const char *p, co
>      close(fd);
>  
>      if (checkpoint)
> -        libxl_domain_unpause(ctx, domid);
> +        libxl_domain_resume(ctx, domid, 1);
>      else
>          libxl_domain_destroy(ctx, domid);
>  

Re: [PATCH 4 of 6 V3] libxl: support suspend_cancel in domain_resume

[Ian Campbell]

On Thu, 2012-02-09 at 09:16 +0000, Ian Campbell wrote:
> On Fri, 2012-02-03 at 06:50 +0000, rshriram@cs.ubc.ca wrote:
> > # HG changeset patch
> > # User Shriram Rajagopalan <rshriram@cs.ubc.ca>
> > # Date 1328251593 28800
> > # Node ID f853c88f0230a2e9d2e1006a9cd220c4cd27e74d
> > # Parent  329b3c94c618addb1e802cebc7fe23b12b432398
> > libxl: support suspend_cancel in domain_resume
> > 
> > Add an extra parameter to libxl_domain_resume indicating
> > if the caller wishes to use the SUSPEND_CANCEL style
> > resume instead of the normal resume.
> > 
> > Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
> > 
> > diff -r 329b3c94c618 -r f853c88f0230 tools/libxl/libxl.c
> > --- a/tools/libxl/libxl.c	Thu Feb 02 22:46:33 2012 -0800
> > +++ b/tools/libxl/libxl.c	Thu Feb 02 22:46:33 2012 -0800
> > @@ -229,24 +229,29 @@ int libxl_domain_rename(libxl_ctx *ctx, 
> >      return rc;
> >  }
> >  
> > -int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid)
> > +int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid, int suspend_cancel)
> >  {
> >      GC_INIT(ctx);
> >      int rc = 0;
> >  
> > -    if (LIBXL__DOMAIN_IS_TYPE(gc,  domid, HVM)) {
> > -        LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Called domain_resume on "
> > -                "non-cooperative hvm domain %u", domid);
> > -        rc = ERROR_NI;
> > -        goto out;
> > -    }
> > -    if (xc_domain_resume(ctx->xch, domid, 0)) {
> > +    if (xc_domain_resume(ctx->xch, domid, suspend_cancel)) {
> >          LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
> >                          "xc_domain_resume failed for domain %u",
> >                          domid);
> >          rc = ERROR_FAIL;
> >          goto out;
> >      }
> > +
> > +    if (LIBXL__DOMAIN_IS_TYPE(gc,  domid, HVM)) {
> > +        rc = libxl__domain_resume_device_model(gc, domid);
> > +        if (rc) {
> > +            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> > +                       "failed to resume device model for domain %u:%d",
> > +                       domid, rc);
> > +            goto out;
> > +        }
> > +    }
> > +
> >      if (!xs_resume_domain(ctx->xsh, domid)) {
> >          LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
> >                          "xs_resume_domain failed for domain %u",
> > diff -r 329b3c94c618 -r f853c88f0230 tools/libxl/libxl.h
> > --- a/tools/libxl/libxl.h	Thu Feb 02 22:46:33 2012 -0800
> > +++ b/tools/libxl/libxl.h	Thu Feb 02 22:46:33 2012 -0800
> > @@ -268,7 +268,12 @@ int libxl_domain_create_restore(libxl_ct
> >  void libxl_domain_config_dispose(libxl_domain_config *d_config);
> >  int libxl_domain_suspend(libxl_ctx *ctx, libxl_domain_suspend_info *info,
> >                            uint32_t domid, int fd);
> > -int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid);
> > +
> > +/* @param suspend_cancel [from xenctrl.h:xc_domain_resume( @param fast )]
> > + *   If this parameter is true, use co-operative resume. The guest
> > + *   must support this.
> > + */
> > +int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid, int suspend_cancel);
> >  int libxl_domain_shutdown(libxl_ctx *ctx, uint32_t domid);
> >  int libxl_domain_reboot(libxl_ctx *ctx, uint32_t domid);
> >  int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid);
> > diff -r 329b3c94c618 -r f853c88f0230 tools/libxl/xl_cmdimpl.c
> > --- a/tools/libxl/xl_cmdimpl.c	Thu Feb 02 22:46:33 2012 -0800
> > +++ b/tools/libxl/xl_cmdimpl.c	Thu Feb 02 22:46:33 2012 -0800
> > @@ -2751,7 +2751,7 @@ static void migrate_domain(const char *d
> >          if (common_domname) {
> >              libxl_domain_rename(ctx, domid, away_domname, common_domname);
> >          }
> > -        rc = libxl_domain_resume(ctx, domid);
> > +        rc = libxl_domain_resume(ctx, domid, 1);
> 
> Previously this code would have been equivalent to passing 0 not 1. It
> may be ok to change this but it should be separate. However I'm a bit
> dubious about changing it without adding some code to detect if the
> guest supports it. (I know libxl_domain_resume is currently broken for
> the non-cooperative suspend case but we shouldn't paper over that)

BTW the rest of the patch looked fine. If those two 1s had been 0s I
would have: Acked-by: Ian Campbell <ian.campbell@citrix.com>

Ian.

Re: [PATCH 4 of 6 V3] libxl: support suspend_cancel in domain_resume

[Shriram Rajagopalan]

On 2012-02-09, at 1:16 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:

> On Fri, 2012-02-03 at 06:50 +0000, rshriram@cs.ubc.ca wrote:
>> # 
> 
> Previously this code would have been equivalent to passing 0 not 1. It
> may be ok to change this but it should be separate.

> However I'm a bit
> dubious about changing it without adding some code to detect if the
> guest supports it.

It requires writing the suspend-cancel entry to xenstore on domain creation (if xen says that the guest supports it)

 And then read this field once during initialization in checkpoint/Remus code.

I haven't properly investigated the domain create code path. Any pointers on where I should start ?

> (I know libxl_domain_resume is currently broken for
> the non-cooperative suspend

It's broken in such a way that neither domain checkpoint or Remus work.
I ll take up your comment on the previous version of the patch:
 Add a new public (or internal) API 
 libxl_domain_suspend_cancel

and make the checkpoint and Remus related code call this instead of the domain_resume. 

Later, after adding the suspend-cancel xenstore entry creation patch, I can switch the code to using the above function with the cooperative flag.

> case but we shouldn't paper over that)
> 
> Ian.
> 
> 

Re: [PATCH 4 of 6 V3] libxl: support suspend_cancel in domain_resume

[Ian Campbell]

On Thu, 2012-02-09 at 18:21 +0000, Shriram Rajagopalan wrote:
> On 2012-02-09, at 1:16 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> 
> > On Fri, 2012-02-03 at 06:50 +0000, rshriram@cs.ubc.ca wrote:
> >> # 
> > 
> > Previously this code would have been equivalent to passing 0 not 1. It
> > may be ok to change this but it should be separate.
> 
> > However I'm a bit
> > dubious about changing it without adding some code to detect if the
> > guest supports it.
> 
> It requires writing the suspend-cancel entry to xenstore on domain creation (if xen says that the guest supports it)
> 
>  And then read this field once during initialization in checkpoint/Remus code.
> 
> I haven't properly investigated the domain create code path. Any pointers on where I should start ?
> 
> > (I know libxl_domain_resume is currently broken for
> > the non-cooperative suspend
> 
> It's broken in such a way that neither domain checkpoint or Remus work.
> I ll take up your comment on the previous version of the patch:
>  Add a new public (or internal) API 
>  libxl_domain_suspend_cancel
> 
> and make the checkpoint and Remus related code call this instead of the domain_resume.

I don't think there is any need for that unless you think that is
actually a better API in its own right. Otherwise please just leave the
semantics of the existing calls to libxl_domain_resume alone when you
add the flag (i.e. pass 0 as the co-operative flag at all existing call
sites, retaining the existing behaviour). Other than that your patch was
fine.

Changing existing places to use co-operative resume without also
validating that the guest supports it is wrong in the normal migration
case and also masks a real bug in libxl_domain_resume which needs to be
fixed (it's on my TODO list) not papered over by assuming that all
guests know about cooperative resume.

If you need to make that argument conditional on Remus in subsequent
patches then that is fine since Remus implies, at least to some degree,
support for co-operative resume.

Ian.

> 
> Later, after adding the suspend-cancel xenstore entry creation patch, I can switch the code to using the above function with the cooperative flag.
> 
> > case but we shouldn't paper over that)
> > 
> > Ian.
> > 
> > 

[PATCH 4 of 6 V4] libxl: support suspend_cancel in domain_resume

[rshriram]

# HG changeset patch
# User Shriram Rajagopalan <rshriram@cs.ubc.ca>
# Date 1328835221 28800
# Node ID 47366457a52076b78c5218c95b87c4ba7c9a723a
# Parent  2fadbad1577ed4222dd2cf8931b79f514619eb0d
libxl: support suspend_cancel in domain_resume

Add an extra parameter to libxl_domain_resume indicating
if the caller wishes to use the SUSPEND_CANCEL style
resume instead of the normal resume.

Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -298,24 +298,29 @@ int libxl_domain_rename(libxl_ctx *ctx, 
     return rc;
 }
 
-int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid)
+int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid, int suspend_cancel)
 {
     GC_INIT(ctx);
     int rc = 0;
 
-    if (LIBXL__DOMAIN_IS_TYPE(gc,  domid, HVM)) {
-        LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Called domain_resume on "
-                "non-cooperative hvm domain %u", domid);
-        rc = ERROR_NI;
-        goto out;
-    }
-    if (xc_domain_resume(ctx->xch, domid, 0)) {
+    if (xc_domain_resume(ctx->xch, domid, suspend_cancel)) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
                         "xc_domain_resume failed for domain %u",
                         domid);
         rc = ERROR_FAIL;
         goto out;
     }
+
+    if (LIBXL__DOMAIN_IS_TYPE(gc,  domid, HVM)) {
+        rc = libxl__domain_resume_device_model(gc, domid);
+        if (rc) {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                       "failed to resume device model for domain %u:%d",
+                       domid, rc);
+            goto out;
+        }
+    }
+
     if (!xs_resume_domain(ctx->xsh, domid)) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
                         "xs_resume_domain failed for domain %u",
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -325,7 +325,12 @@ int libxl_domain_create_restore(libxl_ct
 void libxl_domain_config_dispose(libxl_domain_config *d_config);
 int libxl_domain_suspend(libxl_ctx *ctx, libxl_domain_suspend_info *info,
                           uint32_t domid, int fd);
-int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid);
+
+/* @param suspend_cancel [from xenctrl.h:xc_domain_resume( @param fast )]
+ *   If this parameter is true, use co-operative resume. The guest
+ *   must support this.
+ */
+int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid, int suspend_cancel);
 int libxl_domain_shutdown(libxl_ctx *ctx, uint32_t domid);
 int libxl_domain_reboot(libxl_ctx *ctx, uint32_t domid);
 int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid);
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -2802,7 +2802,7 @@ static void migrate_domain(const char *d
         if (common_domname) {
             libxl_domain_rename(ctx, domid, away_domname, common_domname);
         }
-        rc = libxl_domain_resume(ctx, domid);
+        rc = libxl_domain_resume(ctx, domid, 0);
         if (!rc) fprintf(stderr, "migration sender: Resumed OK.\n");
 
         fprintf(stderr, "Migration failed due to problems at target.\n");
@@ -2824,7 +2824,7 @@ static void migrate_domain(const char *d
     close(send_fd);
     migration_child_report(child, recv_fd);
     fprintf(stderr, "Migration failed, resuming at sender.\n");
-    libxl_domain_resume(ctx, domid);
+    libxl_domain_resume(ctx, domid, 0);
     exit(-ERROR_FAIL);
 
  failed_badly:

Re: [PATCH 4 of 6 V4] libxl: support suspend_cancel in domain_resume

[Ian Campbell]

On Fri, 2012-02-10 at 01:31 +0000, rshriram@cs.ubc.ca wrote:
> # HG changeset patch
> # User Shriram Rajagopalan <rshriram@cs.ubc.ca>
> # Date 1328835221 28800
> # Node ID 47366457a52076b78c5218c95b87c4ba7c9a723a
> # Parent  2fadbad1577ed4222dd2cf8931b79f514619eb0d
> libxl: support suspend_cancel in domain_resume
> 
> Add an extra parameter to libxl_domain_resume indicating
> if the caller wishes to use the SUSPEND_CANCEL style
> resume instead of the normal resume.
> 
> Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>

Thanks
Acked-by: Ian Campbell <ian.campbell@citrix.com>

(although doesn't the libxl__domain_resume_device_model need to be in
another patch to avoid bisection issues? i.e. in with the changes to the
suspend/resume ordering to balance things there?)

> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -298,24 +298,29 @@ int libxl_domain_rename(libxl_ctx *ctx, 
>      return rc;
>  }
>  
> -int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid)
> +int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid, int suspend_cancel)
>  {
>      GC_INIT(ctx);
>      int rc = 0;
>  
> -    if (LIBXL__DOMAIN_IS_TYPE(gc,  domid, HVM)) {
> -        LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Called domain_resume on "
> -                "non-cooperative hvm domain %u", domid);
> -        rc = ERROR_NI;
> -        goto out;
> -    }
> -    if (xc_domain_resume(ctx->xch, domid, 0)) {
> +    if (xc_domain_resume(ctx->xch, domid, suspend_cancel)) {
>          LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
>                          "xc_domain_resume failed for domain %u",
>                          domid);
>          rc = ERROR_FAIL;
>          goto out;
>      }
> +
> +    if (LIBXL__DOMAIN_IS_TYPE(gc,  domid, HVM)) {
> +        rc = libxl__domain_resume_device_model(gc, domid);
> +        if (rc) {
> +            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> +                       "failed to resume device model for domain %u:%d",
> +                       domid, rc);
> +            goto out;
> +        }
> +    }
> +
>      if (!xs_resume_domain(ctx->xsh, domid)) {
>          LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
>                          "xs_resume_domain failed for domain %u",
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -325,7 +325,12 @@ int libxl_domain_create_restore(libxl_ct
>  void libxl_domain_config_dispose(libxl_domain_config *d_config);
>  int libxl_domain_suspend(libxl_ctx *ctx, libxl_domain_suspend_info *info,
>                            uint32_t domid, int fd);
> -int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid);
> +
> +/* @param suspend_cancel [from xenctrl.h:xc_domain_resume( @param fast )]
> + *   If this parameter is true, use co-operative resume. The guest
> + *   must support this.
> + */
> +int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid, int suspend_cancel);
>  int libxl_domain_shutdown(libxl_ctx *ctx, uint32_t domid);
>  int libxl_domain_reboot(libxl_ctx *ctx, uint32_t domid);
>  int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid);
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -2802,7 +2802,7 @@ static void migrate_domain(const char *d
>          if (common_domname) {
>              libxl_domain_rename(ctx, domid, away_domname, common_domname);
>          }
> -        rc = libxl_domain_resume(ctx, domid);
> +        rc = libxl_domain_resume(ctx, domid, 0);
>          if (!rc) fprintf(stderr, "migration sender: Resumed OK.\n");
>  
>          fprintf(stderr, "Migration failed due to problems at target.\n");
> @@ -2824,7 +2824,7 @@ static void migrate_domain(const char *d
>      close(send_fd);
>      migration_child_report(child, recv_fd);
>      fprintf(stderr, "Migration failed, resuming at sender.\n");
> -    libxl_domain_resume(ctx, domid);
> +    libxl_domain_resume(ctx, domid, 0);
>      exit(-ERROR_FAIL);
>  
>   failed_badly:

Re: [PATCH 4 of 6 V4] libxl: support suspend_cancel in domain_resume

[Ian Jackson]

rshriram@cs.ubc.ca writes ("[Xen-devel] [PATCH 4 of 6 V4] libxl: support suspend_cancel in domain_resume"):
> libxl: support suspend_cancel in domain_resume

I get this:

libxl.c: In function 'libxl_domain_resume':
libxl.c:315: error: implicit declaration of function 'libxl__domain_resume_device_model'

Did you compile this ?  Did you test it ?

Thanks,
Ian.

Re: [PATCH 4 of 6 V4] libxl: support suspend_cancel in domain_resume

[Shriram Rajagopalan]

[-- Attachment #1.1: Type: text/plain, Size: 1458 bytes --]

On Mon, Feb 20, 2012 at 11:00 AM, Ian Jackson <Ian.Jackson@eu.citrix.com>wrote:

> rshriram@cs.ubc.ca writes ("[Xen-devel] [PATCH 4 of 6 V4] libxl: support
> suspend_cancel in domain_resume"):
> > libxl: support suspend_cancel in domain_resume
>
> I get this:
>
> libxl.c: In function 'libxl_domain_resume':
> libxl.c:315: error: implicit declaration of function
> 'libxl__domain_resume_device_model'
>
> Did you compile this ?  Did you test it ?
>
>
Yes I did.. Did you apply the previous patch in this series, that
introduces the above (missing)  function ?

As I mentioned in the introductory email, these patches also depend on
Stefano's XC_SAVE_ID_TOOLSTACK patches.

01 Introduce a new save_id to save/restore toolstack specific extra
02 Read Qemu's physmap from xenstore and save it using toolstack_save.
03 Following the recent changes to upstream Qemu, the best monitor command
    (removes qmp_migrate and introduces qmp_save)


Then my patches:
03 libxl: QMP stop/resume & refactor QEMU suspend/resume/save
04 libxl: support suspend_cancel in domain_resume
05 libxl: refactor migrate_domain and generalize migrate_receive
06 libxl: resume instead of unpause on xl save -c

01 libxl: Remus - suspend/postflush/commit callbacks
02 libxl: Remus - xl remus command

Currently, one of stefano's patches doesnt apply cleanly - the first one.
It requires minor fix.
Barring that, all the other patches mentioned above apply cleanly.


Thanks,
> Ian.
>
>

[-- Attachment #1.2: Type: text/html, Size: 2071 bytes --]

[-- Attachment #2: Type: text/plain, Size: 132 bytes --]

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

Re: [PATCH 4 of 6 V4] libxl: support suspend_cancel in domain_resume

[Shriram Rajagopalan]

[-- Attachment #1.1: Type: text/plain, Size: 1719 bytes --]

On Mon, Feb 20, 2012 at 12:02 PM, Shriram Rajagopalan <rshriram@cs.ubc.ca>wrote:

> On Mon, Feb 20, 2012 at 11:00 AM, Ian Jackson <Ian.Jackson@eu.citrix.com>wrote:
>
>> rshriram@cs.ubc.ca writes ("[Xen-devel] [PATCH 4 of 6 V4] libxl: support
>> suspend_cancel in domain_resume"):
>> > libxl: support suspend_cancel in domain_resume
>>
>> I get this:
>>
>> libxl.c: In function 'libxl_domain_resume':
>> libxl.c:315: error: implicit declaration of function
>> 'libxl__domain_resume_device_model'
>>
>> Did you compile this ?  Did you test it ?
>>
>>
> Yes I did.. Did you apply the previous patch in this series, that
> introduces the above (missing)  function ?
>
> As I mentioned in the introductory email, these patches also depend on
> Stefano's XC_SAVE_ID_TOOLSTACK patches.
>
> 01 Introduce a new save_id to save/restore toolstack specific extra
> 02 Read Qemu's physmap from xenstore and save it using toolstack_save.
> 03 Following the recent changes to upstream Qemu, the best monitor command
>     (removes qmp_migrate and introduces qmp_save)
>
>
Stefano, you said a while ago, that the above patches were blocked on some
qemu related acks.
Any updates ?


>
> Then my patches:
> 03 libxl: QMP stop/resume & refactor QEMU suspend/resume/save
> 04 libxl: support suspend_cancel in domain_resume
> 05 libxl: refactor migrate_domain and generalize migrate_receive
> 06 libxl: resume instead of unpause on xl save -c
>
> 01 libxl: Remus - suspend/postflush/commit callbacks
> 02 libxl: Remus - xl remus command
>
> Currently, one of stefano's patches doesnt apply cleanly - the first one.
> It requires minor fix.
> Barring that, all the other patches mentioned above apply cleanly.
>
>
> Thanks,
>> Ian.
>>
>>
>

[-- Attachment #1.2: Type: text/html, Size: 2753 bytes --]

[-- Attachment #2: Type: text/plain, Size: 132 bytes --]

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

Re: [PATCH 4 of 6 V4] libxl: support suspend_cancel in domain_resume

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 1591 bytes --]

On Mon, 20 Feb 2012, Shriram Rajagopalan wrote:
> On Mon, Feb 20, 2012 at 11:00 AM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
>       rshriram@cs.ubc.ca writes ("[Xen-devel] [PATCH 4 of 6 V4] libxl: support suspend_cancel in domain_resume"):
>       > libxl: support suspend_cancel in domain_resume
> 
> I get this:
> 
> libxl.c: In function 'libxl_domain_resume':
> libxl.c:315: error: implicit declaration of function 'libxl__domain_resume_device_model'
> 
> Did you compile this ?  Did you test it ?
> 
> 
> Yes I did.. Did you apply the previous patch in this series, that introduces the above (missing)  function ?
> 
> As I mentioned in the introductory email, these patches also depend on Stefano's XC_SAVE_ID_TOOLSTACK patches.
> 
> 01 Introduce a new save_id to save/restore toolstack specific extra
> 02 Read Qemu's physmap from xenstore and save it using toolstack_save.
> 03 Following the recent changes to upstream Qemu, the best monitor command
>     (removes qmp_migrate and introduces qmp_save)
> 
> 
> Then my patches:
> 03 libxl: QMP stop/resume & refactor QEMU suspend/resume/save
> 04 libxl: support suspend_cancel in domain_resume
> 05 libxl: refactor migrate_domain and generalize migrate_receive
> 06 libxl: resume instead of unpause on xl save -c
> 
> 01 libxl: Remus - suspend/postflush/commit callbacks
> 02 libxl: Remus - xl remus command
> 
> Currently, one of stefano's patches doesnt apply cleanly - the first one. It requires minor fix.
> Barring that, all the other patches mentioned above apply cleanly.
> 

I can rebase and resend if it is needed.

[-- Attachment #2: Type: text/plain, Size: 132 bytes --]

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

Re: [PATCH 4 of 6 V4] libxl: support suspend_cancel in domain_resume

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 1325 bytes --]

On Mon, 20 Feb 2012, Shriram Rajagopalan wrote:
> On Mon, Feb 20, 2012 at 12:02 PM, Shriram Rajagopalan <rshriram@cs.ubc.ca> wrote:
>       On Mon, Feb 20, 2012 at 11:00 AM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
>             rshriram@cs.ubc.ca writes ("[Xen-devel] [PATCH 4 of 6 V4] libxl: support suspend_cancel in
>             domain_resume"):
>             > libxl: support suspend_cancel in domain_resume
> 
> I get this:
> 
> libxl.c: In function 'libxl_domain_resume':
> libxl.c:315: error: implicit declaration of function 'libxl__domain_resume_device_model'
> 
> Did you compile this ?  Did you test it ?
> 
> 
> Yes I did.. Did you apply the previous patch in this series, that introduces the above (missing)  function ?
> 
> As I mentioned in the introductory email, these patches also depend on Stefano's XC_SAVE_ID_TOOLSTACK patches.
> 
> 01 Introduce a new save_id to save/restore toolstack specific extra
> 02 Read Qemu's physmap from xenstore and save it using toolstack_save.
> 03 Following the recent changes to upstream Qemu, the best monitor command
>     (removes qmp_migrate and introduces qmp_save)
> 
> 
> Stefano, you said a while ago, that the above patches were blocked on some qemu related acks.
> Any updates ?

None unfortunately. I'll try to get an answer by the end of the week.

[-- Attachment #2: Type: text/plain, Size: 132 bytes --]

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

Re: [PATCH 4 of 6 V4] libxl: support suspend_cancel in domain_resume

[Ian Jackson]

Shriram Rajagopalan writes ("Re: [Xen-devel] [PATCH 4 of 6 V4] libxl: support suspend_cancel in domain_resume"):
> As I mentioned in the introductory email, these patches also depend on Stefano's XC_SAVE_ID_TOOLSTACK patches.

Sorry, I didn't spot that.

Ian.