[Xen-devel] [PATCH] libxl: fix device removal handling of wait_for_dev_destroy

[Roger Pau Monne <roger.pau@xxxxxxxxxxxxx>]

# HG changeset patch
# User Roger Pau Monne <roger.pau@xxxxxxxxxxxxx>
# Date 1318942350 -7200
# Node ID 973022eb35ed3c2e58980d8c4482721d4e1d7bab
# Parent  c9c228ec718a4c2d22b58ce306f269868af0d12f
libxl: fix device removal handling of wait_for_dev_destroy

This patch is a fix for Ian Campbell's "libxl: rationalise libxl_device_* APIs" 
and addresses the problem of not handling the return value from destroy 
functions correctly. The return value can represent three different options, 
the device is removed correctly, the device is busy or and error occurred.

Signed-off-by: Roger Pau Monne <roger.pau@xxxxxxxxxxxxx>

diff -r c9c228ec718a -r 973022eb35ed tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c        Thu Oct 13 10:52:46 2011 +0100
+++ b/tools/libxl/libxl_device.c        Tue Oct 18 14:52:30 2011 +0200
@@ -367,6 +367,10 @@ int libxl__device_disk_dev_number(const 
     return -1;
 }
 
+/*
+ * Returns 0 on succesful device removal, 1 if a timeout
+ * happenned and ERROR_* if there was an error
+ */
 static int wait_for_dev_destroy(libxl__gc *gc, struct timeval *tv)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
@@ -375,27 +379,47 @@ static int wait_for_dev_destroy(libxl__g
     fd_set rfds;
     char **l1 = NULL;
 
+start:
     rc = 1;
     nfds = xs_fileno(ctx->xsh) + 1;
     FD_ZERO(&rfds);
     FD_SET(xs_fileno(ctx->xsh), &rfds);
-    if (select(nfds, &rfds, NULL, NULL, tv) > 0) {
-        l1 = xs_read_watch(ctx->xsh, &n);
-        if (l1 != NULL) {
-            char *state = libxl__xs_read(gc, XBT_NULL, l1[XS_WATCH_PATH]);
-            if (!state || atoi(state) == 6) {
-                xs_unwatch(ctx->xsh, l1[0], l1[1]);
-                xs_rm(ctx->xsh, XBT_NULL, l1[XS_WATCH_TOKEN]);
-                LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Destroyed device backend at 
%s", l1[XS_WATCH_TOKEN]);
-                rc = 0;
+    switch (select(nfds, &rfds, NULL, NULL, tv)) {
+        case -1:
+            if (errno == EINTR)
+                goto start;
+            rc = ERROR_FAIL;
+            break;
+        case 0:
+            rc = 1;
+            break;
+        case 1:
+            l1 = xs_read_watch(ctx->xsh, &n);
+            if (l1 != NULL) {
+                char *state = libxl__xs_read(gc, XBT_NULL,
+                                             l1[XS_WATCH_PATH]);
+                if (!state || atoi(state) == 6) {
+                    xs_unwatch(ctx->xsh, l1[0], l1[1]);
+                    xs_rm(ctx->xsh, XBT_NULL, l1[XS_WATCH_TOKEN]);
+                    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
+                               "Destroyed device backend at %s",
+                               l1[XS_WATCH_TOKEN]);
+                    rc = 0;
+                }
+                free(l1);
+            } else {
+                rc = ERROR_FAIL;
             }
-            free(l1);
-        }
+            break;
     }
     return rc;
 }
 
-/* Returns 0 on success, ERROR_* on fail */
+/* 
+ * Returns 0 on success, ERROR_* on fail and 1 if there's an event
+ * pending (a device has been added to a watch to wait for
+ * disconnection)
+ */
 int libxl__device_remove(libxl__gc *gc, libxl__device *dev, int wait)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
@@ -428,16 +452,15 @@ retry_transaction:
 
     xs_watch(ctx->xsh, state_path, be_path);
     libxl__device_destroy_tapdisk(gc, be_path);
+    rc = 1;
 
     if (wait) {
         struct timeval tv;
         tv.tv_sec = LIBXL_DESTROY_TIMEOUT;
         tv.tv_usec = 0;
-        (void)wait_for_dev_destroy(gc, &tv);
+        rc = wait_for_dev_destroy(gc, &tv);
         xs_rm(ctx->xsh, XBT_NULL, libxl__device_frontend_path(gc, dev));
     }
-
-    rc = 0;
 out:
     return rc;
 }
@@ -496,7 +519,7 @@ int libxl__devices_destroy(libxl__gc *gc
                 if (force) {
                     libxl__device_destroy(gc, &dev);
                 } else {
-                    if (libxl__device_remove(gc, &dev, 0) == 0)
+                    if (libxl__device_remove(gc, &dev, 0) == 1)
                         n_watches++;
                 }
             }
@@ -515,7 +538,7 @@ int libxl__devices_destroy(libxl__gc *gc
         if (force) {
             libxl__device_destroy(gc, &dev);
         } else {
-            if (libxl__device_remove(gc, &dev, 0) == 0)
+            if (libxl__device_remove(gc, &dev, 0) == 1)
                 n_watches++;
         }
     }
@@ -531,7 +554,7 @@ int libxl__devices_destroy(libxl__gc *gc
         tv.tv_usec = 0;
         while (n_watches > 0) {
             if (wait_for_dev_destroy(gc, &tv)) {
-                break;
+                continue;
             } else {
                 n_watches--;
             }

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