On Thu, 2011-09-22 at 13:01 +0100, Roger Pau Monne wrote:
> # HG changeset patch
> # User Roger Pau Monne <roger.pau@xxxxxxxxxxxxx>
> # Date 1316692512 -7200
> # Node ID 9e9c68768676b0e1c9c76599e00d0922c6184602
> # Parent a5c9a6083bef9d4789a0194389db234740cd6005
> libxl: use hotplug-status to synchronize the destruction of block devices
>
> Use hotplug-status to synchronize the destruction of vbd devices instead of
> state, which did not reflect when the hotplug script had been executed.
>
> Signed-off-by: Roger Pau Monne <roger.pau@xxxxxxxxxxxxx>
I wonder if we can remove the hard coding of the VBD special case
somehow? For example an ops struct in an array mapping DEVICE_* to
int (*watch_for_disconnect)(relevant arguments)
int (*has_disconnected)(relevant arguments)
which would have two implementations, one checking state and the other
hotplug-status.
That leaves the change in wait_for_dev_destroy which unfortunately
doesn't have the context to do much else than what you've done. Perhaps
that's ok as is until I can finish of the patch series I have which
fixes that aspect. I suppose a function pointer could be encoded in the
XS_WATCH_TOKEN and sscanf'd back out again...
>
> diff -r a5c9a6083bef -r 9e9c68768676 tools/libxl/libxl_device.c
> --- a/tools/libxl/libxl_device.c Thu Sep 22 13:55:03 2011 +0200
> +++ b/tools/libxl/libxl_device.c Thu Sep 22 13:55:12 2011 +0200
> @@ -371,14 +371,26 @@ int libxl__device_destroy(libxl__gc *gc,
> libxl_ctx *ctx = libxl__gc_owner(gc);
> xs_transaction_t t;
> char *state_path = libxl__sprintf(gc, "%s/state", be_path);
> + char *hotplug_path = libxl__sprintf(gc, "%s/hotplug-status", be_path);
> char *state = libxl__xs_read(gc, XBT_NULL, state_path);
> + char *hotplug = libxl__xs_read(gc, XBT_NULL, hotplug_path);
> int rc = 0;
>
> if (!state)
> goto out;
> - if (atoi(state) != 4) {
> - xs_rm(ctx->xsh, XBT_NULL, be_path);
> - goto out;
> +
> + if (!strstr(be_path, string_of_kinds[DEVICE_VBD])) {
> + if (atoi(state) != 4) {
> + xs_rm(ctx->xsh, XBT_NULL, be_path);
> + goto out;
> + }
> + } else {
> + if (!hotplug)
> + goto out;
> + if (!strcmp(hotplug, "disconnected")) {
> + xs_rm(ctx->xsh, XBT_NULL, be_path);
> + goto out;
> + }
> }
>
> retry_transaction:
> @@ -394,8 +406,13 @@ retry_transaction:
> }
> }
> if (!force) {
> - xs_watch(ctx->xsh, state_path, be_path);
> - rc = 1;
> + if (strstr(be_path, string_of_kinds[DEVICE_VBD])) {
> + xs_watch(ctx->xsh, hotplug_path, be_path);
> + rc = 1;
> + } else {
> + xs_watch(ctx->xsh, state_path, be_path);
> + rc = 1;
> + }
> } else {
> xs_rm(ctx->xsh, XBT_NULL, be_path);
> }
> @@ -410,6 +427,7 @@ static int wait_for_dev_destroy(libxl__g
> unsigned int n;
> fd_set rfds;
> char **l1 = NULL;
> + char *state, *hotplug;
>
> rc = 1;
> nfds = xs_fileno(ctx->xsh) + 1;
> @@ -418,12 +436,26 @@ static int wait_for_dev_destroy(libxl__g
> 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;
> + if (strstr(l1[XS_WATCH_PATH], "hotplug-status")) {
> + hotplug = libxl__xs_read(gc, XBT_NULL, l1[XS_WATCH_PATH]);
> + if (!hotplug || !strcmp(hotplug, "disconnected")) {
> + 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 hotplug-status: %s",
> + l1[XS_WATCH_TOKEN], hotplug);
> + rc = 0;
> + }
> + } else {
> + 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);
> }
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|