Series comparison

-[Qemu-devel] [PULL 0/5] Block patches
+[PULL 00/17] Block patches
-The following changes since commit d147f7e815f97cb477e223586bcb80c316ae10ea:
+The following changes since commit 418fa86dd465b4fd8394373cf83db8fa65d7611c:
-  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2017-10-03 16:27:24 +0100)
+  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:
+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 f708a5e71cba0d784e307334c07ade5f56f827ab:
+for you to fetch changes up to a541fcc27c98b96da187c7d4573f3270f3ddd283:
-  aio: fix assert when remove poll during destroy (2017-10-03 14:36:19 -0400)
+  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
-Peter Xu (4):
+John Snow (1):
-  qom: provide root container for internal objs
+  iotests: remove 'linux' from default supported platforms
   iothread: provide helpers for internal use
   iothread: export iothread_stop()
   iothread: delay the context release to finalize
-Stefan Hajnoczi (1):
+Pan Nengyuan (1):
-  aio: fix assert when remove poll during destroy
+  block: fix memleaks in bdrv_refresh_filename
- include/qom/object.h      | 11 +++++++++++
+Thomas Huth (5):
- include/sysemu/iothread.h |  9 +++++++++
+  iotests: Test 041 only works on certain systems
- iothread.c                | 46 ++++++++++++++++++++++++++++++++++++----------
+  iotests: Test 183 does not work on macOS and OpenBSD
- qom/object.c              | 11 +++++++++++
+  iotests: Check for the availability of the required devices in 267 and
- util/aio-posix.c          |  9 ++++++++-
-files changed, 75 insertions(+), 11 deletions(-)
+  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
 --
-.13.6
+.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

-[Qemu-devel] [PULL 4/5] iothread: delay the context release to finalize
+[PULL 02/17] block: Use a GString in bdrv_perm_names()
-From: Peter Xu <peterx@redhat.com>
+From: Alberto Garcia <berto@igalia.com>
-When gcontext is used with iothread, the context will be destroyed
+This is a bit more efficient than having to allocate and free memory
-during iothread_stop().  That's not good since sometimes we would like
+for each new permission.
 to keep the resources until iothread is destroyed, but we may want to
 stop the thread before that point.
-Delay the destruction of gcontext to iothread finalize.  Then we can do:
+The default size (30) is enough for "consistent read, write, resize".
-  iothread_stop(thread);
+Signed-off-by: Alberto Garcia <berto@igalia.com>
-  some_cleanup_on_resources();
+Message-id: 20200110171518.22168-1-berto@igalia.com
-  iothread_destroy(thread);
+Reviewed-by: Eric Blake <eblake@redhat.com>
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  block.c | 11 ++++++-----
 file changed, 6 insertions(+), 5 deletions(-)
-We may need this patch if we want to run chardev IOs in iothreads and
+diff --git a/block.c b/block.c
 hopefully clean them up correctly.  For more specific information,
 please see 2b316774f6 ("qemu-char: do not operate on sources from
 finalize callbacks").
 Reviewed-by: Fam Zheng <famz@redhat.com>
 Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
 Signed-off-by: Peter Xu <peterx@redhat.com>
 Message-id: 20170928025958.1420-5-peterx@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
  iothread.c | 6 ++++--
 file changed, 4 insertions(+), 2 deletions(-)
 diff --git a/iothread.c b/iothread.c
 index XXXXXXX..XXXXXXX 100644
---- a/iothread.c
+--- a/block.c
-+++ b/iothread.c
++++ b/block.c
-@@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque)
+@@ -XXX,XX +XXX,XX @@ char *bdrv_perm_names(uint64_t perm)
-             g_main_loop_unref(loop);
+         { 0, NULL }
+     };
-             g_main_context_pop_thread_default(iothread->worker_context);
--            g_main_context_unref(iothread->worker_context);
+-    char *result = g_strdup("");
--            iothread->worker_context = NULL;
++    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);
          }
      }
