[v4,45/45] block/copy-before-write: correct permission scheme

Commit Message

Vladimir Sementsov-Ogievskiy March 30, 2022, 10:12 a.m. UTC

Finally we can strictly unshare write on source node, as all write must
go through copy-before-write filter. For this to work:

 - Declare independent close, so that blockdev-del transaction action
   may detach children of removed node at prepare phase (that's for
   filter removement). We can do it because copy-before-write filter
   doesn't do any IO on its children on close().

 - Support BDRV_O_NOPERM, so that blockdev-add transaction action can
   skip intermediate permission update. We can do it because
   copy-before-write filter doesn't do any IO on its children on
   open().

 - Move to new block-graph modifying API in iotest image-fleecing.
   Separate qom-set + del/add doesn't work anymore for
   copy-before-write filter, because intermediate state violate new
   strict permissions.

Signed-off-by: Vladimir Sementsov-Ogievskiy <[e-mail]>
---
 block/copy-before-write.c                   | 17 ++++++------
 tests/qemu-iotests/tests/image-fleecing     | 29 ++++++++++++++-------
 tests/qemu-iotests/tests/image-fleecing.out |  2 --
 3 files changed, 27 insertions(+), 21 deletions(-)

Patch

diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index 8aa2cb6a85..0bc97b4a47 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -136,12 +136,8 @@  static void cbw_child_perm(BlockDriverState *bs, BdrvChild *c,
         bdrv_default_perms(bs, c, role, reopen_queue,
                            perm, shared, nperm, nshared);
 
-        if (!QLIST_EMPTY(&bs->parents)) {
-            if (perm & BLK_PERM_WRITE) {
-                *nperm = *nperm | BLK_PERM_CONSISTENT_READ;
-            }
-            *nshared &= ~(BLK_PERM_WRITE | BLK_PERM_RESIZE);
-        }
+        *nperm = *nperm | BLK_PERM_CONSISTENT_READ;
+        *nshared &= ~(BLK_PERM_WRITE | BLK_PERM_RESIZE);
     }
 }
 
@@ -152,13 +148,15 @@  static int cbw_open(BlockDriverState *bs, QDict *options, int flags,
     BdrvDirtyBitmap *copy_bitmap;
     int ret;
 
-    ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
+    ret = bdrv_open_file_child_common(NULL, options, "file", bs,
+                                      !(flags & BDRV_O_NOPERM), errp);
     if (ret < 0) {
         return ret;
     }
 
-    s->target = bdrv_open_child(NULL, options, "target", bs, &child_of_bds,
-                                BDRV_CHILD_DATA, false, errp);
+    s->target = bdrv_open_child_common(NULL, options, "target", bs,
+                                       &child_of_bds, BDRV_CHILD_DATA, false,
+                                       !(flags & BDRV_O_NOPERM), errp);
     if (!s->target) {
         return -EINVAL;
     }
@@ -193,6 +191,7 @@  static void cbw_close(BlockDriverState *bs)
 BlockDriver bdrv_cbw_filter = {
     .format_name = "copy-before-write",
     .instance_size = sizeof(BDRVCopyBeforeWriteState),
+    .independent_close = true,
 
     .bdrv_open                  = cbw_open,
     .bdrv_close                 = cbw_close,
diff --git a/tests/qemu-iotests/tests/image-fleecing b/tests/qemu-iotests/tests/image-fleecing
index a58b5a1781..3ee5e64edb 100755
--- a/tests/qemu-iotests/tests/image-fleecing
+++ b/tests/qemu-iotests/tests/image-fleecing
@@ -94,14 +94,17 @@  def do_test(use_cbw, base_img_path, fleece_img_path, nbd_sock_path, vm):
 
     # Establish CBW from source to fleecing node
     if use_cbw:
-        log(vm.qmp('blockdev-add', {
-            'driver': 'copy-before-write',
-            'node-name': 'fl-cbw',
-            'file': src_node,
-            'target': tmp_node
-        }))
-
-        log(vm.qmp('qom-set', path=qom_path, property='drive', value='fl-cbw'))
+        log(vm.qmp('transaction', {'actions': [
+            {'type': 'blockdev-add', 'data': {
+                'driver': 'copy-before-write',
+                'node-name': 'fl-cbw',
+                'file': src_node,
+                'target': tmp_node}},
+            {'type': 'x-blockdev-replace', 'data': {
+                'parent-type': 'qdev',
+                'qdev-id': 'sda',
+                'new-child': 'fl-cbw'}}
+        ]}))
     else:
         log(vm.qmp('blockdev-backup',
                    job_id='fleecing',
@@ -152,8 +155,14 @@  def do_test(use_cbw, base_img_path, fleece_img_path, nbd_sock_path, vm):
     log('')
 
     if use_cbw:
-        log(vm.qmp('qom-set', path=qom_path, property='drive', value=src_node))
-        log(vm.qmp('blockdev-del', node_name='fl-cbw'))
+        log(vm.qmp('transaction', {'actions': [
+            {'type': 'x-blockdev-replace', 'data': {
+                'parent-type': 'qdev',
+                'qdev-id': 'sda',
+                'new-child': src_node}},
+            {'type': 'blockdev-del', 'data': {
+                'node-name': 'fl-cbw'}}
+        ]}))
     else:
         log(vm.qmp('block-job-cancel', device='fleecing'))
         e = vm.event_wait('BLOCK_JOB_CANCELLED')
diff --git a/tests/qemu-iotests/tests/image-fleecing.out b/tests/qemu-iotests/tests/image-fleecing.out
index e96d122a8b..1aed446ccb 100644
--- a/tests/qemu-iotests/tests/image-fleecing.out
+++ b/tests/qemu-iotests/tests/image-fleecing.out
@@ -79,7 +79,6 @@  Done
 
 --- Setting up Fleecing Graph ---
 
-{"return": {}}
 {"return": {}}
 {"return": {}}
 
@@ -124,7 +123,6 @@  read -P0 0x3fe0000 64k
 {"return": {}}
 {"return": {}}
 {"return": {}}
-{"return": {}}
 
 --- Confirming writes ---