libxenlight: check for early failures of qemu-dm

This patch makes xl create check whether qemu-dm has started
correctly, and causes it to fail immediately with appropriate errors
if not.  There are other bugfixes too.

More specifically:

 * libxl_create_device_model forks twice rather than once so that the
   process which calls libxl does not end up being the actual parent
   of qemu.  That avoids the need for the qemu-dm process to be reaped
   at some indefinite time in the future.

 * The first fork generates an intermediate process which is
   responsible for writing the qemu-dm pid to xenstore and then merely
   waits to collect and report on qemu-dm's exit status during
   startup.  New arguments to libxl_create_device_model allow the
   preservation of its pid so that a later call can check whether the
   startup is successful.

 * The core of this functionality (the double fork, waitpid, signal
   handling and so forth) is abstracted away into a new facility
   libxl_spawn_... in libxl_exec.c.

Consequential changes:

 * libxl_wait_for_device_model now takes a callback function parameter
   which is called repeatedly in the loop iteration and allows the
   caller to abort the wait.

 * libxl_exec no longer calls fork; there is a new libxl_fork.

 * There is a hook to override waitpid, which will be necessary for
   some callers.

Remaining problems and other issues I noticed or we found:

 * The error handling is rather inconsistent still and lacking in
   places.

 * destroy_device_model can kill random dom0 processes (!)

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

1 files changed, 173 insertions, 16 deletions

diff --git a/tools/libxl/libxl_exec.c b/tools/libxl/libxl_exec.c
index 5186ac8c7c..de79b64e73 100644
--- a/tools/libxl/libxl_exec.c
+++ b/tools/libxl/libxl_exec.c
@@ -18,34 +18,191 @@
 #include "libxl_osdeps.h"
 
 #include <stdio.h>
+#include <string.h>
 #include <unistd.h>
 #include <stdlib.h>
+#include <unistd.h>
+#include <assert.h>
+#include <sys/types.h>
+#include <sys/wait.h>
 
 #include "libxl.h"
 #include "libxl_internal.h"
 
-int libxl_exec(struct libxl_ctx *ctx, int stdinfd, int stdoutfd, int stderrfd,
-               char *arg0, char **args)
+pid_t libxl_fork(struct libxl_ctx *ctx)
 {
-    int pid, i;
+    pid_t pid;
 
     pid = fork();
     if (pid == -1) {
         XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "fork failed");
         return -1;
     }
-    if (pid == 0) {
-        /* child */
-        if (stdinfd != -1)
-            dup2(stdinfd, STDIN_FILENO);
-        if (stdoutfd != -1)
-            dup2(stdoutfd, STDOUT_FILENO);
-        if (stderrfd != -1)
-            dup2(stderrfd, STDERR_FILENO);
-        for (i = 4; i < 256; i++)
-            close(i);
-        execv(arg0, args);
-        exit(256);
-    }
+
     return pid;
 }
+
+void libxl_exec(struct libxl_ctx *ctx, int stdinfd, int stdoutfd, int stderrfd,
+                char *arg0, char **args)
+     /* call this in the child */
+{
+    int i;
+
+    if (stdinfd != -1)
+        dup2(stdinfd, STDIN_FILENO);
+    if (stdoutfd != -1)
+        dup2(stdoutfd, STDOUT_FILENO);
+    if (stderrfd != -1)
+        dup2(stderrfd, STDERR_FILENO);
+    for (i = 4; i < 256; i++)
+        close(i);
+    execv(arg0, args);
+    XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "exec %s failed", arg0);
+    _exit(-1);
+}
+
+void libxl_report_child_exitstatus(struct libxl_ctx *ctx,
+                                   const char *what, pid_t pid, int status) {
+    /* treats all exit statuses as errors; if that's not what you want,
+     * check status yourself first */
+
+    if (WIFEXITED(status)) {
+        int st= WEXITSTATUS(status);
+        if (st)
+            XL_LOG(ctx, XL_LOG_ERROR, "%s [%ld] exited"
+                   " with error status %d", what, (unsigned long)pid, st);
+        else
+            XL_LOG(ctx, XL_LOG_ERROR, "%s [%ld] unexpectedly"
+                   " exited status zero", what, (unsigned long)pid);
+    } else if (WIFSIGNALED(status)) {
+        int sig= WTERMSIG(status);
+        const char *str= strsignal(sig);
+        const char *coredump= WCOREDUMP(status) ? " (core dumped)" : "";
+        if (str)
+            XL_LOG(ctx, XL_LOG_ERROR, "%s [%ld] died due to"
+                   " fatal signal %s%s", what, (unsigned long)pid,
+                   str, coredump);
+        else
+            XL_LOG(ctx, XL_LOG_ERROR, "%s [%ld] died due to unknown"
+                   " fatal signal number %d%s", what, (unsigned long)pid,
+                   sig, coredump);
+    } else {
+        XL_LOG(ctx, XL_LOG_ERROR, "%s [%ld] gave unknown"
+               " wait status 0x%x", what, (unsigned long)pid, status);
+    }
+}
+
+pid_t libxl_waitpid_instead_default(pid_t pid, int *status, int flags) {
+    return waitpid(pid,status,flags);
+}
+
+
+
+int libxl_spawn_spawn(struct libxl_ctx *ctx,
+                      struct libxl_spawn_starting *for_spawn,
+                      const char *what,
+                      void (*intermediate_hook)(struct libxl_ctx *ctx,
+                                                void *for_spawn,
+                                                pid_t innerchild)) {
+    pid_t child, got;
+    int status;
+    pid_t intermediate;
+
+    if (for_spawn) {
+        for_spawn->what= strdup(what);
+        if (!for_spawn->what) return ERROR_NOMEM;
+    }
+
+    intermediate = libxl_fork(ctx);
+    if (intermediate==-1) {
+        if (for_spawn) free(for_spawn->what);
+        return ERROR_FAIL;
+    }
+    if (intermediate) {
+        /* parent */
+        if (for_spawn) for_spawn->intermediate= intermediate;
+        return 1;
+    }
+
+    /* we are now the intermediate process */
+
+    child = libxl_fork(ctx);
+    if (!child) return 0; /* caller runs child code */
+    if (child<0) exit(255);
+
+    intermediate_hook(ctx, for_spawn, child);
+
+    if (!for_spawn) _exit(0); /* just detach then */
+
+    got = ctx->waitpid_instead(child, &status, 0);
+    assert(got == child);
+
+    libxl_report_child_exitstatus(ctx, what, child, status);
+    _exit(WIFEXITED(status) ? WEXITSTATUS(status) :
+          WIFSIGNALED(status) && WTERMSIG(status)<127
+          ? WTERMSIG(status)+128 : -1);
+}
+
+static void report_spawn_intermediate_status(struct libxl_ctx *ctx,
+                                 struct libxl_spawn_starting *for_spawn,
+                                 int status) {
+    if (!WIFEXITED(status)) {
+        /* intermediate process did the logging itself if it exited */
+        char *intermediate_what=
+            libxl_sprintf(ctx,
+                          "%s intermediate process (startup monitor)",
+                          for_spawn->what);
+        libxl_report_child_exitstatus(ctx, intermediate_what,
+                                      for_spawn->intermediate, status);
+    }
+}
+
+int libxl_spawn_detach(struct libxl_ctx *ctx,
+                       struct libxl_spawn_starting *for_spawn) {
+    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) {
+            XL_LOG_ERRNO(ctx, XL_LOG_ERROR,
+                         "could not kill %s intermediate process [%ld]",
+                         for_spawn->what,
+                         (unsigned long)for_spawn->intermediate);
+            abort(); /* things are very wrong */
+        }
+        got = ctx->waitpid_instead(for_spawn->intermediate, &status, 0);
+        assert(got == for_spawn->intermediate);
+        if (!(WIFSIGNALED(status) && WTERMSIG(status)==SIGKILL)) {
+            report_spawn_intermediate_status(ctx, for_spawn, status);
+            rc = ERROR_FAIL;
+        }
+        for_spawn->intermediate = 0;
+    }
+
+    free(for_spawn->what);
+    for_spawn->what = 0;
+
+    return rc;
+}
+
+int libxl_spawn_check(struct libxl_ctx *ctx, void *for_spawn_void) {
+    struct libxl_spawn_starting *for_spawn = for_spawn_void;
+    pid_t got;
+    int status;
+
+    if (!for_spawn) return 0;
+
+    assert(for_spawn->intermediate);
+    got = ctx->waitpid_instead(for_spawn->intermediate, &status, WNOHANG);
+    if (!got) return 0;
+
+    assert(got == for_spawn->intermediate);
+    report_spawn_intermediate_status(ctx, for_spawn, status);
+
+    for_spawn->intermediate= 0;
+    return ERROR_FAIL;
+}