bitmaps: remove x- prefix from QMP api

When making a backup of a dirty bitmap (for transactions), we want to restore that backup whether or not the bitmap is enabled or not.

It is perfectly valid to write into bitmaps that are disabled. It is only illegitimate for the guest to have done so.

Remove this assertion.

Signed-off-by John Snow <[email protected]>

---
  block/dirty-bitmap.c | 1 -
  1 file changed, 1 deletion(-)

When making a backup of a dirty bitmap (for transactions), we want to restore that backup whether or not the bitmap is enabled or not.

It is perfectly valid to write into bitmaps that are disabled. It is only illegitimate for the guest to have done so.

Remove this assertion.

Signed-off-by John Snow <[email protected]>

---
  block/dirty-bitmap.c | 1 -
  1 file changed, 1 deletion(-)

block/dirty-bitmap.c +0 -1

index 89fd1d7f8b..6b688394e4 100644
@@ -625,7 +625,6 @@ void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out)
  void bdrv_restore_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *backup)
  {
      HBitmap *tmp = bitmap->bitmap;
-    assert(bdrv_dirty_bitmap_enabled(bitmap));
      assert(!bdrv_dirty_bitmap_readonly(bitmap));
      bitmap->bitmap = backup;
      hbitmap_free(tmp);

Especially outside of transactions, it is helpful to provide all-or-nothing semantics for bitmap merges. This facilitates the coalescing of multiple bitmaps into a single target for the "checkpoint" interpretation when assembling bitmaps that represent arbitrary points in time from component bitmaps.

This is an incompatible change from the preliminary version of the API.

Signed-off-by John Snow <[email protected]>

---
  blockdev.c           | 75 ++++++++++++++++++++++++++++++--------------
  qapi/block-core.json | 22 ++++++-------
  2 files changed, 62 insertions(+), 35 deletions(-)

blockdev.c +10 -1

+static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(const char *node,
+                                                    const char *target,
+                                                    strList *bitmaps,
+                                                    HBitmap **backup,
+                                                    Error **errp)
  {
 
-    bdrv_merge_dirty_bitmap(dst, src, NULL, errp);
+    for (lst = bitmaps; lst; lst = lst->next) {
+        src = bdrv_find_dirty_bitmap(bs, lst->value);
+        if (!src) {
+            error_setg(errp, "Dirty bitmap '%s' not found", lst->value);
+            dst = NULL;

qapi/block-core.json +3 -4

@@ -1943,23 +1943,23 @@
  ##
  # @x-block-dirty-bitmap-merge:
  #
-# FIXME: Rename @src_name and @dst_name to src-name and dst-name.
-#
-# Merge @src_name dirty bitmap to @dst_name dirty bitmap. @src_name dirty
-# bitmap is unchanged. On error, @dst_name is unchanged.
+# Merge dirty bitmaps listed in @bitmaps to the @target dirty bitmap.
+# The @bitmaps dirty bitmaps are unchanged.
+# On error, @target is unchanged.

Quoted 6 lines · John Snow

Especially outside of transactions, it is helpful to provide all-or-nothing semantics for bitmap merges. This facilitates the coalescing of multiple bitmaps into a single target for the "checkpoint" interpretation when assembling bitmaps that represent arbitrary points in time from component bitmaps.

Quoted 2 lines

This is an incompatible change from the preliminary version of the API.

but that doesn't matter because it was in the x- namespace, and we're about to rename it anyway.

Quoted 15 lines

Signed-off-by John Snow <[email protected]>

---
  blockdev.c           | 75 ++++++++++++++++++++++++++++++-------------- qapi/block-core.json | 22 ++++++-------

  2 files changed, 62 insertions(+), 35 deletions(-)

+static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(const char *node, +                                                    const char *target, +                                                    strList *bitmaps, +                                                    HBitmap **backup, +                                                    Error **errp) {

Quoted 16 lines

-    bdrv_merge_dirty_bitmap(dst, src, NULL, errp);
+    for (lst = bitmaps; lst; lst = lst->next) {
+        src = bdrv_find_dirty_bitmap(bs, lst->value);
+        if (!src) {
+            error_setg(errp, "Dirty bitmap '%s' not found", lst->value); +            dst = NULL;
+            goto out;
+        }
+
+        bdrv_merge_dirty_bitmap(anon, src, NULL, &local_err); +        if (local_err) {
+            error_propagate(errp, local_err);
+            dst = NULL;
+            goto out;
+        }
+    }

Appears to be a silent no-op when given "bitmaps":[] as the source.  An alternative would be requiring at least one source in the list, but I don't see it as worth changing the patch to special-case an empty list differently from a no-op.

Quoted 21 lines

Review

Quoted 1 line

(quoted diff) +0 -0

@@ -1943,23 +1943,23 @@

  ##
  # @x-block-dirty-bitmap-merge:
  #

qapi/block-core.json +0 -3

-# FIXME: Rename @src_name and @dst_name to src-name and dst-name.
-#
-# Merge @src_name dirty bitmap to @dst_name dirty bitmap. @src_name

dirty

qapi/block-core.json +3 -1

-# bitmap is unchanged. On error, @dst_name is unchanged.
+# Merge dirty bitmaps listed in @bitmaps to the @target dirty bitmap.
+# The @bitmaps dirty bitmaps are unchanged.
+# On error, @target is unchanged.

  #
  # Returns: nothing on success
  #          If @node is not a valid block device, DeviceNotFound

qapi/block-core.json +1 -2

-#          If @dst_name or @src_name is not found, GenericError
-#          If bitmaps has different sizes or granularities, GenericError
+#          If any bitmap in @bitmaps or @target is not found,

GenericError

qapi/block-core.json +2 -0

+#          If any of the bitmaps have different sizes or granularities,
+#              GenericError

Quoted 2 lines

  #
  # Since: 3.0

Could do s/3.0/4.0/ to match the incompatible change here, but you do it in the later patch where your remove the x-.

Reviewed-by Eric Blake <[email protected]>

Especially outside of transactions, it is helpful to provide all-or-nothing semantics for bitmap merges. This facilitates the coalescing of multiple bitmaps into a single target for the "checkpoint" interpretation when assembling bitmaps that represent arbitrary points in time from component bitmaps.

This is an incompatible change from the preliminary version of the API.

Signed-off-by John Snow<[email protected]>

Python before 3.6 does not sort dictionaries (including kwargs). Therefore, printing QMP objects involves sorting the keys to have a predictable ordering in the iotests output.

However, if we want to pretty-print QMP objects being sent to the QEMU process, we need to build the entire command before logging it. Ordinarily, this would then involve "arguments" being sorted above "execute", which would necessitate a rather ugly and harder-to-read change to many iotests outputs.

To facilitate pretty-printing AND maintaining predictable output AND having "arguments" sort before "execute", add a custom sort function

that takes a dictionary and recursively builds an OrderedDict that maintains the specific key order we wish to see in iotests output.

Signed-off-by John Snow <[email protected]>

---
  tests/qemu-iotests/iotests.py | 24 ++++++++++++++++++++----
  1 file changed, 20 insertions(+), 4 deletions(-)

Python before 3.6 does not sort dictionaries (including kwargs). Therefore, printing QMP objects involves sorting the keys to have a predictable ordering in the iotests output.

However, if we want to pretty-print QMP objects being sent to the QEMU process, we need to build the entire command before logging it. Ordinarily, this would then involve "arguments" being sorted above "execute", which would necessitate a rather ugly and harder-to-read change to many iotests outputs.

To facilitate pretty-printing AND maintaining predictable output AND having "arguments" sort before "execute", add a custom sort function that takes a dictionary and recursively builds an OrderedDict that maintains the specific key order we wish to see in iotests output.

Signed-off-by John Snow<[email protected]>

Several places in iotests deal with serializing objects into JSON strings, but to add pretty-printing it seems desireable to localize

all of those cases.

log() seems like a good candidate for that centralized behavior. log() can already serialize json objects, but when it does so, it assumes filters=[] operates on QMP objects, not strings.

qmp_log currently operates by dumping outgoing and incoming QMP objects into strings and filtering them assuming that filters=[] are string filters.

To have qmp_log use log's serialization, qmp_log will need to accept only qmp filters, not text filters.

However, only a single caller of qmp_log actually requires any filters at all. I remove the default filter and add it explicitly to the caller in preparation for refactoring qmp_log to use rich filters instead.

test 206 is amended to name the filter explicitly and the default is removed.

Signed-off-by John Snow <[email protected]>

---
  tests/qemu-iotests/206        | 8 ++++++--
  tests/qemu-iotests/iotests.py | 2 +-
  2 files changed, 7 insertions(+), 3 deletions(-)

Several places in iotests deal with serializing objects into JSON strings, but to add pretty-printing it seems desireable to localize all of those cases.

log() seems like a good candidate for that centralized behavior. log() can already serialize json objects, but when it does so, it assumes filters=[] operates on QMP objects, not strings.

qmp_log currently operates by dumping outgoing and incoming QMP objects into strings and filtering them assuming that filters=[] are string filters.

To have qmp_log use log's serialization, qmp_log will need to accept only qmp filters, not text filters.

However, only a single caller of qmp_log actually requires any filters at all. I remove the default filter and add it explicitly to the caller in preparation for refactoring qmp_log to use rich filters instead.

test 206 is amended to name the filter explicitly and the default is removed.

Signed-off-by John Snow<[email protected]>

As laid out in the previous commit's message:

```
Several places in iotests deal with serializing objects into JSON strings, but to add pretty-printing it seems desireable to localize

all of those cases.

log() seems like a good candidate for that centralized behavior. log() can already serialize json objects, but when it does so, it assumes filters=[] operates on QMP objects, not strings.

qmp_log currently operates by dumping outgoing and incoming QMP objects into strings and filtering them assuming that filters=[] are string filters.
```

Therefore:

Change qmp_log to treat filters as if they're always qmp object filters, then change the logging call to rely on log()'s ability to serialize QMP objects, so we're not duplicating that effort.

Add a qmp version of filter_testfiles and adjust the only caller using it for qmp_log to use the qmp version.

Signed-off-by John Snow <[email protected]>

Signed-off-by John Snow <[email protected]>

---
  tests/qemu-iotests/206        |  4 ++--
  tests/qemu-iotests/iotests.py | 24 +++++++++++++++++++++---
  2 files changed, 23 insertions(+), 5 deletions(-)

Quoted 5 lines · John Snow

As laid out in the previous commit's message:

```
Several places in iotests deal with serializing objects into JSON strings, but to add pretty-printing it seems desireable to localize

s/desireable/desirable/

Quoted 17 lines

all of those cases.

log() seems like a good candidate for that centralized behavior. log() can already serialize json objects, but when it does so, it assumes filters=[] operates on QMP objects, not strings.

qmp_log currently operates by dumping outgoing and incoming QMP objects into strings and filtering them assuming that filters=[] are string filters.
```

Therefore:

Change qmp_log to treat filters as if they're always qmp object filters, then change the logging call to rely on log()'s ability to serialize QMP objects, so we're not duplicating that effort.

Quoted 5 lines

Add a qmp version of filter_testfiles and adjust the only caller using it for qmp_log to use the qmp version.

Signed-off-by John Snow  <[email protected]>

Signed-off-by John Snow <[email protected]>

Odd double S-o-B differing only by space.

Quoted 6 lines

---
  tests/qemu-iotests/206        |  4 ++-- tests/qemu-iotests/iotests.py | 24 +++++++++++++++++++++---

  2 files changed, 23 insertions(+), 5 deletions(-)

Reviewed-by Eric Blake <[email protected]>

As laid out in the previous commit's message:

```
Several places in iotests deal with serializing objects into JSON
strings, but to add pretty-printing it seems desireable to localize
all of those cases.

log() seems like a good candidate for that centralized behavior.
log() can already serialize json objects, but when it does so,
it assumes filters=[] operates on QMP objects, not strings.

qmp_log currently operates by dumping outgoing and incoming QMP
objects into strings and filtering them assuming that filters=[]
are string filters.
```

Therefore:

Change qmp_log to treat filters as if they're always qmp object filters, then change the logging call to rely on log()'s ability to serialize QMP objects, so we're not duplicating that effort.

Add a qmp version of filter_testfiles and adjust the only caller using it for qmp_log to use the qmp version.

Signed-off-by John Snow <[email protected]>

Signed-off-by John Snow <[email protected]>

---
  tests/qemu-iotests/206        |  4 ++--
  tests/qemu-iotests/iotests.py | 24 +++++++++++++++++++++---
  2 files changed, 23 insertions(+), 5 deletions(-)

tests/qemu-iotests/206 +2 -2

index e92550fa59..5bb738bf23 100755
@@ -27,7 +27,7 @@ iotests.verify_image_format(supported_fmts=['qcow2'])
 
  def blockdev_create(vm, options):
      result = vm.qmp_log('blockdev-create',
-                        filters=[iotests.filter_testfiles],
+                        filters=[iotests.filter_qmp_testfiles],
                          job_id='job0', options=options)
 
      if 'return' in result:

@@ -55,7 +55,7 @@ with iotests.FilePath('t.qcow2') as disk_path, \
                            'size': 0 })
 
      vm.qmp_log('blockdev-add',
-               filters=[iotests.filter_testfiles],
+               filters=[iotests.filter_qmp_testfiles],
                 driver='file', filename=disk_path,
                 node_name='imgfile')
 

tests/qemu-iotests/iotests.py +0 -0

index 57fe20db45..dcd0c6f71d 100644

tests/qemu-iotests/iotests.py +14 -3

+        elif isinstance(qmsg[key], dict):
+            qmsg[key] = filter_qmp(qmsg[key], filter_fn)
+        else:
+            qmsg[key] = filter_fn(key, qmsg[key]) > +    return qmsg
+
  def filter_testfiles(msg):
      prefix = os.path.join(test_dir, "%s-" % (os.getpid()))
      return msg.replace(prefix, 'TEST_DIR/PID-')
 
+def filter_qmp_testfiles(qmsg):
+    def _filter(key, value):
+        if key == 'filename' or key == 'backing-file':
+            return filter_testfiles(value)
+        return value
+    return filter_qmp(qmsg, _filter)
+
  def filter_generated_node_ids(msg):
      return re.sub("#block[0-9]+", "NODE_NAME", msg)
 

@@ -465,10 +484,9 @@ class VM(qtest.QEMUQtestMachine):
              ("execute", cmd),
              ("arguments", ordered_kwargs(kwargs))
          ))
-        logmsg = json.dumps(full_cmd)
-        log(logmsg, filters)
+        log(full_cmd, filters)
          result = self.qmp(cmd, **kwargs)
-        log(json.dumps(result, sort_keys=True), filters)
+        log(result, filters)
          return result
 
      def run_job(self, job, auto_finalize=True, auto_dismiss=False):

Quoted 59 lines · John Snow

As laid out in the previous commit's message:

```
Several places in iotests deal with serializing objects into JSON
strings, but to add pretty-printing it seems desireable to localize
all of those cases.

log() seems like a good candidate for that centralized behavior.
log() can already serialize json objects, but when it does so,
it assumes filters=[] operates on QMP objects, not strings.

qmp_log currently operates by dumping outgoing and incoming QMP
objects into strings and filtering them assuming that filters=[]
are string filters.
```

Therefore:

Change qmp_log to treat filters as if they're always qmp object filters, then change the logging call to rely on log()'s ability to serialize QMP objects, so we're not duplicating that effort.

Add a qmp version of filter_testfiles and adjust the only caller using it for qmp_log to use the qmp version.

Signed-off-by John Snow <[email protected]>

Signed-off-by John Snow <[email protected]>

---
  tests/qemu-iotests/206        |  4 ++--
  tests/qemu-iotests/iotests.py | 24 +++++++++++++++++++++---
  2 files changed, 23 insertions(+), 5 deletions(-)

tests/qemu-iotests/206 +2 -2

index e92550fa59..5bb738bf23 100755
@@ -27,7 +27,7 @@ iotests.verify_image_format(supported_fmts=['qcow2'])
 
  def blockdev_create(vm, options):
      result = vm.qmp_log('blockdev-create',
-                        filters=[iotests.filter_testfiles],
+                        filters=[iotests.filter_qmp_testfiles],
                          job_id='job0', options=options)
 
      if 'return' in result:

@@ -55,7 +55,7 @@ with iotests.FilePath('t.qcow2') as disk_path, \
                            'size': 0 })
 
      vm.qmp_log('blockdev-add',
-               filters=[iotests.filter_testfiles],
+               filters=[iotests.filter_qmp_testfiles],
                 driver='file', filename=disk_path,
                 node_name='imgfile')
 