-@@ -XXX,XX +XXX,XX @@ static void iothread_instance_finalize(Object *obj)
+-    return result;
-     IOThread *iothread = IOTHREAD(obj);
++    return g_string_free(result, FALSE);
+ }
-     iothread_stop(iothread);
-+    if (iothread->worker_context) {
+ /*
 +        g_main_context_unref(iothread->worker_context);
 +        iothread->worker_context = NULL;
 +    }
      qemu_cond_destroy(&iothread->init_done_cond);
      qemu_mutex_destroy(&iothread->init_done_lock);
      if (!iothread->ctx) {
 --
-.13.6
+.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

-[Qemu-devel] [PULL 3/5] iothread: export iothread_stop()
+[PULL 08/17] iotests: Check for the availability of the required devices in 267 and 127
-From: Peter Xu <peterx@redhat.com>
+From: Thomas Huth <thuth@redhat.com>
-So that internal iothread users can explicitly stop one iothread without
+We are going to enable 127 in the "auto" group, but it only works if
-destroying it.
+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.
-Since at it, fix iothread_stop() to allow it to be called multiple
+A very similar problem exists in iotest 267 - it has been added to
-times.  Before this patch we may call iothread_stop() more than once on
+the "auto" group already, but requires virtio-blk and thus currently
-single iothread, while that may not be correct since qemu_thread_join()
+fails with qemu-system-tricore for example. Let's also add aproper
-is not allowed to run twice.  From manual of pthread_join():
+check there.
-  Joining with a thread that has previously been joined results in
+Reviewed-by: Max Reitz <mreitz@redhat.com>
-  undefined behavior.
+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(+)
-Reviewed-by: Fam Zheng <famz@redhat.com>
+diff --git a/tests/qemu-iotests/127 b/tests/qemu-iotests/127
-Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
+index XXXXXXX..XXXXXXX 100755
-Signed-off-by: Peter Xu <peterx@redhat.com>
+--- a/tests/qemu-iotests/127
-Message-id: 20170928025958.1420-4-peterx@redhat.com
++++ b/tests/qemu-iotests/127
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
----
+ _supported_fmt qcow2
- include/sysemu/iothread.h |  1 +
+ _supported_proto file
- iothread.c                | 24 ++++++++++++++++--------
-files changed, 17 insertions(+), 8 deletions(-)
++_require_devices virtio-scsi scsi-hd
++
-diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
+ 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/include/sysemu/iothread.h
+--- a/tests/qemu-iotests/common.rc
-+++ b/include/sysemu/iothread.h
++++ b/tests/qemu-iotests/common.rc
-@@ -XXX,XX +XXX,XX @@ GMainContext *iothread_get_g_main_context(IOThread *iothread);
+@@ -XXX,XX +XXX,XX @@ _require_large_file()
-  * "query-iothreads".
+     rm "$TEST_IMG"
   */
  IOThread *iothread_create(const char *id, Error **errp);
 +void iothread_stop(IOThread *iothread);
  void iothread_destroy(IOThread *iothread);
  #endif /* IOTHREAD_H */
 diff --git a/iothread.c b/iothread.c
 index XXXXXXX..XXXXXXX 100644
 --- a/iothread.c
 +++ b/iothread.c
@@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque)
      return NULL;
  }
