WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH 6 of 6] libxl: use hotplug-status to synchronize

To: Roger Pau Monne <roger.pau@xxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 6 of 6] libxl: use hotplug-status to synchronize the destruction of block devices
From: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
Date: Thu, 22 Sep 2011 14:44:55 +0100
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Thu, 22 Sep 2011 06:45:41 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <9e9c68768676b0e1c9c7.1316692873@loki>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Organization: Citrix Systems, Inc.
References: <patchbomb.1316692867@loki> <9e9c68768676b0e1c9c7.1316692873@loki>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
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