tests/qemu-iotests/iotests.py +0 -0

index 57fe20db45..dcd0c6f71d 100644

tests/qemu-iotests/iotests.py def filter_qmp_event(event): · line 251

Quoted 6 lines

tests/qemu-iotests/iotests.py +3 -0

@@ -246,10 +246,29 @@ def filter_qmp_event(event):
          event['timestamp']['microseconds'] = 'USECS'
      return event
 
+def filter_qmp(qmsg, filter_fn):
+    '''Given a string filter, filter a QMP object's values.
+    filter_fn takes a (key, value) pair.'''

hm, I decided to look into PEP8, which in turn refers to PEP257,
which asks:
- For consistency, always use """triple double quotes""" around docstring
- Unless the entire docstring fits on a line, place the closing quotes on a line by themselves

Unfortunately, iotests.py prefers to be in opposition.. And consistency within the file is more important. May be we'll fix it one day..

tests/qemu-iotests/iotests.py def filter_qmp_event(event): · line 252

Quoted 1 line

tests/qemu-iotests/iotests.py +1 -0

+    for key in qmsg:

and here again we can benefit (or all right-value qmsg[key]) of using qmsg.items()

tests/qemu-iotests/iotests.py def filter_qmp_event(event): · line 254

Quoted 2 lines

