Series comparison

-[Qemu-devel] [PULL 00/21] Block patches
+[PULL 0/8] Block patches
-The following changes since commit 8c1ecb590497b0349c550607db923972b37f6963:
+The following changes since commit 6c769690ac845fa62642a5f93b4e4bd906adab95:
-  Merge remote-tracking branch 'remotes/stsquad/tags/pull-testing-next-280519-2' into staging (2019-05-28 17:38:32 +0100)
+  Merge remote-tracking branch 'remotes/vsementsov/tags/pull-simplebench-2021-05-04' into staging (2021-05-21 12:02:34 +0100)
 are available in the Git repository at:
-  https://github.com/XanClic/qemu.git tags/pull-block-2019-05-28
+  https://gitlab.com/stefanha/qemu.git tags/block-pull-request
-for you to fetch changes up to a2d665c1bc3624a8375e2f9a7d569f7565cc1358:
+for you to fetch changes up to 0a6f0c76a030710780ce10d6347a70f098024d21:
-  blockdev: loosen restrictions on drive-backup source node (2019-05-28 20:30:55 +0200)
+  coroutine-sleep: introduce qemu_co_sleep (2021-05-21 18:22:33 +0100)
 ----------------------------------------------------------------
-Block patches:
+Pull request
-- qcow2: Use threads for encrypted I/O
-- qemu-img rebase: Optimizations
+(Resent due to an email preparation mistake.)
 - backup job: Allow any source node, and some refactoring
 - Some general simplifications in the block layer
 ----------------------------------------------------------------
-Alberto Garcia (2):
-  block: Use bdrv_unref_child() for all children in bdrv_close()
-  block: Make bdrv_root_attach_child() unref child_bs on failure
-Andrey Shinkevich (1):
+Paolo Bonzini (6):
-  qcow2-bitmap: initialize bitmap directory alignment
+  coroutine-sleep: use a stack-allocated timer
   coroutine-sleep: disallow NULL QemuCoSleepState** argument
   coroutine-sleep: allow qemu_co_sleep_wake that wakes nothing
   coroutine-sleep: move timer out of QemuCoSleepState
   coroutine-sleep: replace QemuCoSleepState pointer with struct in the
     API
   coroutine-sleep: introduce qemu_co_sleep
-Anton Nefedov (1):
+Philippe Mathieu-Daudé (1):
-  qcow2: skip writing zero buffers to empty COW areas
+  bitops.h: Improve find_xxx_bit() documentation
-John Snow (1):
+Zenghui Yu (1):
-  blockdev: loosen restrictions on drive-backup source node
+  multi-process: Initialize variables declared with g_auto*
-Sam Eiderman (3):
+ include/qemu/bitops.h       | 15 ++++++--
-  qemu-img: rebase: Reuse parent BlockDriverState
+ include/qemu/coroutine.h    | 27 ++++++++-----
-  qemu-img: rebase: Reduce reads on in-chain rebase
+ block/block-copy.c          | 10 ++---
-  qemu-img: rebase: Reuse in-chain BlockDriverState
+ block/nbd.c                 | 14 +++----
+ hw/remote/memory.c          |  5 +--
-Vladimir Sementsov-Ogievskiy (13):
+ hw/remote/proxy.c           |  3 +-
-  qcow2.h: add missing include
+ util/qemu-coroutine-sleep.c | 75 +++++++++++++++++++------------------
-  qcow2: add separate file for threaded data processing functions
+files changed, 79 insertions(+), 70 deletions(-)
   qcow2-threads: use thread_pool_submit_co
   qcow2-threads: qcow2_co_do_compress: protect queuing by mutex
   qcow2-threads: split out generic path
   qcow2: qcow2_co_preadv: improve locking
   qcow2: bdrv_co_pwritev: move encryption code out of the lock
   qcow2: do encryption in threads
   block/backup: simplify backup_incremental_init_copy_bitmap
   block/backup: move to copy_bitmap with granularity
   block/backup: refactor and tolerate unallocated cluster skipping
   block/backup: unify different modes code path
   block/backup: refactor: split out backup_calculate_cluster_size
  block/Makefile.objs         |   2 +-
  qapi/block-core.json        |   4 +-
  block/qcow2.h               |  26 ++-
  block.c                     |  46 +++---
  block/backup.c              | 243 ++++++++++++---------------
  block/block-backend.c       |   3 +-
  block/qcow2-bitmap.c        |   3 +-
  block/qcow2-cache.c         |   1 -
  block/qcow2-cluster.c       |  10 +-
  block/qcow2-refcount.c      |   1 -
  block/qcow2-snapshot.c      |   1 -
  block/qcow2-threads.c       | 268 ++++++++++++++++++++++++++++++
  block/qcow2.c               | 320 +++++++++++++-----------------------
  block/quorum.c              |   1 -
  blockdev.c                  |   7 +-
  blockjob.c                  |   2 +-
  qemu-img.c                  |  85 ++++++----
  tests/test-bdrv-drain.c     |   6 -
  tests/test-bdrv-graph-mod.c |   1 -
  block/trace-events          |   1 +
  tests/qemu-iotests/056      |   2 +-
  tests/qemu-iotests/060      |   7 +-
  tests/qemu-iotests/060.out  |   5 +-
 files changed, 615 insertions(+), 430 deletions(-)
  create mode 100644 block/qcow2-threads.c
 --
-.21.0
+.31.1

-[Qemu-devel] [PULL 01/21] qcow2.h: add missing include
+Deleted patch
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-qcow2.h depends on block_int.h. Compilation isn't broken currently only
-due to block_int.h always included before qcow2.h. Though, it seems
-better to directly include block_int.h in qcow2.h.
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Reviewed-by: Alberto Garcia <berto@igalia.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Message-id: 20190506142741.41731-2-vsementsov@virtuozzo.com
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block/qcow2.h          | 1 +
- block/qcow2-bitmap.c   | 1 -
- block/qcow2-cache.c    | 1 -
- block/qcow2-cluster.c  | 1 -
- block/qcow2-refcount.c | 1 -
- block/qcow2-snapshot.c | 1 -
- block/qcow2.c          | 1 -
-files changed, 1 insertion(+), 6 deletions(-)
-diff --git a/block/qcow2.h b/block/qcow2.h
-index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2.h
-+++ b/block/qcow2.h
-@@ -XXX,XX +XXX,XX @@
- #include "crypto/block.h"
- #include "qemu/coroutine.h"
- #include "qemu/units.h"
-+#include "block/block_int.h"
- //#define DEBUG_ALLOC
- //#define DEBUG_ALLOC2
-diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2-bitmap.c
-+++ b/block/qcow2-bitmap.c
-@@ -XXX,XX +XXX,XX @@
- #include "qapi/error.h"
- #include "qemu/cutils.h"
--#include "block/block_int.h"
- #include "qcow2.h"
- /* NOTICE: BME here means Bitmaps Extension and used as a namespace for
-diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2-cache.c
-+++ b/block/qcow2-cache.c
-@@ -XXX,XX +XXX,XX @@
-  */
- #include "qemu/osdep.h"
--#include "block/block_int.h"
- #include "qemu-common.h"
- #include "qcow2.h"
- #include "trace.h"
-diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2-cluster.c
-+++ b/block/qcow2-cluster.c
-@@ -XXX,XX +XXX,XX @@
- #include "qapi/error.h"
- #include "qemu-common.h"
--#include "block/block_int.h"
- #include "qcow2.h"
- #include "qemu/bswap.h"
- #include "trace.h"
-diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2-refcount.c
-+++ b/block/qcow2-refcount.c
-@@ -XXX,XX +XXX,XX @@
- #include "qemu/osdep.h"
- #include "qapi/error.h"
- #include "qemu-common.h"
--#include "block/block_int.h"
- #include "qcow2.h"
- #include "qemu/range.h"
- #include "qemu/bswap.h"
-diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2-snapshot.c
-+++ b/block/qcow2-snapshot.c
-@@ -XXX,XX +XXX,XX @@
- #include "qemu/osdep.h"
- #include "qapi/error.h"
--#include "block/block_int.h"
- #include "qcow2.h"
- #include "qemu/bswap.h"
- #include "qemu/error-report.h"
-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 @@
- #define ZLIB_CONST
- #include <zlib.h>
--#include "block/block_int.h"
- #include "block/qdict.h"
- #include "sysemu/block-backend.h"
- #include "qemu/module.h"
---
-.21.0

-[Qemu-devel] [PULL 02/21] qcow2: add separate file for threaded data processing functions
+Deleted patch
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Move compression-on-threads to separate file. Encryption will be in it
-too.
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Reviewed-by: Alberto Garcia <berto@igalia.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Message-id: 20190506142741.41731-3-vsementsov@virtuozzo.com
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block/Makefile.objs   |   2 +-
- block/qcow2.h         |   7 ++
- block/qcow2-threads.c | 201 ++++++++++++++++++++++++++++++++++++++++++
- block/qcow2.c         | 169 -----------------------------------
-files changed, 209 insertions(+), 170 deletions(-)
- create mode 100644 block/qcow2-threads.c
-diff --git a/block/Makefile.objs b/block/Makefile.objs
-index XXXXXXX..XXXXXXX 100644
---- a/block/Makefile.objs
-+++ b/block/Makefile.objs
-@@ -XXX,XX +XXX,XX @@ block-obj-$(CONFIG_BOCHS) += bochs.o
- block-obj-$(CONFIG_VVFAT) += vvfat.o
- block-obj-$(CONFIG_DMG) += dmg.o
--block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-cache.o qcow2-bitmap.o
-+block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-cache.o qcow2-bitmap.o qcow2-threads.o
- block-obj-$(CONFIG_QED) += qed.o qed-l2-cache.o qed-table.o qed-cluster.o
- block-obj-$(CONFIG_QED) += qed-check.o
- block-obj-y += vhdx.o vhdx-endian.o vhdx-log.o
-diff --git a/block/qcow2.h b/block/qcow2.h
-index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2.h
-+++ b/block/qcow2.h
-@@ -XXX,XX +XXX,XX @@ void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs,
-                                           const char *name,
-                                           Error **errp);
-+ssize_t coroutine_fn
-+qcow2_co_compress(BlockDriverState *bs, void *dest, size_t dest_size,
-+                  const void *src, size_t src_size);
-+ssize_t coroutine_fn
-+qcow2_co_decompress(BlockDriverState *bs, void *dest, size_t dest_size,
-+                    const void *src, size_t src_size);
-+
- #endif
-diff --git a/block/qcow2-threads.c b/block/qcow2-threads.c
-new file mode 100644
-index XXXXXXX..XXXXXXX
---- /dev/null
-+++ b/block/qcow2-threads.c
-@@ -XXX,XX +XXX,XX @@
-+/*
-+ * Threaded data processing for Qcow2: compression, encryption
-+ *
-+ * Copyright (c) 2004-2006 Fabrice Bellard
-+ * Copyright (c) 2018 Virtuozzo International GmbH. All rights reserved.
-+ *
-+ * 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.
-+ */
-+
-+#include "qemu/osdep.h"
-+
-+#define ZLIB_CONST
-+#include <zlib.h>
-+
-+#include "qcow2.h"
-+#include "block/thread-pool.h"
-+
-+#define MAX_COMPRESS_THREADS 4
-+
-+typedef ssize_t (*Qcow2CompressFunc)(void *dest, size_t dest_size,
-+                                     const void *src, size_t src_size);
-+typedef struct Qcow2CompressData {
-+    void *dest;
-+    size_t dest_size;
-+    const void *src;
-+    size_t src_size;
-+    ssize_t ret;
-+
-+    Qcow2CompressFunc func;
-+} Qcow2CompressData;
-+
-+/*
-+ * qcow2_compress()
-+ *
-+ * @dest - destination buffer, @dest_size bytes
-+ * @src - source buffer, @src_size bytes
-+ *
-+ * Returns: compressed size on success
-+ *          -ENOMEM destination buffer is not enough to store compressed data
-+ *          -EIO    on any other error
-+ */
-+static ssize_t qcow2_compress(void *dest, size_t dest_size,
-+                              const void *src, size_t src_size)
-+{
-+    ssize_t ret;
-+    z_stream strm;
-+
-+    /* best compression, small window, no zlib header */
-+    memset(&strm, 0, sizeof(strm));
-+    ret = deflateInit2(&strm, Z_DEFAULT_COMPRESSION, Z_DEFLATED,
-+                       -12, 9, Z_DEFAULT_STRATEGY);
-+    if (ret != Z_OK) {
-+        return -EIO;
-+    }
-+
-+    /*
-+     * strm.next_in is not const in old zlib versions, such as those used on
-+     * OpenBSD/NetBSD, so cast the const away
-+     */
-+    strm.avail_in = src_size;
-+    strm.next_in = (void *) src;
-+    strm.avail_out = dest_size;
-+    strm.next_out = dest;
-+
-+    ret = deflate(&strm, Z_FINISH);
-+    if (ret == Z_STREAM_END) {
-+        ret = dest_size - strm.avail_out;
-+    } else {
-+        ret = (ret == Z_OK ? -ENOMEM : -EIO);
-+    }
-+
-+    deflateEnd(&strm);
-+
-+    return ret;
-+}
-+
-+/*
-+ * qcow2_decompress()
-+ *
-+ * Decompress some data (not more than @src_size bytes) to produce exactly
-+ * @dest_size bytes.
-+ *
-+ * @dest - destination buffer, @dest_size bytes
-+ * @src - source buffer, @src_size bytes
-+ *
-+ * Returns: 0 on success
-+ *          -1 on fail
-+ */
-+static ssize_t qcow2_decompress(void *dest, size_t dest_size,
-+                                const void *src, size_t src_size)
-+{
-+    int ret = 0;
-+    z_stream strm;
-+
-+    memset(&strm, 0, sizeof(strm));
-+    strm.avail_in = src_size;
-+    strm.next_in = (void *) src;
-+    strm.avail_out = dest_size;
-+    strm.next_out = dest;
-+
-+    ret = inflateInit2(&strm, -12);
-+    if (ret != Z_OK) {
-+        return -1;
-+    }
-+
-+    ret = inflate(&strm, Z_FINISH);
-+    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;
-+    }
-+
-+    inflateEnd(&strm);
-+
-+    return ret;
-+}
-+
-+static int qcow2_compress_pool_func(void *opaque)
-+{
-+    Qcow2CompressData *data = opaque;
-+
-+    data->ret = data->func(data->dest, data->dest_size,
-+                           data->src, data->src_size);
-+
-+    return 0;
-+}
-+
-+static void qcow2_compress_complete(void *opaque, int ret)
-+{
-+    qemu_coroutine_enter(opaque);
-+}
-+
-+static ssize_t coroutine_fn
-+qcow2_co_do_compress(BlockDriverState *bs, void *dest, size_t dest_size,
-+                     const void *src, size_t src_size, Qcow2CompressFunc func)
-+{
-+    BDRVQcow2State *s = bs->opaque;
-+    BlockAIOCB *acb;
-+    ThreadPool *pool = aio_get_thread_pool(bdrv_get_aio_context(bs));
-+    Qcow2CompressData arg = {
-+        .dest = dest,
-+        .dest_size = dest_size,
-+        .src = src,
-+        .src_size = src_size,
-+        .func = func,
-+    };
-+
-+    while (s->nb_compress_threads >= MAX_COMPRESS_THREADS) {
-+        qemu_co_queue_wait(&s->compress_wait_queue, NULL);
-+    }
-+
-+    s->nb_compress_threads++;
-+    acb = thread_pool_submit_aio(pool, qcow2_compress_pool_func, &arg,
-+                                 qcow2_compress_complete,
-+                                 qemu_coroutine_self());
-+
-+    if (!acb) {
-+        s->nb_compress_threads--;
-+        return -EINVAL;
-+    }
-+    qemu_coroutine_yield();
-+    s->nb_compress_threads--;
-+    qemu_co_queue_next(&s->compress_wait_queue);
-+
-+    return arg.ret;
-+}
-+
-+ssize_t coroutine_fn
-+qcow2_co_compress(BlockDriverState *bs, void *dest, size_t dest_size,
-+                  const void *src, size_t src_size)
-+{
-+    return qcow2_co_do_compress(bs, dest, dest_size, src, src_size,
-+                                qcow2_compress);
-+}
-+
-+ssize_t coroutine_fn
-+qcow2_co_decompress(BlockDriverState *bs, void *dest, size_t dest_size,
-+                    const void *src, size_t src_size)
-+{
-+    return qcow2_co_do_compress(bs, dest, dest_size, src, src_size,
-+                                qcow2_decompress);
-+}
-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 @@
- #include "qemu/osdep.h"
--#define ZLIB_CONST
--#include <zlib.h>
--
- #include "block/qdict.h"
- #include "sysemu/block-backend.h"
- #include "qemu/module.h"
-@@ -XXX,XX +XXX,XX @@
- #include "qapi/qobject-input-visitor.h"
- #include "qapi/qapi-visit-block-core.h"
- #include "crypto.h"
--#include "block/thread-pool.h"
- /*
-   Differences with QCOW:
-@@ -XXX,XX +XXX,XX @@ fail:
-     return ret;
- }
--/*
-- * qcow2_compress()
-- *
-- * @dest - destination buffer, @dest_size bytes
-- * @src - source buffer, @src_size bytes
-- *
-- * Returns: compressed size on success
-- *          -ENOMEM destination buffer is not enough to store compressed data
-- *          -EIO    on any other error
-- */
--static ssize_t qcow2_compress(void *dest, size_t dest_size,
--                              const void *src, size_t src_size)
--{
--    ssize_t ret;
--    z_stream strm;
--
--    /* best compression, small window, no zlib header */
--    memset(&strm, 0, sizeof(strm));
--    ret = deflateInit2(&strm, Z_DEFAULT_COMPRESSION, Z_DEFLATED,
--                       -12, 9, Z_DEFAULT_STRATEGY);
--    if (ret != Z_OK) {
--        return -EIO;
--    }
--
--    /* strm.next_in is not const in old zlib versions, such as those used on
--     * OpenBSD/NetBSD, so cast the const away */
--    strm.avail_in = src_size;
--    strm.next_in = (void *) src;
--    strm.avail_out = dest_size;
--    strm.next_out = dest;
--
--    ret = deflate(&strm, Z_FINISH);
--    if (ret == Z_STREAM_END) {
--        ret = dest_size - strm.avail_out;
--    } else {
--        ret = (ret == Z_OK ? -ENOMEM : -EIO);
--    }
--
--    deflateEnd(&strm);
--
--    return ret;
--}
--
--/*
-- * qcow2_decompress()
-- *
-- * Decompress some data (not more than @src_size bytes) to produce exactly
-- * @dest_size bytes.
-- *
-- * @dest - destination buffer, @dest_size bytes
-- * @src - source buffer, @src_size bytes
-- *
-- * Returns: 0 on success
-- *          -1 on fail
-- */
--static ssize_t qcow2_decompress(void *dest, size_t dest_size,
--                                const void *src, size_t src_size)
--{
--    int ret = 0;
--    z_stream strm;
--
--    memset(&strm, 0, sizeof(strm));
--    strm.avail_in = src_size;
--    strm.next_in = (void *) src;
--    strm.avail_out = dest_size;
--    strm.next_out = dest;
--
--    ret = inflateInit2(&strm, -12);
--    if (ret != Z_OK) {
--        return -1;
--    }
--
--    ret = inflate(&strm, Z_FINISH);
--    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;
--    }
--
--    inflateEnd(&strm);
--
--    return ret;
--}
--
--#define MAX_COMPRESS_THREADS 4
--
--typedef ssize_t (*Qcow2CompressFunc)(void *dest, size_t dest_size,
--                                     const void *src, size_t src_size);
--typedef struct Qcow2CompressData {
--    void *dest;
--    size_t dest_size;
--    const void *src;
--    size_t src_size;
--    ssize_t ret;
--
--    Qcow2CompressFunc func;
--} Qcow2CompressData;
--
--static int qcow2_compress_pool_func(void *opaque)
--{
--    Qcow2CompressData *data = opaque;
--
--    data->ret = data->func(data->dest, data->dest_size,
--                           data->src, data->src_size);
--
--    return 0;
--}
--
--static void qcow2_compress_complete(void *opaque, int ret)
--{
--    qemu_coroutine_enter(opaque);
--}
--
--static ssize_t coroutine_fn
--qcow2_co_do_compress(BlockDriverState *bs, void *dest, size_t dest_size,
--                     const void *src, size_t src_size, Qcow2CompressFunc func)
--{
--    BDRVQcow2State *s = bs->opaque;
--    BlockAIOCB *acb;
--    ThreadPool *pool = aio_get_thread_pool(bdrv_get_aio_context(bs));
--    Qcow2CompressData arg = {
--        .dest = dest,
--        .dest_size = dest_size,
--        .src = src,
--        .src_size = src_size,
--        .func = func,
--    };
--
--    while (s->nb_compress_threads >= MAX_COMPRESS_THREADS) {
--        qemu_co_queue_wait(&s->compress_wait_queue, NULL);
--    }
--
--    s->nb_compress_threads++;
--    acb = thread_pool_submit_aio(pool, qcow2_compress_pool_func, &arg,
--                                 qcow2_compress_complete,
--                                 qemu_coroutine_self());
--
--    if (!acb) {
--        s->nb_compress_threads--;
--        return -EINVAL;
--    }
--    qemu_coroutine_yield();
--    s->nb_compress_threads--;
--    qemu_co_queue_next(&s->compress_wait_queue);
--
--    return arg.ret;
--}
--
--static ssize_t coroutine_fn
--qcow2_co_compress(BlockDriverState *bs, void *dest, size_t dest_size,
--                  const void *src, size_t src_size)
--{
--    return qcow2_co_do_compress(bs, dest, dest_size, src, src_size,
--                                qcow2_compress);
--}
--
--static ssize_t coroutine_fn
--qcow2_co_decompress(BlockDriverState *bs, void *dest, size_t dest_size,
--                    const void *src, size_t src_size)
--{
--    return qcow2_co_do_compress(bs, dest, dest_size, src, src_size,
--                                qcow2_decompress);
--}
--
- /* XXX: put compressed sectors first, then all the cluster aligned
-    tables to avoid losing bytes in alignment */
- static coroutine_fn int
---
-.21.0

-[Qemu-devel] [PULL 21/21] blockdev: loosen restrictions on drive-backup source node
+[PULL 1/8] multi-process: Initialize variables declared with g_auto*
-From: John Snow <jsnow@redhat.com>
+From: Zenghui Yu <yuzenghui@huawei.com>
-We mandate that the source node must be a root node; but there's no reason
+Quote docs/devel/style.rst (section "Automatic memory deallocation"):
 I am aware of that it needs to be restricted to such. In some cases, we need
 to make sure that there's a medium present, but in the general case we can
 allow the backup job itself to do the graph checking.
-This patch helps improve the error message when you try to backup from
+* Variables declared with g_auto* MUST always be initialized,
-the same node more than once, which is reflected in the change to test
+  otherwise the cleanup function will use uninitialized stack memory
 .
-For backups with bitmaps, it will also show a better error message that
+Initialize @name properly to get rid of the compilation error (using
-the bitmap is in use instead of giving you something cryptic like "need
+gcc-7.3.0 on CentOS):
 a root node."
-Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1707303
+../hw/remote/proxy.c: In function 'pci_proxy_dev_realize':
-Signed-off-by: John Snow <jsnow@redhat.com>
+/usr/include/glib-2.0/glib/glib-autocleanups.h:28:3: error: 'name' may be used uninitialized in this function [-Werror=maybe-uninitialized]
-Message-id: 20190521210053.8864-1-jsnow@redhat.com
+   g_free (*pp);
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+   ^~~~~~~~~~~~
 ../hw/remote/proxy.c:350:30: note: 'name' was declared here
              g_autofree char *name;
                               ^~~~
 Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
 Reviewed-by: Jagannathan Raman <jag.raman@oracle.com>
 Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
 Reviewed-by: Miroslav Rezanina <mrezanin@redhat.com>
 Message-id: 20210312112143.1369-1-yuzenghui@huawei.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- blockdev.c             | 7 ++++++-
+ hw/remote/memory.c | 5 ++---
- tests/qemu-iotests/056 | 2 +-
+ hw/remote/proxy.c  | 3 +--
-files changed, 7 insertions(+), 2 deletions(-)
+files changed, 3 insertions(+), 5 deletions(-)
-diff --git a/blockdev.c b/blockdev.c
+diff --git a/hw/remote/memory.c b/hw/remote/memory.c
 index XXXXXXX..XXXXXXX 100644
