On 03.06.2011 10:03, Ian Campbell wrote:
> 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.
Only backend one...
> 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?
Also - only backend one (libxl__device_destroy gets only backend path as
parameter).
> 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.
What about frontend device in xenstore? In most cases only backend is
removed. In wait_for_dev_destroy we don't know frontend patch (wheel,
not exactly true, but not always easy). Perhaps there should be two
watches - on backend AND on frontend (with writing "5" to frontend state
also?)? Or backend should be removed as you suggested, but frontend as
in my patch (after wait_for_dev_destroy if was called)?
--
Pozdrawiam / Best Regards,
Marek Marczykowski | RLU #390519
marmarek at mimuw edu pl | xmpp:marmarek at staszic waw pl
smime.p7s
Description: S/MIME Cryptographic Signature
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home