Series comparison

-[Qemu-devel] [PULL for-4.0 0/1] Block patches
+[PULL 00/19] Block patches
-The following changes since commit 3f3bbfc7cef4490c5ed5550766a81e7d18f08db1:
+The following changes since commit 96662996eda78c48aadddd4e76d8615c7eb72d80:
-  Merge remote-tracking branch 'remotes/huth-gitlab/tags/pull-request-2019-03-12' into staging (2019-03-12 21:06:26 +0000)
+  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:
-  git://github.com/stefanha/qemu.git tags/block-pull-request
+  https://github.com/XanClic/qemu.git tags/pull-block-2021-05-14
-for you to fetch changes up to f357fcd890a8d6ced6d261338b859a41414561e9:
+for you to fetch changes up to c61ebf362d0abf288ce266845519d5a550a1d89f:
-  file-posix: add drop-cache=on|off option (2019-03-13 10:54:55 +0000)
+  write-threshold: deal with includes (2021-05-14 16:14:10 +0200)
 ----------------------------------------------------------------
-Pull request
+Block patches:
+- drop block/io write notifiers
- * Add 'drop-cache=on|off' option to file-posix.c.  The default is on.
+- qemu-iotests enhancements to make debugging easier
-   Disabling the option fixes a QEMU 3.0.0 performance regression when live
+- rbd parsing fix
-   migrating on the same host with cache.direct=off.
+- 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
-Stefan Hajnoczi (1):
+Emanuele Giuseppe Esposito (1):
-  file-posix: add drop-cache=on|off option
+  qemu-iotests: fix pylint 2.8 consider-using-with error
- qapi/block-core.json |  6 ++++++
+Paolo Bonzini (5):
- block/file-posix.c   | 16 ++++++++++++++++
+  qemu-iotests: do not buffer the test output
-files changed, 22 insertions(+)
+  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
  docs/tools/qemu-img.rst           |  31 +++++++
  include/block/block_int.h         |  15 +---
  include/block/write-threshold.h   |  27 ++----
  include/qemu/job.h                |   2 +-
  block.c                           |   1 -
  block/backup.c                    |   2 +-
  block/copy-on-read.c              |  33 +------
  block/io.c                        |  11 +--
  block/mirror.c                    |   6 +-
  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(-)
 --
-.20.1
+.31.1

-New patch
+[PULL 01/19] iotests/231: Update expected deprecation message
+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

-New patch
+[PULL 02/19] block/rbd: Add an escape-aware strchr helper
+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

