[PATCH 4 of 4] libxl: add statup checks to libxl__wait_for_device_model

[Ian Campbell <ian.campbell@citrix.com>]

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1304516515 -3600
# Node ID 36871b37a9bd286d6ef291b41513c33325db9719
# Parent  ec008dc43727191f472cd4bd5e59d9d35d9d36c2
libxl: add statup checks to libxl__wait_for_device_model

When the device model is starting up push checks for spawn failure down into
libxl__wait_for_device_model, allowing us to fail more quickly when the device
model fails to start (e.g. due to a missing library or an early setup error
etc).

In order to allow the select loop in libxl__wait_for_device_model to wake when
the child dies add pipe between the parent and the intermediate process which
the intermediate process can use to signal the parent.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r ec008dc43727 -r 36871b37a9bd tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Wed May 04 14:37:21 2011 +0100
+++ b/tools/libxl/libxl.c	Wed May 04 14:41:55 2011 +0100
@@ -522,7 +522,7 @@ int libxl_domain_unpause(libxl_ctx *ctx,
         state = libxl__xs_read(&gc, XBT_NULL, path);
         if (state != NULL && !strcmp(state, "paused")) {
             libxl__xs_write(&gc, XBT_NULL, libxl__sprintf(&gc, "/local/domain/0/device-model/%d/command", domid), "continue");
-            libxl__wait_for_device_model(&gc, domid, "running", NULL, NULL);
+            libxl__wait_for_device_model(&gc, domid, "running", NULL, NULL, NULL);
         }
     }
     ret = xc_domain_unpause(ctx->xch, domid);
