[Qemu-devel] [PATCH v3 0/7] bitmaps: remove x- prefix from QMP api

[Qemu-devel] [PATCH v3 0/7] bitmaps: remove x- prefix from QMP api

[John Snow]

Touch up a few last things and remove the x- prefix.

V3:
 - Reworked qmp_log to pretty-print the outgoing command, too [Vladimir]
 - Modified test to log only bitmap information [Vladimir]
 - Test disable/enable transaction toggle [Eric]

Note that the filter I added is now unused, but I think we will want it
and it's small enough, so I'm going to check it in anyway. If you disagree,
I'll just drop the patch instead.

--js

John Snow (7):
  blockdev: abort transactions in reverse order
  blockdev: n-ary bitmap merge
  block: remove 'x' prefix from experimental bitmap APIs
  iotests.py: don't abort if IMGKEYSECRET is undefined
  iotests: add filter_generated_node_ids
  iotests: allow pretty-print for qmp_log
  iotests: add iotest 236 for testing bitmap merge

 blockdev.c                    |  96 +++++++++-------
 qapi/block-core.json          |  56 +++++-----
 qapi/transaction.json         |  12 +-
 tests/qemu-iotests/223        |   4 +-
 tests/qemu-iotests/236        | 124 +++++++++++++++++++++
 tests/qemu-iotests/236.out    | 200 ++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/group      |   1 +
 tests/qemu-iotests/iotests.py |  22 +++-
 8 files changed, 436 insertions(+), 79 deletions(-)
 create mode 100755 tests/qemu-iotests/236
 create mode 100644 tests/qemu-iotests/236.out

-- 
2.17.2

[Qemu-devel] [PATCH v3 1/7] blockdev: abort transactions in reverse order

[John Snow]

Presently, we abort transactions in the same order they were processed in.
Bitmap commands, though, attempt to restore backup data structures on abort.
To that end, though, they need to be aborted in reverse chronological order.

Replace the QSIMPLEQ data structure with a QTAILQ one, so we can iterate
in reverse for the abort phase of the transaction.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 blockdev.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 81f95d920b..1ba706df8b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1341,7 +1341,7 @@ struct BlkActionState {
     const BlkActionOps *ops;
     JobTxn *block_job_txn;
     TransactionProperties *txn_props;
-    QSIMPLEQ_ENTRY(BlkActionState) entry;
+    QTAILQ_ENTRY(BlkActionState) entry;
 };
 
 /* internal snapshot private data */
@@ -2269,8 +2269,8 @@ void qmp_transaction(TransactionActionList *dev_list,
     BlkActionState *state, *next;
     Error *local_err = NULL;
 
-    QSIMPLEQ_HEAD(snap_bdrv_states, BlkActionState) snap_bdrv_states;
-    QSIMPLEQ_INIT(&snap_bdrv_states);
+    QTAILQ_HEAD(snap_bdrv_states, BlkActionState) snap_bdrv_states;
+    QTAILQ_INIT(&snap_bdrv_states);
 
     /* Does this transaction get canceled as a group on failure?
      * If not, we don't really need to make a JobTxn.
@@ -2301,7 +2301,7 @@ void qmp_transaction(TransactionActionList *dev_list,
         state->action = dev_info;
         state->block_job_txn = block_job_txn;
         state->txn_props = props;
-        QSIMPLEQ_INSERT_TAIL(&snap_bdrv_states, state, entry);
+        QTAILQ_INSERT_TAIL(&snap_bdrv_states, state, entry);
 
         state->ops->prepare(state, &local_err);
         if (local_err) {
@@ -2310,7 +2310,7 @@ void qmp_transaction(TransactionActionList *dev_list,
         }
     }
 
-    QSIMPLEQ_FOREACH(state, &snap_bdrv_states, entry) {
+    QTAILQ_FOREACH(state, &snap_bdrv_states, entry) {
         if (state->ops->commit) {
             state->ops->commit(state);
         }
@@ -2321,13 +2321,13 @@ void qmp_transaction(TransactionActionList *dev_list,
 
 delete_and_fail:
     /* failure, and it is all-or-none; roll back all operations */
-    QSIMPLEQ_FOREACH(state, &snap_bdrv_states, entry) {
+    QTAILQ_FOREACH_REVERSE(state, &snap_bdrv_states, snap_bdrv_states, entry) {
         if (state->ops->abort) {
             state->ops->abort(state);
         }
     }
 exit:
-    QSIMPLEQ_FOREACH_SAFE(state, &snap_bdrv_states, entry, next) {
+    QTAILQ_FOREACH_SAFE(state, &snap_bdrv_states, entry, next) {
         if (state->ops->clean) {
             state->ops->clean(state);
         }
-- 
2.17.2

[Qemu-devel] [PATCH v3 2/7] blockdev: n-ary bitmap merge

[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.

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

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 blockdev.c           | 64 +++++++++++++++++++++++++++++---------------
 qapi/block-core.json | 22 +++++++--------
 2 files changed, 53 insertions(+), 33 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 1ba706df8b..0f740fd964 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2122,33 +2122,26 @@ static void block_dirty_bitmap_disable_abort(BlkActionState *common)
     }
 }
 
+void do_block_dirty_bitmap_merge(const char *node, const char *target,
+                                 strList *bitmaps, HBitmap **backup,
+                                 Error **errp);
+
 static void block_dirty_bitmap_merge_prepare(BlkActionState *common,
                                              Error **errp)
 {
     BlockDirtyBitmapMerge *action;
     BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
                                              common, common);
-    BdrvDirtyBitmap *merge_source;
 
     if (action_check_completion_mode(common, errp) < 0) {
         return;
     }
 
     action = common->action->u.x_block_dirty_bitmap_merge.data;
-    state->bitmap = block_dirty_bitmap_lookup(action->node,
-                                              action->dst_name,
-                                              &state->bs,
-                                              errp);
-    if (!state->bitmap) {
-        return;
-    }
 
-    merge_source = bdrv_find_dirty_bitmap(state->bs, action->src_name);
-    if (!merge_source) {
-        return;
-    }
-
-    bdrv_merge_dirty_bitmap(state->bitmap, merge_source, &state->backup, errp);
+    do_block_dirty_bitmap_merge(action->node, action->target,
+                                action->bitmaps, &state->backup,
+                                errp);
 }
 
 static void abort_prepare(BlkActionState *common, Error **errp)
@@ -2980,24 +2973,51 @@ void qmp_x_block_dirty_bitmap_disable(const char *node, const char *name,
     bdrv_disable_dirty_bitmap(bitmap);
 }
 
-void qmp_x_block_dirty_bitmap_merge(const char *node, const char *dst_name,
-                                    const char *src_name, Error **errp)
+void do_block_dirty_bitmap_merge(const char *node, const char *target,
+                                 strList *bitmaps, HBitmap **backup,
+                                 Error **errp)
 {
     BlockDriverState *bs;
-    BdrvDirtyBitmap *dst, *src;
+    BdrvDirtyBitmap *dst, *src, *anon;
+    strList *lst;
+    Error *local_err = NULL;
 
-    dst = block_dirty_bitmap_lookup(node, dst_name, &bs, errp);
+    dst = block_dirty_bitmap_lookup(node, target, &bs, errp);
     if (!dst) {
         return;
     }
 
-    src = bdrv_find_dirty_bitmap(bs, src_name);
-    if (!src) {
-        error_setg(errp, "Dirty bitmap '%s' not found", src_name);
+    anon = bdrv_create_dirty_bitmap(bs, bdrv_dirty_bitmap_granularity(dst),
+                                    NULL, errp);
+    if (!anon) {
         return;
     }
 
-    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);
+            goto out;
+        }
+
+        bdrv_merge_dirty_bitmap(anon, src, NULL, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            goto out;
+        }
+    }
+
+    /* Merge into dst; dst is unchanged on failure. */
+    bdrv_merge_dirty_bitmap(dst, anon, backup, errp);
+
+ out:
+    bdrv_release_dirty_bitmap(bs, anon);
+}
+
+void qmp_x_block_dirty_bitmap_merge(const char *node, const char *target,
+                                    strList *bitmaps, Error **errp)
+{
+    do_block_dirty_bitmap_merge(node, target, bitmaps, NULL, errp);
 }
 
 BlockDirtyBitmapSha256 *qmp_x_debug_block_dirty_bitmap_sha256(const char *node,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index d4fe710836..320d74ef34 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1818,14 +1818,14 @@
 #
 # @node: name of device/node which the bitmap is tracking
 #
-# @dst_name: name of the destination dirty bitmap
+# @target: name of the destination dirty bitmap
 #
-# @src_name: name of the source dirty bitmap
+# @bitmaps: name(s) of the source dirty bitmap(s)
 #
 # Since: 3.0
 ##
 { 'struct': 'BlockDirtyBitmapMerge',
-  'data': { 'node': 'str', 'dst_name': 'str', 'src_name': 'str' } }
+  'data': { 'node': 'str', 'target': 'str', 'bitmaps': ['str'] } }
 
 ##
 # @block-dirty-bitmap-add:
