[Xen-devel] [PATCH v2] libxl: Atomicaly check backend state and set it to 5 at device_remove

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

# HG changeset patch
# User Roger Pau Monne <roger.pau@xxxxxxxxxxxxx>
# Date 1326548456 -3600
# Node ID 1b95948228b84c986764488cbb7683d0d109be90
# Parent  5b2676ac13218951698c49fa0350f2ac48220f3d
libxl: Atomicaly check backend state and set it to 5 at device_remove

libxl__device_remove was setting backend state to 5, which could
create a race condition, since the previous check for state != 4 and
setting state to 5 was not done inside the same transaction, so the
kernel could change the state to 6 in the space between the check for
state != 4 and setting it to 5.

The state != 4 check and setting it to 5 should happen in the same
transaction, to assure that nobody is modifying it behind our back.

Changes since v1:

 * Do the check and set in the same transaction, instead of removing
   the set state to 5.

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

diff -r 5b2676ac1321 -r 1b95948228b8 tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c        Mon Jan 09 16:01:44 2012 +0100
+++ b/tools/libxl/libxl_device.c        Sat Jan 14 14:40:56 2012 +0100
@@ -442,19 +442,23 @@ int libxl__device_remove(libxl__gc *gc, 
     xs_transaction_t t;
     char *be_path = libxl__device_backend_path(gc, dev);
     char *state_path = libxl__sprintf(gc, "%s/state", be_path);
-    char *state = libxl__xs_read(gc, XBT_NULL, state_path);
+    char *state;
     int rc = 0;
 
-    if (!state)
+retry_transaction:
+    t = xs_transaction_start(ctx->xsh);
+    state = libxl__xs_read(gc, t, state_path);
+    if (!state) {
+        xs_transaction_end(ctx->xsh, t, 0);
         goto out;
+    }
     if (atoi(state) != 4) {
+        xs_transaction_end(ctx->xsh, t, 0);
         libxl__device_destroy_tapdisk(gc, be_path);
         xs_rm(ctx->xsh, XBT_NULL, be_path);
         goto out;
     }
 
-retry_transaction:
-    t = xs_transaction_start(ctx->xsh);
     xs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/online", be_path), "0", 
strlen("0"));
     xs_write(ctx->xsh, t, state_path, "5", strlen("5"));
     if (!xs_transaction_end(ctx->xsh, t, 0)) {

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