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

[Xen-changelog] [xen-unstable] libxl: ao: convert libxl__spawn_*


  • To: xen-changelog@xxxxxxxxxxxxxxxxxxx
  • From: Xen patchbot-unstable <patchbot@xxxxxxx>
  • Date: Mon, 14 May 2012 16:32:41 +0000
  • Delivery-date: Mon, 14 May 2012 16:33:14 +0000
  • List-id: "Change log for Mercurial \(receive only\)" <xen-changelog.lists.xen.org>

# HG changeset patch
# User Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
# Date 1336759143 -3600
# Node ID 885fdd4fd7ac807a2b0e6a322ab492f0f40d1561
# Parent  9524eb476e930960a579bd9e0219450dd4462527
libxl: ao: convert libxl__spawn_*

libxl__spawn_spawn becomes a callback-style asynchronous function.
The implementation is now in terms of libxl__ev_* including
libxl_ev_child.

All the callers need to be updated.  This includes the device model
spawning functions libxl__create_device_model and
libxl__create_stubdom; these are replaced with libxl__spawn_local_dm
and libxl__spawn_stubdom.  libxl__confirm_device_model_startup is
abolished; instead the dm spawner calls back.

(The choice of which kind of device model to create is lifted out of
what used to be libxl__create_device_model, because that function was
indirectly recursive.  Recursive callback-style operations are clumsy
because they require a pointer indirection for the nested states.)

Waiting for proper device model startup it is no longer notionally
optional.  Previously the code appeared to tolerate this by passing
NULL for various libxl__spawner_starting* parameters to device model
spawners.  However, this was not used anywhere.

Conversely, the "for_spawn" parameter to libxl__wait_for_offspring is
no longer supported.  It remains as an unused formal parameter to
avoid updating, in this patch, all the call sites which pass NULL.
libxl__wait_for_offspring is in any case itself an obsolete function,
so this wrinkle will go away when its callers are updated to use the
event system.  Consequently libxl__spawn_check is also abolished.

The "console ready" callback also remains unchanged in this patch.
The API for this needs to be reviewed in the context of the event
series and its reentrancy restrictions documented.

Thus their callers need to be updated.  These are the domain creation
functions libxl_domain_create_new and _restore.  These functions now
take ao_hows, and have a private state structure.

However domain creation remains not completely converted to the event
mechanism; in particular it runs the outward-facing function
libxl_run_bootloader with a NULL ao_how, which is quite wrong.  As it
happens in the current code this is not a bug because none of the rest
of the functionality surrounding the bootloader call will mind if the
event loop is reentered in the middle of its execution.

The file-scope function libxl__set_fd_flag which was used by the
previous spawn arrangements becomes unused and is removed; other
places in libxl can use libxl_fd_set_nonblock and
libxl_fd_set_cloexec, which of course remain.

Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

Changes since v8:
 * Make midproc_cb callback with correct pid (that of the grandchild,
   not "middle" which is zero).  Reported by Roger Pau Monne.

Changes since v7:
 * Rename libxl__spawn_stubdom to libxl__spawn_stub_dm (and ..._state);
   rename the state's stubdom_* members to dm_*.
 * Eliminate the union between the two dm creation states in
   libxl__domain_create_state.  Instead, the domain creation code
   simply uses libxl__stub_dm_spawn_state.dm directly, if we're
   taking the local dm path.
 * Remove a spurious "break".
 * In domain creation, move the PV non-qemu case into the switch.
 * Code style fixes.
 * Constify some convenience aliases.
 * Improve comments (including typo fixes).
Committed-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
---


diff -r 9524eb476e93 -r 885fdd4fd7ac tools/libxl/libxl.h
--- a/tools/libxl/libxl.h       Fri May 11 18:59:02 2012 +0100
+++ b/tools/libxl/libxl.h       Fri May 11 18:59:03 2012 +0100
@@ -465,8 +465,18 @@ int libxl_ctx_free(libxl_ctx *ctx /* 0 i
 
 /* domain related functions */
 typedef int (*libxl_console_ready)(libxl_ctx *ctx, uint32_t domid, void *priv);
-int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config *d_config, 
libxl_console_ready cb, void *priv, uint32_t *domid);
-int libxl_domain_create_restore(libxl_ctx *ctx, libxl_domain_config *d_config, 
libxl_console_ready cb, void *priv, uint32_t *domid, int restore_fd);
+  /* fixme-ao   Need to review this API.  If we keep it, the reentrancy
+   * properties need to be documented but they may turn out to be too
+   * awkward */
+
+int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config *d_config,
+                            libxl_console_ready cb, void *priv, uint32_t 
*domid,
+                            const libxl_asyncop_how *ao_how);
+int libxl_domain_create_restore(libxl_ctx *ctx, libxl_domain_config *d_config,
+                                libxl_console_ready cb, void *priv,
+                                uint32_t *domid, int restore_fd,
+                                const libxl_asyncop_how *ao_how);
+
 void libxl_domain_config_init(libxl_domain_config *d_config);
 void libxl_domain_config_dispose(libxl_domain_config *d_config);
 int libxl_domain_suspend(libxl_ctx *ctx, libxl_domain_suspend_info *info,
diff -r 9524eb476e93 -r 885fdd4fd7ac tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c        Fri May 11 18:59:02 2012 +0100
+++ b/tools/libxl/libxl_create.c        Fri May 11 18:59:03 2012 +0100
@@ -558,16 +558,40 @@ static int store_libxl_entry(libxl__gc *
         libxl_device_model_version_to_string(b_info->device_model_version));
 }
 
-static int do_domain_create(libxl__gc *gc, libxl_domain_config *d_config,
-                            libxl_console_ready cb, void *priv,
-                            uint32_t *domid_out, int restore_fd)
+/*----- main domain creation -----*/
+
+/* We have a linear control flow; only one event callback is
+ * outstanding at any time.  Each initiation and callback function
+ * arranges for the next to be called, as the very last thing it
+ * does.  (If that particular sub-operation is not needed, a
+ * function will call the next event callback directly.)
+ */
+
+/* Event callbacks, in this order: */
+static void domcreate_devmodel_started(libxl__egc *egc,
+                                       libxl__dm_spawn_state *dmss,
+                                       int rc);
+
+/* 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,
+                               libxl__domain_create_state *dcs,
+                               int rc);
+
+static void initiate_domain_create(libxl__egc *egc,
+                                   libxl__domain_create_state *dcs)
 {
+    STATE_AO_GC(dcs->ao);
     libxl_ctx *ctx = libxl__gc_owner(gc);
-    libxl__spawner_starting *dm_starting = 0;
-    libxl__domain_build_state state[1];
     uint32_t domid;
     int i, ret;
 
+    /* convenience aliases */
+    libxl_domain_config *const d_config = dcs->guest_config;
+    const int restore_fd = dcs->restore_fd;
+    const libxl_console_ready cb = dcs->console_cb;
+    void *const priv = dcs->console_cb_priv;
+
     domid = 0;
 
     ret = libxl__domain_create_info_setdefault(gc, &d_config->c_info);
@@ -580,9 +604,12 @@ static int do_domain_create(libxl__gc *g
         goto error_out;
     }
 
+    dcs->guest_domid = domid;
+    dcs->dmss.dm.guest_domid = 0; /* means we haven't spawned */
+
     if ( d_config->c_info.type == LIBXL_DOMAIN_TYPE_PV && cb ) {
-        if ( (*cb)(ctx, domid, priv) )
-            goto error_out;
+        ret = (*cb)(ctx, domid, priv);
+        if (ret) goto error_out;
     }
 
     ret = libxl__domain_build_info_setdefault(gc, &d_config->b_info);