@@ -1940,23 +1940,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.
 #
 # Returns: nothing on success
 #          If @node is not a valid block device, DeviceNotFound
-#          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
+#          If any of the bitmaps have different sizes or granularities,
+#              GenericError
 #
 # Since: 3.0
 #
 # Example:
 #
 # -> { "execute": "x-block-dirty-bitmap-merge",
-#      "arguments": { "node": "drive0", "dst_name": "bitmap0",
-#                     "src_name": "bitmap1" } }
+#      "arguments": { "node": "drive0", "target": "bitmap0",
+#                     "bitmaps": ["bitmap1"] } }
 # <- { "return": {} }
 #
 ##
-- 
2.17.2

[Qemu-devel] [PATCH v3 3/7] block: remove 'x' prefix from experimental bitmap APIs

[John Snow]

The 'x' prefix was added because I was uncertain of the direction we'd
take for the libvirt API. With the general approach solidified, I feel
comfortable committing to this API for 4.0.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 blockdev.c             | 22 +++++++++++-----------
 qapi/block-core.json   | 34 +++++++++++++++++-----------------
 qapi/transaction.json  | 12 ++++++------
 tests/qemu-iotests/223 |  4 ++--
 4 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 0f740fd964..da87aae5cf 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1966,7 +1966,7 @@ static void block_dirty_bitmap_add_prepare(BlkActionState *common,
                                action->has_granularity, action->granularity,
                                action->has_persistent, action->persistent,
                                action->has_autoload, action->autoload,
-                               action->has_x_disabled, action->x_disabled,
+                               action->has_disabled, action->disabled,
                                &local_err);
 
     if (!local_err) {
@@ -2051,7 +2051,7 @@ static void block_dirty_bitmap_enable_prepare(BlkActionState *common,
         return;
     }
 
-    action = common->action->u.x_block_dirty_bitmap_enable.data;
+    action = common->action->u.block_dirty_bitmap_enable.data;
     state->bitmap = block_dirty_bitmap_lookup(action->node,
                                               action->name,
                                               NULL,
@@ -2092,7 +2092,7 @@ static void block_dirty_bitmap_disable_prepare(BlkActionState *common,
         return;
     }
 
-    action = common->action->u.x_block_dirty_bitmap_disable.data;
+    action = common->action->u.block_dirty_bitmap_disable.data;
     state->bitmap = block_dirty_bitmap_lookup(action->node,
                                               action->name,
                                               NULL,
@@ -2137,7 +2137,7 @@ static void block_dirty_bitmap_merge_prepare(BlkActionState *common,
         return;
     }
 
-    action = common->action->u.x_block_dirty_bitmap_merge.data;
+    action = common->action->u.block_dirty_bitmap_merge.data;
 
     do_block_dirty_bitmap_merge(action->node, action->target,
                                 action->bitmaps, &state->backup,
@@ -2205,17 +2205,17 @@ static const BlkActionOps actions[] = {
         .commit = block_dirty_bitmap_free_backup,
         .abort = block_dirty_bitmap_restore,
     },
-    [TRANSACTION_ACTION_KIND_X_BLOCK_DIRTY_BITMAP_ENABLE] = {
+    [TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_ENABLE] = {
         .instance_size = sizeof(BlockDirtyBitmapState),
         .prepare = block_dirty_bitmap_enable_prepare,
         .abort = block_dirty_bitmap_enable_abort,
     },
-    [TRANSACTION_ACTION_KIND_X_BLOCK_DIRTY_BITMAP_DISABLE] = {
+    [TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_DISABLE] = {
         .instance_size = sizeof(BlockDirtyBitmapState),
         .prepare = block_dirty_bitmap_disable_prepare,
         .abort = block_dirty_bitmap_disable_abort,
     },
-    [TRANSACTION_ACTION_KIND_X_BLOCK_DIRTY_BITMAP_MERGE] = {
+    [TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_MERGE] = {
         .instance_size = sizeof(BlockDirtyBitmapState),
         .prepare = block_dirty_bitmap_merge_prepare,
         .commit = block_dirty_bitmap_free_backup,
@@ -2931,7 +2931,7 @@ void qmp_block_dirty_bitmap_clear(const char *node, const char *name,
     bdrv_clear_dirty_bitmap(bitmap, NULL);
 }
 
-void qmp_x_block_dirty_bitmap_enable(const char *node, const char *name,
+void qmp_block_dirty_bitmap_enable(const char *node, const char *name,
                                    Error **errp)
 {
     BlockDriverState *bs;
@@ -2952,7 +2952,7 @@ void qmp_x_block_dirty_bitmap_enable(const char *node, const char *name,
     bdrv_enable_dirty_bitmap(bitmap);
 }
 
-void qmp_x_block_dirty_bitmap_disable(const char *node, const char *name,
+void qmp_block_dirty_bitmap_disable(const char *node, const char *name,
                                     Error **errp)
 {
     BlockDriverState *bs;
@@ -3014,8 +3014,8 @@ void do_block_dirty_bitmap_merge(const char *node, const char *target,
     bdrv_release_dirty_bitmap(bs, anon);
 }
 
-void qmp_x_block_dirty_bitmap_merge(const char *node, const char *target,
-                                    strList *bitmaps, Error **errp)
+void qmp_block_dirty_bitmap_merge(const char *node, const char *target,
+                                  strList *bitmaps, Error **errp)
 {
     do_block_dirty_bitmap_merge(node, target, bitmaps, NULL, errp);
 }
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 320d74ef34..fde96fdb50 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1803,15 +1803,15 @@
 #            Currently, all dirty tracking bitmaps are loaded from Qcow2 on
 #            open.
 #
-# @x-disabled: the bitmap is created in the disabled state, which means that
-#              it will not track drive changes. The bitmap may be enabled with
-#              x-block-dirty-bitmap-enable. Default is false. (Since: 3.0)
+# @disabled: the bitmap is created in the disabled state, which means that
+#            it will not track drive changes. The bitmap may be enabled with
+#            block-dirty-bitmap-enable. Default is false. (Since: 4.0)
 #
 # Since: 2.4
 ##
 { 'struct': 'BlockDirtyBitmapAdd',
   'data': { 'node': 'str', 'name': 'str', '*granularity': 'uint32',
-            '*persistent': 'bool', '*autoload': 'bool', '*x-disabled': 'bool' } }
+            '*persistent': 'bool', '*autoload': 'bool', '*disabled': 'bool' } }
 
 ##
 # @BlockDirtyBitmapMerge:
@@ -1822,7 +1822,7 @@
 #
 # @bitmaps: name(s) of the source dirty bitmap(s)
 #
-# Since: 3.0
+# Since: 4.0
 ##
 { 'struct': 'BlockDirtyBitmapMerge',
   'data': { 'node': 'str', 'target': 'str', 'bitmaps': ['str'] } }
@@ -1896,7 +1896,7 @@
   'data': 'BlockDirtyBitmap' }
 
 ##
-# @x-block-dirty-bitmap-enable:
+# @block-dirty-bitmap-enable:
 #
 # Enables a dirty bitmap so that it will begin tracking disk changes.
 #
@@ -1904,20 +1904,20 @@
 #          If @node is not a valid block device, DeviceNotFound
 #          If @name is not found, GenericError with an explanation
 #
-# Since: 3.0
+# Since: 4.0
 #
 # Example:
 #
-# -> { "execute": "x-block-dirty-bitmap-enable",
+# -> { "execute": "block-dirty-bitmap-enable",
 #      "arguments": { "node": "drive0", "name": "bitmap0" } }
 # <- { "return": {} }
 #
 ##
-  { 'command': 'x-block-dirty-bitmap-enable',
+  { 'command': 'block-dirty-bitmap-enable',
     'data': 'BlockDirtyBitmap' }
 
 ##
-# @x-block-dirty-bitmap-disable:
+# @block-dirty-bitmap-disable:
 #
 # Disables a dirty bitmap so that it will stop tracking disk changes.
 #
@@ -1925,20 +1925,20 @@
 #          If @node is not a valid block device, DeviceNotFound
 #          If @name is not found, GenericError with an explanation
 #
-# Since: 3.0
+# Since: 4.0
 #
 # Example:
 #
-# -> { "execute": "x-block-dirty-bitmap-disable",
+# -> { "execute": "block-dirty-bitmap-disable",
 #      "arguments": { "node": "drive0", "name": "bitmap0" } }
 # <- { "return": {} }
 #
 ##
-    { 'command': 'x-block-dirty-bitmap-disable',
+    { 'command': 'block-dirty-bitmap-disable',
       'data': 'BlockDirtyBitmap' }
 
 ##
-# @x-block-dirty-bitmap-merge:
+# @block-dirty-bitmap-merge:
 #
 # Merge dirty bitmaps listed in @bitmaps to the @target dirty bitmap.
 # The @bitmaps dirty bitmaps are unchanged.
@@ -1950,17 +1950,17 @@
 #          If any of the bitmaps have different sizes or granularities,
 #              GenericError
 #
-# Since: 3.0
+# Since: 4.0
 #
 # Example:
 #
-# -> { "execute": "x-block-dirty-bitmap-merge",
+# -> { "execute": "block-dirty-bitmap-merge",
 #      "arguments": { "node": "drive0", "target": "bitmap0",
 #                     "bitmaps": ["bitmap1"] } }
 # <- { "return": {} }
 #
 ##
-      { 'command': 'x-block-dirty-bitmap-merge',
+      { 'command': 'block-dirty-bitmap-merge',
         'data': 'BlockDirtyBitmapMerge' }
 
 ##
diff --git a/qapi/transaction.json b/qapi/transaction.json
index 5875cdb16c..95edb78227 100644
--- a/qapi/transaction.json
+++ b/qapi/transaction.json
@@ -46,9 +46,9 @@
 # - @abort: since 1.6
 # - @block-dirty-bitmap-add: since 2.5
 # - @block-dirty-bitmap-clear: since 2.5
-# - @x-block-dirty-bitmap-enable: since 3.0
-# - @x-block-dirty-bitmap-disable: since 3.0
-# - @x-block-dirty-bitmap-merge: since 3.1
+# - @block-dirty-bitmap-enable: since 4.0
+# - @block-dirty-bitmap-disable: since 4.0
+# - @block-dirty-bitmap-merge: since 4.0
 # - @blockdev-backup: since 2.3
 # - @blockdev-snapshot: since 2.5
 # - @blockdev-snapshot-internal-sync: since 1.7
@@ -62,9 +62,9 @@
        'abort': 'Abort',
        'block-dirty-bitmap-add': 'BlockDirtyBitmapAdd',
        'block-dirty-bitmap-clear': 'BlockDirtyBitmap',
-       'x-block-dirty-bitmap-enable': 'BlockDirtyBitmap',
-       'x-block-dirty-bitmap-disable': 'BlockDirtyBitmap',
-       'x-block-dirty-bitmap-merge': 'BlockDirtyBitmapMerge',
+       'block-dirty-bitmap-enable': 'BlockDirtyBitmap',
+       'block-dirty-bitmap-disable': 'BlockDirtyBitmap',
+       'block-dirty-bitmap-merge': 'BlockDirtyBitmapMerge',
        'blockdev-backup': 'BlockdevBackup',
        'blockdev-snapshot': 'BlockdevSnapshot',
        'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal',
diff --git a/tests/qemu-iotests/223 b/tests/qemu-iotests/223
index 397b865d34..5513dc6215 100755
--- a/tests/qemu-iotests/223
+++ b/tests/qemu-iotests/223
@@ -112,9 +112,9 @@ _send_qemu_cmd $QEMU_HANDLE '{"execute":"qmp_capabilities"}' "return"
 _send_qemu_cmd $QEMU_HANDLE '{"execute":"blockdev-add",
   "arguments":{"driver":"qcow2", "node-name":"n",
     "file":{"driver":"file", "filename":"'"$TEST_IMG"'"}}}' "return"
-_send_qemu_cmd $QEMU_HANDLE '{"execute":"x-block-dirty-bitmap-disable",
+_send_qemu_cmd $QEMU_HANDLE '{"execute":"block-dirty-bitmap-disable",
   "arguments":{"node":"n", "name":"b"}}' "return"
-_send_qemu_cmd $QEMU_HANDLE '{"execute":"x-block-dirty-bitmap-disable",
+_send_qemu_cmd $QEMU_HANDLE '{"execute":"block-dirty-bitmap-disable",
   "arguments":{"node":"n", "name":"b2"}}' "return"
 _send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-start",
   "arguments":{"addr":{"type":"unix",
-- 
2.17.2

[Qemu-devel] [PATCH v3 4/7] iotests.py: don't abort if IMGKEYSECRET is undefined

[John Snow]

Instead of using os.environ[], use .get with a default of empty string
to match the setup in check to allow us to import the iotests module
(for debugging, say) without needing a crafted environment just to
import the module.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/iotests.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index d537538ba0..a34e66813a 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -63,7 +63,7 @@ socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', 'socket_scm_helper')
 debug = False
 
 luks_default_secret_object = 'secret,id=keysec0,data=' + \
-                             os.environ['IMGKEYSECRET']
+                             os.environ.get('IMGKEYSECRET', '')
 luks_default_key_secret_opt = 'key-secret=keysec0'
 
 
-- 
2.17.2

[Qemu-devel] [PATCH v3 6/7] iotests: allow pretty-print for qmp_log

[John Snow]

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.

Note that this changes the sort order for "command" and "arguments",
so I restrict this reordering to occur only when the indent is specified.
---
 tests/qemu-iotests/iotests.py | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 9595429fea..ab5823c011 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -447,12 +447,21 @@ class VM(qtest.QEMUQtestMachine):
             result.append(filter_qmp_event(ev))
         return result
 
-    def qmp_log(self, cmd, filters=[filter_testfiles], **kwargs):
-        logmsg = '{"execute": "%s", "arguments": %s}' % \
-            (cmd, json.dumps(kwargs, sort_keys=True))
+    def qmp_log(self, cmd, indent=None, filters=[filter_testfiles], **kwargs):
+        # Python < 3.4 needs to know not to add whitespace when pretty-printing:
+        separators = (',', ': ') if indent is not None else (',', ': ')
+        if indent is not None:
+            fullcmd = { "execute": cmd,
+                        "arguments": kwargs }
+            logmsg = json.dumps(fullcmd, indent=indent, separators=separators,
+                                sort_keys=True)
+        else:
+            logmsg = '{"execute": "%s", "arguments": %s}' % \
+                (cmd, json.dumps(kwargs, sort_keys=True))
         log(logmsg, filters)
         result = self.qmp(cmd, **kwargs)
-        log(json.dumps(result, sort_keys=True), filters)
+        log(json.dumps(result, indent=indent, separators=separators,
+                       sort_keys=True), filters)
         return result
 
     def run_job(self, job, auto_finalize=True, auto_dismiss=False):
-- 
2.17.2

[Qemu-devel] [PATCH v3 7/7] iotests: add iotest 236 for testing bitmap merge

[John Snow]

New interface, new smoke test.
---
 tests/qemu-iotests/236     | 124 +++++++++++++++++++++++
 tests/qemu-iotests/236.out | 200 +++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 325 insertions(+)
 create mode 100755 tests/qemu-iotests/236
 create mode 100644 tests/qemu-iotests/236.out

diff --git a/tests/qemu-iotests/236 b/tests/qemu-iotests/236
new file mode 100755
index 0000000000..edf16c4ab1
--- /dev/null
+++ b/tests/qemu-iotests/236
@@ -0,0 +1,124 @@
+#!/usr/bin/env python
+#
+# Test bitmap merges.
+#
+# Copyright (c) 2018 John Snow for Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+# owner=jsnow@redhat.com
+
+import json
+import iotests
+from iotests import log
+
+iotests.verify_image_format(supported_fmts=['qcow2'])
+
+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)
+
+def query_bitmaps(vm):
+    res = vm.qmp("query-block")
+    return {device['device']: device['dirty-bitmaps'] for
+            device in res['return']}
+
+with iotests.FilePath('img') as img_path, \
+     iotests.VM() as vm:
+
+    log('--- Preparing image & VM ---\n')
+    iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, img_path, '64M')
+    vm.add_drive(img_path)
+    vm.launch()
+
+    log('--- Adding preliminary bitmaps A & B ---\n')
+    vm.qmp_log("block-dirty-bitmap-add", node="drive0", name="bitmapA")
+    vm.qmp_log("block-dirty-bitmap-add", node="drive0", name="bitmapB")
+
+    # Dirties 4 clusters. count=262144
+    log('')
+    log('--- Emulating writes ---\n')
+    for p in patterns:
+        cmd = "write -P%s %s %s" % p
+        log(cmd)
+        log(vm.hmp_qemu_io("drive0", cmd))
+
+    log(json.dumps(query_bitmaps(vm), indent=2, separators=(',', ': ')))
+
+    log('')
+    log('--- Disabling B & Adding C ---\n')
+    vm.qmp_log("transaction", indent=2, actions=[
+        { "type": "block-dirty-bitmap-disable",
+          "data": { "node": "drive0", "name": "bitmapB" }},
+        { "type": "block-dirty-bitmap-add",
+          "data": { "node": "drive0", "name": "bitmapC" }},
+        # Purely extraneous, but test that it works:
+        { "type": "block-dirty-bitmap-disable",
+          "data": { "node": "drive0", "name": "bitmapC" }},
+        { "type": "block-dirty-bitmap-enable",
+          "data": { "node": "drive0", "name": "bitmapC" }},
+    ])
+
+    log('')
+    log('--- Emulating further writes ---\n')
+    # Dirties 6 clusters, 3 of which are new in contrast to "A".
+    # A = 64 * 1024 * (4 + 3) = 458752
+    # C = 64 * 1024 * 6       = 393216
+    for p in overwrite:
+        cmd = "write -P%s %s %s" % p
+        log(cmd)
+        log(vm.hmp_qemu_io("drive0", cmd))
+
+    log('')
+    log('--- Disabling A & C ---\n')
+    vm.qmp_log("transaction", indent=2, actions=[
+        { "type": "block-dirty-bitmap-disable",
+          "data": { "node": "drive0", "name": "bitmapA" }},
+        { "type": "block-dirty-bitmap-disable",
+          "data": { "node": "drive0", "name": "bitmapC" }}
+    ])
+
+    # A: 7 clusters
+    # B: 4 clusters
+    # C: 6 clusters
+    log(json.dumps(query_bitmaps(vm), indent=2, separators=(',', ': ')))
+
+    log('')
+    log('--- 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 }},
+        { "type": "block-dirty-bitmap-merge",
+          "data": { "node": "drive0", "target": "bitmapD",
+                    "bitmaps": ["bitmapB", "bitmapC"] }}
+    ])
+
+    # A and D should now both have 7 clusters apiece.
+    # B and C remain unchanged with 4 and 6 respectively.
+    log(json.dumps(query_bitmaps(vm), indent=2, separators=(',', ': ')))
+
+    # A and D should be equivalent.
+    vm.qmp_log('x-debug-block-dirty-bitmap-sha256',
+               node="drive0", name="bitmapA")
+    vm.qmp_log('x-debug-block-dirty-bitmap-sha256',
+               node="drive0", name="bitmapD")
+
+    vm.shutdown()
diff --git a/tests/qemu-iotests/236.out b/tests/qemu-iotests/236.out
new file mode 100644
index 0000000000..42c131504d
--- /dev/null
+++ b/tests/qemu-iotests/236.out
@@ -0,0 +1,200 @@
+--- Preparing image & VM ---
+
+--- Adding preliminary bitmaps A & B ---
+
+{"execute": "block-dirty-bitmap-add", "arguments": {"name": "bitmapA", "node": "drive0"}}
+{"return": {}}
+{"execute": "block-dirty-bitmap-add", "arguments": {"name": "bitmapB", "node": "drive0"}}
+{"return": {}}
+
+--- Emulating writes ---
+
+write -P0x5d 0 64k
+{"return": ""}
+write -P0xd5 1M 64k
+{"return": ""}
+write -P0xdc 32M 64k
+{"return": ""}
+write -P0xcd 0x3ff0000 64k
+{"return": ""}
+{
+  "drive0": [
+    {
+      "status": "active",
+      "count": 262144,
+      "name": "bitmapB",
+      "granularity": 65536
+    },
+    {
+      "status": "active",
+      "count": 262144,
+      "name": "bitmapA",
+      "granularity": 65536
+    }
+  ]
+}
+
+--- Disabling B & Adding C ---
+
+{
+  "arguments": {
+    "actions": [
+      {
+        "data": {
+          "name": "bitmapB",
+          "node": "drive0"
+        },
+        "type": "block-dirty-bitmap-disable"
+      },
+      {
+        "data": {
+          "name": "bitmapC",
+          "node": "drive0"
+        },
+        "type": "block-dirty-bitmap-add"
+      },
+      {
+        "data": {
+          "name": "bitmapC",
+          "node": "drive0"
+        },
+        "type": "block-dirty-bitmap-disable"
+      },
+      {
+        "data": {
+          "name": "bitmapC",
+          "node": "drive0"
+        },
+        "type": "block-dirty-bitmap-enable"
+      }
+    ]
+  },
+  "execute": "transaction"
+}
+{
+  "return": {}
+}
+
+--- Emulating further writes ---
+
+write -P0xab 0 64k
+{"return": ""}
+write -P0xad 0x00f8000 64k
+{"return": ""}
+write -P0x1d 0x2008000 64k
+{"return": ""}
+write -P0xea 0x3fe0000 64k
+{"return": ""}
+
+--- Disabling A & C ---
+
+{
+  "arguments": {
+    "actions": [
+      {
+        "data": {
+          "name": "bitmapA",
+          "node": "drive0"
+        },
+        "type": "block-dirty-bitmap-disable"
+      },
+      {
+        "data": {
+          "name": "bitmapC",
+          "node": "drive0"
+        },
+        "type": "block-dirty-bitmap-disable"
+      }
+    ]
+  },
+  "execute": "transaction"
+}
+{
+  "return": {}
+}
+{
+  "drive0": [
+    {
+      "status": "disabled",
+      "count": 393216,
+      "name": "bitmapC",
+      "granularity": 65536
+    },
+    {
+      "status": "disabled",
+      "count": 262144,
+      "name": "bitmapB",
+      "granularity": 65536
+    },
+    {
+      "status": "disabled",
+      "count": 458752,
+      "name": "bitmapA",
+      "granularity": 65536
+    }
+  ]
+}
+
+--- Creating D as a merge of B & C ---
+
+{
+  "arguments": {
+    "actions": [
+      {
+        "data": {
+          "disabled": true,
+          "name": "bitmapD",
+          "node": "drive0"
+        },
+        "type": "block-dirty-bitmap-add"
+      },
+      {
+        "data": {
+          "bitmaps": [
+            "bitmapB",
+            "bitmapC"
+          ],
+          "node": "drive0",
+          "target": "bitmapD"
+        },
+        "type": "block-dirty-bitmap-merge"
+      }
+    ]
+  },
+  "execute": "transaction"
+}
+{
+  "return": {}
+}
+{
+  "drive0": [
+    {
+      "status": "disabled",
+      "count": 458752,
+      "name": "bitmapD",
+      "granularity": 65536
+    },
+    {
+      "status": "disabled",
+      "count": 393216,
+      "name": "bitmapC",
+      "granularity": 65536
+    },
+    {
+      "status": "disabled",
+      "count": 262144,
+      "name": "bitmapB",
+      "granularity": 65536
+    },
+    {
+      "status": "disabled",
+      "count": 458752,
+      "name": "bitmapA",
+      "granularity": 65536
+    }
+  ]
+}
+{"execute": "x-debug-block-dirty-bitmap-sha256", "arguments": {"name": "bitmapA", "node": "drive0"}}
+{"return": {"sha256": "7abe3d7e3c794cfaf9b08bc9ce599192c96a2206f07b42d9997ff78fdd7af744"}}
+{"execute": "x-debug-block-dirty-bitmap-sha256", "arguments": {"name": "bitmapD", "node": "drive0"}}
+{"return": {"sha256": "7abe3d7e3c794cfaf9b08bc9ce599192c96a2206f07b42d9997ff78fdd7af744"}}
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 61a6d98ebd..a61f9e83d6 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -233,3 +233,4 @@
 233 auto quick
 234 auto quick migration
 235 auto quick
+236 auto quick
\ No newline at end of file
-- 
2.17.2

[Qemu-devel] [PATCH v3 5/7 RESEND] iotests: add filter_generated_node_ids

[John Snow]

To mimic the common filter of the same name, but for the python tests.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/iotests.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index a34e66813a..9595429fea 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -239,6 +239,9 @@ def filter_testfiles(msg):
     prefix = os.path.join(test_dir, "%s-" % (os.getpid()))
     return msg.replace(prefix, 'TEST_DIR/PID-')
 
+def filter_generated_node_ids(msg):
+    return re.sub("#block[0-9]+", "NODE_NAME", msg)
+
 def filter_img_info(output, filename):
     lines = []
     for line in output.split('\n'):
-- 
2.17.2

Re: [Qemu-devel] [PATCH v3 6/7] iotests: allow pretty-print for qmp_log

[Vladimir Sementsov-Ogievskiy]

15.12.2018 2:15, John Snow wrote:
> 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.
> 
> Note that this changes the sort order for "command" and "arguments",
> so I restrict this reordering to occur only when the indent is specified.
> ---
>   tests/qemu-iotests/iotests.py | 17 +++++++++++++----
>   1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 9595429fea..ab5823c011 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -447,12 +447,21 @@ class VM(qtest.QEMUQtestMachine):
>               result.append(filter_qmp_event(ev))
>           return result
>   
> -    def qmp_log(self, cmd, filters=[filter_testfiles], **kwargs):
> -        logmsg = '{"execute": "%s", "arguments": %s}' % \
> -            (cmd, json.dumps(kwargs, sort_keys=True))
> +    def qmp_log(self, cmd, indent=None, filters=[filter_testfiles], **kwargs):
> +        # Python < 3.4 needs to know not to add whitespace when pretty-printing:
> +        separators = (',', ': ') if indent is not None else (',', ': ')
> +        if indent is not None:
> +            fullcmd = { "execute": cmd,
> +                        "arguments": kwargs }
> +            logmsg = json.dumps(fullcmd, indent=indent, separators=separators,
> +                                sort_keys=True)
> +        else:
> +            logmsg = '{"execute": "%s", "arguments": %s}' % \
> +                (cmd, json.dumps(kwargs, sort_keys=True))

definitely overlogic on None/is not None, but anyway, ',' separator leads to less
beautiful output. So, I think it is better to do just "re.sub(r'\s+\n', '\n', longmsg)"
on final message, here or in log(), or in both.

>           log(logmsg, filters)
>           result = self.qmp(cmd, **kwargs)
> -        log(json.dumps(result, sort_keys=True), filters)
> +        log(json.dumps(result, indent=indent, separators=separators,
> +                       sort_keys=True), filters)
>           return result
>   
>       def run_job(self, job, auto_finalize=True, auto_dismiss=False):
> 


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH v3 5/7] iotests: add filter_generated_node_ids

[Vladimir Sementsov-Ogievskiy]

15.12.2018 2:15, John Snow wrote:
> To mimic the common filter of the same name, but for the python tests.
> 
> Signed-off-by: John Snow <jsnow@redhat.com.

oops, '.' -> '>'

and also, strange CC line in header, including "John Snow <jsnow@redhat.com>, John"

> Reviewed-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   tests/qemu-iotests/iotests.py | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index a34e66813a..9595429fea 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -239,6 +239,9 @@ def filter_testfiles(msg):
>       prefix = os.path.join(test_dir, "%s-" % (os.getpid()))
>       return msg.replace(prefix, 'TEST_DIR/PID-')
>   
> +def filter_generated_node_ids(msg):
> +    return re.sub("#block[0-9]+", "NODE_NAME", msg)
> +
>   def filter_img_info(output, filename):
>       lines = []
>       for line in output.split('\n'):
> 


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH v3 7/7] iotests: add iotest 236 for testing bitmap merge

[Vladimir Sementsov-Ogievskiy]

15.12.2018 2:15, John Snow wrote:
> New interface, new smoke test.
> ---
>   tests/qemu-iotests/236     | 124 +++++++++++++++++++++++
>   tests/qemu-iotests/236.out | 200 +++++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/group   |   1 +
>   3 files changed, 325 insertions(+)
>   create mode 100755 tests/qemu-iotests/236
>   create mode 100644 tests/qemu-iotests/236.out
> 
> diff --git a/tests/qemu-iotests/236 b/tests/qemu-iotests/236
> new file mode 100755
> index 0000000000..edf16c4ab1
> --- /dev/null
> +++ b/tests/qemu-iotests/236
> @@ -0,0 +1,124 @@
> +#!/usr/bin/env python
> +#
> +# Test bitmap merges.
> +#
> +# Copyright (c) 2018 John Snow for Red Hat, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +#
> +# owner=jsnow@redhat.com
> +
> +import json
> +import iotests
> +from iotests import log
> +
> +iotests.verify_image_format(supported_fmts=['qcow2'])
> +

we hardcode patterns, so it's better to hardcode size here too:

size = 64 * 1024 * 1024

> +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)
> +
> +def query_bitmaps(vm):
> +    res = vm.qmp("query-block")
> +    return {device['device']: device['dirty-bitmaps'] for
> +            device in res['return']}
> +
> +with iotests.FilePath('img') as img_path, \
> +     iotests.VM() as vm:
> +
> +    log('--- Preparing image & VM ---\n')
> +    iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, img_path, '64M')

no needs for qemu_img_pipe, as you don't use its output, qemu_img() is enough and qemu_img_create() is better,
and the best I think is just:

vm.add_drive_raw('id=drive0,driver=null-co,size=' + str(size))

as qcow2 (and real disk in general) is actually unrelated to the test.


> +    vm.add_drive(img_path)
> +    vm.launch()
> +
> +    log('--- Adding preliminary bitmaps A & B ---\n')
> +    vm.qmp_log("block-dirty-bitmap-add", node="drive0", name="bitmapA")
> +    vm.qmp_log("block-dirty-bitmap-add", node="drive0", name="bitmapB")
> +
> +    # Dirties 4 clusters. count=262144
> +    log('')
> +    log('--- Emulating writes ---\n')
> +    for p in patterns:
> +        cmd = "write -P%s %s %s" % p
> +        log(cmd)
> +        log(vm.hmp_qemu_io("drive0", cmd))
> +
> +    log(json.dumps(query_bitmaps(vm), indent=2, separators=(',', ': ')))

log can do json.dumps, so it's strange to do it here, and I don't like ',' separator, as I described



-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH v3 6/7] iotests: allow pretty-print for qmp_log

[Vladimir Sementsov-Ogievskiy]

17.12.2018 17:26, Vladimir Sementsov-Ogievskiy wrote:
> 15.12.2018 2:15, John Snow wrote:
>> 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.
>>
>> Note that this changes the sort order for "command" and "arguments",
>> so I restrict this reordering to occur only when the indent is specified.
>> ---
>>   tests/qemu-iotests/iotests.py | 17 +++++++++++++----
>>   1 file changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index 9595429fea..ab5823c011 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -447,12 +447,21 @@ class VM(qtest.QEMUQtestMachine):
>>               result.append(filter_qmp_event(ev))
>>           return result
>> -    def qmp_log(self, cmd, filters=[filter_testfiles], **kwargs):
>> -        logmsg = '{"execute": "%s", "arguments": %s}' % \
>> -            (cmd, json.dumps(kwargs, sort_keys=True))
>> +    def qmp_log(self, cmd, indent=None, filters=[filter_testfiles], **kwargs):
>> +        # Python < 3.4 needs to know not to add whitespace when pretty-printing:
>> +        separators = (',', ': ') if indent is not None else (',', ': ')
>> +        if indent is not None:
>> +            fullcmd = { "execute": cmd,
>> +                        "arguments": kwargs }
>> +            logmsg = json.dumps(fullcmd, indent=indent, separators=separators,
>> +                                sort_keys=True)
>> +        else:
>> +            logmsg = '{"execute": "%s", "arguments": %s}' % \
>> +                (cmd, json.dumps(kwargs, sort_keys=True))
> 
> definitely overlogic on None/is not None, but anyway, ',' separator leads to less
> beautiful output. So, I think it is better to do just "re.sub(r'\s+\n', '\n', longmsg)"
> on final message, here or in log(), or in both.

find myself composing counter-proposal patch, will send soon...

> 
>>           log(logmsg, filters)
>>           result = self.qmp(cmd, **kwargs)
>> -        log(json.dumps(result, sort_keys=True), filters)
>> +        log(json.dumps(result, indent=indent, separators=separators,
>> +                       sort_keys=True), filters)
>>           return result
>>       def run_job(self, job, auto_finalize=True, auto_dismiss=False):
>>
> 
> 


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH v3 6/7] iotests: allow pretty-print for qmp_log

[John Snow]

On 12/17/18 11:37 AM, Vladimir Sementsov-Ogievskiy wrote:
> 17.12.2018 17:26, Vladimir Sementsov-Ogievskiy wrote:
>> 15.12.2018 2:15, John Snow wrote:
>>> 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.
>>>
>>> Note that this changes the sort order for "command" and "arguments",
>>> so I restrict this reordering to occur only when the indent is specified.
>>> ---
>>>   tests/qemu-iotests/iotests.py | 17 +++++++++++++----
>>>   1 file changed, 13 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>>> index 9595429fea..ab5823c011 100644
>>> --- a/tests/qemu-iotests/iotests.py
>>> +++ b/tests/qemu-iotests/iotests.py
>>> @@ -447,12 +447,21 @@ class VM(qtest.QEMUQtestMachine):
>>>               result.append(filter_qmp_event(ev))
>>>           return result
>>> -    def qmp_log(self, cmd, filters=[filter_testfiles], **kwargs):
>>> -        logmsg = '{"execute": "%s", "arguments": %s}' % \
>>> -            (cmd, json.dumps(kwargs, sort_keys=True))
>>> +    def qmp_log(self, cmd, indent=None, filters=[filter_testfiles], **kwargs):
>>> +        # Python < 3.4 needs to know not to add whitespace when pretty-printing:
>>> +        separators = (',', ': ') if indent is not None else (',', ': ')
>>> +        if indent is not None:
>>> +            fullcmd = { "execute": cmd,
>>> +                        "arguments": kwargs }
>>> +            logmsg = json.dumps(fullcmd, indent=indent, separators=separators,
>>> +                                sort_keys=True)
>>> +        else:
>>> +            logmsg = '{"execute": "%s", "arguments": %s}' % \
>>> +                (cmd, json.dumps(kwargs, sort_keys=True))
>>
>> definitely overlogic on None/is not None, but anyway, ',' separator leads to less
>> beautiful output. So, I think it is better to do just "re.sub(r'\s+\n', '\n', longmsg)"
>> on final message, here or in log(), or in both.

I can't use "if indent" because 0 is a valid indentation, and I decided
to keep the "correct" order for execute/arguments if no indentation is
requested.

How is ',' less beautiful? When pretty-printing output ',' only occurs
at the end of a line anyway. I don't understand the critique.

> 
> find myself composing counter-proposal patch, will send soon...
> 
>>
>>>           log(logmsg, filters)
>>>           result = self.qmp(cmd, **kwargs)
>>> -        log(json.dumps(result, sort_keys=True), filters)
>>> +        log(json.dumps(result, indent=indent, separators=separators,
>>> +                       sort_keys=True), filters)
>>>           return result
>>>       def run_job(self, job, auto_finalize=True, auto_dismiss=False):
>>>
>>
>>
> 
> 

Re: [Qemu-devel] [PATCH v3 0/7] bitmaps: remove x- prefix from QMP api

[John Snow]

On 12/14/18 6:15 PM, John Snow wrote:
> Touch up a few last things and remove the x- prefix.
> 
> V3:
>  - Reworked qmp_log to pretty-print the outgoing command, too [Vladimir]
>  - Modified test to log only bitmap information [Vladimir]
>  - Test disable/enable transaction toggle [Eric]
> 
> Note that the filter I added is now unused, but I think we will want it
> and it's small enough, so I'm going to check it in anyway. If you disagree,
> I'll just drop the patch instead.
> 
> --js
> 
> John Snow (7):
>   blockdev: abort transactions in reverse order
>   blockdev: n-ary bitmap merge
>   block: remove 'x' prefix from experimental bitmap APIs
>   iotests.py: don't abort if IMGKEYSECRET is undefined
>   iotests: add filter_generated_node_ids
>   iotests: allow pretty-print for qmp_log
>   iotests: add iotest 236 for testing bitmap merge
> 
>  blockdev.c                    |  96 +++++++++-------
>  qapi/block-core.json          |  56 +++++-----
>  qapi/transaction.json         |  12 +-
>  tests/qemu-iotests/223        |   4 +-
>  tests/qemu-iotests/236        | 124 +++++++++++++++++++++
>  tests/qemu-iotests/236.out    | 200 ++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/group      |   1 +
>  tests/qemu-iotests/iotests.py |  22 +++-
>  8 files changed, 436 insertions(+), 79 deletions(-)
>  create mode 100755 tests/qemu-iotests/236
>  create mode 100644 tests/qemu-iotests/236.out
> 

Thanks, I'm staging patches 1-5 and I'll send the PR once we get to the
bottom of patches 6 and 7, just to keep volume on the list down.

--js

Re: [Qemu-devel] [PATCH v3 7/7] iotests: add iotest 236 for testing bitmap merge

[John Snow]

On 12/17/18 10:48 AM, Vladimir Sementsov-Ogievskiy wrote:
> 15.12.2018 2:15, John Snow wrote:
>> New interface, new smoke test.
>> ---
>>   tests/qemu-iotests/236     | 124 +++++++++++++++++++++++
>>   tests/qemu-iotests/236.out | 200 +++++++++++++++++++++++++++++++++++++
>>   tests/qemu-iotests/group   |   1 +
>>   3 files changed, 325 insertions(+)
>>   create mode 100755 tests/qemu-iotests/236
>>   create mode 100644 tests/qemu-iotests/236.out
>>
>> diff --git a/tests/qemu-iotests/236 b/tests/qemu-iotests/236
>> new file mode 100755
>> index 0000000000..edf16c4ab1
>> --- /dev/null
>> +++ b/tests/qemu-iotests/236
>> @@ -0,0 +1,124 @@
>> +#!/usr/bin/env python
>> +#
>> +# Test bitmap merges.
>> +#
>> +# Copyright (c) 2018 John Snow for Red Hat, Inc.
>> +#
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 2 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> +#
>> +# owner=jsnow@redhat.com
>> +
>> +import json
>> +import iotests
>> +from iotests import log
>> +
>> +iotests.verify_image_format(supported_fmts=['qcow2'])
>> +
> 
> we hardcode patterns, so it's better to hardcode size here too:
> 
> size = 64 * 1024 * 1024

If you insist.

> 
>> +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)
>> +
>> +def query_bitmaps(vm):
>> +    res = vm.qmp("query-block")
>> +    return {device['device']: device['dirty-bitmaps'] for
>> +            device in res['return']}
>> +
>> +with iotests.FilePath('img') as img_path, \
>> +     iotests.VM() as vm:
>> +
>> +    log('--- Preparing image & VM ---\n')
>> +    iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, img_path, '64M')
> 
> no needs for qemu_img_pipe, as you don't use its output, qemu_img() is enough and qemu_img_create() is better,
> and the best I think is just:
> 

