Series comparison

-[PULL 00/13] Block layer patches
+[PULL v2 0/4] Block layer patches
-The following changes since commit ba29883206d92a29ad5a466e679ccfc2ee6132ef:
+The following changes since commit 7f21573c822805a8e6be379d9bcf3ad9effef3dc:
-  Merge remote-tracking branch 'remotes/borntraeger/tags/s390x-20200310' into staging (2020-03-10 16:50:28 +0000)
+  Merge remote-tracking branch 'remotes/huth-gitlab/tags/pull-request-2019-10-01' into staging (2019-10-01 13:13:38 +0100)
 are available in the Git repository at:
   git://repo.or.cz/qemu/kevin.git tags/for-upstream
-for you to fetch changes up to 8bb3b023f2055054ee119cb45b42d2b14be7fc8a:
+for you to fetch changes up to 352492e5541e293ea8117412ffd171ce1c7ac15d:
-  qemu-iotests: adding LUKS cleanup for non-UTF8 secret error (2020-03-11 15:54:38 +0100)
+  iotests: Remove Python 2 compatibility code (2019-10-02 16:47:23 +0200)
 ----------------------------------------------------------------
 Block layer patches:
-- Relax restrictions for blockdev-snapshot (allows libvirt to do live
+- Fix internal snapshots with typical -blockdev setups
-  storage migration with blockdev-mirror)
+- iotests: Require Python 3.6 or later
 - luks: Delete created files when block_crypto_co_create_opts_luks fails
 - Fix memleaks in qmp_object_add
 ----------------------------------------------------------------
-Daniel Henrique Barboza (4):
+Kevin Wolf (4):
-      block: introducing 'bdrv_co_delete_file' interface
+      block/snapshot: Restrict set of snapshot nodes
-      block.c: adding bdrv_co_delete_file
+      iotests: Test internal snapshots with -blockdev
-      crypto.c: cleanup created file when block_crypto_co_create_opts_luks fails
+      iotests: Require Python 3.6 or later
-      qemu-iotests: adding LUKS cleanup for non-UTF8 secret error
+      iotests: Remove Python 2 compatibility code
-Kevin Wolf (6):
+ block/snapshot.c                 |  26 ++++--
-      block: Make bdrv_get_cumulative_perm() public
+ tests/qemu-iotests/044           |   3 -
-      block: Relax restrictions for blockdev-snapshot
+ tests/qemu-iotests/163           |   3 -
-      iotests: Fix run_job() with use_log=False
+ tests/qemu-iotests/267           | 168 ++++++++++++++++++++++++++++++++++++
-      iotests: Test mirror with temporarily disabled target backing file
+ tests/qemu-iotests/267.out       | 182 +++++++++++++++++++++++++++++++++++++++
-      block: Fix cross-AioContext blockdev-snapshot
+ tests/qemu-iotests/check         |  13 ++-
-      iotests: Add iothread cases to 155
+ tests/qemu-iotests/common.filter |  11 ++-
  tests/qemu-iotests/group         |   1 +
  tests/qemu-iotests/iotests.py    |  13 +--
 files changed, 392 insertions(+), 28 deletions(-)
  create mode 100755 tests/qemu-iotests/267
  create mode 100644 tests/qemu-iotests/267.out
-Pan Nengyuan (1):
-      qom-qmp-cmds: fix two memleaks in qmp_object_add
-Peter Krempa (1):
-      qapi: Add '@allow-write-only-overlay' feature for 'blockdev-snapshot'
-Philippe Mathieu-Daudé (1):
-      tests/qemu-iotests: Fix socket_scm_helper build path
- qapi/block-core.json          |  9 ++++-
- include/block/block.h         |  1 +
- include/block/block_int.h     |  7 ++++
- block.c                       | 33 ++++++++++++++--
- block/crypto.c                | 18 +++++++++
- block/file-posix.c            | 23 +++++++++++
- blockdev.c                    | 30 ++++-----------
- qom/qom-qmp-cmds.c            | 16 +++-----
- tests/qemu-iotests/iotests.py |  5 ++-
- tests/Makefile.include        |  1 +
- tests/qemu-iotests/085.out    |  4 +-
- tests/qemu-iotests/155        | 88 ++++++++++++++++++++++++++++++++++++-------
- tests/qemu-iotests/155.out    |  4 +-
- tests/qemu-iotests/282        | 67 ++++++++++++++++++++++++++++++++
- tests/qemu-iotests/282.out    | 11 ++++++
- tests/qemu-iotests/group      |  1 +
- tests/qtest/Makefile.include  |  1 -
-files changed, 262 insertions(+), 57 deletions(-)
- create mode 100755 tests/qemu-iotests/282
- create mode 100644 tests/qemu-iotests/282.out