@@ -606,7 +633,17 @@ static int do_domain_create(libxl__gc *g
         }
     }
 
-    memset(state, 0, sizeof(*state));
+    memset(&dcs->build_state, 0, sizeof(dcs->build_state));
+    libxl__domain_build_state *state = &dcs->build_state;
+
+    /* We might be going to call libxl__spawn_local_dm, or _spawn_stub_dm.
+     * Fill in any field required by either, including both relevant
+     * callbacks (_spawn_stub_dm will overwrite our trespass if needed). */
+    dcs->dmss.dm.spawn.ao = ao;
+    dcs->dmss.dm.guest_config = dcs->guest_config;
+    dcs->dmss.dm.build_state = &dcs->build_state;
+    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);
@@ -656,14 +693,12 @@ static int do_domain_create(libxl__gc *g
         libxl_device_vkb_add(ctx, domid, &vkb);
         libxl_device_vkb_dispose(&vkb);
 
-        ret = libxl__create_device_model(gc, domid, d_config,
-                                         state, &dm_starting);
-        if (ret < 0) {
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
-                       "failed to create device model: %d", ret);
-            goto error_out;
-        }
-        break;
+        dcs->dmss.dm.guest_domid = domid;
+        if (libxl_defbool_val(d_config->b_info.device_model_stubdomain))
+            libxl__spawn_stub_dm(egc, &dcs->dmss);
+        else
+            libxl__spawn_local_dm(egc, &dcs->dmss.dm);
+        return;
     }
     case LIBXL_DOMAIN_TYPE_PV:
     {
@@ -687,26 +722,52 @@ static int do_domain_create(libxl__gc *g
         libxl__device_console_dispose(&console);
 
         if (need_qemu) {
-            libxl__create_xenpv_qemu(gc, domid, d_config, state, &dm_starting);
+            dcs->dmss.dm.guest_domid = domid;
+            libxl__spawn_local_dm(egc, &dcs->dmss.dm);
+            return;
+        } else {
+            assert(!dcs->dmss.dm.guest_domid);
+            domcreate_devmodel_started(egc, &dcs->dmss.dm, 0);
+            return;
         }
-        break;
     }
     default:
         ret = ERROR_INVAL;
         goto error_out;
     }
+    abort(); /* not reached */
 
-    if (dm_starting) {
+ error_out:
+    assert(ret);
+    domcreate_complete(egc, dcs, ret);
+}
+
+static void domcreate_devmodel_started(libxl__egc *egc,
+                                       libxl__dm_spawn_state *dmss,
+                                       int ret)
+{
+    libxl__domain_create_state *dcs = CONTAINER_OF(dmss, *dcs, dmss.dm);
+    STATE_AO_GC(dmss->spawn.ao);
+    int i;
+    libxl_ctx *ctx = CTX;
+    int domid = dcs->guest_domid;
+
+    /* convenience aliases */
+    libxl_domain_config *const d_config = dcs->guest_config;
+    const libxl_console_ready cb = dcs->console_cb;
+    void *const priv = dcs->console_cb_priv;
+
+    if (ret) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                   "device model did not start: %d", ret);
+        goto error_out;
+    }
+
+    if (dcs->dmss.dm.guest_domid) {
         if (d_config->b_info.device_model_version
             == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
             libxl__qmp_initializations(gc, domid, d_config);
         }
-        ret = libxl__confirm_device_model_startup(gc, state, dm_starting);
-        if (ret < 0) {
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
-                       "device model did not start: %d", ret);
-            goto error_out;
-        }
     }
 
     for (i = 0; i < d_config->num_pcidevs; i++)
@@ -734,38 +795,95 @@ static int do_domain_create(libxl__gc *g
     if ( cb && (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM ||
                 (d_config->c_info.type == LIBXL_DOMAIN_TYPE_PV &&
                  d_config->b_info.u.pv.bootloader ))) {
-        if ( (*cb)(ctx, domid, priv) )
-            goto error_out;
+        ret = (*cb)(ctx, domid, priv);
+        if (ret) goto error_out;
     }
 
-    *domid_out = domid;
-    return 0;
+    domcreate_complete(egc, dcs, 0);
+    return;
 
 error_out:
-    if (domid)
-        libxl_domain_destroy(ctx, domid);
-
-    return ret;
+    assert(ret);
+    domcreate_complete(egc, dcs, ret);
 }
 
+static void domcreate_complete(libxl__egc *egc,
+                               libxl__domain_create_state *dcs,
+                               int rc)
+{
+    STATE_AO_GC(dcs->ao);
+
+    if (rc) {
+        if (dcs->guest_domid) {
+            int rc2 = libxl_domain_destroy(CTX, dcs->guest_domid);
+            if (rc2)
+                LOG(ERROR, "unable to destroy domain %d following"
+                    " failed creation", dcs->guest_domid);
+        }
+        dcs->guest_domid = -1;
+    }
+    dcs->callback(egc, dcs, rc, dcs->guest_domid);
+}
+
+/*----- application-facing domain creation interface -----*/
+
+typedef struct {
+    libxl__domain_create_state dcs;
+    uint32_t *domid_out;
+} libxl__app_domain_create_state;
+
+static void domain_create_cb(libxl__egc *egc,
+                             libxl__domain_create_state *dcs,
+                             int rc, uint32_t domid);
+
+static int do_domain_create(libxl_ctx *ctx, libxl_domain_config *d_config,
+                            libxl_console_ready cb, void *priv, uint32_t 
*domid,
+                            int restore_fd, const libxl_asyncop_how *ao_how)
+{
+    AO_CREATE(ctx, 0, ao_how);
+    libxl__app_domain_create_state *cdcs;
+
+    GCNEW(cdcs);
+    cdcs->dcs.ao = ao;
+    cdcs->dcs.guest_config = d_config;
+    cdcs->dcs.restore_fd = restore_fd;
+    cdcs->dcs.console_cb = cb;
+    cdcs->dcs.console_cb_priv = priv;
+    cdcs->dcs.callback = domain_create_cb;
+    cdcs->domid_out = domid;
+
+    initiate_domain_create(egc, &cdcs->dcs);
+
+    return AO_INPROGRESS;
+}
+
+static void domain_create_cb(libxl__egc *egc,
+                             libxl__domain_create_state *dcs,
+                             int rc, uint32_t domid)
+{
+    libxl__app_domain_create_state *cdcs = CONTAINER_OF(dcs, *cdcs, dcs);
+    STATE_AO_GC(cdcs->dcs.ao);
+
+    if (!rc)
+        *cdcs->domid_out = domid;
+
+    libxl__ao_complete(egc, ao, rc);
+}
+    
 int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config *d_config,
