Series comparison

-[Qemu-devel] [PULL 00/10] Block patches
+[PULL 00/19] Block patches
-The following changes since commit 411ad78115ebeb3411cf4b7622784b93dfabe259:
+The following changes since commit 96662996eda78c48aadddd4e76d8615c7eb72d80:
-  Merge remote-tracking branch 'remotes/stefanberger/tags/pull-tpm-2017-12-15-1' into staging (2017-12-17 15:27:41 +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:
+are available in the Git repository at:
-  git://github.com/codyprime/qemu-kvm-jtc.git tags/block-pull-request
+  https://github.com/XanClic/qemu.git tags/pull-block-2021-05-14
-for you to fetch changes up to 996922de45299878cdc4c15b72b19edf2bc618a4:
+for you to fetch changes up to c61ebf362d0abf288ce266845519d5a550a1d89f:
-  block/curl: fix minor memory leaks (2017-12-18 15:44:39 -0500)
+  write-threshold: deal with includes (2021-05-14 16:14:10 +0200)
 ----------------------------------------------------------------
-Blockjob and protocol patches
+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
-Jeff Cody (4):
+Emanuele Giuseppe Esposito (1):
-  block/sheepdog: remove spurious NULL check
+  qemu-iotests: fix pylint 2.8 consider-using-with error
   block/sheepdog: code beautification
   block/curl: check error return of curl_global_init()
   block/curl: fix minor memory leaks
-John Snow (1):
+Paolo Bonzini (5):
-  blockjob: kick jobs on set-speed
+  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 (5):
+Vladimir Sementsov-Ogievskiy (10):
-  hbitmap: add next_zero function
+  monitor: hmp_qemu_io: acquire aio contex, fix crash
-  backup: move from done_bitmap to copy_bitmap
+  mirror: stop cancelling in-flight requests on non-force cancel in
-  backup: init copy_bitmap from sync_bitmap for incremental
+    READY
-  backup: simplify non-dirty bits progress processing
+  block/copy-on-read: use bdrv_drop_filter() and drop s->active
-  backup: use copy_bitmap in incremental backup
+  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
- block/backup.c               | 116 +++++++++++++++++-------------
+ docs/tools/qemu-img.rst           |  31 +++++++
- block/curl.c                 |  24 +++++--
+ include/block/block_int.h         |  15 +---
- block/dirty-bitmap.c         |   5 ++
+ include/block/write-threshold.h   |  27 ++----
- block/sheepdog.c             | 166 +++++++++++++++++++++----------------------
+ include/qemu/job.h                |   2 +-
- blockjob.c                   |  30 +++++++-
+ block.c                           |   1 -
- include/block/dirty-bitmap.h |   1 +
+ block/backup.c                    |   2 +-
- include/qemu/hbitmap.h       |   8 +++
+ block/copy-on-read.c              |  33 +------
- tests/test-hbitmap.c         |  61 ++++++++++++++++
+ block/io.c                        |  11 +--
- util/hbitmap.c               |  39 ++++++++++
+ block/mirror.c                    |   6 +-
-files changed, 309 insertions(+), 141 deletions(-)
+ 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(-)
 --
-.9.5
+.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

-[Qemu-devel] [PULL 09/10] block/curl: check error return of curl_global_init()
+[PULL 02/19] block/rbd: Add an escape-aware strchr helper
-If curl_global_init() fails, per the documentation no other curl
+From: Connor Kuehl <ckuehl@redhat.com>
 functions may be called, so make sure to check the return value.
-Also, some minor changes to the initialization latch variable 'inited':
+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.
-- Make it static in the file, for clarity
+However, qemu_rbd_next_tok() will escape characters as it removes tokens
-- Change the name for clarity
+from the token stream and plain strchr() won't. This can make the
-- Make it a bool
+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.
-Signed-off-by: Jeff Cody <jcody@redhat.com>
+Use a custom strchr to avoid mixing escaped and unescaped string
-Reviewed-by: Eric Blake <eblake@redhat.com>
+operations. Furthermore, this code is identical to how
-Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
+qemu_rbd_next_tok() seeks its next token, so incorporate this custom
-Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
+strchr into the body of that function to reduce duplication.
-Signed-off-by: Jeff Cody <jcody@redhat.com>
 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/curl.c | 18 ++++++++++++------
+ block/rbd.c                | 32 +++++++++++++++++++++-----------
-file changed, 12 insertions(+), 6 deletions(-)
+ tests/qemu-iotests/231     |  4 ++++
  tests/qemu-iotests/231.out |  3 +++
 files changed, 28 insertions(+), 11 deletions(-)
-diff --git a/block/curl.c b/block/curl.c
+diff --git a/block/rbd.c b/block/rbd.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/curl.c
+--- a/block/rbd.c
-+++ b/block/curl.c
++++ b/block/rbd.c
-@@ -XXX,XX +XXX,XX @@ static CURLMcode __curl_multi_socket_action(CURLM *multi_handle,
+@@ -XXX,XX +XXX,XX @@ static int qemu_rbd_connect(rados_t *cluster, rados_ioctx_t *io_ctx,
+                             const char *keypairs, const char *secretid,
- struct BDRVCURLState;
+                             Error **errp);
-+static bool libcurl_initialized;
++static char *qemu_rbd_strchr(char *src, char delim)
 +{
 +    char *p;
 +
- typedef struct CURLAIOCB {
++    for (p = src; *p; ++p) {
-     Coroutine *co;
++        if (*p == delim) {
-     QEMUIOVector *qiov;
++            return p;
@@ -XXX,XX +XXX,XX @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
      double d;
      const char *secretid;
      const char *protocol_delimiter;
 +    int ret;
 -    static int inited = 0;
      if (flags & BDRV_O_RDWR) {
          error_setg(errp, "curl block device does not support writes");
          return -EROFS;
      }
 +    if (!libcurl_initialized) {
 +        ret = curl_global_init(CURL_GLOBAL_ALL);
 +        if (ret) {
 +            error_setg(errp, "libcurl initialization failed with %d", ret);
 +            return -EIO;
 +        }
-+        libcurl_initialized = true;
++        if (*p == '\\' && p[1] != '\0') {
 +            ++p;
 +        }
 +    }
 +
-     qemu_mutex_init(&s->mutex);
++    return NULL;
-     opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
++}
-     qemu_opts_absorb_qdict(opts, options, &local_err);
++
-@@ -XXX,XX +XXX,XX @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
++
-         }
+ 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,
--    if (!inited) {
+     qemu_rbd_unescape(found_str);
--        curl_global_init(CURL_GLOBAL_ALL);
+     qdict_put_str(options, "pool", found_str);
--        inited = 1;
--    }
+-    if (strchr(p, '@')) {
--
++    if (qemu_rbd_strchr(p, '@')) {
-     DPRINTF("CURL: Opening %s\n", file);
+         image_name = qemu_rbd_next_tok(p, '@', &p);
-     QSIMPLEQ_INIT(&s->free_state_waitq);
-     s->aio_context = bdrv_get_aio_context(bs);
+         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
 --
-.9.5
+.31.1

-New patch
+[PULL 03/19] monitor: hmp_qemu_io: acquire aio contex, fix crash
+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:
 raise () at /usr/lib/libc.so.6
 abort () at /usr/lib/libc.so.6
 error_exit
    (err=<optimized out>,
    msg=msg@entry=0x55fbb1634790 <__func__.27> "qemu_mutex_unlock_impl")
    at ../util/qemu-thread-posix.c:37
 qemu_mutex_unlock_impl
    (mutex=mutex@entry=0x55fbb25ab6e0,
    file=file@entry=0x55fbb1636957 "../util/async.c",
    line=line@entry=650)
    at ../util/qemu-thread-posix.c:109
 aio_context_release (ctx=ctx@entry=0x55fbb25ab680) at ../util/async.c:650
 bdrv_do_drained_begin
    (bs=bs@entry=0x55fbb3a87000, recursive=recursive@entry=false,
    parent=parent@entry=0x0,
    ignore_bds_parents=ignore_bds_parents@entry=false,
    poll=poll@entry=true) at ../block/io.c:441
 bdrv_do_drained_begin
    (poll=true, ignore_bds_parents=false, parent=0x0, recursive=false,
    bs=0x55fbb3a87000) at ../block/io.c:448
 blk_drain (blk=0x55fbb26c5a00) at ../block/block-backend.c:1718
 blk_unref (blk=0x55fbb26c5a00) at ../block/block-backend.c:498
 blk_unref (blk=0x55fbb26c5a00) at ../block/block-backend.c:491
 hmp_qemu_io (mon=0x7fffaf3fc7d0, qdict=<optimized out>)
    at ../block/monitor/block-hmp-cmds.c:628
 man pthread_mutex_unlock
 ...
     EPERM  The  mutex type is PTHREAD_MUTEX_ERRORCHECK or
     PTHREAD_MUTEX_RECURSIVE, or the mutex is a robust mutex, and the
     current thread does not own the mutex.
 So, thread doesn't own the mutex. And we have iothread here.
 Next, note that AIO_WAIT_WHILE() documents that ctx must be acquired
 exactly once by caller. But where is it acquired in the call stack?
 Seems nowhere.
 qemuio_command do acquire aio context.. But we need context acquired
 around blk_unref() as well and actually around blk_insert_bs() too.
 Let's refactor qemuio_command so that it doesn't acquire aio context
 but callers do that instead. This way we can cleanly acquire aio
 context in hmp_qemu_io() around all three calls.
 Reported-by: Max Reitz <mreitz@redhat.com>
 Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
 Message-Id: <20210423134233.51495-1-vsementsov@virtuozzo.com>
 [mreitz: Fixed comment]
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  block/monitor/block-hmp-cmds.c | 31 +++++++++++++++++++++----------
  qemu-io-cmds.c                 |  8 ++++----
  qemu-io.c                      | 17 +++++++++++++++--
 files changed, 40 insertions(+), 16 deletions(-)
 diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/monitor/block-hmp-cmds.c
 +++ 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) {
 --
 .31.1

-[Qemu-devel] [PULL 03/10] backup: init copy_bitmap from sync_bitmap for incremental
+[PULL 04/19] mirror: stop cancelling in-flight requests on non-force cancel in READY
 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-We should not copy non-dirty clusters in write notifiers. So,
+If mirror is READY than cancel operation is not discarding the whole
-initialize copy_bitmap from sync_bitmap.
+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>
-Reviewed-by: John Snow <jsnow@redhat.com>
+Message-Id: <20210421075858.40197-1-vsementsov@virtuozzo.com>
-Reviewed-by: Jeff Cody <jcody@redhat.com>
+Signed-off-by: Max Reitz <mreitz@redhat.com>
 Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
 Message-id: 20171012135313.227864-4-vsementsov@virtuozzo.com
 Signed-off-by: Jeff Cody <jcody@redhat.com>
 ---
- block/backup.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
+ include/block/block_int.h | 2 +-
-file changed, 43 insertions(+), 1 deletion(-)
+ 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 @@ out:
+@@ -XXX,XX +XXX,XX @@ static void coroutine_fn backup_set_speed(BlockJob *job, int64_t speed)
-     return ret;
+     }
  }
-+/* init copy_bitmap from sync_bitmap */
+-static void backup_cancel(Job *job)
-+static void backup_incremental_init_copy_bitmap(BackupBlockJob *job)
++static void backup_cancel(Job *job, bool force)
-+{
+ {
-+    BdrvDirtyBitmapIter *dbi;
+     BackupBlockJob *s = container_of(job, BackupBlockJob, common.job);
-+    int64_t offset;
-+    int64_t end = DIV_ROUND_UP(bdrv_dirty_bitmap_size(job->sync_bitmap),
+diff --git a/block/mirror.c b/block/mirror.c
-+                               job->cluster_size);
+index XXXXXXX..XXXXXXX 100644
-+
+--- a/block/mirror.c
-+    dbi = bdrv_dirty_iter_new(job->sync_bitmap);
++++ b/block/mirror.c
-+    while ((offset = bdrv_dirty_iter_next(dbi)) != -1) {
+@@ -XXX,XX +XXX,XX @@ static bool mirror_drained_poll(BlockJob *job)
-+        int64_t cluster = offset / job->cluster_size;
+     return !!s->in_flight;
-+        int64_t next_cluster;
+ }
-+
-+        offset += bdrv_dirty_bitmap_granularity(job->sync_bitmap);
+-static void mirror_cancel(Job *job)
-+        if (offset >= bdrv_dirty_bitmap_size(job->sync_bitmap)) {
++static void mirror_cancel(Job *job, bool force)
-+            hbitmap_set(job->copy_bitmap, cluster, end - cluster);
+ {
-+            break;
+     MirrorBlockJob *s = container_of(job, MirrorBlockJob, common.job);
-+        }
+     BlockDriverState *target = blk_bs(s->target);
-+
-+        offset = bdrv_dirty_bitmap_next_zero(job->sync_bitmap, offset);
+-    bdrv_cancel_in_flight(target);
-+        if (offset == -1) {
++    if (force || !job_is_ready(job)) {
-+            hbitmap_set(job->copy_bitmap, cluster, end - cluster);
++        bdrv_cancel_in_flight(target);
 +            break;
 +        }
 +
 +        next_cluster = DIV_ROUND_UP(offset, job->cluster_size);
 +        hbitmap_set(job->copy_bitmap, cluster, next_cluster - cluster);
 +        if (next_cluster >= end) {
 +            break;
 +        }
 +
 +        bdrv_set_dirty_iter(dbi, next_cluster * job->cluster_size);
 +    }
-+
+ }
-+    bdrv_dirty_iter_free(dbi);
-+}
+ static const BlockJobDriver mirror_job_driver = {
-+
+diff --git a/job.c b/job.c
- static void coroutine_fn backup_run(void *opaque)
+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)
  {
-     BackupBlockJob *job = opaque;
+     if (job->driver->cancel) {
-@@ -XXX,XX +XXX,XX @@ static void coroutine_fn backup_run(void *opaque)
+-        job->driver->cancel(job);
++        job->driver->cancel(job, force);
-     nb_clusters = DIV_ROUND_UP(job->common.len, job->cluster_size);
+     }
-     job->copy_bitmap = hbitmap_alloc(nb_clusters, 0);
+     if (job->user_paused) {
--    hbitmap_set(job->copy_bitmap, 0, nb_clusters);
+         /* Do not call job_enter here, the caller will handle it.  */
-+    if (job->sync_mode == MIRROR_SYNC_MODE_INCREMENTAL) {
+diff --git a/tests/qemu-iotests/264 b/tests/qemu-iotests/264
-+        backup_incremental_init_copy_bitmap(job);
+index XXXXXXX..XXXXXXX 100755
-+    } else {
+--- a/tests/qemu-iotests/264
-+        hbitmap_set(job->copy_bitmap, 0, nb_clusters);
++++ b/tests/qemu-iotests/264
-+    }
+@@ -XXX,XX +XXX,XX @@ class TestNbdReconnect(iotests.QMPTestCase):
-+
+             self.assert_qmp(result, 'return', {})
-     job->before_write.notify = backup_before_write_notify;
+     def cancel_job(self):
-     bdrv_add_before_write_notifier(bs, &job->before_write);
+-        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()
 --
-.9.5
+.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

-[Qemu-devel] [PULL 07/10] block/sheepdog: remove spurious NULL check
+[PULL 09/19] qemu-iotests: fix case of SOCK_DIR already in the environment
-'tag' is already checked in the lines immediately preceding this check,
+From: Paolo Bonzini <pbonzini@redhat.com>
 and set to non-NULL if NULL.  No need to check again, it hasn't changed.
-Signed-off-by: Jeff Cody <jcody@redhat.com>
+Due to a typo, in this case the SOCK_DIR was not being created.
-Reviewed-by: Eric Blake <eblake@redhat.com>
-Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
+Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Signed-off-by: Jeff Cody <jcody@redhat.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>
 ---
- block/sheepdog.c | 2 +-
+ tests/qemu-iotests/testenv.py | 2 +-
 file changed, 1 insertion(+), 1 deletion(-)
-diff --git a/block/sheepdog.c b/block/sheepdog.c
+diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
 index XXXXXXX..XXXXXXX 100644
---- a/block/sheepdog.c
+--- a/tests/qemu-iotests/testenv.py
-+++ b/block/sheepdog.c
++++ b/tests/qemu-iotests/testenv.py
-@@ -XXX,XX +XXX,XX @@ static int sd_open(BlockDriverState *bs, QDict *options, int flags,
+@@ -XXX,XX +XXX,XX @@ def init_directories(self) -> None:
-     if (!tag) {
+         try:
-         tag = "";
+             self.sock_dir = os.environ['SOCK_DIR']
-     }
+             self.tmp_sock_dir = False
--    if (tag && strlen(tag) >= SD_MAX_VDI_TAG_LEN) {
+-            Path(self.test_dir).mkdir(parents=True, exist_ok=True)
-+    if (strlen(tag) >= SD_MAX_VDI_TAG_LEN) {
++            Path(self.sock_dir).mkdir(parents=True, exist_ok=True)
-         error_setg(errp, "value of parameter 'tag' is too long");
+         except KeyError:
-         ret = -EINVAL;
+             self.sock_dir = tempfile.mkdtemp()
-         goto err_no_fd;
+             self.tmp_sock_dir = True
 --
-.9.5
+.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

-[Qemu-devel] [PULL 04/10] backup: simplify non-dirty bits progress processing
+[PULL 11/19] block/copy-on-read: use bdrv_drop_filter() and drop s->active
 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Set fake progress for non-dirty clusters in copy_bitmap initialization,
+Now, after huge update of block graph permission update algorithm, we
-to. It simplifies code and allows further refactoring.
+don't need this workaround with active state of the filter. Drop it and
+use new smart bdrv_drop_filter() function.
 This patch changes user's view of backup progress, but formally it
 doesn't changed: progress hops are just moved to the beginning.
 Actually it's just a point of view: when do we actually skip clusters?
 We can say in the very beginning, that we skip these clusters and do
 not think about them later.
 Of course, if go through disk sequentially, it's logical to say, that
 we skip clusters between copied portions to the left and to the right
 of them. But even now copying progress is not sequential because of
 write notifiers. Future patches will introduce new backup architecture
 which will do copying in several coroutines in parallel, so it will
 make no sense to publish fake progress by parts in parallel with
 other copying requests.
 Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Reviewed-by: John Snow <jsnow@redhat.com>
+Message-Id: <20210506194143.394141-1-vsementsov@virtuozzo.com>
-Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
+Signed-off-by: Max Reitz <mreitz@redhat.com>
 Reviewed-by: Jeff Cody <jcody@redhat.com>
 Message-id: 20171012135313.227864-5-vsementsov@virtuozzo.com
 Signed-off-by: Jeff Cody <jcody@redhat.com>
 ---
- block/backup.c | 18 +++---------------
+ block/copy-on-read.c | 33 +--------------------------------
-file changed, 3 insertions(+), 15 deletions(-)
+file changed, 1 insertion(+), 32 deletions(-)
-diff --git a/block/backup.c b/block/backup.c
+diff --git a/block/copy-on-read.c b/block/copy-on-read.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/backup.c
+--- a/block/copy-on-read.c
-+++ b/block/backup.c
++++ b/block/copy-on-read.c
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
+@@ -XXX,XX +XXX,XX @@
-     int64_t offset;
-     int64_t cluster;
-     int64_t end;
+ typedef struct BDRVStateCOR {
--    int64_t last_cluster = -1;
+-    bool active;
-     BdrvDirtyBitmapIter *dbi;
+     BlockDriverState *bottom_bs;
+     bool chain_frozen;
-     granularity = bdrv_dirty_bitmap_granularity(job->sync_bitmap);
+ } BDRVStateCOR;
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
+@@ -XXX,XX +XXX,XX @@ static int cor_open(BlockDriverState *bs, QDict *options, int flags,
-     while ((offset = bdrv_dirty_iter_next(dbi)) >= 0) {
+          */
-         cluster = offset / job->cluster_size;
+         bdrv_ref(bottom_bs);
+     }
--        /* Fake progress updates for any clusters we skipped */
+-    state->active = true;
--        if (cluster != last_cluster + 1) {
+     state->bottom_bs = bottom_bs;
--            job->common.offset += ((cluster - last_cluster - 1) *
--                                   job->cluster_size);
+     /*
--        }
+@@ -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;
 -
-         for (end = cluster + clusters_per_iter; cluster < end; cluster++) {
+-    if (!s->active) {
-             do {
+-        /*
-                 if (yield_and_check(job)) {
+-         * While the filter is being removed
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
+-         */
-         if (granularity < job->cluster_size) {
+-        *nperm = 0;
-             bdrv_set_dirty_iter(dbi, cluster * job->cluster_size);
+-        *nshared = BLK_PERM_ALL;
-         }
+-        return;
 -
 -        last_cluster = cluster - 1;
 -    }
 -
--    /* Play some final catchup with the progress meter */
+     *nperm = perm & PERM_PASSTHROUGH;
--    end = DIV_ROUND_UP(job->common.len, job->cluster_size);
+     *nshared = (shared & PERM_PASSTHROUGH) | PERM_UNCHANGED;
--    if (last_cluster + 1 < end) {
--        job->common.offset += ((end - last_cluster - 1) * job->cluster_size);
+@@ -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);
- out:
+-    bdrv_replace_node(cor_filter_bs, bs, &error_abort);
-@@ -XXX,XX +XXX,XX @@ static void backup_incremental_init_copy_bitmap(BackupBlockJob *job)
+-
-         bdrv_set_dirty_iter(dbi, next_cluster * job->cluster_size);
+-    bdrv_drained_end(bs);
-     }
+-    bdrv_unref(bs);
++    bdrv_drop_filter(cor_filter_bs, &error_abort);
-+    job->common.offset = job->common.len -
+     bdrv_unref(cor_filter_bs);
 +                         hbitmap_count(job->copy_bitmap) * job->cluster_size;
 +
      bdrv_dirty_iter_free(dbi);
  }
 --
-.9.5
+.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

-[Qemu-devel] [PULL 10/10] block/curl: fix minor memory leaks
+[PULL 13/19] block/write-threshold: don't use write notifiers
-Signed-off-by: Jeff Cody <jcody@redhat.com>
+From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
-Signed-off-by: Jeff Cody <jcody@redhat.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>
 ---
- block/curl.c | 6 ++++++
+ include/block/block_int.h       |  1 -
-file changed, 6 insertions(+)
+ include/block/write-threshold.h |  9 +++++
  block/io.c                      |  5 ++-
  block/write-threshold.c         | 70 +++++++--------------------------
 files changed, 27 insertions(+), 58 deletions(-)
-diff --git a/block/curl.c b/block/curl.c
+diff --git a/include/block/block_int.h b/include/block/block_int.h
 index XXXXXXX..XXXXXXX 100644
---- a/block/curl.c
+--- a/include/block/block_int.h
-+++ b/block/curl.c
++++ b/include/block/block_int.h
-@@ -XXX,XX +XXX,XX @@ out_noclean:
+@@ -XXX,XX +XXX,XX @@ struct BlockDriverState {
-     qemu_mutex_destroy(&s->mutex);
-     g_free(s->cookie);
+     /* threshold limit for writes, in bytes. "High water mark". */
-     g_free(s->url);
+     uint64_t write_threshold_offset;
-+    g_free(s->username);
+-    NotifierWithReturn write_threshold_notifier;
-+    g_free(s->proxyusername);
-+    g_free(s->proxypassword);
+     /* Writing to the list requires the BQL _and_ the dirty_bitmap_mutex.
-     qemu_opts_del(opts);
+      * Reading from the list can be done with either the BQL or the
-     return -EINVAL;
+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;
  }
-@@ -XXX,XX +XXX,XX @@ static void curl_close(BlockDriverState *bs)
+-static void write_threshold_disable(BlockDriverState *bs)
-     g_free(s->cookie);
+-{
-     g_free(s->url);
+-    if (bdrv_write_threshold_is_set(bs)) {
-+    g_free(s->username);
+-        notifier_with_return_remove(&bs->write_threshold_notifier);
-+    g_free(s->proxyusername);
+-        bs->write_threshold_offset = 0;
-+    g_free(s->proxypassword);
+-    }
 -}
 -
  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 int64_t curl_getlength(BlockDriverState *bs)
+-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);
 +    }
 +}
 --
-.9.5
+.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

-[Qemu-devel] [PULL 06/10] blockjob: kick jobs on set-speed
+[PULL 15/19] test-write-threshold: rewrite test_threshold_(not_)trigger tests
-From: John Snow <jsnow@redhat.com>
+From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-If users set an unreasonably low speed (like one byte per second), the
+These tests use bdrv_write_threshold_exceeded() API, which is used only
-calculated delay may exceed many hours. While we like to punish users
+for test (since pre-previous commit). Better is testing real API, which
-for asking for stupid things, we do also like to allow users to correct
+is used in block.c as well.
 their wicked ways.
-When a user provides a new speed, kick the job to allow it to recalculate
+So, let's call bdrv_write_threshold_check_write(), and check is
-its delay.
+bs->write_threshold_offset cleared or not (it's cleared iff threshold
 triggered).
-Signed-off-by: John Snow <jsnow@redhat.com>
+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>
-Message-id: 20171213204611.26276-1-jsnow@redhat.com
+Signed-off-by: Max Reitz <mreitz@redhat.com>
 Signed-off-by: Jeff Cody <jcody@redhat.com>
 ---
- blockjob.c | 30 +++++++++++++++++++++++++++++-
+ tests/unit/test-write-threshold.c | 22 ++++------------------
-file changed, 29 insertions(+), 1 deletion(-)
+file changed, 4 insertions(+), 18 deletions(-)
-diff --git a/blockjob.c b/blockjob.c
+diff --git a/tests/unit/test-write-threshold.c b/tests/unit/test-write-threshold.c
 index XXXXXXX..XXXXXXX 100644
---- a/blockjob.c
+--- a/tests/unit/test-write-threshold.c
-+++ b/blockjob.c
++++ b/tests/unit/test-write-threshold.c
-@@ -XXX,XX +XXX,XX @@ static void __attribute__((__constructor__)) block_job_init(void)
+@@ -XXX,XX +XXX,XX @@ static void test_threshold_multi_set_get(void)
- static void block_job_event_cancelled(BlockJob *job);
+ static void test_threshold_not_trigger(void)
- static void block_job_event_completed(BlockJob *job, const char *msg);
+ {
-+static void block_job_enter_cond(BlockJob *job, bool(*fn)(BlockJob *job));
+-    uint64_t amount = 0;
+     uint64_t threshold = 4 * 1024 * 1024;
- /* Transactional group of block jobs */
+     BlockDriverState bs;
- struct BlockJobTxn {
+-    BdrvTrackedRequest req;
-@@ -XXX,XX +XXX,XX @@ static void block_job_completed_txn_success(BlockJob *job)
-     }
+     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);
  }
-+/* Assumes the block_job_mutex is held */
-+static bool block_job_timer_pending(BlockJob *job)
+ static void test_threshold_trigger(void)
 +{
 +    return timer_pending(&job->sleep_timer);
 +}
 +
  void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)
  {
-     Error *local_err = NULL;
+-    uint64_t amount = 0;
-+    int64_t old_speed = job->speed;
+     uint64_t threshold = 4 * 1024 * 1024;
+     BlockDriverState bs;
-     if (!job->driver->set_speed) {
+-    BdrvTrackedRequest req;
-         error_setg(errp, QERR_UNSUPPORTED);
-@@ -XXX,XX +XXX,XX @@ void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)
+     memset(&bs, 0, sizeof(bs));
-     }
+-    memset(&req, 0, sizeof(req));
+-    req.offset = (4 * 1024 * 1024) - 1024;
-     job->speed = speed;
+-    req.bytes = 2 * 1024;
-+    if (speed <= old_speed) {
+-
-+        return;
+-    bdrv_check_request(req.offset, req.bytes, &error_abort);
-+    }
-+
+     bdrv_write_threshold_set(&bs, threshold);
-+    /* kick only if a timer is pending */
+-    amount = bdrv_write_threshold_exceeded(&bs, &req);
-+    block_job_enter_cond(job, block_job_timer_pending);
+-    g_assert_cmpuint(amount, >=, 1024);
 +    bdrv_write_threshold_check_write(&bs, threshold - 1024, 2 * 1024);
 +    g_assert_cmpuint(bdrv_write_threshold_get(&bs), ==, 0);
  }
- void block_job_complete(BlockJob *job, Error **errp)
+ typedef struct TestStruct {
@@ -XXX,XX +XXX,XX @@ void block_job_resume_all(void)
      }
  }
 -void block_job_enter(BlockJob *job)
 +/*
 + * Conditionally enter a block_job pending a call to fn() while
 + * under the block_job_lock critical section.
 + */
 +static void block_job_enter_cond(BlockJob *job, bool(*fn)(BlockJob *job))
  {
      if (!block_job_started(job)) {
          return;
@@ -XXX,XX +XXX,XX @@ void block_job_enter(BlockJob *job)
          return;
      }
 +    if (fn && !fn(job)) {
 +        block_job_unlock();
 +        return;
 +    }
 +
      assert(!job->deferred_to_main_loop);
      timer_del(&job->sleep_timer);
      job->busy = true;
@@ -XXX,XX +XXX,XX @@ void block_job_enter(BlockJob *job)
      aio_co_wake(job->co);
  }
 +void block_job_enter(BlockJob *job)
 +{
 +    block_job_enter_cond(job, NULL);
 +}
 +
  bool block_job_is_cancelled(BlockJob *job)
  {
      return job->cancelled;
 --
-.9.5
+.31.1

-[Qemu-devel] [PULL 02/10] backup: move from done_bitmap to copy_bitmap
+[PULL 16/19] block/write-threshold: drop extra APIs
 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Use HBitmap copy_bitmap instead of done_bitmap. This is needed to
+bdrv_write_threshold_exceeded() is unused.
-improve incremental backup in following patches and to unify backup
-loop for full/incremental modes in future patches.
+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: Jeff Cody <jcody@redhat.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>
-Reviewed-by: John Snow <jsnow@redhat.com>
+[mreitz: Adjusted commit message as per Eric's suggestion]
-Message-id: 20171012135313.227864-3-vsementsov@virtuozzo.com
+Signed-off-by: Max Reitz <mreitz@redhat.com>
 Signed-off-by: Jeff Cody <jcody@redhat.com>
 ---
- block/backup.c | 23 ++++++++++++++---------
+ include/block/write-threshold.h   | 24 ------------------------
-file changed, 14 insertions(+), 9 deletions(-)
+ block/write-threshold.c           | 19 -------------------
  tests/unit/test-write-threshold.c |  4 ----
 files changed, 47 deletions(-)
-diff --git a/block/backup.c b/block/backup.c
+diff --git a/include/block/write-threshold.h b/include/block/write-threshold.h
 index XXXXXXX..XXXXXXX 100644
---- a/block/backup.c
+--- a/include/block/write-threshold.h
-+++ b/block/backup.c
++++ b/include/block/write-threshold.h
-@@ -XXX,XX +XXX,XX @@ typedef struct BackupBlockJob {
+@@ -XXX,XX +XXX,XX @@ void bdrv_write_threshold_set(BlockDriverState *bs, uint64_t threshold_bytes);
-     BlockdevOnError on_target_error;
+  */
-     CoRwlock flush_rwlock;
+ uint64_t bdrv_write_threshold_get(const BlockDriverState *bs);
-     uint64_t bytes_read;
--    unsigned long *done_bitmap;
+-/*
-     int64_t cluster_size;
+- * bdrv_write_threshold_is_set
-     bool compress;
+- *
-     NotifierWithReturn before_write;
+- * Tell if a write threshold is set for a given BDS.
-     QLIST_HEAD(, CowRequest) inflight_reqs;
+- */
-+
+-bool bdrv_write_threshold_is_set(const BlockDriverState *bs);
 +    HBitmap *copy_bitmap;
  } BackupBlockJob;
  /* See if in-flight requests overlap and wait for them to complete */
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
      cow_request_begin(&cow_request, job, start, end);
      for (; start < end; start += job->cluster_size) {
 -        if (test_bit(start / job->cluster_size, job->done_bitmap)) {
 +        if (!hbitmap_get(job->copy_bitmap, start / job->cluster_size)) {
              trace_backup_do_cow_skip(job, start);
              continue; /* already copied */
          }
 +        hbitmap_reset(job->copy_bitmap, start / job->cluster_size, 1);
          trace_backup_do_cow_process(job, start);
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
              if (error_is_read) {
                  *error_is_read = true;
              }
 +            hbitmap_set(job->copy_bitmap, start / job->cluster_size, 1);
              goto out;
          }
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
              if (error_is_read) {
                  *error_is_read = false;
              }
 +            hbitmap_set(job->copy_bitmap, start / job->cluster_size, 1);
              goto out;
          }
 -        set_bit(start / job->cluster_size, job->done_bitmap);
 -
-         /* Publish progress, guest I/O counts as progress too.  Note that the
+-/*
-          * offset field is an opaque progress value, it is not a disk offset.
+- * bdrv_write_threshold_exceeded
-          */
+- *
-@@ -XXX,XX +XXX,XX @@ void backup_do_checkpoint(BlockJob *job, Error **errp)
+- * 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.
-     len = DIV_ROUND_UP(backup_job->common.len, backup_job->cluster_size);
+- *
--    bitmap_zero(backup_job->done_bitmap, len);
+- * NOTE: here we assume the following holds for each request this code
-+    hbitmap_set(backup_job->copy_bitmap, 0, len);
+- * 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;
  }
- void backup_wait_for_overlapping_requests(BlockJob *job, int64_t offset,
+-bool bdrv_write_threshold_is_set(const BlockDriverState *bs)
-@@ -XXX,XX +XXX,XX @@ static void coroutine_fn backup_run(void *opaque)
+-{
-     BackupBlockJob *job = opaque;
+-    return bs->write_threshold_offset > 0;
-     BackupCompleteData *data;
+-}
-     BlockDriverState *bs = blk_bs(job->common.blk);
+-
--    int64_t offset;
+-uint64_t bdrv_write_threshold_exceeded(const BlockDriverState *bs,
-+    int64_t offset, nb_clusters;
+-                                       const BdrvTrackedRequest *req)
-     int ret = 0;
+-{
+-    if (bdrv_write_threshold_is_set(bs)) {
-     QLIST_INIT(&job->inflight_reqs);
+-        if (req->offset > bs->write_threshold_offset) {
-     qemu_co_rwlock_init(&job->flush_rwlock);
+-            return (req->offset - bs->write_threshold_offset) + req->bytes;
+-        }
--    job->done_bitmap = bitmap_new(DIV_ROUND_UP(job->common.len,
+-        if ((req->offset + req->bytes) > bs->write_threshold_offset) {
--                                               job->cluster_size));
+-            return (req->offset + req->bytes) - bs->write_threshold_offset;
-+    nb_clusters = DIV_ROUND_UP(job->common.len, job->cluster_size);
+-        }
-+    job->copy_bitmap = hbitmap_alloc(nb_clusters, 0);
+-    }
-+    hbitmap_set(job->copy_bitmap, 0, nb_clusters);
+-    return 0;
+-}
-     job->before_write.notify = backup_before_write_notify;
+-
-     bdrv_add_before_write_notifier(bs, &job->before_write);
+ void bdrv_write_threshold_set(BlockDriverState *bs, uint64_t threshold_bytes)
+ {
-     if (job->sync_mode == MIRROR_SYNC_MODE_NONE) {
+     bs->write_threshold_offset = threshold_bytes;
-+        /* All bits are set in copy_bitmap to allow any cluster to be copied.
+diff --git a/tests/unit/test-write-threshold.c b/tests/unit/test-write-threshold.c
-+         * This does not actually require them to be copied. */
+index XXXXXXX..XXXXXXX 100644
-         while (!block_job_is_cancelled(&job->common)) {
+--- a/tests/unit/test-write-threshold.c
-             /* Yield until the job is cancelled.  We just let our before_write
++++ b/tests/unit/test-write-threshold.c
-              * notify callback service CoW requests. */
+@@ -XXX,XX +XXX,XX @@ static void test_threshold_not_set_on_init(void)
-@@ -XXX,XX +XXX,XX @@ static void coroutine_fn backup_run(void *opaque)
+     BlockDriverState bs;
-     /* wait until pending backup_do_cow() calls have completed */
+     memset(&bs, 0, sizeof(bs));
-     qemu_co_rwlock_wrlock(&job->flush_rwlock);
-     qemu_co_rwlock_unlock(&job->flush_rwlock);
+-    g_assert(!bdrv_write_threshold_is_set(&bs));
--    g_free(job->done_bitmap);
+-
-+    hbitmap_free(job->copy_bitmap);
+     res = bdrv_write_threshold_get(&bs);
+     g_assert_cmpint(res, ==, 0);
-     data = g_malloc(sizeof(*data));
+ }
-     data->ret = ret;
+@@ -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);
  }
 --
-.9.5
+.31.1

-[Qemu-devel] [PULL 08/10] block/sheepdog: code beautification
+[PULL 17/19] test-write-threshold: drop extra tests
-No functional changes, just whitespace manipulation.
+From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Signed-off-by: Jeff Cody <jcody@redhat.com>
+Testing set/get of one 64bit variable doesn't seem necessary. We have a
-Reviewed-by: Eric Blake <eblake@redhat.com>
+lot of such variables. Also remaining tests do test set/get anyway.
-Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
-Signed-off-by: Jeff Cody <jcody@redhat.com>
+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>
 ---
- block/sheepdog.c | 164 +++++++++++++++++++++++++++----------------------------
+ tests/unit/test-write-threshold.c | 43 -------------------------------
-file changed, 82 insertions(+), 82 deletions(-)
+file changed, 43 deletions(-)
-diff --git a/block/sheepdog.c b/block/sheepdog.c
+diff --git a/tests/unit/test-write-threshold.c b/tests/unit/test-write-threshold.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/sheepdog.c
+--- a/tests/unit/test-write-threshold.c
-+++ b/block/sheepdog.c
++++ b/tests/unit/test-write-threshold.c
-@@ -XXX,XX +XXX,XX @@ typedef struct BDRVSheepdogReopenState {
+@@ -XXX,XX +XXX,XX @@
-     int cache_flags;
+ #include "block/write-threshold.h"
- } BDRVSheepdogReopenState;
--static const char * sd_strerror(int err)
+-static void test_threshold_not_set_on_init(void)
-+static const char *sd_strerror(int err)
+-{
 -    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)
  {
-     int i;
+     uint64_t threshold = 4 * 1024 * 1024;
+@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv)
-@@ -XXX,XX +XXX,XX @@ static QemuOptsList sd_create_opts = {
+ {
- };
+     size_t i;
+     TestStruct tests[] = {
- static BlockDriver bdrv_sheepdog = {
+-        { "/write-threshold/not-set-on-init",
--    .format_name    = "sheepdog",
+-          test_threshold_not_set_on_init },
--    .protocol_name  = "sheepdog",
+-        { "/write-threshold/set-get",
--    .instance_size  = sizeof(BDRVSheepdogState),
+-          test_threshold_set_get },
--    .bdrv_parse_filename    = sd_parse_filename,
+-        { "/write-threshold/multi-set-get",
--    .bdrv_file_open = sd_open,
+-          test_threshold_multi_set_get },
--    .bdrv_reopen_prepare    = sd_reopen_prepare,
+         { "/write-threshold/not-trigger",
--    .bdrv_reopen_commit     = sd_reopen_commit,
+           test_threshold_not_trigger },
--    .bdrv_reopen_abort      = sd_reopen_abort,
+         { "/write-threshold/trigger",
 -    .bdrv_close     = sd_close,
 -    .bdrv_create    = sd_create,
 -    .bdrv_has_zero_init = bdrv_has_zero_init_1,
 -    .bdrv_getlength = sd_getlength,
 +    .format_name                  = "sheepdog",
 +    .protocol_name                = "sheepdog",
 +    .instance_size                = sizeof(BDRVSheepdogState),
 +    .bdrv_parse_filename          = sd_parse_filename,
 +    .bdrv_file_open               = sd_open,
 +    .bdrv_reopen_prepare          = sd_reopen_prepare,
 +    .bdrv_reopen_commit           = sd_reopen_commit,
 +    .bdrv_reopen_abort            = sd_reopen_abort,
 +    .bdrv_close                   = sd_close,
 +    .bdrv_create                  = sd_create,
 +    .bdrv_has_zero_init           = bdrv_has_zero_init_1,
 +    .bdrv_getlength               = sd_getlength,
      .bdrv_get_allocated_file_size = sd_get_allocated_file_size,
 -    .bdrv_truncate  = sd_truncate,
 +    .bdrv_truncate                = sd_truncate,
 -    .bdrv_co_readv  = sd_co_readv,
 -    .bdrv_co_writev = sd_co_writev,
 -    .bdrv_co_flush_to_disk  = sd_co_flush_to_disk,
 -    .bdrv_co_pdiscard = sd_co_pdiscard,
 -    .bdrv_co_get_block_status = sd_co_get_block_status,
 +    .bdrv_co_readv                = sd_co_readv,
 +    .bdrv_co_writev               = sd_co_writev,
 +    .bdrv_co_flush_to_disk        = sd_co_flush_to_disk,
 +    .bdrv_co_pdiscard             = sd_co_pdiscard,
 +    .bdrv_co_get_block_status     = sd_co_get_block_status,
 -    .bdrv_snapshot_create   = sd_snapshot_create,
 -    .bdrv_snapshot_goto     = sd_snapshot_goto,
 -    .bdrv_snapshot_delete   = sd_snapshot_delete,
 -    .bdrv_snapshot_list     = sd_snapshot_list,
 +    .bdrv_snapshot_create         = sd_snapshot_create,
 +    .bdrv_snapshot_goto           = sd_snapshot_goto,
 +    .bdrv_snapshot_delete         = sd_snapshot_delete,
 +    .bdrv_snapshot_list           = sd_snapshot_list,
 -    .bdrv_save_vmstate  = sd_save_vmstate,
 -    .bdrv_load_vmstate  = sd_load_vmstate,
 +    .bdrv_save_vmstate            = sd_save_vmstate,
 +    .bdrv_load_vmstate            = sd_load_vmstate,
 -    .bdrv_detach_aio_context = sd_detach_aio_context,
 -    .bdrv_attach_aio_context = sd_attach_aio_context,
 +    .bdrv_detach_aio_context      = sd_detach_aio_context,
 +    .bdrv_attach_aio_context      = sd_attach_aio_context,
 -    .create_opts    = &sd_create_opts,
 +    .create_opts                  = &sd_create_opts,
  };
  static BlockDriver bdrv_sheepdog_tcp = {
 -    .format_name    = "sheepdog",
 -    .protocol_name  = "sheepdog+tcp",
 -    .instance_size  = sizeof(BDRVSheepdogState),
 -    .bdrv_parse_filename    = sd_parse_filename,
 -    .bdrv_file_open = sd_open,
 -    .bdrv_reopen_prepare    = sd_reopen_prepare,
 -    .bdrv_reopen_commit     = sd_reopen_commit,
 -    .bdrv_reopen_abort      = sd_reopen_abort,
 -    .bdrv_close     = sd_close,
 -    .bdrv_create    = sd_create,
 -    .bdrv_has_zero_init = bdrv_has_zero_init_1,
 -    .bdrv_getlength = sd_getlength,
 +    .format_name                  = "sheepdog",
 +    .protocol_name                = "sheepdog+tcp",
 +    .instance_size                = sizeof(BDRVSheepdogState),
 +    .bdrv_parse_filename          = sd_parse_filename,
 +    .bdrv_file_open               = sd_open,
 +    .bdrv_reopen_prepare          = sd_reopen_prepare,
 +    .bdrv_reopen_commit           = sd_reopen_commit,
 +    .bdrv_reopen_abort            = sd_reopen_abort,
 +    .bdrv_close                   = sd_close,
 +    .bdrv_create                  = sd_create,
 +    .bdrv_has_zero_init           = bdrv_has_zero_init_1,
 +    .bdrv_getlength               = sd_getlength,
      .bdrv_get_allocated_file_size = sd_get_allocated_file_size,
 -    .bdrv_truncate  = sd_truncate,
 +    .bdrv_truncate                = sd_truncate,
 -    .bdrv_co_readv  = sd_co_readv,
 -    .bdrv_co_writev = sd_co_writev,
 -    .bdrv_co_flush_to_disk  = sd_co_flush_to_disk,
 -    .bdrv_co_pdiscard = sd_co_pdiscard,
 -    .bdrv_co_get_block_status = sd_co_get_block_status,
 +    .bdrv_co_readv                = sd_co_readv,
 +    .bdrv_co_writev               = sd_co_writev,
 +    .bdrv_co_flush_to_disk        = sd_co_flush_to_disk,
 +    .bdrv_co_pdiscard             = sd_co_pdiscard,
 +    .bdrv_co_get_block_status     = sd_co_get_block_status,
 -    .bdrv_snapshot_create   = sd_snapshot_create,
 -    .bdrv_snapshot_goto     = sd_snapshot_goto,
 -    .bdrv_snapshot_delete   = sd_snapshot_delete,
 -    .bdrv_snapshot_list     = sd_snapshot_list,
 +    .bdrv_snapshot_create         = sd_snapshot_create,
 +    .bdrv_snapshot_goto           = sd_snapshot_goto,
 +    .bdrv_snapshot_delete         = sd_snapshot_delete,
 +    .bdrv_snapshot_list           = sd_snapshot_list,
 -    .bdrv_save_vmstate  = sd_save_vmstate,
 -    .bdrv_load_vmstate  = sd_load_vmstate,
 +    .bdrv_save_vmstate            = sd_save_vmstate,
 +    .bdrv_load_vmstate            = sd_load_vmstate,
 -    .bdrv_detach_aio_context = sd_detach_aio_context,
 -    .bdrv_attach_aio_context = sd_attach_aio_context,
 +    .bdrv_detach_aio_context      = sd_detach_aio_context,
 +    .bdrv_attach_aio_context      = sd_attach_aio_context,
 -    .create_opts    = &sd_create_opts,
 +    .create_opts                  = &sd_create_opts,
  };
  static BlockDriver bdrv_sheepdog_unix = {
 -    .format_name    = "sheepdog",
 -    .protocol_name  = "sheepdog+unix",
 -    .instance_size  = sizeof(BDRVSheepdogState),
 -    .bdrv_parse_filename    = sd_parse_filename,
 -    .bdrv_file_open = sd_open,
 -    .bdrv_reopen_prepare    = sd_reopen_prepare,
 -    .bdrv_reopen_commit     = sd_reopen_commit,
 -    .bdrv_reopen_abort      = sd_reopen_abort,
 -    .bdrv_close     = sd_close,
 -    .bdrv_create    = sd_create,
 -    .bdrv_has_zero_init = bdrv_has_zero_init_1,
 -    .bdrv_getlength = sd_getlength,
 +    .format_name                  = "sheepdog",
 +    .protocol_name                = "sheepdog+unix",
 +    .instance_size                = sizeof(BDRVSheepdogState),
 +    .bdrv_parse_filename          = sd_parse_filename,
 +    .bdrv_file_open               = sd_open,
 +    .bdrv_reopen_prepare          = sd_reopen_prepare,
 +    .bdrv_reopen_commit           = sd_reopen_commit,
 +    .bdrv_reopen_abort            = sd_reopen_abort,
 +    .bdrv_close                   = sd_close,
 +    .bdrv_create                  = sd_create,
 +    .bdrv_has_zero_init           = bdrv_has_zero_init_1,
 +    .bdrv_getlength               = sd_getlength,
      .bdrv_get_allocated_file_size = sd_get_allocated_file_size,
 -    .bdrv_truncate  = sd_truncate,
 +    .bdrv_truncate                = sd_truncate,
 -    .bdrv_co_readv  = sd_co_readv,
 -    .bdrv_co_writev = sd_co_writev,
 -    .bdrv_co_flush_to_disk  = sd_co_flush_to_disk,
 -    .bdrv_co_pdiscard = sd_co_pdiscard,
 -    .bdrv_co_get_block_status = sd_co_get_block_status,
 +    .bdrv_co_readv                = sd_co_readv,
 +    .bdrv_co_writev               = sd_co_writev,
 +    .bdrv_co_flush_to_disk        = sd_co_flush_to_disk,
 +    .bdrv_co_pdiscard             = sd_co_pdiscard,
 +    .bdrv_co_get_block_status     = sd_co_get_block_status,
 -    .bdrv_snapshot_create   = sd_snapshot_create,
 -    .bdrv_snapshot_goto     = sd_snapshot_goto,
 -    .bdrv_snapshot_delete   = sd_snapshot_delete,
 -    .bdrv_snapshot_list     = sd_snapshot_list,
 +    .bdrv_snapshot_create         = sd_snapshot_create,
 +    .bdrv_snapshot_goto           = sd_snapshot_goto,
 +    .bdrv_snapshot_delete         = sd_snapshot_delete,
 +    .bdrv_snapshot_list           = sd_snapshot_list,
 -    .bdrv_save_vmstate  = sd_save_vmstate,
 -    .bdrv_load_vmstate  = sd_load_vmstate,
 +    .bdrv_save_vmstate            = sd_save_vmstate,
 +    .bdrv_load_vmstate            = sd_load_vmstate,
 -    .bdrv_detach_aio_context = sd_detach_aio_context,
 -    .bdrv_attach_aio_context = sd_attach_aio_context,
 +    .bdrv_detach_aio_context      = sd_detach_aio_context,
 +    .bdrv_attach_aio_context      = sd_attach_aio_context,
 -    .create_opts    = &sd_create_opts,
 +    .create_opts                  = &sd_create_opts,
  };
  static void bdrv_sheepdog_init(void)
 --
-.9.5
+.31.1

-[Qemu-devel] [PULL 01/10] hbitmap: add next_zero function
+[PULL 18/19] test-write-threshold: drop extra TestStruct structure
 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-The function searches for next zero bit.
+We don't need this extra logic: it doesn't make code simpler.
 Also add interface for BdrvDirtyBitmap and unit test.
 Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Reviewed-by: John Snow <jsnow@redhat.com>
+Reviewed-by: Max Reitz <mreitz@redhat.com>
-Message-id: 20171012135313.227864-2-vsementsov@virtuozzo.com
+Message-Id: <20210506090621.11848-8-vsementsov@virtuozzo.com>
-Signed-off-by: Jeff Cody <jcody@redhat.com>
+Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
- block/dirty-bitmap.c         |  5 ++++
+ tests/unit/test-write-threshold.c | 20 +++-----------------
- include/block/dirty-bitmap.h |  1 +
+file changed, 3 insertions(+), 17 deletions(-)
  include/qemu/hbitmap.h       |  8 ++++++
  tests/test-hbitmap.c         | 61 ++++++++++++++++++++++++++++++++++++++++++++
  util/hbitmap.c               | 39 ++++++++++++++++++++++++++++
 files changed, 114 insertions(+)
-diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
+diff --git a/tests/unit/test-write-threshold.c b/tests/unit/test-write-threshold.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/dirty-bitmap.c
+--- a/tests/unit/test-write-threshold.c
-+++ b/block/dirty-bitmap.c
++++ b/tests/unit/test-write-threshold.c
-@@ -XXX,XX +XXX,XX @@ char *bdrv_dirty_bitmap_sha256(const BdrvDirtyBitmap *bitmap, Error **errp)
+@@ -XXX,XX +XXX,XX @@ static void test_threshold_trigger(void)
- {
+     g_assert_cmpuint(bdrv_write_threshold_get(&bs), ==, 0);
      return hbitmap_sha256(bitmap->bitmap, errp);
  }
-+
-+int64_t bdrv_dirty_bitmap_next_zero(BdrvDirtyBitmap *bitmap, uint64_t offset)
+-typedef struct TestStruct {
-+{
+-    const char *name;
-+    return hbitmap_next_zero(bitmap->bitmap, offset);
+-    void (*func)(void);
-+}
+-} TestStruct;
-diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
+-
-index XXXXXXX..XXXXXXX 100644
 --- a/include/block/dirty-bitmap.h
 +++ b/include/block/dirty-bitmap.h
@@ -XXX,XX +XXX,XX @@ bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs);
  BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs,
                                          BdrvDirtyBitmap *bitmap);
  char *bdrv_dirty_bitmap_sha256(const BdrvDirtyBitmap *bitmap, Error **errp);
 +int64_t bdrv_dirty_bitmap_next_zero(BdrvDirtyBitmap *bitmap, uint64_t start);
  #endif
 diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
 index XXXXXXX..XXXXXXX 100644
 --- a/include/qemu/hbitmap.h
 +++ b/include/qemu/hbitmap.h
@@ -XXX,XX +XXX,XX @@ void hbitmap_iter_init(HBitmapIter *hbi, const HBitmap *hb, uint64_t first);
   */
  unsigned long hbitmap_iter_skip_words(HBitmapIter *hbi);
 +/* hbitmap_next_zero:
 + * @hb: The HBitmap to operate on
 + * @start: The bit to start from.
 + *
 + * Find next not dirty bit.
 + */
 +int64_t hbitmap_next_zero(const HBitmap *hb, uint64_t start);
 +
  /* hbitmap_create_meta:
   * Create a "meta" hbitmap to track dirtiness of the bits in this HBitmap.
   * The caller owns the created bitmap and must call hbitmap_free_meta(hb) to
 diff --git a/tests/test-hbitmap.c b/tests/test-hbitmap.c
 index XXXXXXX..XXXXXXX 100644
 --- a/tests/test-hbitmap.c
 +++ b/tests/test-hbitmap.c
@@ -XXX,XX +XXX,XX @@ static void test_hbitmap_iter_and_reset(TestHBitmapData *data,
      hbitmap_iter_next(&hbi);
  }
 +static void test_hbitmap_next_zero_check(TestHBitmapData *data, int64_t start)
 +{
 +    int64_t ret1 = hbitmap_next_zero(data->hb, start);
 +    int64_t ret2 = start;
 +    for ( ; ret2 < data->size && hbitmap_get(data->hb, ret2); ret2++) {
 +        ;
 +    }
 +    if (ret2 == data->size) {
 +        ret2 = -1;
 +    }
 +
 +    g_assert_cmpint(ret1, ==, ret2);
 +}
 +
 +static void test_hbitmap_next_zero_do(TestHBitmapData *data, int granularity)
 +{
 +    hbitmap_test_init(data, L3, granularity);
 +    test_hbitmap_next_zero_check(data, 0);
 +    test_hbitmap_next_zero_check(data, L3 - 1);
 +
 +    hbitmap_set(data->hb, L2, 1);
 +    test_hbitmap_next_zero_check(data, 0);
 +    test_hbitmap_next_zero_check(data, L2 - 1);
 +    test_hbitmap_next_zero_check(data, L2);
 +    test_hbitmap_next_zero_check(data, L2 + 1);
 +
 +    hbitmap_set(data->hb, L2 + 5, L1);
 +    test_hbitmap_next_zero_check(data, 0);
 +    test_hbitmap_next_zero_check(data, L2 + 1);
 +    test_hbitmap_next_zero_check(data, L2 + 2);
 +    test_hbitmap_next_zero_check(data, L2 + 5);
 +    test_hbitmap_next_zero_check(data, L2 + L1 - 1);
 +    test_hbitmap_next_zero_check(data, L2 + L1);
 +
 +    hbitmap_set(data->hb, L2 * 2, L3 - L2 * 2);
 +    test_hbitmap_next_zero_check(data, L2 * 2 - L1);
 +    test_hbitmap_next_zero_check(data, L2 * 2 - 2);
 +    test_hbitmap_next_zero_check(data, L2 * 2 - 1);
 +    test_hbitmap_next_zero_check(data, L2 * 2);
 +    test_hbitmap_next_zero_check(data, L3 - 1);
 +
 +    hbitmap_set(data->hb, 0, L3);
 +    test_hbitmap_next_zero_check(data, 0);
 +}
 +
 +static void test_hbitmap_next_zero_0(TestHBitmapData *data, const void *unused)
 +{
 +    test_hbitmap_next_zero_do(data, 0);
 +}
 +
 +static void test_hbitmap_next_zero_4(TestHBitmapData *data, const void *unused)
 +{
 +    test_hbitmap_next_zero_do(data, 4);
 +}
 +
  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);
-@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv)
+-    for (i = 0; tests[i].name != NULL; i++) {
+-        g_test_add_func(tests[i].name, tests[i].func);
-     hbitmap_test_add("/hbitmap/iter/iter_and_reset",
+-    }
-                      test_hbitmap_iter_and_reset);
++    g_test_add_func("/write-threshold/not-trigger", test_threshold_not_trigger);
 +    g_test_add_func("/write-threshold/trigger", test_threshold_trigger);
 +
-+    hbitmap_test_add("/hbitmap/next_zero/next_zero_0",
+     return g_test_run();
 +                     test_hbitmap_next_zero_0);
 +    hbitmap_test_add("/hbitmap/next_zero/next_zero_4",
 +                     test_hbitmap_next_zero_4);
 +
      g_test_run();
      return 0;
 diff --git a/util/hbitmap.c b/util/hbitmap.c
 index XXXXXXX..XXXXXXX 100644
 --- a/util/hbitmap.c
 +++ b/util/hbitmap.c
@@ -XXX,XX +XXX,XX @@ void hbitmap_iter_init(HBitmapIter *hbi, const HBitmap *hb, uint64_t first)
      }
  }
-+int64_t hbitmap_next_zero(const HBitmap *hb, uint64_t start)
-+{
-+    size_t pos = (start >> hb->granularity) >> BITS_PER_LEVEL;
-+    unsigned long *last_lev = hb->levels[HBITMAP_LEVELS - 1];
-+    uint64_t sz = hb->sizes[HBITMAP_LEVELS - 1];
-+    unsigned long cur = last_lev[pos];
-+    unsigned start_bit_offset =
-+            (start >> hb->granularity) & (BITS_PER_LONG - 1);
-+    int64_t res;
-+
-+    cur |= (1UL << start_bit_offset) - 1;
-+    assert((start >> hb->granularity) < hb->size);
-+
-+    if (cur == (unsigned long)-1) {
-+        do {
-+            pos++;
-+        } while (pos < sz && last_lev[pos] == (unsigned long)-1);
-+
-+        if (pos >= sz) {
-+            return -1;
-+        }
-+
-+        cur = last_lev[pos];
-+    }
-+
-+    res = (pos << BITS_PER_LEVEL) + ctol(cur);
-+    if (res >= hb->size) {
-+        return -1;
-+    }
-+
-+    res = res << hb->granularity;
-+    if (res < start) {
-+        assert(((start - res) >> hb->granularity) == 0);
-+        return start;
-+    }
-+
-+    return res;
-+}
-+
- bool hbitmap_empty(const HBitmap *hb)
- {
-     return hb->count == 0;
 --
-.9.5
+.31.1

-[Qemu-devel] [PULL 05/10] backup: use copy_bitmap in incremental backup
+[PULL 19/19] write-threshold: deal with includes
 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-We can use copy_bitmap instead of sync_bitmap. copy_bitmap is
+"qemu/typedefs.h" is enough for include/block/write-threshold.h header
-initialized from sync_bitmap and it is more informative: we will not try
+with forward declaration of BlockDriverState. Also drop extra includes
-to process data, that is already in progress (by write notifier).
+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>
-Reviewed-by: John Snow <jsnow@redhat.com>
+Signed-off-by: Max Reitz <mreitz@redhat.com>
 Reviewed-by: Jeff Cody <jcody@redhat.com>
 Message-id: 20171012135313.227864-6-vsementsov@virtuozzo.com
 Signed-off-by: Jeff Cody <jcody@redhat.com>
 ---
- block/backup.c | 55 +++++++++++++++++--------------------------------------
+ include/block/write-threshold.h   | 2 +-
-file changed, 17 insertions(+), 38 deletions(-)
+ block/write-threshold.c           | 2 --
  tests/unit/test-write-threshold.c | 1 -
 files changed, 1 insertion(+), 4 deletions(-)
-diff --git a/block/backup.c b/block/backup.c
+diff --git a/include/block/write-threshold.h b/include/block/write-threshold.h
 index XXXXXXX..XXXXXXX 100644
---- a/block/backup.c
+--- a/include/block/write-threshold.h
-+++ b/block/backup.c
++++ b/include/block/write-threshold.h
-@@ -XXX,XX +XXX,XX @@ static bool coroutine_fn yield_and_check(BackupBlockJob *job)
+@@ -XXX,XX +XXX,XX @@
+ #ifndef BLOCK_WRITE_THRESHOLD_H
- static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
+ #define BLOCK_WRITE_THRESHOLD_H
- {
-+    int ret;
+-#include "block/block_int.h"
-     bool error_is_read;
++#include "qemu/typedefs.h"
--    int ret = 0;
--    int clusters_per_iter;
+ /*
--    uint32_t granularity;
+  * bdrv_write_threshold_set:
--    int64_t offset;
+diff --git a/block/write-threshold.c b/block/write-threshold.c
-     int64_t cluster;
+index XXXXXXX..XXXXXXX 100644
--    int64_t end;
+--- a/block/write-threshold.c
--    BdrvDirtyBitmapIter *dbi;
++++ b/block/write-threshold.c
-+    HBitmapIter hbi;
+@@ -XXX,XX +XXX,XX @@
--    granularity = bdrv_dirty_bitmap_granularity(job->sync_bitmap);
+ #include "qemu/osdep.h"
--    clusters_per_iter = MAX((granularity / job->cluster_size), 1);
+ #include "block/block_int.h"
--    dbi = bdrv_dirty_iter_new(job->sync_bitmap);
+-#include "qemu/coroutine.h"
--
+ #include "block/write-threshold.h"
--    /* Find the next dirty sector(s) */
+-#include "qemu/notify.h"
--    while ((offset = bdrv_dirty_iter_next(dbi)) >= 0) {
+ #include "qapi/error.h"
--        cluster = offset / job->cluster_size;
+ #include "qapi/qapi-commands-block-core.h"
--
+ #include "qapi/qapi-events-block-core.h"
--        for (end = cluster + clusters_per_iter; cluster < end; cluster++) {
+diff --git a/tests/unit/test-write-threshold.c b/tests/unit/test-write-threshold.c
--            do {
+index XXXXXXX..XXXXXXX 100644
--                if (yield_and_check(job)) {
+--- a/tests/unit/test-write-threshold.c
--                    goto out;
++++ b/tests/unit/test-write-threshold.c
--                }
+@@ -XXX,XX +XXX,XX @@
--                ret = backup_do_cow(job, cluster * job->cluster_size,
+  */
--                                    job->cluster_size, &error_is_read,
--                                    false);
+ #include "qemu/osdep.h"
--                if ((ret < 0) &&
+-#include "qapi/error.h"
--                    backup_error_action(job, error_is_read, -ret) ==
+ #include "block/block_int.h"
--                    BLOCK_ERROR_ACTION_REPORT) {
+ #include "block/write-threshold.h"
--                    goto out;
 -                }
 -            } while (ret < 0);
 -        }
 -
 -        /* If the bitmap granularity is smaller than the backup granularity,
 -         * we need to advance the iterator pointer to the next cluster. */
 -        if (granularity < job->cluster_size) {
 -            bdrv_set_dirty_iter(dbi, cluster * job->cluster_size);
 -        }
 +    hbitmap_iter_init(&hbi, job->copy_bitmap, 0);
 +    while ((cluster = hbitmap_iter_next(&hbi)) != -1) {
 +        do {
 +            if (yield_and_check(job)) {
 +                return 0;
 +            }
 +            ret = backup_do_cow(job, cluster * job->cluster_size,
 +                                job->cluster_size, &error_is_read, false);
 +            if (ret < 0 && backup_error_action(job, error_is_read, -ret) ==
 +                           BLOCK_ERROR_ACTION_REPORT)
 +            {
 +                return ret;
 +            }
 +        } while (ret < 0);
      }
 -out:
 -    bdrv_dirty_iter_free(dbi);
 -    return ret;
 +    return 0;
  }
  /* init copy_bitmap from sync_bitmap */
 --
-.9.5
+.31.1

The following changes since commit 411ad78115ebeb3411cf4b7622784b93dfabe259:

Merge remote-tracking branch 'remotes/stefanberger/tags/pull-tpm-2017-12-15-1' into staging (2017-12-17 15:27:41 +0000)

are available in the git repository at:

git://github.com/codyprime/qemu-kvm-jtc.git tags/block-pull-request

for you to fetch changes up to 996922de45299878cdc4c15b72b19edf2bc618a4:

block/curl: fix minor memory leaks (2017-12-18 15:44:39 -0500)

----------------------------------------------------------------
Blockjob and protocol patches
----------------------------------------------------------------

Jeff Cody (4):
  block/sheepdog: remove spurious NULL check
  block/sheepdog: code beautification
  block/curl: check error return of curl_global_init()
  block/curl: fix minor memory leaks

John Snow (1):
  blockjob: kick jobs on set-speed

Vladimir Sementsov-Ogievskiy (5):
  hbitmap: add next_zero function
  backup: move from done_bitmap to copy_bitmap
  backup: init copy_bitmap from sync_bitmap for incremental
  backup: simplify non-dirty bits progress processing
  backup: use copy_bitmap in incremental backup

-- 
2.9.5

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

The function searches for next zero bit.
Also add interface for BdrvDirtyBitmap and unit test.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 20171012135313.227864-2-vsementsov@virtuozzo.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/dirty-bitmap.c         |  5 ++++
 include/block/dirty-bitmap.h |  1 +
 include/qemu/hbitmap.h       |  8 ++++++
 tests/test-hbitmap.c         | 61 ++++++++++++++++++++++++++++++++++++++++++++
 util/hbitmap.c               | 39 ++++++++++++++++++++++++++++
 5 files changed, 114 insertions(+)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index XXXXXXX..XXXXXXX 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -XXX,XX +XXX,XX @@ char *bdrv_dirty_bitmap_sha256(const BdrvDirtyBitmap *bitmap, Error **errp)
 {
     return hbitmap_sha256(bitmap->bitmap, errp);
 }
+
+int64_t bdrv_dirty_bitmap_next_zero(BdrvDirtyBitmap *bitmap, uint64_t offset)
+{
+    return hbitmap_next_zero(bitmap->bitmap, offset);
+}
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -XXX,XX +XXX,XX @@ bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs);
 BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs,
                                         BdrvDirtyBitmap *bitmap);
 char *bdrv_dirty_bitmap_sha256(const BdrvDirtyBitmap *bitmap, Error **errp);
+int64_t bdrv_dirty_bitmap_next_zero(BdrvDirtyBitmap *bitmap, uint64_t start);
 
 #endif
diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index XXXXXXX..XXXXXXX 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -XXX,XX +XXX,XX @@ void hbitmap_iter_init(HBitmapIter *hbi, const HBitmap *hb, uint64_t first);
  */
 unsigned long hbitmap_iter_skip_words(HBitmapIter *hbi);
 
+/* hbitmap_next_zero:
+ * @hb: The HBitmap to operate on
+ * @start: The bit to start from.
+ *
+ * Find next not dirty bit.
+ */
+int64_t hbitmap_next_zero(const HBitmap *hb, uint64_t start);
+
 /* hbitmap_create_meta:
  * Create a "meta" hbitmap to track dirtiness of the bits in this HBitmap.
  * The caller owns the created bitmap and must call hbitmap_free_meta(hb) to
diff --git a/tests/test-hbitmap.c b/tests/test-hbitmap.c
index XXXXXXX..XXXXXXX 100644
--- a/tests/test-hbitmap.c
+++ b/tests/test-hbitmap.c
@@ -XXX,XX +XXX,XX @@ static void test_hbitmap_iter_and_reset(TestHBitmapData *data,
     hbitmap_iter_next(&hbi);
 }
 
+static void test_hbitmap_next_zero_check(TestHBitmapData *data, int64_t start)
+{
+    int64_t ret1 = hbitmap_next_zero(data->hb, start);
+    int64_t ret2 = start;
+    for ( ; ret2 < data->size && hbitmap_get(data->hb, ret2); ret2++) {
+        ;
+    }
+    if (ret2 == data->size) {
+        ret2 = -1;
+    }
+
+    g_assert_cmpint(ret1, ==, ret2);
+}
+
+static void test_hbitmap_next_zero_do(TestHBitmapData *data, int granularity)
+{
+    hbitmap_test_init(data, L3, granularity);
+    test_hbitmap_next_zero_check(data, 0);
+    test_hbitmap_next_zero_check(data, L3 - 1);
+
+    hbitmap_set(data->hb, L2, 1);
+    test_hbitmap_next_zero_check(data, 0);
+    test_hbitmap_next_zero_check(data, L2 - 1);
+    test_hbitmap_next_zero_check(data, L2);
+    test_hbitmap_next_zero_check(data, L2 + 1);
+
+    hbitmap_set(data->hb, L2 + 5, L1);
+    test_hbitmap_next_zero_check(data, 0);
+    test_hbitmap_next_zero_check(data, L2 + 1);
+    test_hbitmap_next_zero_check(data, L2 + 2);
+    test_hbitmap_next_zero_check(data, L2 + 5);
+    test_hbitmap_next_zero_check(data, L2 + L1 - 1);
+    test_hbitmap_next_zero_check(data, L2 + L1);
+
+    hbitmap_set(data->hb, L2 * 2, L3 - L2 * 2);
+    test_hbitmap_next_zero_check(data, L2 * 2 - L1);
+    test_hbitmap_next_zero_check(data, L2 * 2 - 2);
+    test_hbitmap_next_zero_check(data, L2 * 2 - 1);
+    test_hbitmap_next_zero_check(data, L2 * 2);
+    test_hbitmap_next_zero_check(data, L3 - 1);
+
+    hbitmap_set(data->hb, 0, L3);
+    test_hbitmap_next_zero_check(data, 0);
+}
+
+static void test_hbitmap_next_zero_0(TestHBitmapData *data, const void *unused)
+{
+    test_hbitmap_next_zero_do(data, 0);
+}
+
+static void test_hbitmap_next_zero_4(TestHBitmapData *data, const void *unused)
+{
+    test_hbitmap_next_zero_do(data, 4);
+}
+
 int main(int argc, char **argv)
 {
     g_test_init(&argc, &argv, NULL);
@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv)
 
     hbitmap_test_add("/hbitmap/iter/iter_and_reset",
                      test_hbitmap_iter_and_reset);
+
+    hbitmap_test_add("/hbitmap/next_zero/next_zero_0",
+                     test_hbitmap_next_zero_0);
+    hbitmap_test_add("/hbitmap/next_zero/next_zero_4",
+                     test_hbitmap_next_zero_4);
+
     g_test_run();
 
     return 0;
diff --git a/util/hbitmap.c b/util/hbitmap.c
index XXXXXXX..XXXXXXX 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -XXX,XX +XXX,XX @@ void hbitmap_iter_init(HBitmapIter *hbi, const HBitmap *hb, uint64_t first)
     }
 }
 
+int64_t hbitmap_next_zero(const HBitmap *hb, uint64_t start)
+{
+    size_t pos = (start >> hb->granularity) >> BITS_PER_LEVEL;
+    unsigned long *last_lev = hb->levels[HBITMAP_LEVELS - 1];
+    uint64_t sz = hb->sizes[HBITMAP_LEVELS - 1];
+    unsigned long cur = last_lev[pos];
+    unsigned start_bit_offset =
+            (start >> hb->granularity) & (BITS_PER_LONG - 1);
+    int64_t res;
+
+    cur |= (1UL << start_bit_offset) - 1;
+    assert((start >> hb->granularity) < hb->size);
+
+    if (cur == (unsigned long)-1) {
+        do {
+            pos++;
+        } while (pos < sz && last_lev[pos] == (unsigned long)-1);
+
+        if (pos >= sz) {
+            return -1;
+        }
+
+        cur = last_lev[pos];
+    }
+
+    res = (pos << BITS_PER_LEVEL) + ctol(cur);
+    if (res >= hb->size) {
+        return -1;
+    }
+
+    res = res << hb->granularity;
+    if (res < start) {
+        assert(((start - res) >> hb->granularity) == 0);
+        return start;
+    }
+
+    return res;
+}
+
 bool hbitmap_empty(const HBitmap *hb)
 {
     return hb->count == 0;
-- 
2.9.5

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

Use HBitmap copy_bitmap instead of done_bitmap. This is needed to
improve incremental backup in following patches and to unify backup
loop for full/incremental modes in future patches.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 20171012135313.227864-3-vsementsov@virtuozzo.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/backup.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index XXXXXXX..XXXXXXX 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -XXX,XX +XXX,XX @@ typedef struct BackupBlockJob {
     BlockdevOnError on_target_error;
     CoRwlock flush_rwlock;
     uint64_t bytes_read;
-    unsigned long *done_bitmap;
     int64_t cluster_size;
     bool compress;
     NotifierWithReturn before_write;
     QLIST_HEAD(, CowRequest) inflight_reqs;
+
+    HBitmap *copy_bitmap;
 } BackupBlockJob;
 
 /* See if in-flight requests overlap and wait for them to complete */
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
     cow_request_begin(&cow_request, job, start, end);
 
     for (; start < end; start += job->cluster_size) {
-        if (test_bit(start / job->cluster_size, job->done_bitmap)) {
+        if (!hbitmap_get(job->copy_bitmap, start / job->cluster_size)) {
             trace_backup_do_cow_skip(job, start);
             continue; /* already copied */
         }
+        hbitmap_reset(job->copy_bitmap, start / job->cluster_size, 1);
 
         trace_backup_do_cow_process(job, start);
 
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
             if (error_is_read) {
                 *error_is_read = true;
             }
+            hbitmap_set(job->copy_bitmap, start / job->cluster_size, 1);
             goto out;
         }
 
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
             if (error_is_read) {
                 *error_is_read = false;
             }
+            hbitmap_set(job->copy_bitmap, start / job->cluster_size, 1);
             goto out;
         }
 
-        set_bit(start / job->cluster_size, job->done_bitmap);
-
         /* Publish progress, guest I/O counts as progress too.  Note that the
          * offset field is an opaque progress value, it is not a disk offset.
          */
@@ -XXX,XX +XXX,XX @@ void backup_do_checkpoint(BlockJob *job, Error **errp)
     }
 
     len = DIV_ROUND_UP(backup_job->common.len, backup_job->cluster_size);
-    bitmap_zero(backup_job->done_bitmap, len);
+    hbitmap_set(backup_job->copy_bitmap, 0, len);
 }
 
 void backup_wait_for_overlapping_requests(BlockJob *job, int64_t offset,
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn backup_run(void *opaque)
     BackupBlockJob *job = opaque;
     BackupCompleteData *data;
     BlockDriverState *bs = blk_bs(job->common.blk);
-    int64_t offset;
+    int64_t offset, nb_clusters;
     int ret = 0;
 
     QLIST_INIT(&job->inflight_reqs);
     qemu_co_rwlock_init(&job->flush_rwlock);
 
-    job->done_bitmap = bitmap_new(DIV_ROUND_UP(job->common.len,
-                                               job->cluster_size));
+    nb_clusters = DIV_ROUND_UP(job->common.len, job->cluster_size);
+    job->copy_bitmap = hbitmap_alloc(nb_clusters, 0);
+    hbitmap_set(job->copy_bitmap, 0, nb_clusters);
 
     job->before_write.notify = backup_before_write_notify;
     bdrv_add_before_write_notifier(bs, &job->before_write);
 
     if (job->sync_mode == MIRROR_SYNC_MODE_NONE) {
+        /* All bits are set in copy_bitmap to allow any cluster to be copied.
+         * This does not actually require them to be copied. */
         while (!block_job_is_cancelled(&job->common)) {
             /* Yield until the job is cancelled.  We just let our before_write
              * notify callback service CoW requests. */
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn backup_run(void *opaque)
     /* wait until pending backup_do_cow() calls have completed */
     qemu_co_rwlock_wrlock(&job->flush_rwlock);
     qemu_co_rwlock_unlock(&job->flush_rwlock);
-    g_free(job->done_bitmap);
+    hbitmap_free(job->copy_bitmap);
 
     data = g_malloc(sizeof(*data));
     data->ret = ret;
-- 
2.9.5

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

We should not copy non-dirty clusters in write notifiers. So,
initialize copy_bitmap from sync_bitmap.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20171012135313.227864-4-vsementsov@virtuozzo.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/backup.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

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 @@ out:
     return ret;
 }
 
+/* init copy_bitmap from sync_bitmap */
+static void backup_incremental_init_copy_bitmap(BackupBlockJob *job)
+{
+    BdrvDirtyBitmapIter *dbi;
+    int64_t offset;
+    int64_t end = DIV_ROUND_UP(bdrv_dirty_bitmap_size(job->sync_bitmap),
+                               job->cluster_size);
+
+    dbi = bdrv_dirty_iter_new(job->sync_bitmap);
+    while ((offset = bdrv_dirty_iter_next(dbi)) != -1) {
+        int64_t cluster = offset / job->cluster_size;
+        int64_t next_cluster;
+
+        offset += bdrv_dirty_bitmap_granularity(job->sync_bitmap);
+        if (offset >= bdrv_dirty_bitmap_size(job->sync_bitmap)) {
+            hbitmap_set(job->copy_bitmap, cluster, end - cluster);
+            break;
+        }
+
+        offset = bdrv_dirty_bitmap_next_zero(job->sync_bitmap, offset);
+        if (offset == -1) {
+            hbitmap_set(job->copy_bitmap, cluster, end - cluster);
+            break;
+        }
+
+        next_cluster = DIV_ROUND_UP(offset, job->cluster_size);
+        hbitmap_set(job->copy_bitmap, cluster, next_cluster - cluster);
+        if (next_cluster >= end) {
+            break;
+        }
+
+        bdrv_set_dirty_iter(dbi, next_cluster * job->cluster_size);
+    }
+
+    bdrv_dirty_iter_free(dbi);
+}
+
 static void coroutine_fn backup_run(void *opaque)
 {
     BackupBlockJob *job = opaque;
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn backup_run(void *opaque)
 
     nb_clusters = DIV_ROUND_UP(job->common.len, job->cluster_size);
     job->copy_bitmap = hbitmap_alloc(nb_clusters, 0);
-    hbitmap_set(job->copy_bitmap, 0, nb_clusters);
+    if (job->sync_mode == MIRROR_SYNC_MODE_INCREMENTAL) {
+        backup_incremental_init_copy_bitmap(job);
+    } else {
+        hbitmap_set(job->copy_bitmap, 0, nb_clusters);
+    }
+
 
     job->before_write.notify = backup_before_write_notify;
     bdrv_add_before_write_notifier(bs, &job->before_write);
-- 
2.9.5

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

Set fake progress for non-dirty clusters in copy_bitmap initialization,
to. It simplifies code and allows further refactoring.

This patch changes user's view of backup progress, but formally it
doesn't changed: progress hops are just moved to the beginning.

Actually it's just a point of view: when do we actually skip clusters?
We can say in the very beginning, that we skip these clusters and do
not think about them later.

Of course, if go through disk sequentially, it's logical to say, that
we skip clusters between copied portions to the left and to the right
of them. But even now copying progress is not sequential because of
write notifiers. Future patches will introduce new backup architecture
which will do copying in several coroutines in parallel, so it will
make no sense to publish fake progress by parts in parallel with
other copying requests.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Message-id: 20171012135313.227864-5-vsementsov@virtuozzo.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/backup.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index XXXXXXX..XXXXXXX 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
     int64_t offset;
     int64_t cluster;
     int64_t end;
-    int64_t last_cluster = -1;
     BdrvDirtyBitmapIter *dbi;
 
     granularity = bdrv_dirty_bitmap_granularity(job->sync_bitmap);
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
     while ((offset = bdrv_dirty_iter_next(dbi)) >= 0) {
         cluster = offset / job->cluster_size;
 
-        /* Fake progress updates for any clusters we skipped */
-        if (cluster != last_cluster + 1) {
-            job->common.offset += ((cluster - last_cluster - 1) *
-                                   job->cluster_size);
-        }
-
         for (end = cluster + clusters_per_iter; cluster < end; cluster++) {
             do {
                 if (yield_and_check(job)) {
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
         if (granularity < job->cluster_size) {
             bdrv_set_dirty_iter(dbi, cluster * job->cluster_size);
         }
-
-        last_cluster = cluster - 1;
-    }
-
-    /* Play some final catchup with the progress meter */
-    end = DIV_ROUND_UP(job->common.len, job->cluster_size);
-    if (last_cluster + 1 < end) {
-        job->common.offset += ((end - last_cluster - 1) * job->cluster_size);
     }
 
 out:
@@ -XXX,XX +XXX,XX @@ static void backup_incremental_init_copy_bitmap(BackupBlockJob *job)
         bdrv_set_dirty_iter(dbi, next_cluster * job->cluster_size);
     }
 
+    job->common.offset = job->common.len -
+                         hbitmap_count(job->copy_bitmap) * job->cluster_size;
+
     bdrv_dirty_iter_free(dbi);
 }
 
-- 
2.9.5

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

We can use copy_bitmap instead of sync_bitmap. copy_bitmap is
initialized from sync_bitmap and it is more informative: we will not try
to process data, that is already in progress (by write notifier).

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Message-id: 20171012135313.227864-6-vsementsov@virtuozzo.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/backup.c | 55 +++++++++++++++++--------------------------------------
 1 file changed, 17 insertions(+), 38 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index XXXXXXX..XXXXXXX 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -XXX,XX +XXX,XX @@ static bool coroutine_fn yield_and_check(BackupBlockJob *job)
 
 static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
 {
+    int ret;
     bool error_is_read;
-    int ret = 0;
-    int clusters_per_iter;
-    uint32_t granularity;
-    int64_t offset;
     int64_t cluster;
-    int64_t end;
-    BdrvDirtyBitmapIter *dbi;
+    HBitmapIter hbi;
 
-    granularity = bdrv_dirty_bitmap_granularity(job->sync_bitmap);
-    clusters_per_iter = MAX((granularity / job->cluster_size), 1);
-    dbi = bdrv_dirty_iter_new(job->sync_bitmap);
-
-    /* Find the next dirty sector(s) */
-    while ((offset = bdrv_dirty_iter_next(dbi)) >= 0) {
-        cluster = offset / job->cluster_size;
-
-        for (end = cluster + clusters_per_iter; cluster < end; cluster++) {
-            do {
-                if (yield_and_check(job)) {
-                    goto out;
-                }
-                ret = backup_do_cow(job, cluster * job->cluster_size,
-                                    job->cluster_size, &error_is_read,
-                                    false);
-                if ((ret < 0) &&
-                    backup_error_action(job, error_is_read, -ret) ==
-                    BLOCK_ERROR_ACTION_REPORT) {
-                    goto out;
-                }
-            } while (ret < 0);
-        }
-
-        /* If the bitmap granularity is smaller than the backup granularity,
-         * we need to advance the iterator pointer to the next cluster. */
-        if (granularity < job->cluster_size) {
-            bdrv_set_dirty_iter(dbi, cluster * job->cluster_size);
-        }
+    hbitmap_iter_init(&hbi, job->copy_bitmap, 0);
+    while ((cluster = hbitmap_iter_next(&hbi)) != -1) {
+        do {
+            if (yield_and_check(job)) {
+                return 0;
+            }
+            ret = backup_do_cow(job, cluster * job->cluster_size,
+                                job->cluster_size, &error_is_read, false);
+            if (ret < 0 && backup_error_action(job, error_is_read, -ret) ==
+                           BLOCK_ERROR_ACTION_REPORT)
+            {
+                return ret;
+            }
+        } while (ret < 0);
     }
 
-out:
-    bdrv_dirty_iter_free(dbi);
-    return ret;
+    return 0;
 }
 
 /* init copy_bitmap from sync_bitmap */
-- 
2.9.5

From: John Snow <jsnow@redhat.com>

If users set an unreasonably low speed (like one byte per second), the
calculated delay may exceed many hours. While we like to punish users
for asking for stupid things, we do also like to allow users to correct
their wicked ways.

When a user provides a new speed, kick the job to allow it to recalculate
its delay.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20171213204611.26276-1-jsnow@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 blockjob.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/blockjob.c b/blockjob.c
index XXXXXXX..XXXXXXX 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -XXX,XX +XXX,XX @@ static void __attribute__((__constructor__)) block_job_init(void)
 
 static void block_job_event_cancelled(BlockJob *job);
 static void block_job_event_completed(BlockJob *job, const char *msg);
+static void block_job_enter_cond(BlockJob *job, bool(*fn)(BlockJob *job));
 
 /* Transactional group of block jobs */
 struct BlockJobTxn {
@@ -XXX,XX +XXX,XX @@ static void block_job_completed_txn_success(BlockJob *job)
     }
 }
 
+/* Assumes the block_job_mutex is held */
+static bool block_job_timer_pending(BlockJob *job)
+{
+    return timer_pending(&job->sleep_timer);
+}
+
 void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)
 {
     Error *local_err = NULL;
+    int64_t old_speed = job->speed;
 
     if (!job->driver->set_speed) {
         error_setg(errp, QERR_UNSUPPORTED);
@@ -XXX,XX +XXX,XX @@ void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)
     }
 
     job->speed = speed;
+    if (speed <= old_speed) {
+        return;
+    }
+
+    /* kick only if a timer is pending */
+    block_job_enter_cond(job, block_job_timer_pending);
 }
 
 void block_job_complete(BlockJob *job, Error **errp)
@@ -XXX,XX +XXX,XX @@ void block_job_resume_all(void)
     }
 }
 
-void block_job_enter(BlockJob *job)
+/*
+ * Conditionally enter a block_job pending a call to fn() while
+ * under the block_job_lock critical section.
+ */
+static void block_job_enter_cond(BlockJob *job, bool(*fn)(BlockJob *job))
 {
     if (!block_job_started(job)) {
         return;
@@ -XXX,XX +XXX,XX @@ void block_job_enter(BlockJob *job)
         return;
     }
 
+    if (fn && !fn(job)) {
+        block_job_unlock();
+        return;
+    }
+
     assert(!job->deferred_to_main_loop);
     timer_del(&job->sleep_timer);
     job->busy = true;
@@ -XXX,XX +XXX,XX @@ void block_job_enter(BlockJob *job)
     aio_co_wake(job->co);
 }
 
+void block_job_enter(BlockJob *job)
+{
+    block_job_enter_cond(job, NULL);
+}
+
 bool block_job_is_cancelled(BlockJob *job)
 {
     return job->cancelled;
-- 
2.9.5

No functional changes, just whitespace manipulation.

Signed-off-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/sheepdog.c | 164 +++++++++++++++++++++++++++----------------------------
 1 file changed, 82 insertions(+), 82 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index XXXXXXX..XXXXXXX 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -XXX,XX +XXX,XX @@ typedef struct BDRVSheepdogReopenState {
     int cache_flags;
 } BDRVSheepdogReopenState;
 
-static const char * sd_strerror(int err)
+static const char *sd_strerror(int err)
 {
     int i;
 
@@ -XXX,XX +XXX,XX @@ static QemuOptsList sd_create_opts = {
 };
 
 static BlockDriver bdrv_sheepdog = {
-    .format_name    = "sheepdog",
-    .protocol_name  = "sheepdog",
-    .instance_size  = sizeof(BDRVSheepdogState),
-    .bdrv_parse_filename    = sd_parse_filename,
-    .bdrv_file_open = sd_open,
-    .bdrv_reopen_prepare    = sd_reopen_prepare,
-    .bdrv_reopen_commit     = sd_reopen_commit,
-    .bdrv_reopen_abort      = sd_reopen_abort,
-    .bdrv_close     = sd_close,
-    .bdrv_create    = sd_create,
-    .bdrv_has_zero_init = bdrv_has_zero_init_1,
-    .bdrv_getlength = sd_getlength,
+    .format_name                  = "sheepdog",
+    .protocol_name                = "sheepdog",
+    .instance_size                = sizeof(BDRVSheepdogState),
+    .bdrv_parse_filename          = sd_parse_filename,
+    .bdrv_file_open               = sd_open,
+    .bdrv_reopen_prepare          = sd_reopen_prepare,
+    .bdrv_reopen_commit           = sd_reopen_commit,
+    .bdrv_reopen_abort            = sd_reopen_abort,
+    .bdrv_close                   = sd_close,
+    .bdrv_create                  = sd_create,
+    .bdrv_has_zero_init           = bdrv_has_zero_init_1,
+    .bdrv_getlength               = sd_getlength,
     .bdrv_get_allocated_file_size = sd_get_allocated_file_size,
-    .bdrv_truncate  = sd_truncate,
+    .bdrv_truncate                = sd_truncate,
 
-    .bdrv_co_readv  = sd_co_readv,
-    .bdrv_co_writev = sd_co_writev,
-    .bdrv_co_flush_to_disk  = sd_co_flush_to_disk,
-    .bdrv_co_pdiscard = sd_co_pdiscard,
-    .bdrv_co_get_block_status = sd_co_get_block_status,
+    .bdrv_co_readv                = sd_co_readv,
+    .bdrv_co_writev               = sd_co_writev,
+    .bdrv_co_flush_to_disk        = sd_co_flush_to_disk,
+    .bdrv_co_pdiscard             = sd_co_pdiscard,
+    .bdrv_co_get_block_status     = sd_co_get_block_status,
 
-    .bdrv_snapshot_create   = sd_snapshot_create,
-    .bdrv_snapshot_goto     = sd_snapshot_goto,
-    .bdrv_snapshot_delete   = sd_snapshot_delete,
-    .bdrv_snapshot_list     = sd_snapshot_list,
+    .bdrv_snapshot_create         = sd_snapshot_create,
+    .bdrv_snapshot_goto           = sd_snapshot_goto,
+    .bdrv_snapshot_delete         = sd_snapshot_delete,
+    .bdrv_snapshot_list           = sd_snapshot_list,
 
-    .bdrv_save_vmstate  = sd_save_vmstate,
-    .bdrv_load_vmstate  = sd_load_vmstate,
+    .bdrv_save_vmstate            = sd_save_vmstate,
+    .bdrv_load_vmstate            = sd_load_vmstate,
 
-    .bdrv_detach_aio_context = sd_detach_aio_context,
-    .bdrv_attach_aio_context = sd_attach_aio_context,
+    .bdrv_detach_aio_context      = sd_detach_aio_context,
+    .bdrv_attach_aio_context      = sd_attach_aio_context,
 
-    .create_opts    = &sd_create_opts,
+    .create_opts                  = &sd_create_opts,
 };
 
 static BlockDriver bdrv_sheepdog_tcp = {
-    .format_name    = "sheepdog",
-    .protocol_name  = "sheepdog+tcp",
-    .instance_size  = sizeof(BDRVSheepdogState),
-    .bdrv_parse_filename    = sd_parse_filename,
-    .bdrv_file_open = sd_open,
-    .bdrv_reopen_prepare    = sd_reopen_prepare,
-    .bdrv_reopen_commit     = sd_reopen_commit,
-    .bdrv_reopen_abort      = sd_reopen_abort,
-    .bdrv_close     = sd_close,
-    .bdrv_create    = sd_create,
-    .bdrv_has_zero_init = bdrv_has_zero_init_1,
-    .bdrv_getlength = sd_getlength,
+    .format_name                  = "sheepdog",
+    .protocol_name                = "sheepdog+tcp",
+    .instance_size                = sizeof(BDRVSheepdogState),
+    .bdrv_parse_filename          = sd_parse_filename,
+    .bdrv_file_open               = sd_open,
+    .bdrv_reopen_prepare          = sd_reopen_prepare,
+    .bdrv_reopen_commit           = sd_reopen_commit,
+    .bdrv_reopen_abort            = sd_reopen_abort,
+    .bdrv_close                   = sd_close,
+    .bdrv_create                  = sd_create,
+    .bdrv_has_zero_init           = bdrv_has_zero_init_1,
+    .bdrv_getlength               = sd_getlength,
     .bdrv_get_allocated_file_size = sd_get_allocated_file_size,
-    .bdrv_truncate  = sd_truncate,
+    .bdrv_truncate                = sd_truncate,
 
-    .bdrv_co_readv  = sd_co_readv,
-    .bdrv_co_writev = sd_co_writev,
-    .bdrv_co_flush_to_disk  = sd_co_flush_to_disk,
-    .bdrv_co_pdiscard = sd_co_pdiscard,
-    .bdrv_co_get_block_status = sd_co_get_block_status,
+    .bdrv_co_readv                = sd_co_readv,
+    .bdrv_co_writev               = sd_co_writev,
+    .bdrv_co_flush_to_disk        = sd_co_flush_to_disk,
+    .bdrv_co_pdiscard             = sd_co_pdiscard,
+    .bdrv_co_get_block_status     = sd_co_get_block_status,
 
-    .bdrv_snapshot_create   = sd_snapshot_create,
-    .bdrv_snapshot_goto     = sd_snapshot_goto,
-    .bdrv_snapshot_delete   = sd_snapshot_delete,
-    .bdrv_snapshot_list     = sd_snapshot_list,
+    .bdrv_snapshot_create         = sd_snapshot_create,
+    .bdrv_snapshot_goto           = sd_snapshot_goto,
+    .bdrv_snapshot_delete         = sd_snapshot_delete,
+    .bdrv_snapshot_list           = sd_snapshot_list,
 
-    .bdrv_save_vmstate  = sd_save_vmstate,
-    .bdrv_load_vmstate  = sd_load_vmstate,
+    .bdrv_save_vmstate            = sd_save_vmstate,
+    .bdrv_load_vmstate            = sd_load_vmstate,
 
-    .bdrv_detach_aio_context = sd_detach_aio_context,
-    .bdrv_attach_aio_context = sd_attach_aio_context,
+    .bdrv_detach_aio_context      = sd_detach_aio_context,
+    .bdrv_attach_aio_context      = sd_attach_aio_context,
 
-    .create_opts    = &sd_create_opts,
+    .create_opts                  = &sd_create_opts,
 };
 
 static BlockDriver bdrv_sheepdog_unix = {
-    .format_name    = "sheepdog",
-    .protocol_name  = "sheepdog+unix",
-    .instance_size  = sizeof(BDRVSheepdogState),
-    .bdrv_parse_filename    = sd_parse_filename,
-    .bdrv_file_open = sd_open,
-    .bdrv_reopen_prepare    = sd_reopen_prepare,
-    .bdrv_reopen_commit     = sd_reopen_commit,
-    .bdrv_reopen_abort      = sd_reopen_abort,
-    .bdrv_close     = sd_close,
-    .bdrv_create    = sd_create,
-    .bdrv_has_zero_init = bdrv_has_zero_init_1,
-    .bdrv_getlength = sd_getlength,
+    .format_name                  = "sheepdog",
+    .protocol_name                = "sheepdog+unix",
+    .instance_size                = sizeof(BDRVSheepdogState),
+    .bdrv_parse_filename          = sd_parse_filename,
+    .bdrv_file_open               = sd_open,
+    .bdrv_reopen_prepare          = sd_reopen_prepare,
+    .bdrv_reopen_commit           = sd_reopen_commit,
+    .bdrv_reopen_abort            = sd_reopen_abort,
+    .bdrv_close                   = sd_close,
+    .bdrv_create                  = sd_create,
+    .bdrv_has_zero_init           = bdrv_has_zero_init_1,
+    .bdrv_getlength               = sd_getlength,
     .bdrv_get_allocated_file_size = sd_get_allocated_file_size,
-    .bdrv_truncate  = sd_truncate,
+    .bdrv_truncate                = sd_truncate,
 
-    .bdrv_co_readv  = sd_co_readv,
-    .bdrv_co_writev = sd_co_writev,
-    .bdrv_co_flush_to_disk  = sd_co_flush_to_disk,
-    .bdrv_co_pdiscard = sd_co_pdiscard,
-    .bdrv_co_get_block_status = sd_co_get_block_status,
+    .bdrv_co_readv                = sd_co_readv,
+    .bdrv_co_writev               = sd_co_writev,
+    .bdrv_co_flush_to_disk        = sd_co_flush_to_disk,
+    .bdrv_co_pdiscard             = sd_co_pdiscard,
+    .bdrv_co_get_block_status     = sd_co_get_block_status,
 
-    .bdrv_snapshot_create   = sd_snapshot_create,
-    .bdrv_snapshot_goto     = sd_snapshot_goto,
-    .bdrv_snapshot_delete   = sd_snapshot_delete,
-    .bdrv_snapshot_list     = sd_snapshot_list,
+    .bdrv_snapshot_create         = sd_snapshot_create,
+    .bdrv_snapshot_goto           = sd_snapshot_goto,
+    .bdrv_snapshot_delete         = sd_snapshot_delete,
+    .bdrv_snapshot_list           = sd_snapshot_list,
 
-    .bdrv_save_vmstate  = sd_save_vmstate,
-    .bdrv_load_vmstate  = sd_load_vmstate,
+    .bdrv_save_vmstate            = sd_save_vmstate,
+    .bdrv_load_vmstate            = sd_load_vmstate,
 
-    .bdrv_detach_aio_context = sd_detach_aio_context,
-    .bdrv_attach_aio_context = sd_attach_aio_context,
+    .bdrv_detach_aio_context      = sd_detach_aio_context,
+    .bdrv_attach_aio_context      = sd_attach_aio_context,
 
-    .create_opts    = &sd_create_opts,
+    .create_opts                  = &sd_create_opts,
 };
 
 static void bdrv_sheepdog_init(void)
-- 
2.9.5

If curl_global_init() fails, per the documentation no other curl
functions may be called, so make sure to check the return value.

Also, some minor changes to the initialization latch variable 'inited':

- Make it static in the file, for clarity
- Change the name for clarity
- Make it a bool

Signed-off-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/curl.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index XXXXXXX..XXXXXXX 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -XXX,XX +XXX,XX @@ static CURLMcode __curl_multi_socket_action(CURLM *multi_handle,
 
 struct BDRVCURLState;
 
+static bool libcurl_initialized;
+
 typedef struct CURLAIOCB {
     Coroutine *co;
     QEMUIOVector *qiov;
@@ -XXX,XX +XXX,XX @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
     double d;
     const char *secretid;
     const char *protocol_delimiter;
+    int ret;
 
-    static int inited = 0;
 
     if (flags & BDRV_O_RDWR) {
         error_setg(errp, "curl block device does not support writes");
         return -EROFS;
     }
 
+    if (!libcurl_initialized) {
+        ret = curl_global_init(CURL_GLOBAL_ALL);
+        if (ret) {
+            error_setg(errp, "libcurl initialization failed with %d", ret);
+            return -EIO;
+        }
+        libcurl_initialized = true;
+    }
+
     qemu_mutex_init(&s->mutex);
     opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
     qemu_opts_absorb_qdict(opts, options, &local_err);
@@ -XXX,XX +XXX,XX @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
         }
     }
 
-    if (!inited) {
-        curl_global_init(CURL_GLOBAL_ALL);
-        inited = 1;
-    }
-
     DPRINTF("CURL: Opening %s\n", file);
     QSIMPLEQ_INIT(&s->free_state_waitq);
     s->aio_context = bdrv_get_aio_context(bs);
-- 
2.9.5

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