Series comparison

-[Qemu-devel] [PULL 0/1] Block patches
+[PULL 00/17] Block patches
-The following changes since commit e4ae62b802cec437f877f2cadc4ef059cc0eca76:
+The following changes since commit 418fa86dd465b4fd8394373cf83db8fa65d7611c:
-  Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2018-03-09 17:28:16 +0000)
+  Merge remote-tracking branch 'remotes/stsquad/tags/pull-testing-040220-1' into staging (2020-02-04 18:55:06 +0000)
 are available in the Git repository at:
-  git://github.com/stefanha/qemu.git tags/block-pull-request
+  https://github.com/XanClic/qemu.git tags/pull-block-2020-02-06
-for you to fetch changes up to 7376eda7c2e0451e819e81bd05fabc56a9deb946:
+for you to fetch changes up to a541fcc27c98b96da187c7d4573f3270f3ddd283:
-  block: make BDRV_POLL_WHILE() re-entrancy safe (2018-03-12 11:07:37 +0000)
+  iotests: add test for backup-top failure on permission activation (2020-02-06 13:47:45 +0100)
 ----------------------------------------------------------------
+Block patches:
+- Drop BDRV_SECTOR_SIZE from qcow2
+- Allow Python iotests to be added to the auto group
+  (and add some)
+- Fix for the backup job
+- Fix memleak in bdrv_refresh_filename()
+- Use GStrings in two places for greater efficiency (than manually
+  handling string allocation)
 ----------------------------------------------------------------
+Alberto Garcia (8):
+  qcow2: Assert that host cluster offsets fit in L2 table entries
+  block: Use a GString in bdrv_perm_names()
+  qcow2: Use a GString in report_unsupported_feature()
+  qcow2: Don't round the L1 table allocation up to the sector size
+  qcow2: Tighten cluster_offset alignment assertions
+  qcow2: Use bs->bl.request_alignment when updating an L1 entry
+  qcow2: Don't require aligned offsets in qcow2_co_copy_range_from()
+  qcow2: Use BDRV_SECTOR_SIZE instead of the hardcoded value
-Stefan Hajnoczi (1):
+John Snow (1):
-  block: make BDRV_POLL_WHILE() re-entrancy safe
+  iotests: remove 'linux' from default supported platforms
- include/block/aio-wait.h | 61 ++++++++++++++++++++++++------------------------
+Pan Nengyuan (1):
- util/aio-wait.c          |  2 +-
+  block: fix memleaks in bdrv_refresh_filename
-files changed, 31 insertions(+), 32 deletions(-)
 Thomas Huth (5):
   iotests: Test 041 only works on certain systems
   iotests: Test 183 does not work on macOS and OpenBSD
   iotests: Check for the availability of the required devices in 267 and
   iotests: Skip Python-based tests if QEMU does not support virtio-blk
   iotests: Enable more tests in the 'auto' group to improve test
     coverage
 Vladimir Sementsov-Ogievskiy (2):
   block/backup-top: fix failure path
   iotests: add test for backup-top failure on permission activation
  block.c                       | 12 +++--
  block/backup-top.c            | 21 ++++----
  block/qcow2-cluster.c         | 44 +++++++++++------
  block/qcow2-refcount.c        |  2 +-
  block/qcow2-snapshot.c        |  3 +-
  block/qcow2.c                 | 46 ++++++++----------
  tests/qemu-iotests/041        |  3 +-
  tests/qemu-iotests/127        |  2 +
  tests/qemu-iotests/183        |  1 +
  tests/qemu-iotests/267        |  2 +
  tests/qemu-iotests/283        | 92 +++++++++++++++++++++++++++++++++++
  tests/qemu-iotests/283.out    |  8 +++
  tests/qemu-iotests/check      | 12 ++++-
  tests/qemu-iotests/common.rc  | 14 ++++++
  tests/qemu-iotests/group      | 15 +++---
  tests/qemu-iotests/iotests.py | 16 ++++--
 files changed, 220 insertions(+), 73 deletions(-)
  create mode 100644 tests/qemu-iotests/283
  create mode 100644 tests/qemu-iotests/283.out
 --
-.14.3
+.24.1

