Series comparison

-[PULL 0/8] Block patches
+[PULL 0/3] Block patches
-The following changes since commit 1be5a765c08cee3a9587c8a8d3fc2ea247b13f9c:
+The following changes since commit 9cf289af47bcfae5c75de37d8e5d6fd23705322c:
-  Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging (2022-04-19 18:22:16 -0700)
+  Merge tag 'qga-pull-request' of gitlab.com:marcandre.lureau/qemu into staging (2022-05-04 03:42:49 -0700)
 are available in the Git repository at:
-  https://gitlab.com/hreitz/qemu.git tags/pull-block-2022-04-20
+  https://gitlab.com/stefanha/qemu.git tags/block-pull-request
-for you to fetch changes up to 0423f75351ab83b844a31349218b0eadd830e07a:
+for you to fetch changes up to bef2e050d6a7feb865854c65570c496ac5a8cf53:
-  qcow2: Add errp to rebuild_refcount_structure() (2022-04-20 12:09:17 +0200)
+  util/event-loop-base: Introduce options to set the thread pool size (2022-05-04 17:02:19 +0100)
 ----------------------------------------------------------------
-Block patches:
+Pull request
-- Some changes for qcow2's refcount repair algorithm to make it work for
-  qcow2 images stored on block devices
+Add new thread-pool-min/thread-pool-max parameters to control the thread pool
-- Skip test cases that require zstd when support for it is missing
+used for async I/O.
 - Some refactoring in the iotests' meson.build
 ----------------------------------------------------------------
-Hanna Reitz (6):
-  iotests.py: Add supports_qcow2_zstd_compression()
-  iotests/065: Check for zstd support
-  iotests/303: Check for zstd support
-  qcow2: Improve refcount structure rebuilding
-  iotests/108: Test new refcount rebuild algorithm
-  qcow2: Add errp to rebuild_refcount_structure()
-Thomas Huth (2):
+Nicolas Saenz Julienne (3):
-  tests/qemu-iotests/meson.build: Improve the indentation
+  Introduce event-loop-base abstract class
-  tests/qemu-iotests: Move the bash and sanitizer checks to meson.build
+  util/main-loop: Introduce the main loop into QOM
   util/event-loop-base: Introduce options to set the thread pool size
- block/qcow2-refcount.c         | 353 +++++++++++++++++++++++----------
+ qapi/qom.json                    |  43 ++++++++--
- tests/check-block.sh           |  26 ---
+ meson.build                      |  26 +++---
- tests/qemu-iotests/065         |  24 ++-
+ include/block/aio.h              |  10 +++
- tests/qemu-iotests/108         | 259 +++++++++++++++++++++++-
+ include/block/thread-pool.h      |   3 +
- tests/qemu-iotests/108.out     |  81 ++++++++
+ include/qemu/main-loop.h         |  10 +++
- tests/qemu-iotests/303         |   4 +-
+ include/sysemu/event-loop-base.h |  41 +++++++++
- tests/qemu-iotests/iotests.py  |  20 ++
+ include/sysemu/iothread.h        |   6 +-
- tests/qemu-iotests/meson.build |  73 ++++---
+ event-loop-base.c                | 140 +++++++++++++++++++++++++++++++
-files changed, 673 insertions(+), 167 deletions(-)
+ iothread.c                       |  68 +++++----------
  util/aio-posix.c                 |   1 +
  util/async.c                     |  20 +++++
  util/main-loop.c                 |  65 ++++++++++++++
  util/thread-pool.c               |  55 +++++++++++-
 files changed, 419 insertions(+), 69 deletions(-)
  create mode 100644 include/sysemu/event-loop-base.h
  create mode 100644 event-loop-base.c
 --
 .35.1

-[PULL 1/8] tests/qemu-iotests/meson.build: Improve the indentation
+Deleted patch
-From: Thomas Huth <thuth@redhat.com>
-By using subdir_done(), we can get rid of one level of indentation
-in this file. This will make it easier to add more conditions to
-skip the iotests in future patches.
-Reviewed-by: Hanna Reitz <hreitz@redhat.com>
-Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
-Signed-off-by: Thomas Huth <thuth@redhat.com>
-Message-Id: <20220223093840.2515281-3-thuth@redhat.com>
-Signed-off-by: Hanna Reitz <hreitz@redhat.com>
----
- tests/qemu-iotests/meson.build | 61 ++++++++++++++++++----------------
-file changed, 32 insertions(+), 29 deletions(-)
-diff --git a/tests/qemu-iotests/meson.build b/tests/qemu-iotests/meson.build
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/meson.build
-+++ b/tests/qemu-iotests/meson.build
-@@ -XXX,XX +XXX,XX @@
--if have_tools and targetos != 'windows' and not get_option('gprof')
--  qemu_iotests_binaries = [qemu_img, qemu_io, qemu_nbd, qsd]
--  qemu_iotests_env = {'PYTHON': python.full_path()}
--  qemu_iotests_formats = {
--    'qcow2': 'quick',
--    'raw': 'slow',
--    'qed': 'thorough',
--    'vmdk': 'thorough',
--    'vpc': 'thorough'
--  }
--
--  foreach k, v : emulators
--    if k.startswith('qemu-system-')
--      qemu_iotests_binaries += v
--    endif
--  endforeach
--  foreach format, speed: qemu_iotests_formats
--    if speed == 'quick'
--      suites = 'block'
--    else
--      suites = ['block-' + speed, speed]
--    endif
--    test('qemu-iotests ' + format, sh, args: [files('../check-block.sh'), format],
--         depends: qemu_iotests_binaries, env: qemu_iotests_env,
--         protocol: 'tap',
--         suite: suites,
--         timeout: 0,
--         is_parallel: false)
--  endforeach
-+if not have_tools or targetos == 'windows' or get_option('gprof')
-+  subdir_done()
- endif
-+
-+qemu_iotests_binaries = [qemu_img, qemu_io, qemu_nbd, qsd]
-+qemu_iotests_env = {'PYTHON': python.full_path()}
-+qemu_iotests_formats = {
-+  'qcow2': 'quick',
-+  'raw': 'slow',
-+  'qed': 'thorough',
-+  'vmdk': 'thorough',
-+  'vpc': 'thorough'
-+}
-+
-+foreach k, v : emulators
-+  if k.startswith('qemu-system-')
-+    qemu_iotests_binaries += v
-+  endif
-+endforeach
-+
-+foreach format, speed: qemu_iotests_formats
-+  if speed == 'quick'
-+    suites = 'block'
-+  else
-+    suites = ['block-' + speed, speed]
-+  endif
-+  test('qemu-iotests ' + format, sh, args: [files('../check-block.sh'), format],
-+       depends: qemu_iotests_binaries, env: qemu_iotests_env,
-+       protocol: 'tap',
-+       suite: suites,
-+       timeout: 0,
-+       is_parallel: false)
-+endforeach
---
-.35.1

-[PULL 6/8] qcow2: Improve refcount structure rebuilding
+[PULL 1/3] Introduce event-loop-base abstract class
-When rebuilding the refcount structures (when qemu-img check -r found
+From: Nicolas Saenz Julienne <nsaenzju@redhat.com>
-errors with refcount = 0, but reference count > 0), the new refcount
-table defaults to being put at the image file end[1].  There is no good
+Introduce the 'event-loop-base' abstract class, it'll hold the
-reason for that except that it means we will not have to rewrite any
+properties common to all event loops and provide the necessary hooks for
-refblocks we already wrote to disk.
+their creation and maintenance. Then have iothread inherit from it.
-Changing the code to rewrite those refblocks is not too difficult,
+EventLoopBaseClass is defined as user creatable and provides a hook for
-though, so let us do that.  That is beneficial for images on block
+its children to attach themselves to the user creatable class 'complete'
-devices, where we cannot really write beyond the end of the image file.
+function. It also provides an update_params() callback to propagate
+property changes onto its children.
-Use this opportunity to add extensive comments to the code, and refactor
-it a bit, getting rid of the backwards-jumping goto.
+The new 'event-loop-base' class will live in the root directory. It is
+built on its own using the 'link_whole' option (there are no direct
-[1] Unless there is something allocated in the area pointed to by the
+function dependencies between the class and its children, it all happens
-    last refblock, so we have to write that refblock.  In that case, we
+trough 'constructor' magic). And also imposes new compilation
-    try to put the reftable in there.
+dependencies:
-Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1519071
+    qom <- event-loop-base <- blockdev (iothread.c)
-Closes: https://gitlab.com/qemu-project/qemu/-/issues/941
-Reviewed-by: Eric Blake <eblake@redhat.com>
+And in subsequent patches:
-Signed-off-by: Hanna Reitz <hreitz@redhat.com>
-Message-Id: <20220405134652.19278-2-hreitz@redhat.com>
+    qom <- event-loop-base <- qemuutil (util/main-loop.c)
 All this forced some amount of reordering in meson.build:
  - Moved qom build definition before qemuutil. Doing it the other way
    around (i.e. moving qemuutil after qom) isn't possible as a lot of
    core libraries that live in between the two depend on it.
  - Process the 'hw' subdir earlier, as it introduces files into the
    'qom' source set.
 No functional changes intended.
 Signed-off-by: Nicolas Saenz Julienne <nsaenzju@redhat.com>
 Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
 Acked-by: Markus Armbruster <armbru@redhat.com>
 Message-id: 20220425075723.20019-2-nsaenzju@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/qcow2-refcount.c | 332 +++++++++++++++++++++++++++++------------
+ qapi/qom.json                    |  22 +++++--
-file changed, 235 insertions(+), 97 deletions(-)
+ meson.build                      |  23 ++++---
+ include/sysemu/event-loop-base.h |  36 +++++++++++
-diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
+ include/sysemu/iothread.h        |   6 +-
  event-loop-base.c                | 104 +++++++++++++++++++++++++++++++
  iothread.c                       |  65 ++++++-------------
 files changed, 192 insertions(+), 64 deletions(-)
  create mode 100644 include/sysemu/event-loop-base.h
  create mode 100644 event-loop-base.c
 diff --git a/qapi/qom.json b/qapi/qom.json
 index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2-refcount.c
+--- a/qapi/qom.json
-+++ b/block/qcow2-refcount.c
++++ b/qapi/qom.json
-@@ -XXX,XX +XXX,XX @@ static int64_t alloc_clusters_imrt(BlockDriverState *bs,
+@@ -XXX,XX +XXX,XX @@
              '*repeat': 'bool',
              '*grab-toggle': 'GrabToggleKeys' } }
 +##
 +# @EventLoopBaseProperties:
 +#
 +# Common properties for event loops
 +#
 +# @aio-max-batch: maximum number of requests in a batch for the AIO engine,
 +#                 0 means that the engine will use its default.
 +#                 (default: 0)
 +#
 +# Since: 7.1
 +##
 +{ 'struct': 'EventLoopBaseProperties',
 +  'data': { '*aio-max-batch': 'int' } }
 +
  ##
  # @IothreadProperties:
  #
@@ -XXX,XX +XXX,XX @@
  #               algorithm detects it is spending too long polling without
  #               encountering events. 0 selects a default behaviour (default: 0)
  #
 -# @aio-max-batch: maximum number of requests in a batch for the AIO engine,
 -#                 0 means that the engine will use its default
 -#                 (default:0, since 6.1)
 +# The @aio-max-batch option is available since 6.1.
  #
  # Since: 2.0
  ##
  { 'struct': 'IothreadProperties',
 +  'base': 'EventLoopBaseProperties',
    'data': { '*poll-max-ns': 'int',
              '*poll-grow': 'int',
 -            '*poll-shrink': 'int',
 -            '*aio-max-batch': 'int' } }
 +            '*poll-shrink': 'int' } }
  ##
  # @MemoryBackendProperties:
 diff --git a/meson.build b/meson.build
 index XXXXXXX..XXXXXXX 100644
 --- a/meson.build
 +++ b/meson.build
@@ -XXX,XX +XXX,XX @@ subdir('qom')
  subdir('authz')
  subdir('crypto')
  subdir('ui')
 +subdir('hw')
  if enable_modules
@@ -XXX,XX +XXX,XX @@ if enable_modules
    modulecommon = declare_dependency(link_whole: libmodulecommon, compile_args: '-DBUILD_DSO')
  endif
 +qom_ss = qom_ss.apply(config_host, strict: false)
 +libqom = static_library('qom', qom_ss.sources() + genh,
 +                        dependencies: [qom_ss.dependencies()],
 +                        name_suffix: 'fa')
 +qom = declare_dependency(link_whole: libqom)
 +
 +event_loop_base = files('event-loop-base.c')
 +event_loop_base = static_library('event-loop-base', sources: event_loop_base + genh,
 +                                 build_by_default: true)
 +event_loop_base = declare_dependency(link_whole: event_loop_base,
 +                                     dependencies: [qom])
 +
  stub_ss = stub_ss.apply(config_all, strict: false)
  util_ss.add_all(trace_ss)
@@ -XXX,XX +XXX,XX @@ subdir('monitor')
  subdir('net')
  subdir('replay')
  subdir('semihosting')
 -subdir('hw')
  subdir('tcg')
  subdir('fpu')
  subdir('accel')
