From patchwork Tue Feb 9 15:57:35 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alberto Garcia X-Patchwork-Id: 8263221 Return-Path: X-Original-To: patchwork-qemu-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 86D989F38B for ; Tue, 9 Feb 2016 16:05:37 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id B760F201ED for ; Tue, 9 Feb 2016 16:05:36 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id BD059201E4 for ; Tue, 9 Feb 2016 16:05:35 +0000 (UTC) Received: from localhost ([::1]:57530 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTAnG-0003d4-VS for patchwork-qemu-devel@patchwork.kernel.org; Tue, 09 Feb 2016 11:05:34 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43675) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTAgM-0001H5-Qa for qemu-devel@nongnu.org; Tue, 09 Feb 2016 10:58:28 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aTAgJ-0000nB-Nf for qemu-devel@nongnu.org; Tue, 09 Feb 2016 10:58:26 -0500 Received: from smtp3.mundo-r.com ([212.51.32.191]:50729 helo=smtp4.mundo-r.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTAgJ-0000mq-A9; Tue, 09 Feb 2016 10:58:23 -0500 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: A2CIAwCUC7pW/5tjdVteGQEBAQEPAQEBAYMKgT+IXJ1RAQEBAQEBBQGBDwGQHIITAQ2BZoYNAoE0OBQBAQEBAQEBgQqEQgEBBCdSED8SPBsZiB8Bvm4BAQgghUqCPYsuBYdTjyWNUI5zjj8eAQFCggMZgUpoiFMBAQE X-IPAS-Result: A2CIAwCUC7pW/5tjdVteGQEBAQEPAQEBAYMKgT+IXJ1RAQEBAQEBBQGBDwGQHIITAQ2BZoYNAoE0OBQBAQEBAQEBgQqEQgEBBCdSED8SPBsZiB8Bvm4BAQgghUqCPYsuBYdTjyWNUI5zjj8eAQFCggMZgUpoiFMBAQE X-IronPort-AV: E=Sophos;i="5.22,421,1449529200"; d="scan'208";a="14549295" Received: from fanzine.igalia.com ([91.117.99.155]) by smtp4.mundo-r.com with ESMTP; 09 Feb 2016 16:58:19 +0100 Received: from dsl-hkibrasgw4-50df50-201.dhcp.inet.fi ([80.223.80.201] helo=perseus.local) by fanzine.igalia.com with esmtpsa (Cipher TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim) id 1aTAgF-0001tp-KM; Tue, 09 Feb 2016 16:58:19 +0100 Received: from berto by perseus.local with local (Exim 4.86) (envelope-from ) id 1aTAg2-0001Ux-0z; Tue, 09 Feb 2016 17:58:06 +0200 From: Alberto Garcia To: qemu-devel@nongnu.org Date: Tue, 9 Feb 2016 17:57:35 +0200 Message-Id: <87083486787f4d4b9752f4b29209e6d0d1d31626.1455033112.git.berto@igalia.com> X-Mailer: git-send-email 2.7.0 In-Reply-To: References: In-Reply-To: References: X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 212.51.32.191 Cc: Kevin Wolf , Alberto Garcia , qemu-block@nongnu.org, Max Reitz Subject: [Qemu-devel] [PATCH v2 1/2] block: Allow x-blockdev-del on a BB with a monitor-owned BDS X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP When x-blockdev-del is performed on a BlockBackend that has inserted media it will only succeed if the BDS doesn't have any additional references. The only problem with this is that if the BDS was created separately using blockdev-add then the backend won't be able to be destroyed unless the BDS is ejected first. This is an unnecessary restriction. Now that we have a list of monitor-owned BDSs we can allow x-blockdev-del to work in this scenario if the BDS has exactly one extra reference and that reference is from the monitor. This patch also updates iotest 139 to reflect this change. Both testAttachMedia() and testSnapshot() are split in two: one version keeps the previous behavior, and a second version checks that the new functionality works as expected. Signed-off-by: Alberto Garcia --- blockdev.c | 13 ++++++++++--- tests/qemu-iotests/139 | 30 +++++++++++++++++++++++++----- tests/qemu-iotests/139.out | 4 ++-- 3 files changed, 37 insertions(+), 10 deletions(-) diff --git a/blockdev.c b/blockdev.c index 1f73478..499a96b 100644 --- a/blockdev.c +++ b/blockdev.c @@ -3966,9 +3966,16 @@ void qmp_x_blockdev_del(bool has_id, const char *id, } if (bs->refcnt > 1) { - error_setg(errp, "Block device %s is in use", - bdrv_get_device_or_node_name(bs)); - goto out; + /* We allow deleting a BlockBackend that has a BDS with an + * extra reference if that extra reference is from the + * monitor. */ + bool bs_has_only_monitor_ref = + blk && bs->monitor_list.tqe_prev && bs->refcnt == 2; + if (!bs_has_only_monitor_ref) { + error_setg(errp, "Block device %s is in use", + bdrv_get_device_or_node_name(bs)); + goto out; + } } } diff --git a/tests/qemu-iotests/139 b/tests/qemu-iotests/139 index a4b9694..a6fc299 100644 --- a/tests/qemu-iotests/139 +++ b/tests/qemu-iotests/139 @@ -330,21 +330,32 @@ class TestBlockdevDel(iotests.QMPTestCase): self.delDeviceModel('device0') self.delBlockBackend('drive0', 'node0') - def testAttachMedia(self): + def testAttachMedia1(self): # This creates a BlockBackend and removes its media self.addBlockBackend('drive0', 'node0') self.ejectDrive('drive0', 'node0') # This creates a new BlockDriverState and inserts it into the backend self.addBlockDriverState('node1') self.insertDrive('drive0', 'node1') - # The backend can't be removed: the new BDS has an extra reference - self.delBlockBackend('drive0', 'node1', expect_error = True) + # The BDS can't be removed because it's attached to a backend self.delBlockDriverState('node1', expect_error = True) # The BDS still exists after being ejected, but now it can be removed self.ejectDrive('drive0', 'node1', destroys_media = False) self.delBlockDriverState('node1') self.delBlockBackend('drive0', None) + def testAttachMedia2(self): + # This creates a BlockBackend and removes its media + self.addBlockBackend('drive0', 'node0') + self.ejectDrive('drive0', 'node0') + # This creates a new BlockDriverState and inserts it into the backend + self.addBlockDriverState('node1') + self.insertDrive('drive0', 'node1') + # The BlockDriverState has a monitor reference so we can destroy the backend + self.delBlockBackend('drive0', 'node1', destroys_media = False) + # The backend has been destroyed, now we can proceed with the BDS + self.delBlockDriverState('node1') + def testSnapshotSync(self): self.addBlockBackend('drive0', 'node0') self.createSnapshotSync('node0', 'overlay0') @@ -354,11 +365,10 @@ class TestBlockdevDel(iotests.QMPTestCase): self.delBlockBackend('drive0', 'overlay0') self.checkBlockDriverState('node0', False) - def testSnapshot(self): + def testSnapshot1(self): self.addBlockBackend('drive0', 'node0') self.addBlockDriverStateOverlay('overlay0') self.createSnapshot('node0', 'overlay0') - self.delBlockBackend('drive0', 'overlay0', expect_error = True) self.delBlockDriverState('node0', expect_error = True) self.delBlockDriverState('overlay0', expect_error = True) self.ejectDrive('drive0', 'overlay0', destroys_media = False) @@ -367,6 +377,16 @@ class TestBlockdevDel(iotests.QMPTestCase): self.delBlockDriverState('overlay0') self.checkBlockDriverState('node0', False) + def testSnapshot2(self): + self.addBlockBackend('drive0', 'node0') + self.addBlockDriverStateOverlay('overlay0') + self.createSnapshot('node0', 'overlay0') + # The BlockDriverState has a monitor reference so we can destroy the backend + self.delBlockBackend('drive0', 'overlay0', destroys_media = False) + self.delBlockDriverState('node0', expect_error = True) + self.delBlockDriverState('overlay0') + self.checkBlockDriverState('node0', False) + def testMirror(self): self.addBlockBackend('drive0', 'node0') self.createMirror('drive0', 'node0', 'mirror0') diff --git a/tests/qemu-iotests/139.out b/tests/qemu-iotests/139.out index 281b69e..6323079 100644 --- a/tests/qemu-iotests/139.out +++ b/tests/qemu-iotests/139.out @@ -1,5 +1,5 @@ -............ +.............. ---------------------------------------------------------------------- -Ran 12 tests +Ran 14 tests OK