[PULL 00/17] Block patches

[PULL 00/17] Block patches

[Max Reitz]

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

 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 ++++--
 16 files changed, 220 insertions(+), 73 deletions(-)
 create mode 100644 tests/qemu-iotests/283
 create mode 100644 tests/qemu-iotests/283.out

-- 
2.24.1

[PULL 01/17] qcow2: Assert that host cluster offsets fit in L2 table entries

[Max Reitz]

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 8982b7b762..e9431f6785 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -777,6 +777,10 @@ 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);
 
@@ -972,6 +976,7 @@ 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
@@ -982,8 +987,10 @@ 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);
      }
 
 
@@ -1913,6 +1920,9 @@ 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

[PULL 02/17] block: Use a GString in bdrv_perm_names()

[Max Reitz]

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 6c2b2bd2e2..fe5050c53f 100644
--- a/block.c
+++ b/block.c
@@ -1998,18 +1998,19 @@ 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

[PULL 03/17] block: fix memleaks in bdrv_refresh_filename

[Max Reitz]

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 fe5050c53f..9c810534d6 100644
--- a/block.c
+++ b/block.c
@@ -6442,6 +6442,7 @@ 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

[PULL 04/17] qcow2: Use a GString in report_unsupported_feature()

[Max Reitz]

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 cef9d72b3a..e29fc07068 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -453,16 +453,15 @@ 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);
             }
         }
@@ -470,14 +469,14 @@ 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

[PULL 05/17] iotests: remove 'linux' from default supported platforms

[Max Reitz]

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 89aa2df2f3..ead04a1ab5 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -931,9 +931,14 @@ 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):
@@ -1028,7 +1033,8 @@ 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=[]):
@@ -1046,7 +1052,7 @@ 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

[PULL 06/17] iotests: Test 041 only works on certain systems

[Max Reitz]

From: Thomas Huth <thuth@redhat.com>

041 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 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index c07437fda1..0181f7a9b6 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -1134,4 +1134,5 @@ 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'])
-- 
2.24.1

[PULL 07/17] iotests: Test 183 does not work on macOS and OpenBSD

[Max Reitz]

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 64621617f5..acdbefa310 100755
--- a/tests/qemu-iotests/183
+++ b/tests/qemu-iotests/183
@@ -42,6 +42,7 @@ 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

[PULL 08/17] iotests: Check for the availability of the required devices in 267 and 127

[Max Reitz]

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 b64926ab31..a4fc866038 100755
--- a/tests/qemu-iotests/127
+++ b/tests/qemu-iotests/127
@@ -43,6 +43,8 @@ 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 c296877168..3146273eef 100755
--- a/tests/qemu-iotests/267
+++ b/tests/qemu-iotests/267
@@ -46,6 +46,8 @@ _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 9ccde32634..8a6366c09d 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -713,5 +713,19 @@ _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

[PULL 09/17] iotests: Skip Python-based tests if QEMU does not support virtio-blk

[Max Reitz]

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 39ed5bc1be..fff5fa956a 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -655,7 +655,15 @@ 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')
@@ -843,7 +851,7 @@ 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

[PULL 10/17] iotests: Enable more tests in the 'auto' group to improve test coverage

[Max Reitz]

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 e041cc1ee3..9d30a4b6c4 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -51,7 +51,7 @@
 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
@@ -61,8 +61,8 @@
 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
@@ -148,7 +148,7 @@
 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
@@ -197,7 +197,7 @@
 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
@@ -218,7 +218,7 @@
 200 rw
 201 rw migration
 202 rw quick
-203 rw migration
+203 rw auto migration
 204 rw quick
 205 rw quick
 206 rw
@@ -270,7 +270,7 @@
 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

[PULL 11/17] qcow2: Don't round the L1 table allocation up to the sector size

[Max Reitz]

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 e9431f6785..0384fb2339 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -124,12 +124,11 @@ 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 f67ac6b2d8..c963bc8de1 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1262,7 +1262,7 @@ 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 5ab64da1ec..82c32d4c9b 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -1024,8 +1024,7 @@ 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 e29fc07068..83db013814 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1491,7 +1491,7 @@ 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

[PULL 12/17] qcow2: Tighten cluster_offset alignment assertions

[Max Reitz]

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 83db013814..6cb5aee4a5 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2167,10 +2167,7 @@ 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);
@@ -2506,7 +2503,7 @@ 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,
@@ -3896,7 +3893,7 @@ 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

[PULL 13/17] qcow2: Use bs->bl.request_alignment when updating an L1 entry

[Max Reitz]

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 0384fb2339..1947f13a2d 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -216,26 +216,31 @@ 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;
     }
@@ -243,7 +248,7 @@ 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

[PULL 14/17] qcow2: Don't require aligned offsets in qcow2_co_copy_range_from()

[Max Reitz]

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 6cb5aee4a5..ff257d1a6c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3828,10 +3828,6 @@ 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

[PULL 15/17] qcow2: Use BDRV_SECTOR_SIZE instead of the hardcoded value

[Max Reitz]

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 ff257d1a6c..ef96606f8d 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3272,7 +3272,8 @@ 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;
     }