More cargo cult copy and paste.

> vm.add_drive_raw('id=drive0,driver=null-co,size=' + str(size))
> 
> as qcow2 (and real disk in general) is actually unrelated to the test.
> 

I think I'll leave the image creation in just so that if you run the
test under different formats it'll actually do something vaguely
different, just in case.

Actually, it did highlight how weird VPC is again and I changed the rest
as a result to accommodate image formats that round their disk sizes.

> 
>> +    vm.add_drive(img_path)
>> +    vm.launch()
>> +
>> +    log('--- Adding preliminary bitmaps A & B ---\n')
>> +    vm.qmp_log("block-dirty-bitmap-add", node="drive0", name="bitmapA")
>> +    vm.qmp_log("block-dirty-bitmap-add", node="drive0", name="bitmapB")
>> +
>> +    # Dirties 4 clusters. count=262144
>> +    log('')
>> +    log('--- Emulating writes ---\n')
>> +    for p in patterns:
>> +        cmd = "write -P%s %s %s" % p
>> +        log(cmd)
>> +        log(vm.hmp_qemu_io("drive0", cmd))
>> +
>> +    log(json.dumps(query_bitmaps(vm), indent=2, separators=(',', ': ')))
> 
> log can do json.dumps, so it's strange to do it here, and I don't like ',' separator, as I described

