Series comparison

-[PULL 00/19] Block patches
+[PULL for-9.0 0/1] Block patches
-The following changes since commit ba29883206d92a29ad5a466e679ccfc2ee6132ef:
+The following changes since commit fea445e8fe9acea4f775a832815ee22bdf2b0222:
-  Merge remote-tracking branch 'remotes/borntraeger/tags/s390x-20200310' into staging (2020-03-10 16:50:28 +0000)
+  Merge tag 'pull-maintainer-final-for-real-this-time-200324-1' of https://gitlab.com/stsquad/qemu into staging (2024-03-21 10:31:56 +0000)
 are available in the Git repository at:
-  https://github.com/XanClic/qemu.git tags/pull-block-2020-03-11
+  https://gitlab.com/stefanha/qemu.git tags/block-pull-request
-for you to fetch changes up to 397f4e9d83e9c0000905f0a988ba1aeda162571c:
+for you to fetch changes up to 9352f80cd926fe2dde7c89b93ee33bb0356ff40e:
-  block/block-copy: hide structure definitions (2020-03-11 12:42:30 +0100)
+  coroutine: reserve 5,000 mappings (2024-03-21 13:14:30 -0400)
 ----------------------------------------------------------------
-Block patches for the 5.0 softfreeze:
+Pull request
-- qemu-img measure for LUKS
-- Improve block-copy's performance by reducing inter-request
+I was too quick in sending the coroutine pool sizing change for -rc0 and still
-  dependencies
+needed to address feedback from Daniel Berrangé.
 - Make curl's detection of accept-ranges more robust
 - Memleak fixes
 - iotest fix
 ----------------------------------------------------------------
-David Edmondson (2):
-  block/curl: HTTP header fields allow whitespace around values
-  block/curl: HTTP header field names are case insensitive
-Eric Blake (1):
+Stefan Hajnoczi (1):
-  iotests: Fix nonportable use of od --endian
+  coroutine: reserve 5,000 mappings
-Pan Nengyuan (2):
+ util/qemu-coroutine.c | 15 ++++++++++-----
-  block/qcow2: do free crypto_opts in qcow2_close()
+file changed, 10 insertions(+), 5 deletions(-)
   qemu-img: free memory before re-assign
 Stefan Hajnoczi (4):
   luks: extract qcrypto_block_calculate_payload_offset()
   luks: implement .bdrv_measure()
   qemu-img: allow qemu-img measure --object without a filename
   iotests: add 288 luks qemu-img measure test
 Vladimir Sementsov-Ogievskiy (10):
   block/qcow2-threads: fix qcow2_decompress
   job: refactor progress to separate object
   block/block-copy: fix progress calculation
   block/block-copy: specialcase first copy_range request
   block/block-copy: use block_status
   block/block-copy: factor out find_conflicting_inflight_req
   block/block-copy: refactor interfaces to use bytes instead of end
   block/block-copy: rename start to offset in interfaces
   block/block-copy: reduce intersecting request lock
   block/block-copy: hide structure definitions
  block/backup-top.c               |   6 +-
  block/backup.c                   |  38 ++-
  block/block-copy.c               | 405 ++++++++++++++++++++++++-------
  block/crypto.c                   |  62 +++++
  block/curl.c                     |  32 ++-
  block/qcow2-threads.c            |  12 +-
  block/qcow2.c                    |  75 ++----
  block/trace-events               |   1 +
  blockjob.c                       |  16 +-
  crypto/block.c                   |  36 +++
  include/block/block-copy.h       |  65 +----
  include/crypto/block.h           |  22 ++
  include/qemu/job.h               |  11 +-
  include/qemu/progress_meter.h    |  58 +++++
  job-qmp.c                        |   4 +-
  job.c                            |   6 +-
  qemu-img.c                       |  14 +-
  tests/qemu-iotests/178           |   2 +-
  tests/qemu-iotests/178.out.qcow2 |   8 +-
  tests/qemu-iotests/178.out.raw   |   8 +-
  tests/qemu-iotests/288           |  93 +++++++
  tests/qemu-iotests/288.out       |  30 +++
  tests/qemu-iotests/common.rc     |  22 +-
  tests/qemu-iotests/group         |   1 +
 files changed, 749 insertions(+), 278 deletions(-)
  create mode 100644 include/qemu/progress_meter.h
  create mode 100755 tests/qemu-iotests/288
  create mode 100644 tests/qemu-iotests/288.out
 --
-.24.1
+.44.0

