Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal

[Ryan Harper]

* Markus Armbruster <address@hidden> [2010-10-29 09:13]:
> [Note cc: Michael]
> 
> Ryan Harper <address@hidden> writes:
> 
> > This patch series decouples the detachment of a block device from the 
> > removal
> > of the backing pci-device.  Removal of a hotplugged pci device requires the
> > guest to respond before qemu tears down the block device. In some cases, the
> > guest may not respond leaving the guest with continued access to the block
> > device.  
> >
> > The new monitor command, drive_unplug, will revoke a guests access to the
> > block device independently of the removal of the pci device.
> >
> > The first patch adds a new drive find method, the second patch implements 
> > the
> > monitor command and block layer changes.
> >
> > Changes since v3:
> > - Moved QMP command for drive_unplug() to separate patch
> >
> > Changes since v2:
> > - Added QMP command for drive_unplug()
> >
> > Changes since v1:
> > - CodingStyle fixes
> > - Added qemu_aio_flush() to bdrv_unplug()
> >
> > Signed-off-by: Ryan Harper <address@hidden>
> 
> If I understand your patch correctly, the difference between your
> drive_unplug and my blockdev_del is as follows:
> 
> * drive_unplug forcefully severs the connection between the host part of
>   the block device and its BlockDriverState.  A shell of the host part
>   remains, to be cleaned up later.  You need forceful disconnect
>   operation to be able to revoke access to an image whether the guest
>   cooperates or not.  Fair enough.
> 
> * blockdev_del deletes a host part.  My current version fails when the
>   host part is in use.  I patterned that after netdev_del, which used to
>   work that way, until commit 2ffcb18d:
> 
>     Make netdev_del delete the netdev even when it's in use
>     
>     To hot-unplug guest and host part of a network device, you do:
>     
>         device_del NIC-ID
>         netdev_del NETDEV-ID
>     
>     For PCI devices, device_del merely tells ACPI to unplug the device.
>     The device goes away for real only after the guest processed the ACPI
>     unplug event.
>     
>     You have to wait until then (e.g. by polling info pci) before you can
>     unplug the netdev.  Not good.
>     
>     Fix by removing the "in use" check from do_netdev_del().  Deleting a
>     netdev while it's in use is safe; packets simply get routed to the bit
>     bucket.
> 
>   Isn't this the very same problem that's behind your drive_unplug?

Yes it is.

> 
> I'd like to have some consistency among net, block and char device
> commands, i.e. a common set of operations that work the same for all of
> them.  Can we agree on such a set?

Yeah; the current trouble (or at least what I perceive to be trouble) is
that in the case where the guest responds to device_del induced ACPI
removal event; the current qdev code already does the host-side device
tear down.  Not sure if it is OK to do a blockdev_del() immediately
after the device_del.  What happens when we do:

device_del
ACPI to guest
blockdev_del /* removes host-side device */
guest responds to ACPI
qdev calls pci device removal code
qemu attempts to destroy the associated host-side block

That may just work today; and if not, it shouldn't be hard to fix up the
code to check for NULLs

> 
> Even if your drive_unplug shouldn't fit in that set, we might want it as
> a stop-gap.  Depends on how urgent the need for it is.  Yet another
> special-purpose command to be deprecated later.

The fix is urgent; but I'm willing to spin a couple patches if it helps
get this into better shape.


-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
address@hidden