-[Qemu-devel] [PULL for-4.0 1/1] file-posix: add drop-cache=on|off option
+[PULL 03/19] monitor: hmp_qemu_io: acquire aio contex, fix crash
-Commit dd577a26ff03b6829721b1ffbbf9e7c411b72378 ("block/file-posix:
+From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-implement bdrv_co_invalidate_cache() on Linux") introduced page cache
-invalidation so that cache.direct=off live migration is safe on Linux.
+Max reported the following bug:
-The invalidation takes a significant amount of time when the file is
+$ ./qemu-img create -f raw src.img 1G
-large and present in the page cache.  Normally this is not the case for
+$ ./qemu-img create -f raw dst.img 1G
-cross-host live migration but it can happen when migrating between QEMU
-processes on the same host.
+$ (echo '
+   {"execute":"qmp_capabilities"}
-On same-host migration we don't need to invalidate pages for correctness
+   {"execute":"blockdev-mirror",
-anyway, so an option to skip page cache invalidation is useful.  I
+    "arguments":{"job-id":"mirror",
-investigated optimizing invalidation and detecting same-host migration,
+                 "device":"source",
-but both are hard to achieve so a user-visible option will suffice.
+                 "target":"target",
+                 "sync":"full",
-As a bonus this option means that the cache invalidation feature will
+                 "filter-node-name":"mirror-top"}}
-now be detectable by libvirt via QMP schema introspection.
+'; sleep 3; echo '
+   {"execute":"human-monitor-command",
-Suggested-by: Neil Skrypuch <neil@tembosocial.com>
+    "arguments":{"command-line":
-Tested-by: Neil Skrypuch <neil@tembosocial.com>
+                 "qemu-io mirror-top \"write 0 1G\""}}') \
-Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
+| x86_64-softmmu/qemu-system-x86_64 \
-Reviewed-by: Eric Blake <eblake@redhat.com>
+   -qmp stdio \
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+   -blockdev file,node-name=source,filename=src.img \
-Message-id: 20190307164941.3322-1-stefanha@redhat.com
+   -blockdev file,node-name=target,filename=dst.img \
-Message-Id: <20190307164941.3322-1-stefanha@redhat.com>
+   -object iothread,id=iothr0 \
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+   -device virtio-blk,drive=source,iothread=iothr0
 crashes:
 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>
 ---
- qapi/block-core.json |  6 ++++++
+ block/monitor/block-hmp-cmds.c | 31 +++++++++++++++++++++----------
- block/file-posix.c   | 16 ++++++++++++++++
+ qemu-io-cmds.c                 |  8 ++++----
-files changed, 22 insertions(+)
+ qemu-io.c                      | 17 +++++++++++++++--
+files changed, 40 insertions(+), 16 deletions(-)
-diff --git a/qapi/block-core.json b/qapi/block-core.json
 diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
 index XXXXXXX..XXXXXXX 100644
---- a/qapi/block-core.json
+--- a/block/monitor/block-hmp-cmds.c
-+++ b/qapi/block-core.json
++++ b/block/monitor/block-hmp-cmds.c
-@@ -XXX,XX +XXX,XX @@
+@@ -XXX,XX +XXX,XX @@ void hmp_eject(Monitor *mon, const QDict *qdict)
- # @locking:     whether to enable file locking. If set to 'auto', only enable
- #               when Open File Descriptor (OFD) locking API is available
+ void hmp_qemu_io(Monitor *mon, const QDict *qdict)
- #               (default: auto, since 2.10)
+ {
-+# @drop-cache:  invalidate page cache during live migration.  This prevents
+-    BlockBackend *blk;
-+#               stale data on the migration destination with cache.direct=off.
++    BlockBackend *blk = NULL;
-+#               Currently only supported on Linux hosts.
++    BlockDriverState *bs = NULL;
-+#               (default: on, since: 4.0)
+     BlockBackend *local_blk = NULL;
- # @x-check-cache-dropped: whether to check that page cache was dropped on live
++    AioContext *ctx = NULL;
- #                         migration.  May cause noticeable delays if the image
+     bool qdev = qdict_get_try_bool(qdict, "qdev", false);
- #                         file is large, do not use in production.
+     const char *device = qdict_get_str(qdict, "device");
-@@ -XXX,XX +XXX,XX @@
+     const char *command = qdict_get_str(qdict, "command");
-             '*pr-manager': 'str',
+@@ -XXX,XX +XXX,XX @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict)
-             '*locking': 'OnOffAuto',
+     } else {
-             '*aio': 'BlockdevAioOptions',
+         blk = blk_by_name(device);
-+        '*drop-cache': {'type': 'bool',
+         if (!blk) {
-+                        'if': 'defined(CONFIG_LINUX)'},
+-            BlockDriverState *bs = bdrv_lookup_bs(NULL, device, &err);
-             '*x-check-cache-dropped': 'bool' } }
+-            if (bs) {
+-                blk = local_blk = blk_new(bdrv_get_aio_context(bs),
- ##
+-                                          0, BLK_PERM_ALL);
-diff --git a/block/file-posix.c b/block/file-posix.c
+-                ret = blk_insert_bs(blk, bs, &err);
-index XXXXXXX..XXXXXXX 100644
+-                if (ret < 0) {
---- a/block/file-posix.c
+-                    goto fail;
-+++ b/block/file-posix.c
+-                }
-@@ -XXX,XX +XXX,XX @@ typedef struct BDRVRawState {
+-            } else {
-     bool page_cache_inconsistent:1;
++            bs = bdrv_lookup_bs(NULL, device, &err);
-     bool has_fallocate;
++            if (!bs) {
-     bool needs_alignment;
+                 goto fail;
-+    bool drop_cache;
+             }
      bool check_cache_dropped;
      PRManager *pr_mgr;
@@ -XXX,XX +XXX,XX @@ typedef struct BDRVRawState {
  typedef struct BDRVRawReopenState {
      int fd;
      int open_flags;
 +    bool drop_cache;
      bool check_cache_dropped;
  } BDRVRawReopenState;
@@ -XXX,XX +XXX,XX @@ static QemuOptsList raw_runtime_opts = {
              .type = QEMU_OPT_STRING,
              .help = "id of persistent reservation manager object (default: none)",
          },
 +#if defined(__linux__)
 +        {
 +            .name = "drop-cache",
 +            .type = QEMU_OPT_BOOL,
 +            .help = "invalidate page cache during live migration (default: on)",
 +        },
 +#endif
          {
              .name = "x-check-cache-dropped",
              .type = QEMU_OPT_BOOL,
@@ -XXX,XX +XXX,XX @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
          }
      }
-+    s->drop_cache = qemu_opt_get_bool(opts, "drop-cache", true);
++    ctx = blk ? blk_get_aio_context(blk) : bdrv_get_aio_context(bs);
-     s->check_cache_dropped = qemu_opt_get_bool(opts, "x-check-cache-dropped",
++    aio_context_acquire(ctx);
-                                                false);
++
++    if (bs) {
-@@ -XXX,XX +XXX,XX @@ static int raw_reopen_prepare(BDRVReopenState *state,
++        blk = local_blk = blk_new(bdrv_get_aio_context(bs), 0, BLK_PERM_ALL);
-         goto out;
++        ret = blk_insert_bs(blk, bs, &err);
-     }
++        if (ret < 0) {
++            goto fail;
-+    rs->drop_cache = qemu_opt_get_bool_del(opts, "drop-cache", true);
++        }
      rs->check_cache_dropped =
          qemu_opt_get_bool_del(opts, "x-check-cache-dropped", false);
@@ -XXX,XX +XXX,XX @@ static void raw_reopen_commit(BDRVReopenState *state)
      BDRVRawState *s = state->bs->opaque;
      Error *local_err = NULL;
 +    s->drop_cache = rs->drop_cache;
      s->check_cache_dropped = rs->check_cache_dropped;
      s->open_flags = rs->open_flags;
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn raw_co_invalidate_cache(BlockDriverState *bs,
          return;
      }
 +    if (!s->drop_cache) {
 +        return;
 +    }
 +
-     if (s->open_flags & O_DIRECT) {
+     /*
-         return; /* No host kernel page cache */
+      * 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) {
 --
-.20.1
+.31.1

-New patch
+[PULL 04/19] mirror: stop cancelling in-flight requests on non-force cancel in READY
+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 +-
+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()
+--
+.31.1

-New patch
+[PULL 05/19] qemu-iotests: do not buffer the test output
+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

-New patch
+[PULL 06/19] qemu-iotests: allow passing unittest.main arguments to the test scripts
+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

-New patch
+[PULL 07/19] qemu-iotests: move command line and environment handling from TestRunner to TestEnv
+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

-New patch
+[PULL 08/19] qemu-iotests: let "check" spawn an arbitrary test command
+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

-New patch
+[PULL 09/19] qemu-iotests: fix case of SOCK_DIR already in the environment
+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

-New patch
+[PULL 10/19] Document qemu-img options data_file and data_file_raw
+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

-New patch
+[PULL 11/19] block/copy-on-read: use bdrv_drop_filter() and drop s->active
+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

-New patch
+[PULL 12/19] qemu-iotests: fix pylint 2.8 consider-using-with error
+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

-New patch
+[PULL 13/19] block/write-threshold: don't use write notifiers
+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 +++++++--------------------------
+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);
++    }
++}
+--
+.31.1

-New patch
+[PULL 14/19] block: drop write notifiers
+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

-New patch
+[PULL 15/19] test-write-threshold: rewrite test_threshold_(not_)trigger tests
+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 ++++------------------
+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 {
+--
+.31.1

-New patch
+[PULL 16/19] block/write-threshold: drop extra APIs
+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 ----
+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);
+ }
+--
+.31.1

-New patch
+[PULL 17/19] test-write-threshold: drop extra tests
+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 -------------------------------
+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",
+--
+.31.1

-New patch
+[PULL 18/19] test-write-threshold: drop extra TestStruct structure
+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 +++-----------------
+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();
+ }
+--
+.31.1

