From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=46938 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PBqUb-00080b-Fk for qemu-devel@nongnu.org; Fri, 29 Oct 2010 11:03:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PBqUa-0006Wv-4Q for qemu-devel@nongnu.org; Fri, 29 Oct 2010 11:03:45 -0400 Received: from e3.ny.us.ibm.com ([32.97.182.143]:35278) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PBqUa-0006WU-04 for qemu-devel@nongnu.org; Fri, 29 Oct 2010 11:03:44 -0400 Received: from d01relay01.pok.ibm.com (d01relay01.pok.ibm.com [9.56.227.233]) by e3.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id o9TEkf8B007026 for ; Fri, 29 Oct 2010 10:46:41 -0400 Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by d01relay01.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id o9TF3ffg393266 for ; Fri, 29 Oct 2010 11:03:41 -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 o9TF3fMc014063 for ; Fri, 29 Oct 2010 13:03:41 -0200 Date: Fri, 29 Oct 2010 10:03:36 -0500 From: Ryan Harper Subject: Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal Message-ID: <20101029150336.GJ22904@us.ibm.com> References: <1288030956-28383-1-git-send-email-ryanh@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 09:13]: > [Note cc: Michael] > > Ryan Harper 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 > > 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 ryanh@us.ibm.com