Series comparison

-[PULL 00/17] Block patches
+[Qemu-devel] [PULL v2 00/23] Block patches
-The following changes since commit 418fa86dd465b4fd8394373cf83db8fa65d7611c:
+The following changes since commit eaefea537b476cb853e2edbdc68e969ec777e4bb:
-  Merge remote-tracking branch 'remotes/stsquad/tags/pull-testing-040220-1' into staging (2020-02-04 18:55:06 +0000)
+  Merge remote-tracking branch 'remotes/mjt/tags/trivial-patches-fetch' into staging (2017-12-18 14:17:42 +0000)
 are available in the Git repository at:
-  https://github.com/XanClic/qemu.git tags/pull-block-2020-02-06
+  git://github.com/stefanha/qemu.git tags/block-pull-request
-for you to fetch changes up to a541fcc27c98b96da187c7d4573f3270f3ddd283:
+for you to fetch changes up to 7a9dda0d7f9831c2432620dcfefdadbb7ae888dc:
-  iotests: add test for backup-top failure on permission activation (2020-02-06 13:47:45 +0100)
+  qemu-iotests: add 203 savevm with IOThreads test (2017-12-19 10:25:09 +0000)
 ----------------------------------------------------------------
-Block patches:
+Pull request
-- Drop BDRV_SECTOR_SIZE from qcow2
-- Allow Python iotests to be added to the auto group
+v2:
-  (and add some)
+ * Fixed incorrect virtio_blk_data_plane_create() local_err refactoring in
-- Fix for the backup job
+   "hw/block: Use errp directly rather than local_err" that broke virtio-blk
-- Fix memleak in bdrv_refresh_filename()
+   over virtio-mmio [Peter]
 - Use GStrings in two places for greater efficiency (than manually
   handling string allocation)
 ----------------------------------------------------------------
-Alberto Garcia (8):
-  qcow2: Assert that host cluster offsets fit in L2 table entries
-  block: Use a GString in bdrv_perm_names()
-  qcow2: Use a GString in report_unsupported_feature()
-  qcow2: Don't round the L1 table allocation up to the sector size
-  qcow2: Tighten cluster_offset alignment assertions
-  qcow2: Use bs->bl.request_alignment when updating an L1 entry
-  qcow2: Don't require aligned offsets in qcow2_co_copy_range_from()
-  qcow2: Use BDRV_SECTOR_SIZE instead of the hardcoded value
-John Snow (1):
+Mao Zhongyi (4):
-  iotests: remove 'linux' from default supported platforms
+  hw/block/nvme: Convert to realize
   hw/block: Fix the return type
   hw/block: Use errp directly rather than local_err
   dev-storage: Fix the unusual function name
-Pan Nengyuan (1):
+Mark Kanda (2):
-  block: fix memleaks in bdrv_refresh_filename
+  virtio-blk: make queue size configurable
   virtio-blk: reject configs with logical block size > physical block
     size
-Thomas Huth (5):
+Paolo Bonzini (1):
-  iotests: Test 041 only works on certain systems
+  block: avoid recursive AioContext acquire in bdrv_inactivate_all()
   iotests: Test 183 does not work on macOS and OpenBSD
   iotests: Check for the availability of the required devices in 267 and
   iotests: Skip Python-based tests if QEMU does not support virtio-blk
   iotests: Enable more tests in the 'auto' group to improve test
     coverage
-Vladimir Sementsov-Ogievskiy (2):
+Stefan Hajnoczi (16):
-  block/backup-top: fix failure path
+  coroutine: simplify co_aio_sleep_ns() prototype
-  iotests: add test for backup-top failure on permission activation
+  qdev: drop unused #include "sysemu/iothread.h"
   blockdev: hold AioContext for bdrv_unref() in
     external_snapshot_clean()
   block: don't keep AioContext acquired after
     external_snapshot_prepare()
   block: don't keep AioContext acquired after drive_backup_prepare()
   block: don't keep AioContext acquired after blockdev_backup_prepare()
   block: don't keep AioContext acquired after
     internal_snapshot_prepare()
   block: drop unused BlockDirtyBitmapState->aio_context field
   iothread: add iothread_by_id() API
   blockdev: add x-blockdev-set-iothread testing command
   qemu-iotests: add 202 external snapshots IOThread test
   docs: mark nested AioContext locking as a legacy API
   blockdev: add x-blockdev-set-iothread force boolean
   iotests: add VM.add_object()
   iothread: fix iothread_stop() race condition
   qemu-iotests: add 203 savevm with IOThreads test
- block.c                       | 12 +++--
+ docs/devel/multiple-iothreads.txt |   7 +-
- block/backup-top.c            | 21 ++++----
+ qapi/block-core.json              |  40 ++++++
- block/qcow2-cluster.c         | 44 +++++++++++------
+ hw/block/dataplane/virtio-blk.h   |   2 +-
- block/qcow2-refcount.c        |  2 +-
+ include/hw/block/block.h          |   4 +-
- block/qcow2-snapshot.c        |  3 +-
+ include/hw/virtio/virtio-blk.h    |   1 +
- block/qcow2.c                 | 46 ++++++++----------
+ include/qemu/coroutine.h          |   6 +-
- tests/qemu-iotests/041        |  3 +-
+ include/sysemu/iothread.h         |   4 +-
- tests/qemu-iotests/127        |  2 +
+ block.c                           |  14 ++-
- tests/qemu-iotests/183        |  1 +
+ block/null.c                      |   3 +-
- tests/qemu-iotests/267        |  2 +
+ block/sheepdog.c                  |   3 +-
- tests/qemu-iotests/283        | 92 +++++++++++++++++++++++++++++++++++
+ blockdev.c                        | 259 +++++++++++++++++++++++++++-----------
- tests/qemu-iotests/283.out    |  8 +++
+ hw/block/block.c                  |  15 ++-
- tests/qemu-iotests/check      | 12 ++++-
+ hw/block/dataplane/virtio-blk.c   |  12 +-
- tests/qemu-iotests/common.rc  | 14 ++++++
+ hw/block/fdc.c                    |  17 +--
- tests/qemu-iotests/group      | 15 +++---
+ hw/block/nvme.c                   |  23 ++--
- tests/qemu-iotests/iotests.py | 16 ++++--
+ hw/block/virtio-blk.c             |  30 +++--
-files changed, 220 insertions(+), 73 deletions(-)
+ hw/core/qdev-properties-system.c  |   1 -
- create mode 100644 tests/qemu-iotests/283
+ hw/ide/qdev.c                     |  12 +-
- create mode 100644 tests/qemu-iotests/283.out
+ hw/scsi/scsi-disk.c               |  13 +-
  hw/usb/dev-storage.c              |  29 ++---
  iothread.c                        |  27 +++-
  util/qemu-coroutine-sleep.c       |   4 +-
  tests/qemu-iotests/202            |  95 ++++++++++++++
  tests/qemu-iotests/202.out        |  11 ++
  tests/qemu-iotests/203            |  59 +++++++++
  tests/qemu-iotests/203.out        |   6 +
  tests/qemu-iotests/group          |   2 +
  tests/qemu-iotests/iotests.py     |   5 +
 files changed, 531 insertions(+), 173 deletions(-)
  create mode 100755 tests/qemu-iotests/202
  create mode 100644 tests/qemu-iotests/202.out
  create mode 100755 tests/qemu-iotests/203
  create mode 100644 tests/qemu-iotests/203.out
 --
-.24.1
+.14.3