-[PULL 01/13] qom-qmp-cmds: fix two memleaks in qmp_object_add
+Deleted patch
-From: Pan Nengyuan <pannengyuan@huawei.com>
-'type/id' forgot to free in qmp_object_add, this patch fix that.
-The leak stack:
-Direct leak of 84 byte(s) in 6 object(s) allocated from:
-    #0 0x7fe2a5ebf768 in __interceptor_malloc (/lib64/libasan.so.5+0xef768)
-    #1 0x7fe2a5044445 in g_malloc (/lib64/libglib-2.0.so.0+0x52445)
-    #2 0x7fe2a505dd92 in g_strdup (/lib64/libglib-2.0.so.0+0x6bd92)
-    #3 0x56344954e692 in qmp_object_add /mnt/sdb/qemu-new/qemu_test/qemu/qom/qom-qmp-cmds.c:258
-    #4 0x563449960f5a in do_qmp_dispatch /mnt/sdb/qemu-new/qemu_test/qemu/qapi/qmp-dispatch.c:132
-    #5 0x563449960f5a in qmp_dispatch /mnt/sdb/qemu-new/qemu_test/qemu/qapi/qmp-dispatch.c:175
-    #6 0x563449498a30 in monitor_qmp_dispatch /mnt/sdb/qemu-new/qemu_test/qemu/monitor/qmp.c:145
-    #7 0x56344949a64f in monitor_qmp_bh_dispatcher /mnt/sdb/qemu-new/qemu_test/qemu/monitor/qmp.c:234
-    #8 0x563449a92a3a in aio_bh_call /mnt/sdb/qemu-new/qemu_test/qemu/util/async.c:136
-Direct leak of 54 byte(s) in 6 object(s) allocated from:
-    #0 0x7fe2a5ebf768 in __interceptor_malloc (/lib64/libasan.so.5+0xef768)
-    #1 0x7fe2a5044445 in g_malloc (/lib64/libglib-2.0.so.0+0x52445)
-    #2 0x7fe2a505dd92 in g_strdup (/lib64/libglib-2.0.so.0+0x6bd92)
-    #3 0x56344954e6c4 in qmp_object_add /mnt/sdb/qemu-new/qemu_test/qemu/qom/qom-qmp-cmds.c:267
-    #4 0x563449960f5a in do_qmp_dispatch /mnt/sdb/qemu-new/qemu_test/qemu/qapi/qmp-dispatch.c:132
-    #5 0x563449960f5a in qmp_dispatch /mnt/sdb/qemu-new/qemu_test/qemu/qapi/qmp-dispatch.c:175
-    #6 0x563449498a30 in monitor_qmp_dispatch /mnt/sdb/qemu-new/qemu_test/qemu/monitor/qmp.c:145
-    #7 0x56344949a64f in monitor_qmp_bh_dispatcher /mnt/sdb/qemu-new/qemu_test/qemu/monitor/qmp.c:234
-    #8 0x563449a92a3a in aio_bh_call /mnt/sdb/qemu-new/qemu_test/qemu/util/async.c:136
-Fixes: 5f07c4d60d091320186e7b0edaf9ed2cc16b2d1e
-Reported-by: Euler Robot <euler.robot@huawei.com>
-Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
-Message-Id: <20200310064640.5059-1-pannengyuan@huawei.com>
-Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
-Acked-by: Igor Mammedov <imammedo@redhat.com>
-Signed-off-by: Kevin Wolf <kwolf@redhat.com>
----
- qom/qom-qmp-cmds.c | 16 ++++++----------
-file changed, 6 insertions(+), 10 deletions(-)
-diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
-index XXXXXXX..XXXXXXX 100644
---- a/qom/qom-qmp-cmds.c
-+++ b/qom/qom-qmp-cmds.c
-@@ -XXX,XX +XXX,XX @@ void qmp_object_add(QDict *qdict, QObject **ret_data, Error **errp)
-     QDict *pdict;
-     Visitor *v;
-     Object *obj;
--    const char *type;
--    const char *id;
-+    g_autofree char *type = NULL;
-+    g_autofree char *id = NULL;
--    type = qdict_get_try_str(qdict, "qom-type");
-+    type = g_strdup(qdict_get_try_str(qdict, "qom-type"));
-     if (!type) {
-         error_setg(errp, QERR_MISSING_PARAMETER, "qom-type");
-         return;
--    } else {
--        type = g_strdup(type);
--        qdict_del(qdict, "qom-type");
-     }
-+    qdict_del(qdict, "qom-type");
--    id = qdict_get_try_str(qdict, "id");
-+    id = g_strdup(qdict_get_try_str(qdict, "id"));
-     if (!id) {
-         error_setg(errp, QERR_MISSING_PARAMETER, "id");
-         return;
--    } else {
--        id = g_strdup(id);
--        qdict_del(qdict, "id");
-     }
-+    qdict_del(qdict, "id");
-     props = qdict_get(qdict, "props");
-     if (props) {
---
-.20.1

-[PULL 02/13] block: Make bdrv_get_cumulative_perm() public
+Deleted patch
-Signed-off-by: Kevin Wolf <kwolf@redhat.com>
-Message-Id: <20200310113831.27293-2-kwolf@redhat.com>
-Reviewed-by: Peter Krempa <pkrempa@redhat.com>
-Signed-off-by: Kevin Wolf <kwolf@redhat.com>
----
- include/block/block_int.h | 3 +++
- block.c                   | 6 ++----
-files changed, 5 insertions(+), 4 deletions(-)
-diff --git a/include/block/block_int.h b/include/block/block_int.h
-index XXXXXXX..XXXXXXX 100644
---- a/include/block/block_int.h
-+++ b/include/block/block_int.h
-@@ -XXX,XX +XXX,XX @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
-                                   void *opaque, Error **errp);
- void bdrv_root_unref_child(BdrvChild *child);
-+void bdrv_get_cumulative_perm(BlockDriverState *bs, uint64_t *perm,
-+                              uint64_t *shared_perm);
-+
- /**
-  * Sets a BdrvChild's permissions.  Avoid if the parent is a BDS; use
-  * bdrv_child_refresh_perms() instead and make the parent's
-diff --git a/block.c b/block.c
-index XXXXXXX..XXXXXXX 100644
---- a/block.c
-+++ b/block.c
-@@ -XXX,XX +XXX,XX @@ static int bdrv_child_check_perm(BdrvChild *c, BlockReopenQueue *q,
-                                  bool *tighten_restrictions, Error **errp);
- static void bdrv_child_abort_perm_update(BdrvChild *c);
- static void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared);
--static void bdrv_get_cumulative_perm(BlockDriverState *bs, uint64_t *perm,
--                                     uint64_t *shared_perm);
- typedef struct BlockReopenQueueEntry {
-      bool prepared;
-@@ -XXX,XX +XXX,XX @@ static void bdrv_set_perm(BlockDriverState *bs, uint64_t cumulative_perms,
-     }
- }
--static void bdrv_get_cumulative_perm(BlockDriverState *bs, uint64_t *perm,
--                                     uint64_t *shared_perm)
-+void bdrv_get_cumulative_perm(BlockDriverState *bs, uint64_t *perm,
-+                              uint64_t *shared_perm)
- {
-     BdrvChild *c;
-     uint64_t cumulative_perms = 0;
---
-.20.1

-[PULL 03/13] block: Relax restrictions for blockdev-snapshot
+Deleted patch
-blockdev-snapshot returned an error if the overlay was already in use,
-which it defined as having any BlockBackend parent. This is in fact both
-too strict (some parents can tolerate the change of visible data caused
-by attaching a backing file) and too loose (some non-BlockBackend
-parents may not be happy with it).
-One important use case that is prevented by the too strict check is live
-storage migration with blockdev-mirror. Here, the target node is
-usually opened without a backing file so that the active layer is
-mirrored while its backing chain can be copied in the background.
-The backing chain should be attached to the mirror target node when
-finalising the job, just before switching the users of the source node
-to the new copy (at which point the mirror job still has a reference to
-the node). drive-mirror did this automatically, but with blockdev-mirror
-this is the job of the QMP client, so it needs a way to do this.
-blockdev-snapshot is the obvious way, so this patch makes it work in
-this scenario. The new condition is that no parent uses CONSISTENT_READ
-permissions. This will ensure that the operation will still be blocked
-when the node is attached to the guest device, so blockdev-snapshot
-remains safe.
-(For the sake of completeness, x-blockdev-reopen can be used to achieve
-the same, however it is a big hammer, performs the graph change
-completely unchecked and is still experimental. So even with the option
-of using x-blockdev-reopen, there are reasons why blockdev-snapshot
-should be able to perform this operation.)
-Signed-off-by: Kevin Wolf <kwolf@redhat.com>
-Message-Id: <20200310113831.27293-3-kwolf@redhat.com>
-Reviewed-by: Peter Krempa <pkrempa@redhat.com>
-Tested-by: Peter Krempa <pkrempa@redhat.com>
-Signed-off-by: Kevin Wolf <kwolf@redhat.com>
----
- blockdev.c                 | 14 ++++++++------
- tests/qemu-iotests/085.out |  4 ++--
-files changed, 10 insertions(+), 8 deletions(-)
-diff --git a/blockdev.c b/blockdev.c
-index XXXXXXX..XXXXXXX 100644
---- a/blockdev.c
-+++ b/blockdev.c
-@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
-     TransactionAction *action = common->action;
-     AioContext *aio_context;
-     AioContext *old_context;
-+    uint64_t perm, shared;
-     int ret;
-     /* 'blockdev-snapshot' and 'blockdev-snapshot-sync' have similar
-@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
-         goto out;
-     }
--    if (bdrv_has_blk(state->new_bs)) {
-+    /*
-+     * Allow attaching a backing file to an overlay that's already in use only
-+     * if the parents don't assume that they are already seeing a valid image.
-+     * (Specifically, allow it as a mirror target, which is write-only access.)
-+     */
-+    bdrv_get_cumulative_perm(state->new_bs, &perm, &shared);
-+    if (perm & BLK_PERM_CONSISTENT_READ) {
-         error_setg(errp, "The overlay is already in use");
-         goto out;
-     }
--    if (bdrv_op_is_blocked(state->new_bs, BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT,
--                           errp)) {
--        goto out;
--    }
--
-     if (state->new_bs->backing != NULL) {
-         error_setg(errp, "The overlay already has a backing image");
-         goto out;
-diff --git a/tests/qemu-iotests/085.out b/tests/qemu-iotests/085.out
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/085.out
-+++ b/tests/qemu-iotests/085.out
-@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/12-snapshot-v0.IMGFMT', fmt=IMGFMT size=134217728 backing_f
- === Invalid command - cannot create a snapshot using a file BDS ===
- { 'execute': 'blockdev-snapshot', 'arguments': { 'node':'virtio0', 'overlay':'file_12' } }
--{"error": {"class": "GenericError", "desc": "The overlay does not support backing images"}}
-+{"error": {"class": "GenericError", "desc": "The overlay is already in use"}}
- === Invalid command - snapshot node used as active layer ===
-@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/12-snapshot-v0.IMGFMT', fmt=IMGFMT size=134217728 backing_f
- === Invalid command - snapshot node used as backing hd ===
- { 'execute': 'blockdev-snapshot', 'arguments': { 'node': 'virtio0', 'overlay':'snap_11' } }
--{"error": {"class": "GenericError", "desc": "Node 'snap_11' is busy: node is used as backing hd of 'snap_12'"}}
-+{"error": {"class": "GenericError", "desc": "The overlay is already in use"}}
- === Invalid command - snapshot node has a backing image ===
---
-.20.1

-[PULL 11/13] block.c: adding bdrv_co_delete_file
+[PULL v2 1/4] block/snapshot: Restrict set of snapshot nodes
-From: Daniel Henrique Barboza <danielhb413@gmail.com>
+Nodes involved in internal snapshots were those that were returned by
 bdrv_next(), inserted and not read-only. bdrv_next() in turn returns all
 nodes that are either the root node of a BlockBackend or monitor-owned
 nodes.
-Using the new 'bdrv_co_delete_file' interface, a pure co_routine function
+With the typical -drive use, this worked well enough. However, in the
-'bdrv_co_delete_file' inside block.c can can be used in a way similar of
+typical -blockdev case, the user defines one node per option, making all
-the existing bdrv_create_file to to clean up a created file.
+nodes monitor-owned nodes. This includes protocol nodes etc. which often
 are not snapshottable, so "savevm" only returns an error.
-We're creating a pure co_routine because the only caller of
+Change the conditions so that internal snapshot still include all nodes
-'bdrv_co_delete_file' will be already in co_routine context, thus there
+that have a BlockBackend attached (we definitely want to snapshot
-is no need to add all the machinery to check for qemu_in_coroutine() and
+anything attached to a guest device and probably also the built-in NBD
-create a separated co_routine to do the job.
+server; snapshotting block job BlockBackends is more of an accident, but
 a preexisting one), but other monitor-owned nodes are only included if
 they have no parents.
-Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
+This makes internal snapshots usable again with typical -blockdev
-Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
+configurations.
-Message-Id: <20200130213907.2830642-3-danielhb413@gmail.com>
 Cc: qemu-stable@nongnu.org
 Signed-off-by: Kevin Wolf <kwolf@redhat.com>
+Reviewed-by: Eric Blake <eblake@redhat.com>
+Reviewed-by: Peter Krempa <pkrempa@redhat.com>
+Tested-by: Peter Krempa <pkrempa@redhat.com>
 ---
- include/block/block.h |  1 +
+ block/snapshot.c | 26 +++++++++++++++++++-------
- block.c               | 26 ++++++++++++++++++++++++++
+file changed, 19 insertions(+), 7 deletions(-)
 files changed, 27 insertions(+)
-diff --git a/include/block/block.h b/include/block/block.h
+diff --git a/block/snapshot.c b/block/snapshot.c
 index XXXXXXX..XXXXXXX 100644
---- a/include/block/block.h
+--- a/block/snapshot.c
-+++ b/include/block/block.h
++++ b/block/snapshot.c
-@@ -XXX,XX +XXX,XX @@ bool bdrv_is_backing_chain_frozen(BlockDriverState *bs, BlockDriverState *base,
+@@ -XXX,XX +XXX,XX @@
- int bdrv_freeze_backing_chain(BlockDriverState *bs, BlockDriverState *base,
+ #include "qapi/qmp/qerror.h"
-                               Error **errp);
+ #include "qapi/qmp/qstring.h"
- void bdrv_unfreeze_backing_chain(BlockDriverState *bs, BlockDriverState *base);
+ #include "qemu/option.h"
-+int coroutine_fn bdrv_co_delete_file(BlockDriverState *bs, Error **errp);
++#include "sysemu/block-backend.h"
+ QemuOptsList internal_snapshot_opts = {
- typedef struct BdrvCheckResult {
+     .name = "snapshot",
-diff --git a/block.c b/block.c
+@@ -XXX,XX +XXX,XX @@ int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState *bs,
-index XXXXXXX..XXXXXXX 100644
+     return ret;
 --- a/block.c
 +++ b/block.c
@@ -XXX,XX +XXX,XX @@ int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp)
      }
  }
-+int coroutine_fn bdrv_co_delete_file(BlockDriverState *bs, Error **errp)
++static bool bdrv_all_snapshots_includes_bs(BlockDriverState *bs)
 +{
-+    Error *local_err = NULL;
++    if (!bdrv_is_inserted(bs) || bdrv_is_read_only(bs)) {
-+    int ret;
++        return false;
 +
 +    assert(bs != NULL);
 +
 +    if (!bs->drv) {
 +        error_setg(errp, "Block node '%s' is not opened", bs->filename);
 +        return -ENOMEDIUM;
 +    }
 +
-+    if (!bs->drv->bdrv_co_delete_file) {
++    /* Include all nodes that are either in use by a BlockBackend, or that
-+        error_setg(errp, "Driver '%s' does not support image deletion",
++     * aren't attached to any node, but owned by the monitor. */
-+                   bs->drv->format_name);
++    return bdrv_has_blk(bs) || QLIST_EMPTY(&bs->parents);
 +        return -ENOTSUP;
 +    }
 +
 +    ret = bs->drv->bdrv_co_delete_file(bs, &local_err);
 +    if (ret < 0) {
 +        error_propagate(errp, local_err);
 +    }
 +
 +    return ret;
 +}
-+
- /**
+ /* Group operations. All block drivers are involved.
-  * Try to get @bs's logical and physical block size.
+  * These functions will properly handle dataplane (take aio_context_acquire
-  * On success, store them in @bsz struct and return 0.
+@@ -XXX,XX +XXX,XX @@ bool bdrv_all_can_snapshot(BlockDriverState **first_bad_bs)
          AioContext *ctx = bdrv_get_aio_context(bs);
          aio_context_acquire(ctx);
 -        if (bdrv_is_inserted(bs) && !bdrv_is_read_only(bs)) {
 +        if (bdrv_all_snapshots_includes_bs(bs)) {
              ok = bdrv_can_snapshot(bs);
          }
          aio_context_release(ctx);
@@ -XXX,XX +XXX,XX @@ int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bad_bs,
          AioContext *ctx = bdrv_get_aio_context(bs);
          aio_context_acquire(ctx);
 -        if (bdrv_can_snapshot(bs) &&
 -                bdrv_snapshot_find(bs, snapshot, name) >= 0) {
 +        if (bdrv_all_snapshots_includes_bs(bs) &&
 +            bdrv_snapshot_find(bs, snapshot, name) >= 0)
 +        {
              ret = bdrv_snapshot_delete(bs, snapshot->id_str,
                                         snapshot->name, err);
          }
@@ -XXX,XX +XXX,XX @@ int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs,
          AioContext *ctx = bdrv_get_aio_context(bs);
          aio_context_acquire(ctx);
 -        if (bdrv_can_snapshot(bs)) {
 +        if (bdrv_all_snapshots_includes_bs(bs)) {
              ret = bdrv_snapshot_goto(bs, name, errp);
          }
          aio_context_release(ctx);
@@ -XXX,XX +XXX,XX @@ int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs)
          AioContext *ctx = bdrv_get_aio_context(bs);
          aio_context_acquire(ctx);
 -        if (bdrv_can_snapshot(bs)) {
 +        if (bdrv_all_snapshots_includes_bs(bs)) {
              err = bdrv_snapshot_find(bs, &sn, name);
          }
          aio_context_release(ctx);
@@ -XXX,XX +XXX,XX @@ int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn,
          if (bs == vm_state_bs) {
              sn->vm_state_size = vm_state_size;
              err = bdrv_snapshot_create(bs, sn);
 -        } else if (bdrv_can_snapshot(bs)) {
 +        } else if (bdrv_all_snapshots_includes_bs(bs)) {
              sn->vm_state_size = 0;
              err = bdrv_snapshot_create(bs, sn);
          }
@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_all_find_vmstate_bs(void)
          bool found;
          aio_context_acquire(ctx);
 -        found = bdrv_can_snapshot(bs);
 +        found = bdrv_all_snapshots_includes_bs(bs) && bdrv_can_snapshot(bs);
          aio_context_release(ctx);
          if (found) {
 --
 .20.1

-[PULL 13/13] qemu-iotests: adding LUKS cleanup for non-UTF8 secret error
+[PULL v2 2/4] iotests: Test internal snapshots with -blockdev
-From: Daniel Henrique Barboza <danielhb413@gmail.com>
+Signed-off-by: Kevin Wolf <kwolf@redhat.com>
 Reviewed-by: Peter Krempa <pkrempa@redhat.com>
 Tested-by: Peter Krempa <pkrempa@redhat.com>
 ---
  tests/qemu-iotests/267           | 168 ++++++++++++++++++++++++++++
  tests/qemu-iotests/267.out       | 182 +++++++++++++++++++++++++++++++
  tests/qemu-iotests/common.filter |  11 +-
  tests/qemu-iotests/group         |   1 +
 files changed, 358 insertions(+), 4 deletions(-)
  create mode 100755 tests/qemu-iotests/267
  create mode 100644 tests/qemu-iotests/267.out
-This patch adds a new test file to exercise the case where
+diff --git a/tests/qemu-iotests/267 b/tests/qemu-iotests/267
 qemu-img fails to complete for the LUKS format when a non-UTF8
 secret is used.
 Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
 Message-Id: <20200130213907.2830642-5-danielhb413@gmail.com>
 Signed-off-by: Kevin Wolf <kwolf@redhat.com>
 ---
  tests/qemu-iotests/282     | 67 ++++++++++++++++++++++++++++++++++++++
  tests/qemu-iotests/282.out | 11 +++++++
  tests/qemu-iotests/group   |  1 +
 files changed, 79 insertions(+)
  create mode 100755 tests/qemu-iotests/282
  create mode 100644 tests/qemu-iotests/282.out
 diff --git a/tests/qemu-iotests/282 b/tests/qemu-iotests/282
 new file mode 100755
 index XXXXXXX..XXXXXXX
 --- /dev/null
-+++ b/tests/qemu-iotests/282
++++ b/tests/qemu-iotests/267
 @@ -XXX,XX +XXX,XX @@
 +#!/usr/bin/env bash
 +#
-+# Test qemu-img file cleanup for LUKS when using a non-UTF8 secret
++# Test which nodes are involved in internal snapshots
 +#
-+# Copyright (C) 2020, IBM Corporation.
++# Copyright (C) 2019 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.
 ...
 +#
 +# You should have received a copy of the GNU General Public License
 +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
 +#
 +
++# creator
++owner=kwolf@redhat.com
++
 +seq=`basename $0`
 +echo "QA output created by $seq"
 +
 +status=1    # failure is the default!
-+TEST_IMAGE_FILE='vol.img'
 +
 +_cleanup()
 +{
-+  _cleanup_test_img
++    _cleanup_test_img
-+  rm non_utf8_secret
++    rm -f "$TEST_DIR/nbd"
 +  rm -f $TEST_IMAGE_FILE
 +}
 +trap "_cleanup; exit \$status" 0 1 2 3 15
 +
 +# get standard environment, filters and checks
 +. ./common.rc
 +. ./common.filter
 +
-+_supported_fmt luks
++_supported_fmt qcow2
-+_supported_proto generic
++_supported_proto file
-+_unsupported_proto vxhs
++_supported_os Linux
 +
-+echo "== Create non-UTF8 secret =="
++# Internal snapshots are (currently) impossible with refcount_bits=1
-+echo -n -e '\x3a\x3c\x3b\xff' > non_utf8_secret
++_unsupported_imgopts 'refcount_bits=1[^0-9]'
-+SECRET="secret,id=sec0,file=non_utf8_secret"
++
-+
++do_run_qemu()
-+echo "== Throws an error because of invalid UTF-8 secret =="
++{
-+$QEMU_IMG create -f $IMGFMT --object $SECRET -o "key-secret=sec0" $TEST_IMAGE_FILE 4M
++    echo Testing: "$@"
-+
++    (
-+echo "== Image file should not exist after the error =="
++        if ! test -t 0; then
-+if test -f "$TEST_IMAGE_FILE"; then
++            while read cmd; do
-+    exit 1
++                echo $cmd
-+fi
++            done
-+
++        fi
-+echo "== Create a stub image file and run qemu-img again =="
++        echo quit
-+touch $TEST_IMAGE_FILE
++    ) | $QEMU -nographic -monitor stdio -nodefaults "$@"
-+$QEMU_IMG create -f $IMGFMT --object $SECRET -o "key-secret=sec0" $TEST_IMAGE_FILE 4M
++    echo
-+
++}
-+echo "== Pre-existing image file should also be deleted after the error =="
++
-+if test -f "$TEST_IMAGE_FILE"; then
++run_qemu()
-+    exit 1
++{
-+fi
++    do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qemu | _filter_hmp |
 +        _filter_generated_node_ids | _filter_imgfmt | _filter_vmstate_size
 +}
 +
 +size=128M
 +
 +run_test()
 +{
 +    _make_test_img $size
 +    printf "savevm snap0\ninfo snapshots\nloadvm snap0\n" | run_qemu "$@" | _filter_date
 +}
 +
 +
 +echo
 +echo "=== No block devices at all ==="
 +echo
 +
 +run_test
 +
 +echo
 +echo "=== -drive if=none ==="
 +echo
 +
 +run_test -drive driver=file,file="$TEST_IMG",if=none
 +run_test -drive driver=$IMGFMT,file="$TEST_IMG",if=none
 +run_test -drive driver=$IMGFMT,file="$TEST_IMG",if=none -device virtio-blk,drive=none0
 +
 +echo
 +echo "=== -drive if=virtio ==="
 +echo
 +
 +run_test -drive driver=file,file="$TEST_IMG",if=virtio
 +run_test -drive driver=$IMGFMT,file="$TEST_IMG",if=virtio
 +
 +echo
 +echo "=== Simple -blockdev ==="
 +echo
 +
 +run_test -blockdev driver=file,filename="$TEST_IMG",node-name=file
 +run_test -blockdev driver=file,filename="$TEST_IMG",node-name=file \
 +         -blockdev driver=$IMGFMT,file=file,node-name=fmt
 +run_test -blockdev driver=file,filename="$TEST_IMG",node-name=file \
 +         -blockdev driver=raw,file=file,node-name=raw \
 +         -blockdev driver=$IMGFMT,file=raw,node-name=fmt
 +
 +echo
 +echo "=== -blockdev with a filter on top ==="
 +echo
 +
 +run_test -blockdev driver=file,filename="$TEST_IMG",node-name=file \
 +         -blockdev driver=$IMGFMT,file=file,node-name=fmt \
 +         -blockdev driver=copy-on-read,file=fmt,node-name=filter
 +
 +echo
 +echo "=== -blockdev with a backing file ==="
 +echo
 +
 +TEST_IMG="$TEST_IMG.base" _make_test_img $size
 +
 +IMGOPTS="backing_file=$TEST_IMG.base" \
 +run_test -blockdev driver=file,filename="$TEST_IMG.base",node-name=backing-file \
 +         -blockdev driver=file,filename="$TEST_IMG",node-name=file \
 +         -blockdev driver=$IMGFMT,file=file,backing=backing-file,node-name=fmt
 +
 +IMGOPTS="backing_file=$TEST_IMG.base" \
 +run_test -blockdev driver=file,filename="$TEST_IMG.base",node-name=backing-file \
 +         -blockdev driver=$IMGFMT,file=backing-file,node-name=backing-fmt \
 +         -blockdev driver=file,filename="$TEST_IMG",node-name=file \
 +         -blockdev driver=$IMGFMT,file=file,backing=backing-fmt,node-name=fmt
 +
 +# A snapshot should be present on the overlay, but not the backing file
 +echo Internal snapshots on overlay:
 +$QEMU_IMG snapshot -l "$TEST_IMG" | _filter_date | _filter_vmstate_size
 +
 +echo Internal snapshots on backing file:
 +$QEMU_IMG snapshot -l "$TEST_IMG.base" | _filter_date | _filter_vmstate_size
 +
 +echo
 +echo "=== -blockdev with NBD server on the backing file ==="
 +echo
 +
 +IMGOPTS="backing_file=$TEST_IMG.base" _make_test_img $size
 +cat <<EOF |
 +nbd_server_start unix:$TEST_DIR/nbd
 +nbd_server_add -w backing-fmt
 +savevm snap0
 +info snapshots
 +loadvm snap0
 +EOF
 +run_qemu -blockdev driver=file,filename="$TEST_IMG.base",node-name=backing-file \
 +         -blockdev driver=$IMGFMT,file=backing-file,node-name=backing-fmt \
 +         -blockdev driver=file,filename="$TEST_IMG",node-name=file \
 +         -blockdev driver=$IMGFMT,file=file,backing=backing-fmt,node-name=fmt |
 +         _filter_date
 +
 +# This time, a snapshot should be created on both files
 +echo Internal snapshots on overlay:
 +$QEMU_IMG snapshot -l "$TEST_IMG" | _filter_date | _filter_vmstate_size
 +
 +echo Internal snapshots on backing file:
 +$QEMU_IMG snapshot -l "$TEST_IMG.base" | _filter_date | _filter_vmstate_size
 +
 +# success, all done
 +echo "*** done"
 +rm -f $seq.full
 +status=0
-diff --git a/tests/qemu-iotests/282.out b/tests/qemu-iotests/282.out
+diff --git a/tests/qemu-iotests/267.out b/tests/qemu-iotests/267.out
 new file mode 100644
 index XXXXXXX..XXXXXXX
 --- /dev/null
-+++ b/tests/qemu-iotests/282.out
++++ b/tests/qemu-iotests/267.out
 @@ -XXX,XX +XXX,XX @@
-+QA output created by 282
++QA output created by 267
-+== Create non-UTF8 secret ==
++
-+== Throws an error because of invalid UTF-8 secret ==
++=== No block devices at all ===
-+qemu-img: vol.img: Data from secret sec0 is not valid UTF-8
++
-+Formatting 'vol.img', fmt=luks size=4194304 key-secret=sec0
++Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
-+== Image file should not exist after the error ==
++Testing:
-+== Create a stub image file and run qemu-img again ==
++QEMU X.Y.Z monitor - type 'help' for more information
-+qemu-img: vol.img: Data from secret sec0 is not valid UTF-8
++(qemu) savevm snap0
-+Formatting 'vol.img', fmt=luks size=4194304 key-secret=sec0
++Error: No block device can accept snapshots
-+== Pre-existing image file should also be deleted after the error ==
++(qemu) info snapshots
-+ *** done
++No available block device supports snapshots
 +(qemu) loadvm snap0
 +Error: No block device supports snapshots
 +(qemu) quit
 +
 +
 +=== -drive if=none ===
 +
 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 +Testing: -drive driver=file,file=TEST_DIR/t.IMGFMT,if=none
 +QEMU X.Y.Z monitor - type 'help' for more information
 +(qemu) savevm snap0
 +Error: Device 'none0' is writable but does not support snapshots
 +(qemu) info snapshots
 +No available block device supports snapshots
 +(qemu) loadvm snap0
 +Error: Device 'none0' is writable but does not support snapshots
 +(qemu) quit
 +
 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 +Testing: -drive driver=IMGFMT,file=TEST_DIR/t.IMGFMT,if=none
 +QEMU X.Y.Z monitor - type 'help' for more information
 +(qemu) savevm snap0
 +(qemu) info snapshots
 +List of snapshots present on all disks:
 +ID        TAG                 VM SIZE                DATE       VM CLOCK
 +--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
 +(qemu) loadvm snap0
 +(qemu) quit
 +
 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 +Testing: -drive driver=IMGFMT,file=TEST_DIR/t.IMGFMT,if=none -device virtio-blk,drive=none0
 +QEMU X.Y.Z monitor - type 'help' for more information
 +(qemu) savevm snap0
 +(qemu) info snapshots
 +List of snapshots present on all disks:
 +ID        TAG                 VM SIZE                DATE       VM CLOCK
 +--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
 +(qemu) loadvm snap0
 +(qemu) quit
 +
 +
 +=== -drive if=virtio ===
 +
 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 +Testing: -drive driver=file,file=TEST_DIR/t.IMGFMT,if=virtio
 +QEMU X.Y.Z monitor - type 'help' for more information
 +(qemu) savevm snap0
 +Error: Device 'virtio0' is writable but does not support snapshots
 +(qemu) info snapshots
 +No available block device supports snapshots
 +(qemu) loadvm snap0
 +Error: Device 'virtio0' is writable but does not support snapshots
 +(qemu) quit
 +
 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 +Testing: -drive driver=IMGFMT,file=TEST_DIR/t.IMGFMT,if=virtio
 +QEMU X.Y.Z monitor - type 'help' for more information
 +(qemu) savevm snap0
 +(qemu) info snapshots
 +List of snapshots present on all disks:
 +ID        TAG                 VM SIZE                DATE       VM CLOCK
 +--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
 +(qemu) loadvm snap0
 +(qemu) quit
 +
 +
 +=== Simple -blockdev ===
 +
 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 +Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file
 +QEMU X.Y.Z monitor - type 'help' for more information
 +(qemu) savevm snap0
 +Error: Device '' is writable but does not support snapshots
 +(qemu) info snapshots
 +No available block device supports snapshots
 +(qemu) loadvm snap0
 +Error: Device '' is writable but does not support snapshots
 +(qemu) quit
 +
 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 +Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=IMGFMT,file=file,node-name=fmt
 +QEMU X.Y.Z monitor - type 'help' for more information
 +(qemu) savevm snap0
 +(qemu) info snapshots
 +List of snapshots present on all disks:
 +ID        TAG                 VM SIZE                DATE       VM CLOCK
 +--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
 +(qemu) loadvm snap0
 +(qemu) quit
 +
 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 +Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=raw,file=file,node-name=raw -blockdev driver=IMGFMT,file=raw,node-name=fmt
 +QEMU X.Y.Z monitor - type 'help' for more information
 +(qemu) savevm snap0
 +(qemu) info snapshots
 +List of snapshots present on all disks:
 +ID        TAG                 VM SIZE                DATE       VM CLOCK
 +--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
 +(qemu) loadvm snap0
 +(qemu) quit
 +
 +
 +=== -blockdev with a filter on top ===
 +
 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 +Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=IMGFMT,file=file,node-name=fmt -blockdev driver=copy-on-read,file=fmt,node-name=filter
 +QEMU X.Y.Z monitor - type 'help' for more information
 +(qemu) savevm snap0
 +(qemu) info snapshots
 +List of snapshots present on all disks:
 +ID        TAG                 VM SIZE                DATE       VM CLOCK
 +--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
 +(qemu) loadvm snap0
 +(qemu) quit
 +
 +
 +=== -blockdev with a backing file ===
 +
 +Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728
 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
 +Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT.base,node-name=backing-file -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=IMGFMT,file=file,backing=backing-file,node-name=fmt
 +QEMU X.Y.Z monitor - type 'help' for more information
 +(qemu) savevm snap0
 +(qemu) info snapshots
 +List of snapshots present on all disks:
 +ID        TAG                 VM SIZE                DATE       VM CLOCK
 +--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
 +(qemu) loadvm snap0
 +(qemu) quit
 +
 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
 +Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT.base,node-name=backing-file -blockdev driver=IMGFMT,file=backing-file,node-name=backing-fmt -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=IMGFMT,file=file,backing=backing-fmt,node-name=fmt
 +QEMU X.Y.Z monitor - type 'help' for more information
 +(qemu) savevm snap0
 +(qemu) info snapshots
 +List of snapshots present on all disks:
 +ID        TAG                 VM SIZE                DATE       VM CLOCK
 +--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
 +(qemu) loadvm snap0
 +(qemu) quit
 +
 +Internal snapshots on overlay:
 +Snapshot list:
 +ID        TAG                 VM SIZE                DATE       VM CLOCK
 +1         snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
 +Internal snapshots on backing file:
 +
 +=== -blockdev with NBD server on the backing file ===
 +
 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
 +Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT.base,node-name=backing-file -blockdev driver=IMGFMT,file=backing-file,node-name=backing-fmt -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=IMGFMT,file=file,backing=backing-fmt,node-name=fmt
 +QEMU X.Y.Z monitor - type 'help' for more information
 +(qemu) nbd_server_start unix:TEST_DIR/nbd
 +(qemu) nbd_server_add -w backing-fmt
 +(qemu) savevm snap0
 +(qemu) info snapshots
 +List of snapshots present on all disks:
 +ID        TAG                 VM SIZE                DATE       VM CLOCK
 +--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
 +(qemu) loadvm snap0
 +(qemu) quit
 +
 +Internal snapshots on overlay:
 +Snapshot list:
 +ID        TAG                 VM SIZE                DATE       VM CLOCK
 +1         snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
 +Internal snapshots on backing file:
 +Snapshot list:
 +ID        TAG                 VM SIZE                DATE       VM CLOCK
 +1         snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
 +*** done
 diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
 index XXXXXXX..XXXXXXX 100644
 --- a/tests/qemu-iotests/common.filter
 +++ b/tests/qemu-iotests/common.filter
@@ -XXX,XX +XXX,XX @@
  # standard filters
  #
 -# ctime(3) dates
 -#
  _filter_date()
  {
 -    $SED \
 -        -e 's/[A-Z][a-z][a-z] [A-z][a-z][a-z]  *[0-9][0-9]* [0-9][0-9]:[0-9][0-9]:[0-9][0-9] [0-9][0-9][0-9][0-9]$/DATE/'
 +    $SED -re 's/[0-9]{4}-[0-9]{2}-[0-9]{2} [0-9]{2}:[0-9]{2}:[0-9]{2}/yyyy-mm-dd hh:mm:ss/'
 +}
 +
 +_filter_vmstate_size()
 +{
 +    $SED -re 's/[0-9. ]{5} [KMGT]iB/     SIZE/' \
 +         -re 's/[0-9. ]{5} B/   SIZE/'
  }
  _filter_generated_node_ids()
 diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
 index XXXXXXX..XXXXXXX 100644
 --- a/tests/qemu-iotests/group
 +++ b/tests/qemu-iotests/group
 @@ -XXX,XX +XXX,XX @@
-rw backing quick
+rw quick
-rw migration quick
+rw auto quick
 rw quick
-+282 rw img quick
++267 rw auto quick snapshot
 auto quick
 rw
 rw quick
 --
 .20.1

-[PULL 12/13] crypto.c: cleanup created file when block_crypto_co_create_opts_luks fails
+[PULL v2 3/4] iotests: Require Python 3.6 or later
-From: Daniel Henrique Barboza <danielhb413@gmail.com>
+Running iotests is not required to build QEMU, so we can have stricter
 version requirements for Python here and can make use of new features
 and drop compatibility code earlier.
-When using a non-UTF8 secret to create a volume using qemu-img, the
+This makes qemu-iotests skip all Python tests if a Python version before
-following error happens:
+.6 is used for the build.
-$ qemu-img create -f luks --object secret,id=vol_1_encrypt0,file=vol_resize_pool.vol_1.secret.qzVQrI -o key-secret=vol_1_encrypt0 /var/tmp/pool_target/vol_1 10240K
+Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
 Signed-off-by: Kevin Wolf <kwolf@redhat.com>
 Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
 Reviewed-by: Thomas Huth <thuth@redhat.com>
 Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
 Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
 ---
  tests/qemu-iotests/check | 13 ++++++++++++-
 file changed, 12 insertions(+), 1 deletion(-)
-Formatting '/var/tmp/pool_target/vol_1', fmt=luks size=10485760 key-secret=vol_1_encrypt0
+diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
-qemu-img: /var/tmp/pool_target/vol_1: Data from secret vol_1_encrypt0 is not valid UTF-8
+index XXXXXXX..XXXXXXX 100755
+--- a/tests/qemu-iotests/check
-However, the created file '/var/tmp/pool_target/vol_1' is left behind in the
++++ b/tests/qemu-iotests/check
-file system after the failure. This behavior can be observed when creating
+@@ -XXX,XX +XXX,XX @@ then
-the volume using Libvirt, via 'virsh vol-create', and then getting "volume
+     export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper"
-target path already exist" errors when trying to re-create the volume.
+ fi
-The volume file is created inside block_crypto_co_create_opts_luks(), in
++python_usable=false
-block/crypto.c. If the bdrv_create_file() call is successful but any
++if $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)'
-succeeding step fails*, the existing 'fail' label does not take into
++then
-account the created file, leaving it behind.
++    python_usable=true
++fi
 This patch changes block_crypto_co_create_opts_luks() to delete
 'filename' in case of failure. A failure in this point means that
 the volume is now truncated/corrupted, so even if 'filename' was an
 existing volume before calling qemu-img, it is now unusable. Deleting
 the file it is not much worse than leaving it in the filesystem in
 this scenario, and we don't have to deal with checking the file
 pre-existence in the code.
 * in our case, block_crypto_co_create_generic calls qcrypto_block_create,
 which calls qcrypto_block_luks_create, and this function fails when
 calling qcrypto_secret_lookup_as_utf8.
 Reported-by: Srikanth Aithal <bssrikanth@in.ibm.com>
 Suggested-by: Kevin Wolf <kwolf@redhat.com>
 Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
 Message-Id: <20200130213907.2830642-4-danielhb413@gmail.com>
 Signed-off-by: Kevin Wolf <kwolf@redhat.com>
 ---
  block/crypto.c | 18 ++++++++++++++++++
 file changed, 18 insertions(+)
 diff --git a/block/crypto.c b/block/crypto.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/crypto.c
 +++ b/block/crypto.c
@@ -XXX,XX +XXX,XX @@
  #include "qapi/error.h"
  #include "qemu/module.h"
  #include "qemu/option.h"
 +#include "qemu/cutils.h"
  #include "crypto.h"
  typedef struct BlockCrypto BlockCrypto;
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn block_crypto_co_create_opts_luks(const char *filename,
      ret = 0;
  fail:
 +    /*
 +     * If an error occurred, delete 'filename'. Even if the file existed
 +     * beforehand, it has been truncated and corrupted in the process.
 +     */
 +    if (ret && bs) {
 +        Error *local_delete_err = NULL;
 +        int r_del = bdrv_co_delete_file(bs, &local_delete_err);
 +        /*
 +         * ENOTSUP will happen if the block driver doesn't support
 +         * the 'bdrv_co_delete_file' interface. This is a predictable
 +         * scenario and shouldn't be reported back to the user.
 +         */
 +        if ((r_del < 0) && (r_del != -ENOTSUP)) {
 +            error_report_err(local_delete_err);
 +        }
 +    }
 +
-     bdrv_unref(bs);
+ default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p')
-     qapi_free_QCryptoBlockCreateOptions(create_opts);
+ default_alias_machine=$($QEMU_PROG -machine help | \
-     qobject_unref(cryptoopts);
+    sed -n "/(alias of $default_machine)/ { s/ .*//p; q; }")
@@ -XXX,XX +XXX,XX @@ do
          start=$(_wallclock)
          if [ "$(head -n 1 "$source_iotests/$seq")" == "#!/usr/bin/env python" ]; then
 -            run_command="$PYTHON $seq"
 +            if $python_usable; then
 +                run_command="$PYTHON $seq"
 +            else
 +                run_command="false"
 +                echo "Unsupported Python version" > $seq.notrun
 +            fi
          else
              run_command="./$seq"
          fi
 --
 .20.1

-[PULL 04/13] iotests: Fix run_job() with use_log=False
+[PULL v2 4/4] iotests: Remove Python 2 compatibility code
-The 'job-complete' QMP command should be run with qmp() rather than
+Some scripts check the Python version number and have two code paths to
-qmp_log() if use_log=False is passed.
+accomodate both Python 2 and 3. Remove the code specific to Python 2 and
 assert the minimum version of 3.6 instead (check skips Python tests in
 this case, so the assertion would only ever trigger if a Python script
 is executed manually).
 Signed-off-by: Kevin Wolf <kwolf@redhat.com>
-Message-Id: <20200310113831.27293-4-kwolf@redhat.com>
+Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
-Reviewed-by: Peter Krempa <pkrempa@redhat.com>
+Reviewed-by: Thomas Huth <thuth@redhat.com>
-Signed-off-by: Kevin Wolf <kwolf@redhat.com>
+Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
 Reviewed-by: John Snow <jsnow@redhat.com>
 Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
 ---
- tests/qemu-iotests/iotests.py | 5 ++++-
+ tests/qemu-iotests/044        |  3 ---
-file changed, 4 insertions(+), 1 deletion(-)
+ tests/qemu-iotests/163        |  3 ---
  tests/qemu-iotests/iotests.py | 13 +++----------
 files changed, 3 insertions(+), 16 deletions(-)
+diff --git a/tests/qemu-iotests/044 b/tests/qemu-iotests/044
+index XXXXXXX..XXXXXXX 100755
+--- a/tests/qemu-iotests/044
++++ b/tests/qemu-iotests/044
+@@ -XXX,XX +XXX,XX @@ import struct
+ import subprocess
+ import sys
+-if sys.version_info.major == 2:
+-    range = xrange
+-
+ test_img = os.path.join(iotests.test_dir, 'test.img')
+ class TestRefcountTableGrowth(iotests.QMPTestCase):
+diff --git a/tests/qemu-iotests/163 b/tests/qemu-iotests/163
+index XXXXXXX..XXXXXXX 100755
+--- a/tests/qemu-iotests/163
++++ b/tests/qemu-iotests/163
+@@ -XXX,XX +XXX,XX @@
+ import os, random, iotests, struct, qcow2, sys
+ from iotests import qemu_img, qemu_io, image_size
+-if sys.version_info.major == 2:
+-    range = xrange
+-
+ test_img = os.path.join(iotests.test_dir, 'test.img')
+ check_img = os.path.join(iotests.test_dir, 'check.img')
 diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
 index XXXXXXX..XXXXXXX 100644
 --- a/tests/qemu-iotests/iotests.py
 +++ b/tests/qemu-iotests/iotests.py
-@@ -XXX,XX +XXX,XX @@ class VM(qtest.QEMUQtestMachine):
+@@ -XXX,XX +XXX,XX @@ from collections import OrderedDict
-                         if use_log:
+ sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
-                             log('Job failed: %s' % (j['error']))
+ from qemu import qtest
-             elif status == 'ready':
--                self.qmp_log('job-complete', id=job)
++assert sys.version_info >= (3,6)
-+                if use_log:
-+                    self.qmp_log('job-complete', id=job)
+ # This will not work if arguments contain spaces but is necessary if we
-+                else:
+ # want to support the override options that ./check supports.
-+                    self.qmp('job-complete', id=job)
+@@ -XXX,XX +XXX,XX @@ def image_size(img):
-             elif status == 'pending' and not auto_finalize:
+     return json.loads(r)['virtual-size']
-                 if pre_finalize:
-                     pre_finalize()
+ def is_str(val):
 -    if sys.version_info.major >= 3:
 -        return isinstance(val, str)
 -    else:
 -        return isinstance(val, str) or isinstance(val, unicode)
 +    return isinstance(val, str)
  test_dir_re = re.compile(r"%s" % test_dir)
  def filter_test_dir(msg):
@@ -XXX,XX +XXX,XX @@ def execute_test(test_function=None,
      else:
          # We need to filter out the time taken from the output so that
          # qemu-iotest can reliably diff the results against master output.
 -        if sys.version_info.major >= 3:
 -            output = io.StringIO()
 -        else:
 -            # io.StringIO is for unicode strings, which is not what
 -            # 2.x's test runner emits.
 -            output = io.BytesIO()
 +        output = io.StringIO()
      logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN))
 --
 .20.1