@@ -3946,8 +3947,9 @@ 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

[PULL 16/17] block/backup-top: fix failure path

[Max Reitz]

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 9aed2eb4c0..fa78f3256d 100644
--- a/block/backup-top.c
+++ b/block/backup-top.c
@@ -190,6 +190,7 @@ 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;
@@ -212,8 +213,9 @@ 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
@@ -224,14 +226,14 @@ 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;
 
@@ -239,14 +241,15 @@ 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

[PULL 17/17] iotests: add test for backup-top failure on permission activation

[Max Reitz]

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 0000000000..293e557bd9
--- /dev/null
+++ b/tests/qemu-iotests/283
@@ -0,0 +1,92 @@
+#!/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 0000000000..daaf5828c1
--- /dev/null
+++ b/tests/qemu-iotests/283.out
@@ -0,0 +1,8 @@
+{"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 9d30a4b6c4..1904223020 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -289,3 +289,4 @@
 279 rw backing quick
 280 rw migration quick
 281 rw quick
+283 auto quick
-- 
2.24.1

Re: [PULL 00/17] Block patches

[Peter Maydell]

On Thu, 6 Feb 2020 at 12:51, Max Reitz <mreitz@redhat.com> wrote:
>
> 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)


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.0
for any user-visible changes.

-- PMM

Re: [PULL 00/17] Block patches

[Richard Henderson]

On 6/5/23 08:45, Hanna Czenczek wrote:
> The following changes since commit 848a6caa88b9f082c89c9b41afa975761262981d:
> 
>    Merge tag 'migration-20230602-pull-request' ofhttps://gitlab.com/juan.quintela/qemu  into staging (2023-06-02 17:33:29 -0700)
> 
> are available in the Git repository at:
> 
>    https://gitlab.com/hreitz/qemu.git  tags/pull-block-2023-06-05
> 
> for you to fetch changes up to 42a2890a76f4783cd1c212f27856edcf2b5e8a75:
> 
>    qcow2: add discard-no-unref option (2023-06-05 13:15:42 +0200)
> 
> ----------------------------------------------------------------
> Block patches
> 
> - Fix padding of unaligned vectored requests to match the host alignment
>    for vectors with 1023 or 1024 buffers
> - Refactor and fix bugs in parallels's image check functionality
> - Add an option to the qcow2 driver to retain (qcow2-level) allocations
>    on discard requests from the guest (while still forwarding the discard
>    to the lower level and marking the range as zero)

Applied, thanks.  Please update https://wiki.qemu.org/ChangeLog/8.1 as appropriate.


r~

[PULL 00/17] Block patches

[Hanna Czenczek]

The following changes since commit 848a6caa88b9f082c89c9b41afa975761262981d:

  Merge tag 'migration-20230602-pull-request' of https://gitlab.com/juan.quintela/qemu into staging (2023-06-02 17:33:29 -0700)

are available in the Git repository at:

  https://gitlab.com/hreitz/qemu.git tags/pull-block-2023-06-05

for you to fetch changes up to 42a2890a76f4783cd1c212f27856edcf2b5e8a75:

  qcow2: add discard-no-unref option (2023-06-05 13:15:42 +0200)

----------------------------------------------------------------
Block patches

- Fix padding of unaligned vectored requests to match the host alignment
  for vectors with 1023 or 1024 buffers
- Refactor and fix bugs in parallels's image check functionality
- Add an option to the qcow2 driver to retain (qcow2-level) allocations
  on discard requests from the guest (while still forwarding the discard
  to the lower level and marking the range as zero)

----------------------------------------------------------------
Alexander Ivanov (12):
  parallels: Out of image offset in BAT leads to image inflation
  parallels: Fix high_off calculation in parallels_co_check()
  parallels: Fix image_end_offset and data_end after out-of-image check
  parallels: create parallels_set_bat_entry_helper() to assign BAT value
  parallels: Use generic infrastructure for BAT writing in
    parallels_co_check()
  parallels: Move check of unclean image to a separate function
  parallels: Move check of cluster outside image to a separate function
  parallels: Fix statistics calculation
  parallels: Move check of leaks to a separate function
  parallels: Move statistic collection to a separate function
  parallels: Replace qemu_co_mutex_lock by WITH_QEMU_LOCK_GUARD
  parallels: Incorrect condition in out-of-image check

Hanna Czenczek (4):
  util/iov: Make qiov_slice() public
  block: Collapse padded I/O vecs exceeding IOV_MAX
  util/iov: Remove qemu_iovec_init_extended()
  iotests/iov-padding: New test

Jean-Louis Dupond (1):
  qcow2: add discard-no-unref option

 qapi/block-core.json                     |  12 ++
 block/qcow2.h                            |   3 +
 include/qemu/iov.h                       |   8 +-
 block/io.c                               | 166 ++++++++++++++++++--
 block/parallels.c                        | 190 ++++++++++++++++-------
 block/qcow2-cluster.c                    |  32 +++-
 block/qcow2.c                            |  18 +++
 util/iov.c                               |  89 ++---------
 qemu-options.hx                          |  12 ++
 tests/qemu-iotests/tests/iov-padding     |  85 ++++++++++
 tests/qemu-iotests/tests/iov-padding.out |  59 +++++++
 11 files changed, 523 insertions(+), 151 deletions(-)
 create mode 100755 tests/qemu-iotests/tests/iov-padding
 create mode 100644 tests/qemu-iotests/tests/iov-padding.out

