Re: [Xen-devel] [PATCH] libxl: don't set backend state to 5 when trying to unplug a device

[Roger Pau Monnà <roger.pau@xxxxxxxxxxxxx>]

2012/1/13 Ian Campbell <Ian.Campbell@xxxxxxxxxx>:
> On Fri, 2012-01-13 at 11:57 +0000, Roger Pau Monne wrote:
>> # HG changeset patch
>> # User Roger Pau Monne <roger.pau@xxxxxxxxxxxxx>
>> # Date 1326454785 -3600
>> # Node ID 887a3229fd7a50c04981e29709bc7210dafef38f
>> # Parent Â5b2676ac13218951698c49fa0350f2ac48220f3d
>> libxl: don't set backend state to 5 when trying to unplug a device
>>
>> libxl__device_remove was setting backend state to 5, which could
>> create a race condition, since the previous check for state != 4 and
>> setting state to 5 was not done inside of the same transaction, so the
>> kernel could change the state to 6 in the space between the check for
>> state != 4 and setting state to 5.
>>
>> I might be wrong, but since I don't think setting backend state to 5
>> helps in any way when disconnecting a device
>
> It's the exact thing which makes the disconnect happen at all, isn't it?

What makes the disconnect happen on NetBSD al least is removing the
frontend or setting the frontend state to 6, but this doesn't do
anything at all (it might be different on Linux though). Can someone
confirm that this is actually useful on Linux?

> Some backends (particularly the Linux ones) might also use the online
> node but I don't think that behaviour is universal.
>
>> ÂI've just removed the
>> xs_write. ÂIf this is necessary, the state != 4 check and setting it
>> to 5 should happen inside the same transaction, to avoid the kernel
>> from changing the state behind our back.
>
> I think that is the right solution.
>
> Ian.
>
>> Signed-off-by: Roger Pau Monne <roger.pau@xxxxxxxxxxxxx>
>>
>> diff -r 5b2676ac1321 -r 887a3229fd7a tools/libxl/libxl_device.c
>> --- a/tools/libxl/libxl_device.c   ÂMon Jan 09 16:01:44 2012 +0100
>> +++ b/tools/libxl/libxl_device.c   ÂFri Jan 13 12:39:45 2012 +0100
>> @@ -456,7 +456,6 @@ int libxl__device_remove(libxl__gc *gc,
>> Âretry_transaction:
>> Â Â Ât = xs_transaction_start(ctx->xsh);
>> Â Â Âxs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/online", be_path), "0", 
>> strlen("0"));
>> - Â Âxs_write(ctx->xsh, t, state_path, "5", strlen("5"));
>> Â Â Âif (!xs_transaction_end(ctx->xsh, t, 0)) {
>> Â Â Â Â Âif (errno == EAGAIN)
>> Â Â Â Â Â Â Âgoto retry_transaction;
>>
>> _______________________________________________
>> 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