tests/qemu-iotests/iotests.py +2 -0

+        if isinstance(qmsg[key], list):
+            qmsg[key] = [filter_qmp(atom, filter_fn) for atom in qmsg[key]]

hmm, stop. filter_qmp() assumes that its argument is dict. but atom may not be dict.

so, to fit into the concept of fn(key, value) filtering function, we should do something like this:

for i in len(qmsg[key]):
if isinstance(qmsg[key], dict):

     qmsg[key][i] = filter_qmp(qmsg[key][i], filter_fn)

qmsg[key] = filter_fn(key, qmsg[key])

---
or, we may want to apply filter_fn only to lists of non-dicts, and filter only list of dicts, assuming that we don't have mixed lists.

Quoted 33 lines

tests/qemu-iotests/iotests.py +14 -3

+        elif isinstance(qmsg[key], dict):
+            qmsg[key] = filter_qmp(qmsg[key], filter_fn)
+        else:
+            qmsg[key] = filter_fn(key, qmsg[key]) > +    return qmsg
+
  def filter_testfiles(msg):
      prefix = os.path.join(test_dir, "%s-" % (os.getpid()))
      return msg.replace(prefix, 'TEST_DIR/PID-')
 
+def filter_qmp_testfiles(qmsg):
+    def _filter(key, value):
+        if key == 'filename' or key == 'backing-file':
+            return filter_testfiles(value)
+        return value
+    return filter_qmp(qmsg, _filter)
+
  def filter_generated_node_ids(msg):
      return re.sub("#block[0-9]+", "NODE_NAME", msg)
 