---- a/blockdev.c
+--- a/hw/remote/memory.c
-+++ b/blockdev.c
++++ b/hw/remote/memory.c
-@@ -XXX,XX +XXX,XX @@ static BlockJob *do_drive_backup(DriveBackup *backup, JobTxn *txn,
+@@ -XXX,XX +XXX,XX @@ void remote_sysmem_reconfig(MPQemuMsg *msg, Error **errp)
-         backup->compress = false;
-     }
+     remote_sysmem_reset();
--    bs = qmp_get_root_bs(backup->device, errp);
+-    for (region = 0; region < msg->num_fds; region++) {
-+    bs = bdrv_lookup_bs(backup->device, backup->device, errp);
+-        g_autofree char *name;
-     if (!bs) {
++    for (region = 0; region < msg->num_fds; region++, suffix++) {
-         return NULL;
++        g_autofree char *name = g_strdup_printf("remote-mem-%u", suffix);
-     }
+         subregion = g_new(MemoryRegion, 1);
+-        name = g_strdup_printf("remote-mem-%u", suffix++);
-+    if (!bs->drv) {
+         memory_region_init_ram_from_fd(subregion, NULL,
-+        error_setg(errp, "Device has no medium");
+                                        name, sysmem_info->sizes[region],
-+        return NULL;
+                                        true, msg->fds[region],
-+    }
+diff --git a/hw/remote/proxy.c b/hw/remote/proxy.c
-+
+index XXXXXXX..XXXXXXX 100644
-     aio_context = bdrv_get_aio_context(bs);
+--- a/hw/remote/proxy.c
-     aio_context_acquire(aio_context);
++++ b/hw/remote/proxy.c
+@@ -XXX,XX +XXX,XX @@ static void probe_pci_info(PCIDevice *dev, Error **errp)
-diff --git a/tests/qemu-iotests/056 b/tests/qemu-iotests/056
+                    PCI_BASE_ADDRESS_SPACE_IO : PCI_BASE_ADDRESS_SPACE_MEMORY;
-index XXXXXXX..XXXXXXX 100755
---- a/tests/qemu-iotests/056
+         if (size) {
-+++ b/tests/qemu-iotests/056
+-            g_autofree char *name;
-@@ -XXX,XX +XXX,XX @@ class BackupTest(iotests.QMPTestCase):
++            g_autofree char *name = g_strdup_printf("bar-region-%d", i);
-         res = self.vm.qmp('query-block-jobs')
+             pdev->region[i].dev = pdev;
-         self.assert_qmp(res, 'return[0]/status', 'concluded')
+             pdev->region[i].present = true;
-         # Leave zombie job un-dismissed, observe a failure:
+             if (type == PCI_BASE_ADDRESS_SPACE_MEMORY) {
--        res = self.qmp_backup_and_wait(serror='Need a root block node',
+                 pdev->region[i].memory = true;
-+        res = self.qmp_backup_and_wait(serror="Node 'drive0' is busy: block device is in use by block job: backup",
+             }
-                                        device='drive0', format=iotests.imgfmt,
+-            name = g_strdup_printf("bar-region-%d", i);
-                                        sync='full', target=self.dest_img,
+             memory_region_init_io(&pdev->region[i].mr, OBJECT(pdev),
-                                        auto_dismiss=False)
+                                   &proxy_mr_ops, &pdev->region[i],
                                    name, size);
 --
-.21.0
+.31.1

-[Qemu-devel] [PULL 20/21] qcow2-bitmap: initialize bitmap directory alignment
+[PULL 2/8] bitops.h: Improve find_xxx_bit() documentation
-From: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
+From: Philippe Mathieu-Daudé <philmd@redhat.com>
-Valgrind detects multiple issues in QEMU iotests when the memory is
+Document the following functions return the bitmap size
-used without being initialized. Valgrind may dump lots of unnecessary
+if no matching bit is found:
 reports what makes the memory issue analysis harder. Particularly,
 that is true for the aligned bitmap directory and can be seen while
 running the iotest #169. Padding the aligned space with zeros eases
 the pain.
-Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
+- find_first_bit
-Message-id: 1558961521-131620-1-git-send-email-andrey.shinkevich@virtuozzo.com
+- find_next_bit
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+- find_last_bit
 - find_first_zero_bit
 - find_next_zero_bit
 Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
 Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
 Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
 Message-id: 20210510200758.2623154-2-philmd@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/qcow2-bitmap.c | 2 +-
+ include/qemu/bitops.h | 15 ++++++++++++---
-file changed, 1 insertion(+), 1 deletion(-)
+file changed, 12 insertions(+), 3 deletions(-)
-diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
+diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
 index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2-bitmap.c
+--- a/include/qemu/bitops.h
-+++ b/block/qcow2-bitmap.c
++++ b/include/qemu/bitops.h
-@@ -XXX,XX +XXX,XX @@ static int bitmap_list_store(BlockDriverState *bs, Qcow2BitmapList *bm_list,
+@@ -XXX,XX +XXX,XX @@ static inline int test_bit(long nr, const unsigned long *addr)
-         dir_offset = *offset;
+  * @addr: The address to start the search at
-     }
+  * @size: The maximum size to search
+  *
--    dir = g_try_malloc(dir_size);
+- * Returns the bit number of the first set bit, or size.
-+    dir = g_try_malloc0(dir_size);
++ * Returns the bit number of the last set bit,
-     if (dir == NULL) {
++ * or @size if there is no set bit in the bitmap.
-         return -ENOMEM;
+  */
-     }
+ unsigned long find_last_bit(const unsigned long *addr,
                              unsigned long size);
@@ -XXX,XX +XXX,XX @@ unsigned long find_last_bit(const unsigned long *addr,
   * @addr: The address to base the search on
   * @offset: The bitnumber to start searching at
   * @size: The bitmap size in bits
 + *
 + * Returns the bit number of the next set bit,
 + * or @size if there are no further set bits in the bitmap.
   */
  unsigned long find_next_bit(const unsigned long *addr,
                              unsigned long size,
@@ -XXX,XX +XXX,XX @@ unsigned long find_next_bit(const unsigned long *addr,
   * @addr: The address to base the search on
   * @offset: The bitnumber to start searching at
   * @size: The bitmap size in bits
 + *
 + * Returns the bit number of the next cleared bit,
 + * or @size if there are no further clear bits in the bitmap.
   */
  unsigned long find_next_zero_bit(const unsigned long *addr,
@@ -XXX,XX +XXX,XX @@ unsigned long find_next_zero_bit(const unsigned long *addr,
   * @addr: The address to start the search at
   * @size: The maximum size to search
   *
 - * Returns the bit number of the first set bit.
 + * Returns the bit number of the first set bit,
 + * or @size if there is no set bit in the bitmap.
   */
  static inline unsigned long find_first_bit(const unsigned long *addr,
                                             unsigned long size)
@@ -XXX,XX +XXX,XX @@ static inline unsigned long find_first_bit(const unsigned long *addr,
   * @addr: The address to start the search at
   * @size: The maximum size to search
   *
 - * Returns the bit number of the first cleared bit.
 + * Returns the bit number of the first cleared bit,
 + * or @size if there is no clear bit in the bitmap.
   */
  static inline unsigned long find_first_zero_bit(const unsigned long *addr,
                                                  unsigned long size)
 --
-.21.0
+.31.1

-[Qemu-devel] [PULL 04/21] qcow2-threads: qcow2_co_do_compress: protect queuing by mutex
+[PULL 3/8] coroutine-sleep: use a stack-allocated timer
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+From: Paolo Bonzini <pbonzini@redhat.com>
-Drop dependence on AioContext lock.
+The lifetime of the timer is well-known (it cannot outlive
 qemu_co_sleep_ns_wakeable, because it's deleted by the time the
 coroutine resumes), so it is not necessary to place it on the heap.
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Reviewed-by: Alberto Garcia <berto@igalia.com>
+Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
-Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
+Message-id: 20210517100548.28806-2-pbonzini@redhat.com
-Reviewed-by: Max Reitz <mreitz@redhat.com>
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 Message-id: 20190506142741.41731-5-vsementsov@virtuozzo.com
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
- block/qcow2-threads.c | 10 +++++++---
+ util/qemu-coroutine-sleep.c | 9 ++++-----
-file changed, 7 insertions(+), 3 deletions(-)
+file changed, 4 insertions(+), 5 deletions(-)
-diff --git a/block/qcow2-threads.c b/block/qcow2-threads.c
+diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2-threads.c
+--- a/util/qemu-coroutine-sleep.c
-+++ b/block/qcow2-threads.c
++++ b/util/qemu-coroutine-sleep.c
-@@ -XXX,XX +XXX,XX @@ qcow2_co_do_compress(BlockDriverState *bs, void *dest, size_t dest_size,
+@@ -XXX,XX +XXX,XX @@ static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";
-         .func = func,
  struct QemuCoSleepState {
      Coroutine *co;
 -    QEMUTimer *ts;
 +    QEMUTimer ts;
      QemuCoSleepState **user_state_pointer;
  };
@@ -XXX,XX +XXX,XX @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
      if (sleep_state->user_state_pointer) {
          *sleep_state->user_state_pointer = NULL;
      }
 -    timer_del(sleep_state->ts);
 +    timer_del(&sleep_state->ts);
      aio_co_wake(sleep_state->co);
  }
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
      AioContext *ctx = qemu_get_current_aio_context();
      QemuCoSleepState state = {
          .co = qemu_coroutine_self(),
 -        .ts = aio_timer_new(ctx, type, SCALE_NS, co_sleep_cb, &state),
          .user_state_pointer = sleep_state,
      };
-+    qemu_co_mutex_lock(&s->lock);
+@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
-     while (s->nb_compress_threads >= MAX_COMPRESS_THREADS) {
+         abort();
 -        qemu_co_queue_wait(&s->compress_wait_queue, NULL);
 +        qemu_co_queue_wait(&s->compress_wait_queue, &s->lock);
      }
--
-     s->nb_compress_threads++;
++    aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state);
-+    qemu_co_mutex_unlock(&s->lock);
+     if (sleep_state) {
-+
+         *sleep_state = &state;
-     thread_pool_submit_co(pool, qcow2_compress_pool_func, &arg);
+     }
--    s->nb_compress_threads--;
+-    timer_mod(state.ts, qemu_clock_get_ns(type) + ns);
++    timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
-+    qemu_co_mutex_lock(&s->lock);
+     qemu_coroutine_yield();
-+    s->nb_compress_threads--;
+     if (sleep_state) {
-     qemu_co_queue_next(&s->compress_wait_queue);
+         /*
-+    qemu_co_mutex_unlock(&s->lock);
+@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
+          */
-     return arg.ret;
+         assert(*sleep_state == NULL);
      }
 -    timer_free(state.ts);
  }
 --
-.21.0
+.31.1

-[Qemu-devel] [PULL 03/21] qcow2-threads: use thread_pool_submit_co
+[PULL 4/8] coroutine-sleep: disallow NULL QemuCoSleepState** argument
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+From: Paolo Bonzini <pbonzini@redhat.com>
-Use thread_pool_submit_co, instead of reinventing it here. Note, that
+Simplify the code by removing conditionals.  qemu_co_sleep_ns
-thread_pool_submit_aio() never returns NULL, so checking it was an
+can simply point the argument to an on-stack temporary.
 extra thing.
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Reviewed-by: Alberto Garcia <berto@igalia.com>
+Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
+Message-id: 20210517100548.28806-3-pbonzini@redhat.com
-Message-id: 20190506142741.41731-4-vsementsov@virtuozzo.com
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
- block/qcow2-threads.c | 17 ++---------------
+ include/qemu/coroutine.h    |  5 +++--
-file changed, 2 insertions(+), 15 deletions(-)
+ util/qemu-coroutine-sleep.c | 18 +++++-------------
 files changed, 8 insertions(+), 15 deletions(-)
-diff --git a/block/qcow2-threads.c b/block/qcow2-threads.c
+diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
 index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2-threads.c
+--- a/include/qemu/coroutine.h
-+++ b/block/qcow2-threads.c
++++ b/include/qemu/coroutine.h
-@@ -XXX,XX +XXX,XX @@ static int qcow2_compress_pool_func(void *opaque)
+@@ -XXX,XX +XXX,XX @@ typedef struct QemuCoSleepState QemuCoSleepState;
-     return 0;
  /**
   * Yield the coroutine for a given duration. During this yield, @sleep_state
 - * (if not NULL) is set to an opaque pointer, which may be used for
 + * is set to an opaque pointer, which may be used for
   * qemu_co_sleep_wake(). Be careful, the pointer is set back to zero when the
   * timer fires. Don't save the obtained value to other variables and don't call
   * qemu_co_sleep_wake from another aio context.
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
                                              QemuCoSleepState **sleep_state);
  static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
  {
 -    qemu_co_sleep_ns_wakeable(type, ns, NULL);
 +    QemuCoSleepState *unused = NULL;
 +    qemu_co_sleep_ns_wakeable(type, ns, &unused);
  }
--static void qcow2_compress_complete(void *opaque, int ret)
+ /**
--{
+diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
--    qemu_coroutine_enter(opaque);
+index XXXXXXX..XXXXXXX 100644
--}
+--- a/util/qemu-coroutine-sleep.c
--
++++ b/util/qemu-coroutine-sleep.c
- static ssize_t coroutine_fn
+@@ -XXX,XX +XXX,XX @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
- qcow2_co_do_compress(BlockDriverState *bs, void *dest, size_t dest_size,
+                                            qemu_co_sleep_ns__scheduled, NULL);
-                      const void *src, size_t src_size, Qcow2CompressFunc func)
- {
+     assert(scheduled == qemu_co_sleep_ns__scheduled);
-     BDRVQcow2State *s = bs->opaque;
+-    if (sleep_state->user_state_pointer) {
--    BlockAIOCB *acb;
+-        *sleep_state->user_state_pointer = NULL;
-     ThreadPool *pool = aio_get_thread_pool(bdrv_get_aio_context(bs));
+-    }
-     Qcow2CompressData arg = {
++    *sleep_state->user_state_pointer = NULL;
-         .dest = dest,
+     timer_del(&sleep_state->ts);
-@@ -XXX,XX +XXX,XX @@ qcow2_co_do_compress(BlockDriverState *bs, void *dest, size_t dest_size,
+     aio_co_wake(sleep_state->co);
  }
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
      }
-     s->nb_compress_threads++;
+     aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state);
--    acb = thread_pool_submit_aio(pool, qcow2_compress_pool_func, &arg,
+-    if (sleep_state) {
--                                 qcow2_compress_complete,
+-        *sleep_state = &state;
 -                                 qemu_coroutine_self());
 -
 -    if (!acb) {
 -        s->nb_compress_threads--;
 -        return -EINVAL;
 -    }
--    qemu_coroutine_yield();
++    *sleep_state = &state;
-+    thread_pool_submit_co(pool, qcow2_compress_pool_func, &arg);
+     timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
-     s->nb_compress_threads--;
+     qemu_coroutine_yield();
 -    if (sleep_state) {
 -        /*
 -         * Note that *sleep_state is cleared during qemu_co_sleep_wake
 -         * before resuming this coroutine.
 -         */
 -        assert(*sleep_state == NULL);
 -    }
 +
-     qemu_co_queue_next(&s->compress_wait_queue);
++    /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine.  */
++    assert(*sleep_state == NULL);
-     return arg.ret;
+ }
 --
-.21.0
+.31.1

-[Qemu-devel] [PULL 05/21] qcow2-threads: split out generic path
+Deleted patch
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Move generic part out of qcow2_co_do_compress, to reuse it for
-encryption and rename things that would be shared with encryption path.
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Reviewed-by: Alberto Garcia <berto@igalia.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Message-id: 20190506142741.41731-6-vsementsov@virtuozzo.com
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block/qcow2.h         |  4 ++--
- block/qcow2-threads.c | 47 ++++++++++++++++++++++++++++---------------
- block/qcow2.c         |  2 +-
-files changed, 34 insertions(+), 19 deletions(-)
-diff --git a/block/qcow2.h b/block/qcow2.h
-index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2.h
-+++ b/block/qcow2.h
-@@ -XXX,XX +XXX,XX @@ typedef struct BDRVQcow2State {
-     char *image_backing_format;
-     char *image_data_file;
--    CoQueue compress_wait_queue;
--    int nb_compress_threads;
-+    CoQueue thread_task_queue;
-+    int nb_threads;
-     BdrvChild *data_file;
- } BDRVQcow2State;
-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 @@
- #include "qcow2.h"
- #include "block/thread-pool.h"
--#define MAX_COMPRESS_THREADS 4
-+#define QCOW2_MAX_THREADS 4
-+
-+static int coroutine_fn
-+qcow2_co_process(BlockDriverState *bs, ThreadPoolFunc *func, void *arg)
-+{
-+    int ret;
-+    BDRVQcow2State *s = bs->opaque;
-+    ThreadPool *pool = aio_get_thread_pool(bdrv_get_aio_context(bs));
-+
-+    qemu_co_mutex_lock(&s->lock);
-+    while (s->nb_threads >= QCOW2_MAX_THREADS) {
-+        qemu_co_queue_wait(&s->thread_task_queue, &s->lock);
-+    }
-+    s->nb_threads++;
-+    qemu_co_mutex_unlock(&s->lock);
-+
-+    ret = thread_pool_submit_co(pool, func, arg);
-+
-+    qemu_co_mutex_lock(&s->lock);
-+    s->nb_threads--;
-+    qemu_co_queue_next(&s->thread_task_queue);
-+    qemu_co_mutex_unlock(&s->lock);
-+
-+    return ret;
-+}
-+
-+
-+/*
-+ * Compression
-+ */
- typedef ssize_t (*Qcow2CompressFunc)(void *dest, size_t dest_size,
-                                      const void *src, size_t src_size);
-@@ -XXX,XX +XXX,XX @@ static ssize_t coroutine_fn
- qcow2_co_do_compress(BlockDriverState *bs, void *dest, size_t dest_size,
-                      const void *src, size_t src_size, Qcow2CompressFunc func)
- {
--    BDRVQcow2State *s = bs->opaque;
--    ThreadPool *pool = aio_get_thread_pool(bdrv_get_aio_context(bs));
-     Qcow2CompressData arg = {
-         .dest = dest,
-         .dest_size = dest_size,
-@@ -XXX,XX +XXX,XX @@ qcow2_co_do_compress(BlockDriverState *bs, void *dest, size_t dest_size,
-         .func = func,
-     };
--    qemu_co_mutex_lock(&s->lock);
--    while (s->nb_compress_threads >= MAX_COMPRESS_THREADS) {
--        qemu_co_queue_wait(&s->compress_wait_queue, &s->lock);
--    }
--    s->nb_compress_threads++;
--    qemu_co_mutex_unlock(&s->lock);
--
--    thread_pool_submit_co(pool, qcow2_compress_pool_func, &arg);
--
--    qemu_co_mutex_lock(&s->lock);
--    s->nb_compress_threads--;
--    qemu_co_queue_next(&s->compress_wait_queue);
--    qemu_co_mutex_unlock(&s->lock);
-+    qcow2_co_process(bs, qcow2_compress_pool_func, &arg);
-     return arg.ret;
- }
-diff --git a/block/qcow2.c b/block/qcow2.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2.c
-+++ b/block/qcow2.c
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
-     }
- #endif
--    qemu_co_queue_init(&s->compress_wait_queue);
-+    qemu_co_queue_init(&s->thread_task_queue);
-     return ret;
---
-.21.0

-[Qemu-devel] [PULL 06/21] qcow2: qcow2_co_preadv: improve locking
+Deleted patch
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Background: decryption will be done in threads, to take benefit of it,
-we should move it out of the lock first.
-But let's go further: it turns out, that only
-qcow2_get_cluster_offset() needs locking, so reduce locking to it.
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Message-id: 20190506142741.41731-7-vsementsov@virtuozzo.com
-Reviewed-by: Alberto Garcia <berto@igalia.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block/qcow2.c | 12 ++----------
-file changed, 2 insertions(+), 10 deletions(-)
-diff --git a/block/qcow2.c b/block/qcow2.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2.c
-+++ b/block/qcow2.c
-@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
-     qemu_iovec_init(&hd_qiov, qiov->niov);
--    qemu_co_mutex_lock(&s->lock);
--
-     while (bytes != 0) {
-         /* prepare next request */
-@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
-                             QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size);
-         }
-+        qemu_co_mutex_lock(&s->lock);
-         ret = qcow2_get_cluster_offset(bs, offset, &cur_bytes, &cluster_offset);
-+        qemu_co_mutex_unlock(&s->lock);
-         if (ret < 0) {
-             goto fail;
-         }
-@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
-             if (bs->backing) {
-                 BLKDBG_EVENT(bs->file, BLKDBG_READ_BACKING_AIO);
--                qemu_co_mutex_unlock(&s->lock);
-                 ret = bdrv_co_preadv(bs->backing, offset, cur_bytes,
-                                      &hd_qiov, 0);
--                qemu_co_mutex_lock(&s->lock);
-                 if (ret < 0) {
-                     goto fail;
-                 }
-@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
-             break;
-         case QCOW2_CLUSTER_COMPRESSED:
--            qemu_co_mutex_unlock(&s->lock);
-             ret = qcow2_co_preadv_compressed(bs, cluster_offset,
-                                              offset, cur_bytes,
-                                              &hd_qiov);
--            qemu_co_mutex_lock(&s->lock);
-             if (ret < 0) {
-                 goto fail;
-             }
-@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
-             }
-             BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
--            qemu_co_mutex_unlock(&s->lock);
-             ret = bdrv_co_preadv(s->data_file,
-                                  cluster_offset + offset_in_cluster,
-                                  cur_bytes, &hd_qiov, 0);
--            qemu_co_mutex_lock(&s->lock);
-             if (ret < 0) {
-                 goto fail;
-             }
-@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
-     ret = 0;
- fail:
--    qemu_co_mutex_unlock(&s->lock);
--
-     qemu_iovec_destroy(&hd_qiov);
-     qemu_vfree(cluster_data);
---
-.21.0

-[Qemu-devel] [PULL 07/21] qcow2: bdrv_co_pwritev: move encryption code out of the lock
+Deleted patch
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Encryption will be done in threads, to take benefit of it, we should
-move it out of the lock first.
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Reviewed-by: Alberto Garcia <berto@igalia.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Message-id: 20190506142741.41731-8-vsementsov@virtuozzo.com
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block/qcow2.c | 35 +++++++++++++++++++++--------------
-file changed, 21 insertions(+), 14 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_pwritev(BlockDriverState *bs, uint64_t offset,
-         ret = qcow2_alloc_cluster_offset(bs, offset, &cur_bytes,
-                                          &cluster_offset, &l2meta);
-         if (ret < 0) {
--            goto fail;
-+            goto out_locked;
-         }
-         assert((cluster_offset & 511) == 0);
-+        ret = qcow2_pre_write_overlap_check(bs, 0,
-+                                            cluster_offset + offset_in_cluster,
-+                                            cur_bytes, true);
-+        if (ret < 0) {
-+            goto out_locked;
-+        }
-+
-+        qemu_co_mutex_unlock(&s->lock);
-+
-         qemu_iovec_reset(&hd_qiov);
-         qemu_iovec_concat(&hd_qiov, qiov, bytes_done, cur_bytes);
-@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
-                                                    * s->cluster_size);
-                 if (cluster_data == NULL) {
-                     ret = -ENOMEM;
--                    goto fail;
-+                    goto out_unlocked;
-                 }
-             }
-@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
-                                       cluster_data,
-                                       cur_bytes, NULL) < 0) {
-                 ret = -EIO;
--                goto fail;
-+                goto out_unlocked;
-             }
-             qemu_iovec_reset(&hd_qiov);
-             qemu_iovec_add(&hd_qiov, cluster_data, cur_bytes);
-         }
--        ret = qcow2_pre_write_overlap_check(bs, 0,
--                cluster_offset + offset_in_cluster, cur_bytes, true);
--        if (ret < 0) {
--            goto fail;
--        }
--
-         /* If we need to do COW, check if it's possible to merge the
-          * writing of the guest data together with that of the COW regions.
-          * If it's not possible (or not necessary) then write the
-          * guest data now. */
-         if (!merge_cow(offset, cur_bytes, &hd_qiov, l2meta)) {
--            qemu_co_mutex_unlock(&s->lock);
-             BLKDBG_EVENT(bs->file, BLKDBG_WRITE_AIO);
-             trace_qcow2_writev_data(qemu_coroutine_self(),
-                                     cluster_offset + offset_in_cluster);
-             ret = bdrv_co_pwritev(s->data_file,
-                                   cluster_offset + offset_in_cluster,
-                                   cur_bytes, &hd_qiov, 0);
--            qemu_co_mutex_lock(&s->lock);
-             if (ret < 0) {
--                goto fail;
-+                goto out_unlocked;
-             }
-         }
-+        qemu_co_mutex_lock(&s->lock);
-+
-         ret = qcow2_handle_l2meta(bs, &l2meta, true);
-         if (ret) {
--            goto fail;
-+            goto out_locked;
-         }
-         bytes -= cur_bytes;
-@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
-         trace_qcow2_writev_done_part(qemu_coroutine_self(), cur_bytes);
-     }
-     ret = 0;
-+    goto out_locked;
--fail:
-+out_unlocked:
-+    qemu_co_mutex_lock(&s->lock);
-+
-+out_locked:
-     qcow2_handle_l2meta(bs, &l2meta, false);
-     qemu_co_mutex_unlock(&s->lock);
---
-.21.0

-[Qemu-devel] [PULL 08/21] qcow2: do encryption in threads
+Deleted patch
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Do encryption/decryption in threads, like it is already done for
-compression. This improves asynchronous encrypted io.
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Reviewed-by: Alberto Garcia <berto@igalia.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Message-id: 20190506142741.41731-9-vsementsov@virtuozzo.com
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block/qcow2.h         |  8 ++++++
- block/qcow2-cluster.c |  7 ++---
- block/qcow2-threads.c | 65 +++++++++++++++++++++++++++++++++++++++++--
- block/qcow2.c         | 22 +++++----------
-files changed, 81 insertions(+), 21 deletions(-)
-diff --git a/block/qcow2.h b/block/qcow2.h
-index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2.h
-+++ b/block/qcow2.h
-@@ -XXX,XX +XXX,XX @@ typedef struct Qcow2BitmapHeaderExt {
-     uint64_t bitmap_directory_offset;
- } QEMU_PACKED Qcow2BitmapHeaderExt;
-+#define QCOW2_MAX_THREADS 4
-+
- typedef struct BDRVQcow2State {
-     int cluster_bits;
-     int cluster_size;
-@@ -XXX,XX +XXX,XX @@ qcow2_co_compress(BlockDriverState *bs, void *dest, size_t dest_size,
- ssize_t coroutine_fn
- qcow2_co_decompress(BlockDriverState *bs, void *dest, size_t dest_size,
-                     const void *src, size_t src_size);
-+int coroutine_fn
-+qcow2_co_encrypt(BlockDriverState *bs, uint64_t file_cluster_offset,
-+                 uint64_t offset, void *buf, size_t len);
-+int coroutine_fn
-+qcow2_co_decrypt(BlockDriverState *bs, uint64_t file_cluster_offset,
-+                 uint64_t offset, void *buf, size_t len);
- #endif
-diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2-cluster.c
-+++ b/block/qcow2-cluster.c
-@@ -XXX,XX +XXX,XX @@ static bool coroutine_fn do_perform_cow_encrypt(BlockDriverState *bs,
- {
-     if (bytes && bs->encrypted) {
-         BDRVQcow2State *s = bs->opaque;
--        int64_t offset = (s->crypt_physical_offset ?
--                          (cluster_offset + offset_in_cluster) :
--                          (src_cluster_offset + offset_in_cluster));
-         assert((offset_in_cluster & ~BDRV_SECTOR_MASK) == 0);
-         assert((bytes & ~BDRV_SECTOR_MASK) == 0);
-         assert(s->crypto);
--        if (qcrypto_block_encrypt(s->crypto, offset, buffer, bytes, NULL) < 0) {
-+        if (qcow2_co_encrypt(bs, cluster_offset,
-+                             src_cluster_offset + offset_in_cluster,
-+                             buffer, bytes) < 0) {
-             return false;
-         }
-     }
-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 @@
- #include "qcow2.h"
- #include "block/thread-pool.h"
--
--#define QCOW2_MAX_THREADS 4
-+#include "crypto.h"
- static int coroutine_fn
- qcow2_co_process(BlockDriverState *bs, ThreadPoolFunc *func, void *arg)
-@@ -XXX,XX +XXX,XX @@ qcow2_co_decompress(BlockDriverState *bs, void *dest, size_t dest_size,
-     return qcow2_co_do_compress(bs, dest, dest_size, src, src_size,
-                                 qcow2_decompress);
- }
-+
-+
-+/*
-+ * Cryptography
-+ */
-+
-+/*
-+ * Qcow2EncDecFunc: common prototype of qcrypto_block_encrypt() and
-+ * qcrypto_block_decrypt() functions.
-+ */
-+typedef int (*Qcow2EncDecFunc)(QCryptoBlock *block, uint64_t offset,
-+                               uint8_t *buf, size_t len, Error **errp);
-+
-+typedef struct Qcow2EncDecData {
-+    QCryptoBlock *block;
-+    uint64_t offset;
-+    uint8_t *buf;
-+    size_t len;
-+
-+    Qcow2EncDecFunc func;
-+} Qcow2EncDecData;
-+
-+static int qcow2_encdec_pool_func(void *opaque)
-+{
-+    Qcow2EncDecData *data = opaque;
-+
-+    return data->func(data->block, data->offset, data->buf, data->len, NULL);
-+}
-+
-+static int coroutine_fn
-+qcow2_co_encdec(BlockDriverState *bs, uint64_t file_cluster_offset,
-+                  uint64_t offset, void *buf, size_t len, Qcow2EncDecFunc func)
-+{
-+    BDRVQcow2State *s = bs->opaque;
-+    Qcow2EncDecData arg = {
-+        .block = s->crypto,
-+        .offset = s->crypt_physical_offset ?
-+                      file_cluster_offset + offset_into_cluster(s, offset) :
-+                      offset,
-+        .buf = buf,
-+        .len = len,
-+        .func = func,
-+    };
-+
-+    return qcow2_co_process(bs, qcow2_encdec_pool_func, &arg);
-+}
-+
-+int coroutine_fn
-+qcow2_co_encrypt(BlockDriverState *bs, uint64_t file_cluster_offset,
-+                 uint64_t offset, void *buf, size_t len)
-+{
-+    return qcow2_co_encdec(bs, file_cluster_offset, offset, buf, len,
-+                             qcrypto_block_encrypt);
-+}
-+
-+int coroutine_fn
-+qcow2_co_decrypt(BlockDriverState *bs, uint64_t file_cluster_offset,
-+                 uint64_t offset, void *buf, size_t len)
-+{
-+    return qcow2_co_encdec(bs, file_cluster_offset, offset, buf, len,
-+                             qcrypto_block_decrypt);
-+}
-diff --git a/block/qcow2.c b/block/qcow2.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2.c
-+++ b/block/qcow2.c
-@@ -XXX,XX +XXX,XX @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
-             }
-             s->crypto = qcrypto_block_open(s->crypto_opts, "encrypt.",
-                                            qcow2_crypto_hdr_read_func,
--                                           bs, cflags, 1, errp);
-+                                           bs, cflags, QCOW2_MAX_THREADS, errp);
-             if (!s->crypto) {
-                 return -EINVAL;
-             }
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
-                 cflags |= QCRYPTO_BLOCK_OPEN_NO_IO;
-             }
-             s->crypto = qcrypto_block_open(s->crypto_opts, "encrypt.",
--                                           NULL, NULL, cflags, 1, errp);
-+                                           NULL, NULL, cflags,
-+                                           QCOW2_MAX_THREADS, errp);
-             if (!s->crypto) {
-                 ret = -EINVAL;
-                 goto fail;
-@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
-                 assert(s->crypto);
-                 assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
-                 assert((cur_bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
--                if (qcrypto_block_decrypt(s->crypto,
--                                          (s->crypt_physical_offset ?
--                                           cluster_offset + offset_in_cluster :
--                                           offset),
--                                          cluster_data,
--                                          cur_bytes,
--                                          NULL) < 0) {
-+                if (qcow2_co_decrypt(bs, cluster_offset, offset,
-+                                     cluster_data, cur_bytes) < 0) {
-                     ret = -EIO;
-                     goto fail;
-                 }
-@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
-                    QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size);
-             qemu_iovec_to_buf(&hd_qiov, 0, cluster_data, hd_qiov.size);
--            if (qcrypto_block_encrypt(s->crypto,
--                                      (s->crypt_physical_offset ?
--                                       cluster_offset + offset_in_cluster :
--                                       offset),
--                                      cluster_data,
--                                      cur_bytes, NULL) < 0) {
-+            if (qcow2_co_encrypt(bs, cluster_offset, offset,
-+                                 cluster_data, cur_bytes) < 0) {
-                 ret = -EIO;
-                 goto out_unlocked;
-             }
---
-.21.0

-[Qemu-devel] [PULL 09/21] block/backup: simplify backup_incremental_init_copy_bitmap
+Deleted patch
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Simplify backup_incremental_init_copy_bitmap using the function
-bdrv_dirty_bitmap_next_dirty_area.
-Note: move to job->len instead of bitmap size: it should not matter but
-less code.
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Message-id: 20190429090842.57910-2-vsementsov@virtuozzo.com
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block/backup.c | 40 ++++++++++++----------------------------
-file changed, 12 insertions(+), 28 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 int coroutine_fn backup_run_incremental(BackupBlockJob *job)
- /* init copy_bitmap from sync_bitmap */
- static void backup_incremental_init_copy_bitmap(BackupBlockJob *job)
- {
--    BdrvDirtyBitmapIter *dbi;
--    int64_t offset;
--    int64_t end = DIV_ROUND_UP(bdrv_dirty_bitmap_size(job->sync_bitmap),
--                               job->cluster_size);
--
--    dbi = bdrv_dirty_iter_new(job->sync_bitmap);
--    while ((offset = bdrv_dirty_iter_next(dbi)) != -1) {
--        int64_t cluster = offset / job->cluster_size;
--        int64_t next_cluster;
--
--        offset += bdrv_dirty_bitmap_granularity(job->sync_bitmap);
--        if (offset >= bdrv_dirty_bitmap_size(job->sync_bitmap)) {
--            hbitmap_set(job->copy_bitmap, cluster, end - cluster);
--            break;
--        }
-+    uint64_t offset = 0;
-+    uint64_t bytes = job->len;
--        offset = bdrv_dirty_bitmap_next_zero(job->sync_bitmap, offset,
--                                             UINT64_MAX);
--        if (offset == -1) {
--            hbitmap_set(job->copy_bitmap, cluster, end - cluster);
--            break;
--        }
-+    while (bdrv_dirty_bitmap_next_dirty_area(job->sync_bitmap,
-+                                             &offset, &bytes))
-+    {
-+        uint64_t cluster = offset / job->cluster_size;
-+        uint64_t end_cluster = DIV_ROUND_UP(offset + bytes, job->cluster_size);
--        next_cluster = DIV_ROUND_UP(offset, job->cluster_size);
--        hbitmap_set(job->copy_bitmap, cluster, next_cluster - cluster);
--        if (next_cluster >= end) {
-+        hbitmap_set(job->copy_bitmap, cluster, end_cluster - cluster);
-+
-+        offset = end_cluster * job->cluster_size;
-+        if (offset >= job->len) {
-             break;
-         }
--
--        bdrv_set_dirty_iter(dbi, next_cluster * job->cluster_size);
-+        bytes = job->len - offset;
-     }
-     /* TODO job_progress_set_remaining() would make more sense */
-     job_progress_update(&job->common.job,
-         job->len - hbitmap_count(job->copy_bitmap) * job->cluster_size);
--
--    bdrv_dirty_iter_free(dbi);
- }
- static int coroutine_fn backup_run(Job *job, Error **errp)
---
-.21.0

-[Qemu-devel] [PULL 10/21] block/backup: move to copy_bitmap with granularity
+[PULL 5/8] coroutine-sleep: allow qemu_co_sleep_wake that wakes nothing
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+From: Paolo Bonzini <pbonzini@redhat.com>
-We are going to share this bitmap between backup and backup-top filter
+All callers of qemu_co_sleep_wake are checking whether they are passing
-driver, so let's share something more meaningful. It also simplifies
+a NULL argument inside the pointer-to-pointer: do the check in
-some calculations.
+qemu_co_sleep_wake itself.
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+As a side effect, qemu_co_sleep_wake can be called more than once and
-Reviewed-by: Max Reitz <mreitz@redhat.com>
+it will only wake the coroutine once; after the first time, the argument
-Message-id: 20190429090842.57910-3-vsementsov@virtuozzo.com
+will be set to NULL via *sleep_state->user_state_pointer.  However, this
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+would not be safe unless co_sleep_cb keeps using the QemuCoSleepState*
 directly, so make it go through the pointer-to-pointer instead.
 Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
 Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
 Message-id: 20210517100548.28806-4-pbonzini@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/backup.c | 48 +++++++++++++++++++++++-------------------------
+ block/block-copy.c          |  4 +---
-file changed, 23 insertions(+), 25 deletions(-)
+ block/nbd.c                 |  8 ++------
  util/qemu-coroutine-sleep.c | 21 ++++++++++++---------
 files changed, 15 insertions(+), 18 deletions(-)
-diff --git a/block/backup.c b/block/backup.c
+diff --git a/block/block-copy.c b/block/block-copy.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/backup.c
+--- a/block/block-copy.c
-+++ b/block/backup.c
++++ b/block/block-copy.c
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_cow_with_bounce_buffer(BackupBlockJob *job,
+@@ -XXX,XX +XXX,XX @@ out:
-     int read_flags = is_write_notifier ? BDRV_REQ_NO_SERIALISING : 0;
-     int write_flags = job->serialize_target_writes ? BDRV_REQ_SERIALISING : 0;
+ void block_copy_kick(BlockCopyCallState *call_state)
+ {
--    hbitmap_reset(job->copy_bitmap, start / job->cluster_size, 1);
+-    if (call_state->sleep_state) {
-+    assert(QEMU_IS_ALIGNED(start, job->cluster_size));
+-        qemu_co_sleep_wake(call_state->sleep_state);
-+    hbitmap_reset(job->copy_bitmap, start, job->cluster_size);
+-    }
-     nbytes = MIN(job->cluster_size, job->len - start);
++    qemu_co_sleep_wake(call_state->sleep_state);
      if (!*bounce_buffer) {
          *bounce_buffer = blk_blockalign(blk, job->cluster_size);
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_cow_with_bounce_buffer(BackupBlockJob *job,
      return nbytes;
  fail:
 -    hbitmap_set(job->copy_bitmap, start / job->cluster_size, 1);
 +    hbitmap_set(job->copy_bitmap, start, job->cluster_size);
      return ret;
  }
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_cow_with_offload(BackupBlockJob *job,
-     int write_flags = job->serialize_target_writes ? BDRV_REQ_SERIALISING : 0;
+ /*
+diff --git a/block/nbd.c b/block/nbd.c
-     assert(QEMU_IS_ALIGNED(job->copy_range_size, job->cluster_size));
+index XXXXXXX..XXXXXXX 100644
-+    assert(QEMU_IS_ALIGNED(start, job->cluster_size));
+--- a/block/nbd.c
-     nbytes = MIN(job->copy_range_size, end - start);
++++ b/block/nbd.c
-     nr_clusters = DIV_ROUND_UP(nbytes, job->cluster_size);
+@@ -XXX,XX +XXX,XX @@ static void coroutine_fn nbd_client_co_drain_begin(BlockDriverState *bs)
--    hbitmap_reset(job->copy_bitmap, start / job->cluster_size,
+     BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
--                  nr_clusters);
-+    hbitmap_reset(job->copy_bitmap, start, job->cluster_size * nr_clusters);
+     s->drained = true;
-     ret = blk_co_copy_range(blk, start, job->target, start, nbytes,
+-    if (s->connection_co_sleep_ns_state) {
-                             read_flags, write_flags);
+-        qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
-     if (ret < 0) {
+-    }
-         trace_backup_do_cow_copy_range_fail(job, start, ret);
++    qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
--        hbitmap_set(job->copy_bitmap, start / job->cluster_size,
--                    nr_clusters);
+     nbd_co_establish_connection_cancel(bs, false);
-+        hbitmap_set(job->copy_bitmap, start, job->cluster_size * nr_clusters);
-         return ret;
+@@ -XXX,XX +XXX,XX @@ static void nbd_teardown_connection(BlockDriverState *bs)
      s->state = NBD_CLIENT_QUIT;
      if (s->connection_co) {
 -        if (s->connection_co_sleep_ns_state) {
 -            qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
 -        }
 +        qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
          nbd_co_establish_connection_cancel(bs, true);
      }
+     if (qemu_in_coroutine()) {
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
+diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
-     cow_request_begin(&cow_request, job, start, end);
+index XXXXXXX..XXXXXXX 100644
+--- a/util/qemu-coroutine-sleep.c
-     while (start < end) {
++++ b/util/qemu-coroutine-sleep.c
--        if (!hbitmap_get(job->copy_bitmap, start / job->cluster_size)) {
+@@ -XXX,XX +XXX,XX @@ struct QemuCoSleepState {
-+        if (!hbitmap_get(job->copy_bitmap, start)) {
-             trace_backup_do_cow_skip(job, start);
+ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
-             start += job->cluster_size;
+ {
-             continue; /* already copied */
+-    /* Write of schedule protected by barrier write in aio_co_schedule */
-@@ -XXX,XX +XXX,XX @@ static void backup_clean(Job *job)
+-    const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled,
-     assert(s->target);
+-                                           qemu_co_sleep_ns__scheduled, NULL);
-     blk_unref(s->target);
++    if (sleep_state) {
-     s->target = NULL;
++        /* Write of schedule protected by barrier write in aio_co_schedule */
-+
++        const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled,
-+    if (s->copy_bitmap) {
++                                                qemu_co_sleep_ns__scheduled, NULL);
-+        hbitmap_free(s->copy_bitmap);
-+        s->copy_bitmap = NULL;
+-    assert(scheduled == qemu_co_sleep_ns__scheduled);
 -    *sleep_state->user_state_pointer = NULL;
 -    timer_del(&sleep_state->ts);
 -    aio_co_wake(sleep_state->co);
 +        assert(scheduled == qemu_co_sleep_ns__scheduled);
 +        *sleep_state->user_state_pointer = NULL;
 +        timer_del(&sleep_state->ts);
 +        aio_co_wake(sleep_state->co);
 +    }
  }
- void backup_do_checkpoint(BlockJob *job, Error **errp)
+ static void co_sleep_cb(void *opaque)
  {
-     BackupBlockJob *backup_job = container_of(job, BackupBlockJob, common);
+-    qemu_co_sleep_wake(opaque);
--    int64_t len;
++    QemuCoSleepState **sleep_state = opaque;
++    qemu_co_sleep_wake(*sleep_state);
-     assert(block_job_driver(job) == &backup_job_driver);
+ }
-@@ -XXX,XX +XXX,XX @@ void backup_do_checkpoint(BlockJob *job, Error **errp)
+ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
-         return;
+@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
          abort();
      }
--    len = DIV_ROUND_UP(backup_job->len, backup_job->cluster_size);
+-    aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state);
--    hbitmap_set(backup_job->copy_bitmap, 0, len);
++    aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, sleep_state);
-+    hbitmap_set(backup_job->copy_bitmap, 0, backup_job->len);
+     *sleep_state = &state;
- }
+     timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
+     qemu_coroutine_yield();
  static void backup_drain(BlockJob *job)
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
  {
      int ret;
      bool error_is_read;
 -    int64_t cluster;
 +    int64_t offset;
      HBitmapIter hbi;
      hbitmap_iter_init(&hbi, job->copy_bitmap, 0);
 -    while ((cluster = hbitmap_iter_next(&hbi)) != -1) {
 +    while ((offset = hbitmap_iter_next(&hbi)) != -1) {
          do {
              if (yield_and_check(job)) {
                  return 0;
              }
 -            ret = backup_do_cow(job, cluster * job->cluster_size,
 +            ret = backup_do_cow(job, offset,
                                  job->cluster_size, &error_is_read, false);
              if (ret < 0 && backup_error_action(job, error_is_read, -ret) ==
                             BLOCK_ERROR_ACTION_REPORT)
@@ -XXX,XX +XXX,XX @@ static void backup_incremental_init_copy_bitmap(BackupBlockJob *job)
      while (bdrv_dirty_bitmap_next_dirty_area(job->sync_bitmap,
                                               &offset, &bytes))
      {
 -        uint64_t cluster = offset / job->cluster_size;
 -        uint64_t end_cluster = DIV_ROUND_UP(offset + bytes, job->cluster_size);
 +        hbitmap_set(job->copy_bitmap, offset, bytes);
 -        hbitmap_set(job->copy_bitmap, cluster, end_cluster - cluster);
 -
 -        offset = end_cluster * job->cluster_size;
 +        offset += bytes;
          if (offset >= job->len) {
              break;
          }
@@ -XXX,XX +XXX,XX @@ static void backup_incremental_init_copy_bitmap(BackupBlockJob *job)
      /* TODO job_progress_set_remaining() would make more sense */
      job_progress_update(&job->common.job,
 -        job->len - hbitmap_count(job->copy_bitmap) * job->cluster_size);
 +        job->len - hbitmap_count(job->copy_bitmap));
  }
  static int coroutine_fn backup_run(Job *job, Error **errp)
  {
      BackupBlockJob *s = container_of(job, BackupBlockJob, common.job);
      BlockDriverState *bs = blk_bs(s->common.blk);
 -    int64_t offset, nb_clusters;
 +    int64_t offset;
      int ret = 0;
      QLIST_INIT(&s->inflight_reqs);
      qemu_co_rwlock_init(&s->flush_rwlock);
 -    nb_clusters = DIV_ROUND_UP(s->len, s->cluster_size);
      job_progress_set_remaining(job, s->len);
 -    s->copy_bitmap = hbitmap_alloc(nb_clusters, 0);
      if (s->sync_mode == MIRROR_SYNC_MODE_INCREMENTAL) {
          backup_incremental_init_copy_bitmap(s);
      } else {
 -        hbitmap_set(s->copy_bitmap, 0, nb_clusters);
 +        hbitmap_set(s->copy_bitmap, 0, s->len);
      }
 -
      s->before_write.notify = backup_before_write_notify;
      bdrv_add_before_write_notifier(bs, &s->before_write);
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_run(Job *job, Error **errp)
      /* wait until pending backup_do_cow() calls have completed */
      qemu_co_rwlock_wrlock(&s->flush_rwlock);
      qemu_co_rwlock_unlock(&s->flush_rwlock);
 -    hbitmap_free(s->copy_bitmap);
      return ret;
  }
@@ -XXX,XX +XXX,XX @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
      } else {
          job->cluster_size = MAX(BACKUP_CLUSTER_SIZE_DEFAULT, bdi.cluster_size);
      }
 +
 +    job->copy_bitmap = hbitmap_alloc(len, ctz32(job->cluster_size));
      job->use_copy_range = true;
      job->copy_range_size = MIN_NON_ZERO(blk_get_max_transfer(job->common.blk),
                                          blk_get_max_transfer(job->target));
 --
-.21.0
+.31.1

-[Qemu-devel] [PULL 11/21] block/backup: refactor and tolerate unallocated cluster skipping
+Deleted patch
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Split allocation checking to separate function and reduce nesting.
-Consider bdrv_is_allocated() fail as allocated area, as copying more
-than needed is not wrong (and we do it anyway) and seems better than
-fail the whole job. And, most probably we will fail on the next read,
-if there are real problem with source.
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Message-id: 20190429090842.57910-4-vsementsov@virtuozzo.com
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block/backup.c | 60 +++++++++++++++++++-------------------------------
-file changed, 23 insertions(+), 37 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 bool coroutine_fn yield_and_check(BackupBlockJob *job)
-     return false;
- }
-+static bool bdrv_is_unallocated_range(BlockDriverState *bs,
-+                                      int64_t offset, int64_t bytes)
-+{
-+    int64_t end = offset + bytes;
-+
-+    while (offset < end && !bdrv_is_allocated(bs, offset, bytes, &bytes)) {
-+        if (bytes == 0) {
-+            return true;
-+        }
-+        offset += bytes;
-+        bytes = end - offset;
-+    }
-+
-+    return offset >= end;
-+}
-+
- static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
- {
-     int ret;
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_run(Job *job, Error **errp)
-         for (offset = 0; offset < s->len;
-              offset += s->cluster_size) {
-             bool error_is_read;
--            int alloced = 0;
-             if (yield_and_check(s)) {
-                 break;
-             }
--            if (s->sync_mode == MIRROR_SYNC_MODE_TOP) {
--                int i;
--                int64_t n;
--
--                /* Check to see if these blocks are already in the
--                 * backing file. */
--
--                for (i = 0; i < s->cluster_size;) {
--                    /* bdrv_is_allocated() only returns true/false based
--                     * on the first set of sectors it comes across that
--                     * are are all in the same state.
--                     * For that reason we must verify each sector in the
--                     * backup cluster length.  We end up copying more than
--                     * needed but at some point that is always the case. */
--                    alloced =
--                        bdrv_is_allocated(bs, offset + i,
--                                          s->cluster_size - i, &n);
--                    i += n;
--
--                    if (alloced || n == 0) {
--                        break;
--                    }
--                }
--
--                /* If the above loop never found any sectors that are in
--                 * the topmost image, skip this backup. */
--                if (alloced == 0) {
--                    continue;
--                }
--            }
--            /* FULL sync mode we copy the whole drive. */
--            if (alloced < 0) {
--                ret = alloced;
--            } else {
--                ret = backup_do_cow(s, offset, s->cluster_size,
--                                    &error_is_read, false);
-+            if (s->sync_mode == MIRROR_SYNC_MODE_TOP &&
-+                bdrv_is_unallocated_range(bs, offset, s->cluster_size))
-+            {
-+                continue;
-             }
-+
-+            ret = backup_do_cow(s, offset, s->cluster_size,
-+                                &error_is_read, false);
-             if (ret < 0) {
-                 /* Depending on error action, fail now or retry cluster */
-                 BlockErrorAction action =
---
-.21.0

-[Qemu-devel] [PULL 12/21] block/backup: unify different modes code path
+Deleted patch
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Do full, top and incremental mode copying all in one place. This
-unifies the code path and helps further improvements.
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Message-id: 20190429090842.57910-5-vsementsov@virtuozzo.com
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block/backup.c | 43 ++++++++++---------------------------------
-file changed, 10 insertions(+), 33 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 bool bdrv_is_unallocated_range(BlockDriverState *bs,
-     return offset >= end;
- }
--static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
-+static int coroutine_fn backup_loop(BackupBlockJob *job)
- {
-     int ret;
-     bool error_is_read;
-     int64_t offset;
-     HBitmapIter hbi;
-+    BlockDriverState *bs = blk_bs(job->common.blk);
-     hbitmap_iter_init(&hbi, job->copy_bitmap, 0);
-     while ((offset = hbitmap_iter_next(&hbi)) != -1) {
-+        if (job->sync_mode == MIRROR_SYNC_MODE_TOP &&
-+            bdrv_is_unallocated_range(bs, offset, job->cluster_size))
-+        {
-+            hbitmap_reset(job->copy_bitmap, offset, job->cluster_size);
-+            continue;
-+        }
-+
-         do {
-             if (yield_and_check(job)) {
-                 return 0;
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_run(Job *job, Error **errp)
- {
-     BackupBlockJob *s = container_of(job, BackupBlockJob, common.job);
-     BlockDriverState *bs = blk_bs(s->common.blk);
--    int64_t offset;
-     int ret = 0;
-     QLIST_INIT(&s->inflight_reqs);
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_run(Job *job, Error **errp)
-              * notify callback service CoW requests. */
-             job_yield(job);
-         }
--    } else if (s->sync_mode == MIRROR_SYNC_MODE_INCREMENTAL) {
--        ret = backup_run_incremental(s);
-     } else {
--        /* Both FULL and TOP SYNC_MODE's require copying.. */
--        for (offset = 0; offset < s->len;
--             offset += s->cluster_size) {
--            bool error_is_read;
--
--            if (yield_and_check(s)) {
--                break;
--            }
--
--            if (s->sync_mode == MIRROR_SYNC_MODE_TOP &&
--                bdrv_is_unallocated_range(bs, offset, s->cluster_size))
--            {
--                continue;
--            }
--
--            ret = backup_do_cow(s, offset, s->cluster_size,
--                                &error_is_read, false);
--            if (ret < 0) {
--                /* Depending on error action, fail now or retry cluster */
--                BlockErrorAction action =
--                    backup_error_action(s, error_is_read, -ret);
--                if (action == BLOCK_ERROR_ACTION_REPORT) {
--                    break;
--                } else {
--                    offset -= s->cluster_size;
--                    continue;
--                }
--            }
--        }
-+        ret = backup_loop(s);
-     }
-     notifier_with_return_remove(&s->before_write);
---
-.21.0

-[Qemu-devel] [PULL 15/21] block: Make bdrv_root_attach_child() unref child_bs on failure
+[PULL 6/8] coroutine-sleep: move timer out of QemuCoSleepState
-From: Alberto Garcia <berto@igalia.com>
+From: Paolo Bonzini <pbonzini@redhat.com>
-A consequence of the previous patch is that bdrv_attach_child()
+This simplification is enabled by the previous patch.  Now aio_co_wake
-transfers the reference to child_bs from the caller to parent_bs,
+will only be called once, therefore we do not care about a spurious
-which will drop it on bdrv_close() or when someone calls
+firing of the timer after a qemu_co_sleep_wake.
 bdrv_unref_child().
-But this only happens when bdrv_attach_child() succeeds. If it fails
+Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-then the caller is responsible for dropping the reference to child_bs.
+Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
 Message-id: 20210517100548.28806-5-pbonzini@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
  util/qemu-coroutine-sleep.c | 8 ++++----
 file changed, 4 insertions(+), 4 deletions(-)
-This patch makes bdrv_attach_child() take the reference also when
+diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
 there is an error, freeing the caller for having to do it.
 A similar situation happens with bdrv_root_attach_child(), so the
 changes on this patch affect both functions.
 Signed-off-by: Alberto Garcia <berto@igalia.com>
 Message-id: 20dfb3d9ccec559cdd1a9690146abad5d204a186.1557754872.git.berto@igalia.com
 [mreitz: Removed now superfluous BdrvChild * variable in
          bdrv_open_child()]
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  block.c               | 30 ++++++++++++++++++------------
  block/block-backend.c |  3 +--
  block/quorum.c        |  1 -
  blockjob.c            |  2 +-
 files changed, 20 insertions(+), 16 deletions(-)
 diff --git a/block.c b/block.c
 index XXXXXXX..XXXXXXX 100644
---- a/block.c
+--- a/util/qemu-coroutine-sleep.c
-+++ b/block.c
++++ b/util/qemu-coroutine-sleep.c
-@@ -XXX,XX +XXX,XX @@ static void bdrv_replace_child(BdrvChild *child, BlockDriverState *new_bs)
+@@ -XXX,XX +XXX,XX @@ static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";
  struct QemuCoSleepState {
      Coroutine *co;
 -    QEMUTimer ts;
      QemuCoSleepState **user_state_pointer;
  };
@@ -XXX,XX +XXX,XX @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
          assert(scheduled == qemu_co_sleep_ns__scheduled);
          *sleep_state->user_state_pointer = NULL;
 -        timer_del(&sleep_state->ts);
          aio_co_wake(sleep_state->co);
      }
  }
+@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
-+/*
+                                             QemuCoSleepState **sleep_state)
-+ * This function steals the reference to child_bs from the caller.
+ {
-+ * That reference is later dropped by bdrv_root_unref_child().
+     AioContext *ctx = qemu_get_current_aio_context();
-+ *
++    QEMUTimer ts;
-+ * On failure NULL is returned, errp is set and the reference to
+     QemuCoSleepState state = {
-+ * child_bs is also dropped.
+         .co = qemu_coroutine_self(),
-+ */
+         .user_state_pointer = sleep_state,
- BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
+@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
-                                   const char *child_name,
+         abort();
                                    const BdrvChildRole *child_role,
@@ -XXX,XX +XXX,XX @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
      ret = bdrv_check_update_perm(child_bs, NULL, perm, shared_perm, NULL, errp);
      if (ret < 0) {
          bdrv_abort_perm_update(child_bs);
 +        bdrv_unref(child_bs);
          return NULL;
      }
-@@ -XXX,XX +XXX,XX @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
+-    aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, sleep_state);
-     return child;
++    aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, sleep_state);
- }
+     *sleep_state = &state;
+-    timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
-+/*
++    timer_mod(&ts, qemu_clock_get_ns(type) + ns);
-+ * This function transfers the reference to child_bs from the caller
+     qemu_coroutine_yield();
-+ * to parent_bs. That reference is later dropped by parent_bs on
++    timer_del(&ts);
-+ * bdrv_close() or if someone calls bdrv_unref_child().
-+ *
+     /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine.  */
-+ * On failure NULL is returned, errp is set and the reference to
+     assert(*sleep_state == NULL);
 + * child_bs is also dropped.
 + */
  BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
                               BlockDriverState *child_bs,
                               const char *child_name,
@@ -XXX,XX +XXX,XX @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
      /* If backing_hd was already part of bs's backing chain, and
       * inherits_from pointed recursively to bs then let's update it to
       * point directly to bs (else it will become NULL). */
 -    if (update_inherits_from) {
 +    if (bs->backing && update_inherits_from) {
          backing_hd->inherits_from = bs;
      }
 -    if (!bs->backing) {
 -        bdrv_unref(backing_hd);
 -    }
  out:
      bdrv_refresh_limits(bs, NULL);
@@ -XXX,XX +XXX,XX @@ BdrvChild *bdrv_open_child(const char *filename,
                             const BdrvChildRole *child_role,
                             bool allow_none, Error **errp)
  {
 -    BdrvChild *c;
      BlockDriverState *bs;
      bs = bdrv_open_child_bs(filename, options, bdref_key, parent, child_role,
@@ -XXX,XX +XXX,XX @@ BdrvChild *bdrv_open_child(const char *filename,
          return NULL;
      }
 -    c = bdrv_attach_child(parent, bs, bdref_key, child_role, errp);
 -    if (!c) {
 -        bdrv_unref(bs);
 -        return NULL;
 -    }
 -
 -    return c;
 +    return bdrv_attach_child(parent, bs, bdref_key, child_role, errp);
  }
  /* TODO Future callers may need to specify parent/child_role in order for
 diff --git a/block/block-backend.c b/block/block-backend.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/block-backend.c
 +++ b/block/block-backend.c
@@ -XXX,XX +XXX,XX @@ BlockBackend *blk_new_open(const char *filename, const char *reference,
      blk->root = bdrv_root_attach_child(bs, "root", &child_root,
                                         perm, BLK_PERM_ALL, blk, errp);
      if (!blk->root) {
 -        bdrv_unref(bs);
          blk_unref(blk);
          return NULL;
      }
@@ -XXX,XX +XXX,XX @@ void blk_remove_bs(BlockBackend *blk)
  int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp)
  {
      ThrottleGroupMember *tgm = &blk->public.throttle_group_member;
 +    bdrv_ref(bs);
      blk->root = bdrv_root_attach_child(bs, "root", &child_root,
                                         blk->perm, blk->shared_perm, blk, errp);
      if (blk->root == NULL) {
          return -EPERM;
      }
 -    bdrv_ref(bs);
      notifier_list_notify(&blk->insert_bs_notifiers, blk);
      if (tgm->throttle_state) {
 diff --git a/block/quorum.c b/block/quorum.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/quorum.c
 +++ b/block/quorum.c
@@ -XXX,XX +XXX,XX @@ static void quorum_add_child(BlockDriverState *bs, BlockDriverState *child_bs,
      child = bdrv_attach_child(bs, child_bs, indexstr, &child_format, errp);
      if (child == NULL) {
          s->next_child_index--;
 -        bdrv_unref(child_bs);
          goto out;
      }
      s->children = g_renew(BdrvChild *, s->children, s->num_children + 1);
 diff --git a/blockjob.c b/blockjob.c
 index XXXXXXX..XXXXXXX 100644
 --- a/blockjob.c
 +++ b/blockjob.c
@@ -XXX,XX +XXX,XX @@ int block_job_add_bdrv(BlockJob *job, const char *name, BlockDriverState *bs,
  {
      BdrvChild *c;
 +    bdrv_ref(bs);
      c = bdrv_root_attach_child(bs, name, &child_job, perm, shared_perm,
                                 job, errp);
      if (c == NULL) {
@@ -XXX,XX +XXX,XX @@ int block_job_add_bdrv(BlockJob *job, const char *name, BlockDriverState *bs,
      }
      job->nodes = g_slist_prepend(job->nodes, c);
 -    bdrv_ref(bs);
      bdrv_op_block_all(bs, job->blocker);
      return 0;
 --
-.21.0
+.31.1

-[Qemu-devel] [PULL 17/21] qemu-img: rebase: Reduce reads on in-chain rebase
+[PULL 7/8] coroutine-sleep: replace QemuCoSleepState pointer with struct in the API
-From: Sam Eiderman <shmuel.eiderman@oracle.com>
+From: Paolo Bonzini <pbonzini@redhat.com>
-In the following case:
+Right now, users of qemu_co_sleep_ns_wakeable are simply passing
+a pointer to QemuCoSleepState by reference to the function.  But
-(base) A <- B <- C (tip)
+QemuCoSleepState really is just a Coroutine*; making the
+content of the struct public is just as efficient and lets us
-when running:
+skip the user_state_pointer indirection.
-    qemu-img rebase -b A C
+Since the usage is changed, take the occasion to rename the
+struct to QemuCoSleep.
-QEMU would read all sectors not allocated in the file being rebased (C)
-and compare them to the new base image (A), regardless of whether they
+Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-were changed or even allocated anywhere along the chain between the new
+Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
-base and the top image (B). This causes many unneeded reads when
+Message-id: 20210517100548.28806-6-pbonzini@redhat.com
-rebasing an image which represents a small diff of a large disk, as it
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 would read most of the disk's sectors.
 Instead, use bdrv_is_allocated_above() to reduce the number of
 unnecessary reads.
 Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com>
 Signed-off-by: Sam Eiderman <shmuel.eiderman@oracle.com>
 Signed-off-by: Eyal Moscovici <eyal.moscovici@oracle.com>
 Message-id: 20190523163337.4497-3-shmuel.eiderman@oracle.com
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
- qemu-img.c | 25 ++++++++++++++++++++++++-
+ include/qemu/coroutine.h    | 23 +++++++++++----------
-file changed, 24 insertions(+), 1 deletion(-)
+ block/block-copy.c          |  8 ++++----
+ block/nbd.c                 | 10 ++++-----
-diff --git a/qemu-img.c b/qemu-img.c
+ util/qemu-coroutine-sleep.c | 41 ++++++++++++++++---------------------
-index XXXXXXX..XXXXXXX 100644
+files changed, 39 insertions(+), 43 deletions(-)
---- a/qemu-img.c
-+++ b/qemu-img.c
+diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
-@@ -XXX,XX +XXX,XX @@ static int img_rebase(int argc, char **argv)
+index XXXXXXX..XXXXXXX 100644
-     BlockBackend *blk = NULL, *blk_old_backing = NULL, *blk_new_backing = NULL;
+--- a/include/qemu/coroutine.h
-     uint8_t *buf_old = NULL;
++++ b/include/qemu/coroutine.h
-     uint8_t *buf_new = NULL;
+@@ -XXX,XX +XXX,XX @@ void qemu_co_rwlock_wrlock(CoRwlock *lock);
--    BlockDriverState *bs = NULL;
+  */
-+    BlockDriverState *bs = NULL, *prefix_chain_bs = NULL;
+ void qemu_co_rwlock_unlock(CoRwlock *lock);
-     char *filename;
-     const char *fmt, *cache, *src_cache, *out_basefmt, *out_baseimg;
+-typedef struct QemuCoSleepState QemuCoSleepState;
-     int c, flags, src_flags, ret;
++typedef struct QemuCoSleep {
-@@ -XXX,XX +XXX,XX @@ static int img_rebase(int argc, char **argv)
++    Coroutine *to_wake;
-                 goto out;
++} QemuCoSleep;
  /**
 - * Yield the coroutine for a given duration. During this yield, @sleep_state
 - * is set to an opaque pointer, which may be used for
 - * qemu_co_sleep_wake(). Be careful, the pointer is set back to zero when the
 - * timer fires. Don't save the obtained value to other variables and don't call
 - * qemu_co_sleep_wake from another aio context.
 + * Yield the coroutine for a given duration. Initializes @w so that,
 + * during this yield, it can be passed to qemu_co_sleep_wake() to
 + * terminate the sleep.
   */
 -void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
 -                                            QemuCoSleepState **sleep_state);
 +void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
 +                                            QEMUClockType type, int64_t ns);
 +
  static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
  {
 -    QemuCoSleepState *unused = NULL;
 -    qemu_co_sleep_ns_wakeable(type, ns, &unused);
 +    QemuCoSleep w = { 0 };
 +    qemu_co_sleep_ns_wakeable(&w, type, ns);
  }
  /**
@@ -XXX,XX +XXX,XX @@ static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
   * qemu_co_sleep_ns() and should be checked to be non-NULL before calling
   * qemu_co_sleep_wake().
   */
 -void qemu_co_sleep_wake(QemuCoSleepState *sleep_state);
 +void qemu_co_sleep_wake(QemuCoSleep *w);
  /**
   * Yield until a file descriptor becomes readable
 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 @@ typedef struct BlockCopyCallState {
      /* State */
      int ret;
      bool finished;
 -    QemuCoSleepState *sleep_state;
 +    QemuCoSleep sleep;
      bool cancelled;
      /* OUT parameters */
@@ -XXX,XX +XXX,XX @@ block_copy_dirty_clusters(BlockCopyCallState *call_state)
                  if (ns > 0) {
                      block_copy_task_end(task, -EAGAIN);
                      g_free(task);
 -                    qemu_co_sleep_ns_wakeable(QEMU_CLOCK_REALTIME, ns,
 -                                              &call_state->sleep_state);
 +                    qemu_co_sleep_ns_wakeable(&call_state->sleep,
 +                                              QEMU_CLOCK_REALTIME, ns);
                      continue;
                  }
              }
+@@ -XXX,XX +XXX,XX @@ out:
-+            /*
-+             * Find out whether we rebase an image on top of a previous image
+ void block_copy_kick(BlockCopyCallState *call_state)
-+             * in its chain.
+ {
-+             */
+-    qemu_co_sleep_wake(call_state->sleep_state);
-+            prefix_chain_bs = bdrv_find_backing_image(bs, out_real_path);
++    qemu_co_sleep_wake(&call_state->sleep);
-+
+ }
-             blk_new_backing = blk_new_open(out_real_path, NULL,
-                                            options, src_flags, &local_err);
+ /*
-             g_free(out_real_path);
+diff --git a/block/nbd.c b/block/nbd.c
-@@ -XXX,XX +XXX,XX @@ static int img_rebase(int argc, char **argv)
+index XXXXXXX..XXXXXXX 100644
 --- a/block/nbd.c
 +++ b/block/nbd.c
@@ -XXX,XX +XXX,XX @@ typedef struct BDRVNBDState {
      CoQueue free_sema;
      Coroutine *connection_co;
      Coroutine *teardown_co;
 -    QemuCoSleepState *connection_co_sleep_ns_state;
 +    QemuCoSleep reconnect_sleep;
      bool drained;
      bool wait_drained_end;
      int in_flight;
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn nbd_client_co_drain_begin(BlockDriverState *bs)
      BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
      s->drained = true;
 -    qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
 +    qemu_co_sleep_wake(&s->reconnect_sleep);
      nbd_co_establish_connection_cancel(bs, false);
@@ -XXX,XX +XXX,XX @@ static void nbd_teardown_connection(BlockDriverState *bs)
      s->state = NBD_CLIENT_QUIT;
      if (s->connection_co) {
 -        qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
 +        qemu_co_sleep_wake(&s->reconnect_sleep);
          nbd_co_establish_connection_cancel(bs, true);
      }
      if (qemu_in_coroutine()) {
@@ -XXX,XX +XXX,XX @@ static coroutine_fn void nbd_co_reconnect_loop(BDRVNBDState *s)
              }
              bdrv_inc_in_flight(s->bs);
          } else {
 -            qemu_co_sleep_ns_wakeable(QEMU_CLOCK_REALTIME, timeout,
 -                                      &s->connection_co_sleep_ns_state);
 +            qemu_co_sleep_ns_wakeable(&s->reconnect_sleep,
 +                                      QEMU_CLOCK_REALTIME, timeout);
              if (s->drained) {
                  continue;
              }
+diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
-+            if (prefix_chain_bs) {
+index XXXXXXX..XXXXXXX 100644
-+                /*
+--- a/util/qemu-coroutine-sleep.c
-+                 * If cluster wasn't changed since prefix_chain, we don't need
++++ b/util/qemu-coroutine-sleep.c
-+                 * to take action
+@@ -XXX,XX +XXX,XX @@
-+                 */
-+                ret = bdrv_is_allocated_above(backing_bs(bs), prefix_chain_bs,
+ static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";
-+                                              offset, n, &n);
-+                if (ret < 0) {
+-struct QemuCoSleepState {
-+                    error_report("error while reading image metadata: %s",
++void qemu_co_sleep_wake(QemuCoSleep *w)
-+                                 strerror(-ret));
++{
-+                    goto out;
+     Coroutine *co;
-+                }
+-    QemuCoSleepState **user_state_pointer;
-+                if (!ret) {
+-};
-+                    continue;
-+                }
+-void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
-+            }
+-{
-+
+-    if (sleep_state) {
-             /*
++    co = w->to_wake;
-              * Read old and new backing file and take into consideration that
++    w->to_wake = NULL;
-              * backing files may be smaller than the COW image.
++    if (co) {
          /* Write of schedule protected by barrier write in aio_co_schedule */
 -        const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled,
 +        const char *scheduled = qatomic_cmpxchg(&co->scheduled,
                                                  qemu_co_sleep_ns__scheduled, NULL);
          assert(scheduled == qemu_co_sleep_ns__scheduled);
 -        *sleep_state->user_state_pointer = NULL;
 -        aio_co_wake(sleep_state->co);
 +        aio_co_wake(co);
      }
  }
  static void co_sleep_cb(void *opaque)
  {
 -    QemuCoSleepState **sleep_state = opaque;
 -    qemu_co_sleep_wake(*sleep_state);
 +    QemuCoSleep *w = opaque;
 +    qemu_co_sleep_wake(w);
  }
 -void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
 -                                            QemuCoSleepState **sleep_state)
 +void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
 +                                            QEMUClockType type, int64_t ns)
  {
 +    Coroutine *co = qemu_coroutine_self();
      AioContext *ctx = qemu_get_current_aio_context();
      QEMUTimer ts;
 -    QemuCoSleepState state = {
 -        .co = qemu_coroutine_self(),
 -        .user_state_pointer = sleep_state,
 -    };
 -    const char *scheduled = qatomic_cmpxchg(&state.co->scheduled, NULL,
 -                                           qemu_co_sleep_ns__scheduled);
 +    const char *scheduled = qatomic_cmpxchg(&co->scheduled, NULL,
 +                                            qemu_co_sleep_ns__scheduled);
      if (scheduled) {
          fprintf(stderr,
                  "%s: Co-routine was already scheduled in '%s'\n",
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
          abort();
      }
 -    aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, sleep_state);
 -    *sleep_state = &state;
 +    w->to_wake = co;
 +    aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w),
      timer_mod(&ts, qemu_clock_get_ns(type) + ns);
      qemu_coroutine_yield();
      timer_del(&ts);
 -    /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine.  */
 -    assert(*sleep_state == NULL);
 +    /* w->to_wake is cleared before resuming this coroutine.  */
 +    assert(w->to_wake == NULL);
  }
 --
-.21.0
+.31.1

-[Qemu-devel] [PULL 13/21] block/backup: refactor: split out backup_calculate_cluster_size
+[PULL 8/8] coroutine-sleep: introduce qemu_co_sleep
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+From: Paolo Bonzini <pbonzini@redhat.com>
-Split out cluster_size calculation. Move copy-bitmap creation above
+Allow using QemuCoSleep to sleep forever until woken by qemu_co_sleep_wake.
-block-job creation, as we are going to share it with upcoming
+This makes the logic of qemu_co_sleep_ns_wakeable easy to understand.
 backup-top filter, which also should be created before actual block job
 creation.
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+In the future we will introduce an API that can work even if the
-Message-id: 20190429090842.57910-6-vsementsov@virtuozzo.com
+sleep and wake happen from different threads.  For now, initializing
-[mreitz: Dropped a paragraph from the commit message that was left over
+w->to_wake after timer_mod is fine because the timer can only fire in
-         from a previous version]
+the same AioContext.
-Signed-off-by: Max Reitz <mreitz@redhat.com>
 Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
 Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
 Message-id: 20210517100548.28806-7-pbonzini@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/backup.c | 82 ++++++++++++++++++++++++++++++++------------------
+ include/qemu/coroutine.h    |  5 +++++
-file changed, 52 insertions(+), 30 deletions(-)
+ util/qemu-coroutine-sleep.c | 26 +++++++++++++++++++-------
 files changed, 24 insertions(+), 7 deletions(-)
-diff --git a/block/backup.c b/block/backup.c
+diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
 index XXXXXXX..XXXXXXX 100644
---- a/block/backup.c
+--- a/include/qemu/coroutine.h
-+++ b/block/backup.c
++++ b/include/qemu/coroutine.h
-@@ -XXX,XX +XXX,XX @@ static const BlockJobDriver backup_job_driver = {
+@@ -XXX,XX +XXX,XX @@ typedef struct QemuCoSleep {
-     .drain                  = backup_drain,
+ void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
- };
+                                             QEMUClockType type, int64_t ns);
-+static int64_t backup_calculate_cluster_size(BlockDriverState *target,
++/**
-+                                             Error **errp)
++ * Yield the coroutine until the next call to qemu_co_sleep_wake.
 + */
 +void coroutine_fn qemu_co_sleep(QemuCoSleep *w);
 +
  static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
  {
      QemuCoSleep w = { 0 };
 diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
 index XXXXXXX..XXXXXXX 100644
 --- a/util/qemu-coroutine-sleep.c
 +++ b/util/qemu-coroutine-sleep.c
@@ -XXX,XX +XXX,XX @@ static void co_sleep_cb(void *opaque)
      qemu_co_sleep_wake(w);
  }
 -void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
 -                                            QEMUClockType type, int64_t ns)
 +void coroutine_fn qemu_co_sleep(QemuCoSleep *w)
  {
      Coroutine *co = qemu_coroutine_self();
 -    AioContext *ctx = qemu_get_current_aio_context();
 -    QEMUTimer ts;
      const char *scheduled = qatomic_cmpxchg(&co->scheduled, NULL,
                                              qemu_co_sleep_ns__scheduled);
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
      }
      w->to_wake = co;
 -    aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w),
 -    timer_mod(&ts, qemu_clock_get_ns(type) + ns);
      qemu_coroutine_yield();
 -    timer_del(&ts);
      /* w->to_wake is cleared before resuming this coroutine.  */
      assert(w->to_wake == NULL);
  }
 +
 +void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
 +                                            QEMUClockType type, int64_t ns)
 +{
-+    int ret;
++    AioContext *ctx = qemu_get_current_aio_context();
-+    BlockDriverInfo bdi;
++    QEMUTimer ts;
 +
 +    aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w);
 +    timer_mod(&ts, qemu_clock_get_ns(type) + ns);
 +
 +    /*
-+     * If there is no backing file on the target, we cannot rely on COW if our
++     * The timer will fire in the current AiOContext, so the callback
-+     * backup cluster size is smaller than the target cluster size. Even for
++     * must happen after qemu_co_sleep yields and there is no race
-+     * targets with a backing file, try to avoid COW if possible.
++     * between timer_mod and qemu_co_sleep.
 +     */
-+    ret = bdrv_get_info(target, &bdi);
++    qemu_co_sleep(w);
-+    if (ret == -ENOTSUP && !target->backing) {
++    timer_del(&ts);
 +        /* Cluster size is not defined */
 +        warn_report("The target block device doesn't provide "
 +                    "information about the block size and it doesn't have a "
 +                    "backing file. The default block size of %u bytes is "
 +                    "used. If the actual block size of the target exceeds "
 +                    "this default, the backup may be unusable",
 +                    BACKUP_CLUSTER_SIZE_DEFAULT);
 +        return BACKUP_CLUSTER_SIZE_DEFAULT;
 +    } else if (ret < 0 && !target->backing) {
 +        error_setg_errno(errp, -ret,
 +            "Couldn't determine the cluster size of the target image, "
 +            "which has no backing file");
 +        error_append_hint(errp,
 +            "Aborting, since this may create an unusable destination image\n");
 +        return ret;
 +    } else if (ret < 0 && target->backing) {
 +        /* Not fatal; just trudge on ahead. */
 +        return BACKUP_CLUSTER_SIZE_DEFAULT;
 +    }
 +
 +    return MAX(BACKUP_CLUSTER_SIZE_DEFAULT, bdi.cluster_size);
 +}
-+
- BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
-                   BlockDriverState *target, int64_t speed,
-                   MirrorSyncMode sync_mode, BdrvDirtyBitmap *sync_bitmap,
-@@ -XXX,XX +XXX,XX @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
-                   JobTxn *txn, Error **errp)
- {
-     int64_t len;
--    BlockDriverInfo bdi;
-     BackupBlockJob *job = NULL;
-     int ret;
-+    int64_t cluster_size;
-+    HBitmap *copy_bitmap = NULL;
-     assert(bs);
-     assert(target);
-@@ -XXX,XX +XXX,XX @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
-         goto error;
-     }
-+    cluster_size = backup_calculate_cluster_size(target, errp);
-+    if (cluster_size < 0) {
-+        goto error;
-+    }
-+
-+    copy_bitmap = hbitmap_alloc(len, ctz32(cluster_size));
-+
-     /* job->len is fixed, so we can't allow resize */
-     job = block_job_create(job_id, &backup_job_driver, txn, bs,
-                            BLK_PERM_CONSISTENT_READ,
-@@ -XXX,XX +XXX,XX @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
-     /* Detect image-fleecing (and similar) schemes */
-     job->serialize_target_writes = bdrv_chain_contains(target, bs);
--
--    /* If there is no backing file on the target, we cannot rely on COW if our
--     * backup cluster size is smaller than the target cluster size. Even for
--     * targets with a backing file, try to avoid COW if possible. */
--    ret = bdrv_get_info(target, &bdi);
--    if (ret == -ENOTSUP && !target->backing) {
--        /* Cluster size is not defined */
--        warn_report("The target block device doesn't provide "
--                    "information about the block size and it doesn't have a "
--                    "backing file. The default block size of %u bytes is "
--                    "used. If the actual block size of the target exceeds "
--                    "this default, the backup may be unusable",
--                    BACKUP_CLUSTER_SIZE_DEFAULT);
--        job->cluster_size = BACKUP_CLUSTER_SIZE_DEFAULT;
--    } else if (ret < 0 && !target->backing) {
--        error_setg_errno(errp, -ret,
--            "Couldn't determine the cluster size of the target image, "
--            "which has no backing file");
--        error_append_hint(errp,
--            "Aborting, since this may create an unusable destination image\n");
--        goto error;
--    } else if (ret < 0 && target->backing) {
--        /* Not fatal; just trudge on ahead. */
--        job->cluster_size = BACKUP_CLUSTER_SIZE_DEFAULT;
--    } else {
--        job->cluster_size = MAX(BACKUP_CLUSTER_SIZE_DEFAULT, bdi.cluster_size);
--    }
--
--    job->copy_bitmap = hbitmap_alloc(len, ctz32(job->cluster_size));
-+    job->cluster_size = cluster_size;
-+    job->copy_bitmap = copy_bitmap;
-+    copy_bitmap = NULL;
-     job->use_copy_range = true;
-     job->copy_range_size = MIN_NON_ZERO(blk_get_max_transfer(job->common.blk),
-                                         blk_get_max_transfer(job->target));
-@@ -XXX,XX +XXX,XX @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
-     return &job->common;
-  error:
-+    if (copy_bitmap) {
-+        assert(!job || !job->copy_bitmap);
-+        hbitmap_free(copy_bitmap);
-+    }
-     if (sync_bitmap) {
-         bdrv_reclaim_dirty_bitmap(bs, sync_bitmap, NULL);
-     }
 --
-.21.0
+.31.1

-[Qemu-devel] [PULL 14/21] block: Use bdrv_unref_child() for all children in bdrv_close()
+Deleted patch
-From: Alberto Garcia <berto@igalia.com>
-bdrv_unref_child() does the following things:
-  - Updates the child->bs->inherits_from pointer.
-  - Calls bdrv_detach_child() to remove the BdrvChild from bs->children.
-  - Calls bdrv_unref() to unref the child BlockDriverState.
-When bdrv_unref_child() was introduced in commit 33a604075c it was not
-used in bdrv_close() because the drivers that had additional children
-(like quorum or blkverify) had already called bdrv_unref() on their
-children during their own close functions.
-This was changed later (in 0bd6e91a7e for quorum, in 3e586be0b2 for
-blkverify) so there's no reason not to use bdrv_unref_child() in
-bdrv_close() anymore.
-After this there's also no need to remove bs->backing and bs->file
-separately from the rest of the children, so bdrv_close() can be
-simplified.
-Now bdrv_close() unrefs all children (before this patch it was only
-bs->file and bs->backing). As a result, none of the callers of
-brvd_attach_child() should remove their reference to child_bs (because
-this function effectively steals that reference). This patch updates a
-couple of tests that were doing their own bdrv_unref().
-Signed-off-by: Alberto Garcia <berto@igalia.com>
-Message-id: 6d1d5feaa53aa1ab127adb73d605dc4503e3abd5.1557754872.git.berto@igalia.com
-[mreitz: s/where/were/]
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block.c                     | 16 +++-------------
- tests/test-bdrv-drain.c     |  6 ------
- tests/test-bdrv-graph-mod.c |  1 -
-files changed, 3 insertions(+), 20 deletions(-)
-diff --git a/block.c b/block.c
-index XXXXXXX..XXXXXXX 100644
---- a/block.c
-+++ b/block.c
-@@ -XXX,XX +XXX,XX @@ static void bdrv_close(BlockDriverState *bs)
-         bs->drv = NULL;
-     }
--    bdrv_set_backing_hd(bs, NULL, &error_abort);
--
--    if (bs->file != NULL) {
--        bdrv_unref_child(bs, bs->file);
--        bs->file = NULL;
--    }
--
-     QLIST_FOREACH_SAFE(child, &bs->children, next, next) {
--        /* TODO Remove bdrv_unref() from drivers' close function and use
--         * bdrv_unref_child() here */
--        if (child->bs->inherits_from == bs) {
--            child->bs->inherits_from = NULL;
--        }
--        bdrv_detach_child(child);
-+        bdrv_unref_child(bs, child);
-     }
-+    bs->backing = NULL;
-+    bs->file = NULL;
-     g_free(bs->opaque);
-     bs->opaque = NULL;
-     atomic_set(&bs->copy_on_read, 0);
-diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c
-index XXXXXXX..XXXXXXX 100644
---- a/tests/test-bdrv-drain.c
-+++ b/tests/test-bdrv-drain.c
-@@ -XXX,XX +XXX,XX @@ static void test_detach_indirect(bool by_parent_cb)
-     bdrv_unref(parent_b);
-     blk_unref(blk);
--    /* XXX Once bdrv_close() unref's children instead of just detaching them,
--     * this won't be necessary any more. */
--    bdrv_unref(a);
--    bdrv_unref(a);
--    bdrv_unref(c);
--
-     g_assert_cmpint(a->refcnt, ==, 1);
-     g_assert_cmpint(b->refcnt, ==, 1);
-     g_assert_cmpint(c->refcnt, ==, 1);
-diff --git a/tests/test-bdrv-graph-mod.c b/tests/test-bdrv-graph-mod.c
-index XXXXXXX..XXXXXXX 100644
---- a/tests/test-bdrv-graph-mod.c
-+++ b/tests/test-bdrv-graph-mod.c
-@@ -XXX,XX +XXX,XX @@ static void test_update_perm_tree(void)
-     g_assert_nonnull(local_err);
-     error_free(local_err);
--    bdrv_unref(bs);
-     blk_unref(root);
- }
---
-.21.0