-New patch
+[PULL 01/17] qcow2: Assert that host cluster offsets fit in L2 table entries
+From: Alberto Garcia <berto@igalia.com>
+The standard cluster descriptor in L2 table entries has a field to
+store the host cluster offset. When we need to get that offset from an
+entry we use L2E_OFFSET_MASK to ensure that we only use the bits that
+belong to that field.
+But while that mask is used every time we read from an L2 entry, it
+is never used when we write to it. Due to the QCOW_MAX_CLUSTER_OFFSET
+limit set in the cluster allocation code QEMU can never produce
+offsets that don't fit in that field so any such offset would indicate
+a bug in QEMU.
+Compressed cluster descriptors contain two fields (host cluster offset
+and size of the compressed data) and the situation with them is
+similar. In this case the masks are not constant but are stored in the
+csize_mask and cluster_offset_mask fields of BDRVQcow2State.
+Signed-off-by: Alberto Garcia <berto@igalia.com>
+Reviewed-by: Eric Blake <eblake@redhat.com>
+Message-id: 20200113161146.20099-1-berto@igalia.com
+Signed-off-by: Max Reitz <mreitz@redhat.com>
+---
+ block/qcow2-cluster.c | 14 ++++++++++++--
+file changed, 12 insertions(+), 2 deletions(-)
+diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
+index XXXXXXX..XXXXXXX 100644
+--- a/block/qcow2-cluster.c
++++ b/block/qcow2-cluster.c
+@@ -XXX,XX +XXX,XX @@ int qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
+         (cluster_offset + compressed_size - 1) / QCOW2_COMPRESSED_SECTOR_SIZE -
+         (cluster_offset / QCOW2_COMPRESSED_SECTOR_SIZE);
++    /* The offset and size must fit in their fields of the L2 table entry */
++    assert((cluster_offset & s->cluster_offset_mask) == cluster_offset);
++    assert((nb_csectors & s->csize_mask) == nb_csectors);
++
+     cluster_offset |= QCOW_OFLAG_COMPRESSED |
+                       ((uint64_t)nb_csectors << s->csize_shift);
+@@ -XXX,XX +XXX,XX @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
+     assert(l2_index + m->nb_clusters <= s->l2_slice_size);
+     for (i = 0; i < m->nb_clusters; i++) {
++        uint64_t offset = cluster_offset + (i << s->cluster_bits);
+         /* if two concurrent writes happen to the same unallocated cluster
+          * each write allocates separate cluster and writes data concurrently.
+          * The first one to complete updates l2 table with pointer to its
+@@ -XXX,XX +XXX,XX @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
+             old_cluster[j++] = l2_slice[l2_index + i];
+         }
+-        l2_slice[l2_index + i] = cpu_to_be64((cluster_offset +
+-                    (i << s->cluster_bits)) | QCOW_OFLAG_COPIED);
++        /* The offset must fit in the offset field of the L2 table entry */
++        assert((offset & L2E_OFFSET_MASK) == offset);
++
++        l2_slice[l2_index + i] = cpu_to_be64(offset | QCOW_OFLAG_COPIED);
+      }
+@@ -XXX,XX +XXX,XX @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
+                         goto fail;
+                     }
++                    /* The offset must fit in the offset field */
++                    assert((offset & L2E_OFFSET_MASK) == offset);
++
+                     if (l2_refcount > 1) {
+                         /* For shared L2 tables, set the refcount accordingly
+                          * (it is already 1 and needs to be l2_refcount) */
+--
+.24.1

-New patch
+[PULL 02/17] block: Use a GString in bdrv_perm_names()
+From: Alberto Garcia <berto@igalia.com>
+This is a bit more efficient than having to allocate and free memory
+for each new permission.
+The default size (30) is enough for "consistent read, write, resize".
+Signed-off-by: Alberto Garcia <berto@igalia.com>
+Message-id: 20200110171518.22168-1-berto@igalia.com
+Reviewed-by: Eric Blake <eblake@redhat.com>
+Signed-off-by: Max Reitz <mreitz@redhat.com>
+---
+ block.c | 11 ++++++-----
+file changed, 6 insertions(+), 5 deletions(-)
+diff --git a/block.c b/block.c
+index XXXXXXX..XXXXXXX 100644
+--- a/block.c
++++ b/block.c
+@@ -XXX,XX +XXX,XX @@ char *bdrv_perm_names(uint64_t perm)
+         { 0, NULL }
+     };
+-    char *result = g_strdup("");
++    GString *result = g_string_sized_new(30);
+     struct perm_name *p;
+     for (p = permissions; p->name; p++) {
+         if (perm & p->perm) {
+-            char *old = result;
+-            result = g_strdup_printf("%s%s%s", old, *old ? ", " : "", p->name);
+-            g_free(old);
++            if (result->len > 0) {
++                g_string_append(result, ", ");
++            }
++            g_string_append(result, p->name);
+         }
+     }
+-    return result;
++    return g_string_free(result, FALSE);
+ }
+ /*
+--
+.24.1

-New patch
+[PULL 03/17] block: fix memleaks in bdrv_refresh_filename
+From: Pan Nengyuan <pannengyuan@huawei.com>
+If we call the qmp 'query-block' while qemu is working on
+'block-commit', it will cause memleaks, the memory leak stack is as
+follow:
+Indirect leak of 12360 byte(s) in 3 object(s) allocated from:
+    #0 0x7f80f0b6d970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
+    #1 0x7f80ee86049d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
+    #2 0x55ea95b5bb67 in qdict_new /mnt/sdb/qemu-4.2.0-rc0/qobject/qdict.c:29
+    #3 0x55ea956cd043 in bdrv_refresh_filename /mnt/sdb/qemu-4.2.0-rc0/block.c:6427
+    #4 0x55ea956cc950 in bdrv_refresh_filename /mnt/sdb/qemu-4.2.0-rc0/block.c:6399
+    #5 0x55ea956cc950 in bdrv_refresh_filename /mnt/sdb/qemu-4.2.0-rc0/block.c:6399
+    #6 0x55ea956cc950 in bdrv_refresh_filename /mnt/sdb/qemu-4.2.0-rc0/block.c:6399
+    #7 0x55ea958818ea in bdrv_block_device_info /mnt/sdb/qemu-4.2.0-rc0/block/qapi.c:56
+    #8 0x55ea958879de in bdrv_query_info /mnt/sdb/qemu-4.2.0-rc0/block/qapi.c:392
+    #9 0x55ea9588b58f in qmp_query_block /mnt/sdb/qemu-4.2.0-rc0/block/qapi.c:578
+    #10 0x55ea95567392 in qmp_marshal_query_block qapi/qapi-commands-block-core.c:95
+Indirect leak of 4120 byte(s) in 1 object(s) allocated from:
+    #0 0x7f80f0b6d970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
+    #1 0x7f80ee86049d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
+    #2 0x55ea95b5bb67 in qdict_new /mnt/sdb/qemu-4.2.0-rc0/qobject/qdict.c:29
+    #3 0x55ea956cd043 in bdrv_refresh_filename /mnt/sdb/qemu-4.2.0-rc0/block.c:6427
+    #4 0x55ea956cc950 in bdrv_refresh_filename /mnt/sdb/qemu-4.2.0-rc0/block.c:6399
+    #5 0x55ea956cc950 in bdrv_refresh_filename /mnt/sdb/qemu-4.2.0-rc0/block.c:6399
+    #6 0x55ea9569f301 in bdrv_backing_attach /mnt/sdb/qemu-4.2.0-rc0/block.c:1064
+    #7 0x55ea956a99dd in bdrv_replace_child_noperm /mnt/sdb/qemu-4.2.0-rc0/block.c:2283
+    #8 0x55ea956b9b53 in bdrv_replace_node /mnt/sdb/qemu-4.2.0-rc0/block.c:4196
+    #9 0x55ea956b9e49 in bdrv_append /mnt/sdb/qemu-4.2.0-rc0/block.c:4236
+    #10 0x55ea958c3472 in commit_start /mnt/sdb/qemu-4.2.0-rc0/block/commit.c:306
+    #11 0x55ea94b68ab0 in qmp_block_commit /mnt/sdb/qemu-4.2.0-rc0/blockdev.c:3459
+    #12 0x55ea9556a7a7 in qmp_marshal_block_commit qapi/qapi-commands-block-core.c:407
+Fixes: bb808d5f5c0978828a974d547e6032402c339555
+Reported-by: Euler Robot <euler.robot@huawei.com>
+Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
+Message-id: 20200116085600.24056-1-pannengyuan@huawei.com
+Signed-off-by: Max Reitz <mreitz@redhat.com>
+---
+ block.c | 1 +
+file changed, 1 insertion(+)
+diff --git a/block.c b/block.c
+index XXXXXXX..XXXXXXX 100644
+--- a/block.c
++++ b/block.c
+@@ -XXX,XX +XXX,XX @@ void bdrv_refresh_filename(BlockDriverState *bs)
+                 child->bs->exact_filename);
+         pstrcpy(bs->filename, sizeof(bs->filename), child->bs->filename);
++        qobject_unref(bs->full_open_options);
+         bs->full_open_options = qobject_ref(child->bs->full_open_options);
+         return;
+--
+.24.1

-New patch
+[PULL 04/17] qcow2: Use a GString in report_unsupported_feature()
+From: Alberto Garcia <berto@igalia.com>
+This is a bit more efficient than having to allocate and free memory
+for each item.
+The default size (60) is enough for all the existing incompatible
+features or the "Unknown incompatible feature" message.
+Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
+Signed-off-by: Alberto Garcia <berto@igalia.com>
+Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
+Message-id: 20200115135626.19442-1-berto@igalia.com
+Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
+Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
+Signed-off-by: Max Reitz <mreitz@redhat.com>
+---
+ block/qcow2.c | 23 +++++++++++------------
+file changed, 11 insertions(+), 12 deletions(-)
+diff --git a/block/qcow2.c b/block/qcow2.c
+index XXXXXXX..XXXXXXX 100644
+--- a/block/qcow2.c
++++ b/block/qcow2.c
+@@ -XXX,XX +XXX,XX @@ static void cleanup_unknown_header_ext(BlockDriverState *bs)
+ static void report_unsupported_feature(Error **errp, Qcow2Feature *table,
+                                        uint64_t mask)
+ {
+-    char *features = g_strdup("");
+-    char *old;
++    g_autoptr(GString) features = g_string_sized_new(60);
+     while (table && table->name[0] != '\0') {
+         if (table->type == QCOW2_FEAT_TYPE_INCOMPATIBLE) {
+             if (mask & (1ULL << table->bit)) {
+-                old = features;
+-                features = g_strdup_printf("%s%s%.46s", old, *old ? ", " : "",
+-                                           table->name);
+-                g_free(old);
++                if (features->len > 0) {
++                    g_string_append(features, ", ");
++                }
++                g_string_append_printf(features, "%.46s", table->name);
+                 mask &= ~(1ULL << table->bit);
+             }
+         }
+@@ -XXX,XX +XXX,XX @@ static void report_unsupported_feature(Error **errp, Qcow2Feature *table,
+     }
+     if (mask) {
+-        old = features;
+-        features = g_strdup_printf("%s%sUnknown incompatible feature: %" PRIx64,
+-                                   old, *old ? ", " : "", mask);
+-        g_free(old);
++        if (features->len > 0) {
++            g_string_append(features, ", ");
++        }
++        g_string_append_printf(features,
++                               "Unknown incompatible feature: %" PRIx64, mask);
+     }
+-    error_setg(errp, "Unsupported qcow2 feature(s): %s", features);
+-    g_free(features);
++    error_setg(errp, "Unsupported qcow2 feature(s): %s", features->str);
+ }
+ /*
+--
+.24.1

-New patch
+[PULL 05/17] iotests: remove 'linux' from default supported platforms
+From: John Snow <jsnow@redhat.com>
+verify_platform will check an explicit whitelist and blacklist instead.
+The default will now be assumed to be allowed to run anywhere.
+For tests that do not specify their platforms explicitly, this has the effect of
+enabling these tests on non-linux platforms. For tests that always specified
+linux explicitly, there is no change.
+For Python tests on FreeBSD at least; only seven python tests fail:
+147 149 169 194 199 211
+and 149 appear to be misconfigurations,
+and 194 are the AF_UNIX path too long error,
+and 199 are bitmap migration bugs, and
+is a bug that shows up on Linux platforms, too.
+This is at least good evidence that these tests are not Linux-only. If
+they aren't suitable for other platforms, they should be disabled on a
+per-platform basis as appropriate.
+Therefore, let's switch these on and deal with the failures.
+Reviewed-by: Max Reitz <mreitz@redhat.com>
+Signed-off-by: John Snow <jsnow@redhat.com>
+Message-id: 20200121095205.26323-2-thuth@redhat.com
+Signed-off-by: Max Reitz <mreitz@redhat.com>
+---
+ tests/qemu-iotests/iotests.py | 16 +++++++++++-----
+file changed, 11 insertions(+), 5 deletions(-)
+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 @@ def verify_protocol(supported=[], unsupported=[]):
+     if not_sup or (imgproto in unsupported):
+         notrun('not suitable for this protocol: %s' % imgproto)
+-def verify_platform(supported_oses=['linux']):
+-    if True not in [sys.platform.startswith(x) for x in supported_oses]:
+-        notrun('not suitable for this OS: %s' % sys.platform)
++def verify_platform(supported=None, unsupported=None):
++    if unsupported is not None:
++        if any((sys.platform.startswith(x) for x in unsupported)):
++            notrun('not suitable for this OS: %s' % sys.platform)
++
++    if supported is not None:
++        if not any((sys.platform.startswith(x) for x in supported)):
++            notrun('not suitable for this OS: %s' % sys.platform)
+ def verify_cache_mode(supported_cache_modes=[]):
+     if supported_cache_modes and (cachemode not in supported_cache_modes):
+@@ -XXX,XX +XXX,XX @@ def execute_unittest(output, verbosity, debug):
+             sys.stderr.write(out)
+ def execute_test(test_function=None,
+-                 supported_fmts=[], supported_oses=['linux'],
++                 supported_fmts=[],
++                 supported_platforms=None,
+                  supported_cache_modes=[], supported_aio_modes={},
+                  unsupported_fmts=[], supported_protocols=[],
+                  unsupported_protocols=[]):
+@@ -XXX,XX +XXX,XX @@ def execute_test(test_function=None,
+     verbosity = 1
+     verify_image_format(supported_fmts, unsupported_fmts)
+     verify_protocol(supported_protocols, unsupported_protocols)
+-    verify_platform(supported_oses)
++    verify_platform(supported=supported_platforms)
+     verify_cache_mode(supported_cache_modes)
+     verify_aio_mode(supported_aio_modes)
+--
+.24.1

-New patch
+[PULL 06/17] iotests: Test 041 only works on certain systems
+From: Thomas Huth <thuth@redhat.com>
+works fine on Linux, FreeBSD, NetBSD and OpenBSD, but fails on macOS.
+Let's mark it as only supported on the systems where we know that it is
+working fine.
+Reviewed-by: Max Reitz <mreitz@redhat.com>
+Signed-off-by: Thomas Huth <thuth@redhat.com>
+Message-id: 20200121095205.26323-3-thuth@redhat.com
+Signed-off-by: Max Reitz <mreitz@redhat.com>
+---
+ tests/qemu-iotests/041 | 3 ++-
+file changed, 2 insertions(+), 1 deletion(-)
+diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
+index XXXXXXX..XXXXXXX 100755
+--- a/tests/qemu-iotests/041
++++ b/tests/qemu-iotests/041
+@@ -XXX,XX +XXX,XX @@ class TestOrphanedSource(iotests.QMPTestCase):
+ if __name__ == '__main__':
+     iotests.main(supported_fmts=['qcow2', 'qed'],
+-                 supported_protocols=['file'])
++                 supported_protocols=['file'],
++                 supported_platforms=['linux', 'freebsd', 'netbsd', 'openbsd'])
+--
+.24.1

-New patch
+[PULL 07/17] iotests: Test 183 does not work on macOS and OpenBSD
+From: Thomas Huth <thuth@redhat.com>
+In the long run, we might want to add test 183 to the "auto" group
+(but it still fails occasionally, so we cannot do that yet). However,
+when running 183 in Cirrus-CI on macOS, or with our vm-build-openbsd
+target, it currently always fails with an "Timeout waiting for return
+on handle 0" error.
+Let's mark it as supported only on systems where the test is working
+most of the time (i.e. Linux, FreeBSD and NetBSD).
+Reviewed-by: Max Reitz <mreitz@redhat.com>
+Signed-off-by: Thomas Huth <thuth@redhat.com>
+Message-id: 20200121095205.26323-4-thuth@redhat.com
+Signed-off-by: Max Reitz <mreitz@redhat.com>
+---
+ tests/qemu-iotests/183 | 1 +
+file changed, 1 insertion(+)
+diff --git a/tests/qemu-iotests/183 b/tests/qemu-iotests/183
+index XXXXXXX..XXXXXXX 100755
+--- a/tests/qemu-iotests/183
++++ b/tests/qemu-iotests/183
+@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
+ . ./common.filter
+ . ./common.qemu
++_supported_os Linux FreeBSD NetBSD
+ _supported_fmt qcow2 raw qed quorum
+ _supported_proto file
+--
+.24.1

-New patch
+[PULL 08/17] iotests: Check for the availability of the required devices in 267 and 127
+From: Thomas Huth <thuth@redhat.com>
+We are going to enable 127 in the "auto" group, but it only works if
+virtio-scsi and scsi-hd are available - which is not the case with
+QEMU binaries like qemu-system-tricore for example, so we need a
+proper check for the availability of these devices here.
+A very similar problem exists in iotest 267 - it has been added to
+the "auto" group already, but requires virtio-blk and thus currently
+fails with qemu-system-tricore for example. Let's also add aproper
+check there.
+Reviewed-by: Max Reitz <mreitz@redhat.com>
+Signed-off-by: Thomas Huth <thuth@redhat.com>
+Message-id: 20200121095205.26323-5-thuth@redhat.com
+Signed-off-by: Max Reitz <mreitz@redhat.com>
+---
+ tests/qemu-iotests/127       |  2 ++
+ tests/qemu-iotests/267       |  2 ++
+ tests/qemu-iotests/common.rc | 14 ++++++++++++++
+files changed, 18 insertions(+)
+diff --git a/tests/qemu-iotests/127 b/tests/qemu-iotests/127
+index XXXXXXX..XXXXXXX 100755
+--- a/tests/qemu-iotests/127
++++ b/tests/qemu-iotests/127
+@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
+ _supported_fmt qcow2
+ _supported_proto file
++_require_devices virtio-scsi scsi-hd
++
+ IMG_SIZE=64K
+ _make_test_img $IMG_SIZE
+diff --git a/tests/qemu-iotests/267 b/tests/qemu-iotests/267
+index XXXXXXX..XXXXXXX 100755
+--- a/tests/qemu-iotests/267
++++ b/tests/qemu-iotests/267
+@@ -XXX,XX +XXX,XX @@ _require_drivers copy-on-read
+ # and generally impossible with external data files
+ _unsupported_imgopts 'refcount_bits=1[^0-9]' data_file
++_require_devices virtio-blk
++
+ do_run_qemu()
+ {
+     echo Testing: "$@"
+diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
+index XXXXXXX..XXXXXXX 100644
+--- a/tests/qemu-iotests/common.rc
++++ b/tests/qemu-iotests/common.rc
+@@ -XXX,XX +XXX,XX @@ _require_large_file()
+     rm "$TEST_IMG"
+ }
++# Check that a set of devices is available in the QEMU binary
++#
++_require_devices()
++{
++    available=$($QEMU -M none -device help | \
++                grep ^name | sed -e 's/^name "//' -e 's/".*$//')
++    for device
++    do
++        if ! echo "$available" | grep -q "$device" ; then
++            _notrun "$device not available"
++        fi
++    done
++}
++
+ # make sure this script returns success
+ true
+--
+.24.1

-New patch
+[PULL 09/17] iotests: Skip Python-based tests if QEMU does not support virtio-blk
+From: Thomas Huth <thuth@redhat.com>
+We are going to enable some of the python-based tests in the "auto" group,
+and these tests require virtio-blk to work properly. Running iotests
+without virtio-blk likely does not make too much sense anyway, so instead
+of adding a check for the availability of virtio-blk to each and every
+test (which does not sound very appealing), let's rather add a check for
+this a central spot in the "check" script instead (so that it is still
+possible to run "make check" for qemu-system-tricore for example).
+Signed-off-by: Thomas Huth <thuth@redhat.com>
+Message-id: 20200121095205.26323-6-thuth@redhat.com
+Signed-off-by: Max Reitz <mreitz@redhat.com>
+---
+ tests/qemu-iotests/check | 12 ++++++++++--
+file changed, 10 insertions(+), 2 deletions(-)
+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 @@ fi
+ python_usable=false
+ if $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)'
+ then
+-    python_usable=true
++    # Our python framework also requires virtio-blk
++    if "$QEMU_PROG" -M none -device help | grep -q virtio-blk >/dev/null 2>&1
++    then
++        python_usable=true
++    else
++        python_unusable_because="Missing virtio-blk in QEMU binary"
++    fi
++else
++    python_unusable_because="Unsupported Python version"
+ fi
+ default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p')
+@@ -XXX,XX +XXX,XX @@ do
+                 run_command="$PYTHON $seq"
+             else
+                 run_command="false"
+-                echo "Unsupported Python version" > $seq.notrun
++                echo "$python_unusable_because" > $seq.notrun
+             fi
+         else
+             run_command="./$seq"
+--
+.24.1

-New patch
+[PULL 10/17] iotests: Enable more tests in the 'auto' group to improve test coverage
+From: Thomas Huth <thuth@redhat.com>
+According to Kevin, tests 030, 040 and 041 are among the most valuable
+tests that we have, so we should always run them if possible, even if
+they take a little bit longer.
+According to Max, it would be good to have a test for iothreads and
+migration. 127 and 256 seem to be good candidates for iothreads. For
+migration, let's enable 181 and 203 (which also tests iothreads).
+(091 would be a good candidate for migration, too, but Alex Bennée
+reported that this test fails on ZFS file systems, so it can't be
+included yet)
+Reviewed-by: Max Reitz <mreitz@redhat.com>
+Signed-off-by: Thomas Huth <thuth@redhat.com>
+Message-id: 20200121095205.26323-7-thuth@redhat.com
+Signed-off-by: Max Reitz <mreitz@redhat.com>
+---
+ tests/qemu-iotests/group | 14 +++++++-------
+file changed, 7 insertions(+), 7 deletions(-)
+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 auto quick
+rw backing quick
+rw auto quick
+-030 rw backing
++030 rw auto backing
+rw auto quick
+rw auto quick
+rw auto quick
+@@ -XXX,XX +XXX,XX @@
+rw auto backing quick
+rw auto backing quick
+rw auto quick
+-040 rw
+-041 rw backing
++040 rw auto
++041 rw auto backing
+rw auto quick
+rw auto backing
+rw
+@@ -XXX,XX +XXX,XX @@
+rw backing
+rw
+rw auto backing
+-127 rw backing quick
++127 rw auto backing quick
+rw quick
+rw quick
+rw quick
+@@ -XXX,XX +XXX,XX @@
+rw auto quick
+img
+rw auto quick
+-181 rw migration
++181 rw auto migration
+rw quick
+rw migration
+rw auto quick
+@@ -XXX,XX +XXX,XX @@
+rw
+rw migration
+rw quick
+-203 rw migration
++203 rw auto migration
+rw quick
+rw quick
+rw
+@@ -XXX,XX +XXX,XX @@
+rw quick
+rw backing quick
+rw quick
+-256 rw quick
++256 rw auto quick
+rw
+rw quick
+rw quick
+--
+.24.1

-New patch
+[PULL 11/17] qcow2: Don't round the L1 table allocation up to the sector size
+From: Alberto Garcia <berto@igalia.com>
+The L1 table is read from disk using the byte-based bdrv_pread() and
+is never accessed beyond its last element, so there's no need to
+allocate more memory than that.
+Signed-off-by: Alberto Garcia <berto@igalia.com>
+Reviewed-by: Max Reitz <mreitz@redhat.com>
+Message-id: b2e27214ec7b03a585931bcf383ee1ac3a641a10.1579374329.git.berto@igalia.com
+Signed-off-by: Max Reitz <mreitz@redhat.com>
+---
+ block/qcow2-cluster.c  | 5 ++---
+ block/qcow2-refcount.c | 2 +-
+ block/qcow2-snapshot.c | 3 +--
+ block/qcow2.c          | 2 +-
+files changed, 5 insertions(+), 7 deletions(-)
+diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
+index XXXXXXX..XXXXXXX 100644
+--- a/block/qcow2-cluster.c
++++ b/block/qcow2-cluster.c
+@@ -XXX,XX +XXX,XX @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
+ #endif
+     new_l1_size2 = sizeof(uint64_t) * new_l1_size;
+-    new_l1_table = qemu_try_blockalign(bs->file->bs,
+-                                       ROUND_UP(new_l1_size2, 512));
++    new_l1_table = qemu_try_blockalign(bs->file->bs, new_l1_size2);
+     if (new_l1_table == NULL) {
+         return -ENOMEM;
+     }
+-    memset(new_l1_table, 0, ROUND_UP(new_l1_size2, 512));
++    memset(new_l1_table, 0, new_l1_size2);
+     if (s->l1_size) {
+         memcpy(new_l1_table, s->l1_table, s->l1_size * sizeof(uint64_t));
+diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
+index XXXXXXX..XXXXXXX 100644
+--- a/block/qcow2-refcount.c
++++ b/block/qcow2-refcount.c
+@@ -XXX,XX +XXX,XX @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
+      * l1_table_offset when it is the current s->l1_table_offset! Be careful
+      * when changing this! */
+     if (l1_table_offset != s->l1_table_offset) {
+-        l1_table = g_try_malloc0(ROUND_UP(l1_size2, 512));
++        l1_table = g_try_malloc0(l1_size2);
+         if (l1_size2 && l1_table == NULL) {
+             ret = -ENOMEM;
+             goto fail;
+diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
+index XXXXXXX..XXXXXXX 100644
+--- a/block/qcow2-snapshot.c
++++ b/block/qcow2-snapshot.c
+@@ -XXX,XX +XXX,XX @@ int qcow2_snapshot_load_tmp(BlockDriverState *bs,
+         return ret;
+     }
+     new_l1_bytes = sn->l1_size * sizeof(uint64_t);
+-    new_l1_table = qemu_try_blockalign(bs->file->bs,
+-                                       ROUND_UP(new_l1_bytes, 512));
++    new_l1_table = qemu_try_blockalign(bs->file->bs, new_l1_bytes);
+     if (new_l1_table == NULL) {
+         return -ENOMEM;
+     }
+diff --git a/block/qcow2.c b/block/qcow2.c
+index XXXXXXX..XXXXXXX 100644
+--- a/block/qcow2.c
++++ b/block/qcow2.c
+@@ -XXX,XX +XXX,XX @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
+     if (s->l1_size > 0) {
+         s->l1_table = qemu_try_blockalign(bs->file->bs,
+-            ROUND_UP(s->l1_size * sizeof(uint64_t), 512));
++                                          s->l1_size * sizeof(uint64_t));
+         if (s->l1_table == NULL) {
+             error_setg(errp, "Could not allocate L1 table");
+             ret = -ENOMEM;
+--
+.24.1

-New patch
+[PULL 12/17] qcow2: Tighten cluster_offset alignment assertions
+From: Alberto Garcia <berto@igalia.com>
+qcow2_alloc_cluster_offset() and qcow2_get_cluster_offset() always
+return offsets that are cluster-aligned so don't just check that they
+are sector-aligned.
+The check in qcow2_co_preadv_task() is also replaced by an assertion
+for the same reason.
+Signed-off-by: Alberto Garcia <berto@igalia.com>
+Reviewed-by: Max Reitz <mreitz@redhat.com>
+Message-id: 558ba339965f858bede4c73ce3f50f0c0493597d.1579374329.git.berto@igalia.com
+Signed-off-by: Max Reitz <mreitz@redhat.com>
+---
+ block/qcow2.c | 9 +++------
+file changed, 3 insertions(+), 6 deletions(-)
+diff --git a/block/qcow2.c b/block/qcow2.c
+index XXXXXXX..XXXXXXX 100644
+--- a/block/qcow2.c
++++ b/block/qcow2.c
+@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_preadv_task(BlockDriverState *bs,
+                                           offset, bytes, qiov, qiov_offset);
+     case QCOW2_CLUSTER_NORMAL:
+-        if ((file_cluster_offset & 511) != 0) {
+-            return -EIO;
+-        }
+-
++        assert(offset_into_cluster(s, file_cluster_offset) == 0);
+         if (bs->encrypted) {
+             return qcow2_co_preadv_encrypted(bs, file_cluster_offset,
+                                              offset, bytes, qiov, qiov_offset);
+@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_pwritev_part(
+             goto out_locked;
+         }
+-        assert((cluster_offset & 511) == 0);
++        assert(offset_into_cluster(s, cluster_offset) == 0);
+         ret = qcow2_pre_write_overlap_check(bs, 0,
+                                             cluster_offset + offset_in_cluster,
+@@ -XXX,XX +XXX,XX @@ qcow2_co_copy_range_to(BlockDriverState *bs,
+             goto fail;
+         }
+-        assert((cluster_offset & 511) == 0);
++        assert(offset_into_cluster(s, cluster_offset) == 0);
+         ret = qcow2_pre_write_overlap_check(bs, 0,
+                 cluster_offset + offset_in_cluster, cur_bytes, true);
+--
+.24.1

-New patch
+[PULL 13/17] qcow2: Use bs->bl.request_alignment when updating an L1 entry
+From: Alberto Garcia <berto@igalia.com>
+When updating an L1 entry the qcow2 driver writes a (512-byte) sector
+worth of data to avoid a read-modify-write cycle. Instead of always
+writing 512 bytes we should follow the alignment requirements of the
+storage backend.
+(the only exception is when the alignment is larger than the cluster
+size because then we could be overwriting data after the L1 table)
+Signed-off-by: Alberto Garcia <berto@igalia.com>
+Message-id: 71f34d4ae4b367b32fb36134acbf4f4f7ee681f4.1579374329.git.berto@igalia.com
+Reviewed-by: Max Reitz <mreitz@redhat.com>
+Signed-off-by: Max Reitz <mreitz@redhat.com>
+---
+ block/qcow2-cluster.c | 25 +++++++++++++++----------
+file changed, 15 insertions(+), 10 deletions(-)
+diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
+index XXXXXXX..XXXXXXX 100644
+--- a/block/qcow2-cluster.c
++++ b/block/qcow2-cluster.c
+@@ -XXX,XX +XXX,XX @@ static int l2_load(BlockDriverState *bs, uint64_t offset,
+ }
+ /*
+- * Writes one sector of the L1 table to the disk (can't update single entries
+- * and we really don't want bdrv_pread to perform a read-modify-write)
++ * Writes an L1 entry to disk (note that depending on the alignment
++ * requirements this function may write more that just one entry in
++ * order to prevent bdrv_pwrite from performing a read-modify-write)
+  */
+-#define L1_ENTRIES_PER_SECTOR (512 / 8)
+ int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index)
+ {
+     BDRVQcow2State *s = bs->opaque;
+-    uint64_t buf[L1_ENTRIES_PER_SECTOR] = { 0 };
+     int l1_start_index;
+     int i, ret;
++    int bufsize = MAX(sizeof(uint64_t),
++                      MIN(bs->file->bs->bl.request_alignment, s->cluster_size));
++    int nentries = bufsize / sizeof(uint64_t);
++    g_autofree uint64_t *buf = g_try_new0(uint64_t, nentries);
+-    l1_start_index = l1_index & ~(L1_ENTRIES_PER_SECTOR - 1);
+-    for (i = 0; i < L1_ENTRIES_PER_SECTOR && l1_start_index + i < s->l1_size;
+-         i++)
+-    {
++    if (buf == NULL) {
++        return -ENOMEM;
++    }
++
++    l1_start_index = QEMU_ALIGN_DOWN(l1_index, nentries);
++    for (i = 0; i < MIN(nentries, s->l1_size - l1_start_index); i++) {
+         buf[i] = cpu_to_be64(s->l1_table[l1_start_index + i]);
+     }
+     ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_ACTIVE_L1,
+-            s->l1_table_offset + 8 * l1_start_index, sizeof(buf), false);
++            s->l1_table_offset + 8 * l1_start_index, bufsize, false);
+     if (ret < 0) {
+         return ret;
+     }
+@@ -XXX,XX +XXX,XX @@ int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index)
+     BLKDBG_EVENT(bs->file, BLKDBG_L1_UPDATE);
+     ret = bdrv_pwrite_sync(bs->file,
+                            s->l1_table_offset + 8 * l1_start_index,
+-                           buf, sizeof(buf));
++                           buf, bufsize);
+     if (ret < 0) {
+         return ret;
+     }
+--
+.24.1

-New patch
+[PULL 14/17] qcow2: Don't require aligned offsets in qcow2_co_copy_range_from()
+From: Alberto Garcia <berto@igalia.com>
+qemu-img's convert_co_copy_range() operates at the sector level and
+block_copy() operates at the cluster level so this condition is always
+true, but it is not necessary to restrict this here, so let's leave it
+to the driver implementation return an error if there is any.
+Signed-off-by: Alberto Garcia <berto@igalia.com>
+Message-id: a4264aaee656910c84161a2965f7a501437379ca.1579374329.git.berto@igalia.com
+Reviewed-by: Max Reitz <mreitz@redhat.com>
+Signed-off-by: Max Reitz <mreitz@redhat.com>
+---
+ block/qcow2.c | 4 ----
+file changed, 4 deletions(-)
+diff --git a/block/qcow2.c b/block/qcow2.c
+index XXXXXXX..XXXXXXX 100644
+--- a/block/qcow2.c
++++ b/block/qcow2.c
+@@ -XXX,XX +XXX,XX @@ qcow2_co_copy_range_from(BlockDriverState *bs,
+         case QCOW2_CLUSTER_NORMAL:
+             child = s->data_file;
+             copy_offset += offset_into_cluster(s, src_offset);
+-            if ((copy_offset & 511) != 0) {
+-                ret = -EIO;
+-                goto out;
+-            }
+             break;
+         default:
+--
+.24.1

-New patch
+[PULL 15/17] qcow2: Use BDRV_SECTOR_SIZE instead of the hardcoded value
+From: Alberto Garcia <berto@igalia.com>
+This replaces all remaining instances in the qcow2 code.
+Signed-off-by: Alberto Garcia <berto@igalia.com>
+Message-id: b5f74b606c2d9873b12d29acdb7fd498029c4025.1579374329.git.berto@igalia.com
+Reviewed-by: Max Reitz <mreitz@redhat.com>
+Signed-off-by: Max Reitz <mreitz@redhat.com>
+---
+ block/qcow2.c | 8 +++++---
+file changed, 5 insertions(+), 3 deletions(-)
+diff --git a/block/qcow2.c b/block/qcow2.c
+index XXXXXXX..XXXXXXX 100644
+--- a/block/qcow2.c
++++ b/block/qcow2.c
+@@ -XXX,XX +XXX,XX @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
+     /* Validate options and set default values */
+     if (!QEMU_IS_ALIGNED(qcow2_opts->size, BDRV_SECTOR_SIZE)) {
+-        error_setg(errp, "Image size must be a multiple of 512 bytes");
++        error_setg(errp, "Image size must be a multiple of %u bytes",
++                   (unsigned) BDRV_SECTOR_SIZE);
+         ret = -EINVAL;
+         goto out;
+     }
+@@ -XXX,XX +XXX,XX @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
+         return -ENOTSUP;
+     }
+-    if (offset & 511) {
+-        error_setg(errp, "The new size must be a multiple of 512");
++    if (!QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)) {
++        error_setg(errp, "The new size must be a multiple of %u",
++                   (unsigned) BDRV_SECTOR_SIZE);
+         return -EINVAL;
+     }
+--
+.24.1

-New patch
+[PULL 16/17] block/backup-top: fix failure path
+From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+We can't access top after call bdrv_backup_top_drop, as it is already
+freed at this time.
+Also, no needs to unref target child by hand, it will be unrefed on
+bdrv_close() automatically.
+So, just do bdrv_backup_top_drop if append succeed and one bdrv_unref
+otherwise.
+Note, that in !appended case bdrv_unref(top) moved into drained section
+on source. It doesn't really matter, but just for code simplicity.
+Fixes: 7df7868b96404
+Cc: qemu-stable@nongnu.org # v4.2.0
+Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+Reviewed-by: Max Reitz <mreitz@redhat.com>
+Message-id: 20200121142802.21467-2-vsementsov@virtuozzo.com
+Signed-off-by: Max Reitz <mreitz@redhat.com>
+---
+ block/backup-top.c | 21 ++++++++++++---------
+file changed, 12 insertions(+), 9 deletions(-)
+diff --git a/block/backup-top.c b/block/backup-top.c
+index XXXXXXX..XXXXXXX 100644
+--- a/block/backup-top.c
++++ b/block/backup-top.c
+@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
+     BlockDriverState *top = bdrv_new_open_driver(&bdrv_backup_top_filter,
+                                                  filter_node_name,
+                                                  BDRV_O_RDWR, errp);
++    bool appended = false;
+     if (!top) {
+         return NULL;
+@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
+     bdrv_append(top, source, &local_err);
+     if (local_err) {
+         error_prepend(&local_err, "Cannot append backup-top filter: ");
+-        goto append_failed;
++        goto fail;
+     }
++    appended = true;
+     /*
+      * bdrv_append() finished successfully, now we can require permissions
+@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
+     if (local_err) {
+         error_prepend(&local_err,
+                       "Cannot set permissions for backup-top filter: ");
+-        goto failed_after_append;
++        goto fail;
+     }
+     state->bcs = block_copy_state_new(top->backing, state->target,
+                                       cluster_size, write_flags, &local_err);
+     if (local_err) {
+         error_prepend(&local_err, "Cannot create block-copy-state: ");
+-        goto failed_after_append;
++        goto fail;
+     }
+     *bcs = state->bcs;
+@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
+     return top;
+-failed_after_append:
+-    state->active = false;
+-    bdrv_backup_top_drop(top);
++fail:
++    if (appended) {
++        state->active = false;
++        bdrv_backup_top_drop(top);
++    } else {
++        bdrv_unref(top);
++    }
+-append_failed:
+     bdrv_drained_end(source);
+-    bdrv_unref_child(top, state->target);
+-    bdrv_unref(top);
+     error_propagate(errp, local_err);
+     return NULL;
+--
+.24.1

-[Qemu-devel] [PULL 1/1] block: make BDRV_POLL_WHILE() re-entrancy safe
+[PULL 17/17] iotests: add test for backup-top failure on permission activation
-Nested BDRV_POLL_WHILE() calls can occur.  Currently
+From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
 assert(!wait_->wakeup) fails in AIO_WAIT_WHILE() when this happens.
-This patch converts the bool wait_->need_kick flag to an unsigned
+This test checks that bug is really fixed by previous commit.
 wait_->num_waiters counter.
-Nesting works correctly because outer AIO_WAIT_WHILE() callers evaluate
+Cc: qemu-stable@nongnu.org # v4.2.0
-the condition again after the inner caller completes (invoking the inner
+Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-caller counts as aio_poll() progress).
+Message-id: 20200121142802.21467-3-vsementsov@virtuozzo.com
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  tests/qemu-iotests/283     | 92 ++++++++++++++++++++++++++++++++++++++
  tests/qemu-iotests/283.out |  8 ++++
  tests/qemu-iotests/group   |  1 +
 files changed, 101 insertions(+)
  create mode 100644 tests/qemu-iotests/283
  create mode 100644 tests/qemu-iotests/283.out
-Reported-by: "fuweiwei (C)" <fuweiwei2@huawei.com>
+diff --git a/tests/qemu-iotests/283 b/tests/qemu-iotests/283
-Reviewed-by: Eric Blake <eblake@redhat.com>
+new file mode 100644
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+index XXXXXXX..XXXXXXX
-Message-id: 20180307124619.6218-1-stefanha@redhat.com
+--- /dev/null
-Cc: Paolo Bonzini <pbonzini@redhat.com>
++++ b/tests/qemu-iotests/283
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+@@ -XXX,XX +XXX,XX @@
----
++#!/usr/bin/env python
- include/block/aio-wait.h | 61 ++++++++++++++++++++++++------------------------
++#
- util/aio-wait.c          |  2 +-
++# Test for backup-top filter permission activation failure
-files changed, 31 insertions(+), 32 deletions(-)
++#
++# Copyright (c) 2019 Virtuozzo International GmbH.
-diff --git a/include/block/aio-wait.h b/include/block/aio-wait.h
++#
 +# 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/>.
 +#
 +
 +import iotests
 +
 +# The test is unrelated to formats, restrict it to qcow2 to avoid extra runs
 +iotests.verify_image_format(supported_fmts=['qcow2'])
 +
 +size = 1024 * 1024
 +
 +""" Test description
 +
 +When performing a backup, all writes on the source subtree must go through the
 +backup-top filter so it can copy all data to the target before it is changed.
 +backup-top filter is appended above source node, to achieve this thing, so all
 +parents of source node are handled. A configuration with side parents of source
 +sub-tree with write permission is unsupported (we'd have append several
 +backup-top filter like nodes to handle such parents). The test create an
 +example of such configuration and checks that a backup is then not allowed
 +(blockdev-backup command should fail).
 +
 +The configuration:
 +
 +    ┌────────┐  target  ┌─────────────┐
 +    │ target │ ◀─────── │ backup_top  │
 +    └────────┘          └─────────────┘
 +                            │
 +                            │ backing
 +                            ▼
 +                        ┌─────────────┐
 +                        │   source    │
 +                        └─────────────┘
 +                            │
 +                            │ file
 +                            ▼
 +                        ┌─────────────┐  write perm   ┌───────┐
 +                        │    base     │ ◀──────────── │ other │
 +                        └─────────────┘               └───────┘
 +
 +On activation (see .active field of backup-top state in block/backup-top.c),
 +backup-top is going to unshare write permission on its source child. Write
 +unsharing will be propagated to the "source->base" link and will conflict with
 +other node write permission. So permission update will fail and backup job will
 +not be started.
 +
 +Note, that the only thing which prevents backup of running on such
 +configuration is default permission propagation scheme. It may be altered by
 +different block drivers, so backup will run in invalid configuration. But
 +something is better than nothing. Also, before the previous commit (commit
 +preceding this test creation), starting backup on such configuration led to
 +crash, so current "something" is a lot better, and this test actual goal is
 +to check that crash is fixed :)
 +"""
 +
 +vm = iotests.VM()
 +vm.launch()
 +
 +vm.qmp_log('blockdev-add', **{'node-name': 'target', 'driver': 'null-co'})
 +
 +vm.qmp_log('blockdev-add', **{
 +    'node-name': 'source',
 +    'driver': 'blkdebug',
 +    'image': {'node-name': 'base', 'driver': 'null-co', 'size': size}
 +})
 +
 +vm.qmp_log('blockdev-add', **{
 +    'node-name': 'other',
 +    'driver': 'blkdebug',
 +    'image': 'base',
 +    'take-child-perms': ['write']
 +})
 +
 +vm.qmp_log('blockdev-backup', sync='full', device='source', target='target')
 +
 +vm.shutdown()
 diff --git a/tests/qemu-iotests/283.out b/tests/qemu-iotests/283.out
 new file mode 100644
 index XXXXXXX..XXXXXXX
 --- /dev/null
 +++ b/tests/qemu-iotests/283.out
@@ -XXX,XX +XXX,XX @@
 +{"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "target"}}
 +{"return": {}}
 +{"execute": "blockdev-add", "arguments": {"driver": "blkdebug", "image": {"driver": "null-co", "node-name": "base", "size": 1048576}, "node-name": "source"}}
 +{"return": {}}
 +{"execute": "blockdev-add", "arguments": {"driver": "blkdebug", "image": "base", "node-name": "other", "take-child-perms": ["write"]}}
 +{"return": {}}
 +{"execute": "blockdev-backup", "arguments": {"device": "source", "sync": "full", "target": "target"}}
 +{"error": {"class": "GenericError", "desc": "Cannot set permissions for backup-top filter: Conflicts with use by other as 'image', which uses 'write' on base"}}
 diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
 index XXXXXXX..XXXXXXX 100644
---- a/include/block/aio-wait.h
+--- a/tests/qemu-iotests/group
-+++ b/include/block/aio-wait.h
++++ b/tests/qemu-iotests/group
 @@ -XXX,XX +XXX,XX @@
-  *   }
+rw backing quick
-  */
+rw migration quick
- typedef struct {
+rw quick
--    /* Is the main loop waiting for a kick?  Accessed with atomic ops. */
++283 auto quick
 -    bool need_kick;
 +    /* Number of waiting AIO_WAIT_WHILE() callers. Accessed with atomic ops. */
 +    unsigned num_waiters;
  } AioWait;
  /**
@@ -XXX,XX +XXX,XX @@ typedef struct {
   * wait on conditions between two IOThreads since that could lead to deadlock,
   * go via the main loop instead.
   */
 -#define AIO_WAIT_WHILE(wait, ctx, cond) ({                  \
 -    bool waited_ = false;                                   \
 -    bool busy_ = true;                                      \
 -    AioWait *wait_ = (wait);                                \
 -    AioContext *ctx_ = (ctx);                               \
 -    if (in_aio_context_home_thread(ctx_)) {                 \
 -        while ((cond) || busy_) {                           \
 -            busy_ = aio_poll(ctx_, (cond));                 \
 -            waited_ |= !!(cond) | busy_;                    \
 -        }                                                   \
 -    } else {                                                \
 -        assert(qemu_get_current_aio_context() ==            \
 -               qemu_get_aio_context());                     \
 -        assert(!wait_->need_kick);                          \
 -        /* Set wait_->need_kick before evaluating cond.  */ \
 -        atomic_mb_set(&wait_->need_kick, true);             \
 -        while (busy_) {                                     \
 -            if ((cond)) {                                   \
 -                waited_ = busy_ = true;                     \
 -                aio_context_release(ctx_);                  \
 -                aio_poll(qemu_get_aio_context(), true);     \
 -                aio_context_acquire(ctx_);                  \
 -            } else {                                        \
 -                busy_ = aio_poll(ctx_, false);              \
 -                waited_ |= busy_;                           \
 -            }                                               \
 -        }                                                   \
 -        atomic_set(&wait_->need_kick, false);               \
 -    }                                                       \
 +#define AIO_WAIT_WHILE(wait, ctx, cond) ({                         \
 +    bool waited_ = false;                                          \
 +    bool busy_ = true;                                             \
 +    AioWait *wait_ = (wait);                                       \
 +    AioContext *ctx_ = (ctx);                                      \
 +    if (in_aio_context_home_thread(ctx_)) {                        \
 +        while ((cond) || busy_) {                                  \
 +            busy_ = aio_poll(ctx_, (cond));                        \
 +            waited_ |= !!(cond) | busy_;                           \
 +        }                                                          \
 +    } else {                                                       \
 +        assert(qemu_get_current_aio_context() ==                   \
 +               qemu_get_aio_context());                            \
 +        /* Increment wait_->num_waiters before evaluating cond. */ \
 +        atomic_inc(&wait_->num_waiters);                           \
 +        while (busy_) {                                            \
 +            if ((cond)) {                                          \
 +                waited_ = busy_ = true;                            \
 +                aio_context_release(ctx_);                         \
 +                aio_poll(qemu_get_aio_context(), true);            \
 +                aio_context_acquire(ctx_);                         \
 +            } else {                                               \
 +                busy_ = aio_poll(ctx_, false);                     \
 +                waited_ |= busy_;                                  \
 +            }                                                      \
 +        }                                                          \
 +        atomic_dec(&wait_->num_waiters);                           \
 +    }                                                              \
      waited_; })
  /**
 diff --git a/util/aio-wait.c b/util/aio-wait.c
 index XXXXXXX..XXXXXXX 100644
 --- a/util/aio-wait.c
 +++ b/util/aio-wait.c
@@ -XXX,XX +XXX,XX @@ static void dummy_bh_cb(void *opaque)
  void aio_wait_kick(AioWait *wait)
  {
      /* The barrier (or an atomic op) is in the caller.  */
 -    if (atomic_read(&wait->need_kick)) {
 +    if (atomic_read(&wait->num_waiters)) {
          aio_bh_schedule_oneshot(qemu_get_aio_context(), dummy_bh_cb, NULL);
      }
  }
 --
-.14.3
+.24.1

Nested BDRV_POLL_WHILE() calls can occur.  Currently
assert(!wait_->wakeup) fails in AIO_WAIT_WHILE() when this happens.

This patch converts the bool wait_->need_kick flag to an unsigned
wait_->num_waiters counter.

Nesting works correctly because outer AIO_WAIT_WHILE() callers evaluate
the condition again after the inner caller completes (invoking the inner
caller counts as aio_poll() progress).

Reported-by: "fuweiwei (C)" <fuweiwei2@huawei.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20180307124619.6218-1-stefanha@redhat.com
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/block/aio-wait.h | 61 ++++++++++++++++++++++++------------------------
 util/aio-wait.c          |  2 +-
 2 files changed, 31 insertions(+), 32 deletions(-)

diff --git a/include/block/aio-wait.h b/include/block/aio-wait.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/aio-wait.h
+++ b/include/block/aio-wait.h
@@ -XXX,XX +XXX,XX @@
  *   }
  */
 typedef struct {
-    /* Is the main loop waiting for a kick?  Accessed with atomic ops. */
-    bool need_kick;
+    /* Number of waiting AIO_WAIT_WHILE() callers. Accessed with atomic ops. */
+    unsigned num_waiters;
 } AioWait;
 
 /**
@@ -XXX,XX +XXX,XX @@ typedef struct {
  * wait on conditions between two IOThreads since that could lead to deadlock,
  * go via the main loop instead.
  */