Because it tries to filter the rich object before it converts, as you've
noticed. I think I'll take a page from your book and try to fix log() to
be better.

As for not liking the ',' separator, it should be identical to your
approach because it only removes the space when pretty printing is
enabled. Can you show me what this approach does that you find to be
ugly and how it's different from your regex approach?

--js

Re: [Qemu-devel] [PATCH v3 6/7] iotests: allow pretty-print for qmp_log

[Vladimir Sementsov-Ogievskiy]

17.12.2018 22:21, John Snow wrote:
> 
> 
> On 12/17/18 11:37 AM, Vladimir Sementsov-Ogievskiy wrote:
>> 17.12.2018 17:26, Vladimir Sementsov-Ogievskiy wrote:
>>> 15.12.2018 2:15, John Snow wrote:
>>>> 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.
>>>>
>>>> Note that this changes the sort order for "command" and "arguments",
>>>> so I restrict this reordering to occur only when the indent is specified.
>>>> ---
>>>>    tests/qemu-iotests/iotests.py | 17 +++++++++++++----
>>>>    1 file changed, 13 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>>>> index 9595429fea..ab5823c011 100644
>>>> --- a/tests/qemu-iotests/iotests.py
>>>> +++ b/tests/qemu-iotests/iotests.py
>>>> @@ -447,12 +447,21 @@ class VM(qtest.QEMUQtestMachine):
>>>>                result.append(filter_qmp_event(ev))
>>>>            return result
>>>> -    def qmp_log(self, cmd, filters=[filter_testfiles], **kwargs):
>>>> -        logmsg = '{"execute": "%s", "arguments": %s}' % \
>>>> -            (cmd, json.dumps(kwargs, sort_keys=True))
>>>> +    def qmp_log(self, cmd, indent=None, filters=[filter_testfiles], **kwargs):
>>>> +        # Python < 3.4 needs to know not to add whitespace when pretty-printing:
>>>> +        separators = (',', ': ') if indent is not None else (',', ': ')
>>>> +        if indent is not None:
>>>> +            fullcmd = { "execute": cmd,
>>>> +                        "arguments": kwargs }
>>>> +            logmsg = json.dumps(fullcmd, indent=indent, separators=separators,
>>>> +                                sort_keys=True)
>>>> +        else:
>>>> +            logmsg = '{"execute": "%s", "arguments": %s}' % \
>>>> +                (cmd, json.dumps(kwargs, sort_keys=True))
>>>
>>> definitely overlogic on None/is not None, but anyway, ',' separator leads to less
>>> beautiful output. So, I think it is better to do just "re.sub(r'\s+\n', '\n', longmsg)"
>>> on final message, here or in log(), or in both.
> 
> I can't use "if indent" because 0 is a valid indentation, and I decided
> to keep the "correct" order for execute/arguments if no indentation is
> requested.
> 
> How is ',' less beautiful? When pretty-printing output ',' only occurs
> at the end of a line anyway. I don't understand the critique.