diff -r ec008dc43727 -r 36871b37a9bd tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Wed May 04 14:37:21 2011 +0100
+++ b/tools/libxl/libxl_device.c	Wed May 04 14:41:55 2011 +0100
@@ -410,6 +410,7 @@ out:
 
 int libxl__wait_for_device_model(libxl__gc *gc,
                                  uint32_t domid, char *state,
+                                 libxl__device_model_starting *starting,
                                  int (*check_callback)(libxl__gc *gc,
                                                        uint32_t domid,
                                                        const char *state,
@@ -439,7 +440,17 @@ int libxl__wait_for_device_model(libxl__
     tv.tv_sec = LIBXL_DEVICE_MODEL_START_TIMEOUT;
     tv.tv_usec = 0;
     nfds = xs_fileno(xsh) + 1;
+    if (starting && starting->for_spawn->fd > xs_fileno(xsh))
+        nfds = starting->for_spawn->fd + 1;
+
     while (rc > 0 || (!rc && tv.tv_sec > 0)) {
+        if ( starting ) {
+            rc = libxl__spawn_check(gc, starting->for_spawn);
+            if ( rc ) {
+                rc = -1;
+                goto err_died;
+            }
+        }
         p = xs_read(xsh, XBT_NULL, path, &len);
         if ( NULL == p )
             goto again;
@@ -461,15 +472,24 @@ again:
         free(p);
         FD_ZERO(&rfds);
         FD_SET(xs_fileno(xsh), &rfds);
+        if (starting)
+            FD_SET(starting->for_spawn->fd, &rfds);
         rc = select(nfds, &rfds, NULL, NULL, &tv);
         if (rc > 0) {
-            l = xs_read_watch(xsh, &num);
-            if (l != NULL)
-                free(l);
-            else
-                goto again;
+            if (FD_ISSET(xs_fileno(xsh), &rfds)) {
+                l = xs_read_watch(xsh, &num);
+                if (l != NULL)
+                    free(l);
+                else
+                    goto again;
+            }
+            if (starting && FD_ISSET(starting->for_spawn->fd, &rfds)) {
+                unsigned char dummy;
+                read(starting->for_spawn->fd, &dummy, sizeof(dummy));
+            }
         }
     }
+err_died:
     xs_unwatch(xsh, path, path);
     xs_daemon_close(xsh);
     LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Device Model not ready");
diff -r ec008dc43727 -r 36871b37a9bd tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Wed May 04 14:37:21 2011 +0100
+++ b/tools/libxl/libxl_dm.c	Wed May 04 14:41:55 2011 +0100
@@ -855,14 +855,12 @@ static int detach_device_model(libxl__gc
     return rc;
 }
 
-
 int libxl__confirm_device_model_startup(libxl__gc *gc,
                                        libxl__device_model_starting *starting)
 {
-    int problem = libxl__wait_for_device_model(gc, starting->domid, "running", NULL, NULL);
     int detach;
-    if ( !problem )
-        problem = libxl__spawn_check(gc, starting->for_spawn);
+    int problem = libxl__wait_for_device_model(gc, starting->domid, "running",
+                                               starting, NULL, NULL);
     detach = detach_device_model(gc, starting);
     return problem ? problem : detach;
 }
diff -r ec008dc43727 -r 36871b37a9bd tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Wed May 04 14:37:21 2011 +0100
+++ b/tools/libxl/libxl_dom.c	Wed May 04 14:41:55 2011 +0100
@@ -543,7 +543,7 @@ int libxl__domain_save_device_model(libx
 
     LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Saving device model state to %s", filename);
     libxl__xs_write(gc, XBT_NULL, libxl__sprintf(gc, "/local/domain/0/device-model/%d/command", domid), "save");
-    libxl__wait_for_device_model(gc, domid, "paused", NULL, NULL);
+    libxl__wait_for_device_model(gc, domid, "paused", NULL, NULL, NULL);
 
     if (stat(filename, &st) < 0)
     {
diff -r ec008dc43727 -r 36871b37a9bd tools/libxl/libxl_exec.c
--- a/tools/libxl/libxl_exec.c	Wed May 04 14:37:21 2011 +0100
+++ b/tools/libxl/libxl_exec.c	Wed May 04 14:41:55 2011 +0100
@@ -26,6 +26,7 @@
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <signal.h> /* for SIGKILL */
+#include <fcntl.h>
 
 #include "libxl.h"
 #include "libxl_internal.h"
@@ -91,6 +92,22 @@ void libxl_report_child_exitstatus(libxl
     }
 }
 
+static int libxl__set_fd_flag(libxl__gc *gc, int fd, int flag)
+{
+    int flags;
+
+    flags = fcntl(fd, F_GETFL);
+    if (flags == -1)
+        return ERROR_FAIL;
+
+    flags |= flag;
+
+    if (fcntl(fd, F_SETFL, flags) == -1)
+        return ERROR_FAIL;
+
+    return 0;
+}
+
 int libxl__spawn_spawn(libxl__gc *gc,
                       libxl__spawn_starting *for_spawn,
                       const char *what,
@@ -100,22 +117,33 @@ int libxl__spawn_spawn(libxl__gc *gc,
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     pid_t child, got;
-    int status;
+    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) {
-        if (for_spawn) free(for_spawn->what);
-        return ERROR_FAIL;
-    }
+    if (intermediate ==-1)
+        goto err_parent_pipes;
+
     if (intermediate) {
         /* parent */
-        if (for_spawn) for_spawn->intermediate = intermediate;
+        if (for_spawn) {
+            for_spawn->intermediate = intermediate;
+            for_spawn->fd = pipes[0];
+            close(pipes[1]);
+        }
         return 1;
     }
 
@@ -124,8 +152,10 @@ int libxl__spawn_spawn(libxl__gc *gc,
     child = fork();
     if (child == -1)
         exit(255);
-    if (!child)
+    if (!child) {
+        if (for_spawn) close(pipes[1]);
         return 0; /* caller runs child code */
+    }
 
     intermediate_hook(hook_data, child);
 
@@ -134,9 +164,23 @@ int libxl__spawn_spawn(libxl__gc *gc,
     got = call_waitpid(ctx->waitpid_instead, child, &status, 0);
     assert(got == child);
 
-    _exit(WIFEXITED(status) ? WEXITSTATUS(status) :
+    rc = (WIFEXITED(status) ? WEXITSTATUS(status) :
           WIFSIGNALED(status) && WTERMSIG(status) < 127
           ? WTERMSIG(status)+128 : -1);
+    if (for_spawn)
+        write(pipes[1], &dummy, sizeof(dummy));
+    _exit(rc);
+
+ err_parent_pipes:
+    if (for_spawn) {
+        close(pipes[0]);
+        close(pipes[1]);
+    }
+
+ err_parent:
+    if (for_spawn) free(for_spawn->what);
+
+    return ERROR_FAIL;
 }
 
 static void report_spawn_intermediate_status(libxl__gc *gc,
diff -r ec008dc43727 -r 36871b37a9bd tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Wed May 04 14:37:21 2011 +0100
+++ b/tools/libxl/libxl_internal.h	Wed May 04 14:41:55 2011 +0100
@@ -233,6 +233,7 @@ 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;
 
@@ -270,6 +271,7 @@ _hidden int libxl__confirm_device_model_
 _hidden int libxl__detach_device_model(libxl__gc *gc, libxl__device_model_starting *starting);
 _hidden int libxl__wait_for_device_model(libxl__gc *gc,
                                 uint32_t domid, char *state,
+                                libxl__device_model_starting *starting,
                                 int (*check_callback)(libxl__gc *gc,
                                                       uint32_t domid,
                                                       const char *state,
diff -r ec008dc43727 -r 36871b37a9bd tools/libxl/libxl_pci.c
--- a/tools/libxl/libxl_pci.c	Wed May 04 14:37:21 2011 +0100
+++ b/tools/libxl/libxl_pci.c	Wed May 04 14:41:55 2011 +0100
@@ -612,7 +612,8 @@ static int do_pci_add(libxl__gc *gc, uin
 
     hvm = libxl__domain_is_hvm(gc, domid);
     if (hvm) {
-        if (libxl__wait_for_device_model(gc, domid, "running", NULL, NULL) < 0) {
+        if (libxl__wait_for_device_model(gc, domid, "running",
+                                         NULL, NULL, NULL) < 0) {
             return ERROR_FAIL;
         }
         path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/state", domid);
@@ -626,7 +627,8 @@ static int do_pci_add(libxl__gc *gc, uin
                            pcidev->bus, pcidev->dev, pcidev->func);
         path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/command", domid);
         xs_write(ctx->xsh, XBT_NULL, path, "pci-ins", strlen("pci-ins"));
-        rc = libxl__wait_for_device_model(gc, domid, NULL, pci_ins_check, state);
+        rc = libxl__wait_for_device_model(gc, domid, NULL, NULL,
+                                          pci_ins_check, state);
         path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/parameter", domid);
         vdevfn = libxl__xs_read(gc, XBT_NULL, path);
         path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/state", domid);
@@ -843,7 +845,8 @@ static int do_pci_remove(libxl__gc *gc, 
 
     hvm = libxl__domain_is_hvm(gc, domid);
     if (hvm) {
-        if (libxl__wait_for_device_model(gc, domid, "running", NULL, NULL) < 0) {
+        if (libxl__wait_for_device_model(gc, domid, "running",
+                                         NULL, NULL, NULL) < 0) {
             return ERROR_FAIL;
         }
         path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/state", domid);
@@ -857,7 +860,8 @@ static int do_pci_remove(libxl__gc *gc, 
          * device-model for function 0 */
         if ( !force && (pcidev->vdevfn & 0x7) == 0 ) {
             xs_write(ctx->xsh, XBT_NULL, path, "pci-rem", strlen("pci-rem"));
-            if (libxl__wait_for_device_model(gc, domid, "pci-removed", NULL, NULL) < 0) {
+            if (libxl__wait_for_device_model(gc, domid, "pci-removed",
+                                             NULL, NULL, NULL) < 0) {
                 LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Device Model didn't respond in time");
                 /* This depends on guest operating system acknowledging the
                  * SCI, if it doesn't respond in time then we may wish to