-#define AIO_WAIT_WHILE(wait, ctx, cond) ({                  \
-    bool waited_ = false;                                   \
-    bool busy_ = true;                                      \
-    AioWait *wait_ = (wait);                                \
-    AioContext *ctx_ = (ctx);                               \
-    if (in_aio_context_home_thread(ctx_)) {                 \
-        while ((cond) || busy_) {                           \
-            busy_ = aio_poll(ctx_, (cond));                 \
-            waited_ |= !!(cond) | busy_;                    \
-        }                                                   \
-    } else {                                                \
-        assert(qemu_get_current_aio_context() ==            \
-               qemu_get_aio_context());                     \
-        assert(!wait_->need_kick);                          \
-        /* Set wait_->need_kick before evaluating cond.  */ \
-        atomic_mb_set(&wait_->need_kick, true);             \
-        while (busy_) {                                     \
-            if ((cond)) {                                   \
-                waited_ = busy_ = true;                     \
-                aio_context_release(ctx_);                  \
-                aio_poll(qemu_get_aio_context(), true);     \
-                aio_context_acquire(ctx_);                  \
-            } else {                                        \
-                busy_ = aio_poll(ctx_, false);              \
-                waited_ |= busy_;                           \
-            }                                               \
-        }                                                   \
-        atomic_set(&wait_->need_kick, false);               \
-    }                                                       \
+#define AIO_WAIT_WHILE(wait, ctx, cond) ({                         \
+    bool waited_ = false;                                          \
+    bool busy_ = true;                                             \
+    AioWait *wait_ = (wait);                                       \
+    AioContext *ctx_ = (ctx);                                      \
+    if (in_aio_context_home_thread(ctx_)) {                        \
+        while ((cond) || busy_) {                                  \
+            busy_ = aio_poll(ctx_, (cond));                        \
+            waited_ |= !!(cond) | busy_;                           \
+        }                                                          \
+    } else {                                                       \
+        assert(qemu_get_current_aio_context() ==                   \
+               qemu_get_aio_context());                            \
+        /* Increment wait_->num_waiters before evaluating cond. */ \
+        atomic_inc(&wait_->num_waiters);                           \
+        while (busy_) {                                            \
+            if ((cond)) {                                          \
+                waited_ = busy_ = true;                            \
+                aio_context_release(ctx_);                         \
+                aio_poll(qemu_get_aio_context(), true);            \
+                aio_context_acquire(ctx_);                         \
+            } else {                                               \
+                busy_ = aio_poll(ctx_, false);                     \
+                waited_ |= busy_;                                  \
+            }                                                      \
+        }                                                          \
+        atomic_dec(&wait_->num_waiters);                           \
+    }                                                              \
     waited_; })
 
 /**
diff --git a/util/aio-wait.c b/util/aio-wait.c
index XXXXXXX..XXXXXXX 100644
--- a/util/aio-wait.c
+++ b/util/aio-wait.c
@@ -XXX,XX +XXX,XX @@ static void dummy_bh_cb(void *opaque)
 void aio_wait_kick(AioWait *wait)
 {
     /* The barrier (or an atomic op) is in the caller.  */