-[Qemu-devel] [PULL 16/21] qemu-img: rebase: Reuse parent BlockDriverState
+Deleted patch
-From: Sam Eiderman <shmuel.eiderman@oracle.com>
-In safe mode we open the entire chain, including the parent backing
-file of the rebased file.
-Do not open a new BlockBackend for the parent backing file, which
-saves opening the rest of the chain twice, which for long chains
-saves many "pricy" bdrv_open() calls.
-Permissions for blk_new() were copied from blk_new_open() when
-flags = 0.
-Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com>
-Reviewed-by: Eyal Moscovici <eyal.moscovici@oracle.com>
-Signed-off-by: Sagi Amit <sagi.amit@oracle.com>
-Co-developed-by: Sagi Amit <sagi.amit@oracle.com>
-Signed-off-by: Sam Eiderman <shmuel.eiderman@oracle.com>
-Message-id: 20190523163337.4497-2-shmuel.eiderman@oracle.com
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- qemu-img.c | 29 +++++++++--------------------
-file changed, 9 insertions(+), 20 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_rebase(int argc, char **argv)
-     /* For safe rebasing we need to compare old and new backing file */
-     if (!unsafe) {
--        char backing_name[PATH_MAX];
-         QDict *options = NULL;
-+        BlockDriverState *base_bs = backing_bs(bs);
--        if (bs->backing) {
--            if (bs->backing_format[0] != '\0') {
--                options = qdict_new();
--                qdict_put_str(options, "driver", bs->backing_format);
--            }
--
--            if (force_share) {
--                if (!options) {
--                    options = qdict_new();
--                }
--                qdict_put_bool(options, BDRV_OPT_FORCE_SHARE, true);
--            }
--            bdrv_get_backing_filename(bs, backing_name, sizeof(backing_name));
--            blk_old_backing = blk_new_open(backing_name, NULL,
--                                           options, src_flags, &local_err);
--            if (!blk_old_backing) {
-+        if (base_bs) {
-+            blk_old_backing = blk_new(BLK_PERM_CONSISTENT_READ,
-+                                      BLK_PERM_ALL);
-+            ret = blk_insert_bs(blk_old_backing, base_bs,
-+                                &local_err);
-+            if (ret < 0) {
-                 error_reportf_err(local_err,
--                                  "Could not open old backing file '%s': ",
--                                  backing_name);
--                ret = -1;
-+                                  "Could not reuse old backing file '%s': ",
-+                                  base_bs->filename);
-                 goto out;
-             }
-         } else {
---
-.21.0

-[Qemu-devel] [PULL 18/21] qemu-img: rebase: Reuse in-chain BlockDriverState
+Deleted patch
-From: Sam Eiderman <shmuel.eiderman@oracle.com>
-If a chain was detected, don't open a new BlockBackend from the target
-backing file which will create a new BlockDriverState. Instead, create
-an empty BlockBackend and attach the already open BlockDriverState.
-Permissions for blk_new() were copied from blk_new_open() when
-flags = 0.
-Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com>
-Reviewed-by: Eyal Moscovici <eyal.moscovici@oracle.com>
-Signed-off-by: Sagi Amit <sagi.amit@oracle.com>
-Co-developed-by: Sagi Amit <sagi.amit@oracle.com>
-Signed-off-by: Sam Eiderman <shmuel.eiderman@oracle.com>
-Message-id: 20190523163337.4497-4-shmuel.eiderman@oracle.com
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- qemu-img.c | 33 +++++++++++++++++++++++----------
-file changed, 23 insertions(+), 10 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_rebase(int argc, char **argv)
-              * in its chain.
-              */
-             prefix_chain_bs = bdrv_find_backing_image(bs, out_real_path);
--
--            blk_new_backing = blk_new_open(out_real_path, NULL,
--                                           options, src_flags, &local_err);
--            g_free(out_real_path);
--            if (!blk_new_backing) {
--                error_reportf_err(local_err,
--                                  "Could not open new backing file '%s': ",
--                                  out_baseimg);
--                ret = -1;
--                goto out;
-+            if (prefix_chain_bs) {
-+                g_free(out_real_path);
-+                blk_new_backing = blk_new(BLK_PERM_CONSISTENT_READ,
-+                                          BLK_PERM_ALL);
-+                ret = blk_insert_bs(blk_new_backing, prefix_chain_bs,
-+                                    &local_err);
-+                if (ret < 0) {
-+                    error_reportf_err(local_err,
-+                                      "Could not reuse backing file '%s': ",
-+                                      out_baseimg);
-+                    goto out;
-+                }
-+            } else {
-+                blk_new_backing = blk_new_open(out_real_path, NULL,
-+                                               options, src_flags, &local_err);
-+                g_free(out_real_path);
-+                if (!blk_new_backing) {
-+                    error_reportf_err(local_err,
-+                                      "Could not open new backing file '%s': ",
-+                                      out_baseimg);
-+                    ret = -1;
-+                    goto out;
-+                }
-             }
-         }
-     }
---
-.21.0

-[Qemu-devel] [PULL 19/21] qcow2: skip writing zero buffers to empty COW areas
+Deleted patch
-From: Anton Nefedov <anton.nefedov@virtuozzo.com>
-If COW areas of the newly allocated clusters are zeroes on the backing
-image, efficient bdrv_write_zeroes(flags=BDRV_REQ_NO_FALLBACK) can be
-used on the whole cluster instead of writing explicit zero buffers later
-in perform_cow().
-iotest 060:
-write to the discarded cluster does not trigger COW anymore.
-Use a backing image instead.
-Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
-Message-id: 20190516142749.81019-2-anton.nefedov@virtuozzo.com
-Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Reviewed-by: Alberto Garcia <berto@igalia.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- qapi/block-core.json       |  4 +-
- block/qcow2.h              |  6 +++
- block/qcow2-cluster.c      |  2 +-
- block/qcow2.c              | 85 ++++++++++++++++++++++++++++++++++++++
- block/trace-events         |  1 +
- tests/qemu-iotests/060     |  7 +++-
- tests/qemu-iotests/060.out |  5 ++-
-files changed, 106 insertions(+), 4 deletions(-)
-diff --git a/qapi/block-core.json b/qapi/block-core.json
-index XXXXXXX..XXXXXXX 100644
---- a/qapi/block-core.json
-+++ b/qapi/block-core.json
-@@ -XXX,XX +XXX,XX @@
- #
- # @cor_write: a write due to copy-on-read (since 2.11)
- #
-+# @cluster_alloc_space: an allocation of file space for a cluster (since 4.1)
-+#
- # Since: 2.9
- ##
- { 'enum': 'BlkdebugEvent', 'prefix': 'BLKDBG',
-@@ -XXX,XX +XXX,XX @@
-             'pwritev_rmw_tail', 'pwritev_rmw_after_tail', 'pwritev',
-             'pwritev_zero', 'pwritev_done', 'empty_image_prepare',
-             'l1_shrink_write_table', 'l1_shrink_free_l2_clusters',
--            'cor_write'] }
-+            'cor_write', 'cluster_alloc_space'] }
- ##
- # @BlkdebugInjectErrorOptions:
-diff --git a/block/qcow2.h b/block/qcow2.h
-index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2.h
-+++ b/block/qcow2.h
-@@ -XXX,XX +XXX,XX @@ typedef struct QCowL2Meta
-      */
-     Qcow2COWRegion cow_end;
-+    /*
-+     * Indicates that COW regions are already handled and do not require
-+     * any more processing.
-+     */
-+    bool skip_cow;
-+
-     /**
-      * The I/O vector with the data from the actual guest write request.
-      * If non-NULL, this is meant to be merged together with the data
-diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2-cluster.c
-+++ b/block/qcow2-cluster.c
-@@ -XXX,XX +XXX,XX @@ static int perform_cow(BlockDriverState *bs, QCowL2Meta *m)
-     assert(start->offset + start->nb_bytes <= end->offset);
-     assert(!m->data_qiov || m->data_qiov->size == data_bytes);
--    if (start->nb_bytes == 0 && end->nb_bytes == 0) {
-+    if ((start->nb_bytes == 0 && end->nb_bytes == 0) || m->skip_cow) {
-         return 0;
-     }
-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 bool merge_cow(uint64_t offset, unsigned bytes,
-             continue;
-         }
-+        /* If COW regions are handled already, skip this too */
-+        if (m->skip_cow) {
-+            continue;
-+        }
-+
-         /* The data (middle) region must be immediately after the
-          * start region */
-         if (l2meta_cow_start(m) + m->cow_start.nb_bytes != offset) {
-@@ -XXX,XX +XXX,XX @@ static bool merge_cow(uint64_t offset, unsigned bytes,
-     return false;
- }
-+static bool is_unallocated(BlockDriverState *bs, int64_t offset, int64_t bytes)
-+{
-+    int64_t nr;
-+    return !bytes ||
-+        (!bdrv_is_allocated_above(bs, NULL, offset, bytes, &nr) && nr == bytes);
-+}
-+
-+static bool is_zero_cow(BlockDriverState *bs, QCowL2Meta *m)
-+{
-+    /*
-+     * This check is designed for optimization shortcut so it must be
-+     * efficient.
-+     * Instead of is_zero(), use is_unallocated() as it is faster (but not
-+     * as accurate and can result in false negatives).
-+     */
-+    return is_unallocated(bs, m->offset + m->cow_start.offset,
-+                          m->cow_start.nb_bytes) &&
-+           is_unallocated(bs, m->offset + m->cow_end.offset,
-+                          m->cow_end.nb_bytes);
-+}
-+
-+static int handle_alloc_space(BlockDriverState *bs, QCowL2Meta *l2meta)
-+{
-+    BDRVQcow2State *s = bs->opaque;
-+    QCowL2Meta *m;
-+
-+    if (!(s->data_file->bs->supported_zero_flags & BDRV_REQ_NO_FALLBACK)) {
-+        return 0;
-+    }
-+
-+    if (bs->encrypted) {
-+        return 0;
-+    }
-+
-+    for (m = l2meta; m != NULL; m = m->next) {
-+        int ret;
-+
-+        if (!m->cow_start.nb_bytes && !m->cow_end.nb_bytes) {
-+            continue;
-+        }
-+
-+        if (!is_zero_cow(bs, m)) {
-+            continue;
-+        }
-+
-+        /*
-+         * instead of writing zero COW buffers,
-+         * efficiently zero out the whole clusters
-+         */
-+
-+        ret = qcow2_pre_write_overlap_check(bs, 0, m->alloc_offset,
-+                                            m->nb_clusters * s->cluster_size,
-+                                            true);
-+        if (ret < 0) {
-+            return ret;
-+        }
-+
-+        BLKDBG_EVENT(bs->file, BLKDBG_CLUSTER_ALLOC_SPACE);
-+        ret = bdrv_co_pwrite_zeroes(s->data_file, m->alloc_offset,
-+                                    m->nb_clusters * s->cluster_size,
-+                                    BDRV_REQ_NO_FALLBACK);
-+        if (ret < 0) {
-+            if (ret != -ENOTSUP && ret != -EAGAIN) {
-+                return ret;
-+            }
-+            continue;
-+        }
-+
-+        trace_qcow2_skip_cow(qemu_coroutine_self(), m->offset, m->nb_clusters);
-+        m->skip_cow = true;
-+    }
-+    return 0;
-+}
-+
- static coroutine_fn int qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
-                                          uint64_t bytes, QEMUIOVector *qiov,
-                                          int flags)
-@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
-             qemu_iovec_add(&hd_qiov, cluster_data, cur_bytes);
-         }
-+        /* Try to efficiently initialize the physical space with zeroes */
-+        ret = handle_alloc_space(bs, l2meta);
-+        if (ret < 0) {
-+            goto out_unlocked;
-+        }
-+
-         /* If we need to do COW, check if it's possible to merge the
-          * writing of the guest data together with that of the COW regions.
-          * If it's not possible (or not necessary) then write the
-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 @@ qcow2_writev_done_part(void *co, int cur_bytes) "co %p cur_bytes %d"
- qcow2_writev_data(void *co, uint64_t offset) "co %p offset 0x%" PRIx64
- qcow2_pwrite_zeroes_start_req(void *co, int64_t offset, int count) "co %p offset 0x%" PRIx64 " count %d"
- qcow2_pwrite_zeroes(void *co, int64_t offset, int count) "co %p offset 0x%" PRIx64 " count %d"
-+qcow2_skip_cow(void *co, uint64_t offset, int nb_clusters) "co %p offset 0x%" PRIx64 " nb_clusters %d"
- # qcow2-cluster.c
- qcow2_alloc_clusters_offset(void *co, uint64_t offset, int bytes) "co %p offset 0x%" PRIx64 " bytes %d"
-diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060
-index XXXXXXX..XXXXXXX 100755
---- a/tests/qemu-iotests/060
-+++ b/tests/qemu-iotests/060
-@@ -XXX,XX +XXX,XX @@ $QEMU_IO -c "$OPEN_RO" -c "read -P 1 0 512" | _filter_qemu_io
- echo
- echo "=== Testing overlap while COW is in flight ==="
- echo
-+BACKING_IMG=$TEST_IMG.base
-+TEST_IMG=$BACKING_IMG _make_test_img 1G
-+
-+$QEMU_IO -c 'write 0k 64k' "$BACKING_IMG" | _filter_qemu_io
-+
- # compat=0.10 is required in order to make the following discard actually
- # unallocate the sector rather than make it a zero sector - we want COW, after
- # all.
--IMGOPTS='compat=0.10' _make_test_img 1G
-+IMGOPTS='compat=0.10' _make_test_img -b "$BACKING_IMG" 1G
- # Write two clusters, the second one enforces creation of an L2 table after
- # the first data cluster.
- $QEMU_IO -c 'write 0k 64k' -c 'write 512M 64k' "$TEST_IMG" | _filter_qemu_io
-diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/060.out
-+++ b/tests/qemu-iotests/060.out
-@@ -XXX,XX +XXX,XX @@ read 512/512 bytes at offset 0
- === Testing overlap while COW is in flight ===
--Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
-+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=1073741824
-+wrote 65536/65536 bytes at offset 0
-+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 backing_file=TEST_DIR/t.IMGFMT.base
- wrote 65536/65536 bytes at offset 0
-KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
- wrote 65536/65536 bytes at offset 536870912
---
-.21.0

The following changes since commit 8c1ecb590497b0349c550607db923972b37f6963:

Merge remote-tracking branch 'remotes/stsquad/tags/pull-testing-next-280519-2' into staging (2019-05-28 17:38:32 +0100)

are available in the Git repository at:

https://github.com/XanClic/qemu.git tags/pull-block-2019-05-28

for you to fetch changes up to a2d665c1bc3624a8375e2f9a7d569f7565cc1358:

blockdev: loosen restrictions on drive-backup source node (2019-05-28 20:30:55 +0200)

----------------------------------------------------------------
Block patches:
- qcow2: Use threads for encrypted I/O
- qemu-img rebase: Optimizations
- backup job: Allow any source node, and some refactoring
- Some general simplifications in the block layer

----------------------------------------------------------------
Alberto Garcia (2):
  block: Use bdrv_unref_child() for all children in bdrv_close()
  block: Make bdrv_root_attach_child() unref child_bs on failure

Andrey Shinkevich (1):
  qcow2-bitmap: initialize bitmap directory alignment

Anton Nefedov (1):
  qcow2: skip writing zero buffers to empty COW areas

John Snow (1):
  blockdev: loosen restrictions on drive-backup source node

Sam Eiderman (3):
  qemu-img: rebase: Reuse parent BlockDriverState
  qemu-img: rebase: Reduce reads on in-chain rebase
  qemu-img: rebase: Reuse in-chain BlockDriverState

Vladimir Sementsov-Ogievskiy (13):
  qcow2.h: add missing include
  qcow2: add separate file for threaded data processing functions
  qcow2-threads: use thread_pool_submit_co
  qcow2-threads: qcow2_co_do_compress: protect queuing by mutex
  qcow2-threads: split out generic path
  qcow2: qcow2_co_preadv: improve locking
  qcow2: bdrv_co_pwritev: move encryption code out of the lock
  qcow2: do encryption in threads
  block/backup: simplify backup_incremental_init_copy_bitmap
  block/backup: move to copy_bitmap with granularity
  block/backup: refactor and tolerate unallocated cluster skipping
  block/backup: unify different modes code path
  block/backup: refactor: split out backup_calculate_cluster_size

-- 
2.21.0

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

qcow2.h depends on block_int.h. Compilation isn't broken currently only
due to block_int.h always included before qcow2.h. Though, it seems
better to directly include block_int.h in qcow2.h.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 20190506142741.41731-2-vsementsov@virtuozzo.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.h          | 1 +
 block/qcow2-bitmap.c   | 1 -
 block/qcow2-cache.c    | 1 -
 block/qcow2-cluster.c  | 1 -
 block/qcow2-refcount.c | 1 -
 block/qcow2-snapshot.c | 1 -
 block/qcow2.c          | 1 -
 7 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -XXX,XX +XXX,XX @@
 #include "crypto/block.h"
 #include "qemu/coroutine.h"
 #include "qemu/units.h"
+#include "block/block_int.h"
 
 //#define DEBUG_ALLOC
 //#define DEBUG_ALLOC2
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -XXX,XX +XXX,XX @@
 #include "qapi/error.h"
 #include "qemu/cutils.h"
 
-#include "block/block_int.h"
 #include "qcow2.h"
 
 /* NOTICE: BME here means Bitmaps Extension and used as a namespace for
diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2-cache.c
+++ b/block/qcow2-cache.c
@@ -XXX,XX +XXX,XX @@
  */
 
 #include "qemu/osdep.h"
