From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53068) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WVPMG-0001W4-Ia for qemu-devel@nongnu.org; Wed, 02 Apr 2014 13:53:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WVPMA-0006wI-Cz for qemu-devel@nongnu.org; Wed, 02 Apr 2014 13:53:52 -0400 Received: from mx1.redhat.com ([209.132.183.28]:25996) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WVPMA-0006w0-5P for qemu-devel@nongnu.org; Wed, 02 Apr 2014 13:53:46 -0400 From: Markus Armbruster References: <1396360537-12520-1-git-send-email-graalfs@linux.vnet.ibm.com> <87eh1h8304.fsf@blackfin.pond.sub.org> <533C1DC5.6050605@linux.vnet.ibm.com> Date: Wed, 02 Apr 2014 19:40:09 +0200 In-Reply-To: <533C1DC5.6050605@linux.vnet.ibm.com> (Heinz Graalfs's message of "Wed, 02 Apr 2014 16:25:09 +0200") Message-ID: <87d2gzpr46.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] drive_del vs. device_del: what should come first? List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Heinz Graalfs Cc: qemu-devel@nongnu.org Heinz Graalfs writes: > On 01/04/14 17:48, Markus Armbruster wrote: >> Heinz Graalfs writes: >> >>> Hi Kevin, >>> >>> doing a >>> >>> virsh detach-device ... >>> >>> ends up in the following QEMU monitor commands: >>> >>> 1. device_del ... >>> 2. drive_del ... >>> >>> qmp_device_del() performs the device unplug path. >>> In case of a block device do_drive_del() tries to >>> prevent further IO against the host device. >>> >>> However, bdrv_find() during drive_del() results in >>> an error, because the device is already gone. Due to >>> this error all the bdrv_xxx calls to quiesce the block >>> driver as well as all other processing is skipped. >>> >>> Is the sequence that libvirt triggers OK? >>> Shouldn't drive_del be executed first? >> >> No. > > OK, I see. The drive is deleted implicitly (release_drive()). > Doing a device_del() requires another drive_add() AND device_add(). Yes. The automatic drive delete on unplug is a wart that will not be preserved in the new interfaces we're building now. > (Doing just a device_add() complains about the missing drive. > A subsequent info qtree lets QEMU abort.) Really? Got a reproducer for me? >> drive_del is nasty. Its purpose is to revoke access to an image even >> when the guest refuses to cooperate. To the guest, this looks like >> hardware failure. > > Deleting a device during active IO is nasty and it should look like a > hardware failure. I would expect lots of errors. > >> >> If you drive_del before device_del, even a perfectly well-behaved guest >> guest is exposed to a terminally broken device between drive_del and >> completion of unplug. > > The early drive_del() would mean that no further IO would be > possible. A polite way to describe the effect of drive_del on the guest. drive_del makes all attempts at actual I/O error out without any forewarning whatsoever. If you do that to your guest, and something breaks, you get to keep the pieces. Even if you "only" do it for a short period of time. >> Always try a device_del first, and only if that does not complete within >> reasonable time, and you absolutely must revoke access to the image, >> then whack it over the head with drive_del. > > What is this reasonable time? Depends on how heavily loaded host and guest are. Suggest to measure with a suitably thrashing guest, then shift left to taste. > On 390 we experience problems (QEMU abort) when asynch block IO > completes and the virtqueues are already gone. I suppose the > bdrv_drain_all() in bdrv_close() is a little late. I don't see such > problems with an early bdrv_drain_all() (drive_del) and an unplug > (device_del) afterwards. Sounds like a bug. Got a reproducer?