--static int iothread_stop(Object *object, void *opaque)
++# Check that a set of devices is available in the QEMU binary
-+void iothread_stop(IOThread *iothread)
++#
- {
++_require_devices()
--    IOThread *iothread;
++{
--
++    available=$($QEMU -M none -device help | \
--    iothread = (IOThread *)object_dynamic_cast(object, TYPE_IOTHREAD);
++                grep ^name | sed -e 's/^name "//' -e 's/".*$//')
--    if (!iothread || !iothread->ctx || iothread->stopping) {
++    for device
--        return 0;
++    do
-+    if (!iothread->ctx || iothread->stopping) {
++        if ! echo "$available" | grep -q "$device" ; then
-+        return;
++            _notrun "$device not available"
-     }
++        fi
-     iothread->stopping = true;
++    done
      aio_notify(iothread->ctx);
@@ -XXX,XX +XXX,XX @@ static int iothread_stop(Object *object, void *opaque)
          g_main_loop_quit(iothread->main_loop);
      }
      qemu_thread_join(&iothread->thread);
 +}
 +
-+static int iothread_stop_iter(Object *object, void *opaque)
+ # make sure this script returns success
-+{
+ true
 +    IOThread *iothread;
 +
 +    iothread = (IOThread *)object_dynamic_cast(object, TYPE_IOTHREAD);
 +    if (!iothread) {
 +        return 0;
 +    }
 +    iothread_stop(iothread);
      return 0;
  }
@@ -XXX,XX +XXX,XX @@ static void iothread_instance_finalize(Object *obj)
  {
      IOThread *iothread = IOTHREAD(obj);
 -    iothread_stop(obj, NULL);
 +    iothread_stop(iothread);
      qemu_cond_destroy(&iothread->init_done_cond);
      qemu_mutex_destroy(&iothread->init_done_lock);
      if (!iothread->ctx) {
@@ -XXX,XX +XXX,XX @@ void iothread_stop_all(void)
          aio_context_release(ctx);
      }
 -    object_child_foreach(container, iothread_stop, NULL);
 +    object_child_foreach(container, iothread_stop_iter, NULL);
  }
  static gpointer iothread_g_main_context_init(gpointer opaque)
 --
-.13.6
+.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

-[Qemu-devel] [PULL 5/5] aio: fix assert when remove poll during destroy
+[PULL 12/17] qcow2: Tighten cluster_offset alignment assertions
-After iothread is enabled internally inside QEMU with GMainContext, we
+From: Alberto Garcia <berto@igalia.com>
 may encounter this warning when destroying the iothread:
-(qemu-system-x86_64:19925): GLib-CRITICAL **: g_source_remove_poll:
+qcow2_alloc_cluster_offset() and qcow2_get_cluster_offset() always
- assertion '!SOURCE_DESTROYED (source)' failed
+return offsets that are cluster-aligned so don't just check that they
 are sector-aligned.
-The problem is that g_source_remove_poll() does not allow to remove one
+The check in qcow2_co_preadv_task() is also replaced by an assertion
-source from array if the source is detached from its owner
+for the same reason.
 context. (peterx: which IMHO does not make much sense)
-Fix it on QEMU side by avoid calling g_source_remove_poll() if we know
+Signed-off-by: Alberto Garcia <berto@igalia.com>
-the object is during destruction, and we won't leak anything after all
+Reviewed-by: Max Reitz <mreitz@redhat.com>
-since the array will be gone soon cleanly even with that fd.
+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(-)
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+diff --git a/block/qcow2.c b/block/qcow2.c
 Reviewed-by: Fam Zheng <famz@redhat.com>
 Signed-off-by: Peter Xu <peterx@redhat.com>
 Message-id: 20170928025958.1420-6-peterx@redhat.com
 [peterx: write the commit message]
 Signed-off-by: Peter Xu <peterx@redhat.com>
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
  util/aio-posix.c | 9 ++++++++-
 file changed, 8 insertions(+), 1 deletion(-)
 diff --git a/util/aio-posix.c b/util/aio-posix.c
 index XXXXXXX..XXXXXXX 100644
---- a/util/aio-posix.c
+--- a/block/qcow2.c
-+++ b/util/aio-posix.c
++++ b/block/qcow2.c
-@@ -XXX,XX +XXX,XX @@ void aio_set_fd_handler(AioContext *ctx,
+@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_preadv_task(BlockDriverState *bs,
-             return;
+                                           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;
          }
--        g_source_remove_poll(&ctx->source, &node->pfd);
+-        assert((cluster_offset & 511) == 0);
-+        /* If the GSource is in the process of being destroyed then
++        assert(offset_into_cluster(s, cluster_offset) == 0);
-+         * g_source_remove_poll() causes an assertion failure.  Skip
-+         * removal in that case, because glib cleans up its state during
+         ret = qcow2_pre_write_overlap_check(bs, 0,
-+         * destruction anyway.
+                                             cluster_offset + offset_in_cluster,
-+         */
+@@ -XXX,XX +XXX,XX @@ qcow2_co_copy_range_to(BlockDriverState *bs,
-+        if (!g_source_is_destroyed(&ctx->source)) {
+             goto fail;
-+            g_source_remove_poll(&ctx->source, &node->pfd);
+         }
-+        }
+-        assert((cluster_offset & 511) == 0);
-         /* If the lock is held, just mark the node as deleted */
++        assert(offset_into_cluster(s, cluster_offset) == 0);
-         if (qemu_lockcnt_count(&ctx->list_lock)) {
          ret = qcow2_pre_write_overlap_check(bs, 0,
                  cluster_offset + offset_in_cluster, cur_bytes, true);
 --
-.13.6
+.24.1

-[Qemu-devel] [PULL 1/5] qom: provide root container for internal objs
+[PULL 13/17] qcow2: Use bs->bl.request_alignment when updating an L1 entry
-From: Peter Xu <peterx@redhat.com>
+From: Alberto Garcia <berto@igalia.com>
-We have object_get_objects_root() to keep user created objects, however
+When updating an L1 entry the qcow2 driver writes a (512-byte) sector
-no place for objects that will be used internally.  Create such a
+worth of data to avoid a read-modify-write cycle. Instead of always
-container for internal objects.
+writing 512 bytes we should follow the alignment requirements of the
 storage backend.
-CC: Andreas Färber <afaerber@suse.de>
+(the only exception is when the alignment is larger than the cluster
-CC: Markus Armbruster <armbru@redhat.com>
+size because then we could be overwriting data after the L1 table)
-CC: Paolo Bonzini <pbonzini@redhat.com>
-Suggested-by: Daniel P. Berrange <berrange@redhat.com>
+Signed-off-by: Alberto Garcia <berto@igalia.com>
-Signed-off-by: Peter Xu <peterx@redhat.com>
+Message-id: 71f34d4ae4b367b32fb36134acbf4f4f7ee681f4.1579374329.git.berto@igalia.com
-Reviewed-by: Fam Zheng <famz@redhat.com>
+Reviewed-by: Max Reitz <mreitz@redhat.com>
-Message-id: 20170928025958.1420-2-peterx@redhat.com
+Signed-off-by: Max Reitz <mreitz@redhat.com>
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- include/qom/object.h | 11 +++++++++++
+ block/qcow2-cluster.c | 25 +++++++++++++++----------
- qom/object.c         | 11 +++++++++++
+file changed, 15 insertions(+), 10 deletions(-)
 files changed, 22 insertions(+)
-diff --git a/include/qom/object.h b/include/qom/object.h
+diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
 index XXXXXXX..XXXXXXX 100644
---- a/include/qom/object.h
+--- a/block/qcow2-cluster.c
-+++ b/include/qom/object.h
++++ b/block/qcow2-cluster.c
-@@ -XXX,XX +XXX,XX @@ Object *object_get_root(void);
+@@ -XXX,XX +XXX,XX @@ static int l2_load(BlockDriverState *bs, uint64_t offset,
  Object *object_get_objects_root(void);
  /**
 + * object_get_internal_root:
 + *
 + * Get the container object that holds internally used object
 + * instances.  Any object which is put into this container must not be
 + * user visible, and it will not be exposed in the QOM tree.
 + *
 + * Returns: the internal object container
 + */
 +Object *object_get_internal_root(void);
 +
 +/**
   * object_get_canonical_path_component:
   *
   * Returns: The final component in the object's canonical path.  The canonical
 diff --git a/qom/object.c b/qom/object.c
 index XXXXXXX..XXXXXXX 100644
 --- a/qom/object.c
 +++ b/qom/object.c
@@ -XXX,XX +XXX,XX @@ Object *object_get_objects_root(void)
      return container_get(object_get_root(), "/objects");
  }
-+Object *object_get_internal_root(void)
+ /*
-+{
+- * Writes one sector of the L1 table to the disk (can't update single entries
-+    static Object *internal_root;
+- * 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
-+    if (!internal_root) {
++ * requirements this function may write more that just one entry in
-+        internal_root = object_new("container");
++ * 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;
 +    }
 +
-+    return internal_root;
++    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]);
- static void object_get_child_property(Object *obj, Visitor *v,
+     }
-                                       const char *name, void *opaque,
-                                       Error **errp)
+     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;
      }
 --
-.13.6
+.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 2/5] iothread: provide helpers for internal use
+[PULL 17/17] iotests: add test for backup-top failure on permission activation
-From: Peter Xu <peterx@redhat.com>
+From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-IOThread is a general framework that contains IO loop environment and a
+This test checks that bug is really fixed by previous commit.
 real thread behind.  It's also good to be used internally inside qemu.
 Provide some helpers for it to create iothreads to be used internally.
-Put all the internal used iothreads into the internal object container.
+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 +
 files changed, 101 insertions(+)
  create mode 100644 tests/qemu-iotests/283
  create mode 100644 tests/qemu-iotests/283.out
-Reviewed-by: Fam Zheng <famz@redhat.com>
+diff --git a/tests/qemu-iotests/283 b/tests/qemu-iotests/283
-Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
+new file mode 100644
-Signed-off-by: Peter Xu <peterx@redhat.com>
+index XXXXXXX..XXXXXXX
-Message-id: 20170928025958.1420-3-peterx@redhat.com
+--- /dev/null
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
++++ b/tests/qemu-iotests/283
----
+@@ -XXX,XX +XXX,XX @@
- include/sysemu/iothread.h |  8 ++++++++
++#!/usr/bin/env python
- iothread.c                | 16 ++++++++++++++++
++#
-files changed, 24 insertions(+)
++# Test for backup-top filter permission activation failure
++#
-diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
++# 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/include/sysemu/iothread.h
+--- a/tests/qemu-iotests/group
-+++ b/include/sysemu/iothread.h
++++ b/tests/qemu-iotests/group
-@@ -XXX,XX +XXX,XX @@ AioContext *iothread_get_aio_context(IOThread *iothread);
+@@ -XXX,XX +XXX,XX @@
- void iothread_stop_all(void);
+rw backing quick
- GMainContext *iothread_get_g_main_context(IOThread *iothread);
+rw migration quick
+rw quick
-+/*
++283 auto quick
 + * Helpers used to allocate iothreads for internal use.  These
 + * iothreads will not be seen by monitor clients when query using
 + * "query-iothreads".
 + */
 +IOThread *iothread_create(const char *id, Error **errp);
 +void iothread_destroy(IOThread *iothread);
 +
  #endif /* IOTHREAD_H */
 diff --git a/iothread.c b/iothread.c
 index XXXXXXX..XXXXXXX 100644
 --- a/iothread.c
 +++ b/iothread.c
@@ -XXX,XX +XXX,XX @@ GMainContext *iothread_get_g_main_context(IOThread *iothread)
      return iothread->worker_context;
  }
 +
 +IOThread *iothread_create(const char *id, Error **errp)
 +{
 +    Object *obj;
 +
 +    obj = object_new_with_props(TYPE_IOTHREAD,
 +                                object_get_internal_root(),
 +                                id, errp, NULL);
 +
 +    return IOTHREAD(obj);
 +}
 +
 +void iothread_destroy(IOThread *iothread)
 +{
 +    object_unparent(OBJECT(iothread));
 +}
 --
-.13.6
+.24.1

The following changes since commit d147f7e815f97cb477e223586bcb80c316ae10ea:

Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2017-10-03 16:27:24 +0100)

are available in the git repository at:

git://github.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to f708a5e71cba0d784e307334c07ade5f56f827ab:

aio: fix assert when remove poll during destroy (2017-10-03 14:36:19 -0400)

----------------------------------------------------------------

Peter Xu (4):
  qom: provide root container for internal objs
  iothread: provide helpers for internal use
  iothread: export iothread_stop()
  iothread: delay the context release to finalize

Stefan Hajnoczi (1):
  aio: fix assert when remove poll during destroy

-- 
2.13.6

From: Peter Xu <peterx@redhat.com>

We have object_get_objects_root() to keep user created objects, however
no place for objects that will be used internally.  Create such a
container for internal objects.

CC: Andreas Färber <afaerber@suse.de>
CC: Markus Armbruster <armbru@redhat.com>
CC: Paolo Bonzini <pbonzini@redhat.com>
Suggested-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Message-id: 20170928025958.1420-2-peterx@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/qom/object.h | 11 +++++++++++
 qom/object.c         | 11 +++++++++++
 2 files changed, 22 insertions(+)

diff --git a/include/qom/object.h b/include/qom/object.h
index XXXXXXX..XXXXXXX 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -XXX,XX +XXX,XX @@ Object *object_get_root(void);
 Object *object_get_objects_root(void);
 
 /**
+ * object_get_internal_root:
+ *
+ * Get the container object that holds internally used object
+ * instances.  Any object which is put into this container must not be
+ * user visible, and it will not be exposed in the QOM tree.
+ *
+ * Returns: the internal object container
+ */
+Object *object_get_internal_root(void);
+
+/**
  * object_get_canonical_path_component:
  *
  * Returns: The final component in the object's canonical path.  The canonical
diff --git a/qom/object.c b/qom/object.c
index XXXXXXX..XXXXXXX 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -XXX,XX +XXX,XX @@ Object *object_get_objects_root(void)
     return container_get(object_get_root(), "/objects");
 }
 
+Object *object_get_internal_root(void)
+{
+    static Object *internal_root;
+
+    if (!internal_root) {
+        internal_root = object_new("container");
+    }
+
+    return internal_root;
+}
+
 static void object_get_child_property(Object *obj, Visitor *v,
                                       const char *name, void *opaque,
                                       Error **errp)
-- 
2.13.6

From: Peter Xu <peterx@redhat.com>

IOThread is a general framework that contains IO loop environment and a
real thread behind.  It's also good to be used internally inside qemu.
Provide some helpers for it to create iothreads to be used internally.

Put all the internal used iothreads into the internal object container.

Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-id: 20170928025958.1420-3-peterx@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/sysemu/iothread.h |  8 ++++++++
 iothread.c                | 16 ++++++++++++++++
 2 files changed, 24 insertions(+)

From: Peter Xu <peterx@redhat.com>

So that internal iothread users can explicitly stop one iothread without
destroying it.

Since at it, fix iothread_stop() to allow it to be called multiple
times.  Before this patch we may call iothread_stop() more than once on
single iothread, while that may not be correct since qemu_thread_join()
is not allowed to run twice.  From manual of pthread_join():

Joining with a thread that has previously been joined results in
  undefined behavior.

Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-id: 20170928025958.1420-4-peterx@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/sysemu/iothread.h |  1 +
 iothread.c                | 24 ++++++++++++++++--------
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
index XXXXXXX..XXXXXXX 100644
--- a/include/sysemu/iothread.h
+++ b/include/sysemu/iothread.h
@@ -XXX,XX +XXX,XX @@ GMainContext *iothread_get_g_main_context(IOThread *iothread);
  * "query-iothreads".
  */
 IOThread *iothread_create(const char *id, Error **errp);
+void iothread_stop(IOThread *iothread);
 void iothread_destroy(IOThread *iothread);
 
 #endif /* IOTHREAD_H */
diff --git a/iothread.c b/iothread.c
index XXXXXXX..XXXXXXX 100644
--- a/iothread.c
+++ b/iothread.c
@@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque)
     return NULL;
 }
 