-[PULL 05/13] iotests: Test mirror with temporarily disabled target backing file
+Deleted patch
-The newly tested scenario is a common live storage migration scenario:
-The target node is opened without a backing file so that the active
-layer is mirrored while its backing chain can be copied in the
-background.
-The backing chain should be attached to the mirror target node when
-finalising the job, just before switching the users of the source node
-to the new copy (at which point the mirror job still has a reference to
-the node). drive-mirror did this automatically, but with blockdev-mirror
-this is the job of the QMP client.
-This patch adds test cases for two ways to achieve the desired result,
-using either x-blockdev-reopen or blockdev-snapshot.
-Signed-off-by: Kevin Wolf <kwolf@redhat.com>
-Message-Id: <20200310113831.27293-5-kwolf@redhat.com>
-Reviewed-by: Peter Krempa <pkrempa@redhat.com>
-Signed-off-by: Kevin Wolf <kwolf@redhat.com>
----
- tests/qemu-iotests/155     | 56 ++++++++++++++++++++++++++++++++++----
- tests/qemu-iotests/155.out |  4 +--
-files changed, 53 insertions(+), 7 deletions(-)
-diff --git a/tests/qemu-iotests/155 b/tests/qemu-iotests/155
-index XXXXXXX..XXXXXXX 100755
---- a/tests/qemu-iotests/155
-+++ b/tests/qemu-iotests/155
-@@ -XXX,XX +XXX,XX @@ target_img = os.path.join(iotests.test_dir, 'target.' + iotests.imgfmt)
- #                      image during runtime, only makes sense if
- #                      target_blockdev_backing is not None
- #                      (None: same as target_backing)
-+# target_open_with_backing: If True, the target image is added with its backing
-+#                           chain opened right away. If False, blockdev-add
-+#                           opens it without a backing file and job completion
-+#                           is supposed to open the backing chain.
- class BaseClass(iotests.QMPTestCase):
-     target_blockdev_backing = None
-     target_real_backing = None
-+    target_open_with_backing = True
-     def setUp(self):
-         qemu_img('create', '-f', iotests.imgfmt, back0_img, '1440K')
-@@ -XXX,XX +XXX,XX @@ class BaseClass(iotests.QMPTestCase):
-                 options = { 'node-name': 'target',
-                             'driver': iotests.imgfmt,
-                             'file': { 'driver': 'file',
-+                                      'node-name': 'target-file',
-                                       'filename': target_img } }
--                if self.target_blockdev_backing:
--                    options['backing'] = self.target_blockdev_backing
-+
-+                if not self.target_open_with_backing:
-+                        options['backing'] = None
-+                elif self.target_blockdev_backing:
-+                        options['backing'] = self.target_blockdev_backing
-                 result = self.vm.qmp('blockdev-add', **options)
-                 self.assert_qmp(result, 'return', {})
-@@ -XXX,XX +XXX,XX @@ class BaseClass(iotests.QMPTestCase):
- # cmd: Mirroring command to execute, either drive-mirror or blockdev-mirror
- class MirrorBaseClass(BaseClass):
-+    def openBacking(self):
-+        pass
-+
-     def runMirror(self, sync):
-         if self.cmd == 'blockdev-mirror':
-             result = self.vm.qmp(self.cmd, job_id='mirror-job', device='source',
--                                 sync=sync, target='target')
-+                                 sync=sync, target='target',
-+                                 auto_finalize=False)
-         else:
-             if self.existing:
-                 mode = 'existing'
-@@ -XXX,XX +XXX,XX @@ class MirrorBaseClass(BaseClass):
-             result = self.vm.qmp(self.cmd, job_id='mirror-job', device='source',
-                                  sync=sync, target=target_img,
-                                  format=iotests.imgfmt, mode=mode,
--                                 node_name='target')
-+                                 node_name='target', auto_finalize=False)
-         self.assert_qmp(result, 'return', {})
--        self.complete_and_wait('mirror-job')
-+        self.vm.run_job('mirror-job', use_log=False, auto_finalize=False,
-+                        pre_finalize=self.openBacking, auto_dismiss=True)
-     def testFull(self):
-         self.runMirror('full')
-@@ -XXX,XX +XXX,XX @@ class TestBlockdevMirrorForcedBacking(MirrorBaseClass):
-     target_blockdev_backing = { 'driver': 'null-co' }
-     target_real_backing = 'null-co://'
-+# Attach the backing chain only during completion, with blockdev-reopen
-+class TestBlockdevMirrorReopen(MirrorBaseClass):
-+    cmd = 'blockdev-mirror'
-+    existing = True
-+    target_backing = 'null-co://'
-+    target_open_with_backing = False
-+
-+    def openBacking(self):
-+        if not self.target_open_with_backing:
-+            result = self.vm.qmp('blockdev-add', node_name="backing",
-+                                 driver="null-co")
-+            self.assert_qmp(result, 'return', {})
-+            result = self.vm.qmp('x-blockdev-reopen', node_name="target",
-+                                 driver=iotests.imgfmt, file="target-file",
-+                                 backing="backing")
-+            self.assert_qmp(result, 'return', {})
-+
-+# Attach the backing chain only during completion, with blockdev-snapshot
-+class TestBlockdevMirrorSnapshot(MirrorBaseClass):
-+    cmd = 'blockdev-mirror'
-+    existing = True
-+    target_backing = 'null-co://'
-+    target_open_with_backing = False
-+
-+    def openBacking(self):
-+        if not self.target_open_with_backing:
-+            result = self.vm.qmp('blockdev-add', node_name="backing",
-+                                 driver="null-co")
-+            self.assert_qmp(result, 'return', {})
-+            result = self.vm.qmp('blockdev-snapshot', node="backing",
-+                                 overlay="target")
-+            self.assert_qmp(result, 'return', {})
- class TestCommit(BaseClass):
-     existing = False
-diff --git a/tests/qemu-iotests/155.out b/tests/qemu-iotests/155.out
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/155.out
-+++ b/tests/qemu-iotests/155.out
-@@ -XXX,XX +XXX,XX @@
--...................
-+.........................
- ----------------------------------------------------------------------
--Ran 19 tests
-+Ran 25 tests
- OK
---
-.20.1

