qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
Date: Fri, 29 Oct 2010 18:10:25 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux)

Ryan Harper <address@hidden> writes:

> * 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 */

Fails in my tree, because the blockdev's still in use.  See below.

> 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

I hate the automatic deletion of host part along with the guest part.
device_del should undo device_add.  {block,net,char}dev_{add,del} should
be similarly paired.

In my blockdev branch, I keep the automatic delete only for backwards
compatibility: if you create the drive with drive_add, it gets
auto-deleted, but if you use blockdev_add, it stays around.

>> 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.

Can we agree on a common solution for block and net?  That's why I cc'ed
Michael.

Currently, we have two different ways:

* The netdev way: "del" always succeeds

  How can it succeed if the host part is in use?

  If all device models are prepared to deal with a missing host part, we
  can delete it right away.

  Else, we need to replace it with a suitable zombie, which is
  auto-deleted when it goes out of use.  Such zombies are not be visible
  elsewhere, in particular, the ID becomes available immediately.

* The unplug way: "del" fails while in use, "unplug" always succeeds

  Feels a bit cleaner to me.  But changing netdev_del might not be
  acceptable.

Either way works for me as an user interface.  But I'd rather not have
both.

Next, we need to consider how to integrate this with the automatic
deletion of drives on qdev destruction.  That's too late for unplug, we
want that right in device_del.  I'd leave the stupid automatic delete
where it is now, in qdev destruction.  The C API need unplug and delete
separate for that.


Regardless of the way we choose, we need to think very clearly on how
exactly device models should behave when their host part is missing or a
zombie, and how that behavior appears in the guest.

For net, making it look exactly like a yanked out network cable would
make sense to me.

What about block?



reply via email to

[Prev in Thread] Current Thread [Next in Thread]