WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [PATCH 4 of 4] libxl: add statup checks to libxl__wait_for_d

To: xen-devel@xxxxxxxxxxxxxxxxxxx
Subject: [Xen-devel] [PATCH 4 of 4] libxl: add statup checks to libxl__wait_for_device_model
From: Ian Campbell <ian.campbell@xxxxxxxxxx>
Date: Wed, 4 May 2011 15:51:12 +0100
Cc: Bastian Blank <waldi@xxxxxxxxxx>
Delivery-date: Wed, 04 May 2011 07:56:32 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <patchbomb.1304520668@xxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <patchbomb.1304520668@xxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mercurial-patchbomb/1.6.4
# HG changeset patch
# User Ian Campbell <ian.campbell@xxxxxxxxxx>
# 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@xxxxxxxxxx>

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 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel