qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/33] block: publish backup-top filter
@ 2021-05-20 14:21 Vladimir Sementsov-Ogievskiy
  2021-05-20 14:21 ` [PATCH v2 01/33] block: rename bdrv_replace_child to bdrv_replace_child_tran Vladimir Sementsov-Ogievskiy
                   ` (33 more replies)
  0 siblings, 34 replies; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:21 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

Hi all!

v2:
01-02: new
03: don't bother with supporting empty child: we should never have such
    at this point
05: add comment
06: keep checking conflict with global
    add realized_set_allowed to qdev_prop_drive_iothread
07: improve cbw_cbw() name
    improve commit message
10: rebased on unchanged backup_calculate_cluster_size(). keep r-b  CHECK ME
12: new
13: drop extra bdrv_unref()
18: add compress local variable
    add comment about x-deprecated-compress
19: new, replacement for "[PATCH 17/21] block/block-copy: switch to fully set bitmap by default"
22: improve qapi documentation
23-33: test: a lot of refactoring

We have image fleecing scheme to export point-in-time state of active
disk (iotest 222):


                                      backup(sync=none)
                     ┌───────────────────────────────────────┐
                     ▼                                       │
┌────────────┐     ┌────────────────┐  backing             ┌─────────────┐
│ NBD export │ ─── │ temp qcow2 img │ ───────────────────▶ │ active disk │
└────────────┘     └────────────────┘                      └─────────────┘
                                                             ▲
┌────────────┐                                               │
│ guest blk  │ ──────────────────────────────────────────────┘
└────────────┘                        


Actually, backup job inserts a backup-top filter, so in detail it looks
like:

                                      backup(sync=none)
                     ┌───────────────────────────────────────┐
                     ▼                                       │
┌────────────┐     ┌────────────────┐  backing             ┌─────────────┐
│ NBD export │ ─── │ temp qcow2 img │ ───────────────────▶ │ active disk │
└────────────┘     └────────────────┘                      └─────────────┘
                     ▲                                       ▲
                     │ target                                │
                     │                                       │
┌────────────┐     ┌────────────────┐  backing               │
│ guest blk  │ ──▶ │   backup-top   │ ───────────────────────┘
└────────────┘     └────────────────┘

And job does nothing here. In a new blockdev world user is intended to
operate on node level, and insert/remove filters by hand. Let's get rid
of job in the scheme:

┌────────────┐     ┌────────────────┐  backing             ┌─────────────┐
│ NBD export │ ─── │ temp qcow2 img │ ───────────────────▶ │ active disk │
└────────────┘     └────────────────┘                      └─────────────┘
                     ▲                                       ▲
                     │ target                                │
                     │                                       │
┌────────────┐     ┌────────────────┐  backing               │
│ guest blk  │ ──▶ │   backup-top   │ ───────────────────────┘
└────────────┘     └────────────────┘


The series prepares qom-set to make possible inserting filters above
root node (patches 03-06), rename backup-top to copy-before-write, do
other preparations for publishing the filter, and finally publish it,
add qapi interface and test new fleecing scheme in 222 (first, some
good test refactoring).

Vladimir Sementsov-Ogievskiy (33):
  block: rename bdrv_replace_child to bdrv_replace_child_tran
  block: comment graph-modifying function not updating permissions
  block: introduce bdrv_replace_child_bs()
  block: introduce blk_replace_bs
  qdev-properties: PropertyInfo: add realized_set_allowed field
  qdev: allow setting drive property for realized device
  block: rename backup-top to copy-before-write
  block/backup: drop support for copy_range
  block-copy: always set BDRV_REQ_SERIALISING flag
  block/backup: move cluster size calculation to block-copy
  block/copy-before-write: relax permission requirements when no parents
  block/copy-before-write: drop extra bdrv_unref on failure path
  block/copy-before-write: use file child instead of backing
  block/copy-before-write: bdrv_cbw_append(): replace child at last
  block/copy-before-write: introduce cbw_init()
  block/copy-before-write: cbw_init(): rename variables
  block/copy-before-write: cbw_init(): use file child after attaching
  block/copy-before-write: cbw_init(): use options
  block/copy-before-write: initialize block-copy bitmap
  block/block-copy: make setting progress optional
  block/copy-before-write: make public block driver
  qapi: publish copy-before-write filter
  python/qemu/machine.py: refactor _qemu_args()
  python/qemu/machine: QEMUMachine: improve qmp() method
  iotests.py: VM: add own __enter__ method
  iotests/222: fix pylint and mypy complains
  iotests/222: constantly use single quotes for strings
  iotests: move 222 to tests/image-fleecing
  iotests.py: hmp_qemu_io: support qdev
  iotests/image-fleecing: proper source device
  iotests/image-fleecing: rename tgt_node
  iotests/image-fleecing: prepare for adding new test-case
  iotests/image-fleecing: add test-case for copy-before-write filter

 qapi/block-core.json                        |  30 ++-
 block/{backup-top.h => copy-before-write.h} |  26 +-
 include/block/block-copy.h                  |   4 +-
 include/block/block.h                       |   2 +
 include/hw/qdev-properties.h                |   1 +
 include/sysemu/block-backend.h              |   1 +
 block.c                                     |  52 +++-
 block/backup-top.c                          | 253 ------------------
 block/backup.c                              | 115 ++-------
 block/block-backend.c                       |   8 +
 block/block-copy.c                          |  98 ++++++-
 block/copy-before-write.c                   | 268 ++++++++++++++++++++
 hw/core/qdev-properties-system.c            |  43 +++-
 hw/core/qdev-properties.c                   |   6 +-
 MAINTAINERS                                 |   4 +-
 block/meson.build                           |   2 +-
 python/qemu/machine.py                      |  30 ++-
 tests/qemu-iotests/222                      | 159 ------------
 tests/qemu-iotests/222.out                  |  67 -----
 tests/qemu-iotests/283                      |  35 ++-
 tests/qemu-iotests/283.out                  |   4 +-
 tests/qemu-iotests/297                      |   2 +-
 tests/qemu-iotests/iotests.py               |   9 +-
 tests/qemu-iotests/tests/image-fleecing     | 192 ++++++++++++++
 tests/qemu-iotests/tests/image-fleecing.out | 139 ++++++++++
 25 files changed, 890 insertions(+), 660 deletions(-)
 rename block/{backup-top.h => copy-before-write.h} (56%)
 delete mode 100644 block/backup-top.c
 create mode 100644 block/copy-before-write.c
 delete mode 100755 tests/qemu-iotests/222
 delete mode 100644 tests/qemu-iotests/222.out
 create mode 100755 tests/qemu-iotests/tests/image-fleecing
 create mode 100644 tests/qemu-iotests/tests/image-fleecing.out

-- 
2.29.2



^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v2 01/33] block: rename bdrv_replace_child to bdrv_replace_child_tran
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:21 ` Vladimir Sementsov-Ogievskiy
  2021-05-31 12:52   ` Max Reitz
  2021-05-20 14:21 ` [PATCH v2 02/33] block: comment graph-modifying function not updating permissions Vladimir Sementsov-Ogievskiy
                   ` (32 subsequent siblings)
  33 siblings, 1 reply; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:21 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

We have bdrv_replace_child() wrapper on bdrv_replace_child_noperm().
But bdrv_replace_child() doesn't update permissions. It's rather
strange, as normally it's expected that foo() should call foo_noperm()
and update permissions.

Let's rename and add comment.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/block.c b/block.c
index 75a82af641..384413c578 100644
--- a/block.c
+++ b/block.c
@@ -2247,12 +2247,14 @@ static TransactionActionDrv bdrv_replace_child_drv = {
 };
 
 /*
- * bdrv_replace_child
+ * bdrv_replace_child_tran
  *
  * Note: real unref of old_bs is done only on commit.
+ *
+ * The function doesn't update permissions, caller is responsible for this.
  */
-static void bdrv_replace_child(BdrvChild *child, BlockDriverState *new_bs,
-                               Transaction *tran)
+static void bdrv_replace_child_tran(BdrvChild *child, BlockDriverState *new_bs,
+                                    Transaction *tran)
 {
     BdrvReplaceChildState *s = g_new(BdrvReplaceChildState, 1);
     *s = (BdrvReplaceChildState) {
@@ -4749,7 +4751,7 @@ static void bdrv_remove_filter_or_cow_child_abort(void *opaque)
     }
 
     /*
-     * We don't have to restore child->bs here to undo bdrv_replace_child()
+     * We don't have to restore child->bs here to undo bdrv_replace_child_tran()
      * because that function is transactionable and it registered own completion
      * entries in @tran, so .abort() for bdrv_replace_child_safe() will be
      * called automatically.
@@ -4785,7 +4787,7 @@ static void bdrv_remove_filter_or_cow_child(BlockDriverState *bs,
     }
 
     if (child->bs) {
-        bdrv_replace_child(child, NULL, tran);
+        bdrv_replace_child_tran(child, NULL, tran);
     }
 
     s = g_new(BdrvRemoveFilterOrCowChild, 1);
@@ -4825,7 +4827,7 @@ static int bdrv_replace_node_noperm(BlockDriverState *from,
                        c->name, from->node_name);
             return -EPERM;
         }
-        bdrv_replace_child(c, to, tran);
+        bdrv_replace_child_tran(c, to, tran);
     }
 
     return 0;
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 02/33] block: comment graph-modifying function not updating permissions
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
  2021-05-20 14:21 ` [PATCH v2 01/33] block: rename bdrv_replace_child to bdrv_replace_child_tran Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:21 ` Vladimir Sementsov-Ogievskiy
  2021-05-31 12:56   ` Max Reitz
  2021-05-20 14:21 ` [PATCH v2 03/33] block: introduce bdrv_replace_child_bs() Vladimir Sementsov-Ogievskiy
                   ` (31 subsequent siblings)
  33 siblings, 1 reply; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:21 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/block.c b/block.c
index 384413c578..8aabfb03ec 100644
--- a/block.c
+++ b/block.c
@@ -2762,6 +2762,8 @@ static TransactionActionDrv bdrv_attach_child_common_drv = {
 
 /*
  * Common part of attaching bdrv child to bs or to blk or to job
+ *
+ * Function doesn't update permissions, caller is responsible for this.
  */
 static int bdrv_attach_child_common(BlockDriverState *child_bs,
                                     const char *child_name,
@@ -2836,6 +2838,7 @@ static int bdrv_attach_child_common(BlockDriverState *child_bs,
     return 0;
 }
 
+/* Function doesn't update permissions, caller is responsible for this. */
 static int bdrv_attach_child_noperm(BlockDriverState *parent_bs,
                                     BlockDriverState *child_bs,
                                     const char *child_name,
@@ -3097,6 +3100,8 @@ static BdrvChildRole bdrv_backing_role(BlockDriverState *bs)
 /*
  * Sets the bs->backing link of a BDS. A new reference is created; callers
  * which don't need their own reference any more must call bdrv_unref().
+ *
+ * Function doesn't update permissions, caller is responsible for this.
  */
 static int bdrv_set_backing_noperm(BlockDriverState *bs,
                                    BlockDriverState *backing_hd,
@@ -4775,6 +4780,8 @@ static TransactionActionDrv bdrv_remove_filter_or_cow_child_drv = {
  * A function to remove backing-chain child of @bs if exists: cow child for
  * format nodes (always .backing) and filter child for filters (may be .file or
  * .backing)
+ *
+ * Function doesn't update permissions, caller is responsible for this.
  */
 static void bdrv_remove_filter_or_cow_child(BlockDriverState *bs,
                                             Transaction *tran)
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 03/33] block: introduce bdrv_replace_child_bs()
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
  2021-05-20 14:21 ` [PATCH v2 01/33] block: rename bdrv_replace_child to bdrv_replace_child_tran Vladimir Sementsov-Ogievskiy
  2021-05-20 14:21 ` [PATCH v2 02/33] block: comment graph-modifying function not updating permissions Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:21 ` Vladimir Sementsov-Ogievskiy
  2021-05-31 14:18   ` Max Reitz
  2021-05-20 14:21 ` [PATCH v2 04/33] block: introduce blk_replace_bs Vladimir Sementsov-Ogievskiy
                   ` (30 subsequent siblings)
  33 siblings, 1 reply; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:21 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

Add function to transactionally replace bs inside BdrvChild.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/block/block.h |  2 ++
 block.c               | 31 +++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/include/block/block.h b/include/block/block.h
index 82185965ff..f9d5fcb108 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -361,6 +361,8 @@ int bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top,
                 Error **errp);
 int bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
                       Error **errp);
+int bdrv_replace_child_bs(BdrvChild *child, BlockDriverState *new_bs,
+                          Error **errp);
 BlockDriverState *bdrv_insert_node(BlockDriverState *bs, QDict *node_options,
                                    int flags, Error **errp);
 int bdrv_drop_filter(BlockDriverState *bs, Error **errp);
diff --git a/block.c b/block.c
index 8aabfb03ec..39a5d4be90 100644
--- a/block.c
+++ b/block.c
@@ -4969,6 +4969,37 @@ out:
     return ret;
 }
 
+/* Not for empty child */
+int bdrv_replace_child_bs(BdrvChild *child, BlockDriverState *new_bs,
+                          Error **errp)
+{
+    int ret;
+    Transaction *tran = tran_new();
+    g_autoptr(GHashTable) found = NULL;
+    g_autoptr(GSList) refresh_list = NULL;
+    BlockDriverState *old_bs = child->bs;
+
+    bdrv_ref(old_bs);
+    bdrv_drained_begin(old_bs);
+    bdrv_drained_begin(new_bs);
+
+    bdrv_replace_child_tran(child, new_bs, tran);
+
+    found = g_hash_table_new(NULL, NULL);
+    refresh_list = bdrv_topological_dfs(refresh_list, found, old_bs);
+    refresh_list = bdrv_topological_dfs(refresh_list, found, new_bs);
+
+    ret = bdrv_list_refresh_perms(refresh_list, NULL, tran, errp);
+
+    tran_finalize(tran, ret);
+
+    bdrv_drained_end(old_bs);
+    bdrv_drained_end(new_bs);
+    bdrv_unref(old_bs);
+
+    return ret;
+}
+
 static void bdrv_delete(BlockDriverState *bs)
 {
     assert(bdrv_op_blocker_is_empty(bs));
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 04/33] block: introduce blk_replace_bs
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (2 preceding siblings ...)
  2021-05-20 14:21 ` [PATCH v2 03/33] block: introduce bdrv_replace_child_bs() Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:21 ` Vladimir Sementsov-Ogievskiy
  2021-05-20 14:21 ` [PATCH v2 05/33] qdev-properties: PropertyInfo: add realized_set_allowed field Vladimir Sementsov-Ogievskiy
                   ` (29 subsequent siblings)
  33 siblings, 0 replies; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:21 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

Add function to change bs inside blk.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 include/sysemu/block-backend.h | 1 +
 block/block-backend.c          | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 880e903293..aec05ef0a0 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -98,6 +98,7 @@ BlockBackend *blk_by_public(BlockBackendPublic *public);
 BlockDriverState *blk_bs(BlockBackend *blk);
 void blk_remove_bs(BlockBackend *blk);
 int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp);
+int blk_replace_bs(BlockBackend *blk, BlockDriverState *new_bs, Error **errp);
 bool bdrv_has_blk(BlockDriverState *bs);
 bool bdrv_is_root_node(BlockDriverState *bs);
 int blk_set_perm(BlockBackend *blk, uint64_t perm, uint64_t shared_perm,
diff --git a/block/block-backend.c b/block/block-backend.c
index de5496af66..b1abc6f3e6 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -870,6 +870,14 @@ int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp)
     return 0;
 }
 
+/*
+ * Change BlockDriverState associated with @blk.
+ */
+int blk_replace_bs(BlockBackend *blk, BlockDriverState *new_bs, Error **errp)
+{
+    return bdrv_replace_child_bs(blk->root, new_bs, errp);
+}
+
 /*
  * Sets the permission bitmasks that the user of the BlockBackend needs.
  */
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 05/33] qdev-properties: PropertyInfo: add realized_set_allowed field
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (3 preceding siblings ...)
  2021-05-20 14:21 ` [PATCH v2 04/33] block: introduce blk_replace_bs Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:21 ` Vladimir Sementsov-Ogievskiy
  2021-05-31 15:51   ` Max Reitz
  2021-05-20 14:21 ` [PATCH v2 06/33] qdev: allow setting drive property for realized device Vladimir Sementsov-Ogievskiy
                   ` (28 subsequent siblings)
  33 siblings, 1 reply; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:21 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

Add field, so property can declare support for setting the property
when device is realized. To be used in the following commit.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/hw/qdev-properties.h | 1 +
 hw/core/qdev-properties.c    | 6 +++---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 0ef97d60ce..f7925f67d0 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -32,6 +32,7 @@ struct PropertyInfo {
     const char *name;
     const char *description;
     const QEnumLookup *enum_table;
+    bool realized_set_allowed; /* allow setting property on realized device */
     int (*print)(Object *obj, Property *prop, char *dest, size_t len);
     void (*set_default_value)(ObjectProperty *op, const Property *prop);
     ObjectProperty *(*create)(ObjectClass *oc, const char *name,
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 50f40949f5..c34aac6ebc 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -26,11 +26,11 @@ void qdev_prop_set_after_realize(DeviceState *dev, const char *name,
 
 /* returns: true if property is allowed to be set, false otherwise */
 static bool qdev_prop_allow_set(Object *obj, const char *name,
-                                Error **errp)
+                                const PropertyInfo *info, Error **errp)
 {
     DeviceState *dev = DEVICE(obj);
 
-    if (dev->realized) {
+    if (dev->realized && !info->realized_set_allowed) {
         qdev_prop_set_after_realize(dev, name, errp);
         return false;
     }
@@ -79,7 +79,7 @@ static void field_prop_set(Object *obj, Visitor *v, const char *name,
 {
     Property *prop = opaque;
 
-    if (!qdev_prop_allow_set(obj, name, errp)) {
+    if (!qdev_prop_allow_set(obj, name, prop->info, errp)) {
         return;
     }
 
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 06/33] qdev: allow setting drive property for realized device
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (4 preceding siblings ...)
  2021-05-20 14:21 ` [PATCH v2 05/33] qdev-properties: PropertyInfo: add realized_set_allowed field Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:21 ` Vladimir Sementsov-Ogievskiy
  2021-05-31 15:59   ` Max Reitz
  2021-05-20 14:21 ` [PATCH v2 07/33] block: rename backup-top to copy-before-write Vladimir Sementsov-Ogievskiy
                   ` (27 subsequent siblings)
  33 siblings, 1 reply; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:21 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

We need an ability to insert filters above top block node, attached to
block device. It can't be achieved with blockdev-reopen command. So, we
want do it with help of qom-set.

Intended usage:

Assume there is a node A that is attached to some guest device.

1. blockdev-add to create a filter node B that has A as its child.

2. qom-set to change the node attached to the guest device’s
   BlockBackend from A to B.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 hw/core/qdev-properties-system.c | 43 +++++++++++++++++++++++---------
 1 file changed, 31 insertions(+), 12 deletions(-)

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 2760c21f11..e71f5d64d1 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -36,11 +36,11 @@
 
 static bool check_prop_still_unset(Object *obj, const char *name,
                                    const void *old_val, const char *new_val,
-                                   Error **errp)
+                                   bool allow_override, Error **errp)
 {
     const GlobalProperty *prop = qdev_find_global_prop(obj, name);
 
-    if (!old_val) {
+    if (!old_val || (!prop && allow_override)) {
         return true;
     }
 
@@ -93,16 +93,34 @@ static void set_drive_helper(Object *obj, Visitor *v, const char *name,
     BlockBackend *blk;
     bool blk_created = false;
     int ret;
+    BlockDriverState *bs;
+    AioContext *ctx;
 
     if (!visit_type_str(v, name, &str, errp)) {
         return;
     }
 
-    /*
-     * TODO Should this really be an error?  If no, the old value
-     * needs to be released before we store the new one.
-     */
-    if (!check_prop_still_unset(obj, name, *ptr, str, errp)) {
+    if (!check_prop_still_unset(obj, name, *ptr, str, true, errp)) {
+        return;
+    }
+
+    if (*ptr) {
+        /* BlockBackend alread exists. So, we want to change attached node */
+        blk = *ptr;
+        ctx = blk_get_aio_context(blk);
+        bs = bdrv_lookup_bs(NULL, str, errp);
+        if (!bs) {
+            return;
+        }
+
+        if (ctx != bdrv_get_aio_context(bs)) {
+            error_setg(errp, "Different aio context is not supported for new "
+                       "node");
+        }
+
+        aio_context_acquire(ctx);
+        blk_replace_bs(blk, bs, errp);
+        aio_context_release(ctx);
         return;
     }
 
@@ -114,7 +132,7 @@ static void set_drive_helper(Object *obj, Visitor *v, const char *name,
 
     blk = blk_by_name(str);
     if (!blk) {
-        BlockDriverState *bs = bdrv_lookup_bs(NULL, str, NULL);
+        bs = bdrv_lookup_bs(NULL, str, NULL);
         if (bs) {
             /*
              * If the device supports iothreads, it will make sure to move the
@@ -123,8 +141,7 @@ static void set_drive_helper(Object *obj, Visitor *v, const char *name,
              * aware of iothreads require their BlockBackends to be in the main
              * AioContext.
              */
-            AioContext *ctx = iothread ? bdrv_get_aio_context(bs) :
-                                         qemu_get_aio_context();
+            ctx = iothread ? bdrv_get_aio_context(bs) : qemu_get_aio_context();
             blk = blk_new(ctx, 0, BLK_PERM_ALL);
             blk_created = true;
 
@@ -196,6 +213,7 @@ static void release_drive(Object *obj, const char *name, void *opaque)
 const PropertyInfo qdev_prop_drive = {
     .name  = "str",
     .description = "Node name or ID of a block device to use as a backend",
+    .realized_set_allowed = true,
     .get   = get_drive,
     .set   = set_drive,
     .release = release_drive,
@@ -204,6 +222,7 @@ const PropertyInfo qdev_prop_drive = {
 const PropertyInfo qdev_prop_drive_iothread = {
     .name  = "str",
     .description = "Node name or ID of a block device to use as a backend",
+    .realized_set_allowed = true,
     .get   = get_drive,
     .set   = set_drive_iothread,
     .release = release_drive,
@@ -238,7 +257,7 @@ static void set_chr(Object *obj, Visitor *v, const char *name, void *opaque,
      * TODO Should this really be an error?  If no, the old value
      * needs to be released before we store the new one.
      */
-    if (!check_prop_still_unset(obj, name, be->chr, str, errp)) {
+    if (!check_prop_still_unset(obj, name, be->chr, str, false, errp)) {
         return;
     }
 
@@ -408,7 +427,7 @@ static void set_netdev(Object *obj, Visitor *v, const char *name,
          * TODO Should this really be an error?  If no, the old value
          * needs to be released before we store the new one.
          */
-        if (!check_prop_still_unset(obj, name, ncs[i], str, errp)) {
+        if (!check_prop_still_unset(obj, name, ncs[i], str, false, errp)) {
             goto out;
         }
 
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 07/33] block: rename backup-top to copy-before-write
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (5 preceding siblings ...)
  2021-05-20 14:21 ` [PATCH v2 06/33] qdev: allow setting drive property for realized device Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:21 ` Vladimir Sementsov-Ogievskiy
  2021-05-31 16:08   ` Max Reitz
  2021-05-20 14:21 ` [PATCH v2 08/33] block/backup: drop support for copy_range Vladimir Sementsov-Ogievskiy
                   ` (26 subsequent siblings)
  33 siblings, 1 reply; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:21 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

We are going to convert backup_top to full featured public filter,
which can be used in separate of backup job. Start from renaming from
"how it used" to "what it does".

While updating comments in 283 iotest, drop and rephrase also things
about ".active", as this field is now dropped, and filter doesn't have
"inactive" mode.

Note that this change may be considered as incompatible interface
change, as backup-top filter format name was visible through
query-block and query-named-block-nodes.

Still, consider the following reasoning:

1. backup-top was never documented, so if someone depends on format
   name (for driver that can't be used other than it is automatically
   inserted on backup job start), it's a kind of "undocumented feature
   use". So I think we are free to change it.

2. There is a hope, that there is no such users: it's a lot more native
   to give a good node-name to backup-top filter if need to operate
   with it somehow, and don't touch format name.

3. Another "incompatible" change in further commit would be moving
   copy-before-write filter from using backing child to file child. And
   this is even more reasonable than renaming: for now all public
   filters are file-child based.

So, it's a risky change, but risk seems small and good interface worth
it.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/{backup-top.h => copy-before-write.h} |  28 +++---
 block/backup.c                              |  22 ++---
 block/{backup-top.c => copy-before-write.c} | 100 ++++++++++----------
 MAINTAINERS                                 |   4 +-
 block/meson.build                           |   2 +-
 tests/qemu-iotests/283                      |  35 +++----
 tests/qemu-iotests/283.out                  |   4 +-
 7 files changed, 95 insertions(+), 100 deletions(-)
 rename block/{backup-top.h => copy-before-write.h} (56%)
 rename block/{backup-top.c => copy-before-write.c} (62%)

diff --git a/block/backup-top.h b/block/copy-before-write.h
similarity index 56%
rename from block/backup-top.h
rename to block/copy-before-write.h
index b28b0031c4..5977b7aa31 100644
--- a/block/backup-top.h
+++ b/block/copy-before-write.h
@@ -1,10 +1,10 @@
 /*
- * backup-top filter driver
+ * copy-before-write filter driver
  *
  * The driver performs Copy-Before-Write (CBW) operation: it is injected above
  * some node, and before each write it copies _old_ data to the target node.
  *
- * Copyright (c) 2018-2019 Virtuozzo International GmbH.
+ * Copyright (c) 2018-2021 Virtuozzo International GmbH.
  *
  * Author:
  *  Sementsov-Ogievskiy Vladimir <vsementsov@virtuozzo.com>
@@ -23,20 +23,20 @@
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  */
 
-#ifndef BACKUP_TOP_H
-#define BACKUP_TOP_H
+#ifndef COPY_BEFORE_WRITE_H
+#define COPY_BEFORE_WRITE_H
 
 #include "block/block_int.h"
 #include "block/block-copy.h"
 
-BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
-                                         BlockDriverState *target,
-                                         const char *filter_node_name,
-                                         uint64_t cluster_size,
-                                         BackupPerf *perf,
-                                         BdrvRequestFlags write_flags,
-                                         BlockCopyState **bcs,
-                                         Error **errp);
-void bdrv_backup_top_drop(BlockDriverState *bs);
+BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
+                                  BlockDriverState *target,
+                                  const char *filter_node_name,
+                                  uint64_t cluster_size,
+                                  BackupPerf *perf,
+                                  BdrvRequestFlags write_flags,
+                                  BlockCopyState **bcs,
+                                  Error **errp);
+void bdrv_cbw_drop(BlockDriverState *bs);
 
-#endif /* BACKUP_TOP_H */
+#endif /* COPY_BEFORE_WRITE_H */
diff --git a/block/backup.c b/block/backup.c
index bd3614ce70..ac91821b08 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -27,13 +27,13 @@
 #include "qemu/bitmap.h"
 #include "qemu/error-report.h"
 
-#include "block/backup-top.h"
+#include "block/copy-before-write.h"
 
 #define BACKUP_CLUSTER_SIZE_DEFAULT (1 << 16)
 
 typedef struct BackupBlockJob {
     BlockJob common;
-    BlockDriverState *backup_top;
+    BlockDriverState *cbw;
     BlockDriverState *source_bs;
     BlockDriverState *target_bs;
 
@@ -104,7 +104,7 @@ static void backup_clean(Job *job)
 {
     BackupBlockJob *s = container_of(job, BackupBlockJob, common.job);
     block_job_remove_all_bdrv(&s->common);
-    bdrv_backup_top_drop(s->backup_top);
+    bdrv_cbw_drop(s->cbw);
 }
 
 void backup_do_checkpoint(BlockJob *job, Error **errp)
@@ -408,7 +408,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
     BackupBlockJob *job = NULL;
     int64_t cluster_size;
     BdrvRequestFlags write_flags;
-    BlockDriverState *backup_top = NULL;
+    BlockDriverState *cbw = NULL;
     BlockCopyState *bcs = NULL;
 
     assert(bs);
@@ -521,22 +521,22 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
     write_flags = (bdrv_chain_contains(target, bs) ? BDRV_REQ_SERIALISING : 0) |
                   (compress ? BDRV_REQ_WRITE_COMPRESSED : 0),
 
-    backup_top = bdrv_backup_top_append(bs, target, filter_node_name,
+    cbw = bdrv_cbw_append(bs, target, filter_node_name,
                                         cluster_size, perf,
                                         write_flags, &bcs, errp);
-    if (!backup_top) {
+    if (!cbw) {
         goto error;
     }
 
     /* job->len is fixed, so we can't allow resize */
-    job = block_job_create(job_id, &backup_job_driver, txn, backup_top,
+    job = block_job_create(job_id, &backup_job_driver, txn, cbw,
                            0, BLK_PERM_ALL,
                            speed, creation_flags, cb, opaque, errp);
     if (!job) {
         goto error;
     }
 
-    job->backup_top = backup_top;
+    job->cbw = cbw;
     job->source_bs = bs;
     job->target_bs = target;
     job->on_source_error = on_source_error;
@@ -552,7 +552,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
     block_copy_set_progress_meter(bcs, &job->common.job.progress);
     block_copy_set_speed(bcs, speed);
 
-    /* Required permissions are already taken by backup-top target */
+    /* Required permissions are taken by copy-before-write filter target */
     block_job_add_bdrv(&job->common, "target", target, 0, BLK_PERM_ALL,
                        &error_abort);
 
@@ -562,8 +562,8 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
     if (sync_bitmap) {
         bdrv_reclaim_dirty_bitmap(sync_bitmap, NULL);
     }
-    if (backup_top) {
-        bdrv_backup_top_drop(backup_top);
+    if (cbw) {
+        bdrv_cbw_drop(cbw);
     }
 
     return NULL;
diff --git a/block/backup-top.c b/block/copy-before-write.c
similarity index 62%
rename from block/backup-top.c
rename to block/copy-before-write.c
index 425e3778be..0dc5a107cf 100644
--- a/block/backup-top.c
+++ b/block/copy-before-write.c
@@ -1,10 +1,10 @@
 /*
- * backup-top filter driver
+ * copy-before-write filter driver
  *
  * The driver performs Copy-Before-Write (CBW) operation: it is injected above
  * some node, and before each write it copies _old_ data to the target node.
  *
- * Copyright (c) 2018-2019 Virtuozzo International GmbH.
+ * Copyright (c) 2018-2021 Virtuozzo International GmbH.
  *
  * Author:
  *  Sementsov-Ogievskiy Vladimir <vsementsov@virtuozzo.com>
@@ -32,25 +32,25 @@
 #include "block/qdict.h"
 #include "block/block-copy.h"
 
-#include "block/backup-top.h"
+#include "block/copy-before-write.h"
 
-typedef struct BDRVBackupTopState {
+typedef struct BDRVCopyBeforeWriteState {
     BlockCopyState *bcs;
     BdrvChild *target;
     int64_t cluster_size;
-} BDRVBackupTopState;
+} BDRVCopyBeforeWriteState;
 
-static coroutine_fn int backup_top_co_preadv(
+static coroutine_fn int cbw_co_preadv(
         BlockDriverState *bs, uint64_t offset, uint64_t bytes,
         QEMUIOVector *qiov, int flags)
 {
     return bdrv_co_preadv(bs->backing, offset, bytes, qiov, flags);
 }
 
-static coroutine_fn int backup_top_cbw(BlockDriverState *bs, uint64_t offset,
-                                       uint64_t bytes, BdrvRequestFlags flags)
+static coroutine_fn int cbw_do_copy_before_write(BlockDriverState *bs,
+        uint64_t offset, uint64_t bytes, BdrvRequestFlags flags)
 {
-    BDRVBackupTopState *s = bs->opaque;
+    BDRVCopyBeforeWriteState *s = bs->opaque;
     uint64_t off, end;
 
     if (flags & BDRV_REQ_WRITE_UNCHANGED) {
@@ -63,10 +63,10 @@ static coroutine_fn int backup_top_cbw(BlockDriverState *bs, uint64_t offset,
     return block_copy(s->bcs, off, end - off, true);
 }
 
-static int coroutine_fn backup_top_co_pdiscard(BlockDriverState *bs,
-                                               int64_t offset, int bytes)
+static int coroutine_fn cbw_co_pdiscard(BlockDriverState *bs,
+                                        int64_t offset, int bytes)
 {
-    int ret = backup_top_cbw(bs, offset, bytes, 0);
+    int ret = cbw_do_copy_before_write(bs, offset, bytes, 0);
     if (ret < 0) {
         return ret;
     }
@@ -74,10 +74,10 @@ static int coroutine_fn backup_top_co_pdiscard(BlockDriverState *bs,
     return bdrv_co_pdiscard(bs->backing, offset, bytes);
 }
 
-static int coroutine_fn backup_top_co_pwrite_zeroes(BlockDriverState *bs,
+static int coroutine_fn cbw_co_pwrite_zeroes(BlockDriverState *bs,
         int64_t offset, int bytes, BdrvRequestFlags flags)
 {
-    int ret = backup_top_cbw(bs, offset, bytes, flags);
+    int ret = cbw_do_copy_before_write(bs, offset, bytes, flags);
     if (ret < 0) {
         return ret;
     }
@@ -85,12 +85,12 @@ static int coroutine_fn backup_top_co_pwrite_zeroes(BlockDriverState *bs,
     return bdrv_co_pwrite_zeroes(bs->backing, offset, bytes, flags);
 }
 
-static coroutine_fn int backup_top_co_pwritev(BlockDriverState *bs,
-                                              uint64_t offset,
-                                              uint64_t bytes,
-                                              QEMUIOVector *qiov, int flags)
+static coroutine_fn int cbw_co_pwritev(BlockDriverState *bs,
+                                       uint64_t offset,
+                                       uint64_t bytes,
+                                       QEMUIOVector *qiov, int flags)
 {
-    int ret = backup_top_cbw(bs, offset, bytes, flags);
+    int ret = cbw_do_copy_before_write(bs, offset, bytes, flags);
     if (ret < 0) {
         return ret;
     }
@@ -98,7 +98,7 @@ static coroutine_fn int backup_top_co_pwritev(BlockDriverState *bs,
     return bdrv_co_pwritev(bs->backing, offset, bytes, qiov, flags);
 }
 
-static int coroutine_fn backup_top_co_flush(BlockDriverState *bs)
+static int coroutine_fn cbw_co_flush(BlockDriverState *bs)
 {
     if (!bs->backing) {
         return 0;
@@ -107,7 +107,7 @@ static int coroutine_fn backup_top_co_flush(BlockDriverState *bs)
     return bdrv_co_flush(bs->backing->bs);
 }
 
-static void backup_top_refresh_filename(BlockDriverState *bs)
+static void cbw_refresh_filename(BlockDriverState *bs)
 {
     if (bs->backing == NULL) {
         /*
@@ -120,11 +120,11 @@ static void backup_top_refresh_filename(BlockDriverState *bs)
             bs->backing->bs->filename);
 }
 
-static void backup_top_child_perm(BlockDriverState *bs, BdrvChild *c,
-                                  BdrvChildRole role,
-                                  BlockReopenQueue *reopen_queue,
-                                  uint64_t perm, uint64_t shared,
-                                  uint64_t *nperm, uint64_t *nshared)
+static void cbw_child_perm(BlockDriverState *bs, BdrvChild *c,
+                           BdrvChildRole role,
+                           BlockReopenQueue *reopen_queue,
+                           uint64_t perm, uint64_t shared,
+                           uint64_t *nperm, uint64_t *nshared)
 {
     if (!(role & BDRV_CHILD_FILTERED)) {
         /*
@@ -149,41 +149,41 @@ static void backup_top_child_perm(BlockDriverState *bs, BdrvChild *c,
     }
 }
 
-BlockDriver bdrv_backup_top_filter = {
-    .format_name = "backup-top",
-    .instance_size = sizeof(BDRVBackupTopState),
+BlockDriver bdrv_cbw_filter = {
+    .format_name = "copy-before-write",
+    .instance_size = sizeof(BDRVCopyBeforeWriteState),
 
-    .bdrv_co_preadv             = backup_top_co_preadv,
-    .bdrv_co_pwritev            = backup_top_co_pwritev,
-    .bdrv_co_pwrite_zeroes      = backup_top_co_pwrite_zeroes,
-    .bdrv_co_pdiscard           = backup_top_co_pdiscard,
-    .bdrv_co_flush              = backup_top_co_flush,
+    .bdrv_co_preadv             = cbw_co_preadv,
+    .bdrv_co_pwritev            = cbw_co_pwritev,
+    .bdrv_co_pwrite_zeroes      = cbw_co_pwrite_zeroes,
+    .bdrv_co_pdiscard           = cbw_co_pdiscard,
+    .bdrv_co_flush              = cbw_co_flush,
 
-    .bdrv_refresh_filename      = backup_top_refresh_filename,
+    .bdrv_refresh_filename      = cbw_refresh_filename,
 
-    .bdrv_child_perm            = backup_top_child_perm,
+    .bdrv_child_perm            = cbw_child_perm,
 
     .is_filter = true,
 };
 
-BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
-                                         BlockDriverState *target,
-                                         const char *filter_node_name,
-                                         uint64_t cluster_size,
-                                         BackupPerf *perf,
-                                         BdrvRequestFlags write_flags,
-                                         BlockCopyState **bcs,
-                                         Error **errp)
+BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
+                                  BlockDriverState *target,
+                                  const char *filter_node_name,
+                                  uint64_t cluster_size,
+                                  BackupPerf *perf,
+                                  BdrvRequestFlags write_flags,
+                                  BlockCopyState **bcs,
+                                  Error **errp)
 {
     ERRP_GUARD();
     int ret;
-    BDRVBackupTopState *state;
+    BDRVCopyBeforeWriteState *state;
     BlockDriverState *top;
     bool appended = false;
 
     assert(source->total_sectors == target->total_sectors);
 
-    top = bdrv_new_open_driver(&bdrv_backup_top_filter, filter_node_name,
+    top = bdrv_new_open_driver(&bdrv_cbw_filter, filter_node_name,
                                BDRV_O_RDWR, errp);
     if (!top) {
         return NULL;
@@ -210,7 +210,7 @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
 
     ret = bdrv_append(top, source, errp);
     if (ret < 0) {
-        error_prepend(errp, "Cannot append backup-top filter: ");
+        error_prepend(errp, "Cannot append copy-before-write filter: ");
         goto fail;
     }
     appended = true;
@@ -231,7 +231,7 @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
 
 fail:
     if (appended) {
-        bdrv_backup_top_drop(top);
+        bdrv_cbw_drop(top);
     } else {
         bdrv_unref(top);
     }
@@ -241,9 +241,9 @@ fail:
     return NULL;
 }
 
-void bdrv_backup_top_drop(BlockDriverState *bs)
+void bdrv_cbw_drop(BlockDriverState *bs)
 {
-    BDRVBackupTopState *s = bs->opaque;
+    BDRVCopyBeforeWriteState *s = bs->opaque;
 
     bdrv_drop_filter(bs, &error_abort);
 
diff --git a/MAINTAINERS b/MAINTAINERS
index eab178aeee..bc59f8bfcf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2281,8 +2281,8 @@ F: block/mirror.c
 F: qapi/job.json
 F: block/block-copy.c
 F: include/block/block-copy.c
-F: block/backup-top.h
-F: block/backup-top.c
+F: block/copy-before-write.h
+F: block/copy-before-write.c
 F: include/block/aio_task.h
 F: block/aio_task.c
 F: util/qemu-co-shared-resource.c
diff --git a/block/meson.build b/block/meson.build
index e687c54dbc..6e27aa9624 100644
--- a/block/meson.build
+++ b/block/meson.build
@@ -4,7 +4,7 @@ block_ss.add(files(
   'aio_task.c',
   'amend.c',
   'backup.c',
-  'backup-top.c',
+  'copy-before-write.c',
   'blkdebug.c',
   'blklogwrites.c',
   'blkverify.c',
diff --git a/tests/qemu-iotests/283 b/tests/qemu-iotests/283
index 010c22f0a2..a09e0183ae 100755
--- a/tests/qemu-iotests/283
+++ b/tests/qemu-iotests/283
@@ -1,7 +1,7 @@
 #!/usr/bin/env python3
 # group: auto quick
 #
-# Test for backup-top filter permission activation failure
+# Test for copy-before-write filter permission conflict
 #
 # Copyright (c) 2019 Virtuozzo International GmbH.
 #
@@ -31,13 +31,13 @@ size = 1024 * 1024
 """ Test description
 
 When performing a backup, all writes on the source subtree must go through the
-backup-top filter so it can copy all data to the target before it is changed.
-backup-top filter is appended above source node, to achieve this thing, so all
-parents of source node are handled. A configuration with side parents of source
-sub-tree with write permission is unsupported (we'd have append several
-backup-top filter like nodes to handle such parents). The test create an
-example of such configuration and checks that a backup is then not allowed
-(blockdev-backup command should fail).
+copy-before-write filter so it can copy all data to the target before it is
+changed.  copy-before-write filter is appended above source node, to achieve
+this thing, so all parents of source node are handled. A configuration with
+side parents of source sub-tree with write permission is unsupported (we'd have
+append several copy-before-write filter like nodes to handle such parents). The
+test create an example of such configuration and checks that a backup is then
+not allowed (blockdev-backup command should fail).
 
 The configuration:
 
@@ -57,11 +57,10 @@ The configuration:
                         │    base     │ ◀──────────── │ other │
                         └─────────────┘               └───────┘
 
-On activation (see .active field of backup-top state in block/backup-top.c),
-backup-top is going to unshare write permission on its source child. Write
-unsharing will be propagated to the "source->base" link and will conflict with
-other node write permission. So permission update will fail and backup job will
-not be started.
+copy-before-write filter wants to unshare write permission on its source child.
+Write unsharing will be propagated to the "source->base" link and will conflict
+with other node write permission. So permission update will fail and backup job
+will not be started.
 
 Note, that the only thing which prevents backup of running on such
 configuration is default permission propagation scheme. It may be altered by
@@ -99,13 +98,9 @@ vm.qmp_log('blockdev-backup', sync='full', device='source', target='target')
 vm.shutdown()
 
 
-print('\n=== backup-top should be gone after job-finalize ===\n')
+print('\n=== copy-before-write filter should be gone after job-finalize ===\n')
 
-# Check that the backup-top node is gone after job-finalize.
-#
-# During finalization, the node becomes inactive and can no longer
-# function.  If it is still present, new parents might be attached, and
-# there would be no meaningful way to handle their I/O requests.
+# Check that the copy-before-write node is gone after job-finalize.
 
 vm = iotests.VM()
 vm.launch()
@@ -131,7 +126,7 @@ vm.qmp_log('blockdev-backup',
 
 vm.event_wait('BLOCK_JOB_PENDING', 5.0)
 
-# The backup-top filter should still be present prior to finalization
+# The copy-before-write filter should still be present prior to finalization
 assert vm.node_info('backup-filter') is not None
 
 vm.qmp_log('job-finalize', id='backup')
diff --git a/tests/qemu-iotests/283.out b/tests/qemu-iotests/283.out
index 97e62a4c94..e08f807dab 100644
--- a/tests/qemu-iotests/283.out
+++ b/tests/qemu-iotests/283.out
@@ -5,9 +5,9 @@
 {"execute": "blockdev-add", "arguments": {"driver": "blkdebug", "image": "base", "node-name": "other", "take-child-perms": ["write"]}}
 {"return": {}}
 {"execute": "blockdev-backup", "arguments": {"device": "source", "sync": "full", "target": "target"}}
-{"error": {"class": "GenericError", "desc": "Cannot append backup-top filter: Conflicts with use by source as 'image', which does not allow 'write' on base"}}
+{"error": {"class": "GenericError", "desc": "Cannot append copy-before-write filter: Conflicts with use by source as 'image', which does not allow 'write' on base"}}
 
-=== backup-top should be gone after job-finalize ===
+=== copy-before-write filter should be gone after job-finalize ===
 
 {"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "source"}}
 {"return": {}}
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 08/33] block/backup: drop support for copy_range
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (6 preceding siblings ...)
  2021-05-20 14:21 ` [PATCH v2 07/33] block: rename backup-top to copy-before-write Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:21 ` Vladimir Sementsov-Ogievskiy
  2021-05-28 15:29   ` Vladimir Sementsov-Ogievskiy
  2021-05-20 14:21 ` [PATCH v2 09/33] block-copy: always set BDRV_REQ_SERIALISING flag Vladimir Sementsov-Ogievskiy
                   ` (25 subsequent siblings)
  33 siblings, 1 reply; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:21 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

copy_range is not a default behavior since 6a30f663d4c0b3c, and it's
now available only though x-perf experimantal argument, so it's OK to
drop it.

Even when backup is used to copy disk to same filesystem, and
filesystem supports zero-copy copy_range, copy_range is probably not
what we want for backup: backup has good property of making a copy of
active disk, with no impact to active disk itself (unlike creating a
snapshot). And if copy_range instead of copying data adds fs-level
references, and on next guest write COW operation occurs, it's seems
most possible, that new block will be allocated for original vm disk,
not for backup disk. Thus, fragmentation of original disk will
increase.

We can simply add support back on demand. Now we want to publish
copy-before-write filter, and instead of thinking how to pass
use-copy-range argument to block-copy (create x-block-copy parameter
for new public filter driver, or may be set it by hand after filter
node creation?), instead of this let's just drop copy-range support in
backup for now.

After this patch copy-range support in block-copy becomes unused. Let's
keep it for a while, it won't hurt:

1. If there would be request for supporting copy_range in backup
   (and/or in a new public copy-before-write filter), it will be easy
   to satisfy it.

2. Probably, qemu-img convert will reuse block-copy, and qemu-img has
   option to enable copy-range. qemu-img convert is not a backup, and
   copy_range may be more reasonable for some cases in context of
   qemu-img convert.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block/copy-before-write.h | 1 -
 block/backup.c            | 3 +--
 block/copy-before-write.c | 4 +---
 3 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/block/copy-before-write.h b/block/copy-before-write.h
index 5977b7aa31..e284dfb6a7 100644
--- a/block/copy-before-write.h
+++ b/block/copy-before-write.h
@@ -33,7 +33,6 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
                                   BlockDriverState *target,
                                   const char *filter_node_name,
                                   uint64_t cluster_size,
-                                  BackupPerf *perf,
                                   BdrvRequestFlags write_flags,
                                   BlockCopyState **bcs,
                                   Error **errp);
diff --git a/block/backup.c b/block/backup.c
index ac91821b08..d41dd30e25 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -522,8 +522,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
                   (compress ? BDRV_REQ_WRITE_COMPRESSED : 0),
 
     cbw = bdrv_cbw_append(bs, target, filter_node_name,
-                                        cluster_size, perf,
-                                        write_flags, &bcs, errp);
+                          cluster_size, write_flags, &bcs, errp);
     if (!cbw) {
         goto error;
     }
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index 0dc5a107cf..bc795adb87 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -170,7 +170,6 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
                                   BlockDriverState *target,
                                   const char *filter_node_name,
                                   uint64_t cluster_size,
-                                  BackupPerf *perf,
                                   BdrvRequestFlags write_flags,
                                   BlockCopyState **bcs,
                                   Error **errp)
@@ -217,8 +216,7 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
 
     state->cluster_size = cluster_size;
     state->bcs = block_copy_state_new(top->backing, state->target,
-                                      cluster_size, perf->use_copy_range,
-                                      write_flags, errp);
+                                      cluster_size, false, write_flags, errp);
     if (!state->bcs) {
         error_prepend(errp, "Cannot create block-copy-state: ");
         goto fail;
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 09/33] block-copy: always set BDRV_REQ_SERIALISING flag
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (7 preceding siblings ...)
  2021-05-20 14:21 ` [PATCH v2 08/33] block/backup: drop support for copy_range Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:21 ` Vladimir Sementsov-Ogievskiy
  2021-05-20 14:21 ` [PATCH v2 10/33] block/backup: move cluster size calculation to block-copy Vladimir Sementsov-Ogievskiy
                   ` (24 subsequent siblings)
  33 siblings, 0 replies; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:21 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

It won't hurt in common case, so let's not bother with detecting image
fleecing.

Also, we want to simplify initialization interface of copy-before-write
filter as we are going to make it public.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block/copy-before-write.h  |  2 +-
 include/block/block-copy.h |  3 +--
 block/backup.c             | 20 +-------------------
 block/block-copy.c         | 29 ++++++++++++++++++++++++++---
 block/copy-before-write.c  |  4 ++--
 5 files changed, 31 insertions(+), 27 deletions(-)

diff --git a/block/copy-before-write.h b/block/copy-before-write.h
index e284dfb6a7..538aab8bdb 100644
--- a/block/copy-before-write.h
+++ b/block/copy-before-write.h
@@ -33,7 +33,7 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
                                   BlockDriverState *target,
                                   const char *filter_node_name,
                                   uint64_t cluster_size,
-                                  BdrvRequestFlags write_flags,
+                                  bool compress,
                                   BlockCopyState **bcs,
                                   Error **errp);
 void bdrv_cbw_drop(BlockDriverState *bs);
diff --git a/include/block/block-copy.h b/include/block/block-copy.h
index 338f2ea7fd..c013a20e1e 100644
--- a/include/block/block-copy.h
+++ b/include/block/block-copy.h
@@ -24,8 +24,7 @@ typedef struct BlockCopyCallState BlockCopyCallState;
 
 BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
                                      int64_t cluster_size, bool use_copy_range,
-                                     BdrvRequestFlags write_flags,
-                                     Error **errp);
+                                     bool compress, Error **errp);
 
 void block_copy_set_progress_meter(BlockCopyState *s, ProgressMeter *pm);
 
diff --git a/block/backup.c b/block/backup.c
index d41dd30e25..29db3a0ef6 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -407,7 +407,6 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
     int64_t len, target_len;
     BackupBlockJob *job = NULL;
     int64_t cluster_size;
-    BdrvRequestFlags write_flags;
     BlockDriverState *cbw = NULL;
     BlockCopyState *bcs = NULL;
 
@@ -504,25 +503,8 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
         goto error;
     }
 
-    /*
-     * If source is in backing chain of target assume that target is going to be
-     * used for "image fleecing", i.e. it should represent a kind of snapshot of
-     * source at backup-start point in time. And target is going to be read by
-     * somebody (for example, used as NBD export) during backup job.
-     *
-     * In this case, we need to add BDRV_REQ_SERIALISING write flag to avoid
-     * intersection of backup writes and third party reads from target,
-     * otherwise reading from target we may occasionally read already updated by
-     * guest data.
-     *
-     * For more information see commit f8d59dfb40bb and test
-     * tests/qemu-iotests/222
-     */
-    write_flags = (bdrv_chain_contains(target, bs) ? BDRV_REQ_SERIALISING : 0) |
-                  (compress ? BDRV_REQ_WRITE_COMPRESSED : 0),
-
     cbw = bdrv_cbw_append(bs, target, filter_node_name,
-                          cluster_size, write_flags, &bcs, errp);
+                          cluster_size, compress, &bcs, errp);
     if (!cbw) {
         goto error;
     }
diff --git a/block/block-copy.c b/block/block-copy.c
index 9b4af00614..daa1a2bf9f 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -245,7 +245,7 @@ static uint32_t block_copy_max_transfer(BdrvChild *source, BdrvChild *target)
 
 BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
                                      int64_t cluster_size, bool use_copy_range,
-                                     BdrvRequestFlags write_flags, Error **errp)
+                                     bool compress, Error **errp)
 {
     BlockCopyState *s;
     BdrvDirtyBitmap *copy_bitmap;
@@ -257,6 +257,28 @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
     }
     bdrv_disable_dirty_bitmap(copy_bitmap);
 
+    /*
+     * Why we always set BDRV_REQ_SERIALISING write flag:
+     *
+     * Assume source is in backing chain of target assume that target is going
+     * to be used for "image fleecing", i.e. it should represent a kind of
+     * snapshot of source at backup-start point in time. And target is going to
+     * be read by somebody (for example, used as NBD export) during backup job.
+     *
+     * In this case, we need to add BDRV_REQ_SERIALISING write flag to avoid
+     * intersection of backup writes and third party reads from target,
+     * otherwise reading from target we may occasionally read already updated by
+     * guest data.
+     *
+     * For more information see commit f8d59dfb40bb and test
+     * tests/qemu-iotests/222
+     *
+     * Other cases? The only known reasonable case is "just copy to target, and
+     * target is not used for something else". In this case BDRV_REQ_SERIALISING
+     * change nothing, so let's not bother with detecting the "image fleecing"
+     * case and enabling BDRV_REQ_SERIALISING only for it.
+     */
+
     s = g_new(BlockCopyState, 1);
     *s = (BlockCopyState) {
         .source = source,
@@ -264,7 +286,8 @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
         .copy_bitmap = copy_bitmap,
         .cluster_size = cluster_size,
         .len = bdrv_dirty_bitmap_size(copy_bitmap),
-        .write_flags = write_flags,
+        .write_flags = BDRV_REQ_SERIALISING |
+            (compress ? BDRV_REQ_WRITE_COMPRESSED : 0),
         .mem = shres_create(BLOCK_COPY_MAX_MEM),
     };
 
@@ -277,7 +300,7 @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
          */
         s->use_copy_range = false;
         s->copy_size = cluster_size;
-    } else if (write_flags & BDRV_REQ_WRITE_COMPRESSED) {
+    } else if (compress) {
         /* Compression supports only cluster-size writes and no copy-range. */
         s->use_copy_range = false;
         s->copy_size = cluster_size;
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index bc795adb87..235251a640 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -170,7 +170,7 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
                                   BlockDriverState *target,
                                   const char *filter_node_name,
                                   uint64_t cluster_size,
-                                  BdrvRequestFlags write_flags,
+                                  bool compress,
                                   BlockCopyState **bcs,
                                   Error **errp)
 {
@@ -216,7 +216,7 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
 
     state->cluster_size = cluster_size;
     state->bcs = block_copy_state_new(top->backing, state->target,
-                                      cluster_size, false, write_flags, errp);
+                                      cluster_size, false, compress, errp);
     if (!state->bcs) {
         error_prepend(errp, "Cannot create block-copy-state: ");
         goto fail;
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 10/33] block/backup: move cluster size calculation to block-copy
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (8 preceding siblings ...)
  2021-05-20 14:21 ` [PATCH v2 09/33] block-copy: always set BDRV_REQ_SERIALISING flag Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:21 ` Vladimir Sementsov-Ogievskiy
  2021-05-20 14:21 ` [PATCH v2 11/33] block/copy-before-write: relax permission requirements when no parents Vladimir Sementsov-Ogievskiy
                   ` (23 subsequent siblings)
  33 siblings, 0 replies; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:21 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

The main consumer of cluster-size is block-copy. Let's calculate it
here instead of passing through backup-top.

We are going to publish copy-before-write filter soon, so it will be
created through options. But we don't want for now to make explicit
option for cluster-size, let's continue to calculate it automatically.
So, now is the time to get rid of cluster_size argument for
bdrv_cbw_append().

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block/copy-before-write.h  |  1 -
 include/block/block-copy.h |  5 +--
 block/backup.c             | 62 ++++++--------------------------------
 block/block-copy.c         | 51 ++++++++++++++++++++++++++++++-
 block/copy-before-write.c  | 10 +++---
 5 files changed, 66 insertions(+), 63 deletions(-)

diff --git a/block/copy-before-write.h b/block/copy-before-write.h
index 538aab8bdb..b386fd8f01 100644
--- a/block/copy-before-write.h
+++ b/block/copy-before-write.h
@@ -32,7 +32,6 @@
 BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
                                   BlockDriverState *target,
                                   const char *filter_node_name,
-                                  uint64_t cluster_size,
                                   bool compress,
                                   BlockCopyState **bcs,
                                   Error **errp);
diff --git a/include/block/block-copy.h b/include/block/block-copy.h
index c013a20e1e..8138686eb4 100644
--- a/include/block/block-copy.h
+++ b/include/block/block-copy.h
@@ -23,8 +23,8 @@ typedef struct BlockCopyState BlockCopyState;
 typedef struct BlockCopyCallState BlockCopyCallState;
 
 BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
-                                     int64_t cluster_size, bool use_copy_range,
-                                     bool compress, Error **errp);
+                                     bool use_copy_range, bool compress,
+                                     Error **errp);
 
 void block_copy_set_progress_meter(BlockCopyState *s, ProgressMeter *pm);
 
@@ -86,6 +86,7 @@ void block_copy_kick(BlockCopyCallState *call_state);
 void block_copy_call_cancel(BlockCopyCallState *call_state);
 
 BdrvDirtyBitmap *block_copy_dirty_bitmap(BlockCopyState *s);
+int64_t block_copy_cluster_size(BlockCopyState *s);
 void block_copy_set_skip_unallocated(BlockCopyState *s, bool skip);
 
 #endif /* BLOCK_COPY_H */
diff --git a/block/backup.c b/block/backup.c
index 29db3a0ef6..af17149f22 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -29,8 +29,6 @@
 
 #include "block/copy-before-write.h"
 
-#define BACKUP_CLUSTER_SIZE_DEFAULT (1 << 16)
-
 typedef struct BackupBlockJob {
     BlockJob common;
     BlockDriverState *cbw;
@@ -354,43 +352,6 @@ static const BlockJobDriver backup_job_driver = {
     .set_speed = backup_set_speed,
 };
 
-static int64_t backup_calculate_cluster_size(BlockDriverState *target,
-                                             Error **errp)
-{
-    int ret;
-    BlockDriverInfo bdi;
-    bool target_does_cow = bdrv_backing_chain_next(target);
-
-    /*
-     * If there is no backing file on the target, we cannot rely on COW if our
-     * backup cluster size is smaller than the target cluster size. Even for
-     * targets with a backing file, try to avoid COW if possible.
-     */
-    ret = bdrv_get_info(target, &bdi);
-    if (ret == -ENOTSUP && !target_does_cow) {
-        /* Cluster size is not defined */
-        warn_report("The target block device doesn't provide "
-                    "information about the block size and it doesn't have a "
-                    "backing file. The default block size of %u bytes is "
-                    "used. If the actual block size of the target exceeds "
-                    "this default, the backup may be unusable",
-                    BACKUP_CLUSTER_SIZE_DEFAULT);
-        return BACKUP_CLUSTER_SIZE_DEFAULT;
-    } else if (ret < 0 && !target_does_cow) {
-        error_setg_errno(errp, -ret,
-            "Couldn't determine the cluster size of the target image, "
-            "which has no backing file");
-        error_append_hint(errp,
-            "Aborting, since this may create an unusable destination image\n");
-        return ret;
-    } else if (ret < 0 && target_does_cow) {
-        /* Not fatal; just trudge on ahead. */
-        return BACKUP_CLUSTER_SIZE_DEFAULT;
-    }
-
-    return MAX(BACKUP_CLUSTER_SIZE_DEFAULT, bdi.cluster_size);
-}
-
 BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
                   BlockDriverState *target, int64_t speed,
                   MirrorSyncMode sync_mode, BdrvDirtyBitmap *sync_bitmap,
@@ -448,11 +409,6 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
         return NULL;
     }
 
-    cluster_size = backup_calculate_cluster_size(target, errp);
-    if (cluster_size < 0) {
-        goto error;
-    }
-
     if (perf->max_workers < 1) {
         error_setg(errp, "max-workers must be greater than zero");
         return NULL;
@@ -464,13 +420,6 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
         return NULL;
     }
 
-    if (perf->max_chunk && perf->max_chunk < cluster_size) {
-        error_setg(errp, "Required max-chunk (%" PRIi64 ") is less than backup "
-                   "cluster size (%" PRIi64 ")", perf->max_chunk, cluster_size);
-        return NULL;
-    }
-
-
     if (sync_bitmap) {
         /* If we need to write to this bitmap, check that we can: */
         if (bitmap_mode != BITMAP_SYNC_MODE_NEVER &&
@@ -503,12 +452,19 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
         goto error;
     }
 
-    cbw = bdrv_cbw_append(bs, target, filter_node_name,
-                          cluster_size, compress, &bcs, errp);
+    cbw = bdrv_cbw_append(bs, target, filter_node_name, compress, &bcs, errp);
     if (!cbw) {
         goto error;
     }
 
+    cluster_size = block_copy_cluster_size(bcs);
+
+    if (perf->max_chunk && perf->max_chunk < cluster_size) {
+        error_setg(errp, "Required max-chunk (%" PRIi64 ") is less than backup "
+                   "cluster size (%" PRIi64 ")", perf->max_chunk, cluster_size);
+        goto error;
+    }
+
     /* job->len is fixed, so we can't allow resize */
     job = block_job_create(job_id, &backup_job_driver, txn, cbw,
                            0, BLK_PERM_ALL,
diff --git a/block/block-copy.c b/block/block-copy.c
index daa1a2bf9f..9389c7c6c8 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -27,6 +27,7 @@
 #define BLOCK_COPY_MAX_MEM (128 * MiB)
 #define BLOCK_COPY_MAX_WORKERS 64
 #define BLOCK_COPY_SLICE_TIME 100000000ULL /* ns */
+#define BLOCK_COPY_CLUSTER_SIZE_DEFAULT (1 << 16)
 
 static coroutine_fn int block_copy_task_entry(AioTask *task);
 
@@ -243,13 +244,56 @@ static uint32_t block_copy_max_transfer(BdrvChild *source, BdrvChild *target)
                                      target->bs->bl.max_transfer));
 }
 
+static int64_t block_copy_calculate_cluster_size(BlockDriverState *target,
+                                                 Error **errp)
+{
+    int ret;
+    BlockDriverInfo bdi;
+    bool target_does_cow = bdrv_backing_chain_next(target);
+
+    /*
+     * If there is no backing file on the target, we cannot rely on COW if our
+     * backup cluster size is smaller than the target cluster size. Even for
+     * targets with a backing file, try to avoid COW if possible.
+     */
+    ret = bdrv_get_info(target, &bdi);
+    if (ret == -ENOTSUP && !target_does_cow) {
+        /* Cluster size is not defined */
+        warn_report("The target block device doesn't provide "
+                    "information about the block size and it doesn't have a "
+                    "backing file. The default block size of %u bytes is "
+                    "used. If the actual block size of the target exceeds "
+                    "this default, the backup may be unusable",
+                    BLOCK_COPY_CLUSTER_SIZE_DEFAULT);
+        return BLOCK_COPY_CLUSTER_SIZE_DEFAULT;
+    } else if (ret < 0 && !target_does_cow) {
+        error_setg_errno(errp, -ret,
+            "Couldn't determine the cluster size of the target image, "
+            "which has no backing file");
+        error_append_hint(errp,
+            "Aborting, since this may create an unusable destination image\n");
+        return ret;
+    } else if (ret < 0 && target_does_cow) {
+        /* Not fatal; just trudge on ahead. */
+        return BLOCK_COPY_CLUSTER_SIZE_DEFAULT;
+    }
+
+    return MAX(BLOCK_COPY_CLUSTER_SIZE_DEFAULT, bdi.cluster_size);
+}
+
 BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
-                                     int64_t cluster_size, bool use_copy_range,
+                                     bool use_copy_range,
                                      bool compress, Error **errp)
 {
     BlockCopyState *s;
+    int64_t cluster_size;
     BdrvDirtyBitmap *copy_bitmap;
 
+    cluster_size = block_copy_calculate_cluster_size(target->bs, errp);
+    if (cluster_size < 0) {
+        return NULL;
+    }
+
     copy_bitmap = bdrv_create_dirty_bitmap(source->bs, cluster_size, NULL,
                                            errp);
     if (!copy_bitmap) {
@@ -843,6 +887,11 @@ BdrvDirtyBitmap *block_copy_dirty_bitmap(BlockCopyState *s)
     return s->copy_bitmap;
 }
 
+int64_t block_copy_cluster_size(BlockCopyState *s)
+{
+    return s->cluster_size;
+}
+
 void block_copy_set_skip_unallocated(BlockCopyState *s, bool skip)
 {
     s->skip_unallocated = skip;
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index 235251a640..a7996d54db 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -37,7 +37,6 @@
 typedef struct BDRVCopyBeforeWriteState {
     BlockCopyState *bcs;
     BdrvChild *target;
-    int64_t cluster_size;
 } BDRVCopyBeforeWriteState;
 
 static coroutine_fn int cbw_co_preadv(
@@ -52,13 +51,14 @@ static coroutine_fn int cbw_do_copy_before_write(BlockDriverState *bs,
 {
     BDRVCopyBeforeWriteState *s = bs->opaque;
     uint64_t off, end;
+    int64_t cluster_size = block_copy_cluster_size(s->bcs);
 
     if (flags & BDRV_REQ_WRITE_UNCHANGED) {
         return 0;
     }
 
-    off = QEMU_ALIGN_DOWN(offset, s->cluster_size);
-    end = QEMU_ALIGN_UP(offset + bytes, s->cluster_size);
+    off = QEMU_ALIGN_DOWN(offset, cluster_size);
+    end = QEMU_ALIGN_UP(offset + bytes, cluster_size);
 
     return block_copy(s->bcs, off, end - off, true);
 }
@@ -169,7 +169,6 @@ BlockDriver bdrv_cbw_filter = {
 BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
                                   BlockDriverState *target,
                                   const char *filter_node_name,
-                                  uint64_t cluster_size,
                                   bool compress,
                                   BlockCopyState **bcs,
                                   Error **errp)
@@ -214,9 +213,8 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
     }
     appended = true;
 
-    state->cluster_size = cluster_size;
     state->bcs = block_copy_state_new(top->backing, state->target,
-                                      cluster_size, false, compress, errp);
+                                      false, compress, errp);
     if (!state->bcs) {
         error_prepend(errp, "Cannot create block-copy-state: ");
         goto fail;
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 11/33] block/copy-before-write: relax permission requirements when no parents
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (9 preceding siblings ...)
  2021-05-20 14:21 ` [PATCH v2 10/33] block/backup: move cluster size calculation to block-copy Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:21 ` Vladimir Sementsov-Ogievskiy
  2021-05-20 14:21 ` [PATCH v2 12/33] block/copy-before-write: drop extra bdrv_unref on failure path Vladimir Sementsov-Ogievskiy
                   ` (22 subsequent siblings)
  33 siblings, 0 replies; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:21 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

We are going to publish copy-before-write filter. So, user should be
able to create it with blockdev-add first, specifying both filtered and
target children. And then do blockdev-reopen, to actually insert the
filter where needed.

Currently, filter unshares write permission unconditionally on source
node. It's good, but it will not allow to do blockdev-add. So, let's
relax restrictions when filter doesn't have any parent.

Test output is modified, as now permission conflict happens only when
job creates a blk parent for filter node.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block/copy-before-write.c  | 8 +++++---
 tests/qemu-iotests/283.out | 2 +-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index a7996d54db..2a51cc64e4 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -142,10 +142,12 @@ static void cbw_child_perm(BlockDriverState *bs, BdrvChild *c,
         bdrv_default_perms(bs, c, role, reopen_queue,
                            perm, shared, nperm, nshared);
 
-        if (perm & BLK_PERM_WRITE) {
-            *nperm = *nperm | BLK_PERM_CONSISTENT_READ;
+        if (!QLIST_EMPTY(&bs->parents)) {
+            if (perm & BLK_PERM_WRITE) {
+                *nperm = *nperm | BLK_PERM_CONSISTENT_READ;
+            }
+            *nshared &= ~(BLK_PERM_WRITE | BLK_PERM_RESIZE);
         }
-        *nshared &= ~(BLK_PERM_WRITE | BLK_PERM_RESIZE);
     }
 }
 
diff --git a/tests/qemu-iotests/283.out b/tests/qemu-iotests/283.out
index e08f807dab..d5350ce7a7 100644
--- a/tests/qemu-iotests/283.out
+++ b/tests/qemu-iotests/283.out
@@ -5,7 +5,7 @@
 {"execute": "blockdev-add", "arguments": {"driver": "blkdebug", "image": "base", "node-name": "other", "take-child-perms": ["write"]}}
 {"return": {}}
 {"execute": "blockdev-backup", "arguments": {"device": "source", "sync": "full", "target": "target"}}
-{"error": {"class": "GenericError", "desc": "Cannot append copy-before-write filter: Conflicts with use by source as 'image', which does not allow 'write' on base"}}
+{"error": {"class": "GenericError", "desc": "Conflicts with use by source as 'image', which does not allow 'write' on base"}}
 
 === copy-before-write filter should be gone after job-finalize ===
 
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 12/33] block/copy-before-write: drop extra bdrv_unref on failure path
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (10 preceding siblings ...)
  2021-05-20 14:21 ` [PATCH v2 11/33] block/copy-before-write: relax permission requirements when no parents Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:21 ` Vladimir Sementsov-Ogievskiy
  2021-05-31 16:19   ` Max Reitz
  2021-05-20 14:21 ` [PATCH v2 13/33] block/copy-before-write: use file child instead of backing Vladimir Sementsov-Ogievskiy
                   ` (21 subsequent siblings)
  33 siblings, 1 reply; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:21 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

bdrv_attach_child() do bdrv_unref() on failure, so we shouldn't do it
by hand here.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/copy-before-write.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index 2a51cc64e4..945d9340f4 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -201,7 +201,6 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
     state->target = bdrv_attach_child(top, target, "target", &child_of_bds,
                                       BDRV_CHILD_DATA, errp);
     if (!state->target) {
-        bdrv_unref(target);
         bdrv_unref(top);
         return NULL;
     }
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 13/33] block/copy-before-write: use file child instead of backing
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (11 preceding siblings ...)
  2021-05-20 14:21 ` [PATCH v2 12/33] block/copy-before-write: drop extra bdrv_unref on failure path Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:21 ` Vladimir Sementsov-Ogievskiy
  2021-05-31 16:22   ` Max Reitz
  2021-05-20 14:21 ` [PATCH v2 14/33] block/copy-before-write: bdrv_cbw_append(): replace child at last Vladimir Sementsov-Ogievskiy
                   ` (20 subsequent siblings)
  33 siblings, 1 reply; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:21 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

We are going to publish copy-before-write filter, and there no public
backing-child-based filter in Qemu. No reason to create a precedent, so
let's refactor copy-before-write filter instead.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/copy-before-write.c | 39 ++++++++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index 945d9340f4..7a6c15f141 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -43,7 +43,7 @@ static coroutine_fn int cbw_co_preadv(
         BlockDriverState *bs, uint64_t offset, uint64_t bytes,
         QEMUIOVector *qiov, int flags)
 {
-    return bdrv_co_preadv(bs->backing, offset, bytes, qiov, flags);
+    return bdrv_co_preadv(bs->file, offset, bytes, qiov, flags);
 }
 
 static coroutine_fn int cbw_do_copy_before_write(BlockDriverState *bs,
@@ -71,7 +71,7 @@ static int coroutine_fn cbw_co_pdiscard(BlockDriverState *bs,
         return ret;
     }
 
-    return bdrv_co_pdiscard(bs->backing, offset, bytes);
+    return bdrv_co_pdiscard(bs->file, offset, bytes);
 }
 
 static int coroutine_fn cbw_co_pwrite_zeroes(BlockDriverState *bs,
@@ -82,7 +82,7 @@ static int coroutine_fn cbw_co_pwrite_zeroes(BlockDriverState *bs,
         return ret;
     }
 
-    return bdrv_co_pwrite_zeroes(bs->backing, offset, bytes, flags);
+    return bdrv_co_pwrite_zeroes(bs->file, offset, bytes, flags);
 }
 
 static coroutine_fn int cbw_co_pwritev(BlockDriverState *bs,
@@ -95,29 +95,22 @@ static coroutine_fn int cbw_co_pwritev(BlockDriverState *bs,
         return ret;
     }
 
-    return bdrv_co_pwritev(bs->backing, offset, bytes, qiov, flags);
+    return bdrv_co_pwritev(bs->file, offset, bytes, qiov, flags);
 }
 
 static int coroutine_fn cbw_co_flush(BlockDriverState *bs)
 {
-    if (!bs->backing) {
+    if (!bs->file) {
         return 0;
     }
 
-    return bdrv_co_flush(bs->backing->bs);
+    return bdrv_co_flush(bs->file->bs);
 }
 
 static void cbw_refresh_filename(BlockDriverState *bs)
 {
-    if (bs->backing == NULL) {
-        /*
-         * we can be here after failed bdrv_attach_child in
-         * bdrv_set_backing_hd
-         */
-        return;
-    }
     pstrcpy(bs->exact_filename, sizeof(bs->exact_filename),
-            bs->backing->bs->filename);
+            bs->file->bs->filename);
 }
 
 static void cbw_child_perm(BlockDriverState *bs, BdrvChild *c,
@@ -186,6 +179,7 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
     top = bdrv_new_open_driver(&bdrv_cbw_filter, filter_node_name,
                                BDRV_O_RDWR, errp);
     if (!top) {
+        error_prepend(errp, "Cannot open driver: ");
         return NULL;
     }
 
@@ -201,21 +195,32 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
     state->target = bdrv_attach_child(top, target, "target", &child_of_bds,
                                       BDRV_CHILD_DATA, errp);
     if (!state->target) {
+        error_prepend(errp, "Cannot attach target child: ");
+        bdrv_unref(top);
+        return NULL;
+    }
+
+    bdrv_ref(source);
+    top->file = bdrv_attach_child(top, source, "file", &child_of_bds,
+                                  BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY,
+                                  errp);
+    if (!top->file) {
+        error_prepend(errp, "Cannot attach file child: ");
         bdrv_unref(top);
         return NULL;
     }
 
     bdrv_drained_begin(source);
 
-    ret = bdrv_append(top, source, errp);
+    ret = bdrv_replace_node(source, top, errp);
     if (ret < 0) {
         error_prepend(errp, "Cannot append copy-before-write filter: ");
         goto fail;
     }
     appended = true;
 
-    state->bcs = block_copy_state_new(top->backing, state->target,
-                                      false, compress, errp);
+    state->bcs = block_copy_state_new(top->file, state->target, false, compress,
+                                      errp);
     if (!state->bcs) {
         error_prepend(errp, "Cannot create block-copy-state: ");
         goto fail;
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 14/33] block/copy-before-write: bdrv_cbw_append(): replace child at last
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (12 preceding siblings ...)
  2021-05-20 14:21 ` [PATCH v2 13/33] block/copy-before-write: use file child instead of backing Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:21 ` Vladimir Sementsov-Ogievskiy
  2021-05-31 16:55   ` Max Reitz
  2021-05-20 14:21 ` [PATCH v2 15/33] block/copy-before-write: introduce cbw_init() Vladimir Sementsov-Ogievskiy
                   ` (19 subsequent siblings)
  33 siblings, 1 reply; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:21 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

Refactor the function to replace child at last. Thus we don't need to
revert it and code is simplified.

block-copy state initilization being done before replacing the child
doesn't need any drained section.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block/copy-before-write.c | 33 +++++++++++----------------------
 1 file changed, 11 insertions(+), 22 deletions(-)

diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index 7a6c15f141..adbbc64aa9 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -172,7 +172,6 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
     int ret;
     BDRVCopyBeforeWriteState *state;
     BlockDriverState *top;
-    bool appended = false;
 
     assert(source->total_sectors == target->total_sectors);
 
@@ -196,8 +195,7 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
                                       BDRV_CHILD_DATA, errp);
     if (!state->target) {
         error_prepend(errp, "Cannot attach target child: ");
-        bdrv_unref(top);
-        return NULL;
+        goto fail;
     }
 
     bdrv_ref(source);
@@ -206,18 +204,8 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
                                   errp);
     if (!top->file) {
         error_prepend(errp, "Cannot attach file child: ");
-        bdrv_unref(top);
-        return NULL;
-    }
-
-    bdrv_drained_begin(source);
-
-    ret = bdrv_replace_node(source, top, errp);
-    if (ret < 0) {
-        error_prepend(errp, "Cannot append copy-before-write filter: ");
         goto fail;
     }
-    appended = true;
 
     state->bcs = block_copy_state_new(top->file, state->target, false, compress,
                                       errp);
@@ -225,21 +213,22 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
         error_prepend(errp, "Cannot create block-copy-state: ");
         goto fail;
     }
-    *bcs = state->bcs;
 
+    bdrv_drained_begin(source);
+    ret = bdrv_replace_node(source, top, errp);
     bdrv_drained_end(source);
+    if (ret < 0) {
+        error_prepend(errp, "Cannot append copy-before-write filter: ");
+        goto fail;
+    }
+
+    *bcs = state->bcs;
 
     return top;
 
 fail:
-    if (appended) {
-        bdrv_cbw_drop(top);
-    } else {
-        bdrv_unref(top);
-    }
-
-    bdrv_drained_end(source);
-
+    block_copy_state_free(state->bcs);
+    bdrv_unref(top);
     return NULL;
 }
 
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 15/33] block/copy-before-write: introduce cbw_init()
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (13 preceding siblings ...)
  2021-05-20 14:21 ` [PATCH v2 14/33] block/copy-before-write: bdrv_cbw_append(): replace child at last Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:21 ` Vladimir Sementsov-Ogievskiy
  2021-05-31 16:57   ` Max Reitz
  2021-05-20 14:21 ` [PATCH v2 16/33] block/copy-before-write: cbw_init(): rename variables Vladimir Sementsov-Ogievskiy
                   ` (18 subsequent siblings)
  33 siblings, 1 reply; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:21 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

Move part of bdrv_cbw_append() to new function cbw_open(). It's an
intermediate step for adding noramal .bdrv_open() handler to the
filter. With this commit no logic is changed, but we have a function
which will be turned into .bdrv_open() handler in future commit.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block/copy-before-write.c | 69 +++++++++++++++++++++++----------------
 1 file changed, 41 insertions(+), 28 deletions(-)

diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index adbbc64aa9..a4fee645fd 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -144,6 +144,45 @@ static void cbw_child_perm(BlockDriverState *bs, BdrvChild *c,
     }
 }
 
+static int cbw_init(BlockDriverState *top, BlockDriverState *source,
+                    BlockDriverState *target, bool compress, Error **errp)
+{
+    BDRVCopyBeforeWriteState *state = top->opaque;
+
+    top->total_sectors = source->total_sectors;
+    top->supported_write_flags = BDRV_REQ_WRITE_UNCHANGED |
+            (BDRV_REQ_FUA & source->supported_write_flags);
+    top->supported_zero_flags = BDRV_REQ_WRITE_UNCHANGED |
+            ((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK) &
+             source->supported_zero_flags);
+
+    bdrv_ref(target);
+    state->target = bdrv_attach_child(top, target, "target", &child_of_bds,
+                                      BDRV_CHILD_DATA, errp);
+    if (!state->target) {
+        error_prepend(errp, "Cannot attach target child: ");
+        return -EINVAL;
+    }
+
+    bdrv_ref(source);
+    top->file = bdrv_attach_child(top, source, "file", &child_of_bds,
+                                  BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY,
+                                  errp);
+    if (!top->file) {
+        error_prepend(errp, "Cannot attach file child: ");
+        return -EINVAL;
+    }
+
+    state->bcs = block_copy_state_new(top->file, state->target, false, compress,
+                                      errp);
+    if (!state->bcs) {
+        error_prepend(errp, "Cannot create block-copy-state: ");
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
 BlockDriver bdrv_cbw_filter = {
     .format_name = "copy-before-write",
     .instance_size = sizeof(BDRVCopyBeforeWriteState),
@@ -181,36 +220,10 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
         error_prepend(errp, "Cannot open driver: ");
         return NULL;
     }
-
     state = top->opaque;
-    top->total_sectors = source->total_sectors;
-    top->supported_write_flags = BDRV_REQ_WRITE_UNCHANGED |
-            (BDRV_REQ_FUA & source->supported_write_flags);
-    top->supported_zero_flags = BDRV_REQ_WRITE_UNCHANGED |
-            ((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK) &
-             source->supported_zero_flags);
-
-    bdrv_ref(target);
-    state->target = bdrv_attach_child(top, target, "target", &child_of_bds,
-                                      BDRV_CHILD_DATA, errp);
-    if (!state->target) {
-        error_prepend(errp, "Cannot attach target child: ");
-        goto fail;
-    }
 
-    bdrv_ref(source);
-    top->file = bdrv_attach_child(top, source, "file", &child_of_bds,
-                                  BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY,
-                                  errp);
-    if (!top->file) {
-        error_prepend(errp, "Cannot attach file child: ");
-        goto fail;
-    }
-
-    state->bcs = block_copy_state_new(top->file, state->target, false, compress,
-                                      errp);
-    if (!state->bcs) {
-        error_prepend(errp, "Cannot create block-copy-state: ");
+    ret = cbw_init(top, source, target, compress, errp);
+    if (ret < 0) {
         goto fail;
     }
 
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 16/33] block/copy-before-write: cbw_init(): rename variables
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (14 preceding siblings ...)
  2021-05-20 14:21 ` [PATCH v2 15/33] block/copy-before-write: introduce cbw_init() Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:21 ` Vladimir Sementsov-Ogievskiy
  2021-05-20 14:21 ` [PATCH v2 17/33] block/copy-before-write: cbw_init(): use file child after attaching Vladimir Sementsov-Ogievskiy
                   ` (17 subsequent siblings)
  33 siblings, 0 replies; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:21 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

One more step closer to real .bdrv_open() handler: use more usual names
for bs being initialized and its state.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block/copy-before-write.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index a4fee645fd..d7f1833efa 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -144,38 +144,37 @@ static void cbw_child_perm(BlockDriverState *bs, BdrvChild *c,
     }
 }
 
-static int cbw_init(BlockDriverState *top, BlockDriverState *source,
+static int cbw_init(BlockDriverState *bs, BlockDriverState *source,
                     BlockDriverState *target, bool compress, Error **errp)
 {
-    BDRVCopyBeforeWriteState *state = top->opaque;
+    BDRVCopyBeforeWriteState *s = bs->opaque;
 
-    top->total_sectors = source->total_sectors;
-    top->supported_write_flags = BDRV_REQ_WRITE_UNCHANGED |
+    bs->total_sectors = source->total_sectors;
+    bs->supported_write_flags = BDRV_REQ_WRITE_UNCHANGED |
             (BDRV_REQ_FUA & source->supported_write_flags);
-    top->supported_zero_flags = BDRV_REQ_WRITE_UNCHANGED |
+    bs->supported_zero_flags = BDRV_REQ_WRITE_UNCHANGED |
             ((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK) &
              source->supported_zero_flags);
 
     bdrv_ref(target);
-    state->target = bdrv_attach_child(top, target, "target", &child_of_bds,
-                                      BDRV_CHILD_DATA, errp);
-    if (!state->target) {
+    s->target = bdrv_attach_child(bs, target, "target", &child_of_bds,
+                                  BDRV_CHILD_DATA, errp);
+    if (!s->target) {
         error_prepend(errp, "Cannot attach target child: ");
         return -EINVAL;
     }
 
     bdrv_ref(source);
-    top->file = bdrv_attach_child(top, source, "file", &child_of_bds,
-                                  BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY,
-                                  errp);
-    if (!top->file) {
+    bs->file = bdrv_attach_child(bs, source, "file", &child_of_bds,
+                                 BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY,
+                                 errp);
+    if (!bs->file) {
         error_prepend(errp, "Cannot attach file child: ");
         return -EINVAL;
     }
 
-    state->bcs = block_copy_state_new(top->file, state->target, false, compress,
-                                      errp);
-    if (!state->bcs) {
+    s->bcs = block_copy_state_new(bs->file, s->target, false, compress, errp);
+    if (!s->bcs) {
         error_prepend(errp, "Cannot create block-copy-state: ");
         return -EINVAL;
     }
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 17/33] block/copy-before-write: cbw_init(): use file child after attaching
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (15 preceding siblings ...)
  2021-05-20 14:21 ` [PATCH v2 16/33] block/copy-before-write: cbw_init(): rename variables Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:21 ` Vladimir Sementsov-Ogievskiy
  2021-05-20 14:21 ` [PATCH v2 18/33] block/copy-before-write: cbw_init(): use options Vladimir Sementsov-Ogievskiy
                   ` (16 subsequent siblings)
  33 siblings, 0 replies; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:21 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

In the next commit we'll get rid of source argument of cbw_init().
Prepare to it now, to make next commit simpler: move the code block
that uses source below attaching the child and use bs->file->bs instead
of source variable.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block/copy-before-write.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index d7f1833efa..4858dcf8ff 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -149,13 +149,6 @@ static int cbw_init(BlockDriverState *bs, BlockDriverState *source,
 {
     BDRVCopyBeforeWriteState *s = bs->opaque;
 
-    bs->total_sectors = source->total_sectors;
-    bs->supported_write_flags = BDRV_REQ_WRITE_UNCHANGED |
-            (BDRV_REQ_FUA & source->supported_write_flags);
-    bs->supported_zero_flags = BDRV_REQ_WRITE_UNCHANGED |
-            ((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK) &
-             source->supported_zero_flags);
-
     bdrv_ref(target);
     s->target = bdrv_attach_child(bs, target, "target", &child_of_bds,
                                   BDRV_CHILD_DATA, errp);
@@ -173,6 +166,13 @@ static int cbw_init(BlockDriverState *bs, BlockDriverState *source,
         return -EINVAL;
     }
 
+    bs->total_sectors = bs->file->bs->total_sectors;
+    bs->supported_write_flags = BDRV_REQ_WRITE_UNCHANGED |
+            (BDRV_REQ_FUA & bs->file->bs->supported_write_flags);
+    bs->supported_zero_flags = BDRV_REQ_WRITE_UNCHANGED |
+            ((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK) &
+             bs->file->bs->supported_zero_flags);
+
     s->bcs = block_copy_state_new(bs->file, s->target, false, compress, errp);
     if (!s->bcs) {
         error_prepend(errp, "Cannot create block-copy-state: ");
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 18/33] block/copy-before-write: cbw_init(): use options
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (16 preceding siblings ...)
  2021-05-20 14:21 ` [PATCH v2 17/33] block/copy-before-write: cbw_init(): use file child after attaching Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:21 ` Vladimir Sementsov-Ogievskiy
  2021-05-31 17:03   ` Max Reitz
  2021-05-20 14:21 ` [PATCH v2 19/33] block/copy-before-write: initialize block-copy bitmap Vladimir Sementsov-Ogievskiy
                   ` (15 subsequent siblings)
  33 siblings, 1 reply; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:21 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

One more step closer to .bdrv_open(): use options instead of plain
arguments. Move to bdrv_open_child() calls, native for drive open
handlers.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/copy-before-write.c | 40 +++++++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index 4858dcf8ff..e889af7b80 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -144,25 +144,21 @@ static void cbw_child_perm(BlockDriverState *bs, BdrvChild *c,
     }
 }
 
-static int cbw_init(BlockDriverState *bs, BlockDriverState *source,
-                    BlockDriverState *target, bool compress, Error **errp)
+static int cbw_init(BlockDriverState *bs, QDict *options, Error **errp)
 {
     BDRVCopyBeforeWriteState *s = bs->opaque;
+    bool compress;
 
-    bdrv_ref(target);
-    s->target = bdrv_attach_child(bs, target, "target", &child_of_bds,
-                                  BDRV_CHILD_DATA, errp);
-    if (!s->target) {
-        error_prepend(errp, "Cannot attach target child: ");
+    bs->file = bdrv_open_child(NULL, options, "file", bs, &child_of_bds,
+                               BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY,
+                               false, errp);
+    if (!bs->file) {
         return -EINVAL;
     }
 
-    bdrv_ref(source);
-    bs->file = bdrv_attach_child(bs, source, "file", &child_of_bds,
-                                 BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY,
-                                 errp);
-    if (!bs->file) {
-        error_prepend(errp, "Cannot attach file child: ");
+    s->target = bdrv_open_child(NULL, options, "target", bs, &child_of_bds,
+                                BDRV_CHILD_DATA, false, errp);
+    if (!s->target) {
         return -EINVAL;
     }
 
@@ -173,6 +169,15 @@ static int cbw_init(BlockDriverState *bs, BlockDriverState *source,
             ((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK) &
              bs->file->bs->supported_zero_flags);
 
+    /*
+     * The option x-deprecated-compress is only for internal use, to support
+     * compress option of backup job. New users should instead use compress
+     * filter, if they want output to be compressed.
+     * Moreover compress option of backup should probably be deprecated too.
+     * So, x-deprecated-compress is not visible through QMP.
+     */
+    compress = qdict_get_try_bool(options, "x-deprecated-compress", false);
+    qdict_del(options, "x-deprecated-compress");
     s->bcs = block_copy_state_new(bs->file, s->target, false, compress, errp);
     if (!s->bcs) {
         error_prepend(errp, "Cannot create block-copy-state: ");
@@ -210,6 +215,7 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
     int ret;
     BDRVCopyBeforeWriteState *state;
     BlockDriverState *top;
+    QDict *opts;
 
     assert(source->total_sectors == target->total_sectors);
 
@@ -221,7 +227,13 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
     }
     state = top->opaque;
 
-    ret = cbw_init(top, source, target, compress, errp);
+    opts = qdict_new();
+    qdict_put_str(opts, "file", bdrv_get_node_name(source));
+    qdict_put_str(opts, "target", bdrv_get_node_name(target));
+    qdict_put_bool(opts, "x-deprecated-compress", compress);
+
+    ret = cbw_init(top, opts, errp);
+    qobject_unref(opts);
     if (ret < 0) {
         goto fail;
     }
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 19/33] block/copy-before-write: initialize block-copy bitmap
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (17 preceding siblings ...)
  2021-05-20 14:21 ` [PATCH v2 18/33] block/copy-before-write: cbw_init(): use options Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:21 ` Vladimir Sementsov-Ogievskiy
  2021-05-31 17:10   ` Max Reitz
  2021-05-20 14:21 ` [PATCH v2 20/33] block/block-copy: make setting progress optional Vladimir Sementsov-Ogievskiy
                   ` (14 subsequent siblings)
  33 siblings, 1 reply; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:21 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

We are going to publish copy-before-write filter to be used in separate
of backup. Future step would support bitmap for the filter. But let's
start from full set bitmap.

We have to modify backup, as bitmap is first initialized by
copy-before-write filter, and then backup modifies it.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/backup.c            | 16 +++++++---------
 block/copy-before-write.c |  4 ++++
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index af17149f22..14652ac98a 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -233,18 +233,16 @@ static void backup_init_bcs_bitmap(BackupBlockJob *job)
     BdrvDirtyBitmap *bcs_bitmap = block_copy_dirty_bitmap(job->bcs);
 
     if (job->sync_mode == MIRROR_SYNC_MODE_BITMAP) {
+        bdrv_clear_dirty_bitmap(bcs_bitmap, NULL);
         ret = bdrv_dirty_bitmap_merge_internal(bcs_bitmap, job->sync_bitmap,
                                                NULL, true);
         assert(ret);
-    } else {
-        if (job->sync_mode == MIRROR_SYNC_MODE_TOP) {
-            /*
-             * We can't hog the coroutine to initialize this thoroughly.
-             * Set a flag and resume work when we are able to yield safely.
-             */
-            block_copy_set_skip_unallocated(job->bcs, true);
-        }
-        bdrv_set_dirty_bitmap(bcs_bitmap, 0, job->len);
+    } else if (job->sync_mode == MIRROR_SYNC_MODE_TOP) {
+        /*
+         * We can't hog the coroutine to initialize this thoroughly.
+         * Set a flag and resume work when we are able to yield safely.
+         */
+        block_copy_set_skip_unallocated(job->bcs, true);
     }
 
     estimate = bdrv_get_dirty_count(bcs_bitmap);
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index e889af7b80..ba2466a328 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -147,6 +147,7 @@ static void cbw_child_perm(BlockDriverState *bs, BdrvChild *c,
 static int cbw_init(BlockDriverState *bs, QDict *options, Error **errp)
 {
     BDRVCopyBeforeWriteState *s = bs->opaque;
+    BdrvDirtyBitmap *copy_bitmap;
     bool compress;
 
     bs->file = bdrv_open_child(NULL, options, "file", bs, &child_of_bds,
@@ -184,6 +185,9 @@ static int cbw_init(BlockDriverState *bs, QDict *options, Error **errp)
         return -EINVAL;
     }
 
+    copy_bitmap = block_copy_dirty_bitmap(s->bcs);
+    bdrv_set_dirty_bitmap(copy_bitmap, 0, bdrv_dirty_bitmap_size(copy_bitmap));
+
     return 0;
 }
 
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 20/33] block/block-copy: make setting progress optional
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (18 preceding siblings ...)
  2021-05-20 14:21 ` [PATCH v2 19/33] block/copy-before-write: initialize block-copy bitmap Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:21 ` Vladimir Sementsov-Ogievskiy
  2021-08-10 15:22   ` Hanna Reitz
  2021-05-20 14:21 ` [PATCH v2 21/33] block/copy-before-write: make public block driver Vladimir Sementsov-Ogievskiy
                   ` (13 subsequent siblings)
  33 siblings, 1 reply; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:21 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

Now block-copy will crash if user don't set progress meter by
block_copy_set_progress_meter(). copy-before-write filter will be used
in separate of backup job, and it doesn't want any progress meter (for
now). So, allow not setting it.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block/block-copy.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/block/block-copy.c b/block/block-copy.c
index 9389c7c6c8..0a9c3692bf 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -509,7 +509,7 @@ static coroutine_fn int block_copy_task_entry(AioTask *task)
     if (ret < 0 && !t->call_state->ret) {
         t->call_state->ret = ret;
         t->call_state->error_is_read = error_is_read;
-    } else {
+    } else if (t->s->progress) {
         progress_work_done(t->s->progress, t->bytes);
     }
     co_put_to_shres(t->s->mem, t->bytes);
@@ -613,9 +613,11 @@ int64_t block_copy_reset_unallocated(BlockCopyState *s,
 
     if (!ret) {
         bdrv_reset_dirty_bitmap(s->copy_bitmap, offset, bytes);
-        progress_set_remaining(s->progress,
-                               bdrv_get_dirty_count(s->copy_bitmap) +
-                               s->in_flight_bytes);
+        if (s->progress) {
+            progress_set_remaining(s->progress,
+                                   bdrv_get_dirty_count(s->copy_bitmap) +
+                                   s->in_flight_bytes);
+        }
     }
 
     *count = bytes;
@@ -675,9 +677,11 @@ block_copy_dirty_clusters(BlockCopyCallState *call_state)
         }
         if (s->skip_unallocated && !(ret & BDRV_BLOCK_ALLOCATED)) {
             block_copy_task_end(task, 0);
-            progress_set_remaining(s->progress,
-                                   bdrv_get_dirty_count(s->copy_bitmap) +
-                                   s->in_flight_bytes);
+            if (s->progress) {
+                progress_set_remaining(s->progress,
+                                       bdrv_get_dirty_count(s->copy_bitmap) +
+                                       s->in_flight_bytes);
+            }
             trace_block_copy_skip_range(s, task->offset, task->bytes);
             offset = task_end(task);
             bytes = end - offset;
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 21/33] block/copy-before-write: make public block driver
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (19 preceding siblings ...)
  2021-05-20 14:21 ` [PATCH v2 20/33] block/block-copy: make setting progress optional Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:21 ` Vladimir Sementsov-Ogievskiy
  2021-05-20 14:21 ` [PATCH v2 22/33] qapi: publish copy-before-write filter Vladimir Sementsov-Ogievskiy
                   ` (12 subsequent siblings)
  33 siblings, 0 replies; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:21 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

Finally, copy-before-write gets own .bdrv_open and .bdrv_close
handlers, block_init() call and becomes available through bdrv_open().

To achieve this:

 - cbw_init gets unused flags argument and becomes cbw_open
 - block_copy_state_free() call moved to new cbw_close()
 - in bdrv_cbw_append:
   - options are completed with driver and node-name, and we can simply
     use bdrv_insert_node() to do both open and drained replacing
 - in bdrv_cbw_drop:
   - cbw_close() is now responsible for freeing s->bcs, so don't do it
     here

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block/copy-before-write.c | 60 ++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 32 deletions(-)

diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index ba2466a328..6ed5bce1f1 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -144,7 +144,8 @@ static void cbw_child_perm(BlockDriverState *bs, BdrvChild *c,
     }
 }
 
-static int cbw_init(BlockDriverState *bs, QDict *options, Error **errp)
+static int cbw_open(BlockDriverState *bs, QDict *options, int flags,
+                    Error **errp)
 {
     BDRVCopyBeforeWriteState *s = bs->opaque;
     BdrvDirtyBitmap *copy_bitmap;
@@ -191,10 +192,21 @@ static int cbw_init(BlockDriverState *bs, QDict *options, Error **errp)
     return 0;
 }
 
+static void cbw_close(BlockDriverState *bs)
+{
+    BDRVCopyBeforeWriteState *s = bs->opaque;
+
+    block_copy_state_free(s->bcs);
+    s->bcs = NULL;
+}
+
 BlockDriver bdrv_cbw_filter = {
     .format_name = "copy-before-write",
     .instance_size = sizeof(BDRVCopyBeforeWriteState),
 
+    .bdrv_open                  = cbw_open,
+    .bdrv_close                 = cbw_close,
+
     .bdrv_co_preadv             = cbw_co_preadv,
     .bdrv_co_pwritev            = cbw_co_pwritev,
     .bdrv_co_pwrite_zeroes      = cbw_co_pwrite_zeroes,
@@ -216,57 +228,41 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
                                   Error **errp)
 {
     ERRP_GUARD();
-    int ret;
     BDRVCopyBeforeWriteState *state;
     BlockDriverState *top;
     QDict *opts;
 
     assert(source->total_sectors == target->total_sectors);
 
-    top = bdrv_new_open_driver(&bdrv_cbw_filter, filter_node_name,
-                               BDRV_O_RDWR, errp);
-    if (!top) {
-        error_prepend(errp, "Cannot open driver: ");
-        return NULL;
-    }
-    state = top->opaque;
-
     opts = qdict_new();
+    qdict_put_str(opts, "driver", "copy-before-write");
+    if (filter_node_name) {
+        qdict_put_str(opts, "node-name", filter_node_name);
+    }
     qdict_put_str(opts, "file", bdrv_get_node_name(source));
     qdict_put_str(opts, "target", bdrv_get_node_name(target));
     qdict_put_bool(opts, "x-deprecated-compress", compress);
 
-    ret = cbw_init(top, opts, errp);
-    qobject_unref(opts);
-    if (ret < 0) {
-        goto fail;
-    }
-
-    bdrv_drained_begin(source);
-    ret = bdrv_replace_node(source, top, errp);
-    bdrv_drained_end(source);
-    if (ret < 0) {
-        error_prepend(errp, "Cannot append copy-before-write filter: ");
-        goto fail;
+    top = bdrv_insert_node(source, opts, BDRV_O_RDWR, errp);
+    if (!top) {
+        return NULL;
     }
 
+    state = top->opaque;
     *bcs = state->bcs;
 
     return top;
-
-fail:
-    block_copy_state_free(state->bcs);
-    bdrv_unref(top);
-    return NULL;
 }
 
 void bdrv_cbw_drop(BlockDriverState *bs)
 {
-    BDRVCopyBeforeWriteState *s = bs->opaque;
-
     bdrv_drop_filter(bs, &error_abort);
-
-    block_copy_state_free(s->bcs);
-
     bdrv_unref(bs);
 }
+
+static void cbw_init(void)
+{
+    bdrv_register(&bdrv_cbw_filter);
+}
+
+block_init(cbw_init);
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 22/33] qapi: publish copy-before-write filter
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (20 preceding siblings ...)
  2021-05-20 14:21 ` [PATCH v2 21/33] block/copy-before-write: make public block driver Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:21 ` Vladimir Sementsov-Ogievskiy
  2021-06-01  9:08   ` Max Reitz
  2021-05-20 14:21 ` [PATCH v2 23/33] python/qemu/machine.py: refactor _qemu_args() Vladimir Sementsov-Ogievskiy
                   ` (11 subsequent siblings)
  33 siblings, 1 reply; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:21 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 qapi/block-core.json | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 2ea294129e..8c4801a13d 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2808,15 +2808,17 @@
 # @blklogwrites: Since 3.0
 # @blkreplay: Since 4.2
 # @compress: Since 5.0
+# @copy-before-write: Since 6.1
 #
 # Since: 2.9
 ##
 { 'enum': 'BlockdevDriver',
   'data': [ 'blkdebug', 'blklogwrites', 'blkreplay', 'blkverify', 'bochs',
-            'cloop', 'compress', 'copy-on-read', 'dmg', 'file', 'ftp', 'ftps',
-            'gluster', 'host_cdrom', 'host_device', 'http', 'https', 'iscsi',
-            'luks', 'nbd', 'nfs', 'null-aio', 'null-co', 'nvme', 'parallels',
-            'preallocate', 'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'rbd',
+            'cloop', 'compress', 'copy-before-write', 'copy-on-read', 'dmg',
+            'file', 'ftp', 'ftps', 'gluster', 'host_cdrom', 'host_device',
+            'http', 'https', 'iscsi', 'luks', 'nbd', 'nfs', 'null-aio',
+            'null-co', 'nvme', 'parallels', 'preallocate', 'qcow', 'qcow2',
+            'qed', 'quorum', 'raw', 'rbd',
             { 'name': 'replication', 'if': 'defined(CONFIG_REPLICATION)' },
             'ssh', 'throttle', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] }
 
@@ -3937,6 +3939,25 @@
   'base': 'BlockdevOptionsGenericFormat',
   'data': { '*bottom': 'str' } }
 
+##
+# @BlockdevOptionsCbw:
+#
+# Driver specific block device options for the copy-before-write driver,
+# which does so called copy-before-write operations: when data is
+# written to the filter, the filter firstly reads corresponding blocks
+# from its file child and copies them to @target child. After successful
+# copying the write request is propagated to file child. If copying
+# filed, the original write request is failed too and no data is written
+# to file child.
+#
+# @target: The target for copy-before-write operations.
+#
+# Since: 6.1
+##
+{ 'struct': 'BlockdevOptionsCbw',
+  'base': 'BlockdevOptionsGenericFormat',
+  'data': { 'target': 'BlockdevRef' } }
+
 ##
 # @BlockdevOptions:
 #
@@ -3989,6 +4010,7 @@
       'bochs':      'BlockdevOptionsGenericFormat',
       'cloop':      'BlockdevOptionsGenericFormat',
       'compress':   'BlockdevOptionsGenericFormat',
+      'copy-before-write':'BlockdevOptionsCbw',
       'copy-on-read':'BlockdevOptionsCor',
       'dmg':        'BlockdevOptionsGenericFormat',
       'file':       'BlockdevOptionsFile',
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 23/33] python/qemu/machine.py: refactor _qemu_args()
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (21 preceding siblings ...)
  2021-05-20 14:21 ` [PATCH v2 22/33] qapi: publish copy-before-write filter Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:21 ` Vladimir Sementsov-Ogievskiy
  2021-06-01 10:05   ` Max Reitz
  2021-05-20 14:21 ` [PATCH v2 24/33] python/qemu/machine: QEMUMachine: improve qmp() method Vladimir Sementsov-Ogievskiy
                   ` (10 subsequent siblings)
  33 siblings, 1 reply; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:21 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

 - use shorter construction
 - don't create new dict if not needed
 - drop extra unpacking key-val arguments
 - drop extra default values

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 python/qemu/machine.py | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 6e44bda337..ff35f2cd6c 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -541,14 +541,12 @@ def _qmp(self) -> qmp.QEMUMonitorProtocol:
         return self._qmp_connection
 
     @classmethod
-    def _qmp_args(cls, _conv_keys: bool = True, **args: Any) -> Dict[str, Any]:
-        qmp_args = dict()
-        for key, value in args.items():
-            if _conv_keys:
-                qmp_args[key.replace('_', '-')] = value
-            else:
-                qmp_args[key] = value
-        return qmp_args
+    def _qmp_args(cls, conv_keys: bool,
+                  args: Dict[str, Any]) -> Dict[str, Any]:
+        if conv_keys:
+            return {k.replace('_', '-'): v for k, v in args.items()}
+        else:
+            return args
 
     def qmp(self, cmd: str,
             conv_keys: bool = True,
@@ -556,7 +554,7 @@ def qmp(self, cmd: str,
         """
         Invoke a QMP command and return the response dict
         """
-        qmp_args = self._qmp_args(conv_keys, **args)
+        qmp_args = self._qmp_args(conv_keys, args)
         return self._qmp.cmd(cmd, args=qmp_args)
 
     def command(self, cmd: str,
@@ -567,7 +565,7 @@ def command(self, cmd: str,
         On success return the response dict.
         On failure raise an exception.
         """
-        qmp_args = self._qmp_args(conv_keys, **args)
+        qmp_args = self._qmp_args(conv_keys, args)
         return self._qmp.command(cmd, **qmp_args)
 
     def get_qmp_event(self, wait: bool = False) -> Optional[QMPMessage]:
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 24/33] python/qemu/machine: QEMUMachine: improve qmp() method
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (22 preceding siblings ...)
  2021-05-20 14:21 ` [PATCH v2 23/33] python/qemu/machine.py: refactor _qemu_args() Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:21 ` Vladimir Sementsov-Ogievskiy
  2021-06-01 10:19   ` Max Reitz
  2021-05-20 14:21 ` [PATCH v2 25/33] iotests.py: VM: add own __enter__ method Vladimir Sementsov-Ogievskiy
                   ` (9 subsequent siblings)
  33 siblings, 1 reply; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:21 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

We often call qmp() with unpacking dict, like qmp('foo', **{...}).
mypy don't really like it, it things that passed unpacked dict is a
positional argument and complains that it type should be bool (because
second argument of qmp() is conv_keys: bool).

Allow possing dict directly, simplifying interface, and giving a way to
satisfy mypy.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 python/qemu/machine.py | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index ff35f2cd6c..7a14605040 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -549,11 +549,21 @@ def _qmp_args(cls, conv_keys: bool,
             return args
 
     def qmp(self, cmd: str,
-            conv_keys: bool = True,
+            args_dict: Optional[Dict[str, Any]] = None,
+            conv_keys: Optional[bool] = None,
             **args: Any) -> QMPMessage:
         """
         Invoke a QMP command and return the response dict
         """
+        if args_dict is not None:
+            assert not args
+            assert conv_keys is None
+            args = args_dict
+            conv_keys = False
+
+        if conv_keys is None:
+            conv_keys = True
+
         qmp_args = self._qmp_args(conv_keys, args)
         return self._qmp.cmd(cmd, args=qmp_args)
 
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 25/33] iotests.py: VM: add own __enter__ method
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (23 preceding siblings ...)
  2021-05-20 14:21 ` [PATCH v2 24/33] python/qemu/machine: QEMUMachine: improve qmp() method Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:21 ` Vladimir Sementsov-Ogievskiy
  2021-06-01 10:58   ` Max Reitz
  2021-05-20 14:21 ` [PATCH v2 26/33] iotests/222: fix pylint and mypy complains Vladimir Sementsov-Ogievskiy
                   ` (8 subsequent siblings)
  33 siblings, 1 reply; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:21 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

In superclass __enter__ method is defined with return value type hint
'QEMUMachine'. So, mypy thinks that return value of VM.__enter__ is
QEMUMachine. Let's redefine __enter__ in VM class, to give it correct
type hint.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/iotests.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 777fa2ec0e..c7ec16c082 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -568,6 +568,10 @@ def remote_filename(path):
 class VM(qtest.QEMUQtestMachine):
     '''A QEMU VM'''
 
+    # Redefine __enter__ with proper type hint
+    def __enter__(self) -> 'VM':
+        return self
+
     def __init__(self, path_suffix=''):
         name = "qemu%s-%d" % (path_suffix, os.getpid())
         super().__init__(qemu_prog, qemu_opts, name=name,
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 26/33] iotests/222: fix pylint and mypy complains
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (24 preceding siblings ...)
  2021-05-20 14:21 ` [PATCH v2 25/33] iotests.py: VM: add own __enter__ method Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:21 ` Vladimir Sementsov-Ogievskiy
  2021-06-01 11:16   ` Max Reitz
  2021-05-20 14:21 ` [PATCH v2 27/33] iotests/222: constantly use single quotes for strings Vladimir Sementsov-Ogievskiy
                   ` (7 subsequent siblings)
  33 siblings, 1 reply; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:21 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

Here:
 - long line
 - move to new interface of vm.qmp() (direct passing dict), to avoid
   mypy false-positive, as it thinks that unpacked dict is a positional
   argument.
 - extra parenthesis
 - handle event_wait possible None value

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/222 | 20 +++++++++++---------
 tests/qemu-iotests/297 |  2 +-
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/tests/qemu-iotests/222 b/tests/qemu-iotests/222
index b48afe623e..5e2556f8df 100755
--- a/tests/qemu-iotests/222
+++ b/tests/qemu-iotests/222
@@ -50,7 +50,8 @@ remainder = [("0xd5", "0x108000",  "32k"), # Right-end of partial-left [1]
 
 with iotests.FilePath('base.img') as base_img_path, \
      iotests.FilePath('fleece.img') as fleece_img_path, \
-     iotests.FilePath('nbd.sock', base_dir=iotests.sock_dir) as nbd_sock_path, \
+     iotests.FilePath('nbd.sock',
+                      base_dir=iotests.sock_dir) as nbd_sock_path, \
      iotests.VM() as vm:
 
     log('--- Setting up images ---')
@@ -81,7 +82,7 @@ with iotests.FilePath('base.img') as base_img_path, \
     tgt_node = "fleeceNode"
 
     # create tgt_node backed by src_node
-    log(vm.qmp("blockdev-add", **{
+    log(vm.qmp("blockdev-add", {
         "driver": "qcow2",
         "node-name": tgt_node,
         "file": {
@@ -103,8 +104,8 @@ with iotests.FilePath('base.img') as base_img_path, \
 
     nbd_uri = 'nbd+unix:///%s?socket=%s' % (tgt_node, nbd_sock_path)
     log(vm.qmp("nbd-server-start",
-               **{"addr": { "type": "unix",
-                            "data": { "path": nbd_sock_path } } }))
+               {"addr": { "type": "unix",
+                          "data": { "path": nbd_sock_path } } }))
 
     log(vm.qmp("nbd-server-add", device=tgt_node))
 
@@ -112,7 +113,7 @@ with iotests.FilePath('base.img') as base_img_path, \
     log('--- Sanity Check ---')
     log('')
 
-    for p in (patterns + zeroes):
+    for p in patterns + zeroes:
         cmd = "read -P%s %s %s" % p
         log(cmd)
         assert qemu_io_silent('-r', '-f', 'raw', '-c', cmd, nbd_uri) == 0
@@ -130,7 +131,7 @@ with iotests.FilePath('base.img') as base_img_path, \
     log('--- Verifying Data ---')
     log('')
 
-    for p in (patterns + zeroes):
+    for p in patterns + zeroes:
         cmd = "read -P%s %s %s" % p
         log(cmd)
         assert qemu_io_silent('-r', '-f', 'raw', '-c', cmd, nbd_uri) == 0
@@ -140,8 +141,9 @@ with iotests.FilePath('base.img') as base_img_path, \
     log('')
 
     log(vm.qmp('block-job-cancel', device=src_node))
-    log(vm.event_wait('BLOCK_JOB_CANCELLED'),
-        filters=[iotests.filter_qmp_event])
+    e = vm.event_wait('BLOCK_JOB_CANCELLED')
+    assert e is not None
+    log(e, filters=[iotests.filter_qmp_event])
     log(vm.qmp('nbd-server-stop'))
     log(vm.qmp('blockdev-del', node_name=tgt_node))
     vm.shutdown()
@@ -150,7 +152,7 @@ with iotests.FilePath('base.img') as base_img_path, \
     log('--- Confirming writes ---')
     log('')
 
-    for p in (overwrite + remainder):
+    for p in overwrite + remainder:
         cmd = "read -P%s %s %s" % p
         log(cmd)
         assert qemu_io_silent(base_img_path, '-c', cmd) == 0
diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index a37910b42d..7cb8c531fd 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -31,7 +31,7 @@ SKIP_FILES = (
     '096', '118', '124', '132', '136', '139', '147', '148', '149',
     '151', '152', '155', '163', '165', '169', '194', '196', '199', '202',
     '203', '205', '206', '207', '208', '210', '211', '212', '213', '216',
-    '218', '219', '222', '224', '228', '234', '235', '236', '237', '238',
+    '218', '219', '224', '228', '234', '235', '236', '237', '238',
     '240', '242', '245', '246', '248', '255', '256', '257', '258', '260',
     '262', '264', '266', '274', '277', '280', '281', '295', '296', '298',
     '299', '302', '303', '304', '307',
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 27/33] iotests/222: constantly use single quotes for strings
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (25 preceding siblings ...)
  2021-05-20 14:21 ` [PATCH v2 26/33] iotests/222: fix pylint and mypy complains Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:21 ` Vladimir Sementsov-Ogievskiy
  2021-06-01 11:17   ` Max Reitz
  2021-05-20 14:22 ` [PATCH v2 28/33] iotests: move 222 to tests/image-fleecing Vladimir Sementsov-Ogievskiy
                   ` (6 subsequent siblings)
  33 siblings, 1 reply; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:21 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

The file use both single and double quotes for strings. Let's be
consistent.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/222 | 68 +++++++++++++++++++++---------------------
 1 file changed, 34 insertions(+), 34 deletions(-)

diff --git a/tests/qemu-iotests/222 b/tests/qemu-iotests/222
index 5e2556f8df..799369e290 100755
--- a/tests/qemu-iotests/222
+++ b/tests/qemu-iotests/222
@@ -30,23 +30,23 @@ iotests.script_initialize(
     supported_platforms=['linux'],
 )
 
-patterns = [("0x5d", "0",         "64k"),
-            ("0xd5", "1M",        "64k"),
-            ("0xdc", "32M",       "64k"),
-            ("0xcd", "0x3ff0000", "64k")]  # 64M - 64K
+patterns = [('0x5d', '0',         '64k'),
+            ('0xd5', '1M',        '64k'),
+            ('0xdc', '32M',       '64k'),
+            ('0xcd', '0x3ff0000', '64k')]  # 64M - 64K
 
-overwrite = [("0xab", "0",         "64k"), # Full overwrite
-             ("0xad", "0x00f8000", "64k"), # Partial-left (1M-32K)
-             ("0x1d", "0x2008000", "64k"), # Partial-right (32M+32K)
-             ("0xea", "0x3fe0000", "64k")] # Adjacent-left (64M - 128K)
+overwrite = [('0xab', '0',         '64k'), # Full overwrite
+             ('0xad', '0x00f8000', '64k'), # Partial-left (1M-32K)
+             ('0x1d', '0x2008000', '64k'), # Partial-right (32M+32K)
+             ('0xea', '0x3fe0000', '64k')] # Adjacent-left (64M - 128K)
 
-zeroes = [("0", "0x00f8000", "32k"), # Left-end of partial-left (1M-32K)
-          ("0", "0x2010000", "32k"), # Right-end of partial-right (32M+64K)
-          ("0", "0x3fe0000", "64k")] # overwrite[3]
+zeroes = [('0', '0x00f8000', '32k'), # Left-end of partial-left (1M-32K)
+          ('0', '0x2010000', '32k'), # Right-end of partial-right (32M+64K)
+          ('0', '0x3fe0000', '64k')] # overwrite[3]
 
-remainder = [("0xd5", "0x108000",  "32k"), # Right-end of partial-left [1]
-             ("0xdc", "32M",       "32k"), # Left-end of partial-right [2]
-             ("0xcd", "0x3ff0000", "64k")] # patterns[3]
+remainder = [('0xd5', '0x108000',  '32k'), # Right-end of partial-left [1]
+             ('0xdc', '32M',       '32k'), # Left-end of partial-right [2]
+             ('0xcd', '0x3ff0000', '64k')] # patterns[3]
 
 with iotests.FilePath('base.img') as base_img_path, \
      iotests.FilePath('fleece.img') as fleece_img_path, \
@@ -58,7 +58,7 @@ with iotests.FilePath('base.img') as base_img_path, \
     log('')
 
     assert qemu_img('create', '-f', iotests.imgfmt, base_img_path, '64M') == 0
-    assert qemu_img('create', '-f', "qcow2", fleece_img_path, '64M') == 0
+    assert qemu_img('create', '-f', 'qcow2', fleece_img_path, '64M') == 0
 
     for p in patterns:
         qemu_io('-f', iotests.imgfmt,
@@ -78,43 +78,43 @@ with iotests.FilePath('base.img') as base_img_path, \
     log('--- Setting up Fleecing Graph ---')
     log('')
 
-    src_node = "drive0"
-    tgt_node = "fleeceNode"
+    src_node = 'drive0'
+    tgt_node = 'fleeceNode'
 
     # create tgt_node backed by src_node
-    log(vm.qmp("blockdev-add", {
-        "driver": "qcow2",
-        "node-name": tgt_node,
-        "file": {
-            "driver": "file",
-            "filename": fleece_img_path,
+    log(vm.qmp('blockdev-add', {
+        'driver': 'qcow2',
+        'node-name': tgt_node,
+        'file': {
+            'driver': 'file',
+            'filename': fleece_img_path,
         },
-        "backing": src_node,
+        'backing': src_node,
     }))
 
     # Establish COW from source to fleecing node
-    log(vm.qmp("blockdev-backup",
+    log(vm.qmp('blockdev-backup',
                device=src_node,
                target=tgt_node,
-               sync="none"))
+               sync='none'))
 
     log('')
     log('--- Setting up NBD Export ---')
     log('')
 
     nbd_uri = 'nbd+unix:///%s?socket=%s' % (tgt_node, nbd_sock_path)
-    log(vm.qmp("nbd-server-start",
-               {"addr": { "type": "unix",
-                          "data": { "path": nbd_sock_path } } }))
+    log(vm.qmp('nbd-server-start',
+               {'addr': { 'type': 'unix',
+                          'data': { 'path': nbd_sock_path } } }))
 
-    log(vm.qmp("nbd-server-add", device=tgt_node))
+    log(vm.qmp('nbd-server-add', device=tgt_node))
 
     log('')
     log('--- Sanity Check ---')
     log('')
 
     for p in patterns + zeroes:
-        cmd = "read -P%s %s %s" % p
+        cmd = 'read -P%s %s %s' % p
         log(cmd)
         assert qemu_io_silent('-r', '-f', 'raw', '-c', cmd, nbd_uri) == 0
 
@@ -123,7 +123,7 @@ with iotests.FilePath('base.img') as base_img_path, \
     log('')
 
     for p in overwrite:
-        cmd = "write -P%s %s %s" % p
+        cmd = 'write -P%s %s %s' % p
         log(cmd)
         log(vm.hmp_qemu_io(src_node, cmd))
 
@@ -132,7 +132,7 @@ with iotests.FilePath('base.img') as base_img_path, \
     log('')
 
     for p in patterns + zeroes:
-        cmd = "read -P%s %s %s" % p
+        cmd = 'read -P%s %s %s' % p
         log(cmd)
         assert qemu_io_silent('-r', '-f', 'raw', '-c', cmd, nbd_uri) == 0
 
@@ -153,7 +153,7 @@ with iotests.FilePath('base.img') as base_img_path, \
     log('')
 
     for p in overwrite + remainder:
-        cmd = "read -P%s %s %s" % p
+        cmd = 'read -P%s %s %s' % p
         log(cmd)
         assert qemu_io_silent(base_img_path, '-c', cmd) == 0
 
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 28/33] iotests: move 222 to tests/image-fleecing
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (26 preceding siblings ...)
  2021-05-20 14:21 ` [PATCH v2 27/33] iotests/222: constantly use single quotes for strings Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:22 ` Vladimir Sementsov-Ogievskiy
  2021-06-01 11:17   ` Max Reitz
  2021-05-20 14:22 ` [PATCH v2 29/33] iotests.py: hmp_qemu_io: support qdev Vladimir Sementsov-Ogievskiy
                   ` (5 subsequent siblings)
  33 siblings, 1 reply; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:22 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

Give a good name to test file.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/{222 => tests/image-fleecing}         | 0
 tests/qemu-iotests/{222.out => tests/image-fleecing.out} | 0
 2 files changed, 0 insertions(+), 0 deletions(-)
 rename tests/qemu-iotests/{222 => tests/image-fleecing} (100%)
 rename tests/qemu-iotests/{222.out => tests/image-fleecing.out} (100%)

diff --git a/tests/qemu-iotests/222 b/tests/qemu-iotests/tests/image-fleecing
similarity index 100%
rename from tests/qemu-iotests/222
rename to tests/qemu-iotests/tests/image-fleecing
diff --git a/tests/qemu-iotests/222.out b/tests/qemu-iotests/tests/image-fleecing.out
similarity index 100%
rename from tests/qemu-iotests/222.out
rename to tests/qemu-iotests/tests/image-fleecing.out
-- 
2.29.2



^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v2 29/33] iotests.py: hmp_qemu_io: support qdev
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (27 preceding siblings ...)
  2021-05-20 14:22 ` [PATCH v2 28/33] iotests: move 222 to tests/image-fleecing Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:22 ` Vladimir Sementsov-Ogievskiy
  2021-06-01 11:19   ` Max Reitz
  2021-05-20 14:22 ` [PATCH v2 30/33] iotests/image-fleecing: proper source device Vladimir Sementsov-Ogievskiy
                   ` (4 subsequent siblings)
  33 siblings, 1 reply; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:22 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/iotests.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index c7ec16c082..9fa0162b07 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -655,9 +655,10 @@ def resume_drive(self, drive: str) -> None:
         self.hmp(f'qemu-io {drive} "remove_break bp_{drive}"')
 
     def hmp_qemu_io(self, drive: str, cmd: str,
-                    use_log: bool = False) -> QMPMessage:
+                    use_log: bool = False, qdev: bool = False) -> QMPMessage:
         """Write to a given drive using an HMP command"""
-        return self.hmp(f'qemu-io {drive} "{cmd}"', use_log=use_log)
+        d = '-d ' if qdev else ''
+        return self.hmp(f'qemu-io {d}{drive} "{cmd}"', use_log=use_log)
 
     def flatten_qmp_object(self, obj, output=None, basestr=''):
         if output is None:
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 30/33] iotests/image-fleecing: proper source device
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (28 preceding siblings ...)
  2021-05-20 14:22 ` [PATCH v2 29/33] iotests.py: hmp_qemu_io: support qdev Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:22 ` Vladimir Sementsov-Ogievskiy
  2021-06-01 11:29   ` Max Reitz
  2021-05-20 14:22 ` [PATCH v2 31/33] iotests/image-fleecing: rename tgt_node Vladimir Sementsov-Ogievskiy
                   ` (3 subsequent siblings)
  33 siblings, 1 reply; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:22 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

Define scsi device to operate with it by qom-set in further patch.

Give a new node-name to source block node, to not look like device
name.

Job now don't want to work without giving explicit id, so, let's call
it "fleecing".

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/tests/image-fleecing     | 12 ++++++++----
 tests/qemu-iotests/tests/image-fleecing.out |  2 +-
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/tests/image-fleecing b/tests/qemu-iotests/tests/image-fleecing
index 799369e290..961941bb27 100755
--- a/tests/qemu-iotests/tests/image-fleecing
+++ b/tests/qemu-iotests/tests/image-fleecing
@@ -70,7 +70,11 @@ with iotests.FilePath('base.img') as base_img_path, \
     log('--- Launching VM ---')
     log('')
 
-    vm.add_drive(base_img_path)
+    src_node = 'source'
+    vm.add_blockdev(f'driver={iotests.imgfmt},file.driver=file,'
+                    f'file.filename={base_img_path},node-name={src_node}')
+    vm.add_device('virtio-scsi')
+    vm.add_device(f'scsi-hd,id=sda,drive={src_node}')
     vm.launch()
     log('Done')
 
@@ -78,7 +82,6 @@ with iotests.FilePath('base.img') as base_img_path, \
     log('--- Setting up Fleecing Graph ---')
     log('')
 
-    src_node = 'drive0'
     tgt_node = 'fleeceNode'
 
     # create tgt_node backed by src_node
@@ -94,6 +97,7 @@ with iotests.FilePath('base.img') as base_img_path, \
 
     # Establish COW from source to fleecing node
     log(vm.qmp('blockdev-backup',
+               job_id='fleecing',
                device=src_node,
                target=tgt_node,
                sync='none'))
@@ -125,7 +129,7 @@ with iotests.FilePath('base.img') as base_img_path, \
     for p in overwrite:
         cmd = 'write -P%s %s %s' % p
         log(cmd)
-        log(vm.hmp_qemu_io(src_node, cmd))
+        log(vm.hmp_qemu_io('/machine/peripheral/sda', cmd, qdev=True))
 
     log('')
     log('--- Verifying Data ---')
@@ -140,7 +144,7 @@ with iotests.FilePath('base.img') as base_img_path, \
     log('--- Cleanup ---')
     log('')
 
-    log(vm.qmp('block-job-cancel', device=src_node))
+    log(vm.qmp('block-job-cancel', device='fleecing'))
     e = vm.event_wait('BLOCK_JOB_CANCELLED')
     assert e is not None
     log(e, filters=[iotests.filter_qmp_event])
diff --git a/tests/qemu-iotests/tests/image-fleecing.out b/tests/qemu-iotests/tests/image-fleecing.out
index 16643dde30..314a1b54e9 100644
--- a/tests/qemu-iotests/tests/image-fleecing.out
+++ b/tests/qemu-iotests/tests/image-fleecing.out
@@ -50,7 +50,7 @@ read -P0 0x3fe0000 64k
 --- Cleanup ---
 
 {"return": {}}
-{"data": {"device": "drive0", "len": 67108864, "offset": 393216, "speed": 0, "type": "backup"}, "event": "BLOCK_JOB_CANCELLED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
+{"data": {"device": "fleecing", "len": 67108864, "offset": 393216, "speed": 0, "type": "backup"}, "event": "BLOCK_JOB_CANCELLED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
 {"return": {}}
 {"return": {}}
 
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 31/33] iotests/image-fleecing: rename tgt_node
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (29 preceding siblings ...)
  2021-05-20 14:22 ` [PATCH v2 30/33] iotests/image-fleecing: proper source device Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:22 ` Vladimir Sementsov-Ogievskiy
  2021-06-01 11:46   ` Max Reitz
  2021-05-20 14:22 ` [PATCH v2 32/33] iotests/image-fleecing: prepare for adding new test-case Vladimir Sementsov-Ogievskiy
                   ` (2 subsequent siblings)
  33 siblings, 1 reply; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:22 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

Actually target of backup(sync=None) is not a final backup target:
image fleecing is intended to be used with external tool, which will
copy data from fleecing node to some real backup target.

Also, we are going to add a test case for "push backup with fleecing",
where instead of exporting fleecing node by NBD, we'll start a backup
job from fleecing node to real backup target.

To avoid confusion, let's rename temporary fleecing node now.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/tests/image-fleecing | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/qemu-iotests/tests/image-fleecing b/tests/qemu-iotests/tests/image-fleecing
index 961941bb27..ec4ef5f3f6 100755
--- a/tests/qemu-iotests/tests/image-fleecing
+++ b/tests/qemu-iotests/tests/image-fleecing
@@ -71,6 +71,7 @@ with iotests.FilePath('base.img') as base_img_path, \
     log('')
 
     src_node = 'source'
+    tmp_node = 'temp'
     vm.add_blockdev(f'driver={iotests.imgfmt},file.driver=file,'
                     f'file.filename={base_img_path},node-name={src_node}')
     vm.add_device('virtio-scsi')
@@ -82,12 +83,11 @@ with iotests.FilePath('base.img') as base_img_path, \
     log('--- Setting up Fleecing Graph ---')
     log('')
 
-    tgt_node = 'fleeceNode'
 
-    # create tgt_node backed by src_node
+    # create tmp_node backed by src_node
     log(vm.qmp('blockdev-add', {
         'driver': 'qcow2',
-        'node-name': tgt_node,
+        'node-name': tmp_node,
         'file': {
             'driver': 'file',
             'filename': fleece_img_path,
@@ -99,19 +99,19 @@ with iotests.FilePath('base.img') as base_img_path, \
     log(vm.qmp('blockdev-backup',
                job_id='fleecing',
                device=src_node,
-               target=tgt_node,
+               target=tmp_node,
                sync='none'))
 
     log('')
     log('--- Setting up NBD Export ---')
     log('')
 
-    nbd_uri = 'nbd+unix:///%s?socket=%s' % (tgt_node, nbd_sock_path)
+    nbd_uri = 'nbd+unix:///%s?socket=%s' % (tmp_node, nbd_sock_path)
     log(vm.qmp('nbd-server-start',
                {'addr': { 'type': 'unix',
                           'data': { 'path': nbd_sock_path } } }))
 
-    log(vm.qmp('nbd-server-add', device=tgt_node))
+    log(vm.qmp('nbd-server-add', device=tmp_node))
 
     log('')
     log('--- Sanity Check ---')
@@ -149,7 +149,7 @@ with iotests.FilePath('base.img') as base_img_path, \
     assert e is not None
     log(e, filters=[iotests.filter_qmp_event])
     log(vm.qmp('nbd-server-stop'))
-    log(vm.qmp('blockdev-del', node_name=tgt_node))
+    log(vm.qmp('blockdev-del', node_name=tmp_node))
     vm.shutdown()
 
     log('')
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 32/33] iotests/image-fleecing: prepare for adding new test-case
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (30 preceding siblings ...)
  2021-05-20 14:22 ` [PATCH v2 31/33] iotests/image-fleecing: rename tgt_node Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:22 ` Vladimir Sementsov-Ogievskiy
  2021-06-01 11:47   ` Max Reitz
  2021-05-20 14:22 ` [PATCH v2 33/33] iotests/image-fleecing: add test-case for copy-before-write filter Vladimir Sementsov-Ogievskiy
  2021-05-31 17:11 ` [PATCH v2 00/33] block: publish backup-top filter Max Reitz
  33 siblings, 1 reply; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:22 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

We are going to add a test-case with some behavior modifications. So,
let's prepare a function to be reused.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/tests/image-fleecing | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/tests/image-fleecing b/tests/qemu-iotests/tests/image-fleecing
index ec4ef5f3f6..e210c00d28 100755
--- a/tests/qemu-iotests/tests/image-fleecing
+++ b/tests/qemu-iotests/tests/image-fleecing
@@ -48,12 +48,7 @@ remainder = [('0xd5', '0x108000',  '32k'), # Right-end of partial-left [1]
              ('0xdc', '32M',       '32k'), # Left-end of partial-right [2]
              ('0xcd', '0x3ff0000', '64k')] # patterns[3]
 
-with iotests.FilePath('base.img') as base_img_path, \
-     iotests.FilePath('fleece.img') as fleece_img_path, \
-     iotests.FilePath('nbd.sock',
-                      base_dir=iotests.sock_dir) as nbd_sock_path, \
-     iotests.VM() as vm:
-
+def do_test(base_img_path, fleece_img_path, nbd_sock_path, vm):
     log('--- Setting up images ---')
     log('')
 
@@ -163,3 +158,15 @@ with iotests.FilePath('base.img') as base_img_path, \
 
     log('')
     log('Done')
+
+
+def test():
+    with iotests.FilePath('base.img') as base_img_path, \
+         iotests.FilePath('fleece.img') as fleece_img_path, \
+         iotests.FilePath('nbd.sock',
+                          base_dir=iotests.sock_dir) as nbd_sock_path, \
+         iotests.VM() as vm:
+        do_test(base_img_path, fleece_img_path, nbd_sock_path, vm)
+
+
+test()
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v2 33/33] iotests/image-fleecing: add test-case for copy-before-write filter
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (31 preceding siblings ...)
  2021-05-20 14:22 ` [PATCH v2 32/33] iotests/image-fleecing: prepare for adding new test-case Vladimir Sementsov-Ogievskiy
@ 2021-05-20 14:22 ` Vladimir Sementsov-Ogievskiy
  2021-06-01 12:02   ` Max Reitz
  2021-05-31 17:11 ` [PATCH v2 00/33] block: publish backup-top filter Max Reitz
  33 siblings, 1 reply; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-20 14:22 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	vsementsov, jsnow, mreitz, kwolf, den

New fleecing method becomes available: copy-before-write filter.

Actually we don't need backup job to setup image fleecing. Add test
for new recommended way of image fleecing.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/tests/image-fleecing     | 50 +++++++++-----
 tests/qemu-iotests/tests/image-fleecing.out | 72 +++++++++++++++++++++
 2 files changed, 107 insertions(+), 15 deletions(-)

diff --git a/tests/qemu-iotests/tests/image-fleecing b/tests/qemu-iotests/tests/image-fleecing
index e210c00d28..404ebc00f1 100755
--- a/tests/qemu-iotests/tests/image-fleecing
+++ b/tests/qemu-iotests/tests/image-fleecing
@@ -48,7 +48,7 @@ remainder = [('0xd5', '0x108000',  '32k'), # Right-end of partial-left [1]
              ('0xdc', '32M',       '32k'), # Left-end of partial-right [2]
              ('0xcd', '0x3ff0000', '64k')] # patterns[3]
 
-def do_test(base_img_path, fleece_img_path, nbd_sock_path, vm):
+def do_test(use_cbw, base_img_path, fleece_img_path, nbd_sock_path, vm):
     log('--- Setting up images ---')
     log('')
 
@@ -67,6 +67,7 @@ def do_test(base_img_path, fleece_img_path, nbd_sock_path, vm):
 
     src_node = 'source'
     tmp_node = 'temp'
+    qom_path = '/machine/peripheral/sda'
     vm.add_blockdev(f'driver={iotests.imgfmt},file.driver=file,'
                     f'file.filename={base_img_path},node-name={src_node}')
     vm.add_device('virtio-scsi')
@@ -90,12 +91,22 @@ def do_test(base_img_path, fleece_img_path, nbd_sock_path, vm):
         'backing': src_node,
     }))
 
-    # Establish COW from source to fleecing node
-    log(vm.qmp('blockdev-backup',
-               job_id='fleecing',
-               device=src_node,
-               target=tmp_node,
-               sync='none'))
+    # 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'))
+    else:
+        log(vm.qmp('blockdev-backup',
+                   job_id='fleecing',
+                   device=src_node,
+                   target=tmp_node,
+                   sync='none'))
 
     log('')
     log('--- Setting up NBD Export ---')
@@ -124,7 +135,7 @@ def do_test(base_img_path, fleece_img_path, nbd_sock_path, vm):
     for p in overwrite:
         cmd = 'write -P%s %s %s' % p
         log(cmd)
-        log(vm.hmp_qemu_io('/machine/peripheral/sda', cmd, qdev=True))
+        log(vm.hmp_qemu_io(qom_path, cmd, qdev=True))
 
     log('')
     log('--- Verifying Data ---')
@@ -139,10 +150,15 @@ def do_test(base_img_path, fleece_img_path, nbd_sock_path, vm):
     log('--- Cleanup ---')
     log('')
 
-    log(vm.qmp('block-job-cancel', device='fleecing'))
-    e = vm.event_wait('BLOCK_JOB_CANCELLED')
-    assert e is not None
-    log(e, filters=[iotests.filter_qmp_event])
+    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'))
+    else:
+        log(vm.qmp('block-job-cancel', device='fleecing'))
+        e = vm.event_wait('BLOCK_JOB_CANCELLED')
+        assert e is not None
+        log(e, filters=[iotests.filter_qmp_event])
+
     log(vm.qmp('nbd-server-stop'))
     log(vm.qmp('blockdev-del', node_name=tmp_node))
     vm.shutdown()
@@ -160,13 +176,17 @@ def do_test(base_img_path, fleece_img_path, nbd_sock_path, vm):
     log('Done')
 
 
-def test():
+def test(use_cbw):
     with iotests.FilePath('base.img') as base_img_path, \
          iotests.FilePath('fleece.img') as fleece_img_path, \
          iotests.FilePath('nbd.sock',
                           base_dir=iotests.sock_dir) as nbd_sock_path, \
          iotests.VM() as vm:
-        do_test(base_img_path, fleece_img_path, nbd_sock_path, vm)
+        do_test(use_cbw, base_img_path, fleece_img_path, nbd_sock_path, vm)
+
 
+log('=== Test backup(sync=none) based fleecing ===\n')
+test(False)
 
-test()
+log('=== Test filter based fleecing ===\n')
+test(True)
diff --git a/tests/qemu-iotests/tests/image-fleecing.out b/tests/qemu-iotests/tests/image-fleecing.out
index 314a1b54e9..e96d122a8b 100644
--- a/tests/qemu-iotests/tests/image-fleecing.out
+++ b/tests/qemu-iotests/tests/image-fleecing.out
@@ -1,3 +1,5 @@
+=== Test backup(sync=none) based fleecing ===
+
 --- Setting up images ---
 
 Done
@@ -65,3 +67,73 @@ read -P0xdc 32M 32k
 read -P0xcd 0x3ff0000 64k
 
 Done
+=== Test filter based fleecing ===
+
+--- Setting up images ---
+
+Done
+
+--- Launching VM ---
+
+Done
+
+--- Setting up Fleecing Graph ---
+
+{"return": {}}
+{"return": {}}
+{"return": {}}
+
+--- Setting up NBD Export ---
+
+{"return": {}}
+{"return": {}}
+
+--- Sanity Check ---
+
+read -P0x5d 0 64k
+read -P0xd5 1M 64k
+read -P0xdc 32M 64k
+read -P0xcd 0x3ff0000 64k
+read -P0 0x00f8000 32k
+read -P0 0x2010000 32k
+read -P0 0x3fe0000 64k
+
+--- Testing COW ---
+
+write -P0xab 0 64k
+{"return": ""}
+write -P0xad 0x00f8000 64k
+{"return": ""}
+write -P0x1d 0x2008000 64k
+{"return": ""}
+write -P0xea 0x3fe0000 64k
+{"return": ""}
+
+--- Verifying Data ---
+
+read -P0x5d 0 64k
+read -P0xd5 1M 64k
+read -P0xdc 32M 64k
+read -P0xcd 0x3ff0000 64k
+read -P0 0x00f8000 32k
+read -P0 0x2010000 32k
+read -P0 0x3fe0000 64k
+
+--- Cleanup ---
+
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+
+--- Confirming writes ---
+
+read -P0xab 0 64k
+read -P0xad 0x00f8000 64k
+read -P0x1d 0x2008000 64k
+read -P0xea 0x3fe0000 64k
+read -P0xd5 0x108000 32k
+read -P0xdc 32M 32k
+read -P0xcd 0x3ff0000 64k
+
+Done
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 08/33] block/backup: drop support for copy_range
  2021-05-20 14:21 ` [PATCH v2 08/33] block/backup: drop support for copy_range Vladimir Sementsov-Ogievskiy
@ 2021-05-28 15:29   ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-28 15:29 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	jsnow, mreitz, kwolf, den

20.05.2021 17:21, Vladimir Sementsov-Ogievskiy wrote:
> copy_range is not a default behavior since 6a30f663d4c0b3c, and it's
> now available only though x-perf experimantal argument, so it's OK to
> drop it.
> 
> Even when backup is used to copy disk to same filesystem, and
> filesystem supports zero-copy copy_range, copy_range is probably not
> what we want for backup: backup has good property of making a copy of
> active disk, with no impact to active disk itself (unlike creating a
> snapshot). And if copy_range instead of copying data adds fs-level
> references, and on next guest write COW operation occurs, it's seems
> most possible, that new block will be allocated for original vm disk,
> not for backup disk. Thus, fragmentation of original disk will
> increase.
> 
> We can simply add support back on demand. Now we want to publish
> copy-before-write filter, and instead of thinking how to pass
> use-copy-range argument to block-copy (create x-block-copy parameter
> for new public filter driver, or may be set it by hand after filter
> node creation?), instead of this let's just drop copy-range support in
> backup for now.
> 
> After this patch copy-range support in block-copy becomes unused. Let's
> keep it for a while, it won't hurt:
> 
> 1. If there would be request for supporting copy_range in backup
>     (and/or in a new public copy-before-write filter), it will be easy
>     to satisfy it.
> 
> 2. Probably, qemu-img convert will reuse block-copy, and qemu-img has
>     option to enable copy-range. qemu-img convert is not a backup, and
>     copy_range may be more reasonable for some cases in context of
>     qemu-img convert.
> 

Actually, I know one case, where copy_range for backup job may be reasonable:

Using backup in push-backup with fleecing scheme in

    [PATCH 0/6] push backup with fleecing

Of-course, no real sense in using push-backup-with-fleecing scheme with both temp image and final backup target being on the same file system (no benefit of fleecing, we can use simple backup without temporary image).

But we absolutely don't care about fragmentation of temp disk.

Still, it doesn't make sense, as temp-image and real-backup-target should not be on same file-system..

Could it be some distributed filesystem, where it still make sense to call copy_range? Theoretically could.


Another thought: I'm going also to implement RAM-cache driver, to optimize push-backup-with-fleecing scheme. I'll need a way to copy data from RAM-cache node to final-target. I can implement copy_range for RAM-cache, and this will allow to not create extra buffer, but use the buffer that is already allocated and own by RAM-cache.. Still, this behavior is obviously good, it should work automatically, no reason to make it optional..


Hmm, so what should be summarized:

- Actually, block-copy does copy_range. So, probably it's good to change the copy_range() function in qemu to fallback to read+write..

And about copy_range itself, what we want:

1. We want to control does it influence fragmentation of source disk. When copying from temporary image we don't care. But when source of block-copy is active disk in we do care to not influence how original disk lay in filesystem. Probably, we even want an option for copy_range() syscall to control this thing.

2. We want to be efficient with copy_size, ie size of chunks to copy. We even have existing issue in block-copy: write-zero is limited to BLOCK_COPY_MAX_BUFFER which is obviously inefficient.

For copy-size we should have some good defaults or automatic detection logic..

For copy_range fragmentation..

If we have some internal copy_range-like optimizations like zero-copy from RAM-cache node, or maybe copy compressed data from one qcow2 node to another without decompression, it should be done anyway, it shouldn't be set by user option. Still, for file-posix, we don't know, does underlying filesystem copy_range() implementation lead to fragmentation or not. And we don't know is user OK with it or not. So we need an option.. So, it's probably better to keep x-perm.copy-range for now, until we don't have a good idea on interface.

-- 
Best regards,
Vladimir


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 01/33] block: rename bdrv_replace_child to bdrv_replace_child_tran
  2021-05-20 14:21 ` [PATCH v2 01/33] block: rename bdrv_replace_child to bdrv_replace_child_tran Vladimir Sementsov-Ogievskiy
@ 2021-05-31 12:52   ` Max Reitz
  0 siblings, 0 replies; 62+ messages in thread
From: Max Reitz @ 2021-05-31 12:52 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: kwolf, berrange, ehabkost, den, jsnow, qemu-devel, armbru, crosa,
	pbonzini

On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote:
> We have bdrv_replace_child() wrapper on bdrv_replace_child_noperm().
> But bdrv_replace_child() doesn't update permissions. It's rather
> strange, as normally it's expected that foo() should call foo_noperm()
> and update permissions.
>
> Let's rename and add comment.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block.c | 14 ++++++++------
>   1 file changed, 8 insertions(+), 6 deletions(-)


Reviewed-by: Max Reitz <mreitz@redhat.com>



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 02/33] block: comment graph-modifying function not updating permissions
  2021-05-20 14:21 ` [PATCH v2 02/33] block: comment graph-modifying function not updating permissions Vladimir Sementsov-Ogievskiy
@ 2021-05-31 12:56   ` Max Reitz
  0 siblings, 0 replies; 62+ messages in thread
From: Max Reitz @ 2021-05-31 12:56 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: kwolf, berrange, ehabkost, den, jsnow, qemu-devel, armbru, crosa,
	pbonzini

On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block.c | 7 +++++++
>   1 file changed, 7 insertions(+)


Reviewed-by: Max Reitz <mreitz@redhat.com>



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 03/33] block: introduce bdrv_replace_child_bs()
  2021-05-20 14:21 ` [PATCH v2 03/33] block: introduce bdrv_replace_child_bs() Vladimir Sementsov-Ogievskiy
@ 2021-05-31 14:18   ` Max Reitz
  0 siblings, 0 replies; 62+ messages in thread
From: Max Reitz @ 2021-05-31 14:18 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: kwolf, berrange, ehabkost, den, jsnow, qemu-devel, armbru, crosa,
	pbonzini

On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote:
> Add function to transactionally replace bs inside BdrvChild.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   include/block/block.h |  2 ++
>   block.c               | 31 +++++++++++++++++++++++++++++++
>   2 files changed, 33 insertions(+)

Reviewed-by: Max Reitz <mreitz@redhat.com>



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 05/33] qdev-properties: PropertyInfo: add realized_set_allowed field
  2021-05-20 14:21 ` [PATCH v2 05/33] qdev-properties: PropertyInfo: add realized_set_allowed field Vladimir Sementsov-Ogievskiy
@ 2021-05-31 15:51   ` Max Reitz
  0 siblings, 0 replies; 62+ messages in thread
From: Max Reitz @ 2021-05-31 15:51 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: kwolf, berrange, ehabkost, den, jsnow, qemu-devel, armbru, crosa,
	pbonzini

On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote:
> Add field, so property can declare support for setting the property
> when device is realized. To be used in the following commit.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   include/hw/qdev-properties.h | 1 +
>   hw/core/qdev-properties.c    | 6 +++---
>   2 files changed, 4 insertions(+), 3 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 06/33] qdev: allow setting drive property for realized device
  2021-05-20 14:21 ` [PATCH v2 06/33] qdev: allow setting drive property for realized device Vladimir Sementsov-Ogievskiy
@ 2021-05-31 15:59   ` Max Reitz
  0 siblings, 0 replies; 62+ messages in thread
From: Max Reitz @ 2021-05-31 15:59 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: kwolf, berrange, ehabkost, den, jsnow, qemu-devel, armbru, crosa,
	pbonzini

On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote:
> We need an ability to insert filters above top block node, attached to
> block device. It can't be achieved with blockdev-reopen command. So, we
> want do it with help of qom-set.
>
> Intended usage:
>
> Assume there is a node A that is attached to some guest device.
>
> 1. blockdev-add to create a filter node B that has A as its child.
>
> 2. qom-set to change the node attached to the guest device’s
>     BlockBackend from A to B.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   hw/core/qdev-properties-system.c | 43 +++++++++++++++++++++++---------
>   1 file changed, 31 insertions(+), 12 deletions(-)

Tentative

Reviewed-by: Max Reitz <mreitz@redhat.com>



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 07/33] block: rename backup-top to copy-before-write
  2021-05-20 14:21 ` [PATCH v2 07/33] block: rename backup-top to copy-before-write Vladimir Sementsov-Ogievskiy
@ 2021-05-31 16:08   ` Max Reitz
  0 siblings, 0 replies; 62+ messages in thread
From: Max Reitz @ 2021-05-31 16:08 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: kwolf, berrange, ehabkost, den, jsnow, qemu-devel, armbru, crosa,
	pbonzini

On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote:
> We are going to convert backup_top to full featured public filter,
> which can be used in separate of backup job. Start from renaming from
> "how it used" to "what it does".
>
> While updating comments in 283 iotest, drop and rephrase also things
> about ".active", as this field is now dropped, and filter doesn't have
> "inactive" mode.
>
> Note that this change may be considered as incompatible interface
> change, as backup-top filter format name was visible through
> query-block and query-named-block-nodes.
>
> Still, consider the following reasoning:
>
> 1. backup-top was never documented, so if someone depends on format
>     name (for driver that can't be used other than it is automatically
>     inserted on backup job start), it's a kind of "undocumented feature
>     use". So I think we are free to change it.
>
> 2. There is a hope, that there is no such users: it's a lot more native
>     to give a good node-name to backup-top filter if need to operate
>     with it somehow, and don't touch format name.
>
> 3. Another "incompatible" change in further commit would be moving
>     copy-before-write filter from using backing child to file child. And
>     this is even more reasonable than renaming: for now all public
>     filters are file-child based.
>
> So, it's a risky change, but risk seems small and good interface worth
> it.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block/{backup-top.h => copy-before-write.h} |  28 +++---
>   block/backup.c                              |  22 ++---
>   block/{backup-top.c => copy-before-write.c} | 100 ++++++++++----------
>   MAINTAINERS                                 |   4 +-
>   block/meson.build                           |   2 +-
>   tests/qemu-iotests/283                      |  35 +++----
>   tests/qemu-iotests/283.out                  |   4 +-
>   7 files changed, 95 insertions(+), 100 deletions(-)
>   rename block/{backup-top.h => copy-before-write.h} (56%)
>   rename block/{backup-top.c => copy-before-write.c} (62%)

Reviewed-by: Max Reitz <mreitz@redhat.com>



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 12/33] block/copy-before-write: drop extra bdrv_unref on failure path
  2021-05-20 14:21 ` [PATCH v2 12/33] block/copy-before-write: drop extra bdrv_unref on failure path Vladimir Sementsov-Ogievskiy
@ 2021-05-31 16:19   ` Max Reitz
  0 siblings, 0 replies; 62+ messages in thread
From: Max Reitz @ 2021-05-31 16:19 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: kwolf, berrange, ehabkost, den, jsnow, qemu-devel, armbru, crosa,
	pbonzini

On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote:
> bdrv_attach_child() do bdrv_unref() on failure, so we shouldn't do it
> by hand here.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block/copy-before-write.c | 1 -
>   1 file changed, 1 deletion(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 13/33] block/copy-before-write: use file child instead of backing
  2021-05-20 14:21 ` [PATCH v2 13/33] block/copy-before-write: use file child instead of backing Vladimir Sementsov-Ogievskiy
@ 2021-05-31 16:22   ` Max Reitz
  0 siblings, 0 replies; 62+ messages in thread
From: Max Reitz @ 2021-05-31 16:22 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: kwolf, berrange, ehabkost, den, jsnow, qemu-devel, armbru, crosa,
	pbonzini

On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote:
> We are going to publish copy-before-write filter, and there no public
> backing-child-based filter in Qemu. No reason to create a precedent, so
> let's refactor copy-before-write filter instead.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block/copy-before-write.c | 39 ++++++++++++++++++++++-----------------
>   1 file changed, 22 insertions(+), 17 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 14/33] block/copy-before-write: bdrv_cbw_append(): replace child at last
  2021-05-20 14:21 ` [PATCH v2 14/33] block/copy-before-write: bdrv_cbw_append(): replace child at last Vladimir Sementsov-Ogievskiy
@ 2021-05-31 16:55   ` Max Reitz
  0 siblings, 0 replies; 62+ messages in thread
From: Max Reitz @ 2021-05-31 16:55 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: kwolf, berrange, ehabkost, den, jsnow, qemu-devel, armbru, crosa,
	pbonzini

On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote:
> Refactor the function to replace child at last. Thus we don't need to
> revert it and code is simplified.
>
> block-copy state initilization being done before replacing the child

still *initialization

Max

> doesn't need any drained section.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
>   block/copy-before-write.c | 33 +++++++++++----------------------
>   1 file changed, 11 insertions(+), 22 deletions(-)



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 15/33] block/copy-before-write: introduce cbw_init()
  2021-05-20 14:21 ` [PATCH v2 15/33] block/copy-before-write: introduce cbw_init() Vladimir Sementsov-Ogievskiy
@ 2021-05-31 16:57   ` Max Reitz
  0 siblings, 0 replies; 62+ messages in thread
From: Max Reitz @ 2021-05-31 16:57 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: kwolf, berrange, ehabkost, den, jsnow, qemu-devel, armbru, crosa,
	pbonzini

On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote:
> Move part of bdrv_cbw_append() to new function cbw_open(). It's an
> intermediate step for adding noramal .bdrv_open() handler to the

Didn’t notice this in v1, but: *normal

Max

> filter. With this commit no logic is changed, but we have a function
> which will be turned into .bdrv_open() handler in future commit.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
>   block/copy-before-write.c | 69 +++++++++++++++++++++++----------------
>   1 file changed, 41 insertions(+), 28 deletions(-)
>   



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 18/33] block/copy-before-write: cbw_init(): use options
  2021-05-20 14:21 ` [PATCH v2 18/33] block/copy-before-write: cbw_init(): use options Vladimir Sementsov-Ogievskiy
@ 2021-05-31 17:03   ` Max Reitz
  0 siblings, 0 replies; 62+ messages in thread
From: Max Reitz @ 2021-05-31 17:03 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: kwolf, berrange, ehabkost, den, jsnow, qemu-devel, armbru, crosa,
	pbonzini

On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote:
> One more step closer to .bdrv_open(): use options instead of plain
> arguments. Move to bdrv_open_child() calls, native for drive open
> handlers.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block/copy-before-write.c | 40 +++++++++++++++++++++++++--------------
>   1 file changed, 26 insertions(+), 14 deletions(-)
>

Reviewed-by: Max Reitz <mreitz@redhat.com>



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 19/33] block/copy-before-write: initialize block-copy bitmap
  2021-05-20 14:21 ` [PATCH v2 19/33] block/copy-before-write: initialize block-copy bitmap Vladimir Sementsov-Ogievskiy
@ 2021-05-31 17:10   ` Max Reitz
  0 siblings, 0 replies; 62+ messages in thread
From: Max Reitz @ 2021-05-31 17:10 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: kwolf, berrange, ehabkost, den, jsnow, qemu-devel, armbru, crosa,
	pbonzini

On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote:
> We are going to publish copy-before-write filter to be used in separate
> of backup. Future step would support bitmap for the filter. But let's
> start from full set bitmap.
>
> We have to modify backup, as bitmap is first initialized by
> copy-before-write filter, and then backup modifies it.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block/backup.c            | 16 +++++++---------
>   block/copy-before-write.c |  4 ++++
>   2 files changed, 11 insertions(+), 9 deletions(-)
>

Reviewed-by: Max Reitz <mreitz@redhat.com>



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 00/33] block: publish backup-top filter
  2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
                   ` (32 preceding siblings ...)
  2021-05-20 14:22 ` [PATCH v2 33/33] iotests/image-fleecing: add test-case for copy-before-write filter Vladimir Sementsov-Ogievskiy
@ 2021-05-31 17:11 ` Max Reitz
  2021-05-31 17:35   ` Vladimir Sementsov-Ogievskiy
  33 siblings, 1 reply; 62+ messages in thread
From: Max Reitz @ 2021-05-31 17:11 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: kwolf, berrange, ehabkost, den, jsnow, qemu-devel, armbru, crosa,
	pbonzini

On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
>
> v2:
> 01-02: new
> 03: don't bother with supporting empty child: we should never have such
>      at this point
> 05: add comment
> 06: keep checking conflict with global
>      add realized_set_allowed to qdev_prop_drive_iothread
> 07: improve cbw_cbw() name
>      improve commit message
> 10: rebased on unchanged backup_calculate_cluster_size(). keep r-b  CHECK ME

I checked you! It’s ok. :)

(and btw, I’ll look at the rest tomorrow)

> 12: new
> 13: drop extra bdrv_unref()
> 18: add compress local variable
>      add comment about x-deprecated-compress
> 19: new, replacement for "[PATCH 17/21] block/block-copy: switch to fully set bitmap by default"
> 22: improve qapi documentation
> 23-33: test: a lot of refactoring



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 00/33] block: publish backup-top filter
  2021-05-31 17:11 ` [PATCH v2 00/33] block: publish backup-top filter Max Reitz
@ 2021-05-31 17:35   ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 62+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-31 17:35 UTC (permalink / raw)
  To: Max Reitz, qemu-block
  Cc: qemu-devel, eblake, armbru, crosa, ehabkost, berrange, pbonzini,
	jsnow, kwolf, den

31.05.2021 20:11, Max Reitz wrote:
> On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote:
>> Hi all!
>>
>> v2:
>> 01-02: new
>> 03: don't bother with supporting empty child: we should never have such
>>      at this point
>> 05: add comment
>> 06: keep checking conflict with global
>>      add realized_set_allowed to qdev_prop_drive_iothread
>> 07: improve cbw_cbw() name
>>      improve commit message
>> 10: rebased on unchanged backup_calculate_cluster_size(). keep r-b  CHECK ME
> 
> I checked you! It’s ok. :)
> 
> (and btw, I’ll look at the rest tomorrow)

Thanks)

Actually, "CHECK ME" was for myself, and "ME" is a patch, not me) I forget to drop it.. Interesting, did I forget to check

> 
>> 12: new
>> 13: drop extra bdrv_unref()
>> 18: add compress local variable
>>      add comment about x-deprecated-compress
>> 19: new, replacement for "[PATCH 17/21] block/block-copy: switch to fully set bitmap by default"
>> 22: improve qapi documentation
>> 23-33: test: a lot of refactoring
> 


-- 
Best regards,
Vladimir


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 22/33] qapi: publish copy-before-write filter
  2021-05-20 14:21 ` [PATCH v2 22/33] qapi: publish copy-before-write filter Vladimir Sementsov-Ogievskiy
@ 2021-06-01  9:08   ` Max Reitz
  0 siblings, 0 replies; 62+ messages in thread
From: Max Reitz @ 2021-06-01  9:08 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: kwolf, berrange, ehabkost, den, jsnow, qemu-devel, armbru, crosa,
	pbonzini

On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   qapi/block-core.json | 30 ++++++++++++++++++++++++++----
>   1 file changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 2ea294129e..8c4801a13d 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2808,15 +2808,17 @@
>   # @blklogwrites: Since 3.0
>   # @blkreplay: Since 4.2
>   # @compress: Since 5.0
> +# @copy-before-write: Since 6.1
>   #
>   # Since: 2.9
>   ##
>   { 'enum': 'BlockdevDriver',
>     'data': [ 'blkdebug', 'blklogwrites', 'blkreplay', 'blkverify', 'bochs',
> -            'cloop', 'compress', 'copy-on-read', 'dmg', 'file', 'ftp', 'ftps',
> -            'gluster', 'host_cdrom', 'host_device', 'http', 'https', 'iscsi',
> -            'luks', 'nbd', 'nfs', 'null-aio', 'null-co', 'nvme', 'parallels',
> -            'preallocate', 'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'rbd',
> +            'cloop', 'compress', 'copy-before-write', 'copy-on-read', 'dmg',
> +            'file', 'ftp', 'ftps', 'gluster', 'host_cdrom', 'host_device',
> +            'http', 'https', 'iscsi', 'luks', 'nbd', 'nfs', 'null-aio',
> +            'null-co', 'nvme', 'parallels', 'preallocate', 'qcow', 'qcow2',
> +            'qed', 'quorum', 'raw', 'rbd',
>               { 'name': 'replication', 'if': 'defined(CONFIG_REPLICATION)' },
>               'ssh', 'throttle', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] }
>   
> @@ -3937,6 +3939,25 @@
>     'base': 'BlockdevOptionsGenericFormat',
>     'data': { '*bottom': 'str' } }
>   
> +##
> +# @BlockdevOptionsCbw:
> +#
> +# Driver specific block device options for the copy-before-write driver,
> +# which does so called copy-before-write operations: when data is
> +# written to the filter, the filter firstly reads corresponding blocks
> +# from its file child and copies them to @target child. After successful
> +# copying the write request is propagated to file child. If copying
> +# filed, the original write request is failed too and no data is written

s/filed/failed/

With that fixed:

Reviewed-by: Max Reitz <mreitz@redhat.com>

> +# to file child.
> +#
> +# @target: The target for copy-before-write operations.
> +#
> +# Since: 6.1
> +##
> +{ 'struct': 'BlockdevOptionsCbw',
> +  'base': 'BlockdevOptionsGenericFormat',
> +  'data': { 'target': 'BlockdevRef' } }
> +
>   ##
>   # @BlockdevOptions:
>   #
> @@ -3989,6 +4010,7 @@
>         'bochs':      'BlockdevOptionsGenericFormat',
>         'cloop':      'BlockdevOptionsGenericFormat',
>         'compress':   'BlockdevOptionsGenericFormat',
> +      'copy-before-write':'BlockdevOptionsCbw',
>         'copy-on-read':'BlockdevOptionsCor',
>         'dmg':        'BlockdevOptionsGenericFormat',
>         'file':       'BlockdevOptionsFile',



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 23/33] python/qemu/machine.py: refactor _qemu_args()
  2021-05-20 14:21 ` [PATCH v2 23/33] python/qemu/machine.py: refactor _qemu_args() Vladimir Sementsov-Ogievskiy
@ 2021-06-01 10:05   ` Max Reitz
  0 siblings, 0 replies; 62+ messages in thread