Oops, you are right, I thought about a,b,c,d all this time :(.

Anyway, I'd prefer to make indent=2 by default, like in my sent patch. But now, your
way looks ok for me too, at least as a first step.

so, with fixed s/else (',', ': ')/else (', ', ': ')/:

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

however, I'd prefer positive conditions: if indent is None, instead of if indent is not None, as we have
else branch, and positive way is always shorter in this case.


> 
>>
>> find myself composing counter-proposal patch, will send soon...
>>
>>>
>>>>            log(logmsg, filters)
>>>>            result = self.qmp(cmd, **kwargs)
>>>> -        log(json.dumps(result, sort_keys=True), filters)
>>>> +        log(json.dumps(result, indent=indent, separators=separators,
>>>> +                       sort_keys=True), filters)
>>>>            return result
>>>>        def run_job(self, job, auto_finalize=True, auto_dismiss=False):
>>>>
>>>
>>>
>>
>>


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH v3 7/7] iotests: add iotest 236 for testing bitmap merge

[Vladimir Sementsov-Ogievskiy]

18.12.2018 0:32, John Snow wrote:
> 
> 
> On 12/17/18 10:48 AM, Vladimir Sementsov-Ogievskiy wrote:
>> 15.12.2018 2:15, John Snow wrote:
>>> New interface, new smoke test.
>>> ---
>>>    tests/qemu-iotests/236     | 124 +++++++++++++++++++++++
>>>    tests/qemu-iotests/236.out | 200 +++++++++++++++++++++++++++++++++++++
>>>    tests/qemu-iotests/group   |   1 +
>>>    3 files changed, 325 insertions(+)
>>>    create mode 100755 tests/qemu-iotests/236
>>>    create mode 100644 tests/qemu-iotests/236.out
>>>
>>> diff --git a/tests/qemu-iotests/236 b/tests/qemu-iotests/236
>>> new file mode 100755
>>> index 0000000000..edf16c4ab1
>>> --- /dev/null
>>> +++ b/tests/qemu-iotests/236
>>> @@ -0,0 +1,124 @@
>>> +#!/usr/bin/env python
>>> +#
>>> +# Test bitmap merges.
>>> +#
>>> +# Copyright (c) 2018 John Snow for Red Hat, Inc.
>>> +#
>>> +# This program is free software; you can redistribute it and/or modify
>>> +# it under the terms of the GNU General Public License as published by
>>> +# the Free Software Foundation; either version 2 of the License, or
>>> +# (at your option) any later version.
>>> +#
>>> +# This program is distributed in the hope that it will be useful,
>>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> +# GNU General Public License for more details.
>>> +#
>>> +# You should have received a copy of the GNU General Public License
>>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>> +#
>>> +# owner=jsnow@redhat.com
>>> +
>>> +import json
>>> +import iotests
>>> +from iotests import log
>>> +
>>> +iotests.verify_image_format(supported_fmts=['qcow2'])
>>> +
>>
>> we hardcode patterns, so it's better to hardcode size here too:
>>
>> size = 64 * 1024 * 1024
> 
> If you insist.
> 
>>
>>> +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)
>>> +
>>> +def query_bitmaps(vm):
>>> +    res = vm.qmp("query-block")
>>> +    return {device['device']: device['dirty-bitmaps'] for
>>> +            device in res['return']}
>>> +
>>> +with iotests.FilePath('img') as img_path, \
>>> +     iotests.VM() as vm:
>>> +
>>> +    log('--- Preparing image & VM ---\n')
>>> +    iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, img_path, '64M')
>>
>> no needs for qemu_img_pipe, as you don't use its output, qemu_img() is enough and qemu_img_create() is better,
>> and the best I think is just:
>>
> 
> More cargo cult copy and paste.
> 
>> vm.add_drive_raw('id=drive0,driver=null-co,size=' + str(size))
>>
>> as qcow2 (and real disk in general) is actually unrelated to the test.
>>
> 
> I think I'll leave the image creation in just so that if you run the
> test under different formats it'll actually do something vaguely
> different, just in case.