-New patch
+[PULL 19/19] write-threshold: deal with includes
+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 -
+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"
+--
+.31.1

Commit dd577a26ff03b6829721b1ffbbf9e7c411b72378 ("block/file-posix:
implement bdrv_co_invalidate_cache() on Linux") introduced page cache
invalidation so that cache.direct=off live migration is safe on Linux.

The invalidation takes a significant amount of time when the file is
large and present in the page cache.  Normally this is not the case for
cross-host live migration but it can happen when migrating between QEMU
processes on the same host.

On same-host migration we don't need to invalidate pages for correctness
anyway, so an option to skip page cache invalidation is useful.  I
investigated optimizing invalidation and detecting same-host migration,
but both are hard to achieve so a user-visible option will suffice.

As a bonus this option means that the cache invalidation feature will
now be detectable by libvirt via QMP schema introspection.

Suggested-by: Neil Skrypuch <neil@tembosocial.com>
Tested-by: Neil Skrypuch <neil@tembosocial.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20190307164941.3322-1-stefanha@redhat.com
Message-Id: <20190307164941.3322-1-stefanha@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 qapi/block-core.json |  6 ++++++
 block/file-posix.c   | 16 ++++++++++++++++
 2 files changed, 22 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 @@
 # @locking:     whether to enable file locking. If set to 'auto', only enable
 #               when Open File Descriptor (OFD) locking API is available
 #               (default: auto, since 2.10)
+# @drop-cache:  invalidate page cache during live migration.  This prevents
+#               stale data on the migration destination with cache.direct=off.
+#               Currently only supported on Linux hosts.
+#               (default: on, since: 4.0)
 # @x-check-cache-dropped: whether to check that page cache was dropped on live
 #                         migration.  May cause noticeable delays if the image
 #                         file is large, do not use in production.
@@ -XXX,XX +XXX,XX @@
             '*pr-manager': 'str',
             '*locking': 'OnOffAuto',
             '*aio': 'BlockdevAioOptions',
+	    '*drop-cache': {'type': 'bool',
+	                    'if': 'defined(CONFIG_LINUX)'},
             '*x-check-cache-dropped': 'bool' } }
 
 ##
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 @@ typedef struct BDRVRawState {
     bool page_cache_inconsistent:1;
     bool has_fallocate;
     bool needs_alignment;
+    bool drop_cache;
     bool check_cache_dropped;
 
     PRManager *pr_mgr;
@@ -XXX,XX +XXX,XX @@ typedef struct BDRVRawState {
 typedef struct BDRVRawReopenState {
     int fd;
     int open_flags;
+    bool drop_cache;
     bool check_cache_dropped;
 } BDRVRawReopenState;
 
@@ -XXX,XX +XXX,XX @@ static QemuOptsList raw_runtime_opts = {
             .type = QEMU_OPT_STRING,
             .help = "id of persistent reservation manager object (default: none)",
         },
+#if defined(__linux__)
+        {
+            .name = "drop-cache",
+            .type = QEMU_OPT_BOOL,
+            .help = "invalidate page cache during live migration (default: on)",
+        },
+#endif
         {
             .name = "x-check-cache-dropped",
             .type = QEMU_OPT_BOOL,
@@ -XXX,XX +XXX,XX @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
         }
     }
 
+    s->drop_cache = qemu_opt_get_bool(opts, "drop-cache", true);
     s->check_cache_dropped = qemu_opt_get_bool(opts, "x-check-cache-dropped",
                                                false);
 
@@ -XXX,XX +XXX,XX @@ static int raw_reopen_prepare(BDRVReopenState *state,
         goto out;
     }
 
+    rs->drop_cache = qemu_opt_get_bool_del(opts, "drop-cache", true);
     rs->check_cache_dropped =
         qemu_opt_get_bool_del(opts, "x-check-cache-dropped", false);
 
@@ -XXX,XX +XXX,XX @@ static void raw_reopen_commit(BDRVReopenState *state)
     BDRVRawState *s = state->bs->opaque;
     Error *local_err = NULL;
 
+    s->drop_cache = rs->drop_cache;
     s->check_cache_dropped = rs->check_cache_dropped;
     s->open_flags = rs->open_flags;
 
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn raw_co_invalidate_cache(BlockDriverState *bs,
         return;
     }
 
+    if (!s->drop_cache) {
+        return;
+    }
+
     if (s->open_flags & O_DIRECT) {
         return; /* No host kernel page cache */
     }
-- 
2.20.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