-[PULL 06/13] block: Fix cross-AioContext blockdev-snapshot
+Deleted patch
-external_snapshot_prepare() tries to move the overlay to the AioContext
-of the backing file (the snapshotted node). However, it's possible that
-this doesn't work, but the backing file can instead be moved to the
-overlay's AioContext (e.g. opening the backing chain for a mirror
-target).
-bdrv_append() already indirectly uses bdrv_attach_node(), which takes
-care to move nodes to make sure they use the same AioContext and which
-tries both directions.
-So the problem has a simple fix: Just delete the unnecessary extra
-bdrv_try_set_aio_context() call in external_snapshot_prepare() and
-instead assert in bdrv_append() that both nodes were indeed moved to the
-same AioContext.
-Signed-off-by: Kevin Wolf <kwolf@redhat.com>
-Message-Id: <20200310113831.27293-6-kwolf@redhat.com>
-Tested-by: Peter Krempa <pkrempa@redhat.com>
-Signed-off-by: Kevin Wolf <kwolf@redhat.com>
----
- block.c    |  1 +
- blockdev.c | 16 ----------------
-files changed, 1 insertion(+), 16 deletions(-)
-diff --git a/block.c b/block.c
-index XXXXXXX..XXXXXXX 100644
---- a/block.c
-+++ b/block.c
-@@ -XXX,XX +XXX,XX @@ void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
-     bdrv_ref(from);
-     assert(qemu_get_current_aio_context() == qemu_get_aio_context());
-+    assert(bdrv_get_aio_context(from) == bdrv_get_aio_context(to));
-     bdrv_drained_begin(from);
-     /* Put all parents into @list and calculate their cumulative permissions */
-diff --git a/blockdev.c b/blockdev.c
-index XXXXXXX..XXXXXXX 100644
---- a/blockdev.c
-+++ b/blockdev.c
-@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
-                              DO_UPCAST(ExternalSnapshotState, common, common);
-     TransactionAction *action = common->action;
-     AioContext *aio_context;
--    AioContext *old_context;
-     uint64_t perm, shared;
--    int ret;
-     /* 'blockdev-snapshot' and 'blockdev-snapshot-sync' have similar
-      * purpose but a different set of parameters */
-@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
-         goto out;
-     }
--    /* Honor bdrv_try_set_aio_context() context acquisition requirements. */
--    old_context = bdrv_get_aio_context(state->new_bs);
--    aio_context_release(aio_context);
--    aio_context_acquire(old_context);
--
--    ret = bdrv_try_set_aio_context(state->new_bs, aio_context, errp);
--
--    aio_context_release(old_context);
--    aio_context_acquire(aio_context);
--
--    if (ret < 0) {
--        goto out;
--    }
--
-     /* This removes our old bs and adds the new bs. This is an operation that
-      * can fail, so we need to do it in .prepare; undoing it for abort is
-      * always possible. */
---
-.20.1

-[PULL 07/13] iotests: Add iothread cases to 155
+Deleted patch
-This patch adds test cases for attaching the backing chain to a mirror
-job target right before finalising the job, where the image is in a
-non-mainloop AioContext (i.e. the backing chain needs to be moved to the
-AioContext of the mirror target).
-This requires switching the test case from virtio-blk to virtio-scsi
-because virtio-blk only actually starts using the iothreads when the
-guest driver initialises the device (which never happens in a test case
-without a guest OS). virtio-scsi always keeps its block nodes in the
-AioContext of the the requested iothread without guest interaction.
-Signed-off-by: Kevin Wolf <kwolf@redhat.com>
-Message-Id: <20200310113831.27293-7-kwolf@redhat.com>
-Reviewed-by: Peter Krempa <pkrempa@redhat.com>
-Signed-off-by: Kevin Wolf <kwolf@redhat.com>
----
- tests/qemu-iotests/155     | 32 +++++++++++++++++++++++---------
- tests/qemu-iotests/155.out |  4 ++--
-files changed, 25 insertions(+), 11 deletions(-)
-diff --git a/tests/qemu-iotests/155 b/tests/qemu-iotests/155
-index XXXXXXX..XXXXXXX 100755
---- a/tests/qemu-iotests/155
-+++ b/tests/qemu-iotests/155
-@@ -XXX,XX +XXX,XX @@ target_img = os.path.join(iotests.test_dir, 'target.' + iotests.imgfmt)
- #                           chain opened right away. If False, blockdev-add
- #                           opens it without a backing file and job completion
- #                           is supposed to open the backing chain.
-+# use_iothread: If True, an iothread is configured for the virtio-blk device
-+#               that uses the image being mirrored
- class BaseClass(iotests.QMPTestCase):
-     target_blockdev_backing = None
-     target_real_backing = None
-     target_open_with_backing = True
-+    use_iothread = False
-     def setUp(self):
-         qemu_img('create', '-f', iotests.imgfmt, back0_img, '1440K')
-@@ -XXX,XX +XXX,XX @@ class BaseClass(iotests.QMPTestCase):
-                     'file': {'driver': 'file',
-                              'filename': source_img}}
-         self.vm.add_blockdev(self.vm.qmp_to_opts(blockdev))
--        self.vm.add_device('virtio-blk,id=qdev0,drive=source')
-+
-+        if self.use_iothread:
-+            self.vm.add_object('iothread,id=iothread0')
-+            iothread = ",iothread=iothread0"
-+        else:
-+            iothread = ""
-+
-+        self.vm.add_device('virtio-scsi%s' % iothread)
-+        self.vm.add_device('scsi-hd,id=qdev0,drive=source')
-+
-         self.vm.launch()
-         self.assertIntactSourceBackingChain()
-@@ -XXX,XX +XXX,XX @@ class MirrorBaseClass(BaseClass):
-     def testFull(self):
-         self.runMirror('full')
--        node = self.findBlockNode('target',
--                                  '/machine/peripheral/qdev0/virtio-backend')
-+        node = self.findBlockNode('target', 'qdev0')
-         self.assertCorrectBackingImage(node, None)
-         self.assertIntactSourceBackingChain()
-     def testTop(self):
-         self.runMirror('top')
--        node = self.findBlockNode('target',
--                                  '/machine/peripheral/qdev0/virtio-backend')
-+        node = self.findBlockNode('target', 'qdev0')
-         self.assertCorrectBackingImage(node, back2_img)
-         self.assertIntactSourceBackingChain()
-     def testNone(self):
-         self.runMirror('none')
--        node = self.findBlockNode('target',
--                                  '/machine/peripheral/qdev0/virtio-backend')
-+        node = self.findBlockNode('target', 'qdev0')
-         self.assertCorrectBackingImage(node, source_img)
-         self.assertIntactSourceBackingChain()
-@@ -XXX,XX +XXX,XX @@ class TestBlockdevMirrorReopen(MirrorBaseClass):
-                                  backing="backing")
-             self.assert_qmp(result, 'return', {})
-+class TestBlockdevMirrorReopenIothread(TestBlockdevMirrorReopen):
-+    use_iothread = True
-+
- # Attach the backing chain only during completion, with blockdev-snapshot
- class TestBlockdevMirrorSnapshot(MirrorBaseClass):
-     cmd = 'blockdev-mirror'
-@@ -XXX,XX +XXX,XX @@ class TestBlockdevMirrorSnapshot(MirrorBaseClass):
-                                  overlay="target")
-             self.assert_qmp(result, 'return', {})
-+class TestBlockdevMirrorSnapshotIothread(TestBlockdevMirrorSnapshot):
-+    use_iothread = True
-+
- class TestCommit(BaseClass):
-     existing = False
-@@ -XXX,XX +XXX,XX @@ class TestCommit(BaseClass):
-         self.vm.event_wait('BLOCK_JOB_COMPLETED')
--        node = self.findBlockNode(None,
--                                  '/machine/peripheral/qdev0/virtio-backend')
-+        node = self.findBlockNode(None, 'qdev0')
-         self.assert_qmp(node, 'image' + '/backing-image' * 0 + '/filename',
-                         back1_img)
-         self.assert_qmp(node, 'image' + '/backing-image' * 1 + '/filename',
-diff --git a/tests/qemu-iotests/155.out b/tests/qemu-iotests/155.out
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/155.out
-+++ b/tests/qemu-iotests/155.out
-@@ -XXX,XX +XXX,XX @@
--.........................
-+...............................
- ----------------------------------------------------------------------
--Ran 25 tests
-+Ran 31 tests
- OK
---
-.20.1

-[PULL 08/13] qapi: Add '@allow-write-only-overlay' feature for 'blockdev-snapshot'
+Deleted patch
-From: Peter Krempa <pkrempa@redhat.com>
-Anounce that 'blockdev-snapshot' command's permissions allow changing
-of the backing file if the 'consistent_read' permission is not required.
-This is useful for libvirt to allow late opening of the backing chain
-during a blockdev-mirror.
-Signed-off-by: Peter Krempa <pkrempa@redhat.com>
-Signed-off-by: Kevin Wolf <kwolf@redhat.com>
-Message-Id: <20200310113831.27293-8-kwolf@redhat.com>
-Signed-off-by: Kevin Wolf <kwolf@redhat.com>
----
- qapi/block-core.json | 9 ++++++++-
-file changed, 8 insertions(+), 1 deletion(-)
-diff --git a/qapi/block-core.json b/qapi/block-core.json
-index XXXXXXX..XXXXXXX 100644
---- a/qapi/block-core.json
-+++ b/qapi/block-core.json
-@@ -XXX,XX +XXX,XX @@
- #
- # For the arguments, see the documentation of BlockdevSnapshot.
- #
-+# Features:
-+# @allow-write-only-overlay: If present, the check whether this operation is safe
-+#                            was relaxed so that it can be used to change
-+#                            backing file of a destination of a blockdev-mirror.
-+#                            (since 5.0)
-+#
- # Since: 2.5
- #
- # Example:
-@@ -XXX,XX +XXX,XX @@
- #
- ##
- { 'command': 'blockdev-snapshot',
--  'data': 'BlockdevSnapshot' }
-+  'data': 'BlockdevSnapshot',
-+  'features': [ 'allow-write-only-overlay' ] }
- ##
- # @change-backing-file:
---
-.20.1

