[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH 3/5] 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.

* 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.

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@xxxxxxxxxxxxx>
---
 tools/libxl/Makefile             |    1 +
 tools/libxl/libxl_create.c       |  245 ++++++++++++++++++++++++--------------
 tools/libxl/libxl_dom.c          |   45 +-------
 tools/libxl/libxl_internal.h     |   19 +++-
 tools/libxl/libxl_save_callout.c |   43 +++++++
 5 files changed, 213 insertions(+), 140 deletions(-)
 create mode 100644 tools/libxl/libxl_save_callout.c

diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 5d9227e..9b84421 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -66,6 +66,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 14721eb..7f42737 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -20,7 +20,6 @@
 #include "libxl_internal.h"
 
 #include <xc_dom.h>
-#include <xenguest.h>
 
 void libxl_domain_config_init(libxl_domain_config *d_config)
 {
@@ -311,89 +310,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*) info->u.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 (info->u.pv.ramdisk.path) {
-            vments[i++] = "image/ramdisk";
-            vments[i++] = (char*) info->u.pv.ramdisk.path;
-        }
-        if (info->u.pv.cmdline) {
-            vments[i++] = "image/cmdline";
-            vments[i++] = (char*) info->u.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(&info->u.pv.kernel);
-        libxl__file_reference_unmap(&info->u.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)
 {
@@ -574,10 +490,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,
@@ -660,20 +579,19 @@ 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;
 
-    if (ret) goto error_out;
+    if (rc) domcreate_rebuild_done(egc, dcs, rc);
 
     /* We might be going to call libxl__spawn_local_dm, or _spawn_stub_dm.
      * Fill in any field required by either, including both relevant
@@ -684,12 +602,155 @@ 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;
+    xc_toolstack_restore_cb *toolstack_restore = NULL;
+    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);
+        toolstack_restore = libxl__toolstack_restore;
+        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,
+                             toolstack_restore);
+    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 restore_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*) info->u.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 (info->u.pv.ramdisk.path) {
+            vments[i++] = "image/ramdisk";
+            vments[i++] = (char*) info->u.pv.ramdisk.path;
+        }
+        if (info->u.pv.cmdline) {
+            vments[i++] = "image/cmdline";
+            vments[i++] = (char*) info->u.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(&info->u.pv.kernel);
+        libxl__file_reference_unmap(&info->u.pv.ramdisk);
+    }
+
+    esave = errno;
+
+    flags = fcntl(restore_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(restore_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 0dc1427..d29c705 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>
 
@@ -461,7 +460,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;
@@ -516,48 +515,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;
-    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 b9afeb8..fa70a00 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"
 
@@ -751,10 +752,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_save_device_model(libxl__gc *gc, uint32_t domid, int 
fd);
 _hidden void libxl__userdata_destroyall(libxl__gc *gc, uint32_t domid);
@@ -1821,6 +1820,18 @@ _hidden int libxl__domain_suspend_common(libxl__gc *gc, 
uint32_t domid, int fd,
                                          libxl_domain_type type,
                                          int live, int debug);
 
+/* 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,
+                                      xc_toolstack_restore_cb*);
+/* 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 0000000..bad4e30
--- /dev/null
+++ b/tools/libxl/libxl_save_callout.c
@@ -0,0 +1,43 @@
+/*
+ * 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,
+                              xc_toolstack_restore_cb *toolstack_restore)
+/* calls libxl__xc_domain_restore_done when done */
+{
+    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;
+
+    struct restore_callbacks callbacks;
+    callbacks.toolstack_restore = toolstack_restore;
+    callbacks.data = gc;
+
+    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, &callbacks);
+    libxl__xc_domain_restore_done(egc, dcs, 0, r, errno);
+}
-- 
1.7.2.5


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.