]> xenbits.xensource.com Git - people/dstodden/blktap.git/commitdiff
CA-33664: Close unplug/plug race window since CA-32254.
authorDaniel Stodden <daniel.stodden@citrix.com>
Tue, 13 Oct 2009 22:07:50 +0000 (15:07 -0700)
committerDaniel Stodden <daniel.stodden@citrix.com>
Tue, 13 Oct 2009 22:07:50 +0000 (15:07 -0700)
Race window in tapdisk_daemon_probe_vb if VBD recreation is observered
before we managed to close the previous channel.

We don't permit duplicate channels on the same XS keys. Instead add
TAPDISK_VBD_RECYCLED, which indicates a channel whose vbd state went
one step beyond DEAD, pending a synchronous re-probe.

Trying to avoid spurious probe failures, we cannot just rerun probe()
after destruction. Instead, perform a test/write transaction cycle.
We abort in the unlikely case where we destroy the channel after the
backend path was already gone again.

The related case of observing a remove event in RECYCLED state would
drive us back to DEAD, which should work okay.

daemon/tapdisk-channel.c
daemon/tapdisk-daemon.c
daemon/tapdisk-dispatch.h

index 1ec1c1e41425649608a92af5ecf4b39e4349cbba..670e4ac58e317410ce2bf31ab94b47e838d7aa59 100644 (file)
@@ -85,6 +85,8 @@ tapdisk_channel_vbd_state_name(vbd_state_t state)
                return "broken";
        case TAPDISK_VBD_DEAD:
                return "dead";
+       case TAPDISK_VBD_RECYCLED:
+               return "recycled";
        }
 
        return "unknown";
@@ -141,7 +143,7 @@ static int tapdisk_channel_connect(tapdisk_channel_t *);
 static void tapdisk_channel_close_tapdisk(tapdisk_channel_t *);
 static void tapdisk_channel_destroy(tapdisk_channel_t *);
 
-static int
+int
 tapdisk_channel_check_uuid(tapdisk_channel_t *channel)
 {
        uint32_t uuid;
@@ -663,6 +665,65 @@ tapdisk_channel_send_pause_request(tapdisk_channel_t *channel)
        return tapdisk_channel_send_message(channel, &message, 2);
 }
 
+static int
+tapdisk_channel_trigger_reprobe(tapdisk_channel_t *channel)
+{
+       xs_transaction_t xbt;
+       int err, abort = 0;
+       unsigned int len;
+       void *data;
+       bool ok;
+
+       /*
+        * NB. Kick the probe watch, paranoia-style. Performing an
+        * atomic test/set on the directory path. Abort if it's
+        * already gone again. Accidentally recreating the node would
+        * lead to a spurious start failure.
+        */
+
+again:
+       xbt = xs_transaction_start(channel->xsh);
+       if (!xbt) {
+               err = -errno;
+               EPRINTF("error starting transaction: %d\n", err);
+               return err;
+       }
+
+       data = xs_read(channel->xsh, xbt, channel->path, &len);
+       if (!data) {
+               err = -errno;
+               abort = err == -ENOENT;
+               if (err && !abort) {
+                       EPRINTF("error reading %s: %d\n",
+                               channel->path, err);
+                       return err;
+               }
+       }
+       free(data);
+
+       if (!abort) {
+               DPRINTF("write %s\n", channel->path);
+               ok = xs_write(channel->xsh, xbt, channel->path, "", 0);
+               if (!ok) {
+                       err = -errno;
+                       EPRINTF("error writing %s: %d\n",
+                               channel->path, err);
+                       return err;
+               }
+       }
+
+       ok = xs_transaction_end(channel->xsh, xbt, abort);
+       if (!ok) {
+               err = -errno;
+               if (err == -EAGAIN && !abort)
+                       goto again;
+               EPRINTF("error ending transaction: %d\n", err);
+               return err;
+       }
+
+       return 0;
+}
+
 static int
 tapdisk_channel_signal_paused(tapdisk_channel_t *channel)
 {
@@ -937,6 +998,7 @@ tapdisk_channel_map_vbd_state(tapdisk_channel_t *channel)
 
                case TAPDISK_VBD_BROKEN:
                case TAPDISK_VBD_DEAD:
+               case TAPDISK_VBD_RECYCLED:
                        return TAPDISK_CHANNEL_CLOSED;
 
                default:
@@ -1000,6 +1062,9 @@ tapdisk_channel_drive_vbd_state(tapdisk_channel_t *channel)
                tapdisk_channel_signal_paused(channel);
                break;
 
+       case TAPDISK_VBD_RECYCLED:
+               tapdisk_channel_trigger_reprobe(channel);
+               /* then destroy */
        case TAPDISK_VBD_DEAD:
                tapdisk_channel_destroy(channel);
        }
index 4b3d24565c1f4742a84e8fb617e03b5fb88ddb82..4f48fa99002072592aa16b2e3562dc6c659a0365 100644 (file)
@@ -327,6 +327,19 @@ tapdisk_daemon_probe_vbd(int domid, int busid, const char *path)
 
        channel = tapdisk_daemon_find_channel(domid, busid);
        if (channel) {
+               err = tapdisk_channel_check_uuid(channel);
+               if (err == -ENOENT) {
+                       /* NB. Fast unplug/plug and we missed the path
+                          removal. Typically a dom0 phenonmenon */
+                       DPRINTF("%s: pending re-probe:"
+                               " channel %d:%d, state %d\n",
+                               path, channel->channel_id, channel->cookie, 
+                               channel->vbd_state);
+                       channel->vbd_state = TAPDISK_VBD_RECYCLED;
+                       tapdisk_channel_drive_vbd_state(channel);
+                       return;
+               }
+
                DPRINTF("%s: ignoring duplicate probe event:"
                        " channel %d:%d, state %d\n",
                        path, channel->channel_id, channel->cookie, 
index 5f3b006063c0c972b0f4835d08ece2f73dcef2a7..9ed9588527d81b6aaea0b3c2c75a71e8d4ab7f45 100644 (file)
@@ -62,6 +62,7 @@ typedef enum {
        TAPDISK_VBD_PAUSED          = 3,
        TAPDISK_VBD_BROKEN          = 4,
        TAPDISK_VBD_DEAD            = 5,
+       TAPDISK_VBD_RECYCLED        = 6,
 } vbd_state_t;
 
 typedef enum {
@@ -129,5 +130,6 @@ int tapdisk_channel_receive_message(tapdisk_channel_t *, tapdisk_message_t *);
 void tapdisk_channel_reap(tapdisk_channel_t *, int status);
 int tapdisk_channel_change_vbd_state(tapdisk_channel_t *, vbd_state_t);
 void tapdisk_channel_drive_vbd_state(tapdisk_channel_t *);
+int tapdisk_channel_check_uuid(tapdisk_channel_t *);
 
 #endif