-- 
2.40.1

Re: [PULL 00/17] Block patches

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 3514 bytes --]

On Sat, Apr 29, 2023 at 11:05:06PM +0100, Richard Henderson wrote:
> On 4/28/23 13:39, Stefan Hajnoczi wrote:
> > The following changes since commit 05d50ba2d4668d43a835c5a502efdec9b92646e6:
> > 
> >    Merge tag 'migration-20230427-pull-request' of https://gitlab.com/juan.quintela/qemu into staging (2023-04-28 08:35:06 +0100)
> > 
> > are available in the Git repository at:
> > 
> >    https://gitlab.com/stefanha/qemu.git tags/block-pull-request
> > 
> > for you to fetch changes up to d3c760be786571d83d5cea01953e543df4d76f51:
> > 
> >    docs/zoned-storage:add zoned emulation use case (2023-04-28 08:34:07 -0400)
> > 
> > ----------------------------------------------------------------
> > Pull request
> > 
> > This pull request contains Sam Li's virtio-blk zoned storage work. These
> > patches were dropped from my previous block pull request due to CI failures.
> 
> 
> More CI build failures, e.g.

Hi Sam,
There are some more CI failures.

> 
> https://gitlab.com/qemu-project/qemu/-/jobs/4202086013#L1720

This Ubuntu 20.04 on s390x CI job failed because <linux/blkzoned.h> is
missing Linux commit e876df1fe0ad ("block: add zone open, close and
finish ioctl support"):

  ../block/file-posix.c: In function ‘raw_co_zone_mgmt’:
  ../block/file-posix.c:3472:14: error: ‘BLKOPENZONE’ undeclared (first use in this function)
   3472 |         zo = BLKOPENZONE;
        |              ^~~~~~~~~~~
  ../block/file-posix.c:3472:14: note: each undeclared identifier is reported only once for each function it appears in
  ../block/file-posix.c:3476:14: error: ‘BLKCLOSEZONE’ undeclared (first use in this function); did you mean ‘BLKRESETZONE’?
   3476 |         zo = BLKCLOSEZONE;
        |              ^~~~~~~~~~~~
        |              BLKRESETZONE
  ../block/file-posix.c:3480:14: error: ‘BLKFINISHZONE’ undeclared (first use in this function)
   3480 |         zo = BLKFINISHZONE;
        |              ^~~~~~~~~~~~~

Older kernels didn't have these ioctls. I don't think it makes sense to
enable file-posix zoned functionality without these ioctls.

I suggest changing the CONFIG_BLKZONED check in meson.build from:

  config_host_data.set('CONFIG_BLKZONED', cc.has_header('linux/blkzoned.h'))

 to:

  config_host_data.set('CONFIG_BLKZONED', cc.has_header_symbol('linux/blkzoned.h', 'BLKOPENZONE'))

> https://gitlab.com/qemu-project/qemu/-/jobs/4202085995#L4088

The <linux/asm/kvm.h> header file started using __DECLARE_FLEX_ARRAY()
and QEMU doesn't have that macro:

  linux-headers/asm/kvm.h:509:3: error: expected specifier-qualifier-list before '__DECLARE_FLEX_ARRAY'
    509 |   __DECLARE_FLEX_ARRAY(struct kvm_vmx_nested_state_data, vmx);
        |   ^~~~~~~~~~~~~~~~~~~~

You could update the sed command in scripts/update-linux-headers.sh to
convert __DECLARE_FLEX_ARRAY(type, field) into type field[] or import
the Linux macro definition of __DECLARE_FLEX_ARRAY().

Another failure is
https://gitlab.com/qemu-project/qemu/-/jobs/4202085991 where
qemu-iotests is failing because the output has changed due to the
addition of zoned fields to block stats.

Another failure is
https://gitlab.com/qemu-project/qemu/-/jobs/4202086041 where
qemu-system-ppc seems to segfault.

You can find the CI results here: https://gitlab.com/qemu-project/qemu/-/pipelines/852908752

You can run the GitLab CI yourself like this:

  $ git push -o ci.variable=QEMU_CI=2 your_gitlab_repo HEAD

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PULL 00/17] Block patches

[Richard Henderson]

On 4/28/23 13:39, Stefan Hajnoczi wrote:
> The following changes since commit 05d50ba2d4668d43a835c5a502efdec9b92646e6:
> 
>    Merge tag 'migration-20230427-pull-request' of https://gitlab.com/juan.quintela/qemu into staging (2023-04-28 08:35:06 +0100)
> 
> are available in the Git repository at:
> 
>    https://gitlab.com/stefanha/qemu.git tags/block-pull-request
> 
> for you to fetch changes up to d3c760be786571d83d5cea01953e543df4d76f51:
> 
>    docs/zoned-storage:add zoned emulation use case (2023-04-28 08:34:07 -0400)
> 
> ----------------------------------------------------------------
> Pull request
> 
> This pull request contains Sam Li's virtio-blk zoned storage work. These
> patches were dropped from my previous block pull request due to CI failures.