-#include "block/block_int.h"
 #include "qemu-common.h"
 #include "qcow2.h"
 #include "trace.h"
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -XXX,XX +XXX,XX @@
 
 #include "qapi/error.h"
 #include "qemu-common.h"
-#include "block/block_int.h"
 #include "qcow2.h"
 #include "qemu/bswap.h"
 #include "trace.h"
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -XXX,XX +XXX,XX @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu-common.h"
-#include "block/block_int.h"
 #include "qcow2.h"
 #include "qemu/range.h"
 #include "qemu/bswap.h"
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -XXX,XX +XXX,XX @@
 
 #include "qemu/osdep.h"
 #include "qapi/error.h"
-#include "block/block_int.h"
 #include "qcow2.h"
 #include "qemu/bswap.h"
 #include "qemu/error-report.h"
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 @@
 #define ZLIB_CONST
 #include <zlib.h>
 
-#include "block/block_int.h"
 #include "block/qdict.h"
 #include "sysemu/block-backend.h"
 #include "qemu/module.h"
-- 
2.21.0

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

Move compression-on-threads to separate file. Encryption will be in it
too.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 20190506142741.41731-3-vsementsov@virtuozzo.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/Makefile.objs   |   2 +-
 block/qcow2.h         |   7 ++
 block/qcow2-threads.c | 201 ++++++++++++++++++++++++++++++++++++++++++
 block/qcow2.c         | 169 -----------------------------------
 4 files changed, 209 insertions(+), 170 deletions(-)
 create mode 100644 block/qcow2-threads.c