-    if (atomic_read(&wait->need_kick)) {
+    if (atomic_read(&wait->num_waiters)) {
         aio_bh_schedule_oneshot(qemu_get_aio_context(), dummy_bh_cb, NULL);
     }
 }
-- 
2.14.3

The following changes since commit 418fa86dd465b4fd8394373cf83db8fa65d7611c:

Merge remote-tracking branch 'remotes/stsquad/tags/pull-testing-040220-1' into staging (2020-02-04 18:55:06 +0000)

are available in the Git repository at:

https://github.com/XanClic/qemu.git tags/pull-block-2020-02-06

for you to fetch changes up to a541fcc27c98b96da187c7d4573f3270f3ddd283:

iotests: add test for backup-top failure on permission activation (2020-02-06 13:47:45 +0100)

----------------------------------------------------------------
Block patches:
- Drop BDRV_SECTOR_SIZE from qcow2
- Allow Python iotests to be added to the auto group
  (and add some)
- Fix for the backup job
- Fix memleak in bdrv_refresh_filename()
- Use GStrings in two places for greater efficiency (than manually
  handling string allocation)

----------------------------------------------------------------
Alberto Garcia (8):
  qcow2: Assert that host cluster offsets fit in L2 table entries
  block: Use a GString in bdrv_perm_names()
  qcow2: Use a GString in report_unsupported_feature()
  qcow2: Don't round the L1 table allocation up to the sector size
  qcow2: Tighten cluster_offset alignment assertions
  qcow2: Use bs->bl.request_alignment when updating an L1 entry
  qcow2: Don't require aligned offsets in qcow2_co_copy_range_from()
  qcow2: Use BDRV_SECTOR_SIZE instead of the hardcoded value