From: Max Reitz @ 2021-06-01 10:05 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: kwolf, berrange, ehabkost, den, jsnow, qemu-devel, armbru, crosa,
	pbonzini

On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote:
>   - use shorter construction
>   - don't create new dict if not needed
>   - drop extra unpacking key-val arguments
>   - drop extra default values
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   python/qemu/machine.py | 18 ++++++++----------
>   1 file changed, 8 insertions(+), 10 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 24/33] python/qemu/machine: QEMUMachine: improve qmp() method
  2021-05-20 14:21 ` [PATCH v2 24/33] python/qemu/machine: QEMUMachine: improve qmp() method Vladimir Sementsov-Ogievskiy
@ 2021-06-01 10:19   ` Max Reitz
  0 siblings, 0 replies; 62+ messages in thread
From: Max Reitz @ 2021-06-01 10:19 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: kwolf, berrange, ehabkost, den, jsnow, qemu-devel, armbru, crosa,
	pbonzini

On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote:
> We often call qmp() with unpacking dict, like qmp('foo', **{...}).
> mypy don't really like it, it things that passed unpacked dict is a

s/things/thinks/

> positional argument and complains that it type should be bool (because
> second argument of qmp() is conv_keys: bool).

I guess it would have been nice to see some examples of this that can 
now be written in a cleaner way, but well.