diff --git a/block/Makefile.objs b/block/Makefile.objs
index XXXXXXX..XXXXXXX 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -XXX,XX +XXX,XX @@ block-obj-$(CONFIG_BOCHS) += bochs.o
 block-obj-$(CONFIG_VVFAT) += vvfat.o
 block-obj-$(CONFIG_DMG) += dmg.o
 
-block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-cache.o qcow2-bitmap.o
+block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-cache.o qcow2-bitmap.o qcow2-threads.o
 block-obj-$(CONFIG_QED) += qed.o qed-l2-cache.o qed-table.o qed-cluster.o
 block-obj-$(CONFIG_QED) += qed-check.o
 block-obj-y += vhdx.o vhdx-endian.o vhdx-log.o
diff --git a/block/qcow2.h b/block/qcow2.h
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -XXX,XX +XXX,XX @@ void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs,
                                           const char *name,
                                           Error **errp);
 
+ssize_t coroutine_fn
+qcow2_co_compress(BlockDriverState *bs, void *dest, size_t dest_size,
+                  const void *src, size_t src_size);
+ssize_t coroutine_fn
+qcow2_co_decompress(BlockDriverState *bs, void *dest, size_t dest_size,
+                    const void *src, size_t src_size);
+
 #endif
diff --git a/block/qcow2-threads.c b/block/qcow2-threads.c
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/block/qcow2-threads.c
@@ -XXX,XX +XXX,XX @@
+/*
+ * Threaded data processing for Qcow2: compression, encryption
+ *
+ * Copyright (c) 2004-2006 Fabrice Bellard
+ * Copyright (c) 2018 Virtuozzo International GmbH. All rights reserved.
+ *
+ * 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.
+ */
+
+#include "qemu/osdep.h"
+
+#define ZLIB_CONST
+#include <zlib.h>
+
+#include "qcow2.h"
+#include "block/thread-pool.h"
+
+#define MAX_COMPRESS_THREADS 4
+
+typedef ssize_t (*Qcow2CompressFunc)(void *dest, size_t dest_size,
+                                     const void *src, size_t src_size);
+typedef struct Qcow2CompressData {
+    void *dest;
+    size_t dest_size;
+    const void *src;
+    size_t src_size;
+    ssize_t ret;
+
+    Qcow2CompressFunc func;
+} Qcow2CompressData;
+
+/*
+ * qcow2_compress()
+ *
+ * @dest - destination buffer, @dest_size bytes
+ * @src - source buffer, @src_size bytes
+ *
+ * Returns: compressed size on success
+ *          -ENOMEM destination buffer is not enough to store compressed data
+ *          -EIO    on any other error
+ */
+static ssize_t qcow2_compress(void *dest, size_t dest_size,
+                              const void *src, size_t src_size)
+{
+    ssize_t ret;
+    z_stream strm;
+
+    /* best compression, small window, no zlib header */
+    memset(&strm, 0, sizeof(strm));
+    ret = deflateInit2(&strm, Z_DEFAULT_COMPRESSION, Z_DEFLATED,
+                       -12, 9, Z_DEFAULT_STRATEGY);
+    if (ret != Z_OK) {
+        return -EIO;
+    }
+
+    /*
+     * strm.next_in is not const in old zlib versions, such as those used on
+     * OpenBSD/NetBSD, so cast the const away
+     */
+    strm.avail_in = src_size;
+    strm.next_in = (void *) src;
+    strm.avail_out = dest_size;
+    strm.next_out = dest;
+
+    ret = deflate(&strm, Z_FINISH);
+    if (ret == Z_STREAM_END) {
+        ret = dest_size - strm.avail_out;
+    } else {
+        ret = (ret == Z_OK ? -ENOMEM : -EIO);
+    }
+
+    deflateEnd(&strm);
+
+    return ret;
+}
+
+/*
+ * qcow2_decompress()
+ *
+ * Decompress some data (not more than @src_size bytes) to produce exactly
+ * @dest_size bytes.
+ *
+ * @dest - destination buffer, @dest_size bytes
+ * @src - source buffer, @src_size bytes
+ *
+ * Returns: 0 on success
+ *          -1 on fail
+ */
+static ssize_t qcow2_decompress(void *dest, size_t dest_size,
+                                const void *src, size_t src_size)
+{
+    int ret = 0;
+    z_stream strm;
+
+    memset(&strm, 0, sizeof(strm));
+    strm.avail_in = src_size;
+    strm.next_in = (void *) src;
+    strm.avail_out = dest_size;
+    strm.next_out = dest;
+
+    ret = inflateInit2(&strm, -12);
+    if (ret != Z_OK) {
+        return -1;
+    }
+
+    ret = inflate(&strm, Z_FINISH);
+    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;
+    }
+
+    inflateEnd(&strm);
+
+    return ret;
+}
+
+static int qcow2_compress_pool_func(void *opaque)
+{
+    Qcow2CompressData *data = opaque;
+
+    data->ret = data->func(data->dest, data->dest_size,
+                           data->src, data->src_size);
+
+    return 0;
+}
+
+static void qcow2_compress_complete(void *opaque, int ret)
+{
+    qemu_coroutine_enter(opaque);
+}
+
+static ssize_t coroutine_fn
+qcow2_co_do_compress(BlockDriverState *bs, void *dest, size_t dest_size,
+                     const void *src, size_t src_size, Qcow2CompressFunc func)
+{
+    BDRVQcow2State *s = bs->opaque;
+    BlockAIOCB *acb;
+    ThreadPool *pool = aio_get_thread_pool(bdrv_get_aio_context(bs));
+    Qcow2CompressData arg = {
+        .dest = dest,
+        .dest_size = dest_size,
+        .src = src,
+        .src_size = src_size,
+        .func = func,
+    };
+
+    while (s->nb_compress_threads >= MAX_COMPRESS_THREADS) {
+        qemu_co_queue_wait(&s->compress_wait_queue, NULL);
+    }
+
+    s->nb_compress_threads++;
+    acb = thread_pool_submit_aio(pool, qcow2_compress_pool_func, &arg,
+                                 qcow2_compress_complete,
+                                 qemu_coroutine_self());
+
+    if (!acb) {
+        s->nb_compress_threads--;
+        return -EINVAL;
+    }
+    qemu_coroutine_yield();
+    s->nb_compress_threads--;
+    qemu_co_queue_next(&s->compress_wait_queue);
+
+    return arg.ret;
+}
+
+ssize_t coroutine_fn
+qcow2_co_compress(BlockDriverState *bs, void *dest, size_t dest_size,
+                  const void *src, size_t src_size)
+{
+    return qcow2_co_do_compress(bs, dest, dest_size, src, src_size,
+                                qcow2_compress);
+}
+
+ssize_t coroutine_fn
+qcow2_co_decompress(BlockDriverState *bs, void *dest, size_t dest_size,
+                    const void *src, size_t src_size)
+{
+    return qcow2_co_do_compress(bs, dest, dest_size, src, src_size,
+                                qcow2_decompress);
+}
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 @@
 
 #include "qemu/osdep.h"
 
-#define ZLIB_CONST
-#include <zlib.h>
-
 #include "block/qdict.h"
 #include "sysemu/block-backend.h"
 #include "qemu/module.h"
@@ -XXX,XX +XXX,XX @@
 #include "qapi/qobject-input-visitor.h"
 #include "qapi/qapi-visit-block-core.h"
 #include "crypto.h"
-#include "block/thread-pool.h"
 
 /*
   Differences with QCOW:
@@ -XXX,XX +XXX,XX @@ fail:
     return ret;
 }
 
-/*
- * qcow2_compress()
- *
- * @dest - destination buffer, @dest_size bytes
- * @src - source buffer, @src_size bytes
- *
- * Returns: compressed size on success
- *          -ENOMEM destination buffer is not enough to store compressed data
- *          -EIO    on any other error
- */
-static ssize_t qcow2_compress(void *dest, size_t dest_size,
-                              const void *src, size_t src_size)
-{
-    ssize_t ret;
-    z_stream strm;
-
-    /* best compression, small window, no zlib header */
-    memset(&strm, 0, sizeof(strm));
-    ret = deflateInit2(&strm, Z_DEFAULT_COMPRESSION, Z_DEFLATED,
-                       -12, 9, Z_DEFAULT_STRATEGY);
-    if (ret != Z_OK) {
-        return -EIO;
-    }
-
-    /* strm.next_in is not const in old zlib versions, such as those used on
-     * OpenBSD/NetBSD, so cast the const away */
-    strm.avail_in = src_size;
-    strm.next_in = (void *) src;
-    strm.avail_out = dest_size;
-    strm.next_out = dest;
-
-    ret = deflate(&strm, Z_FINISH);
-    if (ret == Z_STREAM_END) {
-        ret = dest_size - strm.avail_out;
-    } else {
-        ret = (ret == Z_OK ? -ENOMEM : -EIO);
-    }
-
-    deflateEnd(&strm);
-
-    return ret;
-}
-
-/*
- * qcow2_decompress()
- *
- * Decompress some data (not more than @src_size bytes) to produce exactly
- * @dest_size bytes.
- *
- * @dest - destination buffer, @dest_size bytes
- * @src - source buffer, @src_size bytes
- *
- * Returns: 0 on success
- *          -1 on fail
- */
-static ssize_t qcow2_decompress(void *dest, size_t dest_size,
-                                const void *src, size_t src_size)
-{
-    int ret = 0;
-    z_stream strm;
-
-    memset(&strm, 0, sizeof(strm));
-    strm.avail_in = src_size;
-    strm.next_in = (void *) src;
-    strm.avail_out = dest_size;
-    strm.next_out = dest;
-
-    ret = inflateInit2(&strm, -12);
-    if (ret != Z_OK) {
-        return -1;
-    }
-
-    ret = inflate(&strm, Z_FINISH);
-    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;
-    }
-
-    inflateEnd(&strm);
-
-    return ret;
-}
-
-#define MAX_COMPRESS_THREADS 4
-
-typedef ssize_t (*Qcow2CompressFunc)(void *dest, size_t dest_size,
-                                     const void *src, size_t src_size);
-typedef struct Qcow2CompressData {
-    void *dest;
-    size_t dest_size;
-    const void *src;
-    size_t src_size;
-    ssize_t ret;
-
-    Qcow2CompressFunc func;
-} Qcow2CompressData;
-
-static int qcow2_compress_pool_func(void *opaque)
-{
-    Qcow2CompressData *data = opaque;
-
-    data->ret = data->func(data->dest, data->dest_size,
-                           data->src, data->src_size);
-
-    return 0;
-}
-
-static void qcow2_compress_complete(void *opaque, int ret)
-{
-    qemu_coroutine_enter(opaque);
-}
-
-static ssize_t coroutine_fn
-qcow2_co_do_compress(BlockDriverState *bs, void *dest, size_t dest_size,
-                     const void *src, size_t src_size, Qcow2CompressFunc func)
-{
-    BDRVQcow2State *s = bs->opaque;
-    BlockAIOCB *acb;
-    ThreadPool *pool = aio_get_thread_pool(bdrv_get_aio_context(bs));
-    Qcow2CompressData arg = {
-        .dest = dest,
-        .dest_size = dest_size,
-        .src = src,
-        .src_size = src_size,
-        .func = func,
-    };
-
-    while (s->nb_compress_threads >= MAX_COMPRESS_THREADS) {
-        qemu_co_queue_wait(&s->compress_wait_queue, NULL);
-    }
-
-    s->nb_compress_threads++;
-    acb = thread_pool_submit_aio(pool, qcow2_compress_pool_func, &arg,
-                                 qcow2_compress_complete,
-                                 qemu_coroutine_self());
-
-    if (!acb) {
-        s->nb_compress_threads--;
-        return -EINVAL;
-    }
-    qemu_coroutine_yield();
-    s->nb_compress_threads--;
-    qemu_co_queue_next(&s->compress_wait_queue);
-
-    return arg.ret;
-}
-
-static ssize_t coroutine_fn
-qcow2_co_compress(BlockDriverState *bs, void *dest, size_t dest_size,
-                  const void *src, size_t src_size)
-{
-    return qcow2_co_do_compress(bs, dest, dest_size, src, src_size,
-                                qcow2_compress);
-}
-
-static ssize_t coroutine_fn
-qcow2_co_decompress(BlockDriverState *bs, void *dest, size_t dest_size,
-                    const void *src, size_t src_size)
-{
-    return qcow2_co_do_compress(bs, dest, dest_size, src, src_size,
-                                qcow2_decompress);
-}
-
 /* XXX: put compressed sectors first, then all the cluster aligned
    tables to avoid losing bytes in alignment */
 static coroutine_fn int
-- 
2.21.0

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

Use thread_pool_submit_co, instead of reinventing it here. Note, that
thread_pool_submit_aio() never returns NULL, so checking it was an
extra thing.

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 int qcow2_compress_pool_func(void *opaque)
     return 0;
 }
 
-static void qcow2_compress_complete(void *opaque, int ret)
-{
-    qemu_coroutine_enter(opaque);
-}
-
 static ssize_t coroutine_fn
 qcow2_co_do_compress(BlockDriverState *bs, void *dest, size_t dest_size,
                      const void *src, size_t src_size, Qcow2CompressFunc func)
 {
     BDRVQcow2State *s = bs->opaque;
-    BlockAIOCB *acb;
     ThreadPool *pool = aio_get_thread_pool(bdrv_get_aio_context(bs));
     Qcow2CompressData arg = {
         .dest = dest,
@@ -XXX,XX +XXX,XX @@ qcow2_co_do_compress(BlockDriverState *bs, void *dest, size_t dest_size,
     }
 
     s->nb_compress_threads++;
-    acb = thread_pool_submit_aio(pool, qcow2_compress_pool_func, &arg,
-                                 qcow2_compress_complete,
-                                 qemu_coroutine_self());
-
-    if (!acb) {
-        s->nb_compress_threads--;
-        return -EINVAL;
-    }
-    qemu_coroutine_yield();
+    thread_pool_submit_co(pool, qcow2_compress_pool_func, &arg);
     s->nb_compress_threads--;
+
     qemu_co_queue_next(&s->compress_wait_queue);
 
     return arg.ret;
-- 
2.21.0

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

Drop dependence on AioContext lock.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 20190506142741.41731-5-vsementsov@virtuozzo.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2-threads.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 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 @@ qcow2_co_do_compress(BlockDriverState *bs, void *dest, size_t dest_size,
         .func = func,
     };
 
+    qemu_co_mutex_lock(&s->lock);
     while (s->nb_compress_threads >= MAX_COMPRESS_THREADS) {
-        qemu_co_queue_wait(&s->compress_wait_queue, NULL);
+        qemu_co_queue_wait(&s->compress_wait_queue, &s->lock);
     }
-
     s->nb_compress_threads++;
+    qemu_co_mutex_unlock(&s->lock);
+
     thread_pool_submit_co(pool, qcow2_compress_pool_func, &arg);
-    s->nb_compress_threads--;
 
+    qemu_co_mutex_lock(&s->lock);
+    s->nb_compress_threads--;
     qemu_co_queue_next(&s->compress_wait_queue);
+    qemu_co_mutex_unlock(&s->lock);
 
     return arg.ret;
 }
-- 
2.21.0

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

Move generic part out of qcow2_co_do_compress, to reuse it for
encryption and rename things that would be shared with encryption path.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 20190506142741.41731-6-vsementsov@virtuozzo.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.h         |  4 ++--
 block/qcow2-threads.c | 47 ++++++++++++++++++++++++++++---------------
 block/qcow2.c         |  2 +-
 3 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -XXX,XX +XXX,XX @@ typedef struct BDRVQcow2State {
     char *image_backing_format;
     char *image_data_file;
 
-    CoQueue compress_wait_queue;
-    int nb_compress_threads;
+    CoQueue thread_task_queue;
+    int nb_threads;
 
     BdrvChild *data_file;
 } BDRVQcow2State;
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 @@
 #include "qcow2.h"
 #include "block/thread-pool.h"
 
-#define MAX_COMPRESS_THREADS 4
+#define QCOW2_MAX_THREADS 4
+
+static int coroutine_fn
+qcow2_co_process(BlockDriverState *bs, ThreadPoolFunc *func, void *arg)
+{
+    int ret;
+    BDRVQcow2State *s = bs->opaque;
+    ThreadPool *pool = aio_get_thread_pool(bdrv_get_aio_context(bs));
+
+    qemu_co_mutex_lock(&s->lock);
+    while (s->nb_threads >= QCOW2_MAX_THREADS) {
+        qemu_co_queue_wait(&s->thread_task_queue, &s->lock);
+    }
+    s->nb_threads++;
+    qemu_co_mutex_unlock(&s->lock);
+
+    ret = thread_pool_submit_co(pool, func, arg);
+
+    qemu_co_mutex_lock(&s->lock);
+    s->nb_threads--;
+    qemu_co_queue_next(&s->thread_task_queue);
+    qemu_co_mutex_unlock(&s->lock);
+
+    return ret;
+}
+
+
+/*
+ * Compression
+ */
 
 typedef ssize_t (*Qcow2CompressFunc)(void *dest, size_t dest_size,
                                      const void *src, size_t src_size);
@@ -XXX,XX +XXX,XX @@ static ssize_t coroutine_fn
 qcow2_co_do_compress(BlockDriverState *bs, void *dest, size_t dest_size,
                      const void *src, size_t src_size, Qcow2CompressFunc func)
 {
-    BDRVQcow2State *s = bs->opaque;
-    ThreadPool *pool = aio_get_thread_pool(bdrv_get_aio_context(bs));
     Qcow2CompressData arg = {
         .dest = dest,
         .dest_size = dest_size,
@@ -XXX,XX +XXX,XX @@ qcow2_co_do_compress(BlockDriverState *bs, void *dest, size_t dest_size,
         .func = func,
     };
 
-    qemu_co_mutex_lock(&s->lock);
-    while (s->nb_compress_threads >= MAX_COMPRESS_THREADS) {
-        qemu_co_queue_wait(&s->compress_wait_queue, &s->lock);
-    }
-    s->nb_compress_threads++;
-    qemu_co_mutex_unlock(&s->lock);
-
-    thread_pool_submit_co(pool, qcow2_compress_pool_func, &arg);
-
-    qemu_co_mutex_lock(&s->lock);
-    s->nb_compress_threads--;
-    qemu_co_queue_next(&s->compress_wait_queue);
-    qemu_co_mutex_unlock(&s->lock);
+    qcow2_co_process(bs, qcow2_compress_pool_func, &arg);
 
     return arg.ret;
 }
diff --git a/block/qcow2.c b/block/qcow2.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
     }
 #endif
 
-    qemu_co_queue_init(&s->compress_wait_queue);
+    qemu_co_queue_init(&s->thread_task_queue);
 
     return ret;
 
-- 
2.21.0

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

Background: decryption will be done in threads, to take benefit of it,
we should move it out of the lock first.

But let's go further: it turns out, that only
qcow2_get_cluster_offset() needs locking, so reduce locking to it.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190506142741.41731-7-vsementsov@virtuozzo.com
Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
 
     qemu_iovec_init(&hd_qiov, qiov->niov);
 
-    qemu_co_mutex_lock(&s->lock);
-
     while (bytes != 0) {
 
         /* prepare next request */
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
                             QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size);
         }
 