More CI build failures, e.g.

https://gitlab.com/qemu-project/qemu/-/jobs/4202086013#L1720
https://gitlab.com/qemu-project/qemu/-/jobs/4202085995#L4088


r~

[PULL 00/17] Block patches

[Stefan Hajnoczi]

The following changes since commit 05d50ba2d4668d43a835c5a502efdec9b92646e6:

  Merge tag 'migration-20230427-pull-request' of https://gitlab.com/juan.quintela/qemu into staging (2023-04-28 08:35:06 +0100)

are available in the Git repository at:

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

for you to fetch changes up to d3c760be786571d83d5cea01953e543df4d76f51:

  docs/zoned-storage:add zoned emulation use case (2023-04-28 08:34:07 -0400)

----------------------------------------------------------------
Pull request

This pull request contains Sam Li's virtio-blk zoned storage work. These
patches were dropped from my previous block pull request due to CI failures.

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

Sam Li (17):
  block/block-common: add zoned device structs
  block/file-posix: introduce helper functions for sysfs attributes
  block/block-backend: add block layer APIs resembling Linux
    ZonedBlockDevice ioctls
  block/raw-format: add zone operations to pass through requests
  block: add zoned BlockDriver check to block layer
  iotests: test new zone operations
  block: add some trace events for new block layer APIs
  docs/zoned-storage: add zoned device documentation
  file-posix: add tracking of the zone write pointers
  block: introduce zone append write for zoned devices
  qemu-iotests: test zone append operation
  block: add some trace events for zone append
  include: update virtio_blk headers to v6.3-rc1
  virtio-blk: add zoned storage emulation for zoned devices
  block: add accounting for zone append operation
  virtio-blk: add some trace events for zoned emulation
  docs/zoned-storage:add zoned emulation use case

 docs/devel/index-api.rst                     |   1 +
 docs/devel/zoned-storage.rst                 |  62 ++
 qapi/block-core.json                         |  68 +-
 qapi/block.json                              |   4 +
 meson.build                                  |   4 +
 include/block/accounting.h                   |   1 +
 include/block/block-common.h                 |  57 ++
 include/block/block-io.h                     |  13 +
 include/block/block_int-common.h             |  37 +
 include/block/raw-aio.h                      |   8 +-
 include/standard-headers/drm/drm_fourcc.h    |  12 +
 include/standard-headers/linux/ethtool.h     |  48 +-
 include/standard-headers/linux/fuse.h        |  45 +-
 include/standard-headers/linux/pci_regs.h    |   1 +
 include/standard-headers/linux/vhost_types.h |   2 +
 include/standard-headers/linux/virtio_blk.h  | 105 +++
 include/sysemu/block-backend-io.h            |  27 +
 linux-headers/asm-arm64/kvm.h                |   1 +
 linux-headers/asm-x86/kvm.h                  |  34 +-
 linux-headers/linux/kvm.h                    |   9 +
 linux-headers/linux/vfio.h                   |  15 +-
 linux-headers/linux/vhost.h                  |   8 +
 block.c                                      |  19 +
 block/block-backend.c                        | 198 ++++++
 block/file-posix.c                           | 696 +++++++++++++++++--
 block/io.c                                   |  68 ++
 block/io_uring.c                             |   4 +
 block/linux-aio.c                            |   3 +
 block/qapi-sysemu.c                          |  11 +
 block/qapi.c                                 |  18 +
 block/raw-format.c                           |  26 +
 hw/block/virtio-blk-common.c                 |   2 +
 hw/block/virtio-blk.c                        | 405 +++++++++++
 hw/virtio/virtio-qmp.c                       |   2 +
 qemu-io-cmds.c                               | 224 ++++++
 block/trace-events                           |   4 +
 docs/system/qemu-block-drivers.rst.inc       |   6 +
 hw/block/trace-events                        |   7 +
 tests/qemu-iotests/tests/zoned               | 105 +++
 tests/qemu-iotests/tests/zoned.out           |  69 ++
 40 files changed, 2361 insertions(+), 68 deletions(-)
 create mode 100644 docs/devel/zoned-storage.rst
 create mode 100755 tests/qemu-iotests/tests/zoned
 create mode 100644 tests/qemu-iotests/tests/zoned.out

-- 
2.40.0

Re: [PULL 00/17] Block patches

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 2220 bytes --]