> Allow possing dict directly, simplifying interface, and giving a way to

s/possing/passing/

> satisfy mypy.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   python/qemu/machine.py | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 25/33] iotests.py: VM: add own __enter__ method
  2021-05-20 14:21 ` [PATCH v2 25/33] iotests.py: VM: add own __enter__ method Vladimir Sementsov-Ogievskiy
@ 2021-06-01 10:58   ` Max Reitz
  0 siblings, 0 replies; 62+ messages in thread
From: Max Reitz @ 2021-06-01 10:58 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: kwolf, berrange, ehabkost, den, jsnow, qemu-devel, armbru, crosa,
	pbonzini

On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote:
> In superclass __enter__ method is defined with return value type hint
> 'QEMUMachine'. So, mypy thinks that return value of VM.__enter__ is
> QEMUMachine. Let's redefine __enter__ in VM class, to give it correct
> type hint.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   tests/qemu-iotests/iotests.py | 4 ++++
>   1 file changed, 4 insertions(+)

Reviewed-by: Max Reitz <mreitz@redhat.com>



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 26/33] iotests/222: fix pylint and mypy complains
  2021-05-20 14:21 ` [PATCH v2 26/33] iotests/222: fix pylint and mypy complains Vladimir Sementsov-Ogievskiy
@ 2021-06-01 11:16   ` Max Reitz
  0 siblings, 0 replies; 62+ messages in thread
