From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=34414 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PBsAP-00007z-QW for qemu-devel@nongnu.org; Fri, 29 Oct 2010 12:51:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PBsAH-0004iO-Fu for qemu-devel@nongnu.org; Fri, 29 Oct 2010 12:50:54 -0400 Received: from e6.ny.us.ibm.com ([32.97.182.146]:45234) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PBsAH-0004iA-Bw for qemu-devel@nongnu.org; Fri, 29 Oct 2010 12:50:53 -0400 Received: from d01relay06.pok.ibm.com (d01relay06.pok.ibm.com [9.56.227.116]) by e6.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id o9TGpYnP010110 for ; Fri, 29 Oct 2010 12:51:34 -0400 Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by d01relay06.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id o9TGomW71876144 for ; Fri, 29 Oct 2010 12:50:48 -0400 Received: from d01av02.pok.ibm.com (loopback [127.0.0.1]) by d01av02.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id o9TGomY1031483 for ; Fri, 29 Oct 2010 14:50:48 -0200 Date: Fri, 29 Oct 2010 11:50:44 -0500 From: Ryan Harper Subject: Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal Message-ID: <20101029165044.GM22904@us.ibm.com> References: <1288030956-28383-1-git-send-email-ryanh@us.ibm.com> <20101029150336.GJ22904@us.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Kevin Wolf , "Michael S. Tsirkin" , qemu-devel@nongnu.org, Anthony Liguori , Ryan Harper , Stefan Hajnoczi * Markus Armbruster [2010-10-29 11:11]: > Ryan Harper writes: > > > * Markus Armbruster [2010-10-29 09:13]: > >> [Note cc: Michael] > >> > >> Ryan Harper writes: > >> > >> > >> 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. Agreed. > > 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. But what to do about the case where we're doing drive_add and then a device_del() That's the urgent situation that needs to be resolved. > > >> 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. I didn't see a good way to have block behave the same as net; though I do agree that it would be good to have this be common, long term. > > 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? It seems to me that for block it's like cdrom with no disk, floppy with no media, hard disk that's gone bad. I think we we throw EIO back; it's handled gracefully enough. This is what happens when you do a drive_unplug with my patch; the application using the device gets IO errors. That's expected if a drive were to suddently fail (which is what this looks like). And certainly there is some responsibility at the mgmt console to ensure you're not unplugging a drive that you are currently using. -- Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx ryanh@us.ibm.com