-static int iothread_stop(Object *object, void *opaque)
+void iothread_stop(IOThread *iothread)
 {
-    IOThread *iothread;
-
-    iothread = (IOThread *)object_dynamic_cast(object, TYPE_IOTHREAD);
-    if (!iothread || !iothread->ctx || iothread->stopping) {
-        return 0;
+    if (!iothread->ctx || iothread->stopping) {
+        return;
     }
     iothread->stopping = true;
     aio_notify(iothread->ctx);
@@ -XXX,XX +XXX,XX @@ static int iothread_stop(Object *object, void *opaque)
         g_main_loop_quit(iothread->main_loop);
     }
     qemu_thread_join(&iothread->thread);
+}
+
+static int iothread_stop_iter(Object *object, void *opaque)
+{
+    IOThread *iothread;
+
+    iothread = (IOThread *)object_dynamic_cast(object, TYPE_IOTHREAD);
+    if (!iothread) {
+        return 0;
+    }
+    iothread_stop(iothread);
     return 0;
 }
 
@@ -XXX,XX +XXX,XX @@ static void iothread_instance_finalize(Object *obj)
 {
     IOThread *iothread = IOTHREAD(obj);
 
-    iothread_stop(obj, NULL);
+    iothread_stop(iothread);
     qemu_cond_destroy(&iothread->init_done_cond);
     qemu_mutex_destroy(&iothread->init_done_lock);
     if (!iothread->ctx) {
@@ -XXX,XX +XXX,XX @@ void iothread_stop_all(void)
         aio_context_release(ctx);
     }
 
-    object_child_foreach(container, iothread_stop, NULL);
+    object_child_foreach(container, iothread_stop_iter, NULL);
 }
 
 static gpointer iothread_g_main_context_init(gpointer opaque)