hm, but you said, supported_fmts=['qcow2']?

I'm ok with leaving image creation too, anyway.

> 
> Actually, it did highlight how weird VPC is again and I changed the rest
> as a result to accommodate image formats that round their disk sizes.
> 
>>
>>> +    vm.add_drive(img_path)
>>> +    vm.launch()
>>> +
>>> +    log('--- Adding preliminary bitmaps A & B ---\n')
>>> +    vm.qmp_log("block-dirty-bitmap-add", node="drive0", name="bitmapA")
>>> +    vm.qmp_log("block-dirty-bitmap-add", node="drive0", name="bitmapB")
>>> +
>>> +    # Dirties 4 clusters. count=262144
>>> +    log('')
>>> +    log('--- Emulating writes ---\n')
>>> +    for p in patterns:
>>> +        cmd = "write -P%s %s %s" % p
>>> +        log(cmd)
>>> +        log(vm.hmp_qemu_io("drive0", cmd))
>>> +
>>> +    log(json.dumps(query_bitmaps(vm), indent=2, separators=(',', ': ')))
>>
>> log can do json.dumps, so it's strange to do it here, and I don't like ',' separator, as I described
> 
> Because it tries to filter the rich object before it converts, as you've
> noticed. I think I'll take a page from your book and try to fix log() to
> be better.
> 
> As for not liking the ',' separator, it should be identical to your
> approach because it only removes the space when pretty printing is
> enabled. Can you show me what this approach does that you find to be
> ugly and how it's different from your regex approach?