-[PULL 01/19] luks: extract qcrypto_block_calculate_payload_offset()
+Deleted patch
-From: Stefan Hajnoczi <stefanha@redhat.com>
-The qcow2 .bdrv_measure() code calculates the crypto payload offset.
-This logic really belongs in crypto/block.c where it can be reused by
-other image formats.
-The "luks" block driver will need this same logic in order to implement
-.bdrv_measure(), so extract the qcrypto_block_calculate_payload_offset()
-function now.
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Message-Id: <20200221112522.1497712-2-stefanha@redhat.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block/qcow2.c          | 74 +++++++++++-------------------------------
- crypto/block.c         | 36 ++++++++++++++++++++
- include/crypto/block.h | 22 +++++++++++++
-files changed, 77 insertions(+), 55 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_flush_to_os(BlockDriverState *bs)
-     return ret;
- }
--static ssize_t qcow2_measure_crypto_hdr_init_func(QCryptoBlock *block,
--        size_t headerlen, void *opaque, Error **errp)
--{
--    size_t *headerlenp = opaque;
--
--    /* Stash away the payload size */
--    *headerlenp = headerlen;
--    return 0;
--}
--
--static ssize_t qcow2_measure_crypto_hdr_write_func(QCryptoBlock *block,
--        size_t offset, const uint8_t *buf, size_t buflen,
--        void *opaque, Error **errp)
--{
--    /* Discard the bytes, we're not actually writing to an image */
--    return buflen;
--}
--
--/* Determine the number of bytes for the LUKS payload */
--static bool qcow2_measure_luks_headerlen(QemuOpts *opts, size_t *len,
--                                         Error **errp)
--{
--    QDict *opts_qdict;
--    QDict *cryptoopts_qdict;
--    QCryptoBlockCreateOptions *cryptoopts;
--    QCryptoBlock *crypto;
--
--    /* Extract "encrypt." options into a qdict */
--    opts_qdict = qemu_opts_to_qdict(opts, NULL);
--    qdict_extract_subqdict(opts_qdict, &cryptoopts_qdict, "encrypt.");
--    qobject_unref(opts_qdict);
--
--    /* Build QCryptoBlockCreateOptions object from qdict */
--    qdict_put_str(cryptoopts_qdict, "format", "luks");
--    cryptoopts = block_crypto_create_opts_init(cryptoopts_qdict, errp);
--    qobject_unref(cryptoopts_qdict);
--    if (!cryptoopts) {
--        return false;
--    }
--
--    /* Fake LUKS creation in order to determine the payload size */
--    crypto = qcrypto_block_create(cryptoopts, "encrypt.",
--                                  qcow2_measure_crypto_hdr_init_func,
--                                  qcow2_measure_crypto_hdr_write_func,
--                                  len, errp);
--    qapi_free_QCryptoBlockCreateOptions(cryptoopts);
--    if (!crypto) {
--        return false;
--    }
--
--    qcrypto_block_free(crypto);
--    return true;
--}
--
- static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
-                                        Error **errp)
- {
-@@ -XXX,XX +XXX,XX @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
-     g_free(optstr);
-     if (has_luks) {
-+        g_autoptr(QCryptoBlockCreateOptions) create_opts = NULL;
-+        QDict *opts_qdict;
-+        QDict *cryptoopts;
-         size_t headerlen;
--        if (!qcow2_measure_luks_headerlen(opts, &headerlen, &local_err)) {
-+        opts_qdict = qemu_opts_to_qdict(opts, NULL);
-+        qdict_extract_subqdict(opts_qdict, &cryptoopts, "encrypt.");
-+        qobject_unref(opts_qdict);
-+
-+        qdict_put_str(cryptoopts, "format", "luks");
-+
-+        create_opts = block_crypto_create_opts_init(cryptoopts, errp);
-+        qobject_unref(cryptoopts);
-+        if (!create_opts) {
-+            goto err;
-+        }
-+
-+        if (!qcrypto_block_calculate_payload_offset(create_opts,
-+                                                    "encrypt.",
-+                                                    &headerlen,
-+                                                    &local_err)) {
-             goto err;
-         }
-diff --git a/crypto/block.c b/crypto/block.c
-index XXXXXXX..XXXXXXX 100644
---- a/crypto/block.c
-+++ b/crypto/block.c
-@@ -XXX,XX +XXX,XX @@ QCryptoBlock *qcrypto_block_create(QCryptoBlockCreateOptions *options,
- }
-+static ssize_t qcrypto_block_headerlen_hdr_init_func(QCryptoBlock *block,
-+        size_t headerlen, void *opaque, Error **errp)
-+{
-+    size_t *headerlenp = opaque;
-+
-+    /* Stash away the payload size */
-+    *headerlenp = headerlen;
-+    return 0;
-+}
-+
-+
-+static ssize_t qcrypto_block_headerlen_hdr_write_func(QCryptoBlock *block,
-+        size_t offset, const uint8_t *buf, size_t buflen,
-+        void *opaque, Error **errp)
-+{
-+    /* Discard the bytes, we're not actually writing to an image */
-+    return buflen;
-+}
-+
-+
-+bool
-+qcrypto_block_calculate_payload_offset(QCryptoBlockCreateOptions *create_opts,
-+                                       const char *optprefix,
-+                                       size_t *len,
-+                                       Error **errp)
-+{
-+    /* Fake LUKS creation in order to determine the payload size */
-+    g_autoptr(QCryptoBlock) crypto =
-+        qcrypto_block_create(create_opts, optprefix,
-+                             qcrypto_block_headerlen_hdr_init_func,
-+                             qcrypto_block_headerlen_hdr_write_func,
-+                             len, errp);
-+    return crypto != NULL;
-+}
-+
-+
- QCryptoBlockInfo *qcrypto_block_get_info(QCryptoBlock *block,
-                                          Error **errp)
- {
-diff --git a/include/crypto/block.h b/include/crypto/block.h
-index XXXXXXX..XXXXXXX 100644
---- a/include/crypto/block.h
-+++ b/include/crypto/block.h
-@@ -XXX,XX +XXX,XX @@ QCryptoBlock *qcrypto_block_create(QCryptoBlockCreateOptions *options,
-                                    Error **errp);
-+/**
-+ * qcrypto_block_calculate_payload_offset:
-+ * @create_opts: the encryption options
-+ * @optprefix: name prefix for options
-+ * @len: output for number of header bytes before payload
-+ * @errp: pointer to a NULL-initialized error object
-+ *
-+ * Calculate the number of header bytes before the payload in an encrypted
-+ * storage volume.  The header is an area before the payload that is reserved
-+ * for encryption metadata.
-+ *
-+ * Returns: true on success, false on error
-+ */
-+bool
-+qcrypto_block_calculate_payload_offset(QCryptoBlockCreateOptions *create_opts,
-+                                       const char *optprefix,
-+                                       size_t *len,
-+                                       Error **errp);
-+
-+
- /**
-  * qcrypto_block_get_info:
-  * @block: the block encryption object
-@@ -XXX,XX +XXX,XX @@ uint64_t qcrypto_block_get_sector_size(QCryptoBlock *block);
- void qcrypto_block_free(QCryptoBlock *block);
- G_DEFINE_AUTOPTR_CLEANUP_FUNC(QCryptoBlock, qcrypto_block_free)
-+G_DEFINE_AUTOPTR_CLEANUP_FUNC(QCryptoBlockCreateOptions,
-+                              qapi_free_QCryptoBlockCreateOptions)
- #endif /* QCRYPTO_BLOCK_H */
---
-.24.1

-[PULL 02/19] luks: implement .bdrv_measure()
+[PULL for-9.0 1/1] coroutine: reserve 5,000 mappings
-From: Stefan Hajnoczi <stefanha@redhat.com>
+Daniel P. Berrangé <berrange@redhat.com> pointed out that the coroutine
 pool size heuristic is very conservative. Instead of halving
 max_map_count, he suggested reserving 5,000 mappings for non-coroutine
 users based on observations of guests he has access to.
-Add qemu-img measure support in the "luks" block driver.
+Fixes: 86a637e48104 ("coroutine: cap per-thread local pool size")
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
 Message-id: 20240320181232.1464819-1-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
  util/qemu-coroutine.c | 15 ++++++++++-----
 file changed, 10 insertions(+), 5 deletions(-)
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c
 Reviewed-by: Max Reitz <mreitz@redhat.com>
 Message-Id: <20200221112522.1497712-3-stefanha@redhat.com>
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  block/crypto.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++
 file changed, 62 insertions(+)
 diff --git a/block/crypto.c b/block/crypto.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/crypto.c
+--- a/util/qemu-coroutine.c
-+++ b/block/crypto.c
++++ b/util/qemu-coroutine.c
-@@ -XXX,XX +XXX,XX @@ static int64_t block_crypto_getlength(BlockDriverState *bs)
+@@ -XXX,XX +XXX,XX @@ static unsigned int get_global_pool_hard_max_size(void)
- }
+                             NULL) &&
+         qemu_strtoi(contents, NULL, 10, &max_map_count) == 0) {
+         /*
-+static BlockMeasureInfo *block_crypto_measure(QemuOpts *opts,
+-         * This is a conservative upper bound that avoids exceeding
-+                                              BlockDriverState *in_bs,
+-         * max_map_count. Leave half for non-coroutine users like library
-+                                              Error **errp)
+-         * dependencies, vhost-user, etc. Each coroutine takes up 2 VMAs so
-+{
+-         * halve the amount again.
-+    g_autoptr(QCryptoBlockCreateOptions) create_opts = NULL;
++         * This is an upper bound that avoids exceeding max_map_count. Leave a
-+    Error *local_err = NULL;
++         * fixed amount for non-coroutine users like library dependencies,
-+    BlockMeasureInfo *info;
++         * vhost-user, etc. Each coroutine takes up 2 VMAs so halve the
-+    uint64_t size;
++         * remaining amount.
-+    size_t luks_payload_size;
+          */
-+    QDict *cryptoopts;
+-        return max_map_count / 4;
-+
++        if (max_map_count > 5000) {
-+    /*
++            return (max_map_count - 5000) / 2;
-+     * Preallocation mode doesn't affect size requirements but we must consume
++        } else {
-+     * the option.
++            /* Disable the global pool but threads still have local pools */
-+     */
++            return 0;
 +    g_free(qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC));
 +
 +    size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
 +
 +    if (in_bs) {
 +        int64_t ssize = bdrv_getlength(in_bs);
 +
 +        if (ssize < 0) {
 +            error_setg_errno(&local_err, -ssize,
 +                             "Unable to get image virtual_size");
 +            goto err;
 +        }
-+
+     }
-+        size = ssize;
+ #endif
 +    }
 +
 +    cryptoopts = qemu_opts_to_qdict_filtered(opts, NULL,
 +            &block_crypto_create_opts_luks, true);
 +    qdict_put_str(cryptoopts, "format", "luks");
 +    create_opts = block_crypto_create_opts_init(cryptoopts, &local_err);
 +    qobject_unref(cryptoopts);
 +    if (!create_opts) {
 +        goto err;
 +    }
 +
 +    if (!qcrypto_block_calculate_payload_offset(create_opts, NULL,
 +                                                &luks_payload_size,
 +                                                &local_err)) {
 +        goto err;
 +    }
 +
 +    /*
 +     * Unallocated blocks are still encrypted so allocation status makes no
 +     * difference to the file size.
 +     */
 +    info = g_new(BlockMeasureInfo, 1);
 +    info->fully_allocated = luks_payload_size + size;
 +    info->required = luks_payload_size + size;
 +    return info;
 +
 +err:
 +    error_propagate(errp, local_err);
 +    return NULL;
 +}
 +
 +
  static int block_crypto_probe_luks(const uint8_t *buf,
                                     int buf_size,
                                     const char *filename) {
@@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_crypto_luks = {
      .bdrv_co_preadv     = block_crypto_co_preadv,
      .bdrv_co_pwritev    = block_crypto_co_pwritev,
      .bdrv_getlength     = block_crypto_getlength,
 +    .bdrv_measure       = block_crypto_measure,
      .bdrv_get_info      = block_crypto_get_info_luks,
      .bdrv_get_specific_info = block_crypto_get_specific_info_luks,
 --
-.24.1
+.44.0

-[PULL 03/19] qemu-img: allow qemu-img measure --object without a filename
+Deleted patch
-From: Stefan Hajnoczi <stefanha@redhat.com>
-In most qemu-img sub-commands the --object option only makes sense when
-there is a filename.  qemu-img measure is an exception because objects
-may be referenced from the image creation options instead of an existing
-image file.  Allow --object without a filename.
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Message-Id: <20200221112522.1497712-4-stefanha@redhat.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- qemu-img.c                       | 6 ++----
- tests/qemu-iotests/178           | 2 +-
- tests/qemu-iotests/178.out.qcow2 | 8 ++++----
- tests/qemu-iotests/178.out.raw   | 8 ++++----
-files changed, 11 insertions(+), 13 deletions(-)
-diff --git a/qemu-img.c b/qemu-img.c
-index XXXXXXX..XXXXXXX 100644
---- a/qemu-img.c
-+++ b/qemu-img.c
-@@ -XXX,XX +XXX,XX @@ static int img_measure(int argc, char **argv)
-         filename = argv[optind];
-     }
--    if (!filename &&
--        (object_opts || image_opts || fmt || snapshot_name || sn_opts)) {
--        error_report("--object, --image-opts, -f, and -l "
--                     "require a filename argument.");
-+    if (!filename && (image_opts || fmt || snapshot_name || sn_opts)) {
-+        error_report("--image-opts, -f, and -l require a filename argument.");
-         goto out;
-     }
-     if (filename && img_size != UINT64_MAX) {
-diff --git a/tests/qemu-iotests/178 b/tests/qemu-iotests/178
-index XXXXXXX..XXXXXXX 100755
---- a/tests/qemu-iotests/178
-+++ b/tests/qemu-iotests/178
-@@ -XXX,XX +XXX,XX @@ _make_test_img 1G
- $QEMU_IMG measure # missing arguments
- $QEMU_IMG measure --size 2G "$TEST_IMG" # only one allowed
- $QEMU_IMG measure "$TEST_IMG" a # only one filename allowed
--$QEMU_IMG measure --object secret,id=sec0,data=MTIzNDU2,format=base64 # missing filename
-+$QEMU_IMG measure --object secret,id=sec0,data=MTIzNDU2,format=base64 # size or filename needed
- $QEMU_IMG measure --image-opts # missing filename
- $QEMU_IMG measure -f qcow2 # missing filename
- $QEMU_IMG measure -l snap1 # missing filename
-diff --git a/tests/qemu-iotests/178.out.qcow2 b/tests/qemu-iotests/178.out.qcow2
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/178.out.qcow2
-+++ b/tests/qemu-iotests/178.out.qcow2
-@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
- qemu-img: Either --size N or one filename must be specified.
- qemu-img: --size N cannot be used together with a filename.
- qemu-img: At most one filename argument is allowed.
--qemu-img: --object, --image-opts, -f, and -l require a filename argument.
--qemu-img: --object, --image-opts, -f, and -l require a filename argument.
--qemu-img: --object, --image-opts, -f, and -l require a filename argument.
--qemu-img: --object, --image-opts, -f, and -l require a filename argument.
-+qemu-img: Either --size N or one filename must be specified.
-+qemu-img: --image-opts, -f, and -l require a filename argument.
-+qemu-img: --image-opts, -f, and -l require a filename argument.
-+qemu-img: --image-opts, -f, and -l require a filename argument.
- qemu-img: Invalid option list: ,
- qemu-img: Invalid parameter 'snapshot.foo'
- qemu-img: Failed in parsing snapshot param 'snapshot.foo'
-diff --git a/tests/qemu-iotests/178.out.raw b/tests/qemu-iotests/178.out.raw
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/178.out.raw
-+++ b/tests/qemu-iotests/178.out.raw
-@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
- qemu-img: Either --size N or one filename must be specified.
- qemu-img: --size N cannot be used together with a filename.
- qemu-img: At most one filename argument is allowed.
--qemu-img: --object, --image-opts, -f, and -l require a filename argument.
--qemu-img: --object, --image-opts, -f, and -l require a filename argument.
--qemu-img: --object, --image-opts, -f, and -l require a filename argument.
--qemu-img: --object, --image-opts, -f, and -l require a filename argument.
-+qemu-img: Either --size N or one filename must be specified.
-+qemu-img: --image-opts, -f, and -l require a filename argument.
-+qemu-img: --image-opts, -f, and -l require a filename argument.
-+qemu-img: --image-opts, -f, and -l require a filename argument.
- qemu-img: Invalid option list: ,
- qemu-img: Invalid parameter 'snapshot.foo'
- qemu-img: Failed in parsing snapshot param 'snapshot.foo'
---
-.24.1

-[PULL 04/19] iotests: add 288 luks qemu-img measure test
+Deleted patch
-From: Stefan Hajnoczi <stefanha@redhat.com>
-This test exercises the block/crypto.c "luks" block driver
-.bdrv_measure() code.
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Message-Id: <20200221112522.1497712-5-stefanha@redhat.com>
-[mreitz: Renamed test from 282 to 288]
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- tests/qemu-iotests/288     | 93 ++++++++++++++++++++++++++++++++++++++
- tests/qemu-iotests/288.out | 30 ++++++++++++
- tests/qemu-iotests/group   |  1 +
-files changed, 124 insertions(+)
- create mode 100755 tests/qemu-iotests/288
- create mode 100644 tests/qemu-iotests/288.out
-diff --git a/tests/qemu-iotests/288 b/tests/qemu-iotests/288
-new file mode 100755
-index XXXXXXX..XXXXXXX
---- /dev/null
-+++ b/tests/qemu-iotests/288
-@@ -XXX,XX +XXX,XX @@
-+#!/usr/bin/env bash
-+#
-+# qemu-img measure tests for LUKS images
-+#
-+# Copyright (C) 2020 Red Hat, Inc.
-+#
-+# This program is free software; you can redistribute it and/or modify
-+# it under the terms of the GNU General Public License as published by
-+# the Free Software Foundation; either version 2 of the License, or
-+# (at your option) any later version.
-+#
-+# This program is distributed in the hope that it will be useful,
-+# but WITHOUT ANY WARRANTY; without even the implied warranty of
-+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-+# GNU General Public License for more details.
-+#
-+# You should have received a copy of the GNU General Public License
-+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
-+#
-+
-+# creator
-+owner=stefanha@redhat.com
-+
-+seq=`basename $0`
-+echo "QA output created by $seq"
-+
-+status=1    # failure is the default!
-+
-+_cleanup()
-+{
-+    _cleanup_test_img
-+    rm -f "$TEST_IMG.converted"
-+}
-+trap "_cleanup; exit \$status" 0 1 2 3 15
-+
-+# get standard environment, filters and checks
-+. ./common.rc
-+. ./common.filter
-+. ./common.pattern
-+
-+_supported_fmt luks
-+_supported_proto file
-+_supported_os Linux
-+
-+SECRET=secret,id=sec0,data=passphrase
-+
-+echo "== measure 1G image file =="
-+echo
-+
-+$QEMU_IMG measure --object "$SECRET" \
-+              -O "$IMGFMT" \
-+          -o key-secret=sec0,iter-time=10 \
-+          --size 1G
-+
-+echo
-+echo "== create 1G image file (size should be no greater than measured) =="
-+echo
-+
-+_make_test_img 1G
-+stat -c "image file size in bytes: %s" "$TEST_IMG_FILE"
-+
-+echo
-+echo "== modified 1G image file (size should be no greater than measured) =="
-+echo
-+
-+$QEMU_IO --object "$SECRET" --image-opts "$TEST_IMG" -c "write -P 0x51 0x10000 0x400" | _filter_qemu_io | _filter_testdir
-+stat -c "image file size in bytes: %s" "$TEST_IMG_FILE"
-+
-+echo
-+echo "== measure preallocation=falloc 1G image file =="
-+echo
-+
-+$QEMU_IMG measure --object "$SECRET" \
-+              -O "$IMGFMT" \
-+          -o key-secret=sec0,iter-time=10,preallocation=falloc \
-+          --size 1G
-+
-+echo
-+echo "== measure with input image file =="
-+echo
-+
-+IMGFMT=raw IMGKEYSECRET= IMGOPTS= _make_test_img 1G | _filter_imgfmt
-+QEMU_IO_OPTIONS= IMGOPTSSYNTAX= $QEMU_IO -f raw -c "write -P 0x51 0x10000 0x400" "$TEST_IMG_FILE" | _filter_qemu_io | _filter_testdir
-+$QEMU_IMG measure --object "$SECRET" \
-+              -O "$IMGFMT" \
-+          -o key-secret=sec0,iter-time=10 \
-+          -f raw \
-+          "$TEST_IMG_FILE"
-+
-+# success, all done
-+echo "*** done"
-+rm -f $seq.full
-+status=0
-diff --git a/tests/qemu-iotests/288.out b/tests/qemu-iotests/288.out
-new file mode 100644
-index XXXXXXX..XXXXXXX
---- /dev/null
-+++ b/tests/qemu-iotests/288.out
-@@ -XXX,XX +XXX,XX @@
-+QA output created by 288
-+== measure 1G image file ==
-+
-+required size: 1075810304
-+fully allocated size: 1075810304
-+
-+== create 1G image file (size should be no greater than measured) ==
-+
-+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
-+image file size in bytes: 1075810304
-+
-+== modified 1G image file (size should be no greater than measured) ==
-+
-+wrote 1024/1024 bytes at offset 65536
-+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-+image file size in bytes: 1075810304
-+
-+== measure preallocation=falloc 1G image file ==
-+
-+required size: 1075810304
-+fully allocated size: 1075810304
-+
-+== measure with input image file ==
-+
-+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
-+wrote 1024/1024 bytes at offset 65536
-+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-+required size: 1075810304
-+fully allocated size: 1075810304
-+*** done
-diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/group
-+++ b/tests/qemu-iotests/group
-@@ -XXX,XX +XXX,XX @@
-auto quick
-rw
-rw quick
-+288 quick
---
-.24.1

-[PULL 05/19] block/curl: HTTP header fields allow whitespace around values
+Deleted patch
-From: David Edmondson <david.edmondson@oracle.com>
-RFC 7230 section 3.2 indicates that whitespace is permitted between
-the field name and field value and after the field value.
-Signed-off-by: David Edmondson <david.edmondson@oracle.com>
-Message-Id: <20200224101310.101169-2-david.edmondson@oracle.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block/curl.c | 31 +++++++++++++++++++++++++++----
-file changed, 27 insertions(+), 4 deletions(-)
-diff --git a/block/curl.c b/block/curl.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/curl.c
-+++ b/block/curl.c
-@@ -XXX,XX +XXX,XX @@ static size_t curl_header_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
- {
-     BDRVCURLState *s = opaque;
-     size_t realsize = size * nmemb;
--    const char *accept_line = "Accept-Ranges: bytes";
-+    const char *header = (char *)ptr;
-+    const char *end = header + realsize;
-+    const char *accept_ranges = "Accept-Ranges:";
-+    const char *bytes = "bytes";
--    if (realsize >= strlen(accept_line)
--        && strncmp((char *)ptr, accept_line, strlen(accept_line)) == 0) {
--        s->accept_range = true;
-+    if (realsize >= strlen(accept_ranges)
-+        && strncmp(header, accept_ranges, strlen(accept_ranges)) == 0) {
-+
-+        char *p = strchr(header, ':') + 1;
-+
-+        /* Skip whitespace between the header name and value. */
-+        while (p < end && *p && g_ascii_isspace(*p)) {
-+            p++;
-+        }
-+
-+        if (end - p >= strlen(bytes)
-+            && strncmp(p, bytes, strlen(bytes)) == 0) {
-+
-+            /* Check that there is nothing but whitespace after the value. */
-+            p += strlen(bytes);
-+            while (p < end && *p && g_ascii_isspace(*p)) {
-+                p++;
-+            }
-+
-+            if (p == end || !*p) {
-+                s->accept_range = true;
-+            }
-+        }
-     }
-     return realsize;
---
-.24.1

-[PULL 06/19] block/curl: HTTP header field names are case insensitive
+Deleted patch
-From: David Edmondson <david.edmondson@oracle.com>
-RFC 7230 section 3.2 indicates that HTTP header field names are case
-insensitive.
-Signed-off-by: David Edmondson <david.edmondson@oracle.com>
-Message-Id: <20200224101310.101169-3-david.edmondson@oracle.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block/curl.c | 5 +++--
-file changed, 3 insertions(+), 2 deletions(-)
-diff --git a/block/curl.c b/block/curl.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/curl.c
-+++ b/block/curl.c
-@@ -XXX,XX +XXX,XX @@ static size_t curl_header_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
-     size_t realsize = size * nmemb;
-     const char *header = (char *)ptr;
-     const char *end = header + realsize;
--    const char *accept_ranges = "Accept-Ranges:";
-+    const char *accept_ranges = "accept-ranges:";
-     const char *bytes = "bytes";
-     if (realsize >= strlen(accept_ranges)
--        && strncmp(header, accept_ranges, strlen(accept_ranges)) == 0) {
-+        && g_ascii_strncasecmp(header, accept_ranges,
-+                               strlen(accept_ranges)) == 0) {
-         char *p = strchr(header, ':') + 1;
---
-.24.1

-[PULL 07/19] iotests: Fix nonportable use of od --endian
+Deleted patch
-From: Eric Blake <eblake@redhat.com>
-Tests 261 and 272 fail on RHEL 7 with coreutils 8.22, since od
---endian was not added until coreutils 8.23.  Fix this by manually
-constructing the final value one byte at a time.
-Fixes: fc8ba423
-Reported-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
-Signed-off-by: Eric Blake <eblake@redhat.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Message-Id: <20200226125424.481840-1-eblake@redhat.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- tests/qemu-iotests/common.rc | 22 +++++++++++++++++-----
-file changed, 17 insertions(+), 5 deletions(-)
-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 @@ poke_file()
- # peek_file_le 'test.img' 512 2 => 65534
- peek_file_le()
- {
--    # Wrap in echo $() to strip spaces
--    echo $(od -j"$2" -N"$3" --endian=little -An -vtu"$3" "$1")
-+    local val=0 shift=0 byte
-+
-+    # coreutils' od --endian is not portable, so manually assemble bytes.
-+    for byte in $(od -j"$2" -N"$3" -An -v -tu1 "$1"); do
-+        val=$(( val | (byte << shift) ))
-+        shift=$((shift + 8))
-+    done
-+    printf %llu $val
- }
- # peek_file_be 'test.img' 512 2 => 65279
- peek_file_be()
- {
--    # Wrap in echo $() to strip spaces
--    echo $(od -j"$2" -N"$3" --endian=big -An -vtu"$3" "$1")
-+    local val=0 byte
-+
-+    # coreutils' od --endian is not portable, so manually assemble bytes.
-+    for byte in $(od -j"$2" -N"$3" -An -v -tu1 "$1"); do
-+        val=$(( (val << 8) | byte ))
-+    done
-+    printf %llu $val
- }
--# peek_file_raw 'test.img' 512 2 => '\xff\xfe'
-+# peek_file_raw 'test.img' 512 2 => '\xff\xfe'. Do not use if the raw data
-+# is likely to contain \0 or trailing \n.
- peek_file_raw()
- {
-     dd if="$1" bs=1 skip="$2" count="$3" status=none
---
-.24.1

-[PULL 08/19] block/qcow2: do free crypto_opts in qcow2_close()
+Deleted patch
-From: Pan Nengyuan <pannengyuan@huawei.com>
-'crypto_opts' forgot to free in qcow2_close(), this patch fix the bellow leak stack:
-Direct leak of 24 byte(s) in 1 object(s) allocated from:
-    #0 0x7f0edd81f970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
-    #1 0x7f0edc6d149d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
-    #2 0x55d7eaede63d in qobject_input_start_struct /mnt/sdb/qemu-new/qemu_test/qemu/qapi/qobject-input-visitor.c:295
-    #3 0x55d7eaed78b8 in visit_start_struct /mnt/sdb/qemu-new/qemu_test/qemu/qapi/qapi-visit-core.c:49
-    #4 0x55d7eaf5140b in visit_type_QCryptoBlockOpenOptions qapi/qapi-visit-crypto.c:290
-    #5 0x55d7eae43af3 in block_crypto_open_opts_init /mnt/sdb/qemu-new/qemu_test/qemu/block/crypto.c:163
-    #6 0x55d7eacd2924 in qcow2_update_options_prepare /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1148
-    #7 0x55d7eacd33f7 in qcow2_update_options /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1232
-    #8 0x55d7eacd9680 in qcow2_do_open /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1512
-    #9 0x55d7eacdc55e in qcow2_open_entry /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1792
-    #10 0x55d7eacdc8fe in qcow2_open /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1819
-    #11 0x55d7eac3742d in bdrv_open_driver /mnt/sdb/qemu-new/qemu_test/qemu/block.c:1317
-    #12 0x55d7eac3e990 in bdrv_open_common /mnt/sdb/qemu-new/qemu_test/qemu/block.c:1575
-    #13 0x55d7eac4442c in bdrv_open_inherit /mnt/sdb/qemu-new/qemu_test/qemu/block.c:3126
-    #14 0x55d7eac45c3f in bdrv_open /mnt/sdb/qemu-new/qemu_test/qemu/block.c:3219
-    #15 0x55d7ead8e8a4 in blk_new_open /mnt/sdb/qemu-new/qemu_test/qemu/block/block-backend.c:397
-    #16 0x55d7eacde74c in qcow2_co_create /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:3534
-    #17 0x55d7eacdfa6d in qcow2_co_create_opts /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:3668
-    #18 0x55d7eac1c678 in bdrv_create_co_entry /mnt/sdb/qemu-new/qemu_test/qemu/block.c:485
-    #19 0x55d7eb0024d2 in coroutine_trampoline /mnt/sdb/qemu-new/qemu_test/qemu/util/coroutine-ucontext.c:115
-Reported-by: Euler Robot <euler.robot@huawei.com>
-Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Message-Id: <20200227012950.12256-2-pannengyuan@huawei.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block/qcow2.c | 1 +
-file changed, 1 insertion(+)
-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 qcow2_close(BlockDriverState *bs)
-     qcrypto_block_free(s->crypto);
-     s->crypto = NULL;
-+    qapi_free_QCryptoBlockOpenOptions(s->crypto_opts);
-     g_free(s->unknown_header_fields);
-     cleanup_unknown_header_ext(bs);
---
-.24.1

-[PULL 09/19] qemu-img: free memory before re-assign
+Deleted patch
-From: Pan Nengyuan <pannengyuan@huawei.com>
-collect_image_check() is called twice in img_check(), the filename/format will be alloced without free the original memory.
-It is not a big deal since the process will exit anyway, but seems like a clean code and it will remove the warning spotted by asan.
-Reported-by: Euler Robot <euler.robot@huawei.com>
-Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
-Message-Id: <20200227012950.12256-3-pannengyuan@huawei.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- qemu-img.c | 2 ++
-file changed, 2 insertions(+)
-diff --git a/qemu-img.c b/qemu-img.c
-index XXXXXXX..XXXXXXX 100644
---- a/qemu-img.c
-+++ b/qemu-img.c
-@@ -XXX,XX +XXX,XX @@ static int img_check(int argc, char **argv)
-                     check->corruptions_fixed);
-         }
-+        qapi_free_ImageCheck(check);
-+        check = g_new0(ImageCheck, 1);
-         ret = collect_image_check(bs, check, filename, fmt, 0);
-         check->leaks_fixed          = leaks_fixed;
---
-.24.1

-[PULL 10/19] block/qcow2-threads: fix qcow2_decompress
+Deleted patch
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-On success path we return what inflate() returns instead of 0. And it
-most probably works for Z_STREAM_END as it is positive, but is
-definitely broken for Z_BUF_ERROR.
-While being here, switch to errno return code, to be closer to
-qcow2_compress API (and usual expectations).
-Revert condition in if to be more positive. Drop dead initialization of
-ret.
-Cc: qemu-stable@nongnu.org # v4.0
-Fixes: 341926ab83e2b
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Message-Id: <20200302150930.16218-1-vsementsov@virtuozzo.com>
-Reviewed-by: Alberto Garcia <berto@igalia.com>
-Reviewed-by: Ján Tomko <jtomko@redhat.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block/qcow2-threads.c | 12 +++++++-----
-file changed, 7 insertions(+), 5 deletions(-)
-diff --git a/block/qcow2-threads.c b/block/qcow2-threads.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2-threads.c
-+++ b/block/qcow2-threads.c
-@@ -XXX,XX +XXX,XX @@ static ssize_t qcow2_compress(void *dest, size_t dest_size,
-  * @src - source buffer, @src_size bytes
-  *
-  * Returns: 0 on success
-- *          -1 on fail
-+ *          -EIO on fail
-  */
- static ssize_t qcow2_decompress(void *dest, size_t dest_size,
-                                 const void *src, size_t src_size)
- {
--    int ret = 0;
-+    int ret;
-     z_stream strm;
-     memset(&strm, 0, sizeof(strm));
-@@ -XXX,XX +XXX,XX @@ static ssize_t qcow2_decompress(void *dest, size_t dest_size,
-     ret = inflateInit2(&strm, -12);
-     if (ret != Z_OK) {
--        return -1;
-+        return -EIO;
-     }
-     ret = inflate(&strm, Z_FINISH);
--    if ((ret != Z_STREAM_END && ret != Z_BUF_ERROR) || strm.avail_out != 0) {
-+    if ((ret == Z_STREAM_END || ret == Z_BUF_ERROR) && strm.avail_out == 0) {
-         /*
-          * We approve Z_BUF_ERROR because we need @dest buffer to be filled, but
-          * @src buffer may be processed partly (because in qcow2 we know size of
-          * compressed data with precision of one sector)
-          */
--        ret = -1;
-+        ret = 0;
-+    } else {
-+        ret = -EIO;
-     }
-     inflateEnd(&strm);
---
-.24.1

-[PULL 11/19] job: refactor progress to separate object
+Deleted patch
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-We need it in separate to pass to the block-copy object in the next
-commit.
-Cc: qemu-stable@nongnu.org
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Message-Id: <20200311103004.7649-2-vsementsov@virtuozzo.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- blockjob.c                    | 16 +++++-----
- include/qemu/job.h            | 11 ++-----
- include/qemu/progress_meter.h | 58 +++++++++++++++++++++++++++++++++++
- job-qmp.c                     |  4 +--
- job.c                         |  6 ++--
- qemu-img.c                    |  6 ++--
-files changed, 76 insertions(+), 25 deletions(-)
- create mode 100644 include/qemu/progress_meter.h
-diff --git a/blockjob.c b/blockjob.c
-index XXXXXXX..XXXXXXX 100644
---- a/blockjob.c
-+++ b/blockjob.c
-@@ -XXX,XX +XXX,XX @@ BlockJobInfo *block_job_query(BlockJob *job, Error **errp)
-     info->device    = g_strdup(job->job.id);
-     info->busy      = atomic_read(&job->job.busy);
-     info->paused    = job->job.pause_count > 0;
--    info->offset    = job->job.progress_current;
--    info->len       = job->job.progress_total;
-+    info->offset    = job->job.progress.current;
-+    info->len       = job->job.progress.total;
-     info->speed     = job->speed;
-     info->io_status = job->iostatus;
-     info->ready     = job_is_ready(&job->job),
-@@ -XXX,XX +XXX,XX @@ static void block_job_event_cancelled(Notifier *n, void *opaque)
-     qapi_event_send_block_job_cancelled(job_type(&job->job),
-                                         job->job.id,
--                                        job->job.progress_total,
--                                        job->job.progress_current,
-+                                        job->job.progress.total,
-+                                        job->job.progress.current,
-                                         job->speed);
- }
-@@ -XXX,XX +XXX,XX @@ static void block_job_event_completed(Notifier *n, void *opaque)
-     qapi_event_send_block_job_completed(job_type(&job->job),
-                                         job->job.id,
--                                        job->job.progress_total,
--                                        job->job.progress_current,
-+                                        job->job.progress.total,
-+                                        job->job.progress.current,
-                                         job->speed,
-                                         !!msg,
-                                         msg);
-@@ -XXX,XX +XXX,XX @@ static void block_job_event_ready(Notifier *n, void *opaque)
-     qapi_event_send_block_job_ready(job_type(&job->job),
-                                     job->job.id,
--                                    job->job.progress_total,
--                                    job->job.progress_current,
-+                                    job->job.progress.total,
-+                                    job->job.progress.current,
-                                     job->speed);
- }
-diff --git a/include/qemu/job.h b/include/qemu/job.h
-index XXXXXXX..XXXXXXX 100644
---- a/include/qemu/job.h
-+++ b/include/qemu/job.h
-@@ -XXX,XX +XXX,XX @@
- #include "qapi/qapi-types-job.h"
- #include "qemu/queue.h"
-+#include "qemu/progress_meter.h"
- #include "qemu/coroutine.h"
- #include "block/aio.h"
-@@ -XXX,XX +XXX,XX @@ typedef struct Job {
-     /** True if this job should automatically dismiss itself */
-     bool auto_dismiss;
--    /**
--     * Current progress. The unit is arbitrary as long as the ratio between
--     * progress_current and progress_total represents the estimated percentage
--     * of work already done.
--     */
--    int64_t progress_current;
--
--    /** Estimated progress_current value at the completion of the job */
--    int64_t progress_total;
-+    ProgressMeter progress;
-     /**
-      * Return code from @run and/or @prepare callback(s).
-diff --git a/include/qemu/progress_meter.h b/include/qemu/progress_meter.h
-new file mode 100644
-index XXXXXXX..XXXXXXX
---- /dev/null
-+++ b/include/qemu/progress_meter.h
-@@ -XXX,XX +XXX,XX @@
-+/*
-+ * Helper functionality for some process progress tracking.
-+ *
-+ * Copyright (c) 2011 IBM Corp.
-+ * Copyright (c) 2012, 2018 Red Hat, Inc.
-+ * Copyright (c) 2020 Virtuozzo International GmbH
-+ *
-+ * Permission is hereby granted, free of charge, to any person obtaining a copy
-+ * of this software and associated documentation files (the "Software"), to deal
-+ * in the Software without restriction, including without limitation the rights
-+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
-+ * copies of the Software, and to permit persons to whom the Software is
-+ * furnished to do so, subject to the following conditions:
-+ *
-+ * The above copyright notice and this permission notice shall be included in
-+ * all copies or substantial portions of the Software.
-+ *
-+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
-+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
-+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
-+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
-+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
-+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
-+ * THE SOFTWARE.
-+ */
-+
-+#ifndef QEMU_PROGRESS_METER_H
-+#define QEMU_PROGRESS_METER_H
-+
-+typedef struct ProgressMeter {
-+    /**
-+     * Current progress. The unit is arbitrary as long as the ratio between
-+     * current and total represents the estimated percentage
-+     * of work already done.
-+     */
-+    uint64_t current;
-+
-+    /** Estimated current value at the completion of the process */
-+    uint64_t total;
-+} ProgressMeter;
-+
-+static inline void progress_work_done(ProgressMeter *pm, uint64_t done)
-+{
-+    pm->current += done;
-+}
-+
-+static inline void progress_set_remaining(ProgressMeter *pm, uint64_t remaining)
-+{
-+    pm->total = pm->current + remaining;
-+}
-+
-+static inline void progress_increase_remaining(ProgressMeter *pm,
-+                                               uint64_t delta)
-+{
-+    pm->total += delta;
-+}
-+
-+#endif /* QEMU_PROGRESS_METER_H */
-diff --git a/job-qmp.c b/job-qmp.c
-index XXXXXXX..XXXXXXX 100644
---- a/job-qmp.c
-+++ b/job-qmp.c
-@@ -XXX,XX +XXX,XX @@ static JobInfo *job_query_single(Job *job, Error **errp)
-         .id                 = g_strdup(job->id),
-         .type               = job_type(job),
-         .status             = job->status,
--        .current_progress   = job->progress_current,
--        .total_progress     = job->progress_total,
-+        .current_progress   = job->progress.current,
-+        .total_progress     = job->progress.total,
-         .has_error          = !!job->err,
-         .error              = job->err ? \
-                               g_strdup(error_get_pretty(job->err)) : NULL,
-diff --git a/job.c b/job.c
-index XXXXXXX..XXXXXXX 100644
---- a/job.c
-+++ b/job.c
-@@ -XXX,XX +XXX,XX @@ void job_unref(Job *job)
- void job_progress_update(Job *job, uint64_t done)
- {
--    job->progress_current += done;
-+    progress_work_done(&job->progress, done);
- }
- void job_progress_set_remaining(Job *job, uint64_t remaining)
- {
--    job->progress_total = job->progress_current + remaining;
-+    progress_set_remaining(&job->progress, remaining);
- }
- void job_progress_increase_remaining(Job *job, uint64_t delta)
- {
--    job->progress_total += delta;
-+    progress_increase_remaining(&job->progress, delta);
- }
- void job_event_cancelled(Job *job)
-diff --git a/qemu-img.c b/qemu-img.c
-index XXXXXXX..XXXXXXX 100644
---- a/qemu-img.c
-+++ b/qemu-img.c
-@@ -XXX,XX +XXX,XX @@ static void run_block_job(BlockJob *job, Error **errp)
-     do {
-         float progress = 0.0f;
-         aio_poll(aio_context, true);
--        if (job->job.progress_total) {
--            progress = (float)job->job.progress_current /
--                       job->job.progress_total * 100.f;
-+        if (job->job.progress.total) {
-+            progress = (float)job->job.progress.current /
-+                       job->job.progress.total * 100.f;
-         }
-         qemu_progress_print(progress, 0);
-     } while (!job_is_ready(&job->job) && !job_is_completed(&job->job));
---
-.24.1

-[PULL 12/19] block/block-copy: fix progress calculation
+Deleted patch
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Assume we have two regions, A and B, and region B is in-flight now,
-region A is not yet touched, but it is unallocated and should be
-skipped.
-Correspondingly, as progress we have
-  total = A + B
-  current = 0
-If we reset unallocated region A and call progress_reset_callback,
-it will calculate 0 bytes dirty in the bitmap and call
-job_progress_set_remaining, which will set
-   total = current + 0 = 0 + 0 = 0
-So, B bytes are actually removed from total accounting. When job
-finishes we'll have
-   total = 0
-   current = B
-, which doesn't sound good.
-This is because we didn't considered in-flight bytes, actually when
-calculating remaining, we should have set (in_flight + dirty_bytes)
-as remaining, not only dirty_bytes.
-To fix it, let's refactor progress calculation, moving it to block-copy
-itself instead of fixing callback. And, of course, track in_flight
-bytes count.
-We still have to keep one callback, to maintain backup job bytes_read
-calculation, but it will go on soon, when we turn the whole backup
-process into one block_copy call.
-Cc: qemu-stable@nongnu.org
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
-Message-Id: <20200311103004.7649-3-vsementsov@virtuozzo.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block/backup.c             | 13 ++-----------
- block/block-copy.c         | 16 ++++++++++++----
- include/block/block-copy.h | 15 +++++----------
-files changed, 19 insertions(+), 25 deletions(-)
-diff --git a/block/backup.c b/block/backup.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/backup.c
-+++ b/block/backup.c
-@@ -XXX,XX +XXX,XX @@ static void backup_progress_bytes_callback(int64_t bytes, void *opaque)
-     BackupBlockJob *s = opaque;
-     s->bytes_read += bytes;
--    job_progress_update(&s->common.job, bytes);
--}
--
--static void backup_progress_reset_callback(void *opaque)
--{
--    BackupBlockJob *s = opaque;
--    uint64_t estimate = bdrv_get_dirty_count(s->bcs->copy_bitmap);
--
--    job_progress_set_remaining(&s->common.job, estimate);
- }
- static int coroutine_fn backup_do_cow(BackupBlockJob *job,
-@@ -XXX,XX +XXX,XX @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
-     job->cluster_size = cluster_size;
-     job->len = len;
--    block_copy_set_callbacks(bcs, backup_progress_bytes_callback,
--                             backup_progress_reset_callback, job);
-+    block_copy_set_progress_callback(bcs, backup_progress_bytes_callback, job);
-+    block_copy_set_progress_meter(bcs, &job->common.job.progress);
-     /* Required permissions are already taken by backup-top target */
-     block_job_add_bdrv(&job->common, "target", target, 0, BLK_PERM_ALL,
-diff --git a/block/block-copy.c b/block/block-copy.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/block-copy.c
-+++ b/block/block-copy.c
-@@ -XXX,XX +XXX,XX @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
-     return s;
- }
--void block_copy_set_callbacks(
-+void block_copy_set_progress_callback(
-         BlockCopyState *s,
-         ProgressBytesCallbackFunc progress_bytes_callback,
--        ProgressResetCallbackFunc progress_reset_callback,
-         void *progress_opaque)
- {
-     s->progress_bytes_callback = progress_bytes_callback;
--    s->progress_reset_callback = progress_reset_callback;
-     s->progress_opaque = progress_opaque;
- }
-+void block_copy_set_progress_meter(BlockCopyState *s, ProgressMeter *pm)
-+{
-+    s->progress = pm;
-+}
-+
- /*
-  * block_copy_do_copy
-  *
-@@ -XXX,XX +XXX,XX @@ int64_t block_copy_reset_unallocated(BlockCopyState *s,
-     if (!ret) {
-         bdrv_reset_dirty_bitmap(s->copy_bitmap, offset, bytes);
--        s->progress_reset_callback(s->progress_opaque);
-+        progress_set_remaining(s->progress,
-+                               bdrv_get_dirty_count(s->copy_bitmap) +
-+                               s->in_flight_bytes);
-     }
-     *count = bytes;
-@@ -XXX,XX +XXX,XX @@ int coroutine_fn block_copy(BlockCopyState *s,
-         trace_block_copy_process(s, start);
-         bdrv_reset_dirty_bitmap(s->copy_bitmap, start, chunk_end - start);
-+        s->in_flight_bytes += chunk_end - start;
-         co_get_from_shres(s->mem, chunk_end - start);
-         ret = block_copy_do_copy(s, start, chunk_end, error_is_read);
-         co_put_to_shres(s->mem, chunk_end - start);
-+        s->in_flight_bytes -= chunk_end - start;
-         if (ret < 0) {
-             bdrv_set_dirty_bitmap(s->copy_bitmap, start, chunk_end - start);
-             break;
-         }
-+        progress_work_done(s->progress, chunk_end - start);
-         s->progress_bytes_callback(chunk_end - start, s->progress_opaque);
-         start = chunk_end;
-         ret = 0;
-diff --git a/include/block/block-copy.h b/include/block/block-copy.h
-index XXXXXXX..XXXXXXX 100644
---- a/include/block/block-copy.h
-+++ b/include/block/block-copy.h
-@@ -XXX,XX +XXX,XX @@ typedef struct BlockCopyInFlightReq {
- } BlockCopyInFlightReq;
- typedef void (*ProgressBytesCallbackFunc)(int64_t bytes, void *opaque);
--typedef void (*ProgressResetCallbackFunc)(void *opaque);
- typedef struct BlockCopyState {
-     /*
-      * BdrvChild objects are not owned or managed by block-copy. They are
-@@ -XXX,XX +XXX,XX @@ typedef struct BlockCopyState {
-     BdrvChild *source;
-     BdrvChild *target;
-     BdrvDirtyBitmap *copy_bitmap;
-+    int64_t in_flight_bytes;
-     int64_t cluster_size;
-     bool use_copy_range;
-     int64_t copy_size;
-@@ -XXX,XX +XXX,XX @@ typedef struct BlockCopyState {
-      */
-     bool skip_unallocated;
-+    ProgressMeter *progress;
-     /* progress_bytes_callback: called when some copying progress is done. */
-     ProgressBytesCallbackFunc progress_bytes_callback;
--
--    /*
--     * progress_reset_callback: called when some bytes reset from copy_bitmap
--     * (see @skip_unallocated above). The callee is assumed to recalculate how
--     * many bytes remain based on the dirty bit count of copy_bitmap.
--     */
--    ProgressResetCallbackFunc progress_reset_callback;
-     void *progress_opaque;
-     SharedResource *mem;
-@@ -XXX,XX +XXX,XX @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
-                                      BdrvRequestFlags write_flags,
-                                      Error **errp);
--void block_copy_set_callbacks(
-+void block_copy_set_progress_callback(
-         BlockCopyState *s,
-         ProgressBytesCallbackFunc progress_bytes_callback,
--        ProgressResetCallbackFunc progress_reset_callback,
-         void *progress_opaque);
-+void block_copy_set_progress_meter(BlockCopyState *s, ProgressMeter *pm);
-+
- void block_copy_state_free(BlockCopyState *s);
- int64_t block_copy_reset_unallocated(BlockCopyState *s,
---
-.24.1

-[PULL 13/19] block/block-copy: specialcase first copy_range request
+Deleted patch
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-In block_copy_do_copy we fallback to read+write if copy_range failed.
-In this case copy_size is larger than defined for buffered IO, and
-there is corresponding commit. Still, backup copies data cluster by
-cluster, and most of requests are limited to one cluster anyway, so the
-only source of this one bad-limited request is copy-before-write
-operation.
-Further patch will move backup to use block_copy directly, than for
-cases where copy_range is not supported, first request will be
-oversized in each backup. It's not good, let's change it now.
-Fix is simple: just limit first copy_range request like buffer-based
-request. If it succeed, set larger copy_range limit.
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Message-Id: <20200311103004.7649-4-vsementsov@virtuozzo.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block/block-copy.c | 41 +++++++++++++++++++++++++++++++----------
-file changed, 31 insertions(+), 10 deletions(-)
-diff --git a/block/block-copy.c b/block/block-copy.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/block-copy.c
-+++ b/block/block-copy.c
-@@ -XXX,XX +XXX,XX @@ void block_copy_state_free(BlockCopyState *s)
-     g_free(s);
- }
-+static uint32_t block_copy_max_transfer(BdrvChild *source, BdrvChild *target)
-+{
-+    return MIN_NON_ZERO(INT_MAX,
-+                        MIN_NON_ZERO(source->bs->bl.max_transfer,
-+                                     target->bs->bl.max_transfer));
-+}
-+
- BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
-                                      int64_t cluster_size,
-                                      BdrvRequestFlags write_flags, Error **errp)
- {
-     BlockCopyState *s;
-     BdrvDirtyBitmap *copy_bitmap;
--    uint32_t max_transfer =
--            MIN_NON_ZERO(INT_MAX,
--                         MIN_NON_ZERO(source->bs->bl.max_transfer,
--                                      target->bs->bl.max_transfer));
-     copy_bitmap = bdrv_create_dirty_bitmap(source->bs, cluster_size, NULL,
-                                            errp);
-@@ -XXX,XX +XXX,XX @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
-         .mem = shres_create(BLOCK_COPY_MAX_MEM),
-     };
--    if (max_transfer < cluster_size) {
-+    if (block_copy_max_transfer(source, target) < cluster_size) {
-         /*
-          * copy_range does not respect max_transfer. We don't want to bother
-          * with requests smaller than block-copy cluster size, so fallback to
-@@ -XXX,XX +XXX,XX @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
-         s->copy_size = cluster_size;
-     } else {
-         /*
--         * copy_range does not respect max_transfer (it's a TODO), so we factor
--         * that in here.
-+         * We enable copy-range, but keep small copy_size, until first
-+         * successful copy_range (look at block_copy_do_copy).
-          */
-         s->use_copy_range = true;
--        s->copy_size = MIN(MAX(cluster_size, BLOCK_COPY_MAX_COPY_RANGE),
--                           QEMU_ALIGN_DOWN(max_transfer, cluster_size));
-+        s->copy_size = MAX(s->cluster_size, BLOCK_COPY_MAX_BUFFER);
-     }
-     QLIST_INIT(&s->inflight_reqs);
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn block_copy_do_copy(BlockCopyState *s,
-             s->copy_size = MAX(s->cluster_size, BLOCK_COPY_MAX_BUFFER);
-             /* Fallback to read+write with allocated buffer */
-         } else {
-+            if (s->use_copy_range) {
-+                /*
-+                 * Successful copy-range. Now increase copy_size.  copy_range
-+                 * does not respect max_transfer (it's a TODO), so we factor
-+                 * that in here.
-+                 *
-+                 * Note: we double-check s->use_copy_range for the case when
-+                 * parallel block-copy request unsets it during previous
-+                 * bdrv_co_copy_range call.
-+                 */
-+                s->copy_size =
-+                        MIN(MAX(s->cluster_size, BLOCK_COPY_MAX_COPY_RANGE),
-+                            QEMU_ALIGN_DOWN(block_copy_max_transfer(s->source,
-+                                                                    s->target),
-+                                            s->cluster_size));
-+            }
-             goto out;
-         }
-     }
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn block_copy_do_copy(BlockCopyState *s,
-     /*
-      * In case of failed copy_range request above, we may proceed with buffered
-      * request larger than BLOCK_COPY_MAX_BUFFER. Still, further requests will
--     * be properly limited, so don't care too much.
-+     * be properly limited, so don't care too much. Moreover the most likely
-+     * case (copy_range is unsupported for the configuration, so the very first
-+     * copy_range request fails) is handled by setting large copy_size only
-+     * after first successful copy_range.
-      */
-     bounce_buffer = qemu_blockalign(s->source->bs, nbytes);
---
-.24.1

-[PULL 14/19] block/block-copy: use block_status
+Deleted patch
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Use bdrv_block_status_above to chose effective chunk size and to handle
-zeroes effectively.
-This substitutes checking for just being allocated or not, and drops
-old code path for it. Assistance by backup job is dropped too, as
-caching block-status information is more difficult than just caching
-is-allocated information in our dirty bitmap, and backup job is not
-good place for this caching anyway.
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Message-Id: <20200311103004.7649-5-vsementsov@virtuozzo.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block/block-copy.c | 73 +++++++++++++++++++++++++++++++++++++---------
- block/trace-events |  1 +
-files changed, 61 insertions(+), 13 deletions(-)
-diff --git a/block/block-copy.c b/block/block-copy.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/block-copy.c
-+++ b/block/block-copy.c
-@@ -XXX,XX +XXX,XX @@ void block_copy_set_progress_meter(BlockCopyState *s, ProgressMeter *pm)
-  */
- static int coroutine_fn block_copy_do_copy(BlockCopyState *s,
-                                            int64_t start, int64_t end,
--                                           bool *error_is_read)
-+                                           bool zeroes, bool *error_is_read)
- {
-     int ret;
-     int nbytes = MIN(end, s->len) - start;
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn block_copy_do_copy(BlockCopyState *s,
-     assert(QEMU_IS_ALIGNED(end, s->cluster_size));
-     assert(end < s->len || end == QEMU_ALIGN_UP(s->len, s->cluster_size));
-+    if (zeroes) {
-+        ret = bdrv_co_pwrite_zeroes(s->target, start, nbytes, s->write_flags &
-+                                    ~BDRV_REQ_WRITE_COMPRESSED);
-+        if (ret < 0) {
-+            trace_block_copy_write_zeroes_fail(s, start, ret);
-+            if (error_is_read) {
-+                *error_is_read = false;
-+            }
-+        }
-+        return ret;
-+    }
-+
-     if (s->use_copy_range) {
-         ret = bdrv_co_copy_range(s->source, start, s->target, start, nbytes,
-, s->write_flags);
-@@ -XXX,XX +XXX,XX @@ out:
-     return ret;
- }
-+static int block_copy_block_status(BlockCopyState *s, int64_t offset,
-+                                   int64_t bytes, int64_t *pnum)
-+{
-+    int64_t num;
-+    BlockDriverState *base;
-+    int ret;
-+
-+    if (s->skip_unallocated && s->source->bs->backing) {
-+        base = s->source->bs->backing->bs;
-+    } else {
-+        base = NULL;
-+    }
-+
-+    ret = bdrv_block_status_above(s->source->bs, base, offset, bytes, &num,
-+                                  NULL, NULL);
-+    if (ret < 0 || num < s->cluster_size) {
-+        /*
-+         * On error or if failed to obtain large enough chunk just fallback to
-+         * copy one cluster.
-+         */
-+        num = s->cluster_size;
-+        ret = BDRV_BLOCK_ALLOCATED | BDRV_BLOCK_DATA;
-+    } else if (offset + num == s->len) {
-+        num = QEMU_ALIGN_UP(num, s->cluster_size);
-+    } else {
-+        num = QEMU_ALIGN_DOWN(num, s->cluster_size);
-+    }
-+
-+    *pnum = num;
-+    return ret;
-+}
-+
- /*
-  * Check if the cluster starting at offset is allocated or not.
-  * return via pnum the number of contiguous clusters sharing this allocation.
-@@ -XXX,XX +XXX,XX @@ int coroutine_fn block_copy(BlockCopyState *s,
- {
-     int ret = 0;
-     int64_t end = bytes + start; /* bytes */
--    int64_t status_bytes;
-     BlockCopyInFlightReq req;
-     /*
-@@ -XXX,XX +XXX,XX @@ int coroutine_fn block_copy(BlockCopyState *s,
-     block_copy_inflight_req_begin(s, &req, start, end);
-     while (start < end) {
--        int64_t next_zero, chunk_end;
-+        int64_t next_zero, chunk_end, status_bytes;
-         if (!bdrv_dirty_bitmap_get(s->copy_bitmap, start)) {
-             trace_block_copy_skip(s, start);
-@@ -XXX,XX +XXX,XX @@ int coroutine_fn block_copy(BlockCopyState *s,
-             chunk_end = next_zero;
-         }
--        if (s->skip_unallocated) {
--            ret = block_copy_reset_unallocated(s, start, &status_bytes);
--            if (ret == 0) {
--                trace_block_copy_skip_range(s, start, status_bytes);
--                start += status_bytes;
--                continue;
--            }
--            /* Clamp to known allocated region */
--            chunk_end = MIN(chunk_end, start + status_bytes);
-+        ret = block_copy_block_status(s, start, chunk_end - start,
-+                                      &status_bytes);
-+        if (s->skip_unallocated && !(ret & BDRV_BLOCK_ALLOCATED)) {
-+            bdrv_reset_dirty_bitmap(s->copy_bitmap, start, status_bytes);
-+            progress_set_remaining(s->progress,
-+                                   bdrv_get_dirty_count(s->copy_bitmap) +
-+                                   s->in_flight_bytes);
-+            trace_block_copy_skip_range(s, start, status_bytes);
-+            start += status_bytes;
-+            continue;
-         }
-+        chunk_end = MIN(chunk_end, start + status_bytes);
-+
-         trace_block_copy_process(s, start);
-         bdrv_reset_dirty_bitmap(s->copy_bitmap, start, chunk_end - start);
-         s->in_flight_bytes += chunk_end - start;
-         co_get_from_shres(s->mem, chunk_end - start);
--        ret = block_copy_do_copy(s, start, chunk_end, error_is_read);
-+        ret = block_copy_do_copy(s, start, chunk_end, ret & BDRV_BLOCK_ZERO,
-+                                 error_is_read);
-         co_put_to_shres(s->mem, chunk_end - start);
-         s->in_flight_bytes -= chunk_end - start;
-         if (ret < 0) {
-diff --git a/block/trace-events b/block/trace-events
-index XXXXXXX..XXXXXXX 100644
---- a/block/trace-events
-+++ b/block/trace-events
-@@ -XXX,XX +XXX,XX @@ block_copy_process(void *bcs, int64_t start) "bcs %p start %"PRId64
- block_copy_copy_range_fail(void *bcs, int64_t start, int ret) "bcs %p start %"PRId64" ret %d"
- block_copy_read_fail(void *bcs, int64_t start, int ret) "bcs %p start %"PRId64" ret %d"
- block_copy_write_fail(void *bcs, int64_t start, int ret) "bcs %p start %"PRId64" ret %d"
-+block_copy_write_zeroes_fail(void *bcs, int64_t start, int ret) "bcs %p start %"PRId64" ret %d"
- # ../blockdev.c
- qmp_block_job_cancel(void *job) "job %p"
---
-.24.1

-[PULL 15/19] block/block-copy: factor out find_conflicting_inflight_req
+Deleted patch
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Split find_conflicting_inflight_req to be used separately.
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Message-Id: <20200311103004.7649-6-vsementsov@virtuozzo.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block/block-copy.c | 31 +++++++++++++++++++------------
-file changed, 19 insertions(+), 12 deletions(-)
-diff --git a/block/block-copy.c b/block/block-copy.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/block-copy.c
-+++ b/block/block-copy.c
-@@ -XXX,XX +XXX,XX @@
- #define BLOCK_COPY_MAX_BUFFER (1 * MiB)
- #define BLOCK_COPY_MAX_MEM (128 * MiB)
-+static BlockCopyInFlightReq *find_conflicting_inflight_req(BlockCopyState *s,
-+                                                           int64_t start,
-+                                                           int64_t end)
-+{
-+    BlockCopyInFlightReq *req;
-+
-+    QLIST_FOREACH(req, &s->inflight_reqs, list) {
-+        if (end > req->start_byte && start < req->end_byte) {
-+            return req;
-+        }
-+    }
-+
-+    return NULL;
-+}
-+
- static void coroutine_fn block_copy_wait_inflight_reqs(BlockCopyState *s,
-                                                        int64_t start,
-                                                        int64_t end)
- {
-     BlockCopyInFlightReq *req;
--    bool waited;
--
--    do {
--        waited = false;
--        QLIST_FOREACH(req, &s->inflight_reqs, list) {
--            if (end > req->start_byte && start < req->end_byte) {
--                qemu_co_queue_wait(&req->wait_queue, NULL);
--                waited = true;
--                break;
--            }
--        }
--    } while (waited);
-+
-+    while ((req = find_conflicting_inflight_req(s, start, end))) {
-+        qemu_co_queue_wait(&req->wait_queue, NULL);
-+    }
- }
- static void block_copy_inflight_req_begin(BlockCopyState *s,
---
-.24.1

-[PULL 16/19] block/block-copy: refactor interfaces to use bytes instead of end
+Deleted patch
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-We have a lot of "chunk_end - start" invocations, let's switch to
-bytes/cur_bytes scheme instead.
-While being here, improve check on block_copy_do_copy parameters to not
-overflow when calculating nbytes and use int64_t for bytes in
-block_copy for consistency.
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Message-Id: <20200311103004.7649-7-vsementsov@virtuozzo.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block/block-copy.c         | 78 ++++++++++++++++++++------------------
- include/block/block-copy.h |  6 +--
-files changed, 44 insertions(+), 40 deletions(-)
-diff --git a/block/block-copy.c b/block/block-copy.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/block-copy.c
-+++ b/block/block-copy.c
-@@ -XXX,XX +XXX,XX @@
- static BlockCopyInFlightReq *find_conflicting_inflight_req(BlockCopyState *s,
-                                                            int64_t start,
--                                                           int64_t end)
-+                                                           int64_t bytes)
- {
-     BlockCopyInFlightReq *req;
-     QLIST_FOREACH(req, &s->inflight_reqs, list) {
--        if (end > req->start_byte && start < req->end_byte) {
-+        if (start + bytes > req->start && start < req->start + req->bytes) {
-             return req;
-         }
-     }
-@@ -XXX,XX +XXX,XX @@ static BlockCopyInFlightReq *find_conflicting_inflight_req(BlockCopyState *s,
- static void coroutine_fn block_copy_wait_inflight_reqs(BlockCopyState *s,
-                                                        int64_t start,
--                                                       int64_t end)
-+                                                       int64_t bytes)
- {
-     BlockCopyInFlightReq *req;
--    while ((req = find_conflicting_inflight_req(s, start, end))) {
-+    while ((req = find_conflicting_inflight_req(s, start, bytes))) {
-         qemu_co_queue_wait(&req->wait_queue, NULL);
-     }
- }
- static void block_copy_inflight_req_begin(BlockCopyState *s,
-                                           BlockCopyInFlightReq *req,
--                                          int64_t start, int64_t end)
-+                                          int64_t start, int64_t bytes)
- {
--    req->start_byte = start;
--    req->end_byte = end;
-+    req->start = start;
-+    req->bytes = bytes;
-     qemu_co_queue_init(&req->wait_queue);
-     QLIST_INSERT_HEAD(&s->inflight_reqs, req, list);
- }
-@@ -XXX,XX +XXX,XX @@ void block_copy_set_progress_meter(BlockCopyState *s, ProgressMeter *pm)
- /*
-  * block_copy_do_copy
-  *
-- * Do copy of cluser-aligned chunk. @end is allowed to exceed s->len only to
-- * cover last cluster when s->len is not aligned to clusters.
-+ * Do copy of cluster-aligned chunk. Requested region is allowed to exceed
-+ * s->len only to cover last cluster when s->len is not aligned to clusters.
-  *
-  * No sync here: nor bitmap neighter intersecting requests handling, only copy.
-  *
-  * Returns 0 on success.
-  */
- static int coroutine_fn block_copy_do_copy(BlockCopyState *s,
--                                           int64_t start, int64_t end,
-+                                           int64_t start, int64_t bytes,
-                                            bool zeroes, bool *error_is_read)
- {
-     int ret;
--    int nbytes = MIN(end, s->len) - start;
-+    int64_t nbytes = MIN(start + bytes, s->len) - start;
-     void *bounce_buffer = NULL;
-+    assert(start >= 0 && bytes > 0 && INT64_MAX - start >= bytes);
-     assert(QEMU_IS_ALIGNED(start, s->cluster_size));
--    assert(QEMU_IS_ALIGNED(end, s->cluster_size));
--    assert(end < s->len || end == QEMU_ALIGN_UP(s->len, s->cluster_size));
-+    assert(QEMU_IS_ALIGNED(bytes, s->cluster_size));
-+    assert(start < s->len);
-+    assert(start + bytes <= s->len ||
-+           start + bytes == QEMU_ALIGN_UP(s->len, s->cluster_size));
-+    assert(nbytes < INT_MAX);
-     if (zeroes) {
-         ret = bdrv_co_pwrite_zeroes(s->target, start, nbytes, s->write_flags &
-@@ -XXX,XX +XXX,XX @@ int64_t block_copy_reset_unallocated(BlockCopyState *s,
- }
- int coroutine_fn block_copy(BlockCopyState *s,
--                            int64_t start, uint64_t bytes,
-+                            int64_t start, int64_t bytes,
-                             bool *error_is_read)
- {
-     int ret = 0;
--    int64_t end = bytes + start; /* bytes */
-     BlockCopyInFlightReq req;
-     /*
-@@ -XXX,XX +XXX,XX @@ int coroutine_fn block_copy(BlockCopyState *s,
-            bdrv_get_aio_context(s->target->bs));
-     assert(QEMU_IS_ALIGNED(start, s->cluster_size));
--    assert(QEMU_IS_ALIGNED(end, s->cluster_size));
-+    assert(QEMU_IS_ALIGNED(bytes, s->cluster_size));
-     block_copy_wait_inflight_reqs(s, start, bytes);
--    block_copy_inflight_req_begin(s, &req, start, end);
-+    block_copy_inflight_req_begin(s, &req, start, bytes);
--    while (start < end) {
--        int64_t next_zero, chunk_end, status_bytes;
-+    while (bytes) {
-+        int64_t next_zero, cur_bytes, status_bytes;
-         if (!bdrv_dirty_bitmap_get(s->copy_bitmap, start)) {
-             trace_block_copy_skip(s, start);
-             start += s->cluster_size;
-+            bytes -= s->cluster_size;
-             continue; /* already copied */
-         }
--        chunk_end = MIN(end, start + s->copy_size);
-+        cur_bytes = MIN(bytes, s->copy_size);
-         next_zero = bdrv_dirty_bitmap_next_zero(s->copy_bitmap, start,
--                                                chunk_end - start);
-+                                                cur_bytes);
-         if (next_zero >= 0) {
-             assert(next_zero > start); /* start is dirty */
--            assert(next_zero < chunk_end); /* no need to do MIN() */
--            chunk_end = next_zero;
-+            assert(next_zero < start + cur_bytes); /* no need to do MIN() */
-+            cur_bytes = next_zero - start;
-         }
--        ret = block_copy_block_status(s, start, chunk_end - start,
--                                      &status_bytes);
-+        ret = block_copy_block_status(s, start, cur_bytes, &status_bytes);
-         if (s->skip_unallocated && !(ret & BDRV_BLOCK_ALLOCATED)) {
-             bdrv_reset_dirty_bitmap(s->copy_bitmap, start, status_bytes);
-             progress_set_remaining(s->progress,
-@@ -XXX,XX +XXX,XX @@ int coroutine_fn block_copy(BlockCopyState *s,
-                                    s->in_flight_bytes);
-             trace_block_copy_skip_range(s, start, status_bytes);
-             start += status_bytes;
-+            bytes -= status_bytes;
-             continue;
-         }
--        chunk_end = MIN(chunk_end, start + status_bytes);
-+        cur_bytes = MIN(cur_bytes, status_bytes);
-         trace_block_copy_process(s, start);
--        bdrv_reset_dirty_bitmap(s->copy_bitmap, start, chunk_end - start);
--        s->in_flight_bytes += chunk_end - start;
-+        bdrv_reset_dirty_bitmap(s->copy_bitmap, start, cur_bytes);
-+        s->in_flight_bytes += cur_bytes;
--        co_get_from_shres(s->mem, chunk_end - start);
--        ret = block_copy_do_copy(s, start, chunk_end, ret & BDRV_BLOCK_ZERO,
-+        co_get_from_shres(s->mem, cur_bytes);
-+        ret = block_copy_do_copy(s, start, cur_bytes, ret & BDRV_BLOCK_ZERO,
-                                  error_is_read);
--        co_put_to_shres(s->mem, chunk_end - start);
--        s->in_flight_bytes -= chunk_end - start;
-+        co_put_to_shres(s->mem, cur_bytes);
-+        s->in_flight_bytes -= cur_bytes;
-         if (ret < 0) {
--            bdrv_set_dirty_bitmap(s->copy_bitmap, start, chunk_end - start);
-+            bdrv_set_dirty_bitmap(s->copy_bitmap, start, cur_bytes);
-             break;
-         }
--        progress_work_done(s->progress, chunk_end - start);
--        s->progress_bytes_callback(chunk_end - start, s->progress_opaque);
--        start = chunk_end;
--        ret = 0;
-+        progress_work_done(s->progress, cur_bytes);
-+        s->progress_bytes_callback(cur_bytes, s->progress_opaque);
-+        start += cur_bytes;
-+        bytes -= cur_bytes;
-     }
-     block_copy_inflight_req_end(&req);
-diff --git a/include/block/block-copy.h b/include/block/block-copy.h
-index XXXXXXX..XXXXXXX 100644
---- a/include/block/block-copy.h
-+++ b/include/block/block-copy.h
-@@ -XXX,XX +XXX,XX @@
- #include "qemu/co-shared-resource.h"
- typedef struct BlockCopyInFlightReq {
--    int64_t start_byte;
--    int64_t end_byte;
-+    int64_t start;
-+    int64_t bytes;
-     QLIST_ENTRY(BlockCopyInFlightReq) list;
-     CoQueue wait_queue; /* coroutines blocked on this request */
- } BlockCopyInFlightReq;
-@@ -XXX,XX +XXX,XX @@ void block_copy_state_free(BlockCopyState *s);
- int64_t block_copy_reset_unallocated(BlockCopyState *s,
-                                      int64_t offset, int64_t *count);
--int coroutine_fn block_copy(BlockCopyState *s, int64_t start, uint64_t bytes,
-+int coroutine_fn block_copy(BlockCopyState *s, int64_t start, int64_t bytes,
-                             bool *error_is_read);
- #endif /* BLOCK_COPY_H */
---
-.24.1

-[PULL 17/19] block/block-copy: rename start to offset in interfaces
+Deleted patch
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-offset/bytes pair is more usual naming in block layer, let's use it.
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Message-Id: <20200311103004.7649-8-vsementsov@virtuozzo.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block/block-copy.c         | 82 +++++++++++++++++++-------------------
- include/block/block-copy.h |  4 +-
-files changed, 43 insertions(+), 43 deletions(-)
-diff --git a/block/block-copy.c b/block/block-copy.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/block-copy.c
-+++ b/block/block-copy.c
-@@ -XXX,XX +XXX,XX @@
- #define BLOCK_COPY_MAX_MEM (128 * MiB)
- static BlockCopyInFlightReq *find_conflicting_inflight_req(BlockCopyState *s,
--                                                           int64_t start,
-+                                                           int64_t offset,
-                                                            int64_t bytes)
- {
-     BlockCopyInFlightReq *req;
-     QLIST_FOREACH(req, &s->inflight_reqs, list) {
--        if (start + bytes > req->start && start < req->start + req->bytes) {
-+        if (offset + bytes > req->offset && offset < req->offset + req->bytes) {
-             return req;
-         }
-     }
-@@ -XXX,XX +XXX,XX @@ static BlockCopyInFlightReq *find_conflicting_inflight_req(BlockCopyState *s,
- }
- static void coroutine_fn block_copy_wait_inflight_reqs(BlockCopyState *s,
--                                                       int64_t start,
-+                                                       int64_t offset,
-                                                        int64_t bytes)
- {
-     BlockCopyInFlightReq *req;
--    while ((req = find_conflicting_inflight_req(s, start, bytes))) {
-+    while ((req = find_conflicting_inflight_req(s, offset, bytes))) {
-         qemu_co_queue_wait(&req->wait_queue, NULL);
-     }
- }
- static void block_copy_inflight_req_begin(BlockCopyState *s,
-                                           BlockCopyInFlightReq *req,
--                                          int64_t start, int64_t bytes)
-+                                          int64_t offset, int64_t bytes)
- {
--    req->start = start;
-+    req->offset = offset;
-     req->bytes = bytes;
-     qemu_co_queue_init(&req->wait_queue);
-     QLIST_INSERT_HEAD(&s->inflight_reqs, req, list);
-@@ -XXX,XX +XXX,XX @@ void block_copy_set_progress_meter(BlockCopyState *s, ProgressMeter *pm)
-  * Returns 0 on success.
-  */
- static int coroutine_fn block_copy_do_copy(BlockCopyState *s,
--                                           int64_t start, int64_t bytes,
-+                                           int64_t offset, int64_t bytes,
-                                            bool zeroes, bool *error_is_read)
- {
-     int ret;
--    int64_t nbytes = MIN(start + bytes, s->len) - start;
-+    int64_t nbytes = MIN(offset + bytes, s->len) - offset;
-     void *bounce_buffer = NULL;
--    assert(start >= 0 && bytes > 0 && INT64_MAX - start >= bytes);
--    assert(QEMU_IS_ALIGNED(start, s->cluster_size));
-+    assert(offset >= 0 && bytes > 0 && INT64_MAX - offset >= bytes);
-+    assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
-     assert(QEMU_IS_ALIGNED(bytes, s->cluster_size));
--    assert(start < s->len);
--    assert(start + bytes <= s->len ||
--           start + bytes == QEMU_ALIGN_UP(s->len, s->cluster_size));
-+    assert(offset < s->len);
-+    assert(offset + bytes <= s->len ||
-+           offset + bytes == QEMU_ALIGN_UP(s->len, s->cluster_size));
-     assert(nbytes < INT_MAX);
-     if (zeroes) {
--        ret = bdrv_co_pwrite_zeroes(s->target, start, nbytes, s->write_flags &
-+        ret = bdrv_co_pwrite_zeroes(s->target, offset, nbytes, s->write_flags &
-                                     ~BDRV_REQ_WRITE_COMPRESSED);
-         if (ret < 0) {
--            trace_block_copy_write_zeroes_fail(s, start, ret);
-+            trace_block_copy_write_zeroes_fail(s, offset, ret);
-             if (error_is_read) {
-                 *error_is_read = false;
-             }
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn block_copy_do_copy(BlockCopyState *s,
-     }
-     if (s->use_copy_range) {
--        ret = bdrv_co_copy_range(s->source, start, s->target, start, nbytes,
-+        ret = bdrv_co_copy_range(s->source, offset, s->target, offset, nbytes,
-, s->write_flags);
-         if (ret < 0) {
--            trace_block_copy_copy_range_fail(s, start, ret);
-+            trace_block_copy_copy_range_fail(s, offset, ret);
-             s->use_copy_range = false;
-             s->copy_size = MAX(s->cluster_size, BLOCK_COPY_MAX_BUFFER);
-             /* Fallback to read+write with allocated buffer */
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn block_copy_do_copy(BlockCopyState *s,
-     bounce_buffer = qemu_blockalign(s->source->bs, nbytes);
--    ret = bdrv_co_pread(s->source, start, nbytes, bounce_buffer, 0);
-+    ret = bdrv_co_pread(s->source, offset, nbytes, bounce_buffer, 0);
-     if (ret < 0) {
--        trace_block_copy_read_fail(s, start, ret);
-+        trace_block_copy_read_fail(s, offset, ret);
-         if (error_is_read) {
-             *error_is_read = true;
-         }
-         goto out;
-     }
--    ret = bdrv_co_pwrite(s->target, start, nbytes, bounce_buffer,
-+    ret = bdrv_co_pwrite(s->target, offset, nbytes, bounce_buffer,
-                          s->write_flags);
-     if (ret < 0) {
--        trace_block_copy_write_fail(s, start, ret);
-+        trace_block_copy_write_fail(s, offset, ret);
-         if (error_is_read) {
-             *error_is_read = false;
-         }
-@@ -XXX,XX +XXX,XX @@ int64_t block_copy_reset_unallocated(BlockCopyState *s,
- }
- int coroutine_fn block_copy(BlockCopyState *s,
--                            int64_t start, int64_t bytes,
-+                            int64_t offset, int64_t bytes,
-                             bool *error_is_read)
- {
-     int ret = 0;
-@@ -XXX,XX +XXX,XX @@ int coroutine_fn block_copy(BlockCopyState *s,
-     assert(bdrv_get_aio_context(s->source->bs) ==
-            bdrv_get_aio_context(s->target->bs));
--    assert(QEMU_IS_ALIGNED(start, s->cluster_size));
-+    assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
-     assert(QEMU_IS_ALIGNED(bytes, s->cluster_size));
--    block_copy_wait_inflight_reqs(s, start, bytes);
--    block_copy_inflight_req_begin(s, &req, start, bytes);
-+    block_copy_wait_inflight_reqs(s, offset, bytes);
-+    block_copy_inflight_req_begin(s, &req, offset, bytes);
-     while (bytes) {
-         int64_t next_zero, cur_bytes, status_bytes;
--        if (!bdrv_dirty_bitmap_get(s->copy_bitmap, start)) {
--            trace_block_copy_skip(s, start);
--            start += s->cluster_size;
-+        if (!bdrv_dirty_bitmap_get(s->copy_bitmap, offset)) {
-+            trace_block_copy_skip(s, offset);
-+            offset += s->cluster_size;
-             bytes -= s->cluster_size;
-             continue; /* already copied */
-         }
-         cur_bytes = MIN(bytes, s->copy_size);
--        next_zero = bdrv_dirty_bitmap_next_zero(s->copy_bitmap, start,
-+        next_zero = bdrv_dirty_bitmap_next_zero(s->copy_bitmap, offset,
-                                                 cur_bytes);
-         if (next_zero >= 0) {
--            assert(next_zero > start); /* start is dirty */
--            assert(next_zero < start + cur_bytes); /* no need to do MIN() */
--            cur_bytes = next_zero - start;
-+            assert(next_zero > offset); /* offset is dirty */
-+            assert(next_zero < offset + cur_bytes); /* no need to do MIN() */
-+            cur_bytes = next_zero - offset;
-         }
--        ret = block_copy_block_status(s, start, cur_bytes, &status_bytes);
-+        ret = block_copy_block_status(s, offset, cur_bytes, &status_bytes);
-         if (s->skip_unallocated && !(ret & BDRV_BLOCK_ALLOCATED)) {
--            bdrv_reset_dirty_bitmap(s->copy_bitmap, start, status_bytes);
-+            bdrv_reset_dirty_bitmap(s->copy_bitmap, offset, status_bytes);
-             progress_set_remaining(s->progress,
-                                    bdrv_get_dirty_count(s->copy_bitmap) +
-                                    s->in_flight_bytes);
--            trace_block_copy_skip_range(s, start, status_bytes);
--            start += status_bytes;
-+            trace_block_copy_skip_range(s, offset, status_bytes);
-+            offset += status_bytes;
-             bytes -= status_bytes;
-             continue;
-         }
-         cur_bytes = MIN(cur_bytes, status_bytes);
--        trace_block_copy_process(s, start);
-+        trace_block_copy_process(s, offset);
--        bdrv_reset_dirty_bitmap(s->copy_bitmap, start, cur_bytes);
-+        bdrv_reset_dirty_bitmap(s->copy_bitmap, offset, cur_bytes);
-         s->in_flight_bytes += cur_bytes;
-         co_get_from_shres(s->mem, cur_bytes);
--        ret = block_copy_do_copy(s, start, cur_bytes, ret & BDRV_BLOCK_ZERO,
-+        ret = block_copy_do_copy(s, offset, cur_bytes, ret & BDRV_BLOCK_ZERO,
-                                  error_is_read);
-         co_put_to_shres(s->mem, cur_bytes);
-         s->in_flight_bytes -= cur_bytes;
-         if (ret < 0) {
--            bdrv_set_dirty_bitmap(s->copy_bitmap, start, cur_bytes);
-+            bdrv_set_dirty_bitmap(s->copy_bitmap, offset, cur_bytes);
-             break;
-         }
-         progress_work_done(s->progress, cur_bytes);
-         s->progress_bytes_callback(cur_bytes, s->progress_opaque);
--        start += cur_bytes;
-+        offset += cur_bytes;
-         bytes -= cur_bytes;
-     }
-diff --git a/include/block/block-copy.h b/include/block/block-copy.h
-index XXXXXXX..XXXXXXX 100644
---- a/include/block/block-copy.h
-+++ b/include/block/block-copy.h
-@@ -XXX,XX +XXX,XX @@
- #include "qemu/co-shared-resource.h"
- typedef struct BlockCopyInFlightReq {
--    int64_t start;
-+    int64_t offset;
-     int64_t bytes;
-     QLIST_ENTRY(BlockCopyInFlightReq) list;
-     CoQueue wait_queue; /* coroutines blocked on this request */
-@@ -XXX,XX +XXX,XX @@ void block_copy_state_free(BlockCopyState *s);
- int64_t block_copy_reset_unallocated(BlockCopyState *s,
-                                      int64_t offset, int64_t *count);
--int coroutine_fn block_copy(BlockCopyState *s, int64_t start, int64_t bytes,
-+int coroutine_fn block_copy(BlockCopyState *s, int64_t offset, int64_t bytes,
-                             bool *error_is_read);
- #endif /* BLOCK_COPY_H */
---
-.24.1

-[PULL 18/19] block/block-copy: reduce intersecting request lock
+Deleted patch
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Currently, block_copy operation lock the whole requested region. But
-there is no reason to lock clusters, which are already copied, it will
-disturb other parallel block_copy requests for no reason.
-Let's instead do the following:
-Lock only sub-region, which we are going to operate on. Then, after
-copying all dirty sub-regions, we should wait for intersecting
-requests block-copy, if they failed, we should retry these new dirty
-clusters.
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
-Message-Id: <20200311103004.7649-9-vsementsov@virtuozzo.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block/block-copy.c | 129 ++++++++++++++++++++++++++++++++++++---------
-file changed, 105 insertions(+), 24 deletions(-)
-diff --git a/block/block-copy.c b/block/block-copy.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/block-copy.c
-+++ b/block/block-copy.c
-@@ -XXX,XX +XXX,XX @@ static BlockCopyInFlightReq *find_conflicting_inflight_req(BlockCopyState *s,
-     return NULL;
- }
--static void coroutine_fn block_copy_wait_inflight_reqs(BlockCopyState *s,
--                                                       int64_t offset,
--                                                       int64_t bytes)
-+/*
-+ * If there are no intersecting requests return false. Otherwise, wait for the
-+ * first found intersecting request to finish and return true.
-+ */
-+static bool coroutine_fn block_copy_wait_one(BlockCopyState *s, int64_t offset,
-+                                             int64_t bytes)
- {
--    BlockCopyInFlightReq *req;
-+    BlockCopyInFlightReq *req = find_conflicting_inflight_req(s, offset, bytes);
--    while ((req = find_conflicting_inflight_req(s, offset, bytes))) {
--        qemu_co_queue_wait(&req->wait_queue, NULL);
-+    if (!req) {
-+        return false;
-     }
-+
-+    qemu_co_queue_wait(&req->wait_queue, NULL);
-+
-+    return true;
- }
-+/* Called only on full-dirty region */
- static void block_copy_inflight_req_begin(BlockCopyState *s,
-                                           BlockCopyInFlightReq *req,
-                                           int64_t offset, int64_t bytes)
- {
-+    assert(!find_conflicting_inflight_req(s, offset, bytes));
-+
-+    bdrv_reset_dirty_bitmap(s->copy_bitmap, offset, bytes);
-+    s->in_flight_bytes += bytes;
-+
-     req->offset = offset;
-     req->bytes = bytes;
-     qemu_co_queue_init(&req->wait_queue);
-     QLIST_INSERT_HEAD(&s->inflight_reqs, req, list);
- }
--static void coroutine_fn block_copy_inflight_req_end(BlockCopyInFlightReq *req)
-+/*
-+ * block_copy_inflight_req_shrink
-+ *
-+ * Drop the tail of the request to be handled later. Set dirty bits back and
-+ * wake up all requests waiting for us (may be some of them are not intersecting
-+ * with shrunk request)
-+ */
-+static void coroutine_fn block_copy_inflight_req_shrink(BlockCopyState *s,
-+        BlockCopyInFlightReq *req, int64_t new_bytes)
- {
-+    if (new_bytes == req->bytes) {
-+        return;
-+    }
-+
-+    assert(new_bytes > 0 && new_bytes < req->bytes);
-+
-+    s->in_flight_bytes -= req->bytes - new_bytes;
-+    bdrv_set_dirty_bitmap(s->copy_bitmap,
-+                          req->offset + new_bytes, req->bytes - new_bytes);
-+
-+    req->bytes = new_bytes;
-+    qemu_co_queue_restart_all(&req->wait_queue);
-+}
-+
-+static void coroutine_fn block_copy_inflight_req_end(BlockCopyState *s,
-+                                                     BlockCopyInFlightReq *req,
-+                                                     int ret)
-+{
-+    s->in_flight_bytes -= req->bytes;
-+    if (ret < 0) {
-+        bdrv_set_dirty_bitmap(s->copy_bitmap, req->offset, req->bytes);
-+    }
-     QLIST_REMOVE(req, list);
-     qemu_co_queue_restart_all(&req->wait_queue);
- }
-@@ -XXX,XX +XXX,XX @@ int64_t block_copy_reset_unallocated(BlockCopyState *s,
-     return ret;
- }
--int coroutine_fn block_copy(BlockCopyState *s,
--                            int64_t offset, int64_t bytes,
--                            bool *error_is_read)
-+/*
-+ * block_copy_dirty_clusters
-+ *
-+ * Copy dirty clusters in @offset/@bytes range.
-+ * Returns 1 if dirty clusters found and successfully copied, 0 if no dirty
-+ * clusters found and -errno on failure.
-+ */
-+static int coroutine_fn block_copy_dirty_clusters(BlockCopyState *s,
-+                                                  int64_t offset, int64_t bytes,
-+                                                  bool *error_is_read)
- {
-     int ret = 0;
--    BlockCopyInFlightReq req;
-+    bool found_dirty = false;
-     /*
-      * block_copy() user is responsible for keeping source and target in same
-@@ -XXX,XX +XXX,XX @@ int coroutine_fn block_copy(BlockCopyState *s,
-     assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
-     assert(QEMU_IS_ALIGNED(bytes, s->cluster_size));
--    block_copy_wait_inflight_reqs(s, offset, bytes);
--    block_copy_inflight_req_begin(s, &req, offset, bytes);
--
-     while (bytes) {
-+        BlockCopyInFlightReq req;
-         int64_t next_zero, cur_bytes, status_bytes;
-         if (!bdrv_dirty_bitmap_get(s->copy_bitmap, offset)) {
-@@ -XXX,XX +XXX,XX @@ int coroutine_fn block_copy(BlockCopyState *s,
-             continue; /* already copied */
-         }
-+        found_dirty = true;
-+
-         cur_bytes = MIN(bytes, s->copy_size);
-         next_zero = bdrv_dirty_bitmap_next_zero(s->copy_bitmap, offset,
-@@ -XXX,XX +XXX,XX @@ int coroutine_fn block_copy(BlockCopyState *s,
-             assert(next_zero < offset + cur_bytes); /* no need to do MIN() */
-             cur_bytes = next_zero - offset;
-         }
-+        block_copy_inflight_req_begin(s, &req, offset, cur_bytes);
-         ret = block_copy_block_status(s, offset, cur_bytes, &status_bytes);
-+        assert(ret >= 0); /* never fail */
-+        cur_bytes = MIN(cur_bytes, status_bytes);
-+        block_copy_inflight_req_shrink(s, &req, cur_bytes);
-         if (s->skip_unallocated && !(ret & BDRV_BLOCK_ALLOCATED)) {
--            bdrv_reset_dirty_bitmap(s->copy_bitmap, offset, status_bytes);
-+            block_copy_inflight_req_end(s, &req, 0);
-             progress_set_remaining(s->progress,
-                                    bdrv_get_dirty_count(s->copy_bitmap) +
-                                    s->in_flight_bytes);
-@@ -XXX,XX +XXX,XX @@ int coroutine_fn block_copy(BlockCopyState *s,
-             continue;
-         }
--        cur_bytes = MIN(cur_bytes, status_bytes);
--
-         trace_block_copy_process(s, offset);
--        bdrv_reset_dirty_bitmap(s->copy_bitmap, offset, cur_bytes);
--        s->in_flight_bytes += cur_bytes;
--
-         co_get_from_shres(s->mem, cur_bytes);
-         ret = block_copy_do_copy(s, offset, cur_bytes, ret & BDRV_BLOCK_ZERO,
-                                  error_is_read);
-         co_put_to_shres(s->mem, cur_bytes);
--        s->in_flight_bytes -= cur_bytes;
-+        block_copy_inflight_req_end(s, &req, ret);
-         if (ret < 0) {
--            bdrv_set_dirty_bitmap(s->copy_bitmap, offset, cur_bytes);
--            break;
-+            return ret;
-         }
-         progress_work_done(s->progress, cur_bytes);
-@@ -XXX,XX +XXX,XX @@ int coroutine_fn block_copy(BlockCopyState *s,
-         bytes -= cur_bytes;
-     }
--    block_copy_inflight_req_end(&req);
-+    return found_dirty;
-+}
-+
-+/*
-+ * block_copy
-+ *
-+ * Copy requested region, accordingly to dirty bitmap.
-+ * Collaborate with parallel block_copy requests: if they succeed it will help
-+ * us. If they fail, we will retry not-copied regions. So, if we return error,
-+ * it means that some I/O operation failed in context of _this_ block_copy call,
-+ * not some parallel operation.
-+ */
-+int coroutine_fn block_copy(BlockCopyState *s, int64_t offset, int64_t bytes,
-+                            bool *error_is_read)
-+{
-+    int ret;
-+
-+    do {
-+        ret = block_copy_dirty_clusters(s, offset, bytes, error_is_read);
-+
-+        if (ret == 0) {
-+            ret = block_copy_wait_one(s, offset, bytes);
-+        }
-+
-+        /*
-+         * We retry in two cases:
-+         * 1. Some progress done
-+         *    Something was copied, which means that there were yield points
-+         *    and some new dirty bits may have appeared (due to failed parallel
-+         *    block-copy requests).
-+         * 2. We have waited for some intersecting block-copy request
-+         *    It may have failed and produced new dirty bits.
-+         */
-+    } while (ret > 0);
-     return ret;
- }
---
-.24.1

-[PULL 19/19] block/block-copy: hide structure definitions
+Deleted patch
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Hide structure definitions and add explicit API instead, to keep an
-eye on the scope of the shared fields.
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Message-Id: <20200311103004.7649-10-vsementsov@virtuozzo.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block/backup-top.c         |  6 ++--
- block/backup.c             | 25 ++++++++--------
- block/block-copy.c         | 59 ++++++++++++++++++++++++++++++++++++++
- include/block/block-copy.h | 52 +++------------------------------
-files changed, 80 insertions(+), 62 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 @@ typedef struct BDRVBackupTopState {
-     BlockCopyState *bcs;
-     BdrvChild *target;
-     bool active;
-+    int64_t cluster_size;
- } BDRVBackupTopState;
- static coroutine_fn int backup_top_co_preadv(
-@@ -XXX,XX +XXX,XX @@ static coroutine_fn int backup_top_cbw(BlockDriverState *bs, uint64_t offset,
-         return 0;
-     }
--    off = QEMU_ALIGN_DOWN(offset, s->bcs->cluster_size);
--    end = QEMU_ALIGN_UP(offset + bytes, s->bcs->cluster_size);
-+    off = QEMU_ALIGN_DOWN(offset, s->cluster_size);
-+    end = QEMU_ALIGN_UP(offset + bytes, s->cluster_size);
-     return block_copy(s->bcs, off, end - off, NULL);
- }
-@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
-         goto fail;
-     }
-+    state->cluster_size = cluster_size;
-     state->bcs = block_copy_state_new(top->backing, state->target,
-                                       cluster_size, write_flags, &local_err);
-     if (local_err) {
-diff --git a/block/backup.c b/block/backup.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/backup.c
-+++ b/block/backup.c
-@@ -XXX,XX +XXX,XX @@ static void backup_cleanup_sync_bitmap(BackupBlockJob *job, int ret)
-     if (ret < 0 && job->bitmap_mode == BITMAP_SYNC_MODE_ALWAYS) {
-         /* If we failed and synced, merge in the bits we didn't copy: */
--        bdrv_dirty_bitmap_merge_internal(bm, job->bcs->copy_bitmap,
-+        bdrv_dirty_bitmap_merge_internal(bm, block_copy_dirty_bitmap(job->bcs),
-                                          NULL, true);
-     }
- }
-@@ -XXX,XX +XXX,XX @@ void backup_do_checkpoint(BlockJob *job, Error **errp)
-         return;
-     }
--    bdrv_set_dirty_bitmap(backup_job->bcs->copy_bitmap, 0, backup_job->len);
-+    bdrv_set_dirty_bitmap(block_copy_dirty_bitmap(backup_job->bcs), 0,
-+                          backup_job->len);
- }
- static BlockErrorAction backup_error_action(BackupBlockJob *job,
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_loop(BackupBlockJob *job)
-     BdrvDirtyBitmapIter *bdbi;
-     int ret = 0;
--    bdbi = bdrv_dirty_iter_new(job->bcs->copy_bitmap);
-+    bdbi = bdrv_dirty_iter_new(block_copy_dirty_bitmap(job->bcs));
-     while ((offset = bdrv_dirty_iter_next(bdbi)) != -1) {
-         do {
-             if (yield_and_check(job)) {
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_loop(BackupBlockJob *job)
-     return ret;
- }
--static void backup_init_copy_bitmap(BackupBlockJob *job)
-+static void backup_init_bcs_bitmap(BackupBlockJob *job)
- {
-     bool ret;
-     uint64_t estimate;
-+    BdrvDirtyBitmap *bcs_bitmap = block_copy_dirty_bitmap(job->bcs);
-     if (job->sync_mode == MIRROR_SYNC_MODE_BITMAP) {
--        ret = bdrv_dirty_bitmap_merge_internal(job->bcs->copy_bitmap,
--                                               job->sync_bitmap,
-+        ret = bdrv_dirty_bitmap_merge_internal(bcs_bitmap, job->sync_bitmap,
-                                                NULL, true);
-         assert(ret);
-     } else {
-@@ -XXX,XX +XXX,XX @@ static void backup_init_copy_bitmap(BackupBlockJob *job)
-              * We can't hog the coroutine to initialize this thoroughly.
-              * Set a flag and resume work when we are able to yield safely.
-              */
--            job->bcs->skip_unallocated = true;
-+            block_copy_set_skip_unallocated(job->bcs, true);
-         }
--        bdrv_set_dirty_bitmap(job->bcs->copy_bitmap, 0, job->len);
-+        bdrv_set_dirty_bitmap(bcs_bitmap, 0, job->len);
-     }
--    estimate = bdrv_get_dirty_count(job->bcs->copy_bitmap);
-+    estimate = bdrv_get_dirty_count(bcs_bitmap);
-     job_progress_set_remaining(&job->common.job, estimate);
- }
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_run(Job *job, Error **errp)
-     BackupBlockJob *s = container_of(job, BackupBlockJob, common.job);
-     int ret = 0;
--    backup_init_copy_bitmap(s);
-+    backup_init_bcs_bitmap(s);
-     if (s->sync_mode == MIRROR_SYNC_MODE_TOP) {
-         int64_t offset = 0;
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_run(Job *job, Error **errp)
-             offset += count;
-         }
--        s->bcs->skip_unallocated = false;
-+        block_copy_set_skip_unallocated(s->bcs, false);
-     }
-     if (s->sync_mode == MIRROR_SYNC_MODE_NONE) {
-         /*
--         * All bits are set in copy_bitmap to allow any cluster to be copied.
-+         * All bits are set in bcs bitmap to allow any cluster to be copied.
-          * This does not actually require them to be copied.
-          */
-         while (!job_is_cancelled(job)) {
-diff --git a/block/block-copy.c b/block/block-copy.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/block-copy.c
-+++ b/block/block-copy.c
-@@ -XXX,XX +XXX,XX @@
- #define BLOCK_COPY_MAX_BUFFER (1 * MiB)
- #define BLOCK_COPY_MAX_MEM (128 * MiB)
-+typedef struct BlockCopyInFlightReq {
-+    int64_t offset;
-+    int64_t bytes;
-+    QLIST_ENTRY(BlockCopyInFlightReq) list;
-+    CoQueue wait_queue; /* coroutines blocked on this request */
-+} BlockCopyInFlightReq;
-+
-+typedef struct BlockCopyState {
-+    /*
-+     * BdrvChild objects are not owned or managed by block-copy. They are
-+     * provided by block-copy user and user is responsible for appropriate
-+     * permissions on these children.
-+     */
-+    BdrvChild *source;
-+    BdrvChild *target;
-+    BdrvDirtyBitmap *copy_bitmap;
-+    int64_t in_flight_bytes;
-+    int64_t cluster_size;
-+    bool use_copy_range;
-+    int64_t copy_size;
-+    uint64_t len;
-+    QLIST_HEAD(, BlockCopyInFlightReq) inflight_reqs;
-+
-+    BdrvRequestFlags write_flags;
-+
-+    /*
-+     * skip_unallocated:
-+     *
-+     * Used by sync=top jobs, which first scan the source node for unallocated
-+     * areas and clear them in the copy_bitmap.  During this process, the bitmap
-+     * is thus not fully initialized: It may still have bits set for areas that
-+     * are unallocated and should actually not be copied.
-+     *
-+     * This is indicated by skip_unallocated.
-+     *
-+     * In this case, block_copy() will query the source’s allocation status,
-+     * skip unallocated regions, clear them in the copy_bitmap, and invoke
-+     * block_copy_reset_unallocated() every time it does.
-+     */
-+    bool skip_unallocated;
-+
-+    ProgressMeter *progress;
-+    /* progress_bytes_callback: called when some copying progress is done. */
-+    ProgressBytesCallbackFunc progress_bytes_callback;
-+    void *progress_opaque;
-+
-+    SharedResource *mem;
-+} BlockCopyState;
-+
- static BlockCopyInFlightReq *find_conflicting_inflight_req(BlockCopyState *s,
-                                                            int64_t offset,
-                                                            int64_t bytes)
-@@ -XXX,XX +XXX,XX @@ int coroutine_fn block_copy(BlockCopyState *s, int64_t offset, int64_t bytes,
-     return ret;
- }
-+
-+BdrvDirtyBitmap *block_copy_dirty_bitmap(BlockCopyState *s)
-+{
-+    return s->copy_bitmap;
-+}
-+
-+void block_copy_set_skip_unallocated(BlockCopyState *s, bool skip)
-+{
-+    s->skip_unallocated = skip;
-+}
-diff --git a/include/block/block-copy.h b/include/block/block-copy.h
-index XXXXXXX..XXXXXXX 100644
---- a/include/block/block-copy.h
-+++ b/include/block/block-copy.h
-@@ -XXX,XX +XXX,XX @@
- #include "block/block.h"
- #include "qemu/co-shared-resource.h"
--typedef struct BlockCopyInFlightReq {
--    int64_t offset;
--    int64_t bytes;
--    QLIST_ENTRY(BlockCopyInFlightReq) list;
--    CoQueue wait_queue; /* coroutines blocked on this request */
--} BlockCopyInFlightReq;
--
- typedef void (*ProgressBytesCallbackFunc)(int64_t bytes, void *opaque);
--typedef struct BlockCopyState {
--    /*
--     * BdrvChild objects are not owned or managed by block-copy. They are
--     * provided by block-copy user and user is responsible for appropriate
--     * permissions on these children.
--     */
--    BdrvChild *source;
--    BdrvChild *target;
--    BdrvDirtyBitmap *copy_bitmap;
--    int64_t in_flight_bytes;
--    int64_t cluster_size;
--    bool use_copy_range;
--    int64_t copy_size;
--    uint64_t len;
--    QLIST_HEAD(, BlockCopyInFlightReq) inflight_reqs;
--
--    BdrvRequestFlags write_flags;
--
--    /*
--     * skip_unallocated:
--     *
--     * Used by sync=top jobs, which first scan the source node for unallocated
--     * areas and clear them in the copy_bitmap.  During this process, the bitmap
--     * is thus not fully initialized: It may still have bits set for areas that
--     * are unallocated and should actually not be copied.
--     *
--     * This is indicated by skip_unallocated.
--     *
--     * In this case, block_copy() will query the source’s allocation status,
--     * skip unallocated regions, clear them in the copy_bitmap, and invoke
--     * block_copy_reset_unallocated() every time it does.
--     */
--    bool skip_unallocated;
--
--    ProgressMeter *progress;
--    /* progress_bytes_callback: called when some copying progress is done. */
--    ProgressBytesCallbackFunc progress_bytes_callback;
--    void *progress_opaque;
--
--    SharedResource *mem;
--} BlockCopyState;
-+typedef struct BlockCopyState BlockCopyState;
- BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
-                                      int64_t cluster_size,
-@@ -XXX,XX +XXX,XX @@ int64_t block_copy_reset_unallocated(BlockCopyState *s,
- int coroutine_fn block_copy(BlockCopyState *s, int64_t offset, int64_t bytes,
-                             bool *error_is_read);
-+BdrvDirtyBitmap *block_copy_dirty_bitmap(BlockCopyState *s);
-+void block_copy_set_skip_unallocated(BlockCopyState *s, bool skip);
-+
- #endif /* BLOCK_COPY_H */
---
-.24.1

The following changes since commit ba29883206d92a29ad5a466e679ccfc2ee6132ef:

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

are available in the Git repository at:

https://github.com/XanClic/qemu.git tags/pull-block-2020-03-11

for you to fetch changes up to 397f4e9d83e9c0000905f0a988ba1aeda162571c:

block/block-copy: hide structure definitions (2020-03-11 12:42:30 +0100)

----------------------------------------------------------------
Block patches for the 5.0 softfreeze:
- qemu-img measure for LUKS
- Improve block-copy's performance by reducing inter-request
  dependencies
- Make curl's detection of accept-ranges more robust
- Memleak fixes
- iotest fix

----------------------------------------------------------------
David Edmondson (2):
  block/curl: HTTP header fields allow whitespace around values
  block/curl: HTTP header field names are case insensitive

Eric Blake (1):
  iotests: Fix nonportable use of od --endian

Pan Nengyuan (2):
  block/qcow2: do free crypto_opts in qcow2_close()
  qemu-img: free memory before re-assign

Stefan Hajnoczi (4):
  luks: extract qcrypto_block_calculate_payload_offset()
  luks: implement .bdrv_measure()
  qemu-img: allow qemu-img measure --object without a filename
  iotests: add 288 luks qemu-img measure test

Vladimir Sementsov-Ogievskiy (10):
  block/qcow2-threads: fix qcow2_decompress
  job: refactor progress to separate object
  block/block-copy: fix progress calculation
  block/block-copy: specialcase first copy_range request
  block/block-copy: use block_status
  block/block-copy: factor out find_conflicting_inflight_req
  block/block-copy: refactor interfaces to use bytes instead of end
  block/block-copy: rename start to offset in interfaces
  block/block-copy: reduce intersecting request lock
  block/block-copy: hide structure definitions

-- 
2.24.1

From: Stefan Hajnoczi <stefanha@redhat.com>

The qcow2 .bdrv_measure() code calculates the crypto payload offset.
This logic really belongs in crypto/block.c where it can be reused by
other image formats.

The "luks" block driver will need this same logic in order to implement
.bdrv_measure(), so extract the qcrypto_block_calculate_payload_offset()
function now.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200221112522.1497712-2-stefanha@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.c          | 74 +++++++++++-------------------------------
 crypto/block.c         | 36 ++++++++++++++++++++
 include/crypto/block.h | 22 +++++++++++++
 3 files changed, 77 insertions(+), 55 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_flush_to_os(BlockDriverState *bs)
     return ret;
 }
 
-static ssize_t qcow2_measure_crypto_hdr_init_func(QCryptoBlock *block,
-        size_t headerlen, void *opaque, Error **errp)
-{
-    size_t *headerlenp = opaque;
-
-    /* Stash away the payload size */
-    *headerlenp = headerlen;
-    return 0;
-}
-
-static ssize_t qcow2_measure_crypto_hdr_write_func(QCryptoBlock *block,
-        size_t offset, const uint8_t *buf, size_t buflen,
-        void *opaque, Error **errp)
-{
-    /* Discard the bytes, we're not actually writing to an image */
-    return buflen;
-}
-
-/* Determine the number of bytes for the LUKS payload */
-static bool qcow2_measure_luks_headerlen(QemuOpts *opts, size_t *len,
-                                         Error **errp)
-{
-    QDict *opts_qdict;
-    QDict *cryptoopts_qdict;
-    QCryptoBlockCreateOptions *cryptoopts;
-    QCryptoBlock *crypto;
-
-    /* Extract "encrypt." options into a qdict */
-    opts_qdict = qemu_opts_to_qdict(opts, NULL);
-    qdict_extract_subqdict(opts_qdict, &cryptoopts_qdict, "encrypt.");
-    qobject_unref(opts_qdict);
-
-    /* Build QCryptoBlockCreateOptions object from qdict */
-    qdict_put_str(cryptoopts_qdict, "format", "luks");
-    cryptoopts = block_crypto_create_opts_init(cryptoopts_qdict, errp);
-    qobject_unref(cryptoopts_qdict);
-    if (!cryptoopts) {
-        return false;
-    }
-
-    /* Fake LUKS creation in order to determine the payload size */
-    crypto = qcrypto_block_create(cryptoopts, "encrypt.",
-                                  qcow2_measure_crypto_hdr_init_func,
-                                  qcow2_measure_crypto_hdr_write_func,
-                                  len, errp);
-    qapi_free_QCryptoBlockCreateOptions(cryptoopts);
-    if (!crypto) {
-        return false;
-    }
-
-    qcrypto_block_free(crypto);
-    return true;
-}
-
 static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
                                        Error **errp)
 {
@@ -XXX,XX +XXX,XX @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
     g_free(optstr);
 
     if (has_luks) {
+        g_autoptr(QCryptoBlockCreateOptions) create_opts = NULL;
+        QDict *opts_qdict;
+        QDict *cryptoopts;
         size_t headerlen;
 
-        if (!qcow2_measure_luks_headerlen(opts, &headerlen, &local_err)) {
+        opts_qdict = qemu_opts_to_qdict(opts, NULL);
+        qdict_extract_subqdict(opts_qdict, &cryptoopts, "encrypt.");
+        qobject_unref(opts_qdict);
+
+        qdict_put_str(cryptoopts, "format", "luks");
+
+        create_opts = block_crypto_create_opts_init(cryptoopts, errp);
+        qobject_unref(cryptoopts);
+        if (!create_opts) {
+            goto err;
+        }
+
+        if (!qcrypto_block_calculate_payload_offset(create_opts,
+                                                    "encrypt.",
+                                                    &headerlen,
+                                                    &local_err)) {
             goto err;
         }
 
diff --git a/crypto/block.c b/crypto/block.c
index XXXXXXX..XXXXXXX 100644
--- a/crypto/block.c
+++ b/crypto/block.c
@@ -XXX,XX +XXX,XX @@ QCryptoBlock *qcrypto_block_create(QCryptoBlockCreateOptions *options,
 }
 
 
+static ssize_t qcrypto_block_headerlen_hdr_init_func(QCryptoBlock *block,
+        size_t headerlen, void *opaque, Error **errp)
+{
+    size_t *headerlenp = opaque;
+
+    /* Stash away the payload size */
+    *headerlenp = headerlen;
+    return 0;
+}
+
+
+static ssize_t qcrypto_block_headerlen_hdr_write_func(QCryptoBlock *block,
+        size_t offset, const uint8_t *buf, size_t buflen,
+        void *opaque, Error **errp)
+{
+    /* Discard the bytes, we're not actually writing to an image */
+    return buflen;
+}
+
+
+bool
+qcrypto_block_calculate_payload_offset(QCryptoBlockCreateOptions *create_opts,
+                                       const char *optprefix,
+                                       size_t *len,
+                                       Error **errp)
+{
+    /* Fake LUKS creation in order to determine the payload size */
+    g_autoptr(QCryptoBlock) crypto =
+        qcrypto_block_create(create_opts, optprefix,
+                             qcrypto_block_headerlen_hdr_init_func,
+                             qcrypto_block_headerlen_hdr_write_func,
+                             len, errp);
+    return crypto != NULL;
+}
+
+
 QCryptoBlockInfo *qcrypto_block_get_info(QCryptoBlock *block,
                                          Error **errp)
 {
diff --git a/include/crypto/block.h b/include/crypto/block.h
index XXXXXXX..XXXXXXX 100644
--- a/include/crypto/block.h
+++ b/include/crypto/block.h
@@ -XXX,XX +XXX,XX @@ QCryptoBlock *qcrypto_block_create(QCryptoBlockCreateOptions *options,
                                    Error **errp);
 
 
+/**
+ * qcrypto_block_calculate_payload_offset:
+ * @create_opts: the encryption options
+ * @optprefix: name prefix for options
+ * @len: output for number of header bytes before payload
+ * @errp: pointer to a NULL-initialized error object
+ *
+ * Calculate the number of header bytes before the payload in an encrypted
+ * storage volume.  The header is an area before the payload that is reserved
+ * for encryption metadata.
+ *
+ * Returns: true on success, false on error
+ */
+bool
+qcrypto_block_calculate_payload_offset(QCryptoBlockCreateOptions *create_opts,
+                                       const char *optprefix,
+                                       size_t *len,
+                                       Error **errp);
+
+
 /**
  * qcrypto_block_get_info:
  * @block: the block encryption object
@@ -XXX,XX +XXX,XX @@ uint64_t qcrypto_block_get_sector_size(QCryptoBlock *block);
 void qcrypto_block_free(QCryptoBlock *block);
 
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(QCryptoBlock, qcrypto_block_free)
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(QCryptoBlockCreateOptions,
+                              qapi_free_QCryptoBlockCreateOptions)
 
 #endif /* QCRYPTO_BLOCK_H */
-- 
2.24.1

From: Stefan Hajnoczi <stefanha@redhat.com>

Add qemu-img measure support in the "luks" block driver.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200221112522.1497712-3-stefanha@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/crypto.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/block/crypto.c b/block/crypto.c
index XXXXXXX..XXXXXXX 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -XXX,XX +XXX,XX @@ static int64_t block_crypto_getlength(BlockDriverState *bs)
 }
 
 
+static BlockMeasureInfo *block_crypto_measure(QemuOpts *opts,
+                                              BlockDriverState *in_bs,
+                                              Error **errp)
+{
+    g_autoptr(QCryptoBlockCreateOptions) create_opts = NULL;
+    Error *local_err = NULL;
+    BlockMeasureInfo *info;
+    uint64_t size;
+    size_t luks_payload_size;
+    QDict *cryptoopts;
+
+    /*
+     * Preallocation mode doesn't affect size requirements but we must consume
+     * the option.
+     */
+    g_free(qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC));
+
+    size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
+
+    if (in_bs) {
+        int64_t ssize = bdrv_getlength(in_bs);
+
+        if (ssize < 0) {
+            error_setg_errno(&local_err, -ssize,
+                             "Unable to get image virtual_size");
+            goto err;
+        }
+
+        size = ssize;
+    }
+
+    cryptoopts = qemu_opts_to_qdict_filtered(opts, NULL,
+            &block_crypto_create_opts_luks, true);
+    qdict_put_str(cryptoopts, "format", "luks");
+    create_opts = block_crypto_create_opts_init(cryptoopts, &local_err);
+    qobject_unref(cryptoopts);
+    if (!create_opts) {
+        goto err;
+    }
+
+    if (!qcrypto_block_calculate_payload_offset(create_opts, NULL,
+                                                &luks_payload_size,
+                                                &local_err)) {
+        goto err;
+    }
+
+    /*
+     * Unallocated blocks are still encrypted so allocation status makes no
+     * difference to the file size.
+     */
+    info = g_new(BlockMeasureInfo, 1);
+    info->fully_allocated = luks_payload_size + size;
+    info->required = luks_payload_size + size;
+    return info;
+
+err:
+    error_propagate(errp, local_err);
+    return NULL;
+}
+
+
 static int block_crypto_probe_luks(const uint8_t *buf,
                                    int buf_size,
                                    const char *filename) {
@@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_crypto_luks = {
     .bdrv_co_preadv     = block_crypto_co_preadv,
     .bdrv_co_pwritev    = block_crypto_co_pwritev,
     .bdrv_getlength     = block_crypto_getlength,
+    .bdrv_measure       = block_crypto_measure,
     .bdrv_get_info      = block_crypto_get_info_luks,
     .bdrv_get_specific_info = block_crypto_get_specific_info_luks,
 
-- 
2.24.1

From: Stefan Hajnoczi <stefanha@redhat.com>

In most qemu-img sub-commands the --object option only makes sense when
there is a filename.  qemu-img measure is an exception because objects
may be referenced from the image creation options instead of an existing
image file.  Allow --object without a filename.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200221112522.1497712-4-stefanha@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qemu-img.c                       | 6 ++----
 tests/qemu-iotests/178           | 2 +-
 tests/qemu-iotests/178.out.qcow2 | 8 ++++----
 tests/qemu-iotests/178.out.raw   | 8 ++++----
 4 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index XXXXXXX..XXXXXXX 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -XXX,XX +XXX,XX @@ static int img_measure(int argc, char **argv)
         filename = argv[optind];
     }
 
-    if (!filename &&
-        (object_opts || image_opts || fmt || snapshot_name || sn_opts)) {
-        error_report("--object, --image-opts, -f, and -l "
-                     "require a filename argument.");
+    if (!filename && (image_opts || fmt || snapshot_name || sn_opts)) {
+        error_report("--image-opts, -f, and -l require a filename argument.");
         goto out;
     }
     if (filename && img_size != UINT64_MAX) {
diff --git a/tests/qemu-iotests/178 b/tests/qemu-iotests/178
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/178
+++ b/tests/qemu-iotests/178
@@ -XXX,XX +XXX,XX @@ _make_test_img 1G
 $QEMU_IMG measure # missing arguments
 $QEMU_IMG measure --size 2G "$TEST_IMG" # only one allowed
 $QEMU_IMG measure "$TEST_IMG" a # only one filename allowed
-$QEMU_IMG measure --object secret,id=sec0,data=MTIzNDU2,format=base64 # missing filename
+$QEMU_IMG measure --object secret,id=sec0,data=MTIzNDU2,format=base64 # size or filename needed
 $QEMU_IMG measure --image-opts # missing filename
 $QEMU_IMG measure -f qcow2 # missing filename
 $QEMU_IMG measure -l snap1 # missing filename
diff --git a/tests/qemu-iotests/178.out.qcow2 b/tests/qemu-iotests/178.out.qcow2
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/178.out.qcow2
+++ b/tests/qemu-iotests/178.out.qcow2
@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 qemu-img: Either --size N or one filename must be specified.
 qemu-img: --size N cannot be used together with a filename.
 qemu-img: At most one filename argument is allowed.
-qemu-img: --object, --image-opts, -f, and -l require a filename argument.
-qemu-img: --object, --image-opts, -f, and -l require a filename argument.
-qemu-img: --object, --image-opts, -f, and -l require a filename argument.
-qemu-img: --object, --image-opts, -f, and -l require a filename argument.
+qemu-img: Either --size N or one filename must be specified.
+qemu-img: --image-opts, -f, and -l require a filename argument.
+qemu-img: --image-opts, -f, and -l require a filename argument.
+qemu-img: --image-opts, -f, and -l require a filename argument.
 qemu-img: Invalid option list: ,
 qemu-img: Invalid parameter 'snapshot.foo'
 qemu-img: Failed in parsing snapshot param 'snapshot.foo'
diff --git a/tests/qemu-iotests/178.out.raw b/tests/qemu-iotests/178.out.raw
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/178.out.raw
+++ b/tests/qemu-iotests/178.out.raw
@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 qemu-img: Either --size N or one filename must be specified.
 qemu-img: --size N cannot be used together with a filename.
 qemu-img: At most one filename argument is allowed.
-qemu-img: --object, --image-opts, -f, and -l require a filename argument.
-qemu-img: --object, --image-opts, -f, and -l require a filename argument.
-qemu-img: --object, --image-opts, -f, and -l require a filename argument.
-qemu-img: --object, --image-opts, -f, and -l require a filename argument.
+qemu-img: Either --size N or one filename must be specified.
+qemu-img: --image-opts, -f, and -l require a filename argument.
+qemu-img: --image-opts, -f, and -l require a filename argument.
+qemu-img: --image-opts, -f, and -l require a filename argument.
 qemu-img: Invalid option list: ,
 qemu-img: Invalid parameter 'snapshot.foo'
 qemu-img: Failed in parsing snapshot param 'snapshot.foo'
-- 
2.24.1

From: Stefan Hajnoczi <stefanha@redhat.com>

This test exercises the block/crypto.c "luks" block driver
.bdrv_measure() code.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200221112522.1497712-5-stefanha@redhat.com>
[mreitz: Renamed test from 282 to 288]
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/288     | 93 ++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/288.out | 30 ++++++++++++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 124 insertions(+)
 create mode 100755 tests/qemu-iotests/288
 create mode 100644 tests/qemu-iotests/288.out

diff --git a/tests/qemu-iotests/288 b/tests/qemu-iotests/288
new file mode 100755
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qemu-iotests/288
@@ -XXX,XX +XXX,XX @@
+#!/usr/bin/env bash
+#
+# qemu-img measure tests for LUKS images
+#
+# Copyright (C) 2020 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=stefanha@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+status=1    # failure is the default!
+
+_cleanup()
+{
+    _cleanup_test_img
+    rm -f "$TEST_IMG.converted"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.pattern
+
+_supported_fmt luks
+_supported_proto file
+_supported_os Linux
+
+SECRET=secret,id=sec0,data=passphrase
+
+echo "== measure 1G image file =="
+echo
+
+$QEMU_IMG measure --object "$SECRET" \
+	          -O "$IMGFMT" \
+		  -o key-secret=sec0,iter-time=10 \
+		  --size 1G
+
+echo
+echo "== create 1G image file (size should be no greater than measured) =="
+echo
+
+_make_test_img 1G
+stat -c "image file size in bytes: %s" "$TEST_IMG_FILE"
+
+echo
+echo "== modified 1G image file (size should be no greater than measured) =="
+echo
+
+$QEMU_IO --object "$SECRET" --image-opts "$TEST_IMG" -c "write -P 0x51 0x10000 0x400" | _filter_qemu_io | _filter_testdir
+stat -c "image file size in bytes: %s" "$TEST_IMG_FILE"
+
+echo
+echo "== measure preallocation=falloc 1G image file =="
+echo
+
+$QEMU_IMG measure --object "$SECRET" \
+	          -O "$IMGFMT" \
+		  -o key-secret=sec0,iter-time=10,preallocation=falloc \
+		  --size 1G
+
+echo
+echo "== measure with input image file =="
+echo
+
+IMGFMT=raw IMGKEYSECRET= IMGOPTS= _make_test_img 1G | _filter_imgfmt
+QEMU_IO_OPTIONS= IMGOPTSSYNTAX= $QEMU_IO -f raw -c "write -P 0x51 0x10000 0x400" "$TEST_IMG_FILE" | _filter_qemu_io | _filter_testdir
+$QEMU_IMG measure --object "$SECRET" \
+	          -O "$IMGFMT" \
+		  -o key-secret=sec0,iter-time=10 \
+		  -f raw \
+		  "$TEST_IMG_FILE"
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/288.out b/tests/qemu-iotests/288.out
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qemu-iotests/288.out
@@ -XXX,XX +XXX,XX @@
+QA output created by 288
+== measure 1G image file ==
+
+required size: 1075810304
+fully allocated size: 1075810304
+
+== create 1G image file (size should be no greater than measured) ==
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
+image file size in bytes: 1075810304
+
+== modified 1G image file (size should be no greater than measured) ==
+
+wrote 1024/1024 bytes at offset 65536
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+image file size in bytes: 1075810304
+
+== measure preallocation=falloc 1G image file ==
+
+required size: 1075810304
+fully allocated size: 1075810304
+
+== measure with input image file ==
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
+wrote 1024/1024 bytes at offset 65536
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+required size: 1075810304
+fully allocated size: 1075810304
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -XXX,XX +XXX,XX @@
 283 auto quick
 284 rw
 286 rw quick
+288 quick
-- 
2.24.1

From: David Edmondson <david.edmondson@oracle.com>

RFC 7230 section 3.2 indicates that whitespace is permitted between
the field name and field value and after the field value.

Signed-off-by: David Edmondson <david.edmondson@oracle.com>
Message-Id: <20200224101310.101169-2-david.edmondson@oracle.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/curl.c | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index XXXXXXX..XXXXXXX 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -XXX,XX +XXX,XX @@ static size_t curl_header_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
 {
     BDRVCURLState *s = opaque;
     size_t realsize = size * nmemb;
-    const char *accept_line = "Accept-Ranges: bytes";
+    const char *header = (char *)ptr;
+    const char *end = header + realsize;
+    const char *accept_ranges = "Accept-Ranges:";
+    const char *bytes = "bytes";
 
-    if (realsize >= strlen(accept_line)
-        && strncmp((char *)ptr, accept_line, strlen(accept_line)) == 0) {
-        s->accept_range = true;
+    if (realsize >= strlen(accept_ranges)
+        && strncmp(header, accept_ranges, strlen(accept_ranges)) == 0) {
+
+        char *p = strchr(header, ':') + 1;
+
+        /* Skip whitespace between the header name and value. */
+        while (p < end && *p && g_ascii_isspace(*p)) {
+            p++;
+        }
+
+        if (end - p >= strlen(bytes)
+            && strncmp(p, bytes, strlen(bytes)) == 0) {
+
+            /* Check that there is nothing but whitespace after the value. */
+            p += strlen(bytes);
+            while (p < end && *p && g_ascii_isspace(*p)) {
+                p++;
+            }
+
+            if (p == end || !*p) {
+                s->accept_range = true;
+            }
+        }
     }
 
     return realsize;
-- 
2.24.1

From: David Edmondson <david.edmondson@oracle.com>

RFC 7230 section 3.2 indicates that HTTP header field names are case
insensitive.

Signed-off-by: David Edmondson <david.edmondson@oracle.com>
Message-Id: <20200224101310.101169-3-david.edmondson@oracle.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/curl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index XXXXXXX..XXXXXXX 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -XXX,XX +XXX,XX @@ static size_t curl_header_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
     size_t realsize = size * nmemb;
     const char *header = (char *)ptr;
     const char *end = header + realsize;
-    const char *accept_ranges = "Accept-Ranges:";
+    const char *accept_ranges = "accept-ranges:";
     const char *bytes = "bytes";
 
     if (realsize >= strlen(accept_ranges)
-        && strncmp(header, accept_ranges, strlen(accept_ranges)) == 0) {
+        && g_ascii_strncasecmp(header, accept_ranges,
+                               strlen(accept_ranges)) == 0) {
 
         char *p = strchr(header, ':') + 1;
 
-- 
2.24.1

From: Eric Blake <eblake@redhat.com>

Tests 261 and 272 fail on RHEL 7 with coreutils 8.22, since od
--endian was not added until coreutils 8.23.  Fix this by manually
constructing the final value one byte at a time.

Fixes: fc8ba423
Reported-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200226125424.481840-1-eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/common.rc | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

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 @@ poke_file()
 # peek_file_le 'test.img' 512 2 => 65534
 peek_file_le()
 {
-    # Wrap in echo $() to strip spaces
-    echo $(od -j"$2" -N"$3" --endian=little -An -vtu"$3" "$1")
+    local val=0 shift=0 byte
+
+    # coreutils' od --endian is not portable, so manually assemble bytes.
+    for byte in $(od -j"$2" -N"$3" -An -v -tu1 "$1"); do
+        val=$(( val | (byte << shift) ))
+        shift=$((shift + 8))
+    done
+    printf %llu $val
 }
 
 # peek_file_be 'test.img' 512 2 => 65279
 peek_file_be()
 {
-    # Wrap in echo $() to strip spaces
-    echo $(od -j"$2" -N"$3" --endian=big -An -vtu"$3" "$1")
+    local val=0 byte
+
+    # coreutils' od --endian is not portable, so manually assemble bytes.
+    for byte in $(od -j"$2" -N"$3" -An -v -tu1 "$1"); do
+        val=$(( (val << 8) | byte ))
+    done
+    printf %llu $val
 }
 
-# peek_file_raw 'test.img' 512 2 => '\xff\xfe'
+# peek_file_raw 'test.img' 512 2 => '\xff\xfe'. Do not use if the raw data
+# is likely to contain \0 or trailing \n.
 peek_file_raw()
 {
     dd if="$1" bs=1 skip="$2" count="$3" status=none
-- 
2.24.1

From: Pan Nengyuan <pannengyuan@huawei.com>

'crypto_opts' forgot to free in qcow2_close(), this patch fix the bellow leak stack:

Direct leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x7f0edd81f970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
    #1 0x7f0edc6d149d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
    #2 0x55d7eaede63d in qobject_input_start_struct /mnt/sdb/qemu-new/qemu_test/qemu/qapi/qobject-input-visitor.c:295
    #3 0x55d7eaed78b8 in visit_start_struct /mnt/sdb/qemu-new/qemu_test/qemu/qapi/qapi-visit-core.c:49
    #4 0x55d7eaf5140b in visit_type_QCryptoBlockOpenOptions qapi/qapi-visit-crypto.c:290
    #5 0x55d7eae43af3 in block_crypto_open_opts_init /mnt/sdb/qemu-new/qemu_test/qemu/block/crypto.c:163
    #6 0x55d7eacd2924 in qcow2_update_options_prepare /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1148
    #7 0x55d7eacd33f7 in qcow2_update_options /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1232
    #8 0x55d7eacd9680 in qcow2_do_open /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1512
    #9 0x55d7eacdc55e in qcow2_open_entry /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1792
    #10 0x55d7eacdc8fe in qcow2_open /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:1819
    #11 0x55d7eac3742d in bdrv_open_driver /mnt/sdb/qemu-new/qemu_test/qemu/block.c:1317
    #12 0x55d7eac3e990 in bdrv_open_common /mnt/sdb/qemu-new/qemu_test/qemu/block.c:1575
    #13 0x55d7eac4442c in bdrv_open_inherit /mnt/sdb/qemu-new/qemu_test/qemu/block.c:3126
    #14 0x55d7eac45c3f in bdrv_open /mnt/sdb/qemu-new/qemu_test/qemu/block.c:3219
    #15 0x55d7ead8e8a4 in blk_new_open /mnt/sdb/qemu-new/qemu_test/qemu/block/block-backend.c:397
    #16 0x55d7eacde74c in qcow2_co_create /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:3534
    #17 0x55d7eacdfa6d in qcow2_co_create_opts /mnt/sdb/qemu-new/qemu_test/qemu/block/qcow2.c:3668
    #18 0x55d7eac1c678 in bdrv_create_co_entry /mnt/sdb/qemu-new/qemu_test/qemu/block.c:485
    #19 0x55d7eb0024d2 in coroutine_trampoline /mnt/sdb/qemu-new/qemu_test/qemu/util/coroutine-ucontext.c:115

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200227012950.12256-2-pannengyuan@huawei.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.c | 1 +
 1 file changed, 1 insertion(+)

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 qcow2_close(BlockDriverState *bs)
 
     qcrypto_block_free(s->crypto);
     s->crypto = NULL;
+    qapi_free_QCryptoBlockOpenOptions(s->crypto_opts);
 
     g_free(s->unknown_header_fields);
     cleanup_unknown_header_ext(bs);
-- 
2.24.1

From: Pan Nengyuan <pannengyuan@huawei.com>

collect_image_check() is called twice in img_check(), the filename/format will be alloced without free the original memory.
It is not a big deal since the process will exit anyway, but seems like a clean code and it will remove the warning spotted by asan.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
Message-Id: <20200227012950.12256-3-pannengyuan@huawei.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qemu-img.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/qemu-img.c b/qemu-img.c
index XXXXXXX..XXXXXXX 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -XXX,XX +XXX,XX @@ static int img_check(int argc, char **argv)
                     check->corruptions_fixed);
         }
 
+        qapi_free_ImageCheck(check);
+        check = g_new0(ImageCheck, 1);
         ret = collect_image_check(bs, check, filename, fmt, 0);
 
         check->leaks_fixed          = leaks_fixed;
-- 
2.24.1

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

On success path we return what inflate() returns instead of 0. And it
most probably works for Z_STREAM_END as it is positive, but is
definitely broken for Z_BUF_ERROR.

While being here, switch to errno return code, to be closer to
qcow2_compress API (and usual expectations).

Revert condition in if to be more positive. Drop dead initialization of
ret.

Cc: qemu-stable@nongnu.org # v4.0
Fixes: 341926ab83e2b
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200302150930.16218-1-vsementsov@virtuozzo.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2-threads.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/block/qcow2-threads.c b/block/qcow2-threads.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2-threads.c
+++ b/block/qcow2-threads.c
@@ -XXX,XX +XXX,XX @@ static ssize_t qcow2_compress(void *dest, size_t dest_size,
  * @src - source buffer, @src_size bytes
  *
  * Returns: 0 on success
- *          -1 on fail
+ *          -EIO on fail
  */
 static ssize_t qcow2_decompress(void *dest, size_t dest_size,
                                 const void *src, size_t src_size)
 {
-    int ret = 0;
+    int ret;
     z_stream strm;
 
     memset(&strm, 0, sizeof(strm));
@@ -XXX,XX +XXX,XX @@ static ssize_t qcow2_decompress(void *dest, size_t dest_size,
 
     ret = inflateInit2(&strm, -12);
     if (ret != Z_OK) {
-        return -1;
+        return -EIO;
     }
 
     ret = inflate(&strm, Z_FINISH);
-    if ((ret != Z_STREAM_END && ret != Z_BUF_ERROR) || strm.avail_out != 0) {
+    if ((ret == Z_STREAM_END || ret == Z_BUF_ERROR) && strm.avail_out == 0) {
         /*
          * We approve Z_BUF_ERROR because we need @dest buffer to be filled, but
          * @src buffer may be processed partly (because in qcow2 we know size of
          * compressed data with precision of one sector)
          */
-        ret = -1;
+        ret = 0;
+    } else {
+        ret = -EIO;
     }
 
     inflateEnd(&strm);
-- 
2.24.1

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

We need it in separate to pass to the block-copy object in the next
commit.

Cc: qemu-stable@nongnu.org
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200311103004.7649-2-vsementsov@virtuozzo.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 blockjob.c                    | 16 +++++-----
 include/qemu/job.h            | 11 ++-----
 include/qemu/progress_meter.h | 58 +++++++++++++++++++++++++++++++++++
 job-qmp.c                     |  4 +--
 job.c                         |  6 ++--
 qemu-img.c                    |  6 ++--
 6 files changed, 76 insertions(+), 25 deletions(-)
 create mode 100644 include/qemu/progress_meter.h

diff --git a/blockjob.c b/blockjob.c
index XXXXXXX..XXXXXXX 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -XXX,XX +XXX,XX @@ BlockJobInfo *block_job_query(BlockJob *job, Error **errp)
     info->device    = g_strdup(job->job.id);
     info->busy      = atomic_read(&job->job.busy);
     info->paused    = job->job.pause_count > 0;
-    info->offset    = job->job.progress_current;
-    info->len       = job->job.progress_total;
+    info->offset    = job->job.progress.current;
+    info->len       = job->job.progress.total;
     info->speed     = job->speed;
     info->io_status = job->iostatus;
     info->ready     = job_is_ready(&job->job),
@@ -XXX,XX +XXX,XX @@ static void block_job_event_cancelled(Notifier *n, void *opaque)
 
     qapi_event_send_block_job_cancelled(job_type(&job->job),
                                         job->job.id,
-                                        job->job.progress_total,
-                                        job->job.progress_current,
+                                        job->job.progress.total,
+                                        job->job.progress.current,
                                         job->speed);
 }
 
@@ -XXX,XX +XXX,XX @@ static void block_job_event_completed(Notifier *n, void *opaque)
 
     qapi_event_send_block_job_completed(job_type(&job->job),
                                         job->job.id,
-                                        job->job.progress_total,
-                                        job->job.progress_current,
+                                        job->job.progress.total,
+                                        job->job.progress.current,
                                         job->speed,
                                         !!msg,
                                         msg);
@@ -XXX,XX +XXX,XX @@ static void block_job_event_ready(Notifier *n, void *opaque)
 
     qapi_event_send_block_job_ready(job_type(&job->job),
                                     job->job.id,
-                                    job->job.progress_total,
-                                    job->job.progress_current,
+                                    job->job.progress.total,
+                                    job->job.progress.current,
                                     job->speed);
 }
 
diff --git a/include/qemu/job.h b/include/qemu/job.h
index XXXXXXX..XXXXXXX 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -XXX,XX +XXX,XX @@
 
 #include "qapi/qapi-types-job.h"
 #include "qemu/queue.h"
+#include "qemu/progress_meter.h"
 #include "qemu/coroutine.h"
 #include "block/aio.h"
 
@@ -XXX,XX +XXX,XX @@ typedef struct Job {
     /** True if this job should automatically dismiss itself */
     bool auto_dismiss;
 
-    /**
-     * Current progress. The unit is arbitrary as long as the ratio between
-     * progress_current and progress_total represents the estimated percentage
-     * of work already done.
-     */
-    int64_t progress_current;
-
-    /** Estimated progress_current value at the completion of the job */
-    int64_t progress_total;
+    ProgressMeter progress;
 
     /**
      * Return code from @run and/or @prepare callback(s).
diff --git a/include/qemu/progress_meter.h b/include/qemu/progress_meter.h
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/include/qemu/progress_meter.h
@@ -XXX,XX +XXX,XX @@
+/*
+ * Helper functionality for some process progress tracking.
+ *
+ * Copyright (c) 2011 IBM Corp.
+ * Copyright (c) 2012, 2018 Red Hat, Inc.
+ * Copyright (c) 2020 Virtuozzo International GmbH
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef QEMU_PROGRESS_METER_H
+#define QEMU_PROGRESS_METER_H
+
+typedef struct ProgressMeter {
+    /**
+     * Current progress. The unit is arbitrary as long as the ratio between
+     * current and total represents the estimated percentage
+     * of work already done.
+     */
+    uint64_t current;
+
+    /** Estimated current value at the completion of the process */
+    uint64_t total;
+} ProgressMeter;
+
+static inline void progress_work_done(ProgressMeter *pm, uint64_t done)
+{
+    pm->current += done;
+}
+
+static inline void progress_set_remaining(ProgressMeter *pm, uint64_t remaining)
+{
+    pm->total = pm->current + remaining;
+}
+
+static inline void progress_increase_remaining(ProgressMeter *pm,
+                                               uint64_t delta)
+{
+    pm->total += delta;
+}
+
+#endif /* QEMU_PROGRESS_METER_H */
diff --git a/job-qmp.c b/job-qmp.c
index XXXXXXX..XXXXXXX 100644
--- a/job-qmp.c
+++ b/job-qmp.c
@@ -XXX,XX +XXX,XX @@ static JobInfo *job_query_single(Job *job, Error **errp)
         .id                 = g_strdup(job->id),
         .type               = job_type(job),
         .status             = job->status,
-        .current_progress   = job->progress_current,
-        .total_progress     = job->progress_total,
+        .current_progress   = job->progress.current,
+        .total_progress     = job->progress.total,
         .has_error          = !!job->err,
         .error              = job->err ? \
                               g_strdup(error_get_pretty(job->err)) : NULL,
diff --git a/job.c b/job.c
index XXXXXXX..XXXXXXX 100644
--- a/job.c
+++ b/job.c
@@ -XXX,XX +XXX,XX @@ void job_unref(Job *job)
 
 void job_progress_update(Job *job, uint64_t done)
 {
-    job->progress_current += done;
+    progress_work_done(&job->progress, done);
 }
 
 void job_progress_set_remaining(Job *job, uint64_t remaining)
 {
-    job->progress_total = job->progress_current + remaining;
+    progress_set_remaining(&job->progress, remaining);
 }
 
 void job_progress_increase_remaining(Job *job, uint64_t delta)
 {
-    job->progress_total += delta;
+    progress_increase_remaining(&job->progress, delta);
 }
 
 void job_event_cancelled(Job *job)
diff --git a/qemu-img.c b/qemu-img.c
index XXXXXXX..XXXXXXX 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -XXX,XX +XXX,XX @@ static void run_block_job(BlockJob *job, Error **errp)
     do {
         float progress = 0.0f;
         aio_poll(aio_context, true);
-        if (job->job.progress_total) {
-            progress = (float)job->job.progress_current /
-                       job->job.progress_total * 100.f;
+        if (job->job.progress.total) {
+            progress = (float)job->job.progress.current /
+                       job->job.progress.total * 100.f;
         }
         qemu_progress_print(progress, 0);
     } while (!job_is_ready(&job->job) && !job_is_completed(&job->job));
-- 
2.24.1

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

Assume we have two regions, A and B, and region B is in-flight now,
region A is not yet touched, but it is unallocated and should be
skipped.

Correspondingly, as progress we have

total = A + B
  current = 0

If we reset unallocated region A and call progress_reset_callback,
it will calculate 0 bytes dirty in the bitmap and call
job_progress_set_remaining, which will set

total = current + 0 = 0 + 0 = 0

So, B bytes are actually removed from total accounting. When job
finishes we'll have

total = 0
   current = B

, which doesn't sound good.

This is because we didn't considered in-flight bytes, actually when
calculating remaining, we should have set (in_flight + dirty_bytes)
as remaining, not only dirty_bytes.

To fix it, let's refactor progress calculation, moving it to block-copy
itself instead of fixing callback. And, of course, track in_flight
bytes count.

We still have to keep one callback, to maintain backup job bytes_read
calculation, but it will go on soon, when we turn the whole backup
process into one block_copy call.

Cc: qemu-stable@nongnu.org
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Message-Id: <20200311103004.7649-3-vsementsov@virtuozzo.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/backup.c             | 13 ++-----------
 block/block-copy.c         | 16 ++++++++++++----
 include/block/block-copy.h | 15 +++++----------
 3 files changed, 19 insertions(+), 25 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index XXXXXXX..XXXXXXX 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -XXX,XX +XXX,XX @@ static void backup_progress_bytes_callback(int64_t bytes, void *opaque)
     BackupBlockJob *s = opaque;
 
     s->bytes_read += bytes;
-    job_progress_update(&s->common.job, bytes);
-}
-
-static void backup_progress_reset_callback(void *opaque)
-{
-    BackupBlockJob *s = opaque;
-    uint64_t estimate = bdrv_get_dirty_count(s->bcs->copy_bitmap);
-
-    job_progress_set_remaining(&s->common.job, estimate);
 }
 
 static int coroutine_fn backup_do_cow(BackupBlockJob *job,
@@ -XXX,XX +XXX,XX @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
     job->cluster_size = cluster_size;
     job->len = len;
 
-    block_copy_set_callbacks(bcs, backup_progress_bytes_callback,
-                             backup_progress_reset_callback, job);
+    block_copy_set_progress_callback(bcs, backup_progress_bytes_callback, job);
+    block_copy_set_progress_meter(bcs, &job->common.job.progress);
 
     /* Required permissions are already taken by backup-top target */
     block_job_add_bdrv(&job->common, "target", target, 0, BLK_PERM_ALL,
diff --git a/block/block-copy.c b/block/block-copy.c
index XXXXXXX..XXXXXXX 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -XXX,XX +XXX,XX @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
     return s;
 }
 
-void block_copy_set_callbacks(
+void block_copy_set_progress_callback(
         BlockCopyState *s,
         ProgressBytesCallbackFunc progress_bytes_callback,
-        ProgressResetCallbackFunc progress_reset_callback,
         void *progress_opaque)
 {
     s->progress_bytes_callback = progress_bytes_callback;
-    s->progress_reset_callback = progress_reset_callback;
     s->progress_opaque = progress_opaque;
 }
 
+void block_copy_set_progress_meter(BlockCopyState *s, ProgressMeter *pm)
+{
+    s->progress = pm;
+}
+
 /*
  * block_copy_do_copy
  *
@@ -XXX,XX +XXX,XX @@ int64_t block_copy_reset_unallocated(BlockCopyState *s,
 
     if (!ret) {
         bdrv_reset_dirty_bitmap(s->copy_bitmap, offset, bytes);
-        s->progress_reset_callback(s->progress_opaque);
+        progress_set_remaining(s->progress,
+                               bdrv_get_dirty_count(s->copy_bitmap) +
+                               s->in_flight_bytes);
     }
 
     *count = bytes;
@@ -XXX,XX +XXX,XX @@ int coroutine_fn block_copy(BlockCopyState *s,
         trace_block_copy_process(s, start);
 
         bdrv_reset_dirty_bitmap(s->copy_bitmap, start, chunk_end - start);
+        s->in_flight_bytes += chunk_end - start;
 
         co_get_from_shres(s->mem, chunk_end - start);
         ret = block_copy_do_copy(s, start, chunk_end, error_is_read);
         co_put_to_shres(s->mem, chunk_end - start);
+        s->in_flight_bytes -= chunk_end - start;
         if (ret < 0) {
             bdrv_set_dirty_bitmap(s->copy_bitmap, start, chunk_end - start);
             break;
         }
 
+        progress_work_done(s->progress, chunk_end - start);
         s->progress_bytes_callback(chunk_end - start, s->progress_opaque);
         start = chunk_end;
         ret = 0;
diff --git a/include/block/block-copy.h b/include/block/block-copy.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/block-copy.h
+++ b/include/block/block-copy.h
@@ -XXX,XX +XXX,XX @@ typedef struct BlockCopyInFlightReq {
 } BlockCopyInFlightReq;
 
 typedef void (*ProgressBytesCallbackFunc)(int64_t bytes, void *opaque);
-typedef void (*ProgressResetCallbackFunc)(void *opaque);
 typedef struct BlockCopyState {
     /*
      * BdrvChild objects are not owned or managed by block-copy. They are
@@ -XXX,XX +XXX,XX @@ typedef struct BlockCopyState {
     BdrvChild *source;
     BdrvChild *target;
     BdrvDirtyBitmap *copy_bitmap;
+    int64_t in_flight_bytes;
     int64_t cluster_size;
     bool use_copy_range;
     int64_t copy_size;
@@ -XXX,XX +XXX,XX @@ typedef struct BlockCopyState {
      */
     bool skip_unallocated;
 
+    ProgressMeter *progress;
     /* progress_bytes_callback: called when some copying progress is done. */
     ProgressBytesCallbackFunc progress_bytes_callback;
-
-    /*
-     * progress_reset_callback: called when some bytes reset from copy_bitmap
-     * (see @skip_unallocated above). The callee is assumed to recalculate how
-     * many bytes remain based on the dirty bit count of copy_bitmap.
-     */
-    ProgressResetCallbackFunc progress_reset_callback;
     void *progress_opaque;
 
     SharedResource *mem;
@@ -XXX,XX +XXX,XX @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
                                      BdrvRequestFlags write_flags,
                                      Error **errp);
 
-void block_copy_set_callbacks(
+void block_copy_set_progress_callback(
         BlockCopyState *s,
         ProgressBytesCallbackFunc progress_bytes_callback,
-        ProgressResetCallbackFunc progress_reset_callback,
         void *progress_opaque);
 
+void block_copy_set_progress_meter(BlockCopyState *s, ProgressMeter *pm);
+
 void block_copy_state_free(BlockCopyState *s);
 
 int64_t block_copy_reset_unallocated(BlockCopyState *s,
-- 
2.24.1

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

In block_copy_do_copy we fallback to read+write if copy_range failed.
In this case copy_size is larger than defined for buffered IO, and
there is corresponding commit. Still, backup copies data cluster by
cluster, and most of requests are limited to one cluster anyway, so the
only source of this one bad-limited request is copy-before-write
operation.

Further patch will move backup to use block_copy directly, than for
cases where copy_range is not supported, first request will be
oversized in each backup. It's not good, let's change it now.

Fix is simple: just limit first copy_range request like buffer-based
request. If it succeed, set larger copy_range limit.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200311103004.7649-4-vsementsov@virtuozzo.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/block-copy.c | 41 +++++++++++++++++++++++++++++++----------
 1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/block/block-copy.c b/block/block-copy.c
index XXXXXXX..XXXXXXX 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -XXX,XX +XXX,XX @@ void block_copy_state_free(BlockCopyState *s)
     g_free(s);
 }
 
+static uint32_t block_copy_max_transfer(BdrvChild *source, BdrvChild *target)
+{
+    return MIN_NON_ZERO(INT_MAX,
+                        MIN_NON_ZERO(source->bs->bl.max_transfer,
+                                     target->bs->bl.max_transfer));
+}
+
 BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
                                      int64_t cluster_size,
                                      BdrvRequestFlags write_flags, Error **errp)
 {
     BlockCopyState *s;
     BdrvDirtyBitmap *copy_bitmap;
-    uint32_t max_transfer =
-            MIN_NON_ZERO(INT_MAX,
-                         MIN_NON_ZERO(source->bs->bl.max_transfer,
-                                      target->bs->bl.max_transfer));
 
     copy_bitmap = bdrv_create_dirty_bitmap(source->bs, cluster_size, NULL,
                                            errp);
@@ -XXX,XX +XXX,XX @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
         .mem = shres_create(BLOCK_COPY_MAX_MEM),
     };
 
-    if (max_transfer < cluster_size) {
+    if (block_copy_max_transfer(source, target) < cluster_size) {
         /*
          * copy_range does not respect max_transfer. We don't want to bother
          * with requests smaller than block-copy cluster size, so fallback to
@@ -XXX,XX +XXX,XX @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
         s->copy_size = cluster_size;
     } else {
         /*
-         * copy_range does not respect max_transfer (it's a TODO), so we factor
-         * that in here.
+         * We enable copy-range, but keep small copy_size, until first
+         * successful copy_range (look at block_copy_do_copy).
          */
         s->use_copy_range = true;
-        s->copy_size = MIN(MAX(cluster_size, BLOCK_COPY_MAX_COPY_RANGE),
-                           QEMU_ALIGN_DOWN(max_transfer, cluster_size));
+        s->copy_size = MAX(s->cluster_size, BLOCK_COPY_MAX_BUFFER);
     }
 
     QLIST_INIT(&s->inflight_reqs);
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn block_copy_do_copy(BlockCopyState *s,
             s->copy_size = MAX(s->cluster_size, BLOCK_COPY_MAX_BUFFER);
             /* Fallback to read+write with allocated buffer */
         } else {
+            if (s->use_copy_range) {
+                /*
+                 * Successful copy-range. Now increase copy_size.  copy_range
+                 * does not respect max_transfer (it's a TODO), so we factor
+                 * that in here.
+                 *
+                 * Note: we double-check s->use_copy_range for the case when
+                 * parallel block-copy request unsets it during previous
+                 * bdrv_co_copy_range call.
+                 */
+                s->copy_size =
+                        MIN(MAX(s->cluster_size, BLOCK_COPY_MAX_COPY_RANGE),
+                            QEMU_ALIGN_DOWN(block_copy_max_transfer(s->source,
+                                                                    s->target),
+                                            s->cluster_size));
+            }
             goto out;
         }
     }
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn block_copy_do_copy(BlockCopyState *s,
     /*
      * In case of failed copy_range request above, we may proceed with buffered
      * request larger than BLOCK_COPY_MAX_BUFFER. Still, further requests will
-     * be properly limited, so don't care too much.
+     * be properly limited, so don't care too much. Moreover the most likely
+     * case (copy_range is unsupported for the configuration, so the very first
+     * copy_range request fails) is handled by setting large copy_size only
+     * after first successful copy_range.
      */
 
     bounce_buffer = qemu_blockalign(s->source->bs, nbytes);
-- 
2.24.1

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

Use bdrv_block_status_above to chose effective chunk size and to handle
zeroes effectively.

This substitutes checking for just being allocated or not, and drops
old code path for it. Assistance by backup job is dropped too, as
caching block-status information is more difficult than just caching
is-allocated information in our dirty bitmap, and backup job is not
good place for this caching anyway.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200311103004.7649-5-vsementsov@virtuozzo.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/block-copy.c | 73 +++++++++++++++++++++++++++++++++++++---------
 block/trace-events |  1 +
 2 files changed, 61 insertions(+), 13 deletions(-)

diff --git a/block/block-copy.c b/block/block-copy.c
index XXXXXXX..XXXXXXX 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -XXX,XX +XXX,XX @@ void block_copy_set_progress_meter(BlockCopyState *s, ProgressMeter *pm)
  */
 static int coroutine_fn block_copy_do_copy(BlockCopyState *s,
                                            int64_t start, int64_t end,
-                                           bool *error_is_read)
+                                           bool zeroes, bool *error_is_read)
 {
     int ret;
     int nbytes = MIN(end, s->len) - start;
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn block_copy_do_copy(BlockCopyState *s,
     assert(QEMU_IS_ALIGNED(end, s->cluster_size));
     assert(end < s->len || end == QEMU_ALIGN_UP(s->len, s->cluster_size));
 
+    if (zeroes) {
+        ret = bdrv_co_pwrite_zeroes(s->target, start, nbytes, s->write_flags &
+                                    ~BDRV_REQ_WRITE_COMPRESSED);
+        if (ret < 0) {
+            trace_block_copy_write_zeroes_fail(s, start, ret);
+            if (error_is_read) {
+                *error_is_read = false;
+            }
+        }
+        return ret;
+    }
+
     if (s->use_copy_range) {
         ret = bdrv_co_copy_range(s->source, start, s->target, start, nbytes,
                                  0, s->write_flags);
@@ -XXX,XX +XXX,XX @@ out:
     return ret;
 }
 
+static int block_copy_block_status(BlockCopyState *s, int64_t offset,
+                                   int64_t bytes, int64_t *pnum)
+{
+    int64_t num;
+    BlockDriverState *base;
+    int ret;
+
+    if (s->skip_unallocated && s->source->bs->backing) {
+        base = s->source->bs->backing->bs;
+    } else {
+        base = NULL;
+    }
+
+    ret = bdrv_block_status_above(s->source->bs, base, offset, bytes, &num,
+                                  NULL, NULL);
+    if (ret < 0 || num < s->cluster_size) {
+        /*
+         * On error or if failed to obtain large enough chunk just fallback to
+         * copy one cluster.
+         */
+        num = s->cluster_size;
+        ret = BDRV_BLOCK_ALLOCATED | BDRV_BLOCK_DATA;
+    } else if (offset + num == s->len) {
+        num = QEMU_ALIGN_UP(num, s->cluster_size);
+    } else {
+        num = QEMU_ALIGN_DOWN(num, s->cluster_size);
+    }
+
+    *pnum = num;
+    return ret;
+}
+
 /*
  * Check if the cluster starting at offset is allocated or not.
  * return via pnum the number of contiguous clusters sharing this allocation.
@@ -XXX,XX +XXX,XX @@ int coroutine_fn block_copy(BlockCopyState *s,
 {
     int ret = 0;
     int64_t end = bytes + start; /* bytes */
-    int64_t status_bytes;
     BlockCopyInFlightReq req;
 
     /*
@@ -XXX,XX +XXX,XX @@ int coroutine_fn block_copy(BlockCopyState *s,
     block_copy_inflight_req_begin(s, &req, start, end);
 
     while (start < end) {
-        int64_t next_zero, chunk_end;
+        int64_t next_zero, chunk_end, status_bytes;
 
         if (!bdrv_dirty_bitmap_get(s->copy_bitmap, start)) {
             trace_block_copy_skip(s, start);
@@ -XXX,XX +XXX,XX @@ int coroutine_fn block_copy(BlockCopyState *s,
             chunk_end = next_zero;
         }
 
-        if (s->skip_unallocated) {
-            ret = block_copy_reset_unallocated(s, start, &status_bytes);
-            if (ret == 0) {
-                trace_block_copy_skip_range(s, start, status_bytes);
-                start += status_bytes;
-                continue;
-            }
-            /* Clamp to known allocated region */
-            chunk_end = MIN(chunk_end, start + status_bytes);
+        ret = block_copy_block_status(s, start, chunk_end - start,
+                                      &status_bytes);
+        if (s->skip_unallocated && !(ret & BDRV_BLOCK_ALLOCATED)) {
+            bdrv_reset_dirty_bitmap(s->copy_bitmap, start, status_bytes);
+            progress_set_remaining(s->progress,
+                                   bdrv_get_dirty_count(s->copy_bitmap) +
+                                   s->in_flight_bytes);
+            trace_block_copy_skip_range(s, start, status_bytes);
+            start += status_bytes;
+            continue;
         }
 
+        chunk_end = MIN(chunk_end, start + status_bytes);
+
         trace_block_copy_process(s, start);
 
         bdrv_reset_dirty_bitmap(s->copy_bitmap, start, chunk_end - start);
         s->in_flight_bytes += chunk_end - start;
 
         co_get_from_shres(s->mem, chunk_end - start);
-        ret = block_copy_do_copy(s, start, chunk_end, error_is_read);
+        ret = block_copy_do_copy(s, start, chunk_end, ret & BDRV_BLOCK_ZERO,
+                                 error_is_read);
         co_put_to_shres(s->mem, chunk_end - start);
         s->in_flight_bytes -= chunk_end - start;
         if (ret < 0) {
diff --git a/block/trace-events b/block/trace-events
index XXXXXXX..XXXXXXX 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -XXX,XX +XXX,XX @@ block_copy_process(void *bcs, int64_t start) "bcs %p start %"PRId64
 block_copy_copy_range_fail(void *bcs, int64_t start, int ret) "bcs %p start %"PRId64" ret %d"
 block_copy_read_fail(void *bcs, int64_t start, int ret) "bcs %p start %"PRId64" ret %d"
 block_copy_write_fail(void *bcs, int64_t start, int ret) "bcs %p start %"PRId64" ret %d"
+block_copy_write_zeroes_fail(void *bcs, int64_t start, int ret) "bcs %p start %"PRId64" ret %d"
 
 # ../blockdev.c
 qmp_block_job_cancel(void *job) "job %p"
-- 
2.24.1

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

Split find_conflicting_inflight_req to be used separately.

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

We have a lot of "chunk_end - start" invocations, let's switch to
bytes/cur_bytes scheme instead.

While being here, improve check on block_copy_do_copy parameters to not
overflow when calculating nbytes and use int64_t for bytes in
block_copy for consistency.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200311103004.7649-7-vsementsov@virtuozzo.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/block-copy.c         | 78 ++++++++++++++++++++------------------
 include/block/block-copy.h |  6 +--
 2 files changed, 44 insertions(+), 40 deletions(-)

diff --git a/block/block-copy.c b/block/block-copy.c
index XXXXXXX..XXXXXXX 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -XXX,XX +XXX,XX @@
 
 static BlockCopyInFlightReq *find_conflicting_inflight_req(BlockCopyState *s,
                                                            int64_t start,
-                                                           int64_t end)
+                                                           int64_t bytes)
 {
     BlockCopyInFlightReq *req;
 
     QLIST_FOREACH(req, &s->inflight_reqs, list) {
-        if (end > req->start_byte && start < req->end_byte) {
+        if (start + bytes > req->start && start < req->start + req->bytes) {
             return req;
         }
     }
@@ -XXX,XX +XXX,XX @@ static BlockCopyInFlightReq *find_conflicting_inflight_req(BlockCopyState *s,
 
 static void coroutine_fn block_copy_wait_inflight_reqs(BlockCopyState *s,
                                                        int64_t start,
-                                                       int64_t end)
+                                                       int64_t bytes)
 {
     BlockCopyInFlightReq *req;
 
-    while ((req = find_conflicting_inflight_req(s, start, end))) {
+    while ((req = find_conflicting_inflight_req(s, start, bytes))) {
         qemu_co_queue_wait(&req->wait_queue, NULL);
     }
 }
 
 static void block_copy_inflight_req_begin(BlockCopyState *s,
                                           BlockCopyInFlightReq *req,
-                                          int64_t start, int64_t end)
+                                          int64_t start, int64_t bytes)
 {
-    req->start_byte = start;
-    req->end_byte = end;
+    req->start = start;
+    req->bytes = bytes;
     qemu_co_queue_init(&req->wait_queue);
     QLIST_INSERT_HEAD(&s->inflight_reqs, req, list);
 }
@@ -XXX,XX +XXX,XX @@ void block_copy_set_progress_meter(BlockCopyState *s, ProgressMeter *pm)
 /*
  * block_copy_do_copy
  *
- * Do copy of cluser-aligned chunk. @end is allowed to exceed s->len only to
- * cover last cluster when s->len is not aligned to clusters.
+ * Do copy of cluster-aligned chunk. Requested region is allowed to exceed
+ * s->len only to cover last cluster when s->len is not aligned to clusters.
  *
  * No sync here: nor bitmap neighter intersecting requests handling, only copy.
  *
  * Returns 0 on success.
  */
 static int coroutine_fn block_copy_do_copy(BlockCopyState *s,
-                                           int64_t start, int64_t end,
+                                           int64_t start, int64_t bytes,
                                            bool zeroes, bool *error_is_read)
 {
     int ret;
-    int nbytes = MIN(end, s->len) - start;
+    int64_t nbytes = MIN(start + bytes, s->len) - start;
     void *bounce_buffer = NULL;
 
+    assert(start >= 0 && bytes > 0 && INT64_MAX - start >= bytes);
     assert(QEMU_IS_ALIGNED(start, s->cluster_size));
-    assert(QEMU_IS_ALIGNED(end, s->cluster_size));
-    assert(end < s->len || end == QEMU_ALIGN_UP(s->len, s->cluster_size));
+    assert(QEMU_IS_ALIGNED(bytes, s->cluster_size));
+    assert(start < s->len);
+    assert(start + bytes <= s->len ||
+           start + bytes == QEMU_ALIGN_UP(s->len, s->cluster_size));
+    assert(nbytes < INT_MAX);
 
     if (zeroes) {
         ret = bdrv_co_pwrite_zeroes(s->target, start, nbytes, s->write_flags &
@@ -XXX,XX +XXX,XX @@ int64_t block_copy_reset_unallocated(BlockCopyState *s,
 }
 
 int coroutine_fn block_copy(BlockCopyState *s,
-                            int64_t start, uint64_t bytes,
+                            int64_t start, int64_t bytes,
                             bool *error_is_read)
 {
     int ret = 0;
-    int64_t end = bytes + start; /* bytes */
     BlockCopyInFlightReq req;
 
     /*
@@ -XXX,XX +XXX,XX @@ int coroutine_fn block_copy(BlockCopyState *s,
            bdrv_get_aio_context(s->target->bs));
 
     assert(QEMU_IS_ALIGNED(start, s->cluster_size));
-    assert(QEMU_IS_ALIGNED(end, s->cluster_size));
+    assert(QEMU_IS_ALIGNED(bytes, s->cluster_size));
 
     block_copy_wait_inflight_reqs(s, start, bytes);
-    block_copy_inflight_req_begin(s, &req, start, end);
+    block_copy_inflight_req_begin(s, &req, start, bytes);
 
-    while (start < end) {
-        int64_t next_zero, chunk_end, status_bytes;
+    while (bytes) {
+        int64_t next_zero, cur_bytes, status_bytes;
 
         if (!bdrv_dirty_bitmap_get(s->copy_bitmap, start)) {
             trace_block_copy_skip(s, start);
             start += s->cluster_size;
+            bytes -= s->cluster_size;
             continue; /* already copied */
         }
 
-        chunk_end = MIN(end, start + s->copy_size);
+        cur_bytes = MIN(bytes, s->copy_size);
 
         next_zero = bdrv_dirty_bitmap_next_zero(s->copy_bitmap, start,
-                                                chunk_end - start);
+                                                cur_bytes);
         if (next_zero >= 0) {
             assert(next_zero > start); /* start is dirty */
-            assert(next_zero < chunk_end); /* no need to do MIN() */
-            chunk_end = next_zero;
+            assert(next_zero < start + cur_bytes); /* no need to do MIN() */
+            cur_bytes = next_zero - start;
         }
 
-        ret = block_copy_block_status(s, start, chunk_end - start,
-                                      &status_bytes);
+        ret = block_copy_block_status(s, start, cur_bytes, &status_bytes);
         if (s->skip_unallocated && !(ret & BDRV_BLOCK_ALLOCATED)) {
             bdrv_reset_dirty_bitmap(s->copy_bitmap, start, status_bytes);
             progress_set_remaining(s->progress,
@@ -XXX,XX +XXX,XX @@ int coroutine_fn block_copy(BlockCopyState *s,
                                    s->in_flight_bytes);
             trace_block_copy_skip_range(s, start, status_bytes);
             start += status_bytes;
+            bytes -= status_bytes;
             continue;
         }
 
-        chunk_end = MIN(chunk_end, start + status_bytes);
+        cur_bytes = MIN(cur_bytes, status_bytes);
 
         trace_block_copy_process(s, start);
 
-        bdrv_reset_dirty_bitmap(s->copy_bitmap, start, chunk_end - start);
-        s->in_flight_bytes += chunk_end - start;
+        bdrv_reset_dirty_bitmap(s->copy_bitmap, start, cur_bytes);
+        s->in_flight_bytes += cur_bytes;
 
-        co_get_from_shres(s->mem, chunk_end - start);
-        ret = block_copy_do_copy(s, start, chunk_end, ret & BDRV_BLOCK_ZERO,
+        co_get_from_shres(s->mem, cur_bytes);
+        ret = block_copy_do_copy(s, start, cur_bytes, ret & BDRV_BLOCK_ZERO,
                                  error_is_read);
-        co_put_to_shres(s->mem, chunk_end - start);
-        s->in_flight_bytes -= chunk_end - start;
+        co_put_to_shres(s->mem, cur_bytes);
+        s->in_flight_bytes -= cur_bytes;
         if (ret < 0) {
-            bdrv_set_dirty_bitmap(s->copy_bitmap, start, chunk_end - start);
+            bdrv_set_dirty_bitmap(s->copy_bitmap, start, cur_bytes);
             break;
         }
 
-        progress_work_done(s->progress, chunk_end - start);
-        s->progress_bytes_callback(chunk_end - start, s->progress_opaque);
-        start = chunk_end;
-        ret = 0;
+        progress_work_done(s->progress, cur_bytes);
+        s->progress_bytes_callback(cur_bytes, s->progress_opaque);
+        start += cur_bytes;
+        bytes -= cur_bytes;
     }
 
     block_copy_inflight_req_end(&req);
diff --git a/include/block/block-copy.h b/include/block/block-copy.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/block-copy.h
+++ b/include/block/block-copy.h
@@ -XXX,XX +XXX,XX @@
 #include "qemu/co-shared-resource.h"
 
 typedef struct BlockCopyInFlightReq {
-    int64_t start_byte;
-    int64_t end_byte;
+    int64_t start;
+    int64_t bytes;
     QLIST_ENTRY(BlockCopyInFlightReq) list;
     CoQueue wait_queue; /* coroutines blocked on this request */
 } BlockCopyInFlightReq;
@@ -XXX,XX +XXX,XX @@ void block_copy_state_free(BlockCopyState *s);
 int64_t block_copy_reset_unallocated(BlockCopyState *s,
                                      int64_t offset, int64_t *count);
 
-int coroutine_fn block_copy(BlockCopyState *s, int64_t start, uint64_t bytes,
+int coroutine_fn block_copy(BlockCopyState *s, int64_t start, int64_t bytes,
                             bool *error_is_read);
 
 #endif /* BLOCK_COPY_H */
-- 
2.24.1

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

offset/bytes pair is more usual naming in block layer, let's use it.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200311103004.7649-8-vsementsov@virtuozzo.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/block-copy.c         | 82 +++++++++++++++++++-------------------
 include/block/block-copy.h |  4 +-
 2 files changed, 43 insertions(+), 43 deletions(-)

diff --git a/block/block-copy.c b/block/block-copy.c
index XXXXXXX..XXXXXXX 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -XXX,XX +XXX,XX @@
 #define BLOCK_COPY_MAX_MEM (128 * MiB)
 
 static BlockCopyInFlightReq *find_conflicting_inflight_req(BlockCopyState *s,
-                                                           int64_t start,
+                                                           int64_t offset,
                                                            int64_t bytes)
 {
     BlockCopyInFlightReq *req;
 
     QLIST_FOREACH(req, &s->inflight_reqs, list) {
-        if (start + bytes > req->start && start < req->start + req->bytes) {
+        if (offset + bytes > req->offset && offset < req->offset + req->bytes) {
             return req;
         }
     }
@@ -XXX,XX +XXX,XX @@ static BlockCopyInFlightReq *find_conflicting_inflight_req(BlockCopyState *s,
 }
 
 static void coroutine_fn block_copy_wait_inflight_reqs(BlockCopyState *s,
-                                                       int64_t start,
+                                                       int64_t offset,
                                                        int64_t bytes)
 {
     BlockCopyInFlightReq *req;
 
-    while ((req = find_conflicting_inflight_req(s, start, bytes))) {
+    while ((req = find_conflicting_inflight_req(s, offset, bytes))) {
         qemu_co_queue_wait(&req->wait_queue, NULL);
     }
 }
 
 static void block_copy_inflight_req_begin(BlockCopyState *s,
                                           BlockCopyInFlightReq *req,
-                                          int64_t start, int64_t bytes)
+                                          int64_t offset, int64_t bytes)
 {
-    req->start = start;
+    req->offset = offset;
     req->bytes = bytes;
     qemu_co_queue_init(&req->wait_queue);
     QLIST_INSERT_HEAD(&s->inflight_reqs, req, list);
@@ -XXX,XX +XXX,XX @@ void block_copy_set_progress_meter(BlockCopyState *s, ProgressMeter *pm)
  * Returns 0 on success.
  */
 static int coroutine_fn block_copy_do_copy(BlockCopyState *s,
-                                           int64_t start, int64_t bytes,
+                                           int64_t offset, int64_t bytes,
                                            bool zeroes, bool *error_is_read)
 {
     int ret;
-    int64_t nbytes = MIN(start + bytes, s->len) - start;
+    int64_t nbytes = MIN(offset + bytes, s->len) - offset;
     void *bounce_buffer = NULL;
 
-    assert(start >= 0 && bytes > 0 && INT64_MAX - start >= bytes);
-    assert(QEMU_IS_ALIGNED(start, s->cluster_size));
+    assert(offset >= 0 && bytes > 0 && INT64_MAX - offset >= bytes);
+    assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
     assert(QEMU_IS_ALIGNED(bytes, s->cluster_size));
-    assert(start < s->len);
-    assert(start + bytes <= s->len ||
-           start + bytes == QEMU_ALIGN_UP(s->len, s->cluster_size));
+    assert(offset < s->len);
+    assert(offset + bytes <= s->len ||
+           offset + bytes == QEMU_ALIGN_UP(s->len, s->cluster_size));
     assert(nbytes < INT_MAX);
 
     if (zeroes) {
-        ret = bdrv_co_pwrite_zeroes(s->target, start, nbytes, s->write_flags &
+        ret = bdrv_co_pwrite_zeroes(s->target, offset, nbytes, s->write_flags &
                                     ~BDRV_REQ_WRITE_COMPRESSED);
         if (ret < 0) {
-            trace_block_copy_write_zeroes_fail(s, start, ret);
+            trace_block_copy_write_zeroes_fail(s, offset, ret);
             if (error_is_read) {
                 *error_is_read = false;
             }
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn block_copy_do_copy(BlockCopyState *s,
     }
 
     if (s->use_copy_range) {
-        ret = bdrv_co_copy_range(s->source, start, s->target, start, nbytes,
+        ret = bdrv_co_copy_range(s->source, offset, s->target, offset, nbytes,
                                  0, s->write_flags);
         if (ret < 0) {
-            trace_block_copy_copy_range_fail(s, start, ret);
+            trace_block_copy_copy_range_fail(s, offset, ret);
             s->use_copy_range = false;
             s->copy_size = MAX(s->cluster_size, BLOCK_COPY_MAX_BUFFER);
             /* Fallback to read+write with allocated buffer */
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn block_copy_do_copy(BlockCopyState *s,
 
     bounce_buffer = qemu_blockalign(s->source->bs, nbytes);
 
-    ret = bdrv_co_pread(s->source, start, nbytes, bounce_buffer, 0);
+    ret = bdrv_co_pread(s->source, offset, nbytes, bounce_buffer, 0);
     if (ret < 0) {
-        trace_block_copy_read_fail(s, start, ret);
+        trace_block_copy_read_fail(s, offset, ret);
         if (error_is_read) {
             *error_is_read = true;
         }
         goto out;
     }
 
-    ret = bdrv_co_pwrite(s->target, start, nbytes, bounce_buffer,
+    ret = bdrv_co_pwrite(s->target, offset, nbytes, bounce_buffer,
                          s->write_flags);
     if (ret < 0) {
-        trace_block_copy_write_fail(s, start, ret);
+        trace_block_copy_write_fail(s, offset, ret);
         if (error_is_read) {
             *error_is_read = false;
         }
@@ -XXX,XX +XXX,XX @@ int64_t block_copy_reset_unallocated(BlockCopyState *s,
 }
 
 int coroutine_fn block_copy(BlockCopyState *s,
-                            int64_t start, int64_t bytes,
+                            int64_t offset, int64_t bytes,
                             bool *error_is_read)
 {
     int ret = 0;
@@ -XXX,XX +XXX,XX @@ int coroutine_fn block_copy(BlockCopyState *s,
     assert(bdrv_get_aio_context(s->source->bs) ==
            bdrv_get_aio_context(s->target->bs));
 
-    assert(QEMU_IS_ALIGNED(start, s->cluster_size));
+    assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
     assert(QEMU_IS_ALIGNED(bytes, s->cluster_size));
 
-    block_copy_wait_inflight_reqs(s, start, bytes);
-    block_copy_inflight_req_begin(s, &req, start, bytes);
+    block_copy_wait_inflight_reqs(s, offset, bytes);
+    block_copy_inflight_req_begin(s, &req, offset, bytes);
 
     while (bytes) {
         int64_t next_zero, cur_bytes, status_bytes;
 
-        if (!bdrv_dirty_bitmap_get(s->copy_bitmap, start)) {
-            trace_block_copy_skip(s, start);
-            start += s->cluster_size;
+        if (!bdrv_dirty_bitmap_get(s->copy_bitmap, offset)) {
+            trace_block_copy_skip(s, offset);
+            offset += s->cluster_size;
             bytes -= s->cluster_size;
             continue; /* already copied */
         }
 
         cur_bytes = MIN(bytes, s->copy_size);
 
-        next_zero = bdrv_dirty_bitmap_next_zero(s->copy_bitmap, start,
+        next_zero = bdrv_dirty_bitmap_next_zero(s->copy_bitmap, offset,
                                                 cur_bytes);
         if (next_zero >= 0) {
-            assert(next_zero > start); /* start is dirty */
-            assert(next_zero < start + cur_bytes); /* no need to do MIN() */
-            cur_bytes = next_zero - start;
+            assert(next_zero > offset); /* offset is dirty */
+            assert(next_zero < offset + cur_bytes); /* no need to do MIN() */
+            cur_bytes = next_zero - offset;
         }
 
-        ret = block_copy_block_status(s, start, cur_bytes, &status_bytes);
+        ret = block_copy_block_status(s, offset, cur_bytes, &status_bytes);
         if (s->skip_unallocated && !(ret & BDRV_BLOCK_ALLOCATED)) {
-            bdrv_reset_dirty_bitmap(s->copy_bitmap, start, status_bytes);
+            bdrv_reset_dirty_bitmap(s->copy_bitmap, offset, status_bytes);
             progress_set_remaining(s->progress,
                                    bdrv_get_dirty_count(s->copy_bitmap) +
                                    s->in_flight_bytes);
-            trace_block_copy_skip_range(s, start, status_bytes);
-            start += status_bytes;
+            trace_block_copy_skip_range(s, offset, status_bytes);
+            offset += status_bytes;
             bytes -= status_bytes;
             continue;
         }
 
         cur_bytes = MIN(cur_bytes, status_bytes);
 
-        trace_block_copy_process(s, start);
+        trace_block_copy_process(s, offset);
 
-        bdrv_reset_dirty_bitmap(s->copy_bitmap, start, cur_bytes);
+        bdrv_reset_dirty_bitmap(s->copy_bitmap, offset, cur_bytes);
         s->in_flight_bytes += cur_bytes;
 
         co_get_from_shres(s->mem, cur_bytes);
-        ret = block_copy_do_copy(s, start, cur_bytes, ret & BDRV_BLOCK_ZERO,
+        ret = block_copy_do_copy(s, offset, cur_bytes, ret & BDRV_BLOCK_ZERO,
                                  error_is_read);
         co_put_to_shres(s->mem, cur_bytes);
         s->in_flight_bytes -= cur_bytes;
         if (ret < 0) {
-            bdrv_set_dirty_bitmap(s->copy_bitmap, start, cur_bytes);
+            bdrv_set_dirty_bitmap(s->copy_bitmap, offset, cur_bytes);
             break;
         }
 
         progress_work_done(s->progress, cur_bytes);
         s->progress_bytes_callback(cur_bytes, s->progress_opaque);
-        start += cur_bytes;
+        offset += cur_bytes;
         bytes -= cur_bytes;
     }
 
diff --git a/include/block/block-copy.h b/include/block/block-copy.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/block-copy.h
+++ b/include/block/block-copy.h
@@ -XXX,XX +XXX,XX @@
 #include "qemu/co-shared-resource.h"
 
 typedef struct BlockCopyInFlightReq {
-    int64_t start;
+    int64_t offset;
     int64_t bytes;
     QLIST_ENTRY(BlockCopyInFlightReq) list;
     CoQueue wait_queue; /* coroutines blocked on this request */
@@ -XXX,XX +XXX,XX @@ void block_copy_state_free(BlockCopyState *s);
 int64_t block_copy_reset_unallocated(BlockCopyState *s,
                                      int64_t offset, int64_t *count);
 
-int coroutine_fn block_copy(BlockCopyState *s, int64_t start, int64_t bytes,
+int coroutine_fn block_copy(BlockCopyState *s, int64_t offset, int64_t bytes,
                             bool *error_is_read);
 
 #endif /* BLOCK_COPY_H */
-- 
2.24.1

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

Currently, block_copy operation lock the whole requested region. But
there is no reason to lock clusters, which are already copied, it will
disturb other parallel block_copy requests for no reason.

Let's instead do the following:

Lock only sub-region, which we are going to operate on. Then, after
copying all dirty sub-regions, we should wait for intersecting
requests block-copy, if they failed, we should retry these new dirty
clusters.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Message-Id: <20200311103004.7649-9-vsementsov@virtuozzo.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/block-copy.c | 129 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 105 insertions(+), 24 deletions(-)

diff --git a/block/block-copy.c b/block/block-copy.c
index XXXXXXX..XXXXXXX 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -XXX,XX +XXX,XX @@ static BlockCopyInFlightReq *find_conflicting_inflight_req(BlockCopyState *s,
     return NULL;
 }
 
-static void coroutine_fn block_copy_wait_inflight_reqs(BlockCopyState *s,
-                                                       int64_t offset,
-                                                       int64_t bytes)
+/*
+ * If there are no intersecting requests return false. Otherwise, wait for the
+ * first found intersecting request to finish and return true.
+ */
+static bool coroutine_fn block_copy_wait_one(BlockCopyState *s, int64_t offset,
+                                             int64_t bytes)
 {
-    BlockCopyInFlightReq *req;
+    BlockCopyInFlightReq *req = find_conflicting_inflight_req(s, offset, bytes);
 
-    while ((req = find_conflicting_inflight_req(s, offset, bytes))) {
-        qemu_co_queue_wait(&req->wait_queue, NULL);
+    if (!req) {
+        return false;
     }
+
+    qemu_co_queue_wait(&req->wait_queue, NULL);
+
+    return true;
 }
 
+/* Called only on full-dirty region */
 static void block_copy_inflight_req_begin(BlockCopyState *s,
                                           BlockCopyInFlightReq *req,
                                           int64_t offset, int64_t bytes)
 {
+    assert(!find_conflicting_inflight_req(s, offset, bytes));
+
+    bdrv_reset_dirty_bitmap(s->copy_bitmap, offset, bytes);
+    s->in_flight_bytes += bytes;
+
     req->offset = offset;
     req->bytes = bytes;
     qemu_co_queue_init(&req->wait_queue);
     QLIST_INSERT_HEAD(&s->inflight_reqs, req, list);
 }
 
-static void coroutine_fn block_copy_inflight_req_end(BlockCopyInFlightReq *req)
+/*
+ * block_copy_inflight_req_shrink
+ *
+ * Drop the tail of the request to be handled later. Set dirty bits back and
+ * wake up all requests waiting for us (may be some of them are not intersecting
+ * with shrunk request)
+ */
+static void coroutine_fn block_copy_inflight_req_shrink(BlockCopyState *s,
+        BlockCopyInFlightReq *req, int64_t new_bytes)
 {
+    if (new_bytes == req->bytes) {
+        return;
+    }
+
+    assert(new_bytes > 0 && new_bytes < req->bytes);
+
+    s->in_flight_bytes -= req->bytes - new_bytes;
+    bdrv_set_dirty_bitmap(s->copy_bitmap,
+                          req->offset + new_bytes, req->bytes - new_bytes);
+
+    req->bytes = new_bytes;
+    qemu_co_queue_restart_all(&req->wait_queue);
+}
+
+static void coroutine_fn block_copy_inflight_req_end(BlockCopyState *s,
+                                                     BlockCopyInFlightReq *req,
+                                                     int ret)
+{
+    s->in_flight_bytes -= req->bytes;
+    if (ret < 0) {
+        bdrv_set_dirty_bitmap(s->copy_bitmap, req->offset, req->bytes);
+    }
     QLIST_REMOVE(req, list);
     qemu_co_queue_restart_all(&req->wait_queue);
 }
@@ -XXX,XX +XXX,XX @@ int64_t block_copy_reset_unallocated(BlockCopyState *s,
     return ret;
 }
 
-int coroutine_fn block_copy(BlockCopyState *s,
-                            int64_t offset, int64_t bytes,
-                            bool *error_is_read)
+/*
+ * block_copy_dirty_clusters
+ *
+ * Copy dirty clusters in @offset/@bytes range.
+ * Returns 1 if dirty clusters found and successfully copied, 0 if no dirty
+ * clusters found and -errno on failure.
+ */
+static int coroutine_fn block_copy_dirty_clusters(BlockCopyState *s,
+                                                  int64_t offset, int64_t bytes,
+                                                  bool *error_is_read)
 {
     int ret = 0;
-    BlockCopyInFlightReq req;
+    bool found_dirty = false;
 
     /*
      * block_copy() user is responsible for keeping source and target in same
@@ -XXX,XX +XXX,XX @@ int coroutine_fn block_copy(BlockCopyState *s,
     assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
     assert(QEMU_IS_ALIGNED(bytes, s->cluster_size));
 
-    block_copy_wait_inflight_reqs(s, offset, bytes);
-    block_copy_inflight_req_begin(s, &req, offset, bytes);
-
     while (bytes) {
+        BlockCopyInFlightReq req;
         int64_t next_zero, cur_bytes, status_bytes;
 
         if (!bdrv_dirty_bitmap_get(s->copy_bitmap, offset)) {
@@ -XXX,XX +XXX,XX @@ int coroutine_fn block_copy(BlockCopyState *s,
             continue; /* already copied */
         }
 
+        found_dirty = true;
+
         cur_bytes = MIN(bytes, s->copy_size);
 
         next_zero = bdrv_dirty_bitmap_next_zero(s->copy_bitmap, offset,
@@ -XXX,XX +XXX,XX @@ int coroutine_fn block_copy(BlockCopyState *s,
             assert(next_zero < offset + cur_bytes); /* no need to do MIN() */
             cur_bytes = next_zero - offset;
         }
+        block_copy_inflight_req_begin(s, &req, offset, cur_bytes);
 
         ret = block_copy_block_status(s, offset, cur_bytes, &status_bytes);
+        assert(ret >= 0); /* never fail */
+        cur_bytes = MIN(cur_bytes, status_bytes);
+        block_copy_inflight_req_shrink(s, &req, cur_bytes);
         if (s->skip_unallocated && !(ret & BDRV_BLOCK_ALLOCATED)) {
-            bdrv_reset_dirty_bitmap(s->copy_bitmap, offset, status_bytes);
+            block_copy_inflight_req_end(s, &req, 0);
             progress_set_remaining(s->progress,
                                    bdrv_get_dirty_count(s->copy_bitmap) +
                                    s->in_flight_bytes);
@@ -XXX,XX +XXX,XX @@ int coroutine_fn block_copy(BlockCopyState *s,
             continue;
         }
 
-        cur_bytes = MIN(cur_bytes, status_bytes);
-
         trace_block_copy_process(s, offset);
 
-        bdrv_reset_dirty_bitmap(s->copy_bitmap, offset, cur_bytes);
-        s->in_flight_bytes += cur_bytes;
-
         co_get_from_shres(s->mem, cur_bytes);
         ret = block_copy_do_copy(s, offset, cur_bytes, ret & BDRV_BLOCK_ZERO,
                                  error_is_read);
         co_put_to_shres(s->mem, cur_bytes);
-        s->in_flight_bytes -= cur_bytes;
+        block_copy_inflight_req_end(s, &req, ret);
         if (ret < 0) {
-            bdrv_set_dirty_bitmap(s->copy_bitmap, offset, cur_bytes);
-            break;
+            return ret;
         }
 
         progress_work_done(s->progress, cur_bytes);
@@ -XXX,XX +XXX,XX @@ int coroutine_fn block_copy(BlockCopyState *s,
         bytes -= cur_bytes;
     }
 
-    block_copy_inflight_req_end(&req);
+    return found_dirty;
+}
+
+/*
+ * block_copy
+ *
+ * Copy requested region, accordingly to dirty bitmap.
+ * Collaborate with parallel block_copy requests: if they succeed it will help
+ * us. If they fail, we will retry not-copied regions. So, if we return error,
+ * it means that some I/O operation failed in context of _this_ block_copy call,
+ * not some parallel operation.
+ */
+int coroutine_fn block_copy(BlockCopyState *s, int64_t offset, int64_t bytes,
+                            bool *error_is_read)
+{
+    int ret;
+
+    do {
+        ret = block_copy_dirty_clusters(s, offset, bytes, error_is_read);
+
+        if (ret == 0) {
+            ret = block_copy_wait_one(s, offset, bytes);
+        }
+
+        /*
+         * We retry in two cases:
+         * 1. Some progress done
+         *    Something was copied, which means that there were yield points
+         *    and some new dirty bits may have appeared (due to failed parallel
+         *    block-copy requests).
+         * 2. We have waited for some intersecting block-copy request
+         *    It may have failed and produced new dirty bits.
+         */
+    } while (ret > 0);
 
     return ret;
 }
-- 
2.24.1

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

Hide structure definitions and add explicit API instead, to keep an
eye on the scope of the shared fields.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200311103004.7649-10-vsementsov@virtuozzo.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/backup-top.c         |  6 ++--
 block/backup.c             | 25 ++++++++--------
 block/block-copy.c         | 59 ++++++++++++++++++++++++++++++++++++++
 include/block/block-copy.h | 52 +++------------------------------
 4 files changed, 80 insertions(+), 62 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 @@ typedef struct BDRVBackupTopState {
     BlockCopyState *bcs;
     BdrvChild *target;
     bool active;
+    int64_t cluster_size;
 } BDRVBackupTopState;
 
 static coroutine_fn int backup_top_co_preadv(
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int backup_top_cbw(BlockDriverState *bs, uint64_t offset,
         return 0;
     }
 
-    off = QEMU_ALIGN_DOWN(offset, s->bcs->cluster_size);
-    end = QEMU_ALIGN_UP(offset + bytes, s->bcs->cluster_size);
+    off = QEMU_ALIGN_DOWN(offset, s->cluster_size);
+    end = QEMU_ALIGN_UP(offset + bytes, s->cluster_size);
 
     return block_copy(s->bcs, off, end - off, NULL);
 }
@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
         goto fail;
     }
 
+    state->cluster_size = cluster_size;
     state->bcs = block_copy_state_new(top->backing, state->target,
                                       cluster_size, write_flags, &local_err);
     if (local_err) {
diff --git a/block/backup.c b/block/backup.c
index XXXXXXX..XXXXXXX 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -XXX,XX +XXX,XX @@ static void backup_cleanup_sync_bitmap(BackupBlockJob *job, int ret)
 
     if (ret < 0 && job->bitmap_mode == BITMAP_SYNC_MODE_ALWAYS) {
         /* If we failed and synced, merge in the bits we didn't copy: */
-        bdrv_dirty_bitmap_merge_internal(bm, job->bcs->copy_bitmap,
+        bdrv_dirty_bitmap_merge_internal(bm, block_copy_dirty_bitmap(job->bcs),
                                          NULL, true);
     }
 }
@@ -XXX,XX +XXX,XX @@ void backup_do_checkpoint(BlockJob *job, Error **errp)
         return;
     }
 
-    bdrv_set_dirty_bitmap(backup_job->bcs->copy_bitmap, 0, backup_job->len);
+    bdrv_set_dirty_bitmap(block_copy_dirty_bitmap(backup_job->bcs), 0,
+                          backup_job->len);
 }
 
 static BlockErrorAction backup_error_action(BackupBlockJob *job,
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_loop(BackupBlockJob *job)
     BdrvDirtyBitmapIter *bdbi;
     int ret = 0;
 
-    bdbi = bdrv_dirty_iter_new(job->bcs->copy_bitmap);
+    bdbi = bdrv_dirty_iter_new(block_copy_dirty_bitmap(job->bcs));
     while ((offset = bdrv_dirty_iter_next(bdbi)) != -1) {
         do {
             if (yield_and_check(job)) {
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_loop(BackupBlockJob *job)
     return ret;
 }
 
-static void backup_init_copy_bitmap(BackupBlockJob *job)
+static void backup_init_bcs_bitmap(BackupBlockJob *job)
 {
     bool ret;
     uint64_t estimate;
+    BdrvDirtyBitmap *bcs_bitmap = block_copy_dirty_bitmap(job->bcs);
 
     if (job->sync_mode == MIRROR_SYNC_MODE_BITMAP) {
-        ret = bdrv_dirty_bitmap_merge_internal(job->bcs->copy_bitmap,
-                                               job->sync_bitmap,
+        ret = bdrv_dirty_bitmap_merge_internal(bcs_bitmap, job->sync_bitmap,
                                                NULL, true);
         assert(ret);
     } else {
@@ -XXX,XX +XXX,XX @@ static void backup_init_copy_bitmap(BackupBlockJob *job)
              * We can't hog the coroutine to initialize this thoroughly.
              * Set a flag and resume work when we are able to yield safely.
              */
-            job->bcs->skip_unallocated = true;
+            block_copy_set_skip_unallocated(job->bcs, true);
         }
-        bdrv_set_dirty_bitmap(job->bcs->copy_bitmap, 0, job->len);
+        bdrv_set_dirty_bitmap(bcs_bitmap, 0, job->len);
     }
 
-    estimate = bdrv_get_dirty_count(job->bcs->copy_bitmap);
+    estimate = bdrv_get_dirty_count(bcs_bitmap);
     job_progress_set_remaining(&job->common.job, estimate);
 }
 
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_run(Job *job, Error **errp)
     BackupBlockJob *s = container_of(job, BackupBlockJob, common.job);
     int ret = 0;
 
-    backup_init_copy_bitmap(s);
+    backup_init_bcs_bitmap(s);
 
     if (s->sync_mode == MIRROR_SYNC_MODE_TOP) {
         int64_t offset = 0;
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_run(Job *job, Error **errp)
 
             offset += count;
         }
-        s->bcs->skip_unallocated = false;
+        block_copy_set_skip_unallocated(s->bcs, false);
     }
 
     if (s->sync_mode == MIRROR_SYNC_MODE_NONE) {
         /*
-         * All bits are set in copy_bitmap to allow any cluster to be copied.
+         * All bits are set in bcs bitmap to allow any cluster to be copied.
          * This does not actually require them to be copied.
          */
         while (!job_is_cancelled(job)) {
diff --git a/block/block-copy.c b/block/block-copy.c
index XXXXXXX..XXXXXXX 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -XXX,XX +XXX,XX @@
 #define BLOCK_COPY_MAX_BUFFER (1 * MiB)
 #define BLOCK_COPY_MAX_MEM (128 * MiB)
 
+typedef struct BlockCopyInFlightReq {
+    int64_t offset;
+    int64_t bytes;
+    QLIST_ENTRY(BlockCopyInFlightReq) list;
+    CoQueue wait_queue; /* coroutines blocked on this request */
+} BlockCopyInFlightReq;
+
+typedef struct BlockCopyState {
+    /*
+     * BdrvChild objects are not owned or managed by block-copy. They are
+     * provided by block-copy user and user is responsible for appropriate
+     * permissions on these children.
+     */
+    BdrvChild *source;
+    BdrvChild *target;
+    BdrvDirtyBitmap *copy_bitmap;
+    int64_t in_flight_bytes;
+    int64_t cluster_size;
+    bool use_copy_range;
+    int64_t copy_size;
+    uint64_t len;
+    QLIST_HEAD(, BlockCopyInFlightReq) inflight_reqs;
+
+    BdrvRequestFlags write_flags;
+
+    /*
+     * skip_unallocated:
+     *
+     * Used by sync=top jobs, which first scan the source node for unallocated
+     * areas and clear them in the copy_bitmap.  During this process, the bitmap
+     * is thus not fully initialized: It may still have bits set for areas that
+     * are unallocated and should actually not be copied.
+     *
+     * This is indicated by skip_unallocated.
+     *
+     * In this case, block_copy() will query the source’s allocation status,
+     * skip unallocated regions, clear them in the copy_bitmap, and invoke
+     * block_copy_reset_unallocated() every time it does.
+     */
+    bool skip_unallocated;
+
+    ProgressMeter *progress;
+    /* progress_bytes_callback: called when some copying progress is done. */
+    ProgressBytesCallbackFunc progress_bytes_callback;
+    void *progress_opaque;
+
+    SharedResource *mem;
+} BlockCopyState;
+
 static BlockCopyInFlightReq *find_conflicting_inflight_req(BlockCopyState *s,
                                                            int64_t offset,
                                                            int64_t bytes)
@@ -XXX,XX +XXX,XX @@ int coroutine_fn block_copy(BlockCopyState *s, int64_t offset, int64_t bytes,
 
     return ret;
 }
+
+BdrvDirtyBitmap *block_copy_dirty_bitmap(BlockCopyState *s)
+{
+    return s->copy_bitmap;
+}
+
+void block_copy_set_skip_unallocated(BlockCopyState *s, bool skip)
+{
+    s->skip_unallocated = skip;
+}
diff --git a/include/block/block-copy.h b/include/block/block-copy.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/block-copy.h
+++ b/include/block/block-copy.h
@@ -XXX,XX +XXX,XX @@
 #include "block/block.h"
 #include "qemu/co-shared-resource.h"
 
-typedef struct BlockCopyInFlightReq {
-    int64_t offset;
-    int64_t bytes;
-    QLIST_ENTRY(BlockCopyInFlightReq) list;
-    CoQueue wait_queue; /* coroutines blocked on this request */
-} BlockCopyInFlightReq;
-
 typedef void (*ProgressBytesCallbackFunc)(int64_t bytes, void *opaque);
-typedef struct BlockCopyState {
-    /*
-     * BdrvChild objects are not owned or managed by block-copy. They are
-     * provided by block-copy user and user is responsible for appropriate
-     * permissions on these children.
-     */
-    BdrvChild *source;
-    BdrvChild *target;
-    BdrvDirtyBitmap *copy_bitmap;
-    int64_t in_flight_bytes;
-    int64_t cluster_size;
-    bool use_copy_range;
-    int64_t copy_size;
-    uint64_t len;
-    QLIST_HEAD(, BlockCopyInFlightReq) inflight_reqs;
-
-    BdrvRequestFlags write_flags;
-
-    /*
-     * skip_unallocated:
-     *
-     * Used by sync=top jobs, which first scan the source node for unallocated
-     * areas and clear them in the copy_bitmap.  During this process, the bitmap
-     * is thus not fully initialized: It may still have bits set for areas that
-     * are unallocated and should actually not be copied.
-     *
-     * This is indicated by skip_unallocated.
-     *
-     * In this case, block_copy() will query the source’s allocation status,
-     * skip unallocated regions, clear them in the copy_bitmap, and invoke
-     * block_copy_reset_unallocated() every time it does.
-     */
-    bool skip_unallocated;
-
-    ProgressMeter *progress;
-    /* progress_bytes_callback: called when some copying progress is done. */
-    ProgressBytesCallbackFunc progress_bytes_callback;
-    void *progress_opaque;
-
-    SharedResource *mem;
-} BlockCopyState;
+typedef struct BlockCopyState BlockCopyState;
 
 BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
                                      int64_t cluster_size,
@@ -XXX,XX +XXX,XX @@ int64_t block_copy_reset_unallocated(BlockCopyState *s,
 int coroutine_fn block_copy(BlockCopyState *s, int64_t offset, int64_t bytes,
                             bool *error_is_read);
 
+BdrvDirtyBitmap *block_copy_dirty_bitmap(BlockCopyState *s);
+void block_copy_set_skip_unallocated(BlockCopyState *s, bool skip);
+
 #endif /* BLOCK_COPY_H */
-- 
2.24.1

Daniel P. Berrangé <berrange@redhat.com> pointed out that the coroutine
pool size heuristic is very conservative. Instead of halving
max_map_count, he suggested reserving 5,000 mappings for non-coroutine
users based on observations of guests he has access to.

Fixes: 86a637e48104 ("coroutine: cap per-thread local pool size")
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-id: 20240320181232.1464819-1-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/qemu-coroutine.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c
index XXXXXXX..XXXXXXX 100644
--- a/util/qemu-coroutine.c
+++ b/util/qemu-coroutine.c
@@ -XXX,XX +XXX,XX @@ static unsigned int get_global_pool_hard_max_size(void)
                             NULL) &&
         qemu_strtoi(contents, NULL, 10, &max_map_count) == 0) {
         /*
-         * This is a conservative upper bound that avoids exceeding
-         * max_map_count. Leave half for non-coroutine users like library
-         * dependencies, vhost-user, etc. Each coroutine takes up 2 VMAs so
-         * halve the amount again.
+         * This is an upper bound that avoids exceeding max_map_count. Leave a
+         * fixed amount for non-coroutine users like library dependencies,
+         * vhost-user, etc. Each coroutine takes up 2 VMAs so halve the
+         * remaining amount.
          */
-        return max_map_count / 4;
+        if (max_map_count > 5000) {
+            return (max_map_count - 5000) / 2;
+        } else {
+            /* Disable the global pool but threads still have local pools */
+            return 0;
+        }
     }
 #endif
 
-- 
2.44.0