John Snow (1):
  iotests: remove 'linux' from default supported platforms

Pan Nengyuan (1):
  block: fix memleaks in bdrv_refresh_filename

Thomas Huth (5):
  iotests: Test 041 only works on certain systems
  iotests: Test 183 does not work on macOS and OpenBSD
  iotests: Check for the availability of the required devices in 267 and
    127
  iotests: Skip Python-based tests if QEMU does not support virtio-blk
  iotests: Enable more tests in the 'auto' group to improve test
    coverage

Vladimir Sementsov-Ogievskiy (2):
  block/backup-top: fix failure path
  iotests: add test for backup-top failure on permission activation

-- 
2.24.1

From: Alberto Garcia <berto@igalia.com>

The standard cluster descriptor in L2 table entries has a field to
store the host cluster offset. When we need to get that offset from an
entry we use L2E_OFFSET_MASK to ensure that we only use the bits that
belong to that field.

But while that mask is used every time we read from an L2 entry, it
is never used when we write to it. Due to the QCOW_MAX_CLUSTER_OFFSET
limit set in the cluster allocation code QEMU can never produce
offsets that don't fit in that field so any such offset would indicate
a bug in QEMU.

Compressed cluster descriptors contain two fields (host cluster offset
and size of the compressed data) and the situation with them is
similar. In this case the masks are not constant but are stored in the
csize_mask and cluster_offset_mask fields of BDRVQcow2State.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20200113161146.20099-1-berto@igalia.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2-cluster.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -XXX,XX +XXX,XX @@ int qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
         (cluster_offset + compressed_size - 1) / QCOW2_COMPRESSED_SECTOR_SIZE -
         (cluster_offset / QCOW2_COMPRESSED_SECTOR_SIZE);
 