-- 
2.13.6

From: Peter Xu <peterx@redhat.com>

When gcontext is used with iothread, the context will be destroyed
during iothread_stop().  That's not good since sometimes we would like
to keep the resources until iothread is destroyed, but we may want to
stop the thread before that point.

Delay the destruction of gcontext to iothread finalize.  Then we can do:

iothread_stop(thread);
  some_cleanup_on_resources();
  iothread_destroy(thread);

We may need this patch if we want to run chardev IOs in iothreads and
hopefully clean them up correctly.  For more specific information,
please see 2b316774f6 ("qemu-char: do not operate on sources from
finalize callbacks").

Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-id: 20170928025958.1420-5-peterx@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 iothread.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/iothread.c b/iothread.c
index XXXXXXX..XXXXXXX 100644
--- a/iothread.c
+++ b/iothread.c
@@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque)
             g_main_loop_unref(loop);
 
             g_main_context_pop_thread_default(iothread->worker_context);
-            g_main_context_unref(iothread->worker_context);
-            iothread->worker_context = NULL;
         }
     }
 
@@ -XXX,XX +XXX,XX @@ static void iothread_instance_finalize(Object *obj)
     IOThread *iothread = IOTHREAD(obj);
 
     iothread_stop(iothread);
+    if (iothread->worker_context) {
+        g_main_context_unref(iothread->worker_context);
+        iothread->worker_context = NULL;
+    }
     qemu_cond_destroy(&iothread->init_done_cond);
     qemu_mutex_destroy(&iothread->init_done_lock);
     if (!iothread->ctx) {
-- 
2.13.6

After iothread is enabled internally inside QEMU with GMainContext, we
may encounter this warning when destroying the iothread:

(qemu-system-x86_64:19925): GLib-CRITICAL **: g_source_remove_poll:
 assertion '!SOURCE_DESTROYED (source)' failed

The problem is that g_source_remove_poll() does not allow to remove one
source from array if the source is detached from its owner
context. (peterx: which IMHO does not make much sense)

Fix it on QEMU side by avoid calling g_source_remove_poll() if we know
the object is during destruction, and we won't leak anything after all
since the array will be gone soon cleanly even with that fd.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-id: 20170928025958.1420-6-peterx@redhat.com
[peterx: write the commit message]
Signed-off-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/aio-posix.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/util/aio-posix.c b/util/aio-posix.c
index XXXXXXX..XXXXXXX 100644
--- a/util/aio-posix.c
+++ b/util/aio-posix.c
@@ -XXX,XX +XXX,XX @@ void aio_set_fd_handler(AioContext *ctx,
             return;
         }
 
-        g_source_remove_poll(&ctx->source, &node->pfd);
+        /* If the GSource is in the process of being destroyed then
+         * g_source_remove_poll() causes an assertion failure.  Skip
+         * removal in that case, because glib cleans up its state during
+         * destruction anyway.
+         */
+        if (!g_source_is_destroyed(&ctx->source)) {
+            g_source_remove_poll(&ctx->source, &node->pfd);
+        }
 
         /* If the lock is held, just mark the node as deleted */
         if (qemu_lockcnt_count(&ctx->list_lock)) {
-- 
2.13.6

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