@@ -XXX,XX +XXX,XX @@ qemu_syms = custom_target('qemu.syms', output: 'qemu.syms',
                               capture: true,
                               command: [undefsym, nm, '@INPUT@'])
 -qom_ss = qom_ss.apply(config_host, strict: false)
 -libqom = static_library('qom', qom_ss.sources() + genh,
 -                        dependencies: [qom_ss.dependencies()],
 -                        name_suffix: 'fa')
 -
 -qom = declare_dependency(link_whole: libqom)
 -
  authz_ss = authz_ss.apply(config_host, strict: false)
  libauthz = static_library('authz', authz_ss.sources() + genh,
                            dependencies: [authz_ss.dependencies()],
@@ -XXX,XX +XXX,XX @@ libblockdev = static_library('blockdev', blockdev_ss.sources() + genh,
                               build_by_default: false)
  blockdev = declare_dependency(link_whole: [libblockdev],
 -                              dependencies: [block])
 +                              dependencies: [block, event_loop_base])
  qmp_ss = qmp_ss.apply(config_host, strict: false)
  libqmp = static_library('qmp', qmp_ss.sources() + genh,
 diff --git a/include/sysemu/event-loop-base.h b/include/sysemu/event-loop-base.h
 new file mode 100644
 index XXXXXXX..XXXXXXX
 --- /dev/null
 +++ b/include/sysemu/event-loop-base.h
@@ -XXX,XX +XXX,XX @@
 +/*
 + * QEMU event-loop backend
 + *
 + * Copyright (C) 2022 Red Hat Inc
 + *
 + * Authors:
 + *  Nicolas Saenz Julienne <nsaenzju@redhat.com>
 + *
 + * This work is licensed under the terms of the GNU GPL, version 2 or later.
 + * See the COPYING file in the top-level directory.
 + */
 +#ifndef QEMU_EVENT_LOOP_BASE_H
 +#define QEMU_EVENT_LOOP_BASE_H
 +
 +#include "qom/object.h"
 +#include "block/aio.h"
 +#include "qemu/typedefs.h"
 +
 +#define TYPE_EVENT_LOOP_BASE         "event-loop-base"
 +OBJECT_DECLARE_TYPE(EventLoopBase, EventLoopBaseClass,
 +                    EVENT_LOOP_BASE)
 +
 +struct EventLoopBaseClass {
 +    ObjectClass parent_class;
 +
 +    void (*init)(EventLoopBase *base, Error **errp);
 +    void (*update_params)(EventLoopBase *base, Error **errp);
 +};
 +
 +struct EventLoopBase {
 +    Object parent;
 +
 +    /* AioContext AIO engine parameters */
 +    int64_t aio_max_batch;
 +};
 +#endif
 diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
 index XXXXXXX..XXXXXXX 100644
 --- a/include/sysemu/iothread.h
 +++ b/include/sysemu/iothread.h
@@ -XXX,XX +XXX,XX @@
  #include "block/aio.h"
  #include "qemu/thread.h"
  #include "qom/object.h"
 +#include "sysemu/event-loop-base.h"
  #define TYPE_IOTHREAD "iothread"
  struct IOThread {
 -    Object parent_obj;
 +    EventLoopBase parent_obj;
      QemuThread thread;
      AioContext *ctx;
@@ -XXX,XX +XXX,XX @@ struct IOThread {
      int64_t poll_max_ns;
      int64_t poll_grow;
      int64_t poll_shrink;
 -
 -    /* AioContext AIO engine parameters */
 -    int64_t aio_max_batch;
  };
  typedef struct IOThread IOThread;
 diff --git a/event-loop-base.c b/event-loop-base.c
 new file mode 100644
 index XXXXXXX..XXXXXXX
 --- /dev/null
 +++ b/event-loop-base.c
@@ -XXX,XX +XXX,XX @@
 +/*
 + * QEMU event-loop base
 + *
 + * Copyright (C) 2022 Red Hat Inc
 + *
 + * Authors:
 + *  Stefan Hajnoczi <stefanha@redhat.com>
 + *  Nicolas Saenz Julienne <nsaenzju@redhat.com>
 + *
 + * This work is licensed under the terms of the GNU GPL, version 2 or later.
 + * See the COPYING file in the top-level directory.
 + */
 +
 +#include "qemu/osdep.h"
 +#include "qom/object_interfaces.h"
 +#include "qapi/error.h"
 +#include "sysemu/event-loop-base.h"
 +
 +typedef struct {
 +    const char *name;
 +    ptrdiff_t offset; /* field's byte offset in EventLoopBase struct */
 +} EventLoopBaseParamInfo;
 +
 +static EventLoopBaseParamInfo aio_max_batch_info = {
 +    "aio-max-batch", offsetof(EventLoopBase, aio_max_batch),
 +};
 +
 +static void event_loop_base_get_param(Object *obj, Visitor *v,
 +        const char *name, void *opaque, Error **errp)
 +{
 +    EventLoopBase *event_loop_base = EVENT_LOOP_BASE(obj);
 +    EventLoopBaseParamInfo *info = opaque;
 +    int64_t *field = (void *)event_loop_base + info->offset;
 +
 +    visit_type_int64(v, name, field, errp);
 +}
 +
 +static void event_loop_base_set_param(Object *obj, Visitor *v,
 +        const char *name, void *opaque, Error **errp)
 +{
 +    EventLoopBaseClass *bc = EVENT_LOOP_BASE_GET_CLASS(obj);
 +    EventLoopBase *base = EVENT_LOOP_BASE(obj);
 +    EventLoopBaseParamInfo *info = opaque;
 +    int64_t *field = (void *)base + info->offset;
 +    int64_t value;
 +
 +    if (!visit_type_int64(v, name, &value, errp)) {
 +        return;
 +    }
 +
 +    if (value < 0) {
 +        error_setg(errp, "%s value must be in range [0, %" PRId64 "]",
 +                   info->name, INT64_MAX);
 +        return;
 +    }
 +
 +    *field = value;
 +
 +    if (bc->update_params) {
 +        bc->update_params(base, errp);
 +    }
 +
 +    return;
 +}
 +
 +static void event_loop_base_complete(UserCreatable *uc, Error **errp)
 +{
 +    EventLoopBaseClass *bc = EVENT_LOOP_BASE_GET_CLASS(uc);
 +    EventLoopBase *base = EVENT_LOOP_BASE(uc);
 +
 +    if (bc->init) {
 +        bc->init(base, errp);
 +    }
 +}
 +
 +static void event_loop_base_class_init(ObjectClass *klass, void *class_data)
 +{
 +    UserCreatableClass *ucc = USER_CREATABLE_CLASS(klass);
 +    ucc->complete = event_loop_base_complete;
 +
 +    object_class_property_add(klass, "aio-max-batch", "int",
 +                              event_loop_base_get_param,
 +                              event_loop_base_set_param,
 +                              NULL, &aio_max_batch_info);
 +}
 +
 +static const TypeInfo event_loop_base_info = {
 +    .name = TYPE_EVENT_LOOP_BASE,
 +    .parent = TYPE_OBJECT,
 +    .instance_size = sizeof(EventLoopBase),
 +    .class_size = sizeof(EventLoopBaseClass),
 +    .class_init = event_loop_base_class_init,
 +    .abstract = true,
 +    .interfaces = (InterfaceInfo[]) {
 +        { TYPE_USER_CREATABLE },
 +        { }
 +    }
 +};
 +
 +static void register_types(void)
 +{
 +    type_register_static(&event_loop_base_info);
 +}
 +type_init(register_types);
 diff --git a/iothread.c b/iothread.c
 index XXXXXXX..XXXXXXX 100644
 --- a/iothread.c
 +++ b/iothread.c
@@ -XXX,XX +XXX,XX @@
  #include "qemu/module.h"
  #include "block/aio.h"
  #include "block/block.h"
 +#include "sysemu/event-loop-base.h"
  #include "sysemu/iothread.h"
  #include "qapi/error.h"
  #include "qapi/qapi-commands-misc.h"
@@ -XXX,XX +XXX,XX @@ static void iothread_init_gcontext(IOThread *iothread)
      iothread->main_loop = g_main_loop_new(iothread->worker_context, TRUE);
  }
- /*
+-static void iothread_set_aio_context_params(IOThread *iothread, Error **errp)
-- * Creates a new refcount structure based solely on the in-memory information
++static void iothread_set_aio_context_params(EventLoopBase *base, Error **errp)
 - * given through *refcount_table. All necessary allocations will be reflected
 - * in that array.
 + * Helper function for rebuild_refcount_structure().
   *
 - * On success, the old refcount structure is leaked (it will be covered by the
 - * new refcount structure).
 + * Scan the range of clusters [first_cluster, end_cluster) for allocated
 + * clusters and write all corresponding refblocks to disk.  The refblock
 + * and allocation data is taken from the in-memory refcount table
 + * *refcount_table[] (of size *nb_clusters), which is basically one big
 + * (unlimited size) refblock for the whole image.
 + *
 + * For these refblocks, clusters are allocated using said in-memory
 + * refcount table.  Care is taken that these allocations are reflected
 + * in the refblocks written to disk.
 + *
 + * The refblocks' offsets are written into a reftable, which is
 + * *on_disk_reftable_ptr[] (of size *on_disk_reftable_entries_ptr).  If
 + * that reftable is of insufficient size, it will be resized to fit.
 + * This reftable is not written to disk.
 + *
 + * (If *on_disk_reftable_ptr is not NULL, the entries within are assumed
 + * to point to existing valid refblocks that do not need to be allocated
 + * again.)
 + *
 + * Return whether the on-disk reftable array was resized (true/false),
 + * or -errno on error.
   */
 -static int rebuild_refcount_structure(BlockDriverState *bs,
 -                                      BdrvCheckResult *res,
 -                                      void **refcount_table,
 -                                      int64_t *nb_clusters)
 +static int rebuild_refcounts_write_refblocks(
 +        BlockDriverState *bs, void **refcount_table, int64_t *nb_clusters,
 +        int64_t first_cluster, int64_t end_cluster,
 +        uint64_t **on_disk_reftable_ptr, uint32_t *on_disk_reftable_entries_ptr
 +    )
  {
-     BDRVQcow2State *s = bs->opaque;
++    IOThread *iothread = IOTHREAD(base);
--    int64_t first_free_cluster = 0, reftable_offset = -1, cluster = 0;
+     ERRP_GUARD();
-+    int64_t cluster;
-     int64_t refblock_offset, refblock_start, refblock_index;
++    if (!iothread->ctx) {
--    uint32_t reftable_size = 0;
++        return;
--    uint64_t *on_disk_reftable = NULL;
++    }
-+    int64_t first_free_cluster = 0;
++
-+    uint64_t *on_disk_reftable = *on_disk_reftable_ptr;
+     aio_context_set_poll_params(iothread->ctx,
-+    uint32_t on_disk_reftable_entries = *on_disk_reftable_entries_ptr;
+                                 iothread->poll_max_ns,
-     void *on_disk_refblock;
+                                 iothread->poll_grow,
--    int ret = 0;
+@@ -XXX,XX +XXX,XX @@ static void iothread_set_aio_context_params(IOThread *iothread, Error **errp)
 -    struct {
 -        uint64_t reftable_offset;
 -        uint32_t reftable_clusters;
 -    } QEMU_PACKED reftable_offset_and_clusters;
 -
 -    qcow2_cache_empty(bs, s->refcount_block_cache);
 +    bool reftable_grown = false;
 +    int ret;
 -write_refblocks:
 -    for (; cluster < *nb_clusters; cluster++) {
 +    for (cluster = first_cluster; cluster < end_cluster; cluster++) {
 +        /* Check all clusters to find refblocks that contain non-zero entries */
          if (!s->get_refcount(*refcount_table, cluster)) {
              continue;
          }
 +        /*
 +         * This cluster is allocated, so we need to create a refblock
 +         * for it.  The data we will write to disk is just the
 +         * respective slice from *refcount_table, so it will contain
 +         * accurate refcounts for all clusters belonging to this
 +         * refblock.  After we have written it, we will therefore skip
 +         * all remaining clusters in this refblock.
 +         */
 +
          refblock_index = cluster >> s->refcount_block_bits;
          refblock_start = refblock_index << s->refcount_block_bits;
 -        /* Don't allocate a cluster in a refblock already written to disk */
 -        if (first_free_cluster < refblock_start) {
 -            first_free_cluster = refblock_start;
 -        }
 -        refblock_offset = alloc_clusters_imrt(bs, 1, refcount_table,
 -                                              nb_clusters, &first_free_cluster);
 -        if (refblock_offset < 0) {
 -            fprintf(stderr, "ERROR allocating refblock: %s\n",
 -                    strerror(-refblock_offset));
 -            res->check_errors++;
 -            ret = refblock_offset;
 -            goto fail;
 -        }
 +        if (on_disk_reftable_entries > refblock_index &&
 +            on_disk_reftable[refblock_index])
 +        {
 +            /*
 +             * We can get here after a `goto write_refblocks`: We have a
 +             * reftable from a previous run, and the refblock is already
 +             * allocated.  No need to allocate it again.
 +             */
 +            refblock_offset = on_disk_reftable[refblock_index];
 +        } else {
 +            int64_t refblock_cluster_index;
 -        if (reftable_size <= refblock_index) {
 -            uint32_t old_reftable_size = reftable_size;
 -            uint64_t *new_on_disk_reftable;
 +            /* Don't allocate a cluster in a refblock already written to disk */
 +            if (first_free_cluster < refblock_start) {
 +                first_free_cluster = refblock_start;
 +            }
 +            refblock_offset = alloc_clusters_imrt(bs, 1, refcount_table,
 +                                                  nb_clusters,
 +                                                  &first_free_cluster);
 +            if (refblock_offset < 0) {
 +                fprintf(stderr, "ERROR allocating refblock: %s\n",
 +                        strerror(-refblock_offset));
 +                return refblock_offset;
 +            }
 -            reftable_size = ROUND_UP((refblock_index + 1) * REFTABLE_ENTRY_SIZE,
 -                                     s->cluster_size) / REFTABLE_ENTRY_SIZE;
 -            new_on_disk_reftable = g_try_realloc(on_disk_reftable,
 -                                                 reftable_size *
 -                                                 REFTABLE_ENTRY_SIZE);
 -            if (!new_on_disk_reftable) {
 -                res->check_errors++;
 -                ret = -ENOMEM;
 -                goto fail;
 +            refblock_cluster_index = refblock_offset / s->cluster_size;
 +            if (refblock_cluster_index >= end_cluster) {
 +                /*
 +                 * We must write the refblock that holds this refblock's
 +                 * refcount
 +                 */
 +                end_cluster = refblock_cluster_index + 1;
              }
 -            on_disk_reftable = new_on_disk_reftable;
 -            memset(on_disk_reftable + old_reftable_size, 0,
 -                   (reftable_size - old_reftable_size) * REFTABLE_ENTRY_SIZE);
 +            if (on_disk_reftable_entries <= refblock_index) {
 +                on_disk_reftable_entries =
 +                    ROUND_UP((refblock_index + 1) * REFTABLE_ENTRY_SIZE,
 +                             s->cluster_size) / REFTABLE_ENTRY_SIZE;
 +                on_disk_reftable =
 +                    g_try_realloc(on_disk_reftable,
 +                                  on_disk_reftable_entries *
 +                                  REFTABLE_ENTRY_SIZE);
 +                if (!on_disk_reftable) {
 +                    return -ENOMEM;
 +                }
 -            /* The offset we have for the reftable is now no longer valid;
 -             * this will leak that range, but we can easily fix that by running
 -             * a leak-fixing check after this rebuild operation */
 -            reftable_offset = -1;
 -        } else {
 -            assert(on_disk_reftable);
 -        }
 -        on_disk_reftable[refblock_index] = refblock_offset;
 +                memset(on_disk_reftable + *on_disk_reftable_entries_ptr, 0,
 +                       (on_disk_reftable_entries -
 +                        *on_disk_reftable_entries_ptr) *
 +                       REFTABLE_ENTRY_SIZE);
 -        /* If this is apparently the last refblock (for now), try to squeeze the
 -         * reftable in */
 -        if (refblock_index == (*nb_clusters - 1) >> s->refcount_block_bits &&
 -            reftable_offset < 0)
 -        {
 -            uint64_t reftable_clusters = size_to_clusters(s, reftable_size *
 -                                                          REFTABLE_ENTRY_SIZE);
 -            reftable_offset = alloc_clusters_imrt(bs, reftable_clusters,
 -                                                  refcount_table, nb_clusters,
 -                                                  &first_free_cluster);
 -            if (reftable_offset < 0) {
 -                fprintf(stderr, "ERROR allocating reftable: %s\n",
 -                        strerror(-reftable_offset));
 -                res->check_errors++;
 -                ret = reftable_offset;
 -                goto fail;
 +                *on_disk_reftable_ptr = on_disk_reftable;
 +                *on_disk_reftable_entries_ptr = on_disk_reftable_entries;
 +
 +                reftable_grown = true;
 +            } else {
 +                assert(on_disk_reftable);
              }
 +            on_disk_reftable[refblock_index] = refblock_offset;
          }
 +        /* Refblock is allocated, write it to disk */
 +
          ret = qcow2_pre_write_overlap_check(bs, 0, refblock_offset,
                                              s->cluster_size, false);
          if (ret < 0) {
              fprintf(stderr, "ERROR writing refblock: %s\n", strerror(-ret));
 -            goto fail;
 +            return ret;
          }
 -        /* The size of *refcount_table is always cluster-aligned, therefore the
 -         * write operation will not overflow */
 +        /*
 +         * The refblock is simply a slice of *refcount_table.
 +         * Note that the size of *refcount_table is always aligned to
 +         * whole clusters, so the write operation will not result in
 +         * out-of-bounds accesses.
 +         */
          on_disk_refblock = (void *)((char *) *refcount_table +
                                      refblock_index * s->cluster_size);
@@ -XXX,XX +XXX,XX @@ write_refblocks:
                            s->cluster_size);
          if (ret < 0) {
              fprintf(stderr, "ERROR writing refblock: %s\n", strerror(-ret));
 -            goto fail;
 +            return ret;
          }
 -        /* Go to the end of this refblock */
 +        /* This refblock is done, skip to its end */
          cluster = refblock_start + s->refcount_block_size - 1;
      }
--    if (reftable_offset < 0) {
+     aio_context_set_aio_params(iothread->ctx,
--        uint64_t post_refblock_start, reftable_clusters;
+-                               iothread->aio_max_batch,
-+    return reftable_grown;
++                               iothread->parent_obj.aio_max_batch,
-+}
+                                errp);
-+
+ }
-+/*
-+ * Creates a new refcount structure based solely on the in-memory information
+-static void iothread_complete(UserCreatable *obj, Error **errp)
-+ * given through *refcount_table (this in-memory information is basically just
++
-+ * the concatenation of all refblocks).  All necessary allocations will be
++static void iothread_init(EventLoopBase *base, Error **errp)
-+ * reflected in that array.
+ {
-+ *
+     Error *local_error = NULL;
-+ * On success, the old refcount structure is leaked (it will be covered by the
+-    IOThread *iothread = IOTHREAD(obj);
-+ * new refcount structure).
++    IOThread *iothread = IOTHREAD(base);
-+ */
+     char *thread_name;
-+static int rebuild_refcount_structure(BlockDriverState *bs,
-+                                      BdrvCheckResult *res,
+     iothread->stopping = false;
-+                                      void **refcount_table,
+@@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp)
-+                                      int64_t *nb_clusters)
+      */
-+{
+     iothread_init_gcontext(iothread);
-+    BDRVQcow2State *s = bs->opaque;
-+    int64_t reftable_offset = -1;
+-    iothread_set_aio_context_params(iothread, &local_error);
-+    int64_t reftable_length = 0;
++    iothread_set_aio_context_params(base, &local_error);
-+    int64_t reftable_clusters;
+     if (local_error) {
-+    int64_t refblock_index;
+         error_propagate(errp, local_error);
-+    uint32_t on_disk_reftable_entries = 0;
+         aio_context_unref(iothread->ctx);
-+    uint64_t *on_disk_reftable = NULL;
+@@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp)
-+    int ret = 0;
+      * to inherit.
-+    int reftable_size_changed = 0;
+      */
-+    struct {
+     thread_name = g_strdup_printf("IO %s",
-+        uint64_t reftable_offset;
+-                        object_get_canonical_path_component(OBJECT(obj)));
-+        uint32_t reftable_clusters;
++                        object_get_canonical_path_component(OBJECT(base)));
-+    } QEMU_PACKED reftable_offset_and_clusters;
+     qemu_thread_create(&iothread->thread, thread_name, iothread_run,
-+
+                        iothread, QEMU_THREAD_JOINABLE);
-+    qcow2_cache_empty(bs, s->refcount_block_cache);
+     g_free(thread_name);
-+
+@@ -XXX,XX +XXX,XX @@ static IOThreadParamInfo poll_grow_info = {
-+    /*
+ static IOThreadParamInfo poll_shrink_info = {
-+     * For each refblock containing entries, we try to allocate a
+     "poll-shrink", offsetof(IOThread, poll_shrink),
-+     * cluster (in the in-memory refcount table) and write its offset
+ };
-+     * into on_disk_reftable[].  We then write the whole refblock to
+-static IOThreadParamInfo aio_max_batch_info = {
-+     * disk (as a slice of the in-memory refcount table).
+-    "aio-max-batch", offsetof(IOThread, aio_max_batch),
-+     * This is done by rebuild_refcounts_write_refblocks().
+-};
-+     *
-+     * Once we have scanned all clusters, we try to find space for the
+ static void iothread_get_param(Object *obj, Visitor *v,
-+     * reftable.  This will dirty the in-memory refcount table (i.e.
+         const char *name, IOThreadParamInfo *info, Error **errp)
-+     * make it differ from the refblocks we have already written), so we
+@@ -XXX,XX +XXX,XX @@ static void iothread_set_poll_param(Object *obj, Visitor *v,
-+     * need to run rebuild_refcounts_write_refblocks() again for the
+     }
-+     * range of clusters where the reftable has been allocated.
+ }
-+     *
-+     * This second run might make the reftable grow again, in which case
+-static void iothread_get_aio_param(Object *obj, Visitor *v,
-+     * we will need to allocate another space for it, which is why we
+-        const char *name, void *opaque, Error **errp)
-+     * repeat all this until the reftable stops growing.
+-{
-+     *
+-    IOThreadParamInfo *info = opaque;
-+     * (This loop will terminate, because with every cluster the
+-
-+     * reftable grows, it can accomodate a multitude of more refcounts,
+-    iothread_get_param(obj, v, name, info, errp);
-+     * so that at some point this must be able to cover the reftable
+-}
-+     * and all refblocks describing it.)
+-
-+     *
+-static void iothread_set_aio_param(Object *obj, Visitor *v,
-+     * We then convert the reftable to big-endian and write it to disk.
+-        const char *name, void *opaque, Error **errp)
-+     *
+-{
-+     * Note that we never free any reftable allocations.  Doing so would
+-    IOThread *iothread = IOTHREAD(obj);
-+     * needlessly complicate the algorithm: The eventual second check
+-    IOThreadParamInfo *info = opaque;
-+     * run we do will clean up all leaks we have caused.
+-
-+     */
+-    if (!iothread_set_param(obj, v, name, info, errp)) {
-+
+-        return;
 +    reftable_size_changed =
 +        rebuild_refcounts_write_refblocks(bs, refcount_table, nb_clusters,
 +                                          0, *nb_clusters,
 +                                          &on_disk_reftable,
 +                                          &on_disk_reftable_entries);
 +    if (reftable_size_changed < 0) {
 +        res->check_errors++;
 +        ret = reftable_size_changed;
 +        goto fail;
 +    }
 +
 +    /*
 +     * There was no reftable before, so rebuild_refcounts_write_refblocks()
 +     * must have increased its size (from 0 to something).
 +     */
 +    assert(reftable_size_changed);
 +
 +    do {
 +        int64_t reftable_start_cluster, reftable_end_cluster;
 +        int64_t first_free_cluster = 0;
 +
 +        reftable_length = on_disk_reftable_entries * REFTABLE_ENTRY_SIZE;
 +        reftable_clusters = size_to_clusters(s, reftable_length);
 -        post_refblock_start = ROUND_UP(*nb_clusters, s->refcount_block_size);
 -        reftable_clusters =
 -            size_to_clusters(s, reftable_size * REFTABLE_ENTRY_SIZE);
 -        /* Not pretty but simple */
 -        if (first_free_cluster < post_refblock_start) {
 -            first_free_cluster = post_refblock_start;
 -        }
          reftable_offset = alloc_clusters_imrt(bs, reftable_clusters,
                                                refcount_table, nb_clusters,
                                                &first_free_cluster);
@@ -XXX,XX +XXX,XX @@ write_refblocks:
              goto fail;
          }
 -        goto write_refblocks;
 -    }
-+        /*
+-
-+         * We need to update the affected refblocks, so re-run the
+-    if (iothread->ctx) {
-+         * write_refblocks loop for the reftable's range of clusters.
+-        aio_context_set_aio_params(iothread->ctx,
-+         */
+-                                   iothread->aio_max_batch,
-+        assert(offset_into_cluster(s, reftable_offset) == 0);
+-                                   errp);
-+        reftable_start_cluster = reftable_offset / s->cluster_size;
+-    }
-+        reftable_end_cluster = reftable_start_cluster + reftable_clusters;
+-}
-+        reftable_size_changed =
+-
-+            rebuild_refcounts_write_refblocks(bs, refcount_table, nb_clusters,
+ static void iothread_class_init(ObjectClass *klass, void *class_data)
-+                                              reftable_start_cluster,
+ {
-+                                              reftable_end_cluster,
+-    UserCreatableClass *ucc = USER_CREATABLE_CLASS(klass);
-+                                              &on_disk_reftable,
+-    ucc->complete = iothread_complete;
-+                                              &on_disk_reftable_entries);
++    EventLoopBaseClass *bc = EVENT_LOOP_BASE_CLASS(klass);
-+        if (reftable_size_changed < 0) {
++
-+            res->check_errors++;
++    bc->init = iothread_init;
-+            ret = reftable_size_changed;
++    bc->update_params = iothread_set_aio_context_params;
-+            goto fail;
-+        }
+     object_class_property_add(klass, "poll-max-ns", "int",
-+
+                               iothread_get_poll_param,
-+        /*
+@@ -XXX,XX +XXX,XX @@ static void iothread_class_init(ObjectClass *klass, void *class_data)
-+         * If the reftable size has changed, we will need to find a new
+                               iothread_get_poll_param,
-+         * allocation, repeating the loop.
+                               iothread_set_poll_param,
-+         */
+                               NULL, &poll_shrink_info);
-+    } while (reftable_size_changed);
+-    object_class_property_add(klass, "aio-max-batch", "int",
+-                              iothread_get_aio_param,
--    for (refblock_index = 0; refblock_index < reftable_size; refblock_index++) {
+-                              iothread_set_aio_param,
-+    /* The above loop must have run at least once */
+-                              NULL, &aio_max_batch_info);
-+    assert(reftable_offset >= 0);
+ }
-+
-+    /*
+ static const TypeInfo iothread_info = {
-+     * All allocations are done, all refblocks are written, convert the
+     .name = TYPE_IOTHREAD,
-+     * reftable to big-endian and write it to disk.
+-    .parent = TYPE_OBJECT,
-+     */
++    .parent = TYPE_EVENT_LOOP_BASE,
-+
+     .class_init = iothread_class_init,
-+    for (refblock_index = 0; refblock_index < on_disk_reftable_entries;
+     .instance_size = sizeof(IOThread),
-+         refblock_index++)
+     .instance_init = iothread_instance_init,
-+    {
+     .instance_finalize = iothread_instance_finalize,
-         cpu_to_be64s(&on_disk_reftable[refblock_index]);
+-    .interfaces = (InterfaceInfo[]) {
-     }
+-        {TYPE_USER_CREATABLE},
+-        {}
--    ret = qcow2_pre_write_overlap_check(bs, 0, reftable_offset,
+-    },
--                                        reftable_size * REFTABLE_ENTRY_SIZE,
+ };
-+    ret = qcow2_pre_write_overlap_check(bs, 0, reftable_offset, reftable_length,
-                                         false);
+ static void iothread_register_types(void)
-     if (ret < 0) {
+@@ -XXX,XX +XXX,XX @@ static int query_one_iothread(Object *object, void *opaque)
-         fprintf(stderr, "ERROR writing reftable: %s\n", strerror(-ret));
+     info->poll_max_ns = iothread->poll_max_ns;
-         goto fail;
+     info->poll_grow = iothread->poll_grow;
-     }
+     info->poll_shrink = iothread->poll_shrink;
+-    info->aio_max_batch = iothread->aio_max_batch;
--    assert(reftable_size < INT_MAX / REFTABLE_ENTRY_SIZE);
++    info->aio_max_batch = iothread->parent_obj.aio_max_batch;
-+    assert(reftable_length < INT_MAX);
-     ret = bdrv_pwrite(bs->file, reftable_offset, on_disk_reftable,
+     QAPI_LIST_APPEND(*tail, info);
 -                      reftable_size * REFTABLE_ENTRY_SIZE);
 +                      reftable_length);
      if (ret < 0) {
          fprintf(stderr, "ERROR writing reftable: %s\n", strerror(-ret));
          goto fail;
@@ -XXX,XX +XXX,XX @@ write_refblocks:
      /* Enter new reftable into the image header */
      reftable_offset_and_clusters.reftable_offset = cpu_to_be64(reftable_offset);
      reftable_offset_and_clusters.reftable_clusters =
 -        cpu_to_be32(size_to_clusters(s, reftable_size * REFTABLE_ENTRY_SIZE));
 +        cpu_to_be32(reftable_clusters);
      ret = bdrv_pwrite_sync(bs->file,
                             offsetof(QCowHeader, refcount_table_offset),
                             &reftable_offset_and_clusters,
@@ -XXX,XX +XXX,XX @@ write_refblocks:
          goto fail;
      }
 -    for (refblock_index = 0; refblock_index < reftable_size; refblock_index++) {
 +    for (refblock_index = 0; refblock_index < on_disk_reftable_entries;
 +         refblock_index++)
 +    {
          be64_to_cpus(&on_disk_reftable[refblock_index]);
      }
      s->refcount_table = on_disk_reftable;
      s->refcount_table_offset = reftable_offset;
 -    s->refcount_table_size = reftable_size;
 +    s->refcount_table_size = on_disk_reftable_entries;
      update_max_refcount_table_index(s);
      return 0;
 --
 .35.1

-[PULL 2/8] tests/qemu-iotests: Move the bash and sanitizer checks to meson.build
+[PULL 2/3] util/main-loop: Introduce the main loop into QOM
-From: Thomas Huth <thuth@redhat.com>
+From: Nicolas Saenz Julienne <nsaenzju@redhat.com>
-We want to get rid of check-block.sh in the long run, so let's move
+'event-loop-base' provides basic property handling for all 'AioContext'
-the checks for the bash version and sanitizers from check-block.sh
+based event loops. So let's define a new 'MainLoopClass' that inherits
-into the meson.build file instead.
+from it. This will permit tweaking the main loop's properties through
+qapi as well as through the command line using the '-object' keyword[1].
-Signed-off-by: Thomas Huth <thuth@redhat.com>
+Only one instance of 'MainLoopClass' might be created at any time.
-Message-Id: <20220223093840.2515281-4-thuth@redhat.com>
-Signed-off-by: Hanna Reitz <hreitz@redhat.com>
+'EventLoopBaseClass' learns a new callback, 'can_be_deleted()' so as to
 mark 'MainLoop' as non-deletable.
 [1] For example:
       -object main-loop,id=main-loop,aio-max-batch=<value>
 Signed-off-by: Nicolas Saenz Julienne <nsaenzju@redhat.com>
 Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
 Acked-by: Markus Armbruster <armbru@redhat.com>
 Message-id: 20220425075723.20019-3-nsaenzju@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- tests/check-block.sh           | 26 --------------------------
+ qapi/qom.json                    | 13 ++++++++
- tests/qemu-iotests/meson.build | 14 ++++++++++++++
+ meson.build                      |  3 +-
-files changed, 14 insertions(+), 26 deletions(-)
+ include/qemu/main-loop.h         | 10 ++++++
+ include/sysemu/event-loop-base.h |  1 +
-diff --git a/tests/check-block.sh b/tests/check-block.sh
+ event-loop-base.c                | 13 ++++++++
-index XXXXXXX..XXXXXXX 100755
+ util/main-loop.c                 | 56 ++++++++++++++++++++++++++++++++
---- a/tests/check-block.sh
+files changed, 95 insertions(+), 1 deletion(-)
-+++ b/tests/check-block.sh
-@@ -XXX,XX +XXX,XX @@ skip() {
+diff --git a/qapi/qom.json b/qapi/qom.json
-     exit 0
+index XXXXXXX..XXXXXXX 100644
 --- a/qapi/qom.json
 +++ b/qapi/qom.json
@@ -XXX,XX +XXX,XX @@
              '*poll-grow': 'int',
              '*poll-shrink': 'int' } }
 +##
 +# @MainLoopProperties:
 +#
 +# Properties for the main-loop object.
 +#
 +# Since: 7.1
 +##
 +{ 'struct': 'MainLoopProperties',
 +  'base': 'EventLoopBaseProperties',
 +  'data': {} }
 +
  ##
  # @MemoryBackendProperties:
  #
@@ -XXX,XX +XXX,XX @@
      { 'name': 'input-linux',
        'if': 'CONFIG_LINUX' },
      'iothread',
 +    'main-loop',
      { 'name': 'memory-backend-epc',
        'if': 'CONFIG_LINUX' },
      'memory-backend-file',
@@ -XXX,XX +XXX,XX @@
        'input-linux':                { 'type': 'InputLinuxProperties',
                                        'if': 'CONFIG_LINUX' },
        'iothread':                   'IothreadProperties',
 +      'main-loop':                  'MainLoopProperties',
        'memory-backend-epc':         { 'type': 'MemoryBackendEpcProperties',
                                        'if': 'CONFIG_LINUX' },
        'memory-backend-file':        'MemoryBackendFileProperties',
 diff --git a/meson.build b/meson.build
 index XXXXXXX..XXXXXXX 100644
 --- a/meson.build
 +++ b/meson.build
@@ -XXX,XX +XXX,XX @@ libqemuutil = static_library('qemuutil',
                               sources: util_ss.sources() + stub_ss.sources() + genh,
                               dependencies: [util_ss.dependencies(), libm, threads, glib, socket, malloc, pixman])
  qemuutil = declare_dependency(link_with: libqemuutil,
 -                              sources: genh + version_res)
 +                              sources: genh + version_res,
 +                              dependencies: [event_loop_base])
  if have_system or have_user
    decodetree = generator(find_program('scripts/decodetree.py'),
 diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
 index XXXXXXX..XXXXXXX 100644
 --- a/include/qemu/main-loop.h
 +++ b/include/qemu/main-loop.h
@@ -XXX,XX +XXX,XX @@
  #define QEMU_MAIN_LOOP_H
  #include "block/aio.h"
 +#include "qom/object.h"
 +#include "sysemu/event-loop-base.h"
  #define SIG_IPI SIGUSR1
 +#define TYPE_MAIN_LOOP  "main-loop"
 +OBJECT_DECLARE_TYPE(MainLoop, MainLoopClass, MAIN_LOOP)
 +
 +struct MainLoop {
 +    EventLoopBase parent_obj;
 +};
 +typedef struct MainLoop MainLoop;
 +
  /**
   * qemu_init_main_loop: Set up the process so that it can run the main loop.
   *
 diff --git a/include/sysemu/event-loop-base.h b/include/sysemu/event-loop-base.h
 index XXXXXXX..XXXXXXX 100644
 --- a/include/sysemu/event-loop-base.h
 +++ b/include/sysemu/event-loop-base.h
@@ -XXX,XX +XXX,XX @@ struct EventLoopBaseClass {
      void (*init)(EventLoopBase *base, Error **errp);
      void (*update_params)(EventLoopBase *base, Error **errp);
 +    bool (*can_be_deleted)(EventLoopBase *base);
  };
  struct EventLoopBase {
 diff --git a/event-loop-base.c b/event-loop-base.c
 index XXXXXXX..XXXXXXX 100644
 --- a/event-loop-base.c
 +++ b/event-loop-base.c
@@ -XXX,XX +XXX,XX @@ static void event_loop_base_complete(UserCreatable *uc, Error **errp)
      }
  }
--# Disable tests with any sanitizer except for specific ones
++static bool event_loop_base_can_be_deleted(UserCreatable *uc)
--SANITIZE_FLAGS=$( grep "CFLAGS.*-fsanitize" config-host.mak 2>/dev/null )
++{
--ALLOWED_SANITIZE_FLAGS="safe-stack cfi-icall"
++    EventLoopBaseClass *bc = EVENT_LOOP_BASE_GET_CLASS(uc);
--#Remove all occurrencies of allowed Sanitize flags
++    EventLoopBase *backend = EVENT_LOOP_BASE(uc);
--for j in ${ALLOWED_SANITIZE_FLAGS}; do
++
--    TMP_FLAGS=${SANITIZE_FLAGS}
++    if (bc->can_be_deleted) {
--    SANITIZE_FLAGS=""
++        return bc->can_be_deleted(backend);
--    for i in ${TMP_FLAGS}; do
++    }
--        if ! echo ${i} | grep -q "${j}" 2>/dev/null; then
++
--            SANITIZE_FLAGS="${SANITIZE_FLAGS} ${i}"
++    return true;
--        fi
++}
--    done
++
--done
+ static void event_loop_base_class_init(ObjectClass *klass, void *class_data)
--if echo ${SANITIZE_FLAGS} | grep -q "\-fsanitize" 2>/dev/null; then
+ {
--    # Have a sanitize flag that is not allowed, stop
+     UserCreatableClass *ucc = USER_CREATABLE_CLASS(klass);
--    skip "Sanitizers are enabled ==> Not running the qemu-iotests."
+     ucc->complete = event_loop_base_complete;
--fi
++    ucc->can_be_deleted = event_loop_base_can_be_deleted;
--
- if [ -z "$(find . -name 'qemu-system-*' -print)" ]; then
+     object_class_property_add(klass, "aio-max-batch", "int",
-     skip "No qemu-system binary available ==> Not running the qemu-iotests."
+                               event_loop_base_get_param,
- fi
+diff --git a/util/main-loop.c b/util/main-loop.c
+index XXXXXXX..XXXXXXX 100644
--if ! command -v bash >/dev/null 2>&1 ; then
+--- a/util/main-loop.c
--    skip "bash not available ==> Not running the qemu-iotests."
++++ b/util/main-loop.c
--fi
+@@ -XXX,XX +XXX,XX @@
--
+ #include "qemu/error-report.h"
--if LANG=C bash --version | grep -q 'GNU bash, version [123]' ; then
+ #include "qemu/queue.h"
--    skip "bash version too old ==> Not running the qemu-iotests."
+ #include "qemu/compiler.h"
--fi
++#include "qom/object.h"
--
- cd tests/qemu-iotests
+ #ifndef _WIN32
+ #include <sys/wait.h>
- # QEMU_CHECK_BLOCK_AUTO is used to disable some unstable sub-tests
+@@ -XXX,XX +XXX,XX @@ int qemu_init_main_loop(Error **errp)
-diff --git a/tests/qemu-iotests/meson.build b/tests/qemu-iotests/meson.build
+     return 0;
-index XXXXXXX..XXXXXXX 100644
+ }
---- a/tests/qemu-iotests/meson.build
-+++ b/tests/qemu-iotests/meson.build
++static void main_loop_update_params(EventLoopBase *base, Error **errp)
-@@ -XXX,XX +XXX,XX @@ if not have_tools or targetos == 'windows' or get_option('gprof')
++{
-   subdir_done()
++    if (!qemu_aio_context) {
- endif
++        error_setg(errp, "qemu aio context not ready");
++        return;
-+foreach cflag: config_host['QEMU_CFLAGS'].split()
++    }
-+  if cflag.startswith('-fsanitize') and \
++
-+     not cflag.contains('safe-stack') and not cflag.contains('cfi-icall')
++    aio_context_set_aio_params(qemu_aio_context, base->aio_max_batch, errp);
-+    message('Sanitizers are enabled ==> Disabled the qemu-iotests.')
++}
-+    subdir_done()
++
-+  endif
++MainLoop *mloop;
-+endforeach
++
-+
++static void main_loop_init(EventLoopBase *base, Error **errp)
-+bash = find_program('bash', required: false, version: '>= 4.0')
++{
-+if not bash.found()
++    MainLoop *m = MAIN_LOOP(base);
-+  message('bash >= v4.0 not available ==> Disabled the qemu-iotests.')
++
-+  subdir_done()
++    if (mloop) {
-+endif
++        error_setg(errp, "only one main-loop instance allowed");
-+
++        return;
- qemu_iotests_binaries = [qemu_img, qemu_io, qemu_nbd, qsd]
++    }
- qemu_iotests_env = {'PYTHON': python.full_path()}
++
- qemu_iotests_formats = {
++    main_loop_update_params(base, errp);
 +
 +    mloop = m;
 +    return;
 +}
 +
 +static bool main_loop_can_be_deleted(EventLoopBase *base)
 +{
 +    return false;
 +}
 +
 +static void main_loop_class_init(ObjectClass *oc, void *class_data)
 +{
 +    EventLoopBaseClass *bc = EVENT_LOOP_BASE_CLASS(oc);
 +
 +    bc->init = main_loop_init;
 +    bc->update_params = main_loop_update_params;
 +    bc->can_be_deleted = main_loop_can_be_deleted;
 +}
 +
 +static const TypeInfo main_loop_info = {
 +    .name = TYPE_MAIN_LOOP,
 +    .parent = TYPE_EVENT_LOOP_BASE,
 +    .class_init = main_loop_class_init,
 +    .instance_size = sizeof(MainLoop),
 +};
 +
 +static void main_loop_register_types(void)
 +{
 +    type_register_static(&main_loop_info);
 +}
 +
 +type_init(main_loop_register_types)
 +
  static int max_priority;
  #ifndef _WIN32
 --
 .35.1

-[PULL 3/8] iotests.py: Add supports_qcow2_zstd_compression()
+Deleted patch
-Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Signed-off-by: Hanna Reitz <hreitz@redhat.com>
-Message-Id: <20220323105522.53660-2-hreitz@redhat.com>
-Reviewed-by: Vladimir Sementsov-Ogievskiy <v.sementsov-og@mail.ru>
----
- tests/qemu-iotests/iotests.py | 20 ++++++++++++++++++++
-file changed, 20 insertions(+)
-diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/iotests.py
-+++ b/tests/qemu-iotests/iotests.py
-@@ -XXX,XX +XXX,XX @@ def verify_working_luks():
-     if not working:
-         notrun(reason)
-+def supports_qcow2_zstd_compression() -> bool:
-+    img_file = f'{test_dir}/qcow2-zstd-test.qcow2'
-+    res = qemu_img('create', '-f', 'qcow2', '-o', 'compression_type=zstd',
-+                   img_file, '0',
-+                   check=False)
-+    try:
-+        os.remove(img_file)
-+    except OSError:
-+        pass
-+
-+    if res.returncode == 1 and \
-+            "'compression-type' does not accept value 'zstd'" in res.stdout:
-+        return False
-+    else:
-+        return True
-+
-+def verify_qcow2_zstd_compression():
-+    if not supports_qcow2_zstd_compression():
-+        notrun('zstd compression not supported')
-+
- def qemu_pipe(*args: str) -> str:
-     """
-     Run qemu with an option to print something and exit (e.g. a help option).
---
-.35.1

-[PULL 4/8] iotests/065: Check for zstd support
+Deleted patch
-Some test cases run in iotest 065 want to run with zstd compression just
-for added coverage.  Run them with zlib if there is no zstd support
-compiled in.
-Reported-by: Thomas Huth <thuth@redhat.com>
-Fixes: 12a936171d71f839dc907ff ("iotest 065: explicit compression type")
-Signed-off-by: Hanna Reitz <hreitz@redhat.com>
-Message-Id: <20220323105522.53660-3-hreitz@redhat.com>
-Reviewed-by: Vladimir Sementsov-Ogievskiy <v.sementsov-og@mail.ru>
----
- tests/qemu-iotests/065 | 24 ++++++++++++++++++------
-file changed, 18 insertions(+), 6 deletions(-)
-diff --git a/tests/qemu-iotests/065 b/tests/qemu-iotests/065
-index XXXXXXX..XXXXXXX 100755
---- a/tests/qemu-iotests/065
-+++ b/tests/qemu-iotests/065
-@@ -XXX,XX +XXX,XX @@ import os
- import re
- import json
- import iotests
--from iotests import qemu_img, qemu_img_info
-+from iotests import qemu_img, qemu_img_info, supports_qcow2_zstd_compression
- import unittest
- test_img = os.path.join(iotests.test_dir, 'test.img')
-@@ -XXX,XX +XXX,XX @@ class TestQCow2(TestQemuImgInfo):
- class TestQCow3NotLazy(TestQemuImgInfo):
-     '''Testing a qcow2 version 3 image with lazy refcounts disabled'''
--    img_options = 'compat=1.1,lazy_refcounts=off,compression_type=zstd'
-+    if supports_qcow2_zstd_compression():
-+        compression_type = 'zstd'
-+    else:
-+        compression_type = 'zlib'
-+
-+    img_options = 'compat=1.1,lazy_refcounts=off'
-+    img_options += f',compression_type={compression_type}'
-     json_compare = { 'compat': '1.1', 'lazy-refcounts': False,
-                      'refcount-bits': 16, 'corrupt': False,
--                     'compression-type': 'zstd', 'extended-l2': False }
--    human_compare = [ 'compat: 1.1', 'compression type: zstd',
-+                     'compression-type': compression_type, 'extended-l2': False }
-+    human_compare = [ 'compat: 1.1', f'compression type: {compression_type}',
-                       'lazy refcounts: false', 'refcount bits: 16',
-                       'corrupt: false', 'extended l2: false' ]
-@@ -XXX,XX +XXX,XX @@ class TestQCow3NotLazyQMP(TestQMP):
- class TestQCow3LazyQMP(TestQMP):
-     '''Testing a qcow2 version 3 image with lazy refcounts enabled, opening
-        with lazy refcounts disabled'''
--    img_options = 'compat=1.1,lazy_refcounts=on,compression_type=zstd'
-+    if supports_qcow2_zstd_compression():
-+        compression_type = 'zstd'
-+    else:
-+        compression_type = 'zlib'
-+
-+    img_options = 'compat=1.1,lazy_refcounts=on'
-+    img_options += f',compression_type={compression_type}'
-     qemu_options = 'lazy-refcounts=off'
-     compare = { 'compat': '1.1', 'lazy-refcounts': True,
-                 'refcount-bits': 16, 'corrupt': False,
--                'compression-type': 'zstd', 'extended-l2': False }
-+                'compression-type': compression_type, 'extended-l2': False }
- TestImageInfoSpecific = None
- TestQemuImgInfo = None
---
-.35.1

-[PULL 5/8] iotests/303: Check for zstd support
+Deleted patch
-runs two test cases, one of which requires zstd support.
-Unfortunately, given that this is not a unittest-style test, we cannot
-easily skip that single case, and instead can only skip the whole test.
-(Alternatively, we could split this test into a zlib and a zstd part,
-but that seems excessive, given that this test is not in auto and thus
-likely only run by developers who have zstd support compiled in.)
-Fixes: 677e0bae686e7c670a71d1f ("iotest 303: explicit compression type")
-Signed-off-by: Hanna Reitz <hreitz@redhat.com>
-Reviewed-by: Vladimir Sementsov-Ogievskiy <v.sementsov-og@mail.ru>
-Message-Id: <20220323105522.53660-4-hreitz@redhat.com>
----
- tests/qemu-iotests/303 | 4 +++-
-file changed, 3 insertions(+), 1 deletion(-)
-diff --git a/tests/qemu-iotests/303 b/tests/qemu-iotests/303
-index XXXXXXX..XXXXXXX 100755
---- a/tests/qemu-iotests/303
-+++ b/tests/qemu-iotests/303
-@@ -XXX,XX +XXX,XX @@
- import iotests
- import subprocess
--from iotests import qemu_img_create, qemu_io, file_path, log, filter_qemu_io
-+from iotests import qemu_img_create, qemu_io, file_path, log, filter_qemu_io, \
-+        verify_qcow2_zstd_compression
- iotests.script_initialize(supported_fmts=['qcow2'],
-                           unsupported_imgopts=['refcount_bits', 'compat'])
-+verify_qcow2_zstd_compression()
- disk = file_path('disk')
- chunk = 1024 * 1024
---
-.35.1

-[PULL 7/8] iotests/108: Test new refcount rebuild algorithm
+Deleted patch
-One clear problem with how qcow2's refcount structure rebuild algorithm
-used to be before "qcow2: Improve refcount structure rebuilding" was
-that it is prone to failure for qcow2 images on block devices: There is
-generally unused space after the actual image, and if that exceeds what
-one refblock covers, the old algorithm would invariably write the
-reftable past the block device's end, which cannot work.  The new
-algorithm does not have this problem.
-Test it with three tests:
-(1) Create an image with more empty space at the end than what one
-    refblock covers, see whether rebuilding the refcount structures
-    results in a change in the image file length.  (It should not.)
-(2) Leave precisely enough space somewhere at the beginning of the image
-    for the new reftable (and the refblock for that place), see whether
-    the new algorithm puts the reftable there.  (It should.)
-(3) Test the original problem: Create (something like) a block device
-    with a fixed size, then create a qcow2 image in there, write some
-    data, and then have qemu-img check rebuild the refcount structures.
-    Before HEAD^, the reftable would have been written past the image
-    file end, i.e. outside of what the block device provides, which
-    cannot work.  HEAD^ should have fixed that.
-    ("Something like a block device" means a loop device if we can use
-    one ("sudo -n losetup" works), or a FUSE block export with
-    growable=false otherwise.)
-Reviewed-by: Eric Blake <eblake@redhat.com>
-Signed-off-by: Hanna Reitz <hreitz@redhat.com>
-Message-Id: <20220405134652.19278-3-hreitz@redhat.com>
----
- tests/qemu-iotests/108     | 259 ++++++++++++++++++++++++++++++++++++-
- tests/qemu-iotests/108.out |  81 ++++++++++++
-files changed, 339 insertions(+), 1 deletion(-)
-diff --git a/tests/qemu-iotests/108 b/tests/qemu-iotests/108
-index XXXXXXX..XXXXXXX 100755
---- a/tests/qemu-iotests/108
-+++ b/tests/qemu-iotests/108
-@@ -XXX,XX +XXX,XX @@ status=1    # failure is the default!
- _cleanup()
- {
--    _cleanup_test_img
-+    _cleanup_test_img
-+    if [ -f "$TEST_DIR/qsd.pid" ]; then
-+        qsd_pid=$(cat "$TEST_DIR/qsd.pid")
-+        kill -KILL "$qsd_pid"
-+        fusermount -u "$TEST_DIR/fuse-export" &>/dev/null
-+    fi
-+    rm -f "$TEST_DIR/fuse-export"
- }
- trap "_cleanup; exit \$status" 0 1 2 3 15
- # get standard environment, filters and checks
- . ./common.rc
- . ./common.filter
-+. ./common.qemu
- # This tests qcow2-specific low-level functionality
- _supported_fmt qcow2
-@@ -XXX,XX +XXX,XX @@ _supported_os Linux
- # files
- _unsupported_imgopts 'refcount_bits=\([^1]\|.\([^6]\|$\)\)' data_file
-+# This test either needs sudo -n losetup or FUSE exports to work
-+if sudo -n losetup &>/dev/null; then
-+    loopdev=true
-+else
-+    loopdev=false
-+
-+    # QSD --export fuse will either yield "Parameter 'id' is missing"
-+    # or "Invalid parameter 'fuse'", depending on whether there is
-+    # FUSE support or not.
-+    error=$($QSD --export fuse 2>&1)
-+    if [[ $error = *"'fuse'"* ]]; then
-+        _notrun 'Passwordless sudo for losetup or FUSE support required, but' \
-+                'neither is available'
-+    fi
-+fi
-+
- echo
- echo '=== Repairing an image without any refcount table ==='
- echo
-@@ -XXX,XX +XXX,XX @@ _make_test_img 64M
- poke_file "$TEST_IMG" $((0x10008)) "\xff\xff\xff\xff\xff\xff\x00\x00"
- _check_test_img -r all
-+echo
-+echo '=== Check rebuilt reftable location ==='
-+
-+# In an earlier version of the refcount rebuild algorithm, the
-+# reftable was generally placed at the image end (unless something was
-+# allocated in the area covered by the refblock right before the image
-+# file end, then we would try to place the reftable in that refblock).
-+# This was later changed so the reftable would be placed in the
-+# earliest possible location.  Test this.
-+
-+echo
-+echo '--- Does the image size increase? ---'
-+echo
-+
-+# First test: Just create some image, write some data to it, and
-+# resize it so there is free space at the end of the image (enough
-+# that it spans at least one full refblock, which for cluster_size=512
-+# images, spans 128k).  With the old algorithm, the reftable would
-+# have then been placed at the end of the image file, but with the new
-+# one, it will be put in that free space.
-+# We want to check whether the size of the image file increases due to
-+# rebuilding the refcount structures (it should not).
-+
-+_make_test_img -o 'cluster_size=512' 1M
-+# Write something
-+$QEMU_IO -c 'write 0 64k' "$TEST_IMG" | _filter_qemu_io
-+
-+# Add free space
-+file_len=$(stat -c '%s' "$TEST_IMG")
-+truncate -s $((file_len + 256 * 1024)) "$TEST_IMG"
-+
-+# Corrupt the image by saying the image header was not allocated
-+rt_offset=$(peek_file_be "$TEST_IMG" 48 8)
-+rb_offset=$(peek_file_be "$TEST_IMG" $rt_offset 8)
-+poke_file "$TEST_IMG" $rb_offset "\x00\x00"
-+
-+# Check whether rebuilding the refcount structures increases the image
-+# file size
-+file_len=$(stat -c '%s' "$TEST_IMG")
-+echo
-+# The only leaks there can be are the old refcount structures that are
-+# leaked during rebuilding, no need to clutter the output with them
-+_check_test_img -r all | grep -v '^Repairing cluster.*refcount=1 reference=0'
-+echo
-+post_repair_file_len=$(stat -c '%s' "$TEST_IMG")
-+
-+if [[ $file_len -eq $post_repair_file_len ]]; then
-+    echo 'OK: Image size did not change'
-+else
-+    echo 'ERROR: Image size differs' \
-+        "($file_len before, $post_repair_file_len after)"
-+fi
-+
-+echo
-+echo '--- Will the reftable occupy a hole specifically left for it?  ---'
-+echo
-+
-+# Note: With cluster_size=512, every refblock covers 128k.
-+# The reftable covers 8M per reftable cluster.
-+
-+# Create an image that requires two reftable clusters (just because
-+# this is more interesting than a single-clustered reftable).
-+_make_test_img -o 'cluster_size=512' 9M
-+$QEMU_IO -c 'write 0 8M' "$TEST_IMG" | _filter_qemu_io
-+
-+# Writing 8M will have resized the reftable.  Unfortunately, doing so
-+# will leave holes in the file, so we need to fill them up so we can
-+# be sure the whole file is allocated.  Do that by writing
-+# consecutively smaller chunks starting from 8 MB, until the file
-+# length increases even with a chunk size of 512.  Then we must have
-+# filled all holes.
-+ofs=$((8 * 1024 * 1024))
-+block_len=$((16 * 1024))
-+while [[ $block_len -ge 512 ]]; do
-+    file_len=$(stat -c '%s' "$TEST_IMG")
-+    while [[ $(stat -c '%s' "$TEST_IMG") -eq $file_len ]]; do
-+        # Do not include this in the reference output, it does not
-+        # really matter which qemu-io calls we do here exactly
-+        $QEMU_IO -c "write $ofs $block_len" "$TEST_IMG" >/dev/null
-+        ofs=$((ofs + block_len))
-+    done
-+    block_len=$((block_len / 2))
-+done
-+
-+# Fill up to 9M (do not include this in the reference output either,
-+# $ofs is random for all we know)
-+$QEMU_IO -c "write $ofs $((9 * 1024 * 1024 - ofs))" "$TEST_IMG" >/dev/null
-+
-+# Make space as follows:
-+# - For the first refblock: Right at the beginning of the image (this
-+#   refblock is placed in the first place possible),
-+# - For the reftable somewhere soon afterwards, still near the
-+#   beginning of the image (i.e. covered by the first refblock); the
-+#   reftable too is placed in the first place possible, but only after
-+#   all refblocks have been placed)
-+# No space is needed for the other refblocks, because no refblock is
-+# put before the space it covers.  In this test case, we do not mind
-+# if they are placed at the image file's end.
-+
-+# Before we make that space, we have to find out the host offset of
-+# the area that belonged to the two data clusters at guest offset 4k,
-+# because we expect the reftable to be placed there, and we will have
-+# to verify that it is.
-+
-+l1_offset=$(peek_file_be "$TEST_IMG" 40 8)
-+l2_offset=$(peek_file_be "$TEST_IMG" $l1_offset 8)
-+l2_offset=$((l2_offset & 0x00fffffffffffe00))
-+data_4k_offset=$(peek_file_be "$TEST_IMG" \
-+                 $((l2_offset + 4096 / 512 * 8)) 8)
-+data_4k_offset=$((data_4k_offset & 0x00fffffffffffe00))
-+
-+$QEMU_IO -c "discard 0 512" -c "discard 4k 1k" "$TEST_IMG" | _filter_qemu_io
-+
-+# Corrupt the image by saying the image header was not allocated
-+rt_offset=$(peek_file_be "$TEST_IMG" 48 8)
-+rb_offset=$(peek_file_be "$TEST_IMG" $rt_offset 8)
-+poke_file "$TEST_IMG" $rb_offset "\x00\x00"
-+
-+echo
-+# The only leaks there can be are the old refcount structures that are
-+# leaked during rebuilding, no need to clutter the output with them
-+_check_test_img -r all | grep -v '^Repairing cluster.*refcount=1 reference=0'
-+echo
-+
-+# Check whether the reftable was put where we expected
-+rt_offset=$(peek_file_be "$TEST_IMG" 48 8)
-+if [[ $rt_offset -eq $data_4k_offset ]]; then
-+    echo 'OK: Reftable is where we expect it'
-+else
-+    echo "ERROR: Reftable is at $rt_offset, but was expected at $data_4k_offset"
-+fi
-+
-+echo
-+echo '--- Rebuilding refcount structures on block devices ---'
-+echo
-+
-+# A block device cannot really grow, at least not during qemu-img
-+# check.  As mentioned in the above cases, rebuilding the refcount
-+# structure may lead to new refcount structures being written after
-+# the end of the image, and in the past that happened even if there
-+# was more than sufficient space in the image.  Such post-EOF writes
-+# will not work on block devices, so test that the new algorithm
-+# avoids it.
-+
-+# If we have passwordless sudo and losetup, we can use those to create
-+# a block device.  Otherwise, we can resort to qemu's FUSE export to
-+# create a file that isn't growable, which effectively tests the same
-+# thing.
-+
-+_cleanup_test_img
-+truncate -s $((64 * 1024 * 1024)) "$TEST_IMG"
-+
-+if $loopdev; then
-+    export_mp=$(sudo -n losetup --show -f "$TEST_IMG")
-+    export_mp_driver=host_device
-+    sudo -n chmod go+rw "$export_mp"
-+else
-+    # Create non-growable FUSE export that is a bit like an empty
-+    # block device
-+    export_mp="$TEST_DIR/fuse-export"
-+    export_mp_driver=file
-+    touch "$export_mp"
-+
-+    $QSD \
-+        --blockdev file,node-name=export-node,filename="$TEST_IMG" \
-+        --export fuse,id=fuse-export,node-name=export-node,mountpoint="$export_mp",writable=on,growable=off \
-+        --pidfile "$TEST_DIR/qsd.pid" \
-+        --daemonize
-+fi
-+
-+# Now create a qcow2 image on the device -- unfortunately, qemu-img
-+# create force-creates the file, so we have to resort to the
-+# blockdev-create job.
-+_launch_qemu \
-+    --blockdev $export_mp_driver,node-name=file,filename="$export_mp"
-+
-+_send_qemu_cmd \
-+    $QEMU_HANDLE \
-+    '{ "execute": "qmp_capabilities" }' \
-+    'return'
-+
-+# Small cluster size again, so the image needs multiple refblocks
-+_send_qemu_cmd \
-+    $QEMU_HANDLE \
-+    '{ "execute": "blockdev-create",
-+       "arguments": {
-+           "job-id": "create",
-+           "options": {
-+               "driver": "qcow2",
-+               "file": "file",
-+               "size": '$((64 * 1024 * 1024))',
-+               "cluster-size": 512
-+           } } }' \
-+    '"concluded"'
-+
-+_send_qemu_cmd \
-+    $QEMU_HANDLE \
-+    '{ "execute": "job-dismiss", "arguments": { "id": "create" } }' \
-+    'return'
-+
-+_send_qemu_cmd \
-+    $QEMU_HANDLE \
-+    '{ "execute": "quit" }' \
-+    'return'
-+
-+wait=y _cleanup_qemu
-+echo
-+
-+# Write some data
-+$QEMU_IO -c 'write 0 64k' "$export_mp" | _filter_qemu_io
-+
-+# Corrupt the image by saying the image header was not allocated
-+rt_offset=$(peek_file_be "$export_mp" 48 8)
-+rb_offset=$(peek_file_be "$export_mp" $rt_offset 8)
-+poke_file "$export_mp" $rb_offset "\x00\x00"
-+
-+# Repairing such a simple case should just work
-+# (We used to put the reftable at the end of the image file, which can
-+# never work for non-growable devices.)
-+echo
-+TEST_IMG="$export_mp" _check_test_img -r all \
-+    | grep -v '^Repairing cluster.*refcount=1 reference=0'
-+
-+if $loopdev; then
-+    sudo -n losetup -d "$export_mp"
-+else
-+    qsd_pid=$(cat "$TEST_DIR/qsd.pid")
-+    kill -TERM "$qsd_pid"
-+    # Wait for process to exit (cannot `wait` because the QSD is daemonized)
-+    while [ -f "$TEST_DIR/qsd.pid" ]; do
-+        true
-+    done
-+fi
-+
- # success, all done
- echo '*** done'
- rm -f $seq.full
-diff --git a/tests/qemu-iotests/108.out b/tests/qemu-iotests/108.out
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/108.out
-+++ b/tests/qemu-iotests/108.out
-@@ -XXX,XX +XXX,XX @@ The following inconsistencies were found and repaired:
-leaked clusters
-corruptions
-+Double checking the fixed image now...
-+No errors were found on the image.
-+
-+=== Check rebuilt reftable location ===
-+
-+--- Does the image size increase? ---
-+
-+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
-+wrote 65536/65536 bytes at offset 0
-+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-+
-+ERROR cluster 0 refcount=0 reference=1
-+Rebuilding refcount structure
-+The following inconsistencies were found and repaired:
-+
-+    0 leaked clusters
-+    1 corruptions
-+
-+Double checking the fixed image now...
-+No errors were found on the image.
-+
-+OK: Image size did not change
-+
-+--- Will the reftable occupy a hole specifically left for it?  ---
-+
-+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=9437184
-+wrote 8388608/8388608 bytes at offset 0
-+8 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-+discard 512/512 bytes at offset 0
-+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-+discard 1024/1024 bytes at offset 4096
-+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-+
-+ERROR cluster 0 refcount=0 reference=1
-+Rebuilding refcount structure
-+The following inconsistencies were found and repaired:
-+
-+    0 leaked clusters
-+    1 corruptions
-+
-+Double checking the fixed image now...
-+No errors were found on the image.
-+
-+OK: Reftable is where we expect it
-+
-+--- Rebuilding refcount structures on block devices ---
-+
-+{ "execute": "qmp_capabilities" }
-+{"return": {}}
-+{ "execute": "blockdev-create",
-+       "arguments": {
-+           "job-id": "create",
-+           "options": {
-+               "driver": "IMGFMT",
-+               "file": "file",
-+               "size": 67108864,
-+               "cluster-size": 512
-+           } } }
-+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "create"}}
-+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "create"}}
-+{"return": {}}
-+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "waiting", "id": "create"}}
-+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "pending", "id": "create"}}
-+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "concluded", "id": "create"}}
-+{ "execute": "job-dismiss", "arguments": { "id": "create" } }
-+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "null", "id": "create"}}
-+{"return": {}}
-+{ "execute": "quit" }
-+{"return": {}}
-+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}}
-+
-+wrote 65536/65536 bytes at offset 0
-+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-+
-+ERROR cluster 0 refcount=0 reference=1
-+Rebuilding refcount structure
-+The following inconsistencies were found and repaired:
-+
-+    0 leaked clusters
-+    1 corruptions
-+
- Double checking the fixed image now...
- No errors were found on the image.
- *** done
---
-.35.1

-[PULL 8/8] qcow2: Add errp to rebuild_refcount_structure()
+[PULL 3/3] util/event-loop-base: Introduce options to set the thread pool size
-Instead of fprint()-ing error messages in rebuild_refcount_structure()
+From: Nicolas Saenz Julienne <nsaenzju@redhat.com>
 and its rebuild_refcounts_write_refblocks() helper, pass them through an
 Error object to qcow2_check_refcounts() (which will then print it).
-Suggested-by: Eric Blake <eblake@redhat.com>
+The thread pool regulates itself: when idle, it kills threads until
-Signed-off-by: Hanna Reitz <hreitz@redhat.com>
+empty, when in demand, it creates new threads until full. This behaviour
-Message-Id: <20220405134652.19278-4-hreitz@redhat.com>
+doesn't play well with latency sensitive workloads where the price of
-Reviewed-by: Eric Blake <eblake@redhat.com>
+creating a new thread is too high. For example, when paired with qemu's
 '-mlock', or using safety features like SafeStack, creating a new thread
 has been measured take multiple milliseconds.
 In order to mitigate this let's introduce a new 'EventLoopBase'
 property to set the thread pool size. The threads will be created during
 the pool's initialization or upon updating the property's value, remain
 available during its lifetime regardless of demand, and destroyed upon
 freeing it. A properly characterized workload will then be able to
 configure the pool to avoid any latency spikes.
 Signed-off-by: Nicolas Saenz Julienne <nsaenzju@redhat.com>
 Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
 Acked-by: Markus Armbruster <armbru@redhat.com>
 Message-id: 20220425075723.20019-4-nsaenzju@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/qcow2-refcount.c | 33 +++++++++++++++++++--------------
+ qapi/qom.json                    | 10 +++++-
-file changed, 19 insertions(+), 14 deletions(-)
+ include/block/aio.h              | 10 ++++++
  include/block/thread-pool.h      |  3 ++
  include/sysemu/event-loop-base.h |  4 +++
  event-loop-base.c                | 23 +++++++++++++
  iothread.c                       |  3 ++
  util/aio-posix.c                 |  1 +
  util/async.c                     | 20 ++++++++++++
  util/main-loop.c                 |  9 ++++++
  util/thread-pool.c               | 55 +++++++++++++++++++++++++++++---
 files changed, 133 insertions(+), 5 deletions(-)
-diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
+diff --git a/qapi/qom.json b/qapi/qom.json
 index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2-refcount.c
+--- a/qapi/qom.json
-+++ b/block/qcow2-refcount.c
++++ b/qapi/qom.json
-@@ -XXX,XX +XXX,XX @@ static int64_t alloc_clusters_imrt(BlockDriverState *bs,
+@@ -XXX,XX +XXX,XX @@
- static int rebuild_refcounts_write_refblocks(
+ #                 0 means that the engine will use its default.
-         BlockDriverState *bs, void **refcount_table, int64_t *nb_clusters,
+ #                 (default: 0)
-         int64_t first_cluster, int64_t end_cluster,
+ #
--        uint64_t **on_disk_reftable_ptr, uint32_t *on_disk_reftable_entries_ptr
++# @thread-pool-min: minimum number of threads reserved in the thread pool
-+        uint64_t **on_disk_reftable_ptr, uint32_t *on_disk_reftable_entries_ptr,
++#                   (default:0)
-+        Error **errp
++#
-     )
++# @thread-pool-max: maximum number of threads the thread pool can contain
 +#                   (default:64)
 +#
  # Since: 7.1
  ##
  { 'struct': 'EventLoopBaseProperties',
 -  'data': { '*aio-max-batch': 'int' } }
 +  'data': { '*aio-max-batch': 'int',
 +            '*thread-pool-min': 'int',
 +            '*thread-pool-max': 'int' } }
  ##
  # @IothreadProperties:
 diff --git a/include/block/aio.h b/include/block/aio.h
 index XXXXXXX..XXXXXXX 100644
 --- a/include/block/aio.h
 +++ b/include/block/aio.h
@@ -XXX,XX +XXX,XX @@ struct AioContext {
      QSLIST_HEAD(, Coroutine) scheduled_coroutines;
      QEMUBH *co_schedule_bh;
 +    int thread_pool_min;
 +    int thread_pool_max;
      /* Thread pool for performing work and receiving completion callbacks.
       * Has its own locking.
       */
@@ -XXX,XX +XXX,XX @@ void aio_context_set_poll_params(AioContext *ctx, int64_t max_ns,
  void aio_context_set_aio_params(AioContext *ctx, int64_t max_batch,
                                  Error **errp);
 +/**
 + * aio_context_set_thread_pool_params:
 + * @ctx: the aio context
 + * @min: min number of threads to have readily available in the thread pool
 + * @min: max number of threads the thread pool can contain
 + */
 +void aio_context_set_thread_pool_params(AioContext *ctx, int64_t min,
 +                                        int64_t max, Error **errp);
  #endif
 diff --git a/include/block/thread-pool.h b/include/block/thread-pool.h
 index XXXXXXX..XXXXXXX 100644
 --- a/include/block/thread-pool.h
 +++ b/include/block/thread-pool.h
@@ -XXX,XX +XXX,XX @@
  #include "block/block.h"
 +#define THREAD_POOL_MAX_THREADS_DEFAULT         64
 +
  typedef int ThreadPoolFunc(void *opaque);
  typedef struct ThreadPool ThreadPool;
@@ -XXX,XX +XXX,XX @@ BlockAIOCB *thread_pool_submit_aio(ThreadPool *pool,
  int coroutine_fn thread_pool_submit_co(ThreadPool *pool,
          ThreadPoolFunc *func, void *arg);
  void thread_pool_submit(ThreadPool *pool, ThreadPoolFunc *func, void *arg);
 +void thread_pool_update_params(ThreadPool *pool, struct AioContext *ctx);
  #endif
 diff --git a/include/sysemu/event-loop-base.h b/include/sysemu/event-loop-base.h
 index XXXXXXX..XXXXXXX 100644
 --- a/include/sysemu/event-loop-base.h
 +++ b/include/sysemu/event-loop-base.h
@@ -XXX,XX +XXX,XX @@ struct EventLoopBase {
      /* AioContext AIO engine parameters */
      int64_t aio_max_batch;
 +
 +    /* AioContext thread pool parameters */
 +    int64_t thread_pool_min;
 +    int64_t thread_pool_max;
  };
  #endif
 diff --git a/event-loop-base.c b/event-loop-base.c
 index XXXXXXX..XXXXXXX 100644
 --- a/event-loop-base.c
 +++ b/event-loop-base.c
@@ -XXX,XX +XXX,XX @@
  #include "qemu/osdep.h"
  #include "qom/object_interfaces.h"
  #include "qapi/error.h"
 +#include "block/thread-pool.h"
  #include "sysemu/event-loop-base.h"
  typedef struct {
@@ -XXX,XX +XXX,XX @@ typedef struct {
      ptrdiff_t offset; /* field's byte offset in EventLoopBase struct */
  } EventLoopBaseParamInfo;
 +static void event_loop_base_instance_init(Object *obj)
 +{
 +    EventLoopBase *base = EVENT_LOOP_BASE(obj);
 +
 +    base->thread_pool_max = THREAD_POOL_MAX_THREADS_DEFAULT;
 +}
 +
  static EventLoopBaseParamInfo aio_max_batch_info = {
      "aio-max-batch", offsetof(EventLoopBase, aio_max_batch),
  };
 +static EventLoopBaseParamInfo thread_pool_min_info = {
 +    "thread-pool-min", offsetof(EventLoopBase, thread_pool_min),
 +};
 +static EventLoopBaseParamInfo thread_pool_max_info = {
 +    "thread-pool-max", offsetof(EventLoopBase, thread_pool_max),
 +};
  static void event_loop_base_get_param(Object *obj, Visitor *v,
          const char *name, void *opaque, Error **errp)
@@ -XXX,XX +XXX,XX @@ static void event_loop_base_class_init(ObjectClass *klass, void *class_data)
                                event_loop_base_get_param,
                                event_loop_base_set_param,
                                NULL, &aio_max_batch_info);
 +    object_class_property_add(klass, "thread-pool-min", "int",
 +                              event_loop_base_get_param,
 +                              event_loop_base_set_param,
 +                              NULL, &thread_pool_min_info);
 +    object_class_property_add(klass, "thread-pool-max", "int",
 +                              event_loop_base_get_param,
 +                              event_loop_base_set_param,
 +                              NULL, &thread_pool_max_info);
  }
  static const TypeInfo event_loop_base_info = {
      .name = TYPE_EVENT_LOOP_BASE,
      .parent = TYPE_OBJECT,
      .instance_size = sizeof(EventLoopBase),
 +    .instance_init = event_loop_base_instance_init,
      .class_size = sizeof(EventLoopBaseClass),
      .class_init = event_loop_base_class_init,
      .abstract = true,
 diff --git a/iothread.c b/iothread.c
 index XXXXXXX..XXXXXXX 100644
 --- a/iothread.c
 +++ b/iothread.c
@@ -XXX,XX +XXX,XX @@ static void iothread_set_aio_context_params(EventLoopBase *base, Error **errp)
      aio_context_set_aio_params(iothread->ctx,
                                 iothread->parent_obj.aio_max_batch,
                                 errp);
 +
 +    aio_context_set_thread_pool_params(iothread->ctx, base->thread_pool_min,
 +                                       base->thread_pool_max, errp);
  }
 diff --git a/util/aio-posix.c b/util/aio-posix.c
 index XXXXXXX..XXXXXXX 100644
 --- a/util/aio-posix.c
 +++ b/util/aio-posix.c
@@ -XXX,XX +XXX,XX @@
  #include "qemu/osdep.h"
  #include "block/block.h"
 +#include "block/thread-pool.h"
  #include "qemu/main-loop.h"
  #include "qemu/rcu.h"
  #include "qemu/rcu_queue.h"
 diff --git a/util/async.c b/util/async.c
 index XXXXXXX..XXXXXXX 100644
 --- a/util/async.c
 +++ b/util/async.c
@@ -XXX,XX +XXX,XX @@ AioContext *aio_context_new(Error **errp)
      ctx->aio_max_batch = 0;
 +    ctx->thread_pool_min = 0;
 +    ctx->thread_pool_max = THREAD_POOL_MAX_THREADS_DEFAULT;
 +
      return ctx;
  fail:
      g_source_destroy(&ctx->source);
@@ -XXX,XX +XXX,XX @@ void qemu_set_current_aio_context(AioContext *ctx)
      assert(!get_my_aiocontext());
      set_my_aiocontext(ctx);
  }
 +
 +void aio_context_set_thread_pool_params(AioContext *ctx, int64_t min,
 +                                        int64_t max, Error **errp)
 +{
 +
 +    if (min > max || !max || min > INT_MAX || max > INT_MAX) {
 +        error_setg(errp, "bad thread-pool-min/thread-pool-max values");
 +        return;
 +    }
 +
 +    ctx->thread_pool_min = min;
 +    ctx->thread_pool_max = max;
 +
 +    if (ctx->thread_pool) {
 +        thread_pool_update_params(ctx->thread_pool, ctx);
 +    }
 +}
 diff --git a/util/main-loop.c b/util/main-loop.c
 index XXXXXXX..XXXXXXX 100644
 --- a/util/main-loop.c
 +++ b/util/main-loop.c
@@ -XXX,XX +XXX,XX @@
  #include "sysemu/replay.h"
  #include "qemu/main-loop.h"
  #include "block/aio.h"
 +#include "block/thread-pool.h"
  #include "qemu/error-report.h"
  #include "qemu/queue.h"
  #include "qemu/compiler.h"
@@ -XXX,XX +XXX,XX @@ int qemu_init_main_loop(Error **errp)
  static void main_loop_update_params(EventLoopBase *base, Error **errp)
  {
-     BDRVQcow2State *s = bs->opaque;
++    ERRP_GUARD();
-@@ -XXX,XX +XXX,XX @@ static int rebuild_refcounts_write_refblocks(
++
-                                                   nb_clusters,
+     if (!qemu_aio_context) {
-                                                   &first_free_cluster);
+         error_setg(errp, "qemu aio context not ready");
-             if (refblock_offset < 0) {
+         return;
--                fprintf(stderr, "ERROR allocating refblock: %s\n",
+     }
--                        strerror(-refblock_offset));
-+                error_setg_errno(errp, -refblock_offset,
+     aio_context_set_aio_params(qemu_aio_context, base->aio_max_batch, errp);
-+                                 "ERROR allocating refblock");
++    if (*errp) {
-                 return refblock_offset;
++        return;
-             }
++    }
++
-@@ -XXX,XX +XXX,XX @@ static int rebuild_refcounts_write_refblocks(
++    aio_context_set_thread_pool_params(qemu_aio_context, base->thread_pool_min,
-                                   on_disk_reftable_entries *
++                                       base->thread_pool_max, errp);
-                                   REFTABLE_ENTRY_SIZE);
+ }
-                 if (!on_disk_reftable) {
-+                    error_setg(errp, "ERROR allocating reftable memory");
+ MainLoop *mloop;
-                     return -ENOMEM;
+diff --git a/util/thread-pool.c b/util/thread-pool.c
-                 }
+index XXXXXXX..XXXXXXX 100644
+--- a/util/thread-pool.c
-@@ -XXX,XX +XXX,XX @@ static int rebuild_refcounts_write_refblocks(
++++ b/util/thread-pool.c
-         ret = qcow2_pre_write_overlap_check(bs, 0, refblock_offset,
+@@ -XXX,XX +XXX,XX @@ struct ThreadPool {
-                                             s->cluster_size, false);
+     QemuMutex lock;
-         if (ret < 0) {
+     QemuCond worker_stopped;
--            fprintf(stderr, "ERROR writing refblock: %s\n", strerror(-ret));
+     QemuSemaphore sem;
-+            error_setg_errno(errp, -ret, "ERROR writing refblock");
+-    int max_threads;
-             return ret;
+     QEMUBH *new_thread_bh;
      /* The following variables are only accessed from one AioContext. */
@@ -XXX,XX +XXX,XX @@ struct ThreadPool {
      int new_threads;     /* backlog of threads we need to create */
      int pending_threads; /* threads created but not running yet */
      bool stopping;
 +    int min_threads;
 +    int max_threads;
  };
 +static inline bool back_to_sleep(ThreadPool *pool, int ret)
 +{
 +    /*
 +     * The semaphore timed out, we should exit the loop except when:
 +     *  - There is work to do, we raced with the signal.
 +     *  - The max threads threshold just changed, we raced with the signal.
 +     *  - The thread pool forces a minimum number of readily available threads.
 +     */
 +    if (ret == -1 && (!QTAILQ_EMPTY(&pool->request_list) ||
 +            pool->cur_threads > pool->max_threads ||
 +            pool->cur_threads <= pool->min_threads)) {
 +            return true;
 +    }
 +
 +    return false;
 +}
 +
  static void *worker_thread(void *opaque)
  {
      ThreadPool *pool = opaque;
@@ -XXX,XX +XXX,XX @@ static void *worker_thread(void *opaque)
              ret = qemu_sem_timedwait(&pool->sem, 10000);
              qemu_mutex_lock(&pool->lock);
              pool->idle_threads--;
 -        } while (ret == -1 && !QTAILQ_EMPTY(&pool->request_list));
 -        if (ret == -1 || pool->stopping) {
 +        } while (back_to_sleep(pool, ret));
 +        if (ret == -1 || pool->stopping ||
 +            pool->cur_threads > pool->max_threads) {
              break;
          }
-@@ -XXX,XX +XXX,XX @@ static int rebuild_refcounts_write_refblocks(
+@@ -XXX,XX +XXX,XX @@ void thread_pool_submit(ThreadPool *pool, ThreadPoolFunc *func, void *arg)
-         ret = bdrv_pwrite(bs->file, refblock_offset, on_disk_refblock,
+     thread_pool_submit_aio(pool, func, arg, NULL, NULL);
-                           s->cluster_size);
+ }
-         if (ret < 0) {
--            fprintf(stderr, "ERROR writing refblock: %s\n", strerror(-ret));
++void thread_pool_update_params(ThreadPool *pool, AioContext *ctx)
-+            error_setg_errno(errp, -ret, "ERROR writing refblock");
++{
-             return ret;
++    qemu_mutex_lock(&pool->lock);
-         }
++
++    pool->min_threads = ctx->thread_pool_min;
-@@ -XXX,XX +XXX,XX @@ static int rebuild_refcounts_write_refblocks(
++    pool->max_threads = ctx->thread_pool_max;
- static int rebuild_refcount_structure(BlockDriverState *bs,
++
-                                       BdrvCheckResult *res,
++    /*
-                                       void **refcount_table,
++     * We either have to:
--                                      int64_t *nb_clusters)
++     *  - Increase the number available of threads until over the min_threads
-+                                      int64_t *nb_clusters,
++     *    threshold.
-+                                      Error **errp)
++     *  - Decrease the number of available threads until under the max_threads
 +     *    threshold.
 +     *  - Do nothing. The current number of threads fall in between the min and
 +     *    max thresholds. We'll let the pool manage itself.
 +     */
 +    for (int i = pool->cur_threads; i < pool->min_threads; i++) {
 +        spawn_thread(pool);
 +    }
 +
 +    for (int i = pool->cur_threads; i > pool->max_threads; i--) {
 +        qemu_sem_post(&pool->sem);
 +    }
 +
 +    qemu_mutex_unlock(&pool->lock);
 +}
 +
  static void thread_pool_init_one(ThreadPool *pool, AioContext *ctx)
  {
-     BDRVQcow2State *s = bs->opaque;
+     if (!ctx) {
-     int64_t reftable_offset = -1;
+@@ -XXX,XX +XXX,XX @@ static void thread_pool_init_one(ThreadPool *pool, AioContext *ctx)
-@@ -XXX,XX +XXX,XX @@ static int rebuild_refcount_structure(BlockDriverState *bs,
+     qemu_mutex_init(&pool->lock);
-         rebuild_refcounts_write_refblocks(bs, refcount_table, nb_clusters,
+     qemu_cond_init(&pool->worker_stopped);
-, *nb_clusters,
+     qemu_sem_init(&pool->sem, 0);
-                                           &on_disk_reftable,
+-    pool->max_threads = 64;
--                                          &on_disk_reftable_entries);
+     pool->new_thread_bh = aio_bh_new(ctx, spawn_thread_bh_fn, pool);
-+                                          &on_disk_reftable_entries, errp);
-     if (reftable_size_changed < 0) {
+     QLIST_INIT(&pool->head);
-         res->check_errors++;
+     QTAILQ_INIT(&pool->request_list);
-         ret = reftable_size_changed;
++
-@@ -XXX,XX +XXX,XX @@ static int rebuild_refcount_structure(BlockDriverState *bs,
++    thread_pool_update_params(pool, ctx);
-                                               refcount_table, nb_clusters,
+ }
-                                               &first_free_cluster);
-         if (reftable_offset < 0) {
+ ThreadPool *thread_pool_new(AioContext *ctx)
 -            fprintf(stderr, "ERROR allocating reftable: %s\n",
 -                    strerror(-reftable_offset));
 +            error_setg_errno(errp, -reftable_offset,
 +                             "ERROR allocating reftable");
              res->check_errors++;
              ret = reftable_offset;
              goto fail;
@@ -XXX,XX +XXX,XX @@ static int rebuild_refcount_structure(BlockDriverState *bs,
                                                reftable_start_cluster,
                                                reftable_end_cluster,
                                                &on_disk_reftable,
 -                                              &on_disk_reftable_entries);
 +                                              &on_disk_reftable_entries, errp);
          if (reftable_size_changed < 0) {
              res->check_errors++;
              ret = reftable_size_changed;
@@ -XXX,XX +XXX,XX @@ static int rebuild_refcount_structure(BlockDriverState *bs,
      ret = qcow2_pre_write_overlap_check(bs, 0, reftable_offset, reftable_length,
                                          false);
      if (ret < 0) {
 -        fprintf(stderr, "ERROR writing reftable: %s\n", strerror(-ret));
 +        error_setg_errno(errp, -ret, "ERROR writing reftable");
          goto fail;
      }
@@ -XXX,XX +XXX,XX @@ static int rebuild_refcount_structure(BlockDriverState *bs,
      ret = bdrv_pwrite(bs->file, reftable_offset, on_disk_reftable,
                        reftable_length);
      if (ret < 0) {
 -        fprintf(stderr, "ERROR writing reftable: %s\n", strerror(-ret));
 +        error_setg_errno(errp, -ret, "ERROR writing reftable");
          goto fail;
      }
@@ -XXX,XX +XXX,XX @@ static int rebuild_refcount_structure(BlockDriverState *bs,
                             &reftable_offset_and_clusters,
                             sizeof(reftable_offset_and_clusters));
      if (ret < 0) {
 -        fprintf(stderr, "ERROR setting reftable: %s\n", strerror(-ret));
 +        error_setg_errno(errp, -ret, "ERROR setting reftable");
          goto fail;
      }
@@ -XXX,XX +XXX,XX @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
      if (rebuild && (fix & BDRV_FIX_ERRORS)) {
          BdrvCheckResult old_res = *res;
          int fresh_leaks = 0;
 +        Error *local_err = NULL;
          fprintf(stderr, "Rebuilding refcount structure\n");
          ret = rebuild_refcount_structure(bs, res, &refcount_table,
 -                                         &nb_clusters);
 +                                         &nb_clusters, &local_err);
          if (ret < 0) {
 +            error_report_err(local_err);
              goto fail;
          }
 --
 .35.1

The following changes since commit 1be5a765c08cee3a9587c8a8d3fc2ea247b13f9c:

Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging (2022-04-19 18:22:16 -0700)

are available in the Git repository at:

https://gitlab.com/hreitz/qemu.git tags/pull-block-2022-04-20

for you to fetch changes up to 0423f75351ab83b844a31349218b0eadd830e07a:

qcow2: Add errp to rebuild_refcount_structure() (2022-04-20 12:09:17 +0200)

----------------------------------------------------------------
Block patches:
- Some changes for qcow2's refcount repair algorithm to make it work for
  qcow2 images stored on block devices
- Skip test cases that require zstd when support for it is missing
- Some refactoring in the iotests' meson.build

----------------------------------------------------------------
Hanna Reitz (6):
  iotests.py: Add supports_qcow2_zstd_compression()
  iotests/065: Check for zstd support
  iotests/303: Check for zstd support
  qcow2: Improve refcount structure rebuilding
  iotests/108: Test new refcount rebuild algorithm
  qcow2: Add errp to rebuild_refcount_structure()

Thomas Huth (2):
  tests/qemu-iotests/meson.build: Improve the indentation
  tests/qemu-iotests: Move the bash and sanitizer checks to meson.build

-- 
2.35.1

From: Thomas Huth <thuth@redhat.com>

By using subdir_done(), we can get rid of one level of indentation
in this file. This will make it easier to add more conditions to
skip the iotests in future patches.

Reviewed-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20220223093840.2515281-3-thuth@redhat.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
---
 tests/qemu-iotests/meson.build | 61 ++++++++++++++++++----------------
 1 file changed, 32 insertions(+), 29 deletions(-)

diff --git a/tests/qemu-iotests/meson.build b/tests/qemu-iotests/meson.build
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/meson.build
+++ b/tests/qemu-iotests/meson.build
@@ -XXX,XX +XXX,XX @@
-if have_tools and targetos != 'windows' and not get_option('gprof')
-  qemu_iotests_binaries = [qemu_img, qemu_io, qemu_nbd, qsd]
-  qemu_iotests_env = {'PYTHON': python.full_path()}
-  qemu_iotests_formats = {
-    'qcow2': 'quick',
-    'raw': 'slow',
-    'qed': 'thorough',
-    'vmdk': 'thorough',
-    'vpc': 'thorough'
-  }
-
-  foreach k, v : emulators
-    if k.startswith('qemu-system-')
-      qemu_iotests_binaries += v
-    endif
-  endforeach
-  foreach format, speed: qemu_iotests_formats
-    if speed == 'quick'
-      suites = 'block'
-    else
-      suites = ['block-' + speed, speed]
-    endif
-    test('qemu-iotests ' + format, sh, args: [files('../check-block.sh'), format],
-         depends: qemu_iotests_binaries, env: qemu_iotests_env,
-         protocol: 'tap',
-         suite: suites,
-         timeout: 0,
-         is_parallel: false)
-  endforeach
+if not have_tools or targetos == 'windows' or get_option('gprof')
+  subdir_done()
 endif
+
+qemu_iotests_binaries = [qemu_img, qemu_io, qemu_nbd, qsd]
+qemu_iotests_env = {'PYTHON': python.full_path()}
+qemu_iotests_formats = {
+  'qcow2': 'quick',
+  'raw': 'slow',
+  'qed': 'thorough',
+  'vmdk': 'thorough',
+  'vpc': 'thorough'
+}
+
+foreach k, v : emulators
+  if k.startswith('qemu-system-')
+    qemu_iotests_binaries += v
+  endif
+endforeach
+
+foreach format, speed: qemu_iotests_formats
+  if speed == 'quick'
+    suites = 'block'
+  else
+    suites = ['block-' + speed, speed]
+  endif
+  test('qemu-iotests ' + format, sh, args: [files('../check-block.sh'), format],
+       depends: qemu_iotests_binaries, env: qemu_iotests_env,
+       protocol: 'tap',
+       suite: suites,
+       timeout: 0,
+       is_parallel: false)
+endforeach
-- 
2.35.1

From: Thomas Huth <thuth@redhat.com>

We want to get rid of check-block.sh in the long run, so let's move
the checks for the bash version and sanitizers from check-block.sh
into the meson.build file instead.

Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20220223093840.2515281-4-thuth@redhat.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
---
 tests/check-block.sh           | 26 --------------------------
 tests/qemu-iotests/meson.build | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 26 deletions(-)

diff --git a/tests/check-block.sh b/tests/check-block.sh
index XXXXXXX..XXXXXXX 100755
--- a/tests/check-block.sh
+++ b/tests/check-block.sh
@@ -XXX,XX +XXX,XX @@ skip() {
     exit 0
 }
 
-# Disable tests with any sanitizer except for specific ones
-SANITIZE_FLAGS=$( grep "CFLAGS.*-fsanitize" config-host.mak 2>/dev/null )
-ALLOWED_SANITIZE_FLAGS="safe-stack cfi-icall"
-#Remove all occurrencies of allowed Sanitize flags
-for j in ${ALLOWED_SANITIZE_FLAGS}; do
-    TMP_FLAGS=${SANITIZE_FLAGS}
-    SANITIZE_FLAGS=""
-    for i in ${TMP_FLAGS}; do
-        if ! echo ${i} | grep -q "${j}" 2>/dev/null; then
-            SANITIZE_FLAGS="${SANITIZE_FLAGS} ${i}"
-        fi
-    done
-done
-if echo ${SANITIZE_FLAGS} | grep -q "\-fsanitize" 2>/dev/null; then
-    # Have a sanitize flag that is not allowed, stop
-    skip "Sanitizers are enabled ==> Not running the qemu-iotests."
-fi
-
 if [ -z "$(find . -name 'qemu-system-*' -print)" ]; then
     skip "No qemu-system binary available ==> Not running the qemu-iotests."
 fi
 
-if ! command -v bash >/dev/null 2>&1 ; then
-    skip "bash not available ==> Not running the qemu-iotests."
-fi
-
-if LANG=C bash --version | grep -q 'GNU bash, version [123]' ; then
-    skip "bash version too old ==> Not running the qemu-iotests."
-fi
-
 cd tests/qemu-iotests
 
 # QEMU_CHECK_BLOCK_AUTO is used to disable some unstable sub-tests
diff --git a/tests/qemu-iotests/meson.build b/tests/qemu-iotests/meson.build
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/meson.build
+++ b/tests/qemu-iotests/meson.build
@@ -XXX,XX +XXX,XX @@ if not have_tools or targetos == 'windows' or get_option('gprof')
   subdir_done()
 endif
 
+foreach cflag: config_host['QEMU_CFLAGS'].split()
+  if cflag.startswith('-fsanitize') and \
+     not cflag.contains('safe-stack') and not cflag.contains('cfi-icall')
+    message('Sanitizers are enabled ==> Disabled the qemu-iotests.')
+    subdir_done()
+  endif
+endforeach
+
+bash = find_program('bash', required: false, version: '>= 4.0')
+if not bash.found()
+  message('bash >= v4.0 not available ==> Disabled the qemu-iotests.')
+  subdir_done()
+endif
+
 qemu_iotests_binaries = [qemu_img, qemu_io, qemu_nbd, qsd]
 qemu_iotests_env = {'PYTHON': python.full_path()}
 qemu_iotests_formats = {
-- 
2.35.1

Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20220323105522.53660-2-hreitz@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <v.sementsov-og@mail.ru>
---
 tests/qemu-iotests/iotests.py | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -XXX,XX +XXX,XX @@ def verify_working_luks():
     if not working:
         notrun(reason)
 
+def supports_qcow2_zstd_compression() -> bool:
+    img_file = f'{test_dir}/qcow2-zstd-test.qcow2'
+    res = qemu_img('create', '-f', 'qcow2', '-o', 'compression_type=zstd',
+                   img_file, '0',
+                   check=False)
+    try:
+        os.remove(img_file)
+    except OSError:
+        pass
+
+    if res.returncode == 1 and \
+            "'compression-type' does not accept value 'zstd'" in res.stdout:
+        return False
+    else:
+        return True
+
+def verify_qcow2_zstd_compression():
+    if not supports_qcow2_zstd_compression():
+        notrun('zstd compression not supported')
+
 def qemu_pipe(*args: str) -> str:
     """
     Run qemu with an option to print something and exit (e.g. a help option).
-- 
2.35.1

Some test cases run in iotest 065 want to run with zstd compression just
for added coverage.  Run them with zlib if there is no zstd support
compiled in.

Reported-by: Thomas Huth <thuth@redhat.com>
Fixes: 12a936171d71f839dc907ff ("iotest 065: explicit compression type")
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20220323105522.53660-3-hreitz@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <v.sementsov-og@mail.ru>
---
 tests/qemu-iotests/065 | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/065 b/tests/qemu-iotests/065
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/065
+++ b/tests/qemu-iotests/065
@@ -XXX,XX +XXX,XX @@ import os
 import re
 import json
 import iotests
-from iotests import qemu_img, qemu_img_info
+from iotests import qemu_img, qemu_img_info, supports_qcow2_zstd_compression
 import unittest
 
 test_img = os.path.join(iotests.test_dir, 'test.img')
@@ -XXX,XX +XXX,XX @@ class TestQCow2(TestQemuImgInfo):
 
 class TestQCow3NotLazy(TestQemuImgInfo):
     '''Testing a qcow2 version 3 image with lazy refcounts disabled'''
-    img_options = 'compat=1.1,lazy_refcounts=off,compression_type=zstd'
+    if supports_qcow2_zstd_compression():
+        compression_type = 'zstd'
+    else:
+        compression_type = 'zlib'
+
+    img_options = 'compat=1.1,lazy_refcounts=off'
+    img_options += f',compression_type={compression_type}'
     json_compare = { 'compat': '1.1', 'lazy-refcounts': False,
                      'refcount-bits': 16, 'corrupt': False,
-                     'compression-type': 'zstd', 'extended-l2': False }
-    human_compare = [ 'compat: 1.1', 'compression type: zstd',
+                     'compression-type': compression_type, 'extended-l2': False }
+    human_compare = [ 'compat: 1.1', f'compression type: {compression_type}',
                       'lazy refcounts: false', 'refcount bits: 16',
                       'corrupt: false', 'extended l2: false' ]
 
@@ -XXX,XX +XXX,XX @@ class TestQCow3NotLazyQMP(TestQMP):
 class TestQCow3LazyQMP(TestQMP):
     '''Testing a qcow2 version 3 image with lazy refcounts enabled, opening
        with lazy refcounts disabled'''
-    img_options = 'compat=1.1,lazy_refcounts=on,compression_type=zstd'
+    if supports_qcow2_zstd_compression():
+        compression_type = 'zstd'
+    else:
+        compression_type = 'zlib'
+
+    img_options = 'compat=1.1,lazy_refcounts=on'
+    img_options += f',compression_type={compression_type}'
     qemu_options = 'lazy-refcounts=off'
     compare = { 'compat': '1.1', 'lazy-refcounts': True,
                 'refcount-bits': 16, 'corrupt': False,
-                'compression-type': 'zstd', 'extended-l2': False }
+                'compression-type': compression_type, 'extended-l2': False }
 
 TestImageInfoSpecific = None
 TestQemuImgInfo = None
-- 
2.35.1

303 runs two test cases, one of which requires zstd support.
Unfortunately, given that this is not a unittest-style test, we cannot
easily skip that single case, and instead can only skip the whole test.

(Alternatively, we could split this test into a zlib and a zstd part,
but that seems excessive, given that this test is not in auto and thus
likely only run by developers who have zstd support compiled in.)

Fixes: 677e0bae686e7c670a71d1f ("iotest 303: explicit compression type")
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <v.sementsov-og@mail.ru>
Message-Id: <20220323105522.53660-4-hreitz@redhat.com>
---
 tests/qemu-iotests/303 | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/303 b/tests/qemu-iotests/303
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/303
+++ b/tests/qemu-iotests/303
@@ -XXX,XX +XXX,XX @@
 
 import iotests
 import subprocess
-from iotests import qemu_img_create, qemu_io, file_path, log, filter_qemu_io
+from iotests import qemu_img_create, qemu_io, file_path, log, filter_qemu_io, \
+        verify_qcow2_zstd_compression
 
 iotests.script_initialize(supported_fmts=['qcow2'],
                           unsupported_imgopts=['refcount_bits', 'compat'])
+verify_qcow2_zstd_compression()
 
 disk = file_path('disk')
 chunk = 1024 * 1024
-- 
2.35.1

When rebuilding the refcount structures (when qemu-img check -r found
errors with refcount = 0, but reference count > 0), the new refcount
table defaults to being put at the image file end[1].  There is no good
reason for that except that it means we will not have to rewrite any
refblocks we already wrote to disk.

Changing the code to rewrite those refblocks is not too difficult,
though, so let us do that.  That is beneficial for images on block
devices, where we cannot really write beyond the end of the image file.

Use this opportunity to add extensive comments to the code, and refactor
it a bit, getting rid of the backwards-jumping goto.

[1] Unless there is something allocated in the area pointed to by the
    last refblock, so we have to write that refblock.  In that case, we
    try to put the reftable in there.

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1519071
Closes: https://gitlab.com/qemu-project/qemu/-/issues/941
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20220405134652.19278-2-hreitz@redhat.com>
---
 block/qcow2-refcount.c | 332 +++++++++++++++++++++++++++++------------
 1 file changed, 235 insertions(+), 97 deletions(-)

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 @@ static int64_t alloc_clusters_imrt(BlockDriverState *bs,
 }
 
 /*
- * Creates a new refcount structure based solely on the in-memory information
- * given through *refcount_table. All necessary allocations will be reflected
- * in that array.
+ * Helper function for rebuild_refcount_structure().
  *
- * On success, the old refcount structure is leaked (it will be covered by the
- * new refcount structure).
+ * Scan the range of clusters [first_cluster, end_cluster) for allocated
+ * clusters and write all corresponding refblocks to disk.  The refblock
+ * and allocation data is taken from the in-memory refcount table
+ * *refcount_table[] (of size *nb_clusters), which is basically one big
+ * (unlimited size) refblock for the whole image.
+ *
+ * For these refblocks, clusters are allocated using said in-memory
+ * refcount table.  Care is taken that these allocations are reflected
+ * in the refblocks written to disk.
+ *
+ * The refblocks' offsets are written into a reftable, which is
+ * *on_disk_reftable_ptr[] (of size *on_disk_reftable_entries_ptr).  If
+ * that reftable is of insufficient size, it will be resized to fit.
+ * This reftable is not written to disk.
+ *
+ * (If *on_disk_reftable_ptr is not NULL, the entries within are assumed
+ * to point to existing valid refblocks that do not need to be allocated
+ * again.)
+ *
+ * Return whether the on-disk reftable array was resized (true/false),
+ * or -errno on error.
  */
-static int rebuild_refcount_structure(BlockDriverState *bs,
-                                      BdrvCheckResult *res,
-                                      void **refcount_table,
-                                      int64_t *nb_clusters)
+static int rebuild_refcounts_write_refblocks(
+        BlockDriverState *bs, void **refcount_table, int64_t *nb_clusters,
+        int64_t first_cluster, int64_t end_cluster,
+        uint64_t **on_disk_reftable_ptr, uint32_t *on_disk_reftable_entries_ptr
+    )
 {
     BDRVQcow2State *s = bs->opaque;
-    int64_t first_free_cluster = 0, reftable_offset = -1, cluster = 0;
+    int64_t cluster;
     int64_t refblock_offset, refblock_start, refblock_index;
-    uint32_t reftable_size = 0;
-    uint64_t *on_disk_reftable = NULL;
+    int64_t first_free_cluster = 0;
+    uint64_t *on_disk_reftable = *on_disk_reftable_ptr;
+    uint32_t on_disk_reftable_entries = *on_disk_reftable_entries_ptr;
     void *on_disk_refblock;
-    int ret = 0;
-    struct {
-        uint64_t reftable_offset;
-        uint32_t reftable_clusters;
-    } QEMU_PACKED reftable_offset_and_clusters;
-
-    qcow2_cache_empty(bs, s->refcount_block_cache);
+    bool reftable_grown = false;
+    int ret;
 
-write_refblocks:
-    for (; cluster < *nb_clusters; cluster++) {
+    for (cluster = first_cluster; cluster < end_cluster; cluster++) {
+        /* Check all clusters to find refblocks that contain non-zero entries */
         if (!s->get_refcount(*refcount_table, cluster)) {
             continue;
         }
 
+        /*
+         * This cluster is allocated, so we need to create a refblock
+         * for it.  The data we will write to disk is just the
+         * respective slice from *refcount_table, so it will contain
+         * accurate refcounts for all clusters belonging to this
+         * refblock.  After we have written it, we will therefore skip
+         * all remaining clusters in this refblock.
+         */
+
         refblock_index = cluster >> s->refcount_block_bits;
         refblock_start = refblock_index << s->refcount_block_bits;
 
-        /* Don't allocate a cluster in a refblock already written to disk */
-        if (first_free_cluster < refblock_start) {
-            first_free_cluster = refblock_start;
-        }
-        refblock_offset = alloc_clusters_imrt(bs, 1, refcount_table,
-                                              nb_clusters, &first_free_cluster);
-        if (refblock_offset < 0) {
-            fprintf(stderr, "ERROR allocating refblock: %s\n",
-                    strerror(-refblock_offset));
-            res->check_errors++;
-            ret = refblock_offset;
-            goto fail;
-        }
+        if (on_disk_reftable_entries > refblock_index &&
+            on_disk_reftable[refblock_index])
+        {
+            /*
+             * We can get here after a `goto write_refblocks`: We have a
+             * reftable from a previous run, and the refblock is already
+             * allocated.  No need to allocate it again.
+             */
+            refblock_offset = on_disk_reftable[refblock_index];
+        } else {
+            int64_t refblock_cluster_index;
 
-        if (reftable_size <= refblock_index) {
-            uint32_t old_reftable_size = reftable_size;
-            uint64_t *new_on_disk_reftable;
+            /* Don't allocate a cluster in a refblock already written to disk */
+            if (first_free_cluster < refblock_start) {
+                first_free_cluster = refblock_start;
+            }
+            refblock_offset = alloc_clusters_imrt(bs, 1, refcount_table,
+                                                  nb_clusters,
+                                                  &first_free_cluster);
+            if (refblock_offset < 0) {
+                fprintf(stderr, "ERROR allocating refblock: %s\n",
+                        strerror(-refblock_offset));
+                return refblock_offset;
+            }
 
-            reftable_size = ROUND_UP((refblock_index + 1) * REFTABLE_ENTRY_SIZE,
-                                     s->cluster_size) / REFTABLE_ENTRY_SIZE;
-            new_on_disk_reftable = g_try_realloc(on_disk_reftable,
-                                                 reftable_size *
-                                                 REFTABLE_ENTRY_SIZE);
-            if (!new_on_disk_reftable) {
-                res->check_errors++;
-                ret = -ENOMEM;
-                goto fail;
+            refblock_cluster_index = refblock_offset / s->cluster_size;
+            if (refblock_cluster_index >= end_cluster) {
+                /*
+                 * We must write the refblock that holds this refblock's
+                 * refcount
+                 */
+                end_cluster = refblock_cluster_index + 1;
             }
-            on_disk_reftable = new_on_disk_reftable;
 
-            memset(on_disk_reftable + old_reftable_size, 0,
-                   (reftable_size - old_reftable_size) * REFTABLE_ENTRY_SIZE);
+            if (on_disk_reftable_entries <= refblock_index) {
+                on_disk_reftable_entries =
+                    ROUND_UP((refblock_index + 1) * REFTABLE_ENTRY_SIZE,
+                             s->cluster_size) / REFTABLE_ENTRY_SIZE;
+                on_disk_reftable =
+                    g_try_realloc(on_disk_reftable,
+                                  on_disk_reftable_entries *
+                                  REFTABLE_ENTRY_SIZE);
+                if (!on_disk_reftable) {
+                    return -ENOMEM;
+                }
 
-            /* The offset we have for the reftable is now no longer valid;
-             * this will leak that range, but we can easily fix that by running
-             * a leak-fixing check after this rebuild operation */
-            reftable_offset = -1;
-        } else {
-            assert(on_disk_reftable);
-        }
-        on_disk_reftable[refblock_index] = refblock_offset;
+                memset(on_disk_reftable + *on_disk_reftable_entries_ptr, 0,
+                       (on_disk_reftable_entries -
+                        *on_disk_reftable_entries_ptr) *
+                       REFTABLE_ENTRY_SIZE);
 
-        /* If this is apparently the last refblock (for now), try to squeeze the
-         * reftable in */
-        if (refblock_index == (*nb_clusters - 1) >> s->refcount_block_bits &&
-            reftable_offset < 0)
-        {
-            uint64_t reftable_clusters = size_to_clusters(s, reftable_size *
-                                                          REFTABLE_ENTRY_SIZE);
-            reftable_offset = alloc_clusters_imrt(bs, reftable_clusters,
-                                                  refcount_table, nb_clusters,
-                                                  &first_free_cluster);
-            if (reftable_offset < 0) {
-                fprintf(stderr, "ERROR allocating reftable: %s\n",
-                        strerror(-reftable_offset));
-                res->check_errors++;
-                ret = reftable_offset;
-                goto fail;
+                *on_disk_reftable_ptr = on_disk_reftable;
+                *on_disk_reftable_entries_ptr = on_disk_reftable_entries;
+
+                reftable_grown = true;
+            } else {
+                assert(on_disk_reftable);
             }
+            on_disk_reftable[refblock_index] = refblock_offset;
         }
 
+        /* Refblock is allocated, write it to disk */
+
         ret = qcow2_pre_write_overlap_check(bs, 0, refblock_offset,
                                             s->cluster_size, false);
         if (ret < 0) {
             fprintf(stderr, "ERROR writing refblock: %s\n", strerror(-ret));
-            goto fail;
+            return ret;
         }
 
-        /* The size of *refcount_table is always cluster-aligned, therefore the
-         * write operation will not overflow */
+        /*
+         * The refblock is simply a slice of *refcount_table.
+         * Note that the size of *refcount_table is always aligned to
+         * whole clusters, so the write operation will not result in
+         * out-of-bounds accesses.
+         */
         on_disk_refblock = (void *)((char *) *refcount_table +
                                     refblock_index * s->cluster_size);
 
@@ -XXX,XX +XXX,XX @@ write_refblocks:
                           s->cluster_size);
         if (ret < 0) {
             fprintf(stderr, "ERROR writing refblock: %s\n", strerror(-ret));
-            goto fail;
+            return ret;
         }
 
-        /* Go to the end of this refblock */
+        /* This refblock is done, skip to its end */
         cluster = refblock_start + s->refcount_block_size - 1;
     }
 
-    if (reftable_offset < 0) {
-        uint64_t post_refblock_start, reftable_clusters;
+    return reftable_grown;
+}
+
+/*
+ * Creates a new refcount structure based solely on the in-memory information
+ * given through *refcount_table (this in-memory information is basically just
+ * the concatenation of all refblocks).  All necessary allocations will be
+ * reflected in that array.
+ *
+ * On success, the old refcount structure is leaked (it will be covered by the
+ * new refcount structure).
+ */
+static int rebuild_refcount_structure(BlockDriverState *bs,
+                                      BdrvCheckResult *res,
+                                      void **refcount_table,
+                                      int64_t *nb_clusters)
+{
+    BDRVQcow2State *s = bs->opaque;
+    int64_t reftable_offset = -1;
+    int64_t reftable_length = 0;
+    int64_t reftable_clusters;
+    int64_t refblock_index;
+    uint32_t on_disk_reftable_entries = 0;
+    uint64_t *on_disk_reftable = NULL;
+    int ret = 0;
+    int reftable_size_changed = 0;
+    struct {
+        uint64_t reftable_offset;
+        uint32_t reftable_clusters;
+    } QEMU_PACKED reftable_offset_and_clusters;
+
+    qcow2_cache_empty(bs, s->refcount_block_cache);
+
+    /*
+     * For each refblock containing entries, we try to allocate a
+     * cluster (in the in-memory refcount table) and write its offset
+     * into on_disk_reftable[].  We then write the whole refblock to
+     * disk (as a slice of the in-memory refcount table).
+     * This is done by rebuild_refcounts_write_refblocks().
+     *
+     * Once we have scanned all clusters, we try to find space for the
+     * reftable.  This will dirty the in-memory refcount table (i.e.
+     * make it differ from the refblocks we have already written), so we
+     * need to run rebuild_refcounts_write_refblocks() again for the
+     * range of clusters where the reftable has been allocated.
+     *
+     * This second run might make the reftable grow again, in which case
+     * we will need to allocate another space for it, which is why we
+     * repeat all this until the reftable stops growing.
+     *
+     * (This loop will terminate, because with every cluster the
+     * reftable grows, it can accomodate a multitude of more refcounts,
+     * so that at some point this must be able to cover the reftable
+     * and all refblocks describing it.)
+     *
+     * We then convert the reftable to big-endian and write it to disk.
+     *
+     * Note that we never free any reftable allocations.  Doing so would
+     * needlessly complicate the algorithm: The eventual second check
+     * run we do will clean up all leaks we have caused.
+     */
+
+    reftable_size_changed =
+        rebuild_refcounts_write_refblocks(bs, refcount_table, nb_clusters,
+                                          0, *nb_clusters,
+                                          &on_disk_reftable,
+                                          &on_disk_reftable_entries);
+    if (reftable_size_changed < 0) {
+        res->check_errors++;
+        ret = reftable_size_changed;
+        goto fail;
+    }
+
+    /*
+     * There was no reftable before, so rebuild_refcounts_write_refblocks()
+     * must have increased its size (from 0 to something).
+     */
+    assert(reftable_size_changed);
+
+    do {
+        int64_t reftable_start_cluster, reftable_end_cluster;
+        int64_t first_free_cluster = 0;
+
+        reftable_length = on_disk_reftable_entries * REFTABLE_ENTRY_SIZE;
+        reftable_clusters = size_to_clusters(s, reftable_length);
 
-        post_refblock_start = ROUND_UP(*nb_clusters, s->refcount_block_size);
-        reftable_clusters =
-            size_to_clusters(s, reftable_size * REFTABLE_ENTRY_SIZE);
-        /* Not pretty but simple */
-        if (first_free_cluster < post_refblock_start) {
-            first_free_cluster = post_refblock_start;
-        }
         reftable_offset = alloc_clusters_imrt(bs, reftable_clusters,
                                               refcount_table, nb_clusters,
                                               &first_free_cluster);
@@ -XXX,XX +XXX,XX @@ write_refblocks:
             goto fail;
         }
 
-        goto write_refblocks;
-    }
+        /*
+         * We need to update the affected refblocks, so re-run the
+         * write_refblocks loop for the reftable's range of clusters.
+         */
+        assert(offset_into_cluster(s, reftable_offset) == 0);
+        reftable_start_cluster = reftable_offset / s->cluster_size;
+        reftable_end_cluster = reftable_start_cluster + reftable_clusters;
+        reftable_size_changed =
+            rebuild_refcounts_write_refblocks(bs, refcount_table, nb_clusters,
+                                              reftable_start_cluster,
+                                              reftable_end_cluster,
+                                              &on_disk_reftable,
+                                              &on_disk_reftable_entries);
+        if (reftable_size_changed < 0) {
+            res->check_errors++;
+            ret = reftable_size_changed;
+            goto fail;
+        }
+
+        /*
+         * If the reftable size has changed, we will need to find a new
+         * allocation, repeating the loop.
+         */
+    } while (reftable_size_changed);
 
-    for (refblock_index = 0; refblock_index < reftable_size; refblock_index++) {
+    /* The above loop must have run at least once */
+    assert(reftable_offset >= 0);
+
+    /*
+     * All allocations are done, all refblocks are written, convert the
+     * reftable to big-endian and write it to disk.
+     */
+
+    for (refblock_index = 0; refblock_index < on_disk_reftable_entries;
+         refblock_index++)
+    {
         cpu_to_be64s(&on_disk_reftable[refblock_index]);
     }
 
-    ret = qcow2_pre_write_overlap_check(bs, 0, reftable_offset,
-                                        reftable_size * REFTABLE_ENTRY_SIZE,
+    ret = qcow2_pre_write_overlap_check(bs, 0, reftable_offset, reftable_length,
                                         false);
     if (ret < 0) {
         fprintf(stderr, "ERROR writing reftable: %s\n", strerror(-ret));
         goto fail;
     }
 
-    assert(reftable_size < INT_MAX / REFTABLE_ENTRY_SIZE);
+    assert(reftable_length < INT_MAX);
     ret = bdrv_pwrite(bs->file, reftable_offset, on_disk_reftable,
-                      reftable_size * REFTABLE_ENTRY_SIZE);
+                      reftable_length);
     if (ret < 0) {
         fprintf(stderr, "ERROR writing reftable: %s\n", strerror(-ret));
         goto fail;
@@ -XXX,XX +XXX,XX @@ write_refblocks:
     /* Enter new reftable into the image header */
     reftable_offset_and_clusters.reftable_offset = cpu_to_be64(reftable_offset);
     reftable_offset_and_clusters.reftable_clusters =
-        cpu_to_be32(size_to_clusters(s, reftable_size * REFTABLE_ENTRY_SIZE));
+        cpu_to_be32(reftable_clusters);
     ret = bdrv_pwrite_sync(bs->file,
                            offsetof(QCowHeader, refcount_table_offset),
                            &reftable_offset_and_clusters,
@@ -XXX,XX +XXX,XX @@ write_refblocks:
         goto fail;
     }
 
-    for (refblock_index = 0; refblock_index < reftable_size; refblock_index++) {
+    for (refblock_index = 0; refblock_index < on_disk_reftable_entries;
+         refblock_index++)
+    {
         be64_to_cpus(&on_disk_reftable[refblock_index]);
     }
     s->refcount_table = on_disk_reftable;
     s->refcount_table_offset = reftable_offset;
-    s->refcount_table_size = reftable_size;
+    s->refcount_table_size = on_disk_reftable_entries;
     update_max_refcount_table_index(s);
 
     return 0;
-- 
2.35.1

One clear problem with how qcow2's refcount structure rebuild algorithm
used to be before "qcow2: Improve refcount structure rebuilding" was
that it is prone to failure for qcow2 images on block devices: There is
generally unused space after the actual image, and if that exceeds what
one refblock covers, the old algorithm would invariably write the
reftable past the block device's end, which cannot work.  The new
algorithm does not have this problem.

Test it with three tests:
(1) Create an image with more empty space at the end than what one
    refblock covers, see whether rebuilding the refcount structures
    results in a change in the image file length.  (It should not.)

(2) Leave precisely enough space somewhere at the beginning of the image
    for the new reftable (and the refblock for that place), see whether
    the new algorithm puts the reftable there.  (It should.)

(3) Test the original problem: Create (something like) a block device
    with a fixed size, then create a qcow2 image in there, write some
    data, and then have qemu-img check rebuild the refcount structures.
    Before HEAD^, the reftable would have been written past the image
    file end, i.e. outside of what the block device provides, which
    cannot work.  HEAD^ should have fixed that.
    ("Something like a block device" means a loop device if we can use
    one ("sudo -n losetup" works), or a FUSE block export with
    growable=false otherwise.)

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20220405134652.19278-3-hreitz@redhat.com>
---
 tests/qemu-iotests/108     | 259 ++++++++++++++++++++++++++++++++++++-
 tests/qemu-iotests/108.out |  81 ++++++++++++
 2 files changed, 339 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/108 b/tests/qemu-iotests/108
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/108
+++ b/tests/qemu-iotests/108
@@ -XXX,XX +XXX,XX @@ status=1	# failure is the default!
 
 _cleanup()
 {
-	_cleanup_test_img
+    _cleanup_test_img
+    if [ -f "$TEST_DIR/qsd.pid" ]; then
+        qsd_pid=$(cat "$TEST_DIR/qsd.pid")
+        kill -KILL "$qsd_pid"
+        fusermount -u "$TEST_DIR/fuse-export" &>/dev/null
+    fi
+    rm -f "$TEST_DIR/fuse-export"
 }
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
 # get standard environment, filters and checks
 . ./common.rc
 . ./common.filter
+. ./common.qemu
 
 # This tests qcow2-specific low-level functionality
 _supported_fmt qcow2
@@ -XXX,XX +XXX,XX @@ _supported_os Linux
 # files
 _unsupported_imgopts 'refcount_bits=$[^1]\|.\([^6]\|$$\)' data_file
 
+# This test either needs sudo -n losetup or FUSE exports to work
+if sudo -n losetup &>/dev/null; then
+    loopdev=true
+else
+    loopdev=false
+
+    # QSD --export fuse will either yield "Parameter 'id' is missing"
+    # or "Invalid parameter 'fuse'", depending on whether there is
+    # FUSE support or not.
+    error=$($QSD --export fuse 2>&1)
+    if [[ $error = *"'fuse'"* ]]; then
+        _notrun 'Passwordless sudo for losetup or FUSE support required, but' \
+                'neither is available'
+    fi
+fi
+
 echo
 echo '=== Repairing an image without any refcount table ==='
 echo
@@ -XXX,XX +XXX,XX @@ _make_test_img 64M
 poke_file "$TEST_IMG" $((0x10008)) "\xff\xff\xff\xff\xff\xff\x00\x00"
 _check_test_img -r all
 
+echo
+echo '=== Check rebuilt reftable location ==='
+
+# In an earlier version of the refcount rebuild algorithm, the
+# reftable was generally placed at the image end (unless something was
+# allocated in the area covered by the refblock right before the image
+# file end, then we would try to place the reftable in that refblock).
+# This was later changed so the reftable would be placed in the
+# earliest possible location.  Test this.
+
+echo
+echo '--- Does the image size increase? ---'
+echo
+
+# First test: Just create some image, write some data to it, and
+# resize it so there is free space at the end of the image (enough
+# that it spans at least one full refblock, which for cluster_size=512
+# images, spans 128k).  With the old algorithm, the reftable would
+# have then been placed at the end of the image file, but with the new
+# one, it will be put in that free space.
+# We want to check whether the size of the image file increases due to
+# rebuilding the refcount structures (it should not).
+
+_make_test_img -o 'cluster_size=512' 1M
+# Write something
+$QEMU_IO -c 'write 0 64k' "$TEST_IMG" | _filter_qemu_io
+
+# Add free space
+file_len=$(stat -c '%s' "$TEST_IMG")
+truncate -s $((file_len + 256 * 1024)) "$TEST_IMG"
+
+# Corrupt the image by saying the image header was not allocated
+rt_offset=$(peek_file_be "$TEST_IMG" 48 8)
+rb_offset=$(peek_file_be "$TEST_IMG" $rt_offset 8)
+poke_file "$TEST_IMG" $rb_offset "\x00\x00"
+
+# Check whether rebuilding the refcount structures increases the image
+# file size
+file_len=$(stat -c '%s' "$TEST_IMG")
+echo
+# The only leaks there can be are the old refcount structures that are
+# leaked during rebuilding, no need to clutter the output with them
+_check_test_img -r all | grep -v '^Repairing cluster.*refcount=1 reference=0'
+echo
+post_repair_file_len=$(stat -c '%s' "$TEST_IMG")
+
+if [[ $file_len -eq $post_repair_file_len ]]; then
+    echo 'OK: Image size did not change'
+else
+    echo 'ERROR: Image size differs' \
+        "($file_len before, $post_repair_file_len after)"
+fi
+
+echo
+echo '--- Will the reftable occupy a hole specifically left for it?  ---'
+echo
+
+# Note: With cluster_size=512, every refblock covers 128k.
+# The reftable covers 8M per reftable cluster.
+
+# Create an image that requires two reftable clusters (just because
+# this is more interesting than a single-clustered reftable).
+_make_test_img -o 'cluster_size=512' 9M
+$QEMU_IO -c 'write 0 8M' "$TEST_IMG" | _filter_qemu_io
+
+# Writing 8M will have resized the reftable.  Unfortunately, doing so
+# will leave holes in the file, so we need to fill them up so we can
+# be sure the whole file is allocated.  Do that by writing
+# consecutively smaller chunks starting from 8 MB, until the file
+# length increases even with a chunk size of 512.  Then we must have
+# filled all holes.
+ofs=$((8 * 1024 * 1024))
+block_len=$((16 * 1024))
+while [[ $block_len -ge 512 ]]; do
+    file_len=$(stat -c '%s' "$TEST_IMG")
+    while [[ $(stat -c '%s' "$TEST_IMG") -eq $file_len ]]; do
+        # Do not include this in the reference output, it does not
+        # really matter which qemu-io calls we do here exactly
+        $QEMU_IO -c "write $ofs $block_len" "$TEST_IMG" >/dev/null
+        ofs=$((ofs + block_len))
+    done
+    block_len=$((block_len / 2))
+done
+
+# Fill up to 9M (do not include this in the reference output either,
+# $ofs is random for all we know)
+$QEMU_IO -c "write $ofs $((9 * 1024 * 1024 - ofs))" "$TEST_IMG" >/dev/null
+
+# Make space as follows:
+# - For the first refblock: Right at the beginning of the image (this
+#   refblock is placed in the first place possible),
+# - For the reftable somewhere soon afterwards, still near the
+#   beginning of the image (i.e. covered by the first refblock); the
+#   reftable too is placed in the first place possible, but only after
+#   all refblocks have been placed)
+# No space is needed for the other refblocks, because no refblock is
+# put before the space it covers.  In this test case, we do not mind
+# if they are placed at the image file's end.
+
+# Before we make that space, we have to find out the host offset of
+# the area that belonged to the two data clusters at guest offset 4k,
+# because we expect the reftable to be placed there, and we will have
+# to verify that it is.
+
+l1_offset=$(peek_file_be "$TEST_IMG" 40 8)
+l2_offset=$(peek_file_be "$TEST_IMG" $l1_offset 8)
+l2_offset=$((l2_offset & 0x00fffffffffffe00))
+data_4k_offset=$(peek_file_be "$TEST_IMG" \
+                 $((l2_offset + 4096 / 512 * 8)) 8)
+data_4k_offset=$((data_4k_offset & 0x00fffffffffffe00))
+
+$QEMU_IO -c "discard 0 512" -c "discard 4k 1k" "$TEST_IMG" | _filter_qemu_io
+
+# Corrupt the image by saying the image header was not allocated
+rt_offset=$(peek_file_be "$TEST_IMG" 48 8)
+rb_offset=$(peek_file_be "$TEST_IMG" $rt_offset 8)
+poke_file "$TEST_IMG" $rb_offset "\x00\x00"
+
+echo
+# The only leaks there can be are the old refcount structures that are
+# leaked during rebuilding, no need to clutter the output with them
+_check_test_img -r all | grep -v '^Repairing cluster.*refcount=1 reference=0'
+echo
+
+# Check whether the reftable was put where we expected
+rt_offset=$(peek_file_be "$TEST_IMG" 48 8)
+if [[ $rt_offset -eq $data_4k_offset ]]; then
+    echo 'OK: Reftable is where we expect it'
+else
+    echo "ERROR: Reftable is at $rt_offset, but was expected at $data_4k_offset"
+fi
+
+echo
+echo '--- Rebuilding refcount structures on block devices ---'
+echo
+
+# A block device cannot really grow, at least not during qemu-img
+# check.  As mentioned in the above cases, rebuilding the refcount
+# structure may lead to new refcount structures being written after
+# the end of the image, and in the past that happened even if there
+# was more than sufficient space in the image.  Such post-EOF writes
+# will not work on block devices, so test that the new algorithm
+# avoids it.
+
+# If we have passwordless sudo and losetup, we can use those to create
+# a block device.  Otherwise, we can resort to qemu's FUSE export to
+# create a file that isn't growable, which effectively tests the same
+# thing.
+
+_cleanup_test_img
+truncate -s $((64 * 1024 * 1024)) "$TEST_IMG"
+
+if $loopdev; then
+    export_mp=$(sudo -n losetup --show -f "$TEST_IMG")
+    export_mp_driver=host_device
+    sudo -n chmod go+rw "$export_mp"
+else
+    # Create non-growable FUSE export that is a bit like an empty
+    # block device
+    export_mp="$TEST_DIR/fuse-export"
+    export_mp_driver=file
+    touch "$export_mp"
+
+    $QSD \
+        --blockdev file,node-name=export-node,filename="$TEST_IMG" \
+        --export fuse,id=fuse-export,node-name=export-node,mountpoint="$export_mp",writable=on,growable=off \
+        --pidfile "$TEST_DIR/qsd.pid" \
+        --daemonize
+fi
+
+# Now create a qcow2 image on the device -- unfortunately, qemu-img
+# create force-creates the file, so we have to resort to the
+# blockdev-create job.
+_launch_qemu \
+    --blockdev $export_mp_driver,node-name=file,filename="$export_mp"
+
+_send_qemu_cmd \
+    $QEMU_HANDLE \
+    '{ "execute": "qmp_capabilities" }' \
+    'return'
+
+# Small cluster size again, so the image needs multiple refblocks
+_send_qemu_cmd \
+    $QEMU_HANDLE \
+    '{ "execute": "blockdev-create",
+       "arguments": {
+           "job-id": "create",
+           "options": {
+               "driver": "qcow2",
+               "file": "file",
+               "size": '$((64 * 1024 * 1024))',
+               "cluster-size": 512
+           } } }' \
+    '"concluded"'
+
+_send_qemu_cmd \
+    $QEMU_HANDLE \
+    '{ "execute": "job-dismiss", "arguments": { "id": "create" } }' \
+    'return'
+
+_send_qemu_cmd \
+    $QEMU_HANDLE \
+    '{ "execute": "quit" }' \
+    'return'
+
+wait=y _cleanup_qemu
+echo
+
+# Write some data
+$QEMU_IO -c 'write 0 64k' "$export_mp" | _filter_qemu_io
+
+# Corrupt the image by saying the image header was not allocated
+rt_offset=$(peek_file_be "$export_mp" 48 8)
+rb_offset=$(peek_file_be "$export_mp" $rt_offset 8)
+poke_file "$export_mp" $rb_offset "\x00\x00"
+
+# Repairing such a simple case should just work
+# (We used to put the reftable at the end of the image file, which can
+# never work for non-growable devices.)
+echo
+TEST_IMG="$export_mp" _check_test_img -r all \
+    | grep -v '^Repairing cluster.*refcount=1 reference=0'
+
+if $loopdev; then
+    sudo -n losetup -d "$export_mp"
+else
+    qsd_pid=$(cat "$TEST_DIR/qsd.pid")
+    kill -TERM "$qsd_pid"
+    # Wait for process to exit (cannot `wait` because the QSD is daemonized)
+    while [ -f "$TEST_DIR/qsd.pid" ]; do
+        true
+    done
+fi
+
 # success, all done
 echo '*** done'
 rm -f $seq.full
diff --git a/tests/qemu-iotests/108.out b/tests/qemu-iotests/108.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/108.out
+++ b/tests/qemu-iotests/108.out
@@ -XXX,XX +XXX,XX @@ The following inconsistencies were found and repaired:
     0 leaked clusters
     1 corruptions
 
+Double checking the fixed image now...
+No errors were found on the image.
+
+=== Check rebuilt reftable location ===
+
+--- Does the image size increase? ---
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
+wrote 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+ERROR cluster 0 refcount=0 reference=1
+Rebuilding refcount structure
+The following inconsistencies were found and repaired:
+
+    0 leaked clusters
+    1 corruptions
+
+Double checking the fixed image now...
+No errors were found on the image.
+
+OK: Image size did not change
+
+--- Will the reftable occupy a hole specifically left for it?  ---
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=9437184
+wrote 8388608/8388608 bytes at offset 0
+8 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+discard 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+discard 1024/1024 bytes at offset 4096
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+ERROR cluster 0 refcount=0 reference=1
+Rebuilding refcount structure
+The following inconsistencies were found and repaired:
+
+    0 leaked clusters
+    1 corruptions
+
+Double checking the fixed image now...
+No errors were found on the image.
+
+OK: Reftable is where we expect it
+
+--- Rebuilding refcount structures on block devices ---
+
+{ "execute": "qmp_capabilities" }
+{"return": {}}
+{ "execute": "blockdev-create",
+       "arguments": {
+           "job-id": "create",
+           "options": {
+               "driver": "IMGFMT",
+               "file": "file",
+               "size": 67108864,
+               "cluster-size": 512
+           } } }
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "create"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "create"}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "waiting", "id": "create"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "pending", "id": "create"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "concluded", "id": "create"}}
+{ "execute": "job-dismiss", "arguments": { "id": "create" } }
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "null", "id": "create"}}
+{"return": {}}
+{ "execute": "quit" }
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}}
+
+wrote 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+ERROR cluster 0 refcount=0 reference=1
+Rebuilding refcount structure
+The following inconsistencies were found and repaired:
+
+    0 leaked clusters
+    1 corruptions
+
 Double checking the fixed image now...
 No errors were found on the image.
 *** done
-- 
2.35.1

Instead of fprint()-ing error messages in rebuild_refcount_structure()
and its rebuild_refcounts_write_refblocks() helper, pass them through an
Error object to qcow2_check_refcounts() (which will then print it).

Suggested-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20220405134652.19278-4-hreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block/qcow2-refcount.c | 33 +++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 14 deletions(-)

The following changes since commit 9cf289af47bcfae5c75de37d8e5d6fd23705322c:

Merge tag 'qga-pull-request' of gitlab.com:marcandre.lureau/qemu into staging (2022-05-04 03:42:49 -0700)

are available in the Git repository at:

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

for you to fetch changes up to bef2e050d6a7feb865854c65570c496ac5a8cf53:

util/event-loop-base: Introduce options to set the thread pool size (2022-05-04 17:02:19 +0100)

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

Add new thread-pool-min/thread-pool-max parameters to control the thread pool
used for async I/O.

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

Nicolas Saenz Julienne (3):
  Introduce event-loop-base abstract class
  util/main-loop: Introduce the main loop into QOM
  util/event-loop-base: Introduce options to set the thread pool size

-- 
2.35.1

From: Nicolas Saenz Julienne <nsaenzju@redhat.com>

Introduce the 'event-loop-base' abstract class, it'll hold the
properties common to all event loops and provide the necessary hooks for
their creation and maintenance. Then have iothread inherit from it.

EventLoopBaseClass is defined as user creatable and provides a hook for
its children to attach themselves to the user creatable class 'complete'
function. It also provides an update_params() callback to propagate
property changes onto its children.

The new 'event-loop-base' class will live in the root directory. It is
built on its own using the 'link_whole' option (there are no direct
function dependencies between the class and its children, it all happens
trough 'constructor' magic). And also imposes new compilation
dependencies:

qom <- event-loop-base <- blockdev (iothread.c)

And in subsequent patches:

qom <- event-loop-base <- qemuutil (util/main-loop.c)

All this forced some amount of reordering in meson.build:

- Moved qom build definition before qemuutil. Doing it the other way
   around (i.e. moving qemuutil after qom) isn't possible as a lot of
   core libraries that live in between the two depend on it.

- Process the 'hw' subdir earlier, as it introduces files into the
   'qom' source set.

No functional changes intended.

Signed-off-by: Nicolas Saenz Julienne <nsaenzju@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Acked-by: Markus Armbruster <armbru@redhat.com>
Message-id: 20220425075723.20019-2-nsaenzju@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 qapi/qom.json                    |  22 +++++--
 meson.build                      |  23 ++++---
 include/sysemu/event-loop-base.h |  36 +++++++++++
 include/sysemu/iothread.h        |   6 +-
 event-loop-base.c                | 104 +++++++++++++++++++++++++++++++
 iothread.c                       |  65 ++++++-------------
 6 files changed, 192 insertions(+), 64 deletions(-)
 create mode 100644 include/sysemu/event-loop-base.h
 create mode 100644 event-loop-base.c

diff --git a/qapi/qom.json b/qapi/qom.json
index XXXXXXX..XXXXXXX 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -XXX,XX +XXX,XX @@
             '*repeat': 'bool',
             '*grab-toggle': 'GrabToggleKeys' } }
 
+##
+# @EventLoopBaseProperties:
+#
+# Common properties for event loops
+#
+# @aio-max-batch: maximum number of requests in a batch for the AIO engine,
+#                 0 means that the engine will use its default.
+#                 (default: 0)
+#
+# Since: 7.1
+##
+{ 'struct': 'EventLoopBaseProperties',
+  'data': { '*aio-max-batch': 'int' } }
+
 ##
 # @IothreadProperties:
 #
@@ -XXX,XX +XXX,XX @@
 #               algorithm detects it is spending too long polling without
 #               encountering events. 0 selects a default behaviour (default: 0)
 #
-# @aio-max-batch: maximum number of requests in a batch for the AIO engine,
-#                 0 means that the engine will use its default
-#                 (default:0, since 6.1)
+# The @aio-max-batch option is available since 6.1.
 #
 # Since: 2.0
 ##
 { 'struct': 'IothreadProperties',
+  'base': 'EventLoopBaseProperties',
   'data': { '*poll-max-ns': 'int',
             '*poll-grow': 'int',
-            '*poll-shrink': 'int',
-            '*aio-max-batch': 'int' } }
+            '*poll-shrink': 'int' } }
 
 ##
 # @MemoryBackendProperties:
diff --git a/meson.build b/meson.build
index XXXXXXX..XXXXXXX 100644
--- a/meson.build
+++ b/meson.build
@@ -XXX,XX +XXX,XX @@ subdir('qom')
 subdir('authz')
 subdir('crypto')
 subdir('ui')
+subdir('hw')
 
 
 if enable_modules
@@ -XXX,XX +XXX,XX @@ if enable_modules
   modulecommon = declare_dependency(link_whole: libmodulecommon, compile_args: '-DBUILD_DSO')
 endif
 
+qom_ss = qom_ss.apply(config_host, strict: false)
+libqom = static_library('qom', qom_ss.sources() + genh,
+                        dependencies: [qom_ss.dependencies()],
+                        name_suffix: 'fa')
+qom = declare_dependency(link_whole: libqom)
+
+event_loop_base = files('event-loop-base.c')
+event_loop_base = static_library('event-loop-base', sources: event_loop_base + genh,
+                                 build_by_default: true)
+event_loop_base = declare_dependency(link_whole: event_loop_base,
+                                     dependencies: [qom])
+
 stub_ss = stub_ss.apply(config_all, strict: false)
 
 util_ss.add_all(trace_ss)
@@ -XXX,XX +XXX,XX @@ subdir('monitor')
 subdir('net')
 subdir('replay')
 subdir('semihosting')
-subdir('hw')
 subdir('tcg')
 subdir('fpu')
 subdir('accel')
@@ -XXX,XX +XXX,XX @@ qemu_syms = custom_target('qemu.syms', output: 'qemu.syms',
                              capture: true,
                              command: [undefsym, nm, '@INPUT@'])
 
-qom_ss = qom_ss.apply(config_host, strict: false)
-libqom = static_library('qom', qom_ss.sources() + genh,
-                        dependencies: [qom_ss.dependencies()],
-                        name_suffix: 'fa')
-
-qom = declare_dependency(link_whole: libqom)
-
 authz_ss = authz_ss.apply(config_host, strict: false)
 libauthz = static_library('authz', authz_ss.sources() + genh,
                           dependencies: [authz_ss.dependencies()],
@@ -XXX,XX +XXX,XX @@ libblockdev = static_library('blockdev', blockdev_ss.sources() + genh,
                              build_by_default: false)
 
 blockdev = declare_dependency(link_whole: [libblockdev],
-                              dependencies: [block])
+                              dependencies: [block, event_loop_base])
 
 qmp_ss = qmp_ss.apply(config_host, strict: false)
 libqmp = static_library('qmp', qmp_ss.sources() + genh,
diff --git a/include/sysemu/event-loop-base.h b/include/sysemu/event-loop-base.h
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/include/sysemu/event-loop-base.h
@@ -XXX,XX +XXX,XX @@
+/*
+ * QEMU event-loop backend
+ *
+ * Copyright (C) 2022 Red Hat Inc
+ *
+ * Authors:
+ *  Nicolas Saenz Julienne <nsaenzju@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#ifndef QEMU_EVENT_LOOP_BASE_H
+#define QEMU_EVENT_LOOP_BASE_H
+
+#include "qom/object.h"
+#include "block/aio.h"
+#include "qemu/typedefs.h"
+
+#define TYPE_EVENT_LOOP_BASE         "event-loop-base"
+OBJECT_DECLARE_TYPE(EventLoopBase, EventLoopBaseClass,
+                    EVENT_LOOP_BASE)
+
+struct EventLoopBaseClass {
+    ObjectClass parent_class;
+
+    void (*init)(EventLoopBase *base, Error **errp);
+    void (*update_params)(EventLoopBase *base, Error **errp);
+};
+
+struct EventLoopBase {
+    Object parent;
+
+    /* AioContext AIO engine parameters */
+    int64_t aio_max_batch;
+};
+#endif
diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
index XXXXXXX..XXXXXXX 100644
--- a/include/sysemu/iothread.h
+++ b/include/sysemu/iothread.h
@@ -XXX,XX +XXX,XX @@
 #include "block/aio.h"
 #include "qemu/thread.h"
 #include "qom/object.h"
+#include "sysemu/event-loop-base.h"
 
 #define TYPE_IOTHREAD "iothread"
 
 struct IOThread {
-    Object parent_obj;
+    EventLoopBase parent_obj;
 
     QemuThread thread;
     AioContext *ctx;
@@ -XXX,XX +XXX,XX @@ struct IOThread {
     int64_t poll_max_ns;
     int64_t poll_grow;
     int64_t poll_shrink;
-
-    /* AioContext AIO engine parameters */
-    int64_t aio_max_batch;
 };
 typedef struct IOThread IOThread;
 
diff --git a/event-loop-base.c b/event-loop-base.c
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/event-loop-base.c
@@ -XXX,XX +XXX,XX @@
+/*
+ * QEMU event-loop base
+ *
+ * Copyright (C) 2022 Red Hat Inc
+ *
+ * Authors:
+ *  Stefan Hajnoczi <stefanha@redhat.com>
+ *  Nicolas Saenz Julienne <nsaenzju@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qom/object_interfaces.h"
+#include "qapi/error.h"
+#include "sysemu/event-loop-base.h"
+
+typedef struct {
+    const char *name;
+    ptrdiff_t offset; /* field's byte offset in EventLoopBase struct */
+} EventLoopBaseParamInfo;
+
+static EventLoopBaseParamInfo aio_max_batch_info = {
+    "aio-max-batch", offsetof(EventLoopBase, aio_max_batch),
+};
+
+static void event_loop_base_get_param(Object *obj, Visitor *v,
+        const char *name, void *opaque, Error **errp)
+{
+    EventLoopBase *event_loop_base = EVENT_LOOP_BASE(obj);
+    EventLoopBaseParamInfo *info = opaque;
+    int64_t *field = (void *)event_loop_base + info->offset;
+
+    visit_type_int64(v, name, field, errp);
+}
+
+static void event_loop_base_set_param(Object *obj, Visitor *v,
+        const char *name, void *opaque, Error **errp)
+{
+    EventLoopBaseClass *bc = EVENT_LOOP_BASE_GET_CLASS(obj);
+    EventLoopBase *base = EVENT_LOOP_BASE(obj);
+    EventLoopBaseParamInfo *info = opaque;
+    int64_t *field = (void *)base + info->offset;
+    int64_t value;
+
+    if (!visit_type_int64(v, name, &value, errp)) {
+        return;
+    }
+
+    if (value < 0) {
+        error_setg(errp, "%s value must be in range [0, %" PRId64 "]",
+                   info->name, INT64_MAX);
+        return;
+    }
+
+    *field = value;
+
+    if (bc->update_params) {
+        bc->update_params(base, errp);
+    }
+
+    return;
+}
+
+static void event_loop_base_complete(UserCreatable *uc, Error **errp)
+{
+    EventLoopBaseClass *bc = EVENT_LOOP_BASE_GET_CLASS(uc);
+    EventLoopBase *base = EVENT_LOOP_BASE(uc);
+
+    if (bc->init) {
+        bc->init(base, errp);
+    }
+}
+
+static void event_loop_base_class_init(ObjectClass *klass, void *class_data)
+{
+    UserCreatableClass *ucc = USER_CREATABLE_CLASS(klass);
+    ucc->complete = event_loop_base_complete;
+
+    object_class_property_add(klass, "aio-max-batch", "int",
+                              event_loop_base_get_param,
+                              event_loop_base_set_param,
+                              NULL, &aio_max_batch_info);
+}
+
+static const TypeInfo event_loop_base_info = {
+    .name = TYPE_EVENT_LOOP_BASE,
+    .parent = TYPE_OBJECT,
+    .instance_size = sizeof(EventLoopBase),
+    .class_size = sizeof(EventLoopBaseClass),
+    .class_init = event_loop_base_class_init,
+    .abstract = true,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_USER_CREATABLE },
+        { }
+    }
+};
+
+static void register_types(void)
+{
+    type_register_static(&event_loop_base_info);
+}
+type_init(register_types);
diff --git a/iothread.c b/iothread.c
index XXXXXXX..XXXXXXX 100644
--- a/iothread.c
+++ b/iothread.c
@@ -XXX,XX +XXX,XX @@
 #include "qemu/module.h"
 #include "block/aio.h"
 #include "block/block.h"
+#include "sysemu/event-loop-base.h"
 #include "sysemu/iothread.h"
 #include "qapi/error.h"
 #include "qapi/qapi-commands-misc.h"
@@ -XXX,XX +XXX,XX @@ static void iothread_init_gcontext(IOThread *iothread)
     iothread->main_loop = g_main_loop_new(iothread->worker_context, TRUE);
 }
 
-static void iothread_set_aio_context_params(IOThread *iothread, Error **errp)
+static void iothread_set_aio_context_params(EventLoopBase *base, Error **errp)
 {
+    IOThread *iothread = IOTHREAD(base);
     ERRP_GUARD();
 
+    if (!iothread->ctx) {
+        return;
+    }
+
     aio_context_set_poll_params(iothread->ctx,
                                 iothread->poll_max_ns,
                                 iothread->poll_grow,
@@ -XXX,XX +XXX,XX @@ static void iothread_set_aio_context_params(IOThread *iothread, Error **errp)
     }
 
     aio_context_set_aio_params(iothread->ctx,
-                               iothread->aio_max_batch,
+                               iothread->parent_obj.aio_max_batch,
                                errp);
 }
 
-static void iothread_complete(UserCreatable *obj, Error **errp)
+
+static void iothread_init(EventLoopBase *base, Error **errp)
 {
     Error *local_error = NULL;
-    IOThread *iothread = IOTHREAD(obj);
+    IOThread *iothread = IOTHREAD(base);
     char *thread_name;
 
     iothread->stopping = false;
@@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp)
      */
     iothread_init_gcontext(iothread);
 
-    iothread_set_aio_context_params(iothread, &local_error);
+    iothread_set_aio_context_params(base, &local_error);
     if (local_error) {
         error_propagate(errp, local_error);
         aio_context_unref(iothread->ctx);
@@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp)
      * to inherit.
      */
     thread_name = g_strdup_printf("IO %s",
-                        object_get_canonical_path_component(OBJECT(obj)));
+                        object_get_canonical_path_component(OBJECT(base)));
     qemu_thread_create(&iothread->thread, thread_name, iothread_run,
                        iothread, QEMU_THREAD_JOINABLE);
     g_free(thread_name);
@@ -XXX,XX +XXX,XX @@ static IOThreadParamInfo poll_grow_info = {
 static IOThreadParamInfo poll_shrink_info = {
     "poll-shrink", offsetof(IOThread, poll_shrink),
 };
-static IOThreadParamInfo aio_max_batch_info = {
-    "aio-max-batch", offsetof(IOThread, aio_max_batch),
-};
 
 static void iothread_get_param(Object *obj, Visitor *v,
         const char *name, IOThreadParamInfo *info, Error **errp)
@@ -XXX,XX +XXX,XX @@ static void iothread_set_poll_param(Object *obj, Visitor *v,
     }
 }
 
-static void iothread_get_aio_param(Object *obj, Visitor *v,
-        const char *name, void *opaque, Error **errp)
-{
-    IOThreadParamInfo *info = opaque;
-
-    iothread_get_param(obj, v, name, info, errp);
-}
-
-static void iothread_set_aio_param(Object *obj, Visitor *v,
-        const char *name, void *opaque, Error **errp)
-{
-    IOThread *iothread = IOTHREAD(obj);
-    IOThreadParamInfo *info = opaque;
-
-    if (!iothread_set_param(obj, v, name, info, errp)) {
-        return;
-    }
-
-    if (iothread->ctx) {
-        aio_context_set_aio_params(iothread->ctx,
-                                   iothread->aio_max_batch,
-                                   errp);
-    }
-}
-
 static void iothread_class_init(ObjectClass *klass, void *class_data)
 {
-    UserCreatableClass *ucc = USER_CREATABLE_CLASS(klass);
-    ucc->complete = iothread_complete;
+    EventLoopBaseClass *bc = EVENT_LOOP_BASE_CLASS(klass);
+
+    bc->init = iothread_init;
+    bc->update_params = iothread_set_aio_context_params;
 
     object_class_property_add(klass, "poll-max-ns", "int",
                               iothread_get_poll_param,
@@ -XXX,XX +XXX,XX @@ static void iothread_class_init(ObjectClass *klass, void *class_data)
                               iothread_get_poll_param,
                               iothread_set_poll_param,
                               NULL, &poll_shrink_info);
-    object_class_property_add(klass, "aio-max-batch", "int",
-                              iothread_get_aio_param,
-                              iothread_set_aio_param,
-                              NULL, &aio_max_batch_info);
 }
 
 static const TypeInfo iothread_info = {
     .name = TYPE_IOTHREAD,
-    .parent = TYPE_OBJECT,
+    .parent = TYPE_EVENT_LOOP_BASE,
     .class_init = iothread_class_init,
     .instance_size = sizeof(IOThread),
     .instance_init = iothread_instance_init,
     .instance_finalize = iothread_instance_finalize,
-    .interfaces = (InterfaceInfo[]) {
-        {TYPE_USER_CREATABLE},
-        {}
-    },
 };
 
 static void iothread_register_types(void)
@@ -XXX,XX +XXX,XX @@ static int query_one_iothread(Object *object, void *opaque)
     info->poll_max_ns = iothread->poll_max_ns;
     info->poll_grow = iothread->poll_grow;
     info->poll_shrink = iothread->poll_shrink;
-    info->aio_max_batch = iothread->aio_max_batch;
+    info->aio_max_batch = iothread->parent_obj.aio_max_batch;
 
     QAPI_LIST_APPEND(*tail, info);
     return 0;
-- 
2.35.1

From: Nicolas Saenz Julienne <nsaenzju@redhat.com>

'event-loop-base' provides basic property handling for all 'AioContext'
based event loops. So let's define a new 'MainLoopClass' that inherits
from it. This will permit tweaking the main loop's properties through
qapi as well as through the command line using the '-object' keyword[1].
Only one instance of 'MainLoopClass' might be created at any time.

'EventLoopBaseClass' learns a new callback, 'can_be_deleted()' so as to
mark 'MainLoop' as non-deletable.

[1] For example:
      -object main-loop,id=main-loop,aio-max-batch=<value>

Signed-off-by: Nicolas Saenz Julienne <nsaenzju@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Acked-by: Markus Armbruster <armbru@redhat.com>
Message-id: 20220425075723.20019-3-nsaenzju@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 qapi/qom.json                    | 13 ++++++++
 meson.build                      |  3 +-
 include/qemu/main-loop.h         | 10 ++++++
 include/sysemu/event-loop-base.h |  1 +
 event-loop-base.c                | 13 ++++++++
 util/main-loop.c                 | 56 ++++++++++++++++++++++++++++++++
 6 files changed, 95 insertions(+), 1 deletion(-)

diff --git a/qapi/qom.json b/qapi/qom.json
index XXXXXXX..XXXXXXX 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -XXX,XX +XXX,XX @@
             '*poll-grow': 'int',
             '*poll-shrink': 'int' } }
 
+##
+# @MainLoopProperties:
+#
+# Properties for the main-loop object.
+#
+# Since: 7.1
+##
+{ 'struct': 'MainLoopProperties',
+  'base': 'EventLoopBaseProperties',
+  'data': {} }
+
 ##
 # @MemoryBackendProperties:
 #
@@ -XXX,XX +XXX,XX @@
     { 'name': 'input-linux',
       'if': 'CONFIG_LINUX' },
     'iothread',
+    'main-loop',
     { 'name': 'memory-backend-epc',
       'if': 'CONFIG_LINUX' },
     'memory-backend-file',
@@ -XXX,XX +XXX,XX @@
       'input-linux':                { 'type': 'InputLinuxProperties',
                                       'if': 'CONFIG_LINUX' },
       'iothread':                   'IothreadProperties',
+      'main-loop':                  'MainLoopProperties',
       'memory-backend-epc':         { 'type': 'MemoryBackendEpcProperties',
                                       'if': 'CONFIG_LINUX' },
       'memory-backend-file':        'MemoryBackendFileProperties',
diff --git a/meson.build b/meson.build
index XXXXXXX..XXXXXXX 100644
--- a/meson.build
+++ b/meson.build
@@ -XXX,XX +XXX,XX @@ libqemuutil = static_library('qemuutil',
                              sources: util_ss.sources() + stub_ss.sources() + genh,
                              dependencies: [util_ss.dependencies(), libm, threads, glib, socket, malloc, pixman])
 qemuutil = declare_dependency(link_with: libqemuutil,
-                              sources: genh + version_res)
+                              sources: genh + version_res,
+                              dependencies: [event_loop_base])
 
 if have_system or have_user
   decodetree = generator(find_program('scripts/decodetree.py'),
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index XXXXXXX..XXXXXXX 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -XXX,XX +XXX,XX @@
 #define QEMU_MAIN_LOOP_H
 
 #include "block/aio.h"
+#include "qom/object.h"
+#include "sysemu/event-loop-base.h"
 
 #define SIG_IPI SIGUSR1
 
+#define TYPE_MAIN_LOOP  "main-loop"
+OBJECT_DECLARE_TYPE(MainLoop, MainLoopClass, MAIN_LOOP)
+
+struct MainLoop {
+    EventLoopBase parent_obj;
+};
+typedef struct MainLoop MainLoop;
+
 /**
  * qemu_init_main_loop: Set up the process so that it can run the main loop.
  *
diff --git a/include/sysemu/event-loop-base.h b/include/sysemu/event-loop-base.h
index XXXXXXX..XXXXXXX 100644
--- a/include/sysemu/event-loop-base.h
+++ b/include/sysemu/event-loop-base.h
@@ -XXX,XX +XXX,XX @@ struct EventLoopBaseClass {
 
     void (*init)(EventLoopBase *base, Error **errp);
     void (*update_params)(EventLoopBase *base, Error **errp);
+    bool (*can_be_deleted)(EventLoopBase *base);
 };
 
 struct EventLoopBase {
diff --git a/event-loop-base.c b/event-loop-base.c
index XXXXXXX..XXXXXXX 100644
--- a/event-loop-base.c
+++ b/event-loop-base.c
@@ -XXX,XX +XXX,XX @@ static void event_loop_base_complete(UserCreatable *uc, Error **errp)
     }
 }
 
+static bool event_loop_base_can_be_deleted(UserCreatable *uc)
+{
+    EventLoopBaseClass *bc = EVENT_LOOP_BASE_GET_CLASS(uc);
+    EventLoopBase *backend = EVENT_LOOP_BASE(uc);
+
+    if (bc->can_be_deleted) {
+        return bc->can_be_deleted(backend);
+    }
+
+    return true;
+}
+
 static void event_loop_base_class_init(ObjectClass *klass, void *class_data)
 {
     UserCreatableClass *ucc = USER_CREATABLE_CLASS(klass);
     ucc->complete = event_loop_base_complete;
+    ucc->can_be_deleted = event_loop_base_can_be_deleted;
 
     object_class_property_add(klass, "aio-max-batch", "int",
                               event_loop_base_get_param,
diff --git a/util/main-loop.c b/util/main-loop.c
index XXXXXXX..XXXXXXX 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -XXX,XX +XXX,XX @@
 #include "qemu/error-report.h"
 #include "qemu/queue.h"
 #include "qemu/compiler.h"
+#include "qom/object.h"
 
 #ifndef _WIN32
 #include <sys/wait.h>
@@ -XXX,XX +XXX,XX @@ int qemu_init_main_loop(Error **errp)
     return 0;
 }
 
+static void main_loop_update_params(EventLoopBase *base, Error **errp)
+{
+    if (!qemu_aio_context) {
+        error_setg(errp, "qemu aio context not ready");
+        return;
+    }
+
+    aio_context_set_aio_params(qemu_aio_context, base->aio_max_batch, errp);
+}
+
+MainLoop *mloop;
+
+static void main_loop_init(EventLoopBase *base, Error **errp)
+{
+    MainLoop *m = MAIN_LOOP(base);
+
+    if (mloop) {
+        error_setg(errp, "only one main-loop instance allowed");
+        return;
+    }
+
+    main_loop_update_params(base, errp);
+
+    mloop = m;
+    return;
+}
+
+static bool main_loop_can_be_deleted(EventLoopBase *base)
+{
+    return false;
+}
+
+static void main_loop_class_init(ObjectClass *oc, void *class_data)
+{
+    EventLoopBaseClass *bc = EVENT_LOOP_BASE_CLASS(oc);
+
+    bc->init = main_loop_init;
+    bc->update_params = main_loop_update_params;
+    bc->can_be_deleted = main_loop_can_be_deleted;
+}
+
+static const TypeInfo main_loop_info = {
+    .name = TYPE_MAIN_LOOP,
+    .parent = TYPE_EVENT_LOOP_BASE,
+    .class_init = main_loop_class_init,
+    .instance_size = sizeof(MainLoop),
+};
+
+static void main_loop_register_types(void)
+{
+    type_register_static(&main_loop_info);
+}
+
+type_init(main_loop_register_types)
+
 static int max_priority;
 
 #ifndef _WIN32
-- 
2.35.1

From: Nicolas Saenz Julienne <nsaenzju@redhat.com>

The thread pool regulates itself: when idle, it kills threads until
empty, when in demand, it creates new threads until full. This behaviour
doesn't play well with latency sensitive workloads where the price of
creating a new thread is too high. For example, when paired with qemu's
'-mlock', or using safety features like SafeStack, creating a new thread
has been measured take multiple milliseconds.

In order to mitigate this let's introduce a new 'EventLoopBase'
property to set the thread pool size. The threads will be created during
the pool's initialization or upon updating the property's value, remain
available during its lifetime regardless of demand, and destroyed upon
freeing it. A properly characterized workload will then be able to
configure the pool to avoid any latency spikes.

Signed-off-by: Nicolas Saenz Julienne <nsaenzju@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Acked-by: Markus Armbruster <armbru@redhat.com>
Message-id: 20220425075723.20019-4-nsaenzju@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 qapi/qom.json                    | 10 +++++-
 include/block/aio.h              | 10 ++++++
 include/block/thread-pool.h      |  3 ++
 include/sysemu/event-loop-base.h |  4 +++
 event-loop-base.c                | 23 +++++++++++++
 iothread.c                       |  3 ++
 util/aio-posix.c                 |  1 +
 util/async.c                     | 20 ++++++++++++
 util/main-loop.c                 |  9 ++++++
 util/thread-pool.c               | 55 +++++++++++++++++++++++++++++---
 10 files changed, 133 insertions(+), 5 deletions(-)

diff --git a/qapi/qom.json b/qapi/qom.json
index XXXXXXX..XXXXXXX 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -XXX,XX +XXX,XX @@
 #                 0 means that the engine will use its default.
 #                 (default: 0)
 #
+# @thread-pool-min: minimum number of threads reserved in the thread pool
+#                   (default:0)
+#
+# @thread-pool-max: maximum number of threads the thread pool can contain
+#                   (default:64)
+#
 # Since: 7.1
 ##
 { 'struct': 'EventLoopBaseProperties',
-  'data': { '*aio-max-batch': 'int' } }
+  'data': { '*aio-max-batch': 'int',
+            '*thread-pool-min': 'int',
+            '*thread-pool-max': 'int' } }
 
 ##
 # @IothreadProperties:
diff --git a/include/block/aio.h b/include/block/aio.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -XXX,XX +XXX,XX @@ struct AioContext {
     QSLIST_HEAD(, Coroutine) scheduled_coroutines;
     QEMUBH *co_schedule_bh;
 
+    int thread_pool_min;
+    int thread_pool_max;
     /* Thread pool for performing work and receiving completion callbacks.
      * Has its own locking.
      */
@@ -XXX,XX +XXX,XX @@ void aio_context_set_poll_params(AioContext *ctx, int64_t max_ns,
 void aio_context_set_aio_params(AioContext *ctx, int64_t max_batch,
                                 Error **errp);
 
+/**
+ * aio_context_set_thread_pool_params:
+ * @ctx: the aio context
+ * @min: min number of threads to have readily available in the thread pool
+ * @min: max number of threads the thread pool can contain
+ */
+void aio_context_set_thread_pool_params(AioContext *ctx, int64_t min,
+                                        int64_t max, Error **errp);
 #endif
diff --git a/include/block/thread-pool.h b/include/block/thread-pool.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/thread-pool.h
+++ b/include/block/thread-pool.h
@@ -XXX,XX +XXX,XX @@
 
 #include "block/block.h"
 
+#define THREAD_POOL_MAX_THREADS_DEFAULT         64
+
 typedef int ThreadPoolFunc(void *opaque);
 
 typedef struct ThreadPool ThreadPool;
@@ -XXX,XX +XXX,XX @@ BlockAIOCB *thread_pool_submit_aio(ThreadPool *pool,
 int coroutine_fn thread_pool_submit_co(ThreadPool *pool,
         ThreadPoolFunc *func, void *arg);
 void thread_pool_submit(ThreadPool *pool, ThreadPoolFunc *func, void *arg);
+void thread_pool_update_params(ThreadPool *pool, struct AioContext *ctx);
 
 #endif
diff --git a/include/sysemu/event-loop-base.h b/include/sysemu/event-loop-base.h
index XXXXXXX..XXXXXXX 100644
--- a/include/sysemu/event-loop-base.h
+++ b/include/sysemu/event-loop-base.h
@@ -XXX,XX +XXX,XX @@ struct EventLoopBase {
 
     /* AioContext AIO engine parameters */
     int64_t aio_max_batch;
+
+    /* AioContext thread pool parameters */
+    int64_t thread_pool_min;
+    int64_t thread_pool_max;
 };
 #endif
diff --git a/event-loop-base.c b/event-loop-base.c
index XXXXXXX..XXXXXXX 100644
--- a/event-loop-base.c
+++ b/event-loop-base.c
@@ -XXX,XX +XXX,XX @@
 #include "qemu/osdep.h"
 #include "qom/object_interfaces.h"
 #include "qapi/error.h"
+#include "block/thread-pool.h"
 #include "sysemu/event-loop-base.h"
 
 typedef struct {
@@ -XXX,XX +XXX,XX @@ typedef struct {
     ptrdiff_t offset; /* field's byte offset in EventLoopBase struct */
 } EventLoopBaseParamInfo;
 
+static void event_loop_base_instance_init(Object *obj)
+{
+    EventLoopBase *base = EVENT_LOOP_BASE(obj);
+
+    base->thread_pool_max = THREAD_POOL_MAX_THREADS_DEFAULT;
+}
+
 static EventLoopBaseParamInfo aio_max_batch_info = {
     "aio-max-batch", offsetof(EventLoopBase, aio_max_batch),
 };
+static EventLoopBaseParamInfo thread_pool_min_info = {
+    "thread-pool-min", offsetof(EventLoopBase, thread_pool_min),
+};
+static EventLoopBaseParamInfo thread_pool_max_info = {
+    "thread-pool-max", offsetof(EventLoopBase, thread_pool_max),
+};
 
 static void event_loop_base_get_param(Object *obj, Visitor *v,
         const char *name, void *opaque, Error **errp)
@@ -XXX,XX +XXX,XX @@ static void event_loop_base_class_init(ObjectClass *klass, void *class_data)
                               event_loop_base_get_param,
                               event_loop_base_set_param,
                               NULL, &aio_max_batch_info);
+    object_class_property_add(klass, "thread-pool-min", "int",
+                              event_loop_base_get_param,
+                              event_loop_base_set_param,
+                              NULL, &thread_pool_min_info);
+    object_class_property_add(klass, "thread-pool-max", "int",
+                              event_loop_base_get_param,
+                              event_loop_base_set_param,
+                              NULL, &thread_pool_max_info);
 }
 
 static const TypeInfo event_loop_base_info = {
     .name = TYPE_EVENT_LOOP_BASE,
     .parent = TYPE_OBJECT,
     .instance_size = sizeof(EventLoopBase),
+    .instance_init = event_loop_base_instance_init,
     .class_size = sizeof(EventLoopBaseClass),
     .class_init = event_loop_base_class_init,
     .abstract = true,
diff --git a/iothread.c b/iothread.c
index XXXXXXX..XXXXXXX 100644
--- a/iothread.c
+++ b/iothread.c
@@ -XXX,XX +XXX,XX @@ static void iothread_set_aio_context_params(EventLoopBase *base, Error **errp)
     aio_context_set_aio_params(iothread->ctx,
                                iothread->parent_obj.aio_max_batch,
                                errp);
+
+    aio_context_set_thread_pool_params(iothread->ctx, base->thread_pool_min,
+                                       base->thread_pool_max, errp);
 }
 
 
diff --git a/util/aio-posix.c b/util/aio-posix.c
index XXXXXXX..XXXXXXX 100644
--- a/util/aio-posix.c
+++ b/util/aio-posix.c
@@ -XXX,XX +XXX,XX @@
 
 #include "qemu/osdep.h"
 #include "block/block.h"
+#include "block/thread-pool.h"
 #include "qemu/main-loop.h"
 #include "qemu/rcu.h"
 #include "qemu/rcu_queue.h"
diff --git a/util/async.c b/util/async.c
index XXXXXXX..XXXXXXX 100644
--- a/util/async.c
+++ b/util/async.c
@@ -XXX,XX +XXX,XX @@ AioContext *aio_context_new(Error **errp)
 
     ctx->aio_max_batch = 0;
 
+    ctx->thread_pool_min = 0;
+    ctx->thread_pool_max = THREAD_POOL_MAX_THREADS_DEFAULT;
+
     return ctx;
 fail:
     g_source_destroy(&ctx->source);
@@ -XXX,XX +XXX,XX @@ void qemu_set_current_aio_context(AioContext *ctx)
     assert(!get_my_aiocontext());
     set_my_aiocontext(ctx);
 }
+
+void aio_context_set_thread_pool_params(AioContext *ctx, int64_t min,
+                                        int64_t max, Error **errp)
+{
+
+    if (min > max || !max || min > INT_MAX || max > INT_MAX) {
+        error_setg(errp, "bad thread-pool-min/thread-pool-max values");
+        return;
+    }
+
+    ctx->thread_pool_min = min;
+    ctx->thread_pool_max = max;
+
+    if (ctx->thread_pool) {
+        thread_pool_update_params(ctx->thread_pool, ctx);
+    }
+}
diff --git a/util/main-loop.c b/util/main-loop.c
index XXXXXXX..XXXXXXX 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -XXX,XX +XXX,XX @@
 #include "sysemu/replay.h"
 #include "qemu/main-loop.h"
 #include "block/aio.h"
+#include "block/thread-pool.h"
 #include "qemu/error-report.h"
 #include "qemu/queue.h"
 #include "qemu/compiler.h"
@@ -XXX,XX +XXX,XX @@ int qemu_init_main_loop(Error **errp)
 
 static void main_loop_update_params(EventLoopBase *base, Error **errp)
 {
+    ERRP_GUARD();
+
     if (!qemu_aio_context) {
         error_setg(errp, "qemu aio context not ready");
         return;
     }
 
     aio_context_set_aio_params(qemu_aio_context, base->aio_max_batch, errp);
+    if (*errp) {
+        return;
+    }
+
+    aio_context_set_thread_pool_params(qemu_aio_context, base->thread_pool_min,
+                                       base->thread_pool_max, errp);
 }
 
 MainLoop *mloop;
diff --git a/util/thread-pool.c b/util/thread-pool.c
index XXXXXXX..XXXXXXX 100644
--- a/util/thread-pool.c
+++ b/util/thread-pool.c
@@ -XXX,XX +XXX,XX @@ struct ThreadPool {
     QemuMutex lock;
     QemuCond worker_stopped;
     QemuSemaphore sem;
-    int max_threads;
     QEMUBH *new_thread_bh;
 
     /* The following variables are only accessed from one AioContext. */
@@ -XXX,XX +XXX,XX @@ struct ThreadPool {
     int new_threads;     /* backlog of threads we need to create */
     int pending_threads; /* threads created but not running yet */
     bool stopping;
+    int min_threads;
+    int max_threads;
 };
 
+static inline bool back_to_sleep(ThreadPool *pool, int ret)
+{
+    /*
+     * The semaphore timed out, we should exit the loop except when:
+     *  - There is work to do, we raced with the signal.
+     *  - The max threads threshold just changed, we raced with the signal.
+     *  - The thread pool forces a minimum number of readily available threads.
+     */
+    if (ret == -1 && (!QTAILQ_EMPTY(&pool->request_list) ||
+            pool->cur_threads > pool->max_threads ||
+            pool->cur_threads <= pool->min_threads)) {
+            return true;
+    }
+
+    return false;
+}
+
 static void *worker_thread(void *opaque)
 {
     ThreadPool *pool = opaque;
@@ -XXX,XX +XXX,XX @@ static void *worker_thread(void *opaque)
             ret = qemu_sem_timedwait(&pool->sem, 10000);
             qemu_mutex_lock(&pool->lock);
             pool->idle_threads--;
-        } while (ret == -1 && !QTAILQ_EMPTY(&pool->request_list));
-        if (ret == -1 || pool->stopping) {
+        } while (back_to_sleep(pool, ret));
+        if (ret == -1 || pool->stopping ||
+            pool->cur_threads > pool->max_threads) {
             break;
         }
 
@@ -XXX,XX +XXX,XX @@ void thread_pool_submit(ThreadPool *pool, ThreadPoolFunc *func, void *arg)
     thread_pool_submit_aio(pool, func, arg, NULL, NULL);
 }
 
+void thread_pool_update_params(ThreadPool *pool, AioContext *ctx)
+{
+    qemu_mutex_lock(&pool->lock);
+
+    pool->min_threads = ctx->thread_pool_min;
+    pool->max_threads = ctx->thread_pool_max;
+
+    /*
+     * We either have to:
+     *  - Increase the number available of threads until over the min_threads
+     *    threshold.
+     *  - Decrease the number of available threads until under the max_threads
+     *    threshold.
+     *  - Do nothing. The current number of threads fall in between the min and
+     *    max thresholds. We'll let the pool manage itself.
+     */
+    for (int i = pool->cur_threads; i < pool->min_threads; i++) {
+        spawn_thread(pool);
+    }
+
+    for (int i = pool->cur_threads; i > pool->max_threads; i--) {
+        qemu_sem_post(&pool->sem);
+    }
+
+    qemu_mutex_unlock(&pool->lock);
+}
+
 static void thread_pool_init_one(ThreadPool *pool, AioContext *ctx)
 {
     if (!ctx) {
@@ -XXX,XX +XXX,XX @@ static void thread_pool_init_one(ThreadPool *pool, AioContext *ctx)
     qemu_mutex_init(&pool->lock);
     qemu_cond_init(&pool->worker_stopped);
     qemu_sem_init(&pool->sem, 0);
-    pool->max_threads = 64;
     pool->new_thread_bh = aio_bh_new(ctx, spawn_thread_bh_fn, pool);
 
     QLIST_INIT(&pool->head);
     QTAILQ_INIT(&pool->request_list);
+
+    thread_pool_update_params(pool, ctx);
 }
 
 ThreadPool *thread_pool_new(AioContext *ctx)
-- 
2.35.1