+    /* The offset and size must fit in their fields of the L2 table entry */
+    assert((cluster_offset & s->cluster_offset_mask) == cluster_offset);
+    assert((nb_csectors & s->csize_mask) == nb_csectors);
+
     cluster_offset |= QCOW_OFLAG_COMPRESSED |
                       ((uint64_t)nb_csectors << s->csize_shift);
 
@@ -XXX,XX +XXX,XX @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
 
     assert(l2_index + m->nb_clusters <= s->l2_slice_size);
     for (i = 0; i < m->nb_clusters; i++) {
+        uint64_t offset = cluster_offset + (i << s->cluster_bits);
         /* if two concurrent writes happen to the same unallocated cluster
          * each write allocates separate cluster and writes data concurrently.
          * The first one to complete updates l2 table with pointer to its
@@ -XXX,XX +XXX,XX @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
             old_cluster[j++] = l2_slice[l2_index + i];
         }
 
-        l2_slice[l2_index + i] = cpu_to_be64((cluster_offset +
-                    (i << s->cluster_bits)) | QCOW_OFLAG_COPIED);
+        /* The offset must fit in the offset field of the L2 table entry */
+        assert((offset & L2E_OFFSET_MASK) == offset);
+
+        l2_slice[l2_index + i] = cpu_to_be64(offset | QCOW_OFLAG_COPIED);
      }
 
 
@@ -XXX,XX +XXX,XX @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
                         goto fail;
                     }
 
+                    /* The offset must fit in the offset field */
+                    assert((offset & L2E_OFFSET_MASK) == offset);
+
                     if (l2_refcount > 1) {
                         /* For shared L2 tables, set the refcount accordingly
                          * (it is already 1 and needs to be l2_refcount) */
-- 
2.24.1

From: Alberto Garcia <berto@igalia.com>

This is a bit more efficient than having to allocate and free memory
for each new permission.

The default size (30) is enough for "consistent read, write, resize".

Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-id: 20200110171518.22168-1-berto@igalia.com
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/block.c b/block.c
index XXXXXXX..XXXXXXX 100644
--- a/block.c
+++ b/block.c
@@ -XXX,XX +XXX,XX @@ char *bdrv_perm_names(uint64_t perm)
         { 0, NULL }
     };
 
-    char *result = g_strdup("");
+    GString *result = g_string_sized_new(30);
     struct perm_name *p;
 
     for (p = permissions; p->name; p++) {
         if (perm & p->perm) {
-            char *old = result;
-            result = g_strdup_printf("%s%s%s", old, *old ? ", " : "", p->name);
-            g_free(old);
+            if (result->len > 0) {
+                g_string_append(result, ", ");
+            }
+            g_string_append(result, p->name);
         }
     }
 