On Thu, Oct 01, 2020 at 04:12:12PM +0100, Peter Maydell wrote:
> On Thu, 1 Oct 2020 at 16:03, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > On Thu, Oct 01, 2020 at 12:23:00PM +0100, Peter Maydell wrote:
> > > This produces this error message on ppc64be Linux:
> > >
> > > make: Entering directory `/home/pm215/qemu/build/all'
> > > make[1]: Entering directory `/home/pm215/qemu/slirp'
> > > make[1]: Nothing to be done for `all'.
> > > make[1]: Leaving directory `/home/pm215/qemu/slirp'
> > > Generating qemu-version.h with a meson_exe.py custom command
> > > Generating qemu-options.def with a meson_exe.py custom command
> > > Generating block-gen.c with a custom command
> > > YAML:1:83: error: unknown enumerated scalar
> > > {"IndentWidth": 4, "BraceWrapping": {"AfterFunction": true},
> > > "BreakBeforeBraces": "Custom", "SortIncludes": false,
> > > "MaxEmptyLinesToKeep": 2}
> > >
> > >            ^~~~~~~~
> > > Error parsing -style: Invalid argument, using LLVM style
> > > YAML:1:83: error: unknown enumerated scalar
> > > {"IndentWidth": 4, "BraceWrapping": {"AfterFunction": true},
> > > "BreakBeforeBraces": "Custom", "SortIncludes": false,
> > > "MaxEmptyLinesToKeep": 2}
> > >
> > >            ^~~~~~~~
> > > Error parsing -style: Invalid argument, using LLVM style
> > > Compiling C object libqemuutil.a.p/util_qemu-error.c.o
> > > Compiling C object libqemuutil.a.p/util_qemu-sockets.c.o
> > > Compiling C object libqemuutil.a.p/util_aio-posix.c.o
> > > Compiling C object libqemuutil.a.p/util_osdep.c.o
> > >
> > > The error does not cause the build to fail, which seems
> > > like it's also a bug...
> > >
> > > (My guess is this is due to some script implicitly wanting
> > > a newer version of something or other than the PPC box
> > > happens to have installed, rather than being an endianness
> > > issue.)
> >
> > Please rerun with make -j1 V=1 so the full command is printed. I'm not
> > sure what is emitting these errors.
> 
> Build tree already overwritten to handle a different pullreq,
> I'm afraid. I can come back and retry later...

No problem. Thanks for pointing out the issue, Vladimir and Peter.

I will send a v2.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PULL 00/17] Block patches

[Peter Maydell]

On Thu, 1 Oct 2020 at 16:03, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> Please rerun with make -j1 V=1 so the full command is printed. I'm not
> sure what is emitting these errors.

Vladimir's guess was correct:

/usr/bin/python3
/home/pm215/qemu/block/../scripts/block-coroutine-wrapper.py
block/block-gen.c ../../block/../include/block/block.h
../../block/coroutines.h && if test -e block/block-gen.c; then printf
'%s\n' block/block-gen.c > block/block-gen.c.stamp; fi
YAML:1:83: error: unknown enumerated scalar
{"IndentWidth": 4, "BraceWrapping": {"AfterFunction": true},
"BreakBeforeBraces": "Custom", "SortIncludes": false,
"MaxEmptyLinesToKeep": 2}

           ^~~~~~~~
Error parsing -style: Invalid argument, using LLVM style
YAML:1:83: error: unknown enumerated scalar
{"IndentWidth": 4, "BraceWrapping": {"AfterFunction": true},
"BreakBeforeBraces": "Custom", "SortIncludes": false,
"MaxEmptyLinesToKeep": 2}

           ^~~~~~~~
Error parsing -style: Invalid argument, using LLVM style

thanks
-- PMM

Re: [PULL 00/17] Block patches

[Vladimir Sementsov-Ogievskiy]

01.10.2020 18:02, Stefan Hajnoczi wrote:
> On Thu, Oct 01, 2020 at 12:23:00PM +0100, Peter Maydell wrote:
>> On Wed, 30 Sep 2020 at 11:13, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>>>
>>> The following changes since commit b150cb8f67bf491a49a1cb1c7da151eeacbdbcc9:
>>>
>>>    Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2020-09-29 13:18:54 +0100)
>>>
>>> are available in the Git repository at:
>>>
>>>    https://gitlab.com/stefanha/qemu.git tags/block-pull-request
>>>
>>> for you to fetch changes up to bc47831ff28d6f5830c9c8d74220131dc54c5253:
>>>
>>>    util/vfio-helpers: Rework the IOVA allocator to avoid IOVA reserved regions (2020-09-30 10:23:05 +0100)
>>>
>>> ----------------------------------------------------------------
>>> Pull request
>>>
>>> Note I have switched from GitHub to GitLab.
>>>
>>> ----------------------------------------------------------------
>>
>> This produces this error message on ppc64be Linux:
>>
>> make: Entering directory `/home/pm215/qemu/build/all'
>> make[1]: Entering directory `/home/pm215/qemu/slirp'
>> make[1]: Nothing to be done for `all'.
>> make[1]: Leaving directory `/home/pm215/qemu/slirp'
>> Generating qemu-version.h with a meson_exe.py custom command
>> Generating qemu-options.def with a meson_exe.py custom command
>> Generating block-gen.c with a custom command
>> YAML:1:83: error: unknown enumerated scalar
>> {"IndentWidth": 4, "BraceWrapping": {"AfterFunction": true},
>> "BreakBeforeBraces": "Custom", "SortIncludes": false,
>> "MaxEmptyLinesToKeep": 2}
>>
>>             ^~~~~~~~
>> Error parsing -style: Invalid argument, using LLVM style
>> YAML:1:83: error: unknown enumerated scalar
>> {"IndentWidth": 4, "BraceWrapping": {"AfterFunction": true},
>> "BreakBeforeBraces": "Custom", "SortIncludes": false,
>> "MaxEmptyLinesToKeep": 2}
>>
>>             ^~~~~~~~
>> Error parsing -style: Invalid argument, using LLVM style
>> Compiling C object libqemuutil.a.p/util_qemu-error.c.o
>> Compiling C object libqemuutil.a.p/util_qemu-sockets.c.o
>> Compiling C object libqemuutil.a.p/util_aio-posix.c.o
>> Compiling C object libqemuutil.a.p/util_osdep.c.o
>>
>> The error does not cause the build to fail, which seems
>> like it's also a bug...
>>
>> (My guess is this is due to some script implicitly wanting
>> a newer version of something or other than the PPC box
>> happens to have installed, rather than being an endianness
>> issue.)
> 
> Please rerun with make -j1 V=1 so the full command is printed. I'm not
> sure what is emitting these errors.
> 