-New patch
+[Qemu-devel] [PULL v2 01/23] coroutine: simplify co_aio_sleep_ns() prototype
+The AioContext pointer argument to co_aio_sleep_ns() is only used for
+the sleep timer.  It does not affect where the caller coroutine is
+resumed.
+Due to changes to coroutine and AIO APIs it is now possible to drop the
+AioContext pointer argument.  This is safe to do since no caller has
+specific requirements for which AioContext the timer must run in.
+This patch drops the AioContext pointer argument and renames the
+function to simplify the API.
+Reported-by: Paolo Bonzini <pbonzini@redhat.com>
+Reported-by: Eric Blake <eblake@redhat.com>
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+Reviewed-by: Eric Blake <eblake@redhat.com>
+Message-id: 20171109102652.6360-1-stefanha@redhat.com
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+---
+ include/qemu/coroutine.h    | 6 +-----
+ block/null.c                | 3 +--
+ block/sheepdog.c            | 3 +--
+ util/qemu-coroutine-sleep.c | 4 ++--
+files changed, 5 insertions(+), 11 deletions(-)
+diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
+index XXXXXXX..XXXXXXX 100644
+--- a/include/qemu/coroutine.h
++++ b/include/qemu/coroutine.h
+@@ -XXX,XX +XXX,XX @@ void qemu_co_rwlock_unlock(CoRwlock *lock);
+ /**
+  * Yield the coroutine for a given duration
+- *
+- * Behaves similarly to co_sleep_ns(), but the sleeping coroutine will be
+- * resumed when using aio_poll().
+  */
+-void coroutine_fn co_aio_sleep_ns(AioContext *ctx, QEMUClockType type,
+-                                  int64_t ns);
++void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns);
+ /**
+  * Yield until a file descriptor becomes readable
+diff --git a/block/null.c b/block/null.c
+index XXXXXXX..XXXXXXX 100644
+--- a/block/null.c
++++ b/block/null.c
+@@ -XXX,XX +XXX,XX @@ static coroutine_fn int null_co_common(BlockDriverState *bs)
+     BDRVNullState *s = bs->opaque;
+     if (s->latency_ns) {
+-        co_aio_sleep_ns(bdrv_get_aio_context(bs), QEMU_CLOCK_REALTIME,
+-                        s->latency_ns);
++        qemu_co_sleep_ns(QEMU_CLOCK_REALTIME, s->latency_ns);
+     }
+     return 0;
+ }
+diff --git a/block/sheepdog.c b/block/sheepdog.c
+index XXXXXXX..XXXXXXX 100644
+--- a/block/sheepdog.c
++++ b/block/sheepdog.c
+@@ -XXX,XX +XXX,XX @@ static coroutine_fn void reconnect_to_sdog(void *opaque)
+         if (s->fd < 0) {
+             DPRINTF("Wait for connection to be established\n");
+             error_report_err(local_err);
+-            co_aio_sleep_ns(bdrv_get_aio_context(s->bs), QEMU_CLOCK_REALTIME,
+-                            1000000000ULL);
++            qemu_co_sleep_ns(QEMU_CLOCK_REALTIME, 1000000000ULL);
+         }
+     };
+diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
+index XXXXXXX..XXXXXXX 100644
+--- a/util/qemu-coroutine-sleep.c
++++ b/util/qemu-coroutine-sleep.c
+@@ -XXX,XX +XXX,XX @@ static void co_sleep_cb(void *opaque)
+     aio_co_wake(sleep_cb->co);
+ }
+-void coroutine_fn co_aio_sleep_ns(AioContext *ctx, QEMUClockType type,
+-                                  int64_t ns)
++void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
+ {
++    AioContext *ctx = qemu_get_current_aio_context();
+     CoSleepCB sleep_cb = {
+         .co = qemu_coroutine_self(),
+     };
+--
+.14.3

-[PULL 04/17] qcow2: Use a GString in report_unsupported_feature()
+[Qemu-devel] [PULL v2 02/23] hw/block/nvme: Convert to realize
-From: Alberto Garcia <berto@igalia.com>
+From: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
-This is a bit more efficient than having to allocate and free memory
+Convert nvme_init() to realize and rename it to nvme_realize().
 for each item.
-The default size (60) is enough for all the existing incompatible
+Cc: John Snow <jsnow@redhat.com>
-features or the "Unknown incompatible feature" message.
+Cc: Keith Busch <keith.busch@intel.com>
 Cc: Kevin Wolf <kwolf@redhat.com>
 Cc: Max Reitz <mreitz@redhat.com>
 Cc: Markus Armbruster <armbru@redhat.com>
-Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
+Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
-Signed-off-by: Alberto Garcia <berto@igalia.com>
+Message-id: 2882e72d795e04cbe2120f569d551aef2467ac60.1511317952.git.maozy.fnst@cn.fujitsu.com
-Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 Message-id: 20200115135626.19442-1-berto@igalia.com
 Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
 Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
- block/qcow2.c | 23 +++++++++++------------
+ hw/block/nvme.c | 18 ++++++++++--------
-file changed, 11 insertions(+), 12 deletions(-)
+file changed, 10 insertions(+), 8 deletions(-)
-diff --git a/block/qcow2.c b/block/qcow2.c
+diff --git a/hw/block/nvme.c b/hw/block/nvme.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2.c
+--- a/hw/block/nvme.c
-+++ b/block/qcow2.c
++++ b/hw/block/nvme.c
-@@ -XXX,XX +XXX,XX @@ static void cleanup_unknown_header_ext(BlockDriverState *bs)
+@@ -XXX,XX +XXX,XX @@ static const MemoryRegionOps nvme_cmb_ops = {
- static void report_unsupported_feature(Error **errp, Qcow2Feature *table,
+     },
-                                        uint64_t mask)
+ };
 -static int nvme_init(PCIDevice *pci_dev)
 +static void nvme_realize(PCIDevice *pci_dev, Error **errp)
  {
--    char *features = g_strdup("");
+     NvmeCtrl *n = NVME(pci_dev);
--    char *old;
+     NvmeIdCtrl *id = &n->id_ctrl;
-+    g_autoptr(GString) features = g_string_sized_new(60);
+@@ -XXX,XX +XXX,XX @@ static int nvme_init(PCIDevice *pci_dev)
+     Error *local_err = NULL;
-     while (table && table->name[0] != '\0') {
-         if (table->type == QCOW2_FEAT_TYPE_INCOMPATIBLE) {
+     if (!n->conf.blk) {
-             if (mask & (1ULL << table->bit)) {
+-        return -1;
--                old = features;
++        error_setg(errp, "drive property not set");
--                features = g_strdup_printf("%s%s%.46s", old, *old ? ", " : "",
++        return;
 -                                           table->name);
 -                g_free(old);
 +                if (features->len > 0) {
 +                    g_string_append(features, ", ");
 +                }
 +                g_string_append_printf(features, "%.46s", table->name);
                  mask &= ~(1ULL << table->bit);
              }
          }
@@ -XXX,XX +XXX,XX @@ static void report_unsupported_feature(Error **errp, Qcow2Feature *table,
      }
-     if (mask) {
+     bs_size = blk_getlength(n->conf.blk);
--        old = features;
+     if (bs_size < 0) {
--        features = g_strdup_printf("%s%sUnknown incompatible feature: %" PRIx64,
+-        return -1;
--                                   old, *old ? ", " : "", mask);
++        error_setg(errp, "could not get backing file size");
--        g_free(old);
++        return;
 +        if (features->len > 0) {
 +            g_string_append(features, ", ");
 +        }
 +        g_string_append_printf(features,
 +                               "Unknown incompatible feature: %" PRIx64, mask);
      }
--    error_setg(errp, "Unsupported qcow2 feature(s): %s", features);
+     blkconf_serial(&n->conf, &n->serial);
--    g_free(features);
+     if (!n->serial) {
-+    error_setg(errp, "Unsupported qcow2 feature(s): %s", features->str);
+-        return -1;
 +        error_setg(errp, "serial property not set");
 +        return;
      }
      blkconf_blocksizes(&n->conf);
      blkconf_apply_backend_options(&n->conf, blk_is_read_only(n->conf.blk),
                                    false, &local_err);
      if (local_err) {
 -        error_report_err(local_err);
 -        return -1;
 +        error_propagate(errp, local_err);
 +        return;
      }
      pci_conf = pci_dev->config;
@@ -XXX,XX +XXX,XX @@ static int nvme_init(PCIDevice *pci_dev)
              cpu_to_le64(n->ns_size >>
                  id_ns->lbaf[NVME_ID_NS_FLBAS_INDEX(ns->id_ns.flbas)].ds);
      }
 -    return 0;
  }
- /*
+ static void nvme_exit(PCIDevice *pci_dev)
@@ -XXX,XX +XXX,XX @@ static void nvme_class_init(ObjectClass *oc, void *data)
      DeviceClass *dc = DEVICE_CLASS(oc);
      PCIDeviceClass *pc = PCI_DEVICE_CLASS(oc);
 -    pc->init = nvme_init;
 +    pc->realize = nvme_realize;
      pc->exit = nvme_exit;
      pc->class_id = PCI_CLASS_STORAGE_EXPRESS;
      pc->vendor_id = PCI_VENDOR_ID_INTEL;
 --
-.24.1
+.14.3

-New patch
+[Qemu-devel] [PULL v2 03/23] hw/block: Fix the return type
+From: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
+When the function no success value to transmit, it usually make the
+function return void. It has turned out not to be a success, because
+it means that the extra local_err variable and error_propagate() will
+be needed. It leads to cumbersome code, therefore, transmit success/
+failure in the return value is worth.
+So fix the return type of blkconf_apply_backend_options(),
+blkconf_geometry() and virtio_blk_data_plane_create() to avoid it.
+Cc: John Snow <jsnow@redhat.com>
+Cc: Kevin Wolf <kwolf@redhat.com>
+Cc: Max Reitz <mreitz@redhat.com>
+Cc: Stefan Hajnoczi <stefanha@redhat.com>
+Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
+Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
+Message-id: ac0edc1fc70c4457e5cec94405eb7d1f89f9c2c1.1511317952.git.maozy.fnst@cn.fujitsu.com
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+---
+ hw/block/dataplane/virtio-blk.h |  2 +-
+ include/hw/block/block.h        |  4 ++--
+ hw/block/block.c                | 15 +++++++++------
+ hw/block/dataplane/virtio-blk.c | 12 +++++++-----
+files changed, 19 insertions(+), 14 deletions(-)
+diff --git a/hw/block/dataplane/virtio-blk.h b/hw/block/dataplane/virtio-blk.h
+index XXXXXXX..XXXXXXX 100644
+--- a/hw/block/dataplane/virtio-blk.h
++++ b/hw/block/dataplane/virtio-blk.h
+@@ -XXX,XX +XXX,XX @@
+ typedef struct VirtIOBlockDataPlane VirtIOBlockDataPlane;
+-void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
++bool virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
+                                   VirtIOBlockDataPlane **dataplane,
+                                   Error **errp);
+ void virtio_blk_data_plane_destroy(VirtIOBlockDataPlane *s);
+diff --git a/include/hw/block/block.h b/include/hw/block/block.h
+index XXXXXXX..XXXXXXX 100644
+--- a/include/hw/block/block.h
++++ b/include/hw/block/block.h
+@@ -XXX,XX +XXX,XX @@ static inline unsigned int get_physical_block_exp(BlockConf *conf)
+ /* Configuration helpers */
+ void blkconf_serial(BlockConf *conf, char **serial);
+-void blkconf_geometry(BlockConf *conf, int *trans,
++bool blkconf_geometry(BlockConf *conf, int *trans,
+                       unsigned cyls_max, unsigned heads_max, unsigned secs_max,
+                       Error **errp);
+ void blkconf_blocksizes(BlockConf *conf);
+-void blkconf_apply_backend_options(BlockConf *conf, bool readonly,
++bool blkconf_apply_backend_options(BlockConf *conf, bool readonly,
+                                    bool resizable, Error **errp);
+ /* Hard disk geometry */
+diff --git a/hw/block/block.c b/hw/block/block.c
+index XXXXXXX..XXXXXXX 100644
+--- a/hw/block/block.c
++++ b/hw/block/block.c
+@@ -XXX,XX +XXX,XX @@ void blkconf_blocksizes(BlockConf *conf)
+     }
+ }
+-void blkconf_apply_backend_options(BlockConf *conf, bool readonly,
++bool blkconf_apply_backend_options(BlockConf *conf, bool readonly,
+                                    bool resizable, Error **errp)
+ {
+     BlockBackend *blk = conf->blk;
+@@ -XXX,XX +XXX,XX @@ void blkconf_apply_backend_options(BlockConf *conf, bool readonly,
+     ret = blk_set_perm(blk, perm, shared_perm, errp);
+     if (ret < 0) {
+-        return;
++        return false;
+     }
+     switch (conf->wce) {
+@@ -XXX,XX +XXX,XX @@ void blkconf_apply_backend_options(BlockConf *conf, bool readonly,
+     blk_set_enable_write_cache(blk, wce);
+     blk_set_on_error(blk, rerror, werror);
++
++    return true;
+ }
+-void blkconf_geometry(BlockConf *conf, int *ptrans,
++bool blkconf_geometry(BlockConf *conf, int *ptrans,
+                       unsigned cyls_max, unsigned heads_max, unsigned secs_max,
+                       Error **errp)
+ {
+@@ -XXX,XX +XXX,XX @@ void blkconf_geometry(BlockConf *conf, int *ptrans,
+     if (conf->cyls || conf->heads || conf->secs) {
+         if (conf->cyls < 1 || conf->cyls > cyls_max) {
+             error_setg(errp, "cyls must be between 1 and %u", cyls_max);
+-            return;
++            return false;
+         }
+         if (conf->heads < 1 || conf->heads > heads_max) {
+             error_setg(errp, "heads must be between 1 and %u", heads_max);
+-            return;
++            return false;
+         }
+         if (conf->secs < 1 || conf->secs > secs_max) {
+             error_setg(errp, "secs must be between 1 and %u", secs_max);
+-            return;
++            return false;
+         }
+     }
++    return true;
+ }
+diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
+index XXXXXXX..XXXXXXX 100644
+--- a/hw/block/dataplane/virtio-blk.c
++++ b/hw/block/dataplane/virtio-blk.c
+@@ -XXX,XX +XXX,XX @@ static void notify_guest_bh(void *opaque)
+ }
+ /* Context: QEMU global mutex held */
+-void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
++bool virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
+                                   VirtIOBlockDataPlane **dataplane,
+                                   Error **errp)
+ {
+@@ -XXX,XX +XXX,XX @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
+             error_setg(errp,
+                        "device is incompatible with iothread "
+                        "(transport does not support notifiers)");
+-            return;
++            return false;
+         }
+         if (!virtio_device_ioeventfd_enabled(vdev)) {
+             error_setg(errp, "ioeventfd is required for iothread");
+-            return;
++            return false;
+         }
+         /* If dataplane is (re-)enabled while the guest is running there could
+@@ -XXX,XX +XXX,XX @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
+          */
+         if (blk_op_is_blocked(conf->conf.blk, BLOCK_OP_TYPE_DATAPLANE, errp)) {
+             error_prepend(errp, "cannot start virtio-blk dataplane: ");
+-            return;
++            return false;
+         }
+     }
+     /* Don't try if transport does not support notifiers. */
+     if (!virtio_device_ioeventfd_enabled(vdev)) {
+-        return;
++        return false;
+     }
+     s = g_new0(VirtIOBlockDataPlane, 1);
+@@ -XXX,XX +XXX,XX @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
+     s->batch_notify_vqs = bitmap_new(conf->num_queues);
+     *dataplane = s;
++
++    return true;
+ }
+ /* Context: QEMU global mutex held */
+--
+.14.3

-New patch
+[Qemu-devel] [PULL v2 04/23] hw/block: Use errp directly rather than local_err
+From: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
 [Drop virtio_blk_data_plane_create() change that misinterprets return
 value when the virtio transport does not support dataplane.
 --Stefan]
 Cc: John Snow <jsnow@redhat.com>
 Cc: Kevin Wolf <kwolf@redhat.com>
 Cc: Max Reitz <mreitz@redhat.com>
 Cc: Keith Busch <keith.busch@intel.com>
 Cc: Stefan Hajnoczi <stefanha@redhat.com>
 Cc: "Michael S. Tsirkin" <mst@redhat.com>
 Cc: Paolo Bonzini <pbonzini@redhat.com>
 Cc: Gerd Hoffmann <kraxel@redhat.com>
 Cc: Markus Armbruster <armbru@redhat.com>
 Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
 Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
 Message-id: e77848d3735ba590f23ffbf8094379c646c33d79.1511317952.git.maozy.fnst@cn.fujitsu.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
  hw/block/fdc.c        | 17 ++++++-----------
  hw/block/nvme.c       |  7 ++-----
  hw/block/virtio-blk.c | 13 +++++--------
  hw/ide/qdev.c         | 12 ++++--------
  hw/scsi/scsi-disk.c   | 13 ++++---------
  hw/usb/dev-storage.c  |  9 +++------
 files changed, 24 insertions(+), 47 deletions(-)
 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 fd_revalidate(FDrive *drv)
  static void fd_change_cb(void *opaque, bool load, Error **errp)
  {
      FDrive *drive = opaque;
 -    Error *local_err = NULL;
      if (!load) {
          blk_set_perm(drive->blk, 0, BLK_PERM_ALL, &error_abort);
      } else {
 -        blkconf_apply_backend_options(drive->conf,
 -                                      blk_is_read_only(drive->blk), false,
 -                                      &local_err);
 -        if (local_err) {
 -            error_propagate(errp, local_err);
 +        if (!blkconf_apply_backend_options(drive->conf,
 +                                           blk_is_read_only(drive->blk), false,
 +                                           errp)) {
              return;
          }
      }
@@ -XXX,XX +XXX,XX @@ static void floppy_drive_realize(DeviceState *qdev, Error **errp)
      FloppyDrive *dev = FLOPPY_DRIVE(qdev);
      FloppyBus *bus = FLOPPY_BUS(qdev->parent_bus);
      FDrive *drive;
 -    Error *local_err = NULL;
      int ret;
      if (dev->unit == -1) {
@@ -XXX,XX +XXX,XX @@ static void floppy_drive_realize(DeviceState *qdev, Error **errp)
      dev->conf.rerror = BLOCKDEV_ON_ERROR_AUTO;
      dev->conf.werror = BLOCKDEV_ON_ERROR_AUTO;
 -    blkconf_apply_backend_options(&dev->conf, blk_is_read_only(dev->conf.blk),
 -                                  false, &local_err);
 -    if (local_err) {
 -        error_propagate(errp, local_err);
 +    if (!blkconf_apply_backend_options(&dev->conf,
 +                                       blk_is_read_only(dev->conf.blk),
 +                                       false, errp)) {
          return;
      }
 diff --git a/hw/block/nvme.c b/hw/block/nvme.c
 index XXXXXXX..XXXXXXX 100644
 --- a/hw/block/nvme.c
 +++ b/hw/block/nvme.c
@@ -XXX,XX +XXX,XX @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
      int i;
      int64_t bs_size;
      uint8_t *pci_conf;
 -    Error *local_err = NULL;
      if (!n->conf.blk) {
          error_setg(errp, "drive property not set");
@@ -XXX,XX +XXX,XX @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
          return;
      }
      blkconf_blocksizes(&n->conf);
 -    blkconf_apply_backend_options(&n->conf, blk_is_read_only(n->conf.blk),
 -                                  false, &local_err);
 -    if (local_err) {
 -        error_propagate(errp, local_err);
 +    if (!blkconf_apply_backend_options(&n->conf, blk_is_read_only(n->conf.blk),
 +                                       false, errp)) {
          return;
      }
 diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
 index XXXXXXX..XXXXXXX 100644
 --- a/hw/block/virtio-blk.c
 +++ b/hw/block/virtio-blk.c
@@ -XXX,XX +XXX,XX @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
      }
      blkconf_serial(&conf->conf, &conf->serial);
 -    blkconf_apply_backend_options(&conf->conf,
 -                                  blk_is_read_only(conf->conf.blk), true,
 -                                  &err);
 -    if (err) {
 -        error_propagate(errp, err);
 +    if (!blkconf_apply_backend_options(&conf->conf,
 +                                       blk_is_read_only(conf->conf.blk), true,
 +                                       errp)) {
          return;
      }
      s->original_wce = blk_enable_write_cache(conf->conf.blk);
 -    blkconf_geometry(&conf->conf, NULL, 65535, 255, 255, &err);
 -    if (err) {
 -        error_propagate(errp, err);
 +    if (!blkconf_geometry(&conf->conf, NULL, 65535, 255, 255, errp)) {
          return;
      }
 +
      blkconf_blocksizes(&conf->conf);
      virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
 diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
 index XXXXXXX..XXXXXXX 100644
 --- a/hw/ide/qdev.c
 +++ b/hw/ide/qdev.c
@@ -XXX,XX +XXX,XX @@ static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp)
  {
      IDEBus *bus = DO_UPCAST(IDEBus, qbus, dev->qdev.parent_bus);
      IDEState *s = bus->ifs + dev->unit;
 -    Error *err = NULL;
      int ret;
      if (!dev->conf.blk) {
@@ -XXX,XX +XXX,XX @@ static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp)
      blkconf_serial(&dev->conf, &dev->serial);
      if (kind != IDE_CD) {
 -        blkconf_geometry(&dev->conf, &dev->chs_trans, 65535, 16, 255, &err);
 -        if (err) {
 -            error_propagate(errp, err);
 +        if (!blkconf_geometry(&dev->conf, &dev->chs_trans, 65535, 16, 255,
 +                              errp)) {
              return;
          }
      }
 -    blkconf_apply_backend_options(&dev->conf, kind == IDE_CD, kind != IDE_CD,
 -                                  &err);
 -    if (err) {
 -        error_propagate(errp, err);
 +    if (!blkconf_apply_backend_options(&dev->conf, kind == IDE_CD,
 +                                       kind != IDE_CD, errp)) {
          return;
      }
 diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
 index XXXXXXX..XXXXXXX 100644
 --- a/hw/scsi/scsi-disk.c
 +++ b/hw/scsi/scsi-disk.c
@@ -XXX,XX +XXX,XX @@ static void scsi_disk_unit_attention_reported(SCSIDevice *dev)
  static void scsi_realize(SCSIDevice *dev, Error **errp)
  {
      SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
 -    Error *err = NULL;
      if (!s->qdev.conf.blk) {
          error_setg(errp, "drive property not set");
@@ -XXX,XX +XXX,XX @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
      }
      if (dev->type == TYPE_DISK) {
 -        blkconf_geometry(&dev->conf, NULL, 65535, 255, 255, &err);
 -        if (err) {
 -            error_propagate(errp, err);
 +        if (!blkconf_geometry(&dev->conf, NULL, 65535, 255, 255, errp)) {
              return;
          }
      }
 -    blkconf_apply_backend_options(&dev->conf,
 -                                  blk_is_read_only(s->qdev.conf.blk),
 -                                  dev->type == TYPE_DISK, &err);
 -    if (err) {
 -        error_propagate(errp, err);
 +    if (!blkconf_apply_backend_options(&dev->conf,
 +                                       blk_is_read_only(s->qdev.conf.blk),
 +                                       dev->type == TYPE_DISK, errp)) {
          return;
      }
 diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
 index XXXXXXX..XXXXXXX 100644
 --- a/hw/usb/dev-storage.c
 +++ b/hw/usb/dev-storage.c
@@ -XXX,XX +XXX,XX @@ static void usb_msd_realize_storage(USBDevice *dev, Error **errp)
      MSDState *s = USB_STORAGE_DEV(dev);
      BlockBackend *blk = s->conf.blk;
      SCSIDevice *scsi_dev;
 -    Error *err = NULL;
      if (!blk) {
          error_setg(errp, "drive property not set");
@@ -XXX,XX +XXX,XX @@ static void usb_msd_realize_storage(USBDevice *dev, Error **errp)
      blkconf_serial(&s->conf, &dev->serial);
      blkconf_blocksizes(&s->conf);
 -    blkconf_apply_backend_options(&s->conf, blk_is_read_only(blk), true, &err);
 -    if (err) {
 -        error_propagate(errp, err);
 +    if (!blkconf_apply_backend_options(&s->conf, blk_is_read_only(blk), true,
 +                                       errp)) {
          return;
      }
@@ -XXX,XX +XXX,XX @@ static void usb_msd_realize_storage(USBDevice *dev, Error **errp)
                   &usb_msd_scsi_info_storage, NULL);
      scsi_dev = scsi_bus_legacy_add_drive(&s->bus, blk, 0, !!s->removable,
                                           s->conf.bootindex, dev->serial,
 -                                         &err);
 +                                         errp);
      blk_unref(blk);
      if (!scsi_dev) {
 -        error_propagate(errp, err);
          return;
      }
      usb_msd_handle_reset(dev);
 --
 .14.3

-New patch
+[Qemu-devel] [PULL v2 05/23] dev-storage: Fix the unusual function name
+From: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
+The function name of usb_msd_{realize,unrealize}_*,
+usb_msd_class_initfn_* are unusual. Rename it to
+usb_msd_*_{realize,unrealize}, usb_msd_class_*_initfn.
+Cc: Gerd Hoffmann <kraxel@redhat.com>
+Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
+Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
+Message-id: 11e6003433abce35f3f4970e1acc71ee92dbcf51.1511317952.git.maozy.fnst@cn.fujitsu.com
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+---
+ hw/usb/dev-storage.c | 20 ++++++++++----------
+file changed, 10 insertions(+), 10 deletions(-)
+diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
+index XXXXXXX..XXXXXXX 100644
+--- a/hw/usb/dev-storage.c
++++ b/hw/usb/dev-storage.c
+@@ -XXX,XX +XXX,XX @@ static void usb_msd_unrealize_storage(USBDevice *dev, Error **errp)
+     object_unref(OBJECT(&s->bus));
+ }
+-static void usb_msd_realize_storage(USBDevice *dev, Error **errp)
++static void usb_msd_storage_realize(USBDevice *dev, Error **errp)
+ {
+     MSDState *s = USB_STORAGE_DEV(dev);
+     BlockBackend *blk = s->conf.blk;
+@@ -XXX,XX +XXX,XX @@ static void usb_msd_realize_storage(USBDevice *dev, Error **errp)
+     s->scsi_dev = scsi_dev;
+ }
+-static void usb_msd_unrealize_bot(USBDevice *dev, Error **errp)
++static void usb_msd_bot_unrealize(USBDevice *dev, Error **errp)
+ {
+     MSDState *s = USB_STORAGE_DEV(dev);
+     object_unref(OBJECT(&s->bus));
+ }
+-static void usb_msd_realize_bot(USBDevice *dev, Error **errp)
++static void usb_msd_bot_realize(USBDevice *dev, Error **errp)
+ {
+     MSDState *s = USB_STORAGE_DEV(dev);
+     DeviceState *d = DEVICE(dev);
+@@ -XXX,XX +XXX,XX @@ static void usb_msd_class_initfn_common(ObjectClass *klass, void *data)
+     dc->vmsd = &vmstate_usb_msd;
+ }
+-static void usb_msd_class_initfn_storage(ObjectClass *klass, void *data)
++static void usb_msd_class_storage_initfn(ObjectClass *klass, void *data)
+ {
+     DeviceClass *dc = DEVICE_CLASS(klass);
+     USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
+-    uc->realize = usb_msd_realize_storage;
++    uc->realize = usb_msd_storage_realize;
+     uc->unrealize = usb_msd_unrealize_storage;
+     dc->props = msd_properties;
+ }
+@@ -XXX,XX +XXX,XX @@ static void usb_msd_instance_init(Object *obj)
+     object_property_set_int(obj, -1, "bootindex", NULL);
+ }
+-static void usb_msd_class_initfn_bot(ObjectClass *klass, void *data)
++static void usb_msd_class_bot_initfn(ObjectClass *klass, void *data)
+ {
+     USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
+-    uc->realize = usb_msd_realize_bot;
+-    uc->unrealize = usb_msd_unrealize_bot;
++    uc->realize = usb_msd_bot_realize;
++    uc->unrealize = usb_msd_bot_unrealize;
+     uc->attached_settable = true;
+ }
+ static const TypeInfo msd_info = {
+     .name          = "usb-storage",
+     .parent        = TYPE_USB_STORAGE,
+-    .class_init    = usb_msd_class_initfn_storage,
++    .class_init    = usb_msd_class_storage_initfn,
+     .instance_init = usb_msd_instance_init,
+ };
+ static const TypeInfo bot_info = {
+     .name          = "usb-bot",
+     .parent        = TYPE_USB_STORAGE,
+-    .class_init    = usb_msd_class_initfn_bot,
++    .class_init    = usb_msd_class_bot_initfn,
+ };
+ static void usb_msd_register_types(void)
+--
+.14.3

-New patch
+[Qemu-devel] [PULL v2 06/23] qdev: drop unused #include "sysemu/iothread.h"
+Commit 1351d1ec89eabebc9fdff20451a62c413d7accc1 ("qdev: drop iothread
+property type") forgot to remove this include.
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+Message-id: 20171205133954.31006-1-stefanha@redhat.com
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+---
+ hw/core/qdev-properties-system.c | 1 -
+file changed, 1 deletion(-)
+diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
+index XXXXXXX..XXXXXXX 100644
+--- a/hw/core/qdev-properties-system.c
++++ b/hw/core/qdev-properties-system.c
+@@ -XXX,XX +XXX,XX @@
+ #include "qapi/visitor.h"
+ #include "chardev/char-fe.h"
+ #include "sysemu/tpm_backend.h"
+-#include "sysemu/iothread.h"
+ static void get_pointer(Object *obj, Visitor *v, Property *prop,
+                         char *(*print)(void *ptr),
+--
+.14.3

-[PULL 14/17] qcow2: Don't require aligned offsets in qcow2_co_copy_range_from()
+[Qemu-devel] [PULL v2 07/23] blockdev: hold AioContext for bdrv_unref() in external_snapshot_clean()
-From: Alberto Garcia <berto@igalia.com>
+bdrv_unref() requires the AioContext lock because bdrv_flush() uses
 BDRV_POLL_WHILE(), which assumes the AioContext is currently held.  If
 BDRV_POLL_WHILE() runs without AioContext held the
 pthread_mutex_unlock() call in aio_context_release() fails.
-qemu-img's convert_co_copy_range() operates at the sector level and
+This patch moves bdrv_unref() into the AioContext locked region to solve
-block_copy() operates at the cluster level so this condition is always
+the following pthread_mutex_unlock() failure:
 true, but it is not necessary to restrict this here, so let's leave it
 to the driver implementation return an error if there is any.
-Signed-off-by: Alberto Garcia <berto@igalia.com>
+  #0  0x00007f566181969b in raise () at /lib64/libc.so.6
-Message-id: a4264aaee656910c84161a2965f7a501437379ca.1579374329.git.berto@igalia.com
+  #1  0x00007f566181b3b1 in abort () at /lib64/libc.so.6
-Reviewed-by: Max Reitz <mreitz@redhat.com>
+  #2  0x00005592cd590458 in error_exit (err=<optimized out>, msg=msg@entry=0x5592cdaf6d60 <__func__.23977> "qemu_mutex_unlock") at util/qemu-thread-posix.c:36
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+  #3  0x00005592cd96e738 in qemu_mutex_unlock (mutex=mutex@entry=0x5592ce9505e0) at util/qemu-thread-posix.c:96
   #4  0x00005592cd969b69 in aio_context_release (ctx=ctx@entry=0x5592ce950580) at util/async.c:507
   #5  0x00005592cd8ead78 in bdrv_flush (bs=bs@entry=0x5592cfa87210) at block/io.c:2478
   #6  0x00005592cd89df30 in bdrv_close (bs=0x5592cfa87210) at block.c:3207
   #7  0x00005592cd89df30 in bdrv_delete (bs=0x5592cfa87210) at block.c:3395
   #8  0x00005592cd89df30 in bdrv_unref (bs=0x5592cfa87210) at block.c:4418
   #9  0x00005592cd6b7f86 in qmp_transaction (dev_list=<optimized out>, has_props=<optimized out>, props=<optimized out>, errp=errp@entry=0x7ffe4a1fc9d8) at blockdev.c:2308
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 Reviewed-by: Kevin Wolf <kwolf@redhat.com>
 Reviewed-by: Eric Blake <eblake@redhat.com>
 Message-id: 20171206144550.22295-2-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/qcow2.c | 4 ----
+ blockdev.c | 2 +-
-file changed, 4 deletions(-)
+file changed, 1 insertion(+), 1 deletion(-)
-diff --git a/block/qcow2.c b/block/qcow2.c
+diff --git a/blockdev.c b/blockdev.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2.c
+--- a/blockdev.c
-+++ b/block/qcow2.c
++++ b/blockdev.c
-@@ -XXX,XX +XXX,XX @@ qcow2_co_copy_range_from(BlockDriverState *bs,
+@@ -XXX,XX +XXX,XX @@ static void external_snapshot_clean(BlkActionState *common)
-         case QCOW2_CLUSTER_NORMAL:
+                              DO_UPCAST(ExternalSnapshotState, common, common);
-             child = s->data_file;
+     if (state->aio_context) {
-             copy_offset += offset_into_cluster(s, src_offset);
+         bdrv_drained_end(state->old_bs);
--            if ((copy_offset & 511) != 0) {
+-        aio_context_release(state->aio_context);
--                ret = -EIO;
+         bdrv_unref(state->new_bs);
--                goto out;
++        aio_context_release(state->aio_context);
--            }
+     }
-             break;
+ }
          default:
 --
-.24.1
+.14.3

-New patch
+[Qemu-devel] [PULL v2 08/23] block: don't keep AioContext acquired after external_snapshot_prepare()
+It is not necessary to hold AioContext across transactions anymore since
 bdrv_drained_begin/end() is used to keep the nodes quiesced.  In fact,
 using the AioContext lock for this purpose was always buggy.
 This patch reduces the scope of AioContext locked regions.  This is not
 just a cleanup but also fixes hangs that occur in BDRV_POLL_WHILE()
 because it is unware of recursive locking and does not release the
 AioContext the necessary number of times to allow progress to be made.
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 Reviewed-by: Kevin Wolf <kwolf@redhat.com>
 Reviewed-by: Eric Blake <eblake@redhat.com>
 Message-id: 20171206144550.22295-3-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
  blockdev.c | 71 ++++++++++++++++++++++++++++++++++++++++++--------------------
 file changed, 48 insertions(+), 23 deletions(-)
 diff --git a/blockdev.c b/blockdev.c
 index XXXXXXX..XXXXXXX 100644
 --- a/blockdev.c
 +++ b/blockdev.c
@@ -XXX,XX +XXX,XX @@ typedef struct ExternalSnapshotState {
      BlkActionState common;
      BlockDriverState *old_bs;
      BlockDriverState *new_bs;
 -    AioContext *aio_context;
      bool overlay_appended;
  } ExternalSnapshotState;
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
      ExternalSnapshotState *state =
                               DO_UPCAST(ExternalSnapshotState, common, common);
      TransactionAction *action = common->action;
 +    AioContext *aio_context;
      /* 'blockdev-snapshot' and 'blockdev-snapshot-sync' have similar
       * purpose but a different set of parameters */
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
          return;
      }
 -    /* Acquire AioContext now so any threads operating on old_bs stop */
 -    state->aio_context = bdrv_get_aio_context(state->old_bs);
 -    aio_context_acquire(state->aio_context);
 +    aio_context = bdrv_get_aio_context(state->old_bs);
 +    aio_context_acquire(aio_context);
 +
 +    /* Paired with .clean() */
      bdrv_drained_begin(state->old_bs);
      if (!bdrv_is_inserted(state->old_bs)) {
          error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
 -        return;
 +        goto out;
      }
      if (bdrv_op_is_blocked(state->old_bs,
                             BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT, errp)) {
 -        return;
 +        goto out;
      }
      if (!bdrv_is_read_only(state->old_bs)) {
          if (bdrv_flush(state->old_bs)) {
              error_setg(errp, QERR_IO_ERROR);
 -            return;
 +            goto out;
          }
      }
      if (!bdrv_is_first_non_filter(state->old_bs)) {
          error_setg(errp, QERR_FEATURE_DISABLED, "snapshot");
 -        return;
 +        goto out;
      }
      if (action->type == TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC) {
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
          if (node_name && !snapshot_node_name) {
              error_setg(errp, "New snapshot node name missing");
 -            return;
 +            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");
 -            return;
 +            goto out;
          }
          flags = state->old_bs->open_flags;
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
              int64_t size = bdrv_getlength(state->old_bs);
              if (size < 0) {
                  error_setg_errno(errp, -size, "bdrv_getlength failed");
 -                return;
 +                goto out;
              }
              bdrv_img_create(new_image_file, format,
                              state->old_bs->filename,
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
                              NULL, size, flags, false, &local_err);
              if (local_err) {
                  error_propagate(errp, local_err);
 -                return;
 +                goto out;
              }
          }
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
                                errp);
      /* We will manually add the backing_hd field to the bs later */
      if (!state->new_bs) {
 -        return;
 +        goto out;
      }
      if (bdrv_has_blk(state->new_bs)) {
          error_setg(errp, "The snapshot is already in use");
 -        return;
 +        goto out;
      }
      if (bdrv_op_is_blocked(state->new_bs, BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT,
                             errp)) {
 -        return;
 +        goto out;
      }
      if (state->new_bs->backing != NULL) {
          error_setg(errp, "The snapshot already has a backing image");
 -        return;
 +        goto out;
      }
      if (!state->new_bs->drv->supports_backing) {
          error_setg(errp, "The snapshot does not support backing images");
 -        return;
 +        goto out;
      }
 -    bdrv_set_aio_context(state->new_bs, state->aio_context);
 +    bdrv_set_aio_context(state->new_bs, aio_context);
      /* This removes our old bs and adds the new bs. This is an operation that
       * can fail, so we need to do it in .prepare; undoing it for abort is
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
      bdrv_append(state->new_bs, state->old_bs, &local_err);
      if (local_err) {
          error_propagate(errp, local_err);
 -        return;
 +        goto out;
      }
      state->overlay_appended = true;
 +
 +out:
 +    aio_context_release(aio_context);
  }
  static void external_snapshot_commit(BlkActionState *common)
  {
      ExternalSnapshotState *state =
                               DO_UPCAST(ExternalSnapshotState, common, common);
 +    AioContext *aio_context;
 +
 +    aio_context = bdrv_get_aio_context(state->old_bs);
 +    aio_context_acquire(aio_context);
      /* We don't need (or want) to use the transactional
       * bdrv_reopen_multiple() across all the entries at once, because we
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_commit(BlkActionState *common)
          bdrv_reopen(state->old_bs, state->old_bs->open_flags & ~BDRV_O_RDWR,
                      NULL);
      }
 +
 +    aio_context_release(aio_context);
  }
  static void external_snapshot_abort(BlkActionState *common)
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_abort(BlkActionState *common)
                               DO_UPCAST(ExternalSnapshotState, common, common);
      if (state->new_bs) {
          if (state->overlay_appended) {
 +            AioContext *aio_context;
 +
 +            aio_context = bdrv_get_aio_context(state->old_bs);
 +            aio_context_acquire(aio_context);
 +
              bdrv_ref(state->old_bs);   /* we can't let bdrv_set_backind_hd()
                                            close state->old_bs; we need it */
              bdrv_set_backing_hd(state->new_bs, NULL, &error_abort);
              bdrv_replace_node(state->new_bs, state->old_bs, &error_abort);
              bdrv_unref(state->old_bs); /* bdrv_replace_node() ref'ed old_bs */
 +
 +            aio_context_release(aio_context);
          }
      }
  }
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_clean(BlkActionState *common)
  {
      ExternalSnapshotState *state =
                               DO_UPCAST(ExternalSnapshotState, common, common);
 -    if (state->aio_context) {
 -        bdrv_drained_end(state->old_bs);
 -        bdrv_unref(state->new_bs);
 -        aio_context_release(state->aio_context);
 +    AioContext *aio_context;
 +
 +    if (!state->old_bs) {
 +        return;
      }
 +
 +    aio_context = bdrv_get_aio_context(state->old_bs);
 +    aio_context_acquire(aio_context);
 +
 +    bdrv_drained_end(state->old_bs);
 +    bdrv_unref(state->new_bs);
 +
 +    aio_context_release(aio_context);
  }
  typedef struct DriveBackupState {
 --
 .14.3

-[PULL 12/17] qcow2: Tighten cluster_offset alignment assertions
+[Qemu-devel] [PULL v2 09/23] block: don't keep AioContext acquired after drive_backup_prepare()
-From: Alberto Garcia <berto@igalia.com>
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 Reviewed-by: Kevin Wolf <kwolf@redhat.com>
 Reviewed-by: Eric Blake <eblake@redhat.com>
 Message-id: 20171206144550.22295-4-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
  blockdev.c | 42 ++++++++++++++++++++++++++++++++++--------
 file changed, 34 insertions(+), 8 deletions(-)
-qcow2_alloc_cluster_offset() and qcow2_get_cluster_offset() always
+diff --git a/blockdev.c b/blockdev.c
 return offsets that are cluster-aligned so don't just check that they
 are sector-aligned.
 The check in qcow2_co_preadv_task() is also replaced by an assertion
 for the same reason.
 Signed-off-by: Alberto Garcia <berto@igalia.com>
 Reviewed-by: Max Reitz <mreitz@redhat.com>
 Message-id: 558ba339965f858bede4c73ce3f50f0c0493597d.1579374329.git.berto@igalia.com
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  block/qcow2.c | 9 +++------
 file changed, 3 insertions(+), 6 deletions(-)
 diff --git a/block/qcow2.c b/block/qcow2.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2.c
+--- a/blockdev.c
-+++ b/block/qcow2.c
++++ b/blockdev.c
-@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_preadv_task(BlockDriverState *bs,
+@@ -XXX,XX +XXX,XX @@ static void external_snapshot_clean(BlkActionState *common)
-                                           offset, bytes, qiov, qiov_offset);
+ typedef struct DriveBackupState {
+     BlkActionState common;
-     case QCOW2_CLUSTER_NORMAL:
+     BlockDriverState *bs;
--        if ((file_cluster_offset & 511) != 0) {
+-    AioContext *aio_context;
--            return -EIO;
+     BlockJob *job;
--        }
+ } DriveBackupState;
--
-+        assert(offset_into_cluster(s, file_cluster_offset) == 0);
+@@ -XXX,XX +XXX,XX @@ static void drive_backup_prepare(BlkActionState *common, Error **errp)
-         if (bs->encrypted) {
+     DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
-             return qcow2_co_preadv_encrypted(bs, file_cluster_offset,
+     BlockDriverState *bs;
-                                              offset, bytes, qiov, qiov_offset);
+     DriveBackup *backup;
-@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_pwritev_part(
++    AioContext *aio_context;
-             goto out_locked;
+     Error *local_err = NULL;
-         }
+     assert(common->action->type == TRANSACTION_ACTION_KIND_DRIVE_BACKUP);
--        assert((cluster_offset & 511) == 0);
+@@ -XXX,XX +XXX,XX @@ static void drive_backup_prepare(BlkActionState *common, Error **errp)
-+        assert(offset_into_cluster(s, cluster_offset) == 0);
+         return;
+     }
-         ret = qcow2_pre_write_overlap_check(bs, 0,
-                                             cluster_offset + offset_in_cluster,
+-    /* AioContext is released in .clean() */
-@@ -XXX,XX +XXX,XX @@ qcow2_co_copy_range_to(BlockDriverState *bs,
+-    state->aio_context = bdrv_get_aio_context(bs);
-             goto fail;
+-    aio_context_acquire(state->aio_context);
-         }
++    aio_context = bdrv_get_aio_context(bs);
++    aio_context_acquire(aio_context);
--        assert((cluster_offset & 511) == 0);
++
-+        assert(offset_into_cluster(s, cluster_offset) == 0);
++    /* Paired with .clean() */
+     bdrv_drained_begin(bs);
-         ret = qcow2_pre_write_overlap_check(bs, 0,
++
-                 cluster_offset + offset_in_cluster, cur_bytes, true);
+     state->bs = bs;
      state->job = do_drive_backup(backup, common->block_job_txn, &local_err);
      if (local_err) {
          error_propagate(errp, local_err);
 -        return;
 +        goto out;
      }
 +
 +out:
 +    aio_context_release(aio_context);
  }
  static void drive_backup_commit(BlkActionState *common)
  {
      DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
 +    AioContext *aio_context;
 +
 +    aio_context = bdrv_get_aio_context(state->bs);
 +    aio_context_acquire(aio_context);
 +
      assert(state->job);
      block_job_start(state->job);
 +
 +    aio_context_release(aio_context);
  }
  static void drive_backup_abort(BlkActionState *common)
@@ -XXX,XX +XXX,XX @@ static void drive_backup_abort(BlkActionState *common)
      DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
      if (state->job) {
 +        AioContext *aio_context;
 +
 +        aio_context = bdrv_get_aio_context(state->bs);
 +        aio_context_acquire(aio_context);
 +
          block_job_cancel_sync(state->job);
 +
 +        aio_context_release(aio_context);
      }
  }
  static void drive_backup_clean(BlkActionState *common)
  {
      DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
 +    AioContext *aio_context;
 -    if (state->aio_context) {
 -        bdrv_drained_end(state->bs);
 -        aio_context_release(state->aio_context);
 +    if (!state->bs) {
 +        return;
      }
 +
 +    aio_context = bdrv_get_aio_context(state->bs);
 +    aio_context_acquire(aio_context);
 +
 +    bdrv_drained_end(state->bs);
 +
 +    aio_context_release(aio_context);
  }
  typedef struct BlockdevBackupState {
 --
-.24.1
+.14.3

-[PULL 11/17] qcow2: Don't round the L1 table allocation up to the sector size
+[Qemu-devel] [PULL v2 10/23] block: don't keep AioContext acquired after blockdev_backup_prepare()
-From: Alberto Garcia <berto@igalia.com>
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 Reviewed-by: Kevin Wolf <kwolf@redhat.com>
 Reviewed-by: Eric Blake <eblake@redhat.com>
 Message-id: 20171206144550.22295-5-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
  blockdev.c | 44 ++++++++++++++++++++++++++++++++++----------
 file changed, 34 insertions(+), 10 deletions(-)
-The L1 table is read from disk using the byte-based bdrv_pread() and
+diff --git a/blockdev.c b/blockdev.c
 is never accessed beyond its last element, so there's no need to
 allocate more memory than that.
 Signed-off-by: Alberto Garcia <berto@igalia.com>
 Reviewed-by: Max Reitz <mreitz@redhat.com>
 Message-id: b2e27214ec7b03a585931bcf383ee1ac3a641a10.1579374329.git.berto@igalia.com
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  block/qcow2-cluster.c  | 5 ++---
  block/qcow2-refcount.c | 2 +-
  block/qcow2-snapshot.c | 3 +--
  block/qcow2.c          | 2 +-
 files changed, 5 insertions(+), 7 deletions(-)
 diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2-cluster.c
+--- a/blockdev.c
-+++ b/block/qcow2-cluster.c
++++ b/blockdev.c
-@@ -XXX,XX +XXX,XX @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
+@@ -XXX,XX +XXX,XX @@ typedef struct BlockdevBackupState {
- #endif
+     BlkActionState common;
+     BlockDriverState *bs;
-     new_l1_size2 = sizeof(uint64_t) * new_l1_size;
+     BlockJob *job;
--    new_l1_table = qemu_try_blockalign(bs->file->bs,
+-    AioContext *aio_context;
--                                       ROUND_UP(new_l1_size2, 512));
+ } BlockdevBackupState;
-+    new_l1_table = qemu_try_blockalign(bs->file->bs, new_l1_size2);
-     if (new_l1_table == NULL) {
+ static BlockJob *do_blockdev_backup(BlockdevBackup *backup, BlockJobTxn *txn,
-         return -ENOMEM;
+@@ -XXX,XX +XXX,XX @@ static void blockdev_backup_prepare(BlkActionState *common, Error **errp)
      BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);
      BlockdevBackup *backup;
      BlockDriverState *bs, *target;
 +    AioContext *aio_context;
      Error *local_err = NULL;
      assert(common->action->type == TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP);
@@ -XXX,XX +XXX,XX @@ static void blockdev_backup_prepare(BlkActionState *common, Error **errp)
          return;
      }
--    memset(new_l1_table, 0, ROUND_UP(new_l1_size2, 512));
-+    memset(new_l1_table, 0, new_l1_size2);
+-    /* AioContext is released in .clean() */
+-    state->aio_context = bdrv_get_aio_context(bs);
-     if (s->l1_size) {
+-    if (state->aio_context != bdrv_get_aio_context(target)) {
-         memcpy(new_l1_table, s->l1_table, s->l1_size * sizeof(uint64_t));
+-        state->aio_context = NULL;
-diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
++    aio_context = bdrv_get_aio_context(bs);
-index XXXXXXX..XXXXXXX 100644
++    if (aio_context != bdrv_get_aio_context(target)) {
---- a/block/qcow2-refcount.c
+         error_setg(errp, "Backup between two IO threads is not implemented");
-+++ b/block/qcow2-refcount.c
+         return;
@@ -XXX,XX +XXX,XX @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
       * l1_table_offset when it is the current s->l1_table_offset! Be careful
       * when changing this! */
      if (l1_table_offset != s->l1_table_offset) {
 -        l1_table = g_try_malloc0(ROUND_UP(l1_size2, 512));
 +        l1_table = g_try_malloc0(l1_size2);
          if (l1_size2 && l1_table == NULL) {
              ret = -ENOMEM;
              goto fail;
 diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/qcow2-snapshot.c
 +++ b/block/qcow2-snapshot.c
@@ -XXX,XX +XXX,XX @@ int qcow2_snapshot_load_tmp(BlockDriverState *bs,
          return ret;
      }
-     new_l1_bytes = sn->l1_size * sizeof(uint64_t);
+-    aio_context_acquire(state->aio_context);
--    new_l1_table = qemu_try_blockalign(bs->file->bs,
++    aio_context_acquire(aio_context);
--                                       ROUND_UP(new_l1_bytes, 512));
+     state->bs = bs;
-+    new_l1_table = qemu_try_blockalign(bs->file->bs, new_l1_bytes);
++
-     if (new_l1_table == NULL) {
++    /* Paired with .clean() */
-         return -ENOMEM;
+     bdrv_drained_begin(state->bs);
      state->job = do_blockdev_backup(backup, common->block_job_txn, &local_err);
      if (local_err) {
          error_propagate(errp, local_err);
 -        return;
 +        goto out;
      }
-diff --git a/block/qcow2.c b/block/qcow2.c
++
-index XXXXXXX..XXXXXXX 100644
++out:
---- a/block/qcow2.c
++    aio_context_release(aio_context);
-+++ b/block/qcow2.c
+ }
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
+ static void blockdev_backup_commit(BlkActionState *common)
-     if (s->l1_size > 0) {
+ {
-         s->l1_table = qemu_try_blockalign(bs->file->bs,
+     BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);
--            ROUND_UP(s->l1_size * sizeof(uint64_t), 512));
++    AioContext *aio_context;
-+                                          s->l1_size * sizeof(uint64_t));
++
-         if (s->l1_table == NULL) {
++    aio_context = bdrv_get_aio_context(state->bs);
-             error_setg(errp, "Could not allocate L1 table");
++    aio_context_acquire(aio_context);
-             ret = -ENOMEM;
++
      assert(state->job);
      block_job_start(state->job);
 +
 +    aio_context_release(aio_context);
  }
  static void blockdev_backup_abort(BlkActionState *common)
@@ -XXX,XX +XXX,XX @@ static void blockdev_backup_abort(BlkActionState *common)
      BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);
      if (state->job) {
 +        AioContext *aio_context;
 +
 +        aio_context = bdrv_get_aio_context(state->bs);
 +        aio_context_acquire(aio_context);
 +
          block_job_cancel_sync(state->job);
 +
 +        aio_context_release(aio_context);
      }
  }
  static void blockdev_backup_clean(BlkActionState *common)
  {
      BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);
 +    AioContext *aio_context;
 -    if (state->aio_context) {
 -        bdrv_drained_end(state->bs);
 -        aio_context_release(state->aio_context);
 +    if (!state->bs) {
 +        return;
      }
 +
 +    aio_context = bdrv_get_aio_context(state->bs);
 +    aio_context_acquire(aio_context);
 +
 +    bdrv_drained_end(state->bs);
 +
 +    aio_context_release(aio_context);
  }
  typedef struct BlockDirtyBitmapState {
 --
-.24.1
+.14.3

-[PULL 15/17] qcow2: Use BDRV_SECTOR_SIZE instead of the hardcoded value
+[Qemu-devel] [PULL v2 11/23] block: don't keep AioContext acquired after internal_snapshot_prepare()
-From: Alberto Garcia <berto@igalia.com>
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 Reviewed-by: Kevin Wolf <kwolf@redhat.com>
 Reviewed-by: Eric Blake <eblake@redhat.com>
 Message-id: 20171206144550.22295-6-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
  blockdev.c | 47 +++++++++++++++++++++++++++++++----------------
 file changed, 31 insertions(+), 16 deletions(-)
-This replaces all remaining instances in the qcow2 code.
+diff --git a/blockdev.c b/blockdev.c
 Signed-off-by: Alberto Garcia <berto@igalia.com>
 Message-id: b5f74b606c2d9873b12d29acdb7fd498029c4025.1579374329.git.berto@igalia.com
 Reviewed-by: Max Reitz <mreitz@redhat.com>
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  block/qcow2.c | 8 +++++---
 file changed, 5 insertions(+), 3 deletions(-)
 diff --git a/block/qcow2.c b/block/qcow2.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2.c
+--- a/blockdev.c
-+++ b/block/qcow2.c
++++ b/blockdev.c
-@@ -XXX,XX +XXX,XX @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
+@@ -XXX,XX +XXX,XX @@ struct BlkActionState {
+ typedef struct InternalSnapshotState {
-     /* Validate options and set default values */
+     BlkActionState common;
-     if (!QEMU_IS_ALIGNED(qcow2_opts->size, BDRV_SECTOR_SIZE)) {
+     BlockDriverState *bs;
--        error_setg(errp, "Image size must be a multiple of 512 bytes");
+-    AioContext *aio_context;
-+        error_setg(errp, "Image size must be a multiple of %u bytes",
+     QEMUSnapshotInfo sn;
-+                   (unsigned) BDRV_SECTOR_SIZE);
+     bool created;
-         ret = -EINVAL;
+ } InternalSnapshotState;
-         goto out;
+@@ -XXX,XX +XXX,XX @@ static void internal_snapshot_prepare(BlkActionState *common,
      qemu_timeval tv;
      BlockdevSnapshotInternal *internal;
      InternalSnapshotState *state;
 +    AioContext *aio_context;
      int ret1;
      g_assert(common->action->type ==
@@ -XXX,XX +XXX,XX @@ static void internal_snapshot_prepare(BlkActionState *common,
          return;
      }
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
-         return -ENOTSUP;
+-    /* AioContext is released in .clean() */
 -    state->aio_context = bdrv_get_aio_context(bs);
 -    aio_context_acquire(state->aio_context);
 +    aio_context = bdrv_get_aio_context(bs);
 +    aio_context_acquire(aio_context);
      state->bs = bs;
 +
 +    /* Paired with .clean() */
      bdrv_drained_begin(bs);
      if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_INTERNAL_SNAPSHOT, errp)) {
 -        return;
 +        goto out;
      }
--    if (offset & 511) {
+     if (bdrv_is_read_only(bs)) {
--        error_setg(errp, "The new size must be a multiple of 512");
+         error_setg(errp, "Device '%s' is read only", device);
-+    if (!QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)) {
+-        return;
-+        error_setg(errp, "The new size must be a multiple of %u",
++        goto out;
 +                   (unsigned) BDRV_SECTOR_SIZE);
          return -EINVAL;
      }
+     if (!bdrv_can_snapshot(bs)) {
+         error_setg(errp, "Block format '%s' used by device '%s' "
+                    "does not support internal snapshots",
+                    bs->drv->format_name, device);
+-        return;
++        goto out;
+     }
+     if (!strlen(name)) {
+         error_setg(errp, "Name is empty");
+-        return;
++        goto out;
+     }
+     /* check whether a snapshot with name exist */
+@@ -XXX,XX +XXX,XX @@ static void internal_snapshot_prepare(BlkActionState *common,
+                                             &local_err);
+     if (local_err) {
+         error_propagate(errp, local_err);
+-        return;
++        goto out;
+     } else if (ret) {
+         error_setg(errp,
+                    "Snapshot with name '%s' already exists on device '%s'",
+                    name, device);
+-        return;
++        goto out;
+     }
+     /* 3. take the snapshot */
+@@ -XXX,XX +XXX,XX @@ static void internal_snapshot_prepare(BlkActionState *common,
+         error_setg_errno(errp, -ret1,
+                          "Failed to create snapshot '%s' on device '%s'",
+                          name, device);
+-        return;
++        goto out;
+     }
+     /* 4. succeed, mark a snapshot is created */
+     state->created = true;
++
++out:
++    aio_context_release(aio_context);
+ }
+ static void internal_snapshot_abort(BlkActionState *common)
+@@ -XXX,XX +XXX,XX @@ static void internal_snapshot_abort(BlkActionState *common)
+                              DO_UPCAST(InternalSnapshotState, common, common);
+     BlockDriverState *bs = state->bs;
+     QEMUSnapshotInfo *sn = &state->sn;
++    AioContext *aio_context;
+     Error *local_error = NULL;
+     if (!state->created) {
+         return;
+     }
++    aio_context = bdrv_get_aio_context(state->bs);
++    aio_context_acquire(aio_context);
++
+     if (bdrv_snapshot_delete(bs, sn->id_str, sn->name, &local_error) < 0) {
+         error_reportf_err(local_error,
+                           "Failed to delete snapshot with id '%s' and "
+@@ -XXX,XX +XXX,XX @@ static void internal_snapshot_abort(BlkActionState *common)
+                           sn->id_str, sn->name,
+                           bdrv_get_device_name(bs));
+     }
++
++    aio_context_release(aio_context);
+ }
+ static void internal_snapshot_clean(BlkActionState *common)
+ {
+     InternalSnapshotState *state = DO_UPCAST(InternalSnapshotState,
+                                              common, common);
++    AioContext *aio_context;
+-    if (state->aio_context) {
+-        if (state->bs) {
+-            bdrv_drained_end(state->bs);
+-        }
+-        aio_context_release(state->aio_context);
++    if (!state->bs) {
++        return;
+     }
++
++    aio_context = bdrv_get_aio_context(state->bs);
++    aio_context_acquire(aio_context);
++
++    bdrv_drained_end(state->bs);
++
++    aio_context_release(aio_context);
+ }
+ /* external snapshot private data */
 --
-.24.1
+.14.3

-[PULL 02/17] block: Use a GString in bdrv_perm_names()
+[Qemu-devel] [PULL v2 12/23] block: drop unused BlockDirtyBitmapState->aio_context field
-From: Alberto Garcia <berto@igalia.com>
+The dirty bitmap actions in qmp_transaction have not used AioContext
 since the dirty bitmap locking discipline was introduced in commit
 2119882c7eb7e2c612b24fc0c8d86f5887d6f1c3 ("block: introduce
 dirty_bitmap_mutex").  Remove the unused field.
-This is a bit more efficient than having to allocate and free memory
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-for each new permission.
+Reviewed-by: Kevin Wolf <kwolf@redhat.com>
 Reviewed-by: Eric Blake <eblake@redhat.com>
 Message-id: 20171206144550.22295-7-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
  blockdev.c | 13 -------------
 file changed, 13 deletions(-)
-The default size (30) is enough for "consistent read, write, resize".
+diff --git a/blockdev.c b/blockdev.c
 Signed-off-by: Alberto Garcia <berto@igalia.com>
 Message-id: 20200110171518.22168-1-berto@igalia.com
 Reviewed-by: Eric Blake <eblake@redhat.com>
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  block.c | 11 ++++++-----
 file changed, 6 insertions(+), 5 deletions(-)
 diff --git a/block.c b/block.c
 index XXXXXXX..XXXXXXX 100644
---- a/block.c
+--- a/blockdev.c
-+++ b/block.c
++++ b/blockdev.c
-@@ -XXX,XX +XXX,XX @@ char *bdrv_perm_names(uint64_t perm)
+@@ -XXX,XX +XXX,XX @@ typedef struct BlockDirtyBitmapState {
-         { 0, NULL }
+     BlkActionState common;
-     };
+     BdrvDirtyBitmap *bitmap;
+     BlockDriverState *bs;
--    char *result = g_strdup("");
+-    AioContext *aio_context;
-+    GString *result = g_string_sized_new(30);
+     HBitmap *backup;
-     struct perm_name *p;
+     bool prepared;
+ } BlockDirtyBitmapState;
-     for (p = permissions; p->name; p++) {
+@@ -XXX,XX +XXX,XX @@ static void block_dirty_bitmap_clear_prepare(BlkActionState *common,
          if (perm & p->perm) {
 -            char *old = result;
 -            result = g_strdup_printf("%s%s%s", old, *old ? ", " : "", p->name);
 -            g_free(old);
 +            if (result->len > 0) {
 +                g_string_append(result, ", ");
 +            }
 +            g_string_append(result, p->name);
          }
      }
--    return result;
+     bdrv_clear_dirty_bitmap(state->bitmap, &state->backup);
-+    return g_string_free(result, FALSE);
+-    /* AioContext is released in .clean() */
  }
- /*
+ static void block_dirty_bitmap_clear_abort(BlkActionState *common)
@@ -XXX,XX +XXX,XX @@ static void block_dirty_bitmap_clear_commit(BlkActionState *common)
      hbitmap_free(state->backup);
  }
 -static void block_dirty_bitmap_clear_clean(BlkActionState *common)
 -{
 -    BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
 -                                             common, common);
 -
 -    if (state->aio_context) {
 -        aio_context_release(state->aio_context);
 -    }
 -}
 -
  static void abort_prepare(BlkActionState *common, Error **errp)
  {
      error_setg(errp, "Transaction aborted using Abort action");
@@ -XXX,XX +XXX,XX @@ static const BlkActionOps actions[] = {
          .prepare = block_dirty_bitmap_clear_prepare,
          .commit = block_dirty_bitmap_clear_commit,
          .abort = block_dirty_bitmap_clear_abort,
 -        .clean = block_dirty_bitmap_clear_clean,
      }
  };
 --
-.24.1
+.14.3

-[PULL 09/17] iotests: Skip Python-based tests if QEMU does not support virtio-blk
+[Qemu-devel] [PULL v2 13/23] iothread: add iothread_by_id() API
-From: Thomas Huth <thuth@redhat.com>
+Encapsulate IOThread QOM object lookup so that callers don't need to
 know how and where IOThread objects live.
-We are going to enable some of the python-based tests in the "auto" group,
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-and these tests require virtio-blk to work properly. Running iotests
+Reviewed-by: Kevin Wolf <kwolf@redhat.com>
-without virtio-blk likely does not make too much sense anyway, so instead
+Reviewed-by: Eric Blake <eblake@redhat.com>
-of adding a check for the availability of virtio-blk to each and every
+Message-id: 20171206144550.22295-8-stefanha@redhat.com
-test (which does not sound very appealing), let's rather add a check for
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-this a central spot in the "check" script instead (so that it is still
+---
-possible to run "make check" for qemu-system-tricore for example).
+ include/sysemu/iothread.h | 1 +
  iothread.c                | 7 +++++++
 files changed, 8 insertions(+)
-Signed-off-by: Thomas Huth <thuth@redhat.com>
+diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
-Message-id: 20200121095205.26323-6-thuth@redhat.com
+index XXXXXXX..XXXXXXX 100644
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+--- a/include/sysemu/iothread.h
----
++++ b/include/sysemu/iothread.h
- tests/qemu-iotests/check | 12 ++++++++++--
+@@ -XXX,XX +XXX,XX @@ typedef struct {
-file changed, 10 insertions(+), 2 deletions(-)
+    OBJECT_CHECK(IOThread, obj, TYPE_IOTHREAD)
-diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
+ char *iothread_get_id(IOThread *iothread);
-index XXXXXXX..XXXXXXX 100755
++IOThread *iothread_by_id(const char *id);
---- a/tests/qemu-iotests/check
+ AioContext *iothread_get_aio_context(IOThread *iothread);
-+++ b/tests/qemu-iotests/check
+ void iothread_stop_all(void);
-@@ -XXX,XX +XXX,XX @@ fi
+ GMainContext *iothread_get_g_main_context(IOThread *iothread);
- python_usable=false
+diff --git a/iothread.c b/iothread.c
- if $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)'
+index XXXXXXX..XXXXXXX 100644
- then
+--- a/iothread.c
--    python_usable=true
++++ b/iothread.c
-+    # Our python framework also requires virtio-blk
+@@ -XXX,XX +XXX,XX @@ void iothread_destroy(IOThread *iothread)
-+    if "$QEMU_PROG" -M none -device help | grep -q virtio-blk >/dev/null 2>&1
+ {
-+    then
+     object_unparent(OBJECT(iothread));
-+        python_usable=true
+ }
-+    else
++
-+        python_unusable_because="Missing virtio-blk in QEMU binary"
++/* Lookup IOThread by its id.  Only finds user-created objects, not internal
-+    fi
++ * iothread_create() objects. */
-+else
++IOThread *iothread_by_id(const char *id)
-+    python_unusable_because="Unsupported Python version"
++{
- fi
++    return IOTHREAD(object_resolve_path_type(id, TYPE_IOTHREAD, NULL));
++}
  default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p')
@@ -XXX,XX +XXX,XX @@ do
                  run_command="$PYTHON $seq"
              else
                  run_command="false"
 -                echo "Unsupported Python version" > $seq.notrun
 +                echo "$python_unusable_because" > $seq.notrun
              fi
          else
              run_command="./$seq"
 --
-.24.1
+.14.3

-[PULL 13/17] qcow2: Use bs->bl.request_alignment when updating an L1 entry
+[Qemu-devel] [PULL v2 14/23] blockdev: add x-blockdev-set-iothread testing command
-From: Alberto Garcia <berto@igalia.com>
+Currently there is no easy way for iotests to ensure that a BDS is bound
 to a particular IOThread.  Normally the virtio-blk device calls
 blk_set_aio_context() when dataplane is enabled during guest driver
 initialization.  This never happens in iotests since -machine
 accel=qtest means there is no guest activity (including device driver
 initialization).
-When updating an L1 entry the qcow2 driver writes a (512-byte) sector
+This patch adds a QMP command to explicitly assign IOThreads in test
-worth of data to avoid a read-modify-write cycle. Instead of always
+cases.  See qapi/block-core.json for a description of the command.
 writing 512 bytes we should follow the alignment requirements of the
 storage backend.
-(the only exception is when the alignment is larger than the cluster
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-size because then we could be overwriting data after the L1 table)
+Reviewed-by: Kevin Wolf <kwolf@redhat.com>
 Reviewed-by: Eric Blake <eblake@redhat.com>
 Message-id: 20171206144550.22295-9-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
  qapi/block-core.json | 36 ++++++++++++++++++++++++++++++++++++
  blockdev.c           | 41 +++++++++++++++++++++++++++++++++++++++++
 files changed, 77 insertions(+)
-Signed-off-by: Alberto Garcia <berto@igalia.com>
+diff --git a/qapi/block-core.json b/qapi/block-core.json
 Message-id: 71f34d4ae4b367b32fb36134acbf4f4f7ee681f4.1579374329.git.berto@igalia.com
 Reviewed-by: Max Reitz <mreitz@redhat.com>
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  block/qcow2-cluster.c | 25 +++++++++++++++----------
 file changed, 15 insertions(+), 10 deletions(-)
 diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2-cluster.c
+--- a/qapi/block-core.json
-+++ b/block/qcow2-cluster.c
++++ b/qapi/block-core.json
-@@ -XXX,XX +XXX,XX @@ static int l2_load(BlockDriverState *bs, uint64_t offset,
+@@ -XXX,XX +XXX,XX @@
    'data' : { 'parent': 'str',
               '*child': 'str',
               '*node': 'str' } }
 +
 +##
 +# @x-blockdev-set-iothread:
 +#
 +# Move @node and its children into the @iothread.  If @iothread is null then
 +# move @node and its children into the main loop.
 +#
 +# The node must not be attached to a BlockBackend.
 +#
 +# @node-name: the name of the block driver node
 +#
 +# @iothread: the name of the IOThread object or null for the main loop
 +#
 +# Note: this command is experimental and intended for test cases that need
 +# control over IOThreads only.
 +#
 +# Since: 2.12
 +#
 +# Example:
 +#
 +# 1. Move a node into an IOThread
 +# -> { "execute": "x-blockdev-set-iothread",
 +#      "arguments": { "node-name": "disk1",
 +#                     "iothread": "iothread0" } }
 +# <- { "return": {} }
 +#
 +# 2. Move a node into the main loop
 +# -> { "execute": "x-blockdev-set-iothread",
 +#      "arguments": { "node-name": "disk1",
 +#                     "iothread": null } }
 +# <- { "return": {} }
 +#
 +##
 +{ 'command': 'x-blockdev-set-iothread',
 +  'data' : { 'node-name': 'str',
 +             'iothread': 'StrOrNull' } }
 diff --git a/blockdev.c b/blockdev.c
 index XXXXXXX..XXXXXXX 100644
 --- a/blockdev.c
 +++ b/blockdev.c
@@ -XXX,XX +XXX,XX @@
  #include "qapi/qmp/qerror.h"
  #include "qapi/qobject-output-visitor.h"
  #include "sysemu/sysemu.h"
 +#include "sysemu/iothread.h"
  #include "block/block_int.h"
  #include "qmp-commands.h"
  #include "block/trace.h"
@@ -XXX,XX +XXX,XX @@ BlockJobInfoList *qmp_query_block_jobs(Error **errp)
      return head;
  }
- /*
++void qmp_x_blockdev_set_iothread(const char *node_name, StrOrNull *iothread,
-- * Writes one sector of the L1 table to the disk (can't update single entries
++                                 Error **errp)
-- * and we really don't want bdrv_pread to perform a read-modify-write)
++{
-+ * Writes an L1 entry to disk (note that depending on the alignment
++    AioContext *old_context;
-+ * requirements this function may write more that just one entry in
++    AioContext *new_context;
-+ * order to prevent bdrv_pwrite from performing a read-modify-write)
++    BlockDriverState *bs;
-  */
++
--#define L1_ENTRIES_PER_SECTOR (512 / 8)
++    bs = bdrv_find_node(node_name);
- int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index)
++    if (!bs) {
- {
++        error_setg(errp, "Cannot find node %s", node_name);
-     BDRVQcow2State *s = bs->opaque;
++        return;
 -    uint64_t buf[L1_ENTRIES_PER_SECTOR] = { 0 };
      int l1_start_index;
      int i, ret;
 +    int bufsize = MAX(sizeof(uint64_t),
 +                      MIN(bs->file->bs->bl.request_alignment, s->cluster_size));
 +    int nentries = bufsize / sizeof(uint64_t);
 +    g_autofree uint64_t *buf = g_try_new0(uint64_t, nentries);
 -    l1_start_index = l1_index & ~(L1_ENTRIES_PER_SECTOR - 1);
 -    for (i = 0; i < L1_ENTRIES_PER_SECTOR && l1_start_index + i < s->l1_size;
 -         i++)
 -    {
 +    if (buf == NULL) {
 +        return -ENOMEM;
 +    }
 +
-+    l1_start_index = QEMU_ALIGN_DOWN(l1_index, nentries);
++    /* If we want to allow more extreme test scenarios this guard could be
-+    for (i = 0; i < MIN(nentries, s->l1_size - l1_start_index); i++) {
++     * removed.  For now it protects against accidents. */
-         buf[i] = cpu_to_be64(s->l1_table[l1_start_index + i]);
++    if (bdrv_has_blk(bs)) {
-     }
++        error_setg(errp, "Node %s is in use", node_name);
++        return;
-     ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_ACTIVE_L1,
++    }
--            s->l1_table_offset + 8 * l1_start_index, sizeof(buf), false);
++
-+            s->l1_table_offset + 8 * l1_start_index, bufsize, false);
++    if (iothread->type == QTYPE_QSTRING) {
-     if (ret < 0) {
++        IOThread *obj = iothread_by_id(iothread->u.s);
-         return ret;
++        if (!obj) {
-     }
++            error_setg(errp, "Cannot find iothread %s", iothread->u.s);
-@@ -XXX,XX +XXX,XX @@ int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index)
++            return;
-     BLKDBG_EVENT(bs->file, BLKDBG_L1_UPDATE);
++        }
-     ret = bdrv_pwrite_sync(bs->file,
++
-                            s->l1_table_offset + 8 * l1_start_index,
++        new_context = iothread_get_aio_context(obj);
--                           buf, sizeof(buf));
++    } else {
-+                           buf, bufsize);
++        new_context = qemu_get_aio_context();
-     if (ret < 0) {
++    }
-         return ret;
++
-     }
++    old_context = bdrv_get_aio_context(bs);
 +    aio_context_acquire(old_context);
 +
 +    bdrv_set_aio_context(bs, new_context);
 +
 +    aio_context_release(old_context);
 +}
 +
  QemuOptsList qemu_common_drive_opts = {
      .name = "drive",
      .head = QTAILQ_HEAD_INITIALIZER(qemu_common_drive_opts.head),
 --
-.24.1
+.14.3

-[PULL 10/17] iotests: Enable more tests in the 'auto' group to improve test coverage
+[Qemu-devel] [PULL v2 15/23] qemu-iotests: add 202 external snapshots IOThread test
-From: Thomas Huth <thuth@redhat.com>
+QMP 'transaction' blockdev-snapshot-sync with multiple disks in an
 IOThread is an untested code path.  Several bugs have been found in
 connection with this command.  This patch adds a test case to prevent
 future regressions.
-According to Kevin, tests 030, 040 and 041 are among the most valuable
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-tests that we have, so we should always run them if possible, even if
+Reviewed-by: Kevin Wolf <kwolf@redhat.com>
-they take a little bit longer.
+Reviewed-by: Eric Blake <eblake@redhat.com>
 Message-id: 20171206144550.22295-10-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
  tests/qemu-iotests/202     | 95 ++++++++++++++++++++++++++++++++++++++++++++++
  tests/qemu-iotests/202.out | 11 ++++++
  tests/qemu-iotests/group   |  1 +
 files changed, 107 insertions(+)
  create mode 100755 tests/qemu-iotests/202
  create mode 100644 tests/qemu-iotests/202.out
-According to Max, it would be good to have a test for iothreads and
+diff --git a/tests/qemu-iotests/202 b/tests/qemu-iotests/202
-migration. 127 and 256 seem to be good candidates for iothreads. For
+new file mode 100755
-migration, let's enable 181 and 203 (which also tests iothreads).
+index XXXXXXX..XXXXXXX
-(091 would be a good candidate for migration, too, but Alex Bennée
+--- /dev/null
-reported that this test fails on ZFS file systems, so it can't be
++++ b/tests/qemu-iotests/202
-included yet)
+@@ -XXX,XX +XXX,XX @@
++#!/usr/bin/env python
-Reviewed-by: Max Reitz <mreitz@redhat.com>
++#
-Signed-off-by: Thomas Huth <thuth@redhat.com>
++# Copyright (C) 2017 Red Hat, Inc.
-Message-id: 20200121095205.26323-7-thuth@redhat.com
++#
-Signed-off-by: Max Reitz <mreitz@redhat.com>
++# 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
- tests/qemu-iotests/group | 14 +++++++-------
++# the Free Software Foundation; either version 2 of the License, or
-file changed, 7 insertions(+), 7 deletions(-)
++# (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: Stefan Hajnoczi <stefanha@redhat.com>
 +#
 +# Check that QMP 'transaction' blockdev-snapshot-sync with multiple drives on a
 +# single IOThread completes successfully.  This particular command triggered a
 +# hang due to recursive AioContext locking and BDRV_POLL_WHILE().  Protect
 +# against regressions.
 +
 +import iotests
 +
 +iotests.verify_image_format(supported_fmts=['qcow2'])
 +iotests.verify_platform(['linux'])
 +
 +with iotests.FilePath('disk0.img') as disk0_img_path, \
 +     iotests.FilePath('disk1.img') as disk1_img_path, \
 +     iotests.FilePath('disk0-snap.img') as disk0_snap_img_path, \
 +     iotests.FilePath('disk1-snap.img') as disk1_snap_img_path, \
 +     iotests.VM() as vm:
 +
 +    img_size = '10M'
 +    iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, disk0_img_path, img_size)
 +    iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, disk1_img_path, img_size)
 +
 +    iotests.log('Launching VM...')
 +    vm.launch()
 +
 +    iotests.log('Adding IOThread...')
 +    iotests.log(vm.qmp('object-add',
 +                       qom_type='iothread',
 +                       id='iothread0'))
 +
 +    iotests.log('Adding blockdevs...')
 +    iotests.log(vm.qmp('blockdev-add',
 +                       driver=iotests.imgfmt,
 +                       node_name='disk0',
 +                       file={
 +                           'driver': 'file',
 +                           'filename': disk0_img_path,
 +                       }))
 +    iotests.log(vm.qmp('blockdev-add',
 +                       driver=iotests.imgfmt,
 +                       node_name='disk1',
 +                       file={
 +                           'driver': 'file',
 +                           'filename': disk1_img_path,
 +                       }))
 +
 +    iotests.log('Setting iothread...')
 +    iotests.log(vm.qmp('x-blockdev-set-iothread',
 +                       node_name='disk0',
 +                       iothread='iothread0'))
 +    iotests.log(vm.qmp('x-blockdev-set-iothread',
 +                       node_name='disk1',
 +                       iothread='iothread0'))
 +
 +    iotests.log('Creating external snapshots...')
 +    iotests.log(vm.qmp(
 +                  'transaction',
 +                  actions=[
 +                      {
 +                          'data': {
 +                              'node-name': 'disk0',
 +                              'snapshot-file': disk0_snap_img_path,
 +                              'snapshot-node-name': 'disk0-snap',
 +                              'mode': 'absolute-paths',
 +                              'format': iotests.imgfmt,
 +                          },
 +                          'type': 'blockdev-snapshot-sync'
 +                      }, {
 +                          'data': {
 +                              'node-name': 'disk1',
 +                              'snapshot-file': disk1_snap_img_path,
 +                              'snapshot-node-name': 'disk1-snap',
 +                              'mode': 'absolute-paths',
 +                              'format': iotests.imgfmt
 +                          },
 +                          'type': 'blockdev-snapshot-sync'
 +                      }
 +                  ]))
 diff --git a/tests/qemu-iotests/202.out b/tests/qemu-iotests/202.out
 new file mode 100644
 index XXXXXXX..XXXXXXX
 --- /dev/null
 +++ b/tests/qemu-iotests/202.out
@@ -XXX,XX +XXX,XX @@
 +Launching VM...
 +Adding IOThread...
 +{u'return': {}}
 +Adding blockdevs...
 +{u'return': {}}
 +{u'return': {}}
 +Setting iothread...
 +{u'return': {}}
 +{u'return': {}}
 +Creating external snapshots...
 +{u'return': {}}
 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 backing quick
+rw auto
-rw auto quick
+rw auto
--030 rw backing
++202 rw auto quick
 +030 rw auto backing
 rw auto quick
 rw auto quick
 rw auto quick
@@ -XXX,XX +XXX,XX @@
 rw auto backing quick
 rw auto backing quick
 rw auto quick
 -040 rw
 -041 rw backing
 +040 rw auto
 +041 rw auto backing
 rw auto quick
 rw auto backing
 rw
@@ -XXX,XX +XXX,XX @@
 rw backing
 rw
 rw auto backing
 -127 rw backing quick
 +127 rw auto backing quick
 rw quick
 rw quick
 rw quick
@@ -XXX,XX +XXX,XX @@
 rw auto quick
 img
 rw auto quick
 -181 rw migration
 +181 rw auto migration
 rw quick
 rw migration
 rw auto quick
@@ -XXX,XX +XXX,XX @@
 rw
 rw migration
 rw quick
 -203 rw migration
 +203 rw auto migration
 rw quick
 rw quick
 rw
@@ -XXX,XX +XXX,XX @@
 rw quick
 rw backing quick
 rw quick
 -256 rw quick
 +256 rw auto quick
 rw
 rw quick
 rw quick
 --
-.24.1
+.14.3

-[PULL 06/17] iotests: Test 041 only works on certain systems
+[Qemu-devel] [PULL v2 16/23] virtio-blk: make queue size configurable
-From: Thomas Huth <thuth@redhat.com>
+From: Mark Kanda <mark.kanda@oracle.com>
-works fine on Linux, FreeBSD, NetBSD and OpenBSD, but fails on macOS.
+Depending on the configuration, it can be beneficial to adjust the virtio-blk
-Let's mark it as only supported on the systems where we know that it is
+queue size to something other than the current default of 128. Add a new
-working fine.
+property to make the queue size configurable.
-Reviewed-by: Max Reitz <mreitz@redhat.com>
+Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
-Signed-off-by: Thomas Huth <thuth@redhat.com>
+Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com>
-Message-id: 20200121095205.26323-3-thuth@redhat.com
+Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+Reviewed-by: Ameya More <ameya.more@oracle.com>
 Message-id: 52e6d742811f10dbd16e996e86cf375b9577c187.1513005190.git.mark.kanda@oracle.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- tests/qemu-iotests/041 | 3 ++-
+ include/hw/virtio/virtio-blk.h |  1 +
-file changed, 2 insertions(+), 1 deletion(-)
+ hw/block/virtio-blk.c          | 10 +++++++++-
 files changed, 10 insertions(+), 1 deletion(-)
-diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
+diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
-index XXXXXXX..XXXXXXX 100755
+index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/041
+--- a/include/hw/virtio/virtio-blk.h
-+++ b/tests/qemu-iotests/041
++++ b/include/hw/virtio/virtio-blk.h
-@@ -XXX,XX +XXX,XX @@ class TestOrphanedSource(iotests.QMPTestCase):
+@@ -XXX,XX +XXX,XX @@ struct VirtIOBlkConf
+     uint32_t config_wce;
- if __name__ == '__main__':
+     uint32_t request_merging;
-     iotests.main(supported_fmts=['qcow2', 'qed'],
+     uint16_t num_queues;
--                 supported_protocols=['file'])
++    uint16_t queue_size;
-+                 supported_protocols=['file'],
+ };
-+                 supported_platforms=['linux', 'freebsd', 'netbsd', 'openbsd'])
  struct VirtIOBlockDataPlane;
 diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
 index XXXXXXX..XXXXXXX 100644
 --- a/hw/block/virtio-blk.c
 +++ b/hw/block/virtio-blk.c
@@ -XXX,XX +XXX,XX @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
          error_setg(errp, "num-queues property must be larger than 0");
          return;
      }
 +    if (!is_power_of_2(conf->queue_size) ||
 +        conf->queue_size > VIRTQUEUE_MAX_SIZE) {
 +        error_setg(errp, "invalid queue-size property (%" PRIu16 "), "
 +                   "must be a power of 2 (max %d)",
 +                   conf->queue_size, VIRTQUEUE_MAX_SIZE);
 +        return;
 +    }
      blkconf_serial(&conf->conf, &conf->serial);
      if (!blkconf_apply_backend_options(&conf->conf,
@@ -XXX,XX +XXX,XX @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
      s->sector_mask = (s->conf.conf.logical_block_size / BDRV_SECTOR_SIZE) - 1;
      for (i = 0; i < conf->num_queues; i++) {
 -        virtio_add_queue(vdev, 128, virtio_blk_handle_output);
 +        virtio_add_queue(vdev, conf->queue_size, virtio_blk_handle_output);
      }
      virtio_blk_data_plane_create(vdev, conf, &s->dataplane, &err);
      if (err != NULL) {
@@ -XXX,XX +XXX,XX @@ static Property virtio_blk_properties[] = {
      DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0,
                      true),
      DEFINE_PROP_UINT16("num-queues", VirtIOBlock, conf.num_queues, 1),
 +    DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 128),
      DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD,
                       IOThread *),
      DEFINE_PROP_END_OF_LIST(),
 --
-.24.1
+.14.3

-[PULL 07/17] iotests: Test 183 does not work on macOS and OpenBSD
+[Qemu-devel] [PULL v2 17/23] virtio-blk: reject configs with logical block size > physical block size
-From: Thomas Huth <thuth@redhat.com>
+From: Mark Kanda <mark.kanda@oracle.com>
-In the long run, we might want to add test 183 to the "auto" group
+virtio-blk logical block size should never be larger than physical block
-(but it still fails occasionally, so we cannot do that yet). However,
+size because it doesn't make sense to have such configurations. QEMU doesn't
-when running 183 in Cirrus-CI on macOS, or with our vm-build-openbsd
+have a way to effectively express this condition; the best it can do is
-target, it currently always fails with an "Timeout waiting for return
+report the physical block exponent as 0 - indicating the logical block size
-on handle 0" error.
+equals the physical block size.
-Let's mark it as supported only on systems where the test is working
+This is identical to commit 3da023b5827543ee4c022986ea2ad9d1274410b2
-most of the time (i.e. Linux, FreeBSD and NetBSD).
+but applied to virtio-blk (instead of virtio-scsi).
-Reviewed-by: Max Reitz <mreitz@redhat.com>
+Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
-Signed-off-by: Thomas Huth <thuth@redhat.com>
+Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
-Message-id: 20200121095205.26323-4-thuth@redhat.com
+Reviewed-by: Ameya More <ameya.more@oracle.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
 Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
 Message-id: 773169891f9f2deb4cb7c4ef2655580dbe24c1d1.1513005190.git.mark.kanda@oracle.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- tests/qemu-iotests/183 | 1 +
+ hw/block/virtio-blk.c | 7 +++++++
-file changed, 1 insertion(+)
+file changed, 7 insertions(+)
-diff --git a/tests/qemu-iotests/183 b/tests/qemu-iotests/183
+diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
-index XXXXXXX..XXXXXXX 100755
+index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/183
+--- a/hw/block/virtio-blk.c
-+++ b/tests/qemu-iotests/183
++++ b/hw/block/virtio-blk.c
-@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
+@@ -XXX,XX +XXX,XX @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
- . ./common.filter
- . ./common.qemu
+     blkconf_blocksizes(&conf->conf);
-+_supported_os Linux FreeBSD NetBSD
++    if (conf->conf.logical_block_size >
- _supported_fmt qcow2 raw qed quorum
++        conf->conf.physical_block_size) {
- _supported_proto file
++        error_setg(errp,
 +                   "logical_block_size > physical_block_size not supported");
 +        return;
 +    }
 +
      virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
                  sizeof(struct virtio_blk_config));
 --
-.24.1
+.14.3

-[PULL 03/17] block: fix memleaks in bdrv_refresh_filename
+[Qemu-devel] [PULL v2 18/23] block: avoid recursive AioContext acquire in bdrv_inactivate_all()
-From: Pan Nengyuan <pannengyuan@huawei.com>
+From: Paolo Bonzini <pbonzini@redhat.com>
-If we call the qmp 'query-block' while qemu is working on
+BDRV_POLL_WHILE() does not support recursive AioContext locking.  It
-'block-commit', it will cause memleaks, the memory leak stack is as
+only releases the AioContext lock once regardless of how many times the
-follow:
+caller has acquired it.  This results in a hang since the IOThread does
 not make progress while the AioContext is still locked.
-Indirect leak of 12360 byte(s) in 3 object(s) allocated from:
+The following steps trigger the hang:
     #0 0x7f80f0b6d970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
     #1 0x7f80ee86049d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
     #2 0x55ea95b5bb67 in qdict_new /mnt/sdb/qemu-4.2.0-rc0/qobject/qdict.c:29
     #3 0x55ea956cd043 in bdrv_refresh_filename /mnt/sdb/qemu-4.2.0-rc0/block.c:6427
     #4 0x55ea956cc950 in bdrv_refresh_filename /mnt/sdb/qemu-4.2.0-rc0/block.c:6399
     #5 0x55ea956cc950 in bdrv_refresh_filename /mnt/sdb/qemu-4.2.0-rc0/block.c:6399
     #6 0x55ea956cc950 in bdrv_refresh_filename /mnt/sdb/qemu-4.2.0-rc0/block.c:6399
     #7 0x55ea958818ea in bdrv_block_device_info /mnt/sdb/qemu-4.2.0-rc0/block/qapi.c:56
     #8 0x55ea958879de in bdrv_query_info /mnt/sdb/qemu-4.2.0-rc0/block/qapi.c:392
     #9 0x55ea9588b58f in qmp_query_block /mnt/sdb/qemu-4.2.0-rc0/block/qapi.c:578
     #10 0x55ea95567392 in qmp_marshal_query_block qapi/qapi-commands-block-core.c:95
-Indirect leak of 4120 byte(s) in 1 object(s) allocated from:
+  $ qemu-system-x86_64 -M accel=kvm -m 1G -cpu host \
-    #0 0x7f80f0b6d970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
+                       -object iothread,id=iothread0 \
-    #1 0x7f80ee86049d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
+                       -device virtio-scsi-pci,iothread=iothread0 \
-    #2 0x55ea95b5bb67 in qdict_new /mnt/sdb/qemu-4.2.0-rc0/qobject/qdict.c:29
+                       -drive if=none,id=drive0,file=test.img,format=raw \
-    #3 0x55ea956cd043 in bdrv_refresh_filename /mnt/sdb/qemu-4.2.0-rc0/block.c:6427
+                       -device scsi-hd,drive=drive0 \
-    #4 0x55ea956cc950 in bdrv_refresh_filename /mnt/sdb/qemu-4.2.0-rc0/block.c:6399
+                       -drive if=none,id=drive1,file=test.img,format=raw \
-    #5 0x55ea956cc950 in bdrv_refresh_filename /mnt/sdb/qemu-4.2.0-rc0/block.c:6399
+                       -device scsi-hd,drive=drive1
-    #6 0x55ea9569f301 in bdrv_backing_attach /mnt/sdb/qemu-4.2.0-rc0/block.c:1064
+  $ qemu-system-x86_64 ...same options... \
-    #7 0x55ea956a99dd in bdrv_replace_child_noperm /mnt/sdb/qemu-4.2.0-rc0/block.c:2283
+                       -incoming tcp::1234
-    #8 0x55ea956b9b53 in bdrv_replace_node /mnt/sdb/qemu-4.2.0-rc0/block.c:4196
+  (qemu) migrate tcp:127.0.0.1:1234
-    #9 0x55ea956b9e49 in bdrv_append /mnt/sdb/qemu-4.2.0-rc0/block.c:4236
+  ...hang...
     #10 0x55ea958c3472 in commit_start /mnt/sdb/qemu-4.2.0-rc0/block/commit.c:306
     #11 0x55ea94b68ab0 in qmp_block_commit /mnt/sdb/qemu-4.2.0-rc0/blockdev.c:3459
     #12 0x55ea9556a7a7 in qmp_marshal_block_commit qapi/qapi-commands-block-core.c:407
-Fixes: bb808d5f5c0978828a974d547e6032402c339555
+Tested-by: Stefan Hajnoczi <stefanha@redhat.com>
-Reported-by: Euler Robot <euler.robot@huawei.com>
+Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
-Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
+Reviewed-by: Eric Blake <eblake@redhat.com>
-Message-id: 20200116085600.24056-1-pannengyuan@huawei.com
+Message-id: 20171207201320.19284-2-stefanha@redhat.com
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block.c | 1 +
+ block.c | 14 +++++++++++---
-file changed, 1 insertion(+)
+file changed, 11 insertions(+), 3 deletions(-)
 diff --git a/block.c b/block.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block.c
 +++ b/block.c
-@@ -XXX,XX +XXX,XX @@ void bdrv_refresh_filename(BlockDriverState *bs)
+@@ -XXX,XX +XXX,XX @@ int bdrv_inactivate_all(void)
-                 child->bs->exact_filename);
+     BdrvNextIterator it;
-         pstrcpy(bs->filename, sizeof(bs->filename), child->bs->filename);
+     int ret = 0;
+     int pass;
-+        qobject_unref(bs->full_open_options);
++    GSList *aio_ctxs = NULL, *ctx;
-         bs->full_open_options = qobject_ref(child->bs->full_open_options);
+     for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
-         return;
+-        aio_context_acquire(bdrv_get_aio_context(bs));
 +        AioContext *aio_context = bdrv_get_aio_context(bs);
 +
 +        if (!g_slist_find(aio_ctxs, aio_context)) {
 +            aio_ctxs = g_slist_prepend(aio_ctxs, aio_context);
 +            aio_context_acquire(aio_context);
 +        }
      }
      /* We do two passes of inactivation. The first pass calls to drivers'
@@ -XXX,XX +XXX,XX @@ int bdrv_inactivate_all(void)
      }
  out:
 -    for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
 -        aio_context_release(bdrv_get_aio_context(bs));
 +    for (ctx = aio_ctxs; ctx != NULL; ctx = ctx->next) {
 +        AioContext *aio_context = ctx->data;
 +        aio_context_release(aio_context);
      }
 +    g_slist_free(aio_ctxs);
      return ret;
  }
 --
-.24.1
+.14.3

-[PULL 01/17] qcow2: Assert that host cluster offsets fit in L2 table entries
+[Qemu-devel] [PULL v2 19/23] docs: mark nested AioContext locking as a legacy API
-From: Alberto Garcia <berto@igalia.com>
+See the patch for why nested AioContext locking is no longer allowed.
-The standard cluster descriptor in L2 table entries has a field to
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-store the host cluster offset. When we need to get that offset from an
+Reviewed-by: Eric Blake <eblake@redhat.com>
-entry we use L2E_OFFSET_MASK to ensure that we only use the bits that
+Message-id: 20171207201320.19284-3-stefanha@redhat.com
-belong to that field.
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
  docs/devel/multiple-iothreads.txt | 7 ++++---
 file changed, 4 insertions(+), 3 deletions(-)
-But while that mask is used every time we read from an L2 entry, it
+diff --git a/docs/devel/multiple-iothreads.txt b/docs/devel/multiple-iothreads.txt
 is never used when we write to it. Due to the QCOW_MAX_CLUSTER_OFFSET
 limit set in the cluster allocation code QEMU can never produce
 offsets that don't fit in that field so any such offset would indicate
 a bug in QEMU.
 Compressed cluster descriptors contain two fields (host cluster offset
 and size of the compressed data) and the situation with them is
 similar. In this case the masks are not constant but are stored in the
 csize_mask and cluster_offset_mask fields of BDRVQcow2State.
 Signed-off-by: Alberto Garcia <berto@igalia.com>
 Reviewed-by: Eric Blake <eblake@redhat.com>
 Message-id: 20200113161146.20099-1-berto@igalia.com
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  block/qcow2-cluster.c | 14 ++++++++++++--
 file changed, 12 insertions(+), 2 deletions(-)
 diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2-cluster.c
+--- a/docs/devel/multiple-iothreads.txt
-+++ b/block/qcow2-cluster.c
++++ b/docs/devel/multiple-iothreads.txt
-@@ -XXX,XX +XXX,XX @@ int qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
+@@ -XXX,XX +XXX,XX @@
-         (cluster_offset + compressed_size - 1) / QCOW2_COMPRESSED_SECTOR_SIZE -
+-Copyright (c) 2014 Red Hat Inc.
-         (cluster_offset / QCOW2_COMPRESSED_SECTOR_SIZE);
++Copyright (c) 2014-2017 Red Hat Inc.
-+    /* The offset and size must fit in their fields of the L2 table entry */
+ This work is licensed under the terms of the GNU GPL, version 2 or later.  See
-+    assert((cluster_offset & s->cluster_offset_mask) == cluster_offset);
+ the COPYING file in the top-level directory.
-+    assert((nb_csectors & s->csize_mask) == nb_csectors);
+@@ -XXX,XX +XXX,XX @@ aio_context_acquire()/aio_context_release() for mutual exclusion.  Once the
-+
+ context is acquired no other thread can access it or run event loop iterations
-     cluster_offset |= QCOW_OFLAG_COMPRESSED |
+ in this AioContext.
-                       ((uint64_t)nb_csectors << s->csize_shift);
+-aio_context_acquire()/aio_context_release() calls may be nested.  This
-@@ -XXX,XX +XXX,XX @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
+-means you can call them if you're not sure whether #2 applies.
++Legacy code sometimes nests aio_context_acquire()/aio_context_release() calls.
-     assert(l2_index + m->nb_clusters <= s->l2_slice_size);
++Do not use nesting anymore, it is incompatible with the BDRV_POLL_WHILE() macro
-     for (i = 0; i < m->nb_clusters; i++) {
++used in the block layer and can lead to hangs.
-+        uint64_t offset = cluster_offset + (i << s->cluster_bits);
-         /* if two concurrent writes happen to the same unallocated cluster
+ There is currently no lock ordering rule if a thread needs to acquire multiple
-          * each write allocates separate cluster and writes data concurrently.
+ AioContexts simultaneously.  Therefore, it is only safe for code holding the
           * The first one to complete updates l2 table with pointer to its
@@ -XXX,XX +XXX,XX @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
              old_cluster[j++] = l2_slice[l2_index + i];
          }
 -        l2_slice[l2_index + i] = cpu_to_be64((cluster_offset +
 -                    (i << s->cluster_bits)) | QCOW_OFLAG_COPIED);
 +        /* The offset must fit in the offset field of the L2 table entry */
 +        assert((offset & L2E_OFFSET_MASK) == offset);
 +
 +        l2_slice[l2_index + i] = cpu_to_be64(offset | QCOW_OFLAG_COPIED);
       }
@@ -XXX,XX +XXX,XX @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
                          goto fail;
                      }
 +                    /* The offset must fit in the offset field */
 +                    assert((offset & L2E_OFFSET_MASK) == offset);
 +
                      if (l2_refcount > 1) {
                          /* For shared L2 tables, set the refcount accordingly
                           * (it is already 1 and needs to be l2_refcount) */
 --
-.24.1
+.14.3

-[PULL 16/17] block/backup-top: fix failure path
+[Qemu-devel] [PULL v2 20/23] blockdev: add x-blockdev-set-iothread force boolean
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+When a node is already associated with a BlockBackend the
 x-blockdev-set-iothread command refuses to set the IOThread.  This is to
 prevent accidentally changing the IOThread when the nodes are in use.
-We can't access top after call bdrv_backup_top_drop, as it is already
+When the nodes are created with -drive they automatically get a
-freed at this time.
+BlockBackend.  In that case we know nothing is using them yet and it's
 safe to set the IOThread.  Add a force boolean to override the check.
-Also, no needs to unref target child by hand, it will be unrefed on
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-bdrv_close() automatically.
+Reviewed-by: Eric Blake <eblake@redhat.com>
 Message-id: 20171207201320.19284-4-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
  qapi/block-core.json |  6 +++++-
  blockdev.c           | 11 ++++++-----
 files changed, 11 insertions(+), 6 deletions(-)
-So, just do bdrv_backup_top_drop if append succeed and one bdrv_unref
+diff --git a/qapi/block-core.json b/qapi/block-core.json
 otherwise.
 Note, that in !appended case bdrv_unref(top) moved into drained section
 on source. It doesn't really matter, but just for code simplicity.
 Fixes: 7df7868b96404
 Cc: qemu-stable@nongnu.org # v4.2.0
 Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
 Reviewed-by: Max Reitz <mreitz@redhat.com>
 Message-id: 20200121142802.21467-2-vsementsov@virtuozzo.com
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  block/backup-top.c | 21 ++++++++++++---------
 file changed, 12 insertions(+), 9 deletions(-)
 diff --git a/block/backup-top.c b/block/backup-top.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/backup-top.c
+--- a/qapi/block-core.json
-+++ b/block/backup-top.c
++++ b/qapi/block-core.json
-@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
+@@ -XXX,XX +XXX,XX @@
-     BlockDriverState *top = bdrv_new_open_driver(&bdrv_backup_top_filter,
+ #
-                                                  filter_node_name,
+ # @iothread: the name of the IOThread object or null for the main loop
-                                                  BDRV_O_RDWR, errp);
+ #
-+    bool appended = false;
++# @force: true if the node and its children should be moved when a BlockBackend
++#         is already attached
-     if (!top) {
++#
-         return NULL;
+ # Note: this command is experimental and intended for test cases that need
-@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
+ # control over IOThreads only.
-     bdrv_append(top, source, &local_err);
+ #
-     if (local_err) {
+@@ -XXX,XX +XXX,XX @@
-         error_prepend(&local_err, "Cannot append backup-top filter: ");
+ ##
--        goto append_failed;
+ { 'command': 'x-blockdev-set-iothread',
-+        goto fail;
+   'data' : { 'node-name': 'str',
 -             'iothread': 'StrOrNull' } }
 +             'iothread': 'StrOrNull',
 +             '*force': 'bool' } }
 diff --git a/blockdev.c b/blockdev.c
 index XXXXXXX..XXXXXXX 100644
 --- a/blockdev.c
 +++ b/blockdev.c
@@ -XXX,XX +XXX,XX @@ BlockJobInfoList *qmp_query_block_jobs(Error **errp)
  }
  void qmp_x_blockdev_set_iothread(const char *node_name, StrOrNull *iothread,
 -                                 Error **errp)
 +                                 bool has_force, bool force, Error **errp)
  {
      AioContext *old_context;
      AioContext *new_context;
@@ -XXX,XX +XXX,XX @@ void qmp_x_blockdev_set_iothread(const char *node_name, StrOrNull *iothread,
          return;
      }
-+    appended = true;
+-    /* If we want to allow more extreme test scenarios this guard could be
-     /*
+-     * removed.  For now it protects against accidents. */
-      * bdrv_append() finished successfully, now we can require permissions
+-    if (bdrv_has_blk(bs)) {
-@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
+-        error_setg(errp, "Node %s is in use", node_name);
-     if (local_err) {
++    /* Protects against accidents. */
-         error_prepend(&local_err,
++    if (!(has_force && force) && bdrv_has_blk(bs)) {
-                       "Cannot set permissions for backup-top filter: ");
++        error_setg(errp, "Node %s is associated with a BlockBackend and could "
--        goto failed_after_append;
++                         "be in use (use force=true to override this check)",
-+        goto fail;
++                         node_name);
          return;
      }
-     state->bcs = block_copy_state_new(top->backing, state->target,
-                                       cluster_size, write_flags, &local_err);
-     if (local_err) {
-         error_prepend(&local_err, "Cannot create block-copy-state: ");
--        goto failed_after_append;
-+        goto fail;
-     }
-     *bcs = state->bcs;
-@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
-     return top;
--failed_after_append:
--    state->active = false;
--    bdrv_backup_top_drop(top);
-+fail:
-+    if (appended) {
-+        state->active = false;
-+        bdrv_backup_top_drop(top);
-+    } else {
-+        bdrv_unref(top);
-+    }
--append_failed:
-     bdrv_drained_end(source);
--    bdrv_unref_child(top, state->target);
--    bdrv_unref(top);
-     error_propagate(errp, local_err);
-     return NULL;
 --
-.24.1
+.14.3

-[PULL 05/17] iotests: remove 'linux' from default supported platforms
+[Qemu-devel] [PULL v2 21/23] iotests: add VM.add_object()
-From: John Snow <jsnow@redhat.com>
+The VM.add_object() method can be used to add IOThreads or memory
 backend objects.
-verify_platform will check an explicit whitelist and blacklist instead.
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-The default will now be assumed to be allowed to run anywhere.
+Reviewed-by: Eric Blake <eblake@redhat.com>
+Message-id: 20171207201320.19284-5-stefanha@redhat.com
-For tests that do not specify their platforms explicitly, this has the effect of
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 enabling these tests on non-linux platforms. For tests that always specified
 linux explicitly, there is no change.
 For Python tests on FreeBSD at least; only seven python tests fail:
 147 149 169 194 199 211
 and 149 appear to be misconfigurations,
 and 194 are the AF_UNIX path too long error,
 and 199 are bitmap migration bugs, and
 is a bug that shows up on Linux platforms, too.
 This is at least good evidence that these tests are not Linux-only. If
 they aren't suitable for other platforms, they should be disabled on a
 per-platform basis as appropriate.
 Therefore, let's switch these on and deal with the failures.
 Reviewed-by: Max Reitz <mreitz@redhat.com>
 Signed-off-by: John Snow <jsnow@redhat.com>
 Message-id: 20200121095205.26323-2-thuth@redhat.com
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
- tests/qemu-iotests/iotests.py | 16 +++++++++++-----
+ tests/qemu-iotests/iotests.py | 5 +++++
-file changed, 11 insertions(+), 5 deletions(-)
+file changed, 5 insertions(+)
 diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
 index XXXXXXX..XXXXXXX 100644
 --- a/tests/qemu-iotests/iotests.py
 +++ b/tests/qemu-iotests/iotests.py
-@@ -XXX,XX +XXX,XX @@ def verify_protocol(supported=[], unsupported=[]):
+@@ -XXX,XX +XXX,XX @@ class VM(qtest.QEMUQtestMachine):
-     if not_sup or (imgproto in unsupported):
+                                  socket_scm_helper=socket_scm_helper)
-         notrun('not suitable for this protocol: %s' % imgproto)
+         self._num_drives = 0
--def verify_platform(supported_oses=['linux']):
++    def add_object(self, opts):
--    if True not in [sys.platform.startswith(x) for x in supported_oses]:
++        self._args.append('-object')
--        notrun('not suitable for this OS: %s' % sys.platform)
++        self._args.append(opts)
-+def verify_platform(supported=None, unsupported=None):
++        return self
 +    if unsupported is not None:
 +        if any((sys.platform.startswith(x) for x in unsupported)):
 +            notrun('not suitable for this OS: %s' % sys.platform)
 +
-+    if supported is not None:
+     def add_device(self, opts):
-+        if not any((sys.platform.startswith(x) for x in supported)):
+         self._args.append('-device')
-+            notrun('not suitable for this OS: %s' % sys.platform)
+         self._args.append(opts)
  def verify_cache_mode(supported_cache_modes=[]):
      if supported_cache_modes and (cachemode not in supported_cache_modes):
@@ -XXX,XX +XXX,XX @@ def execute_unittest(output, verbosity, debug):
              sys.stderr.write(out)
  def execute_test(test_function=None,
 -                 supported_fmts=[], supported_oses=['linux'],
 +                 supported_fmts=[],
 +                 supported_platforms=None,
                   supported_cache_modes=[], supported_aio_modes={},
                   unsupported_fmts=[], supported_protocols=[],
                   unsupported_protocols=[]):
@@ -XXX,XX +XXX,XX @@ def execute_test(test_function=None,
      verbosity = 1
      verify_image_format(supported_fmts, unsupported_fmts)
      verify_protocol(supported_protocols, unsupported_protocols)
 -    verify_platform(supported_oses)
 +    verify_platform(supported=supported_platforms)
      verify_cache_mode(supported_cache_modes)
      verify_aio_mode(supported_aio_modes)
 --
-.24.1
+.14.3

-[PULL 08/17] iotests: Check for the availability of the required devices in 267 and 127
+[Qemu-devel] [PULL v2 22/23] iothread: fix iothread_stop() race condition
-From: Thomas Huth <thuth@redhat.com>
+There is a small chance that iothread_stop() hangs as follows:
-We are going to enable 127 in the "auto" group, but it only works if
+  Thread 3 (Thread 0x7f63eba5f700 (LWP 16105)):
-virtio-scsi and scsi-hd are available - which is not the case with
+  #0  0x00007f64012c09b6 in ppoll () at /lib64/libc.so.6
-QEMU binaries like qemu-system-tricore for example, so we need a
+  #1  0x000055959992eac9 in ppoll (__ss=0x0, __timeout=0x0, __nfds=<optimized out>, __fds=<optimized out>) at /usr/include/bits/poll2.h:77
-proper check for the availability of these devices here.
+  #2  0x000055959992eac9 in qemu_poll_ns (fds=<optimized out>, nfds=<optimized out>, timeout=<optimized out>) at util/qemu-timer.c:322
   #3  0x0000559599930711 in aio_poll (ctx=0x55959bdb83c0, blocking=blocking@entry=true) at util/aio-posix.c:629
   #4  0x00005595996806fe in iothread_run (opaque=0x55959bd78400) at iothread.c:59
   #5  0x00007f640159f609 in start_thread () at /lib64/libpthread.so.0
   #6  0x00007f64012cce6f in clone () at /lib64/libc.so.6
-A very similar problem exists in iotest 267 - it has been added to
+  Thread 1 (Thread 0x7f640b45b280 (LWP 16103)):
-the "auto" group already, but requires virtio-blk and thus currently
+  #0  0x00007f64015a0b6d in pthread_join () at /lib64/libpthread.so.0
-fails with qemu-system-tricore for example. Let's also add aproper
+  #1  0x00005595999332ef in qemu_thread_join (thread=<optimized out>) at util/qemu-thread-posix.c:547
-check there.
+  #2  0x00005595996808ae in iothread_stop (iothread=<optimized out>) at iothread.c:91
   #3  0x000055959968094d in iothread_stop_iter (object=<optimized out>, opaque=<optimized out>) at iothread.c:102
   #4  0x0000559599857d97 in do_object_child_foreach (obj=obj@entry=0x55959bdb8100, fn=fn@entry=0x559599680930 <iothread_stop_iter>, opaque=opaque@entry=0x0, recurse=recurse@entry=false) at qom/object.c:852
   #5  0x0000559599859477 in object_child_foreach (obj=obj@entry=0x55959bdb8100, fn=fn@entry=0x559599680930 <iothread_stop_iter>, opaque=opaque@entry=0x0) at qom/object.c:867
   #6  0x0000559599680a6e in iothread_stop_all () at iothread.c:341
   #7  0x000055959955b1d5 in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at vl.c:4913
-Reviewed-by: Max Reitz <mreitz@redhat.com>
+The relevant code from iothread_run() is:
-Signed-off-by: Thomas Huth <thuth@redhat.com>
-Message-id: 20200121095205.26323-5-thuth@redhat.com
+  while (!atomic_read(&iothread->stopping)) {
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+      aio_poll(iothread->ctx, true);
 and iothread_stop():
   iothread->stopping = true;
   aio_notify(iothread->ctx);
   ...
   qemu_thread_join(&iothread->thread);
 The following scenario can occur:
 . IOThread:
   while (!atomic_read(&iothread->stopping)) -> stopping=false
 . Main loop:
   iothread->stopping = true;
   aio_notify(iothread->ctx);
 . IOThread:
   aio_poll(iothread->ctx, true); -> hang
 The bug is explained by the AioContext->notify_me doc comments:
   "If this field is 0, everything (file descriptors, bottom halves,
   timers) will be re-evaluated before the next blocking poll(), thus the
   event_notifier_set call can be skipped."
 The problem is that "everything" does not include checking
 iothread->stopping.  This means iothread_run() will block in aio_poll()
 if aio_notify() was called just before aio_poll().
 This patch fixes the hang by replacing aio_notify() with
 aio_bh_schedule_oneshot().  This makes aio_poll() or g_main_loop_run()
 to return.
 Implementing this properly required a new bool running flag.  The new
 flag prevents races that are tricky if we try to use iothread->stopping.
 Now iothread->stopping is purely for iothread_stop() and
 iothread->running is purely for the iothread_run() thread.
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 Reviewed-by: Eric Blake <eblake@redhat.com>
 Message-id: 20171207201320.19284-6-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- tests/qemu-iotests/127       |  2 ++
+ include/sysemu/iothread.h |  3 ++-
- tests/qemu-iotests/267       |  2 ++
+ iothread.c                | 20 +++++++++++++++-----
- tests/qemu-iotests/common.rc | 14 ++++++++++++++
+files changed, 17 insertions(+), 6 deletions(-)
 files changed, 18 insertions(+)
-diff --git a/tests/qemu-iotests/127 b/tests/qemu-iotests/127
+diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
-index XXXXXXX..XXXXXXX 100755
+index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/127
+--- a/include/sysemu/iothread.h
-+++ b/tests/qemu-iotests/127
++++ b/include/sysemu/iothread.h
-@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
+@@ -XXX,XX +XXX,XX @@ typedef struct {
- _supported_fmt qcow2
+     GOnce once;
- _supported_proto file
+     QemuMutex init_done_lock;
+     QemuCond init_done_cond;    /* is thread initialization done? */
-+_require_devices virtio-scsi scsi-hd
+-    bool stopping;
 +    bool stopping;              /* has iothread_stop() been called? */
 +    bool running;               /* should iothread_run() continue? */
      int thread_id;
      /* AioContext poll parameters */
 diff --git a/iothread.c b/iothread.c
 index XXXXXXX..XXXXXXX 100644
 --- a/iothread.c
 +++ b/iothread.c
@@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque)
      qemu_cond_signal(&iothread->init_done_cond);
      qemu_mutex_unlock(&iothread->init_done_lock);
 -    while (!atomic_read(&iothread->stopping)) {
 +    while (iothread->running) {
          aio_poll(iothread->ctx, true);
          if (atomic_read(&iothread->worker_context)) {
@@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque)
      return NULL;
  }
 +/* Runs in iothread_run() thread */
 +static void iothread_stop_bh(void *opaque)
 +{
 +    IOThread *iothread = opaque;
 +
- IMG_SIZE=64K
++    iothread->running = false; /* stop iothread_run() */
  _make_test_img $IMG_SIZE
 diff --git a/tests/qemu-iotests/267 b/tests/qemu-iotests/267
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/267
 +++ b/tests/qemu-iotests/267
@@ -XXX,XX +XXX,XX @@ _require_drivers copy-on-read
  # and generally impossible with external data files
  _unsupported_imgopts 'refcount_bits=1[^0-9]' data_file
 +_require_devices virtio-blk
 +
- do_run_qemu()
++    if (iothread->main_loop) {
- {
++        g_main_loop_quit(iothread->main_loop);
-     echo Testing: "$@"
++    }
 diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
 index XXXXXXX..XXXXXXX 100644
 --- a/tests/qemu-iotests/common.rc
 +++ b/tests/qemu-iotests/common.rc
@@ -XXX,XX +XXX,XX @@ _require_large_file()
      rm "$TEST_IMG"
  }
 +# Check that a set of devices is available in the QEMU binary
 +#
 +_require_devices()
 +{
 +    available=$($QEMU -M none -device help | \
 +                grep ^name | sed -e 's/^name "//' -e 's/".*$//')
 +    for device
 +    do
 +        if ! echo "$available" | grep -q "$device" ; then
 +            _notrun "$device not available"
 +        fi
 +    done
 +}
 +
- # make sure this script returns success
+ void iothread_stop(IOThread *iothread)
- true
+ {
      if (!iothread->ctx || iothread->stopping) {
          return;
      }
      iothread->stopping = true;
 -    aio_notify(iothread->ctx);
 -    if (atomic_read(&iothread->main_loop)) {
 -        g_main_loop_quit(iothread->main_loop);
 -    }
 +    aio_bh_schedule_oneshot(iothread->ctx, iothread_stop_bh, iothread);
      qemu_thread_join(&iothread->thread);
  }
@@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp)
      char *name, *thread_name;
      iothread->stopping = false;
 +    iothread->running = true;
      iothread->thread_id = -1;
      iothread->ctx = aio_context_new(&local_error);
      if (!iothread->ctx) {
 --
-.24.1
+.14.3

-[PULL 17/17] iotests: add test for backup-top failure on permission activation
+[Qemu-devel] [PULL v2 23/23] qemu-iotests: add 203 savevm with IOThreads test
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+This test case will prevent future regressions with savevm and
 IOThreads.
-This test checks that bug is really fixed by previous commit.
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 Reviewed-by: Eric Blake <eblake@redhat.com>
 Message-id: 20171207201320.19284-7-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
  tests/qemu-iotests/203     | 59 ++++++++++++++++++++++++++++++++++++++++++++++
  tests/qemu-iotests/203.out |  6 +++++
  tests/qemu-iotests/group   |  1 +
 files changed, 66 insertions(+)
  create mode 100755 tests/qemu-iotests/203
  create mode 100644 tests/qemu-iotests/203.out
-Cc: qemu-stable@nongnu.org # v4.2.0
+diff --git a/tests/qemu-iotests/203 b/tests/qemu-iotests/203
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
+new file mode 100755
 Message-id: 20200121142802.21467-3-vsementsov@virtuozzo.com
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  tests/qemu-iotests/283     | 92 ++++++++++++++++++++++++++++++++++++++
  tests/qemu-iotests/283.out |  8 ++++
  tests/qemu-iotests/group   |  1 +
 files changed, 101 insertions(+)
  create mode 100644 tests/qemu-iotests/283
  create mode 100644 tests/qemu-iotests/283.out
 diff --git a/tests/qemu-iotests/283 b/tests/qemu-iotests/283
 new file mode 100644
 index XXXXXXX..XXXXXXX
 --- /dev/null
-+++ b/tests/qemu-iotests/283
++++ b/tests/qemu-iotests/203
 @@ -XXX,XX +XXX,XX @@
 +#!/usr/bin/env python
 +#
-+# Test for backup-top filter permission activation failure
++# Copyright (C) 2017 Red Hat, Inc.
 +#
 +# Copyright (c) 2019 Virtuozzo International GmbH.
 +#
 +# 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.
 ...
 +# 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: Stefan Hajnoczi <stefanha@redhat.com>
++#
++# Check that QMP 'migrate' with multiple drives on a single IOThread completes
++# successfully.  This particular command triggered a hang in the source QEMU
++# process due to recursive AioContext locking in bdrv_invalidate_all() and
++# BDRV_POLL_WHILE().
 +
 +import iotests
 +
-+# The test is unrelated to formats, restrict it to qcow2 to avoid extra runs
 +iotests.verify_image_format(supported_fmts=['qcow2'])
++iotests.verify_platform(['linux'])
 +
-+size = 1024 * 1024
++with iotests.FilePath('disk0.img') as disk0_img_path, \
 +     iotests.FilePath('disk1.img') as disk1_img_path, \
 +     iotests.VM() as vm:
 +
-+""" Test description
++    img_size = '10M'
 +    iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, disk0_img_path, img_size)
 +    iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, disk1_img_path, img_size)
 +
-+When performing a backup, all writes on the source subtree must go through the
++    iotests.log('Launching VM...')
-+backup-top filter so it can copy all data to the target before it is changed.
++    (vm.add_object('iothread,id=iothread0')
-+backup-top filter is appended above source node, to achieve this thing, so all
++       .add_drive(disk0_img_path, 'node-name=drive0-node', interface='none')
-+parents of source node are handled. A configuration with side parents of source
++       .add_drive(disk1_img_path, 'node-name=drive1-node', interface='none')
-+sub-tree with write permission is unsupported (we'd have append several
++       .launch())
 +backup-top filter like nodes to handle such parents). The test create an
 +example of such configuration and checks that a backup is then not allowed
 +(blockdev-backup command should fail).
 +
-+The configuration:
++    iotests.log('Setting IOThreads...')
 +    iotests.log(vm.qmp('x-blockdev-set-iothread',
 +                       node_name='drive0-node', iothread='iothread0',
 +                       force=True))
 +    iotests.log(vm.qmp('x-blockdev-set-iothread',
 +                       node_name='drive1-node', iothread='iothread0',
 +                       force=True))
 +
-+    ┌────────┐  target  ┌─────────────┐
++    iotests.log('Starting migration...')
-+    │ target │ ◀─────── │ backup_top  │
++    iotests.log(vm.qmp('migrate', uri='exec:cat >/dev/null'))
-+    └────────┘          └─────────────┘
++    while True:
-+                            │
++        vm.get_qmp_event(wait=60.0)
-+                            │ backing
++        result = vm.qmp('query-migrate')
-+                            ▼
++        status = result.get('return', {}).get('status', None)
-+                        ┌─────────────┐
++        if status == 'completed':
-+                        │   source    │
++            break
-+                        └─────────────┘
+diff --git a/tests/qemu-iotests/203.out b/tests/qemu-iotests/203.out
 +                            │
 +                            │ file
 +                            ▼
 +                        ┌─────────────┐  write perm   ┌───────┐
 +                        │    base     │ ◀──────────── │ other │
 +                        └─────────────┘               └───────┘
 +
 +On activation (see .active field of backup-top state in block/backup-top.c),
 +backup-top is going to unshare write permission on its source child. Write
 +unsharing will be propagated to the "source->base" link and will conflict with
 +other node write permission. So permission update will fail and backup job will
 +not be started.
 +
 +Note, that the only thing which prevents backup of running on such
 +configuration is default permission propagation scheme. It may be altered by
 +different block drivers, so backup will run in invalid configuration. But
 +something is better than nothing. Also, before the previous commit (commit
 +preceding this test creation), starting backup on such configuration led to
 +crash, so current "something" is a lot better, and this test actual goal is
 +to check that crash is fixed :)
 +"""
 +
 +vm = iotests.VM()
 +vm.launch()
 +
 +vm.qmp_log('blockdev-add', **{'node-name': 'target', 'driver': 'null-co'})
 +
 +vm.qmp_log('blockdev-add', **{
 +    'node-name': 'source',
 +    'driver': 'blkdebug',
 +    'image': {'node-name': 'base', 'driver': 'null-co', 'size': size}
 +})
 +
 +vm.qmp_log('blockdev-add', **{
 +    'node-name': 'other',
 +    'driver': 'blkdebug',
 +    'image': 'base',
 +    'take-child-perms': ['write']
 +})
 +
 +vm.qmp_log('blockdev-backup', sync='full', device='source', target='target')
 +
 +vm.shutdown()
 diff --git a/tests/qemu-iotests/283.out b/tests/qemu-iotests/283.out
 new file mode 100644
 index XXXXXXX..XXXXXXX
 --- /dev/null
-+++ b/tests/qemu-iotests/283.out
++++ b/tests/qemu-iotests/203.out
 @@ -XXX,XX +XXX,XX @@
-+{"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "target"}}
++Launching VM...
-+{"return": {}}
++Setting IOThreads...
-+{"execute": "blockdev-add", "arguments": {"driver": "blkdebug", "image": {"driver": "null-co", "node-name": "base", "size": 1048576}, "node-name": "source"}}
++{u'return': {}}
-+{"return": {}}
++{u'return': {}}
-+{"execute": "blockdev-add", "arguments": {"driver": "blkdebug", "image": "base", "node-name": "other", "take-child-perms": ["write"]}}
++Starting migration...
-+{"return": {}}
++{u'return': {}}
 +{"execute": "blockdev-backup", "arguments": {"device": "source", "sync": "full", "target": "target"}}
 +{"error": {"class": "GenericError", "desc": "Cannot set permissions for backup-top filter: Conflicts with use by other as 'image', which uses 'write' on base"}}
 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 backing quick
+rw auto
-rw migration quick
+rw auto
-rw quick
+rw auto quick
-+283 auto quick
++203 rw auto
 --
-.24.1
+.14.3

The following changes since commit 418fa86dd465b4fd8394373cf83db8fa65d7611c:

Merge remote-tracking branch 'remotes/stsquad/tags/pull-testing-040220-1' into staging (2020-02-04 18:55:06 +0000)

are available in the Git repository at:

https://github.com/XanClic/qemu.git tags/pull-block-2020-02-06

for you to fetch changes up to a541fcc27c98b96da187c7d4573f3270f3ddd283:

iotests: add test for backup-top failure on permission activation (2020-02-06 13:47:45 +0100)

----------------------------------------------------------------
Block patches:
- Drop BDRV_SECTOR_SIZE from qcow2
- Allow Python iotests to be added to the auto group
  (and add some)
- Fix for the backup job
- Fix memleak in bdrv_refresh_filename()
- Use GStrings in two places for greater efficiency (than manually
  handling string allocation)

----------------------------------------------------------------
Alberto Garcia (8):
  qcow2: Assert that host cluster offsets fit in L2 table entries
  block: Use a GString in bdrv_perm_names()
  qcow2: Use a GString in report_unsupported_feature()
  qcow2: Don't round the L1 table allocation up to the sector size
  qcow2: Tighten cluster_offset alignment assertions
  qcow2: Use bs->bl.request_alignment when updating an L1 entry
  qcow2: Don't require aligned offsets in qcow2_co_copy_range_from()
  qcow2: Use BDRV_SECTOR_SIZE instead of the hardcoded value

John Snow (1):
  iotests: remove 'linux' from default supported platforms

Pan Nengyuan (1):
  block: fix memleaks in bdrv_refresh_filename

Thomas Huth (5):
  iotests: Test 041 only works on certain systems
  iotests: Test 183 does not work on macOS and OpenBSD
  iotests: Check for the availability of the required devices in 267 and
    127
  iotests: Skip Python-based tests if QEMU does not support virtio-blk
  iotests: Enable more tests in the 'auto' group to improve test
    coverage

Vladimir Sementsov-Ogievskiy (2):
  block/backup-top: fix failure path
  iotests: add test for backup-top failure on permission activation

-- 
2.24.1

From: Alberto Garcia <berto@igalia.com>

The standard cluster descriptor in L2 table entries has a field to
store the host cluster offset. When we need to get that offset from an
entry we use L2E_OFFSET_MASK to ensure that we only use the bits that
belong to that field.

But while that mask is used every time we read from an L2 entry, it
is never used when we write to it. Due to the QCOW_MAX_CLUSTER_OFFSET
limit set in the cluster allocation code QEMU can never produce
offsets that don't fit in that field so any such offset would indicate
a bug in QEMU.

Compressed cluster descriptors contain two fields (host cluster offset
and size of the compressed data) and the situation with them is
similar. In this case the masks are not constant but are stored in the
csize_mask and cluster_offset_mask fields of BDRVQcow2State.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20200113161146.20099-1-berto@igalia.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2-cluster.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -XXX,XX +XXX,XX @@ int qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
         (cluster_offset + compressed_size - 1) / QCOW2_COMPRESSED_SECTOR_SIZE -
         (cluster_offset / QCOW2_COMPRESSED_SECTOR_SIZE);
 
+    /* The offset and size must fit in their fields of the L2 table entry */
+    assert((cluster_offset & s->cluster_offset_mask) == cluster_offset);
+    assert((nb_csectors & s->csize_mask) == nb_csectors);
+
     cluster_offset |= QCOW_OFLAG_COMPRESSED |
                       ((uint64_t)nb_csectors << s->csize_shift);
 
@@ -XXX,XX +XXX,XX @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
 
     assert(l2_index + m->nb_clusters <= s->l2_slice_size);
     for (i = 0; i < m->nb_clusters; i++) {
+        uint64_t offset = cluster_offset + (i << s->cluster_bits);
         /* if two concurrent writes happen to the same unallocated cluster
          * each write allocates separate cluster and writes data concurrently.
          * The first one to complete updates l2 table with pointer to its
@@ -XXX,XX +XXX,XX @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
             old_cluster[j++] = l2_slice[l2_index + i];
         }
 
-        l2_slice[l2_index + i] = cpu_to_be64((cluster_offset +
-                    (i << s->cluster_bits)) | QCOW_OFLAG_COPIED);
+        /* The offset must fit in the offset field of the L2 table entry */
+        assert((offset & L2E_OFFSET_MASK) == offset);
+
+        l2_slice[l2_index + i] = cpu_to_be64(offset | QCOW_OFLAG_COPIED);
      }
 
 
@@ -XXX,XX +XXX,XX @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
                         goto fail;
                     }
 
+                    /* The offset must fit in the offset field */
+                    assert((offset & L2E_OFFSET_MASK) == offset);
+
                     if (l2_refcount > 1) {
                         /* For shared L2 tables, set the refcount accordingly
                          * (it is already 1 and needs to be l2_refcount) */
-- 
2.24.1

From: Alberto Garcia <berto@igalia.com>

This is a bit more efficient than having to allocate and free memory
for each new permission.

The default size (30) is enough for "consistent read, write, resize".

Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-id: 20200110171518.22168-1-berto@igalia.com
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/block.c b/block.c
index XXXXXXX..XXXXXXX 100644
--- a/block.c
+++ b/block.c
@@ -XXX,XX +XXX,XX @@ char *bdrv_perm_names(uint64_t perm)
         { 0, NULL }
     };
 
-    char *result = g_strdup("");
+    GString *result = g_string_sized_new(30);
     struct perm_name *p;
 
     for (p = permissions; p->name; p++) {
         if (perm & p->perm) {
-            char *old = result;
-            result = g_strdup_printf("%s%s%s", old, *old ? ", " : "", p->name);
-            g_free(old);
+            if (result->len > 0) {
+                g_string_append(result, ", ");
+            }
+            g_string_append(result, p->name);
         }
     }
 
-    return result;
+    return g_string_free(result, FALSE);
 }
 
 /*
-- 
2.24.1

From: Pan Nengyuan <pannengyuan@huawei.com>

If we call the qmp 'query-block' while qemu is working on
'block-commit', it will cause memleaks, the memory leak stack is as
follow:

Indirect leak of 12360 byte(s) in 3 object(s) allocated from:
    #0 0x7f80f0b6d970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
    #1 0x7f80ee86049d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
    #2 0x55ea95b5bb67 in qdict_new /mnt/sdb/qemu-4.2.0-rc0/qobject/qdict.c:29
    #3 0x55ea956cd043 in bdrv_refresh_filename /mnt/sdb/qemu-4.2.0-rc0/block.c:6427
    #4 0x55ea956cc950 in bdrv_refresh_filename /mnt/sdb/qemu-4.2.0-rc0/block.c:6399
    #5 0x55ea956cc950 in bdrv_refresh_filename /mnt/sdb/qemu-4.2.0-rc0/block.c:6399
    #6 0x55ea956cc950 in bdrv_refresh_filename /mnt/sdb/qemu-4.2.0-rc0/block.c:6399
    #7 0x55ea958818ea in bdrv_block_device_info /mnt/sdb/qemu-4.2.0-rc0/block/qapi.c:56
    #8 0x55ea958879de in bdrv_query_info /mnt/sdb/qemu-4.2.0-rc0/block/qapi.c:392
    #9 0x55ea9588b58f in qmp_query_block /mnt/sdb/qemu-4.2.0-rc0/block/qapi.c:578
    #10 0x55ea95567392 in qmp_marshal_query_block qapi/qapi-commands-block-core.c:95

Indirect leak of 4120 byte(s) in 1 object(s) allocated from:
    #0 0x7f80f0b6d970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
    #1 0x7f80ee86049d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
    #2 0x55ea95b5bb67 in qdict_new /mnt/sdb/qemu-4.2.0-rc0/qobject/qdict.c:29
    #3 0x55ea956cd043 in bdrv_refresh_filename /mnt/sdb/qemu-4.2.0-rc0/block.c:6427
    #4 0x55ea956cc950 in bdrv_refresh_filename /mnt/sdb/qemu-4.2.0-rc0/block.c:6399
    #5 0x55ea956cc950 in bdrv_refresh_filename /mnt/sdb/qemu-4.2.0-rc0/block.c:6399
    #6 0x55ea9569f301 in bdrv_backing_attach /mnt/sdb/qemu-4.2.0-rc0/block.c:1064
    #7 0x55ea956a99dd in bdrv_replace_child_noperm /mnt/sdb/qemu-4.2.0-rc0/block.c:2283
    #8 0x55ea956b9b53 in bdrv_replace_node /mnt/sdb/qemu-4.2.0-rc0/block.c:4196
    #9 0x55ea956b9e49 in bdrv_append /mnt/sdb/qemu-4.2.0-rc0/block.c:4236
    #10 0x55ea958c3472 in commit_start /mnt/sdb/qemu-4.2.0-rc0/block/commit.c:306
    #11 0x55ea94b68ab0 in qmp_block_commit /mnt/sdb/qemu-4.2.0-rc0/blockdev.c:3459
    #12 0x55ea9556a7a7 in qmp_marshal_block_commit qapi/qapi-commands-block-core.c:407

Fixes: bb808d5f5c0978828a974d547e6032402c339555
Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
Message-id: 20200116085600.24056-1-pannengyuan@huawei.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block.c b/block.c
index XXXXXXX..XXXXXXX 100644
--- a/block.c
+++ b/block.c
@@ -XXX,XX +XXX,XX @@ void bdrv_refresh_filename(BlockDriverState *bs)
                 child->bs->exact_filename);
         pstrcpy(bs->filename, sizeof(bs->filename), child->bs->filename);
 
+        qobject_unref(bs->full_open_options);
         bs->full_open_options = qobject_ref(child->bs->full_open_options);
 
         return;
-- 
2.24.1

From: Alberto Garcia <berto@igalia.com>

This is a bit more efficient than having to allocate and free memory
for each item.

The default size (60) is enough for all the existing incompatible
features or the "Unknown incompatible feature" message.

Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-id: 20200115135626.19442-1-berto@igalia.com
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -XXX,XX +XXX,XX @@ static void cleanup_unknown_header_ext(BlockDriverState *bs)
 static void report_unsupported_feature(Error **errp, Qcow2Feature *table,
                                        uint64_t mask)
 {
-    char *features = g_strdup("");
-    char *old;
+    g_autoptr(GString) features = g_string_sized_new(60);
 
     while (table && table->name[0] != '\0') {
         if (table->type == QCOW2_FEAT_TYPE_INCOMPATIBLE) {
             if (mask & (1ULL << table->bit)) {
-                old = features;
-                features = g_strdup_printf("%s%s%.46s", old, *old ? ", " : "",
-                                           table->name);
-                g_free(old);
+                if (features->len > 0) {
+                    g_string_append(features, ", ");
+                }
+                g_string_append_printf(features, "%.46s", table->name);
                 mask &= ~(1ULL << table->bit);
             }
         }
@@ -XXX,XX +XXX,XX @@ static void report_unsupported_feature(Error **errp, Qcow2Feature *table,
     }
 
     if (mask) {
-        old = features;
-        features = g_strdup_printf("%s%sUnknown incompatible feature: %" PRIx64,
-                                   old, *old ? ", " : "", mask);
-        g_free(old);
+        if (features->len > 0) {
+            g_string_append(features, ", ");
+        }
+        g_string_append_printf(features,
+                               "Unknown incompatible feature: %" PRIx64, mask);
     }
 
-    error_setg(errp, "Unsupported qcow2 feature(s): %s", features);
-    g_free(features);
+    error_setg(errp, "Unsupported qcow2 feature(s): %s", features->str);
 }
 
 /*
-- 
2.24.1

From: John Snow <jsnow@redhat.com>

verify_platform will check an explicit whitelist and blacklist instead.
The default will now be assumed to be allowed to run anywhere.

For tests that do not specify their platforms explicitly, this has the effect of
enabling these tests on non-linux platforms. For tests that always specified
linux explicitly, there is no change.

For Python tests on FreeBSD at least; only seven python tests fail:
045 147 149 169 194 199 211

045 and 149 appear to be misconfigurations,
147 and 194 are the AF_UNIX path too long error,
169 and 199 are bitmap migration bugs, and
211 is a bug that shows up on Linux platforms, too.

This is at least good evidence that these tests are not Linux-only. If
they aren't suitable for other platforms, they should be disabled on a
per-platform basis as appropriate.

Therefore, let's switch these on and deal with the failures.

Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 20200121095205.26323-2-thuth@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/iotests.py | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -XXX,XX +XXX,XX @@ def verify_protocol(supported=[], unsupported=[]):
     if not_sup or (imgproto in unsupported):
         notrun('not suitable for this protocol: %s' % imgproto)
 
-def verify_platform(supported_oses=['linux']):
-    if True not in [sys.platform.startswith(x) for x in supported_oses]:
-        notrun('not suitable for this OS: %s' % sys.platform)
+def verify_platform(supported=None, unsupported=None):
+    if unsupported is not None:
+        if any((sys.platform.startswith(x) for x in unsupported)):
+            notrun('not suitable for this OS: %s' % sys.platform)
+
+    if supported is not None:
+        if not any((sys.platform.startswith(x) for x in supported)):
+            notrun('not suitable for this OS: %s' % sys.platform)
 
 def verify_cache_mode(supported_cache_modes=[]):
     if supported_cache_modes and (cachemode not in supported_cache_modes):
@@ -XXX,XX +XXX,XX @@ def execute_unittest(output, verbosity, debug):
             sys.stderr.write(out)
 
 def execute_test(test_function=None,
-                 supported_fmts=[], supported_oses=['linux'],
+                 supported_fmts=[],
+                 supported_platforms=None,
                  supported_cache_modes=[], supported_aio_modes={},
                  unsupported_fmts=[], supported_protocols=[],
                  unsupported_protocols=[]):
@@ -XXX,XX +XXX,XX @@ def execute_test(test_function=None,
     verbosity = 1
     verify_image_format(supported_fmts, unsupported_fmts)
     verify_protocol(supported_protocols, unsupported_protocols)
-    verify_platform(supported_oses)
+    verify_platform(supported=supported_platforms)
     verify_cache_mode(supported_cache_modes)
     verify_aio_mode(supported_aio_modes)
 
-- 
2.24.1

From: Thomas Huth <thuth@redhat.com>

In the long run, we might want to add test 183 to the "auto" group
(but it still fails occasionally, so we cannot do that yet). However,
when running 183 in Cirrus-CI on macOS, or with our vm-build-openbsd
target, it currently always fails with an "Timeout waiting for return
on handle 0" error.

Let's mark it as supported only on systems where the test is working
most of the time (i.e. Linux, FreeBSD and NetBSD).

Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-id: 20200121095205.26323-4-thuth@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/183 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qemu-iotests/183 b/tests/qemu-iotests/183
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/183
+++ b/tests/qemu-iotests/183
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 . ./common.filter
 . ./common.qemu
 
+_supported_os Linux FreeBSD NetBSD
 _supported_fmt qcow2 raw qed quorum
 _supported_proto file
 
-- 
2.24.1

From: Thomas Huth <thuth@redhat.com>

We are going to enable 127 in the "auto" group, but it only works if
virtio-scsi and scsi-hd are available - which is not the case with
QEMU binaries like qemu-system-tricore for example, so we need a
proper check for the availability of these devices here.

A very similar problem exists in iotest 267 - it has been added to
the "auto" group already, but requires virtio-blk and thus currently
fails with qemu-system-tricore for example. Let's also add aproper
check there.

Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-id: 20200121095205.26323-5-thuth@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/127       |  2 ++
 tests/qemu-iotests/267       |  2 ++
 tests/qemu-iotests/common.rc | 14 ++++++++++++++
 3 files changed, 18 insertions(+)

diff --git a/tests/qemu-iotests/127 b/tests/qemu-iotests/127
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/127
+++ b/tests/qemu-iotests/127
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt qcow2
 _supported_proto file
 
+_require_devices virtio-scsi scsi-hd
+
 IMG_SIZE=64K
 
 _make_test_img $IMG_SIZE
diff --git a/tests/qemu-iotests/267 b/tests/qemu-iotests/267
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/267
+++ b/tests/qemu-iotests/267
@@ -XXX,XX +XXX,XX @@ _require_drivers copy-on-read
 # and generally impossible with external data files
 _unsupported_imgopts 'refcount_bits=1[^0-9]' data_file
 
+_require_devices virtio-blk
+
 do_run_qemu()
 {
     echo Testing: "$@"
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -XXX,XX +XXX,XX @@ _require_large_file()
     rm "$TEST_IMG"
 }
 
+# Check that a set of devices is available in the QEMU binary
+#
+_require_devices()
+{
+    available=$($QEMU -M none -device help | \
+                grep ^name | sed -e 's/^name "//' -e 's/".*$//')
+    for device
+    do
+        if ! echo "$available" | grep -q "$device" ; then
+            _notrun "$device not available"
+        fi
+    done
+}
+
 # make sure this script returns success
 true
-- 
2.24.1

From: Thomas Huth <thuth@redhat.com>

We are going to enable some of the python-based tests in the "auto" group,
and these tests require virtio-blk to work properly. Running iotests
without virtio-blk likely does not make too much sense anyway, so instead
of adding a check for the availability of virtio-blk to each and every
test (which does not sound very appealing), let's rather add a check for
this a central spot in the "check" script instead (so that it is still
possible to run "make check" for qemu-system-tricore for example).

Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-id: 20200121095205.26323-6-thuth@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/check | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -XXX,XX +XXX,XX @@ fi
 python_usable=false
 if $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)'
 then
-    python_usable=true
+    # Our python framework also requires virtio-blk
+    if "$QEMU_PROG" -M none -device help | grep -q virtio-blk >/dev/null 2>&1
+    then
+        python_usable=true
+    else
+        python_unusable_because="Missing virtio-blk in QEMU binary"
+    fi
+else
+    python_unusable_because="Unsupported Python version"
 fi
 
 default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p')
@@ -XXX,XX +XXX,XX @@ do
                 run_command="$PYTHON $seq"
             else
                 run_command="false"
-                echo "Unsupported Python version" > $seq.notrun
+                echo "$python_unusable_because" > $seq.notrun
             fi
         else
             run_command="./$seq"
-- 
2.24.1

From: Thomas Huth <thuth@redhat.com>

According to Kevin, tests 030, 040 and 041 are among the most valuable
tests that we have, so we should always run them if possible, even if
they take a little bit longer.

According to Max, it would be good to have a test for iothreads and
migration. 127 and 256 seem to be good candidates for iothreads. For
migration, let's enable 181 and 203 (which also tests iothreads).
(091 would be a good candidate for migration, too, but Alex Bennée
reported that this test fails on ZFS file systems, so it can't be
included yet)

Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-id: 20200121095205.26323-7-thuth@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/group | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

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 @@
 027 rw auto quick
 028 rw backing quick
 029 rw auto quick
-030 rw backing
+030 rw auto backing
 031 rw auto quick
 032 rw auto quick
 033 rw auto quick
@@ -XXX,XX +XXX,XX @@
 037 rw auto backing quick
 038 rw auto backing quick
 039 rw auto quick
-040 rw
-041 rw backing
+040 rw auto
+041 rw auto backing
 042 rw auto quick
 043 rw auto backing
 044 rw
@@ -XXX,XX +XXX,XX @@
 124 rw backing
 125 rw
 126 rw auto backing
-127 rw backing quick
+127 rw auto backing quick
 128 rw quick
 129 rw quick
 130 rw quick
@@ -XXX,XX +XXX,XX @@
 177 rw auto quick
 178 img
 179 rw auto quick
-181 rw migration
+181 rw auto migration
 182 rw quick
 183 rw migration
 184 rw auto quick
@@ -XXX,XX +XXX,XX @@
 200 rw
 201 rw migration
 202 rw quick
-203 rw migration
+203 rw auto migration
 204 rw quick
 205 rw quick
 206 rw
@@ -XXX,XX +XXX,XX @@
 253 rw quick
 254 rw backing quick
 255 rw quick
-256 rw quick
+256 rw auto quick
 257 rw
 258 rw quick
 260 rw quick
-- 
2.24.1

From: Alberto Garcia <berto@igalia.com>

The L1 table is read from disk using the byte-based bdrv_pread() and
is never accessed beyond its last element, so there's no need to
allocate more memory than that.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: b2e27214ec7b03a585931bcf383ee1ac3a641a10.1579374329.git.berto@igalia.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2-cluster.c  | 5 ++---
 block/qcow2-refcount.c | 2 +-
 block/qcow2-snapshot.c | 3 +--
 block/qcow2.c          | 2 +-
 4 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -XXX,XX +XXX,XX @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
 #endif
 
     new_l1_size2 = sizeof(uint64_t) * new_l1_size;
-    new_l1_table = qemu_try_blockalign(bs->file->bs,
-                                       ROUND_UP(new_l1_size2, 512));
+    new_l1_table = qemu_try_blockalign(bs->file->bs, new_l1_size2);
     if (new_l1_table == NULL) {
         return -ENOMEM;
     }
-    memset(new_l1_table, 0, ROUND_UP(new_l1_size2, 512));
+    memset(new_l1_table, 0, new_l1_size2);
 
     if (s->l1_size) {
         memcpy(new_l1_table, s->l1_table, s->l1_size * sizeof(uint64_t));
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -XXX,XX +XXX,XX @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
      * l1_table_offset when it is the current s->l1_table_offset! Be careful
      * when changing this! */
     if (l1_table_offset != s->l1_table_offset) {
-        l1_table = g_try_malloc0(ROUND_UP(l1_size2, 512));
+        l1_table = g_try_malloc0(l1_size2);
         if (l1_size2 && l1_table == NULL) {
             ret = -ENOMEM;
             goto fail;
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -XXX,XX +XXX,XX @@ int qcow2_snapshot_load_tmp(BlockDriverState *bs,
         return ret;
     }
     new_l1_bytes = sn->l1_size * sizeof(uint64_t);
-    new_l1_table = qemu_try_blockalign(bs->file->bs,
-                                       ROUND_UP(new_l1_bytes, 512));
+    new_l1_table = qemu_try_blockalign(bs->file->bs, new_l1_bytes);
     if (new_l1_table == NULL) {
         return -ENOMEM;
     }
diff --git a/block/qcow2.c b/block/qcow2.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
 
     if (s->l1_size > 0) {
         s->l1_table = qemu_try_blockalign(bs->file->bs,
-            ROUND_UP(s->l1_size * sizeof(uint64_t), 512));
+                                          s->l1_size * sizeof(uint64_t));
         if (s->l1_table == NULL) {
             error_setg(errp, "Could not allocate L1 table");
             ret = -ENOMEM;
-- 
2.24.1

From: Alberto Garcia <berto@igalia.com>

qcow2_alloc_cluster_offset() and qcow2_get_cluster_offset() always
return offsets that are cluster-aligned so don't just check that they
are sector-aligned.

The check in qcow2_co_preadv_task() is also replaced by an assertion
for the same reason.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 558ba339965f858bede4c73ce3f50f0c0493597d.1579374329.git.berto@igalia.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_preadv_task(BlockDriverState *bs,
                                           offset, bytes, qiov, qiov_offset);
 
     case QCOW2_CLUSTER_NORMAL:
-        if ((file_cluster_offset & 511) != 0) {
-            return -EIO;
-        }
-
+        assert(offset_into_cluster(s, file_cluster_offset) == 0);
         if (bs->encrypted) {
             return qcow2_co_preadv_encrypted(bs, file_cluster_offset,
                                              offset, bytes, qiov, qiov_offset);
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_pwritev_part(
             goto out_locked;
         }
 
-        assert((cluster_offset & 511) == 0);
+        assert(offset_into_cluster(s, cluster_offset) == 0);
 
         ret = qcow2_pre_write_overlap_check(bs, 0,
                                             cluster_offset + offset_in_cluster,
@@ -XXX,XX +XXX,XX @@ qcow2_co_copy_range_to(BlockDriverState *bs,
             goto fail;
         }
 
-        assert((cluster_offset & 511) == 0);
+        assert(offset_into_cluster(s, cluster_offset) == 0);
 
         ret = qcow2_pre_write_overlap_check(bs, 0,
                 cluster_offset + offset_in_cluster, cur_bytes, true);
-- 
2.24.1

From: Alberto Garcia <berto@igalia.com>

When updating an L1 entry the qcow2 driver writes a (512-byte) sector
worth of data to avoid a read-modify-write cycle. Instead of always
writing 512 bytes we should follow the alignment requirements of the
storage backend.

(the only exception is when the alignment is larger than the cluster
size because then we could be overwriting data after the L1 table)

Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-id: 71f34d4ae4b367b32fb36134acbf4f4f7ee681f4.1579374329.git.berto@igalia.com
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2-cluster.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -XXX,XX +XXX,XX @@ static int l2_load(BlockDriverState *bs, uint64_t offset,
 }
 
 /*
- * Writes one sector of the L1 table to the disk (can't update single entries
- * and we really don't want bdrv_pread to perform a read-modify-write)
+ * Writes an L1 entry to disk (note that depending on the alignment
+ * requirements this function may write more that just one entry in
+ * order to prevent bdrv_pwrite from performing a read-modify-write)
  */
-#define L1_ENTRIES_PER_SECTOR (512 / 8)
 int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index)
 {
     BDRVQcow2State *s = bs->opaque;
-    uint64_t buf[L1_ENTRIES_PER_SECTOR] = { 0 };
     int l1_start_index;
     int i, ret;
+    int bufsize = MAX(sizeof(uint64_t),
+                      MIN(bs->file->bs->bl.request_alignment, s->cluster_size));
+    int nentries = bufsize / sizeof(uint64_t);
+    g_autofree uint64_t *buf = g_try_new0(uint64_t, nentries);
 
-    l1_start_index = l1_index & ~(L1_ENTRIES_PER_SECTOR - 1);
-    for (i = 0; i < L1_ENTRIES_PER_SECTOR && l1_start_index + i < s->l1_size;
-         i++)
-    {
+    if (buf == NULL) {
+        return -ENOMEM;
+    }
+
+    l1_start_index = QEMU_ALIGN_DOWN(l1_index, nentries);
+    for (i = 0; i < MIN(nentries, s->l1_size - l1_start_index); i++) {
         buf[i] = cpu_to_be64(s->l1_table[l1_start_index + i]);
     }
 
     ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_ACTIVE_L1,
-            s->l1_table_offset + 8 * l1_start_index, sizeof(buf), false);
+            s->l1_table_offset + 8 * l1_start_index, bufsize, false);
     if (ret < 0) {
         return ret;
     }
@@ -XXX,XX +XXX,XX @@ int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index)
     BLKDBG_EVENT(bs->file, BLKDBG_L1_UPDATE);
     ret = bdrv_pwrite_sync(bs->file,
                            s->l1_table_offset + 8 * l1_start_index,
-                           buf, sizeof(buf));
+                           buf, bufsize);
     if (ret < 0) {
         return ret;
     }
-- 
2.24.1

From: Alberto Garcia <berto@igalia.com>

qemu-img's convert_co_copy_range() operates at the sector level and
block_copy() operates at the cluster level so this condition is always
true, but it is not necessary to restrict this here, so let's leave it
to the driver implementation return an error if there is any.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-id: a4264aaee656910c84161a2965f7a501437379ca.1579374329.git.berto@igalia.com
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -XXX,XX +XXX,XX @@ qcow2_co_copy_range_from(BlockDriverState *bs,
         case QCOW2_CLUSTER_NORMAL:
             child = s->data_file;
             copy_offset += offset_into_cluster(s, src_offset);
-            if ((copy_offset & 511) != 0) {
-                ret = -EIO;
-                goto out;
-            }
             break;
 
         default:
-- 
2.24.1

From: Alberto Garcia <berto@igalia.com>

This replaces all remaining instances in the qcow2 code.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-id: b5f74b606c2d9873b12d29acdb7fd498029c4025.1579374329.git.berto@igalia.com
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -XXX,XX +XXX,XX @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
 
     /* Validate options and set default values */
     if (!QEMU_IS_ALIGNED(qcow2_opts->size, BDRV_SECTOR_SIZE)) {
-        error_setg(errp, "Image size must be a multiple of 512 bytes");
+        error_setg(errp, "Image size must be a multiple of %u bytes",
+                   (unsigned) BDRV_SECTOR_SIZE);
         ret = -EINVAL;
         goto out;
     }
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
         return -ENOTSUP;
     }
 
-    if (offset & 511) {
-        error_setg(errp, "The new size must be a multiple of 512");
+    if (!QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)) {
+        error_setg(errp, "The new size must be a multiple of %u",
+                   (unsigned) BDRV_SECTOR_SIZE);
         return -EINVAL;
     }
 
-- 
2.24.1

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

We can't access top after call bdrv_backup_top_drop, as it is already
freed at this time.

Also, no needs to unref target child by hand, it will be unrefed on
bdrv_close() automatically.

So, just do bdrv_backup_top_drop if append succeed and one bdrv_unref
otherwise.

Note, that in !appended case bdrv_unref(top) moved into drained section
on source. It doesn't really matter, but just for code simplicity.

Fixes: 7df7868b96404
Cc: qemu-stable@nongnu.org # v4.2.0
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 20200121142802.21467-2-vsementsov@virtuozzo.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/backup-top.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/block/backup-top.c b/block/backup-top.c
index XXXXXXX..XXXXXXX 100644
--- a/block/backup-top.c
+++ b/block/backup-top.c
@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
     BlockDriverState *top = bdrv_new_open_driver(&bdrv_backup_top_filter,
                                                  filter_node_name,
                                                  BDRV_O_RDWR, errp);
+    bool appended = false;
 
     if (!top) {
         return NULL;
@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
     bdrv_append(top, source, &local_err);
     if (local_err) {
         error_prepend(&local_err, "Cannot append backup-top filter: ");
-        goto append_failed;
+        goto fail;
     }
+    appended = true;
 
     /*
      * bdrv_append() finished successfully, now we can require permissions
@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
     if (local_err) {
         error_prepend(&local_err,
                       "Cannot set permissions for backup-top filter: ");
-        goto failed_after_append;
+        goto fail;
     }
 
     state->bcs = block_copy_state_new(top->backing, state->target,
                                       cluster_size, write_flags, &local_err);
     if (local_err) {
         error_prepend(&local_err, "Cannot create block-copy-state: ");
-        goto failed_after_append;
+        goto fail;
     }
     *bcs = state->bcs;
 
@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
 
     return top;
 
-failed_after_append:
-    state->active = false;
-    bdrv_backup_top_drop(top);
+fail:
+    if (appended) {
+        state->active = false;
+        bdrv_backup_top_drop(top);
+    } else {
+        bdrv_unref(top);
+    }
 
-append_failed:
     bdrv_drained_end(source);
-    bdrv_unref_child(top, state->target);
-    bdrv_unref(top);
     error_propagate(errp, local_err);
 
     return NULL;
-- 
2.24.1

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

This test checks that bug is really fixed by previous commit.

Cc: qemu-stable@nongnu.org # v4.2.0
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20200121142802.21467-3-vsementsov@virtuozzo.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/283     | 92 ++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/283.out |  8 ++++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 101 insertions(+)
 create mode 100644 tests/qemu-iotests/283
 create mode 100644 tests/qemu-iotests/283.out

diff --git a/tests/qemu-iotests/283 b/tests/qemu-iotests/283
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qemu-iotests/283
@@ -XXX,XX +XXX,XX @@
+#!/usr/bin/env python
+#
+# Test for backup-top filter permission activation failure
+#
+# Copyright (c) 2019 Virtuozzo International GmbH.
+#
+# 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/>.
+#
+
+import iotests
+
+# The test is unrelated to formats, restrict it to qcow2 to avoid extra runs
+iotests.verify_image_format(supported_fmts=['qcow2'])
+
+size = 1024 * 1024
+
+""" Test description
+
+When performing a backup, all writes on the source subtree must go through the
+backup-top filter so it can copy all data to the target before it is changed.
+backup-top filter is appended above source node, to achieve this thing, so all
+parents of source node are handled. A configuration with side parents of source
+sub-tree with write permission is unsupported (we'd have append several
+backup-top filter like nodes to handle such parents). The test create an
+example of such configuration and checks that a backup is then not allowed
+(blockdev-backup command should fail).
+
+The configuration:
+
+    ┌────────┐  target  ┌─────────────┐
+    │ target │ ◀─────── │ backup_top  │
+    └────────┘          └─────────────┘
+                            │
+                            │ backing
+                            ▼
+                        ┌─────────────┐
+                        │   source    │
+                        └─────────────┘
+                            │
+                            │ file
+                            ▼
+                        ┌─────────────┐  write perm   ┌───────┐
+                        │    base     │ ◀──────────── │ other │
+                        └─────────────┘               └───────┘
+
+On activation (see .active field of backup-top state in block/backup-top.c),
+backup-top is going to unshare write permission on its source child. Write
+unsharing will be propagated to the "source->base" link and will conflict with
+other node write permission. So permission update will fail and backup job will
+not be started.
+
+Note, that the only thing which prevents backup of running on such
+configuration is default permission propagation scheme. It may be altered by
+different block drivers, so backup will run in invalid configuration. But
+something is better than nothing. Also, before the previous commit (commit
+preceding this test creation), starting backup on such configuration led to
+crash, so current "something" is a lot better, and this test actual goal is
+to check that crash is fixed :)
+"""
+
+vm = iotests.VM()
+vm.launch()
+
+vm.qmp_log('blockdev-add', **{'node-name': 'target', 'driver': 'null-co'})
+
+vm.qmp_log('blockdev-add', **{
+    'node-name': 'source',
+    'driver': 'blkdebug',
+    'image': {'node-name': 'base', 'driver': 'null-co', 'size': size}
+})
+
+vm.qmp_log('blockdev-add', **{
+    'node-name': 'other',
+    'driver': 'blkdebug',
+    'image': 'base',
+    'take-child-perms': ['write']
+})
+
+vm.qmp_log('blockdev-backup', sync='full', device='source', target='target')
+
+vm.shutdown()
diff --git a/tests/qemu-iotests/283.out b/tests/qemu-iotests/283.out
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qemu-iotests/283.out
@@ -XXX,XX +XXX,XX @@
+{"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "target"}}
+{"return": {}}
+{"execute": "blockdev-add", "arguments": {"driver": "blkdebug", "image": {"driver": "null-co", "node-name": "base", "size": 1048576}, "node-name": "source"}}
+{"return": {}}
+{"execute": "blockdev-add", "arguments": {"driver": "blkdebug", "image": "base", "node-name": "other", "take-child-perms": ["write"]}}
+{"return": {}}
+{"execute": "blockdev-backup", "arguments": {"device": "source", "sync": "full", "target": "target"}}
+{"error": {"class": "GenericError", "desc": "Cannot set permissions for backup-top filter: Conflicts with use by other as 'image', which uses 'write' on base"}}
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 @@
 279 rw backing quick
 280 rw migration quick
 281 rw quick
+283 auto quick
-- 
2.24.1

The following changes since commit eaefea537b476cb853e2edbdc68e969ec777e4bb:

Merge remote-tracking branch 'remotes/mjt/tags/trivial-patches-fetch' into staging (2017-12-18 14:17:42 +0000)

are available in the Git repository at:

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

for you to fetch changes up to 7a9dda0d7f9831c2432620dcfefdadbb7ae888dc:

qemu-iotests: add 203 savevm with IOThreads test (2017-12-19 10:25:09 +0000)

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

v2:
 * Fixed incorrect virtio_blk_data_plane_create() local_err refactoring in
   "hw/block: Use errp directly rather than local_err" that broke virtio-blk
   over virtio-mmio [Peter]

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

Mao Zhongyi (4):
  hw/block/nvme: Convert to realize
  hw/block: Fix the return type
  hw/block: Use errp directly rather than local_err
  dev-storage: Fix the unusual function name

Mark Kanda (2):
  virtio-blk: make queue size configurable
  virtio-blk: reject configs with logical block size > physical block
    size

Paolo Bonzini (1):
  block: avoid recursive AioContext acquire in bdrv_inactivate_all()

Stefan Hajnoczi (16):
  coroutine: simplify co_aio_sleep_ns() prototype
  qdev: drop unused #include "sysemu/iothread.h"
  blockdev: hold AioContext for bdrv_unref() in
    external_snapshot_clean()
  block: don't keep AioContext acquired after
    external_snapshot_prepare()
  block: don't keep AioContext acquired after drive_backup_prepare()
  block: don't keep AioContext acquired after blockdev_backup_prepare()
  block: don't keep AioContext acquired after
    internal_snapshot_prepare()
  block: drop unused BlockDirtyBitmapState->aio_context field
  iothread: add iothread_by_id() API
  blockdev: add x-blockdev-set-iothread testing command
  qemu-iotests: add 202 external snapshots IOThread test
  docs: mark nested AioContext locking as a legacy API
  blockdev: add x-blockdev-set-iothread force boolean
  iotests: add VM.add_object()
  iothread: fix iothread_stop() race condition
  qemu-iotests: add 203 savevm with IOThreads test

-- 
2.14.3

The AioContext pointer argument to co_aio_sleep_ns() is only used for
the sleep timer.  It does not affect where the caller coroutine is
resumed.

Due to changes to coroutine and AIO APIs it is now possible to drop the
AioContext pointer argument.  This is safe to do since no caller has
specific requirements for which AioContext the timer must run in.

This patch drops the AioContext pointer argument and renames the
function to simplify the API.

Reported-by: Paolo Bonzini <pbonzini@redhat.com>
Reported-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20171109102652.6360-1-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/qemu/coroutine.h    | 6 +-----
 block/null.c                | 3 +--
 block/sheepdog.c            | 3 +--
 util/qemu-coroutine-sleep.c | 4 ++--
 4 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index XXXXXXX..XXXXXXX 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -XXX,XX +XXX,XX @@ void qemu_co_rwlock_unlock(CoRwlock *lock);
 
 /**
  * Yield the coroutine for a given duration
- *
- * Behaves similarly to co_sleep_ns(), but the sleeping coroutine will be
- * resumed when using aio_poll().
  */
-void coroutine_fn co_aio_sleep_ns(AioContext *ctx, QEMUClockType type,
-                                  int64_t ns);
+void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns);
 
 /**
  * Yield until a file descriptor becomes readable
diff --git a/block/null.c b/block/null.c
index XXXXXXX..XXXXXXX 100644
--- a/block/null.c
+++ b/block/null.c
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int null_co_common(BlockDriverState *bs)
     BDRVNullState *s = bs->opaque;
 
     if (s->latency_ns) {
-        co_aio_sleep_ns(bdrv_get_aio_context(bs), QEMU_CLOCK_REALTIME,
-                        s->latency_ns);
+        qemu_co_sleep_ns(QEMU_CLOCK_REALTIME, s->latency_ns);
     }
     return 0;
 }
diff --git a/block/sheepdog.c b/block/sheepdog.c
index XXXXXXX..XXXXXXX 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -XXX,XX +XXX,XX @@ static coroutine_fn void reconnect_to_sdog(void *opaque)
         if (s->fd < 0) {
             DPRINTF("Wait for connection to be established\n");
             error_report_err(local_err);
-            co_aio_sleep_ns(bdrv_get_aio_context(s->bs), QEMU_CLOCK_REALTIME,
-                            1000000000ULL);
+            qemu_co_sleep_ns(QEMU_CLOCK_REALTIME, 1000000000ULL);
         }
     };
 
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
index XXXXXXX..XXXXXXX 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -XXX,XX +XXX,XX @@ static void co_sleep_cb(void *opaque)
     aio_co_wake(sleep_cb->co);
 }
 
-void coroutine_fn co_aio_sleep_ns(AioContext *ctx, QEMUClockType type,
-                                  int64_t ns)
+void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
 {
+    AioContext *ctx = qemu_get_current_aio_context();
     CoSleepCB sleep_cb = {
         .co = qemu_coroutine_self(),
     };
-- 
2.14.3

From: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>

Convert nvme_init() to realize and rename it to nvme_realize().

Cc: John Snow <jsnow@redhat.com>
Cc: Keith Busch <keith.busch@intel.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>

Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
Message-id: 2882e72d795e04cbe2120f569d551aef2467ac60.1511317952.git.maozy.fnst@cn.fujitsu.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/block/nvme.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -XXX,XX +XXX,XX @@ static const MemoryRegionOps nvme_cmb_ops = {
     },
 };
 
-static int nvme_init(PCIDevice *pci_dev)
+static void nvme_realize(PCIDevice *pci_dev, Error **errp)
 {
     NvmeCtrl *n = NVME(pci_dev);
     NvmeIdCtrl *id = &n->id_ctrl;
@@ -XXX,XX +XXX,XX @@ static int nvme_init(PCIDevice *pci_dev)
     Error *local_err = NULL;
 
     if (!n->conf.blk) {
-        return -1;
+        error_setg(errp, "drive property not set");
+        return;
     }
 
     bs_size = blk_getlength(n->conf.blk);
     if (bs_size < 0) {
-        return -1;
+        error_setg(errp, "could not get backing file size");
+        return;
     }
 
     blkconf_serial(&n->conf, &n->serial);
     if (!n->serial) {
-        return -1;
+        error_setg(errp, "serial property not set");
+        return;
     }
     blkconf_blocksizes(&n->conf);
     blkconf_apply_backend_options(&n->conf, blk_is_read_only(n->conf.blk),
                                   false, &local_err);
     if (local_err) {
-        error_report_err(local_err);
-        return -1;
+        error_propagate(errp, local_err);
+        return;
     }
 
     pci_conf = pci_dev->config;
@@ -XXX,XX +XXX,XX @@ static int nvme_init(PCIDevice *pci_dev)
             cpu_to_le64(n->ns_size >>
                 id_ns->lbaf[NVME_ID_NS_FLBAS_INDEX(ns->id_ns.flbas)].ds);
     }
-    return 0;
 }
 
 static void nvme_exit(PCIDevice *pci_dev)
@@ -XXX,XX +XXX,XX @@ static void nvme_class_init(ObjectClass *oc, void *data)
     DeviceClass *dc = DEVICE_CLASS(oc);
     PCIDeviceClass *pc = PCI_DEVICE_CLASS(oc);
 
-    pc->init = nvme_init;
+    pc->realize = nvme_realize;
     pc->exit = nvme_exit;
     pc->class_id = PCI_CLASS_STORAGE_EXPRESS;
     pc->vendor_id = PCI_VENDOR_ID_INTEL;
-- 
2.14.3

From: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>

When the function no success value to transmit, it usually make the
function return void. It has turned out not to be a success, because
it means that the extra local_err variable and error_propagate() will
be needed. It leads to cumbersome code, therefore, transmit success/
failure in the return value is worth.

So fix the return type of blkconf_apply_backend_options(),
blkconf_geometry() and virtio_blk_data_plane_create() to avoid it.

Cc: John Snow <jsnow@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>

Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: ac0edc1fc70c4457e5cec94405eb7d1f89f9c2c1.1511317952.git.maozy.fnst@cn.fujitsu.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/block/dataplane/virtio-blk.h |  2 +-
 include/hw/block/block.h        |  4 ++--
 hw/block/block.c                | 15 +++++++++------
 hw/block/dataplane/virtio-blk.c | 12 +++++++-----
 4 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.h b/hw/block/dataplane/virtio-blk.h
index XXXXXXX..XXXXXXX 100644
--- a/hw/block/dataplane/virtio-blk.h
+++ b/hw/block/dataplane/virtio-blk.h
@@ -XXX,XX +XXX,XX @@
 
 typedef struct VirtIOBlockDataPlane VirtIOBlockDataPlane;
 
-void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
+bool virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
                                   VirtIOBlockDataPlane **dataplane,
                                   Error **errp);
 void virtio_blk_data_plane_destroy(VirtIOBlockDataPlane *s);
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index XXXXXXX..XXXXXXX 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -XXX,XX +XXX,XX @@ static inline unsigned int get_physical_block_exp(BlockConf *conf)
 /* Configuration helpers */
 
 void blkconf_serial(BlockConf *conf, char **serial);
-void blkconf_geometry(BlockConf *conf, int *trans,
+bool blkconf_geometry(BlockConf *conf, int *trans,
                       unsigned cyls_max, unsigned heads_max, unsigned secs_max,
                       Error **errp);
 void blkconf_blocksizes(BlockConf *conf);
-void blkconf_apply_backend_options(BlockConf *conf, bool readonly,
+bool blkconf_apply_backend_options(BlockConf *conf, bool readonly,
                                    bool resizable, Error **errp);
 
 /* Hard disk geometry */
diff --git a/hw/block/block.c b/hw/block/block.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -XXX,XX +XXX,XX @@ void blkconf_blocksizes(BlockConf *conf)
     }
 }
 
-void blkconf_apply_backend_options(BlockConf *conf, bool readonly,
+bool blkconf_apply_backend_options(BlockConf *conf, bool readonly,
                                    bool resizable, Error **errp)
 {
     BlockBackend *blk = conf->blk;
@@ -XXX,XX +XXX,XX @@ void blkconf_apply_backend_options(BlockConf *conf, bool readonly,
 
     ret = blk_set_perm(blk, perm, shared_perm, errp);
     if (ret < 0) {
-        return;
+        return false;
     }
 
     switch (conf->wce) {
@@ -XXX,XX +XXX,XX @@ void blkconf_apply_backend_options(BlockConf *conf, bool readonly,
 
     blk_set_enable_write_cache(blk, wce);
     blk_set_on_error(blk, rerror, werror);
+
+    return true;
 }
 
-void blkconf_geometry(BlockConf *conf, int *ptrans,
+bool blkconf_geometry(BlockConf *conf, int *ptrans,
                       unsigned cyls_max, unsigned heads_max, unsigned secs_max,
                       Error **errp)
 {
@@ -XXX,XX +XXX,XX @@ void blkconf_geometry(BlockConf *conf, int *ptrans,
     if (conf->cyls || conf->heads || conf->secs) {
         if (conf->cyls < 1 || conf->cyls > cyls_max) {
             error_setg(errp, "cyls must be between 1 and %u", cyls_max);
-            return;
+            return false;
         }
         if (conf->heads < 1 || conf->heads > heads_max) {
             error_setg(errp, "heads must be between 1 and %u", heads_max);
-            return;
+            return false;
         }
         if (conf->secs < 1 || conf->secs > secs_max) {
             error_setg(errp, "secs must be between 1 and %u", secs_max);
-            return;
+            return false;
         }
     }
+    return true;
 }
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -XXX,XX +XXX,XX @@ static void notify_guest_bh(void *opaque)
 }
 
 /* Context: QEMU global mutex held */
-void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
+bool virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
                                   VirtIOBlockDataPlane **dataplane,
                                   Error **errp)
 {
@@ -XXX,XX +XXX,XX @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
             error_setg(errp,
                        "device is incompatible with iothread "
                        "(transport does not support notifiers)");
-            return;
+            return false;
         }
         if (!virtio_device_ioeventfd_enabled(vdev)) {
             error_setg(errp, "ioeventfd is required for iothread");
-            return;
+            return false;
         }
 
         /* If dataplane is (re-)enabled while the guest is running there could
@@ -XXX,XX +XXX,XX @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
          */
         if (blk_op_is_blocked(conf->conf.blk, BLOCK_OP_TYPE_DATAPLANE, errp)) {
             error_prepend(errp, "cannot start virtio-blk dataplane: ");
-            return;
+            return false;
         }
     }
     /* Don't try if transport does not support notifiers. */
     if (!virtio_device_ioeventfd_enabled(vdev)) {
-        return;
+        return false;
     }
 
     s = g_new0(VirtIOBlockDataPlane, 1);
@@ -XXX,XX +XXX,XX @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
     s->batch_notify_vqs = bitmap_new(conf->num_queues);
 
     *dataplane = s;
+
+    return true;
 }
 
 /* Context: QEMU global mutex held */
-- 
2.14.3

From: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>

[Drop virtio_blk_data_plane_create() change that misinterprets return
value when the virtio transport does not support dataplane.
--Stefan]

Cc: John Snow <jsnow@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>
Cc: Keith Busch <keith.busch@intel.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>

Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: e77848d3735ba590f23ffbf8094379c646c33d79.1511317952.git.maozy.fnst@cn.fujitsu.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/block/fdc.c        | 17 ++++++-----------
 hw/block/nvme.c       |  7 ++-----
 hw/block/virtio-blk.c | 13 +++++--------
 hw/ide/qdev.c         | 12 ++++--------
 hw/scsi/scsi-disk.c   | 13 ++++---------
 hw/usb/dev-storage.c  |  9 +++------
 6 files changed, 24 insertions(+), 47 deletions(-)

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 fd_revalidate(FDrive *drv)
 static void fd_change_cb(void *opaque, bool load, Error **errp)
 {
     FDrive *drive = opaque;
-    Error *local_err = NULL;
 
     if (!load) {
         blk_set_perm(drive->blk, 0, BLK_PERM_ALL, &error_abort);
     } else {
-        blkconf_apply_backend_options(drive->conf,
-                                      blk_is_read_only(drive->blk), false,
-                                      &local_err);
-        if (local_err) {
-            error_propagate(errp, local_err);
+        if (!blkconf_apply_backend_options(drive->conf,
+                                           blk_is_read_only(drive->blk), false,
+                                           errp)) {
             return;
         }
     }
@@ -XXX,XX +XXX,XX @@ static void floppy_drive_realize(DeviceState *qdev, Error **errp)
     FloppyDrive *dev = FLOPPY_DRIVE(qdev);
     FloppyBus *bus = FLOPPY_BUS(qdev->parent_bus);
     FDrive *drive;
-    Error *local_err = NULL;
     int ret;
 
     if (dev->unit == -1) {
@@ -XXX,XX +XXX,XX @@ static void floppy_drive_realize(DeviceState *qdev, Error **errp)
     dev->conf.rerror = BLOCKDEV_ON_ERROR_AUTO;
     dev->conf.werror = BLOCKDEV_ON_ERROR_AUTO;
 
-    blkconf_apply_backend_options(&dev->conf, blk_is_read_only(dev->conf.blk),
-                                  false, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
+    if (!blkconf_apply_backend_options(&dev->conf,
+                                       blk_is_read_only(dev->conf.blk),
+                                       false, errp)) {
         return;
     }
 
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -XXX,XX +XXX,XX @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
     int i;
     int64_t bs_size;
     uint8_t *pci_conf;
-    Error *local_err = NULL;
 
     if (!n->conf.blk) {
         error_setg(errp, "drive property not set");
@@ -XXX,XX +XXX,XX @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
         return;
     }
     blkconf_blocksizes(&n->conf);
-    blkconf_apply_backend_options(&n->conf, blk_is_read_only(n->conf.blk),
-                                  false, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
+    if (!blkconf_apply_backend_options(&n->conf, blk_is_read_only(n->conf.blk),
+                                       false, errp)) {
         return;
     }
 
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -XXX,XX +XXX,XX @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
     }
 
     blkconf_serial(&conf->conf, &conf->serial);
-    blkconf_apply_backend_options(&conf->conf,
-                                  blk_is_read_only(conf->conf.blk), true,
-                                  &err);
-    if (err) {
-        error_propagate(errp, err);
+    if (!blkconf_apply_backend_options(&conf->conf,
+                                       blk_is_read_only(conf->conf.blk), true,
+                                       errp)) {
         return;
     }
     s->original_wce = blk_enable_write_cache(conf->conf.blk);
-    blkconf_geometry(&conf->conf, NULL, 65535, 255, 255, &err);
-    if (err) {
-        error_propagate(errp, err);
+    if (!blkconf_geometry(&conf->conf, NULL, 65535, 255, 255, errp)) {
         return;
     }
+
     blkconf_blocksizes(&conf->conf);
 
     virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -XXX,XX +XXX,XX @@ static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp)
 {
     IDEBus *bus = DO_UPCAST(IDEBus, qbus, dev->qdev.parent_bus);
     IDEState *s = bus->ifs + dev->unit;
-    Error *err = NULL;
     int ret;
 
     if (!dev->conf.blk) {
@@ -XXX,XX +XXX,XX @@ static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp)
 
     blkconf_serial(&dev->conf, &dev->serial);
     if (kind != IDE_CD) {
-        blkconf_geometry(&dev->conf, &dev->chs_trans, 65535, 16, 255, &err);
-        if (err) {
-            error_propagate(errp, err);
+        if (!blkconf_geometry(&dev->conf, &dev->chs_trans, 65535, 16, 255,
+                              errp)) {
             return;
         }
     }
-    blkconf_apply_backend_options(&dev->conf, kind == IDE_CD, kind != IDE_CD,
-                                  &err);
-    if (err) {
-        error_propagate(errp, err);
+    if (!blkconf_apply_backend_options(&dev->conf, kind == IDE_CD,
+                                       kind != IDE_CD, errp)) {
         return;
     }
 
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -XXX,XX +XXX,XX @@ static void scsi_disk_unit_attention_reported(SCSIDevice *dev)
 static void scsi_realize(SCSIDevice *dev, Error **errp)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
-    Error *err = NULL;
 
     if (!s->qdev.conf.blk) {
         error_setg(errp, "drive property not set");
@@ -XXX,XX +XXX,XX @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
     }
 
     if (dev->type == TYPE_DISK) {
-        blkconf_geometry(&dev->conf, NULL, 65535, 255, 255, &err);
-        if (err) {
-            error_propagate(errp, err);
+        if (!blkconf_geometry(&dev->conf, NULL, 65535, 255, 255, errp)) {
             return;
         }
     }
-    blkconf_apply_backend_options(&dev->conf,
-                                  blk_is_read_only(s->qdev.conf.blk),
-                                  dev->type == TYPE_DISK, &err);
-    if (err) {
-        error_propagate(errp, err);
+    if (!blkconf_apply_backend_options(&dev->conf,
+                                       blk_is_read_only(s->qdev.conf.blk),
+                                       dev->type == TYPE_DISK, errp)) {
         return;
     }
 
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -XXX,XX +XXX,XX @@ static void usb_msd_realize_storage(USBDevice *dev, Error **errp)
     MSDState *s = USB_STORAGE_DEV(dev);
     BlockBackend *blk = s->conf.blk;
     SCSIDevice *scsi_dev;
-    Error *err = NULL;
 
     if (!blk) {
         error_setg(errp, "drive property not set");
@@ -XXX,XX +XXX,XX @@ static void usb_msd_realize_storage(USBDevice *dev, Error **errp)
 
     blkconf_serial(&s->conf, &dev->serial);
     blkconf_blocksizes(&s->conf);
-    blkconf_apply_backend_options(&s->conf, blk_is_read_only(blk), true, &err);
-    if (err) {
-        error_propagate(errp, err);
+    if (!blkconf_apply_backend_options(&s->conf, blk_is_read_only(blk), true,
+                                       errp)) {
         return;
     }
 
@@ -XXX,XX +XXX,XX @@ static void usb_msd_realize_storage(USBDevice *dev, Error **errp)
                  &usb_msd_scsi_info_storage, NULL);
     scsi_dev = scsi_bus_legacy_add_drive(&s->bus, blk, 0, !!s->removable,
                                          s->conf.bootindex, dev->serial,
-                                         &err);
+                                         errp);
     blk_unref(blk);
     if (!scsi_dev) {
-        error_propagate(errp, err);
         return;
     }
     usb_msd_handle_reset(dev);
-- 
2.14.3

From: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>

The function name of usb_msd_{realize,unrealize}_*,
usb_msd_class_initfn_* are unusual. Rename it to
usb_msd_*_{realize,unrealize}, usb_msd_class_*_initfn.

Cc: Gerd Hoffmann <kraxel@redhat.com>

Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 11e6003433abce35f3f4970e1acc71ee92dbcf51.1511317952.git.maozy.fnst@cn.fujitsu.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/usb/dev-storage.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -XXX,XX +XXX,XX @@ static void usb_msd_unrealize_storage(USBDevice *dev, Error **errp)
     object_unref(OBJECT(&s->bus));
 }
 
-static void usb_msd_realize_storage(USBDevice *dev, Error **errp)
+static void usb_msd_storage_realize(USBDevice *dev, Error **errp)
 {
     MSDState *s = USB_STORAGE_DEV(dev);
     BlockBackend *blk = s->conf.blk;
@@ -XXX,XX +XXX,XX @@ static void usb_msd_realize_storage(USBDevice *dev, Error **errp)
     s->scsi_dev = scsi_dev;
 }
 
-static void usb_msd_unrealize_bot(USBDevice *dev, Error **errp)
+static void usb_msd_bot_unrealize(USBDevice *dev, Error **errp)
 {
     MSDState *s = USB_STORAGE_DEV(dev);
 
     object_unref(OBJECT(&s->bus));
 }
 
-static void usb_msd_realize_bot(USBDevice *dev, Error **errp)
+static void usb_msd_bot_realize(USBDevice *dev, Error **errp)
 {
     MSDState *s = USB_STORAGE_DEV(dev);
     DeviceState *d = DEVICE(dev);
@@ -XXX,XX +XXX,XX @@ static void usb_msd_class_initfn_common(ObjectClass *klass, void *data)
     dc->vmsd = &vmstate_usb_msd;
 }
 
-static void usb_msd_class_initfn_storage(ObjectClass *klass, void *data)
+static void usb_msd_class_storage_initfn(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
 
-    uc->realize = usb_msd_realize_storage;
+    uc->realize = usb_msd_storage_realize;
     uc->unrealize = usb_msd_unrealize_storage;
     dc->props = msd_properties;
 }
@@ -XXX,XX +XXX,XX @@ static void usb_msd_instance_init(Object *obj)
     object_property_set_int(obj, -1, "bootindex", NULL);
 }
 
-static void usb_msd_class_initfn_bot(ObjectClass *klass, void *data)
+static void usb_msd_class_bot_initfn(ObjectClass *klass, void *data)
 {
     USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
 
-    uc->realize = usb_msd_realize_bot;
-    uc->unrealize = usb_msd_unrealize_bot;
+    uc->realize = usb_msd_bot_realize;
+    uc->unrealize = usb_msd_bot_unrealize;
     uc->attached_settable = true;
 }
 
 static const TypeInfo msd_info = {
     .name          = "usb-storage",
     .parent        = TYPE_USB_STORAGE,
-    .class_init    = usb_msd_class_initfn_storage,
+    .class_init    = usb_msd_class_storage_initfn,
     .instance_init = usb_msd_instance_init,
 };
 
 static const TypeInfo bot_info = {
     .name          = "usb-bot",
     .parent        = TYPE_USB_STORAGE,
-    .class_init    = usb_msd_class_initfn_bot,
+    .class_init    = usb_msd_class_bot_initfn,
 };
 
 static void usb_msd_register_types(void)
-- 
2.14.3

bdrv_unref() requires the AioContext lock because bdrv_flush() uses
BDRV_POLL_WHILE(), which assumes the AioContext is currently held.  If
BDRV_POLL_WHILE() runs without AioContext held the
pthread_mutex_unlock() call in aio_context_release() fails.

This patch moves bdrv_unref() into the AioContext locked region to solve
the following pthread_mutex_unlock() failure:

#0  0x00007f566181969b in raise () at /lib64/libc.so.6
  #1  0x00007f566181b3b1 in abort () at /lib64/libc.so.6
  #2  0x00005592cd590458 in error_exit (err=<optimized out>, msg=msg@entry=0x5592cdaf6d60 <__func__.23977> "qemu_mutex_unlock") at util/qemu-thread-posix.c:36
  #3  0x00005592cd96e738 in qemu_mutex_unlock (mutex=mutex@entry=0x5592ce9505e0) at util/qemu-thread-posix.c:96
  #4  0x00005592cd969b69 in aio_context_release (ctx=ctx@entry=0x5592ce950580) at util/async.c:507
  #5  0x00005592cd8ead78 in bdrv_flush (bs=bs@entry=0x5592cfa87210) at block/io.c:2478
  #6  0x00005592cd89df30 in bdrv_close (bs=0x5592cfa87210) at block.c:3207
  #7  0x00005592cd89df30 in bdrv_delete (bs=0x5592cfa87210) at block.c:3395
  #8  0x00005592cd89df30 in bdrv_unref (bs=0x5592cfa87210) at block.c:4418
  #9  0x00005592cd6b7f86 in qmp_transaction (dev_list=<optimized out>, has_props=<optimized out>, props=<optimized out>, errp=errp@entry=0x7ffe4a1fc9d8) at blockdev.c:2308

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_clean(BlkActionState *common)
                              DO_UPCAST(ExternalSnapshotState, common, common);
     if (state->aio_context) {
         bdrv_drained_end(state->old_bs);
-        aio_context_release(state->aio_context);
         bdrv_unref(state->new_bs);
+        aio_context_release(state->aio_context);
     }
 }
 
-- 
2.14.3

It is not necessary to hold AioContext across transactions anymore since
bdrv_drained_begin/end() is used to keep the nodes quiesced.  In fact,
using the AioContext lock for this purpose was always buggy.

This patch reduces the scope of AioContext locked regions.  This is not
just a cleanup but also fixes hangs that occur in BDRV_POLL_WHILE()
because it is unware of recursive locking and does not release the
AioContext the necessary number of times to allow progress to be made.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20171206144550.22295-3-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 blockdev.c | 71 ++++++++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 48 insertions(+), 23 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index XXXXXXX..XXXXXXX 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -XXX,XX +XXX,XX @@ typedef struct ExternalSnapshotState {
     BlkActionState common;
     BlockDriverState *old_bs;
     BlockDriverState *new_bs;
-    AioContext *aio_context;
     bool overlay_appended;
 } ExternalSnapshotState;
 
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
     ExternalSnapshotState *state =
                              DO_UPCAST(ExternalSnapshotState, common, common);
     TransactionAction *action = common->action;
+    AioContext *aio_context;
 
     /* 'blockdev-snapshot' and 'blockdev-snapshot-sync' have similar
      * purpose but a different set of parameters */
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
         return;
     }
 
-    /* Acquire AioContext now so any threads operating on old_bs stop */
-    state->aio_context = bdrv_get_aio_context(state->old_bs);
-    aio_context_acquire(state->aio_context);
+    aio_context = bdrv_get_aio_context(state->old_bs);
+    aio_context_acquire(aio_context);
+
+    /* Paired with .clean() */
     bdrv_drained_begin(state->old_bs);
 
     if (!bdrv_is_inserted(state->old_bs)) {
         error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
-        return;
+        goto out;
     }
 
     if (bdrv_op_is_blocked(state->old_bs,
                            BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT, errp)) {
-        return;
+        goto out;
     }
 
     if (!bdrv_is_read_only(state->old_bs)) {
         if (bdrv_flush(state->old_bs)) {
             error_setg(errp, QERR_IO_ERROR);
-            return;
+            goto out;
         }
     }
 
     if (!bdrv_is_first_non_filter(state->old_bs)) {
         error_setg(errp, QERR_FEATURE_DISABLED, "snapshot");
-        return;
+        goto out;
     }
 
     if (action->type == TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC) {
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
 
         if (node_name && !snapshot_node_name) {
             error_setg(errp, "New snapshot node name missing");
-            return;
+            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");
-            return;
+            goto out;
         }
 
         flags = state->old_bs->open_flags;
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
             int64_t size = bdrv_getlength(state->old_bs);
             if (size < 0) {
                 error_setg_errno(errp, -size, "bdrv_getlength failed");
-                return;
+                goto out;
             }
             bdrv_img_create(new_image_file, format,
                             state->old_bs->filename,
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
                             NULL, size, flags, false, &local_err);
             if (local_err) {
                 error_propagate(errp, local_err);
-                return;
+                goto out;
             }
         }
 
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
                               errp);
     /* We will manually add the backing_hd field to the bs later */
     if (!state->new_bs) {
-        return;
+        goto out;
     }
 
     if (bdrv_has_blk(state->new_bs)) {
         error_setg(errp, "The snapshot is already in use");
-        return;
+        goto out;
     }
 
     if (bdrv_op_is_blocked(state->new_bs, BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT,
                            errp)) {
-        return;
+        goto out;
     }
 
     if (state->new_bs->backing != NULL) {
         error_setg(errp, "The snapshot already has a backing image");
-        return;
+        goto out;
     }
 
     if (!state->new_bs->drv->supports_backing) {
         error_setg(errp, "The snapshot does not support backing images");
-        return;
+        goto out;
     }
 
-    bdrv_set_aio_context(state->new_bs, state->aio_context);
+    bdrv_set_aio_context(state->new_bs, aio_context);
 
     /* This removes our old bs and adds the new bs. This is an operation that
      * can fail, so we need to do it in .prepare; undoing it for abort is
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
     bdrv_append(state->new_bs, state->old_bs, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
-        return;
+        goto out;
     }
     state->overlay_appended = true;
+
+out:
+    aio_context_release(aio_context);
 }
 
 static void external_snapshot_commit(BlkActionState *common)
 {
     ExternalSnapshotState *state =
                              DO_UPCAST(ExternalSnapshotState, common, common);
+    AioContext *aio_context;
+
+    aio_context = bdrv_get_aio_context(state->old_bs);
+    aio_context_acquire(aio_context);
 
     /* We don't need (or want) to use the transactional
      * bdrv_reopen_multiple() across all the entries at once, because we
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_commit(BlkActionState *common)
         bdrv_reopen(state->old_bs, state->old_bs->open_flags & ~BDRV_O_RDWR,
                     NULL);
     }
+
+    aio_context_release(aio_context);
 }
 
 static void external_snapshot_abort(BlkActionState *common)
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_abort(BlkActionState *common)
                              DO_UPCAST(ExternalSnapshotState, common, common);
     if (state->new_bs) {
         if (state->overlay_appended) {
+            AioContext *aio_context;
+
+            aio_context = bdrv_get_aio_context(state->old_bs);
+            aio_context_acquire(aio_context);
+
             bdrv_ref(state->old_bs);   /* we can't let bdrv_set_backind_hd()
                                           close state->old_bs; we need it */
             bdrv_set_backing_hd(state->new_bs, NULL, &error_abort);
             bdrv_replace_node(state->new_bs, state->old_bs, &error_abort);
             bdrv_unref(state->old_bs); /* bdrv_replace_node() ref'ed old_bs */
+
+            aio_context_release(aio_context);
         }
     }
 }
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_clean(BlkActionState *common)
 {
     ExternalSnapshotState *state =
                              DO_UPCAST(ExternalSnapshotState, common, common);
-    if (state->aio_context) {
-        bdrv_drained_end(state->old_bs);
-        bdrv_unref(state->new_bs);
-        aio_context_release(state->aio_context);
+    AioContext *aio_context;
+
+    if (!state->old_bs) {
+        return;
     }
+
+    aio_context = bdrv_get_aio_context(state->old_bs);
+    aio_context_acquire(aio_context);
+
+    bdrv_drained_end(state->old_bs);
+    bdrv_unref(state->new_bs);
+
+    aio_context_release(aio_context);
 }
 
 typedef struct DriveBackupState {
-- 
2.14.3

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_clean(BlkActionState *common)
 typedef struct DriveBackupState {
     BlkActionState common;
     BlockDriverState *bs;
-    AioContext *aio_context;
     BlockJob *job;
 } DriveBackupState;
 
@@ -XXX,XX +XXX,XX @@ static void drive_backup_prepare(BlkActionState *common, Error **errp)
     DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
     BlockDriverState *bs;
     DriveBackup *backup;
+    AioContext *aio_context;
     Error *local_err = NULL;
 
     assert(common->action->type == TRANSACTION_ACTION_KIND_DRIVE_BACKUP);
@@ -XXX,XX +XXX,XX @@ static void drive_backup_prepare(BlkActionState *common, Error **errp)
         return;
     }
 
-    /* AioContext is released in .clean() */
-    state->aio_context = bdrv_get_aio_context(bs);
-    aio_context_acquire(state->aio_context);
+    aio_context = bdrv_get_aio_context(bs);
+    aio_context_acquire(aio_context);
+
+    /* Paired with .clean() */
     bdrv_drained_begin(bs);
+
     state->bs = bs;
 
     state->job = do_drive_backup(backup, common->block_job_txn, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
-        return;
+        goto out;
     }
+
+out:
+    aio_context_release(aio_context);
 }
 
 static void drive_backup_commit(BlkActionState *common)
 {
     DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
+    AioContext *aio_context;
+
+    aio_context = bdrv_get_aio_context(state->bs);
+    aio_context_acquire(aio_context);
+
     assert(state->job);
     block_job_start(state->job);
+
+    aio_context_release(aio_context);
 }
 
 static void drive_backup_abort(BlkActionState *common)
@@ -XXX,XX +XXX,XX @@ static void drive_backup_abort(BlkActionState *common)
     DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
 
     if (state->job) {
+        AioContext *aio_context;
+
+        aio_context = bdrv_get_aio_context(state->bs);
+        aio_context_acquire(aio_context);
+
         block_job_cancel_sync(state->job);
+
+        aio_context_release(aio_context);
     }
 }
 
 static void drive_backup_clean(BlkActionState *common)
 {
     DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
+    AioContext *aio_context;
 
-    if (state->aio_context) {
-        bdrv_drained_end(state->bs);
-        aio_context_release(state->aio_context);
+    if (!state->bs) {
+        return;
     }
+
+    aio_context = bdrv_get_aio_context(state->bs);
+    aio_context_acquire(aio_context);
+
+    bdrv_drained_end(state->bs);
+
+    aio_context_release(aio_context);
 }
 
 typedef struct BlockdevBackupState {
-- 
2.14.3

diff --git a/blockdev.c b/blockdev.c
index XXXXXXX..XXXXXXX 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -XXX,XX +XXX,XX @@ typedef struct BlockdevBackupState {
     BlkActionState common;
     BlockDriverState *bs;
     BlockJob *job;
-    AioContext *aio_context;
 } BlockdevBackupState;
 
 static BlockJob *do_blockdev_backup(BlockdevBackup *backup, BlockJobTxn *txn,
@@ -XXX,XX +XXX,XX @@ static void blockdev_backup_prepare(BlkActionState *common, Error **errp)
     BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);
     BlockdevBackup *backup;
     BlockDriverState *bs, *target;
+    AioContext *aio_context;
     Error *local_err = NULL;
 
     assert(common->action->type == TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP);
@@ -XXX,XX +XXX,XX @@ static void blockdev_backup_prepare(BlkActionState *common, Error **errp)
         return;
     }
 
-    /* AioContext is released in .clean() */
-    state->aio_context = bdrv_get_aio_context(bs);
-    if (state->aio_context != bdrv_get_aio_context(target)) {
-        state->aio_context = NULL;
+    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(state->aio_context);
+    aio_context_acquire(aio_context);
     state->bs = bs;
+
+    /* Paired with .clean() */
     bdrv_drained_begin(state->bs);
 
     state->job = do_blockdev_backup(backup, common->block_job_txn, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
-        return;
+        goto out;
     }
+
+out:
+    aio_context_release(aio_context);
 }
 
 static void blockdev_backup_commit(BlkActionState *common)
 {
     BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);
+    AioContext *aio_context;
+
+    aio_context = bdrv_get_aio_context(state->bs);
+    aio_context_acquire(aio_context);
+
     assert(state->job);
     block_job_start(state->job);
+
+    aio_context_release(aio_context);
 }
 
 static void blockdev_backup_abort(BlkActionState *common)
@@ -XXX,XX +XXX,XX @@ static void blockdev_backup_abort(BlkActionState *common)
     BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);
 
     if (state->job) {
+        AioContext *aio_context;
+
+        aio_context = bdrv_get_aio_context(state->bs);
+        aio_context_acquire(aio_context);
+
         block_job_cancel_sync(state->job);
+
+        aio_context_release(aio_context);
     }
 }
 
 static void blockdev_backup_clean(BlkActionState *common)
 {
     BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);
+    AioContext *aio_context;
 
-    if (state->aio_context) {
-        bdrv_drained_end(state->bs);
-        aio_context_release(state->aio_context);
+    if (!state->bs) {
+        return;
     }
+
+    aio_context = bdrv_get_aio_context(state->bs);
+    aio_context_acquire(aio_context);
+
+    bdrv_drained_end(state->bs);
+
+    aio_context_release(aio_context);
 }
 
 typedef struct BlockDirtyBitmapState {
-- 
2.14.3

diff --git a/blockdev.c b/blockdev.c
index XXXXXXX..XXXXXXX 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -XXX,XX +XXX,XX @@ struct BlkActionState {
 typedef struct InternalSnapshotState {
     BlkActionState common;
     BlockDriverState *bs;
-    AioContext *aio_context;
     QEMUSnapshotInfo sn;
     bool created;
 } InternalSnapshotState;
@@ -XXX,XX +XXX,XX @@ static void internal_snapshot_prepare(BlkActionState *common,
     qemu_timeval tv;
     BlockdevSnapshotInternal *internal;
     InternalSnapshotState *state;
+    AioContext *aio_context;
     int ret1;
 
     g_assert(common->action->type ==
@@ -XXX,XX +XXX,XX @@ static void internal_snapshot_prepare(BlkActionState *common,
         return;
     }
 
-    /* AioContext is released in .clean() */
-    state->aio_context = bdrv_get_aio_context(bs);
-    aio_context_acquire(state->aio_context);
+    aio_context = bdrv_get_aio_context(bs);
+    aio_context_acquire(aio_context);
 
     state->bs = bs;
+
+    /* Paired with .clean() */
     bdrv_drained_begin(bs);
 
     if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_INTERNAL_SNAPSHOT, errp)) {
-        return;
+        goto out;
     }
 
     if (bdrv_is_read_only(bs)) {
         error_setg(errp, "Device '%s' is read only", device);
-        return;
+        goto out;
     }
 
     if (!bdrv_can_snapshot(bs)) {
         error_setg(errp, "Block format '%s' used by device '%s' "
                    "does not support internal snapshots",
                    bs->drv->format_name, device);
-        return;
+        goto out;
     }
 
     if (!strlen(name)) {
         error_setg(errp, "Name is empty");
-        return;
+        goto out;
     }
 
     /* check whether a snapshot with name exist */
@@ -XXX,XX +XXX,XX @@ static void internal_snapshot_prepare(BlkActionState *common,
                                             &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
-        return;
+        goto out;
     } else if (ret) {
         error_setg(errp,
                    "Snapshot with name '%s' already exists on device '%s'",
                    name, device);
-        return;
+        goto out;
     }
 
     /* 3. take the snapshot */
@@ -XXX,XX +XXX,XX @@ static void internal_snapshot_prepare(BlkActionState *common,
         error_setg_errno(errp, -ret1,
                          "Failed to create snapshot '%s' on device '%s'",
                          name, device);
-        return;
+        goto out;
     }
 
     /* 4. succeed, mark a snapshot is created */
     state->created = true;
+
+out:
+    aio_context_release(aio_context);
 }
 
 static void internal_snapshot_abort(BlkActionState *common)
@@ -XXX,XX +XXX,XX @@ static void internal_snapshot_abort(BlkActionState *common)
                              DO_UPCAST(InternalSnapshotState, common, common);
     BlockDriverState *bs = state->bs;
     QEMUSnapshotInfo *sn = &state->sn;
+    AioContext *aio_context;
     Error *local_error = NULL;
 
     if (!state->created) {
         return;
     }
 
+    aio_context = bdrv_get_aio_context(state->bs);
+    aio_context_acquire(aio_context);
+
     if (bdrv_snapshot_delete(bs, sn->id_str, sn->name, &local_error) < 0) {
         error_reportf_err(local_error,
                           "Failed to delete snapshot with id '%s' and "
@@ -XXX,XX +XXX,XX @@ static void internal_snapshot_abort(BlkActionState *common)
                           sn->id_str, sn->name,
                           bdrv_get_device_name(bs));
     }
+
+    aio_context_release(aio_context);
 }
 
 static void internal_snapshot_clean(BlkActionState *common)
 {
     InternalSnapshotState *state = DO_UPCAST(InternalSnapshotState,
                                              common, common);
+    AioContext *aio_context;
 
-    if (state->aio_context) {
-        if (state->bs) {
-            bdrv_drained_end(state->bs);
-        }
-        aio_context_release(state->aio_context);
+    if (!state->bs) {
+        return;
     }
+
+    aio_context = bdrv_get_aio_context(state->bs);
+    aio_context_acquire(aio_context);
+
+    bdrv_drained_end(state->bs);
+
+    aio_context_release(aio_context);
 }
 
 /* external snapshot private data */
-- 
2.14.3

The dirty bitmap actions in qmp_transaction have not used AioContext
since the dirty bitmap locking discipline was introduced in commit
2119882c7eb7e2c612b24fc0c8d86f5887d6f1c3 ("block: introduce
dirty_bitmap_mutex").  Remove the unused field.

diff --git a/blockdev.c b/blockdev.c
index XXXXXXX..XXXXXXX 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -XXX,XX +XXX,XX @@ typedef struct BlockDirtyBitmapState {
     BlkActionState common;
     BdrvDirtyBitmap *bitmap;
     BlockDriverState *bs;
-    AioContext *aio_context;
     HBitmap *backup;
     bool prepared;
 } BlockDirtyBitmapState;
@@ -XXX,XX +XXX,XX @@ static void block_dirty_bitmap_clear_prepare(BlkActionState *common,
     }
 
     bdrv_clear_dirty_bitmap(state->bitmap, &state->backup);
-    /* AioContext is released in .clean() */
 }
 
 static void block_dirty_bitmap_clear_abort(BlkActionState *common)
@@ -XXX,XX +XXX,XX @@ static void block_dirty_bitmap_clear_commit(BlkActionState *common)
     hbitmap_free(state->backup);
 }
 
-static void block_dirty_bitmap_clear_clean(BlkActionState *common)
-{
-    BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
-                                             common, common);
-
-    if (state->aio_context) {
-        aio_context_release(state->aio_context);
-    }
-}
-
 static void abort_prepare(BlkActionState *common, Error **errp)
 {
     error_setg(errp, "Transaction aborted using Abort action");
@@ -XXX,XX +XXX,XX @@ static const BlkActionOps actions[] = {
         .prepare = block_dirty_bitmap_clear_prepare,
         .commit = block_dirty_bitmap_clear_commit,
         .abort = block_dirty_bitmap_clear_abort,
-        .clean = block_dirty_bitmap_clear_clean,
     }
 };
 
-- 
2.14.3

Encapsulate IOThread QOM object lookup so that callers don't need to
know how and where IOThread objects live.

diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
index XXXXXXX..XXXXXXX 100644
--- a/include/sysemu/iothread.h
+++ b/include/sysemu/iothread.h
@@ -XXX,XX +XXX,XX @@ typedef struct {
    OBJECT_CHECK(IOThread, obj, TYPE_IOTHREAD)
 
 char *iothread_get_id(IOThread *iothread);
+IOThread *iothread_by_id(const char *id);
 AioContext *iothread_get_aio_context(IOThread *iothread);
 void iothread_stop_all(void);
 GMainContext *iothread_get_g_main_context(IOThread *iothread);
diff --git a/iothread.c b/iothread.c
index XXXXXXX..XXXXXXX 100644
--- a/iothread.c
+++ b/iothread.c
@@ -XXX,XX +XXX,XX @@ void iothread_destroy(IOThread *iothread)
 {
     object_unparent(OBJECT(iothread));
 }
+
+/* Lookup IOThread by its id.  Only finds user-created objects, not internal
+ * iothread_create() objects. */
+IOThread *iothread_by_id(const char *id)
+{
+    return IOTHREAD(object_resolve_path_type(id, TYPE_IOTHREAD, NULL));
+}
-- 
2.14.3

Currently there is no easy way for iotests to ensure that a BDS is bound
to a particular IOThread.  Normally the virtio-blk device calls
blk_set_aio_context() when dataplane is enabled during guest driver
initialization.  This never happens in iotests since -machine
accel=qtest means there is no guest activity (including device driver
initialization).

This patch adds a QMP command to explicitly assign IOThreads in test
cases.  See qapi/block-core.json for a description of the command.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20171206144550.22295-9-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 qapi/block-core.json | 36 ++++++++++++++++++++++++++++++++++++
 blockdev.c           | 41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index XXXXXXX..XXXXXXX 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -XXX,XX +XXX,XX @@
   'data' : { 'parent': 'str',
              '*child': 'str',
              '*node': 'str' } }
+
+##
+# @x-blockdev-set-iothread:
+#
+# Move @node and its children into the @iothread.  If @iothread is null then
+# move @node and its children into the main loop.
+#
+# The node must not be attached to a BlockBackend.
+#
+# @node-name: the name of the block driver node
+#
+# @iothread: the name of the IOThread object or null for the main loop
+#
+# Note: this command is experimental and intended for test cases that need
+# control over IOThreads only.
+#
+# Since: 2.12
+#
+# Example:
+#
+# 1. Move a node into an IOThread
+# -> { "execute": "x-blockdev-set-iothread",
+#      "arguments": { "node-name": "disk1",
+#                     "iothread": "iothread0" } }
+# <- { "return": {} }
+#
+# 2. Move a node into the main loop
+# -> { "execute": "x-blockdev-set-iothread",
+#      "arguments": { "node-name": "disk1",
+#                     "iothread": null } }
+# <- { "return": {} }
+#
+##
+{ 'command': 'x-blockdev-set-iothread',
+  'data' : { 'node-name': 'str',
+             'iothread': 'StrOrNull' } }
diff --git a/blockdev.c b/blockdev.c
index XXXXXXX..XXXXXXX 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -XXX,XX +XXX,XX @@
 #include "qapi/qmp/qerror.h"
 #include "qapi/qobject-output-visitor.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/iothread.h"
 #include "block/block_int.h"
 #include "qmp-commands.h"
 #include "block/trace.h"
@@ -XXX,XX +XXX,XX @@ BlockJobInfoList *qmp_query_block_jobs(Error **errp)
     return head;
 }
 
+void qmp_x_blockdev_set_iothread(const char *node_name, StrOrNull *iothread,
+                                 Error **errp)
+{
+    AioContext *old_context;
+    AioContext *new_context;
+    BlockDriverState *bs;
+
+    bs = bdrv_find_node(node_name);
+    if (!bs) {
+        error_setg(errp, "Cannot find node %s", node_name);
+        return;
+    }
+
+    /* If we want to allow more extreme test scenarios this guard could be
+     * removed.  For now it protects against accidents. */
+    if (bdrv_has_blk(bs)) {
+        error_setg(errp, "Node %s is in use", node_name);
+        return;
+    }
+
+    if (iothread->type == QTYPE_QSTRING) {
+        IOThread *obj = iothread_by_id(iothread->u.s);
+        if (!obj) {
+            error_setg(errp, "Cannot find iothread %s", iothread->u.s);
+            return;
+        }
+
+        new_context = iothread_get_aio_context(obj);
+    } else {
+        new_context = qemu_get_aio_context();
+    }
+
+    old_context = bdrv_get_aio_context(bs);
+    aio_context_acquire(old_context);
+
+    bdrv_set_aio_context(bs, new_context);
+
+    aio_context_release(old_context);
+}
+
 QemuOptsList qemu_common_drive_opts = {
     .name = "drive",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_common_drive_opts.head),
-- 
2.14.3

QMP 'transaction' blockdev-snapshot-sync with multiple disks in an
IOThread is an untested code path.  Several bugs have been found in
connection with this command.  This patch adds a test case to prevent
future regressions.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20171206144550.22295-10-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qemu-iotests/202     | 95 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/202.out | 11 ++++++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 107 insertions(+)
 create mode 100755 tests/qemu-iotests/202
 create mode 100644 tests/qemu-iotests/202.out

diff --git a/tests/qemu-iotests/202 b/tests/qemu-iotests/202
new file mode 100755
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qemu-iotests/202
@@ -XXX,XX +XXX,XX @@
+#!/usr/bin/env python
+#
+# Copyright (C) 2017 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: Stefan Hajnoczi <stefanha@redhat.com>
+#
+# Check that QMP 'transaction' blockdev-snapshot-sync with multiple drives on a
+# single IOThread completes successfully.  This particular command triggered a
+# hang due to recursive AioContext locking and BDRV_POLL_WHILE().  Protect
+# against regressions.
+
+import iotests
+
+iotests.verify_image_format(supported_fmts=['qcow2'])
+iotests.verify_platform(['linux'])
+
+with iotests.FilePath('disk0.img') as disk0_img_path, \
+     iotests.FilePath('disk1.img') as disk1_img_path, \
+     iotests.FilePath('disk0-snap.img') as disk0_snap_img_path, \
+     iotests.FilePath('disk1-snap.img') as disk1_snap_img_path, \
+     iotests.VM() as vm:
+
+    img_size = '10M'
+    iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, disk0_img_path, img_size)
+    iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, disk1_img_path, img_size)
+
+    iotests.log('Launching VM...')
+    vm.launch()
+
+    iotests.log('Adding IOThread...')
+    iotests.log(vm.qmp('object-add',
+                       qom_type='iothread',
+                       id='iothread0'))
+
+    iotests.log('Adding blockdevs...')
+    iotests.log(vm.qmp('blockdev-add',
+                       driver=iotests.imgfmt,
+                       node_name='disk0',
+                       file={
+                           'driver': 'file',
+                           'filename': disk0_img_path,
+                       }))
+    iotests.log(vm.qmp('blockdev-add',
+                       driver=iotests.imgfmt,
+                       node_name='disk1',
+                       file={
+                           'driver': 'file',
+                           'filename': disk1_img_path,
+                       }))
+
+    iotests.log('Setting iothread...')
+    iotests.log(vm.qmp('x-blockdev-set-iothread',
+                       node_name='disk0',
+                       iothread='iothread0'))
+    iotests.log(vm.qmp('x-blockdev-set-iothread',
+                       node_name='disk1',
+                       iothread='iothread0'))
+
+    iotests.log('Creating external snapshots...')
+    iotests.log(vm.qmp(
+                  'transaction',
+                  actions=[
+                      {
+                          'data': {
+                              'node-name': 'disk0',
+                              'snapshot-file': disk0_snap_img_path,
+                              'snapshot-node-name': 'disk0-snap',
+                              'mode': 'absolute-paths',
+                              'format': iotests.imgfmt,
+                          },
+                          'type': 'blockdev-snapshot-sync'
+                      }, {
+                          'data': {
+                              'node-name': 'disk1',
+                              'snapshot-file': disk1_snap_img_path,
+                              'snapshot-node-name': 'disk1-snap',
+                              'mode': 'absolute-paths',
+                              'format': iotests.imgfmt
+                          },
+                          'type': 'blockdev-snapshot-sync'
+                      }
+                  ]))
diff --git a/tests/qemu-iotests/202.out b/tests/qemu-iotests/202.out
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qemu-iotests/202.out
@@ -XXX,XX +XXX,XX @@
+Launching VM...
+Adding IOThread...
+{u'return': {}}
+Adding blockdevs...
+{u'return': {}}
+{u'return': {}}
+Setting iothread...
+{u'return': {}}
+{u'return': {}}
+Creating external snapshots...
+{u'return': {}}
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 @@
 197 rw auto quick
 198 rw auto
 200 rw auto
+202 rw auto quick
-- 
2.14.3

From: Mark Kanda <mark.kanda@oracle.com>

Depending on the configuration, it can be beneficial to adjust the virtio-blk
queue size to something other than the current default of 128. Add a new
property to make the queue size configurable.

Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com>
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Reviewed-by: Ameya More <ameya.more@oracle.com>
Message-id: 52e6d742811f10dbd16e996e86cf375b9577c187.1513005190.git.mark.kanda@oracle.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/hw/virtio/virtio-blk.h |  1 +
 hw/block/virtio-blk.c          | 10 +++++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index XXXXXXX..XXXXXXX 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -XXX,XX +XXX,XX @@ struct VirtIOBlkConf
     uint32_t config_wce;
     uint32_t request_merging;
     uint16_t num_queues;
+    uint16_t queue_size;
 };
 
 struct VirtIOBlockDataPlane;
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -XXX,XX +XXX,XX @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
         error_setg(errp, "num-queues property must be larger than 0");
         return;
     }
+    if (!is_power_of_2(conf->queue_size) ||
+        conf->queue_size > VIRTQUEUE_MAX_SIZE) {
+        error_setg(errp, "invalid queue-size property (%" PRIu16 "), "
+                   "must be a power of 2 (max %d)",
+                   conf->queue_size, VIRTQUEUE_MAX_SIZE);
+        return;
+    }
 
     blkconf_serial(&conf->conf, &conf->serial);
     if (!blkconf_apply_backend_options(&conf->conf,
@@ -XXX,XX +XXX,XX @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
     s->sector_mask = (s->conf.conf.logical_block_size / BDRV_SECTOR_SIZE) - 1;
 
     for (i = 0; i < conf->num_queues; i++) {
-        virtio_add_queue(vdev, 128, virtio_blk_handle_output);
+        virtio_add_queue(vdev, conf->queue_size, virtio_blk_handle_output);
     }
     virtio_blk_data_plane_create(vdev, conf, &s->dataplane, &err);
     if (err != NULL) {
@@ -XXX,XX +XXX,XX @@ static Property virtio_blk_properties[] = {
     DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0,
                     true),
     DEFINE_PROP_UINT16("num-queues", VirtIOBlock, conf.num_queues, 1),
+    DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 128),
     DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD,
                      IOThread *),
     DEFINE_PROP_END_OF_LIST(),
-- 
2.14.3

From: Mark Kanda <mark.kanda@oracle.com>

virtio-blk logical block size should never be larger than physical block
size because it doesn't make sense to have such configurations. QEMU doesn't
have a way to effectively express this condition; the best it can do is
report the physical block exponent as 0 - indicating the logical block size
equals the physical block size.

This is identical to commit 3da023b5827543ee4c022986ea2ad9d1274410b2
but applied to virtio-blk (instead of virtio-scsi).

Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Ameya More <ameya.more@oracle.com>
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 773169891f9f2deb4cb7c4ef2655580dbe24c1d1.1513005190.git.mark.kanda@oracle.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/block/virtio-blk.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -XXX,XX +XXX,XX @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
 
     blkconf_blocksizes(&conf->conf);
 
+    if (conf->conf.logical_block_size >
+        conf->conf.physical_block_size) {
+        error_setg(errp,
+                   "logical_block_size > physical_block_size not supported");
+        return;
+    }
+
     virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
                 sizeof(struct virtio_blk_config));
 
-- 
2.14.3

From: Paolo Bonzini <pbonzini@redhat.com>

BDRV_POLL_WHILE() does not support recursive AioContext locking.  It
only releases the AioContext lock once regardless of how many times the
caller has acquired it.  This results in a hang since the IOThread does
not make progress while the AioContext is still locked.

The following steps trigger the hang:

$ qemu-system-x86_64 -M accel=kvm -m 1G -cpu host \
                       -object iothread,id=iothread0 \
                       -device virtio-scsi-pci,iothread=iothread0 \
                       -drive if=none,id=drive0,file=test.img,format=raw \
                       -device scsi-hd,drive=drive0 \
                       -drive if=none,id=drive1,file=test.img,format=raw \
                       -device scsi-hd,drive=drive1
  $ qemu-system-x86_64 ...same options... \
                       -incoming tcp::1234
  (qemu) migrate tcp:127.0.0.1:1234
  ...hang...

Tested-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20171207201320.19284-2-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index XXXXXXX..XXXXXXX 100644
--- a/block.c
+++ b/block.c
@@ -XXX,XX +XXX,XX @@ int bdrv_inactivate_all(void)
     BdrvNextIterator it;
     int ret = 0;
     int pass;
+    GSList *aio_ctxs = NULL, *ctx;
 
     for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
-        aio_context_acquire(bdrv_get_aio_context(bs));
+        AioContext *aio_context = bdrv_get_aio_context(bs);
+
+        if (!g_slist_find(aio_ctxs, aio_context)) {
+            aio_ctxs = g_slist_prepend(aio_ctxs, aio_context);
+            aio_context_acquire(aio_context);
+        }
     }
 
     /* We do two passes of inactivation. The first pass calls to drivers'
@@ -XXX,XX +XXX,XX @@ int bdrv_inactivate_all(void)
     }
 
 out:
-    for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
-        aio_context_release(bdrv_get_aio_context(bs));
+    for (ctx = aio_ctxs; ctx != NULL; ctx = ctx->next) {
+        AioContext *aio_context = ctx->data;
+        aio_context_release(aio_context);
     }
+    g_slist_free(aio_ctxs);
 
     return ret;
 }
-- 
2.14.3

See the patch for why nested AioContext locking is no longer allowed.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20171207201320.19284-3-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 docs/devel/multiple-iothreads.txt | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/docs/devel/multiple-iothreads.txt b/docs/devel/multiple-iothreads.txt
index XXXXXXX..XXXXXXX 100644
--- a/docs/devel/multiple-iothreads.txt
+++ b/docs/devel/multiple-iothreads.txt
@@ -XXX,XX +XXX,XX @@
-Copyright (c) 2014 Red Hat Inc.
+Copyright (c) 2014-2017 Red Hat Inc.
 
 This work is licensed under the terms of the GNU GPL, version 2 or later.  See
 the COPYING file in the top-level directory.
@@ -XXX,XX +XXX,XX @@ aio_context_acquire()/aio_context_release() for mutual exclusion.  Once the
 context is acquired no other thread can access it or run event loop iterations
 in this AioContext.
 
-aio_context_acquire()/aio_context_release() calls may be nested.  This
-means you can call them if you're not sure whether #2 applies.
+Legacy code sometimes nests aio_context_acquire()/aio_context_release() calls.
+Do not use nesting anymore, it is incompatible with the BDRV_POLL_WHILE() macro
+used in the block layer and can lead to hangs.
 
 There is currently no lock ordering rule if a thread needs to acquire multiple
 AioContexts simultaneously.  Therefore, it is only safe for code holding the
-- 
2.14.3

When a node is already associated with a BlockBackend the
x-blockdev-set-iothread command refuses to set the IOThread.  This is to
prevent accidentally changing the IOThread when the nodes are in use.

When the nodes are created with -drive they automatically get a
BlockBackend.  In that case we know nothing is using them yet and it's
safe to set the IOThread.  Add a force boolean to override the check.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20171207201320.19284-4-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 qapi/block-core.json |  6 +++++-
 blockdev.c           | 11 ++++++-----
 2 files changed, 11 insertions(+), 6 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 @@
 #
 # @iothread: the name of the IOThread object or null for the main loop
 #
+# @force: true if the node and its children should be moved when a BlockBackend
+#         is already attached
+#
 # Note: this command is experimental and intended for test cases that need
 # control over IOThreads only.
 #
@@ -XXX,XX +XXX,XX @@
 ##
 { 'command': 'x-blockdev-set-iothread',
   'data' : { 'node-name': 'str',
-             'iothread': 'StrOrNull' } }
+             'iothread': 'StrOrNull',
+             '*force': 'bool' } }
diff --git a/blockdev.c b/blockdev.c
index XXXXXXX..XXXXXXX 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -XXX,XX +XXX,XX @@ BlockJobInfoList *qmp_query_block_jobs(Error **errp)
 }
 
 void qmp_x_blockdev_set_iothread(const char *node_name, StrOrNull *iothread,
-                                 Error **errp)
+                                 bool has_force, bool force, Error **errp)
 {
     AioContext *old_context;
     AioContext *new_context;
@@ -XXX,XX +XXX,XX @@ void qmp_x_blockdev_set_iothread(const char *node_name, StrOrNull *iothread,
         return;
     }
 
-    /* If we want to allow more extreme test scenarios this guard could be
-     * removed.  For now it protects against accidents. */
-    if (bdrv_has_blk(bs)) {
-        error_setg(errp, "Node %s is in use", node_name);
+    /* Protects against accidents. */
+    if (!(has_force && force) && bdrv_has_blk(bs)) {
+        error_setg(errp, "Node %s is associated with a BlockBackend and could "
+                         "be in use (use force=true to override this check)",
+                         node_name);
         return;
     }
 
-- 
2.14.3

There is a small chance that iothread_stop() hangs as follows:

Thread 3 (Thread 0x7f63eba5f700 (LWP 16105)):
  #0  0x00007f64012c09b6 in ppoll () at /lib64/libc.so.6
  #1  0x000055959992eac9 in ppoll (__ss=0x0, __timeout=0x0, __nfds=<optimized out>, __fds=<optimized out>) at /usr/include/bits/poll2.h:77
  #2  0x000055959992eac9 in qemu_poll_ns (fds=<optimized out>, nfds=<optimized out>, timeout=<optimized out>) at util/qemu-timer.c:322
  #3  0x0000559599930711 in aio_poll (ctx=0x55959bdb83c0, blocking=blocking@entry=true) at util/aio-posix.c:629
  #4  0x00005595996806fe in iothread_run (opaque=0x55959bd78400) at iothread.c:59
  #5  0x00007f640159f609 in start_thread () at /lib64/libpthread.so.0
  #6  0x00007f64012cce6f in clone () at /lib64/libc.so.6

Thread 1 (Thread 0x7f640b45b280 (LWP 16103)):
  #0  0x00007f64015a0b6d in pthread_join () at /lib64/libpthread.so.0
  #1  0x00005595999332ef in qemu_thread_join (thread=<optimized out>) at util/qemu-thread-posix.c:547
  #2  0x00005595996808ae in iothread_stop (iothread=<optimized out>) at iothread.c:91
  #3  0x000055959968094d in iothread_stop_iter (object=<optimized out>, opaque=<optimized out>) at iothread.c:102
  #4  0x0000559599857d97 in do_object_child_foreach (obj=obj@entry=0x55959bdb8100, fn=fn@entry=0x559599680930 <iothread_stop_iter>, opaque=opaque@entry=0x0, recurse=recurse@entry=false) at qom/object.c:852
  #5  0x0000559599859477 in object_child_foreach (obj=obj@entry=0x55959bdb8100, fn=fn@entry=0x559599680930 <iothread_stop_iter>, opaque=opaque@entry=0x0) at qom/object.c:867
  #6  0x0000559599680a6e in iothread_stop_all () at iothread.c:341
  #7  0x000055959955b1d5 in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at vl.c:4913

The relevant code from iothread_run() is:

while (!atomic_read(&iothread->stopping)) {
      aio_poll(iothread->ctx, true);

and iothread_stop():

iothread->stopping = true;
  aio_notify(iothread->ctx);
  ...
  qemu_thread_join(&iothread->thread);

The following scenario can occur:

1. IOThread:
  while (!atomic_read(&iothread->stopping)) -> stopping=false

2. Main loop:
  iothread->stopping = true;
  aio_notify(iothread->ctx);

3. IOThread:
  aio_poll(iothread->ctx, true); -> hang

The bug is explained by the AioContext->notify_me doc comments:

"If this field is 0, everything (file descriptors, bottom halves,
  timers) will be re-evaluated before the next blocking poll(), thus the
  event_notifier_set call can be skipped."

The problem is that "everything" does not include checking
iothread->stopping.  This means iothread_run() will block in aio_poll()
if aio_notify() was called just before aio_poll().

This patch fixes the hang by replacing aio_notify() with
aio_bh_schedule_oneshot().  This makes aio_poll() or g_main_loop_run()
to return.

Implementing this properly required a new bool running flag.  The new
flag prevents races that are tricky if we try to use iothread->stopping.
Now iothread->stopping is purely for iothread_stop() and
iothread->running is purely for the iothread_run() thread.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20171207201320.19284-6-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/sysemu/iothread.h |  3 ++-
 iothread.c                | 20 +++++++++++++++-----
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
index XXXXXXX..XXXXXXX 100644
--- a/include/sysemu/iothread.h
+++ b/include/sysemu/iothread.h
@@ -XXX,XX +XXX,XX @@ typedef struct {
     GOnce once;
     QemuMutex init_done_lock;
     QemuCond init_done_cond;    /* is thread initialization done? */
-    bool stopping;
+    bool stopping;              /* has iothread_stop() been called? */
+    bool running;               /* should iothread_run() continue? */
     int thread_id;
 
     /* AioContext poll parameters */
diff --git a/iothread.c b/iothread.c
index XXXXXXX..XXXXXXX 100644
--- a/iothread.c
+++ b/iothread.c
@@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque)
     qemu_cond_signal(&iothread->init_done_cond);
     qemu_mutex_unlock(&iothread->init_done_lock);
 
-    while (!atomic_read(&iothread->stopping)) {
+    while (iothread->running) {
         aio_poll(iothread->ctx, true);
 
         if (atomic_read(&iothread->worker_context)) {
@@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque)
     return NULL;
 }
 
+/* Runs in iothread_run() thread */
+static void iothread_stop_bh(void *opaque)
+{
+    IOThread *iothread = opaque;
+
+    iothread->running = false; /* stop iothread_run() */
+
+    if (iothread->main_loop) {
+        g_main_loop_quit(iothread->main_loop);
+    }
+}
+
 void iothread_stop(IOThread *iothread)
 {
     if (!iothread->ctx || iothread->stopping) {
         return;
     }
     iothread->stopping = true;
-    aio_notify(iothread->ctx);
-    if (atomic_read(&iothread->main_loop)) {
-        g_main_loop_quit(iothread->main_loop);
-    }
+    aio_bh_schedule_oneshot(iothread->ctx, iothread_stop_bh, iothread);
     qemu_thread_join(&iothread->thread);
 }
 
@@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp)
     char *name, *thread_name;
 
     iothread->stopping = false;
+    iothread->running = true;
     iothread->thread_id = -1;
     iothread->ctx = aio_context_new(&local_error);
     if (!iothread->ctx) {
-- 
2.14.3

This test case will prevent future regressions with savevm and
IOThreads.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20171207201320.19284-7-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qemu-iotests/203     | 59 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/203.out |  6 +++++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 66 insertions(+)
 create mode 100755 tests/qemu-iotests/203
 create mode 100644 tests/qemu-iotests/203.out

diff --git a/tests/qemu-iotests/203 b/tests/qemu-iotests/203
new file mode 100755
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qemu-iotests/203
@@ -XXX,XX +XXX,XX @@
+#!/usr/bin/env python
+#
+# Copyright (C) 2017 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: Stefan Hajnoczi <stefanha@redhat.com>
+#
+# Check that QMP 'migrate' with multiple drives on a single IOThread completes
+# successfully.  This particular command triggered a hang in the source QEMU
+# process due to recursive AioContext locking in bdrv_invalidate_all() and
+# BDRV_POLL_WHILE().
+
+import iotests
+
+iotests.verify_image_format(supported_fmts=['qcow2'])
+iotests.verify_platform(['linux'])
+
+with iotests.FilePath('disk0.img') as disk0_img_path, \
+     iotests.FilePath('disk1.img') as disk1_img_path, \
+     iotests.VM() as vm:
+
+    img_size = '10M'
+    iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, disk0_img_path, img_size)
+    iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, disk1_img_path, img_size)
+
+    iotests.log('Launching VM...')
+    (vm.add_object('iothread,id=iothread0')
+       .add_drive(disk0_img_path, 'node-name=drive0-node', interface='none')
+       .add_drive(disk1_img_path, 'node-name=drive1-node', interface='none')
+       .launch())
+
+    iotests.log('Setting IOThreads...')
+    iotests.log(vm.qmp('x-blockdev-set-iothread',
+                       node_name='drive0-node', iothread='iothread0',
+                       force=True))
+    iotests.log(vm.qmp('x-blockdev-set-iothread',
+                       node_name='drive1-node', iothread='iothread0',
+                       force=True))
+
+    iotests.log('Starting migration...')
+    iotests.log(vm.qmp('migrate', uri='exec:cat >/dev/null'))
+    while True:
+        vm.get_qmp_event(wait=60.0)
+        result = vm.qmp('query-migrate')
+        status = result.get('return', {}).get('status', None)
+        if status == 'completed':
+            break
diff --git a/tests/qemu-iotests/203.out b/tests/qemu-iotests/203.out
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qemu-iotests/203.out
@@ -XXX,XX +XXX,XX @@
+Launching VM...
+Setting IOThreads...
+{u'return': {}}
+{u'return': {}}
+Starting migration...
+{u'return': {}}
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 @@
 198 rw auto
 200 rw auto
 202 rw auto quick
+203 rw auto
-- 
2.14.3