+        qemu_co_mutex_lock(&s->lock);
         ret = qcow2_get_cluster_offset(bs, offset, &cur_bytes, &cluster_offset);
+        qemu_co_mutex_unlock(&s->lock);
         if (ret < 0) {
             goto fail;
         }
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
 
             if (bs->backing) {
                 BLKDBG_EVENT(bs->file, BLKDBG_READ_BACKING_AIO);
-                qemu_co_mutex_unlock(&s->lock);
                 ret = bdrv_co_preadv(bs->backing, offset, cur_bytes,
                                      &hd_qiov, 0);
-                qemu_co_mutex_lock(&s->lock);
                 if (ret < 0) {
                     goto fail;
                 }
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
             break;
 
         case QCOW2_CLUSTER_COMPRESSED:
-            qemu_co_mutex_unlock(&s->lock);
             ret = qcow2_co_preadv_compressed(bs, cluster_offset,
                                              offset, cur_bytes,
                                              &hd_qiov);
-            qemu_co_mutex_lock(&s->lock);
             if (ret < 0) {
                 goto fail;
             }
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
             }
 
             BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
-            qemu_co_mutex_unlock(&s->lock);
             ret = bdrv_co_preadv(s->data_file,
                                  cluster_offset + offset_in_cluster,
                                  cur_bytes, &hd_qiov, 0);
-            qemu_co_mutex_lock(&s->lock);
             if (ret < 0) {
                 goto fail;
             }
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
     ret = 0;
 
 fail:
-    qemu_co_mutex_unlock(&s->lock);
-
     qemu_iovec_destroy(&hd_qiov);
     qemu_vfree(cluster_data);
 
-- 
2.21.0

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

Encryption will be done in threads, to take benefit of it, we should
move it out of the lock first.

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_pwritev(BlockDriverState *bs, uint64_t offset,
         ret = qcow2_alloc_cluster_offset(bs, offset, &cur_bytes,
                                          &cluster_offset, &l2meta);
         if (ret < 0) {
-            goto fail;
+            goto out_locked;
         }
 
         assert((cluster_offset & 511) == 0);
 
+        ret = qcow2_pre_write_overlap_check(bs, 0,
+                                            cluster_offset + offset_in_cluster,
+                                            cur_bytes, true);
+        if (ret < 0) {
+            goto out_locked;
+        }
+
+        qemu_co_mutex_unlock(&s->lock);
+
         qemu_iovec_reset(&hd_qiov);
         qemu_iovec_concat(&hd_qiov, qiov, bytes_done, cur_bytes);
 
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
                                                    * s->cluster_size);
                 if (cluster_data == NULL) {
                     ret = -ENOMEM;
-                    goto fail;
+                    goto out_unlocked;
                 }
             }
 
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
                                       cluster_data,
                                       cur_bytes, NULL) < 0) {
                 ret = -EIO;
-                goto fail;
+                goto out_unlocked;
             }
 
             qemu_iovec_reset(&hd_qiov);
             qemu_iovec_add(&hd_qiov, cluster_data, cur_bytes);
         }
 
-        ret = qcow2_pre_write_overlap_check(bs, 0,
-                cluster_offset + offset_in_cluster, cur_bytes, true);
-        if (ret < 0) {
-            goto fail;
-        }
-
         /* If we need to do COW, check if it's possible to merge the
          * writing of the guest data together with that of the COW regions.
          * If it's not possible (or not necessary) then write the
          * guest data now. */
         if (!merge_cow(offset, cur_bytes, &hd_qiov, l2meta)) {
-            qemu_co_mutex_unlock(&s->lock);
             BLKDBG_EVENT(bs->file, BLKDBG_WRITE_AIO);
             trace_qcow2_writev_data(qemu_coroutine_self(),
                                     cluster_offset + offset_in_cluster);
             ret = bdrv_co_pwritev(s->data_file,
                                   cluster_offset + offset_in_cluster,
                                   cur_bytes, &hd_qiov, 0);
-            qemu_co_mutex_lock(&s->lock);
             if (ret < 0) {
-                goto fail;
+                goto out_unlocked;
             }
         }
 
+        qemu_co_mutex_lock(&s->lock);
+
         ret = qcow2_handle_l2meta(bs, &l2meta, true);
         if (ret) {
-            goto fail;
+            goto out_locked;
         }
 
         bytes -= cur_bytes;
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
         trace_qcow2_writev_done_part(qemu_coroutine_self(), cur_bytes);
     }
     ret = 0;
+    goto out_locked;
 
-fail:
+out_unlocked:
+    qemu_co_mutex_lock(&s->lock);
+
+out_locked:
     qcow2_handle_l2meta(bs, &l2meta, false);
 
     qemu_co_mutex_unlock(&s->lock);
-- 
2.21.0

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

Do encryption/decryption in threads, like it is already done for
compression. This improves asynchronous encrypted io.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 20190506142741.41731-9-vsementsov@virtuozzo.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.h         |  8 ++++++
 block/qcow2-cluster.c |  7 ++---
 block/qcow2-threads.c | 65 +++++++++++++++++++++++++++++++++++++++++--
 block/qcow2.c         | 22 +++++----------
 4 files changed, 81 insertions(+), 21 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -XXX,XX +XXX,XX @@ typedef struct Qcow2BitmapHeaderExt {
     uint64_t bitmap_directory_offset;
 } QEMU_PACKED Qcow2BitmapHeaderExt;
 