From: Max Reitz @ 2021-06-01 11:16 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: kwolf, berrange, ehabkost, den, jsnow, qemu-devel, armbru, crosa,
	pbonzini

On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote:
> Here:
>   - long line
>   - move to new interface of vm.qmp() (direct passing dict), to avoid
>     mypy false-positive, as it thinks that unpacked dict is a positional
>     argument.
>   - extra parenthesis
>   - handle event_wait possible None value
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   tests/qemu-iotests/222 | 20 +++++++++++---------
>   tests/qemu-iotests/297 |  2 +-
>   2 files changed, 12 insertions(+), 10 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 27/33] iotests/222: constantly use single quotes for strings
  2021-05-20 14:21 ` [PATCH v2 27/33] iotests/222: constantly use single quotes for strings Vladimir Sementsov-Ogievskiy
@ 2021-06-01 11:17   ` Max Reitz
  0 siblings, 0 replies; 62+ messages in thread
From: Max Reitz @ 2021-06-01 11:17 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: kwolf, berrange, ehabkost, den, jsnow, qemu-devel, armbru, crosa,
	pbonzini

On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote:
> The file use both single and double quotes for strings. Let's be
> consistent.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   tests/qemu-iotests/222 | 68 +++++++++++++++++++++---------------------
>   1 file changed, 34 insertions(+), 34 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 28/33] iotests: move 222 to tests/image-fleecing
  2021-05-20 14:22 ` [PATCH v2 28/33] iotests: move 222 to tests/image-fleecing Vladimir Sementsov-Ogievskiy
@ 2021-06-01 11:17   ` Max Reitz
  0 siblings, 0 replies; 62+ messages in thread
From: Max Reitz @ 2021-06-01 11:17 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: kwolf, berrange, ehabkost, den, jsnow, qemu-devel, armbru, crosa,
	pbonzini

On 20.05.21 16:22, Vladimir Sementsov-Ogievskiy wrote:
> Give a good name to test file.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   tests/qemu-iotests/{222 => tests/image-fleecing}         | 0
>   tests/qemu-iotests/{222.out => tests/image-fleecing.out} | 0
>   2 files changed, 0 insertions(+), 0 deletions(-)
>   rename tests/qemu-iotests/{222 => tests/image-fleecing} (100%)
>   rename tests/qemu-iotests/{222.out => tests/image-fleecing.out} (100%)

Reviewed-by: Max Reitz <mreitz@redhat.com>



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 29/33] iotests.py: hmp_qemu_io: support qdev
  2021-05-20 14:22 ` [PATCH v2 29/33] iotests.py: hmp_qemu_io: support qdev Vladimir Sementsov-Ogievskiy
@ 2021-06-01 11:19   ` Max Reitz
  0 siblings, 0 replies; 62+ messages in thread
From: Max Reitz @ 2021-06-01 11:19 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: kwolf, berrange, ehabkost, den, jsnow, qemu-devel, armbru, crosa,
	pbonzini

On 20.05.21 16:22, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   tests/qemu-iotests/iotests.py | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 30/33] iotests/image-fleecing: proper source device
  2021-05-20 14:22 ` [PATCH v2 30/33] iotests/image-fleecing: proper source device Vladimir Sementsov-Ogievskiy
@ 2021-06-01 11:29   ` Max Reitz
  0 siblings, 0 replies; 62+ messages in thread
From: Max Reitz @ 2021-06-01 11:29 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: kwolf, berrange, ehabkost, den, jsnow, qemu-devel, armbru, crosa,
	pbonzini

On 20.05.21 16:22, Vladimir Sementsov-Ogievskiy wrote:
> Define scsi device to operate with it by qom-set in further patch.
>
> Give a new node-name to source block node, to not look like device
> name.
>
> Job now don't want to work without giving explicit id, so, let's call
> it "fleecing".
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   tests/qemu-iotests/tests/image-fleecing     | 12 ++++++++----
>   tests/qemu-iotests/tests/image-fleecing.out |  2 +-
>   2 files changed, 9 insertions(+), 5 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 31/33] iotests/image-fleecing: rename tgt_node
  2021-05-20 14:22 ` [PATCH v2 31/33] iotests/image-fleecing: rename tgt_node Vladimir Sementsov-Ogievskiy
@ 2021-06-01 11:46   ` Max Reitz
  0 siblings, 0 replies; 62+ messages in thread
From: Max Reitz @ 2021-06-01 11:46 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: kwolf, berrange, ehabkost, den, jsnow, qemu-devel, armbru, crosa,
	pbonzini

On 20.05.21 16:22, Vladimir Sementsov-Ogievskiy wrote:
> Actually target of backup(sync=None) is not a final backup target:
> image fleecing is intended to be used with external tool, which will
> copy data from fleecing node to some real backup target.
>
> Also, we are going to add a test case for "push backup with fleecing",
> where instead of exporting fleecing node by NBD, we'll start a backup
> job from fleecing node to real backup target.
>
> To avoid confusion, let's rename temporary fleecing node now.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   tests/qemu-iotests/tests/image-fleecing | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 32/33] iotests/image-fleecing: prepare for adding new test-case
  2021-05-20 14:22 ` [PATCH v2 32/33] iotests/image-fleecing: prepare for adding new test-case Vladimir Sementsov-Ogievskiy
@ 2021-06-01 11:47   ` Max Reitz
  0 siblings, 0 replies; 62+ messages in thread
From: Max Reitz @ 2021-06-01 11:47 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: kwolf, berrange, ehabkost, den, jsnow, qemu-devel, armbru, crosa,
	pbonzini

On 20.05.21 16:22, Vladimir Sementsov-Ogievskiy wrote:
> We are going to add a test-case with some behavior modifications. So,
> let's prepare a function to be reused.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   tests/qemu-iotests/tests/image-fleecing | 19 +++++++++++++------
>   1 file changed, 13 insertions(+), 6 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 33/33] iotests/image-fleecing: add test-case for copy-before-write filter
  2021-05-20 14:22 ` [PATCH v2 33/33] iotests/image-fleecing: add test-case for copy-before-write filter Vladimir Sementsov-Ogievskiy
@ 2021-06-01 12:02   ` Max Reitz
  0 siblings, 0 replies; 62+ messages in thread
From: Max Reitz @ 2021-06-01 12:02 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: kwolf, berrange, ehabkost, den, jsnow, qemu-devel, armbru, crosa,
	pbonzini

On 20.05.21 16:22, Vladimir Sementsov-Ogievskiy wrote:
> New fleecing method becomes available: copy-before-write filter.
>
> Actually we don't need backup job to setup image fleecing. Add test
> for new recommended way of image fleecing.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   tests/qemu-iotests/tests/image-fleecing     | 50 +++++++++-----
>   tests/qemu-iotests/tests/image-fleecing.out | 72 +++++++++++++++++++++
>   2 files changed, 107 insertions(+), 15 deletions(-)
>
> diff --git a/tests/qemu-iotests/tests/image-fleecing b/tests/qemu-iotests/tests/image-fleecing
> index e210c00d28..404ebc00f1 100755
> --- a/tests/qemu-iotests/tests/image-fleecing
> +++ b/tests/qemu-iotests/tests/image-fleecing

[...]

> @@ -90,12 +91,22 @@ def do_test(base_img_path, fleece_img_path, nbd_sock_path, vm):
>           'backing': src_node,
>       }))
>   
> -    # Establish COW from source to fleecing node
> -    log(vm.qmp('blockdev-backup',
> -               job_id='fleecing',
> -               device=src_node,
> -               target=tmp_node,
> -               sync='none'))
> +    # Establish CBW from source to fleecing node
> +    if use_cbw:
> +        log(vm.qmp('blockdev-add', **{

I thought this should work without ** now.

With them dropped:

Reviewed-by: Max Reitz <mreitz@redhat.com>

> +            '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'))
> +    else:
> +        log(vm.qmp('blockdev-backup',
> +                   job_id='fleecing',
> +                   device=src_node,
> +                   target=tmp_node,
> +                   sync='none'))
>   
>       log('')
>       log('--- Setting up NBD Export ---')



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 20/33] block/block-copy: make setting progress optional
  2021-05-20 14:21 ` [PATCH v2 20/33] block/block-copy: make setting progress optional Vladimir Sementsov-Ogievskiy
@ 2021-08-10 15:22   ` Hanna Reitz
  0 siblings, 0 replies; 62+ messages in thread
From: Hanna Reitz @ 2021-08-10 15:22 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: kwolf, berrange, ehabkost, den, jsnow, qemu-devel, armbru, crosa,
	pbonzini, mreitz, eblake

On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote:
> Now block-copy will crash if user don't set progress meter by
> block_copy_set_progress_meter(). copy-before-write filter will be used
> in separate of backup job, and it doesn't want any progress meter (for
> now). So, allow not setting it.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
>   block/block-copy.c | 18 +++++++++++-------
>   1 file changed, 11 insertions(+), 7 deletions(-)

OK, looks a bit different because of the rebase on 
e3dd339feec2da3bcd82021e4ce4fe09dbf9c8b4, but R-b stands.

Hanna



^ permalink raw reply	[flat|nested] 62+ messages in thread

end of thread, other threads:[~2021-08-10 15:24 UTC | newest]

Thread overview: 62+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-20 14:21 [PATCH v2 00/33] block: publish backup-top filter Vladimir Sementsov-Ogievskiy
2021-05-20 14:21 ` [PATCH v2 01/33] block: rename bdrv_replace_child to bdrv_replace_child_tran Vladimir Sementsov-Ogievskiy
2021-05-31 12:52   ` Max Reitz
2021-05-20 14:21 ` [PATCH v2 02/33] block: comment graph-modifying function not updating permissions Vladimir Sementsov-Ogievskiy
2021-05-31 12:56   ` Max Reitz
2021-05-20 14:21 ` [PATCH v2 03/33] block: introduce bdrv_replace_child_bs() Vladimir Sementsov-Ogievskiy
2021-05-31 14:18   ` Max Reitz
2021-05-20 14:21 ` [PATCH v2 04/33] block: introduce blk_replace_bs Vladimir Sementsov-Ogievskiy
2021-05-20 14:21 ` [PATCH v2 05/33] qdev-properties: PropertyInfo: add realized_set_allowed field Vladimir Sementsov-Ogievskiy
2021-05-31 15:51   ` Max Reitz
2021-05-20 14:21 ` [PATCH v2 06/33] qdev: allow setting drive property for realized device Vladimir Sementsov-Ogievskiy
2021-05-31 15:59   ` Max Reitz
2021-05-20 14:21 ` [PATCH v2 07/33] block: rename backup-top to copy-before-write Vladimir Sementsov-Ogievskiy
2021-05-31 16:08   ` Max Reitz
2021-05-20 14:21 ` [PATCH v2 08/33] block/backup: drop support for copy_range Vladimir Sementsov-Ogievskiy
2021-05-28 15:29   ` Vladimir Sementsov-Ogievskiy
2021-05-20 14:21 ` [PATCH v2 09/33] block-copy: always set BDRV_REQ_SERIALISING flag Vladimir Sementsov-Ogievskiy
2021-05-20 14:21 ` [PATCH v2 10/33] block/backup: move cluster size calculation to block-copy Vladimir Sementsov-Ogievskiy
2021-05-20 14:21 ` [PATCH v2 11/33] block/copy-before-write: relax permission requirements when no parents Vladimir Sementsov-Ogievskiy
2021-05-20 14:21 ` [PATCH v2 12/33] block/copy-before-write: drop extra bdrv_unref on failure path Vladimir Sementsov-Ogievskiy
2021-05-31 16:19   ` Max Reitz
2021-05-20 14:21 ` [PATCH v2 13/33] block/copy-before-write: use file child instead of backing Vladimir Sementsov-Ogievskiy
2021-05-31 16:22   ` Max Reitz
2021-05-20 14:21 ` [PATCH v2 14/33] block/copy-before-write: bdrv_cbw_append(): replace child at last Vladimir Sementsov-Ogievskiy
2021-05-31 16:55   ` Max Reitz
2021-05-20 14:21 ` [PATCH v2 15/33] block/copy-before-write: introduce cbw_init() Vladimir Sementsov-Ogievskiy
2021-05-31 16:57   ` Max Reitz
2021-05-20 14:21 ` [PATCH v2 16/33] block/copy-before-write: cbw_init(): rename variables Vladimir Sementsov-Ogievskiy
2021-05-20 14:21 ` [PATCH v2 17/33] block/copy-before-write: cbw_init(): use file child after attaching Vladimir Sementsov-Ogievskiy
2021-05-20 14:21 ` [PATCH v2 18/33] block/copy-before-write: cbw_init(): use options Vladimir Sementsov-Ogievskiy
2021-05-31 17:03   ` Max Reitz
2021-05-20 14:21 ` [PATCH v2 19/33] block/copy-before-write: initialize block-copy bitmap Vladimir Sementsov-Ogievskiy
2021-05-31 17:10   ` Max Reitz
2021-05-20 14:21 ` [PATCH v2 20/33] block/block-copy: make setting progress optional Vladimir Sementsov-Ogievskiy
2021-08-10 15:22   ` Hanna Reitz
2021-05-20 14:21 ` [PATCH v2 21/33] block/copy-before-write: make public block driver Vladimir Sementsov-Ogievskiy
2021-05-20 14:21 ` [PATCH v2 22/33] qapi: publish copy-before-write filter Vladimir Sementsov-Ogievskiy
2021-06-01  9:08   ` Max Reitz
2021-05-20 14:21 ` [PATCH v2 23/33] python/qemu/machine.py: refactor _qemu_args() Vladimir Sementsov-Ogievskiy
2021-06-01 10:05   ` Max Reitz
2021-05-20 14:21 ` [PATCH v2 24/33] python/qemu/machine: QEMUMachine: improve qmp() method Vladimir Sementsov-Ogievskiy
2021-06-01 10:19   ` Max Reitz
2021-05-20 14:21 ` [PATCH v2 25/33] iotests.py: VM: add own __enter__ method Vladimir Sementsov-Ogievskiy
2021-06-01 10:58   ` Max Reitz
2021-05-20 14:21 ` [PATCH v2 26/33] iotests/222: fix pylint and mypy complains Vladimir Sementsov-Ogievskiy
2021-06-01 11:16   ` Max Reitz
2021-05-20 14:21 ` [PATCH v2 27/33] iotests/222: constantly use single quotes for strings Vladimir Sementsov-Ogievskiy
2021-06-01 11:17   ` Max Reitz
2021-05-20 14:22 ` [PATCH v2 28/33] iotests: move 222 to tests/image-fleecing Vladimir Sementsov-Ogievskiy
2021-06-01 11:17   ` Max Reitz
2021-05-20 14:22 ` [PATCH v2 29/33] iotests.py: hmp_qemu_io: support qdev Vladimir Sementsov-Ogievskiy
2021-06-01 11:19   ` Max Reitz
2021-05-20 14:22 ` [PATCH v2 30/33] iotests/image-fleecing: proper source device Vladimir Sementsov-Ogievskiy
2021-06-01 11:29   ` Max Reitz
2021-05-20 14:22 ` [PATCH v2 31/33] iotests/image-fleecing: rename tgt_node Vladimir Sementsov-Ogievskiy
2021-06-01 11:46   ` Max Reitz
2021-05-20 14:22 ` [PATCH v2 32/33] iotests/image-fleecing: prepare for adding new test-case Vladimir Sementsov-Ogievskiy
2021-06-01 11:47   ` Max Reitz
2021-05-20 14:22 ` [PATCH v2 33/33] iotests/image-fleecing: add test-case for copy-before-write filter Vladimir Sementsov-Ogievskiy
2021-06-01 12:02   ` Max Reitz
2021-05-31 17:11 ` [PATCH v2 00/33] block: publish backup-top filter Max Reitz
2021-05-31 17:35   ` Vladimir Sementsov-Ogievskiy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).