-                            libxl_console_ready cb, void *priv, uint32_t 
*domid)
+                            libxl_console_ready cb, void *priv,
+                            uint32_t *domid,
+                            const libxl_asyncop_how *ao_how)
 {
-    GC_INIT(ctx);
-    int rc;
-    rc = do_domain_create(gc, d_config, cb, priv, domid, -1);
-    GC_FREE;
-    return rc;
+    return do_domain_create(ctx, d_config, cb, priv, domid, -1, ao_how);
 }
 
 int libxl_domain_create_restore(libxl_ctx *ctx, libxl_domain_config *d_config,
-                                libxl_console_ready cb, void *priv, uint32_t 
*domid, int restore_fd)
+                                libxl_console_ready cb, void *priv,
+                                uint32_t *domid, int restore_fd,
+                                const libxl_asyncop_how *ao_how)
 {
-    GC_INIT(ctx);
-    int rc;
-    rc = do_domain_create(gc, d_config, cb, priv, domid, restore_fd);
-    GC_FREE;
-    return rc;
+    return do_domain_create(ctx, d_config, cb, priv, domid, restore_fd, 
ao_how);
 }
 
 /*
diff -r 9524eb476e93 -r 885fdd4fd7ac tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c    Fri May 11 18:59:02 2012 +0100
+++ b/tools/libxl/libxl_dm.c    Fri May 11 18:59:03 2012 +0100
@@ -669,24 +669,28 @@ retry_transaction:
     return 0;
 }
 
-static int libxl__create_stubdom(libxl__gc *gc,
-                                 int guest_domid,
-                                 libxl_domain_config *guest_config,
-                                 libxl__domain_build_state *d_state,
-                                 libxl__spawner_starting **starting_r)
+static void spawn_stubdom_pvqemu_cb(libxl__egc *egc,
+                                libxl__dm_spawn_state *stubdom_dmss,
+                                int rc);
+
+void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
 {
+    STATE_AO_GC(sdss->dm.spawn.ao);
     libxl_ctx *ctx = libxl__gc_owner(gc);
     int i, num_console = STUBDOM_SPECIAL_CONSOLES, ret;
     libxl__device_console *console;
-    libxl_domain_config dm_config[1];
     libxl_device_vfb vfb;
     libxl_device_vkb vkb;
-    libxl__domain_build_state stubdom_state[1];
-    uint32_t dm_domid;
     char **args;
     struct xs_permissions perm[2];
     xs_transaction_t t;
-    libxl__spawner_starting *dm_starting = 0;
+
+    /* convenience aliases */
+    libxl_domain_config *const dm_config = &sdss->dm_config;
+    libxl_domain_config *const guest_config = sdss->dm.guest_config;
+    const int guest_domid = sdss->dm.guest_domid;
+    libxl__domain_build_state *const d_state = sdss->dm.build_state;
+    libxl__domain_build_state *const stubdom_state = &sdss->dm_state;
 
     if (guest_config->b_info.device_model_version !=
         LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL) {
@@ -694,6 +698,8 @@ static int libxl__create_stubdom(libxl__
         goto out;
     }
 
+    sdss->pvqemu.guest_domid = 0;
+
     libxl_domain_create_info_init(&dm_config->c_info);
     dm_config->c_info.type = LIBXL_DOMAIN_TYPE_PV;
     dm_config->c_info.name = libxl__sprintf(gc, "%s-dm",
@@ -741,10 +747,10 @@ static int libxl__create_stubdom(libxl__
     dm_config->num_vkbs = 1;
 
     /* fixme: this function can leak the stubdom if it fails */
-    dm_domid = 0;
-    ret = libxl__domain_make(gc, &dm_config->c_info, &dm_domid);
+    ret = libxl__domain_make(gc, &dm_config->c_info, 
&sdss->pvqemu.guest_domid);
     if (ret)
         goto out;
+    uint32_t dm_domid = sdss->pvqemu.guest_domid;
     ret = libxl__domain_build(gc, &dm_config->b_info, dm_domid, stubdom_state);
     if (ret)
         goto out;
@@ -852,42 +858,67 @@ retry_transaction:
             goto out_free;
     }
 
-    if (libxl__create_xenpv_qemu(gc, dm_domid,
-                                 dm_config,
-                                 stubdom_state,
-                                 &dm_starting) < 0) {
-        ret = ERROR_FAIL;
-        goto out_free;
-    }
-    if (libxl__confirm_device_model_startup(gc, d_state, dm_starting) < 0) {
-        ret = ERROR_FAIL;
-        goto out_free;
-    }
+    sdss->pvqemu.guest_domid = dm_domid;
+    sdss->pvqemu.guest_config = &sdss->dm_config;
+    sdss->pvqemu.build_state = &sdss->dm_state;
+    sdss->pvqemu.callback = spawn_stubdom_pvqemu_cb;
 
-    libxl_domain_unpause(ctx, dm_domid);
+    libxl__spawn_local_dm(egc, &sdss->pvqemu);
 
-    if (starting_r) {
-        *starting_r = calloc(1, sizeof(libxl__spawner_starting));
-        (*starting_r)->domid = guest_domid;
-        (*starting_r)->dom_path = libxl__xs_get_dompath(gc, guest_domid);
-        (*starting_r)->for_spawn = NULL;
-    }
-
-    ret = 0;
+    free(args);
+    return;
 
 out_free:
     free(args);
 out:
-    return ret;
+    assert(ret);
+    spawn_stubdom_pvqemu_cb(egc, &sdss->pvqemu, ret);
 }
 
-int libxl__create_device_model(libxl__gc *gc,
-                              int domid,
-                              libxl_domain_config *guest_config,
-                              libxl__domain_build_state *state,
-                              libxl__spawner_starting **starting_r)
+static void spawn_stubdom_pvqemu_cb(libxl__egc *egc,
+                                libxl__dm_spawn_state *stubdom_dmss,
+                                int rc)
 {
-    libxl_ctx *ctx = libxl__gc_owner(gc);
+    libxl__stub_dm_spawn_state *sdss =
+        CONTAINER_OF(stubdom_dmss, *sdss, pvqemu);
+    STATE_AO_GC(sdss->dm.spawn.ao);
+    uint32_t dm_domid = sdss->pvqemu.guest_domid;
+
+    if (rc) goto out;
+
+    rc = libxl_domain_unpause(CTX, dm_domid);
+    if (rc) goto out;
+
+ out:
+    if (rc) {
+        if (dm_domid)
+            libxl_domain_destroy(CTX, dm_domid);
+    }
+    sdss->callback(egc, &sdss->dm, rc);
+}
+
+/* callbacks passed to libxl__spawn_spawn */
+static void device_model_confirm(libxl__egc *egc, libxl__spawn_state *spawn,
+                                 const char *xsdata);
+static void device_model_startup_failed(libxl__egc *egc,
+                                        libxl__spawn_state *spawn);
+
+/* our "next step" function, called from those callbacks and elsewhere */
+static void device_model_spawn_outcome(libxl__egc *egc,
+                                       libxl__dm_spawn_state *dmss,
+                                       int rc);
+
+void libxl__spawn_local_dm(libxl__egc *egc, libxl__dm_spawn_state *dmss)
+{
+    /* convenience aliases */
+    const int domid = dmss->guest_domid;
+    libxl__domain_build_state *const state = dmss->build_state;
+    libxl__spawn_state *const spawn = &dmss->spawn;
+
+    STATE_AO_GC(dmss->spawn.ao);
+
+    libxl_ctx *ctx = CTX;
+    libxl_domain_config *guest_config = dmss->guest_config;
     const libxl_domain_create_info *c_info = &guest_config->c_info;
     const libxl_domain_build_info *b_info = &guest_config->b_info;
     const libxl_vnc_info *vnc = libxl__dm_vnc(guest_config);
@@ -895,15 +926,13 @@ int libxl__create_device_model(libxl__gc
     int logfile_w, null;
     int rc;
     char **args, **arg;
-    libxl__spawner_starting buf_starting, *p;
     xs_transaction_t t;
     char *vm_path;
     char **pass_stuff;
     const char *dm;
 
     if (libxl_defbool_val(b_info->device_model_stubdomain)) {
-        rc = libxl__create_stubdom(gc, domid, guest_config, state, starting_r);
-        goto out;
+        abort();
     }
 
     dm = libxl__domain_device_model(gc, b_info);
@@ -947,25 +976,8 @@ int libxl__create_device_model(libxl__gc
     free(logfile);
     null = open("/dev/null", O_RDONLY);
 
-    if (starting_r) {
-        rc = ERROR_NOMEM;
-        *starting_r = calloc(1, sizeof(libxl__spawner_starting));
-        if (!*starting_r)
-            goto out_close;
-        p = *starting_r;
-        p->for_spawn = calloc(1, sizeof(libxl__spawn_starting));
-    } else {
-        p = &buf_starting;
-        p->for_spawn = NULL;
-    }
-
-    p->domid = domid;
-    p->dom_path = libxl__xs_get_dompath(gc, domid);
-    p->pid_path = "image/device-model-pid";
-    if (!p->dom_path) {
-        rc = ERROR_FAIL;
-        goto out_close;
-    }
+    const char *dom_path = libxl__xs_get_dompath(gc, domid);
+    spawn->pidpath = GCSPRINTF("%s/%s", dom_path, "image/device-model-pid");
 
     if (vnc && vnc->passwd) {
         /* This xenstore key will only be used by qemu-xen-traditionnal.
@@ -973,7 +985,7 @@ int libxl__create_device_model(libxl__gc
 retry_transaction:
         /* Find uuid and the write the vnc password to xenstore for qemu. */
         t = xs_transaction_start(ctx->xsh);
-        vm_path = libxl__xs_read(gc,t,libxl__sprintf(gc, "%s/vm", 
p->dom_path));
+        vm_path = libxl__xs_read(gc,t,libxl__sprintf(gc, "%s/vm", dom_path));
         if (vm_path) {
             /* Now write the vncpassword into it. */
             pass_stuff = libxl__calloc(gc, 3, sizeof(char *));
@@ -990,8 +1002,15 @@ retry_transaction:
     for (arg = args; *arg; arg++)
         LIBXL__LOG(CTX, XTL_DEBUG, "  %s", *arg);
 
-    rc = libxl__spawn_spawn(gc, p->for_spawn, "device model",
-                            libxl_spawner_record_pid, p);
+    spawn->what = GCSPRINTF("domain %d device model", domid);
+    spawn->xspath = GCSPRINTF("/local/domain/0/device-model/%d/state", domid);
+    spawn->timeout_ms = LIBXL_DEVICE_MODEL_START_TIMEOUT * 1000;
+    spawn->pidpath = GCSPRINTF("%s/image/device-model-pid", dom_path);
+    spawn->midproc_cb = libxl__spawn_record_pid;
+    spawn->confirm_cb = device_model_confirm;
+    spawn->failure_cb = device_model_startup_failed;
+
+    rc = libxl__spawn_spawn(egc, spawn);
     if (rc < 0)
         goto out_close;
     if (!rc) { /* inner child */
@@ -1006,30 +1025,61 @@ out_close:
     close(logfile_w);
     free(args);
 out:
-    return rc;
+    if (rc)
+        device_model_spawn_outcome(egc, dmss, rc);
 }
 
 
-int libxl__confirm_device_model_startup(libxl__gc *gc,
-                                libxl__domain_build_state *state,
-                                libxl__spawner_starting *starting)
+static void device_model_confirm(libxl__egc *egc, libxl__spawn_state *spawn,
+                                 const char *xsdata)
 {
-    char *path;
-    int domid = starting->domid;
-    int ret, ret2;
-    path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/state", domid);
-    ret = libxl__spawn_confirm_offspring_startup(gc,
-                                     LIBXL_DEVICE_MODEL_START_TIMEOUT,
-                                     "Device Model", path, "running", 
starting);
+    libxl__dm_spawn_state *dmss = CONTAINER_OF(spawn, *dmss, spawn);
+    STATE_AO_GC(spawn->ao);
+
+    if (!xsdata)
+        return;
+
+    if (strcmp(xsdata, "running"))
+        return;
+
+    libxl__spawn_detach(gc, spawn);
+
+    device_model_spawn_outcome(egc, dmss, 0);
+}
+
+static void device_model_startup_failed(libxl__egc *egc,
+                                        libxl__spawn_state *spawn)
+{
+    libxl__dm_spawn_state *dmss = CONTAINER_OF(spawn, *dmss, spawn);
+    device_model_spawn_outcome(egc, dmss, ERROR_FAIL);
+}
+
+static void device_model_spawn_outcome(libxl__egc *egc,
+                                       libxl__dm_spawn_state *dmss,
+                                       int rc)
+{
+    STATE_AO_GC(dmss->spawn.ao);
+    int ret2;
+
+    if (rc)
+        LOG(ERROR, "%s: spawn failed (rc=%d)", dmss->spawn.what, rc);
+
+    libxl__domain_build_state *state = dmss->build_state;
+
     if (state->saved_state) {
         ret2 = unlink(state->saved_state);
-        if (ret2) LIBXL__LOG_ERRNO(CTX, XTL_ERROR,
-                                   "failed to remove device-model state %s\n",
-                                   state->saved_state);
-        /* Do not clobber spawn_confirm error code with unlink error code. */
-        if (!ret) ret = ret2;
+        if (ret2) {
+            LOGE(ERROR, "%s: failed to remove device-model state %s",
+                 dmss->spawn.what, state->saved_state);
+            rc = ERROR_FAIL;
+            goto out;
+        }
     }
-    return ret;
+
+    rc = 0;
+
+ out:
+    dmss->callback(egc, dmss, rc);
 }
 
 int libxl__destroy_device_model(libxl__gc *gc, uint32_t domid)
@@ -1131,15 +1181,6 @@ out:
     return ret;
 }
 
-int libxl__create_xenpv_qemu(libxl__gc *gc, uint32_t domid,
-                             libxl_domain_config *guest_config,
-                             libxl__domain_build_state *state,
-                             libxl__spawner_starting **starting_r)
-{
-    libxl__create_device_model(gc, domid, guest_config, state, starting_r);
-    return 0;
-}
-
 /*
  * Local variables:
  * mode: C
diff -r 9524eb476e93 -r 885fdd4fd7ac tools/libxl/libxl_exec.c
--- a/tools/libxl/libxl_exec.c  Fri May 11 18:59:02 2012 +0100
+++ b/tools/libxl/libxl_exec.c  Fri May 11 18:59:03 2012 +0100
@@ -127,29 +127,35 @@ void libxl_report_child_exitstatus(libxl
     }
 }
 
-void libxl_spawner_record_pid(void *for_spawn, pid_t innerchild)
+int libxl__spawn_record_pid(libxl__gc *gc, libxl__spawn_state *spawn,
+                            pid_t innerchild)
 {
-    libxl__spawner_starting *starting = for_spawn;
-    struct xs_handle *xsh;
-    char *path = NULL, *pid = NULL;
-    int len;
+    struct xs_handle *xsh = NULL;
+    const char *pid = NULL;
+    int rc, xsok;
 
-    if (asprintf(&path, "%s/%s", starting->dom_path, starting->pid_path) < 0)
-        goto out;
-
-    len = asprintf(&pid, "%d", innerchild);
-    if (len < 0)
-        goto out;
+    pid = GCSPRINTF("%d", innerchild);
 
     /* we mustn't use the parent's handle in the child */
     xsh = xs_daemon_open();
+    if (!xsh) {
+        LOGE(ERROR, "write %s = %s: xenstore reopen failed",
+             spawn->pidpath, pid);
+        rc = ERROR_FAIL;  goto out;
+    }
 
-    xs_write(xsh, XBT_NULL, path, pid, len);
+    xsok = xs_write(xsh, XBT_NULL, spawn->pidpath, pid, strlen(pid));
+    if (!xsok) {
+        LOGE(ERROR,
+             "write %s = %s: xenstore write failed", spawn->pidpath, pid);
+        rc = ERROR_FAIL;  goto out;
+    }
 
-    xs_daemon_close(xsh);
+    rc = 0;
+
 out:
-    free(path);
-    free(pid);
+    if (xsh) xs_daemon_close(xsh);
+    return rc ? SIGTERM : 0;
 }
 
 int libxl__wait_for_offspring(libxl__gc *gc,
@@ -184,19 +190,9 @@ int libxl__wait_for_offspring(libxl__gc 
     tv.tv_sec = timeout;
     tv.tv_usec = 0;
     nfds = xs_fileno(xsh) + 1;
-    if (spawning && spawning->fd > xs_fileno(xsh))
-        nfds = spawning->fd + 1;
+    assert(!spawning);
 
     while (rc > 0 || (!rc && tv.tv_sec > 0)) {
-        if ( spawning ) {
-            rc = libxl__spawn_check(gc, spawning);
-            if ( rc ) {
-                LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
-                           "%s died during startup", what);
-                rc = -1;
-                goto err_died;
-            }
-        }
         p = xs_read(xsh, XBT_NULL, path, &len);
         if ( NULL == p )
             goto again;
@@ -218,8 +214,6 @@ again:
         free(p);
         FD_ZERO(&rfds);
         FD_SET(xs_fileno(xsh), &rfds);
-        if (spawning)
-            FD_SET(spawning->fd, &rfds);
         rc = select(nfds, &rfds, NULL, NULL, &tv);
         if (rc > 0) {
             if (FD_ISSET(xs_fileno(xsh), &rfds)) {
@@ -229,207 +223,215 @@ again:
                 else
                     goto again;
             }
-            if (spawning && FD_ISSET(spawning->fd, &rfds)) {
-                unsigned char dummy;
-                if (read(spawning->fd, &dummy, sizeof(dummy)) != 1)
-                    LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_DEBUG,
-                                     "failed to read spawn status pipe");
-            }
         }
     }
     LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "%s not ready", what);
-err_died:
+
     xs_unwatch(xsh, path, path);
     xs_daemon_close(xsh);
 err:
     return -1;
 }
 
-static int detach_offspring(libxl__gc *gc,
-                               libxl__spawner_starting *starting)
+
+/*----- spawn implementation -----*/
+
+/*
+ * Full set of possible states of a libxl__spawn_state and its _detachable:
+ *
+ *               ss->        ss->        ss->    | ssd->       ssd->
+ *               timeout     xswatch     ssd     |  mid         ss
+ *  - Undefined   undef       undef       no     |  -           -
+ *  - Idle        Idle        Idle        no     |  -           -
+ *  - Active      Active      Active      yes    |  Active      yes
+ *  - Partial     Active/Idle Active/Idle maybe  |  Active/Idle yes  (if 
exists)
+ *  - Detached    -           -           -      |  Active      no
+ *
+ * When in state Detached, the middle process has been sent a SIGKILL.
+ */
+
+/* Event callbacks. */
+static void spawn_watch_event(libxl__egc *egc, libxl__ev_xswatch *xsw,
+                              const char *watch_path, const char *event_path);
+static void spawn_timeout(libxl__egc *egc, libxl__ev_time *ev,
+                          const struct timeval *requested_abs);
+static void spawn_middle_death(libxl__egc *egc, libxl__ev_child *childw,
+                               pid_t pid, int status);
+
+/* Precondition: Partial.  Results: Detached. */
+static void spawn_cleanup(libxl__gc *gc, libxl__spawn_state *ss);
+
+/* Precondition: Partial; caller has logged failure reason.
+ * Results: Caller notified of failure;
+ *  after return, ss may be completely invalid as caller may reuse it */
+static void spawn_failed(libxl__egc *egc, libxl__spawn_state *ss);
+
+void libxl__spawn_init(libxl__spawn_state *ss)
 {
-    int rc;
-    rc = libxl__spawn_detach(gc, starting->for_spawn);
-    if (starting->for_spawn)
-        free(starting->for_spawn);
-    free(starting);
-    return rc;
+    libxl__ev_time_init(&ss->timeout);
+    libxl__ev_xswatch_init(&ss->xswatch);
+    ss->ssd = 0;
 }
 
-int libxl__spawn_confirm_offspring_startup(libxl__gc *gc,
-                                       uint32_t timeout, char *what,
-                                       char *path, char *state,
-                                       libxl__spawner_starting *starting)
+int libxl__spawn_spawn(libxl__egc *egc, libxl__spawn_state *ss)
 {
-    int detach;
-    int problem = libxl__wait_for_offspring(gc, starting->domid, timeout, what,
-                                               path, state,
-                                               starting->for_spawn, NULL, 
NULL);
-    detach = detach_offspring(gc, starting);
-    return problem ? problem : detach;
-}
+    STATE_AO_GC(ss->ao);
+    int r;
+    pid_t child;
+    int status, rc;
 
-static int libxl__set_fd_flag(libxl__gc *gc, int fd, int flag)
-{
-    int flags;
+    libxl__spawn_init(ss);
+    ss->ssd = libxl__zalloc(0, sizeof(*ss->ssd));
+    libxl__ev_child_init(&ss->ssd->mid);
 
-    flags = fcntl(fd, F_GETFL);
-    if (flags == -1)
-        return ERROR_FAIL;
+    rc = libxl__ev_time_register_rel(gc, &ss->timeout,
+                                     spawn_timeout, ss->timeout_ms);
+    if (rc) goto out_err;
 
-    flags |= flag;
+    rc = libxl__ev_xswatch_register(gc, &ss->xswatch,
+                                    spawn_watch_event, ss->xspath);
+    if (rc) goto out_err;
 
-    if (fcntl(fd, F_SETFL, flags) == -1)
-        return ERROR_FAIL;
+    pid_t middle = libxl__ev_child_fork(gc, &ss->ssd->mid, spawn_middle_death);
+    if (middle ==-1) { rc = ERROR_FAIL; goto out_err; }
 
-    return 0;
-}
-
-int libxl__spawn_spawn(libxl__gc *gc,
-                      libxl__spawn_starting *for_spawn,
-                      const char *what,
-                      void (*intermediate_hook)(void *for_spawn,
-                                                pid_t innerchild),
-                      void *hook_data)
-{
-    libxl_ctx *ctx = libxl__gc_owner(gc);
-    pid_t child, got;
-    int status, rc;
-    pid_t intermediate;
-    int pipes[2];
-    unsigned char dummy = 0;
-
-    if (for_spawn) {
-        for_spawn->what = strdup(what);
-        if (!for_spawn->what) return ERROR_NOMEM;
-
-        if (libxl_pipe(ctx, pipes) < 0)
-            goto err_parent;
-        if (libxl__set_fd_flag(gc, pipes[0], O_NONBLOCK) < 0 ||
-            libxl__set_fd_flag(gc, pipes[1], O_NONBLOCK) < 0)
-            goto err_parent_pipes;
-    }
-
-    intermediate = libxl_fork(ctx);
-    if (intermediate ==-1)
-        goto err_parent_pipes;
-
-    if (intermediate) {
+    if (middle) {
         /* parent */
-        if (for_spawn) {
-            for_spawn->intermediate = intermediate;
-            for_spawn->fd = pipes[0];
-            close(pipes[1]);
-        }
         return 1;
     }
 
-    /* we are now the intermediate process */
-    if (for_spawn) close(pipes[0]);
+    /* we are now the middle process */
 
     child = fork();
     if (child == -1)
         exit(255);
     if (!child) {
-        if (for_spawn) close(pipes[1]);
         return 0; /* caller runs child code */
     }
 
-    intermediate_hook(hook_data, child);
-
-    if (!for_spawn) _exit(0); /* just detach then */
-
-    got = waitpid(child, &status, 0);
-    assert(got == child);
-
-    rc = (WIFEXITED(status) ? WEXITSTATUS(status) :
-          WIFSIGNALED(status) && WTERMSIG(status) < 127
-          ? WTERMSIG(status)+128 : -1);
-    if (for_spawn) {
-        if (write(pipes[1], &dummy, sizeof(dummy)) != 1)
-            perror("libxl__spawn_spawn: unable to signal child exit to 
parent");
-    }
-    _exit(rc);
-
- err_parent_pipes:
-    if (for_spawn) {
-        close(pipes[0]);
-        close(pipes[1]);
+    int failsig = ss->midproc_cb(gc, ss, child);
+    if (failsig) {
+        kill(child, failsig);
+        _exit(127);
     }
 
- err_parent:
-    if (for_spawn) free(for_spawn->what);
+    for (;;) {
+        pid_t got = waitpid(child, &status, 0);
+        if (got == -1) {
+            assert(errno == EINTR);
+            continue;
+        }
+        assert(got == child);
+        break;
+    }
 
-    return ERROR_FAIL;
+    r = (WIFEXITED(status) && WEXITSTATUS(status) <= 127 ? WEXITSTATUS(status) 
:
+         WIFSIGNALED(status) && WTERMSIG(status) < 127 ? WTERMSIG(status)+128 :
+         -1);
+    _exit(r);
+
+ out_err:
+    spawn_cleanup(gc, ss);
+    return rc;
 }
 
-static void report_spawn_intermediate_status(libxl__gc *gc,
-                                             libxl__spawn_starting *for_spawn,
-                                             int status)
+static void spawn_cleanup(libxl__gc *gc, libxl__spawn_state *ss)
 {
-    if (!WIFEXITED(status)) {
-        libxl_ctx *ctx = libxl__gc_owner(gc);
-        char *intermediate_what;
-        /* intermediate process did the logging itself if it exited */
-        if ( asprintf(&intermediate_what,
-                 "%s intermediate process (startup monitor)",
-                 for_spawn->what) < 0 )
-            intermediate_what = "intermediate process (startup monitor)";
-        libxl_report_child_exitstatus(ctx, LIBXL__LOG_ERROR, intermediate_what,
-                                      for_spawn->intermediate, status);
+    int r;
+
+    libxl__ev_time_deregister(gc, &ss->timeout);
+    libxl__ev_xswatch_deregister(gc, &ss->xswatch);
+
+    libxl__spawn_state_detachable *ssd = ss->ssd;
+    if (ssd) {
+        if (libxl__ev_child_inuse(&ssd->mid)) {
+            pid_t child = ssd->mid.pid;
+            r = kill(child, SIGKILL);
+            if (r && errno != ESRCH)
+                LOGE(WARN, "%s: failed to kill intermediate child (pid=%lu)",
+                     ss->what, (unsigned long)child);
+        }
+
+        /* disconnect the ss and ssd from each other */
+        ssd->ss = 0;
+        ss->ssd = 0;
     }
 }
 
-int libxl__spawn_detach(libxl__gc *gc,
-                       libxl__spawn_starting *for_spawn)
+static void spawn_failed(libxl__egc *egc, libxl__spawn_state *ss)
 {
-    libxl_ctx *ctx = libxl__gc_owner(gc);
-    int r, status;
-    pid_t got;
-    int rc = 0;
-
-    if (!for_spawn) return 0;
-
-    if (for_spawn->intermediate) {
-        r = kill(for_spawn->intermediate, SIGKILL);
-        if (r) {
-            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
-                         "could not kill %s intermediate process [%ld]",
-                         for_spawn->what,
-                         (unsigned long)for_spawn->intermediate);
-            abort(); /* things are very wrong */
-        }
-        got = waitpid(for_spawn->intermediate, &status, 0);
-        assert(got == for_spawn->intermediate);
-        if (!(WIFSIGNALED(status) && WTERMSIG(status) == SIGKILL)) {
-            report_spawn_intermediate_status(gc, for_spawn, status);
-            rc = ERROR_FAIL;
-        }
-        for_spawn->intermediate = 0;
-    }
-
-    free(for_spawn->what);
-    for_spawn->what = 0;
-
-    return rc;
+    EGC_GC;
+    spawn_cleanup(gc, ss);
+    ss->failure_cb(egc, ss); /* must be last; callback may do anything to ss */
 }
 
-int libxl__spawn_check(libxl__gc *gc, libxl__spawn_starting *for_spawn)
+static void spawn_timeout(libxl__egc *egc, libxl__ev_time *ev,
+                          const struct timeval *requested_abs)
 {
-    pid_t got;
-    int status;
+    /* Before event, was Active; is now Partial. */
+    EGC_GC;
+    libxl__spawn_state *ss = CONTAINER_OF(ev, *ss, timeout);
+    LOG(ERROR, "%s: startup timed out", ss->what);
+    spawn_failed(egc, ss); /* must be last */
+}
 
-    if (!for_spawn) return 0;
+static void spawn_watch_event(libxl__egc *egc, libxl__ev_xswatch *xsw,
+                              const char *watch_path, const char *event_path)
+{
+    /* On entry, is Active. */
+    EGC_GC;
+    libxl__spawn_state *ss = CONTAINER_OF(xsw, *ss, xswatch);
+    char *p = libxl__xs_read(gc, 0, ss->xspath);
+    if (!p && errno != ENOENT) {
+        LOG(ERROR, "%s: xenstore read of %s failed", ss->what, ss->xspath);
+        spawn_failed(egc, ss); /* must be last */
+        return;
+    }
+    ss->confirm_cb(egc, ss, p); /* must be last */
+}
 
-    assert(for_spawn->intermediate);
-    got = waitpid(for_spawn->intermediate, &status, WNOHANG);
-    if (!got) return 0;
+static void spawn_middle_death(libxl__egc *egc, libxl__ev_child *childw,
+                               pid_t pid, int status)
+    /* Before event, was Active or Detached;
+     * is now Active or Detached except that ssd->mid is Idle */
+{
+    EGC_GC;
+    libxl__spawn_state_detachable *ssd = CONTAINER_OF(childw, *ssd, mid);
+    libxl__spawn_state *ss = ssd->ss;
 
-    assert(got == for_spawn->intermediate);
-    report_spawn_intermediate_status(gc, for_spawn, status);
+    if (!WIFEXITED(status)) {
+        const char *what =
+            GCSPRINTF("%s intermediate process (startup monitor)",
+                      ss ? ss->what : "(detached)");
+        int loglevel = ss ? XTL_ERROR : XTL_WARN;
+        libxl_report_child_exitstatus(CTX, loglevel, what, pid, status);
+    } else if (ss) { /* otherwise it was supposed to be a daemon by now */
+        if (!status)
+            LOG(ERROR, "%s [%ld]: unexpectedly exited with exit status 0,"
+                " when we were waiting for it to confirm startup",
+                ss->what, (unsigned long)pid);
+        else if (status <= 127)
+            LOG(ERROR, "%s [%ld]: failed startup with non-zero exit status %d",
+                ss->what, (unsigned long)pid, status);
+        else if (status < 255) {
+            int sig = status - 128;
+            const char *str = strsignal(sig);
+            if (str)
+                LOG(ERROR, "%s [%ld]: died during startup due to fatal"
+                    " signal %s", ss->what, (unsigned long)pid, str);
+            else
+                LOG(ERROR, "%s [%ld]: died during startup due to unknown fatal"
+                    " signal number %d", ss->what, (unsigned long)pid, sig);
+        }
+        ss->ssd = 0; /* detatch the ssd to make the ss be in state Partial */
+        spawn_failed(egc, ss); /* must be last use of ss */
+    }
+    free(ssd);
+}
 
-    for_spawn->intermediate = 0;
-    return ERROR_FAIL;
+void libxl__spawn_detach(libxl__gc *gc, libxl__spawn_state *ss)
+{
+    spawn_cleanup(gc, ss);
 }
 
 /*
diff -r 9524eb476e93 -r 885fdd4fd7ac tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h      Fri May 11 18:59:02 2012 +0100
+++ b/tools/libxl/libxl_internal.h      Fri May 11 18:59:03 2012 +0100
@@ -847,75 +847,198 @@ _hidden int libxl__create_pci_backend(li
                                       libxl_device_pci *pcidev, int num);
 _hidden int libxl__device_pci_destroy_all(libxl__gc *gc, uint32_t domid);
 
-/* xl_exec */
+/*
+ *----- spawn -----
+ *
+ * Higher-level double-fork and separate detach eg as for device models
+ *
+ * Each libxl__spawn_state is in one of the conventional states
+ *    Undefined, Idle, Active
+ */
 
- /* higher-level double-fork and separate detach eg as for device models */
+typedef struct libxl__obsolete_spawn_starting libxl__spawn_starting;
+/* this type is never defined, so no objects of this type exist
+ * fixme-ao  This should go away completely.  */
 
-typedef struct {
-    /* put this in your own status structure as returned to application */
-    /* all fields are private to libxl_spawn_... */
-    pid_t intermediate;
-    int fd;
-    char *what; /* malloc'd in spawn_spawn */
-} libxl__spawn_starting;
+typedef struct libxl__spawn_state libxl__spawn_state;
 
-typedef struct {
-    char *dom_path; /* from libxl_malloc, only for libxl_spawner_record_pid */
-    const char *pid_path; /* only for libxl_spawner_record_pid */
-    int domid;
-    libxl__spawn_starting *for_spawn;
-} libxl__spawner_starting;
+/* Clears out a spawn state; idempotent. */
+_hidden void libxl__spawn_init(libxl__spawn_state*);
 
 /*
- * libxl__spawn_spawn - Create a new process
- * gc: allocation pool
- * for_spawn: malloc'd pointer to libxl__spawn_starting (optional)
- * what: string describing the spawned process
- * intermediate_hook: helper to record pid, such as libxl_spawner_record_pid
- * hook_data: data to pass to the hook function
+ * libxl__spawn_spawn - Create a new process which will become daemonic
+ * Forks twice, to allow the child to detach entirely from the parent.
+ *
+ * We call the two generated processes the "middle child" (result of
+ * the first fork) and the "inner child" (result of the second fork
+ * which takes place in the middle child).
+ *
+ * The inner child must soon exit or exec.  It must also soon exit or
+ * notify the parent of its successful startup by writing to the
+ * xenstore path xspath.
+ *
+ * The user (in the parent) will be called back (confirm_cb) every
+ * time that xenstore path is modified.
+ *
+ * In both children, the ctx is not fully usable: gc and logging
+ * operations are OK, but operations on Xen and xenstore are not.
+ * (The restrictions are the same as those which apply to children
+ * made with libxl__ev_child_fork.)
+ *
+ * midproc_cb will be called in the middle child, with the pid of the
+ * inner child; this could for example record the pid.  midproc_cb
+ * should be fast, and should return.  It will be called (reentrantly)
+ * within libxl__spawn_init.
+ *
+ * failure_cb will be called in the parent on failure of the
+ * intermediate or final child; an error message will have been
+ * logged.
+ *
+ * confirm_cb and failure_cb will not be called reentrantly from
+ * within libxl__spawn_spawn.
+ *
+ * what: string describing the spawned process, used for logging
  *
  * Logs errors.  A copy of "what" is taken. 
  * Return values:
- *  < 0   error, for_spawn need not be detached
- *   +1   caller is the parent, must call detach on *for_spawn eventually
+ *  < 0   error, *spawn is now Idle and need not be detached
+ *   +1   caller is the parent, *spawn is Active and must eventually be 
detached
  *    0   caller is now the inner child, should probably call libxl__exec
- * Caller, may pass 0 for for_spawn, in which case no need to detach.
+ *
+ * The spawn state must be Undefined or Idle on entry.
  */
-_hidden int libxl__spawn_spawn(libxl__gc *gc,
-                      libxl__spawn_starting *for_spawn,
-                      const char *what,
-                      void (*intermediate_hook)(void *for_spawn, pid_t 
innerchild),
-                      void *hook_data);
+_hidden int libxl__spawn_spawn(libxl__egc *egc, libxl__spawn_state *spawn);
+
+/*
+ * libxl__spawn_detach - Detaches the daemonic child.
+ *
+ * Works by killing the intermediate process from spawn_spawn.
+ * After this function returns, failures of either child are no
+ * longer reported via failure_cb.
+ *
+ * If called before the inner child has been created, this may prevent
+ * it from running at all.  Thus this should be called only when the
+ * inner child has notified that it is ready.  Normally it will be
+ * called from within confirm_cb.
+ *
+ * Logs errors.
+ *
+ * The spawn state must be Active or Idle on entry and will be Idle
+ * on return.
+ */
+_hidden void libxl__spawn_detach(libxl__gc *gc, libxl__spawn_state*);
+
+/*
+ * If successful, this should return 0.
+ *
+ * Otherwise it should return a signal number, which will be
+ * sent to the inner child; the overall spawn will then fail.
+ */
+typedef int /* signal number */
+libxl__spawn_midproc_cb(libxl__gc*, libxl__spawn_state*, pid_t inner);
+
+/*
+ * Called if the spawn failed.  The reason will have been logged.
+ * The spawn state will be Idle on entry to the callback (and
+ * it may be reused immediately if desired).
+ */
+typedef void libxl__spawn_failure_cb(libxl__egc*, libxl__spawn_state*);
+
+/*
+ * Called when the xspath watch triggers.  xspath will have been read
+ * and the result placed in xsdata; if that failed because the key
+ * didn't exist, xspath==0.  (If it failed for some other reason,
+ * the spawn machinery calls failure_cb instead.)
+ *
+ * If the child has indicated its successful startup, or a failure
+ * has occurred, this should call libxl__spawn_detach.
+ *
+ * If the child is still starting up, should simply return, doing
+ * nothing.
+ *
+ * The spawn state will be Active on entry to the callback; there
+ * are no restrictions on the state on return; it may even have
+ * been detached and reused.
+ */
+typedef void libxl__spawn_confirm_cb(libxl__egc*, libxl__spawn_state*,
+                                     const char *xsdata);
+
+typedef struct {
+    /* Private to the spawn implementation.
+     */
+    /* This separate struct, from malloc, allows us to "detach"
+     * the child and reap it later, when our user has gone
+     * away and freed its libxl__spawn_state */
+    struct libxl__spawn_state *ss;
+    libxl__ev_child mid;
+} libxl__spawn_state_detachable;
+
+struct libxl__spawn_state {
+    /* must be filled in by user and remain valid */
+    libxl__ao *ao;
+    const char *what;
+    const char *xspath;
+    const char *pidpath; /* only used by libxl__spawn_midproc_record_pid */
+    int timeout_ms; /* -1 means forever */
+    libxl__spawn_midproc_cb *midproc_cb;
+    libxl__spawn_failure_cb *failure_cb;
+    libxl__spawn_confirm_cb *confirm_cb;
+
+    /* remaining fields are private to libxl_spawn_... */
+    libxl__ev_time timeout;
+    libxl__ev_xswatch xswatch;
+    libxl__spawn_state_detachable *ssd;
+};
+
+static inline int libxl__spawn_inuse(libxl__spawn_state *ss)
+    { return !!ss->ssd; }
 
 /*
  * libxl_spawner_record_pid - Record given pid in xenstore
- * for_spawn: malloc'd pointer to libxl__spawn_starting (optional)
- * innerchild: pid of the child
  *
- * This function is passed as intermediate_hook to libxl__spawn_spawn.
+ * This function can be passed directly as an intermediate_hook to
+ * libxl__spawn_spawn.  On failure, returns the value SIGTERM.
  */
-_hidden void libxl_spawner_record_pid(void *for_spawn, pid_t innerchild);
+_hidden int libxl__spawn_record_pid(libxl__gc*, libxl__spawn_state*,
+                                    pid_t innerchild);
 
-/*
- * libxl__spawn_confirm_offspring_startup - Wait for child state
- * gc: allocation pool
- * timeout: how many seconds to wait for the child
- * what: string describing the spawned process
- * path: path to the state file in xenstore
- * state: expected string to wait for in path (optional)
- * starting: malloc'd pointer to libxl__spawner_starting
- *
- * Returns 0 on success, and < 0 on error.
- *
- * This function waits the given timeout for the given path to appear
- * in xenstore, and optionally for state in path.
- * The intermediate process created in libxl__spawn_spawn is killed.
- * The memory referenced by starting->for_spawn and starting is free'd.
- */
-_hidden int libxl__spawn_confirm_offspring_startup(libxl__gc *gc,
-                                       uint32_t timeout, char *what,
-                                       char *path, char *state,
-                                       libxl__spawner_starting *starting);
+/*----- device model creation -----*/
+
+/* First layer; wraps libxl__spawn_spawn. */
+
+typedef struct libxl__dm_spawn_state libxl__dm_spawn_state;
+
+typedef void libxl__dm_spawn_cb(libxl__egc *egc, libxl__dm_spawn_state*,
+                                int rc /* if !0, error was logged */);
+
+struct libxl__dm_spawn_state {
+    /* mixed - spawn.ao must be initialised by user; rest is private: */
+    libxl__spawn_state spawn;
+    /* filled in by user, must remain valid: */
+    uint32_t guest_domid; /* domain being served */
+    libxl_domain_config *guest_config;
+    libxl__domain_build_state *build_state; /* relates to guest_domid */
+    libxl__dm_spawn_cb *callback;
+};
+
+_hidden void libxl__spawn_local_dm(libxl__egc *egc, libxl__dm_spawn_state*);
+
+/* Stubdom device models. */
+
+typedef struct {
+    /* Mixed - user must fill in public parts EXCEPT callback,
+     * which may be undefined on entry.  (See above for details) */
+    libxl__dm_spawn_state dm; /* the stub domain device model */
+    /* filled in by user, must remain valid: */
+    libxl__dm_spawn_cb *callback; /* called as callback(,&sdss->dm,) */
+    /* private to libxl__spawn_stub_dm: */
+    libxl_domain_config dm_config;
+    libxl__domain_build_state dm_state;
+    libxl__dm_spawn_state pvqemu;
+} libxl__stub_dm_spawn_state;
+
+_hidden void libxl__spawn_stub_dm(libxl__egc *egc, 
libxl__stub_dm_spawn_state*);
+
 
 /*
  * libxl__wait_for_offspring - Wait for child state
@@ -948,31 +1071,6 @@ _hidden int libxl__wait_for_offspring(li
                                                        void *userdata),
                                  void *check_callback_userdata);
 
-/*
- * libxl__spawn_detach - Kill intermediate process from spawn_spawn
- * gc: allocation pool
- * for_spawn: malloc'd pointer to libxl__spawn_starting (optional)
- *
- * Returns 0 on success, and < 0 on error.
- *
- * Logs errors.  Idempotent, but only permitted after successful
- * call to libxl__spawn_spawn, and no point calling it again if it fails.
- */
-_hidden int libxl__spawn_detach(libxl__gc *gc,
-                       libxl__spawn_starting *for_spawn);
-
-/*
- * libxl__spawn_check - Check intermediate child process
- * gc: allocation pool
- * for_spawn: malloc'd pointer to libxl__spawn_starting (optional)
- *
- * Returns 0 on success, and < 0 on error.
- *
- * Logs errors but also returns them.
- * Caller must still call detach.
- */
-_hidden int libxl__spawn_check(libxl__gc *gc,
-                       libxl__spawn_starting *for_spawn);
 
  /* low-level stuff, for synchronous subprocesses etc. */
 
@@ -991,15 +1089,6 @@ _hidden int libxl__domain_build(libxl__g
 /* for device model creation */
 _hidden const char *libxl__domain_device_model(libxl__gc *gc,
                                         const libxl_domain_build_info *info);
-_hidden int libxl__create_device_model(libxl__gc *gc,
-                              int domid,
-                              libxl_domain_config *guest_config,
-                              libxl__domain_build_state *state,
-                              libxl__spawner_starting **starting_r);
-_hidden int libxl__create_xenpv_qemu(libxl__gc *gc, uint32_t domid,
-                              libxl_domain_config *guest_config,
-                              libxl__domain_build_state *state,
-                              libxl__spawner_starting **starting_r);
 _hidden int libxl__need_xenpv_qemu(libxl__gc *gc,
         int nr_consoles, libxl__device_console *consoles,
         int nr_vfbs, libxl_device_vfb *vfbs,
@@ -1007,10 +1096,6 @@ _hidden int libxl__need_xenpv_qemu(libxl
   /* Caller must either: pass starting_r==0, or on successful
    * return pass *starting_r (which will be non-0) to
    * libxl__confirm_device_model_startup or libxl__detach_device_model. */
-_hidden int libxl__confirm_device_model_startup(libxl__gc *gc,
-                              libxl__domain_build_state *state,
-                              libxl__spawner_starting *starting);
-_hidden int libxl__detach_device_model(libxl__gc *gc, libxl__spawner_starting 
*starting);
 _hidden int libxl__wait_for_device_model(libxl__gc *gc,
                                 uint32_t domid, char *state,
                                 libxl__spawn_starting *spawning,
@@ -1581,6 +1666,31 @@ _hidden void libxl__bootloader_init(libx
 _hidden void libxl__bootloader_run(libxl__egc*, libxl__bootloader_state *st);
 
 
+/*----- Domain creation -----*/
+
+typedef struct libxl__domain_create_state libxl__domain_create_state;
+
+typedef void libxl__domain_create_cb(libxl__egc *egc,
+                                     libxl__domain_create_state*,
+                                     int rc, uint32_t domid);
+
+struct libxl__domain_create_state {
+    /* filled in by user */
+    libxl__ao *ao;
+    libxl_domain_config *guest_config;
+    int restore_fd;
+    libxl_console_ready console_cb;
+    void *console_cb_priv;
+    libxl__domain_create_cb *callback;
+    /* private to domain_create */
+    int guest_domid;
+    libxl__domain_build_state build_state;
+    libxl__stub_dm_spawn_state dmss;
+        /* If we're not doing stubdom, we use only dmss.dm,
+         * for the non-stubdom device model. */
+};
+
+
 /*
  * Convenience macros.
  */
diff -r 9524eb476e93 -r 885fdd4fd7ac tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c  Fri May 11 18:59:02 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c  Fri May 11 18:59:03 2012 +0100
@@ -1695,8 +1695,8 @@ start:
 
     if ( restore_file ) {
         ret = libxl_domain_create_restore(ctx, &d_config,
-                                            cb, &child_console_pid,
-                                            &domid, restore_fd);
+                                          cb, &child_console_pid,
+                                          &domid, restore_fd, 0);
         /*
          * On subsequent reboot etc we should create the domain, not
          * restore/migrate-receive it again.
@@ -1704,7 +1704,7 @@ start:
         restore_file = NULL;
     }else{
         ret = libxl_domain_create_new(ctx, &d_config,
-                                        cb, &child_console_pid, &domid);
+                                      cb, &child_console_pid, &domid, 0);
     }
     if ( ret )
         goto error_out;

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


 


Rackspace

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