For sure it's block-coroutine-wrapper.py. I've send a letter with some investigation and solution, did you get it?


-- 
Best regards,
Vladimir

Re: [PULL 00/17] Block patches

[Peter Maydell]

On Thu, 1 Oct 2020 at 16:03, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> On Thu, Oct 01, 2020 at 12:23:00PM +0100, Peter Maydell wrote:
> > This produces this error message on ppc64be Linux:
> >
> > make: Entering directory `/home/pm215/qemu/build/all'
> > make[1]: Entering directory `/home/pm215/qemu/slirp'
> > make[1]: Nothing to be done for `all'.
> > make[1]: Leaving directory `/home/pm215/qemu/slirp'
> > Generating qemu-version.h with a meson_exe.py custom command
> > Generating qemu-options.def with a meson_exe.py custom command
> > Generating block-gen.c with a custom command
> > YAML:1:83: error: unknown enumerated scalar
> > {"IndentWidth": 4, "BraceWrapping": {"AfterFunction": true},
> > "BreakBeforeBraces": "Custom", "SortIncludes": false,
> > "MaxEmptyLinesToKeep": 2}
> >
> >            ^~~~~~~~
> > Error parsing -style: Invalid argument, using LLVM style
> > YAML:1:83: error: unknown enumerated scalar
> > {"IndentWidth": 4, "BraceWrapping": {"AfterFunction": true},
> > "BreakBeforeBraces": "Custom", "SortIncludes": false,
> > "MaxEmptyLinesToKeep": 2}
> >
> >            ^~~~~~~~
> > Error parsing -style: Invalid argument, using LLVM style
> > Compiling C object libqemuutil.a.p/util_qemu-error.c.o
> > Compiling C object libqemuutil.a.p/util_qemu-sockets.c.o
> > Compiling C object libqemuutil.a.p/util_aio-posix.c.o
> > Compiling C object libqemuutil.a.p/util_osdep.c.o
> >
> > The error does not cause the build to fail, which seems
> > like it's also a bug...
> >
> > (My guess is this is due to some script implicitly wanting
> > a newer version of something or other than the PPC box
> > happens to have installed, rather than being an endianness
> > issue.)
>
> Please rerun with make -j1 V=1 so the full command is printed. I'm not
> sure what is emitting these errors.

Build tree already overwritten to handle a different pullreq,
I'm afraid. I can come back and retry later...

thanks
-- PMM

Re: [PULL 00/17] Block patches

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 2550 bytes --]

On Thu, Oct 01, 2020 at 12:23:00PM +0100, Peter Maydell wrote:
> On Wed, 30 Sep 2020 at 11:13, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > The following changes since commit b150cb8f67bf491a49a1cb1c7da151eeacbdbcc9:
> >
> >   Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2020-09-29 13:18:54 +0100)
> >
> > are available in the Git repository at:
> >
> >   https://gitlab.com/stefanha/qemu.git tags/block-pull-request
> >
> > for you to fetch changes up to bc47831ff28d6f5830c9c8d74220131dc54c5253:
> >
> >   util/vfio-helpers: Rework the IOVA allocator to avoid IOVA reserved regions (2020-09-30 10:23:05 +0100)
> >
> > ----------------------------------------------------------------
> > Pull request
> >
> > Note I have switched from GitHub to GitLab.
> >
> > ----------------------------------------------------------------
> 
> This produces this error message on ppc64be Linux:
> 
> make: Entering directory `/home/pm215/qemu/build/all'
> make[1]: Entering directory `/home/pm215/qemu/slirp'
> make[1]: Nothing to be done for `all'.
> make[1]: Leaving directory `/home/pm215/qemu/slirp'
> Generating qemu-version.h with a meson_exe.py custom command
> Generating qemu-options.def with a meson_exe.py custom command
> Generating block-gen.c with a custom command
> YAML:1:83: error: unknown enumerated scalar
> {"IndentWidth": 4, "BraceWrapping": {"AfterFunction": true},
> "BreakBeforeBraces": "Custom", "SortIncludes": false,
> "MaxEmptyLinesToKeep": 2}
> 
>            ^~~~~~~~
> Error parsing -style: Invalid argument, using LLVM style
> YAML:1:83: error: unknown enumerated scalar
> {"IndentWidth": 4, "BraceWrapping": {"AfterFunction": true},
> "BreakBeforeBraces": "Custom", "SortIncludes": false,
> "MaxEmptyLinesToKeep": 2}
> 
>            ^~~~~~~~
> Error parsing -style: Invalid argument, using LLVM style
> Compiling C object libqemuutil.a.p/util_qemu-error.c.o
> Compiling C object libqemuutil.a.p/util_qemu-sockets.c.o
> Compiling C object libqemuutil.a.p/util_aio-posix.c.o
> Compiling C object libqemuutil.a.p/util_osdep.c.o
> 
> The error does not cause the build to fail, which seems
> like it's also a bug...
> 
> (My guess is this is due to some script implicitly wanting
> a newer version of something or other than the PPC box
> happens to have installed, rather than being an endianness
> issue.)

