Re: [Xen-devel] [PATCH 01 of 10] libxl: Remove frontend and backend devices from xenstore after destroy

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

Hi Marek,

Please can you add:
[diff]
showfunc = True
to your ~/.hgrc. It makes patches much easier to read.

 On Thu, 2011-06-02 at 23:35 +0100, Marek Marczykowski wrote:
> # HG changeset patch
> # User Marek Marczykowski <marmarek@xxxxxxxxxxxx>
> # Date 1306962865 -7200
> # Node ID e3a3f5cc95349e92b7cb8b1448e999ffc16bd060
> # Parent  43acc031eb24945973dffda2b7caf976993bbd5f
> libxl: Remove frontend and backend devices from xenstore after destroy
> 
> Cleanup frontend and backend devices from xenstore for all dev types - not 
> only
> disks.
> 
> Signed-off-by: Marek Marczykowski <marmarek@xxxxxxxxxxxx>

Thanks!

The whole device destroy/teardown path is a bit of a twisty maze of
confusingly named functions and parameters (e.g. force == !wait, destroy
vs delete vs remove etc). Any attempt to try and untangle things is most
welcome and I think this is a step in the right direction.

> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -1065,8 +1065,6 @@
>      device.devid            = devid;
>      device.kind             = DEVICE_VBD;
>      rc = libxl__device_del(ctx, &device, wait);
> -    xs_rm(ctx->xsh, XBT_NULL, libxl__device_backend_path(&gc, &device));
> -    xs_rm(ctx->xsh, XBT_NULL, libxl__device_frontend_path(&gc, &device));
>      libxl__free_all(&gc);
>      return rc;
>  }
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -401,6 +401,8 @@
>          (void)wait_for_dev_destroy(ctx, &tv);
>      }
>  
> +    xs_rm(ctx->xsh, XBT_NULL, libxl__device_backend_path(&gc, dev));
> +    xs_rm(ctx->xsh, XBT_NULL, libxl__device_frontend_path(&gc, dev));

In the case where wait == true libxl__device_destroy will add a watch on
the be path and wait_for_dev_destroy will subsequently remove the path.

I think it would be better to add the xs_rm for be path to
libxl__device_destroy as the else clause to the !force which adds the
watch? I think this allow the other caller (libxl__devices_destroy) to
be simplified too since all that mucking around with the toremove
flexarray could be dropped.

Ian.

>      rc = 0;
>  
>  out:
> 
> 
> 
> _______________________________________________
> 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