Series comparison

-[PULL 00/19] Block patches
+[PULL 0/8] Block patches
-The following changes since commit 96662996eda78c48aadddd4e76d8615c7eb72d80:
+The following changes since commit c6a5fc2ac76c5ab709896ee1b0edd33685a67ed1:
-  Merge remote-tracking branch 'remotes/dgilbert/tags/pull-migration-20210513a' into staging (2021-05-14 12:03:47 +0100)
+  decodetree: Add --output-null for meson testing (2023-05-31 19:56:42 -0700)
 are available in the Git repository at:
-  https://github.com/XanClic/qemu.git tags/pull-block-2021-05-14
+  https://gitlab.com/stefanha/qemu.git tags/block-pull-request
-for you to fetch changes up to c61ebf362d0abf288ce266845519d5a550a1d89f:
+for you to fetch changes up to 98b126f5e3228a346c774e569e26689943b401dd:
-  write-threshold: deal with includes (2021-05-14 16:14:10 +0200)
+  qapi: add '@fdset' feature for BlockdevOptionsVirtioBlkVhostVdpa (2023-06-01 11:08:21 -0400)
 ----------------------------------------------------------------
-Block patches:
+Pull request
-- drop block/io write notifiers
-- qemu-iotests enhancements to make debugging easier
+- Stefano Garzarella's blkio block driver 'fd' parameter
-- rbd parsing fix
+- My thread-local blk_io_plug() series
 - HMP qemu-io fix (for iothreads)
 - mirror job cancel relaxation (do not cancel in-flight requests when a
   READY mirror job is canceled with force=false)
 - document qcow2's data_file and data_file_raw features
 - fix iotest 297 for pylint 2.8
 - block/copy-on-read refactoring
 ----------------------------------------------------------------
-Connor Kuehl (3):
-  iotests/231: Update expected deprecation message
-  block/rbd: Add an escape-aware strchr helper
-  Document qemu-img options data_file and data_file_raw
-Emanuele Giuseppe Esposito (1):
+Stefan Hajnoczi (6):
-  qemu-iotests: fix pylint 2.8 consider-using-with error
+  block: add blk_io_plug_call() API
   block/nvme: convert to blk_io_plug_call() API
   block/blkio: convert to blk_io_plug_call() API
   block/io_uring: convert to blk_io_plug_call() API
   block/linux-aio: convert to blk_io_plug_call() API
   block: remove bdrv_co_io_plug() API
-Paolo Bonzini (5):
+Stefano Garzarella (2):
-  qemu-iotests: do not buffer the test output
+  block/blkio: use qemu_open() to support fd passing for virtio-blk
-  qemu-iotests: allow passing unittest.main arguments to the test
+  qapi: add '@fdset' feature for BlockdevOptionsVirtioBlkVhostVdpa
     scripts
   qemu-iotests: move command line and environment handling from
     TestRunner to TestEnv
   qemu-iotests: let "check" spawn an arbitrary test command
   qemu-iotests: fix case of SOCK_DIR already in the environment
-Vladimir Sementsov-Ogievskiy (10):
+ MAINTAINERS                       |   1 +
-  monitor: hmp_qemu_io: acquire aio contex, fix crash
+ qapi/block-core.json              |   6 ++
-  mirror: stop cancelling in-flight requests on non-force cancel in
+ meson.build                       |   4 +
-    READY
+ include/block/block-io.h          |   3 -
-  block/copy-on-read: use bdrv_drop_filter() and drop s->active
+ include/block/block_int-common.h  |  11 ---
-  block/write-threshold: don't use write notifiers
+ include/block/raw-aio.h           |  14 ---
-  block: drop write notifiers
+ include/sysemu/block-backend-io.h |  13 +--
-  test-write-threshold: rewrite test_threshold_(not_)trigger tests
+ block/blkio.c                     |  96 ++++++++++++------
-  block/write-threshold: drop extra APIs
+ block/block-backend.c             |  22 -----
-  test-write-threshold: drop extra tests
+ block/file-posix.c                |  38 -------
-  test-write-threshold: drop extra TestStruct structure
+ block/io.c                        |  37 -------
-  write-threshold: deal with includes
+ block/io_uring.c                  |  44 ++++-----
+ block/linux-aio.c                 |  41 +++-----
- docs/tools/qemu-img.rst           |  31 +++++++
+ block/nvme.c                      |  44 +++------
- include/block/block_int.h         |  15 +---
+ block/plug.c                      | 159 ++++++++++++++++++++++++++++++
- include/block/write-threshold.h   |  27 ++----
+ hw/block/dataplane/xen-block.c    |   8 +-
- include/qemu/job.h                |   2 +-
+ hw/block/virtio-blk.c             |   4 +-
- block.c                           |   1 -
+ hw/scsi/virtio-scsi.c             |   6 +-
- block/backup.c                    |   2 +-
+ block/meson.build                 |   1 +
- block/copy-on-read.c              |  33 +------
+ block/trace-events                |   6 +-
- block/io.c                        |  11 +--
+files changed, 293 insertions(+), 265 deletions(-)
- block/mirror.c                    |   6 +-
+ create mode 100644 block/plug.c
  block/monitor/block-hmp-cmds.c    |  31 ++++---
  block/rbd.c                       |  32 ++++---
  block/write-threshold.c           |  91 ++++---------------
  job.c                             |   2 +-
  qemu-io-cmds.c                    |   8 +-
  qemu-io.c                         |  17 +++-
  tests/unit/test-write-threshold.c |  90 ++-----------------
  tests/qemu-iotests/231            |   4 +
  tests/qemu-iotests/231.out        |   7 +-
  tests/qemu-iotests/240.out        |   8 +-
  tests/qemu-iotests/245.out        |   8 +-
  tests/qemu-iotests/264            |   2 +-
  tests/qemu-iotests/295.out        |   6 +-
  tests/qemu-iotests/296.out        |   8 +-
  tests/qemu-iotests/check          |  19 +++-
  tests/qemu-iotests/iotests.py     | 145 +++++++++++++++++-------------
  tests/qemu-iotests/pylintrc       |   3 +
  tests/qemu-iotests/testenv.py     |  22 ++++-
  tests/qemu-iotests/testrunner.py  |  37 +++-----
 files changed, 289 insertions(+), 379 deletions(-)
 --
-.31.1
+.40.1

-[PULL 01/19] iotests/231: Update expected deprecation message
+Deleted patch
-From: Connor Kuehl <ckuehl@redhat.com>
-The deprecation message in the expected output has technically been
-wrong since the wrong version of a patch was applied to it. Because of
-this, the test fails. Correct the expected output so that it passes.
-Signed-off-by: Connor Kuehl <ckuehl@redhat.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
-Message-Id: <20210421212343.85524-2-ckuehl@redhat.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- tests/qemu-iotests/231.out | 4 +---
-file changed, 1 insertion(+), 3 deletions(-)
-diff --git a/tests/qemu-iotests/231.out b/tests/qemu-iotests/231.out
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/231.out
-+++ b/tests/qemu-iotests/231.out
-@@ -XXX,XX +XXX,XX @@
- QA output created by 231
--qemu-img: RBD options encoded in the filename as keyvalue pairs is deprecated.  Future versions may cease to parse these options in the future.
-+qemu-img: warning: RBD options encoded in the filename as keyvalue pairs is deprecated
- unable to get monitor info from DNS SRV with service name: ceph-mon
--no monitors specified to connect to.
- qemu-img: Could not open 'json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=BOGUS_CONF'}': error connecting: No such file or directory
- unable to get monitor info from DNS SRV with service name: ceph-mon
--no monitors specified to connect to.
- qemu-img: Could not open 'json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'BOGUS_CONF'}': error connecting: No such file or directory
- *** done
---
-.31.1