I was wrong, sorry)

But, on the other hand, right-stripping lines in log() is a good thing anyway. Or, may be even better (as we
don't have such tests yet), just error out if any trailing whitespaces found (and use correct separators for
json.dumps)

> 
> --js
> 


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH v3 7/7] iotests: add iotest 236 for testing bitmap merge

[Vladimir Sementsov-Ogievskiy]

15.12.2018 2:15, John Snow wrote:
> New interface, new smoke test.

don't forget to add s-o-b here and in previous patch :)


Feel, that I do too much nitpicking.
Actually test is good and works for me,
so, with at least
qemu_img_pipe -> qemu_img or qemu_img_create
and added newline at the end of group file, as git recommends:

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH v3 0/7] bitmaps: remove x- prefix from QMP api

[John Snow]

On 12/17/18 4:14 PM, John Snow wrote:
> 
> 
> On 12/14/18 6:15 PM, John Snow wrote:
>> Touch up a few last things and remove the x- prefix.
>>
>> V3:
>>  - Reworked qmp_log to pretty-print the outgoing command, too [Vladimir]
>>  - Modified test to log only bitmap information [Vladimir]
>>  - Test disable/enable transaction toggle [Eric]
>>
>> Note that the filter I added is now unused, but I think we will want it
>> and it's small enough, so I'm going to check it in anyway. If you disagree,
>> I'll just drop the patch instead.
>>
>> --js
>>
>> John Snow (7):
>>   blockdev: abort transactions in reverse order
>>   blockdev: n-ary bitmap merge
>>   block: remove 'x' prefix from experimental bitmap APIs
>>   iotests.py: don't abort if IMGKEYSECRET is undefined
>>   iotests: add filter_generated_node_ids
>>   iotests: allow pretty-print for qmp_log
>>   iotests: add iotest 236 for testing bitmap merge
>>
>>  blockdev.c                    |  96 +++++++++-------
>>  qapi/block-core.json          |  56 +++++-----
>>  qapi/transaction.json         |  12 +-
>>  tests/qemu-iotests/223        |   4 +-
>>  tests/qemu-iotests/236        | 124 +++++++++++++++++++++
>>  tests/qemu-iotests/236.out    | 200 ++++++++++++++++++++++++++++++++++
>>  tests/qemu-iotests/group      |   1 +
>>  tests/qemu-iotests/iotests.py |  22 +++-
>>  8 files changed, 436 insertions(+), 79 deletions(-)
>>  create mode 100755 tests/qemu-iotests/236
>>  create mode 100644 tests/qemu-iotests/236.out
>>
> 
> Thanks, I'm staging patches 1-5 and I'll send the PR once we get to the
> bottom of patches 6 and 7, just to keep volume on the list down.
> 