-    return result;
+    return g_string_free(result, FALSE);
 }
 
 /*
-- 
2.24.1

From: Pan Nengyuan <pannengyuan@huawei.com>

If we call the qmp 'query-block' while qemu is working on
'block-commit', it will cause memleaks, the memory leak stack is as
follow:

Indirect leak of 12360 byte(s) in 3 object(s) allocated from:
    #0 0x7f80f0b6d970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
    #1 0x7f80ee86049d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
    #2 0x55ea95b5bb67 in qdict_new /mnt/sdb/qemu-4.2.0-rc0/qobject/qdict.c:29
    #3 0x55ea956cd043 in bdrv_refresh_filename /mnt/sdb/qemu-4.2.0-rc0/block.c:6427
    #4 0x55ea956cc950 in bdrv_refresh_filename /mnt/sdb/qemu-4.2.0-rc0/block.c:6399
    #5 0x55ea956cc950 in bdrv_refresh_filename /mnt/sdb/qemu-4.2.0-rc0/block.c:6399
    #6 0x55ea956cc950 in bdrv_refresh_filename /mnt/sdb/qemu-4.2.0-rc0/block.c:6399
    #7 0x55ea958818ea in bdrv_block_device_info /mnt/sdb/qemu-4.2.0-rc0/block/qapi.c:56
    #8 0x55ea958879de in bdrv_query_info /mnt/sdb/qemu-4.2.0-rc0/block/qapi.c:392
    #9 0x55ea9588b58f in qmp_query_block /mnt/sdb/qemu-4.2.0-rc0/block/qapi.c:578
    #10 0x55ea95567392 in qmp_marshal_query_block qapi/qapi-commands-block-core.c:95

Indirect leak of 4120 byte(s) in 1 object(s) allocated from:
    #0 0x7f80f0b6d970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
    #1 0x7f80ee86049d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
    #2 0x55ea95b5bb67 in qdict_new /mnt/sdb/qemu-4.2.0-rc0/qobject/qdict.c:29
    #3 0x55ea956cd043 in bdrv_refresh_filename /mnt/sdb/qemu-4.2.0-rc0/block.c:6427
    #4 0x55ea956cc950 in bdrv_refresh_filename /mnt/sdb/qemu-4.2.0-rc0/block.c:6399
    #5 0x55ea956cc950 in bdrv_refresh_filename /mnt/sdb/qemu-4.2.0-rc0/block.c:6399
    #6 0x55ea9569f301 in bdrv_backing_attach /mnt/sdb/qemu-4.2.0-rc0/block.c:1064
    #7 0x55ea956a99dd in bdrv_replace_child_noperm /mnt/sdb/qemu-4.2.0-rc0/block.c:2283
    #8 0x55ea956b9b53 in bdrv_replace_node /mnt/sdb/qemu-4.2.0-rc0/block.c:4196
    #9 0x55ea956b9e49 in bdrv_append /mnt/sdb/qemu-4.2.0-rc0/block.c:4236
    #10 0x55ea958c3472 in commit_start /mnt/sdb/qemu-4.2.0-rc0/block/commit.c:306
    #11 0x55ea94b68ab0 in qmp_block_commit /mnt/sdb/qemu-4.2.0-rc0/blockdev.c:3459
    #12 0x55ea9556a7a7 in qmp_marshal_block_commit qapi/qapi-commands-block-core.c:407

Fixes: bb808d5f5c0978828a974d547e6032402c339555
Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
Message-id: 20200116085600.24056-1-pannengyuan@huawei.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block.c b/block.c
index XXXXXXX..XXXXXXX 100644
--- a/block.c
+++ b/block.c
@@ -XXX,XX +XXX,XX @@ void bdrv_refresh_filename(BlockDriverState *bs)
                 child->bs->exact_filename);
         pstrcpy(bs->filename, sizeof(bs->filename), child->bs->filename);
 
+        qobject_unref(bs->full_open_options);
         bs->full_open_options = qobject_ref(child->bs->full_open_options);
 
         return;
-- 
2.24.1

From: Alberto Garcia <berto@igalia.com>

This is a bit more efficient than having to allocate and free memory
for each item.

The default size (60) is enough for all the existing incompatible
features or the "Unknown incompatible feature" message.

Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-id: 20200115135626.19442-1-berto@igalia.com
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -XXX,XX +XXX,XX @@ static void cleanup_unknown_header_ext(BlockDriverState *bs)
 static void report_unsupported_feature(Error **errp, Qcow2Feature *table,
                                        uint64_t mask)
 {
-    char *features = g_strdup("");
-    char *old;
+    g_autoptr(GString) features = g_string_sized_new(60);
 
     while (table && table->name[0] != '\0') {
         if (table->type == QCOW2_FEAT_TYPE_INCOMPATIBLE) {
             if (mask & (1ULL << table->bit)) {
-                old = features;
-                features = g_strdup_printf("%s%s%.46s", old, *old ? ", " : "",
-                                           table->name);
-                g_free(old);
+                if (features->len > 0) {
+                    g_string_append(features, ", ");
+                }
+                g_string_append_printf(features, "%.46s", table->name);
                 mask &= ~(1ULL << table->bit);
             }
         }
@@ -XXX,XX +XXX,XX @@ static void report_unsupported_feature(Error **errp, Qcow2Feature *table,
     }
 
     if (mask) {
-        old = features;
-        features = g_strdup_printf("%s%sUnknown incompatible feature: %" PRIx64,
-                                   old, *old ? ", " : "", mask);
-        g_free(old);
+        if (features->len > 0) {
+            g_string_append(features, ", ");
+        }
+        g_string_append_printf(features,
+                               "Unknown incompatible feature: %" PRIx64, mask);
     }
 
-    error_setg(errp, "Unsupported qcow2 feature(s): %s", features);
-    g_free(features);
+    error_setg(errp, "Unsupported qcow2 feature(s): %s", features->str);
 }
 
 /*
-- 
2.24.1

From: John Snow <jsnow@redhat.com>

verify_platform will check an explicit whitelist and blacklist instead.
The default will now be assumed to be allowed to run anywhere.

For tests that do not specify their platforms explicitly, this has the effect of
enabling these tests on non-linux platforms. For tests that always specified
linux explicitly, there is no change.

For Python tests on FreeBSD at least; only seven python tests fail:
045 147 149 169 194 199 211

045 and 149 appear to be misconfigurations,
147 and 194 are the AF_UNIX path too long error,
169 and 199 are bitmap migration bugs, and
211 is a bug that shows up on Linux platforms, too.

This is at least good evidence that these tests are not Linux-only. If
they aren't suitable for other platforms, they should be disabled on a
per-platform basis as appropriate.

Therefore, let's switch these on and deal with the failures.

Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 20200121095205.26323-2-thuth@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/iotests.py | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

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 @@ def verify_protocol(supported=[], unsupported=[]):
     if not_sup or (imgproto in unsupported):
         notrun('not suitable for this protocol: %s' % imgproto)
 
-def verify_platform(supported_oses=['linux']):
-    if True not in [sys.platform.startswith(x) for x in supported_oses]:
-        notrun('not suitable for this OS: %s' % sys.platform)
+def verify_platform(supported=None, unsupported=None):
+    if unsupported is not None:
+        if any((sys.platform.startswith(x) for x in unsupported)):
+            notrun('not suitable for this OS: %s' % sys.platform)
+
+    if supported is not None:
+        if not any((sys.platform.startswith(x) for x in supported)):
+            notrun('not suitable for this OS: %s' % sys.platform)
 
 def verify_cache_mode(supported_cache_modes=[]):
     if supported_cache_modes and (cachemode not in supported_cache_modes):
@@ -XXX,XX +XXX,XX @@ def execute_unittest(output, verbosity, debug):
             sys.stderr.write(out)
 
 def execute_test(test_function=None,
-                 supported_fmts=[], supported_oses=['linux'],
+                 supported_fmts=[],
+                 supported_platforms=None,
                  supported_cache_modes=[], supported_aio_modes={},
                  unsupported_fmts=[], supported_protocols=[],
                  unsupported_protocols=[]):
@@ -XXX,XX +XXX,XX @@ def execute_test(test_function=None,
     verbosity = 1
     verify_image_format(supported_fmts, unsupported_fmts)
     verify_protocol(supported_protocols, unsupported_protocols)
-    verify_platform(supported_oses)
+    verify_platform(supported=supported_platforms)
     verify_cache_mode(supported_cache_modes)
     verify_aio_mode(supported_aio_modes)
 
-- 
2.24.1

From: Thomas Huth <thuth@redhat.com>

In the long run, we might want to add test 183 to the "auto" group
(but it still fails occasionally, so we cannot do that yet). However,
when running 183 in Cirrus-CI on macOS, or with our vm-build-openbsd
target, it currently always fails with an "Timeout waiting for return
on handle 0" error.

Let's mark it as supported only on systems where the test is working
most of the time (i.e. Linux, FreeBSD and NetBSD).

Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-id: 20200121095205.26323-4-thuth@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/183 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qemu-iotests/183 b/tests/qemu-iotests/183
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/183
+++ b/tests/qemu-iotests/183
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 . ./common.filter
 . ./common.qemu
 
+_supported_os Linux FreeBSD NetBSD
 _supported_fmt qcow2 raw qed quorum
 _supported_proto file
 
-- 
2.24.1

From: Thomas Huth <thuth@redhat.com>

We are going to enable 127 in the "auto" group, but it only works if
virtio-scsi and scsi-hd are available - which is not the case with
QEMU binaries like qemu-system-tricore for example, so we need a
proper check for the availability of these devices here.

A very similar problem exists in iotest 267 - it has been added to
the "auto" group already, but requires virtio-blk and thus currently
fails with qemu-system-tricore for example. Let's also add aproper
check there.

Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-id: 20200121095205.26323-5-thuth@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/127       |  2 ++
 tests/qemu-iotests/267       |  2 ++
 tests/qemu-iotests/common.rc | 14 ++++++++++++++
 3 files changed, 18 insertions(+)

diff --git a/tests/qemu-iotests/127 b/tests/qemu-iotests/127
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/127
+++ b/tests/qemu-iotests/127
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt qcow2
 _supported_proto file
 
+_require_devices virtio-scsi scsi-hd
+
 IMG_SIZE=64K
 
 _make_test_img $IMG_SIZE
diff --git a/tests/qemu-iotests/267 b/tests/qemu-iotests/267
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/267
+++ b/tests/qemu-iotests/267
@@ -XXX,XX +XXX,XX @@ _require_drivers copy-on-read
 # and generally impossible with external data files
 _unsupported_imgopts 'refcount_bits=1[^0-9]' data_file
 
+_require_devices virtio-blk
+
 do_run_qemu()
 {
     echo Testing: "$@"
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -XXX,XX +XXX,XX @@ _require_large_file()
     rm "$TEST_IMG"
 }
 
+# Check that a set of devices is available in the QEMU binary
+#
+_require_devices()
+{
+    available=$($QEMU -M none -device help | \
+                grep ^name | sed -e 's/^name "//' -e 's/".*$//')
+    for device
+    do
+        if ! echo "$available" | grep -q "$device" ; then
+            _notrun "$device not available"
+        fi
+    done
+}
+
 # make sure this script returns success
 true
-- 
2.24.1

From: Thomas Huth <thuth@redhat.com>

We are going to enable some of the python-based tests in the "auto" group,
and these tests require virtio-blk to work properly. Running iotests
without virtio-blk likely does not make too much sense anyway, so instead
of adding a check for the availability of virtio-blk to each and every
test (which does not sound very appealing), let's rather add a check for
this a central spot in the "check" script instead (so that it is still
possible to run "make check" for qemu-system-tricore for example).

Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-id: 20200121095205.26323-6-thuth@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/check | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

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 @@ fi
 python_usable=false
 if $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)'
 then
-    python_usable=true
+    # Our python framework also requires virtio-blk
+    if "$QEMU_PROG" -M none -device help | grep -q virtio-blk >/dev/null 2>&1
+    then
+        python_usable=true
+    else
+        python_unusable_because="Missing virtio-blk in QEMU binary"
+    fi
+else
+    python_unusable_because="Unsupported Python version"
 fi
 
 default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p')
@@ -XXX,XX +XXX,XX @@ do
                 run_command="$PYTHON $seq"
             else
                 run_command="false"
-                echo "Unsupported Python version" > $seq.notrun
+                echo "$python_unusable_because" > $seq.notrun
             fi
         else
             run_command="./$seq"
-- 
2.24.1

From: Thomas Huth <thuth@redhat.com>

According to Kevin, tests 030, 040 and 041 are among the most valuable
tests that we have, so we should always run them if possible, even if
they take a little bit longer.

According to Max, it would be good to have a test for iothreads and
migration. 127 and 256 seem to be good candidates for iothreads. For
migration, let's enable 181 and 203 (which also tests iothreads).
(091 would be a good candidate for migration, too, but Alex Bennée
reported that this test fails on ZFS file systems, so it can't be
included yet)

Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-id: 20200121095205.26323-7-thuth@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/group | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

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 @@
 027 rw auto quick
 028 rw backing quick
 029 rw auto quick
-030 rw backing
+030 rw auto backing
 031 rw auto quick
 032 rw auto quick
 033 rw auto quick
@@ -XXX,XX +XXX,XX @@
 037 rw auto backing quick
 038 rw auto backing quick
 039 rw auto quick
-040 rw
-041 rw backing
+040 rw auto
+041 rw auto backing
 042 rw auto quick
 043 rw auto backing
 044 rw
@@ -XXX,XX +XXX,XX @@
 124 rw backing
 125 rw
 126 rw auto backing
-127 rw backing quick
+127 rw auto backing quick
 128 rw quick
 129 rw quick
 130 rw quick
@@ -XXX,XX +XXX,XX @@
 177 rw auto quick
 178 img
 179 rw auto quick
-181 rw migration
+181 rw auto migration
 182 rw quick
 183 rw migration
 184 rw auto quick
@@ -XXX,XX +XXX,XX @@
 200 rw
 201 rw migration
 202 rw quick
-203 rw migration
+203 rw auto migration
 204 rw quick
 205 rw quick
 206 rw
@@ -XXX,XX +XXX,XX @@
 253 rw quick
 254 rw backing quick
 255 rw quick
-256 rw quick
+256 rw auto quick
 257 rw
 258 rw quick
 260 rw quick
-- 
2.24.1

From: Alberto Garcia <berto@igalia.com>

The L1 table is read from disk using the byte-based bdrv_pread() and
is never accessed beyond its last element, so there's no need to
allocate more memory than that.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: b2e27214ec7b03a585931bcf383ee1ac3a641a10.1579374329.git.berto@igalia.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2-cluster.c  | 5 ++---
 block/qcow2-refcount.c | 2 +-
 block/qcow2-snapshot.c | 3 +--
 block/qcow2.c          | 2 +-
 4 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -XXX,XX +XXX,XX @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
 #endif
 
     new_l1_size2 = sizeof(uint64_t) * new_l1_size;
-    new_l1_table = qemu_try_blockalign(bs->file->bs,
-                                       ROUND_UP(new_l1_size2, 512));
+    new_l1_table = qemu_try_blockalign(bs->file->bs, new_l1_size2);
     if (new_l1_table == NULL) {
         return -ENOMEM;
     }
-    memset(new_l1_table, 0, ROUND_UP(new_l1_size2, 512));
+    memset(new_l1_table, 0, new_l1_size2);
 
     if (s->l1_size) {
         memcpy(new_l1_table, s->l1_table, s->l1_size * sizeof(uint64_t));
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -XXX,XX +XXX,XX @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
      * l1_table_offset when it is the current s->l1_table_offset! Be careful
      * when changing this! */
     if (l1_table_offset != s->l1_table_offset) {
-        l1_table = g_try_malloc0(ROUND_UP(l1_size2, 512));
+        l1_table = g_try_malloc0(l1_size2);
         if (l1_size2 && l1_table == NULL) {
             ret = -ENOMEM;
             goto fail;
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -XXX,XX +XXX,XX @@ int qcow2_snapshot_load_tmp(BlockDriverState *bs,
         return ret;
     }
     new_l1_bytes = sn->l1_size * sizeof(uint64_t);
-    new_l1_table = qemu_try_blockalign(bs->file->bs,
-                                       ROUND_UP(new_l1_bytes, 512));
+    new_l1_table = qemu_try_blockalign(bs->file->bs, new_l1_bytes);
     if (new_l1_table == NULL) {
         return -ENOMEM;
     }
diff --git a/block/qcow2.c b/block/qcow2.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
 
     if (s->l1_size > 0) {
         s->l1_table = qemu_try_blockalign(bs->file->bs,
-            ROUND_UP(s->l1_size * sizeof(uint64_t), 512));
+                                          s->l1_size * sizeof(uint64_t));
         if (s->l1_table == NULL) {
             error_setg(errp, "Could not allocate L1 table");
             ret = -ENOMEM;
-- 
2.24.1

From: Alberto Garcia <berto@igalia.com>

qcow2_alloc_cluster_offset() and qcow2_get_cluster_offset() always
return offsets that are cluster-aligned so don't just check that they
are sector-aligned.

The check in qcow2_co_preadv_task() is also replaced by an assertion
for the same reason.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 558ba339965f858bede4c73ce3f50f0c0493597d.1579374329.git.berto@igalia.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_preadv_task(BlockDriverState *bs,
                                           offset, bytes, qiov, qiov_offset);
 
     case QCOW2_CLUSTER_NORMAL:
-        if ((file_cluster_offset & 511) != 0) {
-            return -EIO;
-        }
-
+        assert(offset_into_cluster(s, file_cluster_offset) == 0);
         if (bs->encrypted) {
             return qcow2_co_preadv_encrypted(bs, file_cluster_offset,
                                              offset, bytes, qiov, qiov_offset);
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_pwritev_part(
             goto out_locked;
         }
 
-        assert((cluster_offset & 511) == 0);
+        assert(offset_into_cluster(s, cluster_offset) == 0);
 
         ret = qcow2_pre_write_overlap_check(bs, 0,
                                             cluster_offset + offset_in_cluster,
@@ -XXX,XX +XXX,XX @@ qcow2_co_copy_range_to(BlockDriverState *bs,
             goto fail;
         }
 
-        assert((cluster_offset & 511) == 0);
+        assert(offset_into_cluster(s, cluster_offset) == 0);
 
         ret = qcow2_pre_write_overlap_check(bs, 0,
                 cluster_offset + offset_in_cluster, cur_bytes, true);
-- 
2.24.1

From: Alberto Garcia <berto@igalia.com>

When updating an L1 entry the qcow2 driver writes a (512-byte) sector
worth of data to avoid a read-modify-write cycle. Instead of always
writing 512 bytes we should follow the alignment requirements of the
storage backend.

(the only exception is when the alignment is larger than the cluster
size because then we could be overwriting data after the L1 table)

Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-id: 71f34d4ae4b367b32fb36134acbf4f4f7ee681f4.1579374329.git.berto@igalia.com
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2-cluster.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -XXX,XX +XXX,XX @@ static int l2_load(BlockDriverState *bs, uint64_t offset,
 }
 
 /*
- * Writes one sector of the L1 table to the disk (can't update single entries
- * and we really don't want bdrv_pread to perform a read-modify-write)
+ * Writes an L1 entry to disk (note that depending on the alignment
+ * requirements this function may write more that just one entry in
+ * order to prevent bdrv_pwrite from performing a read-modify-write)
  */
-#define L1_ENTRIES_PER_SECTOR (512 / 8)
 int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index)
 {
     BDRVQcow2State *s = bs->opaque;
-    uint64_t buf[L1_ENTRIES_PER_SECTOR] = { 0 };
     int l1_start_index;
     int i, ret;
+    int bufsize = MAX(sizeof(uint64_t),
+                      MIN(bs->file->bs->bl.request_alignment, s->cluster_size));
+    int nentries = bufsize / sizeof(uint64_t);
+    g_autofree uint64_t *buf = g_try_new0(uint64_t, nentries);
 
-    l1_start_index = l1_index & ~(L1_ENTRIES_PER_SECTOR - 1);
-    for (i = 0; i < L1_ENTRIES_PER_SECTOR && l1_start_index + i < s->l1_size;
-         i++)
-    {
+    if (buf == NULL) {
+        return -ENOMEM;
+    }
+
+    l1_start_index = QEMU_ALIGN_DOWN(l1_index, nentries);
+    for (i = 0; i < MIN(nentries, s->l1_size - l1_start_index); i++) {
         buf[i] = cpu_to_be64(s->l1_table[l1_start_index + i]);
     }
 
     ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_ACTIVE_L1,
-            s->l1_table_offset + 8 * l1_start_index, sizeof(buf), false);
+            s->l1_table_offset + 8 * l1_start_index, bufsize, false);
     if (ret < 0) {
         return ret;
     }
@@ -XXX,XX +XXX,XX @@ int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index)
     BLKDBG_EVENT(bs->file, BLKDBG_L1_UPDATE);
     ret = bdrv_pwrite_sync(bs->file,
                            s->l1_table_offset + 8 * l1_start_index,
-                           buf, sizeof(buf));
+                           buf, bufsize);
     if (ret < 0) {
         return ret;
     }
-- 
2.24.1

From: Alberto Garcia <berto@igalia.com>

qemu-img's convert_co_copy_range() operates at the sector level and
block_copy() operates at the cluster level so this condition is always
true, but it is not necessary to restrict this here, so let's leave it
to the driver implementation return an error if there is any.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-id: a4264aaee656910c84161a2965f7a501437379ca.1579374329.git.berto@igalia.com
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -XXX,XX +XXX,XX @@ qcow2_co_copy_range_from(BlockDriverState *bs,
         case QCOW2_CLUSTER_NORMAL:
             child = s->data_file;
             copy_offset += offset_into_cluster(s, src_offset);
-            if ((copy_offset & 511) != 0) {
-                ret = -EIO;
-                goto out;
-            }
             break;
 
         default:
-- 
2.24.1

From: Alberto Garcia <berto@igalia.com>

This replaces all remaining instances in the qcow2 code.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-id: b5f74b606c2d9873b12d29acdb7fd498029c4025.1579374329.git.berto@igalia.com
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -XXX,XX +XXX,XX @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
 
     /* Validate options and set default values */
     if (!QEMU_IS_ALIGNED(qcow2_opts->size, BDRV_SECTOR_SIZE)) {
-        error_setg(errp, "Image size must be a multiple of 512 bytes");
+        error_setg(errp, "Image size must be a multiple of %u bytes",
+                   (unsigned) BDRV_SECTOR_SIZE);
         ret = -EINVAL;
         goto out;
     }
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
         return -ENOTSUP;
     }
 
-    if (offset & 511) {
-        error_setg(errp, "The new size must be a multiple of 512");
+    if (!QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)) {
+        error_setg(errp, "The new size must be a multiple of %u",
+                   (unsigned) BDRV_SECTOR_SIZE);
         return -EINVAL;
     }
 
-- 
2.24.1

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

We can't access top after call bdrv_backup_top_drop, as it is already
freed at this time.

Also, no needs to unref target child by hand, it will be unrefed on
bdrv_close() automatically.

So, just do bdrv_backup_top_drop if append succeed and one bdrv_unref
otherwise.

Note, that in !appended case bdrv_unref(top) moved into drained section
on source. It doesn't really matter, but just for code simplicity.

Fixes: 7df7868b96404
Cc: qemu-stable@nongnu.org # v4.2.0
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 20200121142802.21467-2-vsementsov@virtuozzo.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/backup-top.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/block/backup-top.c b/block/backup-top.c
index XXXXXXX..XXXXXXX 100644
--- a/block/backup-top.c
+++ b/block/backup-top.c
@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
     BlockDriverState *top = bdrv_new_open_driver(&bdrv_backup_top_filter,
                                                  filter_node_name,
                                                  BDRV_O_RDWR, errp);
+    bool appended = false;
 
     if (!top) {
         return NULL;
@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
     bdrv_append(top, source, &local_err);
     if (local_err) {
         error_prepend(&local_err, "Cannot append backup-top filter: ");
-        goto append_failed;
+        goto fail;
     }
+    appended = true;
 
     /*
      * bdrv_append() finished successfully, now we can require permissions
@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
     if (local_err) {
         error_prepend(&local_err,
                       "Cannot set permissions for backup-top filter: ");
-        goto failed_after_append;
+        goto fail;
     }
 
     state->bcs = block_copy_state_new(top->backing, state->target,
                                       cluster_size, write_flags, &local_err);
     if (local_err) {
         error_prepend(&local_err, "Cannot create block-copy-state: ");
-        goto failed_after_append;
+        goto fail;
     }
     *bcs = state->bcs;
 
@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
 
     return top;
 
-failed_after_append:
-    state->active = false;
-    bdrv_backup_top_drop(top);
+fail:
+    if (appended) {
+        state->active = false;
+        bdrv_backup_top_drop(top);
+    } else {
+        bdrv_unref(top);
+    }
 
-append_failed:
     bdrv_drained_end(source);
-    bdrv_unref_child(top, state->target);
-    bdrv_unref(top);
     error_propagate(errp, local_err);
 
     return NULL;
-- 
2.24.1

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

This test checks that bug is really fixed by previous commit.

Cc: qemu-stable@nongnu.org # v4.2.0
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20200121142802.21467-3-vsementsov@virtuozzo.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/283     | 92 ++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/283.out |  8 ++++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 101 insertions(+)
 create mode 100644 tests/qemu-iotests/283
 create mode 100644 tests/qemu-iotests/283.out

diff --git a/tests/qemu-iotests/283 b/tests/qemu-iotests/283
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qemu-iotests/283
@@ -XXX,XX +XXX,XX @@
+#!/usr/bin/env python
+#
+# Test for backup-top filter permission activation failure
+#
+# Copyright (c) 2019 Virtuozzo International GmbH.
+#
+# 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/>.
+#
+
+import iotests
+
+# The test is unrelated to formats, restrict it to qcow2 to avoid extra runs
+iotests.verify_image_format(supported_fmts=['qcow2'])
+
+size = 1024 * 1024
+
+""" Test description
+
+When performing a backup, all writes on the source subtree must go through the
+backup-top filter so it can copy all data to the target before it is changed.
+backup-top filter is appended above source node, to achieve this thing, so all
+parents of source node are handled. A configuration with side parents of source
+sub-tree with write permission is unsupported (we'd have append several
+backup-top filter like nodes to handle such parents). The test create an
+example of such configuration and checks that a backup is then not allowed
+(blockdev-backup command should fail).
+
+The configuration:
+
+    ┌────────┐  target  ┌─────────────┐
+    │ target │ ◀─────── │ backup_top  │
+    └────────┘          └─────────────┘
+                            │
+                            │ backing
+                            ▼
+                        ┌─────────────┐
+                        │   source    │
+                        └─────────────┘
+                            │
+                            │ file
+                            ▼
+                        ┌─────────────┐  write perm   ┌───────┐
+                        │    base     │ ◀──────────── │ other │
+                        └─────────────┘               └───────┘
+
+On activation (see .active field of backup-top state in block/backup-top.c),
+backup-top is going to unshare write permission on its source child. Write
+unsharing will be propagated to the "source->base" link and will conflict with
+other node write permission. So permission update will fail and backup job will
+not be started.
+
+Note, that the only thing which prevents backup of running on such
+configuration is default permission propagation scheme. It may be altered by
+different block drivers, so backup will run in invalid configuration. But
+something is better than nothing. Also, before the previous commit (commit
+preceding this test creation), starting backup on such configuration led to
+crash, so current "something" is a lot better, and this test actual goal is
+to check that crash is fixed :)
+"""
+
+vm = iotests.VM()
+vm.launch()
+
+vm.qmp_log('blockdev-add', **{'node-name': 'target', 'driver': 'null-co'})
+
+vm.qmp_log('blockdev-add', **{
+    'node-name': 'source',
+    'driver': 'blkdebug',
+    'image': {'node-name': 'base', 'driver': 'null-co', 'size': size}
+})
+
+vm.qmp_log('blockdev-add', **{
+    'node-name': 'other',
+    'driver': 'blkdebug',
+    'image': 'base',
+    'take-child-perms': ['write']
+})
+
+vm.qmp_log('blockdev-backup', sync='full', device='source', target='target')
+
+vm.shutdown()
diff --git a/tests/qemu-iotests/283.out b/tests/qemu-iotests/283.out
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qemu-iotests/283.out
@@ -XXX,XX +XXX,XX @@
+{"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "target"}}
+{"return": {}}
+{"execute": "blockdev-add", "arguments": {"driver": "blkdebug", "image": {"driver": "null-co", "node-name": "base", "size": 1048576}, "node-name": "source"}}
+{"return": {}}
+{"execute": "blockdev-add", "arguments": {"driver": "blkdebug", "image": "base", "node-name": "other", "take-child-perms": ["write"]}}
+{"return": {}}
+{"execute": "blockdev-backup", "arguments": {"device": "source", "sync": "full", "target": "target"}}
+{"error": {"class": "GenericError", "desc": "Cannot set permissions for backup-top filter: Conflicts with use by other as 'image', which uses 'write' on base"}}
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
+283 auto quick
-- 
2.24.1