Re: [Qemu-devel] [PATCH 2/2] v2 Fix Block Hotplug race with drive_unplug()

[Markus Armbruster]

Kevin Wolf <address@hidden> writes:

> Am 21.10.2010 23:37, schrieb Ryan Harper:
>> * Daniel P. Berrange <address@hidden> [2010-10-21 08:29]:
>>> On Tue, Oct 19, 2010 at 09:32:29AM -0500, Ryan Harper wrote:
>>>> Block hot unplug is racy since the guest is required to acknowlege the ACPI
>>>> unplug event; this may not happen synchronously with the device removal 
>>>> command
>>>>
>>>> This series aims to close a gap where by mgmt applications that assume the
>>>> block resource has been removed without confirming that the guest has
>>>> acknowledged the removal may re-assign the underlying device to a second 
>>>> guest
>>>> leading to data leakage.
>>>>
>>>> This series introduces a new montor command to decouple asynchornous device
>>>> removal from restricting guest access to a block device.  We do this by 
>>>> creating
>>>> a new monitor command drive_unplug which maps to a bdrv_unplug() command 
>>>> which
>>>> does a qemu_aio_flush; bdrv_flush() and bdrv_close().  Once complete, 
>>>> subsequent
>>>> IO is rejected from the device and the guest will get IO errors but 
>>>> continue to
>>>> function.
>>>>
>>>> A subsequent device removal command can be issued to remove the device, to 
>>>> which
>>>> the guest may or maynot respond, but as long as the unplugged bit is set, 
>>>> no IO
>>>> will be sumbitted.
>>>
>>> The name 'drive_unplug' suggests to me that the drive object is
>>> not being deleted/free()d ? Is that correct understanding, and if
>>> so, what is responsible for finally free()ing the drive backend ?
>> 
>> It's technically the BlockDriverState Driver that we're closing.  To
>> fully release the remaining resources, a device_del is required (which
>> of course requires guest participation with the current
>> interface).
>
> So is this basically what blockdev_del is supposed to become one day?
>
> Copying Markus to have a look at this. I'm sure he has some thoughts on
> it as he was planning to implement blockdev_add/del.

Yes, Ryan's drive_unplug is quite close to my blockdev_del.  However, my
blockdev_del is part of a more ambitious job, namely to cleanly separate
host and guest part of block devices.  A whole lot of preliminary
cleanups have made it in so far, but not the actual commands.

I'll reply in more detail to the latest version of the patch series.

[...]