NACK.

Patch 2 is incomplete an additional bugfix is needed.

--js

Re: [Qemu-devel] [PATCH v3 0/7] bitmaps: remove x- prefix from QMP api

[no-reply]

Patchew URL: https://patchew.org/QEMU/20181214231512.5295-1-jsnow@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20181214231512.5295-1-jsnow@redhat.com
Type: series
Subject: [Qemu-devel] [PATCH v3 0/7] bitmaps: remove x- prefix from QMP api

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
24334c9 iotests: add iotest 236 for testing bitmap merge
4a6a51f iotests: allow pretty-print for qmp_log
196c922 iotests: add filter_generated_node_ids
eae2953 iotests.py: don't abort if IMGKEYSECRET is undefined
6aaaafa block: remove 'x' prefix from experimental bitmap APIs
14c3ee1 blockdev: n-ary bitmap merge
24d5528 blockdev: abort transactions in reverse order

=== OUTPUT BEGIN ===
Checking PATCH 1/7: blockdev: abort transactions in reverse order...
Checking PATCH 2/7: blockdev: n-ary bitmap merge...
ERROR: externs should be avoided in .c files
#28: FILE: blockdev.c:2125:
+void do_block_dirty_bitmap_merge(const char *node, const char *target,

total: 1 errors, 0 warnings, 147 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 3/7: block: remove 'x' prefix from experimental bitmap APIs...
Checking PATCH 4/7: iotests.py: don't abort if IMGKEYSECRET is undefined...
Checking PATCH 5/7: iotests: add filter_generated_node_ids...
Checking PATCH 6/7: iotests: allow pretty-print for qmp_log...
Checking PATCH 7/7: iotests: add iotest 236 for testing bitmap merge...
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20181214231512.5295-1-jsnow@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com