+#define QCOW2_MAX_THREADS 4
+
 typedef struct BDRVQcow2State {
     int cluster_bits;
     int cluster_size;
@@ -XXX,XX +XXX,XX @@ qcow2_co_compress(BlockDriverState *bs, void *dest, size_t dest_size,
 ssize_t coroutine_fn
 qcow2_co_decompress(BlockDriverState *bs, void *dest, size_t dest_size,
                     const void *src, size_t src_size);
+int coroutine_fn
+qcow2_co_encrypt(BlockDriverState *bs, uint64_t file_cluster_offset,
+                 uint64_t offset, void *buf, size_t len);
+int coroutine_fn
+qcow2_co_decrypt(BlockDriverState *bs, uint64_t file_cluster_offset,
+                 uint64_t offset, void *buf, size_t len);
 
 #endif
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -XXX,XX +XXX,XX @@ static bool coroutine_fn do_perform_cow_encrypt(BlockDriverState *bs,
 {
     if (bytes && bs->encrypted) {
         BDRVQcow2State *s = bs->opaque;
-        int64_t offset = (s->crypt_physical_offset ?
-                          (cluster_offset + offset_in_cluster) :
-                          (src_cluster_offset + offset_in_cluster));
         assert((offset_in_cluster & ~BDRV_SECTOR_MASK) == 0);
         assert((bytes & ~BDRV_SECTOR_MASK) == 0);
         assert(s->crypto);
-        if (qcrypto_block_encrypt(s->crypto, offset, buffer, bytes, NULL) < 0) {
+        if (qcow2_co_encrypt(bs, cluster_offset,
+                             src_cluster_offset + offset_in_cluster,
+                             buffer, bytes) < 0) {
             return false;
         }
     }
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 @@
 
 #include "qcow2.h"
 #include "block/thread-pool.h"
-
-#define QCOW2_MAX_THREADS 4
+#include "crypto.h"
 
 static int coroutine_fn
 qcow2_co_process(BlockDriverState *bs, ThreadPoolFunc *func, void *arg)
@@ -XXX,XX +XXX,XX @@ qcow2_co_decompress(BlockDriverState *bs, void *dest, size_t dest_size,
     return qcow2_co_do_compress(bs, dest, dest_size, src, src_size,
                                 qcow2_decompress);
 }
+
+
+/*
+ * Cryptography
+ */
+
+/*
+ * Qcow2EncDecFunc: common prototype of qcrypto_block_encrypt() and
+ * qcrypto_block_decrypt() functions.
+ */
+typedef int (*Qcow2EncDecFunc)(QCryptoBlock *block, uint64_t offset,
+                               uint8_t *buf, size_t len, Error **errp);
+
+typedef struct Qcow2EncDecData {
+    QCryptoBlock *block;
+    uint64_t offset;
+    uint8_t *buf;
+    size_t len;
+
+    Qcow2EncDecFunc func;
+} Qcow2EncDecData;
+
+static int qcow2_encdec_pool_func(void *opaque)
+{
+    Qcow2EncDecData *data = opaque;
+
+    return data->func(data->block, data->offset, data->buf, data->len, NULL);
+}
+
+static int coroutine_fn
+qcow2_co_encdec(BlockDriverState *bs, uint64_t file_cluster_offset,
+                  uint64_t offset, void *buf, size_t len, Qcow2EncDecFunc func)
+{
+    BDRVQcow2State *s = bs->opaque;
+    Qcow2EncDecData arg = {
+        .block = s->crypto,
+        .offset = s->crypt_physical_offset ?
+                      file_cluster_offset + offset_into_cluster(s, offset) :
+                      offset,
+        .buf = buf,
+        .len = len,
+        .func = func,
+    };
+
+    return qcow2_co_process(bs, qcow2_encdec_pool_func, &arg);
+}
+
+int coroutine_fn
+qcow2_co_encrypt(BlockDriverState *bs, uint64_t file_cluster_offset,
+                 uint64_t offset, void *buf, size_t len)
+{
+    return qcow2_co_encdec(bs, file_cluster_offset, offset, buf, len,
+                             qcrypto_block_encrypt);
+}
+
+int coroutine_fn
+qcow2_co_decrypt(BlockDriverState *bs, uint64_t file_cluster_offset,
+                 uint64_t offset, void *buf, size_t len)
+{
+    return qcow2_co_encdec(bs, file_cluster_offset, offset, buf, len,
+                             qcrypto_block_decrypt);
+}
diff --git a/block/qcow2.c b/block/qcow2.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -XXX,XX +XXX,XX @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
             }
             s->crypto = qcrypto_block_open(s->crypto_opts, "encrypt.",
                                            qcow2_crypto_hdr_read_func,
-                                           bs, cflags, 1, errp);
+                                           bs, cflags, QCOW2_MAX_THREADS, errp);
             if (!s->crypto) {
                 return -EINVAL;
             }
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
                 cflags |= QCRYPTO_BLOCK_OPEN_NO_IO;
             }
             s->crypto = qcrypto_block_open(s->crypto_opts, "encrypt.",
-                                           NULL, NULL, cflags, 1, errp);
+                                           NULL, NULL, cflags,
+                                           QCOW2_MAX_THREADS, errp);
             if (!s->crypto) {
                 ret = -EINVAL;
                 goto fail;
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
                 assert(s->crypto);
                 assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
                 assert((cur_bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
-                if (qcrypto_block_decrypt(s->crypto,
-                                          (s->crypt_physical_offset ?
-                                           cluster_offset + offset_in_cluster :
-                                           offset),
-                                          cluster_data,
-                                          cur_bytes,
-                                          NULL) < 0) {
+                if (qcow2_co_decrypt(bs, cluster_offset, offset,
+                                     cluster_data, cur_bytes) < 0) {
                     ret = -EIO;
                     goto fail;
                 }
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
                    QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size);
             qemu_iovec_to_buf(&hd_qiov, 0, cluster_data, hd_qiov.size);
 
-            if (qcrypto_block_encrypt(s->crypto,
-                                      (s->crypt_physical_offset ?
-                                       cluster_offset + offset_in_cluster :
-                                       offset),
-                                      cluster_data,
-                                      cur_bytes, NULL) < 0) {
+            if (qcow2_co_encrypt(bs, cluster_offset, offset,
+                                 cluster_data, cur_bytes) < 0) {
                 ret = -EIO;
                 goto out_unlocked;
             }
-- 
2.21.0

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

Simplify backup_incremental_init_copy_bitmap using the function
bdrv_dirty_bitmap_next_dirty_area.

Note: move to job->len instead of bitmap size: it should not matter but
less code.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 20190429090842.57910-2-vsementsov@virtuozzo.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/backup.c | 40 ++++++++++++----------------------------
 1 file changed, 12 insertions(+), 28 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 int coroutine_fn backup_run_incremental(BackupBlockJob *job)
 /* init copy_bitmap from sync_bitmap */
 static void backup_incremental_init_copy_bitmap(BackupBlockJob *job)
 {
-    BdrvDirtyBitmapIter *dbi;
-    int64_t offset;
-    int64_t end = DIV_ROUND_UP(bdrv_dirty_bitmap_size(job->sync_bitmap),
-                               job->cluster_size);
-
-    dbi = bdrv_dirty_iter_new(job->sync_bitmap);
-    while ((offset = bdrv_dirty_iter_next(dbi)) != -1) {
-        int64_t cluster = offset / job->cluster_size;
-        int64_t next_cluster;
-
-        offset += bdrv_dirty_bitmap_granularity(job->sync_bitmap);
-        if (offset >= bdrv_dirty_bitmap_size(job->sync_bitmap)) {
-            hbitmap_set(job->copy_bitmap, cluster, end - cluster);
-            break;
-        }
+    uint64_t offset = 0;
+    uint64_t bytes = job->len;
 
-        offset = bdrv_dirty_bitmap_next_zero(job->sync_bitmap, offset,
-                                             UINT64_MAX);
-        if (offset == -1) {
-            hbitmap_set(job->copy_bitmap, cluster, end - cluster);
-            break;
-        }
+    while (bdrv_dirty_bitmap_next_dirty_area(job->sync_bitmap,
+                                             &offset, &bytes))
+    {
+        uint64_t cluster = offset / job->cluster_size;
+        uint64_t end_cluster = DIV_ROUND_UP(offset + bytes, job->cluster_size);
 
-        next_cluster = DIV_ROUND_UP(offset, job->cluster_size);
-        hbitmap_set(job->copy_bitmap, cluster, next_cluster - cluster);
-        if (next_cluster >= end) {
+        hbitmap_set(job->copy_bitmap, cluster, end_cluster - cluster);
+
+        offset = end_cluster * job->cluster_size;
+        if (offset >= job->len) {
             break;
         }
-
-        bdrv_set_dirty_iter(dbi, next_cluster * job->cluster_size);
+        bytes = job->len - offset;
     }
 
     /* TODO job_progress_set_remaining() would make more sense */
     job_progress_update(&job->common.job,
         job->len - hbitmap_count(job->copy_bitmap) * job->cluster_size);
-
-    bdrv_dirty_iter_free(dbi);
 }
 
 static int coroutine_fn backup_run(Job *job, Error **errp)
-- 
2.21.0

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

We are going to share this bitmap between backup and backup-top filter
driver, so let's share something more meaningful. It also simplifies
some calculations.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 20190429090842.57910-3-vsementsov@virtuozzo.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/backup.c | 48 +++++++++++++++++++++++-------------------------
 1 file changed, 23 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 int coroutine_fn backup_cow_with_bounce_buffer(BackupBlockJob *job,
     int read_flags = is_write_notifier ? BDRV_REQ_NO_SERIALISING : 0;
     int write_flags = job->serialize_target_writes ? BDRV_REQ_SERIALISING : 0;
 
-    hbitmap_reset(job->copy_bitmap, start / job->cluster_size, 1);
+    assert(QEMU_IS_ALIGNED(start, job->cluster_size));
+    hbitmap_reset(job->copy_bitmap, start, job->cluster_size);
     nbytes = MIN(job->cluster_size, job->len - start);
     if (!*bounce_buffer) {
         *bounce_buffer = blk_blockalign(blk, job->cluster_size);
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_cow_with_bounce_buffer(BackupBlockJob *job,
 
     return nbytes;
 fail:
-    hbitmap_set(job->copy_bitmap, start / job->cluster_size, 1);
+    hbitmap_set(job->copy_bitmap, start, job->cluster_size);
     return ret;
 
 }
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_cow_with_offload(BackupBlockJob *job,
     int write_flags = job->serialize_target_writes ? BDRV_REQ_SERIALISING : 0;
 
     assert(QEMU_IS_ALIGNED(job->copy_range_size, job->cluster_size));
+    assert(QEMU_IS_ALIGNED(start, job->cluster_size));
     nbytes = MIN(job->copy_range_size, end - start);
     nr_clusters = DIV_ROUND_UP(nbytes, job->cluster_size);
-    hbitmap_reset(job->copy_bitmap, start / job->cluster_size,
-                  nr_clusters);
+    hbitmap_reset(job->copy_bitmap, start, job->cluster_size * nr_clusters);
     ret = blk_co_copy_range(blk, start, job->target, start, nbytes,
                             read_flags, write_flags);
     if (ret < 0) {
         trace_backup_do_cow_copy_range_fail(job, start, ret);
-        hbitmap_set(job->copy_bitmap, start / job->cluster_size,
-                    nr_clusters);
+        hbitmap_set(job->copy_bitmap, start, job->cluster_size * nr_clusters);
         return ret;
     }
 
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
     cow_request_begin(&cow_request, job, start, end);
 
     while (start < end) {
-        if (!hbitmap_get(job->copy_bitmap, start / job->cluster_size)) {
+        if (!hbitmap_get(job->copy_bitmap, start)) {
             trace_backup_do_cow_skip(job, start);
             start += job->cluster_size;
             continue; /* already copied */
@@ -XXX,XX +XXX,XX @@ static void backup_clean(Job *job)
     assert(s->target);
     blk_unref(s->target);
     s->target = NULL;
+
+    if (s->copy_bitmap) {
+        hbitmap_free(s->copy_bitmap);
+        s->copy_bitmap = NULL;
+    }
 }
 
 void backup_do_checkpoint(BlockJob *job, Error **errp)
 {
     BackupBlockJob *backup_job = container_of(job, BackupBlockJob, common);
-    int64_t len;
 
     assert(block_job_driver(job) == &backup_job_driver);
 
@@ -XXX,XX +XXX,XX @@ void backup_do_checkpoint(BlockJob *job, Error **errp)
         return;
     }
 
-    len = DIV_ROUND_UP(backup_job->len, backup_job->cluster_size);
-    hbitmap_set(backup_job->copy_bitmap, 0, len);
+    hbitmap_set(backup_job->copy_bitmap, 0, backup_job->len);
 }
 
 static void backup_drain(BlockJob *job)
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
 {
     int ret;
     bool error_is_read;
-    int64_t cluster;
+    int64_t offset;
     HBitmapIter hbi;
 
     hbitmap_iter_init(&hbi, job->copy_bitmap, 0);
-    while ((cluster = hbitmap_iter_next(&hbi)) != -1) {
+    while ((offset = hbitmap_iter_next(&hbi)) != -1) {
         do {
             if (yield_and_check(job)) {
                 return 0;
             }
-            ret = backup_do_cow(job, cluster * job->cluster_size,
+            ret = backup_do_cow(job, offset,
                                 job->cluster_size, &error_is_read, false);
             if (ret < 0 && backup_error_action(job, error_is_read, -ret) ==
                            BLOCK_ERROR_ACTION_REPORT)
@@ -XXX,XX +XXX,XX @@ static void backup_incremental_init_copy_bitmap(BackupBlockJob *job)
     while (bdrv_dirty_bitmap_next_dirty_area(job->sync_bitmap,
                                              &offset, &bytes))
     {
-        uint64_t cluster = offset / job->cluster_size;
-        uint64_t end_cluster = DIV_ROUND_UP(offset + bytes, job->cluster_size);
+        hbitmap_set(job->copy_bitmap, offset, bytes);
 
-        hbitmap_set(job->copy_bitmap, cluster, end_cluster - cluster);
-
-        offset = end_cluster * job->cluster_size;
+        offset += bytes;
         if (offset >= job->len) {
             break;
         }
@@ -XXX,XX +XXX,XX @@ static void backup_incremental_init_copy_bitmap(BackupBlockJob *job)
 
     /* TODO job_progress_set_remaining() would make more sense */
     job_progress_update(&job->common.job,
-        job->len - hbitmap_count(job->copy_bitmap) * job->cluster_size);
+        job->len - hbitmap_count(job->copy_bitmap));
 }
 
 static int coroutine_fn backup_run(Job *job, Error **errp)
 {
     BackupBlockJob *s = container_of(job, BackupBlockJob, common.job);
     BlockDriverState *bs = blk_bs(s->common.blk);
-    int64_t offset, nb_clusters;
+    int64_t offset;
     int ret = 0;
 
     QLIST_INIT(&s->inflight_reqs);
     qemu_co_rwlock_init(&s->flush_rwlock);
 
-    nb_clusters = DIV_ROUND_UP(s->len, s->cluster_size);
     job_progress_set_remaining(job, s->len);
 
-    s->copy_bitmap = hbitmap_alloc(nb_clusters, 0);
     if (s->sync_mode == MIRROR_SYNC_MODE_INCREMENTAL) {
         backup_incremental_init_copy_bitmap(s);
     } else {
-        hbitmap_set(s->copy_bitmap, 0, nb_clusters);
+        hbitmap_set(s->copy_bitmap, 0, s->len);
     }
 
-
     s->before_write.notify = backup_before_write_notify;
     bdrv_add_before_write_notifier(bs, &s->before_write);
 
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_run(Job *job, Error **errp)
     /* wait until pending backup_do_cow() calls have completed */
     qemu_co_rwlock_wrlock(&s->flush_rwlock);
     qemu_co_rwlock_unlock(&s->flush_rwlock);
-    hbitmap_free(s->copy_bitmap);
 
     return ret;
 }
@@ -XXX,XX +XXX,XX @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
     } else {
         job->cluster_size = MAX(BACKUP_CLUSTER_SIZE_DEFAULT, bdi.cluster_size);
     }
+
+    job->copy_bitmap = hbitmap_alloc(len, ctz32(job->cluster_size));
     job->use_copy_range = true;
     job->copy_range_size = MIN_NON_ZERO(blk_get_max_transfer(job->common.blk),
                                         blk_get_max_transfer(job->target));
-- 
2.21.0

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

Split allocation checking to separate function and reduce nesting.
Consider bdrv_is_allocated() fail as allocated area, as copying more
than needed is not wrong (and we do it anyway) and seems better than
fail the whole job. And, most probably we will fail on the next read,
if there are real problem with source.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 20190429090842.57910-4-vsementsov@virtuozzo.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/backup.c | 60 +++++++++++++++++++-------------------------------
 1 file changed, 23 insertions(+), 37 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 bool coroutine_fn yield_and_check(BackupBlockJob *job)
     return false;
 }
 
+static bool bdrv_is_unallocated_range(BlockDriverState *bs,
+                                      int64_t offset, int64_t bytes)
+{
+    int64_t end = offset + bytes;
+
+    while (offset < end && !bdrv_is_allocated(bs, offset, bytes, &bytes)) {
+        if (bytes == 0) {
+            return true;
+        }
+        offset += bytes;
+        bytes = end - offset;
+    }
+
+    return offset >= end;
+}
+
 static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
 {
     int ret;
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_run(Job *job, Error **errp)
         for (offset = 0; offset < s->len;
              offset += s->cluster_size) {
             bool error_is_read;
-            int alloced = 0;
 
             if (yield_and_check(s)) {
                 break;
             }
 
-            if (s->sync_mode == MIRROR_SYNC_MODE_TOP) {
-                int i;
-                int64_t n;
-
-                /* Check to see if these blocks are already in the
-                 * backing file. */
-
-                for (i = 0; i < s->cluster_size;) {
-                    /* bdrv_is_allocated() only returns true/false based
-                     * on the first set of sectors it comes across that
-                     * are are all in the same state.
-                     * For that reason we must verify each sector in the
-                     * backup cluster length.  We end up copying more than
-                     * needed but at some point that is always the case. */
-                    alloced =
-                        bdrv_is_allocated(bs, offset + i,
-                                          s->cluster_size - i, &n);
-                    i += n;
-
-                    if (alloced || n == 0) {
-                        break;
-                    }
-                }
-
-                /* If the above loop never found any sectors that are in
-                 * the topmost image, skip this backup. */
-                if (alloced == 0) {
-                    continue;
-                }
-            }
-            /* FULL sync mode we copy the whole drive. */
-            if (alloced < 0) {
-                ret = alloced;
-            } else {
-                ret = backup_do_cow(s, offset, s->cluster_size,
-                                    &error_is_read, false);
+            if (s->sync_mode == MIRROR_SYNC_MODE_TOP &&
+                bdrv_is_unallocated_range(bs, offset, s->cluster_size))
+            {
+                continue;
             }
+
+            ret = backup_do_cow(s, offset, s->cluster_size,
+                                &error_is_read, false);
             if (ret < 0) {
                 /* Depending on error action, fail now or retry cluster */
                 BlockErrorAction action =
-- 
2.21.0

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

Do full, top and incremental mode copying all in one place. This
unifies the code path and helps further improvements.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 20190429090842.57910-5-vsementsov@virtuozzo.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/backup.c | 43 ++++++++++---------------------------------
 1 file changed, 10 insertions(+), 33 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 bool bdrv_is_unallocated_range(BlockDriverState *bs,
     return offset >= end;
 }
 
-static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
+static int coroutine_fn backup_loop(BackupBlockJob *job)
 {
     int ret;
     bool error_is_read;
     int64_t offset;
     HBitmapIter hbi;
+    BlockDriverState *bs = blk_bs(job->common.blk);
 
     hbitmap_iter_init(&hbi, job->copy_bitmap, 0);
     while ((offset = hbitmap_iter_next(&hbi)) != -1) {
+        if (job->sync_mode == MIRROR_SYNC_MODE_TOP &&
+            bdrv_is_unallocated_range(bs, offset, job->cluster_size))
+        {
+            hbitmap_reset(job->copy_bitmap, offset, job->cluster_size);
+            continue;
+        }
+
         do {
             if (yield_and_check(job)) {
                 return 0;
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_run(Job *job, Error **errp)
 {
     BackupBlockJob *s = container_of(job, BackupBlockJob, common.job);
     BlockDriverState *bs = blk_bs(s->common.blk);
-    int64_t offset;
     int ret = 0;
 
     QLIST_INIT(&s->inflight_reqs);
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_run(Job *job, Error **errp)
              * notify callback service CoW requests. */
             job_yield(job);
         }
-    } else if (s->sync_mode == MIRROR_SYNC_MODE_INCREMENTAL) {
-        ret = backup_run_incremental(s);
     } else {
-        /* Both FULL and TOP SYNC_MODE's require copying.. */
-        for (offset = 0; offset < s->len;
-             offset += s->cluster_size) {
-            bool error_is_read;
-
-            if (yield_and_check(s)) {
-                break;
-            }
-
-            if (s->sync_mode == MIRROR_SYNC_MODE_TOP &&
-                bdrv_is_unallocated_range(bs, offset, s->cluster_size))
-            {
-                continue;
-            }
-
-            ret = backup_do_cow(s, offset, s->cluster_size,
-                                &error_is_read, false);
-            if (ret < 0) {
-                /* Depending on error action, fail now or retry cluster */
-                BlockErrorAction action =
-                    backup_error_action(s, error_is_read, -ret);
-                if (action == BLOCK_ERROR_ACTION_REPORT) {
-                    break;
-                } else {
-                    offset -= s->cluster_size;
-                    continue;
-                }
-            }
-        }
+        ret = backup_loop(s);
     }
 
     notifier_with_return_remove(&s->before_write);
-- 
2.21.0

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

Split out cluster_size calculation. Move copy-bitmap creation above
block-job creation, as we are going to share it with upcoming
backup-top filter, which also should be created before actual block job
creation.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190429090842.57910-6-vsementsov@virtuozzo.com
[mreitz: Dropped a paragraph from the commit message that was left over
         from a previous version]
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/backup.c | 82 ++++++++++++++++++++++++++++++++------------------
 1 file changed, 52 insertions(+), 30 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 const BlockJobDriver backup_job_driver = {
     .drain                  = backup_drain,
 };
 
+static int64_t backup_calculate_cluster_size(BlockDriverState *target,
+                                             Error **errp)
+{
+    int ret;
+    BlockDriverInfo bdi;
+
+    /*
+     * If there is no backing file on the target, we cannot rely on COW if our
+     * backup cluster size is smaller than the target cluster size. Even for
+     * targets with a backing file, try to avoid COW if possible.
+     */
+    ret = bdrv_get_info(target, &bdi);
+    if (ret == -ENOTSUP && !target->backing) {
+        /* Cluster size is not defined */
+        warn_report("The target block device doesn't provide "
+                    "information about the block size and it doesn't have a "
+                    "backing file. The default block size of %u bytes is "
+                    "used. If the actual block size of the target exceeds "
+                    "this default, the backup may be unusable",
+                    BACKUP_CLUSTER_SIZE_DEFAULT);
+        return BACKUP_CLUSTER_SIZE_DEFAULT;
+    } else if (ret < 0 && !target->backing) {
+        error_setg_errno(errp, -ret,
+            "Couldn't determine the cluster size of the target image, "
+            "which has no backing file");
+        error_append_hint(errp,
+            "Aborting, since this may create an unusable destination image\n");
+        return ret;
+    } else if (ret < 0 && target->backing) {
+        /* Not fatal; just trudge on ahead. */
+        return BACKUP_CLUSTER_SIZE_DEFAULT;
+    }
+
+    return MAX(BACKUP_CLUSTER_SIZE_DEFAULT, bdi.cluster_size);
+}
+
 BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
                   BlockDriverState *target, int64_t speed,
                   MirrorSyncMode sync_mode, BdrvDirtyBitmap *sync_bitmap,
@@ -XXX,XX +XXX,XX @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
                   JobTxn *txn, Error **errp)
 {
     int64_t len;
-    BlockDriverInfo bdi;
     BackupBlockJob *job = NULL;
     int ret;
+    int64_t cluster_size;
+    HBitmap *copy_bitmap = NULL;
 
     assert(bs);
     assert(target);
@@ -XXX,XX +XXX,XX @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
         goto error;
     }
 
+    cluster_size = backup_calculate_cluster_size(target, errp);
+    if (cluster_size < 0) {
+        goto error;
+    }
+
+    copy_bitmap = hbitmap_alloc(len, ctz32(cluster_size));
+
     /* job->len is fixed, so we can't allow resize */
     job = block_job_create(job_id, &backup_job_driver, txn, bs,
                            BLK_PERM_CONSISTENT_READ,
@@ -XXX,XX +XXX,XX @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
 
     /* Detect image-fleecing (and similar) schemes */
     job->serialize_target_writes = bdrv_chain_contains(target, bs);
-
-    /* If there is no backing file on the target, we cannot rely on COW if our
-     * backup cluster size is smaller than the target cluster size. Even for
-     * targets with a backing file, try to avoid COW if possible. */
-    ret = bdrv_get_info(target, &bdi);
-    if (ret == -ENOTSUP && !target->backing) {
-        /* Cluster size is not defined */
-        warn_report("The target block device doesn't provide "
-                    "information about the block size and it doesn't have a "
-                    "backing file. The default block size of %u bytes is "
-                    "used. If the actual block size of the target exceeds "
-                    "this default, the backup may be unusable",
-                    BACKUP_CLUSTER_SIZE_DEFAULT);
-        job->cluster_size = BACKUP_CLUSTER_SIZE_DEFAULT;
-    } else if (ret < 0 && !target->backing) {
-        error_setg_errno(errp, -ret,
-            "Couldn't determine the cluster size of the target image, "
-            "which has no backing file");
-        error_append_hint(errp,
-            "Aborting, since this may create an unusable destination image\n");
-        goto error;
-    } else if (ret < 0 && target->backing) {
-        /* Not fatal; just trudge on ahead. */
-        job->cluster_size = BACKUP_CLUSTER_SIZE_DEFAULT;
-    } else {
-        job->cluster_size = MAX(BACKUP_CLUSTER_SIZE_DEFAULT, bdi.cluster_size);
-    }
-
-    job->copy_bitmap = hbitmap_alloc(len, ctz32(job->cluster_size));
+    job->cluster_size = cluster_size;
+    job->copy_bitmap = copy_bitmap;
+    copy_bitmap = NULL;
     job->use_copy_range = true;
     job->copy_range_size = MIN_NON_ZERO(blk_get_max_transfer(job->common.blk),
                                         blk_get_max_transfer(job->target));
@@ -XXX,XX +XXX,XX @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
     return &job->common;
 
  error:
+    if (copy_bitmap) {
+        assert(!job || !job->copy_bitmap);
+        hbitmap_free(copy_bitmap);
+    }
     if (sync_bitmap) {
         bdrv_reclaim_dirty_bitmap(bs, sync_bitmap, NULL);
     }
-- 
2.21.0

From: Alberto Garcia <berto@igalia.com>

bdrv_unref_child() does the following things:

- Updates the child->bs->inherits_from pointer.
  - Calls bdrv_detach_child() to remove the BdrvChild from bs->children.
  - Calls bdrv_unref() to unref the child BlockDriverState.

When bdrv_unref_child() was introduced in commit 33a604075c it was not
used in bdrv_close() because the drivers that had additional children
(like quorum or blkverify) had already called bdrv_unref() on their
children during their own close functions.

This was changed later (in 0bd6e91a7e for quorum, in 3e586be0b2 for
blkverify) so there's no reason not to use bdrv_unref_child() in
bdrv_close() anymore.

After this there's also no need to remove bs->backing and bs->file
separately from the rest of the children, so bdrv_close() can be
simplified.

Now bdrv_close() unrefs all children (before this patch it was only
bs->file and bs->backing). As a result, none of the callers of
brvd_attach_child() should remove their reference to child_bs (because
this function effectively steals that reference). This patch updates a
couple of tests that were doing their own bdrv_unref().

Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-id: 6d1d5feaa53aa1ab127adb73d605dc4503e3abd5.1557754872.git.berto@igalia.com
[mreitz: s/where/were/]
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c                     | 16 +++-------------
 tests/test-bdrv-drain.c     |  6 ------
 tests/test-bdrv-graph-mod.c |  1 -
 3 files changed, 3 insertions(+), 20 deletions(-)

diff --git a/block.c b/block.c
index XXXXXXX..XXXXXXX 100644
--- a/block.c
+++ b/block.c
@@ -XXX,XX +XXX,XX @@ static void bdrv_close(BlockDriverState *bs)
         bs->drv = NULL;
     }
 
-    bdrv_set_backing_hd(bs, NULL, &error_abort);
-
-    if (bs->file != NULL) {
-        bdrv_unref_child(bs, bs->file);
-        bs->file = NULL;
-    }
-
     QLIST_FOREACH_SAFE(child, &bs->children, next, next) {
-        /* TODO Remove bdrv_unref() from drivers' close function and use
-         * bdrv_unref_child() here */
-        if (child->bs->inherits_from == bs) {
-            child->bs->inherits_from = NULL;
-        }
-        bdrv_detach_child(child);
+        bdrv_unref_child(bs, child);
     }
 
+    bs->backing = NULL;
+    bs->file = NULL;
     g_free(bs->opaque);
     bs->opaque = NULL;
     atomic_set(&bs->copy_on_read, 0);
diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c
index XXXXXXX..XXXXXXX 100644
--- a/tests/test-bdrv-drain.c
+++ b/tests/test-bdrv-drain.c
@@ -XXX,XX +XXX,XX @@ static void test_detach_indirect(bool by_parent_cb)
     bdrv_unref(parent_b);
     blk_unref(blk);
 
-    /* XXX Once bdrv_close() unref's children instead of just detaching them,
-     * this won't be necessary any more. */
-    bdrv_unref(a);
-    bdrv_unref(a);
-    bdrv_unref(c);
-
     g_assert_cmpint(a->refcnt, ==, 1);
     g_assert_cmpint(b->refcnt, ==, 1);
     g_assert_cmpint(c->refcnt, ==, 1);
diff --git a/tests/test-bdrv-graph-mod.c b/tests/test-bdrv-graph-mod.c
index XXXXXXX..XXXXXXX 100644
--- a/tests/test-bdrv-graph-mod.c
+++ b/tests/test-bdrv-graph-mod.c
@@ -XXX,XX +XXX,XX @@ static void test_update_perm_tree(void)
     g_assert_nonnull(local_err);
     error_free(local_err);
 
-    bdrv_unref(bs);
     blk_unref(root);
 }
 
-- 
2.21.0

From: Alberto Garcia <berto@igalia.com>

A consequence of the previous patch is that bdrv_attach_child()
transfers the reference to child_bs from the caller to parent_bs,
which will drop it on bdrv_close() or when someone calls
bdrv_unref_child().

But this only happens when bdrv_attach_child() succeeds. If it fails
then the caller is responsible for dropping the reference to child_bs.

This patch makes bdrv_attach_child() take the reference also when
there is an error, freeing the caller for having to do it.

A similar situation happens with bdrv_root_attach_child(), so the
changes on this patch affect both functions.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-id: 20dfb3d9ccec559cdd1a9690146abad5d204a186.1557754872.git.berto@igalia.com
[mreitz: Removed now superfluous BdrvChild * variable in
         bdrv_open_child()]
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c               | 30 ++++++++++++++++++------------
 block/block-backend.c |  3 +--
 block/quorum.c        |  1 -
 blockjob.c            |  2 +-
 4 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/block.c b/block.c
index XXXXXXX..XXXXXXX 100644
--- a/block.c
+++ b/block.c
@@ -XXX,XX +XXX,XX @@ static void bdrv_replace_child(BdrvChild *child, BlockDriverState *new_bs)
     }
 }
 
+/*
+ * This function steals the reference to child_bs from the caller.
+ * That reference is later dropped by bdrv_root_unref_child().
+ *
+ * On failure NULL is returned, errp is set and the reference to
+ * child_bs is also dropped.
+ */
 BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
                                   const char *child_name,
                                   const BdrvChildRole *child_role,
@@ -XXX,XX +XXX,XX @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
     ret = bdrv_check_update_perm(child_bs, NULL, perm, shared_perm, NULL, errp);
     if (ret < 0) {
         bdrv_abort_perm_update(child_bs);
+        bdrv_unref(child_bs);
         return NULL;
     }
 
@@ -XXX,XX +XXX,XX @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
     return child;
 }
 
+/*
+ * This function transfers the reference to child_bs from the caller
+ * to parent_bs. That reference is later dropped by parent_bs on
+ * bdrv_close() or if someone calls bdrv_unref_child().
+ *
+ * On failure NULL is returned, errp is set and the reference to
+ * child_bs is also dropped.
+ */
 BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
                              BlockDriverState *child_bs,
                              const char *child_name,
@@ -XXX,XX +XXX,XX @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
     /* If backing_hd was already part of bs's backing chain, and
      * inherits_from pointed recursively to bs then let's update it to
      * point directly to bs (else it will become NULL). */
-    if (update_inherits_from) {
+    if (bs->backing && update_inherits_from) {
         backing_hd->inherits_from = bs;
     }
-    if (!bs->backing) {
-        bdrv_unref(backing_hd);
-    }
 
 out:
     bdrv_refresh_limits(bs, NULL);
@@ -XXX,XX +XXX,XX @@ BdrvChild *bdrv_open_child(const char *filename,
                            const BdrvChildRole *child_role,
                            bool allow_none, Error **errp)
 {
-    BdrvChild *c;
     BlockDriverState *bs;
 
     bs = bdrv_open_child_bs(filename, options, bdref_key, parent, child_role,
@@ -XXX,XX +XXX,XX @@ BdrvChild *bdrv_open_child(const char *filename,
         return NULL;
     }
 
-    c = bdrv_attach_child(parent, bs, bdref_key, child_role, errp);
-    if (!c) {
-        bdrv_unref(bs);
-        return NULL;
-    }
-
-    return c;
+    return bdrv_attach_child(parent, bs, bdref_key, child_role, errp);
 }
 
 /* TODO Future callers may need to specify parent/child_role in order for
diff --git a/block/block-backend.c b/block/block-backend.c
index XXXXXXX..XXXXXXX 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -XXX,XX +XXX,XX @@ BlockBackend *blk_new_open(const char *filename, const char *reference,
     blk->root = bdrv_root_attach_child(bs, "root", &child_root,
                                        perm, BLK_PERM_ALL, blk, errp);
     if (!blk->root) {
-        bdrv_unref(bs);
         blk_unref(blk);
         return NULL;
     }
@@ -XXX,XX +XXX,XX @@ void blk_remove_bs(BlockBackend *blk)
 int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp)
 {
     ThrottleGroupMember *tgm = &blk->public.throttle_group_member;
+    bdrv_ref(bs);
     blk->root = bdrv_root_attach_child(bs, "root", &child_root,
                                        blk->perm, blk->shared_perm, blk, errp);
     if (blk->root == NULL) {
         return -EPERM;
     }
-    bdrv_ref(bs);
 
     notifier_list_notify(&blk->insert_bs_notifiers, blk);
     if (tgm->throttle_state) {
diff --git a/block/quorum.c b/block/quorum.c
index XXXXXXX..XXXXXXX 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -XXX,XX +XXX,XX @@ static void quorum_add_child(BlockDriverState *bs, BlockDriverState *child_bs,
     child = bdrv_attach_child(bs, child_bs, indexstr, &child_format, errp);
     if (child == NULL) {
         s->next_child_index--;
-        bdrv_unref(child_bs);
         goto out;
     }
     s->children = g_renew(BdrvChild *, s->children, s->num_children + 1);
diff --git a/blockjob.c b/blockjob.c
index XXXXXXX..XXXXXXX 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -XXX,XX +XXX,XX @@ int block_job_add_bdrv(BlockJob *job, const char *name, BlockDriverState *bs,
 {
     BdrvChild *c;
 
+    bdrv_ref(bs);
     c = bdrv_root_attach_child(bs, name, &child_job, perm, shared_perm,
                                job, errp);
     if (c == NULL) {
@@ -XXX,XX +XXX,XX @@ int block_job_add_bdrv(BlockJob *job, const char *name, BlockDriverState *bs,
     }
 
     job->nodes = g_slist_prepend(job->nodes, c);
-    bdrv_ref(bs);
     bdrv_op_block_all(bs, job->blocker);
 
     return 0;
-- 
2.21.0

From: Sam Eiderman <shmuel.eiderman@oracle.com>

In safe mode we open the entire chain, including the parent backing
file of the rebased file.
Do not open a new BlockBackend for the parent backing file, which
saves opening the rest of the chain twice, which for long chains
saves many "pricy" bdrv_open() calls.

Permissions for blk_new() were copied from blk_new_open() when
flags = 0.

Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com>
Reviewed-by: Eyal Moscovici <eyal.moscovici@oracle.com>
Signed-off-by: Sagi Amit <sagi.amit@oracle.com>
Co-developed-by: Sagi Amit <sagi.amit@oracle.com>
Signed-off-by: Sam Eiderman <shmuel.eiderman@oracle.com>
Message-id: 20190523163337.4497-2-shmuel.eiderman@oracle.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qemu-img.c | 29 +++++++++--------------------
 1 file changed, 9 insertions(+), 20 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_rebase(int argc, char **argv)
 
     /* For safe rebasing we need to compare old and new backing file */
     if (!unsafe) {
-        char backing_name[PATH_MAX];
         QDict *options = NULL;
+        BlockDriverState *base_bs = backing_bs(bs);
 
-        if (bs->backing) {
-            if (bs->backing_format[0] != '\0') {
-                options = qdict_new();
-                qdict_put_str(options, "driver", bs->backing_format);
-            }
-
-            if (force_share) {
-                if (!options) {
-                    options = qdict_new();
-                }
-                qdict_put_bool(options, BDRV_OPT_FORCE_SHARE, true);
-            }
-            bdrv_get_backing_filename(bs, backing_name, sizeof(backing_name));
-            blk_old_backing = blk_new_open(backing_name, NULL,
-                                           options, src_flags, &local_err);
-            if (!blk_old_backing) {
+        if (base_bs) {
+            blk_old_backing = blk_new(BLK_PERM_CONSISTENT_READ,
+                                      BLK_PERM_ALL);
+            ret = blk_insert_bs(blk_old_backing, base_bs,
+                                &local_err);
+            if (ret < 0) {
                 error_reportf_err(local_err,
-                                  "Could not open old backing file '%s': ",
-                                  backing_name);
-                ret = -1;
+                                  "Could not reuse old backing file '%s': ",
+                                  base_bs->filename);
                 goto out;
             }
         } else {
-- 
2.21.0

From: Sam Eiderman <shmuel.eiderman@oracle.com>

In the following case:

(base) A <- B <- C (tip)

when running:

qemu-img rebase -b A C

QEMU would read all sectors not allocated in the file being rebased (C)
and compare them to the new base image (A), regardless of whether they
were changed or even allocated anywhere along the chain between the new
base and the top image (B). This causes many unneeded reads when
rebasing an image which represents a small diff of a large disk, as it
would read most of the disk's sectors.

Instead, use bdrv_is_allocated_above() to reduce the number of
unnecessary reads.

Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com>
Signed-off-by: Sam Eiderman <shmuel.eiderman@oracle.com>
Signed-off-by: Eyal Moscovici <eyal.moscovici@oracle.com>
Message-id: 20190523163337.4497-3-shmuel.eiderman@oracle.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qemu-img.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

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_rebase(int argc, char **argv)
     BlockBackend *blk = NULL, *blk_old_backing = NULL, *blk_new_backing = NULL;
     uint8_t *buf_old = NULL;
     uint8_t *buf_new = NULL;
-    BlockDriverState *bs = NULL;
+    BlockDriverState *bs = NULL, *prefix_chain_bs = NULL;
     char *filename;
     const char *fmt, *cache, *src_cache, *out_basefmt, *out_baseimg;
     int c, flags, src_flags, ret;
@@ -XXX,XX +XXX,XX @@ static int img_rebase(int argc, char **argv)
                 goto out;
             }
 
+            /*
+             * Find out whether we rebase an image on top of a previous image
+             * in its chain.
+             */
+            prefix_chain_bs = bdrv_find_backing_image(bs, out_real_path);
+
             blk_new_backing = blk_new_open(out_real_path, NULL,
                                            options, src_flags, &local_err);
             g_free(out_real_path);
@@ -XXX,XX +XXX,XX @@ static int img_rebase(int argc, char **argv)
                 continue;
             }
 
+            if (prefix_chain_bs) {
+                /*
+                 * If cluster wasn't changed since prefix_chain, we don't need
+                 * to take action
+                 */
+                ret = bdrv_is_allocated_above(backing_bs(bs), prefix_chain_bs,
+                                              offset, n, &n);
+                if (ret < 0) {
+                    error_report("error while reading image metadata: %s",
+                                 strerror(-ret));
+                    goto out;
+                }
+                if (!ret) {
+                    continue;
+                }
+            }
+
             /*
              * Read old and new backing file and take into consideration that
              * backing files may be smaller than the COW image.
-- 
2.21.0

From: Sam Eiderman <shmuel.eiderman@oracle.com>

If a chain was detected, don't open a new BlockBackend from the target
backing file which will create a new BlockDriverState. Instead, create
an empty BlockBackend and attach the already open BlockDriverState.

Permissions for blk_new() were copied from blk_new_open() when
flags = 0.

Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com>
Reviewed-by: Eyal Moscovici <eyal.moscovici@oracle.com>
Signed-off-by: Sagi Amit <sagi.amit@oracle.com>
Co-developed-by: Sagi Amit <sagi.amit@oracle.com>
Signed-off-by: Sam Eiderman <shmuel.eiderman@oracle.com>
Message-id: 20190523163337.4497-4-shmuel.eiderman@oracle.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qemu-img.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 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_rebase(int argc, char **argv)
              * in its chain.
              */
             prefix_chain_bs = bdrv_find_backing_image(bs, out_real_path);
-
-            blk_new_backing = blk_new_open(out_real_path, NULL,
-                                           options, src_flags, &local_err);
-            g_free(out_real_path);
-            if (!blk_new_backing) {
-                error_reportf_err(local_err,
-                                  "Could not open new backing file '%s': ",
-                                  out_baseimg);
-                ret = -1;
-                goto out;
+            if (prefix_chain_bs) {
+                g_free(out_real_path);
+                blk_new_backing = blk_new(BLK_PERM_CONSISTENT_READ,
+                                          BLK_PERM_ALL);
+                ret = blk_insert_bs(blk_new_backing, prefix_chain_bs,
+                                    &local_err);
+                if (ret < 0) {
+                    error_reportf_err(local_err,
+                                      "Could not reuse backing file '%s': ",
+                                      out_baseimg);
+                    goto out;
+                }
+            } else {
+                blk_new_backing = blk_new_open(out_real_path, NULL,
+                                               options, src_flags, &local_err);
+                g_free(out_real_path);
+                if (!blk_new_backing) {
+                    error_reportf_err(local_err,
+                                      "Could not open new backing file '%s': ",
+                                      out_baseimg);
+                    ret = -1;
+                    goto out;
+                }
             }
         }
     }
-- 
2.21.0

From: Anton Nefedov <anton.nefedov@virtuozzo.com>

If COW areas of the newly allocated clusters are zeroes on the backing
image, efficient bdrv_write_zeroes(flags=BDRV_REQ_NO_FALLBACK) can be
used on the whole cluster instead of writing explicit zero buffers later
in perform_cow().

iotest 060:
write to the discarded cluster does not trigger COW anymore.
Use a backing image instead.

Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
Message-id: 20190516142749.81019-2-anton.nefedov@virtuozzo.com
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qapi/block-core.json       |  4 +-
 block/qcow2.h              |  6 +++
 block/qcow2-cluster.c      |  2 +-
 block/qcow2.c              | 85 ++++++++++++++++++++++++++++++++++++++
 block/trace-events         |  1 +
 tests/qemu-iotests/060     |  7 +++-
 tests/qemu-iotests/060.out |  5 ++-
 7 files changed, 106 insertions(+), 4 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index XXXXXXX..XXXXXXX 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -XXX,XX +XXX,XX @@
 #
 # @cor_write: a write due to copy-on-read (since 2.11)
 #
+# @cluster_alloc_space: an allocation of file space for a cluster (since 4.1)
+#
 # Since: 2.9
 ##
 { 'enum': 'BlkdebugEvent', 'prefix': 'BLKDBG',
@@ -XXX,XX +XXX,XX @@
             'pwritev_rmw_tail', 'pwritev_rmw_after_tail', 'pwritev',
             'pwritev_zero', 'pwritev_done', 'empty_image_prepare',
             'l1_shrink_write_table', 'l1_shrink_free_l2_clusters',
-            'cor_write'] }
+            'cor_write', 'cluster_alloc_space'] }
 
 ##
 # @BlkdebugInjectErrorOptions:
diff --git a/block/qcow2.h b/block/qcow2.h
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -XXX,XX +XXX,XX @@ typedef struct QCowL2Meta
      */
     Qcow2COWRegion cow_end;
 
+    /*
+     * Indicates that COW regions are already handled and do not require
+     * any more processing.
+     */
+    bool skip_cow;
+
     /**
      * The I/O vector with the data from the actual guest write request.
      * If non-NULL, this is meant to be merged together with the data
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -XXX,XX +XXX,XX @@ static int perform_cow(BlockDriverState *bs, QCowL2Meta *m)
     assert(start->offset + start->nb_bytes <= end->offset);
     assert(!m->data_qiov || m->data_qiov->size == data_bytes);
 
-    if (start->nb_bytes == 0 && end->nb_bytes == 0) {
+    if ((start->nb_bytes == 0 && end->nb_bytes == 0) || m->skip_cow) {
         return 0;
     }
 
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 bool merge_cow(uint64_t offset, unsigned bytes,
             continue;
         }
 
+        /* If COW regions are handled already, skip this too */
+        if (m->skip_cow) {
+            continue;
+        }
+
         /* The data (middle) region must be immediately after the
          * start region */
         if (l2meta_cow_start(m) + m->cow_start.nb_bytes != offset) {
@@ -XXX,XX +XXX,XX @@ static bool merge_cow(uint64_t offset, unsigned bytes,
     return false;
 }
 
+static bool is_unallocated(BlockDriverState *bs, int64_t offset, int64_t bytes)
+{
+    int64_t nr;
+    return !bytes ||
+        (!bdrv_is_allocated_above(bs, NULL, offset, bytes, &nr) && nr == bytes);
+}
+
+static bool is_zero_cow(BlockDriverState *bs, QCowL2Meta *m)
+{
+    /*
+     * This check is designed for optimization shortcut so it must be
+     * efficient.
+     * Instead of is_zero(), use is_unallocated() as it is faster (but not
+     * as accurate and can result in false negatives).
+     */
+    return is_unallocated(bs, m->offset + m->cow_start.offset,
+                          m->cow_start.nb_bytes) &&
+           is_unallocated(bs, m->offset + m->cow_end.offset,
+                          m->cow_end.nb_bytes);
+}
+
+static int handle_alloc_space(BlockDriverState *bs, QCowL2Meta *l2meta)
+{
+    BDRVQcow2State *s = bs->opaque;
+    QCowL2Meta *m;
+
+    if (!(s->data_file->bs->supported_zero_flags & BDRV_REQ_NO_FALLBACK)) {
+        return 0;
+    }
+
+    if (bs->encrypted) {
+        return 0;
+    }
+
+    for (m = l2meta; m != NULL; m = m->next) {
+        int ret;
+
+        if (!m->cow_start.nb_bytes && !m->cow_end.nb_bytes) {
+            continue;
+        }
+
+        if (!is_zero_cow(bs, m)) {
+            continue;
+        }
+
+        /*
+         * instead of writing zero COW buffers,
+         * efficiently zero out the whole clusters
+         */
+
+        ret = qcow2_pre_write_overlap_check(bs, 0, m->alloc_offset,
+                                            m->nb_clusters * s->cluster_size,
+                                            true);
+        if (ret < 0) {
+            return ret;
+        }
+
+        BLKDBG_EVENT(bs->file, BLKDBG_CLUSTER_ALLOC_SPACE);
+        ret = bdrv_co_pwrite_zeroes(s->data_file, m->alloc_offset,
+                                    m->nb_clusters * s->cluster_size,
+                                    BDRV_REQ_NO_FALLBACK);
+        if (ret < 0) {
+            if (ret != -ENOTSUP && ret != -EAGAIN) {
+                return ret;
+            }
+            continue;
+        }
+
+        trace_qcow2_skip_cow(qemu_coroutine_self(), m->offset, m->nb_clusters);
+        m->skip_cow = true;
+    }
+    return 0;
+}
+
 static coroutine_fn int qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
                                          uint64_t bytes, QEMUIOVector *qiov,
                                          int flags)
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
             qemu_iovec_add(&hd_qiov, cluster_data, cur_bytes);
         }
 
+        /* Try to efficiently initialize the physical space with zeroes */
+        ret = handle_alloc_space(bs, l2meta);
+        if (ret < 0) {
+            goto out_unlocked;
+        }
+
         /* If we need to do COW, check if it's possible to merge the
          * writing of the guest data together with that of the COW regions.
          * If it's not possible (or not necessary) then write the
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 @@ qcow2_writev_done_part(void *co, int cur_bytes) "co %p cur_bytes %d"
 qcow2_writev_data(void *co, uint64_t offset) "co %p offset 0x%" PRIx64
 qcow2_pwrite_zeroes_start_req(void *co, int64_t offset, int count) "co %p offset 0x%" PRIx64 " count %d"
 qcow2_pwrite_zeroes(void *co, int64_t offset, int count) "co %p offset 0x%" PRIx64 " count %d"
+qcow2_skip_cow(void *co, uint64_t offset, int nb_clusters) "co %p offset 0x%" PRIx64 " nb_clusters %d"
 
 # qcow2-cluster.c
 qcow2_alloc_clusters_offset(void *co, uint64_t offset, int bytes) "co %p offset 0x%" PRIx64 " bytes %d"
diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/060
+++ b/tests/qemu-iotests/060
@@ -XXX,XX +XXX,XX @@ $QEMU_IO -c "$OPEN_RO" -c "read -P 1 0 512" | _filter_qemu_io
 echo
 echo "=== Testing overlap while COW is in flight ==="
 echo
+BACKING_IMG=$TEST_IMG.base
+TEST_IMG=$BACKING_IMG _make_test_img 1G
+
+$QEMU_IO -c 'write 0k 64k' "$BACKING_IMG" | _filter_qemu_io
+
 # compat=0.10 is required in order to make the following discard actually
 # unallocate the sector rather than make it a zero sector - we want COW, after
 # all.
-IMGOPTS='compat=0.10' _make_test_img 1G
+IMGOPTS='compat=0.10' _make_test_img -b "$BACKING_IMG" 1G
 # Write two clusters, the second one enforces creation of an L2 table after
 # the first data cluster.
 $QEMU_IO -c 'write 0k 64k' -c 'write 512M 64k' "$TEST_IMG" | _filter_qemu_io
diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/060.out
+++ b/tests/qemu-iotests/060.out
@@ -XXX,XX +XXX,XX @@ read 512/512 bytes at offset 0
 
 === Testing overlap while COW is in flight ===
 
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=1073741824
+wrote 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 backing_file=TEST_DIR/t.IMGFMT.base
 wrote 65536/65536 bytes at offset 0
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 wrote 65536/65536 bytes at offset 536870912
-- 
2.21.0

From: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>

Valgrind detects multiple issues in QEMU iotests when the memory is
used without being initialized. Valgrind may dump lots of unnecessary
reports what makes the memory issue analysis harder. Particularly,
that is true for the aligned bitmap directory and can be seen while
running the iotest #169. Padding the aligned space with zeros eases
the pain.

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Message-id: 1558961521-131620-1-git-send-email-andrey.shinkevich@virtuozzo.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2-bitmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -XXX,XX +XXX,XX @@ static int bitmap_list_store(BlockDriverState *bs, Qcow2BitmapList *bm_list,
         dir_offset = *offset;
     }
 
-    dir = g_try_malloc(dir_size);
+    dir = g_try_malloc0(dir_size);
     if (dir == NULL) {
         return -ENOMEM;
     }
-- 
2.21.0

From: John Snow <jsnow@redhat.com>

We mandate that the source node must be a root node; but there's no reason
I am aware of that it needs to be restricted to such. In some cases, we need
to make sure that there's a medium present, but in the general case we can
allow the backup job itself to do the graph checking.

This patch helps improve the error message when you try to backup from
the same node more than once, which is reflected in the change to test
056.

For backups with bitmaps, it will also show a better error message that
the bitmap is in use instead of giving you something cryptic like "need
a root node."

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1707303
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 20190521210053.8864-1-jsnow@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 blockdev.c             | 7 ++++++-
 tests/qemu-iotests/056 | 2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index XXXXXXX..XXXXXXX 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -XXX,XX +XXX,XX @@ static BlockJob *do_drive_backup(DriveBackup *backup, JobTxn *txn,
         backup->compress = false;
     }
 
-    bs = qmp_get_root_bs(backup->device, errp);
+    bs = bdrv_lookup_bs(backup->device, backup->device, errp);
     if (!bs) {
         return NULL;
     }
 
+    if (!bs->drv) {
+        error_setg(errp, "Device has no medium");
+        return NULL;
+    }
+
     aio_context = bdrv_get_aio_context(bs);
     aio_context_acquire(aio_context);
 
diff --git a/tests/qemu-iotests/056 b/tests/qemu-iotests/056
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/056
+++ b/tests/qemu-iotests/056
@@ -XXX,XX +XXX,XX @@ class BackupTest(iotests.QMPTestCase):
         res = self.vm.qmp('query-block-jobs')
         self.assert_qmp(res, 'return[0]/status', 'concluded')
         # Leave zombie job un-dismissed, observe a failure:
-        res = self.qmp_backup_and_wait(serror='Need a root block node',
+        res = self.qmp_backup_and_wait(serror="Node 'drive0' is busy: block device is in use by block job: backup",
                                        device='drive0', format=iotests.imgfmt,
                                        sync='full', target=self.dest_img,
                                        auto_dismiss=False)
-- 
2.21.0

The following changes since commit 6c769690ac845fa62642a5f93b4e4bd906adab95:

Merge remote-tracking branch 'remotes/vsementsov/tags/pull-simplebench-2021-05-04' into staging (2021-05-21 12:02:34 +0100)

are available in the Git repository at:

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

for you to fetch changes up to 0a6f0c76a030710780ce10d6347a70f098024d21:

coroutine-sleep: introduce qemu_co_sleep (2021-05-21 18:22:33 +0100)

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

(Resent due to an email preparation mistake.)

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

Paolo Bonzini (6):
  coroutine-sleep: use a stack-allocated timer
  coroutine-sleep: disallow NULL QemuCoSleepState** argument
  coroutine-sleep: allow qemu_co_sleep_wake that wakes nothing
  coroutine-sleep: move timer out of QemuCoSleepState
  coroutine-sleep: replace QemuCoSleepState pointer with struct in the
    API
  coroutine-sleep: introduce qemu_co_sleep

Philippe Mathieu-Daudé (1):
  bitops.h: Improve find_xxx_bit() documentation

Zenghui Yu (1):
  multi-process: Initialize variables declared with g_auto*

-- 
2.31.1

From: Zenghui Yu <yuzenghui@huawei.com>

Quote docs/devel/style.rst (section "Automatic memory deallocation"):

* Variables declared with g_auto* MUST always be initialized,
  otherwise the cleanup function will use uninitialized stack memory

Initialize @name properly to get rid of the compilation error (using
gcc-7.3.0 on CentOS):

../hw/remote/proxy.c: In function 'pci_proxy_dev_realize':
/usr/include/glib-2.0/glib/glib-autocleanups.h:28:3: error: 'name' may be used uninitialized in this function [-Werror=maybe-uninitialized]
   g_free (*pp);
   ^~~~~~~~~~~~
../hw/remote/proxy.c:350:30: note: 'name' was declared here
             g_autofree char *name;
                              ^~~~

Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
Reviewed-by: Jagannathan Raman <jag.raman@oracle.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Miroslav Rezanina <mrezanin@redhat.com>
Message-id: 20210312112143.1369-1-yuzenghui@huawei.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/remote/memory.c | 5 ++---
 hw/remote/proxy.c  | 3 +--
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/hw/remote/memory.c b/hw/remote/memory.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/remote/memory.c
+++ b/hw/remote/memory.c
@@ -XXX,XX +XXX,XX @@ void remote_sysmem_reconfig(MPQemuMsg *msg, Error **errp)
 
     remote_sysmem_reset();
 
-    for (region = 0; region < msg->num_fds; region++) {
-        g_autofree char *name;
+    for (region = 0; region < msg->num_fds; region++, suffix++) {
+        g_autofree char *name = g_strdup_printf("remote-mem-%u", suffix);
         subregion = g_new(MemoryRegion, 1);
-        name = g_strdup_printf("remote-mem-%u", suffix++);
         memory_region_init_ram_from_fd(subregion, NULL,
                                        name, sysmem_info->sizes[region],
                                        true, msg->fds[region],
diff --git a/hw/remote/proxy.c b/hw/remote/proxy.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/remote/proxy.c
+++ b/hw/remote/proxy.c
@@ -XXX,XX +XXX,XX @@ static void probe_pci_info(PCIDevice *dev, Error **errp)
                    PCI_BASE_ADDRESS_SPACE_IO : PCI_BASE_ADDRESS_SPACE_MEMORY;
 
         if (size) {
-            g_autofree char *name;
+            g_autofree char *name = g_strdup_printf("bar-region-%d", i);
             pdev->region[i].dev = pdev;
             pdev->region[i].present = true;
             if (type == PCI_BASE_ADDRESS_SPACE_MEMORY) {
                 pdev->region[i].memory = true;
             }
-            name = g_strdup_printf("bar-region-%d", i);
             memory_region_init_io(&pdev->region[i].mr, OBJECT(pdev),
                                   &proxy_mr_ops, &pdev->region[i],
                                   name, size);
-- 
2.31.1

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

Document the following functions return the bitmap size
if no matching bit is found:

- find_first_bit
- find_next_bit
- find_last_bit
- find_first_zero_bit
- find_next_zero_bit

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20210510200758.2623154-2-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/qemu/bitops.h | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
index XXXXXXX..XXXXXXX 100644
--- a/include/qemu/bitops.h
+++ b/include/qemu/bitops.h
@@ -XXX,XX +XXX,XX @@ static inline int test_bit(long nr, const unsigned long *addr)
  * @addr: The address to start the search at
  * @size: The maximum size to search
  *
- * Returns the bit number of the first set bit, or size.
+ * Returns the bit number of the last set bit,
+ * or @size if there is no set bit in the bitmap.
  */
 unsigned long find_last_bit(const unsigned long *addr,
                             unsigned long size);
@@ -XXX,XX +XXX,XX @@ unsigned long find_last_bit(const unsigned long *addr,
  * @addr: The address to base the search on
  * @offset: The bitnumber to start searching at
  * @size: The bitmap size in bits
+ *
+ * Returns the bit number of the next set bit,
+ * or @size if there are no further set bits in the bitmap.
  */
 unsigned long find_next_bit(const unsigned long *addr,
                             unsigned long size,
@@ -XXX,XX +XXX,XX @@ unsigned long find_next_bit(const unsigned long *addr,
  * @addr: The address to base the search on
  * @offset: The bitnumber to start searching at
  * @size: The bitmap size in bits
+ *
+ * Returns the bit number of the next cleared bit,
+ * or @size if there are no further clear bits in the bitmap.
  */
 
 unsigned long find_next_zero_bit(const unsigned long *addr,
@@ -XXX,XX +XXX,XX @@ unsigned long find_next_zero_bit(const unsigned long *addr,
  * @addr: The address to start the search at
  * @size: The maximum size to search
  *
- * Returns the bit number of the first set bit.
+ * Returns the bit number of the first set bit,
+ * or @size if there is no set bit in the bitmap.
  */
 static inline unsigned long find_first_bit(const unsigned long *addr,
                                            unsigned long size)
@@ -XXX,XX +XXX,XX @@ static inline unsigned long find_first_bit(const unsigned long *addr,
  * @addr: The address to start the search at
  * @size: The maximum size to search
  *
- * Returns the bit number of the first cleared bit.
+ * Returns the bit number of the first cleared bit,
+ * or @size if there is no clear bit in the bitmap.
  */
 static inline unsigned long find_first_zero_bit(const unsigned long *addr,
                                                 unsigned long size)
-- 
2.31.1

From: Paolo Bonzini <pbonzini@redhat.com>

The lifetime of the timer is well-known (it cannot outlive
qemu_co_sleep_ns_wakeable, because it's deleted by the time the
coroutine resumes), so it is not necessary to place it on the heap.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20210517100548.28806-2-pbonzini@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/qemu-coroutine-sleep.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
index XXXXXXX..XXXXXXX 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -XXX,XX +XXX,XX @@ static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";
 
 struct QemuCoSleepState {
     Coroutine *co;
-    QEMUTimer *ts;
+    QEMUTimer ts;
     QemuCoSleepState **user_state_pointer;
 };
 
@@ -XXX,XX +XXX,XX @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
     if (sleep_state->user_state_pointer) {
         *sleep_state->user_state_pointer = NULL;
     }
-    timer_del(sleep_state->ts);
+    timer_del(&sleep_state->ts);
     aio_co_wake(sleep_state->co);
 }
 
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
     AioContext *ctx = qemu_get_current_aio_context();
     QemuCoSleepState state = {
         .co = qemu_coroutine_self(),
-        .ts = aio_timer_new(ctx, type, SCALE_NS, co_sleep_cb, &state),
         .user_state_pointer = sleep_state,
     };
 
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
         abort();
     }
 
+    aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state);
     if (sleep_state) {
         *sleep_state = &state;
     }
-    timer_mod(state.ts, qemu_clock_get_ns(type) + ns);
+    timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
     qemu_coroutine_yield();
     if (sleep_state) {
         /*
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
          */
         assert(*sleep_state == NULL);
     }
-    timer_free(state.ts);
 }
-- 
2.31.1

From: Paolo Bonzini <pbonzini@redhat.com>

Simplify the code by removing conditionals.  qemu_co_sleep_ns
can simply point the argument to an on-stack temporary.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20210517100548.28806-3-pbonzini@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/qemu/coroutine.h    |  5 +++--
 util/qemu-coroutine-sleep.c | 18 +++++-------------
 2 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index XXXXXXX..XXXXXXX 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -XXX,XX +XXX,XX @@ typedef struct QemuCoSleepState QemuCoSleepState;
 
 /**
  * Yield the coroutine for a given duration. During this yield, @sleep_state
- * (if not NULL) is set to an opaque pointer, which may be used for
+ * is set to an opaque pointer, which may be used for
  * qemu_co_sleep_wake(). Be careful, the pointer is set back to zero when the
  * timer fires. Don't save the obtained value to other variables and don't call
  * qemu_co_sleep_wake from another aio context.
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
                                             QemuCoSleepState **sleep_state);
 static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
 {
-    qemu_co_sleep_ns_wakeable(type, ns, NULL);
+    QemuCoSleepState *unused = NULL;
+    qemu_co_sleep_ns_wakeable(type, ns, &unused);
 }
 
 /**
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
index XXXXXXX..XXXXXXX 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -XXX,XX +XXX,XX @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
                                            qemu_co_sleep_ns__scheduled, NULL);
 
     assert(scheduled == qemu_co_sleep_ns__scheduled);
-    if (sleep_state->user_state_pointer) {
-        *sleep_state->user_state_pointer = NULL;
-    }
+    *sleep_state->user_state_pointer = NULL;
     timer_del(&sleep_state->ts);
     aio_co_wake(sleep_state->co);
 }
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
     }
 
     aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state);
-    if (sleep_state) {
-        *sleep_state = &state;
-    }
+    *sleep_state = &state;
     timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
     qemu_coroutine_yield();
-    if (sleep_state) {
-        /*
-         * Note that *sleep_state is cleared during qemu_co_sleep_wake
-         * before resuming this coroutine.
-         */
-        assert(*sleep_state == NULL);
-    }
+
+    /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine.  */
+    assert(*sleep_state == NULL);
 }
-- 
2.31.1

From: Paolo Bonzini <pbonzini@redhat.com>

All callers of qemu_co_sleep_wake are checking whether they are passing
a NULL argument inside the pointer-to-pointer: do the check in
qemu_co_sleep_wake itself.

As a side effect, qemu_co_sleep_wake can be called more than once and
it will only wake the coroutine once; after the first time, the argument
will be set to NULL via *sleep_state->user_state_pointer.  However, this
would not be safe unless co_sleep_cb keeps using the QemuCoSleepState*
directly, so make it go through the pointer-to-pointer instead.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20210517100548.28806-4-pbonzini@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/block-copy.c          |  4 +---
 block/nbd.c                 |  8 ++------
 util/qemu-coroutine-sleep.c | 21 ++++++++++++---------
 3 files changed, 15 insertions(+), 18 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 @@ out:
 
 void block_copy_kick(BlockCopyCallState *call_state)
 {
-    if (call_state->sleep_state) {
-        qemu_co_sleep_wake(call_state->sleep_state);
-    }
+    qemu_co_sleep_wake(call_state->sleep_state);
 }
 
 /*
diff --git a/block/nbd.c b/block/nbd.c
index XXXXXXX..XXXXXXX 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn nbd_client_co_drain_begin(BlockDriverState *bs)
     BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
 
     s->drained = true;
-    if (s->connection_co_sleep_ns_state) {
-        qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
-    }
+    qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
 
     nbd_co_establish_connection_cancel(bs, false);
 
@@ -XXX,XX +XXX,XX @@ static void nbd_teardown_connection(BlockDriverState *bs)
 
     s->state = NBD_CLIENT_QUIT;
     if (s->connection_co) {
-        if (s->connection_co_sleep_ns_state) {
-            qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
-        }
+        qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
         nbd_co_establish_connection_cancel(bs, true);
     }
     if (qemu_in_coroutine()) {
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
index XXXXXXX..XXXXXXX 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -XXX,XX +XXX,XX @@ struct QemuCoSleepState {
 
 void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
 {
-    /* Write of schedule protected by barrier write in aio_co_schedule */
-    const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled,
-                                           qemu_co_sleep_ns__scheduled, NULL);
+    if (sleep_state) {
+        /* Write of schedule protected by barrier write in aio_co_schedule */
+        const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled,
+                                                qemu_co_sleep_ns__scheduled, NULL);
 
-    assert(scheduled == qemu_co_sleep_ns__scheduled);
-    *sleep_state->user_state_pointer = NULL;
-    timer_del(&sleep_state->ts);
-    aio_co_wake(sleep_state->co);
+        assert(scheduled == qemu_co_sleep_ns__scheduled);
+        *sleep_state->user_state_pointer = NULL;
+        timer_del(&sleep_state->ts);
+        aio_co_wake(sleep_state->co);
+    }
 }
 
 static void co_sleep_cb(void *opaque)
 {
-    qemu_co_sleep_wake(opaque);
+    QemuCoSleepState **sleep_state = opaque;
+    qemu_co_sleep_wake(*sleep_state);
 }
 
 void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
         abort();
     }
 
-    aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state);
+    aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, sleep_state);
     *sleep_state = &state;
     timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
     qemu_coroutine_yield();
-- 
2.31.1

From: Paolo Bonzini <pbonzini@redhat.com>

This simplification is enabled by the previous patch.  Now aio_co_wake
will only be called once, therefore we do not care about a spurious
firing of the timer after a qemu_co_sleep_wake.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20210517100548.28806-5-pbonzini@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/qemu-coroutine-sleep.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
index XXXXXXX..XXXXXXX 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -XXX,XX +XXX,XX @@ static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";
 
 struct QemuCoSleepState {
     Coroutine *co;
-    QEMUTimer ts;
     QemuCoSleepState **user_state_pointer;
 };
 
@@ -XXX,XX +XXX,XX @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
 
         assert(scheduled == qemu_co_sleep_ns__scheduled);
         *sleep_state->user_state_pointer = NULL;
-        timer_del(&sleep_state->ts);
         aio_co_wake(sleep_state->co);
     }
 }
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
                                             QemuCoSleepState **sleep_state)
 {
     AioContext *ctx = qemu_get_current_aio_context();
+    QEMUTimer ts;
     QemuCoSleepState state = {
         .co = qemu_coroutine_self(),
         .user_state_pointer = sleep_state,
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
         abort();
     }
 
-    aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, sleep_state);
+    aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, sleep_state);
     *sleep_state = &state;
-    timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
+    timer_mod(&ts, qemu_clock_get_ns(type) + ns);
     qemu_coroutine_yield();
+    timer_del(&ts);
 
     /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine.  */
     assert(*sleep_state == NULL);
-- 
2.31.1

From: Paolo Bonzini <pbonzini@redhat.com>

Right now, users of qemu_co_sleep_ns_wakeable are simply passing
a pointer to QemuCoSleepState by reference to the function.  But
QemuCoSleepState really is just a Coroutine*; making the
content of the struct public is just as efficient and lets us
skip the user_state_pointer indirection.

Since the usage is changed, take the occasion to rename the
struct to QemuCoSleep.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20210517100548.28806-6-pbonzini@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/qemu/coroutine.h    | 23 +++++++++++----------
 block/block-copy.c          |  8 ++++----
 block/nbd.c                 | 10 ++++-----
 util/qemu-coroutine-sleep.c | 41 ++++++++++++++++---------------------
 4 files changed, 39 insertions(+), 43 deletions(-)

diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index XXXXXXX..XXXXXXX 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -XXX,XX +XXX,XX @@ void qemu_co_rwlock_wrlock(CoRwlock *lock);
  */
 void qemu_co_rwlock_unlock(CoRwlock *lock);
 
-typedef struct QemuCoSleepState QemuCoSleepState;
+typedef struct QemuCoSleep {
+    Coroutine *to_wake;
+} QemuCoSleep;
 
 /**
- * Yield the coroutine for a given duration. During this yield, @sleep_state
- * is set to an opaque pointer, which may be used for
- * qemu_co_sleep_wake(). Be careful, the pointer is set back to zero when the
- * timer fires. Don't save the obtained value to other variables and don't call
- * qemu_co_sleep_wake from another aio context.
+ * Yield the coroutine for a given duration. Initializes @w so that,
+ * during this yield, it can be passed to qemu_co_sleep_wake() to
+ * terminate the sleep.
  */
-void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
-                                            QemuCoSleepState **sleep_state);
+void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
+                                            QEMUClockType type, int64_t ns);
+
 static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
 {
-    QemuCoSleepState *unused = NULL;
-    qemu_co_sleep_ns_wakeable(type, ns, &unused);
+    QemuCoSleep w = { 0 };
+    qemu_co_sleep_ns_wakeable(&w, type, ns);
 }
 
 /**
@@ -XXX,XX +XXX,XX @@ static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
  * qemu_co_sleep_ns() and should be checked to be non-NULL before calling
  * qemu_co_sleep_wake().
  */
-void qemu_co_sleep_wake(QemuCoSleepState *sleep_state);
+void qemu_co_sleep_wake(QemuCoSleep *w);
 
 /**
  * Yield until a file descriptor becomes readable
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 @@ typedef struct BlockCopyCallState {
     /* State */
     int ret;
     bool finished;
-    QemuCoSleepState *sleep_state;
+    QemuCoSleep sleep;
     bool cancelled;
 
     /* OUT parameters */
@@ -XXX,XX +XXX,XX @@ block_copy_dirty_clusters(BlockCopyCallState *call_state)
                 if (ns > 0) {
                     block_copy_task_end(task, -EAGAIN);
                     g_free(task);
-                    qemu_co_sleep_ns_wakeable(QEMU_CLOCK_REALTIME, ns,
-                                              &call_state->sleep_state);
+                    qemu_co_sleep_ns_wakeable(&call_state->sleep,
+                                              QEMU_CLOCK_REALTIME, ns);
                     continue;
                 }
             }