Please rerun with make -j1 V=1 so the full command is printed. I'm not
sure what is emitting these errors.

Thanks,
Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PULL 00/17] Block patches

[Vladimir Sementsov-Ogievskiy]

01.10.2020 14:23, Peter Maydell wrote:
> On Wed, 30 Sep 2020 at 11:13, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>>
>> The following changes since commit b150cb8f67bf491a49a1cb1c7da151eeacbdbcc9:
>>
>>    Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2020-09-29 13:18:54 +0100)
>>
>> are available in the Git repository at:
>>
>>    https://gitlab.com/stefanha/qemu.git tags/block-pull-request
>>
>> for you to fetch changes up to bc47831ff28d6f5830c9c8d74220131dc54c5253:
>>
>>    util/vfio-helpers: Rework the IOVA allocator to avoid IOVA reserved regions (2020-09-30 10:23:05 +0100)
>>
>> ----------------------------------------------------------------
>> Pull request
>>
>> Note I have switched from GitHub to GitLab.
>>
>> ----------------------------------------------------------------
> 
> This produces this error message on ppc64be Linux:
> 
> make: Entering directory `/home/pm215/qemu/build/all'
> make[1]: Entering directory `/home/pm215/qemu/slirp'
> make[1]: Nothing to be done for `all'.
> make[1]: Leaving directory `/home/pm215/qemu/slirp'
> Generating qemu-version.h with a meson_exe.py custom command
> Generating qemu-options.def with a meson_exe.py custom command
> Generating block-gen.c with a custom command
> YAML:1:83: error: unknown enumerated scalar
> {"IndentWidth": 4, "BraceWrapping": {"AfterFunction": true},
> "BreakBeforeBraces": "Custom", "SortIncludes": false,
> "MaxEmptyLinesToKeep": 2}
> 
>             ^~~~~~~~
> Error parsing -style: Invalid argument, using LLVM style
> YAML:1:83: error: unknown enumerated scalar
> {"IndentWidth": 4, "BraceWrapping": {"AfterFunction": true},
> "BreakBeforeBraces": "Custom", "SortIncludes": false,
> "MaxEmptyLinesToKeep": 2}
> 
>             ^~~~~~~~
> Error parsing -style: Invalid argument, using LLVM style
> Compiling C object libqemuutil.a.p/util_qemu-error.c.o
> Compiling C object libqemuutil.a.p/util_qemu-sockets.c.o
> Compiling C object libqemuutil.a.p/util_aio-posix.c.o
> Compiling C object libqemuutil.a.p/util_osdep.c.o
> 
> The error does not cause the build to fail, which seems
> like it's also a bug...

YAML:1:83 is most probably "Custom" value for "BeakBeforeBraces" key,
which is needed to enable custom brace wrapping, set with separate
BraceWrapping key.

Not simple to find the version of clang which is first to support "Custom'
here, but at least clang 3.6.1 doesn't support it.

Hm, I can "reproduce" like this:

[root@kvm work]# clang-format -style='{"BreakBeforeBraces": "Mega"}'
int c() {}
YAML:1:23: error: unknown enumerated scalar
{"BreakBeforeBraces": "Mega"}
                       ^~~~~~
Error parsing -style: Invalid argument
[root@kvm work]# echo $?
1

.

Build doesn't fail because clang-formatting of generated code is optional,
and error is ignored. But stderr output of clang-format is still printed
and this is wrong.

To fix we need:

diff --git a/scripts/block-coroutine-wrapper.py b/scripts/block-coroutine-wrapper.py
index 505e070660..1f66eb903b 100644
--- a/scripts/block-coroutine-wrapper.py
+++ b/scripts/block-coroutine-wrapper.py
@@ -43,7 +43,7 @@ def prettify(code: str) -> str:
          })
          p = subprocess.run(['clang-format', f'-style={style}'], check=True,
                             encoding='utf-8', input=code,
-                           stdout=subprocess.PIPE)
+                           stdout=subprocess.PIPE, stderr=subprocess.PIPE)
          return p.stdout
      except FileNotFoundError:
          return code



I can send a separate patch if it is convenient.


> 
> (My guess is this is due to some script implicitly wanting
> a newer version of something or other than the PPC box
> happens to have installed, rather than being an endianness
> issue.)
> 
> thanks
> -- PMM
> 