@@ -465,10 +484,9 @@ class VM(qtest.QEMUQtestMachine):
              ("execute", cmd),
              ("arguments", ordered_kwargs(kwargs))
          ))
-        logmsg = json.dumps(full_cmd)
-        log(logmsg, filters)
+        log(full_cmd, filters)
          result = self.qmp(cmd, **kwargs)
-        log(json.dumps(result, sort_keys=True), filters)
+        log(result, filters)
          return result
 
      def run_job(self, job, auto_finalize=True, auto_dismiss=False):

If iotests have lines exceeding >998 characters long, git doesn't want to send it plaintext to the list. We can solve this by allowing the iotests to use pretty printed QMP output that we can match against instead.

As a bonus, it's much nicer for human eyes too.

Signed-off-by John Snow <[email protected]>

---
  tests/qemu-iotests/iotests.py | 15 ++++++++++-----
  1 file changed, 10 insertions(+), 5 deletions(-)

If iotests have lines exceeding >998 characters long, git doesn't want to send it plaintext to the list. We can solve this by allowing the iotests to use pretty printed QMP output that we can match against instead.

As a bonus, it's much nicer for human eyes too.

Signed-off-by John Snow<[email protected]>

New interface, new smoke test.

Signed-off-by John Snow <[email protected]>

