[Qemu-devel] [PATCH v2 1/2] block: Allow x-blockdev-del on a BB with a monitor-owned BDS

[Alberto Garcia]

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 <address@hidden>
---
 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
-- 
2.7.0