-[PULL 02/19] block/rbd: Add an escape-aware strchr helper
+Deleted patch
-From: Connor Kuehl <ckuehl@redhat.com>
-Sometimes the parser needs to further split a token it has collected
-from the token input stream. Right now, it does a cursory check to see
-if the relevant characters appear in the token to determine if it should
-break it down further.
-However, qemu_rbd_next_tok() will escape characters as it removes tokens
-from the token stream and plain strchr() won't. This can make the
-initial strchr() check slightly misleading since it implies
-qemu_rbd_next_tok() will find the token and split on it, except the
-reality is that qemu_rbd_next_tok() will pass over it if it is escaped.
-Use a custom strchr to avoid mixing escaped and unescaped string
-operations. Furthermore, this code is identical to how
-qemu_rbd_next_tok() seeks its next token, so incorporate this custom
-strchr into the body of that function to reduce duplication.
-Reported-by: Han Han <hhan@redhat.com>
-Fixes: https://bugzilla.redhat.com/1873913
-Signed-off-by: Connor Kuehl <ckuehl@redhat.com>
-Message-Id: <20210421212343.85524-3-ckuehl@redhat.com>
-Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block/rbd.c                | 32 +++++++++++++++++++++-----------
- tests/qemu-iotests/231     |  4 ++++
- tests/qemu-iotests/231.out |  3 +++
-files changed, 28 insertions(+), 11 deletions(-)
-diff --git a/block/rbd.c b/block/rbd.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/rbd.c
-+++ b/block/rbd.c
-@@ -XXX,XX +XXX,XX @@ static int qemu_rbd_connect(rados_t *cluster, rados_ioctx_t *io_ctx,
-                             const char *keypairs, const char *secretid,
-                             Error **errp);
-+static char *qemu_rbd_strchr(char *src, char delim)
-+{
-+    char *p;
-+
-+    for (p = src; *p; ++p) {
-+        if (*p == delim) {
-+            return p;
-+        }
-+        if (*p == '\\' && p[1] != '\0') {
-+            ++p;
-+        }
-+    }
-+
-+    return NULL;
-+}
-+
-+
- static char *qemu_rbd_next_tok(char *src, char delim, char **p)
- {
-     char *end;
-     *p = NULL;
--    for (end = src; *end; ++end) {
--        if (*end == delim) {
--            break;
--        }
--        if (*end == '\\' && end[1] != '\0') {
--            end++;
--        }
--    }
--    if (*end == delim) {
-+    end = qemu_rbd_strchr(src, delim);
-+    if (end) {
-         *p = end + 1;
-         *end = '\0';
-     }
-@@ -XXX,XX +XXX,XX @@ static void qemu_rbd_parse_filename(const char *filename, QDict *options,
-     qemu_rbd_unescape(found_str);
-     qdict_put_str(options, "pool", found_str);
--    if (strchr(p, '@')) {
-+    if (qemu_rbd_strchr(p, '@')) {
-         image_name = qemu_rbd_next_tok(p, '@', &p);
-         found_str = qemu_rbd_next_tok(p, ':', &p);
-@@ -XXX,XX +XXX,XX @@ static void qemu_rbd_parse_filename(const char *filename, QDict *options,
-         image_name = qemu_rbd_next_tok(p, ':', &p);
-     }
-     /* Check for namespace in the image_name */
--    if (strchr(image_name, '/')) {
-+    if (qemu_rbd_strchr(image_name, '/')) {
-         found_str = qemu_rbd_next_tok(image_name, '/', &image_name);
-         qemu_rbd_unescape(found_str);
-         qdict_put_str(options, "namespace", found_str);
-diff --git a/tests/qemu-iotests/231 b/tests/qemu-iotests/231
-index XXXXXXX..XXXXXXX 100755
---- a/tests/qemu-iotests/231
-+++ b/tests/qemu-iotests/231
-@@ -XXX,XX +XXX,XX @@ _filter_conf()
- $QEMU_IMG info "json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=${BOGUS_CONF}'}" 2>&1 | _filter_conf
- $QEMU_IMG info "json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'${BOGUS_CONF}'}" 2>&1 | _filter_conf
-+# Regression test: the qemu-img invocation is expected to fail, but it should
-+# not seg fault the parser.
-+$QEMU_IMG create "rbd:rbd/aa\/bb:conf=${BOGUS_CONF}" 1M 2>&1 | _filter_conf
-+
- # success, all done
- echo "*** done"
- rm -f $seq.full
-diff --git a/tests/qemu-iotests/231.out b/tests/qemu-iotests/231.out
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/231.out
-+++ b/tests/qemu-iotests/231.out
-@@ -XXX,XX +XXX,XX @@ unable to get monitor info from DNS SRV with service name: ceph-mon
- qemu-img: Could not open 'json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=BOGUS_CONF'}': error connecting: No such file or directory
- unable to get monitor info from DNS SRV with service name: ceph-mon
- qemu-img: Could not open 'json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'BOGUS_CONF'}': error connecting: No such file or directory
-+Formatting 'rbd:rbd/aa\/bb:conf=BOGUS_CONF', fmt=raw size=1048576
-+unable to get monitor info from DNS SRV with service name: ceph-mon
-+qemu-img: rbd:rbd/aa\/bb:conf=BOGUS_CONF: error connecting: No such file or directory
- *** done
---
-.31.1

-[PULL 16/19] block/write-threshold: drop extra APIs
+[PULL 1/8] block: add blk_io_plug_call() API
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+Introduce a new API for thread-local blk_io_plug() that does not
+traverse the block graph. The goal is to make blk_io_plug() multi-queue
-bdrv_write_threshold_exceeded() is unused.
+friendly.
-bdrv_write_threshold_is_set() is used only to double check the value of
+Instead of having block drivers track whether or not we're in a plugged
-bs->write_threshold_offset in tests. No real sense in it (both tests do
+section, provide an API that allows them to defer a function call until
-check real value with help of bdrv_write_threshold_get())
+we're unplugged: blk_io_plug_call(fn, opaque). If blk_io_plug_call() is
+called multiple times with the same fn/opaque pair, then fn() is only
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+called once at the end of the function - resulting in batching.
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Message-Id: <20210506090621.11848-5-vsementsov@virtuozzo.com>
+This patch introduces the API and changes blk_io_plug()/blk_io_unplug().
 blk_io_plug()/blk_io_unplug() no longer require a BlockBackend argument
 because the plug state is now thread-local.
 Later patches convert block drivers to blk_io_plug_call() and then we
 can finally remove .bdrv_co_io_plug() once all block drivers have been
 converted.
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 Reviewed-by: Eric Blake <eblake@redhat.com>
-Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
+Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
-[mreitz: Adjusted commit message as per Eric's suggestion]
+Acked-by: Kevin Wolf <kwolf@redhat.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+Message-id: 20230530180959.1108766-2-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- include/block/write-threshold.h   | 24 ------------------------
+ MAINTAINERS                       |   1 +
- block/write-threshold.c           | 19 -------------------
+ include/sysemu/block-backend-io.h |  13 +--
- tests/unit/test-write-threshold.c |  4 ----
+ block/block-backend.c             |  22 -----
-files changed, 47 deletions(-)
+ block/plug.c                      | 159 ++++++++++++++++++++++++++++++
+ hw/block/dataplane/xen-block.c    |   8 +-
-diff --git a/include/block/write-threshold.h b/include/block/write-threshold.h
+ hw/block/virtio-blk.c             |   4 +-
-index XXXXXXX..XXXXXXX 100644
+ hw/scsi/virtio-scsi.c             |   6 +-
---- a/include/block/write-threshold.h
+ block/meson.build                 |   1 +
-+++ b/include/block/write-threshold.h
+files changed, 173 insertions(+), 41 deletions(-)
-@@ -XXX,XX +XXX,XX @@ void bdrv_write_threshold_set(BlockDriverState *bs, uint64_t threshold_bytes);
+ create mode 100644 block/plug.c
-  */
- uint64_t bdrv_write_threshold_get(const BlockDriverState *bs);
+diff --git a/MAINTAINERS b/MAINTAINERS
 index XXXXXXX..XXXXXXX 100644
 --- a/MAINTAINERS
 +++ b/MAINTAINERS
@@ -XXX,XX +XXX,XX @@ F: util/aio-*.c
  F: util/aio-*.h
  F: util/fdmon-*.c
  F: block/io.c
 +F: block/plug.c
  F: migration/block*
  F: include/block/aio.h
  F: include/block/aio-wait.h
 diff --git a/include/sysemu/block-backend-io.h b/include/sysemu/block-backend-io.h
 index XXXXXXX..XXXXXXX 100644
 --- a/include/sysemu/block-backend-io.h
 +++ b/include/sysemu/block-backend-io.h
@@ -XXX,XX +XXX,XX @@ void blk_iostatus_set_err(BlockBackend *blk, int error);
  int blk_get_max_iov(BlockBackend *blk);
  int blk_get_max_hw_iov(BlockBackend *blk);
 -/*
-- * bdrv_write_threshold_is_set
+- * blk_io_plug/unplug are thread-local operations. This means that multiple
-- *
+- * IOThreads can simultaneously call plug/unplug, but the caller must ensure
-- * Tell if a write threshold is set for a given BDS.
+- * that each unplug() is called in the same IOThread of the matching plug().
 - */
--bool bdrv_write_threshold_is_set(const BlockDriverState *bs);
+-void coroutine_fn blk_co_io_plug(BlockBackend *blk);
--
+-void co_wrapper blk_io_plug(BlockBackend *blk);
--/*
+-
-- * bdrv_write_threshold_exceeded
+-void coroutine_fn blk_co_io_unplug(BlockBackend *blk);
-- *
+-void co_wrapper blk_io_unplug(BlockBackend *blk);
-- * Return the extent of a write request that exceeded the threshold,
++void blk_io_plug(void);
-- * or zero if the request is below the threshold.
++void blk_io_unplug(void);
-- * Return zero also if the threshold was not set.
++void blk_io_plug_call(void (*fn)(void *), void *opaque);
-- *
-- * NOTE: here we assume the following holds for each request this code
+ AioContext *blk_get_aio_context(BlockBackend *blk);
-- * deals with:
+ BlockAcctStats *blk_get_stats(BlockBackend *blk);
-- *
+diff --git a/block/block-backend.c b/block/block-backend.c
-- * assert((req->offset + req->bytes) <= UINT64_MAX)
+index XXXXXXX..XXXXXXX 100644
-- *
+--- a/block/block-backend.c
-- * Please not there is *not* an actual C assert().
++++ b/block/block-backend.c
-- */
+@@ -XXX,XX +XXX,XX @@ void blk_add_insert_bs_notifier(BlockBackend *blk, Notifier *notify)
--uint64_t bdrv_write_threshold_exceeded(const BlockDriverState *bs,
+     notifier_list_add(&blk->insert_bs_notifiers, notify);
 -                                       const BdrvTrackedRequest *req);
 -
  /*
   * bdrv_write_threshold_check_write
   *
 diff --git a/block/write-threshold.c b/block/write-threshold.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/write-threshold.c
 +++ b/block/write-threshold.c
@@ -XXX,XX +XXX,XX @@ uint64_t bdrv_write_threshold_get(const BlockDriverState *bs)
      return bs->write_threshold_offset;
  }
--bool bdrv_write_threshold_is_set(const BlockDriverState *bs)
+-void coroutine_fn blk_co_io_plug(BlockBackend *blk)
 -{
--    return bs->write_threshold_offset > 0;
+-    BlockDriverState *bs = blk_bs(blk);
 -    IO_CODE();
 -    GRAPH_RDLOCK_GUARD();
 -
 -    if (bs) {
 -        bdrv_co_io_plug(bs);
 -    }
 -}
 -
--uint64_t bdrv_write_threshold_exceeded(const BlockDriverState *bs,
+-void coroutine_fn blk_co_io_unplug(BlockBackend *blk)
 -                                       const BdrvTrackedRequest *req)
 -{
--    if (bdrv_write_threshold_is_set(bs)) {
+-    BlockDriverState *bs = blk_bs(blk);
--        if (req->offset > bs->write_threshold_offset) {
+-    IO_CODE();
--            return (req->offset - bs->write_threshold_offset) + req->bytes;
+-    GRAPH_RDLOCK_GUARD();
--        }
+-
--        if ((req->offset + req->bytes) > bs->write_threshold_offset) {
+-    if (bs) {
--            return (req->offset + req->bytes) - bs->write_threshold_offset;
+-        bdrv_co_io_unplug(bs);
 -        }
 -    }
--    return 0;
 -}
 -
- void bdrv_write_threshold_set(BlockDriverState *bs, uint64_t threshold_bytes)
+ BlockAcctStats *blk_get_stats(BlockBackend *blk)
  {
-     bs->write_threshold_offset = threshold_bytes;
+     IO_CODE();
-diff --git a/tests/unit/test-write-threshold.c b/tests/unit/test-write-threshold.c
+diff --git a/block/plug.c b/block/plug.c
-index XXXXXXX..XXXXXXX 100644
+new file mode 100644
---- a/tests/unit/test-write-threshold.c
+index XXXXXXX..XXXXXXX
-+++ b/tests/unit/test-write-threshold.c
+--- /dev/null
-@@ -XXX,XX +XXX,XX @@ static void test_threshold_not_set_on_init(void)
++++ b/block/plug.c
-     BlockDriverState bs;
+@@ -XXX,XX +XXX,XX @@
-     memset(&bs, 0, sizeof(bs));
++/* SPDX-License-Identifier: GPL-2.0-or-later */
++/*
--    g_assert(!bdrv_write_threshold_is_set(&bs));
++ * Block I/O plugging
--
++ *
-     res = bdrv_write_threshold_get(&bs);
++ * Copyright Red Hat.
-     g_assert_cmpint(res, ==, 0);
++ *
 + * This API defers a function call within a blk_io_plug()/blk_io_unplug()
 + * section, allowing multiple calls to batch up. This is a performance
 + * optimization that is used in the block layer to submit several I/O requests
 + * at once instead of individually:
 + *
 + *   blk_io_plug(); <-- start of plugged region
 + *   ...
 + *   blk_io_plug_call(my_func, my_obj); <-- deferred my_func(my_obj) call
 + *   blk_io_plug_call(my_func, my_obj); <-- another
 + *   blk_io_plug_call(my_func, my_obj); <-- another
 + *   ...
 + *   blk_io_unplug(); <-- end of plugged region, my_func(my_obj) is called once
 + *
 + * This code is actually generic and not tied to the block layer. If another
 + * subsystem needs this functionality, it could be renamed.
 + */
 +
 +#include "qemu/osdep.h"
 +#include "qemu/coroutine-tls.h"
 +#include "qemu/notify.h"
 +#include "qemu/thread.h"
 +#include "sysemu/block-backend.h"
 +
 +/* A function call that has been deferred until unplug() */
 +typedef struct {
 +    void (*fn)(void *);
 +    void *opaque;
 +} UnplugFn;
 +
 +/* Per-thread state */
 +typedef struct {
 +    unsigned count;       /* how many times has plug() been called? */
 +    GArray *unplug_fns;   /* functions to call at unplug time */
 +} Plug;
 +
 +/* Use get_ptr_plug() to fetch this thread-local value */
 +QEMU_DEFINE_STATIC_CO_TLS(Plug, plug);
 +
 +/* Called at thread cleanup time */
 +static void blk_io_plug_atexit(Notifier *n, void *value)
 +{
 +    Plug *plug = get_ptr_plug();
 +    g_array_free(plug->unplug_fns, TRUE);
 +}
 +
 +/* This won't involve coroutines, so use __thread */
 +static __thread Notifier blk_io_plug_atexit_notifier;
 +
 +/**
 + * blk_io_plug_call:
 + * @fn: a function pointer to be invoked
 + * @opaque: a user-defined argument to @fn()
 + *
 + * Call @fn(@opaque) immediately if not within a blk_io_plug()/blk_io_unplug()
 + * section.
 + *
 + * Otherwise defer the call until the end of the outermost
 + * blk_io_plug()/blk_io_unplug() section in this thread. If the same
 + * @fn/@opaque pair has already been deferred, it will only be called once upon
 + * blk_io_unplug() so that accumulated calls are batched into a single call.
 + *
 + * The caller must ensure that @opaque is not freed before @fn() is invoked.
 + */
 +void blk_io_plug_call(void (*fn)(void *), void *opaque)
 +{
 +    Plug *plug = get_ptr_plug();
 +
 +    /* Call immediately if we're not plugged */
 +    if (plug->count == 0) {
 +        fn(opaque);
 +        return;
 +    }
 +
 +    GArray *array = plug->unplug_fns;
 +    if (!array) {
 +        array = g_array_new(FALSE, FALSE, sizeof(UnplugFn));
 +        plug->unplug_fns = array;
 +        blk_io_plug_atexit_notifier.notify = blk_io_plug_atexit;
 +        qemu_thread_atexit_add(&blk_io_plug_atexit_notifier);
 +    }
 +
 +    UnplugFn *fns = (UnplugFn *)array->data;
 +    UnplugFn new_fn = {
 +        .fn = fn,
 +        .opaque = opaque,
 +    };
 +
 +    /*
 +     * There won't be many, so do a linear search. If this becomes a bottleneck
 +     * then a binary search (glib 2.62+) or different data structure could be
 +     * used.
 +     */
 +    for (guint i = 0; i < array->len; i++) {
 +        if (memcmp(&fns[i], &new_fn, sizeof(new_fn)) == 0) {
 +            return; /* already exists */
 +        }
 +    }
 +
 +    g_array_append_val(array, new_fn);
 +}
 +
 +/**
 + * blk_io_plug: Defer blk_io_plug_call() functions until blk_io_unplug()
 + *
 + * blk_io_plug/unplug are thread-local operations. This means that multiple
 + * threads can simultaneously call plug/unplug, but the caller must ensure that
 + * each unplug() is called in the same thread of the matching plug().
 + *
 + * Nesting is supported. blk_io_plug_call() functions are only called at the
 + * outermost blk_io_unplug().
 + */
 +void blk_io_plug(void)
 +{
 +    Plug *plug = get_ptr_plug();
 +
 +    assert(plug->count < UINT32_MAX);
 +
 +    plug->count++;
 +}
 +
 +/**
 + * blk_io_unplug: Run any pending blk_io_plug_call() functions
 + *
 + * There must have been a matching blk_io_plug() call in the same thread prior
 + * to this blk_io_unplug() call.
 + */
 +void blk_io_unplug(void)
 +{
 +    Plug *plug = get_ptr_plug();
 +
 +    assert(plug->count > 0);
 +
 +    if (--plug->count > 0) {
 +        return;
 +    }
 +
 +    GArray *array = plug->unplug_fns;
 +    if (!array) {
 +        return;
 +    }
 +
 +    UnplugFn *fns = (UnplugFn *)array->data;
 +
 +    for (guint i = 0; i < array->len; i++) {
 +        fns[i].fn(fns[i].opaque);
 +    }
 +
 +    /*
 +     * This resets the array without freeing memory so that appending is cheap
 +     * in the future.
 +     */
 +    g_array_set_size(array, 0);
 +}
 diff --git a/hw/block/dataplane/xen-block.c b/hw/block/dataplane/xen-block.c
 index XXXXXXX..XXXXXXX 100644
 --- a/hw/block/dataplane/xen-block.c
 +++ b/hw/block/dataplane/xen-block.c
@@ -XXX,XX +XXX,XX @@ static bool xen_block_handle_requests(XenBlockDataPlane *dataplane)
       * is below us.
       */
      if (inflight_atstart > IO_PLUG_THRESHOLD) {
 -        blk_io_plug(dataplane->blk);
 +        blk_io_plug();
      }
      while (rc != rp) {
          /* pull request from ring */
@@ -XXX,XX +XXX,XX @@ static bool xen_block_handle_requests(XenBlockDataPlane *dataplane)
          if (inflight_atstart > IO_PLUG_THRESHOLD &&
              batched >= inflight_atstart) {
 -            blk_io_unplug(dataplane->blk);
 +            blk_io_unplug();
          }
          xen_block_do_aio(request);
          if (inflight_atstart > IO_PLUG_THRESHOLD) {
              if (batched >= inflight_atstart) {
 -                blk_io_plug(dataplane->blk);
 +                blk_io_plug();
                  batched = 0;
              } else {
                  batched++;
@@ -XXX,XX +XXX,XX @@ static bool xen_block_handle_requests(XenBlockDataPlane *dataplane)
          }
      }
      if (inflight_atstart > IO_PLUG_THRESHOLD) {
 -        blk_io_unplug(dataplane->blk);
 +        blk_io_unplug();
      }
      return done_something;
 diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
 index XXXXXXX..XXXXXXX 100644
 --- a/hw/block/virtio-blk.c
 +++ b/hw/block/virtio-blk.c
@@ -XXX,XX +XXX,XX @@ void virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq)
      bool suppress_notifications = virtio_queue_get_notification(vq);
      aio_context_acquire(blk_get_aio_context(s->blk));
 -    blk_io_plug(s->blk);
 +    blk_io_plug();
      do {
          if (suppress_notifications) {
@@ -XXX,XX +XXX,XX @@ void virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq)
          virtio_blk_submit_multireq(s, &mrb);
      }
 -    blk_io_unplug(s->blk);
 +    blk_io_unplug();
      aio_context_release(blk_get_aio_context(s->blk));
  }
-@@ -XXX,XX +XXX,XX @@ static void test_threshold_set_get(void)
+diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
-     bdrv_write_threshold_set(&bs, threshold);
+index XXXXXXX..XXXXXXX 100644
+--- a/hw/scsi/virtio-scsi.c
--    g_assert(bdrv_write_threshold_is_set(&bs));
++++ b/hw/scsi/virtio-scsi.c
--
+@@ -XXX,XX +XXX,XX @@ static int virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req)
-     res = bdrv_write_threshold_get(&bs);
+         return -ENOBUFS;
-     g_assert_cmpint(res, ==, threshold);
+     }
      scsi_req_ref(req->sreq);
 -    blk_io_plug(d->conf.blk);
 +    blk_io_plug();
      object_unref(OBJECT(d));
      return 0;
  }
+@@ -XXX,XX +XXX,XX @@ static void virtio_scsi_handle_cmd_req_submit(VirtIOSCSI *s, VirtIOSCSIReq *req)
+     if (scsi_req_enqueue(sreq)) {
+         scsi_req_continue(sreq);
+     }
+-    blk_io_unplug(sreq->dev->conf.blk);
++    blk_io_unplug();
+     scsi_req_unref(sreq);
+ }
+@@ -XXX,XX +XXX,XX @@ static void virtio_scsi_handle_cmd_vq(VirtIOSCSI *s, VirtQueue *vq)
+                 while (!QTAILQ_EMPTY(&reqs)) {
+                     req = QTAILQ_FIRST(&reqs);
+                     QTAILQ_REMOVE(&reqs, req, next);
+-                    blk_io_unplug(req->sreq->dev->conf.blk);
++                    blk_io_unplug();
+                     scsi_req_unref(req->sreq);
+                     virtqueue_detach_element(req->vq, &req->elem, 0);
+                     virtio_scsi_free_req(req);
+diff --git a/block/meson.build b/block/meson.build
+index XXXXXXX..XXXXXXX 100644
+--- a/block/meson.build
++++ b/block/meson.build
+@@ -XXX,XX +XXX,XX @@ block_ss.add(files(
+   'mirror.c',
+   'nbd.c',
+   'null.c',
++  'plug.c',
+   'qapi.c',
+   'qcow2-bitmap.c',
+   'qcow2-cache.c',
 --
-.31.1
+.40.1

-[PULL 17/19] test-write-threshold: drop extra tests
+[PULL 2/8] block/nvme: convert to blk_io_plug_call() API
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+Stop using the .bdrv_co_io_plug() API because it is not multi-queue
 block layer friendly. Use the new blk_io_plug_call() API to batch I/O
 submission instead.
-Testing set/get of one 64bit variable doesn't seem necessary. We have a
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-lot of such variables. Also remaining tests do test set/get anyway.
+Reviewed-by: Eric Blake <eblake@redhat.com>
 Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
 Acked-by: Kevin Wolf <kwolf@redhat.com>
 Message-id: 20230530180959.1108766-3-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
  block/nvme.c       | 44 ++++++++++++--------------------------------
  block/trace-events |  1 -
 files changed, 12 insertions(+), 33 deletions(-)
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+diff --git a/block/nvme.c b/block/nvme.c
 Reviewed-by: Max Reitz <mreitz@redhat.com>
 Message-Id: <20210506090621.11848-7-vsementsov@virtuozzo.com>
 Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  tests/unit/test-write-threshold.c | 43 -------------------------------
 file changed, 43 deletions(-)
 diff --git a/tests/unit/test-write-threshold.c b/tests/unit/test-write-threshold.c
 index XXXXXXX..XXXXXXX 100644
---- a/tests/unit/test-write-threshold.c
+--- a/block/nvme.c
-+++ b/tests/unit/test-write-threshold.c
++++ b/block/nvme.c
 @@ -XXX,XX +XXX,XX @@
- #include "block/write-threshold.h"
+ #include "qemu/vfio-helpers.h"
+ #include "block/block-io.h"
+ #include "block/block_int.h"
--static void test_threshold_not_set_on_init(void)
++#include "sysemu/block-backend.h"
  #include "sysemu/replay.h"
  #include "trace.h"
@@ -XXX,XX +XXX,XX @@ struct BDRVNVMeState {
      int blkshift;
      uint64_t max_transfer;
 -    bool plugged;
      bool supports_write_zeroes;
      bool supports_discard;
@@ -XXX,XX +XXX,XX @@ static void nvme_kick(NVMeQueuePair *q)
  {
      BDRVNVMeState *s = q->s;
 -    if (s->plugged || !q->need_kick) {
 +    if (!q->need_kick) {
          return;
      }
      trace_nvme_kick(s, q->index);
@@ -XXX,XX +XXX,XX @@ static bool nvme_process_completion(NVMeQueuePair *q)
      NvmeCqe *c;
      trace_nvme_process_completion(s, q->index, q->inflight);
 -    if (s->plugged) {
 -        trace_nvme_process_completion_queue_plugged(s, q->index);
 -        return false;
 -    }
      /*
       * Support re-entrancy when a request cb() function invokes aio_poll().
@@ -XXX,XX +XXX,XX @@ static void nvme_trace_command(const NvmeCmd *cmd)
      }
  }
 +static void nvme_unplug_fn(void *opaque)
 +{
 +    NVMeQueuePair *q = opaque;
 +
 +    QEMU_LOCK_GUARD(&q->lock);
 +    nvme_kick(q);
 +    nvme_process_completion(q);
 +}
 +
  static void nvme_submit_command(NVMeQueuePair *q, NVMeRequest *req,
                                  NvmeCmd *cmd, BlockCompletionFunc cb,
                                  void *opaque)
@@ -XXX,XX +XXX,XX @@ static void nvme_submit_command(NVMeQueuePair *q, NVMeRequest *req,
             q->sq.tail * NVME_SQ_ENTRY_BYTES, cmd, sizeof(*cmd));
      q->sq.tail = (q->sq.tail + 1) % NVME_QUEUE_SIZE;
      q->need_kick++;
 -    nvme_kick(q);
 -    nvme_process_completion(q);
 +    blk_io_plug_call(nvme_unplug_fn, q);
      qemu_mutex_unlock(&q->lock);
  }
@@ -XXX,XX +XXX,XX @@ static void nvme_attach_aio_context(BlockDriverState *bs,
      }
  }
 -static void coroutine_fn nvme_co_io_plug(BlockDriverState *bs)
 -{
--    uint64_t res;
+-    BDRVNVMeState *s = bs->opaque;
--    BlockDriverState bs;
+-    assert(!s->plugged);
--    memset(&bs, 0, sizeof(bs));
+-    s->plugged = true;
 -
 -    res = bdrv_write_threshold_get(&bs);
 -    g_assert_cmpint(res, ==, 0);
 -}
 -
--static void test_threshold_set_get(void)
+-static void coroutine_fn nvme_co_io_unplug(BlockDriverState *bs)
 -{
--    uint64_t threshold = 4 * 1024 * 1024;
+-    BDRVNVMeState *s = bs->opaque;
--    uint64_t res;
+-    assert(s->plugged);
--    BlockDriverState bs;
+-    s->plugged = false;
--    memset(&bs, 0, sizeof(bs));
+-    for (unsigned i = INDEX_IO(0); i < s->queue_count; i++) {
--
+-        NVMeQueuePair *q = s->queues[i];
--    bdrv_write_threshold_set(&bs, threshold);
+-        qemu_mutex_lock(&q->lock);
--
+-        nvme_kick(q);
--    res = bdrv_write_threshold_get(&bs);
+-        nvme_process_completion(q);
--    g_assert_cmpint(res, ==, threshold);
+-        qemu_mutex_unlock(&q->lock);
 -    }
 -}
 -
--static void test_threshold_multi_set_get(void)
+ static bool nvme_register_buf(BlockDriverState *bs, void *host, size_t size,
--{
+                               Error **errp)
--    uint64_t threshold1 = 4 * 1024 * 1024;
+ {
--    uint64_t threshold2 = 15 * 1024 * 1024;
+@@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_nvme = {
--    uint64_t res;
+     .bdrv_detach_aio_context  = nvme_detach_aio_context,
--    BlockDriverState bs;
+     .bdrv_attach_aio_context  = nvme_attach_aio_context,
--    memset(&bs, 0, sizeof(bs));
 -    .bdrv_co_io_plug          = nvme_co_io_plug,
 -    .bdrv_co_io_unplug        = nvme_co_io_unplug,
 -
--    bdrv_write_threshold_set(&bs, threshold1);
+     .bdrv_register_buf        = nvme_register_buf,
--    bdrv_write_threshold_set(&bs, threshold2);
+     .bdrv_unregister_buf      = nvme_unregister_buf,
--    res = bdrv_write_threshold_get(&bs);
+ };
--    g_assert_cmpint(res, ==, threshold2);
+diff --git a/block/trace-events b/block/trace-events
--}
+index XXXXXXX..XXXXXXX 100644
--
+--- a/block/trace-events
- static void test_threshold_not_trigger(void)
++++ b/block/trace-events
- {
+@@ -XXX,XX +XXX,XX @@ nvme_kick(void *s, unsigned q_index) "s %p q #%u"
-     uint64_t threshold = 4 * 1024 * 1024;
+ nvme_dma_flush_queue_wait(void *s) "s %p"
-@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv)
+ nvme_error(int cmd_specific, int sq_head, int sqid, int cid, int status) "cmd_specific %d sq_head %d sqid %d cid %d status 0x%x"
- {
+ nvme_process_completion(void *s, unsigned q_index, int inflight) "s %p q #%u inflight %d"
-     size_t i;
+-nvme_process_completion_queue_plugged(void *s, unsigned q_index) "s %p q #%u"
-     TestStruct tests[] = {
+ nvme_complete_command(void *s, unsigned q_index, int cid) "s %p q #%u cid %d"
--        { "/write-threshold/not-set-on-init",
+ nvme_submit_command(void *s, unsigned q_index, int cid) "s %p q #%u cid %d"
--          test_threshold_not_set_on_init },
+ nvme_submit_command_raw(int c0, int c1, int c2, int c3, int c4, int c5, int c6, int c7) "%02x %02x %02x %02x %02x %02x %02x %02x"
 -        { "/write-threshold/set-get",
 -          test_threshold_set_get },
 -        { "/write-threshold/multi-set-get",
 -          test_threshold_multi_set_get },
          { "/write-threshold/not-trigger",
            test_threshold_not_trigger },
          { "/write-threshold/trigger",
 --
-.31.1
+.40.1

-[PULL 04/19] mirror: stop cancelling in-flight requests on non-force cancel in READY
+[PULL 3/8] block/blkio: convert to blk_io_plug_call() API
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+Stop using the .bdrv_co_io_plug() API because it is not multi-queue
 block layer friendly. Use the new blk_io_plug_call() API to batch I/O
 submission instead.
-If mirror is READY than cancel operation is not discarding the whole
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-result of the operation, but instead it's a documented way get a
+Reviewed-by: Eric Blake <eblake@redhat.com>
-point-in-time snapshot of source disk.
+Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
 Acked-by: Kevin Wolf <kwolf@redhat.com>
 Message-id: 20230530180959.1108766-4-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
  block/blkio.c | 43 ++++++++++++++++++++++++-------------------
 file changed, 24 insertions(+), 19 deletions(-)
-So, we should not cancel any requests if mirror is READ and
+diff --git a/block/blkio.c b/block/blkio.c
 force=false. Let's fix that case.
 Note, that bug that we have before this commit is not critical, as the
 only .bdrv_cancel_in_flight implementation is nbd_cancel_in_flight()
 and it cancels only requests waiting for reconnection, so it should be
 rare case.
 Fixes: 521ff8b779b11c394dbdc43f02e158dd99df308a
 Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
 Message-Id: <20210421075858.40197-1-vsementsov@virtuozzo.com>
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  include/block/block_int.h | 2 +-
  include/qemu/job.h        | 2 +-
  block/backup.c            | 2 +-
  block/mirror.c            | 6 ++++--
  job.c                     | 2 +-
  tests/qemu-iotests/264    | 2 +-
 files changed, 9 insertions(+), 7 deletions(-)
 diff --git a/include/block/block_int.h b/include/block/block_int.h
 index XXXXXXX..XXXXXXX 100644
---- a/include/block/block_int.h
+--- a/block/blkio.c
-+++ b/include/block/block_int.h
++++ b/block/blkio.c
-@@ -XXX,XX +XXX,XX @@ struct BlockDriver {
+@@ -XXX,XX +XXX,XX @@
-      * of in-flight requests, so don't waste the time if possible.
+ #include "qemu/error-report.h"
-      *
+ #include "qapi/qmp/qdict.h"
-      * One example usage is to avoid waiting for an nbd target node reconnect
+ #include "qemu/module.h"
--     * timeout during job-cancel.
++#include "sysemu/block-backend.h"
-+     * timeout during job-cancel with force=true.
+ #include "exec/memory.h" /* for ram_block_discard_disable() */
-      */
-     void (*bdrv_cancel_in_flight)(BlockDriverState *bs);
+ #include "block/block-io.h"
+@@ -XXX,XX +XXX,XX @@ static void blkio_detach_aio_context(BlockDriverState *bs)
-diff --git a/include/qemu/job.h b/include/qemu/job.h
+                        NULL, NULL, NULL);
-index XXXXXXX..XXXXXXX 100644
+ }
---- a/include/qemu/job.h
-+++ b/include/qemu/job.h
+-/* Call with s->blkio_lock held to submit I/O after enqueuing a new request */
-@@ -XXX,XX +XXX,XX @@ struct JobDriver {
+-static void blkio_submit_io(BlockDriverState *bs)
-     /**
++/*
-      * If the callback is not NULL, it will be invoked in job_cancel_async
++ * Called by blk_io_unplug() or immediately if not plugged. Called without
-      */
++ * blkio_lock.
--    void (*cancel)(Job *job);
++ */
-+    void (*cancel)(Job *job, bool force);
++static void blkio_unplug_fn(void *opaque)
+ {
+-    if (qatomic_read(&bs->io_plugged) == 0) {
-     /** Called when the job is freed */
+-        BDRVBlkioState *s = bs->opaque;
-diff --git a/block/backup.c b/block/backup.c
++    BDRVBlkioState *s = opaque;
-index XXXXXXX..XXXXXXX 100644
---- a/block/backup.c
++    WITH_QEMU_LOCK_GUARD(&s->blkio_lock) {
-+++ b/block/backup.c
+         blkioq_do_io(s->blkioq, NULL, 0, 0, NULL);
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn backup_set_speed(BlockJob *job, int64_t speed)
      }
  }
--static void backup_cancel(Job *job)
++/*
-+static void backup_cancel(Job *job, bool force)
++ * Schedule I/O submission after enqueuing a new request. Called without
 + * blkio_lock.
 + */
 +static void blkio_submit_io(BlockDriverState *bs)
 +{
 +    BDRVBlkioState *s = bs->opaque;
 +
 +    blk_io_plug_call(blkio_unplug_fn, s);
 +}
 +
  static int coroutine_fn
  blkio_co_pdiscard(BlockDriverState *bs, int64_t offset, int64_t bytes)
  {
-     BackupBlockJob *s = container_of(job, BackupBlockJob, common.job);
+@@ -XXX,XX +XXX,XX @@ blkio_co_pdiscard(BlockDriverState *bs, int64_t offset, int64_t bytes)
-diff --git a/block/mirror.c b/block/mirror.c
+     WITH_QEMU_LOCK_GUARD(&s->blkio_lock) {
-index XXXXXXX..XXXXXXX 100644
+         blkioq_discard(s->blkioq, offset, bytes, &cod, 0);
---- a/block/mirror.c
+-        blkio_submit_io(bs);
-+++ b/block/mirror.c
+     }
-@@ -XXX,XX +XXX,XX @@ static bool mirror_drained_poll(BlockJob *job)
-     return !!s->in_flight;
++    blkio_submit_io(bs);
      qemu_coroutine_yield();
      return cod.ret;
  }
+@@ -XXX,XX +XXX,XX @@ blkio_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
--static void mirror_cancel(Job *job)
-+static void mirror_cancel(Job *job, bool force)
+     WITH_QEMU_LOCK_GUARD(&s->blkio_lock) {
- {
+         blkioq_readv(s->blkioq, offset, iov, iovcnt, &cod, 0);
-     MirrorBlockJob *s = container_of(job, MirrorBlockJob, common.job);
+-        blkio_submit_io(bs);
-     BlockDriverState *target = blk_bs(s->target);
+     }
--    bdrv_cancel_in_flight(target);
++    blkio_submit_io(bs);
-+    if (force || !job_is_ready(job)) {
+     qemu_coroutine_yield();
-+        bdrv_cancel_in_flight(target);
-+    }
+     if (use_bounce_buffer) {
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn blkio_co_pwritev(BlockDriverState *bs, int64_t offset,
      WITH_QEMU_LOCK_GUARD(&s->blkio_lock) {
          blkioq_writev(s->blkioq, offset, iov, iovcnt, &cod, blkio_flags);
 -        blkio_submit_io(bs);
      }
 +    blkio_submit_io(bs);
      qemu_coroutine_yield();
      if (use_bounce_buffer) {
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn blkio_co_flush(BlockDriverState *bs)
      WITH_QEMU_LOCK_GUARD(&s->blkio_lock) {
          blkioq_flush(s->blkioq, &cod, 0);
 -        blkio_submit_io(bs);
      }
 +    blkio_submit_io(bs);
      qemu_coroutine_yield();
      return cod.ret;
  }
+@@ -XXX,XX +XXX,XX @@ static int coroutine_fn blkio_co_pwrite_zeroes(BlockDriverState *bs,
- static const BlockJobDriver mirror_job_driver = {
-diff --git a/job.c b/job.c
+     WITH_QEMU_LOCK_GUARD(&s->blkio_lock) {
-index XXXXXXX..XXXXXXX 100644
+         blkioq_write_zeroes(s->blkioq, offset, bytes, &cod, blkio_flags);
---- a/job.c
+-        blkio_submit_io(bs);
 +++ b/job.c
@@ -XXX,XX +XXX,XX @@ static int job_finalize_single(Job *job)
  static void job_cancel_async(Job *job, bool force)
  {
      if (job->driver->cancel) {
 -        job->driver->cancel(job);
 +        job->driver->cancel(job, force);
      }
-     if (job->user_paused) {
-         /* Do not call job_enter here, the caller will handle it.  */
++    blkio_submit_io(bs);
-diff --git a/tests/qemu-iotests/264 b/tests/qemu-iotests/264
+     qemu_coroutine_yield();
-index XXXXXXX..XXXXXXX 100755
+     return cod.ret;
---- a/tests/qemu-iotests/264
+ }
-+++ b/tests/qemu-iotests/264
-@@ -XXX,XX +XXX,XX @@ class TestNbdReconnect(iotests.QMPTestCase):
+-static void coroutine_fn blkio_co_io_unplug(BlockDriverState *bs)
-             self.assert_qmp(result, 'return', {})
+-{
+-    BDRVBlkioState *s = bs->opaque;
-     def cancel_job(self):
+-
--        result = self.vm.qmp('block-job-cancel', device='drive0')
+-    WITH_QEMU_LOCK_GUARD(&s->blkio_lock) {
-+        result = self.vm.qmp('block-job-cancel', device='drive0', force=True)
+-        blkio_submit_io(bs);
-         self.assert_qmp(result, 'return', {})
+-    }
+-}
-         start_t = time.time()
+-
  typedef enum {
      BMRR_OK,
      BMRR_SKIP,
@@ -XXX,XX +XXX,XX @@ static void blkio_refresh_limits(BlockDriverState *bs, Error **errp)
          .bdrv_co_pwritev         = blkio_co_pwritev, \
          .bdrv_co_flush_to_disk   = blkio_co_flush, \
          .bdrv_co_pwrite_zeroes   = blkio_co_pwrite_zeroes, \
 -        .bdrv_co_io_unplug       = blkio_co_io_unplug, \
          .bdrv_refresh_limits     = blkio_refresh_limits, \
          .bdrv_register_buf       = blkio_register_buf, \
          .bdrv_unregister_buf     = blkio_unregister_buf, \
 --
-.31.1
+.40.1

-[PULL 18/19] test-write-threshold: drop extra TestStruct structure
+[PULL 4/8] block/io_uring: convert to blk_io_plug_call() API
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+Stop using the .bdrv_co_io_plug() API because it is not multi-queue
 block layer friendly. Use the new blk_io_plug_call() API to batch I/O
 submission instead.
-We don't need this extra logic: it doesn't make code simpler.
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 Reviewed-by: Eric Blake <eblake@redhat.com>
 Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
 Acked-by: Kevin Wolf <kwolf@redhat.com>
 Message-id: 20230530180959.1108766-5-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
  include/block/raw-aio.h |  7 -------
  block/file-posix.c      | 10 ----------
  block/io_uring.c        | 44 ++++++++++++++++-------------------------
  block/trace-events      |  5 ++---
 files changed, 19 insertions(+), 47 deletions(-)
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+diff --git a/include/block/raw-aio.h b/include/block/raw-aio.h
 Reviewed-by: Max Reitz <mreitz@redhat.com>
 Message-Id: <20210506090621.11848-8-vsementsov@virtuozzo.com>
 Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  tests/unit/test-write-threshold.c | 20 +++-----------------
 file changed, 3 insertions(+), 17 deletions(-)
 diff --git a/tests/unit/test-write-threshold.c b/tests/unit/test-write-threshold.c
 index XXXXXXX..XXXXXXX 100644
---- a/tests/unit/test-write-threshold.c
+--- a/include/block/raw-aio.h
-+++ b/tests/unit/test-write-threshold.c
++++ b/include/block/raw-aio.h
-@@ -XXX,XX +XXX,XX @@ static void test_threshold_trigger(void)
+@@ -XXX,XX +XXX,XX @@ int coroutine_fn luring_co_submit(BlockDriverState *bs, int fd, uint64_t offset,
-     g_assert_cmpuint(bdrv_write_threshold_get(&bs), ==, 0);
+                                   QEMUIOVector *qiov, int type);
  void luring_detach_aio_context(LuringState *s, AioContext *old_context);
  void luring_attach_aio_context(LuringState *s, AioContext *new_context);
 -
 -/*
 - * luring_io_plug/unplug work in the thread's current AioContext, therefore the
 - * caller must ensure that they are paired in the same IOThread.
 - */
 -void luring_io_plug(void);
 -void luring_io_unplug(void);
  #endif
  #ifdef _WIN32
 diff --git a/block/file-posix.c b/block/file-posix.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/file-posix.c
 +++ b/block/file-posix.c
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn raw_co_io_plug(BlockDriverState *bs)
          laio_io_plug();
      }
  #endif
 -#ifdef CONFIG_LINUX_IO_URING
 -    if (s->use_linux_io_uring) {
 -        luring_io_plug();
 -    }
 -#endif
  }
--typedef struct TestStruct {
+ static void coroutine_fn raw_co_io_unplug(BlockDriverState *bs)
--    const char *name;
+@@ -XXX,XX +XXX,XX @@ static void coroutine_fn raw_co_io_unplug(BlockDriverState *bs)
--    void (*func)(void);
+         laio_io_unplug(s->aio_max_batch);
--} TestStruct;
+     }
  #endif
 -#ifdef CONFIG_LINUX_IO_URING
 -    if (s->use_linux_io_uring) {
 -        luring_io_unplug();
 -    }
 -#endif
  }
  static int coroutine_fn raw_co_flush_to_disk(BlockDriverState *bs)
 diff --git a/block/io_uring.c b/block/io_uring.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/io_uring.c
 +++ b/block/io_uring.c
@@ -XXX,XX +XXX,XX @@
  #include "block/raw-aio.h"
  #include "qemu/coroutine.h"
  #include "qapi/error.h"
 +#include "sysemu/block-backend.h"
  #include "trace.h"
  /* Only used for assertions.  */
@@ -XXX,XX +XXX,XX @@ typedef struct LuringAIOCB {
  } LuringAIOCB;
  typedef struct LuringQueue {
 -    int plugged;
      unsigned int in_queue;
      unsigned int in_flight;
      bool blocked;
@@ -XXX,XX +XXX,XX @@ static void luring_process_completions_and_submit(LuringState *s)
  {
      luring_process_completions(s);
 -    if (!s->io_q.plugged && s->io_q.in_queue > 0) {
 +    if (s->io_q.in_queue > 0) {
          ioq_submit(s);
      }
  }
@@ -XXX,XX +XXX,XX @@ static void qemu_luring_poll_ready(void *opaque)
  static void ioq_init(LuringQueue *io_q)
  {
      QSIMPLEQ_INIT(&io_q->submit_queue);
 -    io_q->plugged = 0;
      io_q->in_queue = 0;
      io_q->in_flight = 0;
      io_q->blocked = false;
  }
 -void luring_io_plug(void)
 +static void luring_unplug_fn(void *opaque)
  {
 -    AioContext *ctx = qemu_get_current_aio_context();
 -    LuringState *s = aio_get_linux_io_uring(ctx);
 -    trace_luring_io_plug(s);
 -    s->io_q.plugged++;
 -}
 -
+-void luring_io_unplug(void)
- int main(int argc, char **argv)
+-{
- {
+-    AioContext *ctx = qemu_get_current_aio_context();
--    size_t i;
+-    LuringState *s = aio_get_linux_io_uring(ctx);
--    TestStruct tests[] = {
+-    assert(s->io_q.plugged);
--        { "/write-threshold/not-trigger",
+-    trace_luring_io_unplug(s, s->io_q.blocked, s->io_q.plugged,
--          test_threshold_not_trigger },
+-                           s->io_q.in_queue, s->io_q.in_flight);
--        { "/write-threshold/trigger",
+-    if (--s->io_q.plugged == 0 &&
--          test_threshold_trigger },
+-        !s->io_q.blocked && s->io_q.in_queue > 0) {
--        { NULL, NULL }
++    LuringState *s = opaque;
--    };
++    trace_luring_unplug_fn(s, s->io_q.blocked, s->io_q.in_queue,
--
++                           s->io_q.in_flight);
-     g_test_init(&argc, &argv, NULL);
++    if (!s->io_q.blocked && s->io_q.in_queue > 0) {
--    for (i = 0; tests[i].name != NULL; i++) {
+         ioq_submit(s);
--        g_test_add_func(tests[i].name, tests[i].func);
+     }
--    }
+ }
-+    g_test_add_func("/write-threshold/not-trigger", test_threshold_not_trigger);
+@@ -XXX,XX +XXX,XX @@ static int luring_do_submit(int fd, LuringAIOCB *luringcb, LuringState *s,
-+    g_test_add_func("/write-threshold/trigger", test_threshold_trigger);
      QSIMPLEQ_INSERT_TAIL(&s->io_q.submit_queue, luringcb, next);
      s->io_q.in_queue++;
 -    trace_luring_do_submit(s, s->io_q.blocked, s->io_q.plugged,
 -                           s->io_q.in_queue, s->io_q.in_flight);
 -    if (!s->io_q.blocked &&
 -        (!s->io_q.plugged ||
 -         s->io_q.in_flight + s->io_q.in_queue >= MAX_ENTRIES)) {
 -        ret = ioq_submit(s);
 -        trace_luring_do_submit_done(s, ret);
 -        return ret;
 +    trace_luring_do_submit(s, s->io_q.blocked, s->io_q.in_queue,
 +                           s->io_q.in_flight);
 +    if (!s->io_q.blocked) {
 +        if (s->io_q.in_flight + s->io_q.in_queue >= MAX_ENTRIES) {
 +            ret = ioq_submit(s);
 +            trace_luring_do_submit_done(s, ret);
 +            return ret;
 +        }
 +
-     return g_test_run();
++        blk_io_plug_call(luring_unplug_fn, s);
      }
      return 0;
  }
+diff --git a/block/trace-events b/block/trace-events
+index XXXXXXX..XXXXXXX 100644
+--- a/block/trace-events
++++ b/block/trace-events
+@@ -XXX,XX +XXX,XX @@ file_paio_submit(void *acb, void *opaque, int64_t offset, int count, int type) "
+ # io_uring.c
+ luring_init_state(void *s, size_t size) "s %p size %zu"
+ luring_cleanup_state(void *s) "%p freed"
+-luring_io_plug(void *s) "LuringState %p plug"
+-luring_io_unplug(void *s, int blocked, int plugged, int queued, int inflight) "LuringState %p blocked %d plugged %d queued %d inflight %d"
+-luring_do_submit(void *s, int blocked, int plugged, int queued, int inflight) "LuringState %p blocked %d plugged %d queued %d inflight %d"
++luring_unplug_fn(void *s, int blocked, int queued, int inflight) "LuringState %p blocked %d queued %d inflight %d"
++luring_do_submit(void *s, int blocked, int queued, int inflight) "LuringState %p blocked %d queued %d inflight %d"
+ luring_do_submit_done(void *s, int ret) "LuringState %p submitted to kernel %d"
+ luring_co_submit(void *bs, void *s, void *luringcb, int fd, uint64_t offset, size_t nbytes, int type) "bs %p s %p luringcb %p fd %d offset %" PRId64 " nbytes %zd type %d"
+ luring_process_completion(void *s, void *aiocb, int ret) "LuringState %p luringcb %p ret %d"
 --
-.31.1
+.40.1

-[PULL 19/19] write-threshold: deal with includes
+[PULL 5/8] block/linux-aio: convert to blk_io_plug_call() API
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+Stop using the .bdrv_co_io_plug() API because it is not multi-queue
+block layer friendly. Use the new blk_io_plug_call() API to batch I/O
-"qemu/typedefs.h" is enough for include/block/write-threshold.h header
+submission instead.
-with forward declaration of BlockDriverState. Also drop extra includes
-from block/write-threshold.c and tests/unit/test-write-threshold.c
+Note that a dev_max_batch check is dropped in laio_io_unplug() because
+the semantics of unplug_fn() are different from .bdrv_co_unplug():
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+. unplug_fn() is only called when the last blk_io_unplug() call occurs,
-Message-Id: <20210506090621.11848-9-vsementsov@virtuozzo.com>
+   not every time blk_io_unplug() is called.
-Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
+. unplug_fn() is per-thread, not per-BlockDriverState, so there is no
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+   way to get per-BlockDriverState fields like dev_max_batch.
 Therefore this condition cannot be moved to laio_unplug_fn(). It is not
 obvious that this condition affects performance in practice, so I am
 removing it instead of trying to come up with a more complex mechanism
 to preserve the condition.
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 Reviewed-by: Eric Blake <eblake@redhat.com>
 Acked-by: Kevin Wolf <kwolf@redhat.com>
 Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
 Message-id: 20230530180959.1108766-6-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- include/block/write-threshold.h   | 2 +-
+ include/block/raw-aio.h |  7 -------
- block/write-threshold.c           | 2 --
+ block/file-posix.c      | 28 ----------------------------
- tests/unit/test-write-threshold.c | 1 -
+ block/linux-aio.c       | 41 +++++++++++------------------------------
-files changed, 1 insertion(+), 4 deletions(-)
+files changed, 11 insertions(+), 65 deletions(-)
-diff --git a/include/block/write-threshold.h b/include/block/write-threshold.h
+diff --git a/include/block/raw-aio.h b/include/block/raw-aio.h
 index XXXXXXX..XXXXXXX 100644
---- a/include/block/write-threshold.h
+--- a/include/block/raw-aio.h
-+++ b/include/block/write-threshold.h
++++ b/include/block/raw-aio.h
@@ -XXX,XX +XXX,XX @@ int coroutine_fn laio_co_submit(int fd, uint64_t offset, QEMUIOVector *qiov,
  void laio_detach_aio_context(LinuxAioState *s, AioContext *old_context);
  void laio_attach_aio_context(LinuxAioState *s, AioContext *new_context);
 -
 -/*
 - * laio_io_plug/unplug work in the thread's current AioContext, therefore the
 - * caller must ensure that they are paired in the same IOThread.
 - */
 -void laio_io_plug(void);
 -void laio_io_unplug(uint64_t dev_max_batch);
  #endif
  /* io_uring.c - Linux io_uring implementation */
  #ifdef CONFIG_LINUX_IO_URING
 diff --git a/block/file-posix.c b/block/file-posix.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/file-posix.c
 +++ b/block/file-posix.c
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_pwritev(BlockDriverState *bs, int64_t offset,
      return raw_co_prw(bs, offset, bytes, qiov, QEMU_AIO_WRITE);
  }
 -static void coroutine_fn raw_co_io_plug(BlockDriverState *bs)
 -{
 -    BDRVRawState __attribute__((unused)) *s = bs->opaque;
 -#ifdef CONFIG_LINUX_AIO
 -    if (s->use_linux_aio) {
 -        laio_io_plug();
 -    }
 -#endif
 -}
 -
 -static void coroutine_fn raw_co_io_unplug(BlockDriverState *bs)
 -{
 -    BDRVRawState __attribute__((unused)) *s = bs->opaque;
 -#ifdef CONFIG_LINUX_AIO
 -    if (s->use_linux_aio) {
 -        laio_io_unplug(s->aio_max_batch);
 -    }
 -#endif
 -}
 -
  static int coroutine_fn raw_co_flush_to_disk(BlockDriverState *bs)
  {
      BDRVRawState *s = bs->opaque;
@@ -XXX,XX +XXX,XX @@ BlockDriver bdrv_file = {
      .bdrv_co_copy_range_from = raw_co_copy_range_from,
      .bdrv_co_copy_range_to  = raw_co_copy_range_to,
      .bdrv_refresh_limits = raw_refresh_limits,
 -    .bdrv_co_io_plug        = raw_co_io_plug,
 -    .bdrv_co_io_unplug      = raw_co_io_unplug,
      .bdrv_attach_aio_context = raw_aio_attach_aio_context,
      .bdrv_co_truncate                   = raw_co_truncate,
@@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_host_device = {
      .bdrv_co_copy_range_from = raw_co_copy_range_from,
      .bdrv_co_copy_range_to  = raw_co_copy_range_to,
      .bdrv_refresh_limits = raw_refresh_limits,
 -    .bdrv_co_io_plug        = raw_co_io_plug,
 -    .bdrv_co_io_unplug      = raw_co_io_unplug,
      .bdrv_attach_aio_context = raw_aio_attach_aio_context,
      .bdrv_co_truncate                   = raw_co_truncate,
@@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_host_cdrom = {
      .bdrv_co_pwritev        = raw_co_pwritev,
      .bdrv_co_flush_to_disk  = raw_co_flush_to_disk,
      .bdrv_refresh_limits    = cdrom_refresh_limits,
 -    .bdrv_co_io_plug        = raw_co_io_plug,
 -    .bdrv_co_io_unplug      = raw_co_io_unplug,
      .bdrv_attach_aio_context = raw_aio_attach_aio_context,
      .bdrv_co_truncate                   = raw_co_truncate,
@@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_host_cdrom = {
      .bdrv_co_pwritev        = raw_co_pwritev,
      .bdrv_co_flush_to_disk  = raw_co_flush_to_disk,
      .bdrv_refresh_limits    = cdrom_refresh_limits,
 -    .bdrv_co_io_plug        = raw_co_io_plug,
 -    .bdrv_co_io_unplug      = raw_co_io_unplug,
      .bdrv_attach_aio_context = raw_aio_attach_aio_context,
      .bdrv_co_truncate                   = raw_co_truncate,
 diff --git a/block/linux-aio.c b/block/linux-aio.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/linux-aio.c
 +++ b/block/linux-aio.c
 @@ -XXX,XX +XXX,XX @@
- #ifndef BLOCK_WRITE_THRESHOLD_H
+ #include "qemu/event_notifier.h"
- #define BLOCK_WRITE_THRESHOLD_H
+ #include "qemu/coroutine.h"
 -#include "block/block_int.h"
 +#include "qemu/typedefs.h"
  /*
   * bdrv_write_threshold_set:
 diff --git a/block/write-threshold.c b/block/write-threshold.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/write-threshold.c
 +++ b/block/write-threshold.c
@@ -XXX,XX +XXX,XX @@
  #include "qemu/osdep.h"
  #include "block/block_int.h"
 -#include "qemu/coroutine.h"
  #include "block/write-threshold.h"
 -#include "qemu/notify.h"
  #include "qapi/error.h"
- #include "qapi/qapi-commands-block-core.h"
++#include "sysemu/block-backend.h"
- #include "qapi/qapi-events-block-core.h"
-diff --git a/tests/unit/test-write-threshold.c b/tests/unit/test-write-threshold.c
+ /* Only used for assertions.  */
-index XXXXXXX..XXXXXXX 100644
+ #include "qemu/coroutine_int.h"
---- a/tests/unit/test-write-threshold.c
+@@ -XXX,XX +XXX,XX @@ struct qemu_laiocb {
-+++ b/tests/unit/test-write-threshold.c
+ };
-@@ -XXX,XX +XXX,XX @@
-  */
+ typedef struct {
+-    int plugged;
- #include "qemu/osdep.h"
+     unsigned int in_queue;
--#include "qapi/error.h"
+     unsigned int in_flight;
- #include "block/block_int.h"
+     bool blocked;
- #include "block/write-threshold.h"
+@@ -XXX,XX +XXX,XX @@ static void qemu_laio_process_completions_and_submit(LinuxAioState *s)
+ {
      qemu_laio_process_completions(s);
 -    if (!s->io_q.plugged && !QSIMPLEQ_EMPTY(&s->io_q.pending)) {
 +    if (!QSIMPLEQ_EMPTY(&s->io_q.pending)) {
          ioq_submit(s);
      }
  }
@@ -XXX,XX +XXX,XX @@ static void qemu_laio_poll_ready(EventNotifier *opaque)
  static void ioq_init(LaioQueue *io_q)
  {
      QSIMPLEQ_INIT(&io_q->pending);
 -    io_q->plugged = 0;
      io_q->in_queue = 0;
      io_q->in_flight = 0;
      io_q->blocked = false;
@@ -XXX,XX +XXX,XX @@ static uint64_t laio_max_batch(LinuxAioState *s, uint64_t dev_max_batch)
      return max_batch;
  }
 -void laio_io_plug(void)
 +static void laio_unplug_fn(void *opaque)
  {
 -    AioContext *ctx = qemu_get_current_aio_context();
 -    LinuxAioState *s = aio_get_linux_aio(ctx);
 +    LinuxAioState *s = opaque;
 -    s->io_q.plugged++;
 -}
 -
 -void laio_io_unplug(uint64_t dev_max_batch)
 -{
 -    AioContext *ctx = qemu_get_current_aio_context();
 -    LinuxAioState *s = aio_get_linux_aio(ctx);
 -
 -    assert(s->io_q.plugged);
 -    s->io_q.plugged--;
 -
 -    /*
 -     * Why max batch checking is performed here:
 -     * Another BDS may have queued requests with a higher dev_max_batch and
 -     * therefore in_queue could now exceed our dev_max_batch. Re-check the max
 -     * batch so we can honor our device's dev_max_batch.
 -     */
 -    if (s->io_q.in_queue >= laio_max_batch(s, dev_max_batch) ||
 -        (!s->io_q.plugged &&
 -         !s->io_q.blocked && !QSIMPLEQ_EMPTY(&s->io_q.pending))) {
 +    if (!s->io_q.blocked && !QSIMPLEQ_EMPTY(&s->io_q.pending)) {
          ioq_submit(s);
      }
  }
@@ -XXX,XX +XXX,XX @@ static int laio_do_submit(int fd, struct qemu_laiocb *laiocb, off_t offset,
      QSIMPLEQ_INSERT_TAIL(&s->io_q.pending, laiocb, next);
      s->io_q.in_queue++;
 -    if (!s->io_q.blocked &&
 -        (!s->io_q.plugged ||
 -         s->io_q.in_queue >= laio_max_batch(s, dev_max_batch))) {
 -        ioq_submit(s);
 +    if (!s->io_q.blocked) {
 +        if (s->io_q.in_queue >= laio_max_batch(s, dev_max_batch)) {
 +            ioq_submit(s);
 +        } else {
 +            blk_io_plug_call(laio_unplug_fn, s);
 +        }
      }
      return 0;
 --
-.31.1
+.40.1

-[PULL 13/19] block/write-threshold: don't use write notifiers
+[PULL 6/8] block: remove bdrv_co_io_plug() API
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+No block driver implements .bdrv_co_io_plug() anymore. Get rid of the
 function pointers.
-write-notifiers are used only for write-threshold. New code for such
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-purpose should create filters.
+Reviewed-by: Eric Blake <eblake@redhat.com>
 Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
 Acked-by: Kevin Wolf <kwolf@redhat.com>
 Message-id: 20230530180959.1108766-7-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
  include/block/block-io.h         |  3 ---
  include/block/block_int-common.h | 11 ----------
  block/io.c                       | 37 --------------------------------
 files changed, 51 deletions(-)
-Let's better special-case write-threshold and drop write notifiers at
+diff --git a/include/block/block-io.h b/include/block/block-io.h
 all. (Actually, write-threshold is special-cased anyway, as the only
 user of write-notifiers)
 So, create a new direct interface for bdrv_co_write_req_prepare() and
 drop all write-notifier related logic from write-threshold.c.
 Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
 Reviewed-by: Max Reitz <mreitz@redhat.com>
 Message-Id: <20210506090621.11848-2-vsementsov@virtuozzo.com>
 Reviewed-by: Eric Blake <eblake@redhat.com>
 Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
 [mreitz: Adjusted comment as per Eric's suggestion]
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  include/block/block_int.h       |  1 -
  include/block/write-threshold.h |  9 +++++
  block/io.c                      |  5 ++-
  block/write-threshold.c         | 70 +++++++--------------------------
 files changed, 27 insertions(+), 58 deletions(-)
 diff --git a/include/block/block_int.h b/include/block/block_int.h
 index XXXXXXX..XXXXXXX 100644
---- a/include/block/block_int.h
+--- a/include/block/block-io.h
-+++ b/include/block/block_int.h
++++ b/include/block/block-io.h
@@ -XXX,XX +XXX,XX @@ void coroutine_fn bdrv_co_leave(BlockDriverState *bs, AioContext *old_ctx);
  AioContext *child_of_bds_get_parent_aio_context(BdrvChild *c);
 -void coroutine_fn GRAPH_RDLOCK bdrv_co_io_plug(BlockDriverState *bs);
 -void coroutine_fn GRAPH_RDLOCK bdrv_co_io_unplug(BlockDriverState *bs);
 -
  bool coroutine_fn GRAPH_RDLOCK
  bdrv_co_can_store_new_dirty_bitmap(BlockDriverState *bs, const char *name,
                                     uint32_t granularity, Error **errp);
 diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
 index XXXXXXX..XXXXXXX 100644
 --- a/include/block/block_int-common.h
 +++ b/include/block/block_int-common.h
@@ -XXX,XX +XXX,XX @@ struct BlockDriver {
      void coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_debug_event)(
          BlockDriverState *bs, BlkdebugEvent event);
 -    /* io queue for linux-aio */
 -    void coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_io_plug)(BlockDriverState *bs);
 -    void coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_io_unplug)(
 -        BlockDriverState *bs);
 -
      bool (*bdrv_supports_persistent_dirty_bitmap)(BlockDriverState *bs);
      bool coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_can_store_new_dirty_bitmap)(
 @@ -XXX,XX +XXX,XX @@ struct BlockDriverState {
+     unsigned int in_flight;
-     /* threshold limit for writes, in bytes. "High water mark". */
+     unsigned int serialising_in_flight;
-     uint64_t write_threshold_offset;
--    NotifierWithReturn write_threshold_notifier;
+-    /*
+-     * counter for nested bdrv_io_plug.
-     /* Writing to the list requires the BQL _and_ the dirty_bitmap_mutex.
+-     * Accessed with atomic ops.
-      * Reading from the list can be done with either the BQL or the
+-     */
-diff --git a/include/block/write-threshold.h b/include/block/write-threshold.h
+-    unsigned io_plugged;
-index XXXXXXX..XXXXXXX 100644
+-
---- a/include/block/write-threshold.h
+     /* do we need to tell the quest if we have a volatile write cache? */
-+++ b/include/block/write-threshold.h
+     int enable_write_cache;
-@@ -XXX,XX +XXX,XX @@ bool bdrv_write_threshold_is_set(const BlockDriverState *bs);
  uint64_t bdrv_write_threshold_exceeded(const BlockDriverState *bs,
                                         const BdrvTrackedRequest *req);
 +/*
 + * bdrv_write_threshold_check_write
 + *
 + * Check whether the specified request exceeds the write threshold.
 + * If so, send a corresponding event and disable write threshold checking.
 + */
 +void bdrv_write_threshold_check_write(BlockDriverState *bs, int64_t offset,
 +                                      int64_t bytes);
 +
  #endif
 diff --git a/block/io.c b/block/io.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/io.c
 +++ b/block/io.c
-@@ -XXX,XX +XXX,XX @@
+@@ -XXX,XX +XXX,XX @@ void *qemu_try_blockalign0(BlockDriverState *bs, size_t size)
- #include "block/blockjob_int.h"
+     return mem;
  #include "block/block_int.h"
  #include "block/coroutines.h"
 +#include "block/write-threshold.h"
  #include "qemu/cutils.h"
  #include "qapi/error.h"
  #include "qemu/error-report.h"
@@ -XXX,XX +XXX,XX @@ bdrv_co_write_req_prepare(BdrvChild *child, int64_t offset, int64_t bytes,
          } else {
              assert(child->perm & BLK_PERM_WRITE);
          }
 -        return notifier_with_return_list_notify(&bs->before_write_notifiers,
 -                                                req);
 +        bdrv_write_threshold_check_write(bs, offset, bytes);
 +        return 0;
      case BDRV_TRACKED_TRUNCATE:
          assert(child->perm & BLK_PERM_RESIZE);
          return 0;
 diff --git a/block/write-threshold.c b/block/write-threshold.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/write-threshold.c
 +++ b/block/write-threshold.c
@@ -XXX,XX +XXX,XX @@ bool bdrv_write_threshold_is_set(const BlockDriverState *bs)
      return bs->write_threshold_offset > 0;
  }
--static void write_threshold_disable(BlockDriverState *bs)
+-void coroutine_fn bdrv_co_io_plug(BlockDriverState *bs)
 -{
--    if (bdrv_write_threshold_is_set(bs)) {
+-    BdrvChild *child;
--        notifier_with_return_remove(&bs->write_threshold_notifier);
+-    IO_CODE();
--        bs->write_threshold_offset = 0;
+-    assert_bdrv_graph_readable();
 -
 -    QLIST_FOREACH(child, &bs->children, next) {
 -        bdrv_co_io_plug(child->bs);
 -    }
 -
 -    if (qatomic_fetch_inc(&bs->io_plugged) == 0) {
 -        BlockDriver *drv = bs->drv;
 -        if (drv && drv->bdrv_co_io_plug) {
 -            drv->bdrv_co_io_plug(bs);
 -        }
 -    }
 -}
 -
- uint64_t bdrv_write_threshold_exceeded(const BlockDriverState *bs,
+-void coroutine_fn bdrv_co_io_unplug(BlockDriverState *bs)
                                         const BdrvTrackedRequest *req)
  {
@@ -XXX,XX +XXX,XX @@ uint64_t bdrv_write_threshold_exceeded(const BlockDriverState *bs,
      return 0;
  }
 -static int coroutine_fn before_write_notify(NotifierWithReturn *notifier,
 -                                            void *opaque)
 -{
--    BdrvTrackedRequest *req = opaque;
+-    BdrvChild *child;
--    BlockDriverState *bs = req->bs;
+-    IO_CODE();
--    uint64_t amount = 0;
+-    assert_bdrv_graph_readable();
 -
--    amount = bdrv_write_threshold_exceeded(bs, req);
+-    assert(bs->io_plugged);
--    if (amount > 0) {
+-    if (qatomic_fetch_dec(&bs->io_plugged) == 1) {
--        qapi_event_send_block_write_threshold(
+-        BlockDriver *drv = bs->drv;
--            bs->node_name,
+-        if (drv && drv->bdrv_co_io_unplug) {
--            amount,
+-            drv->bdrv_co_io_unplug(bs);
--            bs->write_threshold_offset);
+-        }
 -
 -        /* autodisable to avoid flooding the monitor */
 -        write_threshold_disable(bs);
 -    }
 -
--    return 0; /* should always let other notifiers run */
+-    QLIST_FOREACH(child, &bs->children, next) {
 -        bdrv_co_io_unplug(child->bs);
 -    }
 -}
 -
--static void write_threshold_register_notifier(BlockDriverState *bs)
+ /* Helper that undoes bdrv_register_buf() when it fails partway through */
--{
+ static void GRAPH_RDLOCK
--    bs->write_threshold_notifier.notify = before_write_notify;
+ bdrv_register_buf_rollback(BlockDriverState *bs, void *host, size_t size,
 -    bdrv_add_before_write_notifier(bs, &bs->write_threshold_notifier);
 -}
 -
 -static void write_threshold_update(BlockDriverState *bs,
 -                                   int64_t threshold_bytes)
 -{
 -    bs->write_threshold_offset = threshold_bytes;
 -}
 -
  void bdrv_write_threshold_set(BlockDriverState *bs, uint64_t threshold_bytes)
  {
 -    if (bdrv_write_threshold_is_set(bs)) {
 -        if (threshold_bytes > 0) {
 -            write_threshold_update(bs, threshold_bytes);
 -        } else {
 -            write_threshold_disable(bs);
 -        }
 -    } else {
 -        if (threshold_bytes > 0) {
 -            /* avoid multiple registration */
 -            write_threshold_register_notifier(bs);
 -            write_threshold_update(bs, threshold_bytes);
 -        }
 -        /* discard bogus disable request */
 -    }
 +    bs->write_threshold_offset = threshold_bytes;
  }
  void qmp_block_set_write_threshold(const char *node_name,
@@ -XXX,XX +XXX,XX @@ void qmp_block_set_write_threshold(const char *node_name,
      aio_context_release(aio_context);
  }
 +
 +void bdrv_write_threshold_check_write(BlockDriverState *bs, int64_t offset,
 +                                      int64_t bytes)
 +{
 +    int64_t end = offset + bytes;
 +    uint64_t wtr = bs->write_threshold_offset;
 +
 +    if (wtr > 0 && end > wtr) {
 +        qapi_event_send_block_write_threshold(bs->node_name, end - wtr, wtr);
 +
 +        /* autodisable to avoid flooding the monitor */
 +        bdrv_write_threshold_set(bs, 0);
 +    }
 +}
 --
-.31.1
+.40.1

-[PULL 03/19] monitor: hmp_qemu_io: acquire aio contex, fix crash
+[PULL 7/8] block/blkio: use qemu_open() to support fd passing for virtio-blk
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+From: Stefano Garzarella <sgarzare@redhat.com>
-Max reported the following bug:
+Some virtio-blk drivers (e.g. virtio-blk-vhost-vdpa) supports the fd
 passing. Let's expose this to the user, so the management layer
 can pass the file descriptor of an already opened path.
-$ ./qemu-img create -f raw src.img 1G
+If the libblkio virtio-blk driver supports fd passing, let's always
-$ ./qemu-img create -f raw dst.img 1G
+use qemu_open() to open the `path`, so we can handle fd passing
 from the management layer through the "/dev/fdset/N" special path.
-$ (echo '
+Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
-   {"execute":"qmp_capabilities"}
+Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
-   {"execute":"blockdev-mirror",
+Message-id: 20230530071941.8954-2-sgarzare@redhat.com
-    "arguments":{"job-id":"mirror",
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-                 "device":"source",
+---
-                 "target":"target",
+ block/blkio.c | 53 ++++++++++++++++++++++++++++++++++++++++++---------
-                 "sync":"full",
+file changed, 44 insertions(+), 9 deletions(-)
                  "filter-node-name":"mirror-top"}}
 '; sleep 3; echo '
    {"execute":"human-monitor-command",
     "arguments":{"command-line":
                  "qemu-io mirror-top \"write 0 1G\""}}') \
 | x86_64-softmmu/qemu-system-x86_64 \
    -qmp stdio \
    -blockdev file,node-name=source,filename=src.img \
    -blockdev file,node-name=target,filename=dst.img \
    -object iothread,id=iothr0 \
    -device virtio-blk,drive=source,iothread=iothr0
-crashes:
+diff --git a/block/blkio.c b/block/blkio.c
 raise () at /usr/lib/libc.so.6
 abort () at /usr/lib/libc.so.6
 error_exit
    (err=<optimized out>,
    msg=msg@entry=0x55fbb1634790 <__func__.27> "qemu_mutex_unlock_impl")
    at ../util/qemu-thread-posix.c:37
 qemu_mutex_unlock_impl
    (mutex=mutex@entry=0x55fbb25ab6e0,
    file=file@entry=0x55fbb1636957 "../util/async.c",
    line=line@entry=650)
    at ../util/qemu-thread-posix.c:109
 aio_context_release (ctx=ctx@entry=0x55fbb25ab680) at ../util/async.c:650
 bdrv_do_drained_begin
    (bs=bs@entry=0x55fbb3a87000, recursive=recursive@entry=false,
    parent=parent@entry=0x0,
    ignore_bds_parents=ignore_bds_parents@entry=false,
    poll=poll@entry=true) at ../block/io.c:441
 bdrv_do_drained_begin
    (poll=true, ignore_bds_parents=false, parent=0x0, recursive=false,
    bs=0x55fbb3a87000) at ../block/io.c:448
 blk_drain (blk=0x55fbb26c5a00) at ../block/block-backend.c:1718
 blk_unref (blk=0x55fbb26c5a00) at ../block/block-backend.c:498
 blk_unref (blk=0x55fbb26c5a00) at ../block/block-backend.c:491
 hmp_qemu_io (mon=0x7fffaf3fc7d0, qdict=<optimized out>)
    at ../block/monitor/block-hmp-cmds.c:628
 man pthread_mutex_unlock
 ...
     EPERM  The  mutex type is PTHREAD_MUTEX_ERRORCHECK or
     PTHREAD_MUTEX_RECURSIVE, or the mutex is a robust mutex, and the
     current thread does not own the mutex.
 So, thread doesn't own the mutex. And we have iothread here.
 Next, note that AIO_WAIT_WHILE() documents that ctx must be acquired
 exactly once by caller. But where is it acquired in the call stack?
 Seems nowhere.
 qemuio_command do acquire aio context.. But we need context acquired
 around blk_unref() as well and actually around blk_insert_bs() too.
 Let's refactor qemuio_command so that it doesn't acquire aio context
 but callers do that instead. This way we can cleanly acquire aio
 context in hmp_qemu_io() around all three calls.
 Reported-by: Max Reitz <mreitz@redhat.com>
 Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
 Message-Id: <20210423134233.51495-1-vsementsov@virtuozzo.com>
 [mreitz: Fixed comment]
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  block/monitor/block-hmp-cmds.c | 31 +++++++++++++++++++++----------
  qemu-io-cmds.c                 |  8 ++++----
  qemu-io.c                      | 17 +++++++++++++++--
 files changed, 40 insertions(+), 16 deletions(-)
 diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/monitor/block-hmp-cmds.c
+--- a/block/blkio.c
-+++ b/block/monitor/block-hmp-cmds.c
++++ b/block/blkio.c
-@@ -XXX,XX +XXX,XX @@ void hmp_eject(Monitor *mon, const QDict *qdict)
+@@ -XXX,XX +XXX,XX @@ static int blkio_virtio_blk_common_open(BlockDriverState *bs,
  void hmp_qemu_io(Monitor *mon, const QDict *qdict)
  {
--    BlockBackend *blk;
+     const char *path = qdict_get_try_str(options, "path");
-+    BlockBackend *blk = NULL;
+     BDRVBlkioState *s = bs->opaque;
-+    BlockDriverState *bs = NULL;
+-    int ret;
-     BlockBackend *local_blk = NULL;
++    bool fd_supported = false;
-+    AioContext *ctx = NULL;
++    int fd, ret;
-     bool qdev = qdict_get_try_bool(qdict, "qdev", false);
-     const char *device = qdict_get_str(qdict, "device");
+     if (!path) {
-     const char *command = qdict_get_str(qdict, "command");
+         error_setg(errp, "missing 'path' option");
-@@ -XXX,XX +XXX,XX @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict)
+         return -EINVAL;
      } else {
          blk = blk_by_name(device);
          if (!blk) {
 -            BlockDriverState *bs = bdrv_lookup_bs(NULL, device, &err);
 -            if (bs) {
 -                blk = local_blk = blk_new(bdrv_get_aio_context(bs),
 -                                          0, BLK_PERM_ALL);
 -                ret = blk_insert_bs(blk, bs, &err);
 -                if (ret < 0) {
 -                    goto fail;
 -                }
 -            } else {
 +            bs = bdrv_lookup_bs(NULL, device, &err);
 +            if (!bs) {
                  goto fail;
              }
          }
      }
-+    ctx = blk ? blk_get_aio_context(blk) : bdrv_get_aio_context(bs);
+-    ret = blkio_set_str(s->blkio, "path", path);
-+    aio_context_acquire(ctx);
+-    qdict_del(options, "path");
 -    if (ret < 0) {
 -        error_setg_errno(errp, -ret, "failed to set path: %s",
 -                         blkio_get_error_msg());
 -        return ret;
 -    }
 -
      if (!(flags & BDRV_O_NOCACHE)) {
          error_setg(errp, "cache.direct=off is not supported");
          return -EINVAL;
      }
 +
-+    if (bs) {
++    if (blkio_get_int(s->blkio, "fd", &fd) == 0) {
-+        blk = local_blk = blk_new(bdrv_get_aio_context(bs), 0, BLK_PERM_ALL);
++        fd_supported = true;
-+        ret = blk_insert_bs(blk, bs, &err);
++    }
 +
 +    /*
 +     * If the libblkio driver supports fd passing, let's always use qemu_open()
 +     * to open the `path`, so we can handle fd passing from the management
 +     * layer through the "/dev/fdset/N" special path.
 +     */
 +    if (fd_supported) {
 +        int open_flags;
 +
 +        if (flags & BDRV_O_RDWR) {
 +            open_flags = O_RDWR;
 +        } else {
 +            open_flags = O_RDONLY;
 +        }
 +
 +        fd = qemu_open(path, open_flags, errp);
 +        if (fd < 0) {
 +            return -EINVAL;
 +        }
 +
 +        ret = blkio_set_int(s->blkio, "fd", fd);
 +        if (ret < 0) {
-+            goto fail;
++            error_setg_errno(errp, -ret, "failed to set fd: %s",
 +                             blkio_get_error_msg());
 +            qemu_close(fd);
 +            return ret;
 +        }
 +    } else {
 +        ret = blkio_set_str(s->blkio, "path", path);
 +        if (ret < 0) {
 +            error_setg_errno(errp, -ret, "failed to set path: %s",
 +                             blkio_get_error_msg());
 +            return ret;
 +        }
 +    }
 +
-     /*
++    qdict_del(options, "path");
       * Notably absent: Proper permission management. This is sad, but it seems
       * almost impossible to achieve without changing the semantics and thereby
@@ -XXX,XX +XXX,XX @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict)
  fail:
      blk_unref(local_blk);
 +
-+    if (ctx) {
+     return 0;
 +        aio_context_release(ctx);
 +    }
 +
      hmp_handle_error(mon, err);
  }
-diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
-index XXXXXXX..XXXXXXX 100644
---- a/qemu-io-cmds.c
-+++ b/qemu-io-cmds.c
-@@ -XXX,XX +XXX,XX @@ static const cmdinfo_t help_cmd = {
-     .oneline    = "help for one or all commands",
- };
-+/*
-+ * Called with aio context of blk acquired. Or with qemu_get_aio_context()
-+ * context acquired if blk is NULL.
-+ */
- int qemuio_command(BlockBackend *blk, const char *cmd)
- {
--    AioContext *ctx;
-     char *input;
-     const cmdinfo_t *ct;
-     char **v;
-@@ -XXX,XX +XXX,XX @@ int qemuio_command(BlockBackend *blk, const char *cmd)
-     if (c) {
-         ct = find_command(v[0]);
-         if (ct) {
--            ctx = blk ? blk_get_aio_context(blk) : qemu_get_aio_context();
--            aio_context_acquire(ctx);
-             ret = command(blk, ct, c, v);
--            aio_context_release(ctx);
-         } else {
-             fprintf(stderr, "command \"%s\" not found\n", v[0]);
-             ret = -EINVAL;
-diff --git a/qemu-io.c b/qemu-io.c
-index XXXXXXX..XXXXXXX 100644
---- a/qemu-io.c
-+++ b/qemu-io.c
-@@ -XXX,XX +XXX,XX @@ static void prep_fetchline(void *opaque)
-     *fetchable= 1;
- }
-+static int do_qemuio_command(const char *cmd)
-+{
-+    int ret;
-+    AioContext *ctx =
-+        qemuio_blk ? blk_get_aio_context(qemuio_blk) : qemu_get_aio_context();
-+
-+    aio_context_acquire(ctx);
-+    ret = qemuio_command(qemuio_blk, cmd);
-+    aio_context_release(ctx);
-+
-+    return ret;
-+}
-+
- static int command_loop(void)
- {
-     int i, fetchable = 0, prompted = 0;
-@@ -XXX,XX +XXX,XX @@ static int command_loop(void)
-     char *input;
-     for (i = 0; !quit_qemu_io && i < ncmdline; i++) {
--        ret = qemuio_command(qemuio_blk, cmdline[i]);
-+        ret = do_qemuio_command(cmdline[i]);
-         if (ret < 0) {
-             last_error = ret;
-         }
-@@ -XXX,XX +XXX,XX @@ static int command_loop(void)
-         if (input == NULL) {
-             break;
-         }
--        ret = qemuio_command(qemuio_blk, input);
-+        ret = do_qemuio_command(input);
-         g_free(input);
-         if (ret < 0) {
 --
-.31.1
+.40.1

-[PULL 05/19] qemu-iotests: do not buffer the test output
+Deleted patch
-From: Paolo Bonzini <pbonzini@redhat.com>
-Instead of buffering the test output into a StringIO, patch it on
-the fly by wrapping sys.stdout's write method.  This can be
-done unconditionally, even if using -d, which makes execute_unittest
-a bit simpler.
-Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
-Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Tested-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
-Message-Id: <20210323181928.311862-2-pbonzini@redhat.com>
-Message-Id: <20210503110110.476887-2-pbonzini@redhat.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- tests/qemu-iotests/240.out    |  8 ++--
- tests/qemu-iotests/245.out    |  8 ++--
- tests/qemu-iotests/295.out    |  6 +--
- tests/qemu-iotests/296.out    |  8 ++--
- tests/qemu-iotests/iotests.py | 70 ++++++++++++++++++++---------------
-files changed, 56 insertions(+), 44 deletions(-)
-diff --git a/tests/qemu-iotests/240.out b/tests/qemu-iotests/240.out
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/240.out
-+++ b/tests/qemu-iotests/240.out
-@@ -XXX,XX +XXX,XX @@
- {"return": {}}
- {"execute": "blockdev-del", "arguments": {"node-name": "hd0"}}
- {"return": {}}
--==Attach two SCSI disks using the same block device and the same iothread==
-+.==Attach two SCSI disks using the same block device and the same iothread==
- {"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "hd0", "read-only": true, "read-zeroes": true}}
- {"return": {}}
- {"execute": "object-add", "arguments": {"id": "iothread0", "qom-type": "iothread"}}
-@@ -XXX,XX +XXX,XX @@
- {"return": {}}
- {"execute": "blockdev-del", "arguments": {"node-name": "hd0"}}
- {"return": {}}
--==Attach two SCSI disks using the same block device but different iothreads==
-+.==Attach two SCSI disks using the same block device but different iothreads==
- {"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "hd0", "read-only": true, "read-zeroes": true}}
- {"return": {}}
- {"execute": "object-add", "arguments": {"id": "iothread0", "qom-type": "iothread"}}
-@@ -XXX,XX +XXX,XX @@
- {"return": {}}
- {"execute": "blockdev-del", "arguments": {"node-name": "hd0"}}
- {"return": {}}
--==Attach a SCSI disks using the same block device as a NBD server==
-+.==Attach a SCSI disks using the same block device as a NBD server==
- {"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "hd0", "read-only": true, "read-zeroes": true}}
- {"return": {}}
- {"execute": "nbd-server-start", "arguments": {"addr": {"data": {"path": "SOCK_DIR/PID-nbd.sock"}, "type": "unix"}}}
-@@ -XXX,XX +XXX,XX @@
- {"return": {}}
- {"execute": "device_add", "arguments": {"drive": "hd0", "driver": "scsi-hd", "id": "scsi-hd0"}}
- {"return": {}}
--....
-+.
- ----------------------------------------------------------------------
- Ran 4 tests
-diff --git a/tests/qemu-iotests/245.out b/tests/qemu-iotests/245.out
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/245.out
-+++ b/tests/qemu-iotests/245.out
-@@ -XXX,XX +XXX,XX @@
--{"execute": "job-finalize", "arguments": {"id": "commit0"}}
-+..{"execute": "job-finalize", "arguments": {"id": "commit0"}}
- {"return": {}}
- {"data": {"id": "commit0", "type": "commit"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
- {"data": {"device": "commit0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "commit"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
--{"execute": "job-finalize", "arguments": {"id": "stream0"}}
-+...{"execute": "job-finalize", "arguments": {"id": "stream0"}}
- {"return": {}}
- {"data": {"id": "stream0", "type": "stream"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
- {"data": {"device": "stream0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "stream"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
--{"execute": "job-finalize", "arguments": {"id": "stream0"}}
-+.{"execute": "job-finalize", "arguments": {"id": "stream0"}}
- {"return": {}}
- {"data": {"id": "stream0", "type": "stream"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
- {"data": {"device": "stream0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "stream"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
--.....................
-+...............
- ----------------------------------------------------------------------
- Ran 21 tests
-diff --git a/tests/qemu-iotests/295.out b/tests/qemu-iotests/295.out
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/295.out
-+++ b/tests/qemu-iotests/295.out
-@@ -XXX,XX +XXX,XX @@
- {"return": {}}
- {"execute": "job-dismiss", "arguments": {"id": "job_erase_key"}}
- {"return": {}}
--{"execute": "job-dismiss", "arguments": {"id": "job_add_key"}}
-+.{"execute": "job-dismiss", "arguments": {"id": "job_add_key"}}
- {"return": {}}
- {"execute": "job-dismiss", "arguments": {"id": "job_erase_key"}}
- {"return": {}}
-@@ -XXX,XX +XXX,XX @@ Job failed: Invalid password, cannot unlock any keyslot
- {"return": {}}
- {"execute": "job-dismiss", "arguments": {"id": "job_add_key"}}
- {"return": {}}
--{"execute": "job-dismiss", "arguments": {"id": "job_add_key"}}
-+.{"execute": "job-dismiss", "arguments": {"id": "job_add_key"}}
- {"return": {}}
- {"execute": "job-dismiss", "arguments": {"id": "job_add_key"}}
- {"return": {}}
-@@ -XXX,XX +XXX,XX @@ Job failed: All the active keyslots match the (old) password that was given and
- {"return": {}}
- {"execute": "job-dismiss", "arguments": {"id": "job_erase_key"}}
- {"return": {}}
--...
-+.
- ----------------------------------------------------------------------
- Ran 3 tests
-diff --git a/tests/qemu-iotests/296.out b/tests/qemu-iotests/296.out
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/296.out
-+++ b/tests/qemu-iotests/296.out
-@@ -XXX,XX +XXX,XX @@ Job failed: Failed to get shared "consistent read" lock
- qemu-img: Failed to get shared "consistent read" lock
- Is another process using the image [TEST_DIR/test.img]?
--Formatting 'TEST_DIR/test.img', fmt=luks size=1048576 key-secret=keysec0 iter-time=10
-+.Formatting 'TEST_DIR/test.img', fmt=luks size=1048576 key-secret=keysec0 iter-time=10
- Job failed: Block node is read-only
- {"execute": "job-dismiss", "arguments": {"id": "job0"}}
-@@ -XXX,XX +XXX,XX @@ Job failed: Failed to get shared "consistent read" lock
- {"return": {}}
- {"execute": "job-dismiss", "arguments": {"id": "job0"}}
- {"return": {}}
--Formatting 'TEST_DIR/test.img', fmt=luks size=1048576 key-secret=keysec0 iter-time=10
-+.Formatting 'TEST_DIR/test.img', fmt=luks size=1048576 key-secret=keysec0 iter-time=10
- {"return": {}}
- {"error": {"class": "GenericError", "desc": "Failed to get \"write\" lock"}}
--Formatting 'TEST_DIR/test.img', fmt=luks size=1048576 key-secret=keysec0 iter-time=10
-+.Formatting 'TEST_DIR/test.img', fmt=luks size=1048576 key-secret=keysec0 iter-time=10
- {"return": {}}
- {"return": {}}
--....
-+.
- ----------------------------------------------------------------------
- Ran 4 tests
-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 @@
- import bz2
- from collections import OrderedDict
- import faulthandler
--import io
- import json
- import logging
- import os
-@@ -XXX,XX +XXX,XX @@
- import sys
- import time
- from typing import (Any, Callable, Dict, Iterable,
--                    List, Optional, Sequence, Tuple, TypeVar)
-+                    List, Optional, Sequence, TextIO, Tuple, Type, TypeVar)
- import unittest
- from contextlib import contextmanager
-@@ -XXX,XX +XXX,XX @@ def func_wrapper(*args, **kwargs):
-             return func(*args, **kwargs)
-     return func_wrapper
-+# We need to filter out the time taken from the output so that
-+# qemu-iotest can reliably diff the results against master output,
-+# and hide skipped tests from the reference output.
-+
-+class ReproducibleTestResult(unittest.TextTestResult):
-+    def addSkip(self, test, reason):
-+        # Same as TextTestResult, but print dot instead of "s"
-+        unittest.TestResult.addSkip(self, test, reason)
-+        if self.showAll:
-+            self.stream.writeln("skipped {0!r}".format(reason))
-+        elif self.dots:
-+            self.stream.write(".")
-+            self.stream.flush()
-+
-+class ReproducibleStreamWrapper:
-+    def __init__(self, stream: TextIO):
-+        self.stream = stream
-+
-+    def __getattr__(self, attr):
-+        if attr in ('stream', '__getstate__'):
-+            raise AttributeError(attr)
-+        return getattr(self.stream, attr)
-+
-+    def write(self, arg=None):
-+        arg = re.sub(r'Ran (\d+) tests? in [\d.]+s', r'Ran \1 tests', arg)
-+        arg = re.sub(r' \(skipped=\d+\)', r'', arg)
-+        self.stream.write(arg)
-+
-+class ReproducibleTestRunner(unittest.TextTestRunner):
-+    def __init__(self, stream: Optional[TextIO] = None,
-+             resultclass: Type[unittest.TestResult] = ReproducibleTestResult,
-+             **kwargs: Any) -> None:
-+        rstream = ReproducibleStreamWrapper(stream or sys.stdout)
-+        super().__init__(stream=rstream,           # type: ignore
-+                         descriptions=True,
-+                         resultclass=resultclass,
-+                         **kwargs)
-+
- def execute_unittest(debug=False):
-     """Executes unittests within the calling module."""
-     verbosity = 2 if debug else 1
--
--    if debug:
--        output = sys.stdout
--    else:
--        # We need to filter out the time taken from the output so that
--        # qemu-iotest can reliably diff the results against master output.
--        output = io.StringIO()
--
--    runner = unittest.TextTestRunner(stream=output, descriptions=True,
--                                     verbosity=verbosity)
--    try:
--        # unittest.main() will use sys.exit(); so expect a SystemExit
--        # exception
--        unittest.main(testRunner=runner)
--    finally:
--        # We need to filter out the time taken from the output so that
--        # qemu-iotest can reliably diff the results against master output.
--        if not debug:
--            out = output.getvalue()
--            out = re.sub(r'Ran (\d+) tests? in [\d.]+s', r'Ran \1 tests', out)
--
--            # Hide skipped tests from the reference output
--            out = re.sub(r'OK \(skipped=\d+\)', 'OK', out)
--            out_first_line, out_rest = out.split('\n', 1)
--            out = out_first_line.replace('s', '.') + '\n' + out_rest
--
--            sys.stderr.write(out)
-+    runner = ReproducibleTestRunner(verbosity=verbosity)
-+    unittest.main(testRunner=runner)
- def execute_setup_common(supported_fmts: Sequence[str] = (),
-                          supported_platforms: Sequence[str] = (),
---
-.31.1

-[PULL 06/19] qemu-iotests: allow passing unittest.main arguments to the test scripts
+Deleted patch
-From: Paolo Bonzini <pbonzini@redhat.com>
-Python test scripts that use unittest consist of multiple tests.
-unittest.main allows selecting which tests to run, but currently this
-is not possible because the iotests wrapper ignores sys.argv.
-unittest.main command line options also allow the user to pick the
-desired options for verbosity, failfast mode, etc.  While "-d" is
-currently translated to "-v", it also enables extra debug output,
-and other options are not available at all.
-These command line options only work if the unittest.main testRunner
-argument is a type, rather than a TestRunner instance.  Therefore, pass
-the class name and "verbosity" argument to unittest.main, and adjust for
-the different default warnings between TextTestRunner and unittest.main.
-Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
-Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Tested-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
-Message-Id: <20210323181928.311862-3-pbonzini@redhat.com>
-Message-Id: <20210503110110.476887-3-pbonzini@redhat.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- tests/qemu-iotests/iotests.py | 14 +++++++++-----
-file changed, 9 insertions(+), 5 deletions(-)
-diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/iotests.py
-+++ b/tests/qemu-iotests/iotests.py
-@@ -XXX,XX +XXX,XX @@ def __init__(self, stream: Optional[TextIO] = None,
-                          resultclass=resultclass,
-                          **kwargs)
--def execute_unittest(debug=False):
-+def execute_unittest(argv: List[str], debug: bool = False) -> None:
-     """Executes unittests within the calling module."""
--    verbosity = 2 if debug else 1
--    runner = ReproducibleTestRunner(verbosity=verbosity)
--    unittest.main(testRunner=runner)
-+    # Some tests have warnings, especially ResourceWarnings for unclosed
-+    # files and sockets.  Ignore them for now to ensure reproducibility of
-+    # the test output.
-+    unittest.main(argv=argv,
-+                  testRunner=ReproducibleTestRunner,
-+                  verbosity=2 if debug else 1,
-+                  warnings=None if sys.warnoptions else 'ignore')
- def execute_setup_common(supported_fmts: Sequence[str] = (),
-                          supported_platforms: Sequence[str] = (),
-@@ -XXX,XX +XXX,XX @@ def execute_test(*args, test_function=None, **kwargs):
-     debug = execute_setup_common(*args, **kwargs)
-     if not test_function:
--        execute_unittest(debug)
-+        execute_unittest(sys.argv, debug)
-     else:
-         test_function()
---
-.31.1

-[PULL 07/19] qemu-iotests: move command line and environment handling from TestRunner to TestEnv
+Deleted patch
-From: Paolo Bonzini <pbonzini@redhat.com>
-In the next patch, "check" will learn how to execute a test script without
-going through TestRunner.  To enable this, keep only the text output
-and subprocess handling in the TestRunner; move into TestEnv the logic
-to prepare for running a subprocess.
-Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
-Tested-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
-Message-Id: <20210323181928.311862-4-pbonzini@redhat.com>
-Message-Id: <20210503110110.476887-4-pbonzini@redhat.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- tests/qemu-iotests/testenv.py    | 17 ++++++++++++++++-
- tests/qemu-iotests/testrunner.py | 14 +-------------
-files changed, 17 insertions(+), 14 deletions(-)
-diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/testenv.py
-+++ b/tests/qemu-iotests/testenv.py
-@@ -XXX,XX +XXX,XX @@
- import random
- import subprocess
- import glob
--from typing import Dict, Any, Optional, ContextManager
-+from typing import List, Dict, Any, Optional, ContextManager
- def isxfile(path: str) -> bool:
-@@ -XXX,XX +XXX,XX @@ class TestEnv(ContextManager['TestEnv']):
-                      'CACHEMODE_IS_DEFAULT', 'IMGFMT_GENERIC', 'IMGOPTSSYNTAX',
-                      'IMGKEYSECRET', 'QEMU_DEFAULT_MACHINE', 'MALLOC_PERTURB_']
-+    def prepare_subprocess(self, args: List[str]) -> Dict[str, str]:
-+        if self.debug:
-+            args.append('-d')
-+
-+        with open(args[0], encoding="utf-8") as f:
-+            try:
-+                if f.readline().rstrip() == '#!/usr/bin/env python3':
-+                    args.insert(0, self.python)
-+            except UnicodeDecodeError:  # binary test? for future.
-+                pass
-+
-+        os_env = os.environ.copy()
-+        os_env.update(self.get_env())
-+        return os_env
-+
-     def get_env(self) -> Dict[str, str]:
-         env = {}
-         for v in self.env_variables:
-diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/testrunner.py
-+++ b/tests/qemu-iotests/testrunner.py
-@@ -XXX,XX +XXX,XX @@ class TestRunner(ContextManager['TestRunner']):
-     def __init__(self, env: TestEnv, makecheck: bool = False,
-                  color: str = 'auto') -> None:
-         self.env = env
--        self.test_run_env = self.env.get_env()
-         self.makecheck = makecheck
-         self.last_elapsed = LastElapsedTime('.last-elapsed-cache', env)
-@@ -XXX,XX +XXX,XX @@ def do_run_test(self, test: str) -> TestResult:
-             silent_unlink(p)
-         args = [str(f_test.resolve())]
--        if self.env.debug:
--            args.append('-d')
--
--        with f_test.open(encoding="utf-8") as f:
--            try:
--                if f.readline().rstrip() == '#!/usr/bin/env python3':
--                    args.insert(0, self.env.python)
--            except UnicodeDecodeError:  # binary test? for future.
--                pass
--
--        env = os.environ.copy()
--        env.update(self.test_run_env)
-+        env = self.env.prepare_subprocess(args)
-         t0 = time.time()
-         with f_bad.open('w', encoding="utf-8") as f:
---
-.31.1

-[PULL 08/19] qemu-iotests: let "check" spawn an arbitrary test command
+Deleted patch
-From: Paolo Bonzini <pbonzini@redhat.com>
-Right now there is no easy way for "check" to print a reproducer command.
-Because such a reproducer command line would be huge, we can instead teach
-check to start a command of our choice.  This can be for example a Python
-unit test with arguments to only run a specific subtest.
-Move the trailing empty line to print_env(), since it always looks better
-and one caller was not adding it.
-Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
-Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Tested-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
-Message-Id: <20210323181928.311862-5-pbonzini@redhat.com>
-Message-Id: <20210503110110.476887-5-pbonzini@redhat.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- tests/qemu-iotests/check         | 19 ++++++++++++++++++-
- tests/qemu-iotests/testenv.py    |  3 ++-
- tests/qemu-iotests/testrunner.py |  1 -
-files changed, 20 insertions(+), 3 deletions(-)
-diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
-index XXXXXXX..XXXXXXX 100755
---- a/tests/qemu-iotests/check
-+++ b/tests/qemu-iotests/check
-@@ -XXX,XX +XXX,XX @@
- import os
- import sys
- import argparse
-+import shutil
-+from pathlib import Path
-+
- from findtests import TestFinder
- from testenv import TestEnv
- from testrunner import TestRunner
-@@ -XXX,XX +XXX,XX @@ def make_argparser() -> argparse.ArgumentParser:
-                        'rerun failed ./check command, starting from the '
-                        'middle of the process.')
-     g_sel.add_argument('tests', metavar='TEST_FILES', nargs='*',
--                       help='tests to run')
-+                       help='tests to run, or "--" followed by a command')
-     return p
-@@ -XXX,XX +XXX,XX @@ if __name__ == '__main__':
-                   imgopts=args.imgopts, misalign=args.misalign,
-                   debug=args.debug, valgrind=args.valgrind)
-+    if len(sys.argv) > 1 and sys.argv[-len(args.tests)-1] == '--':
-+        if not args.tests:
-+            sys.exit("missing command after '--'")
-+        cmd = args.tests
-+        env.print_env()
-+        exec_pathstr = shutil.which(cmd[0])
-+        if exec_pathstr is None:
-+            sys.exit('command not found: ' + cmd[0])
-+        exec_path = Path(exec_pathstr).resolve()
-+        cmd[0] = str(exec_path)
-+        full_env = env.prepare_subprocess(cmd)
-+        os.chdir(exec_path.parent)
-+        os.execve(cmd[0], cmd, full_env)
-+
-     testfinder = TestFinder(test_dir=env.source_iotests)
-     groups = args.groups.split(',') if args.groups else None
-diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/testenv.py
-+++ b/tests/qemu-iotests/testenv.py
-@@ -XXX,XX +XXX,XX @@ def print_env(self) -> None:
- PLATFORM      -- {platform}
- TEST_DIR      -- {TEST_DIR}
- SOCK_DIR      -- {SOCK_DIR}
--SOCKET_SCM_HELPER -- {SOCKET_SCM_HELPER}"""
-+SOCKET_SCM_HELPER -- {SOCKET_SCM_HELPER}
-+"""
-         args = collections.defaultdict(str, self.get_env())
-diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/testrunner.py
-+++ b/tests/qemu-iotests/testrunner.py
-@@ -XXX,XX +XXX,XX @@ def run_tests(self, tests: List[str]) -> bool:
-         if not self.makecheck:
-             self.env.print_env()
--            print()
-         test_field_width = max(len(os.path.basename(t)) for t in tests) + 2
---
-.31.1

-[PULL 09/19] qemu-iotests: fix case of SOCK_DIR already in the environment
+Deleted patch
-From: Paolo Bonzini <pbonzini@redhat.com>
-Due to a typo, in this case the SOCK_DIR was not being created.
-Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
-Tested-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
-Message-Id: <20210323181928.311862-6-pbonzini@redhat.com>
-Message-Id: <20210503110110.476887-6-pbonzini@redhat.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- tests/qemu-iotests/testenv.py | 2 +-
-file changed, 1 insertion(+), 1 deletion(-)
-diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/testenv.py
-+++ b/tests/qemu-iotests/testenv.py
-@@ -XXX,XX +XXX,XX @@ def init_directories(self) -> None:
-         try:
-             self.sock_dir = os.environ['SOCK_DIR']
-             self.tmp_sock_dir = False
--            Path(self.test_dir).mkdir(parents=True, exist_ok=True)
-+            Path(self.sock_dir).mkdir(parents=True, exist_ok=True)
-         except KeyError:
-             self.sock_dir = tempfile.mkdtemp()
-             self.tmp_sock_dir = True
---
-.31.1

-[PULL 10/19] Document qemu-img options data_file and data_file_raw
+Deleted patch
-From: Connor Kuehl <ckuehl@redhat.com>
-The contents of this patch were initially developed and posted by Han
-Han[1], however, it appears the original patch was not applied. Since
-then, the relevant documentation has been moved and adapted to a new
-format.
-I've taken most of the original wording and tweaked it according to
-some of the feedback from the original patch submission. I've also
-adapted it to restructured text, which is the format the documentation
-currently uses.
-[1] https://lists.nongnu.org/archive/html/qemu-block/2019-10/msg01253.html
-Fixes: https://bugzilla.redhat.com/1763105
-Signed-off-by: Han Han <hhan@redhat.com>
-Suggested-by: Max Reitz <mreitz@redhat.com>
-[ Max: provided description of data_file_raw behavior ]
-Signed-off-by: Connor Kuehl <ckuehl@redhat.com>
-Message-Id: <20210505195512.391128-1-ckuehl@redhat.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- docs/tools/qemu-img.rst | 31 +++++++++++++++++++++++++++++++
-file changed, 31 insertions(+)
-diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
-index XXXXXXX..XXXXXXX 100644
---- a/docs/tools/qemu-img.rst
-+++ b/docs/tools/qemu-img.rst
-@@ -XXX,XX +XXX,XX @@ Supported image file formats:
-     issue ``lsattr filename`` to check if the NOCOW flag is set or not
-     (Capital 'C' is NOCOW flag).
-+  ``data_file``
-+    Filename where all guest data will be stored. If this option is used,
-+    the qcow2 file will only contain the image's metadata.
-+
-+    Note: Data loss will occur if the given filename already exists when
-+    using this option with ``qemu-img create`` since ``qemu-img`` will create
-+    the data file anew, overwriting the file's original contents. To simply
-+    update the reference to point to the given pre-existing file, use
-+    ``qemu-img amend``.
-+
-+  ``data_file_raw``
-+    If this option is set to ``on``, QEMU will always keep the external data
-+    file consistent as a standalone read-only raw image.
-+
-+    It does this by forwarding all write accesses to the qcow2 file through to
-+    the raw data file, including their offsets. Therefore, data that is visible
-+    on the qcow2 node (i.e., to the guest) at some offset is visible at the same
-+    offset in the raw data file. This results in a read-only raw image. Writes
-+    that bypass the qcow2 metadata may corrupt the qcow2 metadata because the
-+    out-of-band writes may result in the metadata falling out of sync with the
-+    raw image.
-+
-+    If this option is ``off``, QEMU will use the data file to store data in an
-+    arbitrary manner. The file’s content will not make sense without the
-+    accompanying qcow2 metadata. Where data is written will have no relation to
-+    its offset as seen by the guest, and some writes (specifically zero writes)
-+    may not be forwarded to the data file at all, but will only be handled by
-+    modifying qcow2 metadata.
-+
-+    This option can only be enabled if ``data_file`` is set.
-+
- ``Other``
-   QEMU also supports various other image file formats for
---
-.31.1

-[PULL 11/19] block/copy-on-read: use bdrv_drop_filter() and drop s->active
+Deleted patch
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Now, after huge update of block graph permission update algorithm, we
-don't need this workaround with active state of the filter. Drop it and
-use new smart bdrv_drop_filter() function.
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Message-Id: <20210506194143.394141-1-vsementsov@virtuozzo.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block/copy-on-read.c | 33 +--------------------------------
-file changed, 1 insertion(+), 32 deletions(-)
-diff --git a/block/copy-on-read.c b/block/copy-on-read.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/copy-on-read.c
-+++ b/block/copy-on-read.c
-@@ -XXX,XX +XXX,XX @@
- typedef struct BDRVStateCOR {
--    bool active;
-     BlockDriverState *bottom_bs;
-     bool chain_frozen;
- } BDRVStateCOR;
-@@ -XXX,XX +XXX,XX @@ static int cor_open(BlockDriverState *bs, QDict *options, int flags,
-          */
-         bdrv_ref(bottom_bs);
-     }
--    state->active = true;
-     state->bottom_bs = bottom_bs;
-     /*
-@@ -XXX,XX +XXX,XX @@ static void cor_child_perm(BlockDriverState *bs, BdrvChild *c,
-                            uint64_t perm, uint64_t shared,
-                            uint64_t *nperm, uint64_t *nshared)
- {
--    BDRVStateCOR *s = bs->opaque;
--
--    if (!s->active) {
--        /*
--         * While the filter is being removed
--         */
--        *nperm = 0;
--        *nshared = BLK_PERM_ALL;
--        return;
--    }
--
-     *nperm = perm & PERM_PASSTHROUGH;
-     *nshared = (shared & PERM_PASSTHROUGH) | PERM_UNCHANGED;
-@@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_copy_on_read = {
- void bdrv_cor_filter_drop(BlockDriverState *cor_filter_bs)
- {
--    BdrvChild *child;
--    BlockDriverState *bs;
-     BDRVStateCOR *s = cor_filter_bs->opaque;
--    child = bdrv_filter_child(cor_filter_bs);
--    if (!child) {
--        return;
--    }
--    bs = child->bs;
--
--    /* Retain the BDS until we complete the graph change. */
--    bdrv_ref(bs);
--    /* Hold a guest back from writing while permissions are being reset. */
--    bdrv_drained_begin(bs);
--    /* Drop permissions before the graph change. */
--    s->active = false;
-     /* unfreeze, as otherwise bdrv_replace_node() will fail */
-     if (s->chain_frozen) {
-         s->chain_frozen = false;
-         bdrv_unfreeze_backing_chain(cor_filter_bs, s->bottom_bs);
-     }
--    bdrv_child_refresh_perms(cor_filter_bs, child, &error_abort);
--    bdrv_replace_node(cor_filter_bs, bs, &error_abort);
--
--    bdrv_drained_end(bs);
--    bdrv_unref(bs);
-+    bdrv_drop_filter(cor_filter_bs, &error_abort);
-     bdrv_unref(cor_filter_bs);
- }
---
-.31.1

-[PULL 12/19] qemu-iotests: fix pylint 2.8 consider-using-with error
+Deleted patch
-From: Emanuele Giuseppe Esposito <eesposit@redhat.com>
-pylint 2.8 introduces consider-using-with error, suggesting
-to use the 'with' block statement when possible.
-Modify all subprocess.Popen call to use the 'with' statement,
-except the one in __init__ of QemuIoInteractive class, since
-it is assigned to a class field and used in other methods.
-Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
-Message-Id: <20210510190449.65948-1-eesposit@redhat.com>
-[mreitz: Disable bad-option-value warning in the iotests' pylintrc, so
-         that disabling consider-using-with in QemuIoInteractive will
-         not produce a warning in pre-2.8 pylint versions]
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- tests/qemu-iotests/iotests.py    | 65 ++++++++++++++++----------------
- tests/qemu-iotests/pylintrc      |  3 ++
- tests/qemu-iotests/testrunner.py | 22 +++++------
-files changed, 47 insertions(+), 43 deletions(-)
-diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/iotests.py
-+++ b/tests/qemu-iotests/iotests.py
-@@ -XXX,XX +XXX,XX @@ def qemu_tool_pipe_and_status(tool: str, args: Sequence[str],
-     Run a tool and return both its output and its exit code
-     """
-     stderr = subprocess.STDOUT if connect_stderr else None
--    subp = subprocess.Popen(args,
--                            stdout=subprocess.PIPE,
--                            stderr=stderr,
--                            universal_newlines=True)
--    output = subp.communicate()[0]
--    if subp.returncode < 0:
--        cmd = ' '.join(args)
--        sys.stderr.write(f'{tool} received signal {-subp.returncode}: {cmd}\n')
--    return (output, subp.returncode)
-+    with subprocess.Popen(args, stdout=subprocess.PIPE,
-+                          stderr=stderr, universal_newlines=True) as subp:
-+        output = subp.communicate()[0]
-+        if subp.returncode < 0:
-+            cmd = ' '.join(args)
-+            sys.stderr.write(f'{tool} received signal \
-+                               {-subp.returncode}: {cmd}\n')
-+        return (output, subp.returncode)
- def qemu_img_pipe_and_status(*args: str) -> Tuple[str, int]:
-     """
-@@ -XXX,XX +XXX,XX @@ def qemu_io_silent_check(*args):
- class QemuIoInteractive:
-     def __init__(self, *args):
-         self.args = qemu_io_args_no_fmt + list(args)
-+        # We need to keep the Popen objext around, and not
-+        # close it immediately. Therefore, disable the pylint check:
-+        # pylint: disable=consider-using-with
-         self._p = subprocess.Popen(self.args, stdin=subprocess.PIPE,
-                                    stdout=subprocess.PIPE,
-                                    stderr=subprocess.STDOUT,
-@@ -XXX,XX +XXX,XX @@ def qemu_nbd_popen(*args):
-     cmd.extend(args)
-     log('Start NBD server')
--    p = subprocess.Popen(cmd)
--    try:
--        while not os.path.exists(pid_file):
--            if p.poll() is not None:
--                raise RuntimeError(
--                    "qemu-nbd terminated with exit code {}: {}"
--                    .format(p.returncode, ' '.join(cmd)))
--
--            time.sleep(0.01)
--        yield
--    finally:
--        if os.path.exists(pid_file):
--            os.remove(pid_file)
--        log('Kill NBD server')
--        p.kill()
--        p.wait()
-+    with subprocess.Popen(cmd) as p:
-+        try:
-+            while not os.path.exists(pid_file):
-+                if p.poll() is not None:
-+                    raise RuntimeError(
-+                        "qemu-nbd terminated with exit code {}: {}"
-+                        .format(p.returncode, ' '.join(cmd)))
-+
-+                time.sleep(0.01)
-+            yield
-+        finally:
-+            if os.path.exists(pid_file):
-+                os.remove(pid_file)
-+            log('Kill NBD server')
-+            p.kill()
-+            p.wait()
- def compare_images(img1, img2, fmt1=imgfmt, fmt2=imgfmt):
-     '''Return True if two image files are identical'''
-@@ -XXX,XX +XXX,XX @@ def compare_images(img1, img2, fmt1=imgfmt, fmt2=imgfmt):
- def create_image(name, size):
-     '''Create a fully-allocated raw image with sector markers'''
--    file = open(name, 'wb')
--    i = 0
--    while i < size:
--        sector = struct.pack('>l504xl', i // 512, i // 512)
--        file.write(sector)
--        i = i + 512
--    file.close()
-+    with open(name, 'wb') as file:
-+        i = 0
-+        while i < size:
-+            sector = struct.pack('>l504xl', i // 512, i // 512)
-+            file.write(sector)
-+            i = i + 512
- def image_size(img):
-     '''Return image's virtual size'''
-diff --git a/tests/qemu-iotests/pylintrc b/tests/qemu-iotests/pylintrc
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/pylintrc
-+++ b/tests/qemu-iotests/pylintrc
-@@ -XXX,XX +XXX,XX @@ disable=invalid-name,
-         too-many-public-methods,
-         # pylint warns about Optional[] etc. as unsubscriptable in 3.9
-         unsubscriptable-object,
-+        # Sometimes we need to disable a newly introduced pylint warning.
-+        # Doing so should not produce a warning in older versions of pylint.
-+        bad-option-value,
-         # These are temporary, and should be removed:
-         missing-docstring,
-         too-many-return-statements,
-diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/testrunner.py
-+++ b/tests/qemu-iotests/testrunner.py
-@@ -XXX,XX +XXX,XX @@ def do_run_test(self, test: str) -> TestResult:
-         t0 = time.time()
-         with f_bad.open('w', encoding="utf-8") as f:
--            proc = subprocess.Popen(args, cwd=str(f_test.parent), env=env,
--                                    stdout=f, stderr=subprocess.STDOUT)
--            try:
--                proc.wait()
--            except KeyboardInterrupt:
--                proc.terminate()
--                proc.wait()
--                return TestResult(status='not run',
--                                  description='Interrupted by user',
--                                  interrupted=True)
--            ret = proc.returncode
-+            with subprocess.Popen(args, cwd=str(f_test.parent), env=env,
-+                                  stdout=f, stderr=subprocess.STDOUT) as proc:
-+                try:
-+                    proc.wait()
-+                except KeyboardInterrupt:
-+                    proc.terminate()
-+                    proc.wait()
-+                    return TestResult(status='not run',
-+                                      description='Interrupted by user',
-+                                      interrupted=True)
-+                ret = proc.returncode
-         elapsed = round(time.time() - t0, 1)
---
-.31.1

-[PULL 14/19] block: drop write notifiers
+Deleted patch
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-They are unused now.
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Message-Id: <20210506090621.11848-3-vsementsov@virtuozzo.com>
-Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- include/block/block_int.h | 12 ------------
- block.c                   |  1 -
- block/io.c                |  6 ------
-files changed, 19 deletions(-)
-diff --git a/include/block/block_int.h b/include/block/block_int.h
-index XXXXXXX..XXXXXXX 100644
---- a/include/block/block_int.h
-+++ b/include/block/block_int.h
-@@ -XXX,XX +XXX,XX @@ struct BlockDriverState {
-      */
-     int64_t total_sectors;
--    /* Callback before write request is processed */
--    NotifierWithReturnList before_write_notifiers;
--
-     /* threshold limit for writes, in bytes. "High water mark". */
-     uint64_t write_threshold_offset;
-@@ -XXX,XX +XXX,XX @@ void bdrv_parse_filename_strip_prefix(const char *filename, const char *prefix,
- bool bdrv_backing_overridden(BlockDriverState *bs);
--/**
-- * bdrv_add_before_write_notifier:
-- *
-- * Register a callback that is invoked before write requests are processed but
-- * after any throttling or waiting for overlapping requests.
-- */
--void bdrv_add_before_write_notifier(BlockDriverState *bs,
--                                    NotifierWithReturn *notifier);
--
- /**
-  * bdrv_add_aio_context_notifier:
-  *
-diff --git a/block.c b/block.c
-index XXXXXXX..XXXXXXX 100644
---- a/block.c
-+++ b/block.c
-@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_new(void)
-     for (i = 0; i < BLOCK_OP_TYPE_MAX; i++) {
-         QLIST_INIT(&bs->op_blockers[i]);
-     }
--    notifier_with_return_list_init(&bs->before_write_notifiers);
-     qemu_co_mutex_init(&bs->reqs_lock);
-     qemu_mutex_init(&bs->dirty_bitmap_mutex);
-     bs->refcnt = 1;
-diff --git a/block/io.c b/block/io.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/io.c
-+++ b/block/io.c
-@@ -XXX,XX +XXX,XX @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov)
-     return true;
- }
--void bdrv_add_before_write_notifier(BlockDriverState *bs,
--                                    NotifierWithReturn *notifier)
--{
--    notifier_with_return_list_add(&bs->before_write_notifiers, notifier);
--}
--
- void bdrv_io_plug(BlockDriverState *bs)
- {
-     BdrvChild *child;
---
-.31.1

-[PULL 15/19] test-write-threshold: rewrite test_threshold_(not_)trigger tests
+[PULL 8/8] qapi: add '@fdset' feature for BlockdevOptionsVirtioBlkVhostVdpa
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+From: Stefano Garzarella <sgarzare@redhat.com>
-These tests use bdrv_write_threshold_exceeded() API, which is used only
+The virtio-blk-vhost-vdpa driver in libblkio 1.3.0 supports the fd
-for test (since pre-previous commit). Better is testing real API, which
+passing through the new 'fd' property.
 is used in block.c as well.
-So, let's call bdrv_write_threshold_check_write(), and check is
+Since now we are using qemu_open() on '@path' if the virtio-blk driver
-bs->write_threshold_offset cleared or not (it's cleared iff threshold
+supports the fd passing, let's announce it.
-triggered).
+In this way, the management layer can pass the file descriptor of an
 already opened vhost-vdpa character device. This is useful especially
 when the device can only be accessed with certain privileges.
-Also we get rid of BdrvTrackedRequest use here. Note, that paranoiac
+Add the '@fdset' feature only when the virtio-blk-vhost-vdpa driver
-bdrv_check_request() calls were added in 8b1170012b1 to protect
+in libblkio supports it.
 BdrvTrackedRequest. Drop them now.
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+Suggested-by: Markus Armbruster <armbru@redhat.com>
 Reviewed-by: Max Reitz <mreitz@redhat.com>
 Message-Id: <20210506090621.11848-4-vsementsov@virtuozzo.com>
 Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
 Message-id: 20230530071941.8954-3-sgarzare@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- tests/unit/test-write-threshold.c | 22 ++++------------------
+ qapi/block-core.json | 6 ++++++
-file changed, 4 insertions(+), 18 deletions(-)
+ meson.build          | 4 ++++
 files changed, 10 insertions(+)
-diff --git a/tests/unit/test-write-threshold.c b/tests/unit/test-write-threshold.c
+diff --git a/qapi/block-core.json b/qapi/block-core.json
 index XXXXXXX..XXXXXXX 100644
---- a/tests/unit/test-write-threshold.c
+--- a/qapi/block-core.json
-+++ b/tests/unit/test-write-threshold.c
++++ b/qapi/block-core.json
-@@ -XXX,XX +XXX,XX @@ static void test_threshold_multi_set_get(void)
+@@ -XXX,XX +XXX,XX @@
+ #
- static void test_threshold_not_trigger(void)
+ # @path: path to the vhost-vdpa character device.
- {
+ #
--    uint64_t amount = 0;
++# Features:
-     uint64_t threshold = 4 * 1024 * 1024;
++# @fdset: Member @path supports the special "/dev/fdset/N" path
-     BlockDriverState bs;
++#     (since 8.1)
--    BdrvTrackedRequest req;
++#
+ # Since: 7.2
-     memset(&bs, 0, sizeof(bs));
+ ##
--    memset(&req, 0, sizeof(req));
+ { 'struct': 'BlockdevOptionsVirtioBlkVhostVdpa',
--    req.offset = 1024;
+   'data': { 'path': 'str' },
--    req.bytes = 1024;
++  'features': [ { 'name' :'fdset',
--
++                  'if': 'CONFIG_BLKIO_VHOST_VDPA_FD' } ],
--    bdrv_check_request(req.offset, req.bytes, &error_abort);
+   'if': 'CONFIG_BLKIO' }
-     bdrv_write_threshold_set(&bs, threshold);
+ ##
--    amount = bdrv_write_threshold_exceeded(&bs, &req);
+diff --git a/meson.build b/meson.build
--    g_assert_cmpuint(amount, ==, 0);
+index XXXXXXX..XXXXXXX 100644
-+    bdrv_write_threshold_check_write(&bs, 1024, 1024);
+--- a/meson.build
-+    g_assert_cmpuint(bdrv_write_threshold_get(&bs), ==, threshold);
++++ b/meson.build
- }
+@@ -XXX,XX +XXX,XX @@ config_host_data.set('CONFIG_LZO', lzo.found())
+ config_host_data.set('CONFIG_MPATH', mpathpersist.found())
+ config_host_data.set('CONFIG_MPATH_NEW_API', mpathpersist_new_api)
- static void test_threshold_trigger(void)
+ config_host_data.set('CONFIG_BLKIO', blkio.found())
- {
++if blkio.found()
--    uint64_t amount = 0;
++  config_host_data.set('CONFIG_BLKIO_VHOST_VDPA_FD',
-     uint64_t threshold = 4 * 1024 * 1024;
++                       blkio.version().version_compare('>=1.3.0'))
-     BlockDriverState bs;
++endif
--    BdrvTrackedRequest req;
+ config_host_data.set('CONFIG_CURL', curl.found())
+ config_host_data.set('CONFIG_CURSES', curses.found())
-     memset(&bs, 0, sizeof(bs));
+ config_host_data.set('CONFIG_GBM', gbm.found())
 -    memset(&req, 0, sizeof(req));
 -    req.offset = (4 * 1024 * 1024) - 1024;
 -    req.bytes = 2 * 1024;
 -
 -    bdrv_check_request(req.offset, req.bytes, &error_abort);
      bdrv_write_threshold_set(&bs, threshold);
 -    amount = bdrv_write_threshold_exceeded(&bs, &req);
 -    g_assert_cmpuint(amount, >=, 1024);
 +    bdrv_write_threshold_check_write(&bs, threshold - 1024, 2 * 1024);
 +    g_assert_cmpuint(bdrv_write_threshold_get(&bs), ==, 0);
  }
  typedef struct TestStruct {
 --
-.31.1
+.40.1

The following changes since commit 96662996eda78c48aadddd4e76d8615c7eb72d80:

Merge remote-tracking branch 'remotes/dgilbert/tags/pull-migration-20210513a' into staging (2021-05-14 12:03:47 +0100)

are available in the Git repository at:

https://github.com/XanClic/qemu.git tags/pull-block-2021-05-14

for you to fetch changes up to c61ebf362d0abf288ce266845519d5a550a1d89f:

write-threshold: deal with includes (2021-05-14 16:14:10 +0200)

----------------------------------------------------------------
Block patches:
- drop block/io write notifiers
- qemu-iotests enhancements to make debugging easier
- rbd parsing fix
- HMP qemu-io fix (for iothreads)
- mirror job cancel relaxation (do not cancel in-flight requests when a
  READY mirror job is canceled with force=false)
- document qcow2's data_file and data_file_raw features
- fix iotest 297 for pylint 2.8
- block/copy-on-read refactoring

----------------------------------------------------------------
Connor Kuehl (3):
  iotests/231: Update expected deprecation message
  block/rbd: Add an escape-aware strchr helper
  Document qemu-img options data_file and data_file_raw

Emanuele Giuseppe Esposito (1):
  qemu-iotests: fix pylint 2.8 consider-using-with error

Paolo Bonzini (5):
  qemu-iotests: do not buffer the test output
  qemu-iotests: allow passing unittest.main arguments to the test
    scripts
  qemu-iotests: move command line and environment handling from
    TestRunner to TestEnv
  qemu-iotests: let "check" spawn an arbitrary test command
  qemu-iotests: fix case of SOCK_DIR already in the environment

Vladimir Sementsov-Ogievskiy (10):
  monitor: hmp_qemu_io: acquire aio contex, fix crash
  mirror: stop cancelling in-flight requests on non-force cancel in
    READY
  block/copy-on-read: use bdrv_drop_filter() and drop s->active
  block/write-threshold: don't use write notifiers
  block: drop write notifiers
  test-write-threshold: rewrite test_threshold_(not_)trigger tests
  block/write-threshold: drop extra APIs
  test-write-threshold: drop extra tests
  test-write-threshold: drop extra TestStruct structure
  write-threshold: deal with includes

-- 
2.31.1

From: Connor Kuehl <ckuehl@redhat.com>

The deprecation message in the expected output has technically been
wrong since the wrong version of a patch was applied to it. Because of
this, the test fails. Correct the expected output so that it passes.

Signed-off-by: Connor Kuehl <ckuehl@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Message-Id: <20210421212343.85524-2-ckuehl@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/231.out | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/231.out b/tests/qemu-iotests/231.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/231.out
+++ b/tests/qemu-iotests/231.out
@@ -XXX,XX +XXX,XX @@
 QA output created by 231
-qemu-img: RBD options encoded in the filename as keyvalue pairs is deprecated.  Future versions may cease to parse these options in the future.
+qemu-img: warning: RBD options encoded in the filename as keyvalue pairs is deprecated
 unable to get monitor info from DNS SRV with service name: ceph-mon
-no monitors specified to connect to.
 qemu-img: Could not open 'json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=BOGUS_CONF'}': error connecting: No such file or directory
 unable to get monitor info from DNS SRV with service name: ceph-mon
-no monitors specified to connect to.
 qemu-img: Could not open 'json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'BOGUS_CONF'}': error connecting: No such file or directory
 *** done
-- 
2.31.1

From: Connor Kuehl <ckuehl@redhat.com>

Sometimes the parser needs to further split a token it has collected
from the token input stream. Right now, it does a cursory check to see
if the relevant characters appear in the token to determine if it should
break it down further.

However, qemu_rbd_next_tok() will escape characters as it removes tokens
from the token stream and plain strchr() won't. This can make the
initial strchr() check slightly misleading since it implies
qemu_rbd_next_tok() will find the token and split on it, except the
reality is that qemu_rbd_next_tok() will pass over it if it is escaped.

Use a custom strchr to avoid mixing escaped and unescaped string
operations. Furthermore, this code is identical to how
qemu_rbd_next_tok() seeks its next token, so incorporate this custom
strchr into the body of that function to reduce duplication.

Reported-by: Han Han <hhan@redhat.com>
Fixes: https://bugzilla.redhat.com/1873913
Signed-off-by: Connor Kuehl <ckuehl@redhat.com>
Message-Id: <20210421212343.85524-3-ckuehl@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/rbd.c                | 32 +++++++++++++++++++++-----------
 tests/qemu-iotests/231     |  4 ++++
 tests/qemu-iotests/231.out |  3 +++
 3 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index XXXXXXX..XXXXXXX 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -XXX,XX +XXX,XX @@ static int qemu_rbd_connect(rados_t *cluster, rados_ioctx_t *io_ctx,
                             const char *keypairs, const char *secretid,
                             Error **errp);
 
+static char *qemu_rbd_strchr(char *src, char delim)
+{
+    char *p;
+
+    for (p = src; *p; ++p) {
+        if (*p == delim) {
+            return p;
+        }
+        if (*p == '\\' && p[1] != '\0') {
+            ++p;
+        }
+    }
+
+    return NULL;
+}
+
+
 static char *qemu_rbd_next_tok(char *src, char delim, char **p)
 {
     char *end;
 
     *p = NULL;
 
-    for (end = src; *end; ++end) {
-        if (*end == delim) {
-            break;
-        }
-        if (*end == '\\' && end[1] != '\0') {
-            end++;
-        }
-    }
-    if (*end == delim) {
+    end = qemu_rbd_strchr(src, delim);
+    if (end) {
         *p = end + 1;
         *end = '\0';
     }
@@ -XXX,XX +XXX,XX @@ static void qemu_rbd_parse_filename(const char *filename, QDict *options,
     qemu_rbd_unescape(found_str);
     qdict_put_str(options, "pool", found_str);
 
-    if (strchr(p, '@')) {
+    if (qemu_rbd_strchr(p, '@')) {
         image_name = qemu_rbd_next_tok(p, '@', &p);
 
         found_str = qemu_rbd_next_tok(p, ':', &p);
@@ -XXX,XX +XXX,XX @@ static void qemu_rbd_parse_filename(const char *filename, QDict *options,
         image_name = qemu_rbd_next_tok(p, ':', &p);
     }
     /* Check for namespace in the image_name */
-    if (strchr(image_name, '/')) {
+    if (qemu_rbd_strchr(image_name, '/')) {
         found_str = qemu_rbd_next_tok(image_name, '/', &image_name);
         qemu_rbd_unescape(found_str);
         qdict_put_str(options, "namespace", found_str);
diff --git a/tests/qemu-iotests/231 b/tests/qemu-iotests/231
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/231
+++ b/tests/qemu-iotests/231
@@ -XXX,XX +XXX,XX @@ _filter_conf()
 $QEMU_IMG info "json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=${BOGUS_CONF}'}" 2>&1 | _filter_conf
 $QEMU_IMG info "json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'${BOGUS_CONF}'}" 2>&1 | _filter_conf
 
+# Regression test: the qemu-img invocation is expected to fail, but it should
+# not seg fault the parser.
+$QEMU_IMG create "rbd:rbd/aa\/bb:conf=${BOGUS_CONF}" 1M 2>&1 | _filter_conf
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/231.out b/tests/qemu-iotests/231.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/231.out
+++ b/tests/qemu-iotests/231.out
@@ -XXX,XX +XXX,XX @@ unable to get monitor info from DNS SRV with service name: ceph-mon
 qemu-img: Could not open 'json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=BOGUS_CONF'}': error connecting: No such file or directory
 unable to get monitor info from DNS SRV with service name: ceph-mon
 qemu-img: Could not open 'json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'BOGUS_CONF'}': error connecting: No such file or directory
+Formatting 'rbd:rbd/aa\/bb:conf=BOGUS_CONF', fmt=raw size=1048576
+unable to get monitor info from DNS SRV with service name: ceph-mon
+qemu-img: rbd:rbd/aa\/bb:conf=BOGUS_CONF: error connecting: No such file or directory
 *** done
-- 
2.31.1

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

Max reported the following bug:

$ ./qemu-img create -f raw src.img 1G
$ ./qemu-img create -f raw dst.img 1G

$ (echo '
   {"execute":"qmp_capabilities"}
   {"execute":"blockdev-mirror",
    "arguments":{"job-id":"mirror",
                 "device":"source",
                 "target":"target",
                 "sync":"full",
                 "filter-node-name":"mirror-top"}}
'; sleep 3; echo '
   {"execute":"human-monitor-command",
    "arguments":{"command-line":
                 "qemu-io mirror-top \"write 0 1G\""}}') \
| x86_64-softmmu/qemu-system-x86_64 \
   -qmp stdio \
   -blockdev file,node-name=source,filename=src.img \
   -blockdev file,node-name=target,filename=dst.img \
   -object iothread,id=iothr0 \
   -device virtio-blk,drive=source,iothread=iothr0

crashes:

0  raise () at /usr/lib/libc.so.6
1  abort () at /usr/lib/libc.so.6
2  error_exit
   (err=<optimized out>,
   msg=msg@entry=0x55fbb1634790 <__func__.27> "qemu_mutex_unlock_impl")
   at ../util/qemu-thread-posix.c:37
3  qemu_mutex_unlock_impl
   (mutex=mutex@entry=0x55fbb25ab6e0,
   file=file@entry=0x55fbb1636957 "../util/async.c",
   line=line@entry=650)
   at ../util/qemu-thread-posix.c:109
4  aio_context_release (ctx=ctx@entry=0x55fbb25ab680) at ../util/async.c:650
5  bdrv_do_drained_begin
   (bs=bs@entry=0x55fbb3a87000, recursive=recursive@entry=false,
   parent=parent@entry=0x0,
   ignore_bds_parents=ignore_bds_parents@entry=false,
   poll=poll@entry=true) at ../block/io.c:441
6  bdrv_do_drained_begin
   (poll=true, ignore_bds_parents=false, parent=0x0, recursive=false,
   bs=0x55fbb3a87000) at ../block/io.c:448
7  blk_drain (blk=0x55fbb26c5a00) at ../block/block-backend.c:1718
8  blk_unref (blk=0x55fbb26c5a00) at ../block/block-backend.c:498
9  blk_unref (blk=0x55fbb26c5a00) at ../block/block-backend.c:491
10 hmp_qemu_io (mon=0x7fffaf3fc7d0, qdict=<optimized out>)
   at ../block/monitor/block-hmp-cmds.c:628

man pthread_mutex_unlock
...
    EPERM  The  mutex type is PTHREAD_MUTEX_ERRORCHECK or
    PTHREAD_MUTEX_RECURSIVE, or the mutex is a robust mutex, and the
    current thread does not own the mutex.

So, thread doesn't own the mutex. And we have iothread here.

Next, note that AIO_WAIT_WHILE() documents that ctx must be acquired
exactly once by caller. But where is it acquired in the call stack?
Seems nowhere.

qemuio_command do acquire aio context.. But we need context acquired
around blk_unref() as well and actually around blk_insert_bs() too.

Let's refactor qemuio_command so that it doesn't acquire aio context
but callers do that instead. This way we can cleanly acquire aio
context in hmp_qemu_io() around all three calls.

Reported-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210423134233.51495-1-vsementsov@virtuozzo.com>
[mreitz: Fixed comment]
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/monitor/block-hmp-cmds.c | 31 +++++++++++++++++++++----------
 qemu-io-cmds.c                 |  8 ++++----
 qemu-io.c                      | 17 +++++++++++++++--
 3 files changed, 40 insertions(+), 16 deletions(-)

diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index XXXXXXX..XXXXXXX 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -XXX,XX +XXX,XX @@ void hmp_eject(Monitor *mon, const QDict *qdict)
 
 void hmp_qemu_io(Monitor *mon, const QDict *qdict)
 {
-    BlockBackend *blk;
+    BlockBackend *blk = NULL;
+    BlockDriverState *bs = NULL;
     BlockBackend *local_blk = NULL;
+    AioContext *ctx = NULL;
     bool qdev = qdict_get_try_bool(qdict, "qdev", false);
     const char *device = qdict_get_str(qdict, "device");
     const char *command = qdict_get_str(qdict, "command");
@@ -XXX,XX +XXX,XX @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict)
     } else {
         blk = blk_by_name(device);
         if (!blk) {
-            BlockDriverState *bs = bdrv_lookup_bs(NULL, device, &err);
-            if (bs) {
-                blk = local_blk = blk_new(bdrv_get_aio_context(bs),
-                                          0, BLK_PERM_ALL);
-                ret = blk_insert_bs(blk, bs, &err);
-                if (ret < 0) {
-                    goto fail;
-                }
-            } else {
+            bs = bdrv_lookup_bs(NULL, device, &err);
+            if (!bs) {
                 goto fail;
             }
         }
     }
 
+    ctx = blk ? blk_get_aio_context(blk) : bdrv_get_aio_context(bs);
+    aio_context_acquire(ctx);
+
+    if (bs) {
+        blk = local_blk = blk_new(bdrv_get_aio_context(bs), 0, BLK_PERM_ALL);
+        ret = blk_insert_bs(blk, bs, &err);
+        if (ret < 0) {
+            goto fail;
+        }
+    }
+
     /*
      * Notably absent: Proper permission management. This is sad, but it seems
      * almost impossible to achieve without changing the semantics and thereby
@@ -XXX,XX +XXX,XX @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict)
 
 fail:
     blk_unref(local_blk);
+
+    if (ctx) {
+        aio_context_release(ctx);
+    }
+
     hmp_handle_error(mon, err);
 }
 
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index XXXXXXX..XXXXXXX 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -XXX,XX +XXX,XX @@ static const cmdinfo_t help_cmd = {
     .oneline    = "help for one or all commands",
 };
 
+/*
+ * Called with aio context of blk acquired. Or with qemu_get_aio_context()
+ * context acquired if blk is NULL.
+ */
 int qemuio_command(BlockBackend *blk, const char *cmd)
 {
-    AioContext *ctx;
     char *input;
     const cmdinfo_t *ct;
     char **v;
@@ -XXX,XX +XXX,XX @@ int qemuio_command(BlockBackend *blk, const char *cmd)
     if (c) {
         ct = find_command(v[0]);
         if (ct) {
-            ctx = blk ? blk_get_aio_context(blk) : qemu_get_aio_context();
-            aio_context_acquire(ctx);
             ret = command(blk, ct, c, v);
-            aio_context_release(ctx);
         } else {
             fprintf(stderr, "command \"%s\" not found\n", v[0]);
             ret = -EINVAL;
diff --git a/qemu-io.c b/qemu-io.c
index XXXXXXX..XXXXXXX 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -XXX,XX +XXX,XX @@ static void prep_fetchline(void *opaque)
     *fetchable= 1;
 }
 
+static int do_qemuio_command(const char *cmd)
+{
+    int ret;
+    AioContext *ctx =
+        qemuio_blk ? blk_get_aio_context(qemuio_blk) : qemu_get_aio_context();
+
+    aio_context_acquire(ctx);
+    ret = qemuio_command(qemuio_blk, cmd);
+    aio_context_release(ctx);
+
+    return ret;
+}
+
 static int command_loop(void)
 {
     int i, fetchable = 0, prompted = 0;
@@ -XXX,XX +XXX,XX @@ static int command_loop(void)
     char *input;
 
     for (i = 0; !quit_qemu_io && i < ncmdline; i++) {
-        ret = qemuio_command(qemuio_blk, cmdline[i]);
+        ret = do_qemuio_command(cmdline[i]);
         if (ret < 0) {
             last_error = ret;
         }
@@ -XXX,XX +XXX,XX @@ static int command_loop(void)
         if (input == NULL) {
             break;
         }
-        ret = qemuio_command(qemuio_blk, input);
+        ret = do_qemuio_command(input);
         g_free(input);
 
         if (ret < 0) {
-- 
2.31.1

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

If mirror is READY than cancel operation is not discarding the whole
result of the operation, but instead it's a documented way get a
point-in-time snapshot of source disk.

So, we should not cancel any requests if mirror is READ and
force=false. Let's fix that case.

Note, that bug that we have before this commit is not critical, as the
only .bdrv_cancel_in_flight implementation is nbd_cancel_in_flight()
and it cancels only requests waiting for reconnection, so it should be
rare case.

Fixes: 521ff8b779b11c394dbdc43f02e158dd99df308a
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210421075858.40197-1-vsementsov@virtuozzo.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 include/block/block_int.h | 2 +-
 include/qemu/job.h        | 2 +-
 block/backup.c            | 2 +-
 block/mirror.c            | 6 ++++--
 job.c                     | 2 +-
 tests/qemu-iotests/264    | 2 +-
 6 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -XXX,XX +XXX,XX @@ struct BlockDriver {
      * of in-flight requests, so don't waste the time if possible.
      *
      * One example usage is to avoid waiting for an nbd target node reconnect
-     * timeout during job-cancel.
+     * timeout during job-cancel with force=true.
      */
     void (*bdrv_cancel_in_flight)(BlockDriverState *bs);
 
diff --git a/include/qemu/job.h b/include/qemu/job.h
index XXXXXXX..XXXXXXX 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -XXX,XX +XXX,XX @@ struct JobDriver {
     /**
      * If the callback is not NULL, it will be invoked in job_cancel_async
      */
-    void (*cancel)(Job *job);
+    void (*cancel)(Job *job, bool force);
 
 
     /** Called when the job is freed */
diff --git a/block/backup.c b/block/backup.c
index XXXXXXX..XXXXXXX 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn backup_set_speed(BlockJob *job, int64_t speed)
     }
 }
 
-static void backup_cancel(Job *job)
+static void backup_cancel(Job *job, bool force)
 {
     BackupBlockJob *s = container_of(job, BackupBlockJob, common.job);
 
diff --git a/block/mirror.c b/block/mirror.c
index XXXXXXX..XXXXXXX 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -XXX,XX +XXX,XX @@ static bool mirror_drained_poll(BlockJob *job)
     return !!s->in_flight;
 }
 
-static void mirror_cancel(Job *job)
+static void mirror_cancel(Job *job, bool force)
 {
     MirrorBlockJob *s = container_of(job, MirrorBlockJob, common.job);
     BlockDriverState *target = blk_bs(s->target);
 
-    bdrv_cancel_in_flight(target);
+    if (force || !job_is_ready(job)) {
+        bdrv_cancel_in_flight(target);
+    }
 }
 
 static const BlockJobDriver mirror_job_driver = {
diff --git a/job.c b/job.c
index XXXXXXX..XXXXXXX 100644
--- a/job.c
+++ b/job.c
@@ -XXX,XX +XXX,XX @@ static int job_finalize_single(Job *job)
 static void job_cancel_async(Job *job, bool force)
 {
     if (job->driver->cancel) {
-        job->driver->cancel(job);
+        job->driver->cancel(job, force);
     }
     if (job->user_paused) {
         /* Do not call job_enter here, the caller will handle it.  */
diff --git a/tests/qemu-iotests/264 b/tests/qemu-iotests/264
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/264
+++ b/tests/qemu-iotests/264
@@ -XXX,XX +XXX,XX @@ class TestNbdReconnect(iotests.QMPTestCase):
             self.assert_qmp(result, 'return', {})
 
     def cancel_job(self):
-        result = self.vm.qmp('block-job-cancel', device='drive0')
+        result = self.vm.qmp('block-job-cancel', device='drive0', force=True)
         self.assert_qmp(result, 'return', {})
 
         start_t = time.time()
-- 
2.31.1

From: Paolo Bonzini <pbonzini@redhat.com>

Instead of buffering the test output into a StringIO, patch it on
the fly by wrapping sys.stdout's write method.  This can be
done unconditionally, even if using -d, which makes execute_unittest
a bit simpler.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Tested-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Message-Id: <20210323181928.311862-2-pbonzini@redhat.com>
Message-Id: <20210503110110.476887-2-pbonzini@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/240.out    |  8 ++--
 tests/qemu-iotests/245.out    |  8 ++--
 tests/qemu-iotests/295.out    |  6 +--
 tests/qemu-iotests/296.out    |  8 ++--
 tests/qemu-iotests/iotests.py | 70 ++++++++++++++++++++---------------
 5 files changed, 56 insertions(+), 44 deletions(-)

diff --git a/tests/qemu-iotests/240.out b/tests/qemu-iotests/240.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/240.out
+++ b/tests/qemu-iotests/240.out
@@ -XXX,XX +XXX,XX @@
 {"return": {}}
 {"execute": "blockdev-del", "arguments": {"node-name": "hd0"}}
 {"return": {}}
-==Attach two SCSI disks using the same block device and the same iothread==
+.==Attach two SCSI disks using the same block device and the same iothread==
 {"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "hd0", "read-only": true, "read-zeroes": true}}
 {"return": {}}
 {"execute": "object-add", "arguments": {"id": "iothread0", "qom-type": "iothread"}}
@@ -XXX,XX +XXX,XX @@
 {"return": {}}
 {"execute": "blockdev-del", "arguments": {"node-name": "hd0"}}
 {"return": {}}
-==Attach two SCSI disks using the same block device but different iothreads==
+.==Attach two SCSI disks using the same block device but different iothreads==
 {"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "hd0", "read-only": true, "read-zeroes": true}}
 {"return": {}}
 {"execute": "object-add", "arguments": {"id": "iothread0", "qom-type": "iothread"}}
@@ -XXX,XX +XXX,XX @@
 {"return": {}}
 {"execute": "blockdev-del", "arguments": {"node-name": "hd0"}}
 {"return": {}}
-==Attach a SCSI disks using the same block device as a NBD server==
+.==Attach a SCSI disks using the same block device as a NBD server==
 {"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "hd0", "read-only": true, "read-zeroes": true}}
 {"return": {}}
 {"execute": "nbd-server-start", "arguments": {"addr": {"data": {"path": "SOCK_DIR/PID-nbd.sock"}, "type": "unix"}}}
@@ -XXX,XX +XXX,XX @@
 {"return": {}}
 {"execute": "device_add", "arguments": {"drive": "hd0", "driver": "scsi-hd", "id": "scsi-hd0"}}
 {"return": {}}
-....
+.
 ----------------------------------------------------------------------
 Ran 4 tests
 
diff --git a/tests/qemu-iotests/245.out b/tests/qemu-iotests/245.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/245.out
+++ b/tests/qemu-iotests/245.out
@@ -XXX,XX +XXX,XX @@
-{"execute": "job-finalize", "arguments": {"id": "commit0"}}
+..{"execute": "job-finalize", "arguments": {"id": "commit0"}}
 {"return": {}}
 {"data": {"id": "commit0", "type": "commit"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
 {"data": {"device": "commit0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "commit"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
-{"execute": "job-finalize", "arguments": {"id": "stream0"}}
+...{"execute": "job-finalize", "arguments": {"id": "stream0"}}
 {"return": {}}
 {"data": {"id": "stream0", "type": "stream"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
 {"data": {"device": "stream0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "stream"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
-{"execute": "job-finalize", "arguments": {"id": "stream0"}}
+.{"execute": "job-finalize", "arguments": {"id": "stream0"}}
 {"return": {}}
 {"data": {"id": "stream0", "type": "stream"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
 {"data": {"device": "stream0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "stream"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
-.....................
+...............
 ----------------------------------------------------------------------
 Ran 21 tests
 
diff --git a/tests/qemu-iotests/295.out b/tests/qemu-iotests/295.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/295.out
+++ b/tests/qemu-iotests/295.out
@@ -XXX,XX +XXX,XX @@
 {"return": {}}
 {"execute": "job-dismiss", "arguments": {"id": "job_erase_key"}}
 {"return": {}}
-{"execute": "job-dismiss", "arguments": {"id": "job_add_key"}}
+.{"execute": "job-dismiss", "arguments": {"id": "job_add_key"}}
 {"return": {}}
 {"execute": "job-dismiss", "arguments": {"id": "job_erase_key"}}
 {"return": {}}
@@ -XXX,XX +XXX,XX @@ Job failed: Invalid password, cannot unlock any keyslot
 {"return": {}}
 {"execute": "job-dismiss", "arguments": {"id": "job_add_key"}}
 {"return": {}}
-{"execute": "job-dismiss", "arguments": {"id": "job_add_key"}}
+.{"execute": "job-dismiss", "arguments": {"id": "job_add_key"}}
 {"return": {}}
 {"execute": "job-dismiss", "arguments": {"id": "job_add_key"}}
 {"return": {}}
@@ -XXX,XX +XXX,XX @@ Job failed: All the active keyslots match the (old) password that was given and
 {"return": {}}
 {"execute": "job-dismiss", "arguments": {"id": "job_erase_key"}}
 {"return": {}}
-...
+.
 ----------------------------------------------------------------------
 Ran 3 tests
 
diff --git a/tests/qemu-iotests/296.out b/tests/qemu-iotests/296.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/296.out
+++ b/tests/qemu-iotests/296.out
@@ -XXX,XX +XXX,XX @@ Job failed: Failed to get shared "consistent read" lock
 qemu-img: Failed to get shared "consistent read" lock
 Is another process using the image [TEST_DIR/test.img]?
 
-Formatting 'TEST_DIR/test.img', fmt=luks size=1048576 key-secret=keysec0 iter-time=10
+.Formatting 'TEST_DIR/test.img', fmt=luks size=1048576 key-secret=keysec0 iter-time=10
 
 Job failed: Block node is read-only
 {"execute": "job-dismiss", "arguments": {"id": "job0"}}
@@ -XXX,XX +XXX,XX @@ Job failed: Failed to get shared "consistent read" lock
 {"return": {}}
 {"execute": "job-dismiss", "arguments": {"id": "job0"}}
 {"return": {}}
-Formatting 'TEST_DIR/test.img', fmt=luks size=1048576 key-secret=keysec0 iter-time=10
+.Formatting 'TEST_DIR/test.img', fmt=luks size=1048576 key-secret=keysec0 iter-time=10
 
 {"return": {}}
 {"error": {"class": "GenericError", "desc": "Failed to get \"write\" lock"}}
-Formatting 'TEST_DIR/test.img', fmt=luks size=1048576 key-secret=keysec0 iter-time=10
+.Formatting 'TEST_DIR/test.img', fmt=luks size=1048576 key-secret=keysec0 iter-time=10
 
 {"return": {}}
 {"return": {}}
-....
+.
 ----------------------------------------------------------------------
 Ran 4 tests
 
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 @@
 import bz2
 from collections import OrderedDict
 import faulthandler
-import io
 import json
 import logging
 import os
@@ -XXX,XX +XXX,XX @@
 import sys
 import time
 from typing import (Any, Callable, Dict, Iterable,
-                    List, Optional, Sequence, Tuple, TypeVar)
+                    List, Optional, Sequence, TextIO, Tuple, Type, TypeVar)
 import unittest
 
 from contextlib import contextmanager
@@ -XXX,XX +XXX,XX @@ def func_wrapper(*args, **kwargs):
             return func(*args, **kwargs)
     return func_wrapper
 
+# We need to filter out the time taken from the output so that
+# qemu-iotest can reliably diff the results against master output,
+# and hide skipped tests from the reference output.
+
+class ReproducibleTestResult(unittest.TextTestResult):
+    def addSkip(self, test, reason):
+        # Same as TextTestResult, but print dot instead of "s"
+        unittest.TestResult.addSkip(self, test, reason)
+        if self.showAll:
+            self.stream.writeln("skipped {0!r}".format(reason))
+        elif self.dots:
+            self.stream.write(".")
+            self.stream.flush()
+
+class ReproducibleStreamWrapper:
+    def __init__(self, stream: TextIO):
+        self.stream = stream
+
+    def __getattr__(self, attr):
+        if attr in ('stream', '__getstate__'):
+            raise AttributeError(attr)
+        return getattr(self.stream, attr)
+
+    def write(self, arg=None):
+        arg = re.sub(r'Ran (\d+) tests? in [\d.]+s', r'Ran \1 tests', arg)
+        arg = re.sub(r' $skipped=\d+$', r'', arg)
+        self.stream.write(arg)
+
+class ReproducibleTestRunner(unittest.TextTestRunner):
+    def __init__(self, stream: Optional[TextIO] = None,
+             resultclass: Type[unittest.TestResult] = ReproducibleTestResult,
+             **kwargs: Any) -> None:
+        rstream = ReproducibleStreamWrapper(stream or sys.stdout)
+        super().__init__(stream=rstream,           # type: ignore
+                         descriptions=True,
+                         resultclass=resultclass,
+                         **kwargs)
+
 def execute_unittest(debug=False):
     """Executes unittests within the calling module."""
 
     verbosity = 2 if debug else 1
-
-    if debug:
-        output = sys.stdout
-    else:
-        # We need to filter out the time taken from the output so that
-        # qemu-iotest can reliably diff the results against master output.
-        output = io.StringIO()
-
-    runner = unittest.TextTestRunner(stream=output, descriptions=True,
-                                     verbosity=verbosity)
-    try:
-        # unittest.main() will use sys.exit(); so expect a SystemExit
-        # exception
-        unittest.main(testRunner=runner)
-    finally:
-        # We need to filter out the time taken from the output so that
-        # qemu-iotest can reliably diff the results against master output.
-        if not debug:
-            out = output.getvalue()
-            out = re.sub(r'Ran (\d+) tests? in [\d.]+s', r'Ran \1 tests', out)
-
-            # Hide skipped tests from the reference output
-            out = re.sub(r'OK $skipped=\d+$', 'OK', out)
-            out_first_line, out_rest = out.split('\n', 1)
-            out = out_first_line.replace('s', '.') + '\n' + out_rest
-
-            sys.stderr.write(out)
+    runner = ReproducibleTestRunner(verbosity=verbosity)
+    unittest.main(testRunner=runner)
 
 def execute_setup_common(supported_fmts: Sequence[str] = (),
                          supported_platforms: Sequence[str] = (),
-- 
2.31.1

From: Paolo Bonzini <pbonzini@redhat.com>

Python test scripts that use unittest consist of multiple tests.
unittest.main allows selecting which tests to run, but currently this
is not possible because the iotests wrapper ignores sys.argv.

unittest.main command line options also allow the user to pick the
desired options for verbosity, failfast mode, etc.  While "-d" is
currently translated to "-v", it also enables extra debug output,
and other options are not available at all.

These command line options only work if the unittest.main testRunner
argument is a type, rather than a TestRunner instance.  Therefore, pass
the class name and "verbosity" argument to unittest.main, and adjust for
the different default warnings between TextTestRunner and unittest.main.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Tested-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Message-Id: <20210323181928.311862-3-pbonzini@redhat.com>
Message-Id: <20210503110110.476887-3-pbonzini@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/iotests.py | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -XXX,XX +XXX,XX @@ def __init__(self, stream: Optional[TextIO] = None,
                          resultclass=resultclass,
                          **kwargs)
 
-def execute_unittest(debug=False):
+def execute_unittest(argv: List[str], debug: bool = False) -> None:
     """Executes unittests within the calling module."""
 
-    verbosity = 2 if debug else 1
-    runner = ReproducibleTestRunner(verbosity=verbosity)
-    unittest.main(testRunner=runner)
+    # Some tests have warnings, especially ResourceWarnings for unclosed
+    # files and sockets.  Ignore them for now to ensure reproducibility of
+    # the test output.
+    unittest.main(argv=argv,
+                  testRunner=ReproducibleTestRunner,
+                  verbosity=2 if debug else 1,
+                  warnings=None if sys.warnoptions else 'ignore')
 
 def execute_setup_common(supported_fmts: Sequence[str] = (),
                          supported_platforms: Sequence[str] = (),
@@ -XXX,XX +XXX,XX @@ def execute_test(*args, test_function=None, **kwargs):
 
     debug = execute_setup_common(*args, **kwargs)
     if not test_function:
-        execute_unittest(debug)
+        execute_unittest(sys.argv, debug)
     else:
         test_function()
 
-- 
2.31.1

From: Paolo Bonzini <pbonzini@redhat.com>

In the next patch, "check" will learn how to execute a test script without
going through TestRunner.  To enable this, keep only the text output
and subprocess handling in the TestRunner; move into TestEnv the logic
to prepare for running a subprocess.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Tested-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Message-Id: <20210323181928.311862-4-pbonzini@redhat.com>
Message-Id: <20210503110110.476887-4-pbonzini@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/testenv.py    | 17 ++++++++++++++++-
 tests/qemu-iotests/testrunner.py | 14 +-------------
 2 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -XXX,XX +XXX,XX @@
 import random
 import subprocess
 import glob
-from typing import Dict, Any, Optional, ContextManager
+from typing import List, Dict, Any, Optional, ContextManager
 
 
 def isxfile(path: str) -> bool:
@@ -XXX,XX +XXX,XX @@ class TestEnv(ContextManager['TestEnv']):
                      'CACHEMODE_IS_DEFAULT', 'IMGFMT_GENERIC', 'IMGOPTSSYNTAX',
                      'IMGKEYSECRET', 'QEMU_DEFAULT_MACHINE', 'MALLOC_PERTURB_']
 
+    def prepare_subprocess(self, args: List[str]) -> Dict[str, str]:
+        if self.debug:
+            args.append('-d')
+
+        with open(args[0], encoding="utf-8") as f:
+            try:
+                if f.readline().rstrip() == '#!/usr/bin/env python3':
+                    args.insert(0, self.python)
+            except UnicodeDecodeError:  # binary test? for future.
+                pass
+
+        os_env = os.environ.copy()
+        os_env.update(self.get_env())
+        return os_env
+
     def get_env(self) -> Dict[str, str]:
         env = {}
         for v in self.env_variables:
diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/testrunner.py
+++ b/tests/qemu-iotests/testrunner.py
@@ -XXX,XX +XXX,XX @@ class TestRunner(ContextManager['TestRunner']):
     def __init__(self, env: TestEnv, makecheck: bool = False,
                  color: str = 'auto') -> None:
         self.env = env
-        self.test_run_env = self.env.get_env()
         self.makecheck = makecheck
         self.last_elapsed = LastElapsedTime('.last-elapsed-cache', env)
 
@@ -XXX,XX +XXX,XX @@ def do_run_test(self, test: str) -> TestResult:
             silent_unlink(p)
 
         args = [str(f_test.resolve())]
-        if self.env.debug:
-            args.append('-d')
-
-        with f_test.open(encoding="utf-8") as f:
-            try:
-                if f.readline().rstrip() == '#!/usr/bin/env python3':
-                    args.insert(0, self.env.python)
-            except UnicodeDecodeError:  # binary test? for future.
-                pass
-
-        env = os.environ.copy()
-        env.update(self.test_run_env)
+        env = self.env.prepare_subprocess(args)
 
         t0 = time.time()
         with f_bad.open('w', encoding="utf-8") as f:
-- 
2.31.1

From: Paolo Bonzini <pbonzini@redhat.com>

Right now there is no easy way for "check" to print a reproducer command.
Because such a reproducer command line would be huge, we can instead teach
check to start a command of our choice.  This can be for example a Python
unit test with arguments to only run a specific subtest.

Move the trailing empty line to print_env(), since it always looks better
and one caller was not adding it.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Tested-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Message-Id: <20210323181928.311862-5-pbonzini@redhat.com>
Message-Id: <20210503110110.476887-5-pbonzini@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/check         | 19 ++++++++++++++++++-
 tests/qemu-iotests/testenv.py    |  3 ++-
 tests/qemu-iotests/testrunner.py |  1 -
 3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -XXX,XX +XXX,XX @@
 import os
 import sys
 import argparse
+import shutil
+from pathlib import Path
+
 from findtests import TestFinder
 from testenv import TestEnv
 from testrunner import TestRunner
@@ -XXX,XX +XXX,XX @@ def make_argparser() -> argparse.ArgumentParser:
                        'rerun failed ./check command, starting from the '
                        'middle of the process.')
     g_sel.add_argument('tests', metavar='TEST_FILES', nargs='*',
-                       help='tests to run')
+                       help='tests to run, or "--" followed by a command')
 
     return p
 
@@ -XXX,XX +XXX,XX @@ if __name__ == '__main__':
                   imgopts=args.imgopts, misalign=args.misalign,
                   debug=args.debug, valgrind=args.valgrind)
 
+    if len(sys.argv) > 1 and sys.argv[-len(args.tests)-1] == '--':
+        if not args.tests:
+            sys.exit("missing command after '--'")
+        cmd = args.tests
+        env.print_env()
+        exec_pathstr = shutil.which(cmd[0])
+        if exec_pathstr is None:
+            sys.exit('command not found: ' + cmd[0])
+        exec_path = Path(exec_pathstr).resolve()
+        cmd[0] = str(exec_path)
+        full_env = env.prepare_subprocess(cmd)
+        os.chdir(exec_path.parent)
+        os.execve(cmd[0], cmd, full_env)
+
     testfinder = TestFinder(test_dir=env.source_iotests)
 
     groups = args.groups.split(',') if args.groups else None
diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -XXX,XX +XXX,XX @@ def print_env(self) -> None:
 PLATFORM      -- {platform}
 TEST_DIR      -- {TEST_DIR}
 SOCK_DIR      -- {SOCK_DIR}
-SOCKET_SCM_HELPER -- {SOCKET_SCM_HELPER}"""
+SOCKET_SCM_HELPER -- {SOCKET_SCM_HELPER}
+"""
 
         args = collections.defaultdict(str, self.get_env())
 
diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/testrunner.py
+++ b/tests/qemu-iotests/testrunner.py
@@ -XXX,XX +XXX,XX @@ def run_tests(self, tests: List[str]) -> bool:
 
         if not self.makecheck:
             self.env.print_env()
-            print()
 
         test_field_width = max(len(os.path.basename(t)) for t in tests) + 2
 
-- 
2.31.1

From: Paolo Bonzini <pbonzini@redhat.com>

Due to a typo, in this case the SOCK_DIR was not being created.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Tested-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Message-Id: <20210323181928.311862-6-pbonzini@redhat.com>
Message-Id: <20210503110110.476887-6-pbonzini@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/testenv.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -XXX,XX +XXX,XX @@ def init_directories(self) -> None:
         try:
             self.sock_dir = os.environ['SOCK_DIR']
             self.tmp_sock_dir = False
-            Path(self.test_dir).mkdir(parents=True, exist_ok=True)
+            Path(self.sock_dir).mkdir(parents=True, exist_ok=True)
         except KeyError:
             self.sock_dir = tempfile.mkdtemp()
             self.tmp_sock_dir = True
-- 
2.31.1

From: Connor Kuehl <ckuehl@redhat.com>

The contents of this patch were initially developed and posted by Han
Han[1], however, it appears the original patch was not applied. Since
then, the relevant documentation has been moved and adapted to a new
format.

I've taken most of the original wording and tweaked it according to
some of the feedback from the original patch submission. I've also
adapted it to restructured text, which is the format the documentation
currently uses.

[1] https://lists.nongnu.org/archive/html/qemu-block/2019-10/msg01253.html

Fixes: https://bugzilla.redhat.com/1763105
Signed-off-by: Han Han <hhan@redhat.com>
Suggested-by: Max Reitz <mreitz@redhat.com>
[ Max: provided description of data_file_raw behavior ]
Signed-off-by: Connor Kuehl <ckuehl@redhat.com>
Message-Id: <20210505195512.391128-1-ckuehl@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 docs/tools/qemu-img.rst | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
index XXXXXXX..XXXXXXX 100644
--- a/docs/tools/qemu-img.rst
+++ b/docs/tools/qemu-img.rst
@@ -XXX,XX +XXX,XX @@ Supported image file formats:
     issue ``lsattr filename`` to check if the NOCOW flag is set or not
     (Capital 'C' is NOCOW flag).
 
+  ``data_file``
+    Filename where all guest data will be stored. If this option is used,
+    the qcow2 file will only contain the image's metadata.
+
+    Note: Data loss will occur if the given filename already exists when
+    using this option with ``qemu-img create`` since ``qemu-img`` will create
+    the data file anew, overwriting the file's original contents. To simply
+    update the reference to point to the given pre-existing file, use
+    ``qemu-img amend``.
+
+  ``data_file_raw``
+    If this option is set to ``on``, QEMU will always keep the external data
+    file consistent as a standalone read-only raw image.
+
+    It does this by forwarding all write accesses to the qcow2 file through to
+    the raw data file, including their offsets. Therefore, data that is visible
+    on the qcow2 node (i.e., to the guest) at some offset is visible at the same
+    offset in the raw data file. This results in a read-only raw image. Writes
+    that bypass the qcow2 metadata may corrupt the qcow2 metadata because the
+    out-of-band writes may result in the metadata falling out of sync with the
+    raw image.
+
+    If this option is ``off``, QEMU will use the data file to store data in an
+    arbitrary manner. The file’s content will not make sense without the
+    accompanying qcow2 metadata. Where data is written will have no relation to
+    its offset as seen by the guest, and some writes (specifically zero writes)
+    may not be forwarded to the data file at all, but will only be handled by
+    modifying qcow2 metadata.
+
+    This option can only be enabled if ``data_file`` is set.
+
 ``Other``
 
   QEMU also supports various other image file formats for
-- 
2.31.1

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

Now, after huge update of block graph permission update algorithm, we
don't need this workaround with active state of the filter. Drop it and
use new smart bdrv_drop_filter() function.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210506194143.394141-1-vsementsov@virtuozzo.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/copy-on-read.c | 33 +--------------------------------
 1 file changed, 1 insertion(+), 32 deletions(-)

diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index XXXXXXX..XXXXXXX 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -XXX,XX +XXX,XX @@
 
 
 typedef struct BDRVStateCOR {
-    bool active;
     BlockDriverState *bottom_bs;
     bool chain_frozen;
 } BDRVStateCOR;
@@ -XXX,XX +XXX,XX @@ static int cor_open(BlockDriverState *bs, QDict *options, int flags,
          */
         bdrv_ref(bottom_bs);
     }
-    state->active = true;
     state->bottom_bs = bottom_bs;
 
     /*
@@ -XXX,XX +XXX,XX @@ static void cor_child_perm(BlockDriverState *bs, BdrvChild *c,
                            uint64_t perm, uint64_t shared,
                            uint64_t *nperm, uint64_t *nshared)
 {
-    BDRVStateCOR *s = bs->opaque;
-
-    if (!s->active) {
-        /*
-         * While the filter is being removed
-         */
-        *nperm = 0;
-        *nshared = BLK_PERM_ALL;
-        return;
-    }
-
     *nperm = perm & PERM_PASSTHROUGH;
     *nshared = (shared & PERM_PASSTHROUGH) | PERM_UNCHANGED;
 
@@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_copy_on_read = {
 
 void bdrv_cor_filter_drop(BlockDriverState *cor_filter_bs)
 {
-    BdrvChild *child;
-    BlockDriverState *bs;
     BDRVStateCOR *s = cor_filter_bs->opaque;
 
-    child = bdrv_filter_child(cor_filter_bs);
-    if (!child) {
-        return;
-    }
-    bs = child->bs;
-
-    /* Retain the BDS until we complete the graph change. */
-    bdrv_ref(bs);
-    /* Hold a guest back from writing while permissions are being reset. */
-    bdrv_drained_begin(bs);
-    /* Drop permissions before the graph change. */
-    s->active = false;
     /* unfreeze, as otherwise bdrv_replace_node() will fail */
     if (s->chain_frozen) {
         s->chain_frozen = false;
         bdrv_unfreeze_backing_chain(cor_filter_bs, s->bottom_bs);
     }
-    bdrv_child_refresh_perms(cor_filter_bs, child, &error_abort);
-    bdrv_replace_node(cor_filter_bs, bs, &error_abort);
-
-    bdrv_drained_end(bs);
-    bdrv_unref(bs);
+    bdrv_drop_filter(cor_filter_bs, &error_abort);
     bdrv_unref(cor_filter_bs);
 }
 
-- 
2.31.1

From: Emanuele Giuseppe Esposito <eesposit@redhat.com>

pylint 2.8 introduces consider-using-with error, suggesting
to use the 'with' block statement when possible.

Modify all subprocess.Popen call to use the 'with' statement,
except the one in __init__ of QemuIoInteractive class, since
it is assigned to a class field and used in other methods.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Message-Id: <20210510190449.65948-1-eesposit@redhat.com>
[mreitz: Disable bad-option-value warning in the iotests' pylintrc, so
         that disabling consider-using-with in QemuIoInteractive will
         not produce a warning in pre-2.8 pylint versions]
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/iotests.py    | 65 ++++++++++++++++----------------
 tests/qemu-iotests/pylintrc      |  3 ++
 tests/qemu-iotests/testrunner.py | 22 +++++------
 3 files changed, 47 insertions(+), 43 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -XXX,XX +XXX,XX @@ def qemu_tool_pipe_and_status(tool: str, args: Sequence[str],
     Run a tool and return both its output and its exit code
     """
     stderr = subprocess.STDOUT if connect_stderr else None
-    subp = subprocess.Popen(args,
-                            stdout=subprocess.PIPE,
-                            stderr=stderr,
-                            universal_newlines=True)
-    output = subp.communicate()[0]
-    if subp.returncode < 0:
-        cmd = ' '.join(args)
-        sys.stderr.write(f'{tool} received signal {-subp.returncode}: {cmd}\n')
-    return (output, subp.returncode)
+    with subprocess.Popen(args, stdout=subprocess.PIPE,
+                          stderr=stderr, universal_newlines=True) as subp:
+        output = subp.communicate()[0]
+        if subp.returncode < 0:
+            cmd = ' '.join(args)
+            sys.stderr.write(f'{tool} received signal \
+                               {-subp.returncode}: {cmd}\n')
+        return (output, subp.returncode)
 
 def qemu_img_pipe_and_status(*args: str) -> Tuple[str, int]:
     """
@@ -XXX,XX +XXX,XX @@ def qemu_io_silent_check(*args):
 class QemuIoInteractive:
     def __init__(self, *args):
         self.args = qemu_io_args_no_fmt + list(args)
+        # We need to keep the Popen objext around, and not
+        # close it immediately. Therefore, disable the pylint check:
+        # pylint: disable=consider-using-with
         self._p = subprocess.Popen(self.args, stdin=subprocess.PIPE,
                                    stdout=subprocess.PIPE,
                                    stderr=subprocess.STDOUT,
@@ -XXX,XX +XXX,XX @@ def qemu_nbd_popen(*args):
     cmd.extend(args)
 
     log('Start NBD server')
-    p = subprocess.Popen(cmd)
-    try:
-        while not os.path.exists(pid_file):
-            if p.poll() is not None:
-                raise RuntimeError(
-                    "qemu-nbd terminated with exit code {}: {}"
-                    .format(p.returncode, ' '.join(cmd)))
-
-            time.sleep(0.01)
-        yield
-    finally:
-        if os.path.exists(pid_file):
-            os.remove(pid_file)
-        log('Kill NBD server')
-        p.kill()
-        p.wait()
+    with subprocess.Popen(cmd) as p:
+        try:
+            while not os.path.exists(pid_file):
+                if p.poll() is not None:
+                    raise RuntimeError(
+                        "qemu-nbd terminated with exit code {}: {}"
+                        .format(p.returncode, ' '.join(cmd)))
+
+                time.sleep(0.01)
+            yield
+        finally:
+            if os.path.exists(pid_file):
+                os.remove(pid_file)
+            log('Kill NBD server')
+            p.kill()
+            p.wait()
 
 def compare_images(img1, img2, fmt1=imgfmt, fmt2=imgfmt):
     '''Return True if two image files are identical'''
@@ -XXX,XX +XXX,XX @@ def compare_images(img1, img2, fmt1=imgfmt, fmt2=imgfmt):
 
 def create_image(name, size):
     '''Create a fully-allocated raw image with sector markers'''
-    file = open(name, 'wb')
-    i = 0
-    while i < size:
-        sector = struct.pack('>l504xl', i // 512, i // 512)
-        file.write(sector)
-        i = i + 512
-    file.close()
+    with open(name, 'wb') as file:
+        i = 0
+        while i < size:
+            sector = struct.pack('>l504xl', i // 512, i // 512)
+            file.write(sector)
+            i = i + 512
 
 def image_size(img):
     '''Return image's virtual size'''
diff --git a/tests/qemu-iotests/pylintrc b/tests/qemu-iotests/pylintrc
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/pylintrc
+++ b/tests/qemu-iotests/pylintrc
@@ -XXX,XX +XXX,XX @@ disable=invalid-name,
         too-many-public-methods,
         # pylint warns about Optional[] etc. as unsubscriptable in 3.9
         unsubscriptable-object,
+        # Sometimes we need to disable a newly introduced pylint warning.
+        # Doing so should not produce a warning in older versions of pylint.
+        bad-option-value,
         # These are temporary, and should be removed:
         missing-docstring,
         too-many-return-statements,
diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/testrunner.py
+++ b/tests/qemu-iotests/testrunner.py
@@ -XXX,XX +XXX,XX @@ def do_run_test(self, test: str) -> TestResult:
 
         t0 = time.time()
         with f_bad.open('w', encoding="utf-8") as f:
-            proc = subprocess.Popen(args, cwd=str(f_test.parent), env=env,
-                                    stdout=f, stderr=subprocess.STDOUT)
-            try:
-                proc.wait()
-            except KeyboardInterrupt:
-                proc.terminate()
-                proc.wait()
-                return TestResult(status='not run',
-                                  description='Interrupted by user',
-                                  interrupted=True)
-            ret = proc.returncode
+            with subprocess.Popen(args, cwd=str(f_test.parent), env=env,
+                                  stdout=f, stderr=subprocess.STDOUT) as proc:
+                try:
+                    proc.wait()
+                except KeyboardInterrupt:
+                    proc.terminate()
+                    proc.wait()
+                    return TestResult(status='not run',
+                                      description='Interrupted by user',
+                                      interrupted=True)
+                ret = proc.returncode
 
         elapsed = round(time.time() - t0, 1)
 
-- 
2.31.1

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

write-notifiers are used only for write-threshold. New code for such
purpose should create filters.

Let's better special-case write-threshold and drop write notifiers at
all. (Actually, write-threshold is special-cased anyway, as the only
user of write-notifiers)

So, create a new direct interface for bdrv_co_write_req_prepare() and
drop all write-notifier related logic from write-threshold.c.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20210506090621.11848-2-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[mreitz: Adjusted comment as per Eric's suggestion]
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 include/block/block_int.h       |  1 -
 include/block/write-threshold.h |  9 +++++
 block/io.c                      |  5 ++-
 block/write-threshold.c         | 70 +++++++--------------------------
 4 files changed, 27 insertions(+), 58 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -XXX,XX +XXX,XX @@ struct BlockDriverState {
 
     /* threshold limit for writes, in bytes. "High water mark". */
     uint64_t write_threshold_offset;
-    NotifierWithReturn write_threshold_notifier;
 
     /* Writing to the list requires the BQL _and_ the dirty_bitmap_mutex.
      * Reading from the list can be done with either the BQL or the
diff --git a/include/block/write-threshold.h b/include/block/write-threshold.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/write-threshold.h
+++ b/include/block/write-threshold.h
@@ -XXX,XX +XXX,XX @@ bool bdrv_write_threshold_is_set(const BlockDriverState *bs);
 uint64_t bdrv_write_threshold_exceeded(const BlockDriverState *bs,
                                        const BdrvTrackedRequest *req);
 
+/*
+ * bdrv_write_threshold_check_write
+ *
+ * Check whether the specified request exceeds the write threshold.
+ * If so, send a corresponding event and disable write threshold checking.
+ */
+void bdrv_write_threshold_check_write(BlockDriverState *bs, int64_t offset,
+                                      int64_t bytes);
+
 #endif
diff --git a/block/io.c b/block/io.c
index XXXXXXX..XXXXXXX 100644
--- a/block/io.c
+++ b/block/io.c
@@ -XXX,XX +XXX,XX @@
 #include "block/blockjob_int.h"
 #include "block/block_int.h"
 #include "block/coroutines.h"
+#include "block/write-threshold.h"
 #include "qemu/cutils.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
@@ -XXX,XX +XXX,XX @@ bdrv_co_write_req_prepare(BdrvChild *child, int64_t offset, int64_t bytes,
         } else {
             assert(child->perm & BLK_PERM_WRITE);
         }
-        return notifier_with_return_list_notify(&bs->before_write_notifiers,
-                                                req);
+        bdrv_write_threshold_check_write(bs, offset, bytes);
+        return 0;
     case BDRV_TRACKED_TRUNCATE:
         assert(child->perm & BLK_PERM_RESIZE);
         return 0;
diff --git a/block/write-threshold.c b/block/write-threshold.c
index XXXXXXX..XXXXXXX 100644
--- a/block/write-threshold.c
+++ b/block/write-threshold.c
@@ -XXX,XX +XXX,XX @@ bool bdrv_write_threshold_is_set(const BlockDriverState *bs)
     return bs->write_threshold_offset > 0;
 }
 
-static void write_threshold_disable(BlockDriverState *bs)
-{
-    if (bdrv_write_threshold_is_set(bs)) {
-        notifier_with_return_remove(&bs->write_threshold_notifier);
-        bs->write_threshold_offset = 0;
-    }
-}
-
 uint64_t bdrv_write_threshold_exceeded(const BlockDriverState *bs,
                                        const BdrvTrackedRequest *req)
 {
@@ -XXX,XX +XXX,XX @@ uint64_t bdrv_write_threshold_exceeded(const BlockDriverState *bs,
     return 0;
 }
 
-static int coroutine_fn before_write_notify(NotifierWithReturn *notifier,
-                                            void *opaque)
-{
-    BdrvTrackedRequest *req = opaque;
-    BlockDriverState *bs = req->bs;
-    uint64_t amount = 0;
-
-    amount = bdrv_write_threshold_exceeded(bs, req);
-    if (amount > 0) {
-        qapi_event_send_block_write_threshold(
-            bs->node_name,
-            amount,
-            bs->write_threshold_offset);
-
-        /* autodisable to avoid flooding the monitor */
-        write_threshold_disable(bs);
-    }
-
-    return 0; /* should always let other notifiers run */
-}
-
-static void write_threshold_register_notifier(BlockDriverState *bs)
-{
-    bs->write_threshold_notifier.notify = before_write_notify;
-    bdrv_add_before_write_notifier(bs, &bs->write_threshold_notifier);
-}
-
-static void write_threshold_update(BlockDriverState *bs,
-                                   int64_t threshold_bytes)
-{
-    bs->write_threshold_offset = threshold_bytes;
-}
-
 void bdrv_write_threshold_set(BlockDriverState *bs, uint64_t threshold_bytes)
 {
-    if (bdrv_write_threshold_is_set(bs)) {
-        if (threshold_bytes > 0) {
-            write_threshold_update(bs, threshold_bytes);
-        } else {
-            write_threshold_disable(bs);
-        }
-    } else {
-        if (threshold_bytes > 0) {
-            /* avoid multiple registration */
-            write_threshold_register_notifier(bs);
-            write_threshold_update(bs, threshold_bytes);
-        }
-        /* discard bogus disable request */
-    }
+    bs->write_threshold_offset = threshold_bytes;
 }
 
 void qmp_block_set_write_threshold(const char *node_name,
@@ -XXX,XX +XXX,XX @@ void qmp_block_set_write_threshold(const char *node_name,
 
     aio_context_release(aio_context);
 }
+
+void bdrv_write_threshold_check_write(BlockDriverState *bs, int64_t offset,
+                                      int64_t bytes)
+{
+    int64_t end = offset + bytes;
+    uint64_t wtr = bs->write_threshold_offset;
+
+    if (wtr > 0 && end > wtr) {
+        qapi_event_send_block_write_threshold(bs->node_name, end - wtr, wtr);
+
+        /* autodisable to avoid flooding the monitor */
+        bdrv_write_threshold_set(bs, 0);
+    }
+}
-- 
2.31.1

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

They are unused now.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20210506090621.11848-3-vsementsov@virtuozzo.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 include/block/block_int.h | 12 ------------
 block.c                   |  1 -
 block/io.c                |  6 ------
 3 files changed, 19 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -XXX,XX +XXX,XX @@ struct BlockDriverState {
      */
     int64_t total_sectors;
 
-    /* Callback before write request is processed */
-    NotifierWithReturnList before_write_notifiers;
-
     /* threshold limit for writes, in bytes. "High water mark". */
     uint64_t write_threshold_offset;
 
@@ -XXX,XX +XXX,XX @@ void bdrv_parse_filename_strip_prefix(const char *filename, const char *prefix,
 bool bdrv_backing_overridden(BlockDriverState *bs);
 
 
-/**
- * bdrv_add_before_write_notifier:
- *
- * Register a callback that is invoked before write requests are processed but
- * after any throttling or waiting for overlapping requests.
- */
-void bdrv_add_before_write_notifier(BlockDriverState *bs,
-                                    NotifierWithReturn *notifier);
-
 /**
  * bdrv_add_aio_context_notifier:
  *
diff --git a/block.c b/block.c
index XXXXXXX..XXXXXXX 100644
--- a/block.c
+++ b/block.c
@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_new(void)
     for (i = 0; i < BLOCK_OP_TYPE_MAX; i++) {
         QLIST_INIT(&bs->op_blockers[i]);
     }
-    notifier_with_return_list_init(&bs->before_write_notifiers);
     qemu_co_mutex_init(&bs->reqs_lock);
     qemu_mutex_init(&bs->dirty_bitmap_mutex);
     bs->refcnt = 1;
diff --git a/block/io.c b/block/io.c
index XXXXXXX..XXXXXXX 100644
--- a/block/io.c
+++ b/block/io.c
@@ -XXX,XX +XXX,XX @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov)
     return true;
 }
 
-void bdrv_add_before_write_notifier(BlockDriverState *bs,
-                                    NotifierWithReturn *notifier)
-{
-    notifier_with_return_list_add(&bs->before_write_notifiers, notifier);
-}
-
 void bdrv_io_plug(BlockDriverState *bs)
 {
     BdrvChild *child;
-- 
2.31.1

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

These tests use bdrv_write_threshold_exceeded() API, which is used only
for test (since pre-previous commit). Better is testing real API, which
is used in block.c as well.

So, let's call bdrv_write_threshold_check_write(), and check is
bs->write_threshold_offset cleared or not (it's cleared iff threshold
triggered).

Also we get rid of BdrvTrackedRequest use here. Note, that paranoiac
bdrv_check_request() calls were added in 8b1170012b1 to protect
BdrvTrackedRequest. Drop them now.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20210506090621.11848-4-vsementsov@virtuozzo.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/unit/test-write-threshold.c | 22 ++++------------------
 1 file changed, 4 insertions(+), 18 deletions(-)

diff --git a/tests/unit/test-write-threshold.c b/tests/unit/test-write-threshold.c
index XXXXXXX..XXXXXXX 100644
--- a/tests/unit/test-write-threshold.c
+++ b/tests/unit/test-write-threshold.c
@@ -XXX,XX +XXX,XX @@ static void test_threshold_multi_set_get(void)
 
 static void test_threshold_not_trigger(void)
 {
-    uint64_t amount = 0;
     uint64_t threshold = 4 * 1024 * 1024;
     BlockDriverState bs;
-    BdrvTrackedRequest req;
 
     memset(&bs, 0, sizeof(bs));
-    memset(&req, 0, sizeof(req));
-    req.offset = 1024;
-    req.bytes = 1024;
-
-    bdrv_check_request(req.offset, req.bytes, &error_abort);
 
     bdrv_write_threshold_set(&bs, threshold);
-    amount = bdrv_write_threshold_exceeded(&bs, &req);
-    g_assert_cmpuint(amount, ==, 0);
+    bdrv_write_threshold_check_write(&bs, 1024, 1024);
+    g_assert_cmpuint(bdrv_write_threshold_get(&bs), ==, threshold);
 }
 
 
 static void test_threshold_trigger(void)
 {
-    uint64_t amount = 0;
     uint64_t threshold = 4 * 1024 * 1024;
     BlockDriverState bs;
-    BdrvTrackedRequest req;
 
     memset(&bs, 0, sizeof(bs));
-    memset(&req, 0, sizeof(req));
-    req.offset = (4 * 1024 * 1024) - 1024;
-    req.bytes = 2 * 1024;
-
-    bdrv_check_request(req.offset, req.bytes, &error_abort);
 
     bdrv_write_threshold_set(&bs, threshold);
-    amount = bdrv_write_threshold_exceeded(&bs, &req);
-    g_assert_cmpuint(amount, >=, 1024);
+    bdrv_write_threshold_check_write(&bs, threshold - 1024, 2 * 1024);
+    g_assert_cmpuint(bdrv_write_threshold_get(&bs), ==, 0);
 }
 
 typedef struct TestStruct {
-- 
2.31.1

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

bdrv_write_threshold_exceeded() is unused.

bdrv_write_threshold_is_set() is used only to double check the value of
bs->write_threshold_offset in tests. No real sense in it (both tests do
check real value with help of bdrv_write_threshold_get())

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20210506090621.11848-5-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[mreitz: Adjusted commit message as per Eric's suggestion]
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 include/block/write-threshold.h   | 24 ------------------------
 block/write-threshold.c           | 19 -------------------
 tests/unit/test-write-threshold.c |  4 ----
 3 files changed, 47 deletions(-)

diff --git a/include/block/write-threshold.h b/include/block/write-threshold.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/write-threshold.h
+++ b/include/block/write-threshold.h
@@ -XXX,XX +XXX,XX @@ void bdrv_write_threshold_set(BlockDriverState *bs, uint64_t threshold_bytes);
  */
 uint64_t bdrv_write_threshold_get(const BlockDriverState *bs);
 
-/*
- * bdrv_write_threshold_is_set
- *
- * Tell if a write threshold is set for a given BDS.
- */
-bool bdrv_write_threshold_is_set(const BlockDriverState *bs);
-
-/*
- * bdrv_write_threshold_exceeded
- *
- * Return the extent of a write request that exceeded the threshold,
- * or zero if the request is below the threshold.
- * Return zero also if the threshold was not set.
- *
- * NOTE: here we assume the following holds for each request this code
- * deals with:
- *
- * assert((req->offset + req->bytes) <= UINT64_MAX)
- *
- * Please not there is *not* an actual C assert().
- */
-uint64_t bdrv_write_threshold_exceeded(const BlockDriverState *bs,
-                                       const BdrvTrackedRequest *req);
-
 /*
  * bdrv_write_threshold_check_write
  *
diff --git a/block/write-threshold.c b/block/write-threshold.c
index XXXXXXX..XXXXXXX 100644
--- a/block/write-threshold.c
+++ b/block/write-threshold.c
@@ -XXX,XX +XXX,XX @@ uint64_t bdrv_write_threshold_get(const BlockDriverState *bs)
     return bs->write_threshold_offset;
 }
 
-bool bdrv_write_threshold_is_set(const BlockDriverState *bs)
-{
-    return bs->write_threshold_offset > 0;
-}
-
-uint64_t bdrv_write_threshold_exceeded(const BlockDriverState *bs,
-                                       const BdrvTrackedRequest *req)
-{
-    if (bdrv_write_threshold_is_set(bs)) {
-        if (req->offset > bs->write_threshold_offset) {
-            return (req->offset - bs->write_threshold_offset) + req->bytes;
-        }
-        if ((req->offset + req->bytes) > bs->write_threshold_offset) {
-            return (req->offset + req->bytes) - bs->write_threshold_offset;
-        }
-    }
-    return 0;
-}
-
 void bdrv_write_threshold_set(BlockDriverState *bs, uint64_t threshold_bytes)
 {
     bs->write_threshold_offset = threshold_bytes;
diff --git a/tests/unit/test-write-threshold.c b/tests/unit/test-write-threshold.c
index XXXXXXX..XXXXXXX 100644
--- a/tests/unit/test-write-threshold.c
+++ b/tests/unit/test-write-threshold.c
@@ -XXX,XX +XXX,XX @@ static void test_threshold_not_set_on_init(void)
     BlockDriverState bs;
     memset(&bs, 0, sizeof(bs));
 
-    g_assert(!bdrv_write_threshold_is_set(&bs));
-
     res = bdrv_write_threshold_get(&bs);
     g_assert_cmpint(res, ==, 0);
 }
@@ -XXX,XX +XXX,XX @@ static void test_threshold_set_get(void)
 
     bdrv_write_threshold_set(&bs, threshold);
 
-    g_assert(bdrv_write_threshold_is_set(&bs));
-
     res = bdrv_write_threshold_get(&bs);
     g_assert_cmpint(res, ==, threshold);
 }
-- 
2.31.1

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

Testing set/get of one 64bit variable doesn't seem necessary. We have a
lot of such variables. Also remaining tests do test set/get anyway.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20210506090621.11848-7-vsementsov@virtuozzo.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/unit/test-write-threshold.c | 43 -------------------------------
 1 file changed, 43 deletions(-)

diff --git a/tests/unit/test-write-threshold.c b/tests/unit/test-write-threshold.c
index XXXXXXX..XXXXXXX 100644
--- a/tests/unit/test-write-threshold.c
+++ b/tests/unit/test-write-threshold.c
@@ -XXX,XX +XXX,XX @@
 #include "block/write-threshold.h"
 
 
-static void test_threshold_not_set_on_init(void)
-{
-    uint64_t res;
-    BlockDriverState bs;
-    memset(&bs, 0, sizeof(bs));
-
-    res = bdrv_write_threshold_get(&bs);
-    g_assert_cmpint(res, ==, 0);
-}
-
-static void test_threshold_set_get(void)
-{
-    uint64_t threshold = 4 * 1024 * 1024;
-    uint64_t res;
-    BlockDriverState bs;
-    memset(&bs, 0, sizeof(bs));
-
-    bdrv_write_threshold_set(&bs, threshold);
-
-    res = bdrv_write_threshold_get(&bs);
-    g_assert_cmpint(res, ==, threshold);
-}
-
-static void test_threshold_multi_set_get(void)
-{
-    uint64_t threshold1 = 4 * 1024 * 1024;
-    uint64_t threshold2 = 15 * 1024 * 1024;
-    uint64_t res;
-    BlockDriverState bs;
-    memset(&bs, 0, sizeof(bs));
-
-    bdrv_write_threshold_set(&bs, threshold1);
-    bdrv_write_threshold_set(&bs, threshold2);
-    res = bdrv_write_threshold_get(&bs);
-    g_assert_cmpint(res, ==, threshold2);
-}
-
 static void test_threshold_not_trigger(void)
 {
     uint64_t threshold = 4 * 1024 * 1024;
@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv)
 {
     size_t i;
     TestStruct tests[] = {
-        { "/write-threshold/not-set-on-init",
-          test_threshold_not_set_on_init },
-        { "/write-threshold/set-get",
-          test_threshold_set_get },
-        { "/write-threshold/multi-set-get",
-          test_threshold_multi_set_get },
         { "/write-threshold/not-trigger",
           test_threshold_not_trigger },
         { "/write-threshold/trigger",
-- 
2.31.1

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

We don't need this extra logic: it doesn't make code simpler.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20210506090621.11848-8-vsementsov@virtuozzo.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/unit/test-write-threshold.c | 20 +++-----------------
 1 file changed, 3 insertions(+), 17 deletions(-)

diff --git a/tests/unit/test-write-threshold.c b/tests/unit/test-write-threshold.c
index XXXXXXX..XXXXXXX 100644
--- a/tests/unit/test-write-threshold.c
+++ b/tests/unit/test-write-threshold.c
@@ -XXX,XX +XXX,XX @@ static void test_threshold_trigger(void)
     g_assert_cmpuint(bdrv_write_threshold_get(&bs), ==, 0);
 }
 
-typedef struct TestStruct {
-    const char *name;
-    void (*func)(void);
-} TestStruct;
-
 
 int main(int argc, char **argv)
 {
-    size_t i;
-    TestStruct tests[] = {
-        { "/write-threshold/not-trigger",
-          test_threshold_not_trigger },
-        { "/write-threshold/trigger",
-          test_threshold_trigger },
-        { NULL, NULL }
-    };
-
     g_test_init(&argc, &argv, NULL);
-    for (i = 0; tests[i].name != NULL; i++) {
-        g_test_add_func(tests[i].name, tests[i].func);
-    }
+    g_test_add_func("/write-threshold/not-trigger", test_threshold_not_trigger);
+    g_test_add_func("/write-threshold/trigger", test_threshold_trigger);
+
     return g_test_run();
 }
-- 
2.31.1

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

"qemu/typedefs.h" is enough for include/block/write-threshold.h header
with forward declaration of BlockDriverState. Also drop extra includes
from block/write-threshold.c and tests/unit/test-write-threshold.c

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210506090621.11848-9-vsementsov@virtuozzo.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 include/block/write-threshold.h   | 2 +-
 block/write-threshold.c           | 2 --
 tests/unit/test-write-threshold.c | 1 -
 3 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/include/block/write-threshold.h b/include/block/write-threshold.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/write-threshold.h
+++ b/include/block/write-threshold.h
@@ -XXX,XX +XXX,XX @@
 #ifndef BLOCK_WRITE_THRESHOLD_H
 #define BLOCK_WRITE_THRESHOLD_H
 
-#include "block/block_int.h"
+#include "qemu/typedefs.h"
 
 /*
  * bdrv_write_threshold_set:
diff --git a/block/write-threshold.c b/block/write-threshold.c
index XXXXXXX..XXXXXXX 100644
--- a/block/write-threshold.c
+++ b/block/write-threshold.c
@@ -XXX,XX +XXX,XX @@
 
 #include "qemu/osdep.h"
 #include "block/block_int.h"
-#include "qemu/coroutine.h"
 #include "block/write-threshold.h"
-#include "qemu/notify.h"
 #include "qapi/error.h"
 #include "qapi/qapi-commands-block-core.h"
 #include "qapi/qapi-events-block-core.h"
diff --git a/tests/unit/test-write-threshold.c b/tests/unit/test-write-threshold.c
index XXXXXXX..XXXXXXX 100644
--- a/tests/unit/test-write-threshold.c
+++ b/tests/unit/test-write-threshold.c
@@ -XXX,XX +XXX,XX @@
  */
 
 #include "qemu/osdep.h"
-#include "qapi/error.h"
 #include "block/block_int.h"
 #include "block/write-threshold.h"
 
-- 
2.31.1

The following changes since commit c6a5fc2ac76c5ab709896ee1b0edd33685a67ed1:

decodetree: Add --output-null for meson testing (2023-05-31 19:56:42 -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 98b126f5e3228a346c774e569e26689943b401dd:

qapi: add '@fdset' feature for BlockdevOptionsVirtioBlkVhostVdpa (2023-06-01 11:08:21 -0400)

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

- Stefano Garzarella's blkio block driver 'fd' parameter
- My thread-local blk_io_plug() series

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

Stefan Hajnoczi (6):
  block: add blk_io_plug_call() API
  block/nvme: convert to blk_io_plug_call() API
  block/blkio: convert to blk_io_plug_call() API
  block/io_uring: convert to blk_io_plug_call() API
  block/linux-aio: convert to blk_io_plug_call() API
  block: remove bdrv_co_io_plug() API

Stefano Garzarella (2):
  block/blkio: use qemu_open() to support fd passing for virtio-blk
  qapi: add '@fdset' feature for BlockdevOptionsVirtioBlkVhostVdpa

-- 
2.40.1

Introduce a new API for thread-local blk_io_plug() that does not
traverse the block graph. The goal is to make blk_io_plug() multi-queue
friendly.

Instead of having block drivers track whether or not we're in a plugged
section, provide an API that allows them to defer a function call until
we're unplugged: blk_io_plug_call(fn, opaque). If blk_io_plug_call() is
called multiple times with the same fn/opaque pair, then fn() is only
called once at the end of the function - resulting in batching.

This patch introduces the API and changes blk_io_plug()/blk_io_unplug().
blk_io_plug()/blk_io_unplug() no longer require a BlockBackend argument
because the plug state is now thread-local.

Later patches convert block drivers to blk_io_plug_call() and then we
can finally remove .bdrv_co_io_plug() once all block drivers have been
converted.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Acked-by: Kevin Wolf <kwolf@redhat.com>
Message-id: 20230530180959.1108766-2-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 MAINTAINERS                       |   1 +
 include/sysemu/block-backend-io.h |  13 +--
 block/block-backend.c             |  22 -----
 block/plug.c                      | 159 ++++++++++++++++++++++++++++++
 hw/block/dataplane/xen-block.c    |   8 +-
 hw/block/virtio-blk.c             |   4 +-
 hw/scsi/virtio-scsi.c             |   6 +-
 block/meson.build                 |   1 +
 8 files changed, 173 insertions(+), 41 deletions(-)
 create mode 100644 block/plug.c

diff --git a/MAINTAINERS b/MAINTAINERS
index XXXXXXX..XXXXXXX 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -XXX,XX +XXX,XX @@ F: util/aio-*.c
 F: util/aio-*.h
 F: util/fdmon-*.c
 F: block/io.c
+F: block/plug.c
 F: migration/block*
 F: include/block/aio.h
 F: include/block/aio-wait.h
diff --git a/include/sysemu/block-backend-io.h b/include/sysemu/block-backend-io.h
index XXXXXXX..XXXXXXX 100644
--- a/include/sysemu/block-backend-io.h
+++ b/include/sysemu/block-backend-io.h
@@ -XXX,XX +XXX,XX @@ void blk_iostatus_set_err(BlockBackend *blk, int error);
 int blk_get_max_iov(BlockBackend *blk);
 int blk_get_max_hw_iov(BlockBackend *blk);
 
-/*
- * blk_io_plug/unplug are thread-local operations. This means that multiple
- * IOThreads can simultaneously call plug/unplug, but the caller must ensure
- * that each unplug() is called in the same IOThread of the matching plug().
- */
-void coroutine_fn blk_co_io_plug(BlockBackend *blk);
-void co_wrapper blk_io_plug(BlockBackend *blk);
-
-void coroutine_fn blk_co_io_unplug(BlockBackend *blk);
-void co_wrapper blk_io_unplug(BlockBackend *blk);
+void blk_io_plug(void);
+void blk_io_unplug(void);
+void blk_io_plug_call(void (*fn)(void *), void *opaque);
 
 AioContext *blk_get_aio_context(BlockBackend *blk);
 BlockAcctStats *blk_get_stats(BlockBackend *blk);
diff --git a/block/block-backend.c b/block/block-backend.c
index XXXXXXX..XXXXXXX 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -XXX,XX +XXX,XX @@ void blk_add_insert_bs_notifier(BlockBackend *blk, Notifier *notify)
     notifier_list_add(&blk->insert_bs_notifiers, notify);
 }
 
-void coroutine_fn blk_co_io_plug(BlockBackend *blk)
-{
-    BlockDriverState *bs = blk_bs(blk);
-    IO_CODE();
-    GRAPH_RDLOCK_GUARD();
-
-    if (bs) {
-        bdrv_co_io_plug(bs);
-    }
-}
-
-void coroutine_fn blk_co_io_unplug(BlockBackend *blk)
-{
-    BlockDriverState *bs = blk_bs(blk);
-    IO_CODE();
-    GRAPH_RDLOCK_GUARD();
-
-    if (bs) {
-        bdrv_co_io_unplug(bs);
-    }
-}
-
 BlockAcctStats *blk_get_stats(BlockBackend *blk)
 {
     IO_CODE();
diff --git a/block/plug.c b/block/plug.c
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/block/plug.c
@@ -XXX,XX +XXX,XX @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Block I/O plugging
+ *
+ * Copyright Red Hat.
+ *
+ * This API defers a function call within a blk_io_plug()/blk_io_unplug()
+ * section, allowing multiple calls to batch up. This is a performance
+ * optimization that is used in the block layer to submit several I/O requests
+ * at once instead of individually:
+ *
+ *   blk_io_plug(); <-- start of plugged region
+ *   ...
+ *   blk_io_plug_call(my_func, my_obj); <-- deferred my_func(my_obj) call
+ *   blk_io_plug_call(my_func, my_obj); <-- another
+ *   blk_io_plug_call(my_func, my_obj); <-- another
+ *   ...
+ *   blk_io_unplug(); <-- end of plugged region, my_func(my_obj) is called once
+ *
+ * This code is actually generic and not tied to the block layer. If another
+ * subsystem needs this functionality, it could be renamed.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/coroutine-tls.h"
+#include "qemu/notify.h"
+#include "qemu/thread.h"
+#include "sysemu/block-backend.h"
+
+/* A function call that has been deferred until unplug() */
+typedef struct {
+    void (*fn)(void *);
+    void *opaque;
+} UnplugFn;
+
+/* Per-thread state */
+typedef struct {
+    unsigned count;       /* how many times has plug() been called? */
+    GArray *unplug_fns;   /* functions to call at unplug time */
+} Plug;
+
+/* Use get_ptr_plug() to fetch this thread-local value */
+QEMU_DEFINE_STATIC_CO_TLS(Plug, plug);
+
+/* Called at thread cleanup time */
+static void blk_io_plug_atexit(Notifier *n, void *value)
+{
+    Plug *plug = get_ptr_plug();
+    g_array_free(plug->unplug_fns, TRUE);
+}
+
+/* This won't involve coroutines, so use __thread */
+static __thread Notifier blk_io_plug_atexit_notifier;
+
+/**
+ * blk_io_plug_call:
+ * @fn: a function pointer to be invoked
+ * @opaque: a user-defined argument to @fn()
+ *
+ * Call @fn(@opaque) immediately if not within a blk_io_plug()/blk_io_unplug()
+ * section.
+ *
+ * Otherwise defer the call until the end of the outermost
+ * blk_io_plug()/blk_io_unplug() section in this thread. If the same
+ * @fn/@opaque pair has already been deferred, it will only be called once upon
+ * blk_io_unplug() so that accumulated calls are batched into a single call.
+ *
+ * The caller must ensure that @opaque is not freed before @fn() is invoked.
+ */
+void blk_io_plug_call(void (*fn)(void *), void *opaque)
+{
+    Plug *plug = get_ptr_plug();
+
+    /* Call immediately if we're not plugged */
+    if (plug->count == 0) {
+        fn(opaque);
+        return;
+    }
+
+    GArray *array = plug->unplug_fns;
+    if (!array) {
+        array = g_array_new(FALSE, FALSE, sizeof(UnplugFn));
+        plug->unplug_fns = array;
+        blk_io_plug_atexit_notifier.notify = blk_io_plug_atexit;
+        qemu_thread_atexit_add(&blk_io_plug_atexit_notifier);
+    }
+
+    UnplugFn *fns = (UnplugFn *)array->data;
+    UnplugFn new_fn = {
+        .fn = fn,
+        .opaque = opaque,
+    };
+
+    /*
+     * There won't be many, so do a linear search. If this becomes a bottleneck
+     * then a binary search (glib 2.62+) or different data structure could be
+     * used.
+     */
+    for (guint i = 0; i < array->len; i++) {
+        if (memcmp(&fns[i], &new_fn, sizeof(new_fn)) == 0) {
+            return; /* already exists */
+        }
+    }
+
+    g_array_append_val(array, new_fn);
+}
+
+/**
+ * blk_io_plug: Defer blk_io_plug_call() functions until blk_io_unplug()
+ *
+ * blk_io_plug/unplug are thread-local operations. This means that multiple
+ * threads can simultaneously call plug/unplug, but the caller must ensure that
+ * each unplug() is called in the same thread of the matching plug().
+ *
+ * Nesting is supported. blk_io_plug_call() functions are only called at the
+ * outermost blk_io_unplug().
+ */
+void blk_io_plug(void)
+{
+    Plug *plug = get_ptr_plug();
+
+    assert(plug->count < UINT32_MAX);
+
+    plug->count++;
+}
+
+/**
+ * blk_io_unplug: Run any pending blk_io_plug_call() functions
+ *
+ * There must have been a matching blk_io_plug() call in the same thread prior
+ * to this blk_io_unplug() call.
+ */
+void blk_io_unplug(void)
+{
+    Plug *plug = get_ptr_plug();
+
+    assert(plug->count > 0);
+
+    if (--plug->count > 0) {
+        return;
+    }
+
+    GArray *array = plug->unplug_fns;
+    if (!array) {
+        return;
+    }
+
+    UnplugFn *fns = (UnplugFn *)array->data;
+
+    for (guint i = 0; i < array->len; i++) {
+        fns[i].fn(fns[i].opaque);
+    }
+
+    /*
+     * This resets the array without freeing memory so that appending is cheap
+     * in the future.
+     */
+    g_array_set_size(array, 0);
+}
diff --git a/hw/block/dataplane/xen-block.c b/hw/block/dataplane/xen-block.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/block/dataplane/xen-block.c
+++ b/hw/block/dataplane/xen-block.c
@@ -XXX,XX +XXX,XX @@ static bool xen_block_handle_requests(XenBlockDataPlane *dataplane)
      * is below us.
      */
     if (inflight_atstart > IO_PLUG_THRESHOLD) {
-        blk_io_plug(dataplane->blk);
+        blk_io_plug();
     }
     while (rc != rp) {
         /* pull request from ring */
@@ -XXX,XX +XXX,XX @@ static bool xen_block_handle_requests(XenBlockDataPlane *dataplane)
 
         if (inflight_atstart > IO_PLUG_THRESHOLD &&
             batched >= inflight_atstart) {
-            blk_io_unplug(dataplane->blk);
+            blk_io_unplug();
         }
         xen_block_do_aio(request);
         if (inflight_atstart > IO_PLUG_THRESHOLD) {
             if (batched >= inflight_atstart) {
-                blk_io_plug(dataplane->blk);
+                blk_io_plug();
                 batched = 0;
             } else {
                 batched++;
@@ -XXX,XX +XXX,XX @@ static bool xen_block_handle_requests(XenBlockDataPlane *dataplane)
         }
     }
     if (inflight_atstart > IO_PLUG_THRESHOLD) {
-        blk_io_unplug(dataplane->blk);
+        blk_io_unplug();
     }
 
     return done_something;
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -XXX,XX +XXX,XX @@ void virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq)
     bool suppress_notifications = virtio_queue_get_notification(vq);
 
     aio_context_acquire(blk_get_aio_context(s->blk));
-    blk_io_plug(s->blk);
+    blk_io_plug();
 
     do {
         if (suppress_notifications) {
@@ -XXX,XX +XXX,XX @@ void virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq)
         virtio_blk_submit_multireq(s, &mrb);
     }
 
-    blk_io_unplug(s->blk);
+    blk_io_unplug();
     aio_context_release(blk_get_aio_context(s->blk));
 }
 
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -XXX,XX +XXX,XX @@ static int virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req)
         return -ENOBUFS;
     }
     scsi_req_ref(req->sreq);
-    blk_io_plug(d->conf.blk);
+    blk_io_plug();
     object_unref(OBJECT(d));
     return 0;
 }
@@ -XXX,XX +XXX,XX @@ static void virtio_scsi_handle_cmd_req_submit(VirtIOSCSI *s, VirtIOSCSIReq *req)
     if (scsi_req_enqueue(sreq)) {
         scsi_req_continue(sreq);
     }
-    blk_io_unplug(sreq->dev->conf.blk);
+    blk_io_unplug();
     scsi_req_unref(sreq);
 }
 
@@ -XXX,XX +XXX,XX @@ static void virtio_scsi_handle_cmd_vq(VirtIOSCSI *s, VirtQueue *vq)
                 while (!QTAILQ_EMPTY(&reqs)) {
                     req = QTAILQ_FIRST(&reqs);
                     QTAILQ_REMOVE(&reqs, req, next);
-                    blk_io_unplug(req->sreq->dev->conf.blk);
+                    blk_io_unplug();
                     scsi_req_unref(req->sreq);
                     virtqueue_detach_element(req->vq, &req->elem, 0);
                     virtio_scsi_free_req(req);
diff --git a/block/meson.build b/block/meson.build
index XXXXXXX..XXXXXXX 100644
--- a/block/meson.build
+++ b/block/meson.build
@@ -XXX,XX +XXX,XX @@ block_ss.add(files(
   'mirror.c',
   'nbd.c',
   'null.c',
+  'plug.c',
   'qapi.c',
   'qcow2-bitmap.c',
   'qcow2-cache.c',
-- 
2.40.1

Stop using the .bdrv_co_io_plug() API because it is not multi-queue
block layer friendly. Use the new blk_io_plug_call() API to batch I/O
submission instead.

diff --git a/block/nvme.c b/block/nvme.c
index XXXXXXX..XXXXXXX 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -XXX,XX +XXX,XX @@
 #include "qemu/vfio-helpers.h"
 #include "block/block-io.h"
 #include "block/block_int.h"
+#include "sysemu/block-backend.h"
 #include "sysemu/replay.h"
 #include "trace.h"
 
@@ -XXX,XX +XXX,XX @@ struct BDRVNVMeState {
     int blkshift;
 
     uint64_t max_transfer;
-    bool plugged;
 
     bool supports_write_zeroes;
     bool supports_discard;
@@ -XXX,XX +XXX,XX @@ static void nvme_kick(NVMeQueuePair *q)
 {
     BDRVNVMeState *s = q->s;
 
-    if (s->plugged || !q->need_kick) {
+    if (!q->need_kick) {
         return;
     }
     trace_nvme_kick(s, q->index);
@@ -XXX,XX +XXX,XX @@ static bool nvme_process_completion(NVMeQueuePair *q)
     NvmeCqe *c;
 
     trace_nvme_process_completion(s, q->index, q->inflight);
-    if (s->plugged) {
-        trace_nvme_process_completion_queue_plugged(s, q->index);
-        return false;
-    }
 
     /*
      * Support re-entrancy when a request cb() function invokes aio_poll().
@@ -XXX,XX +XXX,XX @@ static void nvme_trace_command(const NvmeCmd *cmd)
     }
 }
 
+static void nvme_unplug_fn(void *opaque)
+{
+    NVMeQueuePair *q = opaque;
+
+    QEMU_LOCK_GUARD(&q->lock);
+    nvme_kick(q);
+    nvme_process_completion(q);
+}
+
 static void nvme_submit_command(NVMeQueuePair *q, NVMeRequest *req,
                                 NvmeCmd *cmd, BlockCompletionFunc cb,
                                 void *opaque)
@@ -XXX,XX +XXX,XX @@ static void nvme_submit_command(NVMeQueuePair *q, NVMeRequest *req,
            q->sq.tail * NVME_SQ_ENTRY_BYTES, cmd, sizeof(*cmd));
     q->sq.tail = (q->sq.tail + 1) % NVME_QUEUE_SIZE;
     q->need_kick++;
-    nvme_kick(q);
-    nvme_process_completion(q);
+    blk_io_plug_call(nvme_unplug_fn, q);
     qemu_mutex_unlock(&q->lock);
 }
 
@@ -XXX,XX +XXX,XX @@ static void nvme_attach_aio_context(BlockDriverState *bs,
     }
 }
 
-static void coroutine_fn nvme_co_io_plug(BlockDriverState *bs)
-{
-    BDRVNVMeState *s = bs->opaque;
-    assert(!s->plugged);
-    s->plugged = true;
-}
-
-static void coroutine_fn nvme_co_io_unplug(BlockDriverState *bs)
-{
-    BDRVNVMeState *s = bs->opaque;
-    assert(s->plugged);
-    s->plugged = false;
-    for (unsigned i = INDEX_IO(0); i < s->queue_count; i++) {
-        NVMeQueuePair *q = s->queues[i];
-        qemu_mutex_lock(&q->lock);
-        nvme_kick(q);
-        nvme_process_completion(q);
-        qemu_mutex_unlock(&q->lock);
-    }
-}
-
 static bool nvme_register_buf(BlockDriverState *bs, void *host, size_t size,
                               Error **errp)
 {
@@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_nvme = {
     .bdrv_detach_aio_context  = nvme_detach_aio_context,
     .bdrv_attach_aio_context  = nvme_attach_aio_context,
 
-    .bdrv_co_io_plug          = nvme_co_io_plug,
-    .bdrv_co_io_unplug        = nvme_co_io_unplug,
-
     .bdrv_register_buf        = nvme_register_buf,
     .bdrv_unregister_buf      = nvme_unregister_buf,
 };
diff --git a/block/trace-events b/block/trace-events
index XXXXXXX..XXXXXXX 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -XXX,XX +XXX,XX @@ nvme_kick(void *s, unsigned q_index) "s %p q #%u"
 nvme_dma_flush_queue_wait(void *s) "s %p"
 nvme_error(int cmd_specific, int sq_head, int sqid, int cid, int status) "cmd_specific %d sq_head %d sqid %d cid %d status 0x%x"
 nvme_process_completion(void *s, unsigned q_index, int inflight) "s %p q #%u inflight %d"
-nvme_process_completion_queue_plugged(void *s, unsigned q_index) "s %p q #%u"
 nvme_complete_command(void *s, unsigned q_index, int cid) "s %p q #%u cid %d"
 nvme_submit_command(void *s, unsigned q_index, int cid) "s %p q #%u cid %d"
 nvme_submit_command_raw(int c0, int c1, int c2, int c3, int c4, int c5, int c6, int c7) "%02x %02x %02x %02x %02x %02x %02x %02x"
-- 
2.40.1

Stop using the .bdrv_co_io_plug() API because it is not multi-queue
block layer friendly. Use the new blk_io_plug_call() API to batch I/O
submission instead.

diff --git a/block/blkio.c b/block/blkio.c
index XXXXXXX..XXXXXXX 100644
--- a/block/blkio.c
+++ b/block/blkio.c
@@ -XXX,XX +XXX,XX @@
 #include "qemu/error-report.h"
 #include "qapi/qmp/qdict.h"
 #include "qemu/module.h"
+#include "sysemu/block-backend.h"
 #include "exec/memory.h" /* for ram_block_discard_disable() */
 
 #include "block/block-io.h"
@@ -XXX,XX +XXX,XX @@ static void blkio_detach_aio_context(BlockDriverState *bs)
                        NULL, NULL, NULL);
 }
 
-/* Call with s->blkio_lock held to submit I/O after enqueuing a new request */
-static void blkio_submit_io(BlockDriverState *bs)
+/*
+ * Called by blk_io_unplug() or immediately if not plugged. Called without
+ * blkio_lock.
+ */
+static void blkio_unplug_fn(void *opaque)
 {
-    if (qatomic_read(&bs->io_plugged) == 0) {
-        BDRVBlkioState *s = bs->opaque;
+    BDRVBlkioState *s = opaque;
 
+    WITH_QEMU_LOCK_GUARD(&s->blkio_lock) {
         blkioq_do_io(s->blkioq, NULL, 0, 0, NULL);
     }
 }
 
+/*
+ * Schedule I/O submission after enqueuing a new request. Called without
+ * blkio_lock.
+ */
+static void blkio_submit_io(BlockDriverState *bs)
+{
+    BDRVBlkioState *s = bs->opaque;
+
+    blk_io_plug_call(blkio_unplug_fn, s);
+}
+
 static int coroutine_fn
 blkio_co_pdiscard(BlockDriverState *bs, int64_t offset, int64_t bytes)
 {
@@ -XXX,XX +XXX,XX @@ blkio_co_pdiscard(BlockDriverState *bs, int64_t offset, int64_t bytes)
 
     WITH_QEMU_LOCK_GUARD(&s->blkio_lock) {
         blkioq_discard(s->blkioq, offset, bytes, &cod, 0);
-        blkio_submit_io(bs);
     }
 
+    blkio_submit_io(bs);
     qemu_coroutine_yield();
     return cod.ret;
 }
@@ -XXX,XX +XXX,XX @@ blkio_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
 
     WITH_QEMU_LOCK_GUARD(&s->blkio_lock) {
         blkioq_readv(s->blkioq, offset, iov, iovcnt, &cod, 0);
-        blkio_submit_io(bs);
     }
 
+    blkio_submit_io(bs);
     qemu_coroutine_yield();
 
     if (use_bounce_buffer) {
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn blkio_co_pwritev(BlockDriverState *bs, int64_t offset,
 
     WITH_QEMU_LOCK_GUARD(&s->blkio_lock) {
         blkioq_writev(s->blkioq, offset, iov, iovcnt, &cod, blkio_flags);
-        blkio_submit_io(bs);
     }
 
+    blkio_submit_io(bs);
     qemu_coroutine_yield();
 
     if (use_bounce_buffer) {
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn blkio_co_flush(BlockDriverState *bs)
 
     WITH_QEMU_LOCK_GUARD(&s->blkio_lock) {
         blkioq_flush(s->blkioq, &cod, 0);
-        blkio_submit_io(bs);
     }
 
+    blkio_submit_io(bs);
     qemu_coroutine_yield();
     return cod.ret;
 }
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn blkio_co_pwrite_zeroes(BlockDriverState *bs,
 
     WITH_QEMU_LOCK_GUARD(&s->blkio_lock) {
         blkioq_write_zeroes(s->blkioq, offset, bytes, &cod, blkio_flags);
-        blkio_submit_io(bs);
     }
 
+    blkio_submit_io(bs);
     qemu_coroutine_yield();
     return cod.ret;
 }
 
-static void coroutine_fn blkio_co_io_unplug(BlockDriverState *bs)
-{
-    BDRVBlkioState *s = bs->opaque;
-
-    WITH_QEMU_LOCK_GUARD(&s->blkio_lock) {
-        blkio_submit_io(bs);
-    }
-}
-
 typedef enum {
     BMRR_OK,
     BMRR_SKIP,
@@ -XXX,XX +XXX,XX @@ static void blkio_refresh_limits(BlockDriverState *bs, Error **errp)
         .bdrv_co_pwritev         = blkio_co_pwritev, \
         .bdrv_co_flush_to_disk   = blkio_co_flush, \
         .bdrv_co_pwrite_zeroes   = blkio_co_pwrite_zeroes, \
-        .bdrv_co_io_unplug       = blkio_co_io_unplug, \
         .bdrv_refresh_limits     = blkio_refresh_limits, \
         .bdrv_register_buf       = blkio_register_buf, \
         .bdrv_unregister_buf     = blkio_unregister_buf, \
-- 
2.40.1

Stop using the .bdrv_co_io_plug() API because it is not multi-queue
block layer friendly. Use the new blk_io_plug_call() API to batch I/O
submission instead.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Acked-by: Kevin Wolf <kwolf@redhat.com>
Message-id: 20230530180959.1108766-5-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/block/raw-aio.h |  7 -------
 block/file-posix.c      | 10 ----------
 block/io_uring.c        | 44 ++++++++++++++++-------------------------
 block/trace-events      |  5 ++---
 4 files changed, 19 insertions(+), 47 deletions(-)

diff --git a/include/block/raw-aio.h b/include/block/raw-aio.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/raw-aio.h
+++ b/include/block/raw-aio.h
@@ -XXX,XX +XXX,XX @@ int coroutine_fn luring_co_submit(BlockDriverState *bs, int fd, uint64_t offset,
                                   QEMUIOVector *qiov, int type);
 void luring_detach_aio_context(LuringState *s, AioContext *old_context);
 void luring_attach_aio_context(LuringState *s, AioContext *new_context);
-
-/*
- * luring_io_plug/unplug work in the thread's current AioContext, therefore the
- * caller must ensure that they are paired in the same IOThread.
- */
-void luring_io_plug(void);
-void luring_io_unplug(void);
 #endif
 
 #ifdef _WIN32
diff --git a/block/file-posix.c b/block/file-posix.c
index XXXXXXX..XXXXXXX 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn raw_co_io_plug(BlockDriverState *bs)
         laio_io_plug();
     }
 #endif
-#ifdef CONFIG_LINUX_IO_URING
-    if (s->use_linux_io_uring) {
-        luring_io_plug();
-    }
-#endif
 }
 
 static void coroutine_fn raw_co_io_unplug(BlockDriverState *bs)
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn raw_co_io_unplug(BlockDriverState *bs)
         laio_io_unplug(s->aio_max_batch);
     }
 #endif
-#ifdef CONFIG_LINUX_IO_URING
-    if (s->use_linux_io_uring) {
-        luring_io_unplug();
-    }
-#endif
 }
 
 static int coroutine_fn raw_co_flush_to_disk(BlockDriverState *bs)
diff --git a/block/io_uring.c b/block/io_uring.c
index XXXXXXX..XXXXXXX 100644
--- a/block/io_uring.c
+++ b/block/io_uring.c
@@ -XXX,XX +XXX,XX @@
 #include "block/raw-aio.h"
 #include "qemu/coroutine.h"
 #include "qapi/error.h"
+#include "sysemu/block-backend.h"
 #include "trace.h"
 
 /* Only used for assertions.  */
@@ -XXX,XX +XXX,XX @@ typedef struct LuringAIOCB {
 } LuringAIOCB;
 
 typedef struct LuringQueue {
-    int plugged;
     unsigned int in_queue;
     unsigned int in_flight;
     bool blocked;
@@ -XXX,XX +XXX,XX @@ static void luring_process_completions_and_submit(LuringState *s)
 {
     luring_process_completions(s);
 
-    if (!s->io_q.plugged && s->io_q.in_queue > 0) {
+    if (s->io_q.in_queue > 0) {
         ioq_submit(s);
     }
 }
@@ -XXX,XX +XXX,XX @@ static void qemu_luring_poll_ready(void *opaque)
 static void ioq_init(LuringQueue *io_q)
 {
     QSIMPLEQ_INIT(&io_q->submit_queue);
-    io_q->plugged = 0;
     io_q->in_queue = 0;
     io_q->in_flight = 0;
     io_q->blocked = false;
 }
 
-void luring_io_plug(void)
+static void luring_unplug_fn(void *opaque)
 {
-    AioContext *ctx = qemu_get_current_aio_context();
-    LuringState *s = aio_get_linux_io_uring(ctx);
-    trace_luring_io_plug(s);
-    s->io_q.plugged++;
-}
-
-void luring_io_unplug(void)
-{
-    AioContext *ctx = qemu_get_current_aio_context();
-    LuringState *s = aio_get_linux_io_uring(ctx);
-    assert(s->io_q.plugged);
-    trace_luring_io_unplug(s, s->io_q.blocked, s->io_q.plugged,
-                           s->io_q.in_queue, s->io_q.in_flight);
-    if (--s->io_q.plugged == 0 &&
-        !s->io_q.blocked && s->io_q.in_queue > 0) {
+    LuringState *s = opaque;
+    trace_luring_unplug_fn(s, s->io_q.blocked, s->io_q.in_queue,
+                           s->io_q.in_flight);
+    if (!s->io_q.blocked && s->io_q.in_queue > 0) {
         ioq_submit(s);
     }
 }
@@ -XXX,XX +XXX,XX @@ static int luring_do_submit(int fd, LuringAIOCB *luringcb, LuringState *s,
 
     QSIMPLEQ_INSERT_TAIL(&s->io_q.submit_queue, luringcb, next);
     s->io_q.in_queue++;
-    trace_luring_do_submit(s, s->io_q.blocked, s->io_q.plugged,
-                           s->io_q.in_queue, s->io_q.in_flight);
-    if (!s->io_q.blocked &&
-        (!s->io_q.plugged ||
-         s->io_q.in_flight + s->io_q.in_queue >= MAX_ENTRIES)) {
-        ret = ioq_submit(s);
-        trace_luring_do_submit_done(s, ret);
-        return ret;
+    trace_luring_do_submit(s, s->io_q.blocked, s->io_q.in_queue,
+                           s->io_q.in_flight);
+    if (!s->io_q.blocked) {
+        if (s->io_q.in_flight + s->io_q.in_queue >= MAX_ENTRIES) {
+            ret = ioq_submit(s);
+            trace_luring_do_submit_done(s, ret);
+            return ret;
+        }
+
+        blk_io_plug_call(luring_unplug_fn, s);
     }
     return 0;
 }
diff --git a/block/trace-events b/block/trace-events
index XXXXXXX..XXXXXXX 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -XXX,XX +XXX,XX @@ file_paio_submit(void *acb, void *opaque, int64_t offset, int count, int type) "
 # io_uring.c
 luring_init_state(void *s, size_t size) "s %p size %zu"
 luring_cleanup_state(void *s) "%p freed"
-luring_io_plug(void *s) "LuringState %p plug"
-luring_io_unplug(void *s, int blocked, int plugged, int queued, int inflight) "LuringState %p blocked %d plugged %d queued %d inflight %d"
-luring_do_submit(void *s, int blocked, int plugged, int queued, int inflight) "LuringState %p blocked %d plugged %d queued %d inflight %d"
+luring_unplug_fn(void *s, int blocked, int queued, int inflight) "LuringState %p blocked %d queued %d inflight %d"
+luring_do_submit(void *s, int blocked, int queued, int inflight) "LuringState %p blocked %d queued %d inflight %d"
 luring_do_submit_done(void *s, int ret) "LuringState %p submitted to kernel %d"
 luring_co_submit(void *bs, void *s, void *luringcb, int fd, uint64_t offset, size_t nbytes, int type) "bs %p s %p luringcb %p fd %d offset %" PRId64 " nbytes %zd type %d"
 luring_process_completion(void *s, void *aiocb, int ret) "LuringState %p luringcb %p ret %d"
-- 
2.40.1

Stop using the .bdrv_co_io_plug() API because it is not multi-queue
block layer friendly. Use the new blk_io_plug_call() API to batch I/O
submission instead.

Note that a dev_max_batch check is dropped in laio_io_unplug() because
the semantics of unplug_fn() are different from .bdrv_co_unplug():
1. unplug_fn() is only called when the last blk_io_unplug() call occurs,
   not every time blk_io_unplug() is called.
2. unplug_fn() is per-thread, not per-BlockDriverState, so there is no
   way to get per-BlockDriverState fields like dev_max_batch.

Therefore this condition cannot be moved to laio_unplug_fn(). It is not
obvious that this condition affects performance in practice, so I am
removing it instead of trying to come up with a more complex mechanism
to preserve the condition.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Acked-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Message-id: 20230530180959.1108766-6-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/block/raw-aio.h |  7 -------
 block/file-posix.c      | 28 ----------------------------
 block/linux-aio.c       | 41 +++++++++++------------------------------
 3 files changed, 11 insertions(+), 65 deletions(-)

diff --git a/include/block/raw-aio.h b/include/block/raw-aio.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/raw-aio.h
+++ b/include/block/raw-aio.h
@@ -XXX,XX +XXX,XX @@ int coroutine_fn laio_co_submit(int fd, uint64_t offset, QEMUIOVector *qiov,
 
 void laio_detach_aio_context(LinuxAioState *s, AioContext *old_context);
 void laio_attach_aio_context(LinuxAioState *s, AioContext *new_context);
-
-/*
- * laio_io_plug/unplug work in the thread's current AioContext, therefore the
- * caller must ensure that they are paired in the same IOThread.
- */
-void laio_io_plug(void);
-void laio_io_unplug(uint64_t dev_max_batch);
 #endif
 /* io_uring.c - Linux io_uring implementation */
 #ifdef CONFIG_LINUX_IO_URING
diff --git a/block/file-posix.c b/block/file-posix.c
index XXXXXXX..XXXXXXX 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_pwritev(BlockDriverState *bs, int64_t offset,
     return raw_co_prw(bs, offset, bytes, qiov, QEMU_AIO_WRITE);
 }
 
-static void coroutine_fn raw_co_io_plug(BlockDriverState *bs)
-{
-    BDRVRawState __attribute__((unused)) *s = bs->opaque;
-#ifdef CONFIG_LINUX_AIO
-    if (s->use_linux_aio) {
-        laio_io_plug();
-    }
-#endif
-}
-
-static void coroutine_fn raw_co_io_unplug(BlockDriverState *bs)
-{
-    BDRVRawState __attribute__((unused)) *s = bs->opaque;
-#ifdef CONFIG_LINUX_AIO
-    if (s->use_linux_aio) {
-        laio_io_unplug(s->aio_max_batch);
-    }
-#endif
-}
-
 static int coroutine_fn raw_co_flush_to_disk(BlockDriverState *bs)
 {
     BDRVRawState *s = bs->opaque;
@@ -XXX,XX +XXX,XX @@ BlockDriver bdrv_file = {
     .bdrv_co_copy_range_from = raw_co_copy_range_from,
     .bdrv_co_copy_range_to  = raw_co_copy_range_to,
     .bdrv_refresh_limits = raw_refresh_limits,
-    .bdrv_co_io_plug        = raw_co_io_plug,
-    .bdrv_co_io_unplug      = raw_co_io_unplug,
     .bdrv_attach_aio_context = raw_aio_attach_aio_context,
 
     .bdrv_co_truncate                   = raw_co_truncate,
@@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_host_device = {
     .bdrv_co_copy_range_from = raw_co_copy_range_from,
     .bdrv_co_copy_range_to  = raw_co_copy_range_to,
     .bdrv_refresh_limits = raw_refresh_limits,
-    .bdrv_co_io_plug        = raw_co_io_plug,
-    .bdrv_co_io_unplug      = raw_co_io_unplug,
     .bdrv_attach_aio_context = raw_aio_attach_aio_context,
 
     .bdrv_co_truncate                   = raw_co_truncate,
@@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_host_cdrom = {
     .bdrv_co_pwritev        = raw_co_pwritev,
     .bdrv_co_flush_to_disk  = raw_co_flush_to_disk,
     .bdrv_refresh_limits    = cdrom_refresh_limits,
-    .bdrv_co_io_plug        = raw_co_io_plug,
-    .bdrv_co_io_unplug      = raw_co_io_unplug,
     .bdrv_attach_aio_context = raw_aio_attach_aio_context,
 
     .bdrv_co_truncate                   = raw_co_truncate,
@@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_host_cdrom = {
     .bdrv_co_pwritev        = raw_co_pwritev,
     .bdrv_co_flush_to_disk  = raw_co_flush_to_disk,
     .bdrv_refresh_limits    = cdrom_refresh_limits,
-    .bdrv_co_io_plug        = raw_co_io_plug,
-    .bdrv_co_io_unplug      = raw_co_io_unplug,
     .bdrv_attach_aio_context = raw_aio_attach_aio_context,
 
     .bdrv_co_truncate                   = raw_co_truncate,
diff --git a/block/linux-aio.c b/block/linux-aio.c
index XXXXXXX..XXXXXXX 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -XXX,XX +XXX,XX @@
 #include "qemu/event_notifier.h"
 #include "qemu/coroutine.h"
 #include "qapi/error.h"
+#include "sysemu/block-backend.h"
 
 /* Only used for assertions.  */
 #include "qemu/coroutine_int.h"
@@ -XXX,XX +XXX,XX @@ struct qemu_laiocb {
 };
 
 typedef struct {
-    int plugged;
     unsigned int in_queue;
     unsigned int in_flight;
     bool blocked;
@@ -XXX,XX +XXX,XX @@ static void qemu_laio_process_completions_and_submit(LinuxAioState *s)
 {
     qemu_laio_process_completions(s);
 
-    if (!s->io_q.plugged && !QSIMPLEQ_EMPTY(&s->io_q.pending)) {
+    if (!QSIMPLEQ_EMPTY(&s->io_q.pending)) {
         ioq_submit(s);
     }
 }
@@ -XXX,XX +XXX,XX @@ static void qemu_laio_poll_ready(EventNotifier *opaque)
 static void ioq_init(LaioQueue *io_q)
 {
     QSIMPLEQ_INIT(&io_q->pending);
-    io_q->plugged = 0;
     io_q->in_queue = 0;
     io_q->in_flight = 0;
     io_q->blocked = false;
@@ -XXX,XX +XXX,XX @@ static uint64_t laio_max_batch(LinuxAioState *s, uint64_t dev_max_batch)
     return max_batch;
 }
 
-void laio_io_plug(void)
+static void laio_unplug_fn(void *opaque)
 {
-    AioContext *ctx = qemu_get_current_aio_context();
-    LinuxAioState *s = aio_get_linux_aio(ctx);
+    LinuxAioState *s = opaque;
 
-    s->io_q.plugged++;
-}
-
-void laio_io_unplug(uint64_t dev_max_batch)
-{
-    AioContext *ctx = qemu_get_current_aio_context();
-    LinuxAioState *s = aio_get_linux_aio(ctx);
-
-    assert(s->io_q.plugged);
-    s->io_q.plugged--;
-
-    /*
-     * Why max batch checking is performed here:
-     * Another BDS may have queued requests with a higher dev_max_batch and
-     * therefore in_queue could now exceed our dev_max_batch. Re-check the max
-     * batch so we can honor our device's dev_max_batch.
-     */
-    if (s->io_q.in_queue >= laio_max_batch(s, dev_max_batch) ||
-        (!s->io_q.plugged &&
-         !s->io_q.blocked && !QSIMPLEQ_EMPTY(&s->io_q.pending))) {
+    if (!s->io_q.blocked && !QSIMPLEQ_EMPTY(&s->io_q.pending)) {
         ioq_submit(s);
     }
 }
@@ -XXX,XX +XXX,XX @@ static int laio_do_submit(int fd, struct qemu_laiocb *laiocb, off_t offset,
 
     QSIMPLEQ_INSERT_TAIL(&s->io_q.pending, laiocb, next);
     s->io_q.in_queue++;
-    if (!s->io_q.blocked &&
-        (!s->io_q.plugged ||
-         s->io_q.in_queue >= laio_max_batch(s, dev_max_batch))) {
-        ioq_submit(s);
+    if (!s->io_q.blocked) {
+        if (s->io_q.in_queue >= laio_max_batch(s, dev_max_batch)) {
+            ioq_submit(s);
+        } else {
+            blk_io_plug_call(laio_unplug_fn, s);
+        }
     }
 
     return 0;
-- 
2.40.1

No block driver implements .bdrv_co_io_plug() anymore. Get rid of the
function pointers.

diff --git a/include/block/block-io.h b/include/block/block-io.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -XXX,XX +XXX,XX @@ void coroutine_fn bdrv_co_leave(BlockDriverState *bs, AioContext *old_ctx);
 
 AioContext *child_of_bds_get_parent_aio_context(BdrvChild *c);
 
-void coroutine_fn GRAPH_RDLOCK bdrv_co_io_plug(BlockDriverState *bs);
-void coroutine_fn GRAPH_RDLOCK bdrv_co_io_unplug(BlockDriverState *bs);
-
 bool coroutine_fn GRAPH_RDLOCK
 bdrv_co_can_store_new_dirty_bitmap(BlockDriverState *bs, const char *name,
                                    uint32_t granularity, Error **errp);
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -XXX,XX +XXX,XX @@ struct BlockDriver {
     void coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_debug_event)(
         BlockDriverState *bs, BlkdebugEvent event);
 
-    /* io queue for linux-aio */
-    void coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_io_plug)(BlockDriverState *bs);
-    void coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_io_unplug)(
-        BlockDriverState *bs);
-
     bool (*bdrv_supports_persistent_dirty_bitmap)(BlockDriverState *bs);
 
     bool coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_can_store_new_dirty_bitmap)(
@@ -XXX,XX +XXX,XX @@ struct BlockDriverState {
     unsigned int in_flight;
     unsigned int serialising_in_flight;
 
-    /*
-     * counter for nested bdrv_io_plug.
-     * Accessed with atomic ops.
-     */
-    unsigned io_plugged;
-
     /* do we need to tell the quest if we have a volatile write cache? */
     int enable_write_cache;
 
diff --git a/block/io.c b/block/io.c
index XXXXXXX..XXXXXXX 100644
--- a/block/io.c
+++ b/block/io.c
@@ -XXX,XX +XXX,XX @@ void *qemu_try_blockalign0(BlockDriverState *bs, size_t size)
     return mem;
 }
 
-void coroutine_fn bdrv_co_io_plug(BlockDriverState *bs)
-{
-    BdrvChild *child;
-    IO_CODE();
-    assert_bdrv_graph_readable();
-
-    QLIST_FOREACH(child, &bs->children, next) {
-        bdrv_co_io_plug(child->bs);
-    }
-
-    if (qatomic_fetch_inc(&bs->io_plugged) == 0) {
-        BlockDriver *drv = bs->drv;
-        if (drv && drv->bdrv_co_io_plug) {
-            drv->bdrv_co_io_plug(bs);
-        }
-    }
-}
-
-void coroutine_fn bdrv_co_io_unplug(BlockDriverState *bs)
-{
-    BdrvChild *child;
-    IO_CODE();
-    assert_bdrv_graph_readable();
-
-    assert(bs->io_plugged);
-    if (qatomic_fetch_dec(&bs->io_plugged) == 1) {
-        BlockDriver *drv = bs->drv;
-        if (drv && drv->bdrv_co_io_unplug) {
-            drv->bdrv_co_io_unplug(bs);
-        }
-    }
-
-    QLIST_FOREACH(child, &bs->children, next) {
-        bdrv_co_io_unplug(child->bs);
-    }
-}
-
 /* Helper that undoes bdrv_register_buf() when it fails partway through */
 static void GRAPH_RDLOCK
 bdrv_register_buf_rollback(BlockDriverState *bs, void *host, size_t size,
-- 
2.40.1

From: Stefano Garzarella <sgarzare@redhat.com>

Some virtio-blk drivers (e.g. virtio-blk-vhost-vdpa) supports the fd
passing. Let's expose this to the user, so the management layer
can pass the file descriptor of an already opened path.

If the libblkio virtio-blk driver supports fd passing, let's always
use qemu_open() to open the `path`, so we can handle fd passing
from the management layer through the "/dev/fdset/N" special path.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Message-id: 20230530071941.8954-2-sgarzare@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/blkio.c | 53 ++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 44 insertions(+), 9 deletions(-)

diff --git a/block/blkio.c b/block/blkio.c
index XXXXXXX..XXXXXXX 100644
--- a/block/blkio.c
+++ b/block/blkio.c
@@ -XXX,XX +XXX,XX @@ static int blkio_virtio_blk_common_open(BlockDriverState *bs,
 {
     const char *path = qdict_get_try_str(options, "path");
     BDRVBlkioState *s = bs->opaque;
-    int ret;
+    bool fd_supported = false;
+    int fd, ret;
 
     if (!path) {
         error_setg(errp, "missing 'path' option");
         return -EINVAL;
     }
 
-    ret = blkio_set_str(s->blkio, "path", path);
-    qdict_del(options, "path");
-    if (ret < 0) {
-        error_setg_errno(errp, -ret, "failed to set path: %s",
-                         blkio_get_error_msg());
-        return ret;
-    }
-
     if (!(flags & BDRV_O_NOCACHE)) {
         error_setg(errp, "cache.direct=off is not supported");
         return -EINVAL;
     }
+
+    if (blkio_get_int(s->blkio, "fd", &fd) == 0) {
+        fd_supported = true;
+    }
+
+    /*
+     * If the libblkio driver supports fd passing, let's always use qemu_open()
+     * to open the `path`, so we can handle fd passing from the management
+     * layer through the "/dev/fdset/N" special path.
+     */
+    if (fd_supported) {
+        int open_flags;
+
+        if (flags & BDRV_O_RDWR) {
+            open_flags = O_RDWR;
+        } else {
+            open_flags = O_RDONLY;
+        }
+
+        fd = qemu_open(path, open_flags, errp);
+        if (fd < 0) {
+            return -EINVAL;
+        }
+
+        ret = blkio_set_int(s->blkio, "fd", fd);
+        if (ret < 0) {
+            error_setg_errno(errp, -ret, "failed to set fd: %s",
+                             blkio_get_error_msg());
+            qemu_close(fd);
+            return ret;
+        }
+    } else {
+        ret = blkio_set_str(s->blkio, "path", path);
+        if (ret < 0) {
+            error_setg_errno(errp, -ret, "failed to set path: %s",
+                             blkio_get_error_msg());
+            return ret;
+        }
+    }
+
+    qdict_del(options, "path");
+
     return 0;
 }
 
-- 
2.40.1

From: Stefano Garzarella <sgarzare@redhat.com>

The virtio-blk-vhost-vdpa driver in libblkio 1.3.0 supports the fd
passing through the new 'fd' property.

Since now we are using qemu_open() on '@path' if the virtio-blk driver
supports the fd passing, let's announce it.
In this way, the management layer can pass the file descriptor of an
already opened vhost-vdpa character device. This is useful especially
when the device can only be accessed with certain privileges.

Add the '@fdset' feature only when the virtio-blk-vhost-vdpa driver
in libblkio supports it.

Suggested-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Message-id: 20230530071941.8954-3-sgarzare@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 qapi/block-core.json | 6 ++++++
 meson.build          | 4 ++++
 2 files changed, 10 insertions(+)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index XXXXXXX..XXXXXXX 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -XXX,XX +XXX,XX @@
 #
 # @path: path to the vhost-vdpa character device.
 #
+# Features:
+# @fdset: Member @path supports the special "/dev/fdset/N" path
+#     (since 8.1)
+#
 # Since: 7.2
 ##
 { 'struct': 'BlockdevOptionsVirtioBlkVhostVdpa',
   'data': { 'path': 'str' },
+  'features': [ { 'name' :'fdset',
+                  'if': 'CONFIG_BLKIO_VHOST_VDPA_FD' } ],
   'if': 'CONFIG_BLKIO' }
 
 ##
diff --git a/meson.build b/meson.build
index XXXXXXX..XXXXXXX 100644
--- a/meson.build
+++ b/meson.build
@@ -XXX,XX +XXX,XX @@ config_host_data.set('CONFIG_LZO', lzo.found())
 config_host_data.set('CONFIG_MPATH', mpathpersist.found())
 config_host_data.set('CONFIG_MPATH_NEW_API', mpathpersist_new_api)
 config_host_data.set('CONFIG_BLKIO', blkio.found())
+if blkio.found()
+  config_host_data.set('CONFIG_BLKIO_VHOST_VDPA_FD',
+                       blkio.version().version_compare('>=1.3.0'))
+endif
 config_host_data.set('CONFIG_CURL', curl.found())
 config_host_data.set('CONFIG_CURSES', curses.found())
 config_host_data.set('CONFIG_GBM', gbm.found())
-- 
2.40.1