-[PULL 09/13] tests/qemu-iotests: Fix socket_scm_helper build path
+Deleted patch
-From: Philippe Mathieu-Daudé <philmd@redhat.com>
-The socket_scm_helper path got corrupted during the mechanical
-refactor moving the qtests files into their own sub-directory.
-Fixes: 1e8a1fae7 ("test: Move qtests to a separate directory")
-Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
-Message-Id: <20200306165751.18986-1-philmd@redhat.com>
-Reviewed-by: Laurent Vivier <laurent@vivier.eu>
-Signed-off-by: Kevin Wolf <kwolf@redhat.com>
----
- tests/Makefile.include       | 1 +
- tests/qtest/Makefile.include | 1 -
-files changed, 1 insertion(+), 1 deletion(-)
-diff --git a/tests/Makefile.include b/tests/Makefile.include
-index XXXXXXX..XXXXXXX 100644
---- a/tests/Makefile.include
-+++ b/tests/Makefile.include
-@@ -XXX,XX +XXX,XX @@ include $(SRC_PATH)/tests/qtest/Makefile.include
- tests/test-qga$(EXESUF): qemu-ga$(EXESUF)
- tests/test-qga$(EXESUF): tests/test-qga.o $(qtest-obj-y)
- tests/vhost-user-bridge$(EXESUF): tests/vhost-user-bridge.o $(test-util-obj-y) libvhost-user.a
-+tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o
- SPEED = quick
-diff --git a/tests/qtest/Makefile.include b/tests/qtest/Makefile.include
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qtest/Makefile.include
-+++ b/tests/qtest/Makefile.include
-@@ -XXX,XX +XXX,XX @@ tests/qtest/usb-hcd-ehci-test$(EXESUF): tests/qtest/usb-hcd-ehci-test.o $(libqos
- tests/qtest/usb-hcd-xhci-test$(EXESUF): tests/qtest/usb-hcd-xhci-test.o $(libqos-usb-obj-y)
- tests/qtest/cpu-plug-test$(EXESUF): tests/qtest/cpu-plug-test.o
- tests/qtest/migration-test$(EXESUF): tests/qtest/migration-test.o tests/qtest/migration-helpers.o
--tests/qtest/qemu-iotests/qtest/socket_scm_helper$(EXESUF): tests/qtest/qemu-iotests/qtest/socket_scm_helper.o
- tests/qtest/test-netfilter$(EXESUF): tests/qtest/test-netfilter.o $(qtest-obj-y)
- tests/qtest/test-filter-mirror$(EXESUF): tests/qtest/test-filter-mirror.o $(qtest-obj-y)
- tests/qtest/test-filter-redirector$(EXESUF): tests/qtest/test-filter-redirector.o $(qtest-obj-y)
---
-.20.1

-[PULL 10/13] block: introducing 'bdrv_co_delete_file' interface
+Deleted patch
-From: Daniel Henrique Barboza <danielhb413@gmail.com>
-Adding to Block Drivers the capability of being able to clean up
-its created files can be useful in certain situations. For the
-LUKS driver, for instance, a failure in one of its authentication
-steps can leave files in the host that weren't there before.
-This patch adds the 'bdrv_co_delete_file' interface to block
-drivers and add it to the 'file' driver in file-posix.c. The
-implementation is given by 'raw_co_delete_file'.
-Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
-Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
-Message-Id: <20200130213907.2830642-2-danielhb413@gmail.com>
-Signed-off-by: Kevin Wolf <kwolf@redhat.com>
----
- include/block/block_int.h |  4 ++++
- block/file-posix.c        | 23 +++++++++++++++++++++++
-files changed, 27 insertions(+)
-diff --git a/include/block/block_int.h b/include/block/block_int.h
-index XXXXXXX..XXXXXXX 100644
---- a/include/block/block_int.h
-+++ b/include/block/block_int.h
-@@ -XXX,XX +XXX,XX @@ struct BlockDriver {
-      */
-     int coroutine_fn (*bdrv_co_flush)(BlockDriverState *bs);
-+    /* Delete a created file. */
-+    int coroutine_fn (*bdrv_co_delete_file)(BlockDriverState *bs,
-+                                            Error **errp);
-+
-     /*
-      * Flushes all data that was already written to the OS all the way down to
-      * the disk (for example file-posix.c calls fsync()).
-diff --git a/block/file-posix.c b/block/file-posix.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/file-posix.c
-+++ b/block/file-posix.c
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_create_opts(const char *filename, QemuOpts *opts,
-     return raw_co_create(&options, errp);
- }
-+static int coroutine_fn raw_co_delete_file(BlockDriverState *bs,
-+                                           Error **errp)
-+{
-+    struct stat st;
-+    int ret;
-+
-+    if (!(stat(bs->filename, &st) == 0) || !S_ISREG(st.st_mode)) {
-+        error_setg_errno(errp, ENOENT, "%s is not a regular file",
-+                         bs->filename);
-+        return -ENOENT;
-+    }
-+
-+    ret = unlink(bs->filename);
-+    if (ret < 0) {
-+        ret = -errno;
-+        error_setg_errno(errp, -ret, "Error when deleting file %s",
-+                         bs->filename);
-+    }
-+
-+    return ret;
-+}
-+
- /*
-  * Find allocation range in @bs around offset @start.
-  * May change underlying file descriptor's file offset.
-@@ -XXX,XX +XXX,XX @@ BlockDriver bdrv_file = {
-     .bdrv_co_block_status = raw_co_block_status,
-     .bdrv_co_invalidate_cache = raw_co_invalidate_cache,
-     .bdrv_co_pwrite_zeroes = raw_co_pwrite_zeroes,
-+    .bdrv_co_delete_file = raw_co_delete_file,
-     .bdrv_co_preadv         = raw_co_preadv,
-     .bdrv_co_pwritev        = raw_co_pwritev,
---
-.20.1

The following changes since commit ba29883206d92a29ad5a466e679ccfc2ee6132ef:

Merge remote-tracking branch 'remotes/borntraeger/tags/s390x-20200310' into staging (2020-03-10 16:50:28 +0000)

are available in the Git repository at:

git://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to 8bb3b023f2055054ee119cb45b42d2b14be7fc8a:

qemu-iotests: adding LUKS cleanup for non-UTF8 secret error (2020-03-11 15:54:38 +0100)

----------------------------------------------------------------
Block layer patches:

- Relax restrictions for blockdev-snapshot (allows libvirt to do live
  storage migration with blockdev-mirror)
- luks: Delete created files when block_crypto_co_create_opts_luks fails
- Fix memleaks in qmp_object_add

----------------------------------------------------------------
Daniel Henrique Barboza (4):
      block: introducing 'bdrv_co_delete_file' interface
      block.c: adding bdrv_co_delete_file
      crypto.c: cleanup created file when block_crypto_co_create_opts_luks fails
      qemu-iotests: adding LUKS cleanup for non-UTF8 secret error

Kevin Wolf (6):
      block: Make bdrv_get_cumulative_perm() public
      block: Relax restrictions for blockdev-snapshot
      iotests: Fix run_job() with use_log=False
      iotests: Test mirror with temporarily disabled target backing file
      block: Fix cross-AioContext blockdev-snapshot
      iotests: Add iothread cases to 155

Pan Nengyuan (1):
      qom-qmp-cmds: fix two memleaks in qmp_object_add

Peter Krempa (1):
      qapi: Add '@allow-write-only-overlay' feature for 'blockdev-snapshot'

Philippe Mathieu-Daudé (1):
      tests/qemu-iotests: Fix socket_scm_helper build path

From: Pan Nengyuan <pannengyuan@huawei.com>

'type/id' forgot to free in qmp_object_add, this patch fix that.

The leak stack:
Direct leak of 84 byte(s) in 6 object(s) allocated from:
    #0 0x7fe2a5ebf768 in __interceptor_malloc (/lib64/libasan.so.5+0xef768)
    #1 0x7fe2a5044445 in g_malloc (/lib64/libglib-2.0.so.0+0x52445)
    #2 0x7fe2a505dd92 in g_strdup (/lib64/libglib-2.0.so.0+0x6bd92)
    #3 0x56344954e692 in qmp_object_add /mnt/sdb/qemu-new/qemu_test/qemu/qom/qom-qmp-cmds.c:258
    #4 0x563449960f5a in do_qmp_dispatch /mnt/sdb/qemu-new/qemu_test/qemu/qapi/qmp-dispatch.c:132
    #5 0x563449960f5a in qmp_dispatch /mnt/sdb/qemu-new/qemu_test/qemu/qapi/qmp-dispatch.c:175
    #6 0x563449498a30 in monitor_qmp_dispatch /mnt/sdb/qemu-new/qemu_test/qemu/monitor/qmp.c:145
    #7 0x56344949a64f in monitor_qmp_bh_dispatcher /mnt/sdb/qemu-new/qemu_test/qemu/monitor/qmp.c:234
    #8 0x563449a92a3a in aio_bh_call /mnt/sdb/qemu-new/qemu_test/qemu/util/async.c:136

Direct leak of 54 byte(s) in 6 object(s) allocated from:
    #0 0x7fe2a5ebf768 in __interceptor_malloc (/lib64/libasan.so.5+0xef768)
    #1 0x7fe2a5044445 in g_malloc (/lib64/libglib-2.0.so.0+0x52445)
    #2 0x7fe2a505dd92 in g_strdup (/lib64/libglib-2.0.so.0+0x6bd92)
    #3 0x56344954e6c4 in qmp_object_add /mnt/sdb/qemu-new/qemu_test/qemu/qom/qom-qmp-cmds.c:267
    #4 0x563449960f5a in do_qmp_dispatch /mnt/sdb/qemu-new/qemu_test/qemu/qapi/qmp-dispatch.c:132
    #5 0x563449960f5a in qmp_dispatch /mnt/sdb/qemu-new/qemu_test/qemu/qapi/qmp-dispatch.c:175
    #6 0x563449498a30 in monitor_qmp_dispatch /mnt/sdb/qemu-new/qemu_test/qemu/monitor/qmp.c:145
    #7 0x56344949a64f in monitor_qmp_bh_dispatcher /mnt/sdb/qemu-new/qemu_test/qemu/monitor/qmp.c:234
    #8 0x563449a92a3a in aio_bh_call /mnt/sdb/qemu-new/qemu_test/qemu/util/async.c:136

Fixes: 5f07c4d60d091320186e7b0edaf9ed2cc16b2d1e
Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
Message-Id: <20200310064640.5059-1-pannengyuan@huawei.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Acked-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qom/qom-qmp-cmds.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
index XXXXXXX..XXXXXXX 100644
--- a/qom/qom-qmp-cmds.c
+++ b/qom/qom-qmp-cmds.c
@@ -XXX,XX +XXX,XX @@ void qmp_object_add(QDict *qdict, QObject **ret_data, Error **errp)
     QDict *pdict;
     Visitor *v;
     Object *obj;
-    const char *type;
-    const char *id;
+    g_autofree char *type = NULL;
+    g_autofree char *id = NULL;
 
-    type = qdict_get_try_str(qdict, "qom-type");
+    type = g_strdup(qdict_get_try_str(qdict, "qom-type"));
     if (!type) {
         error_setg(errp, QERR_MISSING_PARAMETER, "qom-type");
         return;
-    } else {
-        type = g_strdup(type);
-        qdict_del(qdict, "qom-type");
     }
+    qdict_del(qdict, "qom-type");
 
-    id = qdict_get_try_str(qdict, "id");
+    id = g_strdup(qdict_get_try_str(qdict, "id"));
     if (!id) {
         error_setg(errp, QERR_MISSING_PARAMETER, "id");
         return;
-    } else {
-        id = g_strdup(id);
-        qdict_del(qdict, "id");
     }
+    qdict_del(qdict, "id");
 
     props = qdict_get(qdict, "props");
     if (props) {
-- 
2.20.1

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200310113831.27293-2-kwolf@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 include/block/block_int.h | 3 +++
 block.c                   | 6 ++----
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -XXX,XX +XXX,XX @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
                                   void *opaque, Error **errp);
 void bdrv_root_unref_child(BdrvChild *child);
 
+void bdrv_get_cumulative_perm(BlockDriverState *bs, uint64_t *perm,
+                              uint64_t *shared_perm);
+
 /**
  * Sets a BdrvChild's permissions.  Avoid if the parent is a BDS; use
  * bdrv_child_refresh_perms() instead and make the parent's
diff --git a/block.c b/block.c
index XXXXXXX..XXXXXXX 100644
--- a/block.c
+++ b/block.c
@@ -XXX,XX +XXX,XX @@ static int bdrv_child_check_perm(BdrvChild *c, BlockReopenQueue *q,
                                  bool *tighten_restrictions, Error **errp);
 static void bdrv_child_abort_perm_update(BdrvChild *c);
 static void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared);
-static void bdrv_get_cumulative_perm(BlockDriverState *bs, uint64_t *perm,
-                                     uint64_t *shared_perm);
 
 typedef struct BlockReopenQueueEntry {
      bool prepared;
@@ -XXX,XX +XXX,XX @@ static void bdrv_set_perm(BlockDriverState *bs, uint64_t cumulative_perms,
     }
 }
 
-static void bdrv_get_cumulative_perm(BlockDriverState *bs, uint64_t *perm,
-                                     uint64_t *shared_perm)
+void bdrv_get_cumulative_perm(BlockDriverState *bs, uint64_t *perm,
+                              uint64_t *shared_perm)
 {
     BdrvChild *c;
     uint64_t cumulative_perms = 0;
-- 
2.20.1

blockdev-snapshot returned an error if the overlay was already in use,
which it defined as having any BlockBackend parent. This is in fact both
too strict (some parents can tolerate the change of visible data caused
by attaching a backing file) and too loose (some non-BlockBackend
parents may not be happy with it).

One important use case that is prevented by the too strict check is live
storage migration with blockdev-mirror. Here, the target node is
usually opened without a backing file so that the active layer is
mirrored while its backing chain can be copied in the background.

The backing chain should be attached to the mirror target node when
finalising the job, just before switching the users of the source node
to the new copy (at which point the mirror job still has a reference to
the node). drive-mirror did this automatically, but with blockdev-mirror
this is the job of the QMP client, so it needs a way to do this.

blockdev-snapshot is the obvious way, so this patch makes it work in
this scenario. The new condition is that no parent uses CONSISTENT_READ
permissions. This will ensure that the operation will still be blocked
when the node is attached to the guest device, so blockdev-snapshot
remains safe.

(For the sake of completeness, x-blockdev-reopen can be used to achieve
the same, however it is a big hammer, performs the graph change
completely unchecked and is still experimental. So even with the option
of using x-blockdev-reopen, there are reasons why blockdev-snapshot
should be able to perform this operation.)

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200310113831.27293-3-kwolf@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c                 | 14 ++++++++------
 tests/qemu-iotests/085.out |  4 ++--
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index XXXXXXX..XXXXXXX 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
     TransactionAction *action = common->action;
     AioContext *aio_context;
     AioContext *old_context;
+    uint64_t perm, shared;
     int ret;
 
     /* 'blockdev-snapshot' and 'blockdev-snapshot-sync' have similar
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
         goto out;
     }
 
-    if (bdrv_has_blk(state->new_bs)) {
+    /*
+     * Allow attaching a backing file to an overlay that's already in use only
+     * if the parents don't assume that they are already seeing a valid image.
+     * (Specifically, allow it as a mirror target, which is write-only access.)
+     */
+    bdrv_get_cumulative_perm(state->new_bs, &perm, &shared);
+    if (perm & BLK_PERM_CONSISTENT_READ) {
         error_setg(errp, "The overlay is already in use");
         goto out;
     }
 
-    if (bdrv_op_is_blocked(state->new_bs, BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT,
-                           errp)) {
-        goto out;
-    }
-
     if (state->new_bs->backing != NULL) {
         error_setg(errp, "The overlay already has a backing image");
         goto out;
diff --git a/tests/qemu-iotests/085.out b/tests/qemu-iotests/085.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/085.out
+++ b/tests/qemu-iotests/085.out
@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/12-snapshot-v0.IMGFMT', fmt=IMGFMT size=134217728 backing_f
 === Invalid command - cannot create a snapshot using a file BDS ===
 
 { 'execute': 'blockdev-snapshot', 'arguments': { 'node':'virtio0', 'overlay':'file_12' } }
-{"error": {"class": "GenericError", "desc": "The overlay does not support backing images"}}
+{"error": {"class": "GenericError", "desc": "The overlay is already in use"}}
 
 === Invalid command - snapshot node used as active layer ===
 
@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/12-snapshot-v0.IMGFMT', fmt=IMGFMT size=134217728 backing_f
 === Invalid command - snapshot node used as backing hd ===
 
 { 'execute': 'blockdev-snapshot', 'arguments': { 'node': 'virtio0', 'overlay':'snap_11' } }
-{"error": {"class": "GenericError", "desc": "Node 'snap_11' is busy: node is used as backing hd of 'snap_12'"}}
+{"error": {"class": "GenericError", "desc": "The overlay is already in use"}}
 
 === Invalid command - snapshot node has a backing image ===
 
-- 
2.20.1

The 'job-complete' QMP command should be run with qmp() rather than
qmp_log() if use_log=False is passed.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200310113831.27293-4-kwolf@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/iotests.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -XXX,XX +XXX,XX @@ class VM(qtest.QEMUQtestMachine):
                         if use_log:
                             log('Job failed: %s' % (j['error']))
             elif status == 'ready':
-                self.qmp_log('job-complete', id=job)
+                if use_log:
+                    self.qmp_log('job-complete', id=job)
+                else:
+                    self.qmp('job-complete', id=job)
             elif status == 'pending' and not auto_finalize:
                 if pre_finalize:
                     pre_finalize()
-- 
2.20.1

The newly tested scenario is a common live storage migration scenario:
The target node is opened without a backing file so that the active
layer is mirrored while its backing chain can be copied in the
background.

This patch adds test cases for two ways to achieve the desired result,
using either x-blockdev-reopen or blockdev-snapshot.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200310113831.27293-5-kwolf@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/155     | 56 ++++++++++++++++++++++++++++++++++----
 tests/qemu-iotests/155.out |  4 +--
 2 files changed, 53 insertions(+), 7 deletions(-)

diff --git a/tests/qemu-iotests/155 b/tests/qemu-iotests/155
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/155
+++ b/tests/qemu-iotests/155
@@ -XXX,XX +XXX,XX @@ target_img = os.path.join(iotests.test_dir, 'target.' + iotests.imgfmt)
 #                      image during runtime, only makes sense if
 #                      target_blockdev_backing is not None
 #                      (None: same as target_backing)
+# target_open_with_backing: If True, the target image is added with its backing
+#                           chain opened right away. If False, blockdev-add
+#                           opens it without a backing file and job completion
+#                           is supposed to open the backing chain.
 
 class BaseClass(iotests.QMPTestCase):
     target_blockdev_backing = None
     target_real_backing = None
+    target_open_with_backing = True
 
     def setUp(self):
         qemu_img('create', '-f', iotests.imgfmt, back0_img, '1440K')
@@ -XXX,XX +XXX,XX @@ class BaseClass(iotests.QMPTestCase):
                 options = { 'node-name': 'target',
                             'driver': iotests.imgfmt,
                             'file': { 'driver': 'file',
+                                      'node-name': 'target-file',
                                       'filename': target_img } }
-                if self.target_blockdev_backing:
-                    options['backing'] = self.target_blockdev_backing
+
+                if not self.target_open_with_backing:
+                        options['backing'] = None
+                elif self.target_blockdev_backing:
+                        options['backing'] = self.target_blockdev_backing
 
                 result = self.vm.qmp('blockdev-add', **options)
                 self.assert_qmp(result, 'return', {})
@@ -XXX,XX +XXX,XX @@ class BaseClass(iotests.QMPTestCase):
 # cmd: Mirroring command to execute, either drive-mirror or blockdev-mirror
 
 class MirrorBaseClass(BaseClass):
+    def openBacking(self):
+        pass
+
     def runMirror(self, sync):
         if self.cmd == 'blockdev-mirror':
             result = self.vm.qmp(self.cmd, job_id='mirror-job', device='source',
-                                 sync=sync, target='target')
+                                 sync=sync, target='target',
+                                 auto_finalize=False)
         else:
             if self.existing:
                 mode = 'existing'
@@ -XXX,XX +XXX,XX @@ class MirrorBaseClass(BaseClass):
             result = self.vm.qmp(self.cmd, job_id='mirror-job', device='source',
                                  sync=sync, target=target_img,
                                  format=iotests.imgfmt, mode=mode,
-                                 node_name='target')
+                                 node_name='target', auto_finalize=False)
 
         self.assert_qmp(result, 'return', {})
 
-        self.complete_and_wait('mirror-job')
+        self.vm.run_job('mirror-job', use_log=False, auto_finalize=False,
+                        pre_finalize=self.openBacking, auto_dismiss=True)
 
     def testFull(self):
         self.runMirror('full')
@@ -XXX,XX +XXX,XX @@ class TestBlockdevMirrorForcedBacking(MirrorBaseClass):
     target_blockdev_backing = { 'driver': 'null-co' }
     target_real_backing = 'null-co://'
 
+# Attach the backing chain only during completion, with blockdev-reopen
+class TestBlockdevMirrorReopen(MirrorBaseClass):
+    cmd = 'blockdev-mirror'
+    existing = True
+    target_backing = 'null-co://'
+    target_open_with_backing = False
+
+    def openBacking(self):
+        if not self.target_open_with_backing:
+            result = self.vm.qmp('blockdev-add', node_name="backing",
+                                 driver="null-co")
+            self.assert_qmp(result, 'return', {})
+            result = self.vm.qmp('x-blockdev-reopen', node_name="target",
+                                 driver=iotests.imgfmt, file="target-file",
+                                 backing="backing")
+            self.assert_qmp(result, 'return', {})
+
+# Attach the backing chain only during completion, with blockdev-snapshot
+class TestBlockdevMirrorSnapshot(MirrorBaseClass):
+    cmd = 'blockdev-mirror'
+    existing = True
+    target_backing = 'null-co://'
+    target_open_with_backing = False
+
+    def openBacking(self):
+        if not self.target_open_with_backing:
+            result = self.vm.qmp('blockdev-add', node_name="backing",
+                                 driver="null-co")
+            self.assert_qmp(result, 'return', {})
+            result = self.vm.qmp('blockdev-snapshot', node="backing",
+                                 overlay="target")
+            self.assert_qmp(result, 'return', {})
 
 class TestCommit(BaseClass):
     existing = False
diff --git a/tests/qemu-iotests/155.out b/tests/qemu-iotests/155.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/155.out
+++ b/tests/qemu-iotests/155.out
@@ -XXX,XX +XXX,XX @@
-...................
+.........................
 ----------------------------------------------------------------------
-Ran 19 tests
+Ran 25 tests
 
 OK
-- 
2.20.1

external_snapshot_prepare() tries to move the overlay to the AioContext
of the backing file (the snapshotted node). However, it's possible that
this doesn't work, but the backing file can instead be moved to the
overlay's AioContext (e.g. opening the backing chain for a mirror
target).

