Series comparison

-[Qemu-devel] [PULL 00/12] Block patches
+[Qemu-devel] [PULL 00/20] Block patches
-The following changes since commit 9964e96dc9999cf7f7c936ee854a795415d19b60:
+The following changes since commit 5ec2eca83dc478ddf24077e02a8b34dd26cd3ff9:
-  Merge remote-tracking branch 'jasowang/tags/net-pull-request' into staging (2017-05-23 15:01:31 +0100)
+  Merge remote-tracking branch 'remotes/awilliam/tags/vfio-updates-20190613.0' into staging (2019-06-14 09:33:55 +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-2019-06-14
-for you to fetch changes up to 223a23c198787328ae75bc65d84edf5fde33c0b6:
+for you to fetch changes up to 21c1ce592a144188dfe59b9e156a97da412a59a2:
-  block/gluster: glfs_lseek() workaround (2017-05-24 16:44:46 -0400)
+  iotests: Test qemu-img convert -C --salvage (2019-06-14 15:09:42 +0200)
 ----------------------------------------------------------------
-Block patches
+Block patches:
 - Allow blockdev-backup from nodes that are not in qemu's main AIO
   context to newly added nodes
 - Add salvaging mode to qemu-img convert
 - Minor fixes to tests, documentation, and for less Valgrind annoyance
 ----------------------------------------------------------------
+Andrey Shinkevich (1):
+  hw/block/fdc: floppy command FIFO memory initialization
-Jeff Cody (1):
+John Snow (6):
-  block/gluster: glfs_lseek() workaround
+  blockdev-backup: don't check aio_context too early
   iotests.py: do not use infinite waits
   QEMUMachine: add events_wait method
   iotests.py: rewrite run_job to be pickier
   iotests: add iotest 256 for testing blockdev-backup across iothread
     contexts
   event_match: always match on None value
-Paolo Bonzini (11):
+Max Reitz (12):
-  blockjob: remove unnecessary check
+  iotests: Filter 175's allocation information
-  blockjob: remove iostatus_reset callback
+  iotests: Fix intermittent failure in 219
-  blockjob: introduce block_job_early_fail
+  qemu-img: Fix options leakage in img_rebase()
-  blockjob: introduce block_job_pause/resume_all
+  qapi/block-core: Overlays are not snapshots
-  blockjob: separate monitor and blockjob APIs
+  blockdev: Overlays are not snapshots
-  blockjob: move iostatus reset inside block_job_user_resume
+  qemu-img: Move quiet into ImgConvertState
-  blockjob: introduce block_job_cancel_async, check iostatus invariants
+  qemu-img: Add salvaging mode to convert
-  blockjob: group BlockJob transaction functions together
+  blkdebug: Add @iotype error option
-  blockjob: strengthen a bit test-blockjob-txn
+  blkdebug: Add "none" event
-  blockjob: reorganize block_job_completed_txn_abort
+  blkdebug: Inject errors on .bdrv_co_block_status()
-  blockjob: use deferred_to_main_loop to indicate the coroutine has
+  iotests: Test qemu-img convert --salvage
-    ended
+  iotests: Test qemu-img convert -C --salvage
- block/backup.c               |   2 +-
+Vladimir Sementsov-Ogievskiy (1):
- block/commit.c               |   2 +-
+  iotests: restrict 254 to support only qcow2
- block/gluster.c              |  18 +-
- block/io.c                   |  19 +-
+ qapi/block-core.json          |  53 ++++++++---
- block/mirror.c               |   2 +-
+ block/blkdebug.c              |  60 ++++++++++--
- blockdev.c                   |   1 -
+ blockdev.c                    |  14 +--
- blockjob.c                   | 750 ++++++++++++++++++++++++-------------------
+ hw/block/fdc.c                |   1 +
- include/block/blockjob.h     |  16 -
+ qemu-img.c                    | 106 +++++++++++++++------
- include/block/blockjob_int.h |  27 +-
+ python/qemu/__init__.py       |  67 ++++++++++----
- tests/test-blockjob-txn.c    |   7 +-
+ qemu-img-cmds.hx              |   4 +-
- tests/test-blockjob.c        |  10 +-
+ qemu-img.texi                 |   4 +
-files changed, 463 insertions(+), 391 deletions(-)
+ tests/qemu-iotests/082        |   1 +
  tests/qemu-iotests/082.out    |   3 +
  tests/qemu-iotests/085.out    |  10 +-
  tests/qemu-iotests/175        |  26 +++++-
  tests/qemu-iotests/175.out    |   8 +-
  tests/qemu-iotests/219        |  13 ++-
  tests/qemu-iotests/251        | 170 ++++++++++++++++++++++++++++++++++
  tests/qemu-iotests/251.out    |  43 +++++++++
  tests/qemu-iotests/254        |   2 +
  tests/qemu-iotests/256        | 122 ++++++++++++++++++++++++
  tests/qemu-iotests/256.out    | 119 ++++++++++++++++++++++++
  tests/qemu-iotests/group      |   2 +
  tests/qemu-iotests/iotests.py |  60 +++++++-----
 files changed, 772 insertions(+), 116 deletions(-)
  create mode 100755 tests/qemu-iotests/251
  create mode 100644 tests/qemu-iotests/251.out
  create mode 100755 tests/qemu-iotests/256
  create mode 100644 tests/qemu-iotests/256.out
 --
-.9.3
+.21.0

-[Qemu-devel] [PULL 08/12] blockjob: group BlockJob transaction functions together
+[Qemu-devel] [PULL 01/20] blockdev-backup: don't check aio_context too early
-From: Paolo Bonzini <pbonzini@redhat.com>
+From: John Snow <jsnow@redhat.com>
-Yet another pure code movement patch, preparing for the next change.
+in blockdev_backup_prepare, we check to make sure that the target is
 associated with a compatible aio context. However, do_blockdev_backup is
 called later and has some logic to move the target to a compatible
 aio_context. The transaction version will fail certain commands
 needlessly early as a result.
-Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
+Allow blockdev_backup_prepare to simply call do_blockdev_backup, which
-Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
+will ultimately decide if the contexts are compatible or not.
-Message-id: 20170508141310.8674-9-pbonzini@redhat.com
-Signed-off-by: Jeff Cody <jcody@redhat.com>
+Note: the transaction version has always disallowed this operation since
 its initial commit bd8baecd (2014), whereas the version of
 qmp_blockdev_backup at the time, from commit c29c1dd312f, tried to
 enforce the aio_context switch instead. It's not clear, and I can't see
 from the mailing list archives at the time, why the two functions take a
 different approach. It wasn't until later in efd7556708b (2016) that the
 standalone version tried to determine if it could set the context or
 not.
 Reported-by: aihua liang <aliang@redhat.com>
 Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1683498
 Signed-off-by: John Snow <jsnow@redhat.com>
 Message-id: 20190523170643.20794-2-jsnow@redhat.com
 Reviewed-by: Max Reitz <mreitz@redhat.com>
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
- blockjob.c | 128 ++++++++++++++++++++++++++++++-------------------------------
+ blockdev.c | 4 ----
-file changed, 64 insertions(+), 64 deletions(-)
+file changed, 4 deletions(-)
-diff --git a/blockjob.c b/blockjob.c
+diff --git a/blockdev.c b/blockdev.c
 index XXXXXXX..XXXXXXX 100644
---- a/blockjob.c
+--- a/blockdev.c
-+++ b/blockjob.c
++++ b/blockdev.c
-@@ -XXX,XX +XXX,XX @@ BlockJob *block_job_get(const char *id)
+@@ -XXX,XX +XXX,XX @@ static void blockdev_backup_prepare(BlkActionState *common, Error **errp)
      return NULL;
  }
 +BlockJobTxn *block_job_txn_new(void)
 +{
 +    BlockJobTxn *txn = g_new0(BlockJobTxn, 1);
 +    QLIST_INIT(&txn->jobs);
 +    txn->refcnt = 1;
 +    return txn;
 +}
 +
 +static void block_job_txn_ref(BlockJobTxn *txn)
 +{
 +    txn->refcnt++;
 +}
 +
 +void block_job_txn_unref(BlockJobTxn *txn)
 +{
 +    if (txn && --txn->refcnt == 0) {
 +        g_free(txn);
 +    }
 +}
 +
 +void block_job_txn_add_job(BlockJobTxn *txn, BlockJob *job)
 +{
 +    if (!txn) {
 +        return;
 +    }
 +
 +    assert(!job->txn);
 +    job->txn = txn;
 +
 +    QLIST_INSERT_HEAD(&txn->jobs, job, txn_list);
 +    block_job_txn_ref(txn);
 +}
 +
  static void block_job_pause(BlockJob *job)
  {
      job->pause_count++;
@@ -XXX,XX +XXX,XX @@ static void block_job_cancel_async(BlockJob *job)
      job->cancelled = true;
  }
 +static int block_job_finish_sync(BlockJob *job,
 +                                 void (*finish)(BlockJob *, Error **errp),
 +                                 Error **errp)
 +{
 +    Error *local_err = NULL;
 +    int ret;
 +
 +    assert(blk_bs(job->blk)->job == job);
 +
 +    block_job_ref(job);
 +
 +    finish(job, &local_err);
 +    if (local_err) {
 +        error_propagate(errp, local_err);
 +        block_job_unref(job);
 +        return -EBUSY;
 +    }
 +    /* block_job_drain calls block_job_enter, and it should be enough to
 +     * induce progress until the job completes or moves to the main thread.
 +    */
 +    while (!job->deferred_to_main_loop && !job->completed) {
 +        block_job_drain(job);
 +    }
 +    while (!job->completed) {
 +        aio_poll(qemu_get_aio_context(), true);
 +    }
 +    ret = (job->cancelled && job->ret == 0) ? -ECANCELED : job->ret;
 +    block_job_unref(job);
 +    return ret;
 +}
 +
  static void block_job_completed_txn_abort(BlockJob *job)
  {
      AioContext *ctx;
@@ -XXX,XX +XXX,XX @@ void block_job_cancel(BlockJob *job)
      }
- }
+     aio_context = bdrv_get_aio_context(bs);
--static int block_job_finish_sync(BlockJob *job,
+-    if (aio_context != bdrv_get_aio_context(target)) {
--                                 void (*finish)(BlockJob *, Error **errp),
+-        error_setg(errp, "Backup between two IO threads is not implemented");
 -                                 Error **errp)
 -{
 -    Error *local_err = NULL;
 -    int ret;
 -
 -    assert(blk_bs(job->blk)->job == job);
 -
 -    block_job_ref(job);
 -
 -    finish(job, &local_err);
 -    if (local_err) {
 -        error_propagate(errp, local_err);
 -        block_job_unref(job);
 -        return -EBUSY;
 -    }
 -    /* block_job_drain calls block_job_enter, and it should be enough to
 -     * induce progress until the job completes or moves to the main thread.
 -    */
 -    while (!job->deferred_to_main_loop && !job->completed) {
 -        block_job_drain(job);
 -    }
 -    while (!job->completed) {
 -        aio_poll(qemu_get_aio_context(), true);
 -    }
 -    ret = (job->cancelled && job->ret == 0) ? -ECANCELED : job->ret;
 -    block_job_unref(job);
 -    return ret;
 -}
 -
  /* A wrapper around block_job_cancel() taking an Error ** parameter so it may be
   * used with block_job_finish_sync() without the need for (rather nasty)
   * function pointer casts there. */
@@ -XXX,XX +XXX,XX @@ void block_job_defer_to_main_loop(BlockJob *job,
      aio_bh_schedule_oneshot(qemu_get_aio_context(),
                              block_job_defer_to_main_loop_bh, data);
  }
 -
 -BlockJobTxn *block_job_txn_new(void)
 -{
 -    BlockJobTxn *txn = g_new0(BlockJobTxn, 1);
 -    QLIST_INIT(&txn->jobs);
 -    txn->refcnt = 1;
 -    return txn;
 -}
 -
 -static void block_job_txn_ref(BlockJobTxn *txn)
 -{
 -    txn->refcnt++;
 -}
 -
 -void block_job_txn_unref(BlockJobTxn *txn)
 -{
 -    if (txn && --txn->refcnt == 0) {
 -        g_free(txn);
 -    }
 -}
 -
 -void block_job_txn_add_job(BlockJobTxn *txn, BlockJob *job)
 -{
 -    if (!txn) {
 -        return;
 -    }
--
+     aio_context_acquire(aio_context);
--    assert(!job->txn);
+     state->bs = bs;
--    job->txn = txn;
 -
 -    QLIST_INSERT_HEAD(&txn->jobs, job, txn_list);
 -    block_job_txn_ref(txn);
 -}
 --
-.9.3
+.21.0

-New patch
+[Qemu-devel] [PULL 02/20] iotests.py: do not use infinite waits
+From: John Snow <jsnow@redhat.com>
+Cap waits to 60 seconds so that iotests can fail gracefully if something
+goes wrong.
+Signed-off-by: John Snow <jsnow@redhat.com>
+Message-id: 20190523170643.20794-3-jsnow@redhat.com
+Reviewed-by: Max Reitz <mreitz@redhat.com>
+Signed-off-by: Max Reitz <mreitz@redhat.com>
+---
+ tests/qemu-iotests/iotests.py | 14 +++++++-------
+file changed, 7 insertions(+), 7 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 @@ class VM(qtest.QEMUQtestMachine):
+             output_list += [key + '=' + obj[key]]
+         return ','.join(output_list)
+-    def get_qmp_events_filtered(self, wait=True):
++    def get_qmp_events_filtered(self, wait=60.0):
+         result = []
+         for ev in self.get_qmp_events(wait=wait):
+             result.append(filter_qmp_event(ev))
+@@ -XXX,XX +XXX,XX @@ class VM(qtest.QEMUQtestMachine):
+     # Returns None on success, and an error string on failure
+     def run_job(self, job, auto_finalize=True, auto_dismiss=False,
+-                pre_finalize=None):
++                pre_finalize=None, wait=60.0):
+         error = None
+         while True:
+-            for ev in self.get_qmp_events_filtered(wait=True):
++            for ev in self.get_qmp_events_filtered(wait=wait):
+                 if ev['event'] == 'JOB_STATUS_CHANGE':
+                     status = ev['data']['status']
+                     if status == 'aborting':
+@@ -XXX,XX +XXX,XX @@ class QMPTestCase(unittest.TestCase):
+         self.assertEqual(self.vm.flatten_qmp_object(json.loads(json_filename[5:])),
+                          self.vm.flatten_qmp_object(reference))
+-    def cancel_and_wait(self, drive='drive0', force=False, resume=False):
++    def cancel_and_wait(self, drive='drive0', force=False, resume=False, wait=60.0):
+         '''Cancel a block job and wait for it to finish, returning the event'''
+         result = self.vm.qmp('block-job-cancel', device=drive, force=force)
+         self.assert_qmp(result, 'return', {})
+@@ -XXX,XX +XXX,XX @@ class QMPTestCase(unittest.TestCase):
+         cancelled = False
+         result = None
+         while not cancelled:
+-            for event in self.vm.get_qmp_events(wait=True):
++            for event in self.vm.get_qmp_events(wait=wait):
+                 if event['event'] == 'BLOCK_JOB_COMPLETED' or \
+                    event['event'] == 'BLOCK_JOB_CANCELLED':
+                     self.assert_qmp(event, 'data/device', drive)
+@@ -XXX,XX +XXX,XX @@ class QMPTestCase(unittest.TestCase):
+         self.assert_no_active_block_jobs()
+         return result
+-    def wait_until_completed(self, drive='drive0', check_offset=True):
++    def wait_until_completed(self, drive='drive0', check_offset=True, wait=60.0):
+         '''Wait for a block job to finish, returning the event'''
+         while True:
+-            for event in self.vm.get_qmp_events(wait=True):
++            for event in self.vm.get_qmp_events(wait=wait):
+                 if event['event'] == 'BLOCK_JOB_COMPLETED':
+                     self.assert_qmp(event, 'data/device', drive)
+                     self.assert_qmp_absent(event, 'data/error')
+--
+.21.0

-[Qemu-devel] [PULL 02/12] blockjob: remove iostatus_reset callback
+[Qemu-devel] [PULL 03/20] QEMUMachine: add events_wait method
-From: Paolo Bonzini <pbonzini@redhat.com>
+From: John Snow <jsnow@redhat.com>
-This is unused since commit 66a0fae ("blockjob: Don't touch BDS iostatus",
+Instead of event_wait which looks for a single event, add an events_wait
--05-19).
+which can look for any number of events simultaneously. However, it
 will still only return one at a time, whichever happens first.
-Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
+Signed-off-by: John Snow <jsnow@redhat.com>
-Reviewed-by: John Snow <jsnow@redhat.com>
+Message-id: 20190523170643.20794-4-jsnow@redhat.com
-Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
+Reviewed-by: Max Reitz <mreitz@redhat.com>
-Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
+Signed-off-by: Max Reitz <mreitz@redhat.com>
 Reviewed-by: Jeff Cody <jcody@redhat.com>
 Message-id: 20170508141310.8674-3-pbonzini@redhat.com
 Signed-off-by: Jeff Cody <jcody@redhat.com>
 ---
- blockjob.c                   | 3 ---
+ python/qemu/__init__.py | 69 +++++++++++++++++++++++++++++------------
- include/block/blockjob_int.h | 3 ---
+file changed, 49 insertions(+), 20 deletions(-)
 files changed, 6 deletions(-)
-diff --git a/blockjob.c b/blockjob.c
+diff --git a/python/qemu/__init__.py b/python/qemu/__init__.py
 index XXXXXXX..XXXXXXX 100644
---- a/blockjob.c
+--- a/python/qemu/__init__.py
-+++ b/blockjob.c
++++ b/python/qemu/__init__.py
-@@ -XXX,XX +XXX,XX @@ bool block_job_is_cancelled(BlockJob *job)
+@@ -XXX,XX +XXX,XX @@ class QEMUMachine(object):
- void block_job_iostatus_reset(BlockJob *job)
+         self._qmp.clear_events()
- {
+         return events
-     job->iostatus = BLOCK_DEVICE_IO_STATUS_OK;
--    if (job->driver->iostatus_reset) {
+-    def event_wait(self, name, timeout=60.0, match=None):
--        job->driver->iostatus_reset(job);
++    @staticmethod
--    }
++    def event_match(event, match=None):
- }
+         """
+-        Wait for specified timeout on named event in QMP; optionally filter
- static int block_job_finish_sync(BlockJob *job,
+-        results by match.
-diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
++        Check if an event matches optional match criteria.
-index XXXXXXX..XXXXXXX 100644
---- a/include/block/blockjob_int.h
+-        The 'match' is checked to be a recursive subset of the 'event'; skips
-+++ b/include/block/blockjob_int.h
+-        branch processing on match's value None
-@@ -XXX,XX +XXX,XX @@ struct BlockJobDriver {
+-           {"foo": {"bar": 1}} matches {"foo": None}
-     /** Optional callback for job types that support setting a speed limit */
+-           {"foo": {"bar": 1}} does not matches {"foo": {"baz": None}}
-     void (*set_speed)(BlockJob *job, int64_t speed, Error **errp);
++        The match criteria takes the form of a matching subdict. The event is
++        checked to be a superset of the subdict, recursively, with matching
--    /** Optional callback for job types that need to forward I/O status reset */
++        values whenever those values are not None.
--    void (*iostatus_reset)(BlockJob *job);
++
--
++        Examples, with the subdict queries on the left:
-     /** Mandatory: Entrypoint for the Coroutine. */
++         - None matches any object.
-     CoroutineEntry *start;
++         - {"foo": None} matches {"foo": {"bar": 1}}
 +         - {"foo": {"baz": None}} does not match {"foo": {"bar": 1}}
 +         - {"foo": {"baz": 2}} matches {"foo": {"bar": 1, "baz": 2}}
          """
 -        def event_match(event, match=None):
 -            if match is None:
 -                return True
 +        if match is None:
 +            return True
 -            for key in match:
 -                if key in event:
 -                    if isinstance(event[key], dict):
 -                        if not event_match(event[key], match[key]):
 -                            return False
 -                    elif event[key] != match[key]:
 +        for key in match:
 +            if key in event:
 +                if isinstance(event[key], dict):
 +                    if not QEMUMachine.event_match(event[key], match[key]):
                          return False
 -                else:
 +                elif event[key] != match[key]:
                      return False
 +            else:
 +                return False
 +        return True
 -            return True
 +    def event_wait(self, name, timeout=60.0, match=None):
 +        """
 +        event_wait waits for and returns a named event from QMP with a timeout.
 +
 +        name: The event to wait for.
 +        timeout: QEMUMonitorProtocol.pull_event timeout parameter.
 +        match: Optional match criteria. See event_match for details.
 +        """
 +        return self.events_wait([(name, match)], timeout)
 +
 +    def events_wait(self, events, timeout=60.0):
 +        """
 +        events_wait waits for and returns a named event from QMP with a timeout.
 +
 +        events: a sequence of (name, match_criteria) tuples.
 +                The match criteria are optional and may be None.
 +                See event_match for details.
 +        timeout: QEMUMonitorProtocol.pull_event timeout parameter.
 +        """
 +        def _match(event):
 +            for name, match in events:
 +                if (event['event'] == name and
 +                    self.event_match(event, match)):
 +                    return True
 +            return False
          # Search cached events
          for event in self._events:
 -            if (event['event'] == name) and event_match(event, match):
 +            if _match(event):
                  self._events.remove(event)
                  return event
          # Poll for new events
          while True:
              event = self._qmp.pull_event(wait=timeout)
 -            if (event['event'] == name) and event_match(event, match):
 +            if _match(event):
                  return event
              self._events.append(event)
 --
-.9.3
+.21.0

-New patch
+[Qemu-devel] [PULL 04/20] iotests.py: rewrite run_job to be pickier
+From: John Snow <jsnow@redhat.com>
+Don't pull events out of the queue that don't belong to us;
+be choosier so that we can use this method to drive jobs that
+were launched by transactions that may have more jobs.
+Signed-off-by: John Snow <jsnow@redhat.com>
+Message-id: 20190523170643.20794-5-jsnow@redhat.com
+Reviewed-by: Max Reitz <mreitz@redhat.com>
+Signed-off-by: Max Reitz <mreitz@redhat.com>
+---
+ tests/qemu-iotests/iotests.py | 48 +++++++++++++++++++++--------------
+file changed, 29 insertions(+), 19 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 @@ class VM(qtest.QEMUQtestMachine):
+     # Returns None on success, and an error string on failure
+     def run_job(self, job, auto_finalize=True, auto_dismiss=False,
+                 pre_finalize=None, wait=60.0):
++        match_device = {'data': {'device': job}}
++        match_id = {'data': {'id': job}}
++        events = [
++            ('BLOCK_JOB_COMPLETED', match_device),
++            ('BLOCK_JOB_CANCELLED', match_device),
++            ('BLOCK_JOB_ERROR', match_device),
++            ('BLOCK_JOB_READY', match_device),
++            ('BLOCK_JOB_PENDING', match_id),
++            ('JOB_STATUS_CHANGE', match_id)
++        ]
+         error = None
+         while True:
+-            for ev in self.get_qmp_events_filtered(wait=wait):
+-                if ev['event'] == 'JOB_STATUS_CHANGE':
+-                    status = ev['data']['status']
+-                    if status == 'aborting':
+-                        result = self.qmp('query-jobs')
+-                        for j in result['return']:
+-                            if j['id'] == job:
+-                                error = j['error']
+-                                log('Job failed: %s' % (j['error']))
+-                    elif status == 'pending' and not auto_finalize:
+-                        if pre_finalize:
+-                            pre_finalize()
+-                        self.qmp_log('job-finalize', id=job)
+-                    elif status == 'concluded' and not auto_dismiss:
+-                        self.qmp_log('job-dismiss', id=job)
+-                    elif status == 'null':
+-                        return error
+-                else:
+-                    log(ev)
++            ev = filter_qmp_event(self.events_wait(events))
++            if ev['event'] != 'JOB_STATUS_CHANGE':
++                log(ev)
++                continue
++            status = ev['data']['status']
++            if status == 'aborting':
++                result = self.qmp('query-jobs')
++                for j in result['return']:
++                    if j['id'] == job:
++                        error = j['error']
++                        log('Job failed: %s' % (j['error']))
++            elif status == 'pending' and not auto_finalize:
++                if pre_finalize:
++                    pre_finalize()
++                self.qmp_log('job-finalize', id=job)
++            elif status == 'concluded' and not auto_dismiss:
++                self.qmp_log('job-dismiss', id=job)
++            elif status == 'null':
++                return error
+     def node_info(self, node_name):
+         nodes = self.qmp('query-named-block-nodes')
+--
+.21.0

-New patch
+[Qemu-devel] [PULL 05/20] iotests: add iotest 256 for testing blockdev-backup across iothread contexts
+From: John Snow <jsnow@redhat.com>
 Signed-off-by: John Snow <jsnow@redhat.com>
 Message-id: 20190523170643.20794-6-jsnow@redhat.com
 Reviewed-by: Max Reitz <mreitz@redhat.com>
 [mreitz: Moved from 250 to 256]
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  tests/qemu-iotests/256     | 122 +++++++++++++++++++++++++++++++++++++
  tests/qemu-iotests/256.out | 119 ++++++++++++++++++++++++++++++++++++
  tests/qemu-iotests/group   |   1 +
 files changed, 242 insertions(+)
  create mode 100755 tests/qemu-iotests/256
  create mode 100644 tests/qemu-iotests/256.out
 diff --git a/tests/qemu-iotests/256 b/tests/qemu-iotests/256
 new file mode 100755
 index XXXXXXX..XXXXXXX
 --- /dev/null
 +++ b/tests/qemu-iotests/256
@@ -XXX,XX +XXX,XX @@
 +#!/usr/bin/env python
 +#
 +# Test incremental/backup across iothread contexts
 +#
 +# Copyright (c) 2019 John Snow for Red Hat, Inc.
 +#
 +# This program is free software; you can redistribute it and/or modify
 +# it under the terms of the GNU General Public License as published by
 +# the Free Software Foundation; either version 2 of the License, or
 +# (at your option) any later version.
 +#
 +# This program is distributed in the hope that it will be useful,
 +# but WITHOUT ANY WARRANTY; without even the implied warranty of
 +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 +# GNU General Public License for more details.
 +#
 +# You should have received a copy of the GNU General Public License
 +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
 +#
 +# owner=jsnow@redhat.com
 +
 +import os
 +import iotests
 +from iotests import log
 +
 +iotests.verify_image_format(supported_fmts=['qcow2'])
 +size = 64 * 1024 * 1024
 +
 +with iotests.FilePath('img0') as img0_path, \
 +     iotests.FilePath('img1') as img1_path, \
 +     iotests.FilePath('img0-full') as img0_full_path, \
 +     iotests.FilePath('img1-full') as img1_full_path, \
 +     iotests.FilePath('img0-incr') as img0_incr_path, \
 +     iotests.FilePath('img1-incr') as img1_incr_path, \
 +     iotests.VM() as vm:
 +
 +    def create_target(filepath, name, size):
 +        basename = os.path.basename(filepath)
 +        nodename = "file_{}".format(basename)
 +        log(vm.command('blockdev-create', job_id='job1',
 +                       options={
 +                           'driver': 'file',
 +                           'filename': filepath,
 +                           'size': 0,
 +                       }))
 +        vm.run_job('job1')
 +        log(vm.command('blockdev-add', driver='file',
 +                       node_name=nodename, filename=filepath))
 +        log(vm.command('blockdev-create', job_id='job2',
 +                       options={
 +                           'driver': iotests.imgfmt,
 +                           'file': nodename,
 +                           'size': size,
 +                       }))
 +        vm.run_job('job2')
 +        log(vm.command('blockdev-add', driver=iotests.imgfmt,
 +                       node_name=name,
 +                       file=nodename))
 +
 +    log('--- Preparing images & VM ---\n')
 +    vm.add_object('iothread,id=iothread0')
 +    vm.add_object('iothread,id=iothread1')
 +    vm.add_device('virtio-scsi-pci,id=scsi0,iothread=iothread0')
 +    vm.add_device('virtio-scsi-pci,id=scsi1,iothread=iothread1')
 +    iotests.qemu_img_create('-f', iotests.imgfmt, img0_path, str(size))
 +    iotests.qemu_img_create('-f', iotests.imgfmt, img1_path, str(size))
 +    vm.add_drive(img0_path, interface='none')
 +    vm.add_device('scsi-hd,id=device0,drive=drive0,bus=scsi0.0')
 +    vm.add_drive(img1_path, interface='none')
 +    vm.add_device('scsi-hd,id=device1,drive=drive1,bus=scsi1.0')
 +
 +    log('--- Starting VM ---\n')
 +    vm.launch()
 +
 +    log('--- Create Targets & Full Backups ---\n')
 +    create_target(img0_full_path, 'img0-full', size)
 +    create_target(img1_full_path, 'img1-full', size)
 +    ret = vm.qmp_log('transaction', indent=2, actions=[
 +        { 'type': 'block-dirty-bitmap-add',
 +          'data': { 'node': 'drive0', 'name': 'bitmap0' }},
 +        { 'type': 'block-dirty-bitmap-add',
 +          'data': { 'node': 'drive1', 'name': 'bitmap1' }},
 +        { 'type': 'blockdev-backup',
 +          'data': { 'device': 'drive0',
 +                    'target': 'img0-full',
 +                    'sync': 'full',
 +                    'job-id': 'j0' }},
 +        { 'type': 'blockdev-backup',
 +          'data': { 'device': 'drive1',
 +                    'target': 'img1-full',
 +                    'sync': 'full',
 +                    'job-id': 'j1' }}
 +    ])
 +    if "error" in ret:
 +        raise Exception(ret['error']['desc'])
 +    vm.run_job('j0', auto_dismiss=True)
 +    vm.run_job('j1', auto_dismiss=True)
 +
 +    log('\n--- Create Targets & Incremental Backups ---\n')
 +    create_target(img0_incr_path, 'img0-incr', size)
 +    create_target(img1_incr_path, 'img1-incr', size)
 +    ret = vm.qmp_log('transaction', indent=2, actions=[
 +        { 'type': 'blockdev-backup',
 +          'data': { 'device': 'drive0',
 +                    'target': 'img0-incr',
 +                    'sync': 'incremental',
 +                    'bitmap': 'bitmap0',
 +                    'job-id': 'j2' }},
 +        { 'type': 'blockdev-backup',
 +          'data': { 'device': 'drive1',
 +                    'target': 'img1-incr',
 +                    'sync': 'incremental',
 +                    'bitmap': 'bitmap1',
 +                    'job-id': 'j3' }}
 +    ])
 +    if "error" in ret:
 +        raise Exception(ret['error']['desc'])
 +    vm.run_job('j2', auto_dismiss=True)
 +    vm.run_job('j3', auto_dismiss=True)
 +
 +    log('\n--- Done ---')
 +    vm.shutdown()
 diff --git a/tests/qemu-iotests/256.out b/tests/qemu-iotests/256.out
 new file mode 100644
 index XXXXXXX..XXXXXXX
 --- /dev/null
 +++ b/tests/qemu-iotests/256.out
