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@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

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@eu.citrix.com>

1 files changed, 178 insertions, 176 deletions

diff --git a/tools/libxl/libxl_exec.c b/tools/libxl/libxl_exec.c
index 2ee2154e77..882c048faf 100644
--- a/tools/libxl/libxl_exec.c
+++ b/tools/libxl/libxl_exec.c
@@ -127,29 +127,35 @@ void libxl_report_child_exitstatus(libxl_ctx *ctx,
     }
 }
 
-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;
-
-    if (asprintf(&path, "%s/%s", starting->dom_path, starting->pid_path) < 0)
-        goto out;
+    struct xs_handle *xsh = NULL;
+    const char *pid = NULL;
+    int rc, xsok;
 
-    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;
+    }
+
+    rc = 0;
 
-    xs_daemon_close(xsh);
 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 *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)
-{
-    int rc;
-    rc = libxl__spawn_detach(gc, starting->for_spawn);
-    if (starting->for_spawn)
-        free(starting->for_spawn);
-    free(starting);
-    return rc;
-}
 
-int libxl__spawn_confirm_offspring_startup(libxl__gc *gc,
-                                       uint32_t timeout, char *what,
-                                       char *path, char *state,
-                                       libxl__spawner_starting *starting)
-{
-    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;
-}
+/*----- spawn implementation -----*/
 
-static int libxl__set_fd_flag(libxl__gc *gc, int fd, int flag)
-{
-    int flags;
+/*
+ * 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.
+ */
 
-    flags = fcntl(fd, F_GETFL);
-    if (flags == -1)
-        return ERROR_FAIL;
+/* 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);
 
-    flags |= flag;
+/* Precondition: Partial.  Results: Detached. */
+static void spawn_cleanup(libxl__gc *gc, libxl__spawn_state *ss);
 
-    if (fcntl(fd, F_SETFL, flags) == -1)
-        return ERROR_FAIL;
+/* 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);
 
-    return 0;
+void libxl__spawn_init(libxl__spawn_state *ss)
+{
+    libxl__ev_time_init(&ss->timeout);
+    libxl__ev_xswatch_init(&ss->xswatch);
+    ss->ssd = 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)
+int libxl__spawn_spawn(libxl__egc *egc, libxl__spawn_state *ss)
 {
-    libxl_ctx *ctx = libxl__gc_owner(gc);
-    pid_t child, got;
+    STATE_AO_GC(ss->ao);
+    int r;
+    pid_t child;
     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;
+    libxl__spawn_init(ss);
+    ss->ssd = libxl__zalloc(0, sizeof(*ss->ssd));
+    libxl__ev_child_init(&ss->ssd->mid);
+
+    rc = libxl__ev_time_register_rel(gc, &ss->timeout,
+                                     spawn_timeout, ss->timeout_ms);
+    if (rc) goto out_err;
 
-    if (intermediate) {
+    rc = libxl__ev_xswatch_register(gc, &ss->xswatch,
+                                    spawn_watch_event, ss->xspath);
+    if (rc) goto out_err;
+
+    pid_t middle = libxl__ev_child_fork(gc, &ss->ssd->mid, spawn_middle_death);
+    if (middle ==-1) { rc = ERROR_FAIL; goto out_err; }
+
+    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");
+    int failsig = ss->midproc_cb(gc, ss, child);
+    if (failsig) {
+        kill(child, failsig);
+        _exit(127);
     }
-    _exit(rc);
 
- err_parent_pipes:
-    if (for_spawn) {
-        close(pipes[0]);
-        close(pipes[1]);
+    for (;;) {
+        pid_t got = waitpid(child, &status, 0);
+        if (got == -1) {
+            assert(errno == EINTR);
+            continue;
+        }
+        assert(got == child);
+        break;
     }
 
- err_parent:
-    if (for_spawn) free(for_spawn->what);
+    r = (WIFEXITED(status) && WEXITSTATUS(status) <= 127 ? WEXITSTATUS(status) :
+         WIFSIGNALED(status) && WTERMSIG(status) < 127 ? WTERMSIG(status)+128 :
+         -1);
+    _exit(r);
 
-    return ERROR_FAIL;
+ 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;
+    EGC_GC;
+    spawn_cleanup(gc, ss);
+    ss->failure_cb(egc, ss); /* must be last; callback may do anything to ss */
+}
 
-    if (!for_spawn) return 0;
+static void spawn_timeout(libxl__egc *egc, libxl__ev_time *ev,
+                          const struct timeval *requested_abs)
+{
+    /* 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->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;
+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;
     }
-
-    free(for_spawn->what);
-    for_spawn->what = 0;
-
-    return rc;
+    ss->confirm_cb(egc, ss, p); /* must be last */
 }
 
-int libxl__spawn_check(libxl__gc *gc, libxl__spawn_starting *for_spawn)
+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 */
 {
-    pid_t got;
-    int status;
-
-    if (!for_spawn) return 0;
+    EGC_GC;
+    libxl__spawn_state_detachable *ssd = CONTAINER_OF(childw, *ssd, mid);
+    libxl__spawn_state *ss = ssd->ss;
 
-    assert(for_spawn->intermediate);
-    got = waitpid(for_spawn->intermediate, &status, WNOHANG);
-    if (!got) return 0;
-
-    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);
 }
 
 /*