@@ -XXX,XX +XXX,XX @@ out:
 
 void block_copy_kick(BlockCopyCallState *call_state)
 {
-    qemu_co_sleep_wake(call_state->sleep_state);
+    qemu_co_sleep_wake(&call_state->sleep);
 }
 
 /*
diff --git a/block/nbd.c b/block/nbd.c
index XXXXXXX..XXXXXXX 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -XXX,XX +XXX,XX @@ typedef struct BDRVNBDState {
     CoQueue free_sema;
     Coroutine *connection_co;
     Coroutine *teardown_co;
-    QemuCoSleepState *connection_co_sleep_ns_state;
+    QemuCoSleep reconnect_sleep;
     bool drained;
     bool wait_drained_end;
     int in_flight;
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn nbd_client_co_drain_begin(BlockDriverState *bs)
     BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
 
     s->drained = true;
-    qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
+    qemu_co_sleep_wake(&s->reconnect_sleep);
 
     nbd_co_establish_connection_cancel(bs, false);
 
@@ -XXX,XX +XXX,XX @@ static void nbd_teardown_connection(BlockDriverState *bs)
 
     s->state = NBD_CLIENT_QUIT;
     if (s->connection_co) {
-        qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
+        qemu_co_sleep_wake(&s->reconnect_sleep);
         nbd_co_establish_connection_cancel(bs, true);
     }
     if (qemu_in_coroutine()) {
@@ -XXX,XX +XXX,XX @@ static coroutine_fn void nbd_co_reconnect_loop(BDRVNBDState *s)
             }
             bdrv_inc_in_flight(s->bs);
         } else {
-            qemu_co_sleep_ns_wakeable(QEMU_CLOCK_REALTIME, timeout,
-                                      &s->connection_co_sleep_ns_state);
+            qemu_co_sleep_ns_wakeable(&s->reconnect_sleep,
+                                      QEMU_CLOCK_REALTIME, timeout);
             if (s->drained) {
                 continue;
             }
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
index XXXXXXX..XXXXXXX 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -XXX,XX +XXX,XX @@
 
 static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";
 
-struct QemuCoSleepState {
+void qemu_co_sleep_wake(QemuCoSleep *w)
+{
     Coroutine *co;
-    QemuCoSleepState **user_state_pointer;
-};
 
-void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
-{
-    if (sleep_state) {
+    co = w->to_wake;
+    w->to_wake = NULL;
+    if (co) {
         /* Write of schedule protected by barrier write in aio_co_schedule */
-        const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled,
+        const char *scheduled = qatomic_cmpxchg(&co->scheduled,
                                                 qemu_co_sleep_ns__scheduled, NULL);
 
         assert(scheduled == qemu_co_sleep_ns__scheduled);
-        *sleep_state->user_state_pointer = NULL;
-        aio_co_wake(sleep_state->co);
+        aio_co_wake(co);
     }
 }
 
 static void co_sleep_cb(void *opaque)
 {
-    QemuCoSleepState **sleep_state = opaque;
-    qemu_co_sleep_wake(*sleep_state);
+    QemuCoSleep *w = opaque;
+    qemu_co_sleep_wake(w);
 }
 
-void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
-                                            QemuCoSleepState **sleep_state)
+void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
+                                            QEMUClockType type, int64_t ns)
 {
+    Coroutine *co = qemu_coroutine_self();
     AioContext *ctx = qemu_get_current_aio_context();
     QEMUTimer ts;
-    QemuCoSleepState state = {
-        .co = qemu_coroutine_self(),
-        .user_state_pointer = sleep_state,
-    };
 
-    const char *scheduled = qatomic_cmpxchg(&state.co->scheduled, NULL,
-                                           qemu_co_sleep_ns__scheduled);
+    const char *scheduled = qatomic_cmpxchg(&co->scheduled, NULL,
+                                            qemu_co_sleep_ns__scheduled);
     if (scheduled) {
         fprintf(stderr,
                 "%s: Co-routine was already scheduled in '%s'\n",
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
         abort();
     }
 
-    aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, sleep_state);
-    *sleep_state = &state;
+    w->to_wake = co;
+    aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w),
     timer_mod(&ts, qemu_clock_get_ns(type) + ns);
     qemu_coroutine_yield();
     timer_del(&ts);
 
-    /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine.  */
-    assert(*sleep_state == NULL);
+    /* w->to_wake is cleared before resuming this coroutine.  */
+    assert(w->to_wake == NULL);
 }
-- 
2.31.1

From: Paolo Bonzini <pbonzini@redhat.com>

Allow using QemuCoSleep to sleep forever until woken by qemu_co_sleep_wake.
This makes the logic of qemu_co_sleep_ns_wakeable easy to understand.

In the future we will introduce an API that can work even if the
sleep and wake happen from different threads.  For now, initializing
w->to_wake after timer_mod is fine because the timer can only fire in
the same AioContext.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20210517100548.28806-7-pbonzini@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/qemu/coroutine.h    |  5 +++++
 util/qemu-coroutine-sleep.c | 26 +++++++++++++++++++-------
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index XXXXXXX..XXXXXXX 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -XXX,XX +XXX,XX @@ typedef struct QemuCoSleep {
 void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
                                             QEMUClockType type, int64_t ns);
 
+/**
+ * Yield the coroutine until the next call to qemu_co_sleep_wake.
+ */
+void coroutine_fn qemu_co_sleep(QemuCoSleep *w);
+
 static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
 {
     QemuCoSleep w = { 0 };
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
index XXXXXXX..XXXXXXX 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -XXX,XX +XXX,XX @@ static void co_sleep_cb(void *opaque)
     qemu_co_sleep_wake(w);
 }
 
-void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
-                                            QEMUClockType type, int64_t ns)
+void coroutine_fn qemu_co_sleep(QemuCoSleep *w)
 {
     Coroutine *co = qemu_coroutine_self();
-    AioContext *ctx = qemu_get_current_aio_context();
-    QEMUTimer ts;
 
     const char *scheduled = qatomic_cmpxchg(&co->scheduled, NULL,
                                             qemu_co_sleep_ns__scheduled);
@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
     }
 
     w->to_wake = co;
-    aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w),
-    timer_mod(&ts, qemu_clock_get_ns(type) + ns);
     qemu_coroutine_yield();
-    timer_del(&ts);
 
     /* w->to_wake is cleared before resuming this coroutine.  */
     assert(w->to_wake == NULL);
 }
+
+void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
+                                            QEMUClockType type, int64_t ns)
+{
+    AioContext *ctx = qemu_get_current_aio_context();
+    QEMUTimer ts;
+
+    aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w);
+    timer_mod(&ts, qemu_clock_get_ns(type) + ns);
+
+    /*
+     * The timer will fire in the current AiOContext, so the callback
+     * must happen after qemu_co_sleep yields and there is no race
+     * between timer_mod and qemu_co_sleep.
+     */
+    qemu_co_sleep(w);
+    timer_del(&ts);
+}
-- 
2.31.1