---
  tests/qemu-iotests/236     | 161 +++++++++++++++++
  tests/qemu-iotests/236.out | 351 +++++++++++++++++++++++++++++++++++++
  tests/qemu-iotests/group   |   1 +
  3 files changed, 513 insertions(+)

create mode 100755 tests/qemu-iotests/236
create mode 100644 tests/qemu-iotests/236.out

Quoted 11 lines · John Snow

New interface, new smoke test.

Signed-off-by John Snow <[email protected]>

---
  tests/qemu-iotests/236     | 161 +++++++++++++++++ tests/qemu-iotests/236.out | 351 +++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/group   |   1 +

  3 files changed, 513 insertions(+)

  create mode 100755 tests/qemu-iotests/236
  create mode 100644 tests/qemu-iotests/236.out

Reviewed-by Eric Blake <[email protected]>

(and glad that my insistence on beefing up the test has caught bugs)

New interface, new smoke test.

Signed-off-by John Snow <[email protected]>

---

tests/qemu-iotests/236 new file +18 -0

+    vm.qmp_log("transaction", indent=2, actions=[
+        { "type": "block-dirty-bitmap-add",
+          "data": { "node": "drive0", "name": "bitmapD",
+                    "disabled": True, "granularity": granularity }},
+        { "type": "block-dirty-bitmap-merge",
+          "data": { "node": "drive0", "target": "bitmapD",
+                    "bitmaps": ["bitmapB", "bitmapC"] }},
+        { "type": "abort", "data": {}}
+    ])
+    log(query_bitmaps(vm), indent=2)
+
+    log('\n--- Creating D as a merge of B & C ---\n')
+    # Good hygiene: create a disabled bitmap as a merge target.
+    vm.qmp_log("transaction", indent=2, actions=[
+        { "type": "block-dirty-bitmap-add",
+          "data": { "node": "drive0", "name": "bitmapD",
+                    "disabled": True, "granularity": granularity }},
+        { "type": "block-dirty-bitmap-merge",

Quoted 4 lines · John Snow

New interface, new smoke test.

Signed-off-by John Snow <[email protected]>

---

[...]

tests/qemu-iotests/236 line 116

Quoted 6 lines

tests/qemu-iotests/236 new file +6 -0

+    # A: 7 clusters
+    # B: 4 clusters
+    # C: 6 clusters
+    log(query_bitmaps(vm), indent=2)
+
+    log('\n--- Submitting Bad Merge ---\n')

aha, spent some time, trying to understand, what is bad with merge, until understand that that is abort. I didn't sleep enough last night, but anyway, 'Aborting Merge Transaction' is a bit clearer, I think.

Quoted 22 lines

tests/qemu-iotests/236 new file +22 -0

+    vm.qmp_log("transaction", indent=2, actions=[
+        { "type": "block-dirty-bitmap-add",
+          "data": { "node": "drive0", "name": "bitmapD",
+                    "disabled": True, "granularity": granularity }},
+        { "type": "block-dirty-bitmap-merge",
+          "data": { "node": "drive0", "target": "bitmapD",
+                    "bitmaps": ["bitmapB", "bitmapC"] }},
+        { "type": "abort", "data": {}}
+    ])
+    log(query_bitmaps(vm), indent=2)
+
+    log('\n--- Creating D as a merge of B & C ---\n')
+    # Good hygiene: create a disabled bitmap as a merge target.
+    vm.qmp_log("transaction", indent=2, actions=[
+        { "type": "block-dirty-bitmap-add",
+          "data": { "node": "drive0", "name": "bitmapD",
+                    "disabled": True, "granularity": granularity }},
+        { "type": "block-dirty-bitmap-merge",
+          "data": { "node": "drive0", "target": "bitmapD",
+                    "bitmaps": ["bitmapB", "bitmapC"] }}
+    ])
+

tests/qemu-iotests/236 line 144

Quoted 6 lines

tests/qemu-iotests/236 new file +6 -0

+    # A and D should now both have 7 clusters apiece.
+    # B and C remain unchanged with 4 and 6 respectively.
+    log(query_bitmaps(vm), indent=2)
+
+    # A and D should be equivalent.
+    # Some formats round the size of the disk, so don't print the checksums.

Just interested: round 64M? to what?

tests/qemu-iotests/236 line 149

Quoted 5 lines

tests/qemu-iotests/236 new file +5 -0

+    check_a = vm.qmp('x-debug-block-dirty-bitmap-sha256',
+                     node="drive0", name="bitmapA")['return']['sha256']
+    check_b = vm.qmp('x-debug-block-dirty-bitmap-sha256',
+                     node="drive0", name="bitmapD")['return']['sha256']
+    assert(check_a == check_b)

hmm, a funny suggestion: s/check_b/check_d/

Review

Quoted 3 lines

(quoted diff) +2 -0

+
+    log('\n--- Removing bitmaps A, B, C, and D ---\n')

what about failed transaction with remove command, for a full kit?

tests/qemu-iotests/236 line 161

Quoted 10 lines

tests/qemu-iotests/236 new file +10 -0

+    vm.qmp_log("block-dirty-bitmap-remove", node="drive0", name="bitmapA")
+    vm.qmp_log("block-dirty-bitmap-remove", node="drive0", name="bitmapB")
+    vm.qmp_log("block-dirty-bitmap-remove", node="drive0", name="bitmapC")
+    vm.qmp_log("block-dirty-bitmap-remove", node="drive0", name="bitmapD")
+
+    log('\n--- Final Query ---\n')
+    log(query_bitmaps(vm), indent=2)
+
+    log('\n--- Done ---\n')
+    vm.shutdown()

with or without any of my suggestions:

Reviewed-by Vladimir Sementsov-Ogievskiy <[email protected]>