@@ -XXX,XX +XXX,XX @@
 +--- Preparing images & VM ---
 +
 +--- Starting VM ---
 +
 +--- Create Targets & Full Backups ---
 +
 +{}
 +{"execute": "job-dismiss", "arguments": {"id": "job1"}}
 +{"return": {}}
 +{}
 +{}
 +{"execute": "job-dismiss", "arguments": {"id": "job2"}}
 +{"return": {}}
 +{}
 +{}
 +{"execute": "job-dismiss", "arguments": {"id": "job1"}}
 +{"return": {}}
 +{}
 +{}
 +{"execute": "job-dismiss", "arguments": {"id": "job2"}}
 +{"return": {}}
 +{}
 +{
 +  "execute": "transaction",
 +  "arguments": {
 +    "actions": [
 +      {
 +        "data": {
 +          "name": "bitmap0",
 +          "node": "drive0"
 +        },
 +        "type": "block-dirty-bitmap-add"
 +      },
 +      {
 +        "data": {
 +          "name": "bitmap1",
 +          "node": "drive1"
 +        },
 +        "type": "block-dirty-bitmap-add"
 +      },
 +      {
 +        "data": {
 +          "device": "drive0",
 +          "job-id": "j0",
 +          "sync": "full",
 +          "target": "img0-full"
 +        },
 +        "type": "blockdev-backup"
 +      },
 +      {
 +        "data": {
 +          "device": "drive1",
 +          "job-id": "j1",
 +          "sync": "full",
 +          "target": "img1-full"
 +        },
 +        "type": "blockdev-backup"
 +      }
 +    ]
 +  }
 +}
 +{
 +  "return": {}
 +}
 +{"data": {"device": "j0", "len": 67108864, "offset": 67108864, "speed": 0, "type": "backup"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
 +{"data": {"device": "j1", "len": 67108864, "offset": 67108864, "speed": 0, "type": "backup"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
 +
 +--- Create Targets & Incremental Backups ---
 +
 +{}
 +{"execute": "job-dismiss", "arguments": {"id": "job1"}}
 +{"return": {}}
 +{}
 +{}
 +{"execute": "job-dismiss", "arguments": {"id": "job2"}}
 +{"return": {}}
 +{}
 +{}
 +{"execute": "job-dismiss", "arguments": {"id": "job1"}}
 +{"return": {}}
 +{}
 +{}
 +{"execute": "job-dismiss", "arguments": {"id": "job2"}}
 +{"return": {}}
 +{}
 +{
 +  "execute": "transaction",
 +  "arguments": {
 +    "actions": [
 +      {
 +        "data": {
 +          "bitmap": "bitmap0",
 +          "device": "drive0",
 +          "job-id": "j2",
 +          "sync": "incremental",
 +          "target": "img0-incr"
 +        },
 +        "type": "blockdev-backup"
 +      },
 +      {
 +        "data": {
 +          "bitmap": "bitmap1",
 +          "device": "drive1",
 +          "job-id": "j3",
 +          "sync": "incremental",
 +          "target": "img1-incr"
 +        },
 +        "type": "blockdev-backup"
 +      }
 +    ]
 +  }
 +}
 +{
 +  "return": {}
 +}
 +{"data": {"device": "j2", "len": 67108864, "offset": 67108864, "speed": 0, "type": "backup"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
 +{"data": {"device": "j3", "len": 67108864, "offset": 67108864, "speed": 0, "type": "backup"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
 +
 +--- Done ---
 diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
 index XXXXXXX..XXXXXXX 100644
 --- a/tests/qemu-iotests/group
 +++ b/tests/qemu-iotests/group
@@ -XXX,XX +XXX,XX @@
 rw auto quick
 rw auto backing quick
 rw auto quick
 +256 rw auto quick
 --
 .21.0

-New patch
+[Qemu-devel] [PULL 06/20] event_match: always match on None value
+From: John Snow <jsnow@redhat.com>
+Before, event_match didn't always recurse if the event value was not a
+dictionary, and would instead check for equality immediately.
+By delaying equality checking to post-recursion, we can allow leaf
+values like "5" to match "None" and take advantage of the generic
+None-returns-True clause.
+This makes the matching a little more obviously consistent at the
+expense of being able to check for explicit None values, which is
+probably not that important given what this function is used for.
+Signed-off-by: John Snow <jsnow@redhat.com>
+Message-id: 20190528183857.26167-1-jsnow@redhat.com
+Signed-off-by: Max Reitz <mreitz@redhat.com>
+---
+ python/qemu/__init__.py | 24 ++++++++++++++----------
+file changed, 14 insertions(+), 10 deletions(-)
+diff --git a/python/qemu/__init__.py b/python/qemu/__init__.py
+index XXXXXXX..XXXXXXX 100644
+--- a/python/qemu/__init__.py
++++ b/python/qemu/__init__.py
+@@ -XXX,XX +XXX,XX @@ class QEMUMachine(object):
+         The match criteria takes the form of a matching subdict. The event is
+         checked to be a superset of the subdict, recursively, with matching
+-        values whenever those values are not None.
++        values whenever the subdict values are not None.
++
++        This has a limitation that you cannot explicitly check for None values.
+         Examples, with the subdict queries on the left:
+          - None matches any object.
+          - {"foo": None} matches {"foo": {"bar": 1}}
+-         - {"foo": {"baz": None}} does not match {"foo": {"bar": 1}}
+-         - {"foo": {"baz": 2}} matches {"foo": {"bar": 1, "baz": 2}}
++         - {"foo": None} matches {"foo": 5}
++         - {"foo": {"abc": None}} does not match {"foo": {"bar": 1}}
++         - {"foo": {"rab": 2}} matches {"foo": {"bar": 1, "rab": 2}}
+         """
+         if match is None:
+             return True
+-        for key in match:
+-            if key in event:
+-                if isinstance(event[key], dict):
++        try:
++            for key in match:
++                if key in event:
+                     if not QEMUMachine.event_match(event[key], match[key]):
+                         return False
+-                elif event[key] != match[key]:
++                else:
+                     return False
+-            else:
+-                return False
+-        return True
++            return True
++        except TypeError:
++            # either match or event wasn't iterable (not a dict)
++            return match == event
+     def event_wait(self, name, timeout=60.0, match=None):
+         """
+--
+.21.0

-[Qemu-devel] [PULL 03/12] blockjob: introduce block_job_early_fail
+[Qemu-devel] [PULL 07/20] iotests: Filter 175's allocation information
-From: Paolo Bonzini <pbonzini@redhat.com>
+It is possible for an empty file to take up blocks on a filesystem, for
 example:
-Outside blockjob.c, block_job_unref is only used when a block job fails
+$ qemu-img create -f raw test.img 1G
-to start, and block_job_ref is not used at all.  The reference counting
+Formatting 'test.img', fmt=raw size=1073741824
-thus is pretty well hidden.  Introduce a separate function to be used
+$ mkfs.ext4 -I 128 -q test.img
-by block jobs; because block_job_ref and block_job_unref now become
+$ mkdir test-mount
-static, move them earlier in blockjob.c.
+$ sudo mount -o loop test.img test-mount
 $ sudo touch test-mount/test-file
 $ stat -c 'blocks=%b' test-mount/test-file
 blocks=8
-Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
+These extra blocks (one cluster) are apparently used for metadata,
-Reviewed-by: John Snow <jsnow@redhat.com>
+because they are always there, on top of blocks used for data:
-Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
-Reviewed-by: Jeff Cody <jcody@redhat.com>
+$ sudo dd if=/dev/zero of=test-mount/test-file bs=1M count=1
-Message-id: 20170508141310.8674-4-pbonzini@redhat.com
++0 records in
-Signed-off-by: Jeff Cody <jcody@redhat.com>
++0 records out
 1048576 bytes (1.0 MB, 1.0 MiB) copied, 0.00135339 s, 775 MB/s
 $ stat -c 'blocks=%b' test-mount/test-file
 blocks=2056
 Make iotest 175 take this into account.
 Reported-by: Thomas Huth <thuth@redhat.com>
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 Reviewed-by: Eric Blake <eblake@redhat.com>
 Reviewed-by: Nir Soffer <nsoffer@redhat.com>
 Message-id: 20190516144319.12570-1-mreitz@redhat.com
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
- block/backup.c               |  2 +-
+ tests/qemu-iotests/175     | 26 ++++++++++++++++++++++----
- block/commit.c               |  2 +-
+ tests/qemu-iotests/175.out |  8 ++++----
- block/mirror.c               |  2 +-
+files changed, 26 insertions(+), 8 deletions(-)
  blockjob.c                   | 47 ++++++++++++++++++++++++++------------------
  include/block/blockjob_int.h | 15 +++-----------
  tests/test-blockjob.c        | 10 +++++-----
 files changed, 39 insertions(+), 39 deletions(-)
-diff --git a/block/backup.c b/block/backup.c
+diff --git a/tests/qemu-iotests/175 b/tests/qemu-iotests/175
-index XXXXXXX..XXXXXXX 100644
+index XXXXXXX..XXXXXXX 100755
---- a/block/backup.c
+--- a/tests/qemu-iotests/175
-+++ b/block/backup.c
++++ b/tests/qemu-iotests/175
-@@ -XXX,XX +XXX,XX @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
+@@ -XXX,XX +XXX,XX @@ status=1    # failure is the default!
-     }
-     if (job) {
+ _cleanup()
-         backup_clean(&job->common);
+ {
--        block_job_unref(&job->common);
+-    _cleanup_test_img
-+        block_job_early_fail(&job->common);
++    _cleanup_test_img
-     }
++    rm -f "$TEST_DIR/empty"
      return NULL;
 diff --git a/block/commit.c b/block/commit.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/commit.c
 +++ b/block/commit.c
@@ -XXX,XX +XXX,XX @@ fail:
      if (commit_top_bs) {
          bdrv_set_backing_hd(overlay_bs, top, &error_abort);
      }
 -    block_job_unref(&s->common);
 +    block_job_early_fail(&s->common);
  }
+ trap "_cleanup; exit \$status" 0 1 2 3 15
-diff --git a/block/mirror.c b/block/mirror.c
++# Some file systems sometimes allocate extra blocks independently of
-index XXXXXXX..XXXXXXX 100644
++# the file size.  This function hides the resulting difference in the
---- a/block/mirror.c
++# stat -c '%b' output.
-+++ b/block/mirror.c
++# Parameter 1: Number of blocks an empty file occupies
-@@ -XXX,XX +XXX,XX @@ fail:
++# Parameter 2: Image size in bytes
++_filter_blocks()
          g_free(s->replaces);
          blk_unref(s->target);
 -        block_job_unref(&s->common);
 +        block_job_early_fail(&s->common);
      }
      bdrv_child_try_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL,
 diff --git a/blockjob.c b/blockjob.c
 index XXXXXXX..XXXXXXX 100644
 --- a/blockjob.c
 +++ b/blockjob.c
@@ -XXX,XX +XXX,XX @@ BlockJob *block_job_get(const char *id)
      return NULL;
  }
 +static void block_job_ref(BlockJob *job)
 +{
-+    ++job->refcnt;
++    extra_blocks=$1
 +    img_size=$2
 +
 +    sed -e "s/blocks=$extra_blocks\\(\$\\|[^0-9]\\)/nothing allocated/" \
 +        -e "s/blocks=$((extra_blocks + img_size / 512))\\(\$\\|[^0-9]\\)/everything allocated/"
 +}
 +
-+static void block_job_attached_aio_context(AioContext *new_context,
+ # get standard environment, filters and checks
-+                                           void *opaque);
+ . ./common.rc
-+static void block_job_detach_aio_context(void *opaque);
+ . ./common.filter
@@ -XXX,XX +XXX,XX @@ _supported_fmt raw
  _supported_proto file
  _supported_os Linux
 -size=1m
 +size=$((1 * 1024 * 1024))
 +
-+static void block_job_unref(BlockJob *job)
++touch "$TEST_DIR/empty"
-+{
++extra_blocks=$(stat -c '%b' "$TEST_DIR/empty")
-+    if (--job->refcnt == 0) {
-+        BlockDriverState *bs = blk_bs(job->blk);
+ echo
-+        bs->job = NULL;
+ echo "== creating image with default preallocation =="
-+        block_job_remove_all_bdrv(job);
+ _make_test_img $size | _filter_imgfmt
-+        blk_remove_aio_context_notifier(job->blk,
+-stat -c "size=%s, blocks=%b" $TEST_IMG
-+                                        block_job_attached_aio_context,
++stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $size
-+                                        block_job_detach_aio_context, job);
-+        blk_unref(job->blk);
+ for mode in off full falloc; do
-+        error_free(job->blocker);
+     echo
-+        g_free(job->id);
+     echo "== creating image with preallocation $mode =="
-+        QLIST_REMOVE(job, job_list);
+     IMGOPTS=preallocation=$mode _make_test_img $size | _filter_imgfmt
-+        g_free(job);
+-    stat -c "size=%s, blocks=%b" $TEST_IMG
-+    }
++    stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $size
-+}
+ done
-+
- static void block_job_attached_aio_context(AioContext *new_context,
+ # success, all done
-                                            void *opaque)
+diff --git a/tests/qemu-iotests/175.out b/tests/qemu-iotests/175.out
  {
@@ -XXX,XX +XXX,XX @@ void block_job_start(BlockJob *job)
      bdrv_coroutine_enter(blk_bs(job->blk), job->co);
  }
 -void block_job_ref(BlockJob *job)
 +void block_job_early_fail(BlockJob *job)
  {
 -    ++job->refcnt;
 -}
 -
 -void block_job_unref(BlockJob *job)
 -{
 -    if (--job->refcnt == 0) {
 -        BlockDriverState *bs = blk_bs(job->blk);
 -        bs->job = NULL;
 -        block_job_remove_all_bdrv(job);
 -        blk_remove_aio_context_notifier(job->blk,
 -                                        block_job_attached_aio_context,
 -                                        block_job_detach_aio_context, job);
 -        blk_unref(job->blk);
 -        error_free(job->blocker);
 -        g_free(job->id);
 -        QLIST_REMOVE(job, job_list);
 -        g_free(job);
 -    }
 +    block_job_unref(job);
  }
  static void block_job_completed_single(BlockJob *job)
 diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
 index XXXXXXX..XXXXXXX 100644
---- a/include/block/blockjob_int.h
+--- a/tests/qemu-iotests/175.out
-+++ b/include/block/blockjob_int.h
++++ b/tests/qemu-iotests/175.out
-@@ -XXX,XX +XXX,XX @@ void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns);
+@@ -XXX,XX +XXX,XX @@ QA output created by 175
- void block_job_yield(BlockJob *job);
+ == creating image with default preallocation ==
- /**
+ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
-- * block_job_ref:
+-size=1048576, blocks=0
-+ * block_job_early_fail:
++size=1048576, nothing allocated
-  * @bs: The block device.
-  *
+ == creating image with preallocation off ==
-- * Grab a reference to the block job. Should be paired with block_job_unref.
+ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 preallocation=off
-+ * The block job could not be started, free it.
+-size=1048576, blocks=0
-  */
++size=1048576, nothing allocated
--void block_job_ref(BlockJob *job);
--
+ == creating image with preallocation full ==
--/**
+ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 preallocation=full
-- * block_job_unref:
+-size=1048576, blocks=2048
-- * @bs: The block device.
++size=1048576, everything allocated
-- *
-- * Release reference to the block job and release resources if it is the last
+ == creating image with preallocation falloc ==
-- * reference.
+ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 preallocation=falloc
-- */
+-size=1048576, blocks=2048
--void block_job_unref(BlockJob *job);
++size=1048576, everything allocated
-+void block_job_early_fail(BlockJob *job);
+  *** done
  /**
   * block_job_completed:
 diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c
 index XXXXXXX..XXXXXXX 100644
 --- a/tests/test-blockjob.c
 +++ b/tests/test-blockjob.c
@@ -XXX,XX +XXX,XX @@ static void test_job_ids(void)
      job[1] = do_test_id(blk[1], "id0", false);
      /* But once job[0] finishes we can reuse its ID */
 -    block_job_unref(job[0]);
 +    block_job_early_fail(job[0]);
      job[1] = do_test_id(blk[1], "id0", true);
      /* No job ID specified, defaults to the backend name ('drive1') */
 -    block_job_unref(job[1]);
 +    block_job_early_fail(job[1]);
      job[1] = do_test_id(blk[1], NULL, true);
      /* Duplicate job ID */
@@ -XXX,XX +XXX,XX @@ static void test_job_ids(void)
      /* This one is valid */
      job[2] = do_test_id(blk[2], "id_2", true);
 -    block_job_unref(job[0]);
 -    block_job_unref(job[1]);
 -    block_job_unref(job[2]);
 +    block_job_early_fail(job[0]);
 +    block_job_early_fail(job[1]);
 +    block_job_early_fail(job[2]);
      destroy_blk(blk[0]);
      destroy_blk(blk[1]);
 --
-.9.3
+.21.0

-New patch
+[Qemu-devel] [PULL 08/20] iotests: Fix intermittent failure in 219
+In 219, we wait for the job to make progress before we emit its status.
+This makes the output reliable.  We do not wait for any more progress if
+the job's current-progress already matches its total-progress.
+Unfortunately, there is a bug: Right after the job has been started,
+it's possible that total-progress is still 0.  In that case, we may skip
+the first progress-making step and keep ending up 64 kB short.
+To fix that bug, we can simply wait for total-progress to reach 4 MB
+(the image size) after starting the job.
+Reported-by: Karen Mezick <kmezick@redhat.com>
+Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1686651
+Signed-off-by: Max Reitz <mreitz@redhat.com>
+Message-id: 20190516161114.27596-1-mreitz@redhat.com
+Reviewed-by: John Snow <jsnow@redhat.com>
+[mreitz: Adjusted commit message as per John's proposal]
+Signed-off-by: Max Reitz <mreitz@redhat.com>
+---
+ tests/qemu-iotests/219 | 13 ++++++++++---
+file changed, 10 insertions(+), 3 deletions(-)
+diff --git a/tests/qemu-iotests/219 b/tests/qemu-iotests/219
+index XXXXXXX..XXXXXXX 100755
+--- a/tests/qemu-iotests/219
++++ b/tests/qemu-iotests/219
+@@ -XXX,XX +XXX,XX @@ import iotests
+ iotests.verify_image_format(supported_fmts=['qcow2'])
++img_size = 4 * 1024 * 1024
++
+ def pause_wait(vm, job_id):
+     with iotests.Timeout(3, "Timeout waiting for job to pause"):
+         while True:
+@@ -XXX,XX +XXX,XX @@ def test_pause_resume(vm):
+                 iotests.log(vm.qmp('query-jobs'))
+ def test_job_lifecycle(vm, job, job_args, has_ready=False):
++    global img_size
++
+     iotests.log('')
+     iotests.log('')
+     iotests.log('Starting block job: %s (auto-finalize: %s; auto-dismiss: %s)' %
+@@ -XXX,XX +XXX,XX @@ def test_job_lifecycle(vm, job, job_args, has_ready=False):
+     iotests.log(iotests.filter_qmp_event(vm.event_wait('JOB_STATUS_CHANGE')))
+     iotests.log(iotests.filter_qmp_event(vm.event_wait('JOB_STATUS_CHANGE')))
++    # Wait for total-progress to stabilize
++    while vm.qmp('query-jobs')['return'][0]['total-progress'] < img_size:
++        pass
++
+     # RUNNING state:
+     # pause/resume should work, complete/finalize/dismiss should error out
+     iotests.log('')
+@@ -XXX,XX +XXX,XX @@ with iotests.FilePath('disk.img') as disk_path, \
+      iotests.FilePath('copy.img') as copy_path, \
+      iotests.VM() as vm:
+-    img_size = '4M'
+-    iotests.qemu_img_create('-f', iotests.imgfmt, disk_path, img_size)
+-    iotests.qemu_io('-c', 'write 0 %s' % (img_size),
++    iotests.qemu_img_create('-f', iotests.imgfmt, disk_path, str(img_size))
++    iotests.qemu_io('-c', 'write 0 %i' % (img_size),
+                     '-f', iotests.imgfmt, disk_path)
+     iotests.log('Launching VM...')
+--
+.21.0

-[Qemu-devel] [PULL 12/12] block/gluster: glfs_lseek() workaround
+[Qemu-devel] [PULL 09/20] hw/block/fdc: floppy command FIFO memory initialization
-On current released versions of glusterfs, glfs_lseek() will sometimes
+From: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
 return invalid values for SEEK_DATA or SEEK_HOLE.  For SEEK_DATA and
 SEEK_HOLE, the returned value should be >= the passed offset, or < 0 in
 the case of error:
-LSEEK(2):
+The uninitialized memory allocated for the command FIFO of the
 floppy controller during the VM hardware initialization incurs
 many unwanted reports by Valgrind when VM state is being saved.
 That verbosity hardens a search for the real memory issues when
 the iotests run. Particularly, the patch eliminates 20 unnecessary
 reports of the Valgrind tool in the iotest #169.
-    off_t lseek(int fd, off_t offset, int whence);
+Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
 Message-id: 1559154027-282547-1-git-send-email-andrey.shinkevich@virtuozzo.com
 Reviewed-by: John Snow <jsnow@redhat.com>
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  hw/block/fdc.c | 1 +
 file changed, 1 insertion(+)
-    [...]
+diff --git a/hw/block/fdc.c b/hw/block/fdc.c
     SEEK_HOLE
               Adjust  the file offset to the next hole in the file greater
               than or equal to offset.  If offset points into the middle of
               a hole, then the file offset is set to offset.  If there is no
               hole past offset, then the file offset is adjusted to the end
               of the file (i.e., there is  an implicit hole at the end of
               any file).
     [...]
     RETURN VALUE
               Upon  successful  completion,  lseek()  returns  the resulting
               offset location as measured in bytes from the beginning of the
               file.  On error, the value (off_t) -1 is returned and errno is
               set to indicate the error
 However, occasionally glfs_lseek() for SEEK_HOLE/DATA will return a
 value less than the passed offset, yet greater than zero.
 For instance, here are example values observed from this call:
     offs = glfs_lseek(s->fd, start, SEEK_HOLE);
     if (offs < 0) {
         return -errno;          /* D1 and (H3 or H4) */
     }
 start == 7608336384
 offs == 7607877632
 This causes QEMU to abort on the assert test.  When this value is
 returned, errno is also 0.
 This is a reported and known bug to glusterfs:
 https://bugzilla.redhat.com/show_bug.cgi?id=1425293
 Although this is being fixed in gluster, we still should work around it
 in QEMU, given that multiple released versions of gluster behave this
 way.
 This patch treats the return case of (offs < start) the same as if an
 error value other than ENXIO is returned; we will assume we learned
 nothing, and there are no holes in the file.
 Signed-off-by: Jeff Cody <jcody@redhat.com>
 Reviewed-by: Eric Blake <eblake@redhat.com>
 Reviewed-by: Niels de Vos <ndevos@redhat.com>
 Message-id: 87c0140e9407c08f6e74b04131b610f2e27c014c.1495560397.git.jcody@redhat.com
 Signed-off-by: Jeff Cody <jcody@redhat.com>
 ---
  block/gluster.c | 18 ++++++++++++++++--
 file changed, 16 insertions(+), 2 deletions(-)
 diff --git a/block/gluster.c b/block/gluster.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/gluster.c
+--- a/hw/block/fdc.c
-+++ b/block/gluster.c
++++ b/hw/block/fdc.c
-@@ -XXX,XX +XXX,XX @@ static int find_allocation(BlockDriverState *bs, off_t start,
+@@ -XXX,XX +XXX,XX @@ static void fdctrl_realize_common(DeviceState *dev, FDCtrl *fdctrl,
-     if (offs < 0) {
-         return -errno;          /* D3 or D4 */
+     FLOPPY_DPRINTF("init controller\n");
-     }
+     fdctrl->fifo = qemu_memalign(512, FD_SECTOR_LEN);
--    assert(offs >= start);
++    memset(fdctrl->fifo, 0, FD_SECTOR_LEN);
-+
+     fdctrl->fifo_size = 512;
-+    if (offs < start) {
+     fdctrl->result_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
-+        /* This is not a valid return by lseek().  We are safe to just return
+                                              fdctrl_result_timer, fdctrl);
 +         * -EIO in this case, and we'll treat it like D4. Unfortunately some
 +         *  versions of gluster server will return offs < start, so an assert
 +         *  here will unnecessarily abort QEMU. */
 +        return -EIO;
 +    }
      if (offs > start) {
          /* D2: in hole, next data at offs */
@@ -XXX,XX +XXX,XX @@ static int find_allocation(BlockDriverState *bs, off_t start,
      if (offs < 0) {
          return -errno;          /* D1 and (H3 or H4) */
      }
 -    assert(offs >= start);
 +
 +    if (offs < start) {
 +        /* This is not a valid return by lseek().  We are safe to just return
 +         * -EIO in this case, and we'll treat it like H4. Unfortunately some
 +         *  versions of gluster server will return offs < start, so an assert
 +         *  here will unnecessarily abort QEMU. */
 +        return -EIO;
 +    }
      if (offs > start) {
          /*
 --
-.9.3
+.21.0

-[Qemu-devel] [PULL 07/12] blockjob: introduce block_job_cancel_async, check iostatus invariants
+[Qemu-devel] [PULL 10/20] iotests: restrict 254 to support only qcow2
-From: Paolo Bonzini <pbonzini@redhat.com>
+From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-The new functions helps respecting the invariant that the coroutine
+Test fails at least for qcow, because of different cluster sizes in
-is entered with false user_resume, zero pause count and no error
+base and top (and therefore different granularities of bitmaps we are
-recorded in the iostatus.
+trying to merge).
-Resetting the iostatus is now common to all of block_job_cancel_async,
+The test aim is to check block-dirty-bitmap-merge between different
-block_job_user_resume and block_job_iostatus_reset, albeit with slight
+nodes functionality, no needs to check all formats. So, let's just drop
-differences:
+support for anything except qcow2.
-- block_job_cancel_async resets the iostatus, and resumes the job if
+Reported-by: Max Reitz <mreitz@redhat.com>
-there was an error, but the coroutine is not restarted immediately.
+Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-For example the caller may continue with a call to block_job_finish_sync.
+Message-id: 20190605155405.104384-1-vsementsov@virtuozzo.com
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  tests/qemu-iotests/254 | 2 ++
 file changed, 2 insertions(+)
-- block_job_user_resume resets the iostatus.  It wants to resume the job
+diff --git a/tests/qemu-iotests/254 b/tests/qemu-iotests/254
-unconditionally, even if there was no error.
+index XXXXXXX..XXXXXXX 100755
+--- a/tests/qemu-iotests/254
-- block_job_iostatus_reset doesn't resume the job at all.  Maybe that's
++++ b/tests/qemu-iotests/254
-a bug but it should be fixed separately.
+@@ -XXX,XX +XXX,XX @@
+ import iotests
-block_job_iostatus_reset does the least common denominator, so add some
+ from iotests import qemu_img_create, file_path, log
-checking but otherwise leave it as the entry point for resetting the
-iostatus.
++iotests.verify_image_format(supported_fmts=['qcow2'])
 Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
 Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
 Message-id: 20170508141310.8674-8-pbonzini@redhat.com
 Signed-off-by: Jeff Cody <jcody@redhat.com>
 ---
  blockjob.c | 24 ++++++++++++++++++++----
 file changed, 20 insertions(+), 4 deletions(-)
 diff --git a/blockjob.c b/blockjob.c
 index XXXXXXX..XXXXXXX 100644
 --- a/blockjob.c
 +++ b/blockjob.c
@@ -XXX,XX +XXX,XX @@ static void block_job_completed_single(BlockJob *job)
      block_job_unref(job);
  }
 +static void block_job_cancel_async(BlockJob *job)
 +{
 +    if (job->iostatus != BLOCK_DEVICE_IO_STATUS_OK) {
 +        block_job_iostatus_reset(job);
 +    }
 +    if (job->user_paused) {
 +        /* Do not call block_job_enter here, the caller will handle it.  */
 +        job->user_paused = false;
 +        job->pause_count--;
 +    }
 +    job->cancelled = true;
 +}
 +
- static void block_job_completed_txn_abort(BlockJob *job)
+ disk, top = file_path('disk', 'top')
- {
+ size = 1024 * 1024
      AioContext *ctx;
@@ -XXX,XX +XXX,XX @@ static void block_job_completed_txn_abort(BlockJob *job)
               * them; this job, however, may or may not be cancelled, depending
               * on the caller, so leave it. */
              if (other_job != job) {
 -                other_job->cancelled = true;
 +                block_job_cancel_async(other_job);
              }
              continue;
          }
@@ -XXX,XX +XXX,XX @@ bool block_job_user_paused(BlockJob *job)
  void block_job_user_resume(BlockJob *job)
  {
      if (job && job->user_paused && job->pause_count > 0) {
 -        job->user_paused = false;
          block_job_iostatus_reset(job);
 +        job->user_paused = false;
          block_job_resume(job);
      }
  }
@@ -XXX,XX +XXX,XX @@ void block_job_user_resume(BlockJob *job)
  void block_job_cancel(BlockJob *job)
  {
      if (block_job_started(job)) {
 -        job->cancelled = true;
 -        block_job_iostatus_reset(job);
 +        block_job_cancel_async(job);
          block_job_enter(job);
      } else {
          block_job_completed(job, -ECANCELED);
@@ -XXX,XX +XXX,XX @@ void block_job_yield(BlockJob *job)
  void block_job_iostatus_reset(BlockJob *job)
  {
 +    if (job->iostatus == BLOCK_DEVICE_IO_STATUS_OK) {
 +        return;
 +    }
 +    assert(job->user_paused && job->pause_count > 0);
      job->iostatus = BLOCK_DEVICE_IO_STATUS_OK;
  }
 --
-.9.3
+.21.0

-[Qemu-devel] [PULL 11/12] blockjob: use deferred_to_main_loop to indicate the coroutine has ended
+[Qemu-devel] [PULL 11/20] qemu-img: Fix options leakage in img_rebase()
-From: Paolo Bonzini <pbonzini@redhat.com>
+img_rebase() can leak a QDict in two occasions.  Fix it.
-All block jobs are using block_job_defer_to_main_loop as the final
+Coverity: CID 1401416
-step just before the coroutine terminates.  At this point,
+Fixes: d16699b64671466b42079c45b89127aeea1ca565
-block_job_enter should do nothing, but currently it restarts
+Fixes: 330c72957196e0ae382abcaa97ebf4eb9bc8574f
-the freed coroutine.
+Signed-off-by: Max Reitz <mreitz@redhat.com>
 Message-id: 20190528195338.12376-1-mreitz@redhat.com
 Reviewed-by: John Snow <jsnow@redhat.com>
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  qemu-img.c | 3 +++
 file changed, 3 insertions(+)
-Now, the job->co states should probably be changed to an enum
+diff --git a/qemu-img.c b/qemu-img.c
 (e.g. BEFORE_START, STARTED, YIELDED, COMPLETED) subsuming
 block_job_started, job->deferred_to_main_loop and job->busy.
 For now, this patch eliminates the problematic reenter by
 removing the reset of job->deferred_to_main_loop (which served
 no purpose, as far as I could see) and checking the flag in
 block_job_enter.
 Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
 Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
 Message-id: 20170508141310.8674-12-pbonzini@redhat.com
 Signed-off-by: Jeff Cody <jcody@redhat.com>
 ---
  blockjob.c                   | 10 ++++++++--
  include/block/blockjob_int.h |  3 ++-
 files changed, 10 insertions(+), 3 deletions(-)
 diff --git a/blockjob.c b/blockjob.c
 index XXXXXXX..XXXXXXX 100644
---- a/blockjob.c
+--- a/qemu-img.c
-+++ b/blockjob.c
++++ b/qemu-img.c
-@@ -XXX,XX +XXX,XX @@ void block_job_resume_all(void)
+@@ -XXX,XX +XXX,XX @@ static int img_rebase(int argc, char **argv)
+                                                              out_baseimg,
- void block_job_enter(BlockJob *job)
+                                                              &local_err);
- {
+             if (local_err) {
--    if (job->co && !job->busy) {
++                qobject_unref(options);
-+    if (!block_job_started(job)) {
+                 error_reportf_err(local_err,
-+        return;
+                                   "Could not resolve backing filename: ");
-+    }
+                 ret = -1;
-+    if (job->deferred_to_main_loop) {
+@@ -XXX,XX +XXX,XX @@ static int img_rebase(int argc, char **argv)
-+        return;
+              */
-+    }
+             prefix_chain_bs = bdrv_find_backing_image(bs, out_real_path);
              if (prefix_chain_bs) {
 +                qobject_unref(options);
                  g_free(out_real_path);
 +
-+    if (!job->busy) {
+                 blk_new_backing = blk_new(qemu_get_aio_context(),
-         bdrv_coroutine_enter(blk_bs(job->blk), job->co);
+                                           BLK_PERM_CONSISTENT_READ,
-     }
+                                           BLK_PERM_ALL);
  }
@@ -XXX,XX +XXX,XX @@ static void block_job_defer_to_main_loop_bh(void *opaque)
          aio_context_acquire(aio_context);
      }
 -    data->job->deferred_to_main_loop = false;
      data->fn(data->job, data->opaque);
      if (aio_context != data->aio_context) {
 diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
 index XXXXXXX..XXXXXXX 100644
 --- a/include/block/blockjob_int.h
 +++ b/include/block/blockjob_int.h
@@ -XXX,XX +XXX,XX @@ typedef void BlockJobDeferToMainLoopFn(BlockJob *job, void *opaque);
   * @fn: The function to run in the main loop
   * @opaque: The opaque value that is passed to @fn
   *
 - * Execute a given function in the main loop with the BlockDriverState
 + * This function must be called by the main job coroutine just before it
 + * returns.  @fn is executed in the main loop with the BlockDriverState
   * AioContext acquired.  Block jobs must call bdrv_unref(), bdrv_close(), and
   * anything that uses bdrv_drain_all() in the main loop.
   *
 --
-.9.3
+.21.0

-[Qemu-devel] [PULL 10/12] blockjob: reorganize block_job_completed_txn_abort
+[Qemu-devel] [PULL 12/20] qapi/block-core: Overlays are not snapshots
-From: Paolo Bonzini <pbonzini@redhat.com>
+A snapshot is something that reflects the state of something at a
 certain point in time.  It does not change.
-This splits the part that touches job states from the part that invokes
+The file our snapshot commands create (or the node they install) is not
-callbacks.  It will make the code simpler to understand once job states will
+a snapshot, as it does change over time.  It is an overlay.  We cannot
-be protected by a different mutex than the AioContext lock.
+do anything about the parameter names, but we can at least adjust the
 descriptions to reflect that fact.
-Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
+Signed-off-by: Max Reitz <mreitz@redhat.com>
-Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
+Reviewed-by: Eric Blake <eblake@redhat.com>
-Message-id: 20170508141310.8674-11-pbonzini@redhat.com
+Message-id: 20190603202236.1342-2-mreitz@redhat.com
-Signed-off-by: Jeff Cody <jcody@redhat.com>
+Reviewed-by: John Snow <jsnow@redhat.com>
 Reviewed-by: Alberto Garcia <berto@igalia.com>
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
- blockjob.c | 35 ++++++++++++++++++++++-------------
+ qapi/block-core.json | 20 ++++++++++----------
-file changed, 22 insertions(+), 13 deletions(-)
+file changed, 10 insertions(+), 10 deletions(-)
-diff --git a/blockjob.c b/blockjob.c
+diff --git a/qapi/block-core.json b/qapi/block-core.json
 index XXXXXXX..XXXXXXX 100644
---- a/blockjob.c
+--- a/qapi/block-core.json
-+++ b/blockjob.c
++++ b/qapi/block-core.json
-@@ -XXX,XX +XXX,XX @@ void block_job_start(BlockJob *job)
+@@ -XXX,XX +XXX,XX @@
+ #
- static void block_job_completed_single(BlockJob *job)
+ # Either @device or @node-name must be set but not both.
- {
+ #
-+    assert(job->completed);
+-# @device: the name of the device to generate the snapshot from.
-+
++# @device: the name of the device to take a snapshot of.
-     if (!job->ret) {
+ #
-         if (job->driver->commit) {
+ # @node-name: graph node name to generate the snapshot from (Since 2.0)
-             job->driver->commit(job);
+ #
-@@ -XXX,XX +XXX,XX @@ static int block_job_finish_sync(BlockJob *job,
+-# @snapshot-file: the target of the new image. If the file exists, or
+-# if it is a device, the snapshot will be created in the existing
-     block_job_ref(job);
+-# file/device. Otherwise, a new file will be created.
++# @snapshot-file: the target of the new overlay image. If the file
--    finish(job, &local_err);
++# exists, or if it is a device, the overlay will be created in the
-+    if (finish) {
++# existing file/device. Otherwise, a new file will be created.
-+        finish(job, &local_err);
+ #
-+    }
+ # @snapshot-node-name: the graph node name of the new image (Since 2.0)
-     if (local_err) {
+ #
-         error_propagate(errp, local_err);
+-# @format: the format of the snapshot image, default is 'qcow2'.
-         block_job_unref(job);
++# @format: the format of the overlay image, default is 'qcow2'.
-@@ -XXX,XX +XXX,XX @@ static void block_job_completed_txn_abort(BlockJob *job)
+ #
- {
+ # @mode: whether and how QEMU should create a new image, default is
-     AioContext *ctx;
+ #        'absolute-paths'.
-     BlockJobTxn *txn = job->txn;
+@@ -XXX,XX +XXX,XX @@
--    BlockJob *other_job, *next;
+ ##
-+    BlockJob *other_job;
+ # @BlockdevSnapshot:
+ #
-     if (txn->aborting) {
+-# @node: device or node name that will have a snapshot created.
-         /*
++# @node: device or node name that will have a snapshot taken.
-@@ -XXX,XX +XXX,XX @@ static void block_job_completed_txn_abort(BlockJob *job)
+ #
-         return;
+ # @overlay: reference to the existing block device that will become
-     }
+-#           the overlay of @node, as part of creating the snapshot.
-     txn->aborting = true;
++#           the overlay of @node, as part of taking the snapshot.
-+    block_job_txn_ref(txn);
+ #           It must not have a current backing file (this can be
-+
+ #           achieved by passing "backing": null to blockdev-add).
-     /* We are the first failed job. Cancel other jobs. */
+ #
-     QLIST_FOREACH(other_job, &txn->jobs, txn_list) {
+@@ -XXX,XX +XXX,XX @@
-         ctx = blk_get_aio_context(other_job->blk);
+ ##
-         aio_context_acquire(ctx);
+ # @blockdev-snapshot-sync:
-     }
+ #
-+
+-# Generates a synchronous snapshot of a block device.
-+    /* Other jobs are effectively cancelled by us, set the status for
++# Takes a synchronous snapshot of a block device.
-+     * them; this job, however, may or may not be cancelled, depending
+ #
-+     * on the caller, so leave it. */
+ # For the arguments, see the documentation of BlockdevSnapshotSync.
-     QLIST_FOREACH(other_job, &txn->jobs, txn_list) {
+ #
--        if (other_job == job || other_job->completed) {
+@@ -XXX,XX +XXX,XX @@
--            /* Other jobs are "effectively" cancelled by us, set the status for
+ ##
--             * them; this job, however, may or may not be cancelled, depending
+ # @blockdev-snapshot:
--             * on the caller, so leave it. */
+ #
--            if (other_job != job) {
+-# Generates a snapshot of a block device.
--                block_job_cancel_async(other_job);
++# Takes a snapshot of a block device.
--            }
+ #
--            continue;
+-# Create a snapshot, by installing 'node' as the backing image of
-+        if (other_job != job) {
++# Take a snapshot, by installing 'node' as the backing image of
-+            block_job_cancel_async(other_job);
+ # 'overlay'. Additionally, if 'node' is associated with a block
-         }
+ # device, the block device changes to using 'overlay' as its new active
--        block_job_cancel_sync(other_job);
+ # image.
 -        assert(other_job->completed);
      }
 -    QLIST_FOREACH_SAFE(other_job, &txn->jobs, txn_list, next) {
 +    while (!QLIST_EMPTY(&txn->jobs)) {
 +        other_job = QLIST_FIRST(&txn->jobs);
          ctx = blk_get_aio_context(other_job->blk);
 +        if (!other_job->completed) {
 +            assert(other_job->cancelled);
 +            block_job_finish_sync(other_job, NULL, NULL);
 +        }
          block_job_completed_single(other_job);
          aio_context_release(ctx);
      }
 +
 +    block_job_txn_unref(txn);
  }
  static void block_job_completed_txn_success(BlockJob *job)
 --
-.9.3
+.21.0

-[Qemu-devel] [PULL 06/12] blockjob: move iostatus reset inside block_job_user_resume
+[Qemu-devel] [PULL 13/20] blockdev: Overlays are not snapshots
-From: Paolo Bonzini <pbonzini@redhat.com>
+There are error messages which refer to an overlay node as the snapshot.
 That is wrong, those are two different things.
-Outside blockjob.c, the block_job_iostatus_reset function is used once
+Signed-off-by: Max Reitz <mreitz@redhat.com>
-in the monitor and once in BlockBackend.  When we introduce the block
+Reviewed-by: Eric Blake <eblake@redhat.com>
-job mutex, block_job_iostatus_reset's client is going to be the block
+Message-id: 20190603202236.1342-3-mreitz@redhat.com
 layer (for which blockjob.c will take the block job mutex) rather than
 the monitor (which will take the block job mutex by itself).
 The monitor's call to block_job_iostatus_reset from the monitor comes
 just before the sole call to block_job_user_resume, so reset the
 iostatus directly from block_job_iostatus_reset.  This will avoid
 the need to introduce separate block_job_iostatus_reset and
 block_job_iostatus_reset_locked APIs.
 After making this change, move the function together with the others
 that were moved in the previous patch.
 Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
 Reviewed-by: John Snow <jsnow@redhat.com>
-Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
+Reviewed-by: Alberto Garcia <berto@igalia.com>
-Reviewed-by: Jeff Cody <jcody@redhat.com>
+Signed-off-by: Max Reitz <mreitz@redhat.com>
 Message-id: 20170508141310.8674-7-pbonzini@redhat.com
 Signed-off-by: Jeff Cody <jcody@redhat.com>
 ---
- blockdev.c |  1 -
+ blockdev.c                 | 10 +++++-----
- blockjob.c | 11 ++++++-----
+ tests/qemu-iotests/085.out | 10 +++++-----
-files changed, 6 insertions(+), 6 deletions(-)
+files changed, 10 insertions(+), 10 deletions(-)
 diff --git a/blockdev.c b/blockdev.c
 index XXXXXXX..XXXXXXX 100644
 --- a/blockdev.c
 +++ b/blockdev.c
-@@ -XXX,XX +XXX,XX @@ void qmp_block_job_resume(const char *device, Error **errp)
+@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
              s->has_snapshot_node_name ? s->snapshot_node_name : NULL;
          if (node_name && !snapshot_node_name) {
 -            error_setg(errp, "New snapshot node name missing");
 +            error_setg(errp, "New overlay node name missing");
              goto out;
          }
          if (snapshot_node_name &&
              bdrv_lookup_bs(snapshot_node_name, snapshot_node_name, NULL)) {
 -            error_setg(errp, "New snapshot node name already in use");
 +            error_setg(errp, "New overlay node name already in use");
              goto out;
          }
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
      }
-     trace_qmp_block_job_resume(job);
+     if (bdrv_has_blk(state->new_bs)) {
--    block_job_iostatus_reset(job);
+-        error_setg(errp, "The snapshot is already in use");
-     block_job_user_resume(job);
++        error_setg(errp, "The overlay is already in use");
-     aio_context_release(aio_context);
+         goto out;
- }
+     }
-diff --git a/blockjob.c b/blockjob.c
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
      }
      if (state->new_bs->backing != NULL) {
 -        error_setg(errp, "The snapshot already has a backing image");
 +        error_setg(errp, "The overlay already has a backing image");
          goto out;
      }
      if (!state->new_bs->drv->supports_backing) {
 -        error_setg(errp, "The snapshot does not support backing images");
 +        error_setg(errp, "The overlay does not support backing images");
          goto out;
      }
 diff --git a/tests/qemu-iotests/085.out b/tests/qemu-iotests/085.out
 index XXXXXXX..XXXXXXX 100644
---- a/blockjob.c
+--- a/tests/qemu-iotests/085.out
-+++ b/blockjob.c
++++ b/tests/qemu-iotests/085.out
-@@ -XXX,XX +XXX,XX @@ void block_job_user_resume(BlockJob *job)
+@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/
- {
-     if (job && job->user_paused && job->pause_count > 0) {
+ === Invalid command - cannot create a snapshot using a file BDS ===
-         job->user_paused = false;
-+        block_job_iostatus_reset(job);
+-{"error": {"class": "GenericError", "desc": "The snapshot does not support backing images"}}
-         block_job_resume(job);
++{"error": {"class": "GenericError", "desc": "The overlay does not support backing images"}}
-     }
- }
+ === Invalid command - snapshot node used as active layer ===
-@@ -XXX,XX +XXX,XX @@ void block_job_cancel(BlockJob *job)
-     }
+-{"error": {"class": "GenericError", "desc": "The snapshot is already in use"}}
- }
+-{"error": {"class": "GenericError", "desc": "The snapshot is already in use"}}
+-{"error": {"class": "GenericError", "desc": "The snapshot is already in use"}}
--void block_job_iostatus_reset(BlockJob *job)
++{"error": {"class": "GenericError", "desc": "The overlay is already in use"}}
--{
++{"error": {"class": "GenericError", "desc": "The overlay is already in use"}}
--    job->iostatus = BLOCK_DEVICE_IO_STATUS_OK;
++{"error": {"class": "GenericError", "desc": "The overlay is already in use"}}
--}
--
+ === Invalid command - snapshot node used as backing hd ===
- static int block_job_finish_sync(BlockJob *job,
-                                  void (*finish)(BlockJob *, Error **errp),
+@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/
-                                  Error **errp)
+ Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728
-@@ -XXX,XX +XXX,XX @@ void block_job_yield(BlockJob *job)
+ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
-     block_job_pause_point(job);
+ {"return": {}}
- }
+-{"error": {"class": "GenericError", "desc": "The snapshot already has a backing image"}}
++{"error": {"class": "GenericError", "desc": "The overlay already has a backing image"}}
-+void block_job_iostatus_reset(BlockJob *job)
-+{
+ === Invalid command - The node does not exist ===
-+    job->iostatus = BLOCK_DEVICE_IO_STATUS_OK;
 +}
 +
  void block_job_event_ready(BlockJob *job)
  {
      job->ready = true;
 --
-.9.3
+.21.0

-[Qemu-devel] [PULL 09/12] blockjob: strengthen a bit test-blockjob-txn
+[Qemu-devel] [PULL 14/20] qemu-img: Move quiet into ImgConvertState
-From: Paolo Bonzini <pbonzini@redhat.com>
+Move img_convert()'s quiet flag into the ImgConvertState so it is
 accessible by nested functions.  -q dictates that it suppresses anything
 but errors, so if those functions want to emit warnings, they need to
 query this flag first.  (There currently are no such warnings, but there
 will be as of the next patch.)
-Unlike test-blockjob-txn, QMP releases the reference to the transaction
+Signed-off-by: Max Reitz <mreitz@redhat.com>
-before the jobs finish.  Thus, qemu-iotest 124 showed a failure while
+Reviewed-by: Eric Blake <eblake@redhat.com>
-working on the next patch that the unit tests did not have.  Make
+Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-the test a little nastier.
+Message-id: 20190507203508.18026-2-mreitz@redhat.com
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  qemu-img.c | 13 +++++++------
 file changed, 7 insertions(+), 6 deletions(-)
-Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
+diff --git a/qemu-img.c b/qemu-img.c
 Reviewed-by: John Snow <jsnow@redhat.com>
 Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
 Message-id: 20170508141310.8674-10-pbonzini@redhat.com
 Signed-off-by: Jeff Cody <jcody@redhat.com>
 ---
  tests/test-blockjob-txn.c | 7 +++++--
 file changed, 5 insertions(+), 2 deletions(-)
 diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c
 index XXXXXXX..XXXXXXX 100644
---- a/tests/test-blockjob-txn.c
+--- a/qemu-img.c
-+++ b/tests/test-blockjob-txn.c
++++ b/qemu-img.c
-@@ -XXX,XX +XXX,XX @@ static void test_pair_jobs(int expected1, int expected2)
+@@ -XXX,XX +XXX,XX @@ typedef struct ImgConvertState {
-     block_job_start(job1);
+     int64_t target_backing_sectors; /* negative if unknown */
-     block_job_start(job2);
+     bool wr_in_order;
+     bool copy_range;
-+    /* Release our reference now to trigger as many nice
++    bool quiet;
-+     * use-after-free bugs as possible.
+     int min_sparse;
-+     */
+     int alignment;
-+    block_job_txn_unref(txn);
+     size_t cluster_sectors;
-+
+@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
-     if (expected1 == -ECANCELED) {
+     QDict *open_opts = NULL;
-         block_job_cancel(job1);
+     char *options = NULL;
      Error *local_err = NULL;
 -    bool writethrough, src_writethrough, quiet = false, image_opts = false,
 +    bool writethrough, src_writethrough, image_opts = false,
           skip_create = false, progress = false, tgt_image_opts = false;
      int64_t ret = -EINVAL;
      bool force_share = false;
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
              src_cache = optarg;
              break;
          case 'q':
 -            quiet = true;
 +            s.quiet = true;
              break;
          case 'n':
              skip_create = true;
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
      }
-@@ -XXX,XX +XXX,XX @@ static void test_pair_jobs(int expected1, int expected2)
+     /* Initialize before goto out */
-     g_assert_cmpint(result1, ==, expected1);
+-    if (quiet) {
-     g_assert_cmpint(result2, ==, expected2);
++    if (s.quiet) {
--
+         progress = false;
--    block_job_txn_unref(txn);
+     }
- }
+     qemu_progress_init(progress, 1.0);
+@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
- static void test_pair_jobs_success(void)
      for (bs_i = 0; bs_i < s.src_num; bs_i++) {
          s.src[bs_i] = img_open(image_opts, argv[optind + bs_i],
 -                               fmt, src_flags, src_writethrough, quiet,
 +                               fmt, src_flags, src_writethrough, s.quiet,
                                 force_share);
          if (!s.src[bs_i]) {
              ret = -1;
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
      if (skip_create) {
          s.target = img_open(tgt_image_opts, out_filename, out_fmt,
 -                            flags, writethrough, quiet, false);
 +                            flags, writethrough, s.quiet, false);
      } else {
          /* TODO ultimately we should allow --target-image-opts
           * to be used even when -n is not given.
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
           * to allow filenames in option syntax
           */
          s.target = img_open_file(out_filename, open_opts, out_fmt,
 -                                 flags, writethrough, quiet, false);
 +                                 flags, writethrough, s.quiet, false);
          open_opts = NULL; /* blk_new_open will have freed it */
      }
      if (!s.target) {
 --
-.9.3
+.21.0

-New patch
+[Qemu-devel] [PULL 15/20] qemu-img: Add salvaging mode to convert
+This adds a salvaging mode (--salvage) to qemu-img convert which ignores
 read errors and treats the respective areas as containing only zeroes.
 This can be used for instance to at least partially recover the data
 from terminally corrupted qcow2 images.
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
 Message-id: 20190507203508.18026-3-mreitz@redhat.com
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  qemu-img.c       | 90 +++++++++++++++++++++++++++++++++++++-----------
  qemu-img-cmds.hx |  4 +--
  qemu-img.texi    |  4 +++
 files changed, 75 insertions(+), 23 deletions(-)
 diff --git a/qemu-img.c b/qemu-img.c
 index XXXXXXX..XXXXXXX 100644
 --- a/qemu-img.c
 +++ b/qemu-img.c
@@ -XXX,XX +XXX,XX @@ enum {
      OPTION_SIZE = 264,
      OPTION_PREALLOCATION = 265,
      OPTION_SHRINK = 266,
 +    OPTION_SALVAGE = 267,
  };
  typedef enum OutputFormat {
@@ -XXX,XX +XXX,XX @@ typedef struct ImgConvertState {
      int64_t target_backing_sectors; /* negative if unknown */
      bool wr_in_order;
      bool copy_range;
 +    bool salvage;
      bool quiet;
      int min_sparse;
      int alignment;
@@ -XXX,XX +XXX,XX @@ static int convert_iteration_sectors(ImgConvertState *s, int64_t sector_num)
      }
      if (s->sector_next_status <= sector_num) {
 -        int64_t count = n * BDRV_SECTOR_SIZE;
 +        uint64_t offset = (sector_num - src_cur_offset) * BDRV_SECTOR_SIZE;
 +        int64_t count;
 -        if (s->target_has_backing) {
 +        do {
 +            count = n * BDRV_SECTOR_SIZE;
 +
 +            if (s->target_has_backing) {
 +                ret = bdrv_block_status(blk_bs(s->src[src_cur]), offset,
 +                                        count, &count, NULL, NULL);
 +            } else {
 +                ret = bdrv_block_status_above(blk_bs(s->src[src_cur]), NULL,
 +                                              offset, count, &count, NULL,
 +                                              NULL);
 +            }
 +
 +            if (ret < 0) {
 +                if (s->salvage) {
 +                    if (n == 1) {
 +                        if (!s->quiet) {
 +                            warn_report("error while reading block status at "
 +                                        "offset %" PRIu64 ": %s", offset,
 +                                        strerror(-ret));
 +                        }
 +                        /* Just try to read the data, then */
 +                        ret = BDRV_BLOCK_DATA;
 +                        count = BDRV_SECTOR_SIZE;
 +                    } else {
 +                        /* Retry on a shorter range */
 +                        n = DIV_ROUND_UP(n, 4);
 +                    }
 +                } else {
 +                    error_report("error while reading block status at offset "
 +                                 "%" PRIu64 ": %s", offset, strerror(-ret));
 +                    return ret;
 +                }
 +            }
 +        } while (ret < 0);
 -            ret = bdrv_block_status(blk_bs(s->src[src_cur]),
 -                                    (sector_num - src_cur_offset) *
 -                                    BDRV_SECTOR_SIZE,
 -                                    count, &count, NULL, NULL);
 -        } else {
 -            ret = bdrv_block_status_above(blk_bs(s->src[src_cur]), NULL,
 -                                          (sector_num - src_cur_offset) *
 -                                          BDRV_SECTOR_SIZE,
 -                                          count, &count, NULL, NULL);
 -        }
 -        if (ret < 0) {
 -            error_report("error while reading block status of sector %" PRId64
 -                         ": %s", sector_num, strerror(-ret));
 -            return ret;
 -        }
          n = DIV_ROUND_UP(count, BDRV_SECTOR_SIZE);
          if (ret & BDRV_BLOCK_ZERO) {
@@ -XXX,XX +XXX,XX @@ static int convert_iteration_sectors(ImgConvertState *s, int64_t sector_num)
  static int coroutine_fn convert_co_read(ImgConvertState *s, int64_t sector_num,
                                          int nb_sectors, uint8_t *buf)
  {
 +    uint64_t single_read_until = 0;
      int n, ret;
      assert(nb_sectors <= s->buf_sectors);
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn convert_co_read(ImgConvertState *s, int64_t sector_num,
          BlockBackend *blk;
          int src_cur;
          int64_t bs_sectors, src_cur_offset;
 +        uint64_t offset;
          /* In the case of compression with multiple source files, we can get a
           * nb_sectors that spreads into the next part. So we must be able to
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn convert_co_read(ImgConvertState *s, int64_t sector_num,
          blk = s->src[src_cur];
          bs_sectors = s->src_sectors[src_cur];
 +        offset = (sector_num - src_cur_offset) << BDRV_SECTOR_BITS;
 +
          n = MIN(nb_sectors, bs_sectors - (sector_num - src_cur_offset));
 +        if (single_read_until > offset) {
 +            n = 1;
 +        }
 -        ret = blk_co_pread(
 -                blk, (sector_num - src_cur_offset) << BDRV_SECTOR_BITS,
 -                n << BDRV_SECTOR_BITS, buf, 0);
 +        ret = blk_co_pread(blk, offset, n << BDRV_SECTOR_BITS, buf, 0);
          if (ret < 0) {
 -            return ret;
 +            if (s->salvage) {
 +                if (n > 1) {
 +                    single_read_until = offset + (n << BDRV_SECTOR_BITS);
 +                    continue;
 +                } else {
 +                    if (!s->quiet) {
 +                        warn_report("error while reading offset %" PRIu64
 +                                    ": %s", offset, strerror(-ret));
 +                    }
 +                    memset(buf, 0, BDRV_SECTOR_SIZE);
 +                }
 +            } else {
 +                return ret;
 +            }
          }
          sector_num += n;
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
              {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
              {"force-share", no_argument, 0, 'U'},
              {"target-image-opts", no_argument, 0, OPTION_TARGET_IMAGE_OPTS},
 +            {"salvage", no_argument, 0, OPTION_SALVAGE},
              {0, 0, 0, 0}
          };
          c = getopt_long(argc, argv, ":hf:O:B:Cco:l:S:pt:T:qnm:WU",
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
          case OPTION_IMAGE_OPTS:
              image_opts = true;
              break;
 +        case OPTION_SALVAGE:
 +            s.salvage = true;
 +            break;
          case OPTION_TARGET_IMAGE_OPTS:
              tgt_image_opts = true;
              break;
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
          goto fail_getopt;
      }
 +    if (s.copy_range && s.salvage) {
 +        error_report("Cannot use copy offloading in salvaging mode");
 +        goto fail_getopt;
 +    }
 +
      if (tgt_image_opts && !skip_create) {
          error_report("--target-image-opts requires use of -n flag");
          goto fail_getopt;
 diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
 index XXXXXXX..XXXXXXX 100644
 --- a/qemu-img-cmds.hx
 +++ b/qemu-img-cmds.hx
@@ -XXX,XX +XXX,XX @@ STEXI
  ETEXI
  DEF("convert", img_convert,
 -    "convert [--object objectdef] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")
 +    "convert [--object objectdef] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] [--salvage] filename [filename2 [...]] output_filename")
  STEXI
 -@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
 +@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] [--salvage] @var{filename} [@var{filename2} [...]] @var{output_filename}
  ETEXI
  DEF("create", img_create,
 diff --git a/qemu-img.texi b/qemu-img.texi
 index XXXXXXX..XXXXXXX 100644
 --- a/qemu-img.texi
 +++ b/qemu-img.texi
@@ -XXX,XX +XXX,XX @@ improve performance if the data is remote, such as with NFS or iSCSI backends,
  but will not automatically sparsify zero sectors, and may result in a fully
  allocated target image depending on the host support for getting allocation
  information.
 +@item --salvage
 +Try to ignore I/O errors when reading.  Unless in quiet mode (@code{-q}), errors
 +will still be printed.  Areas that cannot be read from the source will be
 +treated as containing only zeroes.
  @end table
  Parameters to dd subcommand:
 --
 .21.0

-New patch
+[Qemu-devel] [PULL 16/20] blkdebug: Add @iotype error option
+This new error option allows users of blkdebug to inject errors only on
 certain kinds of I/O operations.  Users usually want to make a very
 specific operation fail, not just any; but right now they simply hope
 that the event that triggers the error injection is followed up with
 that very operation.  That may not be true, however, because the block
 layer is changing (including blkdebug, which may increase the number of
 types of I/O operations on which to inject errors).
 The new option's default has been chosen to keep backwards
 compatibility.
 Note that similar to the internal representation, we could choose to
 expose this option as a list of I/O types.  But there is no practical
 use for this, because as described above, users usually know exactly
 which kind of operation they want to make fail, so there is no need to
 specify multiple I/O types at once.  In addition, exposing this option
 as a list would require non-trivial changes to qemu_opts_absorb_qdict().
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
 Message-id: 20190507203508.18026-4-mreitz@redhat.com
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  qapi/block-core.json | 26 +++++++++++++++++++++++
  block/blkdebug.c     | 50 ++++++++++++++++++++++++++++++++++++--------
 files changed, 67 insertions(+), 9 deletions(-)
 diff --git a/qapi/block-core.json b/qapi/block-core.json
 index XXXXXXX..XXXXXXX 100644
 --- a/qapi/block-core.json
 +++ b/qapi/block-core.json
@@ -XXX,XX +XXX,XX @@
              'l1_shrink_write_table', 'l1_shrink_free_l2_clusters',
              'cor_write', 'cluster_alloc_space'] }
 +##
 +# @BlkdebugIOType:
 +#
 +# Kinds of I/O that blkdebug can inject errors in.
 +#
 +# @read: .bdrv_co_preadv()
 +#
 +# @write: .bdrv_co_pwritev()
 +#
 +# @write-zeroes: .bdrv_co_pwrite_zeroes()
 +#
 +# @discard: .bdrv_co_pdiscard()
 +#
 +# @flush: .bdrv_co_flush_to_disk()
 +#
 +# Since: 4.1
 +##
 +{ 'enum': 'BlkdebugIOType', 'prefix': 'BLKDEBUG_IO_TYPE',
 +  'data': [ 'read', 'write', 'write-zeroes', 'discard', 'flush' ] }
 +
  ##
  # @BlkdebugInjectErrorOptions:
  #
@@ -XXX,XX +XXX,XX @@
  # @state:       the state identifier blkdebug needs to be in to
  #               actually trigger the event; defaults to "any"
  #
 +# @iotype:      the type of I/O operations on which this error should
 +#               be injected; defaults to "all read, write,
 +#               write-zeroes, discard, and flush operations"
 +#               (since: 4.1)
 +#
  # @errno:       error identifier (errno) to be returned; defaults to
  #               EIO
  #
@@ -XXX,XX +XXX,XX @@
  { 'struct': 'BlkdebugInjectErrorOptions',
    'data': { 'event': 'BlkdebugEvent',
              '*state': 'int',
 +            '*iotype': 'BlkdebugIOType',
              '*errno': 'int',
              '*sector': 'int',
              '*once': 'bool',
 diff --git a/block/blkdebug.c b/block/blkdebug.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/blkdebug.c
 +++ b/block/blkdebug.c
@@ -XXX,XX +XXX,XX @@ typedef struct BlkdebugRule {
      int state;
      union {
          struct {
 +            uint64_t iotype_mask;
              int error;
              int immediately;
              int once;
@@ -XXX,XX +XXX,XX @@ typedef struct BlkdebugRule {
      QSIMPLEQ_ENTRY(BlkdebugRule) active_next;
  } BlkdebugRule;
 +QEMU_BUILD_BUG_MSG(BLKDEBUG_IO_TYPE__MAX > 64,
 +                   "BlkdebugIOType mask does not fit into an uint64_t");
 +
  static QemuOptsList inject_error_opts = {
      .name = "inject-error",
      .head = QTAILQ_HEAD_INITIALIZER(inject_error_opts.head),
@@ -XXX,XX +XXX,XX @@ static QemuOptsList inject_error_opts = {
              .name = "state",
              .type = QEMU_OPT_NUMBER,
          },
 +        {
 +            .name = "iotype",
 +            .type = QEMU_OPT_STRING,
 +        },
          {
              .name = "errno",
              .type = QEMU_OPT_NUMBER,
@@ -XXX,XX +XXX,XX @@ static int add_rule(void *opaque, QemuOpts *opts, Error **errp)
      int event;
      struct BlkdebugRule *rule;
      int64_t sector;
 +    BlkdebugIOType iotype;
 +    Error *local_error = NULL;
      /* Find the right event for the rule */
      event_name = qemu_opt_get(opts, "event");
@@ -XXX,XX +XXX,XX @@ static int add_rule(void *opaque, QemuOpts *opts, Error **errp)
          sector = qemu_opt_get_number(opts, "sector", -1);
          rule->options.inject.offset =
              sector == -1 ? -1 : sector * BDRV_SECTOR_SIZE;
 +
 +        iotype = qapi_enum_parse(&BlkdebugIOType_lookup,
 +                                 qemu_opt_get(opts, "iotype"),
 +                                 BLKDEBUG_IO_TYPE__MAX, &local_error);
 +        if (local_error) {
 +            error_propagate(errp, local_error);
 +            return -1;
 +        }
 +        if (iotype != BLKDEBUG_IO_TYPE__MAX) {
 +            rule->options.inject.iotype_mask = (1ull << iotype);
 +        } else {
 +            /* Apply the default */
 +            rule->options.inject.iotype_mask =
 +                (1ull << BLKDEBUG_IO_TYPE_READ)
 +                | (1ull << BLKDEBUG_IO_TYPE_WRITE)
 +                | (1ull << BLKDEBUG_IO_TYPE_WRITE_ZEROES)
 +                | (1ull << BLKDEBUG_IO_TYPE_DISCARD)
 +                | (1ull << BLKDEBUG_IO_TYPE_FLUSH);
 +        }
 +
          break;
      case ACTION_SET_STATE:
@@ -XXX,XX +XXX,XX @@ out:
      return ret;
  }
 -static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes)
 +static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
 +                      BlkdebugIOType iotype)
  {
      BDRVBlkdebugState *s = bs->opaque;
      BlkdebugRule *rule = NULL;
@@ -XXX,XX +XXX,XX @@ static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes)
      QSIMPLEQ_FOREACH(rule, &s->active_rules, active_next) {
          uint64_t inject_offset = rule->options.inject.offset;
 -        if (inject_offset == -1 ||
 -            (bytes && inject_offset >= offset &&
 -             inject_offset < offset + bytes))
 +        if ((inject_offset == -1 ||
 +             (bytes && inject_offset >= offset &&
 +              inject_offset < offset + bytes)) &&
 +            (rule->options.inject.iotype_mask & (1ull << iotype)))
          {
              break;
          }
@@ -XXX,XX +XXX,XX @@ blkdebug_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
          assert(bytes <= bs->bl.max_transfer);
      }
 -    err = rule_check(bs, offset, bytes);
 +    err = rule_check(bs, offset, bytes, BLKDEBUG_IO_TYPE_READ);
      if (err) {
          return err;
      }
@@ -XXX,XX +XXX,XX @@ blkdebug_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
          assert(bytes <= bs->bl.max_transfer);
      }
 -    err = rule_check(bs, offset, bytes);
 +    err = rule_check(bs, offset, bytes, BLKDEBUG_IO_TYPE_WRITE);
      if (err) {
          return err;
      }
@@ -XXX,XX +XXX,XX @@ blkdebug_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
  static int blkdebug_co_flush(BlockDriverState *bs)
  {
 -    int err = rule_check(bs, 0, 0);
 +    int err = rule_check(bs, 0, 0, BLKDEBUG_IO_TYPE_FLUSH);
      if (err) {
          return err;
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn blkdebug_co_pwrite_zeroes(BlockDriverState *bs,
          assert(bytes <= bs->bl.max_pwrite_zeroes);
      }
 -    err = rule_check(bs, offset, bytes);
 +    err = rule_check(bs, offset, bytes, BLKDEBUG_IO_TYPE_WRITE_ZEROES);
      if (err) {
          return err;
      }
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn blkdebug_co_pdiscard(BlockDriverState *bs,
          assert(bytes <= bs->bl.max_pdiscard);
      }
 -    err = rule_check(bs, offset, bytes);
 +    err = rule_check(bs, offset, bytes, BLKDEBUG_IO_TYPE_DISCARD);
      if (err) {
          return err;
      }
 --
 .21.0

-[Qemu-devel] [PULL 04/12] blockjob: introduce block_job_pause/resume_all
+[Qemu-devel] [PULL 17/20] blkdebug: Add "none" event
-From: Paolo Bonzini <pbonzini@redhat.com>
+Together with @iotypes and @sector, this can be used to trap e.g. the
 first read or write access to a certain sector without having to know
 what happens internally in the block layer, i.e. which "real" events
 happen right before such an access.
-Remove use of block_job_pause/resume from outside blockjob.c, thus
+Signed-off-by: Max Reitz <mreitz@redhat.com>
-making them static.  The new functions are used by the block layer,
+Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-so place them in blockjob_int.h.
+Message-id: 20190507203508.18026-5-mreitz@redhat.com
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  qapi/block-core.json | 4 +++-
  block/blkdebug.c     | 2 ++
 files changed, 5 insertions(+), 1 deletion(-)
-Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
+diff --git a/qapi/block-core.json b/qapi/block-core.json
 Reviewed-by: John Snow <jsnow@redhat.com>
 Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
 Reviewed-by: Jeff Cody <jcody@redhat.com>
 Message-id: 20170508141310.8674-5-pbonzini@redhat.com
 Signed-off-by: Jeff Cody <jcody@redhat.com>
 ---
  block/io.c                   |  19 ++------
  blockjob.c                   | 114 ++++++++++++++++++++++++++-----------------
  include/block/blockjob.h     |  16 ------
  include/block/blockjob_int.h |  14 ++++++
 files changed, 86 insertions(+), 77 deletions(-)
 diff --git a/block/io.c b/block/io.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/io.c
+--- a/qapi/block-core.json
-+++ b/block/io.c
++++ b/qapi/block-core.json
 @@ -XXX,XX +XXX,XX @@
- #include "trace.h"
+ #
- #include "sysemu/block-backend.h"
+ # @cluster_alloc_space: an allocation of file space for a cluster (since 4.1)
- #include "block/blockjob.h"
+ #
-+#include "block/blockjob_int.h"
++# @none: triggers once at creation of the blkdebug node (since 4.1)
- #include "block/block_int.h"
++#
- #include "qemu/cutils.h"
+ # Since: 2.9
- #include "qapi/error.h"
+ ##
-@@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_begin(void)
+ { 'enum': 'BlkdebugEvent', 'prefix': 'BLKDBG',
-     bool waited = true;
+@@ -XXX,XX +XXX,XX @@
-     BlockDriverState *bs;
+             'pwritev_rmw_tail', 'pwritev_rmw_after_tail', 'pwritev',
-     BdrvNextIterator it;
+             'pwritev_zero', 'pwritev_done', 'empty_image_prepare',
--    BlockJob *job = NULL;
+             'l1_shrink_write_table', 'l1_shrink_free_l2_clusters',
-     GSList *aio_ctxs = NULL, *ctx;
+-            'cor_write', 'cluster_alloc_space'] }
++            'cor_write', 'cluster_alloc_space', 'none'] }
--    while ((job = block_job_next(job))) {
--        AioContext *aio_context = blk_get_aio_context(job->blk);
+ ##
--
+ # @BlkdebugIOType:
--        aio_context_acquire(aio_context);
+diff --git a/block/blkdebug.c b/block/blkdebug.c
--        block_job_pause(job);
+index XXXXXXX..XXXXXXX 100644
--        aio_context_release(aio_context);
+--- a/block/blkdebug.c
--    }
++++ b/block/blkdebug.c
-+    block_job_pause_all();
+@@ -XXX,XX +XXX,XX @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
+         goto out;
      for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
          AioContext *aio_context = bdrv_get_aio_context(bs);
@@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_end(void)
  {
      BlockDriverState *bs;
      BdrvNextIterator it;
 -    BlockJob *job = NULL;
      for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
          AioContext *aio_context = bdrv_get_aio_context(bs);
@@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_end(void)
          aio_context_release(aio_context);
      }
--    while ((job = block_job_next(job))) {
++    bdrv_debug_event(bs, BLKDBG_NONE);
 -        AioContext *aio_context = blk_get_aio_context(job->blk);
 -
 -        aio_context_acquire(aio_context);
 -        block_job_resume(job);
 -        aio_context_release(aio_context);
 -    }
 +    block_job_resume_all();
  }
  void bdrv_drain_all(void)
 diff --git a/blockjob.c b/blockjob.c
 index XXXXXXX..XXXXXXX 100644
 --- a/blockjob.c
 +++ b/blockjob.c
@@ -XXX,XX +XXX,XX @@ struct BlockJobTxn {
  static QLIST_HEAD(, BlockJob) block_jobs = QLIST_HEAD_INITIALIZER(block_jobs);
 -static char *child_job_get_parent_desc(BdrvChild *c)
 -{
 -    BlockJob *job = c->opaque;
 -    return g_strdup_printf("%s job '%s'",
 -                           BlockJobType_lookup[job->driver->job_type],
 -                           job->id);
 -}
 -
 -static const BdrvChildRole child_job = {
 -    .get_parent_desc    = child_job_get_parent_desc,
 -    .stay_at_node       = true,
 -};
 -
 -static void block_job_drained_begin(void *opaque)
 -{
 -    BlockJob *job = opaque;
 -    block_job_pause(job);
 -}
 -
 -static void block_job_drained_end(void *opaque)
 -{
 -    BlockJob *job = opaque;
 -    block_job_resume(job);
 -}
 -
 -static const BlockDevOps block_job_dev_ops = {
 -    .drained_begin = block_job_drained_begin,
 -    .drained_end = block_job_drained_end,
 -};
 -
  BlockJob *block_job_next(BlockJob *job)
  {
      if (!job) {
@@ -XXX,XX +XXX,XX @@ BlockJob *block_job_get(const char *id)
      return NULL;
  }
 +static void block_job_pause(BlockJob *job)
 +{
 +    job->pause_count++;
 +}
 +
-+static void block_job_resume(BlockJob *job)
+     ret = 0;
-+{
+ out:
-+    assert(job->pause_count > 0);
+     if (ret < 0) {
 +    job->pause_count--;
 +    if (job->pause_count) {
 +        return;
 +    }
 +    block_job_enter(job);
 +}
 +
  static void block_job_ref(BlockJob *job)
  {
      ++job->refcnt;
@@ -XXX,XX +XXX,XX @@ static void block_job_detach_aio_context(void *opaque)
      block_job_unref(job);
  }
 +static char *child_job_get_parent_desc(BdrvChild *c)
 +{
 +    BlockJob *job = c->opaque;
 +    return g_strdup_printf("%s job '%s'",
 +                           BlockJobType_lookup[job->driver->job_type],
 +                           job->id);
 +}
 +
 +static const BdrvChildRole child_job = {
 +    .get_parent_desc    = child_job_get_parent_desc,
 +    .stay_at_node       = true,
 +};
 +
 +static void block_job_drained_begin(void *opaque)
 +{
 +    BlockJob *job = opaque;
 +    block_job_pause(job);
 +}
 +
 +static void block_job_drained_end(void *opaque)
 +{
 +    BlockJob *job = opaque;
 +    block_job_resume(job);
 +}
 +
 +static const BlockDevOps block_job_dev_ops = {
 +    .drained_begin = block_job_drained_begin,
 +    .drained_end = block_job_drained_end,
 +};
 +
  void block_job_remove_all_bdrv(BlockJob *job)
  {
      GSList *l;
@@ -XXX,XX +XXX,XX @@ void block_job_complete(BlockJob *job, Error **errp)
      job->driver->complete(job, errp);
  }
 -void block_job_pause(BlockJob *job)
 -{
 -    job->pause_count++;
 -}
 -
  void block_job_user_pause(BlockJob *job)
  {
      job->user_paused = true;
@@ -XXX,XX +XXX,XX @@ void coroutine_fn block_job_pause_point(BlockJob *job)
      }
  }
 -void block_job_resume(BlockJob *job)
 -{
 -    assert(job->pause_count > 0);
 -    job->pause_count--;
 -    if (job->pause_count) {
 -        return;
 -    }
 -    block_job_enter(job);
 -}
 -
  void block_job_user_resume(BlockJob *job)
  {
      if (job && job->user_paused && job->pause_count > 0) {
@@ -XXX,XX +XXX,XX @@ static void block_job_event_completed(BlockJob *job, const char *msg)
                                          &error_abort);
  }
 +void block_job_pause_all(void)
 +{
 +    BlockJob *job = NULL;
 +    while ((job = block_job_next(job))) {
 +        AioContext *aio_context = blk_get_aio_context(job->blk);
 +
 +        aio_context_acquire(aio_context);
 +        block_job_pause(job);
 +        aio_context_release(aio_context);
 +    }
 +}
 +
 +void block_job_resume_all(void)
 +{
 +    BlockJob *job = NULL;
 +    while ((job = block_job_next(job))) {
 +        AioContext *aio_context = blk_get_aio_context(job->blk);
 +
 +        aio_context_acquire(aio_context);
 +        block_job_resume(job);
 +        aio_context_release(aio_context);
 +    }
 +}
 +
  void block_job_event_ready(BlockJob *job)
  {
      job->ready = true;
 diff --git a/include/block/blockjob.h b/include/block/blockjob.h
 index XXXXXXX..XXXXXXX 100644
 --- a/include/block/blockjob.h
 +++ b/include/block/blockjob.h
@@ -XXX,XX +XXX,XX @@ void block_job_complete(BlockJob *job, Error **errp);
  BlockJobInfo *block_job_query(BlockJob *job, Error **errp);
  /**
 - * block_job_pause:
 - * @job: The job to be paused.
 - *
 - * Asynchronously pause the specified job.
 - */
 -void block_job_pause(BlockJob *job);
 -
 -/**
   * block_job_user_pause:
   * @job: The job to be paused.
   *
@@ -XXX,XX +XXX,XX @@ void block_job_user_pause(BlockJob *job);
  bool block_job_user_paused(BlockJob *job);
  /**
 - * block_job_resume:
 - * @job: The job to be resumed.
 - *
 - * Resume the specified job.  Must be paired with a preceding block_job_pause.
 - */
 -void block_job_resume(BlockJob *job);
 -
 -/**
   * block_job_user_resume:
   * @job: The job to be resumed.
   *
 diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
 index XXXXXXX..XXXXXXX 100644
 --- a/include/block/blockjob_int.h
 +++ b/include/block/blockjob_int.h
@@ -XXX,XX +XXX,XX @@ void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns);
  void block_job_yield(BlockJob *job);
  /**
 + * block_job_pause_all:
 + *
 + * Asynchronously pause all jobs.
 + */
 +void block_job_pause_all(void);
 +
 +/**
 + * block_job_resume_all:
 + *
 + * Resume all block jobs.  Must be paired with a preceding block_job_pause_all.
 + */
 +void block_job_resume_all(void);
 +
 +/**
   * block_job_early_fail:
   * @bs: The block device.
   *
 --
-.9.3
+.21.0

-[Qemu-devel] [PULL 05/12] blockjob: separate monitor and blockjob APIs
+[Qemu-devel] [PULL 18/20] blkdebug: Inject errors on .bdrv_co_block_status()
-From: Paolo Bonzini <pbonzini@redhat.com>
+Signed-off-by: Max Reitz <mreitz@redhat.com>
 Reviewed-by: Eric Blake <eblake@redhat.com>
 Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
 Message-id: 20190507203508.18026-6-mreitz@redhat.com
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  qapi/block-core.json | 5 ++++-
  block/blkdebug.c     | 8 ++++++++
 files changed, 12 insertions(+), 1 deletion(-)
-We have two different headers for block job operations, blockjob.h
+diff --git a/qapi/block-core.json b/qapi/block-core.json
 and blockjob_int.h.  The former contains APIs called by the monitor,
 the latter contains APIs called by the block job drivers and the
 block layer itself.
 Keep the two APIs separate in the blockjob.c file too.  This will
 be useful when transitioning away from the AioContext lock, because
 there will be locking policies for the two categories, too---the
 monitor will have to call new block_job_lock/unlock APIs, while blockjob
 APIs will take care of this for the users.
 Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
 Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
 Message-id: 20170508141310.8674-6-pbonzini@redhat.com
 Signed-off-by: Jeff Cody <jcody@redhat.com>
 ---
  blockjob.c | 390 ++++++++++++++++++++++++++++++++-----------------------------
 file changed, 205 insertions(+), 185 deletions(-)
 diff --git a/blockjob.c b/blockjob.c
 index XXXXXXX..XXXXXXX 100644
---- a/blockjob.c
+--- a/qapi/block-core.json
-+++ b/blockjob.c
++++ b/qapi/block-core.json
-@@ -XXX,XX +XXX,XX @@ struct BlockJobTxn {
+@@ -XXX,XX +XXX,XX @@
+ #
- static QLIST_HEAD(, BlockJob) block_jobs = QLIST_HEAD_INITIALIZER(block_jobs);
+ # @flush: .bdrv_co_flush_to_disk()
+ #
-+/*
++# @block-status: .bdrv_co_block_status()
-+ * The block job API is composed of two categories of functions.
++#
-+ *
+ # Since: 4.1
-+ * The first includes functions used by the monitor.  The monitor is
+ ##
-+ * peculiar in that it accesses the block job list with block_job_get, and
+ { 'enum': 'BlkdebugIOType', 'prefix': 'BLKDEBUG_IO_TYPE',
-+ * therefore needs consistency across block_job_get and the actual operation
+-  'data': [ 'read', 'write', 'write-zeroes', 'discard', 'flush' ] }
-+ * (e.g. block_job_set_speed).  The consistency is achieved with
++  'data': [ 'read', 'write', 'write-zeroes', 'discard', 'flush',
-+ * aio_context_acquire/release.  These functions are declared in blockjob.h.
++            'block-status' ] }
-+ *
-+ * The second includes functions used by the block job drivers and sometimes
+ ##
-+ * by the core block layer.  These do not care about locking, because the
+ # @BlkdebugInjectErrorOptions:
-+ * whole coroutine runs under the AioContext lock, and are declared in
+diff --git a/block/blkdebug.c b/block/blkdebug.c
-+ * blockjob_int.h.
+index XXXXXXX..XXXXXXX 100644
-+ */
+--- a/block/blkdebug.c
 +++ b/block/blkdebug.c
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn blkdebug_co_block_status(BlockDriverState *bs,
                                                   int64_t *map,
                                                   BlockDriverState **file)
  {
 +    int err;
 +
- BlockJob *block_job_next(BlockJob *job)
+     assert(QEMU_IS_ALIGNED(offset | bytes, bs->bl.request_alignment));
  {
      if (!job) {
@@ -XXX,XX +XXX,XX @@ int block_job_add_bdrv(BlockJob *job, const char *name, BlockDriverState *bs,
      return 0;
  }
 -void *block_job_create(const char *job_id, const BlockJobDriver *driver,
 -                       BlockDriverState *bs, uint64_t perm,
 -                       uint64_t shared_perm, int64_t speed, int flags,
 -                       BlockCompletionFunc *cb, void *opaque, Error **errp)
 -{
 -    BlockBackend *blk;
 -    BlockJob *job;
 -    int ret;
 -
 -    if (bs->job) {
 -        error_setg(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(bs));
 -        return NULL;
 -    }
 -
 -    if (job_id == NULL && !(flags & BLOCK_JOB_INTERNAL)) {
 -        job_id = bdrv_get_device_name(bs);
 -        if (!*job_id) {
 -            error_setg(errp, "An explicit job ID is required for this node");
 -            return NULL;
 -        }
 -    }
 -
 -    if (job_id) {
 -        if (flags & BLOCK_JOB_INTERNAL) {
 -            error_setg(errp, "Cannot specify job ID for internal block job");
 -            return NULL;
 -        }
 -
 -        if (!id_wellformed(job_id)) {
 -            error_setg(errp, "Invalid job ID '%s'", job_id);
 -            return NULL;
 -        }
 -
 -        if (block_job_get(job_id)) {
 -            error_setg(errp, "Job ID '%s' already in use", job_id);
 -            return NULL;
 -        }
 -    }
 -
 -    blk = blk_new(perm, shared_perm);
 -    ret = blk_insert_bs(blk, bs, errp);
 -    if (ret < 0) {
 -        blk_unref(blk);
 -        return NULL;
 -    }
 -
 -    job = g_malloc0(driver->instance_size);
 -    job->driver        = driver;
 -    job->id            = g_strdup(job_id);
 -    job->blk           = blk;
 -    job->cb            = cb;
 -    job->opaque        = opaque;
 -    job->busy          = false;
 -    job->paused        = true;
 -    job->pause_count   = 1;
 -    job->refcnt        = 1;
 -
 -    error_setg(&job->blocker, "block device is in use by block job: %s",
 -               BlockJobType_lookup[driver->job_type]);
 -    block_job_add_bdrv(job, "main node", bs, 0, BLK_PERM_ALL, &error_abort);
 -    bs->job = job;
 -
 -    blk_set_dev_ops(blk, &block_job_dev_ops, job);
 -    bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker);
 -
 -    QLIST_INSERT_HEAD(&block_jobs, job, job_list);
 -
 -    blk_add_aio_context_notifier(blk, block_job_attached_aio_context,
 -                                 block_job_detach_aio_context, job);
 -
 -    /* Only set speed when necessary to avoid NotSupported error */
 -    if (speed != 0) {
 -        Error *local_err = NULL;
 -
 -        block_job_set_speed(job, speed, &local_err);
 -        if (local_err) {
 -            block_job_unref(job);
 -            error_propagate(errp, local_err);
 -            return NULL;
 -        }
 -    }
 -    return job;
 -}
 -
  bool block_job_is_internal(BlockJob *job)
  {
      return (job->id == NULL);
@@ -XXX,XX +XXX,XX @@ void block_job_start(BlockJob *job)
      bdrv_coroutine_enter(blk_bs(job->blk), job->co);
  }
 -void block_job_early_fail(BlockJob *job)
 -{
 -    block_job_unref(job);
 -}
 -
  static void block_job_completed_single(BlockJob *job)
  {
      if (!job->ret) {
@@ -XXX,XX +XXX,XX @@ static void block_job_completed_txn_success(BlockJob *job)
      }
  }
 -void block_job_completed(BlockJob *job, int ret)
 -{
 -    assert(blk_bs(job->blk)->job == job);
 -    assert(!job->completed);
 -    job->completed = true;
 -    job->ret = ret;
 -    if (!job->txn) {
 -        block_job_completed_single(job);
 -    } else if (ret < 0 || block_job_is_cancelled(job)) {
 -        block_job_completed_txn_abort(job);
 -    } else {
 -        block_job_completed_txn_success(job);
 -    }
 -}
 -
  void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)
  {
      Error *local_err = NULL;
@@ -XXX,XX +XXX,XX @@ void block_job_user_pause(BlockJob *job)
      block_job_pause(job);
  }
 -static bool block_job_should_pause(BlockJob *job)
 -{
 -    return job->pause_count > 0;
 -}
 -
  bool block_job_user_paused(BlockJob *job)
  {
      return job->user_paused;
  }
 -void coroutine_fn block_job_pause_point(BlockJob *job)
 -{
 -    assert(job && block_job_started(job));
 -
 -    if (!block_job_should_pause(job)) {
 -        return;
 -    }
 -    if (block_job_is_cancelled(job)) {
 -        return;
 -    }
 -
 -    if (job->driver->pause) {
 -        job->driver->pause(job);
 -    }
 -
 -    if (block_job_should_pause(job) && !block_job_is_cancelled(job)) {
 -        job->paused = true;
 -        job->busy = false;
 -        qemu_coroutine_yield(); /* wait for block_job_resume() */
 -        job->busy = true;
 -        job->paused = false;
 -    }
 -
 -    if (job->driver->resume) {
 -        job->driver->resume(job);
 -    }
 -}
 -
  void block_job_user_resume(BlockJob *job)
  {
      if (job && job->user_paused && job->pause_count > 0) {
@@ -XXX,XX +XXX,XX @@ void block_job_user_resume(BlockJob *job)
      }
  }
 -void block_job_enter(BlockJob *job)
 -{
 -    if (job->co && !job->busy) {
 -        bdrv_coroutine_enter(blk_bs(job->blk), job->co);
 -    }
 -}
 -
  void block_job_cancel(BlockJob *job)
  {
      if (block_job_started(job)) {
@@ -XXX,XX +XXX,XX @@ void block_job_cancel(BlockJob *job)
      }
  }
 -bool block_job_is_cancelled(BlockJob *job)
 -{
 -    return job->cancelled;
 -}
 -
  void block_job_iostatus_reset(BlockJob *job)
  {
      job->iostatus = BLOCK_DEVICE_IO_STATUS_OK;
@@ -XXX,XX +XXX,XX @@ int block_job_complete_sync(BlockJob *job, Error **errp)
      return block_job_finish_sync(job, &block_job_complete, errp);
  }
 -void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns)
 -{
 -    assert(job->busy);
 -
 -    /* Check cancellation *before* setting busy = false, too!  */
 -    if (block_job_is_cancelled(job)) {
 -        return;
 -    }
 -
 -    job->busy = false;
 -    if (!block_job_should_pause(job)) {
 -        co_aio_sleep_ns(blk_get_aio_context(job->blk), type, ns);
 -    }
 -    job->busy = true;
 -
 -    block_job_pause_point(job);
 -}
 -
 -void block_job_yield(BlockJob *job)
 -{
 -    assert(job->busy);
 -
 -    /* Check cancellation *before* setting busy = false, too!  */
 -    if (block_job_is_cancelled(job)) {
 -        return;
 -    }
 -
 -    job->busy = false;
 -    if (!block_job_should_pause(job)) {
 -        qemu_coroutine_yield();
 -    }
 -    job->busy = true;
 -
 -    block_job_pause_point(job);
 -}
 -
  BlockJobInfo *block_job_query(BlockJob *job, Error **errp)
  {
      BlockJobInfo *info;
@@ -XXX,XX +XXX,XX @@ static void block_job_event_completed(BlockJob *job, const char *msg)
                                          &error_abort);
  }
 +/*
 + * API for block job drivers and the block layer.  These functions are
 + * declared in blockjob_int.h.
 + */
 +
-+void *block_job_create(const char *job_id, const BlockJobDriver *driver,
++    err = rule_check(bs, offset, bytes, BLKDEBUG_IO_TYPE_BLOCK_STATUS);
-+                       BlockDriverState *bs, uint64_t perm,
++    if (err) {
-+                       uint64_t shared_perm, int64_t speed, int flags,
++        return err;
 +                       BlockCompletionFunc *cb, void *opaque, Error **errp)
 +{
 +    BlockBackend *blk;
 +    BlockJob *job;
 +    int ret;
 +
 +    if (bs->job) {
 +        error_setg(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(bs));
 +        return NULL;
 +    }
 +
-+    if (job_id == NULL && !(flags & BLOCK_JOB_INTERNAL)) {
+     return bdrv_co_block_status_from_file(bs, want_zero, offset, bytes,
-+        job_id = bdrv_get_device_name(bs);
+                                           pnum, map, file);
 +        if (!*job_id) {
 +            error_setg(errp, "An explicit job ID is required for this node");
 +            return NULL;
 +        }
 +    }
 +
 +    if (job_id) {
 +        if (flags & BLOCK_JOB_INTERNAL) {
 +            error_setg(errp, "Cannot specify job ID for internal block job");
 +            return NULL;
 +        }
 +
 +        if (!id_wellformed(job_id)) {
 +            error_setg(errp, "Invalid job ID '%s'", job_id);
 +            return NULL;
 +        }
 +
 +        if (block_job_get(job_id)) {
 +            error_setg(errp, "Job ID '%s' already in use", job_id);
 +            return NULL;
 +        }
 +    }
 +
 +    blk = blk_new(perm, shared_perm);
 +    ret = blk_insert_bs(blk, bs, errp);
 +    if (ret < 0) {
 +        blk_unref(blk);
 +        return NULL;
 +    }
 +
 +    job = g_malloc0(driver->instance_size);
 +    job->driver        = driver;
 +    job->id            = g_strdup(job_id);
 +    job->blk           = blk;
 +    job->cb            = cb;
 +    job->opaque        = opaque;
 +    job->busy          = false;
 +    job->paused        = true;
 +    job->pause_count   = 1;
 +    job->refcnt        = 1;
 +
 +    error_setg(&job->blocker, "block device is in use by block job: %s",
 +               BlockJobType_lookup[driver->job_type]);
 +    block_job_add_bdrv(job, "main node", bs, 0, BLK_PERM_ALL, &error_abort);
 +    bs->job = job;
 +
 +    blk_set_dev_ops(blk, &block_job_dev_ops, job);
 +    bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker);
 +
 +    QLIST_INSERT_HEAD(&block_jobs, job, job_list);
 +
 +    blk_add_aio_context_notifier(blk, block_job_attached_aio_context,
 +                                 block_job_detach_aio_context, job);
 +
 +    /* Only set speed when necessary to avoid NotSupported error */
 +    if (speed != 0) {
 +        Error *local_err = NULL;
 +
 +        block_job_set_speed(job, speed, &local_err);
 +        if (local_err) {
 +            block_job_unref(job);
 +            error_propagate(errp, local_err);
 +            return NULL;
 +        }
 +    }
 +    return job;
 +}
 +
  void block_job_pause_all(void)
  {
      BlockJob *job = NULL;
@@ -XXX,XX +XXX,XX @@ void block_job_pause_all(void)
      }
  }
-+void block_job_early_fail(BlockJob *job)
-+{
-+    block_job_unref(job);
-+}
-+
-+void block_job_completed(BlockJob *job, int ret)
-+{
-+    assert(blk_bs(job->blk)->job == job);
-+    assert(!job->completed);
-+    job->completed = true;
-+    job->ret = ret;
-+    if (!job->txn) {
-+        block_job_completed_single(job);
-+    } else if (ret < 0 || block_job_is_cancelled(job)) {
-+        block_job_completed_txn_abort(job);
-+    } else {
-+        block_job_completed_txn_success(job);
-+    }
-+}
-+
-+static bool block_job_should_pause(BlockJob *job)
-+{
-+    return job->pause_count > 0;
-+}
-+
-+void coroutine_fn block_job_pause_point(BlockJob *job)
-+{
-+    assert(job && block_job_started(job));
-+
-+    if (!block_job_should_pause(job)) {
-+        return;
-+    }
-+    if (block_job_is_cancelled(job)) {
-+        return;
-+    }
-+
-+    if (job->driver->pause) {
-+        job->driver->pause(job);
-+    }
-+
-+    if (block_job_should_pause(job) && !block_job_is_cancelled(job)) {
-+        job->paused = true;
-+        job->busy = false;
-+        qemu_coroutine_yield(); /* wait for block_job_resume() */
-+        job->busy = true;
-+        job->paused = false;
-+    }
-+
-+    if (job->driver->resume) {
-+        job->driver->resume(job);
-+    }
-+}
-+
- void block_job_resume_all(void)
- {
-     BlockJob *job = NULL;
-@@ -XXX,XX +XXX,XX @@ void block_job_resume_all(void)
-     }
- }
-+void block_job_enter(BlockJob *job)
-+{
-+    if (job->co && !job->busy) {
-+        bdrv_coroutine_enter(blk_bs(job->blk), job->co);
-+    }
-+}
-+
-+bool block_job_is_cancelled(BlockJob *job)
-+{
-+    return job->cancelled;
-+}
-+
-+void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns)
-+{
-+    assert(job->busy);
-+
-+    /* Check cancellation *before* setting busy = false, too!  */
-+    if (block_job_is_cancelled(job)) {
-+        return;
-+    }
-+
-+    job->busy = false;
-+    if (!block_job_should_pause(job)) {
-+        co_aio_sleep_ns(blk_get_aio_context(job->blk), type, ns);
-+    }
-+    job->busy = true;
-+
-+    block_job_pause_point(job);
-+}
-+
-+void block_job_yield(BlockJob *job)
-+{
-+    assert(job->busy);
-+
-+    /* Check cancellation *before* setting busy = false, too!  */
-+    if (block_job_is_cancelled(job)) {
-+        return;
-+    }
-+
-+    job->busy = false;
-+    if (!block_job_should_pause(job)) {
-+        qemu_coroutine_yield();
-+    }
-+    job->busy = true;
-+
-+    block_job_pause_point(job);
-+}
-+
- void block_job_event_ready(BlockJob *job)
- {
-     job->ready = true;
 --
-.9.3
+.21.0

-New patch
+[Qemu-devel] [PULL 19/20] iotests: Test qemu-img convert --salvage
+This test converts a simple image to another, but blkdebug injects
 block_status and read faults at some offsets.  The resulting image
 should be the same as the input image, except that sectors that could
 not be read have to be 0.
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 Message-id: 20190507203508.18026-7-mreitz@redhat.com
 Tested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
 Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
 [mreitz: Dropped superfluous printf from _filter_offsets, as suggested
          by Vladimir; disable test for VDI and IMGOPTSSYNTAX]
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  tests/qemu-iotests/251     | 170 +++++++++++++++++++++++++++++++++++++
  tests/qemu-iotests/251.out |  43 ++++++++++
  tests/qemu-iotests/group   |   1 +
 files changed, 214 insertions(+)
  create mode 100755 tests/qemu-iotests/251
  create mode 100644 tests/qemu-iotests/251.out
 diff --git a/tests/qemu-iotests/251 b/tests/qemu-iotests/251
 new file mode 100755
 index XXXXXXX..XXXXXXX
 --- /dev/null
 +++ b/tests/qemu-iotests/251
@@ -XXX,XX +XXX,XX @@
 +#!/usr/bin/env bash
 +#
 +# Test qemu-img convert --salvage
 +#
 +# Copyright (C) 2019 Red Hat, Inc.
 +#
 +# This program is free software; you can redistribute it and/or modify
 +# it under the terms of the GNU General Public License as published by
 +# the Free Software Foundation; either version 2 of the License, or
 +# (at your option) any later version.
 +#
 +# This program is distributed in the hope that it will be useful,
 +# but WITHOUT ANY WARRANTY; without even the implied warranty of
 +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 +# GNU General Public License for more details.
 +#
 +# You should have received a copy of the GNU General Public License
 +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
 +#
 +
 +# creator
 +owner=mreitz@redhat.com
 +
 +seq=$(basename $0)
 +echo "QA output created by $seq"
 +
 +status=1    # failure is the default!
 +
 +_cleanup()
 +{
 +    _cleanup_test_img
 +}
 +trap "_cleanup; exit \$status" 0 1 2 3 15
 +
 +# get standard environment, filters and checks
 +. ./common.rc
 +. ./common.filter
 +. ./common.qemu
 +
 +_supported_fmt generic
 +_supported_proto file
 +_supported_os Linux
 +
 +if [ "$IMGOPTSSYNTAX" = "true" ]; then
 +    # We use json:{} filenames here, so we cannot work with additional options.
 +    _unsupported_fmt $IMGFMT
 +else
 +    # With VDI, the output is ordered differently.  Just disable it.
 +    _unsupported_fmt vdi
 +fi
 +
 +
 +TEST_IMG="$TEST_IMG.orig" _make_test_img 64M
 +
 +$QEMU_IO -c 'write -P 42 0 64M' "$TEST_IMG.orig" | _filter_qemu_io
 +
 +
 +sector_size=512
 +
 +# Offsets on which to fail block-status.  Keep in ascending order so
 +# the indexing done by _filter_offsets will appear in ascending order
 +# in the output as well.
 +status_fail_offsets="$((16 * 1024 * 1024 + 8192))
 +                     $((33 * 1024 * 1024 + 512))"
 +
 +# Offsets on which to fail reads.  Keep in ascending order for the
 +# same reason.
 +# The second element is shared with $status_fail_offsets on purpose.
 +# Starting with the third element, we test what happens when a
 +# continuous range of sectors is inaccessible.
 +read_fail_offsets="$((32 * 1024 * 1024 - 65536))
 +                   $((33 * 1024 * 1024 + 512))
 +                   $(seq $((34 * 1024 * 1024)) $sector_size \
 +                         $((34 * 1024 * 1024 + 4096 - $sector_size)))"
 +
 +
 +# blkdebug must be above the format layer so it can intercept all
 +# block-status events
 +source_img="json:{'driver': 'blkdebug',
 +                  'image': {
 +                      'driver': '$IMGFMT',
 +                      'file': {
 +                          'driver': 'file',
 +                          'filename': '$TEST_IMG.orig'
 +                      }
 +                  },
 +                  'inject-error': ["
 +
 +for ofs in $status_fail_offsets
 +do
 +    source_img+="{ 'event': 'none',
 +                   'iotype': 'block-status',
 +                   'errno': 5,
 +                   'sector': $((ofs / sector_size)) },"
 +done
 +
 +for ofs in $read_fail_offsets
 +do
 +    source_img+="{ 'event': 'none',
 +                   'iotype': 'read',
 +                   'errno': 5,
 +                   'sector': $((ofs / sector_size)) },"
 +done
 +
 +# Remove the trailing comma and terminate @inject-error and json:{}
 +source_img="${source_img%,} ] }"
 +
 +
 +echo
 +
 +
 +_filter_offsets() {
 +    filters=
 +
 +    index=0
 +    for ofs in $1
 +    do
 +        filters+=" -e s/$ofs/status_fail_offset_$index/"
 +        index=$((index + 1))
 +    done
 +
 +    index=0
 +    for ofs in $2
 +    do
 +        filters+=" -e s/$ofs/read_fail_offset_$index/"
 +        index=$((index + 1))
 +    done
 +
 +    sed $filters
 +}
 +
 +# While determining the number of allocated sectors in the input
 +# image, we should see one block status warning per element of
 +# $status_fail_offsets.
 +#
 +# Then, the image is read.  Since the block status is queried in
 +# basically the same way, the same warnings as in the previous step
 +# should reappear.  Interleaved with those we should see a read
 +# warning per element of $read_fail_offsets.
 +# Note that $read_fail_offsets and $status_fail_offsets share an
 +# element (read_fail_offset_1 == status_fail_offset_1), so
 +# "status_fail_offset_1" in the output is the same as
 +# "read_fail_offset_1".
 +$QEMU_IMG convert --salvage "$source_img" "$TEST_IMG" 2>&1 \
 +    | _filter_offsets "$status_fail_offsets" "$read_fail_offsets"
 +
 +echo
 +
 +# The offsets where the block status could not be determined should
 +# have been treated as containing data and thus should be correct in
 +# the output image.
 +# The offsets where reading failed altogether should be 0.  Make them
 +# 0 in the input image, too, so we can compare both images.
 +for ofs in $read_fail_offsets
 +do
 +    $QEMU_IO -c "write -z $ofs $sector_size" "$TEST_IMG.orig" \
 +        | _filter_qemu_io \
 +        | _filter_offsets '' "$read_fail_offsets"
 +done
 +
 +echo
 +
 +# These should be equal now.
 +$QEMU_IMG compare "$TEST_IMG.orig" "$TEST_IMG"
 +
 +
 +# success, all done
 +echo "*** done"
 +rm -f $seq.full
 +status=0
 diff --git a/tests/qemu-iotests/251.out b/tests/qemu-iotests/251.out
 new file mode 100644
 index XXXXXXX..XXXXXXX
 --- /dev/null
 +++ b/tests/qemu-iotests/251.out
@@ -XXX,XX +XXX,XX @@
 +QA output created by 251
 +Formatting 'TEST_DIR/t.IMGFMT.orig', fmt=IMGFMT size=67108864
 +wrote 67108864/67108864 bytes at offset 0
 +64 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 +
 +qemu-img: warning: error while reading block status at offset status_fail_offset_0: Input/output error
 +qemu-img: warning: error while reading block status at offset status_fail_offset_1: Input/output error
 +qemu-img: warning: error while reading block status at offset status_fail_offset_0: Input/output error
 +qemu-img: warning: error while reading offset read_fail_offset_0: Input/output error
 +qemu-img: warning: error while reading block status at offset status_fail_offset_1: Input/output error
 +qemu-img: warning: error while reading offset status_fail_offset_1: Input/output error
 +qemu-img: warning: error while reading offset read_fail_offset_2: Input/output error
 +qemu-img: warning: error while reading offset read_fail_offset_3: Input/output error
 +qemu-img: warning: error while reading offset read_fail_offset_4: Input/output error
 +qemu-img: warning: error while reading offset read_fail_offset_5: Input/output error
 +qemu-img: warning: error while reading offset read_fail_offset_6: Input/output error
 +qemu-img: warning: error while reading offset read_fail_offset_7: Input/output error
 +qemu-img: warning: error while reading offset read_fail_offset_8: Input/output error
 +qemu-img: warning: error while reading offset read_fail_offset_9: Input/output error
 +
 +wrote 512/512 bytes at offset read_fail_offset_0
 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 +wrote 512/512 bytes at offset read_fail_offset_1
 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 +wrote 512/512 bytes at offset read_fail_offset_2
 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 +wrote 512/512 bytes at offset read_fail_offset_3
 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 +wrote 512/512 bytes at offset read_fail_offset_4
 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 +wrote 512/512 bytes at offset read_fail_offset_5
 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 +wrote 512/512 bytes at offset read_fail_offset_6
 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 +wrote 512/512 bytes at offset read_fail_offset_7
 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 +wrote 512/512 bytes at offset read_fail_offset_8
 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 +wrote 512/512 bytes at offset read_fail_offset_9
 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 +
 +Images are identical.
 +*** done
 diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
 index XXXXXXX..XXXXXXX 100644
 --- a/tests/qemu-iotests/group
 +++ b/tests/qemu-iotests/group
@@ -XXX,XX +XXX,XX @@
 rw quick
 rw auto quick
 rw auto quick
 +251 rw auto quick
 rw auto backing quick
 rw auto quick
 rw auto backing quick
 --
 .21.0

-[Qemu-devel] [PULL 01/12] blockjob: remove unnecessary check
+[Qemu-devel] [PULL 20/20] iotests: Test qemu-img convert -C --salvage
-From: Paolo Bonzini <pbonzini@redhat.com>
+We do not support this combination (yet), so this should yield an error
 message.
-!job is always checked prior to the call, drop it from here.
+Signed-off-by: Max Reitz <mreitz@redhat.com>
 Tested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
 Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
 Message-id: 20190507203508.18026-8-mreitz@redhat.com
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  tests/qemu-iotests/082     | 1 +
  tests/qemu-iotests/082.out | 3 +++
 files changed, 4 insertions(+)
-Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
+diff --git a/tests/qemu-iotests/082 b/tests/qemu-iotests/082
-Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
+index XXXXXXX..XXXXXXX 100755
-Reviewed-by: Jeff Cody <jcody@redhat.com>
+--- a/tests/qemu-iotests/082
-Message-id: 20170508141310.8674-2-pbonzini@redhat.com
++++ b/tests/qemu-iotests/082
-Signed-off-by: Jeff Cody <jcody@redhat.com>
+@@ -XXX,XX +XXX,XX @@ echo === convert: -C and other options ===
----
+ run_qemu_img convert -C -S 4k -O $IMGFMT "$TEST_IMG" "$TEST_IMG".target
- blockjob.c | 2 +-
+ run_qemu_img convert -C -S 8k -O $IMGFMT "$TEST_IMG" "$TEST_IMG".target
-file changed, 1 insertion(+), 1 deletion(-)
+ run_qemu_img convert -C -c -O $IMGFMT "$TEST_IMG" "$TEST_IMG".target
++run_qemu_img convert -C --salvage -O $IMGFMT "$TEST_IMG" "$TEST_IMG".target
-diff --git a/blockjob.c b/blockjob.c
  echo
  echo === amend: Options specified more than once ===
 diff --git a/tests/qemu-iotests/082.out b/tests/qemu-iotests/082.out
 index XXXXXXX..XXXXXXX 100644
---- a/blockjob.c
+--- a/tests/qemu-iotests/082.out
-+++ b/blockjob.c
++++ b/tests/qemu-iotests/082.out
-@@ -XXX,XX +XXX,XX @@ static bool block_job_should_pause(BlockJob *job)
+@@ -XXX,XX +XXX,XX @@ qemu-img: Cannot enable copy offloading when -S is used
+ Testing: convert -C -c -O qcow2 TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.target
- bool block_job_user_paused(BlockJob *job)
+ qemu-img: Cannot enable copy offloading when -c is used
- {
--    return job ? job->user_paused : 0;
++Testing: convert -C --salvage -O qcow2 TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.target
-+    return job->user_paused;
++qemu-img: Cannot use copy offloading in salvaging mode
- }
++
+ === amend: Options specified more than once ===
- void coroutine_fn block_job_pause_point(BlockJob *job)
  Testing: amend -f foo -f qcow2 -o lazy_refcounts=on TEST_DIR/t.qcow2
 --
-.9.3
+.21.0

The following changes since commit 9964e96dc9999cf7f7c936ee854a795415d19b60:

Merge remote-tracking branch 'jasowang/tags/net-pull-request' into staging (2017-05-23 15:01:31 +0100)

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 223a23c198787328ae75bc65d84edf5fde33c0b6:

block/gluster: glfs_lseek() workaround (2017-05-24 16:44:46 -0400)

----------------------------------------------------------------
Block patches
----------------------------------------------------------------

Jeff Cody (1):
  block/gluster: glfs_lseek() workaround

Paolo Bonzini (11):
  blockjob: remove unnecessary check
  blockjob: remove iostatus_reset callback
  blockjob: introduce block_job_early_fail
  blockjob: introduce block_job_pause/resume_all
  blockjob: separate monitor and blockjob APIs
  blockjob: move iostatus reset inside block_job_user_resume
  blockjob: introduce block_job_cancel_async, check iostatus invariants
  blockjob: group BlockJob transaction functions together
  blockjob: strengthen a bit test-blockjob-txn
  blockjob: reorganize block_job_completed_txn_abort
  blockjob: use deferred_to_main_loop to indicate the coroutine has
    ended

-- 
2.9.3

From: Paolo Bonzini <pbonzini@redhat.com>

This is unused since commit 66a0fae ("blockjob: Don't touch BDS iostatus",
2016-05-19).

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Message-id: 20170508141310.8674-3-pbonzini@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 blockjob.c                   | 3 ---
 include/block/blockjob_int.h | 3 ---
 2 files changed, 6 deletions(-)

diff --git a/blockjob.c b/blockjob.c
index XXXXXXX..XXXXXXX 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -XXX,XX +XXX,XX @@ bool block_job_is_cancelled(BlockJob *job)
 void block_job_iostatus_reset(BlockJob *job)
 {
     job->iostatus = BLOCK_DEVICE_IO_STATUS_OK;
-    if (job->driver->iostatus_reset) {
-        job->driver->iostatus_reset(job);
-    }
 }
 
 static int block_job_finish_sync(BlockJob *job,
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -XXX,XX +XXX,XX @@ struct BlockJobDriver {
     /** Optional callback for job types that support setting a speed limit */
     void (*set_speed)(BlockJob *job, int64_t speed, Error **errp);
 
-    /** Optional callback for job types that need to forward I/O status reset */
-    void (*iostatus_reset)(BlockJob *job);
-
     /** Mandatory: Entrypoint for the Coroutine. */
     CoroutineEntry *start;
 
-- 
2.9.3

From: Paolo Bonzini <pbonzini@redhat.com>

Outside blockjob.c, block_job_unref is only used when a block job fails
to start, and block_job_ref is not used at all.  The reference counting
thus is pretty well hidden.  Introduce a separate function to be used
by block jobs; because block_job_ref and block_job_unref now become
static, move them earlier in blockjob.c.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Message-id: 20170508141310.8674-4-pbonzini@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/backup.c               |  2 +-
 block/commit.c               |  2 +-
 block/mirror.c               |  2 +-
 blockjob.c                   | 47 ++++++++++++++++++++++++++------------------
 include/block/blockjob_int.h | 15 +++-----------
 tests/test-blockjob.c        | 10 +++++-----
 6 files changed, 39 insertions(+), 39 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 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
     }
     if (job) {
         backup_clean(&job->common);
-        block_job_unref(&job->common);
+        block_job_early_fail(&job->common);
     }
 
     return NULL;
diff --git a/block/commit.c b/block/commit.c
index XXXXXXX..XXXXXXX 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -XXX,XX +XXX,XX @@ fail:
     if (commit_top_bs) {
         bdrv_set_backing_hd(overlay_bs, top, &error_abort);
     }
-    block_job_unref(&s->common);
+    block_job_early_fail(&s->common);
 }
 
 
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 @@ fail:
 
         g_free(s->replaces);
         blk_unref(s->target);
-        block_job_unref(&s->common);
+        block_job_early_fail(&s->common);
     }
 
     bdrv_child_try_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL,
diff --git a/blockjob.c b/blockjob.c
index XXXXXXX..XXXXXXX 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -XXX,XX +XXX,XX @@ BlockJob *block_job_get(const char *id)
     return NULL;
 }
 
+static void block_job_ref(BlockJob *job)
+{
+    ++job->refcnt;
+}
+
+static void block_job_attached_aio_context(AioContext *new_context,
+                                           void *opaque);
+static void block_job_detach_aio_context(void *opaque);
+
+static void block_job_unref(BlockJob *job)
+{
+    if (--job->refcnt == 0) {
+        BlockDriverState *bs = blk_bs(job->blk);
+        bs->job = NULL;
+        block_job_remove_all_bdrv(job);
+        blk_remove_aio_context_notifier(job->blk,
+                                        block_job_attached_aio_context,
+                                        block_job_detach_aio_context, job);
+        blk_unref(job->blk);
+        error_free(job->blocker);
+        g_free(job->id);
+        QLIST_REMOVE(job, job_list);
+        g_free(job);
+    }
+}
+
 static void block_job_attached_aio_context(AioContext *new_context,
                                            void *opaque)
 {
@@ -XXX,XX +XXX,XX @@ void block_job_start(BlockJob *job)
     bdrv_coroutine_enter(blk_bs(job->blk), job->co);
 }
 
-void block_job_ref(BlockJob *job)
+void block_job_early_fail(BlockJob *job)
 {
-    ++job->refcnt;
-}
-
-void block_job_unref(BlockJob *job)
-{
-    if (--job->refcnt == 0) {
-        BlockDriverState *bs = blk_bs(job->blk);
-        bs->job = NULL;
-        block_job_remove_all_bdrv(job);
-        blk_remove_aio_context_notifier(job->blk,
-                                        block_job_attached_aio_context,
-                                        block_job_detach_aio_context, job);
-        blk_unref(job->blk);
-        error_free(job->blocker);
-        g_free(job->id);
-        QLIST_REMOVE(job, job_list);
-        g_free(job);
-    }
+    block_job_unref(job);
 }
 
 static void block_job_completed_single(BlockJob *job)
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -XXX,XX +XXX,XX @@ void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns);
 void block_job_yield(BlockJob *job);
 
 /**
- * block_job_ref:
+ * block_job_early_fail:
  * @bs: The block device.
  *
- * Grab a reference to the block job. Should be paired with block_job_unref.
+ * The block job could not be started, free it.
  */
-void block_job_ref(BlockJob *job);
-
-/**
- * block_job_unref:
- * @bs: The block device.
- *
- * Release reference to the block job and release resources if it is the last
- * reference.
- */
-void block_job_unref(BlockJob *job);
+void block_job_early_fail(BlockJob *job);
 
 /**
  * block_job_completed:
diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c
index XXXXXXX..XXXXXXX 100644
--- a/tests/test-blockjob.c
+++ b/tests/test-blockjob.c
@@ -XXX,XX +XXX,XX @@ static void test_job_ids(void)
     job[1] = do_test_id(blk[1], "id0", false);
 
     /* But once job[0] finishes we can reuse its ID */
-    block_job_unref(job[0]);
+    block_job_early_fail(job[0]);
     job[1] = do_test_id(blk[1], "id0", true);
 
     /* No job ID specified, defaults to the backend name ('drive1') */
-    block_job_unref(job[1]);
+    block_job_early_fail(job[1]);
     job[1] = do_test_id(blk[1], NULL, true);
 
     /* Duplicate job ID */
@@ -XXX,XX +XXX,XX @@ static void test_job_ids(void)
     /* This one is valid */
     job[2] = do_test_id(blk[2], "id_2", true);
 
-    block_job_unref(job[0]);
-    block_job_unref(job[1]);
-    block_job_unref(job[2]);
+    block_job_early_fail(job[0]);
+    block_job_early_fail(job[1]);
+    block_job_early_fail(job[2]);
 
     destroy_blk(blk[0]);
     destroy_blk(blk[1]);
-- 
2.9.3

From: Paolo Bonzini <pbonzini@redhat.com>

Remove use of block_job_pause/resume from outside blockjob.c, thus
making them static.  The new functions are used by the block layer,
so place them in blockjob_int.h.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Message-id: 20170508141310.8674-5-pbonzini@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/io.c                   |  19 ++------
 blockjob.c                   | 114 ++++++++++++++++++++++++++-----------------
 include/block/blockjob.h     |  16 ------
 include/block/blockjob_int.h |  14 ++++++
 4 files changed, 86 insertions(+), 77 deletions(-)

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 "trace.h"
 #include "sysemu/block-backend.h"
 #include "block/blockjob.h"
+#include "block/blockjob_int.h"
 #include "block/block_int.h"
 #include "qemu/cutils.h"
 #include "qapi/error.h"
@@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_begin(void)
     bool waited = true;
     BlockDriverState *bs;
     BdrvNextIterator it;
-    BlockJob *job = NULL;
     GSList *aio_ctxs = NULL, *ctx;
 
-    while ((job = block_job_next(job))) {
-        AioContext *aio_context = blk_get_aio_context(job->blk);
-
-        aio_context_acquire(aio_context);
-        block_job_pause(job);
-        aio_context_release(aio_context);
-    }
+    block_job_pause_all();
 
     for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
         AioContext *aio_context = bdrv_get_aio_context(bs);
@@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_end(void)
 {
     BlockDriverState *bs;
     BdrvNextIterator it;
-    BlockJob *job = NULL;
 
     for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
         AioContext *aio_context = bdrv_get_aio_context(bs);
@@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_end(void)
         aio_context_release(aio_context);
     }
 
-    while ((job = block_job_next(job))) {
-        AioContext *aio_context = blk_get_aio_context(job->blk);
-
-        aio_context_acquire(aio_context);
-        block_job_resume(job);
-        aio_context_release(aio_context);
-    }
+    block_job_resume_all();
 }
 
 void bdrv_drain_all(void)
diff --git a/blockjob.c b/blockjob.c
index XXXXXXX..XXXXXXX 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -XXX,XX +XXX,XX @@ struct BlockJobTxn {
 
 static QLIST_HEAD(, BlockJob) block_jobs = QLIST_HEAD_INITIALIZER(block_jobs);
 
-static char *child_job_get_parent_desc(BdrvChild *c)
-{
-    BlockJob *job = c->opaque;
-    return g_strdup_printf("%s job '%s'",
-                           BlockJobType_lookup[job->driver->job_type],
-                           job->id);
-}
-
-static const BdrvChildRole child_job = {
-    .get_parent_desc    = child_job_get_parent_desc,
-    .stay_at_node       = true,
-};
-
-static void block_job_drained_begin(void *opaque)
-{
-    BlockJob *job = opaque;
-    block_job_pause(job);
-}
-
-static void block_job_drained_end(void *opaque)
-{
-    BlockJob *job = opaque;
-    block_job_resume(job);
-}
-
-static const BlockDevOps block_job_dev_ops = {
-    .drained_begin = block_job_drained_begin,
-    .drained_end = block_job_drained_end,
-};
-
 BlockJob *block_job_next(BlockJob *job)
 {
     if (!job) {
@@ -XXX,XX +XXX,XX @@ BlockJob *block_job_get(const char *id)
     return NULL;
 }
 
+static void block_job_pause(BlockJob *job)
+{
+    job->pause_count++;
+}
+
+static void block_job_resume(BlockJob *job)
+{
+    assert(job->pause_count > 0);
+    job->pause_count--;
+    if (job->pause_count) {
+        return;
+    }
+    block_job_enter(job);
+}
+
 static void block_job_ref(BlockJob *job)
 {
     ++job->refcnt;
@@ -XXX,XX +XXX,XX @@ static void block_job_detach_aio_context(void *opaque)
     block_job_unref(job);
 }
 
+static char *child_job_get_parent_desc(BdrvChild *c)
+{
+    BlockJob *job = c->opaque;
+    return g_strdup_printf("%s job '%s'",
+                           BlockJobType_lookup[job->driver->job_type],
+                           job->id);
+}
+
+static const BdrvChildRole child_job = {
+    .get_parent_desc    = child_job_get_parent_desc,
+    .stay_at_node       = true,
+};
+
+static void block_job_drained_begin(void *opaque)
+{
+    BlockJob *job = opaque;
+    block_job_pause(job);
+}
+
+static void block_job_drained_end(void *opaque)
+{
+    BlockJob *job = opaque;
+    block_job_resume(job);
+}
+
+static const BlockDevOps block_job_dev_ops = {
+    .drained_begin = block_job_drained_begin,
+    .drained_end = block_job_drained_end,
+};
+
 void block_job_remove_all_bdrv(BlockJob *job)
 {
     GSList *l;
@@ -XXX,XX +XXX,XX @@ void block_job_complete(BlockJob *job, Error **errp)
     job->driver->complete(job, errp);
 }
 
-void block_job_pause(BlockJob *job)
-{
-    job->pause_count++;
-}
-
 void block_job_user_pause(BlockJob *job)
 {
     job->user_paused = true;
@@ -XXX,XX +XXX,XX @@ void coroutine_fn block_job_pause_point(BlockJob *job)
     }
 }
 
-void block_job_resume(BlockJob *job)
-{
-    assert(job->pause_count > 0);
-    job->pause_count--;
-    if (job->pause_count) {
-        return;
-    }
-    block_job_enter(job);
-}
-
 void block_job_user_resume(BlockJob *job)
 {
     if (job && job->user_paused && job->pause_count > 0) {
@@ -XXX,XX +XXX,XX @@ static void block_job_event_completed(BlockJob *job, const char *msg)
                                         &error_abort);
 }
 
+void block_job_pause_all(void)
+{
+    BlockJob *job = NULL;
+    while ((job = block_job_next(job))) {
+        AioContext *aio_context = blk_get_aio_context(job->blk);
+
+        aio_context_acquire(aio_context);
+        block_job_pause(job);
+        aio_context_release(aio_context);
+    }
+}
+
+void block_job_resume_all(void)
+{
+    BlockJob *job = NULL;
+    while ((job = block_job_next(job))) {
+        AioContext *aio_context = blk_get_aio_context(job->blk);
+
+        aio_context_acquire(aio_context);
+        block_job_resume(job);
+        aio_context_release(aio_context);
+    }
+}
+
 void block_job_event_ready(BlockJob *job)
 {
     job->ready = true;
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -XXX,XX +XXX,XX @@ void block_job_complete(BlockJob *job, Error **errp);
 BlockJobInfo *block_job_query(BlockJob *job, Error **errp);
 
 /**
- * block_job_pause:
- * @job: The job to be paused.
- *
- * Asynchronously pause the specified job.
- */
-void block_job_pause(BlockJob *job);
-
-/**
  * block_job_user_pause:
  * @job: The job to be paused.
  *
@@ -XXX,XX +XXX,XX @@ void block_job_user_pause(BlockJob *job);
 bool block_job_user_paused(BlockJob *job);
 
 /**
- * block_job_resume:
- * @job: The job to be resumed.
- *
- * Resume the specified job.  Must be paired with a preceding block_job_pause.
- */
-void block_job_resume(BlockJob *job);
-
-/**
  * block_job_user_resume:
  * @job: The job to be resumed.
  *
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -XXX,XX +XXX,XX @@ void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns);
 void block_job_yield(BlockJob *job);
 
 /**
+ * block_job_pause_all:
+ *
+ * Asynchronously pause all jobs.
+ */
+void block_job_pause_all(void);
+
+/**
+ * block_job_resume_all:
+ *
+ * Resume all block jobs.  Must be paired with a preceding block_job_pause_all.
+ */
+void block_job_resume_all(void);
+
+/**
  * block_job_early_fail:
  * @bs: The block device.
  *
-- 
2.9.3

From: Paolo Bonzini <pbonzini@redhat.com>

We have two different headers for block job operations, blockjob.h
and blockjob_int.h.  The former contains APIs called by the monitor,
the latter contains APIs called by the block job drivers and the
block layer itself.

Keep the two APIs separate in the blockjob.c file too.  This will
be useful when transitioning away from the AioContext lock, because
there will be locking policies for the two categories, too---the
monitor will have to call new block_job_lock/unlock APIs, while blockjob
APIs will take care of this for the users.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20170508141310.8674-6-pbonzini@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 blockjob.c | 390 ++++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 205 insertions(+), 185 deletions(-)

diff --git a/blockjob.c b/blockjob.c
index XXXXXXX..XXXXXXX 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -XXX,XX +XXX,XX @@ struct BlockJobTxn {
 
 static QLIST_HEAD(, BlockJob) block_jobs = QLIST_HEAD_INITIALIZER(block_jobs);
 
+/*
+ * The block job API is composed of two categories of functions.
+ *
+ * The first includes functions used by the monitor.  The monitor is
+ * peculiar in that it accesses the block job list with block_job_get, and
+ * therefore needs consistency across block_job_get and the actual operation
+ * (e.g. block_job_set_speed).  The consistency is achieved with
+ * aio_context_acquire/release.  These functions are declared in blockjob.h.
+ *
+ * The second includes functions used by the block job drivers and sometimes
+ * by the core block layer.  These do not care about locking, because the
+ * whole coroutine runs under the AioContext lock, and are declared in
+ * blockjob_int.h.
+ */
+
 BlockJob *block_job_next(BlockJob *job)
 {
     if (!job) {
@@ -XXX,XX +XXX,XX @@ int block_job_add_bdrv(BlockJob *job, const char *name, BlockDriverState *bs,
     return 0;
 }
 
-void *block_job_create(const char *job_id, const BlockJobDriver *driver,
-                       BlockDriverState *bs, uint64_t perm,
-                       uint64_t shared_perm, int64_t speed, int flags,
-                       BlockCompletionFunc *cb, void *opaque, Error **errp)
-{
-    BlockBackend *blk;
-    BlockJob *job;
-    int ret;
-
-    if (bs->job) {
-        error_setg(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(bs));
-        return NULL;
-    }
-
-    if (job_id == NULL && !(flags & BLOCK_JOB_INTERNAL)) {
-        job_id = bdrv_get_device_name(bs);
-        if (!*job_id) {
-            error_setg(errp, "An explicit job ID is required for this node");
-            return NULL;
-        }
-    }
-
-    if (job_id) {
-        if (flags & BLOCK_JOB_INTERNAL) {
-            error_setg(errp, "Cannot specify job ID for internal block job");
-            return NULL;
-        }
-
-        if (!id_wellformed(job_id)) {
-            error_setg(errp, "Invalid job ID '%s'", job_id);
-            return NULL;
-        }
-
-        if (block_job_get(job_id)) {
-            error_setg(errp, "Job ID '%s' already in use", job_id);
-            return NULL;
-        }
-    }
-
-    blk = blk_new(perm, shared_perm);
-    ret = blk_insert_bs(blk, bs, errp);
-    if (ret < 0) {
-        blk_unref(blk);
-        return NULL;
-    }
-
-    job = g_malloc0(driver->instance_size);
-    job->driver        = driver;
-    job->id            = g_strdup(job_id);
-    job->blk           = blk;
-    job->cb            = cb;
-    job->opaque        = opaque;
-    job->busy          = false;
-    job->paused        = true;
-    job->pause_count   = 1;
-    job->refcnt        = 1;
-
-    error_setg(&job->blocker, "block device is in use by block job: %s",
-               BlockJobType_lookup[driver->job_type]);
-    block_job_add_bdrv(job, "main node", bs, 0, BLK_PERM_ALL, &error_abort);
-    bs->job = job;
-
-    blk_set_dev_ops(blk, &block_job_dev_ops, job);
-    bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker);
-
-    QLIST_INSERT_HEAD(&block_jobs, job, job_list);
-
-    blk_add_aio_context_notifier(blk, block_job_attached_aio_context,
-                                 block_job_detach_aio_context, job);
-
-    /* Only set speed when necessary to avoid NotSupported error */
-    if (speed != 0) {
-        Error *local_err = NULL;
-
-        block_job_set_speed(job, speed, &local_err);
-        if (local_err) {
-            block_job_unref(job);
-            error_propagate(errp, local_err);
-            return NULL;
-        }
-    }
-    return job;
-}
-
 bool block_job_is_internal(BlockJob *job)
 {
     return (job->id == NULL);
@@ -XXX,XX +XXX,XX @@ void block_job_start(BlockJob *job)
     bdrv_coroutine_enter(blk_bs(job->blk), job->co);
 }
 
-void block_job_early_fail(BlockJob *job)
-{
-    block_job_unref(job);
-}
-
 static void block_job_completed_single(BlockJob *job)
 {
     if (!job->ret) {
@@ -XXX,XX +XXX,XX @@ static void block_job_completed_txn_success(BlockJob *job)
     }
 }
 
-void block_job_completed(BlockJob *job, int ret)
-{
-    assert(blk_bs(job->blk)->job == job);
-    assert(!job->completed);
-    job->completed = true;
-    job->ret = ret;
-    if (!job->txn) {
-        block_job_completed_single(job);
-    } else if (ret < 0 || block_job_is_cancelled(job)) {
-        block_job_completed_txn_abort(job);
-    } else {
-        block_job_completed_txn_success(job);
-    }
-}
-
 void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)
 {
     Error *local_err = NULL;
@@ -XXX,XX +XXX,XX @@ void block_job_user_pause(BlockJob *job)
     block_job_pause(job);
 }
 
-static bool block_job_should_pause(BlockJob *job)
-{
-    return job->pause_count > 0;
-}
-
 bool block_job_user_paused(BlockJob *job)
 {
     return job->user_paused;
 }
 
-void coroutine_fn block_job_pause_point(BlockJob *job)
-{
-    assert(job && block_job_started(job));
-
-    if (!block_job_should_pause(job)) {
-        return;
-    }
-    if (block_job_is_cancelled(job)) {
-        return;
-    }
-
-    if (job->driver->pause) {
-        job->driver->pause(job);
-    }
-
-    if (block_job_should_pause(job) && !block_job_is_cancelled(job)) {
-        job->paused = true;
-        job->busy = false;
-        qemu_coroutine_yield(); /* wait for block_job_resume() */
-        job->busy = true;
-        job->paused = false;
-    }
-
-    if (job->driver->resume) {
-        job->driver->resume(job);
-    }
-}
-
 void block_job_user_resume(BlockJob *job)
 {
     if (job && job->user_paused && job->pause_count > 0) {
@@ -XXX,XX +XXX,XX @@ void block_job_user_resume(BlockJob *job)
     }
 }
 
-void block_job_enter(BlockJob *job)
-{
-    if (job->co && !job->busy) {
-        bdrv_coroutine_enter(blk_bs(job->blk), job->co);
-    }
-}
-
 void block_job_cancel(BlockJob *job)
 {
     if (block_job_started(job)) {
@@ -XXX,XX +XXX,XX @@ void block_job_cancel(BlockJob *job)
     }
 }
 
-bool block_job_is_cancelled(BlockJob *job)
-{
-    return job->cancelled;
-}
-
 void block_job_iostatus_reset(BlockJob *job)
 {
     job->iostatus = BLOCK_DEVICE_IO_STATUS_OK;
@@ -XXX,XX +XXX,XX @@ int block_job_complete_sync(BlockJob *job, Error **errp)
     return block_job_finish_sync(job, &block_job_complete, errp);
 }
 
-void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns)
-{
-    assert(job->busy);
-
-    /* Check cancellation *before* setting busy = false, too!  */
-    if (block_job_is_cancelled(job)) {
-        return;
-    }
-
-    job->busy = false;
-    if (!block_job_should_pause(job)) {
-        co_aio_sleep_ns(blk_get_aio_context(job->blk), type, ns);
-    }
-    job->busy = true;
-
-    block_job_pause_point(job);
-}
-
-void block_job_yield(BlockJob *job)
-{
-    assert(job->busy);
-
-    /* Check cancellation *before* setting busy = false, too!  */
-    if (block_job_is_cancelled(job)) {
-        return;
-    }
-
-    job->busy = false;
-    if (!block_job_should_pause(job)) {
-        qemu_coroutine_yield();
-    }
-    job->busy = true;
-
-    block_job_pause_point(job);
-}
-
 BlockJobInfo *block_job_query(BlockJob *job, Error **errp)
 {
     BlockJobInfo *info;
@@ -XXX,XX +XXX,XX @@ static void block_job_event_completed(BlockJob *job, const char *msg)
                                         &error_abort);
 }
 
+/*
+ * API for block job drivers and the block layer.  These functions are
+ * declared in blockjob_int.h.
+ */
+
+void *block_job_create(const char *job_id, const BlockJobDriver *driver,
+                       BlockDriverState *bs, uint64_t perm,
+                       uint64_t shared_perm, int64_t speed, int flags,
+                       BlockCompletionFunc *cb, void *opaque, Error **errp)
+{
+    BlockBackend *blk;
+    BlockJob *job;
+    int ret;
+
+    if (bs->job) {
+        error_setg(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(bs));
+        return NULL;
+    }
+
+    if (job_id == NULL && !(flags & BLOCK_JOB_INTERNAL)) {
+        job_id = bdrv_get_device_name(bs);
+        if (!*job_id) {
+            error_setg(errp, "An explicit job ID is required for this node");
+            return NULL;
+        }
+    }
+
+    if (job_id) {
+        if (flags & BLOCK_JOB_INTERNAL) {
+            error_setg(errp, "Cannot specify job ID for internal block job");
+            return NULL;
+        }
+
+        if (!id_wellformed(job_id)) {
+            error_setg(errp, "Invalid job ID '%s'", job_id);
+            return NULL;
+        }
+
+        if (block_job_get(job_id)) {
+            error_setg(errp, "Job ID '%s' already in use", job_id);
+            return NULL;
+        }
+    }
+
+    blk = blk_new(perm, shared_perm);
+    ret = blk_insert_bs(blk, bs, errp);
+    if (ret < 0) {
+        blk_unref(blk);
+        return NULL;
+    }
+
+    job = g_malloc0(driver->instance_size);
+    job->driver        = driver;
+    job->id            = g_strdup(job_id);
+    job->blk           = blk;
+    job->cb            = cb;
+    job->opaque        = opaque;
+    job->busy          = false;
+    job->paused        = true;
+    job->pause_count   = 1;
+    job->refcnt        = 1;
+
+    error_setg(&job->blocker, "block device is in use by block job: %s",
+               BlockJobType_lookup[driver->job_type]);
+    block_job_add_bdrv(job, "main node", bs, 0, BLK_PERM_ALL, &error_abort);
+    bs->job = job;
+
+    blk_set_dev_ops(blk, &block_job_dev_ops, job);
+    bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker);
+
+    QLIST_INSERT_HEAD(&block_jobs, job, job_list);
+
+    blk_add_aio_context_notifier(blk, block_job_attached_aio_context,
+                                 block_job_detach_aio_context, job);
+
+    /* Only set speed when necessary to avoid NotSupported error */
+    if (speed != 0) {
+        Error *local_err = NULL;
+
+        block_job_set_speed(job, speed, &local_err);
+        if (local_err) {
+            block_job_unref(job);
+            error_propagate(errp, local_err);
+            return NULL;
+        }
+    }
+    return job;
+}
+
 void block_job_pause_all(void)
 {
     BlockJob *job = NULL;
@@ -XXX,XX +XXX,XX @@ void block_job_pause_all(void)
     }
 }
 
+void block_job_early_fail(BlockJob *job)
+{
+    block_job_unref(job);
+}
+
+void block_job_completed(BlockJob *job, int ret)
+{
+    assert(blk_bs(job->blk)->job == job);
+    assert(!job->completed);
+    job->completed = true;
+    job->ret = ret;
+    if (!job->txn) {
+        block_job_completed_single(job);
+    } else if (ret < 0 || block_job_is_cancelled(job)) {
+        block_job_completed_txn_abort(job);
+    } else {
+        block_job_completed_txn_success(job);
+    }
+}
+
+static bool block_job_should_pause(BlockJob *job)
+{
+    return job->pause_count > 0;
+}
+
+void coroutine_fn block_job_pause_point(BlockJob *job)
+{
+    assert(job && block_job_started(job));
+
+    if (!block_job_should_pause(job)) {
+        return;
+    }
+    if (block_job_is_cancelled(job)) {
+        return;
+    }
+
+    if (job->driver->pause) {
+        job->driver->pause(job);
+    }
+
+    if (block_job_should_pause(job) && !block_job_is_cancelled(job)) {
+        job->paused = true;
+        job->busy = false;
+        qemu_coroutine_yield(); /* wait for block_job_resume() */
+        job->busy = true;
+        job->paused = false;
+    }
+
+    if (job->driver->resume) {
+        job->driver->resume(job);
+    }
+}
+
 void block_job_resume_all(void)
 {
     BlockJob *job = NULL;
@@ -XXX,XX +XXX,XX @@ void block_job_resume_all(void)
     }
 }
 
+void block_job_enter(BlockJob *job)
+{
+    if (job->co && !job->busy) {
+        bdrv_coroutine_enter(blk_bs(job->blk), job->co);
+    }
+}
+
+bool block_job_is_cancelled(BlockJob *job)
+{
+    return job->cancelled;
+}
+
+void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns)
+{
+    assert(job->busy);
+
+    /* Check cancellation *before* setting busy = false, too!  */
+    if (block_job_is_cancelled(job)) {
+        return;
+    }
+
+    job->busy = false;
+    if (!block_job_should_pause(job)) {
+        co_aio_sleep_ns(blk_get_aio_context(job->blk), type, ns);
+    }
+    job->busy = true;
+
+    block_job_pause_point(job);
+}
+
+void block_job_yield(BlockJob *job)
+{
+    assert(job->busy);
+
+    /* Check cancellation *before* setting busy = false, too!  */
+    if (block_job_is_cancelled(job)) {
+        return;
+    }
+
+    job->busy = false;
+    if (!block_job_should_pause(job)) {
+        qemu_coroutine_yield();
+    }
+    job->busy = true;
+
+    block_job_pause_point(job);
+}
+
 void block_job_event_ready(BlockJob *job)
 {
     job->ready = true;
-- 
2.9.3

From: Paolo Bonzini <pbonzini@redhat.com>

Outside blockjob.c, the block_job_iostatus_reset function is used once
in the monitor and once in BlockBackend.  When we introduce the block
job mutex, block_job_iostatus_reset's client is going to be the block
layer (for which blockjob.c will take the block job mutex) rather than
the monitor (which will take the block job mutex by itself).

The monitor's call to block_job_iostatus_reset from the monitor comes
just before the sole call to block_job_user_resume, so reset the
iostatus directly from block_job_iostatus_reset.  This will avoid
the need to introduce separate block_job_iostatus_reset and
block_job_iostatus_reset_locked APIs.

After making this change, move the function together with the others
that were moved in the previous patch.

diff --git a/blockdev.c b/blockdev.c
index XXXXXXX..XXXXXXX 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -XXX,XX +XXX,XX @@ void qmp_block_job_resume(const char *device, Error **errp)
     }
 
     trace_qmp_block_job_resume(job);
-    block_job_iostatus_reset(job);
     block_job_user_resume(job);
     aio_context_release(aio_context);
 }
diff --git a/blockjob.c b/blockjob.c
index XXXXXXX..XXXXXXX 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -XXX,XX +XXX,XX @@ void block_job_user_resume(BlockJob *job)
 {
     if (job && job->user_paused && job->pause_count > 0) {
         job->user_paused = false;
+        block_job_iostatus_reset(job);
         block_job_resume(job);
     }
 }
@@ -XXX,XX +XXX,XX @@ void block_job_cancel(BlockJob *job)
     }
 }
 
-void block_job_iostatus_reset(BlockJob *job)
-{
-    job->iostatus = BLOCK_DEVICE_IO_STATUS_OK;
-}
-
 static int block_job_finish_sync(BlockJob *job,
                                  void (*finish)(BlockJob *, Error **errp),
                                  Error **errp)
@@ -XXX,XX +XXX,XX @@ void block_job_yield(BlockJob *job)
     block_job_pause_point(job);
 }
 
+void block_job_iostatus_reset(BlockJob *job)
+{
+    job->iostatus = BLOCK_DEVICE_IO_STATUS_OK;
+}
+
 void block_job_event_ready(BlockJob *job)
 {
     job->ready = true;
-- 
2.9.3

From: Paolo Bonzini <pbonzini@redhat.com>

The new functions helps respecting the invariant that the coroutine
is entered with false user_resume, zero pause count and no error
recorded in the iostatus.

Resetting the iostatus is now common to all of block_job_cancel_async,
block_job_user_resume and block_job_iostatus_reset, albeit with slight
differences:

- block_job_cancel_async resets the iostatus, and resumes the job if
there was an error, but the coroutine is not restarted immediately.
For example the caller may continue with a call to block_job_finish_sync.

- block_job_user_resume resets the iostatus.  It wants to resume the job
unconditionally, even if there was no error.

- block_job_iostatus_reset doesn't resume the job at all.  Maybe that's
a bug but it should be fixed separately.

block_job_iostatus_reset does the least common denominator, so add some
checking but otherwise leave it as the entry point for resetting the
iostatus.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20170508141310.8674-8-pbonzini@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 blockjob.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/blockjob.c b/blockjob.c
index XXXXXXX..XXXXXXX 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -XXX,XX +XXX,XX @@ static void block_job_completed_single(BlockJob *job)
     block_job_unref(job);
 }
 
+static void block_job_cancel_async(BlockJob *job)
+{
+    if (job->iostatus != BLOCK_DEVICE_IO_STATUS_OK) {
+        block_job_iostatus_reset(job);
+    }
+    if (job->user_paused) {
+        /* Do not call block_job_enter here, the caller will handle it.  */
+        job->user_paused = false;
+        job->pause_count--;
+    }
+    job->cancelled = true;
+}
+
 static void block_job_completed_txn_abort(BlockJob *job)
 {
     AioContext *ctx;
@@ -XXX,XX +XXX,XX @@ static void block_job_completed_txn_abort(BlockJob *job)
              * them; this job, however, may or may not be cancelled, depending
              * on the caller, so leave it. */
             if (other_job != job) {
-                other_job->cancelled = true;
+                block_job_cancel_async(other_job);
             }
             continue;
         }
@@ -XXX,XX +XXX,XX @@ bool block_job_user_paused(BlockJob *job)
 void block_job_user_resume(BlockJob *job)
 {
     if (job && job->user_paused && job->pause_count > 0) {
-        job->user_paused = false;
         block_job_iostatus_reset(job);
+        job->user_paused = false;
         block_job_resume(job);
     }
 }
@@ -XXX,XX +XXX,XX @@ void block_job_user_resume(BlockJob *job)
 void block_job_cancel(BlockJob *job)
 {
     if (block_job_started(job)) {
-        job->cancelled = true;
-        block_job_iostatus_reset(job);
+        block_job_cancel_async(job);
         block_job_enter(job);
     } else {
         block_job_completed(job, -ECANCELED);
@@ -XXX,XX +XXX,XX @@ void block_job_yield(BlockJob *job)
 
 void block_job_iostatus_reset(BlockJob *job)
 {
+    if (job->iostatus == BLOCK_DEVICE_IO_STATUS_OK) {
+        return;
+    }
+    assert(job->user_paused && job->pause_count > 0);
     job->iostatus = BLOCK_DEVICE_IO_STATUS_OK;
 }
 
-- 
2.9.3

From: Paolo Bonzini <pbonzini@redhat.com>

Yet another pure code movement patch, preparing for the next change.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20170508141310.8674-9-pbonzini@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 blockjob.c | 128 ++++++++++++++++++++++++++++++-------------------------------
 1 file changed, 64 insertions(+), 64 deletions(-)

diff --git a/blockjob.c b/blockjob.c
index XXXXXXX..XXXXXXX 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -XXX,XX +XXX,XX @@ BlockJob *block_job_get(const char *id)
     return NULL;
 }
 
+BlockJobTxn *block_job_txn_new(void)
+{
+    BlockJobTxn *txn = g_new0(BlockJobTxn, 1);
+    QLIST_INIT(&txn->jobs);
+    txn->refcnt = 1;
+    return txn;
+}
+
+static void block_job_txn_ref(BlockJobTxn *txn)
+{
+    txn->refcnt++;
+}
+
+void block_job_txn_unref(BlockJobTxn *txn)
+{
+    if (txn && --txn->refcnt == 0) {
+        g_free(txn);
+    }
+}
+
+void block_job_txn_add_job(BlockJobTxn *txn, BlockJob *job)
+{
+    if (!txn) {
+        return;
+    }
+
+    assert(!job->txn);
+    job->txn = txn;
+
+    QLIST_INSERT_HEAD(&txn->jobs, job, txn_list);
+    block_job_txn_ref(txn);
+}
+
 static void block_job_pause(BlockJob *job)
 {
     job->pause_count++;
@@ -XXX,XX +XXX,XX @@ static void block_job_cancel_async(BlockJob *job)
     job->cancelled = true;
 }
 
+static int block_job_finish_sync(BlockJob *job,
+                                 void (*finish)(BlockJob *, Error **errp),
+                                 Error **errp)
+{
+    Error *local_err = NULL;
+    int ret;
+
+    assert(blk_bs(job->blk)->job == job);
+
+    block_job_ref(job);
+
+    finish(job, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        block_job_unref(job);
+        return -EBUSY;
+    }
+    /* block_job_drain calls block_job_enter, and it should be enough to
+     * induce progress until the job completes or moves to the main thread.
+    */
+    while (!job->deferred_to_main_loop && !job->completed) {
+        block_job_drain(job);
+    }
+    while (!job->completed) {
+        aio_poll(qemu_get_aio_context(), true);
+    }
+    ret = (job->cancelled && job->ret == 0) ? -ECANCELED : job->ret;
+    block_job_unref(job);
+    return ret;
+}
+
 static void block_job_completed_txn_abort(BlockJob *job)
 {
     AioContext *ctx;
@@ -XXX,XX +XXX,XX @@ void block_job_cancel(BlockJob *job)
     }
 }
 
-static int block_job_finish_sync(BlockJob *job,
-                                 void (*finish)(BlockJob *, Error **errp),
-                                 Error **errp)
-{
-    Error *local_err = NULL;
-    int ret;
-
-    assert(blk_bs(job->blk)->job == job);
-
-    block_job_ref(job);
-
-    finish(job, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        block_job_unref(job);
-        return -EBUSY;
-    }
-    /* block_job_drain calls block_job_enter, and it should be enough to
-     * induce progress until the job completes or moves to the main thread.
-    */
-    while (!job->deferred_to_main_loop && !job->completed) {
-        block_job_drain(job);
-    }
-    while (!job->completed) {
-        aio_poll(qemu_get_aio_context(), true);
-    }
-    ret = (job->cancelled && job->ret == 0) ? -ECANCELED : job->ret;
-    block_job_unref(job);
-    return ret;
-}
-
 /* A wrapper around block_job_cancel() taking an Error ** parameter so it may be
  * used with block_job_finish_sync() without the need for (rather nasty)
  * function pointer casts there. */
@@ -XXX,XX +XXX,XX @@ void block_job_defer_to_main_loop(BlockJob *job,
     aio_bh_schedule_oneshot(qemu_get_aio_context(),
                             block_job_defer_to_main_loop_bh, data);
 }
-
-BlockJobTxn *block_job_txn_new(void)
-{
-    BlockJobTxn *txn = g_new0(BlockJobTxn, 1);
-    QLIST_INIT(&txn->jobs);
-    txn->refcnt = 1;
-    return txn;
-}
-
-static void block_job_txn_ref(BlockJobTxn *txn)
-{
-    txn->refcnt++;
-}
-
-void block_job_txn_unref(BlockJobTxn *txn)
-{
-    if (txn && --txn->refcnt == 0) {
-        g_free(txn);
-    }
-}
-
-void block_job_txn_add_job(BlockJobTxn *txn, BlockJob *job)
-{
-    if (!txn) {
-        return;
-    }
-
-    assert(!job->txn);
-    job->txn = txn;
-
-    QLIST_INSERT_HEAD(&txn->jobs, job, txn_list);
-    block_job_txn_ref(txn);
-}
-- 
2.9.3

From: Paolo Bonzini <pbonzini@redhat.com>

Unlike test-blockjob-txn, QMP releases the reference to the transaction
before the jobs finish.  Thus, qemu-iotest 124 showed a failure while
working on the next patch that the unit tests did not have.  Make
the test a little nastier.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20170508141310.8674-10-pbonzini@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 tests/test-blockjob-txn.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c
index XXXXXXX..XXXXXXX 100644
--- a/tests/test-blockjob-txn.c
+++ b/tests/test-blockjob-txn.c
@@ -XXX,XX +XXX,XX @@ static void test_pair_jobs(int expected1, int expected2)
     block_job_start(job1);
     block_job_start(job2);
 
+    /* Release our reference now to trigger as many nice
+     * use-after-free bugs as possible.
+     */
+    block_job_txn_unref(txn);
+
     if (expected1 == -ECANCELED) {
         block_job_cancel(job1);
     }
@@ -XXX,XX +XXX,XX @@ static void test_pair_jobs(int expected1, int expected2)
 
     g_assert_cmpint(result1, ==, expected1);
     g_assert_cmpint(result2, ==, expected2);
-
-    block_job_txn_unref(txn);
 }
 
 static void test_pair_jobs_success(void)
-- 
2.9.3

From: Paolo Bonzini <pbonzini@redhat.com>

This splits the part that touches job states from the part that invokes
callbacks.  It will make the code simpler to understand once job states will
be protected by a different mutex than the AioContext lock.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20170508141310.8674-11-pbonzini@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 blockjob.c | 35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/blockjob.c b/blockjob.c
index XXXXXXX..XXXXXXX 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -XXX,XX +XXX,XX @@ void block_job_start(BlockJob *job)
 
 static void block_job_completed_single(BlockJob *job)
 {
+    assert(job->completed);
+
     if (!job->ret) {
         if (job->driver->commit) {
             job->driver->commit(job);
@@ -XXX,XX +XXX,XX @@ static int block_job_finish_sync(BlockJob *job,
 
     block_job_ref(job);
 
-    finish(job, &local_err);
+    if (finish) {
+        finish(job, &local_err);
+    }
     if (local_err) {
         error_propagate(errp, local_err);
         block_job_unref(job);
@@ -XXX,XX +XXX,XX @@ static void block_job_completed_txn_abort(BlockJob *job)
 {
     AioContext *ctx;
     BlockJobTxn *txn = job->txn;
-    BlockJob *other_job, *next;
+    BlockJob *other_job;
 
     if (txn->aborting) {
         /*
@@ -XXX,XX +XXX,XX @@ static void block_job_completed_txn_abort(BlockJob *job)
         return;
     }
     txn->aborting = true;
+    block_job_txn_ref(txn);
+
     /* We are the first failed job. Cancel other jobs. */
     QLIST_FOREACH(other_job, &txn->jobs, txn_list) {
         ctx = blk_get_aio_context(other_job->blk);
         aio_context_acquire(ctx);
     }
+
+    /* Other jobs are effectively cancelled by us, set the status for
+     * them; this job, however, may or may not be cancelled, depending
+     * on the caller, so leave it. */
     QLIST_FOREACH(other_job, &txn->jobs, txn_list) {
-        if (other_job == job || other_job->completed) {
-            /* Other jobs are "effectively" cancelled by us, set the status for
-             * them; this job, however, may or may not be cancelled, depending
-             * on the caller, so leave it. */
-            if (other_job != job) {
-                block_job_cancel_async(other_job);
-            }
-            continue;
+        if (other_job != job) {
+            block_job_cancel_async(other_job);
         }
-        block_job_cancel_sync(other_job);
-        assert(other_job->completed);
     }
-    QLIST_FOREACH_SAFE(other_job, &txn->jobs, txn_list, next) {
+    while (!QLIST_EMPTY(&txn->jobs)) {
+        other_job = QLIST_FIRST(&txn->jobs);
         ctx = blk_get_aio_context(other_job->blk);
+        if (!other_job->completed) {
+            assert(other_job->cancelled);
+            block_job_finish_sync(other_job, NULL, NULL);
+        }
         block_job_completed_single(other_job);
         aio_context_release(ctx);
     }
+
+    block_job_txn_unref(txn);
 }
 
 static void block_job_completed_txn_success(BlockJob *job)
-- 
2.9.3

From: Paolo Bonzini <pbonzini@redhat.com>

All block jobs are using block_job_defer_to_main_loop as the final
step just before the coroutine terminates.  At this point,
block_job_enter should do nothing, but currently it restarts
the freed coroutine.

Now, the job->co states should probably be changed to an enum
(e.g. BEFORE_START, STARTED, YIELDED, COMPLETED) subsuming
block_job_started, job->deferred_to_main_loop and job->busy.
For now, this patch eliminates the problematic reenter by
removing the reset of job->deferred_to_main_loop (which served
no purpose, as far as I could see) and checking the flag in
block_job_enter.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20170508141310.8674-12-pbonzini@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 blockjob.c                   | 10 ++++++++--
 include/block/blockjob_int.h |  3 ++-
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/blockjob.c b/blockjob.c
index XXXXXXX..XXXXXXX 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -XXX,XX +XXX,XX @@ void block_job_resume_all(void)
 
 void block_job_enter(BlockJob *job)
 {
-    if (job->co && !job->busy) {
+    if (!block_job_started(job)) {
+        return;
+    }
+    if (job->deferred_to_main_loop) {
+        return;
+    }
+
+    if (!job->busy) {
         bdrv_coroutine_enter(blk_bs(job->blk), job->co);
     }
 }
@@ -XXX,XX +XXX,XX @@ static void block_job_defer_to_main_loop_bh(void *opaque)
         aio_context_acquire(aio_context);
     }
 
-    data->job->deferred_to_main_loop = false;
     data->fn(data->job, data->opaque);
 
     if (aio_context != data->aio_context) {
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -XXX,XX +XXX,XX @@ typedef void BlockJobDeferToMainLoopFn(BlockJob *job, void *opaque);
  * @fn: The function to run in the main loop
  * @opaque: The opaque value that is passed to @fn
  *
- * Execute a given function in the main loop with the BlockDriverState
+ * This function must be called by the main job coroutine just before it
+ * returns.  @fn is executed in the main loop with the BlockDriverState
  * AioContext acquired.  Block jobs must call bdrv_unref(), bdrv_close(), and
  * anything that uses bdrv_drain_all() in the main loop.
  *
-- 
2.9.3

On current released versions of glusterfs, glfs_lseek() will sometimes
return invalid values for SEEK_DATA or SEEK_HOLE.  For SEEK_DATA and
SEEK_HOLE, the returned value should be >= the passed offset, or < 0 in
the case of error:

LSEEK(2):

off_t lseek(int fd, off_t offset, int whence);

[...]

SEEK_HOLE
              Adjust  the file offset to the next hole in the file greater
              than or equal to offset.  If offset points into the middle of
              a hole, then the file offset is set to offset.  If there is no
              hole past offset, then the file offset is adjusted to the end
              of the file (i.e., there is  an implicit hole at the end of
              any file).

[...]

RETURN VALUE
              Upon  successful  completion,  lseek()  returns  the resulting
              offset location as measured in bytes from the beginning of the
              file.  On error, the value (off_t) -1 is returned and errno is
              set to indicate the error

However, occasionally glfs_lseek() for SEEK_HOLE/DATA will return a
value less than the passed offset, yet greater than zero.

For instance, here are example values observed from this call:

offs = glfs_lseek(s->fd, start, SEEK_HOLE);
    if (offs < 0) {
        return -errno;          /* D1 and (H3 or H4) */
    }

start == 7608336384
offs == 7607877632

This causes QEMU to abort on the assert test.  When this value is
returned, errno is also 0.

This is a reported and known bug to glusterfs:
https://bugzilla.redhat.com/show_bug.cgi?id=1425293

Although this is being fixed in gluster, we still should work around it
in QEMU, given that multiple released versions of gluster behave this
way.

This patch treats the return case of (offs < start) the same as if an
error value other than ENXIO is returned; we will assume we learned
nothing, and there are no holes in the file.

Signed-off-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Niels de Vos <ndevos@redhat.com>
Message-id: 87c0140e9407c08f6e74b04131b610f2e27c014c.1495560397.git.jcody@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/gluster.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/block/gluster.c b/block/gluster.c
index XXXXXXX..XXXXXXX 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -XXX,XX +XXX,XX @@ static int find_allocation(BlockDriverState *bs, off_t start,
     if (offs < 0) {
         return -errno;          /* D3 or D4 */
     }
-    assert(offs >= start);
+
+    if (offs < start) {
+        /* This is not a valid return by lseek().  We are safe to just return
+         * -EIO in this case, and we'll treat it like D4. Unfortunately some
+         *  versions of gluster server will return offs < start, so an assert
+         *  here will unnecessarily abort QEMU. */
+        return -EIO;
+    }
 
     if (offs > start) {
         /* D2: in hole, next data at offs */
@@ -XXX,XX +XXX,XX @@ static int find_allocation(BlockDriverState *bs, off_t start,
     if (offs < 0) {
         return -errno;          /* D1 and (H3 or H4) */
     }
-    assert(offs >= start);
+
+    if (offs < start) {
+        /* This is not a valid return by lseek().  We are safe to just return
+         * -EIO in this case, and we'll treat it like H4. Unfortunately some
+         *  versions of gluster server will return offs < start, so an assert
+         *  here will unnecessarily abort QEMU. */
+        return -EIO;
+    }
 
     if (offs > start) {
         /*
-- 
2.9.3

The following changes since commit 5ec2eca83dc478ddf24077e02a8b34dd26cd3ff9:

Merge remote-tracking branch 'remotes/awilliam/tags/vfio-updates-20190613.0' into staging (2019-06-14 09:33:55 +0100)

are available in the Git repository at:

https://github.com/XanClic/qemu.git tags/pull-block-2019-06-14

for you to fetch changes up to 21c1ce592a144188dfe59b9e156a97da412a59a2:

iotests: Test qemu-img convert -C --salvage (2019-06-14 15:09:42 +0200)

----------------------------------------------------------------
Block patches:
- Allow blockdev-backup from nodes that are not in qemu's main AIO
  context to newly added nodes
- Add salvaging mode to qemu-img convert
- Minor fixes to tests, documentation, and for less Valgrind annoyance

----------------------------------------------------------------
Andrey Shinkevich (1):
  hw/block/fdc: floppy command FIFO memory initialization

John Snow (6):
  blockdev-backup: don't check aio_context too early
  iotests.py: do not use infinite waits
  QEMUMachine: add events_wait method
  iotests.py: rewrite run_job to be pickier
  iotests: add iotest 256 for testing blockdev-backup across iothread
    contexts
  event_match: always match on None value

Max Reitz (12):
  iotests: Filter 175's allocation information
  iotests: Fix intermittent failure in 219
  qemu-img: Fix options leakage in img_rebase()
  qapi/block-core: Overlays are not snapshots
  blockdev: Overlays are not snapshots
  qemu-img: Move quiet into ImgConvertState
  qemu-img: Add salvaging mode to convert
  blkdebug: Add @iotype error option
  blkdebug: Add "none" event
  blkdebug: Inject errors on .bdrv_co_block_status()
  iotests: Test qemu-img convert --salvage
  iotests: Test qemu-img convert -C --salvage

Vladimir Sementsov-Ogievskiy (1):
  iotests: restrict 254 to support only qcow2

qapi/block-core.json          |  53 ++++++++---
 block/blkdebug.c              |  60 ++++++++++--
 blockdev.c                    |  14 +--
 hw/block/fdc.c                |   1 +
 qemu-img.c                    | 106 +++++++++++++++------
 python/qemu/__init__.py       |  67 ++++++++++----
 qemu-img-cmds.hx              |   4 +-
 qemu-img.texi                 |   4 +
 tests/qemu-iotests/082        |   1 +
 tests/qemu-iotests/082.out    |   3 +
 tests/qemu-iotests/085.out    |  10 +-
 tests/qemu-iotests/175        |  26 +++++-
 tests/qemu-iotests/175.out    |   8 +-
 tests/qemu-iotests/219        |  13 ++-
 tests/qemu-iotests/251        | 170 ++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/251.out    |  43 +++++++++
 tests/qemu-iotests/254        |   2 +
 tests/qemu-iotests/256        | 122 ++++++++++++++++++++++++
 tests/qemu-iotests/256.out    | 119 ++++++++++++++++++++++++
 tests/qemu-iotests/group      |   2 +
 tests/qemu-iotests/iotests.py |  60 +++++++-----
 21 files changed, 772 insertions(+), 116 deletions(-)
 create mode 100755 tests/qemu-iotests/251
 create mode 100644 tests/qemu-iotests/251.out
 create mode 100755 tests/qemu-iotests/256
 create mode 100644 tests/qemu-iotests/256.out

-- 
2.21.0

From: John Snow <jsnow@redhat.com>

in blockdev_backup_prepare, we check to make sure that the target is
associated with a compatible aio context. However, do_blockdev_backup is
called later and has some logic to move the target to a compatible
aio_context. The transaction version will fail certain commands
needlessly early as a result.

Allow blockdev_backup_prepare to simply call do_blockdev_backup, which
will ultimately decide if the contexts are compatible or not.

Note: the transaction version has always disallowed this operation since
its initial commit bd8baecd (2014), whereas the version of
qmp_blockdev_backup at the time, from commit c29c1dd312f, tried to
enforce the aio_context switch instead. It's not clear, and I can't see
from the mailing list archives at the time, why the two functions take a
different approach. It wasn't until later in efd7556708b (2016) that the
standalone version tried to determine if it could set the context or
not.

Reported-by: aihua liang <aliang@redhat.com>
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1683498
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 20190523170643.20794-2-jsnow@redhat.com
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 blockdev.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index XXXXXXX..XXXXXXX 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -XXX,XX +XXX,XX @@ static void blockdev_backup_prepare(BlkActionState *common, Error **errp)
     }
 
     aio_context = bdrv_get_aio_context(bs);
-    if (aio_context != bdrv_get_aio_context(target)) {
-        error_setg(errp, "Backup between two IO threads is not implemented");
-        return;
-    }
     aio_context_acquire(aio_context);
     state->bs = bs;
 
-- 
2.21.0

From: John Snow <jsnow@redhat.com>

Cap waits to 60 seconds so that iotests can fail gracefully if something
goes wrong.

Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 20190523170643.20794-3-jsnow@redhat.com
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/iotests.py | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 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 @@ class VM(qtest.QEMUQtestMachine):
             output_list += [key + '=' + obj[key]]
         return ','.join(output_list)
 
-    def get_qmp_events_filtered(self, wait=True):
+    def get_qmp_events_filtered(self, wait=60.0):
         result = []
         for ev in self.get_qmp_events(wait=wait):
             result.append(filter_qmp_event(ev))
@@ -XXX,XX +XXX,XX @@ class VM(qtest.QEMUQtestMachine):
 
     # Returns None on success, and an error string on failure
     def run_job(self, job, auto_finalize=True, auto_dismiss=False,
-                pre_finalize=None):
+                pre_finalize=None, wait=60.0):
         error = None
         while True:
-            for ev in self.get_qmp_events_filtered(wait=True):
+            for ev in self.get_qmp_events_filtered(wait=wait):
                 if ev['event'] == 'JOB_STATUS_CHANGE':
                     status = ev['data']['status']
                     if status == 'aborting':
@@ -XXX,XX +XXX,XX @@ class QMPTestCase(unittest.TestCase):
         self.assertEqual(self.vm.flatten_qmp_object(json.loads(json_filename[5:])),
                          self.vm.flatten_qmp_object(reference))
 
-    def cancel_and_wait(self, drive='drive0', force=False, resume=False):
+    def cancel_and_wait(self, drive='drive0', force=False, resume=False, wait=60.0):
         '''Cancel a block job and wait for it to finish, returning the event'''
         result = self.vm.qmp('block-job-cancel', device=drive, force=force)
         self.assert_qmp(result, 'return', {})
@@ -XXX,XX +XXX,XX @@ class QMPTestCase(unittest.TestCase):
         cancelled = False
         result = None
         while not cancelled:
-            for event in self.vm.get_qmp_events(wait=True):
+            for event in self.vm.get_qmp_events(wait=wait):
                 if event['event'] == 'BLOCK_JOB_COMPLETED' or \
                    event['event'] == 'BLOCK_JOB_CANCELLED':
                     self.assert_qmp(event, 'data/device', drive)
@@ -XXX,XX +XXX,XX @@ class QMPTestCase(unittest.TestCase):
         self.assert_no_active_block_jobs()
         return result
 
-    def wait_until_completed(self, drive='drive0', check_offset=True):
+    def wait_until_completed(self, drive='drive0', check_offset=True, wait=60.0):
         '''Wait for a block job to finish, returning the event'''
         while True:
-            for event in self.vm.get_qmp_events(wait=True):
+            for event in self.vm.get_qmp_events(wait=wait):
                 if event['event'] == 'BLOCK_JOB_COMPLETED':
                     self.assert_qmp(event, 'data/device', drive)
                     self.assert_qmp_absent(event, 'data/error')
-- 
2.21.0

From: John Snow <jsnow@redhat.com>

Instead of event_wait which looks for a single event, add an events_wait
which can look for any number of events simultaneously. However, it
will still only return one at a time, whichever happens first.

Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 20190523170643.20794-4-jsnow@redhat.com
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 python/qemu/__init__.py | 69 +++++++++++++++++++++++++++++------------
 1 file changed, 49 insertions(+), 20 deletions(-)

diff --git a/python/qemu/__init__.py b/python/qemu/__init__.py
index XXXXXXX..XXXXXXX 100644
--- a/python/qemu/__init__.py
+++ b/python/qemu/__init__.py
@@ -XXX,XX +XXX,XX @@ class QEMUMachine(object):
         self._qmp.clear_events()
         return events
 
-    def event_wait(self, name, timeout=60.0, match=None):
+    @staticmethod
+    def event_match(event, match=None):
         """
-        Wait for specified timeout on named event in QMP; optionally filter
-        results by match.
+        Check if an event matches optional match criteria.
 
-        The 'match' is checked to be a recursive subset of the 'event'; skips
-        branch processing on match's value None
-           {"foo": {"bar": 1}} matches {"foo": None}
-           {"foo": {"bar": 1}} does not matches {"foo": {"baz": None}}
+        The match criteria takes the form of a matching subdict. The event is
+        checked to be a superset of the subdict, recursively, with matching
+        values whenever those values are not None.
+
+        Examples, with the subdict queries on the left:
+         - None matches any object.
+         - {"foo": None} matches {"foo": {"bar": 1}}
+         - {"foo": {"baz": None}} does not match {"foo": {"bar": 1}}
+         - {"foo": {"baz": 2}} matches {"foo": {"bar": 1, "baz": 2}}
         """
-        def event_match(event, match=None):
-            if match is None:
-                return True
+        if match is None:
+            return True
 
-            for key in match:
-                if key in event:
-                    if isinstance(event[key], dict):
-                        if not event_match(event[key], match[key]):
-                            return False
-                    elif event[key] != match[key]:
+        for key in match:
+            if key in event:
+                if isinstance(event[key], dict):
+                    if not QEMUMachine.event_match(event[key], match[key]):
                         return False
-                else:
+                elif event[key] != match[key]:
                     return False
+            else:
+                return False
+        return True
 
-            return True
+    def event_wait(self, name, timeout=60.0, match=None):
+        """
+        event_wait waits for and returns a named event from QMP with a timeout.
+
+        name: The event to wait for.
+        timeout: QEMUMonitorProtocol.pull_event timeout parameter.
+        match: Optional match criteria. See event_match for details.
+        """
+        return self.events_wait([(name, match)], timeout)
+
+    def events_wait(self, events, timeout=60.0):
+        """
+        events_wait waits for and returns a named event from QMP with a timeout.
+
+        events: a sequence of (name, match_criteria) tuples.
+                The match criteria are optional and may be None.
+                See event_match for details.
+        timeout: QEMUMonitorProtocol.pull_event timeout parameter.
+        """
+        def _match(event):
+            for name, match in events:
+                if (event['event'] == name and
+                    self.event_match(event, match)):
+                    return True
+            return False
 
         # Search cached events
         for event in self._events:
-            if (event['event'] == name) and event_match(event, match):
+            if _match(event):
                 self._events.remove(event)
                 return event
 
         # Poll for new events
         while True:
             event = self._qmp.pull_event(wait=timeout)
-            if (event['event'] == name) and event_match(event, match):
+            if _match(event):
                 return event
             self._events.append(event)
 
-- 
2.21.0

From: John Snow <jsnow@redhat.com>

Don't pull events out of the queue that don't belong to us;
be choosier so that we can use this method to drive jobs that
were launched by transactions that may have more jobs.

Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 20190523170643.20794-5-jsnow@redhat.com
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/iotests.py | 48 +++++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 19 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 @@ class VM(qtest.QEMUQtestMachine):
     # Returns None on success, and an error string on failure
     def run_job(self, job, auto_finalize=True, auto_dismiss=False,
                 pre_finalize=None, wait=60.0):
+        match_device = {'data': {'device': job}}
+        match_id = {'data': {'id': job}}
+        events = [
+            ('BLOCK_JOB_COMPLETED', match_device),
+            ('BLOCK_JOB_CANCELLED', match_device),
+            ('BLOCK_JOB_ERROR', match_device),
+            ('BLOCK_JOB_READY', match_device),
+            ('BLOCK_JOB_PENDING', match_id),
+            ('JOB_STATUS_CHANGE', match_id)
+        ]
         error = None
         while True:
-            for ev in self.get_qmp_events_filtered(wait=wait):
-                if ev['event'] == 'JOB_STATUS_CHANGE':
-                    status = ev['data']['status']
-                    if status == 'aborting':
-                        result = self.qmp('query-jobs')
-                        for j in result['return']:
-                            if j['id'] == job:
-                                error = j['error']
-                                log('Job failed: %s' % (j['error']))
-                    elif status == 'pending' and not auto_finalize:
-                        if pre_finalize:
-                            pre_finalize()
-                        self.qmp_log('job-finalize', id=job)
-                    elif status == 'concluded' and not auto_dismiss:
-                        self.qmp_log('job-dismiss', id=job)
-                    elif status == 'null':
-                        return error
-                else:
-                    log(ev)
+            ev = filter_qmp_event(self.events_wait(events))
+            if ev['event'] != 'JOB_STATUS_CHANGE':
+                log(ev)
+                continue
+            status = ev['data']['status']
+            if status == 'aborting':
+                result = self.qmp('query-jobs')
+                for j in result['return']:
+                    if j['id'] == job:
+                        error = j['error']
+                        log('Job failed: %s' % (j['error']))
+            elif status == 'pending' and not auto_finalize:
+                if pre_finalize:
+                    pre_finalize()
+                self.qmp_log('job-finalize', id=job)
+            elif status == 'concluded' and not auto_dismiss:
+                self.qmp_log('job-dismiss', id=job)
+            elif status == 'null':
+                return error
 
     def node_info(self, node_name):
         nodes = self.qmp('query-named-block-nodes')
-- 
2.21.0

From: John Snow <jsnow@redhat.com>

Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 20190523170643.20794-6-jsnow@redhat.com
Reviewed-by: Max Reitz <mreitz@redhat.com>
[mreitz: Moved from 250 to 256]
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/256     | 122 +++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/256.out | 119 ++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 242 insertions(+)
 create mode 100755 tests/qemu-iotests/256
 create mode 100644 tests/qemu-iotests/256.out

diff --git a/tests/qemu-iotests/256 b/tests/qemu-iotests/256
new file mode 100755
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qemu-iotests/256
@@ -XXX,XX +XXX,XX @@
+#!/usr/bin/env python
+#
+# Test incremental/backup across iothread contexts
+#
+# Copyright (c) 2019 John Snow for Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+# owner=jsnow@redhat.com
+
+import os
+import iotests
+from iotests import log
+
+iotests.verify_image_format(supported_fmts=['qcow2'])
+size = 64 * 1024 * 1024
+
+with iotests.FilePath('img0') as img0_path, \
+     iotests.FilePath('img1') as img1_path, \
+     iotests.FilePath('img0-full') as img0_full_path, \
+     iotests.FilePath('img1-full') as img1_full_path, \
+     iotests.FilePath('img0-incr') as img0_incr_path, \
+     iotests.FilePath('img1-incr') as img1_incr_path, \
+     iotests.VM() as vm:
+
+    def create_target(filepath, name, size):
+        basename = os.path.basename(filepath)
+        nodename = "file_{}".format(basename)
+        log(vm.command('blockdev-create', job_id='job1',
+                       options={
+                           'driver': 'file',
+                           'filename': filepath,
+                           'size': 0,
+                       }))
+        vm.run_job('job1')
+        log(vm.command('blockdev-add', driver='file',
+                       node_name=nodename, filename=filepath))
+        log(vm.command('blockdev-create', job_id='job2',
+                       options={
+                           'driver': iotests.imgfmt,
+                           'file': nodename,
+                           'size': size,
+                       }))
+        vm.run_job('job2')
+        log(vm.command('blockdev-add', driver=iotests.imgfmt,
+                       node_name=name,
+                       file=nodename))
+
+    log('--- Preparing images & VM ---\n')
+    vm.add_object('iothread,id=iothread0')
+    vm.add_object('iothread,id=iothread1')
+    vm.add_device('virtio-scsi-pci,id=scsi0,iothread=iothread0')
+    vm.add_device('virtio-scsi-pci,id=scsi1,iothread=iothread1')
+    iotests.qemu_img_create('-f', iotests.imgfmt, img0_path, str(size))
+    iotests.qemu_img_create('-f', iotests.imgfmt, img1_path, str(size))
+    vm.add_drive(img0_path, interface='none')
+    vm.add_device('scsi-hd,id=device0,drive=drive0,bus=scsi0.0')
+    vm.add_drive(img1_path, interface='none')
+    vm.add_device('scsi-hd,id=device1,drive=drive1,bus=scsi1.0')
+
+    log('--- Starting VM ---\n')
+    vm.launch()
+
+    log('--- Create Targets & Full Backups ---\n')
+    create_target(img0_full_path, 'img0-full', size)
+    create_target(img1_full_path, 'img1-full', size)
+    ret = vm.qmp_log('transaction', indent=2, actions=[
+        { 'type': 'block-dirty-bitmap-add',
+          'data': { 'node': 'drive0', 'name': 'bitmap0' }},
+        { 'type': 'block-dirty-bitmap-add',
+          'data': { 'node': 'drive1', 'name': 'bitmap1' }},
+        { 'type': 'blockdev-backup',
+          'data': { 'device': 'drive0',
+                    'target': 'img0-full',
+                    'sync': 'full',
+                    'job-id': 'j0' }},
+        { 'type': 'blockdev-backup',
+          'data': { 'device': 'drive1',
+                    'target': 'img1-full',
+                    'sync': 'full',
+                    'job-id': 'j1' }}
+    ])
+    if "error" in ret:
+        raise Exception(ret['error']['desc'])
+    vm.run_job('j0', auto_dismiss=True)
+    vm.run_job('j1', auto_dismiss=True)
+
+    log('\n--- Create Targets & Incremental Backups ---\n')
+    create_target(img0_incr_path, 'img0-incr', size)
+    create_target(img1_incr_path, 'img1-incr', size)
+    ret = vm.qmp_log('transaction', indent=2, actions=[
+        { 'type': 'blockdev-backup',
+          'data': { 'device': 'drive0',
+                    'target': 'img0-incr',
+                    'sync': 'incremental',
+                    'bitmap': 'bitmap0',
+                    'job-id': 'j2' }},
+        { 'type': 'blockdev-backup',
+          'data': { 'device': 'drive1',
+                    'target': 'img1-incr',
+                    'sync': 'incremental',
+                    'bitmap': 'bitmap1',
+                    'job-id': 'j3' }}
+    ])
+    if "error" in ret:
+        raise Exception(ret['error']['desc'])
+    vm.run_job('j2', auto_dismiss=True)
+    vm.run_job('j3', auto_dismiss=True)
+
+    log('\n--- Done ---')
+    vm.shutdown()
diff --git a/tests/qemu-iotests/256.out b/tests/qemu-iotests/256.out
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qemu-iotests/256.out
@@ -XXX,XX +XXX,XX @@
+--- Preparing images & VM ---
+
+--- Starting VM ---
+
+--- Create Targets & Full Backups ---
+
+{}
+{"execute": "job-dismiss", "arguments": {"id": "job1"}}
+{"return": {}}
+{}
+{}
+{"execute": "job-dismiss", "arguments": {"id": "job2"}}
+{"return": {}}
+{}
+{}
+{"execute": "job-dismiss", "arguments": {"id": "job1"}}
+{"return": {}}
+{}
+{}
+{"execute": "job-dismiss", "arguments": {"id": "job2"}}
+{"return": {}}
+{}
+{
+  "execute": "transaction",
+  "arguments": {
+    "actions": [
+      {
+        "data": {
+          "name": "bitmap0",
+          "node": "drive0"
+        },
+        "type": "block-dirty-bitmap-add"
+      },
+      {
+        "data": {
+          "name": "bitmap1",
+          "node": "drive1"
+        },
+        "type": "block-dirty-bitmap-add"
+      },
+      {
+        "data": {
+          "device": "drive0",
+          "job-id": "j0",
+          "sync": "full",
+          "target": "img0-full"
+        },
+        "type": "blockdev-backup"
+      },
+      {
+        "data": {
+          "device": "drive1",
+          "job-id": "j1",
+          "sync": "full",
+          "target": "img1-full"
+        },
+        "type": "blockdev-backup"
+      }
+    ]
+  }
+}
+{
+  "return": {}
+}
+{"data": {"device": "j0", "len": 67108864, "offset": 67108864, "speed": 0, "type": "backup"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
+{"data": {"device": "j1", "len": 67108864, "offset": 67108864, "speed": 0, "type": "backup"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
+
+--- Create Targets & Incremental Backups ---
+
+{}
+{"execute": "job-dismiss", "arguments": {"id": "job1"}}
+{"return": {}}
+{}
+{}
+{"execute": "job-dismiss", "arguments": {"id": "job2"}}
+{"return": {}}
+{}
+{}
+{"execute": "job-dismiss", "arguments": {"id": "job1"}}
+{"return": {}}
+{}
+{}
+{"execute": "job-dismiss", "arguments": {"id": "job2"}}
+{"return": {}}
+{}
+{
+  "execute": "transaction",
+  "arguments": {
+    "actions": [
+      {
+        "data": {
+          "bitmap": "bitmap0",
+          "device": "drive0",
+          "job-id": "j2",
+          "sync": "incremental",
+          "target": "img0-incr"
+        },
+        "type": "blockdev-backup"
+      },
+      {
+        "data": {
+          "bitmap": "bitmap1",
+          "device": "drive1",
+          "job-id": "j3",
+          "sync": "incremental",
+          "target": "img1-incr"
+        },
+        "type": "blockdev-backup"
+      }
+    ]
+  }
+}
+{
+  "return": {}
+}
+{"data": {"device": "j2", "len": 67108864, "offset": 67108864, "speed": 0, "type": "backup"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
+{"data": {"device": "j3", "len": 67108864, "offset": 67108864, "speed": 0, "type": "backup"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
+
+--- Done ---
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -XXX,XX +XXX,XX @@
 253 rw auto quick
 254 rw auto backing quick
 255 rw auto quick
+256 rw auto quick
-- 
2.21.0

From: John Snow <jsnow@redhat.com>

Before, event_match didn't always recurse if the event value was not a
dictionary, and would instead check for equality immediately.

By delaying equality checking to post-recursion, we can allow leaf
values like "5" to match "None" and take advantage of the generic
None-returns-True clause.

This makes the matching a little more obviously consistent at the
expense of being able to check for explicit None values, which is
probably not that important given what this function is used for.

Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 20190528183857.26167-1-jsnow@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 python/qemu/__init__.py | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/python/qemu/__init__.py b/python/qemu/__init__.py
index XXXXXXX..XXXXXXX 100644
--- a/python/qemu/__init__.py
+++ b/python/qemu/__init__.py
@@ -XXX,XX +XXX,XX @@ class QEMUMachine(object):
 
         The match criteria takes the form of a matching subdict. The event is
         checked to be a superset of the subdict, recursively, with matching
-        values whenever those values are not None.
+        values whenever the subdict values are not None.
+
+        This has a limitation that you cannot explicitly check for None values.
 
         Examples, with the subdict queries on the left:
          - None matches any object.
          - {"foo": None} matches {"foo": {"bar": 1}}
-         - {"foo": {"baz": None}} does not match {"foo": {"bar": 1}}
-         - {"foo": {"baz": 2}} matches {"foo": {"bar": 1, "baz": 2}}
+         - {"foo": None} matches {"foo": 5}
+         - {"foo": {"abc": None}} does not match {"foo": {"bar": 1}}
+         - {"foo": {"rab": 2}} matches {"foo": {"bar": 1, "rab": 2}}
         """
         if match is None:
             return True
 
-        for key in match:
-            if key in event:
-                if isinstance(event[key], dict):
+        try:
+            for key in match:
+                if key in event:
                     if not QEMUMachine.event_match(event[key], match[key]):
                         return False
-                elif event[key] != match[key]:
+                else:
                     return False
-            else:
-                return False
-        return True
+            return True
+        except TypeError:
+            # either match or event wasn't iterable (not a dict)
+            return match == event
 
     def event_wait(self, name, timeout=60.0, match=None):
         """
-- 
2.21.0

It is possible for an empty file to take up blocks on a filesystem, for
example:

$ qemu-img create -f raw test.img 1G
Formatting 'test.img', fmt=raw size=1073741824
$ mkfs.ext4 -I 128 -q test.img
$ mkdir test-mount
$ sudo mount -o loop test.img test-mount
$ sudo touch test-mount/test-file
$ stat -c 'blocks=%b' test-mount/test-file
blocks=8

These extra blocks (one cluster) are apparently used for metadata,
because they are always there, on top of blocks used for data:

$ sudo dd if=/dev/zero of=test-mount/test-file bs=1M count=1
1+0 records in
1+0 records out
1048576 bytes (1.0 MB, 1.0 MiB) copied, 0.00135339 s, 775 MB/s
$ stat -c 'blocks=%b' test-mount/test-file
blocks=2056

Make iotest 175 take this into account.

Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Nir Soffer <nsoffer@redhat.com>
Message-id: 20190516144319.12570-1-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/175     | 26 ++++++++++++++++++++++----
 tests/qemu-iotests/175.out |  8 ++++----
 2 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/tests/qemu-iotests/175 b/tests/qemu-iotests/175
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/175
+++ b/tests/qemu-iotests/175
@@ -XXX,XX +XXX,XX @@ status=1	# failure is the default!
 
 _cleanup()
 {
-	_cleanup_test_img
+    _cleanup_test_img
+    rm -f "$TEST_DIR/empty"
 }
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
+# Some file systems sometimes allocate extra blocks independently of
+# the file size.  This function hides the resulting difference in the
+# stat -c '%b' output.
+# Parameter 1: Number of blocks an empty file occupies
+# Parameter 2: Image size in bytes
+_filter_blocks()
+{
+    extra_blocks=$1
+    img_size=$2
+
+    sed -e "s/blocks=$extra_blocks\$\$\\|[^0-9]\$/nothing allocated/" \
+        -e "s/blocks=$((extra_blocks + img_size / 512))\$\$\\|[^0-9]\$/everything allocated/"
+}
+
 # get standard environment, filters and checks
 . ./common.rc
 . ./common.filter
@@ -XXX,XX +XXX,XX @@ _supported_fmt raw
 _supported_proto file
 _supported_os Linux
 
-size=1m
+size=$((1 * 1024 * 1024))
+
+touch "$TEST_DIR/empty"
+extra_blocks=$(stat -c '%b' "$TEST_DIR/empty")
 
 echo
 echo "== creating image with default preallocation =="
 _make_test_img $size | _filter_imgfmt
-stat -c "size=%s, blocks=%b" $TEST_IMG
+stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $size
 
 for mode in off full falloc; do
     echo
     echo "== creating image with preallocation $mode =="
     IMGOPTS=preallocation=$mode _make_test_img $size | _filter_imgfmt
-    stat -c "size=%s, blocks=%b" $TEST_IMG
+    stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $size
 done
 
 # success, all done
diff --git a/tests/qemu-iotests/175.out b/tests/qemu-iotests/175.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/175.out
+++ b/tests/qemu-iotests/175.out
@@ -XXX,XX +XXX,XX @@ QA output created by 175
 
 == creating image with default preallocation ==
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
-size=1048576, blocks=0
+size=1048576, nothing allocated
 
 == creating image with preallocation off ==
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 preallocation=off
-size=1048576, blocks=0
+size=1048576, nothing allocated
 
 == creating image with preallocation full ==
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 preallocation=full
-size=1048576, blocks=2048
+size=1048576, everything allocated
 
 == creating image with preallocation falloc ==
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 preallocation=falloc
-size=1048576, blocks=2048
+size=1048576, everything allocated
  *** done
-- 
2.21.0

In 219, we wait for the job to make progress before we emit its status.
This makes the output reliable.  We do not wait for any more progress if
the job's current-progress already matches its total-progress.

Unfortunately, there is a bug: Right after the job has been started,
it's possible that total-progress is still 0.  In that case, we may skip
the first progress-making step and keep ending up 64 kB short.

To fix that bug, we can simply wait for total-progress to reach 4 MB
(the image size) after starting the job.

Reported-by: Karen Mezick <kmezick@redhat.com>
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1686651
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20190516161114.27596-1-mreitz@redhat.com
Reviewed-by: John Snow <jsnow@redhat.com>
[mreitz: Adjusted commit message as per John's proposal]
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/219 | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/219 b/tests/qemu-iotests/219
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/219
+++ b/tests/qemu-iotests/219
@@ -XXX,XX +XXX,XX @@ import iotests
 
 iotests.verify_image_format(supported_fmts=['qcow2'])
 
+img_size = 4 * 1024 * 1024
+
 def pause_wait(vm, job_id):
     with iotests.Timeout(3, "Timeout waiting for job to pause"):
         while True:
@@ -XXX,XX +XXX,XX @@ def test_pause_resume(vm):
                 iotests.log(vm.qmp('query-jobs'))
 
 def test_job_lifecycle(vm, job, job_args, has_ready=False):
+    global img_size
+
     iotests.log('')
     iotests.log('')
     iotests.log('Starting block job: %s (auto-finalize: %s; auto-dismiss: %s)' %
@@ -XXX,XX +XXX,XX @@ def test_job_lifecycle(vm, job, job_args, has_ready=False):
     iotests.log(iotests.filter_qmp_event(vm.event_wait('JOB_STATUS_CHANGE')))
     iotests.log(iotests.filter_qmp_event(vm.event_wait('JOB_STATUS_CHANGE')))
 
+    # Wait for total-progress to stabilize
+    while vm.qmp('query-jobs')['return'][0]['total-progress'] < img_size:
+        pass
+
     # RUNNING state:
     # pause/resume should work, complete/finalize/dismiss should error out
     iotests.log('')
@@ -XXX,XX +XXX,XX @@ with iotests.FilePath('disk.img') as disk_path, \
      iotests.FilePath('copy.img') as copy_path, \
      iotests.VM() as vm:
 
-    img_size = '4M'
-    iotests.qemu_img_create('-f', iotests.imgfmt, disk_path, img_size)
-    iotests.qemu_io('-c', 'write 0 %s' % (img_size),
+    iotests.qemu_img_create('-f', iotests.imgfmt, disk_path, str(img_size))
+    iotests.qemu_io('-c', 'write 0 %i' % (img_size),
                     '-f', iotests.imgfmt, disk_path)
 
     iotests.log('Launching VM...')
-- 
2.21.0

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

The uninitialized memory allocated for the command FIFO of the
floppy controller during the VM hardware initialization incurs
many unwanted reports by Valgrind when VM state is being saved.
That verbosity hardens a search for the real memory issues when
the iotests run. Particularly, the patch eliminates 20 unnecessary
reports of the Valgrind tool in the iotest #169.

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Message-id: 1559154027-282547-1-git-send-email-andrey.shinkevich@virtuozzo.com
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 hw/block/fdc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -XXX,XX +XXX,XX @@ static void fdctrl_realize_common(DeviceState *dev, FDCtrl *fdctrl,
 
     FLOPPY_DPRINTF("init controller\n");
     fdctrl->fifo = qemu_memalign(512, FD_SECTOR_LEN);
+    memset(fdctrl->fifo, 0, FD_SECTOR_LEN);
     fdctrl->fifo_size = 512;
     fdctrl->result_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
                                              fdctrl_result_timer, fdctrl);
-- 
2.21.0

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

Test fails at least for qcow, because of different cluster sizes in
base and top (and therefore different granularities of bitmaps we are
trying to merge).

The test aim is to check block-dirty-bitmap-merge between different
nodes functionality, no needs to check all formats. So, let's just drop
support for anything except qcow2.

Reported-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190605155405.104384-1-vsementsov@virtuozzo.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/254 | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/qemu-iotests/254 b/tests/qemu-iotests/254
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/254
+++ b/tests/qemu-iotests/254
@@ -XXX,XX +XXX,XX @@
 import iotests
 from iotests import qemu_img_create, file_path, log
 
+iotests.verify_image_format(supported_fmts=['qcow2'])
+
 disk, top = file_path('disk', 'top')
 size = 1024 * 1024
 
-- 
2.21.0

img_rebase() can leak a QDict in two occasions.  Fix it.

Coverity: CID 1401416
Fixes: d16699b64671466b42079c45b89127aeea1ca565
Fixes: 330c72957196e0ae382abcaa97ebf4eb9bc8574f
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20190528195338.12376-1-mreitz@redhat.com
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qemu-img.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/qemu-img.c b/qemu-img.c
index XXXXXXX..XXXXXXX 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -XXX,XX +XXX,XX @@ static int img_rebase(int argc, char **argv)
                                                              out_baseimg,
                                                              &local_err);
             if (local_err) {
+                qobject_unref(options);
                 error_reportf_err(local_err,
                                   "Could not resolve backing filename: ");
                 ret = -1;
@@ -XXX,XX +XXX,XX @@ static int img_rebase(int argc, char **argv)
              */
             prefix_chain_bs = bdrv_find_backing_image(bs, out_real_path);
             if (prefix_chain_bs) {
+                qobject_unref(options);
                 g_free(out_real_path);
+
                 blk_new_backing = blk_new(qemu_get_aio_context(),
                                           BLK_PERM_CONSISTENT_READ,
                                           BLK_PERM_ALL);
-- 
2.21.0

A snapshot is something that reflects the state of something at a
certain point in time.  It does not change.

The file our snapshot commands create (or the node they install) is not
a snapshot, as it does change over time.  It is an overlay.  We cannot
do anything about the parameter names, but we can at least adjust the
descriptions to reflect that fact.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20190603202236.1342-2-mreitz@redhat.com
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qapi/block-core.json | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index XXXXXXX..XXXXXXX 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -XXX,XX +XXX,XX @@
 #
 # Either @device or @node-name must be set but not both.
 #
-# @device: the name of the device to generate the snapshot from.
+# @device: the name of the device to take a snapshot of.
 #
 # @node-name: graph node name to generate the snapshot from (Since 2.0)
 #
-# @snapshot-file: the target of the new image. If the file exists, or
-# if it is a device, the snapshot will be created in the existing
-# file/device. Otherwise, a new file will be created.
+# @snapshot-file: the target of the new overlay image. If the file
+# exists, or if it is a device, the overlay will be created in the
+# existing file/device. Otherwise, a new file will be created.
 #
 # @snapshot-node-name: the graph node name of the new image (Since 2.0)
 #
-# @format: the format of the snapshot image, default is 'qcow2'.
+# @format: the format of the overlay image, default is 'qcow2'.
 #
 # @mode: whether and how QEMU should create a new image, default is
 #        'absolute-paths'.
@@ -XXX,XX +XXX,XX @@
 ##
 # @BlockdevSnapshot:
 #
-# @node: device or node name that will have a snapshot created.
+# @node: device or node name that will have a snapshot taken.
 #
 # @overlay: reference to the existing block device that will become
-#           the overlay of @node, as part of creating the snapshot.
+#           the overlay of @node, as part of taking the snapshot.
 #           It must not have a current backing file (this can be
 #           achieved by passing "backing": null to blockdev-add).
 #
@@ -XXX,XX +XXX,XX @@
 ##
 # @blockdev-snapshot-sync:
 #
-# Generates a synchronous snapshot of a block device.
+# Takes a synchronous snapshot of a block device.
 #
 # For the arguments, see the documentation of BlockdevSnapshotSync.
 #
@@ -XXX,XX +XXX,XX @@
 ##
 # @blockdev-snapshot:
 #
-# Generates a snapshot of a block device.
+# Takes a snapshot of a block device.
 #
-# Create a snapshot, by installing 'node' as the backing image of
+# Take a snapshot, by installing 'node' as the backing image of
 # 'overlay'. Additionally, if 'node' is associated with a block
 # device, the block device changes to using 'overlay' as its new active
 # image.
-- 
2.21.0

There are error messages which refer to an overlay node as the snapshot.
That is wrong, those are two different things.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20190603202236.1342-3-mreitz@redhat.com
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 blockdev.c                 | 10 +++++-----
 tests/qemu-iotests/085.out | 10 +++++-----
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index XXXXXXX..XXXXXXX 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
             s->has_snapshot_node_name ? s->snapshot_node_name : NULL;
 
         if (node_name && !snapshot_node_name) {
-            error_setg(errp, "New snapshot node name missing");
+            error_setg(errp, "New overlay node name missing");
             goto out;
         }
 
         if (snapshot_node_name &&
             bdrv_lookup_bs(snapshot_node_name, snapshot_node_name, NULL)) {
-            error_setg(errp, "New snapshot node name already in use");
+            error_setg(errp, "New overlay node name already in use");
             goto out;
         }
 
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
     }
 
     if (bdrv_has_blk(state->new_bs)) {
-        error_setg(errp, "The snapshot is already in use");
+        error_setg(errp, "The overlay is already in use");
         goto out;
     }
 
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
     }
 
     if (state->new_bs->backing != NULL) {
-        error_setg(errp, "The snapshot already has a backing image");
+        error_setg(errp, "The overlay already has a backing image");
         goto out;
     }
 
     if (!state->new_bs->drv->supports_backing) {
-        error_setg(errp, "The snapshot does not support backing images");
+        error_setg(errp, "The overlay does not support backing images");
         goto out;
     }
 
diff --git a/tests/qemu-iotests/085.out b/tests/qemu-iotests/085.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/085.out
+++ b/tests/qemu-iotests/085.out
@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/
 
 === Invalid command - cannot create a snapshot using a file BDS ===
 
-{"error": {"class": "GenericError", "desc": "The snapshot does not support backing images"}}
+{"error": {"class": "GenericError", "desc": "The overlay does not support backing images"}}
 
 === Invalid command - snapshot node used as active layer ===
 
-{"error": {"class": "GenericError", "desc": "The snapshot is already in use"}}
-{"error": {"class": "GenericError", "desc": "The snapshot is already in use"}}
-{"error": {"class": "GenericError", "desc": "The snapshot is already in use"}}
+{"error": {"class": "GenericError", "desc": "The overlay is already in use"}}
+{"error": {"class": "GenericError", "desc": "The overlay is already in use"}}
+{"error": {"class": "GenericError", "desc": "The overlay is already in use"}}
 
 === Invalid command - snapshot node used as backing hd ===
 
@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/
 Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
 {"return": {}}
-{"error": {"class": "GenericError", "desc": "The snapshot already has a backing image"}}
+{"error": {"class": "GenericError", "desc": "The overlay already has a backing image"}}
 
 === Invalid command - The node does not exist ===
 
-- 
2.21.0

Move img_convert()'s quiet flag into the ImgConvertState so it is
accessible by nested functions.  -q dictates that it suppresses anything
but errors, so if those functions want to emit warnings, they need to
query this flag first.  (There currently are no such warnings, but there
will be as of the next patch.)

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190507203508.18026-2-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qemu-img.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index XXXXXXX..XXXXXXX 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -XXX,XX +XXX,XX @@ typedef struct ImgConvertState {
     int64_t target_backing_sectors; /* negative if unknown */
     bool wr_in_order;
     bool copy_range;
+    bool quiet;
     int min_sparse;
     int alignment;
     size_t cluster_sectors;
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
     QDict *open_opts = NULL;
     char *options = NULL;
     Error *local_err = NULL;
-    bool writethrough, src_writethrough, quiet = false, image_opts = false,
+    bool writethrough, src_writethrough, image_opts = false,
          skip_create = false, progress = false, tgt_image_opts = false;
     int64_t ret = -EINVAL;
     bool force_share = false;
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
             src_cache = optarg;
             break;
         case 'q':
-            quiet = true;
+            s.quiet = true;
             break;
         case 'n':
             skip_create = true;
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
     }
 
     /* Initialize before goto out */
-    if (quiet) {
+    if (s.quiet) {
         progress = false;
     }
     qemu_progress_init(progress, 1.0);
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
 
     for (bs_i = 0; bs_i < s.src_num; bs_i++) {
         s.src[bs_i] = img_open(image_opts, argv[optind + bs_i],
-                               fmt, src_flags, src_writethrough, quiet,
+                               fmt, src_flags, src_writethrough, s.quiet,
                                force_share);
         if (!s.src[bs_i]) {
             ret = -1;
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
 
     if (skip_create) {
         s.target = img_open(tgt_image_opts, out_filename, out_fmt,
-                            flags, writethrough, quiet, false);
+                            flags, writethrough, s.quiet, false);
     } else {
         /* TODO ultimately we should allow --target-image-opts
          * to be used even when -n is not given.
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
          * to allow filenames in option syntax
          */
         s.target = img_open_file(out_filename, open_opts, out_fmt,
-                                 flags, writethrough, quiet, false);
+                                 flags, writethrough, s.quiet, false);
         open_opts = NULL; /* blk_new_open will have freed it */
     }
     if (!s.target) {
-- 
2.21.0

This adds a salvaging mode (--salvage) to qemu-img convert which ignores
read errors and treats the respective areas as containing only zeroes.
This can be used for instance to at least partially recover the data
from terminally corrupted qcow2 images.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190507203508.18026-3-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qemu-img.c       | 90 +++++++++++++++++++++++++++++++++++++-----------
 qemu-img-cmds.hx |  4 +--
 qemu-img.texi    |  4 +++
 3 files changed, 75 insertions(+), 23 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index XXXXXXX..XXXXXXX 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -XXX,XX +XXX,XX @@ enum {
     OPTION_SIZE = 264,
     OPTION_PREALLOCATION = 265,
     OPTION_SHRINK = 266,
+    OPTION_SALVAGE = 267,
 };
 
 typedef enum OutputFormat {
@@ -XXX,XX +XXX,XX @@ typedef struct ImgConvertState {
     int64_t target_backing_sectors; /* negative if unknown */
     bool wr_in_order;
     bool copy_range;
+    bool salvage;
     bool quiet;
     int min_sparse;
     int alignment;
@@ -XXX,XX +XXX,XX @@ static int convert_iteration_sectors(ImgConvertState *s, int64_t sector_num)
     }
 
     if (s->sector_next_status <= sector_num) {
-        int64_t count = n * BDRV_SECTOR_SIZE;
+        uint64_t offset = (sector_num - src_cur_offset) * BDRV_SECTOR_SIZE;
+        int64_t count;
 
-        if (s->target_has_backing) {
+        do {
+            count = n * BDRV_SECTOR_SIZE;
+
+            if (s->target_has_backing) {
+                ret = bdrv_block_status(blk_bs(s->src[src_cur]), offset,
+                                        count, &count, NULL, NULL);
+            } else {
+                ret = bdrv_block_status_above(blk_bs(s->src[src_cur]), NULL,
+                                              offset, count, &count, NULL,
+                                              NULL);
+            }
+
+            if (ret < 0) {
+                if (s->salvage) {
+                    if (n == 1) {
+                        if (!s->quiet) {
+                            warn_report("error while reading block status at "
+                                        "offset %" PRIu64 ": %s", offset,
+                                        strerror(-ret));
+                        }
+                        /* Just try to read the data, then */
+                        ret = BDRV_BLOCK_DATA;
+                        count = BDRV_SECTOR_SIZE;
+                    } else {
+                        /* Retry on a shorter range */
+                        n = DIV_ROUND_UP(n, 4);
+                    }
+                } else {
+                    error_report("error while reading block status at offset "
+                                 "%" PRIu64 ": %s", offset, strerror(-ret));
+                    return ret;
+                }
+            }
+        } while (ret < 0);
 
-            ret = bdrv_block_status(blk_bs(s->src[src_cur]),
-                                    (sector_num - src_cur_offset) *
-                                    BDRV_SECTOR_SIZE,
-                                    count, &count, NULL, NULL);
-        } else {
-            ret = bdrv_block_status_above(blk_bs(s->src[src_cur]), NULL,
-                                          (sector_num - src_cur_offset) *
-                                          BDRV_SECTOR_SIZE,
-                                          count, &count, NULL, NULL);
-        }
-        if (ret < 0) {
-            error_report("error while reading block status of sector %" PRId64
-                         ": %s", sector_num, strerror(-ret));
-            return ret;
-        }
         n = DIV_ROUND_UP(count, BDRV_SECTOR_SIZE);
 
         if (ret & BDRV_BLOCK_ZERO) {
@@ -XXX,XX +XXX,XX @@ static int convert_iteration_sectors(ImgConvertState *s, int64_t sector_num)
 static int coroutine_fn convert_co_read(ImgConvertState *s, int64_t sector_num,
                                         int nb_sectors, uint8_t *buf)
 {
+    uint64_t single_read_until = 0;
     int n, ret;
 
     assert(nb_sectors <= s->buf_sectors);
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn convert_co_read(ImgConvertState *s, int64_t sector_num,
         BlockBackend *blk;
         int src_cur;
         int64_t bs_sectors, src_cur_offset;
+        uint64_t offset;
 
         /* In the case of compression with multiple source files, we can get a
          * nb_sectors that spreads into the next part. So we must be able to
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn convert_co_read(ImgConvertState *s, int64_t sector_num,
         blk = s->src[src_cur];
         bs_sectors = s->src_sectors[src_cur];
 
+        offset = (sector_num - src_cur_offset) << BDRV_SECTOR_BITS;
+
         n = MIN(nb_sectors, bs_sectors - (sector_num - src_cur_offset));
+        if (single_read_until > offset) {
+            n = 1;
+        }
 
-        ret = blk_co_pread(
-                blk, (sector_num - src_cur_offset) << BDRV_SECTOR_BITS,
-                n << BDRV_SECTOR_BITS, buf, 0);
+        ret = blk_co_pread(blk, offset, n << BDRV_SECTOR_BITS, buf, 0);
         if (ret < 0) {
-            return ret;
+            if (s->salvage) {
+                if (n > 1) {
+                    single_read_until = offset + (n << BDRV_SECTOR_BITS);
+                    continue;
+                } else {
+                    if (!s->quiet) {
+                        warn_report("error while reading offset %" PRIu64
+                                    ": %s", offset, strerror(-ret));
+                    }
+                    memset(buf, 0, BDRV_SECTOR_SIZE);
+                }
+            } else {
+                return ret;
+            }
         }
 
         sector_num += n;
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
             {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
             {"force-share", no_argument, 0, 'U'},
             {"target-image-opts", no_argument, 0, OPTION_TARGET_IMAGE_OPTS},
+            {"salvage", no_argument, 0, OPTION_SALVAGE},
             {0, 0, 0, 0}
         };
         c = getopt_long(argc, argv, ":hf:O:B:Cco:l:S:pt:T:qnm:WU",
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
         case OPTION_IMAGE_OPTS:
             image_opts = true;
             break;
+        case OPTION_SALVAGE:
+            s.salvage = true;
+            break;
         case OPTION_TARGET_IMAGE_OPTS:
             tgt_image_opts = true;
             break;
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
         goto fail_getopt;
     }
 
+    if (s.copy_range && s.salvage) {
+        error_report("Cannot use copy offloading in salvaging mode");
+        goto fail_getopt;
+    }
+
     if (tgt_image_opts && !skip_create) {
         error_report("--target-image-opts requires use of -n flag");
         goto fail_getopt;
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index XXXXXXX..XXXXXXX 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -XXX,XX +XXX,XX @@ STEXI
 ETEXI
 
 DEF("convert", img_convert,
-    "convert [--object objectdef] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")
+    "convert [--object objectdef] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] [--salvage] filename [filename2 [...]] output_filename")
 STEXI
-@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
+@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] [--salvage] @var{filename} [@var{filename2} [...]] @var{output_filename}
 ETEXI
 
 DEF("create", img_create,
diff --git a/qemu-img.texi b/qemu-img.texi
index XXXXXXX..XXXXXXX 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -XXX,XX +XXX,XX @@ improve performance if the data is remote, such as with NFS or iSCSI backends,
 but will not automatically sparsify zero sectors, and may result in a fully
 allocated target image depending on the host support for getting allocation
 information.
+@item --salvage
+Try to ignore I/O errors when reading.  Unless in quiet mode (@code{-q}), errors
+will still be printed.  Areas that cannot be read from the source will be
+treated as containing only zeroes.
 @end table
 
 Parameters to dd subcommand:
-- 
2.21.0

This new error option allows users of blkdebug to inject errors only on
certain kinds of I/O operations.  Users usually want to make a very
specific operation fail, not just any; but right now they simply hope
that the event that triggers the error injection is followed up with
that very operation.  That may not be true, however, because the block
layer is changing (including blkdebug, which may increase the number of
types of I/O operations on which to inject errors).

The new option's default has been chosen to keep backwards
compatibility.

Note that similar to the internal representation, we could choose to
expose this option as a list of I/O types.  But there is no practical
use for this, because as described above, users usually know exactly
which kind of operation they want to make fail, so there is no need to
specify multiple I/O types at once.  In addition, exposing this option
as a list would require non-trivial changes to qemu_opts_absorb_qdict().

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190507203508.18026-4-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qapi/block-core.json | 26 +++++++++++++++++++++++
 block/blkdebug.c     | 50 ++++++++++++++++++++++++++++++++++++--------
 2 files changed, 67 insertions(+), 9 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index XXXXXXX..XXXXXXX 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -XXX,XX +XXX,XX @@
             'l1_shrink_write_table', 'l1_shrink_free_l2_clusters',
             'cor_write', 'cluster_alloc_space'] }
 
+##
+# @BlkdebugIOType:
+#
+# Kinds of I/O that blkdebug can inject errors in.
+#
+# @read: .bdrv_co_preadv()
+#
+# @write: .bdrv_co_pwritev()
+#
+# @write-zeroes: .bdrv_co_pwrite_zeroes()
+#
+# @discard: .bdrv_co_pdiscard()
+#
+# @flush: .bdrv_co_flush_to_disk()
+#
+# Since: 4.1
+##
+{ 'enum': 'BlkdebugIOType', 'prefix': 'BLKDEBUG_IO_TYPE',
+  'data': [ 'read', 'write', 'write-zeroes', 'discard', 'flush' ] }
+
 ##
 # @BlkdebugInjectErrorOptions:
 #
@@ -XXX,XX +XXX,XX @@
 # @state:       the state identifier blkdebug needs to be in to
 #               actually trigger the event; defaults to "any"
 #
+# @iotype:      the type of I/O operations on which this error should
+#               be injected; defaults to "all read, write,
+#               write-zeroes, discard, and flush operations"
+#               (since: 4.1)
+#
 # @errno:       error identifier (errno) to be returned; defaults to
 #               EIO
 #
@@ -XXX,XX +XXX,XX @@
 { 'struct': 'BlkdebugInjectErrorOptions',
   'data': { 'event': 'BlkdebugEvent',
             '*state': 'int',
+            '*iotype': 'BlkdebugIOType',
             '*errno': 'int',
             '*sector': 'int',
             '*once': 'bool',
diff --git a/block/blkdebug.c b/block/blkdebug.c
index XXXXXXX..XXXXXXX 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -XXX,XX +XXX,XX @@ typedef struct BlkdebugRule {
     int state;
     union {
         struct {
+            uint64_t iotype_mask;
             int error;
             int immediately;
             int once;
@@ -XXX,XX +XXX,XX @@ typedef struct BlkdebugRule {
     QSIMPLEQ_ENTRY(BlkdebugRule) active_next;
 } BlkdebugRule;
 
+QEMU_BUILD_BUG_MSG(BLKDEBUG_IO_TYPE__MAX > 64,
+                   "BlkdebugIOType mask does not fit into an uint64_t");
+
 static QemuOptsList inject_error_opts = {
     .name = "inject-error",
     .head = QTAILQ_HEAD_INITIALIZER(inject_error_opts.head),
@@ -XXX,XX +XXX,XX @@ static QemuOptsList inject_error_opts = {
             .name = "state",
             .type = QEMU_OPT_NUMBER,
         },
+        {
+            .name = "iotype",
+            .type = QEMU_OPT_STRING,
+        },
         {
             .name = "errno",
             .type = QEMU_OPT_NUMBER,
@@ -XXX,XX +XXX,XX @@ static int add_rule(void *opaque, QemuOpts *opts, Error **errp)
     int event;
     struct BlkdebugRule *rule;
     int64_t sector;
+    BlkdebugIOType iotype;
+    Error *local_error = NULL;
 
     /* Find the right event for the rule */
     event_name = qemu_opt_get(opts, "event");
@@ -XXX,XX +XXX,XX @@ static int add_rule(void *opaque, QemuOpts *opts, Error **errp)
         sector = qemu_opt_get_number(opts, "sector", -1);
         rule->options.inject.offset =
             sector == -1 ? -1 : sector * BDRV_SECTOR_SIZE;
+
+        iotype = qapi_enum_parse(&BlkdebugIOType_lookup,
+                                 qemu_opt_get(opts, "iotype"),
+                                 BLKDEBUG_IO_TYPE__MAX, &local_error);
+        if (local_error) {
+            error_propagate(errp, local_error);
+            return -1;
+        }
+        if (iotype != BLKDEBUG_IO_TYPE__MAX) {
+            rule->options.inject.iotype_mask = (1ull << iotype);
+        } else {
+            /* Apply the default */
+            rule->options.inject.iotype_mask =
+                (1ull << BLKDEBUG_IO_TYPE_READ)
+                | (1ull << BLKDEBUG_IO_TYPE_WRITE)
+                | (1ull << BLKDEBUG_IO_TYPE_WRITE_ZEROES)
+                | (1ull << BLKDEBUG_IO_TYPE_DISCARD)
+                | (1ull << BLKDEBUG_IO_TYPE_FLUSH);
+        }
+
         break;
 
     case ACTION_SET_STATE:
@@ -XXX,XX +XXX,XX @@ out:
     return ret;
 }
 
-static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes)
+static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+                      BlkdebugIOType iotype)
 {
     BDRVBlkdebugState *s = bs->opaque;
     BlkdebugRule *rule = NULL;
@@ -XXX,XX +XXX,XX @@ static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes)
     QSIMPLEQ_FOREACH(rule, &s->active_rules, active_next) {
         uint64_t inject_offset = rule->options.inject.offset;
 
-        if (inject_offset == -1 ||
-            (bytes && inject_offset >= offset &&
-             inject_offset < offset + bytes))
+        if ((inject_offset == -1 ||
+             (bytes && inject_offset >= offset &&
+              inject_offset < offset + bytes)) &&
+            (rule->options.inject.iotype_mask & (1ull << iotype)))
         {
             break;
         }
@@ -XXX,XX +XXX,XX @@ blkdebug_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
         assert(bytes <= bs->bl.max_transfer);
     }
 
-    err = rule_check(bs, offset, bytes);
+    err = rule_check(bs, offset, bytes, BLKDEBUG_IO_TYPE_READ);
     if (err) {
         return err;
     }
@@ -XXX,XX +XXX,XX @@ blkdebug_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
         assert(bytes <= bs->bl.max_transfer);
     }
 
-    err = rule_check(bs, offset, bytes);
+    err = rule_check(bs, offset, bytes, BLKDEBUG_IO_TYPE_WRITE);
     if (err) {
         return err;
     }
@@ -XXX,XX +XXX,XX @@ blkdebug_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
 
 static int blkdebug_co_flush(BlockDriverState *bs)
 {
-    int err = rule_check(bs, 0, 0);
+    int err = rule_check(bs, 0, 0, BLKDEBUG_IO_TYPE_FLUSH);
 
     if (err) {
         return err;
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn blkdebug_co_pwrite_zeroes(BlockDriverState *bs,
         assert(bytes <= bs->bl.max_pwrite_zeroes);
     }
 
-    err = rule_check(bs, offset, bytes);
+    err = rule_check(bs, offset, bytes, BLKDEBUG_IO_TYPE_WRITE_ZEROES);
     if (err) {
         return err;
     }
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn blkdebug_co_pdiscard(BlockDriverState *bs,
         assert(bytes <= bs->bl.max_pdiscard);
     }
 
-    err = rule_check(bs, offset, bytes);
+    err = rule_check(bs, offset, bytes, BLKDEBUG_IO_TYPE_DISCARD);
     if (err) {
         return err;
     }
-- 
2.21.0

Together with @iotypes and @sector, this can be used to trap e.g. the
first read or write access to a certain sector without having to know
what happens internally in the block layer, i.e. which "real" events
happen right before such an access.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190507203508.18026-5-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qapi/block-core.json | 4 +++-
 block/blkdebug.c     | 2 ++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index XXXXXXX..XXXXXXX 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -XXX,XX +XXX,XX @@
 #
 # @cluster_alloc_space: an allocation of file space for a cluster (since 4.1)
 #
+# @none: triggers once at creation of the blkdebug node (since 4.1)
+#
 # Since: 2.9
 ##
 { 'enum': 'BlkdebugEvent', 'prefix': 'BLKDBG',
@@ -XXX,XX +XXX,XX @@
             'pwritev_rmw_tail', 'pwritev_rmw_after_tail', 'pwritev',
             'pwritev_zero', 'pwritev_done', 'empty_image_prepare',
             'l1_shrink_write_table', 'l1_shrink_free_l2_clusters',
-            'cor_write', 'cluster_alloc_space'] }
+            'cor_write', 'cluster_alloc_space', 'none'] }
 
 ##
 # @BlkdebugIOType:
diff --git a/block/blkdebug.c b/block/blkdebug.c
index XXXXXXX..XXXXXXX 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -XXX,XX +XXX,XX @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
         goto out;
     }
 
+    bdrv_debug_event(bs, BLKDBG_NONE);
+
     ret = 0;
 out:
     if (ret < 0) {
-- 
2.21.0

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190507203508.18026-6-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qapi/block-core.json | 5 ++++-
 block/blkdebug.c     | 8 ++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index XXXXXXX..XXXXXXX 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -XXX,XX +XXX,XX @@
 #
 # @flush: .bdrv_co_flush_to_disk()
 #
+# @block-status: .bdrv_co_block_status()
+#
 # Since: 4.1
 ##
 { 'enum': 'BlkdebugIOType', 'prefix': 'BLKDEBUG_IO_TYPE',
-  'data': [ 'read', 'write', 'write-zeroes', 'discard', 'flush' ] }
+  'data': [ 'read', 'write', 'write-zeroes', 'discard', 'flush',
+            'block-status' ] }
 
 ##
 # @BlkdebugInjectErrorOptions:
diff --git a/block/blkdebug.c b/block/blkdebug.c
index XXXXXXX..XXXXXXX 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn blkdebug_co_block_status(BlockDriverState *bs,
                                                  int64_t *map,
                                                  BlockDriverState **file)
 {
+    int err;
+
     assert(QEMU_IS_ALIGNED(offset | bytes, bs->bl.request_alignment));
+
+    err = rule_check(bs, offset, bytes, BLKDEBUG_IO_TYPE_BLOCK_STATUS);
+    if (err) {
+        return err;
+    }
+
     return bdrv_co_block_status_from_file(bs, want_zero, offset, bytes,
                                           pnum, map, file);
 }
-- 
2.21.0

This test converts a simple image to another, but blkdebug injects
block_status and read faults at some offsets.  The resulting image
should be the same as the input image, except that sectors that could
not be read have to be 0.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20190507203508.18026-7-mreitz@redhat.com
Tested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
[mreitz: Dropped superfluous printf from _filter_offsets, as suggested
         by Vladimir; disable test for VDI and IMGOPTSSYNTAX]
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/251     | 170 +++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/251.out |  43 ++++++++++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 214 insertions(+)
 create mode 100755 tests/qemu-iotests/251
 create mode 100644 tests/qemu-iotests/251.out

diff --git a/tests/qemu-iotests/251 b/tests/qemu-iotests/251
new file mode 100755
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qemu-iotests/251
@@ -XXX,XX +XXX,XX @@
+#!/usr/bin/env bash
+#
+# Test qemu-img convert --salvage
+#
+# Copyright (C) 2019 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=mreitz@redhat.com
+
+seq=$(basename $0)
+echo "QA output created by $seq"
+
+status=1	# failure is the default!
+
+_cleanup()
+{
+    _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.qemu
+
+_supported_fmt generic
+_supported_proto file
+_supported_os Linux
+
+if [ "$IMGOPTSSYNTAX" = "true" ]; then
+    # We use json:{} filenames here, so we cannot work with additional options.
+    _unsupported_fmt $IMGFMT
+else
+    # With VDI, the output is ordered differently.  Just disable it.
+    _unsupported_fmt vdi
+fi
+
+
+TEST_IMG="$TEST_IMG.orig" _make_test_img 64M
+
+$QEMU_IO -c 'write -P 42 0 64M' "$TEST_IMG.orig" | _filter_qemu_io
+
+
+sector_size=512
+
+# Offsets on which to fail block-status.  Keep in ascending order so
+# the indexing done by _filter_offsets will appear in ascending order
+# in the output as well.
+status_fail_offsets="$((16 * 1024 * 1024 + 8192))
+                     $((33 * 1024 * 1024 + 512))"
+
+# Offsets on which to fail reads.  Keep in ascending order for the
+# same reason.
+# The second element is shared with $status_fail_offsets on purpose.
+# Starting with the third element, we test what happens when a
+# continuous range of sectors is inaccessible.
+read_fail_offsets="$((32 * 1024 * 1024 - 65536))
+                   $((33 * 1024 * 1024 + 512))
+                   $(seq $((34 * 1024 * 1024)) $sector_size \
+                         $((34 * 1024 * 1024 + 4096 - $sector_size)))"
+
+
+# blkdebug must be above the format layer so it can intercept all
+# block-status events
+source_img="json:{'driver': 'blkdebug',
+                  'image': {
+                      'driver': '$IMGFMT',
+                      'file': {
+                          'driver': 'file',
+                          'filename': '$TEST_IMG.orig'
+                      }
+                  },
+                  'inject-error': ["
+
+for ofs in $status_fail_offsets
+do
+    source_img+="{ 'event': 'none',
+                   'iotype': 'block-status',
+                   'errno': 5,
+                   'sector': $((ofs / sector_size)) },"
+done
+
+for ofs in $read_fail_offsets
+do
+    source_img+="{ 'event': 'none',
+                   'iotype': 'read',
+                   'errno': 5,
+                   'sector': $((ofs / sector_size)) },"
+done
+
+# Remove the trailing comma and terminate @inject-error and json:{}
+source_img="${source_img%,} ] }"
+
+
+echo
+
+
+_filter_offsets() {
+    filters=
+
+    index=0
+    for ofs in $1
+    do
+        filters+=" -e s/$ofs/status_fail_offset_$index/"
+        index=$((index + 1))
+    done
+
+    index=0
+    for ofs in $2
+    do
+        filters+=" -e s/$ofs/read_fail_offset_$index/"
+        index=$((index + 1))
+    done
+
+    sed $filters
+}
+
+# While determining the number of allocated sectors in the input
+# image, we should see one block status warning per element of
+# $status_fail_offsets.
+#
+# Then, the image is read.  Since the block status is queried in
+# basically the same way, the same warnings as in the previous step
+# should reappear.  Interleaved with those we should see a read
+# warning per element of $read_fail_offsets.
+# Note that $read_fail_offsets and $status_fail_offsets share an
+# element (read_fail_offset_1 == status_fail_offset_1), so
+# "status_fail_offset_1" in the output is the same as
+# "read_fail_offset_1".
+$QEMU_IMG convert --salvage "$source_img" "$TEST_IMG" 2>&1 \
+    | _filter_offsets "$status_fail_offsets" "$read_fail_offsets"
+
+echo
+
+# The offsets where the block status could not be determined should
+# have been treated as containing data and thus should be correct in
+# the output image.
+# The offsets where reading failed altogether should be 0.  Make them
+# 0 in the input image, too, so we can compare both images.
+for ofs in $read_fail_offsets
+do
+    $QEMU_IO -c "write -z $ofs $sector_size" "$TEST_IMG.orig" \
+        | _filter_qemu_io \
+        | _filter_offsets '' "$read_fail_offsets"
+done
+
+echo
+
+# These should be equal now.
+$QEMU_IMG compare "$TEST_IMG.orig" "$TEST_IMG"
+
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/251.out b/tests/qemu-iotests/251.out
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qemu-iotests/251.out
@@ -XXX,XX +XXX,XX @@
+QA output created by 251
+Formatting 'TEST_DIR/t.IMGFMT.orig', fmt=IMGFMT size=67108864
+wrote 67108864/67108864 bytes at offset 0
+64 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+qemu-img: warning: error while reading block status at offset status_fail_offset_0: Input/output error
+qemu-img: warning: error while reading block status at offset status_fail_offset_1: Input/output error
+qemu-img: warning: error while reading block status at offset status_fail_offset_0: Input/output error
+qemu-img: warning: error while reading offset read_fail_offset_0: Input/output error
+qemu-img: warning: error while reading block status at offset status_fail_offset_1: Input/output error
+qemu-img: warning: error while reading offset status_fail_offset_1: Input/output error
+qemu-img: warning: error while reading offset read_fail_offset_2: Input/output error
+qemu-img: warning: error while reading offset read_fail_offset_3: Input/output error
+qemu-img: warning: error while reading offset read_fail_offset_4: Input/output error
+qemu-img: warning: error while reading offset read_fail_offset_5: Input/output error
+qemu-img: warning: error while reading offset read_fail_offset_6: Input/output error
+qemu-img: warning: error while reading offset read_fail_offset_7: Input/output error
+qemu-img: warning: error while reading offset read_fail_offset_8: Input/output error
+qemu-img: warning: error while reading offset read_fail_offset_9: Input/output error
+
+wrote 512/512 bytes at offset read_fail_offset_0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset read_fail_offset_1
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset read_fail_offset_2
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset read_fail_offset_3
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset read_fail_offset_4
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset read_fail_offset_5
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset read_fail_offset_6
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset read_fail_offset_7
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset read_fail_offset_8
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset read_fail_offset_9
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+Images are identical.
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -XXX,XX +XXX,XX @@
 248 rw quick
 249 rw auto quick
 250 rw auto quick
+251 rw auto quick
 252 rw auto backing quick
 253 rw auto quick
 254 rw auto backing quick
-- 
2.21.0

We do not support this combination (yet), so this should yield an error
message.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Tested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190507203508.18026-8-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/082     | 1 +
 tests/qemu-iotests/082.out | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/tests/qemu-iotests/082 b/tests/qemu-iotests/082
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/082
+++ b/tests/qemu-iotests/082
@@ -XXX,XX +XXX,XX @@ echo === convert: -C and other options ===
 run_qemu_img convert -C -S 4k -O $IMGFMT "$TEST_IMG" "$TEST_IMG".target
 run_qemu_img convert -C -S 8k -O $IMGFMT "$TEST_IMG" "$TEST_IMG".target
 run_qemu_img convert -C -c -O $IMGFMT "$TEST_IMG" "$TEST_IMG".target
+run_qemu_img convert -C --salvage -O $IMGFMT "$TEST_IMG" "$TEST_IMG".target
 
 echo
 echo === amend: Options specified more than once ===
diff --git a/tests/qemu-iotests/082.out b/tests/qemu-iotests/082.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/082.out
+++ b/tests/qemu-iotests/082.out
@@ -XXX,XX +XXX,XX @@ qemu-img: Cannot enable copy offloading when -S is used
 Testing: convert -C -c -O qcow2 TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.target
 qemu-img: Cannot enable copy offloading when -c is used
 
+Testing: convert -C --salvage -O qcow2 TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.target
+qemu-img: Cannot use copy offloading in salvaging mode
+
 === amend: Options specified more than once ===
 
 Testing: amend -f foo -f qcow2 -o lazy_refcounts=on TEST_DIR/t.qcow2
-- 
2.21.0