bdrv_append() already indirectly uses bdrv_attach_node(), which takes
care to move nodes to make sure they use the same AioContext and which
tries both directions.

So the problem has a simple fix: Just delete the unnecessary extra
bdrv_try_set_aio_context() call in external_snapshot_prepare() and
instead assert in bdrv_append() that both nodes were indeed moved to the
same AioContext.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200310113831.27293-6-kwolf@redhat.com>
Tested-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c    |  1 +
 blockdev.c | 16 ----------------
 2 files changed, 1 insertion(+), 16 deletions(-)

diff --git a/block.c b/block.c
index XXXXXXX..XXXXXXX 100644
--- a/block.c
+++ b/block.c
@@ -XXX,XX +XXX,XX @@ void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
     bdrv_ref(from);
 
     assert(qemu_get_current_aio_context() == qemu_get_aio_context());
+    assert(bdrv_get_aio_context(from) == bdrv_get_aio_context(to));
     bdrv_drained_begin(from);
 
     /* Put all parents into @list and calculate their cumulative permissions */
diff --git a/blockdev.c b/blockdev.c
index XXXXXXX..XXXXXXX 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
                              DO_UPCAST(ExternalSnapshotState, common, common);
     TransactionAction *action = common->action;
     AioContext *aio_context;
-    AioContext *old_context;
     uint64_t perm, shared;
-    int ret;
 
     /* 'blockdev-snapshot' and 'blockdev-snapshot-sync' have similar
      * purpose but a different set of parameters */
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
         goto out;
     }
 
-    /* Honor bdrv_try_set_aio_context() context acquisition requirements. */
-    old_context = bdrv_get_aio_context(state->new_bs);
-    aio_context_release(aio_context);
-    aio_context_acquire(old_context);
-
-    ret = bdrv_try_set_aio_context(state->new_bs, aio_context, errp);
-
-    aio_context_release(old_context);
-    aio_context_acquire(aio_context);
-
-    if (ret < 0) {
-        goto out;
-    }
-
     /* This removes our old bs and adds the new bs. This is an operation that
      * can fail, so we need to do it in .prepare; undoing it for abort is
      * always possible. */
-- 
2.20.1

This patch adds test cases for attaching the backing chain to a mirror
job target right before finalising the job, where the image is in a
non-mainloop AioContext (i.e. the backing chain needs to be moved to the
AioContext of the mirror target).

This requires switching the test case from virtio-blk to virtio-scsi
because virtio-blk only actually starts using the iothreads when the
guest driver initialises the device (which never happens in a test case
without a guest OS). virtio-scsi always keeps its block nodes in the
AioContext of the the requested iothread without guest interaction.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200310113831.27293-7-kwolf@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/155     | 32 +++++++++++++++++++++++---------
 tests/qemu-iotests/155.out |  4 ++--
 2 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/tests/qemu-iotests/155 b/tests/qemu-iotests/155
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/155
+++ b/tests/qemu-iotests/155
@@ -XXX,XX +XXX,XX @@ target_img = os.path.join(iotests.test_dir, 'target.' + iotests.imgfmt)
 #                           chain opened right away. If False, blockdev-add
 #                           opens it without a backing file and job completion
 #                           is supposed to open the backing chain.
+# use_iothread: If True, an iothread is configured for the virtio-blk device
+#               that uses the image being mirrored
 
 class BaseClass(iotests.QMPTestCase):
     target_blockdev_backing = None
     target_real_backing = None
     target_open_with_backing = True
+    use_iothread = False
 
     def setUp(self):
         qemu_img('create', '-f', iotests.imgfmt, back0_img, '1440K')
@@ -XXX,XX +XXX,XX @@ class BaseClass(iotests.QMPTestCase):
                     'file': {'driver': 'file',
                              'filename': source_img}}
         self.vm.add_blockdev(self.vm.qmp_to_opts(blockdev))
-        self.vm.add_device('virtio-blk,id=qdev0,drive=source')
+
+        if self.use_iothread:
+            self.vm.add_object('iothread,id=iothread0')
+            iothread = ",iothread=iothread0"
+        else:
+            iothread = ""
+
+        self.vm.add_device('virtio-scsi%s' % iothread)
+        self.vm.add_device('scsi-hd,id=qdev0,drive=source')
+
         self.vm.launch()
 
         self.assertIntactSourceBackingChain()
@@ -XXX,XX +XXX,XX @@ class MirrorBaseClass(BaseClass):
     def testFull(self):
         self.runMirror('full')
 
-        node = self.findBlockNode('target',
-                                  '/machine/peripheral/qdev0/virtio-backend')
+        node = self.findBlockNode('target', 'qdev0')
         self.assertCorrectBackingImage(node, None)
         self.assertIntactSourceBackingChain()
 
     def testTop(self):
         self.runMirror('top')
 
-        node = self.findBlockNode('target',
-                                  '/machine/peripheral/qdev0/virtio-backend')
+        node = self.findBlockNode('target', 'qdev0')
         self.assertCorrectBackingImage(node, back2_img)
         self.assertIntactSourceBackingChain()
 
     def testNone(self):
         self.runMirror('none')
 
-        node = self.findBlockNode('target',
-                                  '/machine/peripheral/qdev0/virtio-backend')
+        node = self.findBlockNode('target', 'qdev0')
         self.assertCorrectBackingImage(node, source_img)
         self.assertIntactSourceBackingChain()
 
@@ -XXX,XX +XXX,XX @@ class TestBlockdevMirrorReopen(MirrorBaseClass):
                                  backing="backing")
             self.assert_qmp(result, 'return', {})
 
+class TestBlockdevMirrorReopenIothread(TestBlockdevMirrorReopen):
+    use_iothread = True
+
 # Attach the backing chain only during completion, with blockdev-snapshot
 class TestBlockdevMirrorSnapshot(MirrorBaseClass):
     cmd = 'blockdev-mirror'
@@ -XXX,XX +XXX,XX @@ class TestBlockdevMirrorSnapshot(MirrorBaseClass):
                                  overlay="target")
             self.assert_qmp(result, 'return', {})
 
+class TestBlockdevMirrorSnapshotIothread(TestBlockdevMirrorSnapshot):
+    use_iothread = True
+
 class TestCommit(BaseClass):
     existing = False
 
@@ -XXX,XX +XXX,XX @@ class TestCommit(BaseClass):
 
         self.vm.event_wait('BLOCK_JOB_COMPLETED')
 
-        node = self.findBlockNode(None,
-                                  '/machine/peripheral/qdev0/virtio-backend')
+        node = self.findBlockNode(None, 'qdev0')
         self.assert_qmp(node, 'image' + '/backing-image' * 0 + '/filename',
                         back1_img)
         self.assert_qmp(node, 'image' + '/backing-image' * 1 + '/filename',
diff --git a/tests/qemu-iotests/155.out b/tests/qemu-iotests/155.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/155.out
+++ b/tests/qemu-iotests/155.out
@@ -XXX,XX +XXX,XX @@
-.........................
+...............................
 ----------------------------------------------------------------------
-Ran 25 tests
+Ran 31 tests
 
 OK
-- 
2.20.1

From: Peter Krempa <pkrempa@redhat.com>

Anounce that 'blockdev-snapshot' command's permissions allow changing
of the backing file if the 'consistent_read' permission is not required.

This is useful for libvirt to allow late opening of the backing chain
during a blockdev-mirror.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200310113831.27293-8-kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qapi/block-core.json | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index XXXXXXX..XXXXXXX 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -XXX,XX +XXX,XX @@
 #
 # For the arguments, see the documentation of BlockdevSnapshot.
 #
+# Features:
+# @allow-write-only-overlay: If present, the check whether this operation is safe
+#                            was relaxed so that it can be used to change
+#                            backing file of a destination of a blockdev-mirror.
+#                            (since 5.0)
+#
 # Since: 2.5
 #
 # Example:
@@ -XXX,XX +XXX,XX @@
 #
 ##
 { 'command': 'blockdev-snapshot',
-  'data': 'BlockdevSnapshot' }
+  'data': 'BlockdevSnapshot',
+  'features': [ 'allow-write-only-overlay' ] }
 
 ##
 # @change-backing-file:
-- 
2.20.1

From: Philippe Mathieu-Daudé <philmd@redhat.com>

The socket_scm_helper path got corrupted during the mechanical
refactor moving the qtests files into their own sub-directory.

Fixes: 1e8a1fae7 ("test: Move qtests to a separate directory")
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200306165751.18986-1-philmd@redhat.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/Makefile.include       | 1 +
 tests/qtest/Makefile.include | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index XXXXXXX..XXXXXXX 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -XXX,XX +XXX,XX @@ include $(SRC_PATH)/tests/qtest/Makefile.include
 tests/test-qga$(EXESUF): qemu-ga$(EXESUF)
 tests/test-qga$(EXESUF): tests/test-qga.o $(qtest-obj-y)
 tests/vhost-user-bridge$(EXESUF): tests/vhost-user-bridge.o $(test-util-obj-y) libvhost-user.a
+tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o
 
 SPEED = quick
 