-- 
Best regards,
Vladimir

Re: [PULL 00/17] Block patches

[Peter Maydell]

On Wed, 30 Sep 2020 at 11:13, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> The following changes since commit b150cb8f67bf491a49a1cb1c7da151eeacbdbcc9:
>
>   Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2020-09-29 13:18:54 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/stefanha/qemu.git tags/block-pull-request
>
> for you to fetch changes up to bc47831ff28d6f5830c9c8d74220131dc54c5253:
>
>   util/vfio-helpers: Rework the IOVA allocator to avoid IOVA reserved regions (2020-09-30 10:23:05 +0100)
>
> ----------------------------------------------------------------
> Pull request
>
> Note I have switched from GitHub to GitLab.
>
> ----------------------------------------------------------------

This produces this error message on ppc64be Linux:

make: Entering directory `/home/pm215/qemu/build/all'
make[1]: Entering directory `/home/pm215/qemu/slirp'
make[1]: Nothing to be done for `all'.
make[1]: Leaving directory `/home/pm215/qemu/slirp'
Generating qemu-version.h with a meson_exe.py custom command
Generating qemu-options.def with a meson_exe.py custom command
Generating block-gen.c with a custom command
YAML:1:83: error: unknown enumerated scalar
{"IndentWidth": 4, "BraceWrapping": {"AfterFunction": true},
"BreakBeforeBraces": "Custom", "SortIncludes": false,
"MaxEmptyLinesToKeep": 2}

           ^~~~~~~~
Error parsing -style: Invalid argument, using LLVM style
YAML:1:83: error: unknown enumerated scalar
{"IndentWidth": 4, "BraceWrapping": {"AfterFunction": true},
"BreakBeforeBraces": "Custom", "SortIncludes": false,
"MaxEmptyLinesToKeep": 2}

           ^~~~~~~~
Error parsing -style: Invalid argument, using LLVM style
Compiling C object libqemuutil.a.p/util_qemu-error.c.o
Compiling C object libqemuutil.a.p/util_qemu-sockets.c.o
Compiling C object libqemuutil.a.p/util_aio-posix.c.o
Compiling C object libqemuutil.a.p/util_osdep.c.o

The error does not cause the build to fail, which seems
like it's also a bug...

(My guess is this is due to some script implicitly wanting
a newer version of something or other than the PPC box
happens to have installed, rather than being an endianness
issue.)

thanks
-- PMM

[PULL 00/17] Block patches

[Stefan Hajnoczi]

The following changes since commit b150cb8f67bf491a49a1cb1c7da151eeacbdbcc9:

  Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2020-09-29 13:18:54 +0100)

are available in the Git repository at:

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

for you to fetch changes up to bc47831ff28d6f5830c9c8d74220131dc54c5253:

  util/vfio-helpers: Rework the IOVA allocator to avoid IOVA reserved regions (2020-09-30 10:23:05 +0100)

----------------------------------------------------------------
Pull request

Note I have switched from GitHub to GitLab.

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

Eric Auger (2):
  util/vfio-helpers: Collect IOVA reserved regions
  util/vfio-helpers: Rework the IOVA allocator to avoid IOVA reserved
    regions

Philippe Mathieu-Daudé (6):
  util/vfio-helpers: Pass page protections to qemu_vfio_pci_map_bar()
  block/nvme: Map doorbells pages write-only
  block/nvme: Reduce I/O registers scope
  block/nvme: Drop NVMeRegs structure, directly use NvmeBar
  block/nvme: Use register definitions from 'block/nvme.h'
  block/nvme: Replace magic value by SCALE_MS definition

Stefano Garzarella (1):
  docs: add 'io_uring' option to 'aio' param in qemu-options.hx

Vladimir Sementsov-Ogievskiy (8):
  block: return error-code from bdrv_invalidate_cache
  block/io: refactor coroutine wrappers
  block: declare some coroutine functions in block/coroutines.h
  scripts: add block-coroutine-wrapper.py
  block: generate coroutine-wrapper code
  block: drop bdrv_prwv
  block/io: refactor save/load vmstate
  include/block/block.h: drop non-ascii quotation mark

 block/block-gen.h                      |  49 ++++
 block/coroutines.h                     |  65 +++++
 include/block/block.h                  |  36 ++-
 include/qemu/vfio-helpers.h            |   2 +-
 block.c                                |  97 +------
 block/io.c                             | 339 ++++---------------------
 block/nvme.c                           |  73 +++---
 tests/test-bdrv-drain.c                |   2 +-
 util/vfio-helpers.c                    | 133 +++++++++-
 block/meson.build                      |   8 +
 docs/devel/block-coroutine-wrapper.rst |  54 ++++
 docs/devel/index.rst                   |   1 +
 qemu-options.hx                        |  10 +-
 scripts/block-coroutine-wrapper.py     | 188 ++++++++++++++
 14 files changed, 629 insertions(+), 428 deletions(-)
 create mode 100644 block/block-gen.h
 create mode 100644 block/coroutines.h
 create mode 100644 docs/devel/block-coroutine-wrapper.rst
 create mode 100644 scripts/block-coroutine-wrapper.py

-- 
2.26.2