libxl: domain restore: reshuffle, preparing for ao

We are going to arrange that libxl, instead of calling
xc_domain_restore, calls a stub function which forks and execs a
helper program, so that restore can be asynchronous rather than
blocking the whole toolstack.

This stub function will be called libxl__xc_domain_restore.

However, its prospective call site is unsuitable for a function which
needs to make a callback, and is buried in two nested single-call-site
functions which are logically part of the domain creation procedure.

So we first abolish those single-call-site functions, integrate their
contents into domain creation in their proper temporal order, and
break out libxl__xc_domain_restore ready for its reimplementation.

No functional change - just the following reorganisation:

* Abolish libxl__domain_restore_common, as it had only one caller.
  Move its contents into (what was) domain_restore.

* There is a new stage function domcreate_rebuild_done containing what
  used to be the bulk of domcreate_bootloader_done, since
  domcreate_bootloader_done now simply starts the restore (or does the
  rebuild) and arranges to call the next stage.

* Move the contents of domain_restore into its correct place in the
  domain creation sequence.  We put it inside
  domcreate_bootloader_done, which now either calls
  libxl__xc_domain_restore which will call the new function
  domcreate_rebuild_done, or calls domcreate_rebuild_done directly.

* Various general-purpose local variables (`i' etc.) and convenience
  alias variables need to be shuffled about accordingly.

* Consequently libxl__toolstack_restore needs to gain external linkage
  as it is now in a different file to its user.

* Move the xc_domain_save callbacks struct from the stack into
  libxl__domain_create_state.

In general the moved code remains almost identical.  Two returns in
what used to be libxl__domain_restore_common have been changed to set
the return value and "goto out", and the call sites for the abolished
and new functions have been adjusted.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

5 files changed, 206 insertions, 139 deletions

diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index e7d5cc2638..1d8b80affe 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -67,6 +67,7 @@ LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o libxl_pci.o \
 			libxl_dom.o libxl_exec.o libxl_xshelp.o libxl_device.o \
 			libxl_internal.o libxl_utils.o libxl_uuid.o \
 			libxl_json.o libxl_aoutils.o \
+			libxl_save_callout.o \
 			libxl_qmp.o libxl_event.o libxl_fork.o $(LIBXL_OBJS-y)
 LIBXL_OBJS += _libxl_types.o libxl_flask.o _libxl_types_internal.o
 
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 4599e733b1..36236275c1 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -377,89 +377,6 @@ out:
     return ret;
 }
 
-static int domain_restore(libxl__gc *gc, libxl_domain_build_info *info,
-                          uint32_t domid, int fd,
-                          libxl__domain_build_state *state)
-{
-    libxl_ctx *ctx = libxl__gc_owner(gc);
-    char **vments = NULL, **localents = NULL;
-    struct timeval start_time;
-    int i, ret, esave, flags;
-
-    ret = libxl__build_pre(gc, domid, info, state);
-    if (ret)
-        goto out;
-
-    ret = libxl__domain_restore_common(gc, domid, info, state, fd);
-    if (ret)
-        goto out;
-
-    gettimeofday(&start_time, NULL);
-
-    switch (info->type) {
-    case LIBXL_DOMAIN_TYPE_HVM:
-        vments = libxl__calloc(gc, 7, sizeof(char *));
-        vments[0] = "rtc/timeoffset";
-        vments[1] = (info->u.hvm.timeoffset) ? info->u.hvm.timeoffset : "";
-        vments[2] = "image/ostype";
-        vments[3] = "hvm";
-        vments[4] = "start_time";
-        vments[5] = libxl__sprintf(gc, "%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
-        break;
-    case LIBXL_DOMAIN_TYPE_PV:
-        vments = libxl__calloc(gc, 11, sizeof(char *));
-        i = 0;
-        vments[i++] = "image/ostype";
-        vments[i++] = "linux";
-        vments[i++] = "image/kernel";
-        vments[i++] = (char *) state->pv_kernel.path;
-        vments[i++] = "start_time";
-        vments[i++] = libxl__sprintf(gc, "%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
-        if (state->pv_ramdisk.path) {
-            vments[i++] = "image/ramdisk";
-            vments[i++] = (char *) state->pv_ramdisk.path;
-        }
-        if (state->pv_cmdline) {
-            vments[i++] = "image/cmdline";
-            vments[i++] = (char *) state->pv_cmdline;
-        }
-        break;
-    default:
-        ret = ERROR_INVAL;
-        goto out;
-    }
-    ret = libxl__build_post(gc, domid, info, state, vments, localents);
-    if (ret)
-        goto out;
-
-    if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
-        ret = asprintf(&state->saved_state,
-                       XC_DEVICE_MODEL_RESTORE_FILE".%d", domid);
-        ret = (ret < 0) ? ERROR_FAIL : 0;
-    }
-
-out:
-    if (info->type == LIBXL_DOMAIN_TYPE_PV) {
-        libxl__file_reference_unmap(&state->pv_kernel);
-        libxl__file_reference_unmap(&state->pv_ramdisk);
-    }
-
-    esave = errno;
-
-    flags = fcntl(fd, F_GETFL);
-    if (flags == -1) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unable to get flags on restore fd");
-    } else {
-        flags &= ~O_NONBLOCK;
-        if (fcntl(fd, F_SETFL, flags) == -1)
-            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unable to put restore fd"
-                         " back to blocking mode");
-    }
-
-    errno = esave;
-    return ret;
-}
-
 int libxl__domain_make(libxl__gc *gc, libxl_domain_create_info *info,
                        uint32_t *domid)
 {
@@ -640,10 +557,13 @@ static void domcreate_bootloader_console_available(libxl__egc *egc,
 static void domcreate_bootloader_done(libxl__egc *egc,
                                       libxl__bootloader_state *bl,
                                       int rc);
-
 static void domcreate_console_available(libxl__egc *egc,
                                         libxl__domain_create_state *dcs);
 
+static void domcreate_rebuild_done(libxl__egc *egc,
+                                   libxl__domain_create_state *dcs,
+                                   int ret);
+
 /* Our own function to clean up and call the user's callback.
  * The final call in the sequence. */
 static void domcreate_complete(libxl__egc *egc,
@@ -737,20 +657,20 @@ static void domcreate_console_available(libxl__egc *egc,
 
 static void domcreate_bootloader_done(libxl__egc *egc,
                                       libxl__bootloader_state *bl,
-                                      int ret)
+                                      int rc)
 {
     libxl__domain_create_state *dcs = CONTAINER_OF(bl, *dcs, bl);
     STATE_AO_GC(bl->ao);
-    int i;
 
     /* convenience aliases */
     const uint32_t domid = dcs->guest_domid;
     libxl_domain_config *const d_config = dcs->guest_config;
+    libxl_domain_build_info *const info = &d_config->b_info;
     const int restore_fd = dcs->restore_fd;
     libxl__domain_build_state *const state = &dcs->build_state;
-    libxl_ctx *const ctx = CTX;
+    struct restore_callbacks *const callbacks = &dcs->callbacks;
 
-    if (ret) goto error_out;
+    if (rc) domcreate_rebuild_done(egc, dcs, rc);
 
     /* consume bootloader outputs. state->pv_{kernel,ramdisk} have
      * been initialised by the bootloader already.
@@ -766,12 +686,153 @@ static void domcreate_bootloader_done(libxl__egc *egc,
     dcs->dmss.dm.callback = domcreate_devmodel_started;
     dcs->dmss.callback = domcreate_devmodel_started;
 
-    if ( restore_fd >= 0 ) {
-        ret = domain_restore(gc, &d_config->b_info, domid, restore_fd, state);
+    if ( restore_fd < 0 ) {
+        rc = libxl__domain_build(gc, &d_config->b_info, domid, state);
+        domcreate_rebuild_done(egc, dcs, rc);
+        return;
+    }
+
+    /* Restore */
+
+    rc = libxl__build_pre(gc, domid, info, state);
+    if (rc)
+        goto out;
+
+    /* read signature */
+    int hvm, pae, superpages;
+    int no_incr_generationid;
+    switch (info->type) {
+    case LIBXL_DOMAIN_TYPE_HVM:
+        hvm = 1;
+        superpages = 1;
+        pae = libxl_defbool_val(info->u.hvm.pae);
+        no_incr_generationid = !libxl_defbool_val(info->u.hvm.incr_generationid);
+        callbacks->toolstack_restore = libxl__toolstack_restore;
+        callbacks->data = gc;
+        break;
+    case LIBXL_DOMAIN_TYPE_PV:
+        hvm = 0;
+        superpages = 0;
+        pae = 1;
+        no_incr_generationid = 0;
+        break;
+    default:
+        rc = ERROR_INVAL;
+        goto out;
+    }
+    libxl__xc_domain_restore(egc, dcs,
+                             hvm, pae, superpages, no_incr_generationid);
+    return;
+
+ out:
+    libxl__xc_domain_restore_done(egc, dcs, rc, 0, 0);
+}
+
+void libxl__xc_domain_restore_done(libxl__egc *egc,
+                                   libxl__domain_create_state *dcs,
+                                   int ret, int retval, int errnoval)
+{
+    STATE_AO_GC(dcs->ao);
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    char **vments = NULL, **localents = NULL;
+    struct timeval start_time;
+    int i, esave, flags;
+
+    /* convenience aliases */
+    const uint32_t domid = dcs->guest_domid;
+    libxl_domain_config *const d_config = dcs->guest_config;
+    libxl_domain_build_info *const info = &d_config->b_info;
+    libxl__domain_build_state *const state = &dcs->build_state;
+    const int fd = dcs->restore_fd;
+
+    if (ret)
+        goto out;
+
+    if (retval) {
+        LOGEV(ERROR, errnoval, "restoring domain");
+        ret = ERROR_FAIL;
+        goto out;
+    }
+
+    gettimeofday(&start_time, NULL);
+
+    switch (info->type) {
+    case LIBXL_DOMAIN_TYPE_HVM:
+        vments = libxl__calloc(gc, 7, sizeof(char *));
+        vments[0] = "rtc/timeoffset";
+        vments[1] = (info->u.hvm.timeoffset) ? info->u.hvm.timeoffset : "";
+        vments[2] = "image/ostype";
+        vments[3] = "hvm";
+        vments[4] = "start_time";
+        vments[5] = libxl__sprintf(gc, "%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
+        break;
+    case LIBXL_DOMAIN_TYPE_PV:
+        vments = libxl__calloc(gc, 11, sizeof(char *));
+        i = 0;
+        vments[i++] = "image/ostype";
+        vments[i++] = "linux";
+        vments[i++] = "image/kernel";
+        vments[i++] = (char *) state->pv_kernel.path;
+        vments[i++] = "start_time";
+        vments[i++] = libxl__sprintf(gc, "%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
+        if (state->pv_ramdisk.path) {
+            vments[i++] = "image/ramdisk";
+            vments[i++] = (char *) state->pv_ramdisk.path;
+        }
+        if (state->pv_cmdline) {
+            vments[i++] = "image/cmdline";
+            vments[i++] = (char *) state->pv_cmdline;
+        }
+        break;
+    default:
+        ret = ERROR_INVAL;
+        goto out;
+    }
+    ret = libxl__build_post(gc, domid, info, state, vments, localents);
+    if (ret)
+        goto out;
+
+    if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
+        ret = asprintf(&state->saved_state,
+                       XC_DEVICE_MODEL_RESTORE_FILE".%d", domid);
+        ret = (ret < 0) ? ERROR_FAIL : 0;
+    }
+
+out:
+    if (info->type == LIBXL_DOMAIN_TYPE_PV) {
+        libxl__file_reference_unmap(&state->pv_kernel);
+        libxl__file_reference_unmap(&state->pv_ramdisk);
+    }
+
+    esave = errno;
+
+    flags = fcntl(fd, F_GETFL);
+    if (flags == -1) {
+        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unable to get flags on restore fd");
     } else {
-        ret = libxl__domain_build(gc, &d_config->b_info, domid, state);
+        flags &= ~O_NONBLOCK;
+        if (fcntl(fd, F_SETFL, flags) == -1)
+            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unable to put restore fd"
+                         " back to blocking mode");
     }
 
+    errno = esave;
+    domcreate_rebuild_done(egc, dcs, ret);
+}
+
+static void domcreate_rebuild_done(libxl__egc *egc,
+                                   libxl__domain_create_state *dcs,
+                                   int ret)
+{
+    STATE_AO_GC(dcs->ao);
+    int i;
+
+    /* convenience aliases */
+    const uint32_t domid = dcs->guest_domid;
+    libxl_domain_config *const d_config = dcs->guest_config;
+    libxl__domain_build_state *const state = &dcs->build_state;
+    libxl_ctx *const ctx = CTX;
+
     if (ret) {
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot (re-)build domain: %d", ret);
         ret = ERROR_FAIL;
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 812c001b34..3bf1e26eda 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -19,7 +19,6 @@
 
 #include <xenctrl.h>
 #include <xc_dom.h>
-#include <xenguest.h>
 
 #include <xen/hvm/hvm_info_table.h>
 
@@ -469,7 +468,7 @@ static inline char *restore_helper(libxl__gc *gc, uint32_t domid,
             domid, phys_offset, node);
 }
 
-static int libxl__toolstack_restore(uint32_t domid, const uint8_t *buf,
+int libxl__toolstack_restore(uint32_t domid, const uint8_t *buf,
         uint32_t size, void *data)
 {
     libxl__gc *gc = data;
@@ -524,48 +523,6 @@ static int libxl__toolstack_restore(uint32_t domid, const uint8_t *buf,
     return 0;
 }
 
-int libxl__domain_restore_common(libxl__gc *gc, uint32_t domid,
-                                 libxl_domain_build_info *info,
-                                 libxl__domain_build_state *state,
-                                 int fd)
-{
-    libxl_ctx *ctx = libxl__gc_owner(gc);
-    /* read signature */
-    int rc;
-    int hvm, pae, superpages;
-    struct restore_callbacks callbacks[1];
-    int no_incr_generationid;
-    switch (info->type) {
-    case LIBXL_DOMAIN_TYPE_HVM:
-        hvm = 1;
-        superpages = 1;
-        pae = libxl_defbool_val(info->u.hvm.pae);
-        no_incr_generationid = !libxl_defbool_val(info->u.hvm.incr_generationid);
-        callbacks->toolstack_restore = libxl__toolstack_restore;
-        callbacks->data = gc;
-        break;
-    case LIBXL_DOMAIN_TYPE_PV:
-        hvm = 0;
-        superpages = 0;
-        pae = 1;
-        no_incr_generationid = 0;
-        break;
-    default:
-        return ERROR_INVAL;
-    }
-    rc = xc_domain_restore(ctx->xch, fd, domid,
-                           state->store_port, &state->store_mfn,
-                           state->store_domid, state->console_port,
-                           &state->console_mfn, state->console_domid,
-                           hvm, pae, superpages, no_incr_generationid,
-                           &state->vm_generationid_addr, callbacks);
-    if ( rc ) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "restoring domain");
-        return ERROR_FAIL;
-    }
-    return 0;
-}
-
 static int libxl__domain_suspend_common_switch_qemu_logdirty
                                (int domid, unsigned int enable, void *data)
 {
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 37e0d098da..7a3b7f8722 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -46,6 +46,7 @@
 
 #include <xenstore.h>
 #include <xenctrl.h>
+#include <xenguest.h>
 
 #include "xentoollog.h"
 
@@ -782,10 +783,8 @@ _hidden int libxl__domain_rename(libxl__gc *gc, uint32_t domid,
                                  const char *old_name, const char *new_name,
                                  xs_transaction_t trans);
 
-_hidden int libxl__domain_restore_common(libxl__gc *gc, uint32_t domid,
-                                         libxl_domain_build_info *info,
-                                         libxl__domain_build_state *state,
-                                         int fd);
+_hidden int libxl__toolstack_restore(uint32_t domid, const uint8_t *buf,
+                                     uint32_t size, void *data);
 _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);
@@ -1899,6 +1898,7 @@ struct libxl__domain_create_state {
     libxl__stub_dm_spawn_state dmss;
         /* If we're not doing stubdom, we use only dmss.dm,
          * for the non-stubdom device model. */
+    struct restore_callbacks callbacks;
 };
 
 /*----- Domain suspend (save) functions -----*/
@@ -1908,6 +1908,17 @@ _hidden int libxl__domain_suspend_common(libxl__gc *gc, uint32_t domid, int fd,
                                          int live, int debug,
                                          const libxl_domain_remus_info *r_info);
 
+/* calls libxl__xc_domain_restore_done when done */
+_hidden void libxl__xc_domain_restore(libxl__egc *egc,
+                                      libxl__domain_create_state *dcs,
+                                      int hvm, int pae, int superpages,
+                                      int no_incr_generationid);
+/* If rc==0 then retval is the return value from xc_domain_save
+ * and errnoval is the errno value it provided.
+ * If rc!=0, retval and errnoval are undefined. */
+_hidden void libxl__xc_domain_restore_done(libxl__egc *egc,
+                                           libxl__domain_create_state *dcs,
+                                           int rc, int retval, int errnoval);
 
 
 /*
diff --git a/tools/libxl/libxl_save_callout.c b/tools/libxl/libxl_save_callout.c
new file mode 100644
index 0000000000..2f8db9f0b6
--- /dev/null
+++ b/tools/libxl/libxl_save_callout.c
@@ -0,0 +1,37 @@
+/*
+ * Copyright (C) 2012      Citrix Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ */
+
+#include "libxl_osdeps.h"
+
+#include "libxl_internal.h"
+
+void libxl__xc_domain_restore(libxl__egc *egc, libxl__domain_create_state *dcs,
+                              int hvm, int pae, int superpages,
+                              int no_incr_generationid)
+{
+    STATE_AO_GC(dcs->ao);
+
+    /* Convenience aliases */
+    const uint32_t domid = dcs->guest_domid;
+    const int restore_fd = dcs->restore_fd;
+    libxl__domain_build_state *const state = &dcs->build_state;
+
+    int r = xc_domain_restore(CTX->xch, restore_fd, domid,
+                              state->store_port, &state->store_mfn,
+                              state->store_domid, state->console_port,
+                              &state->console_mfn, state->console_domid,
+                              hvm, pae, superpages, no_incr_generationid,
+                              &state->vm_generationid_addr, &dcs->callbacks);
+    libxl__xc_domain_restore_done(egc, dcs, 0, r, errno);
+}