diff --git a/tests/qtest/Makefile.include b/tests/qtest/Makefile.include
index XXXXXXX..XXXXXXX 100644
--- a/tests/qtest/Makefile.include
+++ b/tests/qtest/Makefile.include
@@ -XXX,XX +XXX,XX @@ tests/qtest/usb-hcd-ehci-test$(EXESUF): tests/qtest/usb-hcd-ehci-test.o $(libqos
 tests/qtest/usb-hcd-xhci-test$(EXESUF): tests/qtest/usb-hcd-xhci-test.o $(libqos-usb-obj-y)
 tests/qtest/cpu-plug-test$(EXESUF): tests/qtest/cpu-plug-test.o
 tests/qtest/migration-test$(EXESUF): tests/qtest/migration-test.o tests/qtest/migration-helpers.o
-tests/qtest/qemu-iotests/qtest/socket_scm_helper$(EXESUF): tests/qtest/qemu-iotests/qtest/socket_scm_helper.o
 tests/qtest/test-netfilter$(EXESUF): tests/qtest/test-netfilter.o $(qtest-obj-y)
 tests/qtest/test-filter-mirror$(EXESUF): tests/qtest/test-filter-mirror.o $(qtest-obj-y)
 tests/qtest/test-filter-redirector$(EXESUF): tests/qtest/test-filter-redirector.o $(qtest-obj-y)
-- 
2.20.1

From: Daniel Henrique Barboza <danielhb413@gmail.com>

Adding to Block Drivers the capability of being able to clean up
its created files can be useful in certain situations. For the
LUKS driver, for instance, a failure in one of its authentication
steps can leave files in the host that weren't there before.

This patch adds the 'bdrv_co_delete_file' interface to block
drivers and add it to the 'file' driver in file-posix.c. The
implementation is given by 'raw_co_delete_file'.

Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Message-Id: <20200130213907.2830642-2-danielhb413@gmail.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 include/block/block_int.h |  4 ++++
 block/file-posix.c        | 23 +++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -XXX,XX +XXX,XX @@ struct BlockDriver {
      */
     int coroutine_fn (*bdrv_co_flush)(BlockDriverState *bs);
 
+    /* Delete a created file. */
+    int coroutine_fn (*bdrv_co_delete_file)(BlockDriverState *bs,
+                                            Error **errp);
+
     /*
      * Flushes all data that was already written to the OS all the way down to
      * the disk (for example file-posix.c calls fsync()).
diff --git a/block/file-posix.c b/block/file-posix.c
index XXXXXXX..XXXXXXX 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_create_opts(const char *filename, QemuOpts *opts,
     return raw_co_create(&options, errp);
 }
 
+static int coroutine_fn raw_co_delete_file(BlockDriverState *bs,
+                                           Error **errp)
+{
+    struct stat st;
+    int ret;
+
+    if (!(stat(bs->filename, &st) == 0) || !S_ISREG(st.st_mode)) {
+        error_setg_errno(errp, ENOENT, "%s is not a regular file",
+                         bs->filename);
+        return -ENOENT;
+    }
+
+    ret = unlink(bs->filename);
+    if (ret < 0) {
+        ret = -errno;
+        error_setg_errno(errp, -ret, "Error when deleting file %s",
+                         bs->filename);
+    }
+
+    return ret;
+}
+
 /*
  * Find allocation range in @bs around offset @start.
  * May change underlying file descriptor's file offset.
@@ -XXX,XX +XXX,XX @@ BlockDriver bdrv_file = {
     .bdrv_co_block_status = raw_co_block_status,
     .bdrv_co_invalidate_cache = raw_co_invalidate_cache,
     .bdrv_co_pwrite_zeroes = raw_co_pwrite_zeroes,
+    .bdrv_co_delete_file = raw_co_delete_file,
 
     .bdrv_co_preadv         = raw_co_preadv,
     .bdrv_co_pwritev        = raw_co_pwritev,
-- 
2.20.1

From: Daniel Henrique Barboza <danielhb413@gmail.com>

Using the new 'bdrv_co_delete_file' interface, a pure co_routine function
'bdrv_co_delete_file' inside block.c can can be used in a way similar of
the existing bdrv_create_file to to clean up a created file.

We're creating a pure co_routine because the only caller of
'bdrv_co_delete_file' will be already in co_routine context, thus there
is no need to add all the machinery to check for qemu_in_coroutine() and
create a separated co_routine to do the job.

Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Message-Id: <20200130213907.2830642-3-danielhb413@gmail.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 include/block/block.h |  1 +
 block.c               | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/include/block/block.h b/include/block/block.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -XXX,XX +XXX,XX @@ bool bdrv_is_backing_chain_frozen(BlockDriverState *bs, BlockDriverState *base,
 int bdrv_freeze_backing_chain(BlockDriverState *bs, BlockDriverState *base,
                               Error **errp);
 void bdrv_unfreeze_backing_chain(BlockDriverState *bs, BlockDriverState *base);
+int coroutine_fn bdrv_co_delete_file(BlockDriverState *bs, Error **errp);
 
 
 typedef struct BdrvCheckResult {
diff --git a/block.c b/block.c
index XXXXXXX..XXXXXXX 100644
--- a/block.c
+++ b/block.c
@@ -XXX,XX +XXX,XX @@ int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp)
     }
 }
 
+int coroutine_fn bdrv_co_delete_file(BlockDriverState *bs, Error **errp)
+{
+    Error *local_err = NULL;
+    int ret;
+
+    assert(bs != NULL);
+
+    if (!bs->drv) {
+        error_setg(errp, "Block node '%s' is not opened", bs->filename);
+        return -ENOMEDIUM;
+    }
+
+    if (!bs->drv->bdrv_co_delete_file) {
+        error_setg(errp, "Driver '%s' does not support image deletion",
+                   bs->drv->format_name);
+        return -ENOTSUP;
+    }
+
+    ret = bs->drv->bdrv_co_delete_file(bs, &local_err);
+    if (ret < 0) {
+        error_propagate(errp, local_err);
+    }
+
+    return ret;
+}
+
 /**
  * Try to get @bs's logical and physical block size.
  * On success, store them in @bsz struct and return 0.
-- 
2.20.1

From: Daniel Henrique Barboza <danielhb413@gmail.com>

When using a non-UTF8 secret to create a volume using qemu-img, the
following error happens:

$ qemu-img create -f luks --object secret,id=vol_1_encrypt0,file=vol_resize_pool.vol_1.secret.qzVQrI -o key-secret=vol_1_encrypt0 /var/tmp/pool_target/vol_1 10240K

Formatting '/var/tmp/pool_target/vol_1', fmt=luks size=10485760 key-secret=vol_1_encrypt0
qemu-img: /var/tmp/pool_target/vol_1: Data from secret vol_1_encrypt0 is not valid UTF-8

However, the created file '/var/tmp/pool_target/vol_1' is left behind in the
file system after the failure. This behavior can be observed when creating
the volume using Libvirt, via 'virsh vol-create', and then getting "volume
target path already exist" errors when trying to re-create the volume.

The volume file is created inside block_crypto_co_create_opts_luks(), in
block/crypto.c. If the bdrv_create_file() call is successful but any
succeeding step fails*, the existing 'fail' label does not take into
account the created file, leaving it behind.

This patch changes block_crypto_co_create_opts_luks() to delete
'filename' in case of failure. A failure in this point means that
the volume is now truncated/corrupted, so even if 'filename' was an
existing volume before calling qemu-img, it is now unusable. Deleting
the file it is not much worse than leaving it in the filesystem in
this scenario, and we don't have to deal with checking the file
pre-existence in the code.

* in our case, block_crypto_co_create_generic calls qcrypto_block_create,
which calls qcrypto_block_luks_create, and this function fails when
calling qcrypto_secret_lookup_as_utf8.

Reported-by: Srikanth Aithal <bssrikanth@in.ibm.com>
Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Message-Id: <20200130213907.2830642-4-danielhb413@gmail.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/crypto.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/block/crypto.c b/block/crypto.c
index XXXXXXX..XXXXXXX 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -XXX,XX +XXX,XX @@
 #include "qapi/error.h"
 #include "qemu/module.h"
 #include "qemu/option.h"
+#include "qemu/cutils.h"
 #include "crypto.h"
 
 typedef struct BlockCrypto BlockCrypto;
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn block_crypto_co_create_opts_luks(const char *filename,
 
     ret = 0;
 fail:
+    /*
+     * If an error occurred, delete 'filename'. Even if the file existed
+     * beforehand, it has been truncated and corrupted in the process.
+     */
+    if (ret && bs) {
+        Error *local_delete_err = NULL;
+        int r_del = bdrv_co_delete_file(bs, &local_delete_err);
+        /*
+         * ENOTSUP will happen if the block driver doesn't support
+         * the 'bdrv_co_delete_file' interface. This is a predictable
+         * scenario and shouldn't be reported back to the user.
+         */
+        if ((r_del < 0) && (r_del != -ENOTSUP)) {
+            error_report_err(local_delete_err);
+        }
+    }
+
     bdrv_unref(bs);
     qapi_free_QCryptoBlockCreateOptions(create_opts);
     qobject_unref(cryptoopts);
-- 
2.20.1

From: Daniel Henrique Barboza <danielhb413@gmail.com>

This patch adds a new test file to exercise the case where
qemu-img fails to complete for the LUKS format when a non-UTF8
secret is used.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Message-Id: <20200130213907.2830642-5-danielhb413@gmail.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/282     | 67 ++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/282.out | 11 +++++++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 79 insertions(+)
 create mode 100755 tests/qemu-iotests/282
 create mode 100644 tests/qemu-iotests/282.out

diff --git a/tests/qemu-iotests/282 b/tests/qemu-iotests/282
new file mode 100755
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qemu-iotests/282
@@ -XXX,XX +XXX,XX @@
+#!/usr/bin/env bash
+#
+# Test qemu-img file cleanup for LUKS when using a non-UTF8 secret
+#
+# Copyright (C) 2020, IBM Corporation.
+#
+# 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/>.
+#
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+status=1	# failure is the default!
+TEST_IMAGE_FILE='vol.img'
+
+_cleanup()
+{
+  _cleanup_test_img
+  rm non_utf8_secret
+  rm -f $TEST_IMAGE_FILE
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt luks
+_supported_proto generic
+_unsupported_proto vxhs
+
+echo "== Create non-UTF8 secret =="
+echo -n -e '\x3a\x3c\x3b\xff' > non_utf8_secret
+SECRET="secret,id=sec0,file=non_utf8_secret"
+
+echo "== Throws an error because of invalid UTF-8 secret =="
+$QEMU_IMG create -f $IMGFMT --object $SECRET -o "key-secret=sec0" $TEST_IMAGE_FILE 4M
+
+echo "== Image file should not exist after the error =="
+if test -f "$TEST_IMAGE_FILE"; then
+    exit 1
+fi
+
+echo "== Create a stub image file and run qemu-img again =="
+touch $TEST_IMAGE_FILE
+$QEMU_IMG create -f $IMGFMT --object $SECRET -o "key-secret=sec0" $TEST_IMAGE_FILE 4M
+
+echo "== Pre-existing image file should also be deleted after the error =="
+if test -f "$TEST_IMAGE_FILE"; then
+    exit 1
+fi
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/282.out b/tests/qemu-iotests/282.out
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qemu-iotests/282.out
@@ -XXX,XX +XXX,XX @@
+QA output created by 282
+== Create non-UTF8 secret ==
+== Throws an error because of invalid UTF-8 secret ==
+qemu-img: vol.img: Data from secret sec0 is not valid UTF-8
+Formatting 'vol.img', fmt=luks size=4194304 key-secret=sec0
+== Image file should not exist after the error ==
+== Create a stub image file and run qemu-img again ==
+qemu-img: vol.img: Data from secret sec0 is not valid UTF-8
+Formatting 'vol.img', fmt=luks size=4194304 key-secret=sec0
+== Pre-existing image file should also be deleted after the error ==
+ *** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -XXX,XX +XXX,XX @@
 279 rw backing quick
 280 rw migration quick
 281 rw quick
+282 rw img quick
 283 auto quick
 284 rw
 286 rw quick
-- 
2.20.1

The following changes since commit 7f21573c822805a8e6be379d9bcf3ad9effef3dc:

Merge remote-tracking branch 'remotes/huth-gitlab/tags/pull-request-2019-10-01' into staging (2019-10-01 13:13:38 +0100)

are available in the Git repository at:

git://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to 352492e5541e293ea8117412ffd171ce1c7ac15d:

iotests: Remove Python 2 compatibility code (2019-10-02 16:47:23 +0200)

----------------------------------------------------------------
Block layer patches:

- Fix internal snapshots with typical -blockdev setups
- iotests: Require Python 3.6 or later

----------------------------------------------------------------
Kevin Wolf (4):
      block/snapshot: Restrict set of snapshot nodes
      iotests: Test internal snapshots with -blockdev
      iotests: Require Python 3.6 or later
      iotests: Remove Python 2 compatibility code

block/snapshot.c                 |  26 ++++--
 tests/qemu-iotests/044           |   3 -
 tests/qemu-iotests/163           |   3 -
 tests/qemu-iotests/267           | 168 ++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/267.out       | 182 +++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/check         |  13 ++-
 tests/qemu-iotests/common.filter |  11 ++-
 tests/qemu-iotests/group         |   1 +
 tests/qemu-iotests/iotests.py    |  13 +--
 9 files changed, 392 insertions(+), 28 deletions(-)
 create mode 100755 tests/qemu-iotests/267
 create mode 100644 tests/qemu-iotests/267.out

Nodes involved in internal snapshots were those that were returned by
bdrv_next(), inserted and not read-only. bdrv_next() in turn returns all
nodes that are either the root node of a BlockBackend or monitor-owned
nodes.

With the typical -drive use, this worked well enough. However, in the
typical -blockdev case, the user defines one node per option, making all
nodes monitor-owned nodes. This includes protocol nodes etc. which often
are not snapshottable, so "savevm" only returns an error.

Change the conditions so that internal snapshot still include all nodes
that have a BlockBackend attached (we definitely want to snapshot
anything attached to a guest device and probably also the built-in NBD
server; snapshotting block job BlockBackends is more of an accident, but
a preexisting one), but other monitor-owned nodes are only included if
they have no parents.

This makes internal snapshots usable again with typical -blockdev
configurations.

Cc: qemu-stable@nongnu.org
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Peter Krempa <pkrempa@redhat.com>
---
 block/snapshot.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/block/snapshot.c b/block/snapshot.c
index XXXXXXX..XXXXXXX 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -XXX,XX +XXX,XX @@
 #include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qstring.h"
 #include "qemu/option.h"
+#include "sysemu/block-backend.h"
 
 QemuOptsList internal_snapshot_opts = {
     .name = "snapshot",
@@ -XXX,XX +XXX,XX @@ int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState *bs,
     return ret;
 }
 
+static bool bdrv_all_snapshots_includes_bs(BlockDriverState *bs)
+{
+    if (!bdrv_is_inserted(bs) || bdrv_is_read_only(bs)) {
+        return false;
+    }
+
+    /* Include all nodes that are either in use by a BlockBackend, or that
+     * aren't attached to any node, but owned by the monitor. */
+    return bdrv_has_blk(bs) || QLIST_EMPTY(&bs->parents);
+}
 
 /* Group operations. All block drivers are involved.
  * These functions will properly handle dataplane (take aio_context_acquire
@@ -XXX,XX +XXX,XX @@ bool bdrv_all_can_snapshot(BlockDriverState **first_bad_bs)
         AioContext *ctx = bdrv_get_aio_context(bs);
 
         aio_context_acquire(ctx);
-        if (bdrv_is_inserted(bs) && !bdrv_is_read_only(bs)) {
+        if (bdrv_all_snapshots_includes_bs(bs)) {
             ok = bdrv_can_snapshot(bs);
         }
         aio_context_release(ctx);
@@ -XXX,XX +XXX,XX @@ int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bad_bs,
         AioContext *ctx = bdrv_get_aio_context(bs);
 
         aio_context_acquire(ctx);
-        if (bdrv_can_snapshot(bs) &&
-                bdrv_snapshot_find(bs, snapshot, name) >= 0) {
+        if (bdrv_all_snapshots_includes_bs(bs) &&
+            bdrv_snapshot_find(bs, snapshot, name) >= 0)
+        {
             ret = bdrv_snapshot_delete(bs, snapshot->id_str,
                                        snapshot->name, err);
         }
@@ -XXX,XX +XXX,XX @@ int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs,
         AioContext *ctx = bdrv_get_aio_context(bs);
 
         aio_context_acquire(ctx);
-        if (bdrv_can_snapshot(bs)) {
+        if (bdrv_all_snapshots_includes_bs(bs)) {
             ret = bdrv_snapshot_goto(bs, name, errp);
         }
         aio_context_release(ctx);
@@ -XXX,XX +XXX,XX @@ int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs)
         AioContext *ctx = bdrv_get_aio_context(bs);
 
         aio_context_acquire(ctx);
-        if (bdrv_can_snapshot(bs)) {
+        if (bdrv_all_snapshots_includes_bs(bs)) {
             err = bdrv_snapshot_find(bs, &sn, name);
         }
         aio_context_release(ctx);
@@ -XXX,XX +XXX,XX @@ int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn,
         if (bs == vm_state_bs) {
             sn->vm_state_size = vm_state_size;
             err = bdrv_snapshot_create(bs, sn);
-        } else if (bdrv_can_snapshot(bs)) {
+        } else if (bdrv_all_snapshots_includes_bs(bs)) {
             sn->vm_state_size = 0;
             err = bdrv_snapshot_create(bs, sn);
         }
@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_all_find_vmstate_bs(void)
         bool found;
 
         aio_context_acquire(ctx);
-        found = bdrv_can_snapshot(bs);
+        found = bdrv_all_snapshots_includes_bs(bs) && bdrv_can_snapshot(bs);
         aio_context_release(ctx);
 
         if (found) {
-- 
2.20.1

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Peter Krempa <pkrempa@redhat.com>
---
 tests/qemu-iotests/267           | 168 ++++++++++++++++++++++++++++
 tests/qemu-iotests/267.out       | 182 +++++++++++++++++++++++++++++++
 tests/qemu-iotests/common.filter |  11 +-
 tests/qemu-iotests/group         |   1 +
 4 files changed, 358 insertions(+), 4 deletions(-)
 create mode 100755 tests/qemu-iotests/267
 create mode 100644 tests/qemu-iotests/267.out

diff --git a/tests/qemu-iotests/267 b/tests/qemu-iotests/267
new file mode 100755
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qemu-iotests/267
@@ -XXX,XX +XXX,XX @@
+#!/usr/bin/env bash
+#
+# Test which nodes are involved in internal snapshots
+#
+# Copyright (C) 2019 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/>.
+#
+
+# creator
+owner=kwolf@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+status=1	# failure is the default!
+
+_cleanup()
+{
+    _cleanup_test_img
+    rm -f "$TEST_DIR/nbd"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+
+# Internal snapshots are (currently) impossible with refcount_bits=1
+_unsupported_imgopts 'refcount_bits=1[^0-9]'
+
+do_run_qemu()
+{
+    echo Testing: "$@"
+    (
+        if ! test -t 0; then
+            while read cmd; do
+                echo $cmd
+            done
+        fi
+        echo quit
+    ) | $QEMU -nographic -monitor stdio -nodefaults "$@"
+    echo
+}
+
+run_qemu()
+{
+    do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qemu | _filter_hmp |
+        _filter_generated_node_ids | _filter_imgfmt | _filter_vmstate_size
+}
+
+size=128M
+
+run_test()
+{
+    _make_test_img $size
+    printf "savevm snap0\ninfo snapshots\nloadvm snap0\n" | run_qemu "$@" | _filter_date
+}
+
+
+echo
+echo "=== No block devices at all ==="
+echo
+
+run_test
+
+echo
+echo "=== -drive if=none ==="
+echo
+
+run_test -drive driver=file,file="$TEST_IMG",if=none
+run_test -drive driver=$IMGFMT,file="$TEST_IMG",if=none
+run_test -drive driver=$IMGFMT,file="$TEST_IMG",if=none -device virtio-blk,drive=none0
+
+echo
+echo "=== -drive if=virtio ==="
+echo
+
+run_test -drive driver=file,file="$TEST_IMG",if=virtio
+run_test -drive driver=$IMGFMT,file="$TEST_IMG",if=virtio
+
+echo
+echo "=== Simple -blockdev ==="
+echo
+
+run_test -blockdev driver=file,filename="$TEST_IMG",node-name=file
+run_test -blockdev driver=file,filename="$TEST_IMG",node-name=file \
+         -blockdev driver=$IMGFMT,file=file,node-name=fmt
+run_test -blockdev driver=file,filename="$TEST_IMG",node-name=file \
+         -blockdev driver=raw,file=file,node-name=raw \
+         -blockdev driver=$IMGFMT,file=raw,node-name=fmt
+
+echo
+echo "=== -blockdev with a filter on top ==="
+echo
+
+run_test -blockdev driver=file,filename="$TEST_IMG",node-name=file \
+         -blockdev driver=$IMGFMT,file=file,node-name=fmt \
+         -blockdev driver=copy-on-read,file=fmt,node-name=filter
+
+echo
+echo "=== -blockdev with a backing file ==="
+echo
+
+TEST_IMG="$TEST_IMG.base" _make_test_img $size
+
+IMGOPTS="backing_file=$TEST_IMG.base" \
+run_test -blockdev driver=file,filename="$TEST_IMG.base",node-name=backing-file \
+         -blockdev driver=file,filename="$TEST_IMG",node-name=file \
+         -blockdev driver=$IMGFMT,file=file,backing=backing-file,node-name=fmt
+
+IMGOPTS="backing_file=$TEST_IMG.base" \
+run_test -blockdev driver=file,filename="$TEST_IMG.base",node-name=backing-file \
+         -blockdev driver=$IMGFMT,file=backing-file,node-name=backing-fmt \
+         -blockdev driver=file,filename="$TEST_IMG",node-name=file \
+         -blockdev driver=$IMGFMT,file=file,backing=backing-fmt,node-name=fmt
+
+# A snapshot should be present on the overlay, but not the backing file
+echo Internal snapshots on overlay:
+$QEMU_IMG snapshot -l "$TEST_IMG" | _filter_date | _filter_vmstate_size
+
+echo Internal snapshots on backing file:
+$QEMU_IMG snapshot -l "$TEST_IMG.base" | _filter_date | _filter_vmstate_size
+
+echo
+echo "=== -blockdev with NBD server on the backing file ==="
+echo
+
+IMGOPTS="backing_file=$TEST_IMG.base" _make_test_img $size
+cat <<EOF |
+nbd_server_start unix:$TEST_DIR/nbd
+nbd_server_add -w backing-fmt
+savevm snap0
+info snapshots
+loadvm snap0
+EOF
+run_qemu -blockdev driver=file,filename="$TEST_IMG.base",node-name=backing-file \
+         -blockdev driver=$IMGFMT,file=backing-file,node-name=backing-fmt \
+         -blockdev driver=file,filename="$TEST_IMG",node-name=file \
+         -blockdev driver=$IMGFMT,file=file,backing=backing-fmt,node-name=fmt |
+         _filter_date
+
+# This time, a snapshot should be created on both files
+echo Internal snapshots on overlay:
+$QEMU_IMG snapshot -l "$TEST_IMG" | _filter_date | _filter_vmstate_size
+
+echo Internal snapshots on backing file:
+$QEMU_IMG snapshot -l "$TEST_IMG.base" | _filter_date | _filter_vmstate_size
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/267.out b/tests/qemu-iotests/267.out
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qemu-iotests/267.out
@@ -XXX,XX +XXX,XX @@
+QA output created by 267
+
+=== No block devices at all ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing:
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+Error: No block device can accept snapshots
+(qemu) info snapshots
+No available block device supports snapshots
+(qemu) loadvm snap0
+Error: No block device supports snapshots
+(qemu) quit
+
+
+=== -drive if=none ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing: -drive driver=file,file=TEST_DIR/t.IMGFMT,if=none
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+Error: Device 'none0' is writable but does not support snapshots
+(qemu) info snapshots
+No available block device supports snapshots
+(qemu) loadvm snap0
+Error: Device 'none0' is writable but does not support snapshots
+(qemu) quit
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing: -drive driver=IMGFMT,file=TEST_DIR/t.IMGFMT,if=none
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+(qemu) info snapshots
+List of snapshots present on all disks:
+ID        TAG                 VM SIZE                DATE       VM CLOCK
+--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+(qemu) loadvm snap0
+(qemu) quit
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing: -drive driver=IMGFMT,file=TEST_DIR/t.IMGFMT,if=none -device virtio-blk,drive=none0
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+(qemu) info snapshots
+List of snapshots present on all disks:
+ID        TAG                 VM SIZE                DATE       VM CLOCK
+--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+(qemu) loadvm snap0
+(qemu) quit
+
+
+=== -drive if=virtio ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing: -drive driver=file,file=TEST_DIR/t.IMGFMT,if=virtio
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+Error: Device 'virtio0' is writable but does not support snapshots
+(qemu) info snapshots
+No available block device supports snapshots
+(qemu) loadvm snap0
+Error: Device 'virtio0' is writable but does not support snapshots
+(qemu) quit
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing: -drive driver=IMGFMT,file=TEST_DIR/t.IMGFMT,if=virtio
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+(qemu) info snapshots
+List of snapshots present on all disks:
+ID        TAG                 VM SIZE                DATE       VM CLOCK
+--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+(qemu) loadvm snap0
+(qemu) quit
+
+
+=== Simple -blockdev ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+Error: Device '' is writable but does not support snapshots
+(qemu) info snapshots
+No available block device supports snapshots
+(qemu) loadvm snap0
+Error: Device '' is writable but does not support snapshots
+(qemu) quit
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=IMGFMT,file=file,node-name=fmt
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+(qemu) info snapshots
+List of snapshots present on all disks:
+ID        TAG                 VM SIZE                DATE       VM CLOCK
+--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+(qemu) loadvm snap0
+(qemu) quit
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=raw,file=file,node-name=raw -blockdev driver=IMGFMT,file=raw,node-name=fmt
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+(qemu) info snapshots
+List of snapshots present on all disks:
+ID        TAG                 VM SIZE                DATE       VM CLOCK
+--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+(qemu) loadvm snap0
+(qemu) quit
+
+
+=== -blockdev with a filter on top ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=IMGFMT,file=file,node-name=fmt -blockdev driver=copy-on-read,file=fmt,node-name=filter
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+(qemu) info snapshots
+List of snapshots present on all disks:
+ID        TAG                 VM SIZE                DATE       VM CLOCK
+--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+(qemu) loadvm snap0
+(qemu) quit
+
+
+=== -blockdev with a backing file ===
+
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT.base,node-name=backing-file -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=IMGFMT,file=file,backing=backing-file,node-name=fmt
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+(qemu) info snapshots
+List of snapshots present on all disks:
+ID        TAG                 VM SIZE                DATE       VM CLOCK
+--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+(qemu) loadvm snap0
+(qemu) quit
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT.base,node-name=backing-file -blockdev driver=IMGFMT,file=backing-file,node-name=backing-fmt -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=IMGFMT,file=file,backing=backing-fmt,node-name=fmt
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+(qemu) info snapshots
+List of snapshots present on all disks:
+ID        TAG                 VM SIZE                DATE       VM CLOCK
+--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+(qemu) loadvm snap0
+(qemu) quit
+
+Internal snapshots on overlay:
+Snapshot list:
+ID        TAG                 VM SIZE                DATE       VM CLOCK
+1         snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+Internal snapshots on backing file:
+
+=== -blockdev with NBD server on the backing file ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT.base,node-name=backing-file -blockdev driver=IMGFMT,file=backing-file,node-name=backing-fmt -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=IMGFMT,file=file,backing=backing-fmt,node-name=fmt
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) nbd_server_start unix:TEST_DIR/nbd
+(qemu) nbd_server_add -w backing-fmt
+(qemu) savevm snap0
+(qemu) info snapshots
+List of snapshots present on all disks:
+ID        TAG                 VM SIZE                DATE       VM CLOCK
+--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+(qemu) loadvm snap0
+(qemu) quit
+
+Internal snapshots on overlay:
+Snapshot list:
+ID        TAG                 VM SIZE                DATE       VM CLOCK
+1         snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+Internal snapshots on backing file:
+Snapshot list:
+ID        TAG                 VM SIZE                DATE       VM CLOCK
+1         snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+*** done
diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -XXX,XX +XXX,XX @@
 # standard filters
 #
 
-# ctime(3) dates
-#
 _filter_date()
 {
-    $SED \
-        -e 's/[A-Z][a-z][a-z] [A-z][a-z][a-z]  *[0-9][0-9]* [0-9][0-9]:[0-9][0-9]:[0-9][0-9] [0-9][0-9][0-9][0-9]$/DATE/'
+    $SED -re 's/[0-9]{4}-[0-9]{2}-[0-9]{2} [0-9]{2}:[0-9]{2}:[0-9]{2}/yyyy-mm-dd hh:mm:ss/'
+}
+
+_filter_vmstate_size()
+{
+    $SED -re 's/[0-9. ]{5} [KMGT]iB/     SIZE/' \
+         -re 's/[0-9. ]{5} B/   SIZE/'
 }
 
 _filter_generated_node_ids()
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -XXX,XX +XXX,XX @@
 263 rw quick
 265 rw auto quick
 266 rw quick
+267 rw auto quick snapshot
-- 
2.20.1

Running iotests is not required to build QEMU, so we can have stricter
version requirements for Python here and can make use of new features
and drop compatibility code earlier.

This makes qemu-iotests skip all Python tests if a Python version before
3.6 is used for the build.

Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/check | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -XXX,XX +XXX,XX @@ then
     export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper"
 fi
 
+python_usable=false
+if $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)'
+then
+    python_usable=true
+fi
+
 default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p')
 default_alias_machine=$($QEMU_PROG -machine help | \
    sed -n "/(alias of $default_machine)/ { s/ .*//p; q; }")
@@ -XXX,XX +XXX,XX @@ do
         start=$(_wallclock)
 
         if [ "$(head -n 1 "$source_iotests/$seq")" == "#!/usr/bin/env python" ]; then
-            run_command="$PYTHON $seq"
+            if $python_usable; then
+                run_command="$PYTHON $seq"
+            else
+                run_command="false"
+                echo "Unsupported Python version" > $seq.notrun
+            fi
         else
             run_command="./$seq"
         fi
-- 
2.20.1

Some scripts check the Python version number and have two code paths to
accomodate both Python 2 and 3. Remove the code specific to Python 2 and
assert the minimum version of 3.6 instead (check skips Python tests in
this case, so the assertion would only ever trigger if a Python script
is executed manually).

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/044        |  3 ---
 tests/qemu-iotests/163        |  3 ---
 tests/qemu-iotests/iotests.py | 13 +++----------
 3 files changed, 3 insertions(+), 16 deletions(-)

diff --git a/tests/qemu-iotests/044 b/tests/qemu-iotests/044
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/044
+++ b/tests/qemu-iotests/044
@@ -XXX,XX +XXX,XX @@ import struct
 import subprocess
 import sys
 
-if sys.version_info.major == 2:
-    range = xrange
-
 test_img = os.path.join(iotests.test_dir, 'test.img')
 
 class TestRefcountTableGrowth(iotests.QMPTestCase):
diff --git a/tests/qemu-iotests/163 b/tests/qemu-iotests/163
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/163
+++ b/tests/qemu-iotests/163
@@ -XXX,XX +XXX,XX @@
 import os, random, iotests, struct, qcow2, sys
 from iotests import qemu_img, qemu_io, image_size
 
-if sys.version_info.major == 2:
-    range = xrange
-
 test_img = os.path.join(iotests.test_dir, 'test.img')
 check_img = os.path.join(iotests.test_dir, 'check.img')
 
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -XXX,XX +XXX,XX @@ from collections import OrderedDict
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
 from qemu import qtest
 
+assert sys.version_info >= (3,6)
 
 # This will not work if arguments contain spaces but is necessary if we
 # want to support the override options that ./check supports.
@@ -XXX,XX +XXX,XX @@ def image_size(img):
     return json.loads(r)['virtual-size']
 
 def is_str(val):
-    if sys.version_info.major >= 3:
-        return isinstance(val, str)
-    else:
-        return isinstance(val, str) or isinstance(val, unicode)
+    return isinstance(val, str)
 
 test_dir_re = re.compile(r"%s" % test_dir)
 def filter_test_dir(msg):
@@ -XXX,XX +XXX,XX @@ def execute_test(test_function=None,
     else:
         # We need to filter out the time taken from the output so that
         # qemu-iotest can reliably diff the results against master output.
-        if sys.version_info.major >= 3:
-            output = io.StringIO()
-        else:
-            # io.StringIO is for unicode strings, which is not what
-            # 2.x's test runner emits.
-            output = io.BytesIO()
+        output = io.StringIO()
 
     logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN))
 
-- 
2.20.1