[Xen-devel] [PATCH 2/3] libxl: fix hang due to libxl__initiate_device_remove

[Ian Jackson <ian.jackson@xxxxxxxxxxxxx>]

libxl__initiate_device_remove might discover that the operation was
complete, immediately (typically, if the device is already removed).

Previously, in this situation, it would return 0 to the caller but
never call libxl__ao_complete.  Fix this.  This necessitates passing
the egc in from the functions which are the ao initiators.

Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
---
 tools/libxl/libxl.c          |    8 ++++----
 tools/libxl/libxl_device.c   |   18 ++++++++++++------
 tools/libxl/libxl_internal.h |    3 ++-
 3 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 68bba8f..7735223 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1362,7 +1362,7 @@ int libxl_device_disk_remove(libxl_ctx *ctx, uint32_t 
domid,
     rc = libxl__device_from_disk(gc, domid, disk, &device);
     if (rc != 0) goto out;
 
-    rc = libxl__initiate_device_remove(ao, &device);
+    rc = libxl__initiate_device_remove(egc, ao, &device);
     if (rc) goto out;
 
     return AO_INPROGRESS;
@@ -1815,7 +1815,7 @@ int libxl_device_nic_remove(libxl_ctx *ctx, uint32_t 
domid,
     rc = libxl__device_from_nic(gc, domid, nic, &device);
     if (rc != 0) goto out;
 
-    rc = libxl__initiate_device_remove(ao, &device);
+    rc = libxl__initiate_device_remove(egc, ao, &device);
     if (rc) goto out;
 
     return AO_INPROGRESS;
@@ -2159,7 +2159,7 @@ int libxl_device_vkb_remove(libxl_ctx *ctx, uint32_t 
domid,
     rc = libxl__device_from_vkb(gc, domid, vkb, &device);
     if (rc != 0) goto out;
 
-    rc = libxl__initiate_device_remove(ao, &device);
+    rc = libxl__initiate_device_remove(egc, ao, &device);
     if (rc) goto out;
 
     return AO_INPROGRESS;
@@ -2286,7 +2286,7 @@ int libxl_device_vfb_remove(libxl_ctx *ctx, uint32_t 
domid,
     rc = libxl__device_from_vfb(gc, domid, vfb, &device);
     if (rc != 0) goto out;
 
-    rc = libxl__initiate_device_remove(ao, &device);
+    rc = libxl__initiate_device_remove(egc, ao, &device);
     if (rc) goto out;
 
     return AO_INPROGRESS;
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index c2880e0..c7e057d 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -376,7 +376,8 @@ static void device_remove_callback(libxl__egc *egc, 
libxl__ev_devstate *ds,
     device_remove_cleanup(gc, aorm);
 }
 
-int libxl__initiate_device_remove(libxl__ao *ao, libxl__device *dev)
+int libxl__initiate_device_remove(libxl__egc *egc, libxl__ao *ao,
+                                  libxl__device *dev)
 {
     AO_GC;
     libxl_ctx *ctx = libxl__gc_owner(gc);
@@ -388,11 +389,11 @@ int libxl__initiate_device_remove(libxl__ao *ao, 
libxl__device *dev)
     libxl__ao_device_remove *aorm = 0;
 
     if (!state)
-        goto out;
+        goto out_ok;
     if (atoi(state) != 4) {
         libxl__device_destroy_tapdisk(gc, be_path);
         xs_rm(ctx->xsh, XBT_NULL, be_path);
-        goto out;
+        goto out_ok;
     }
 
 retry_transaction:
@@ -404,7 +405,7 @@ retry_transaction:
             goto retry_transaction;
         else {
             rc = ERROR_FAIL;
-            goto out;
+            goto out_fail;
         }
     }
 
@@ -417,13 +418,18 @@ retry_transaction:
     rc = libxl__ev_devstate_wait(gc, &aorm->ds, device_remove_callback,
                                  state_path, XenbusStateClosed,
                                  LIBXL_DESTROY_TIMEOUT * 1000);
-    if (rc) goto out;
+    if (rc) goto out_fail;
 
     return 0;
 
- out:
+ out_fail:
+    assert(rc);
     device_remove_cleanup(gc, aorm);
     return rc;
+
+ out_ok:
+    libxl__ao_complete(egc, ao, 0);
+    return 0;
 }
 
 int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 49688e1..696c1ed 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -669,7 +669,8 @@ _hidden int libxl__wait_for_backend(libxl__gc *gc, char 
*be_path, char *state);
  * this is done, the ao will be completed.  An error
  * return from libxl__initiate_device_remove means that the ao
  * will _not_ be completed and the caller must do so. */
-_hidden int libxl__initiate_device_remove(libxl__ao*, libxl__device *dev);
+_hidden int libxl__initiate_device_remove(libxl__egc*, libxl__ao*,
+                                          libxl__device *dev);
 
 /*
  * libxl__ev_devstate - waits a given time for a device to
-- 
1.7.2.5


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