[Qemu-devel] [PULL 00/23] Block patches

[Qemu-devel] [PULL 00/23] Block patches

[Stefan Hajnoczi]

The following changes since commit 411ad78115ebeb3411cf4b7622784b93dfabe259:

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

are available in the Git repository at:

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

for you to fetch changes up to 585426c518958aa768564596091474be786aae51:

  qemu-iotests: add 203 savevm with IOThreads test (2017-12-18 13:12:53 +0000)

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

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

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

 docs/devel/multiple-iothreads.txt |   7 +-
 qapi/block-core.json              |  40 ++++++
 hw/block/dataplane/virtio-blk.h   |   2 +-
 include/hw/block/block.h          |   4 +-
 include/hw/virtio/virtio-blk.h    |   1 +
 include/qemu/coroutine.h          |   6 +-
 include/sysemu/iothread.h         |   4 +-
 block.c                           |  14 ++-
 block/null.c                      |   3 +-
 block/sheepdog.c                  |   3 +-
 blockdev.c                        | 259 +++++++++++++++++++++++++++-----------
 hw/block/block.c                  |  15 ++-
 hw/block/dataplane/virtio-blk.c   |  12 +-
 hw/block/fdc.c                    |  17 +--
 hw/block/nvme.c                   |  23 ++--
 hw/block/virtio-blk.c             |  35 ++++--
 hw/core/qdev-properties-system.c  |   1 -
 hw/ide/qdev.c                     |  12 +-
 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 +
 28 files changed, 532 insertions(+), 177 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

-- 
2.14.3

[Qemu-devel] [PULL 01/23] coroutine: simplify co_aio_sleep_ns() prototype

[Stefan Hajnoczi]

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 9aff9a735e..ce2eb73670 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -261,12 +261,8 @@ 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 dd9c13f9ba..0cdabaa440 100644
--- a/block/null.c
+++ b/block/null.c
@@ -110,8 +110,7 @@ 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 696a71442a..a1edb992ff 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -776,8 +776,7 @@ 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 254349cdbb..afb678fbe5 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -31,9 +31,9 @@ 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

[Qemu-devel] [PULL 02/23] hw/block/nvme: Convert to realize

[Stefan Hajnoczi]

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 441e21ed1f..e530ba7a30 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -920,7 +920,7 @@ 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;
@@ -931,24 +931,27 @@ 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;
@@ -1046,7 +1049,6 @@ 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)
@@ -1081,7 +1083,7 @@ 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

[Qemu-devel] [PULL 03/23] hw/block: Fix the return type

[Stefan Hajnoczi]

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 db3f47b173..5e18bb99ae 100644
--- a/hw/block/dataplane/virtio-blk.h
+++ b/hw/block/dataplane/virtio-blk.h
@@ -19,7 +19,7 @@
 
 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 f3f6e8ef02..64b9298829 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -72,11 +72,11 @@ 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 27878d0087..b0269c857f 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -51,7 +51,7 @@ 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;
@@ -76,7 +76,7 @@ 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) {
@@ -99,9 +99,11 @@ 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)
 {
@@ -129,15 +131,16 @@ 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 5556f0e64e..f6fc639e88 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -76,7 +76,7 @@ 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)
 {
@@ -91,11 +91,11 @@ 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
@@ -103,12 +103,12 @@ 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);
@@ -126,6 +126,8 @@ 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

[Qemu-devel] [PULL 04/23] hw/block: Use errp directly rather than local_err

[Stefan Hajnoczi]

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

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 | 18 ++++++------------
 hw/ide/qdev.c         | 12 ++++--------
 hw/scsi/scsi-disk.c   | 13 ++++---------
 hw/usb/dev-storage.c  |  9 +++------
 6 files changed, 25 insertions(+), 51 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 67f78ac702..7b7dd41296 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -473,16 +473,13 @@ 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;
         }
     }
@@ -522,7 +519,6 @@ 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) {
@@ -568,10 +564,9 @@ 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 e530ba7a30..e529e88e4e 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -928,7 +928,6 @@ 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");
@@ -947,10 +946,8 @@ 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 05d1440786..ae3356f6a8 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -913,7 +913,6 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIOBlock *s = VIRTIO_BLK(dev);
     VirtIOBlkConf *conf = &s->conf;
-    Error *err = NULL;
     unsigned i;
 
     if (!conf->conf.blk) {
@@ -930,19 +929,16 @@ 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,
@@ -955,9 +951,7 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
     for (i = 0; i < conf->num_queues; i++) {
         virtio_add_queue(vdev, 128, virtio_blk_handle_output);
     }
-    virtio_blk_data_plane_create(vdev, conf, &s->dataplane, &err);
-    if (err != NULL) {
-        error_propagate(errp, err);
+    if (!virtio_blk_data_plane_create(vdev, conf, &s->dataplane, errp)) {
         virtio_cleanup(vdev);
         return;
     }
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index a5181b4448..f395d24592 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -160,7 +160,6 @@ 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) {
@@ -191,16 +190,13 @@ 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 12431177a7..870d9ae85a 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2332,7 +2332,6 @@ 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");
@@ -2356,17 +2355,13 @@ 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 8a61ec94c8..a9bcc67e67 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -601,7 +601,6 @@ 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");
@@ -610,9 +609,8 @@ 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;
     }
 
@@ -636,10 +634,9 @@ 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

[Qemu-devel] [PULL 05/23] dev-storage: Fix the unusual function name

[Stefan Hajnoczi]

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 a9bcc67e67..9722ac854c 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -596,7 +596,7 @@ 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;
@@ -643,14 +643,14 @@ 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);
@@ -764,12 +764,12 @@ 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;
 }
@@ -828,26 +828,26 @@ 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

[Qemu-devel] [PULL 06/23] qdev: drop unused #include "sysemu/iothread.h"

[Stefan Hajnoczi]

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 -
 1 file changed, 1 deletion(-)

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index c17364655c..46b3843cf8 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -22,7 +22,6 @@
 #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),
-- 
2.14.3

[Qemu-devel] [PULL 07/23] blockdev: hold AioContext for bdrv_unref() in external_snapshot_clean()

[Stefan Hajnoczi]

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

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>
---
 blockdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/blockdev.c b/blockdev.c
index 56a6b24a0b..3c8d994ced 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1812,8 +1812,8 @@ 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

[Qemu-devel] [PULL 08/23] block: don't keep AioContext acquired after external_snapshot_prepare()

[Stefan Hajnoczi]

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 3c8d994ced..3b598f8f0e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1606,7 +1606,6 @@ typedef struct ExternalSnapshotState {
     BlkActionState common;
     BlockDriverState *old_bs;
     BlockDriverState *new_bs;
-    AioContext *aio_context;
     bool overlay_appended;
 } ExternalSnapshotState;
 
@@ -1626,6 +1625,7 @@ 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 */
@@ -1662,31 +1662,32 @@ 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) {
@@ -1698,13 +1699,13 @@ 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;
@@ -1717,7 +1718,7 @@ 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,
@@ -1725,7 +1726,7 @@ static void external_snapshot_prepare(BlkActionState *common,
                             NULL, size, flags, false, &local_err);
             if (local_err) {
                 error_propagate(errp, local_err);
-                return;
+                goto out;
             }
         }
 
@@ -1740,30 +1741,30 @@ 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
@@ -1772,15 +1773,22 @@ 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
@@ -1789,6 +1797,8 @@ 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)
@@ -1797,11 +1807,18 @@ 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);
         }
     }
 }
@@ -1810,11 +1827,19 @@ 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

[Qemu-devel] [PULL 09/23] block: don't keep AioContext acquired after drive_backup_prepare()

[Stefan Hajnoczi]

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 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 8 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 3b598f8f0e..5a56a1abf2 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1845,7 +1845,6 @@ static void external_snapshot_clean(BlkActionState *common)
 typedef struct DriveBackupState {
     BlkActionState common;
     BlockDriverState *bs;
-    AioContext *aio_context;
     BlockJob *job;
 } DriveBackupState;
 
@@ -1857,6 +1856,7 @@ 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);
@@ -1867,24 +1867,36 @@ 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)
@@ -1892,18 +1904,32 @@ 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

[Qemu-devel] [PULL 10/23] block: don't keep AioContext acquired after blockdev_backup_prepare()

[Stefan Hajnoczi]

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 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 10 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 5a56a1abf2..d7ad76416e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1936,7 +1936,6 @@ typedef struct BlockdevBackupState {
     BlkActionState common;
     BlockDriverState *bs;
     BlockJob *job;
-    AioContext *aio_context;
 } BlockdevBackupState;
 
 static BlockJob *do_blockdev_backup(BlockdevBackup *backup, BlockJobTxn *txn,
@@ -1947,6 +1946,7 @@ 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);
@@ -1962,29 +1962,39 @@ 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)
@@ -1992,18 +2002,32 @@ 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

[Qemu-devel] [PULL 11/23] block: don't keep AioContext acquired after internal_snapshot_prepare()

[Stefan Hajnoczi]

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 +++++++++++++++++++++++++++++++----------------
 1 file changed, 31 insertions(+), 16 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index d7ad76416e..6332a249ea 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1454,7 +1454,6 @@ struct BlkActionState {
 typedef struct InternalSnapshotState {
     BlkActionState common;
     BlockDriverState *bs;
-    AioContext *aio_context;
     QEMUSnapshotInfo sn;
     bool created;
 } InternalSnapshotState;
@@ -1485,6 +1484,7 @@ static void internal_snapshot_prepare(BlkActionState *common,
     qemu_timeval tv;
     BlockdevSnapshotInternal *internal;
     InternalSnapshotState *state;
+    AioContext *aio_context;
     int ret1;
 
     g_assert(common->action->type ==
@@ -1506,32 +1506,33 @@ 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 */
@@ -1539,12 +1540,12 @@ 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 */
@@ -1560,11 +1561,14 @@ 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)
@@ -1573,12 +1577,16 @@ 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 "
@@ -1586,19 +1594,26 @@ 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

[Qemu-devel] [PULL 12/23] block: drop unused BlockDirtyBitmapState->aio_context field

[Stefan Hajnoczi]

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.

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-7-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 blockdev.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 6332a249ea..e865ae4873 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2049,7 +2049,6 @@ typedef struct BlockDirtyBitmapState {
     BlkActionState common;
     BdrvDirtyBitmap *bitmap;
     BlockDriverState *bs;
-    AioContext *aio_context;
     HBitmap *backup;
     bool prepared;
 } BlockDirtyBitmapState;
@@ -2128,7 +2127,6 @@ 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)
@@ -2149,16 +2147,6 @@ 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");
@@ -2219,7 +2207,6 @@ 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

[Qemu-devel] [PULL 13/23] iothread: add iothread_by_id() API

[Stefan Hajnoczi]

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

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-8-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/sysemu/iothread.h | 1 +
 iothread.c                | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
index 110329b2b4..55de1715c7 100644
--- a/include/sysemu/iothread.h
+++ b/include/sysemu/iothread.h
@@ -42,6 +42,7 @@ 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 27a4288578..e7b93e02a3 100644
--- a/iothread.c
+++ b/iothread.c
@@ -380,3 +380,10 @@ 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

[Qemu-devel] [PULL 14/23] blockdev: add x-blockdev-set-iothread testing command

[Stefan Hajnoczi]

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 dd763dcf87..741d6c4367 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3949,3 +3949,39 @@
   '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 e865ae4873..f75c01f664 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -45,6 +45,7 @@
 #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"
@@ -4129,6 +4130,46 @@ 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

[Qemu-devel] [PULL 15/23] qemu-iotests: add 202 external snapshots IOThread test

[Stefan Hajnoczi]

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 0000000000..581ca34d79
--- /dev/null
+++ b/tests/qemu-iotests/202
@@ -0,0 +1,95 @@
+#!/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 0000000000..d5ea374e17
--- /dev/null
+++ b/tests/qemu-iotests/202.out
@@ -0,0 +1,11 @@
+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 3e688678dd..d0ee1e2e55 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -197,3 +197,4 @@
 197 rw auto quick
 198 rw auto
 200 rw auto
+202 rw auto quick
-- 
2.14.3

[Qemu-devel] [PULL 16/23] virtio-blk: make queue size configurable

[Stefan Hajnoczi]

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 d3c8a6fa8c..5117431d96 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -39,6 +39,7 @@ 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 ae3356f6a8..d8cfdc81f6 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -927,6 +927,13 @@ 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,
@@ -949,7 +956,7 @@ 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);
     }
     if (!virtio_blk_data_plane_create(vdev, conf, &s->dataplane, errp)) {
         virtio_cleanup(vdev);
@@ -1006,6 +1013,7 @@ 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

[Qemu-devel] [PULL 17/23] virtio-blk: reject configs with logical block size > physical block size

[Stefan Hajnoczi]

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 d8cfdc81f6..58a5def75d 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -948,6 +948,13 @@ 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

[Qemu-devel] [PULL 18/23] block: avoid recursive AioContext acquire in bdrv_inactivate_all()

[Stefan Hajnoczi]

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 9a1a0d1e73..1c37ce4554 100644
--- a/block.c
+++ b/block.c
@@ -4320,9 +4320,15 @@ 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'
@@ -4340,9 +4346,11 @@ 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

[Qemu-devel] [PULL 19/23] docs: mark nested AioContext locking as a legacy API

[Stefan Hajnoczi]

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 e4d340bbb7..4f9012d154 100644
--- a/docs/devel/multiple-iothreads.txt
+++ b/docs/devel/multiple-iothreads.txt
@@ -1,4 +1,4 @@
-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.
@@ -92,8 +92,9 @@ 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

[Qemu-devel] [PULL 20/23] blockdev: add x-blockdev-set-iothread force boolean

[Stefan Hajnoczi]

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 741d6c4367..a8cdbc300b 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3962,6 +3962,9 @@
 #
 # @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.
 #
@@ -3984,4 +3987,5 @@
 ##
 { 'command': 'x-blockdev-set-iothread',
   'data' : { 'node-name': 'str',
-             'iothread': 'StrOrNull' } }
+             'iothread': 'StrOrNull',
+             '*force': 'bool' } }
diff --git a/blockdev.c b/blockdev.c
index f75c01f664..9c3a430cfb 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -4131,7 +4131,7 @@ 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;
@@ -4143,10 +4143,11 @@ 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

[Qemu-devel] [PULL 21/23] iotests: add VM.add_object()

[Stefan Hajnoczi]

The VM.add_object() method can be used to add IOThreads or memory
backend objects.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20171207201320.19284-5-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qemu-iotests/iotests.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 6f057904a9..44477e9295 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -197,6 +197,11 @@ class VM(qtest.QEMUQtestMachine):
                                  socket_scm_helper=socket_scm_helper)
         self._num_drives = 0
 
+    def add_object(self, opts):
+        self._args.append('-object')
+        self._args.append(opts)
+        return self
+
     def add_device(self, opts):
         self._args.append('-device')
         self._args.append(opts)
-- 
2.14.3

[Qemu-devel] [PULL 22/23] iothread: fix iothread_stop() race condition

[Stefan Hajnoczi]

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 55de1715c7..799614ffd2 100644
--- a/include/sysemu/iothread.h
+++ b/include/sysemu/iothread.h
@@ -29,7 +29,8 @@ 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 e7b93e02a3..d8b6c1fb27 100644
--- a/iothread.c
+++ b/iothread.c
@@ -55,7 +55,7 @@ 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)) {
@@ -78,16 +78,25 @@ 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);
 }
 
@@ -134,6 +143,7 @@ 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

[Qemu-devel] [PULL 23/23] qemu-iotests: add 203 savevm with IOThreads test

[Stefan Hajnoczi]

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 0000000000..2c811917d8
--- /dev/null
+++ b/tests/qemu-iotests/203
@@ -0,0 +1,59 @@
+#!/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 0000000000..3f1ff900e4
--- /dev/null
+++ b/tests/qemu-iotests/203.out
@@ -0,0 +1,6 @@
+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 d0ee1e2e55..93d96fb22f 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -198,3 +198,4 @@
 198 rw auto
 200 rw auto
 202 rw auto quick
+203 rw auto
-- 
2.14.3

Re: [Qemu-devel] [PULL 00/23] Block patches

[Peter Maydell]

On 18 December 2017 at 14:35, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> The following changes since commit 411ad78115ebeb3411cf4b7622784b93dfabe259:
>
>   Merge remote-tracking branch 'remotes/stefanberger/tags/pull-tpm-2017-12-15-1' into staging (2017-12-17 15:27:41 +0000)
>
> are available in the Git repository at:
>
>   git://github.com/stefanha/qemu.git tags/block-pull-request
>
> for you to fetch changes up to 585426c518958aa768564596091474be786aae51:
>
>   qemu-iotests: add 203 savevm with IOThreads test (2017-12-18 13:12:53 +0000)
>
> ----------------------------------------------------------------
>

This failed 'make check' on all hosts:

TEST: tests/virtio-blk-test... (pid=49005)
  /arm/virtio/blk/mmio/basic:                                          **
ERROR:/home/pm215/qemu/tests/libqos/virtio.c:94:qvirtio_wait_queue_isr:
assertion failed: (g_get_monotonic_time() - start_time <= timeout_us)
FAIL
GTester: last random seed: R02S8877aa3278e71261919aae7707897a80
(pid=49024)
FAIL: tests/virtio-blk-test


thanks
-- PMM

Re: [Qemu-devel] [PULL 00/23] Block patches

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1277 bytes --]

On Tue, Dec 19, 2017 at 12:15:18AM +0000, Peter Maydell wrote:
> On 18 December 2017 at 14:35, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > The following changes since commit 411ad78115ebeb3411cf4b7622784b93dfabe259:
> >
> >   Merge remote-tracking branch 'remotes/stefanberger/tags/pull-tpm-2017-12-15-1' into staging (2017-12-17 15:27:41 +0000)
> >
> > are available in the Git repository at:
> >
> >   git://github.com/stefanha/qemu.git tags/block-pull-request
> >
> > for you to fetch changes up to 585426c518958aa768564596091474be786aae51:
> >
> >   qemu-iotests: add 203 savevm with IOThreads test (2017-12-18 13:12:53 +0000)
> >
> > ----------------------------------------------------------------
> >
> 
> This failed 'make check' on all hosts:
> 
> TEST: tests/virtio-blk-test... (pid=49005)
>   /arm/virtio/blk/mmio/basic:                                          **
> ERROR:/home/pm215/qemu/tests/libqos/virtio.c:94:qvirtio_wait_queue_isr:
> assertion failed: (g_get_monotonic_time() - start_time <= timeout_us)
> FAIL
> GTester: last random seed: R02S8877aa3278e71261919aae7707897a80
> (pid=49024)
> FAIL: tests/virtio-blk-test

Thanks, didn't notice it because x86 passes and Travis was looking
flakey.  Will send a v2.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

[Qemu-devel] [PULL 00/23] Block patches

[Stefan Hajnoczi]

The following changes since commit b00a0ddb31a393b8386d30a9bef4d9bbb249e7ec:

  Merge remote-tracking branch 'remotes/kraxel/tags/pull-input-20141002-1' into staging (2014-10-02 15:01:48 +0100)

are available in the git repository at:


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

for you to fetch changes up to 767c86d3e752dfc68ff5d018c3b0b63b333371b2:

  blockdev-test: Test device_del after drive_del (2014-10-04 19:28:39 +0100)

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

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

Fam Zheng (1):
  vmdk: Fix integer overflow in offset calculation

John Snow (6):
  blockdev: Orphaned drive search
  blockdev: Allow overriding if_max_dev property
  pc/vl: Add units-per-default-bus property
  ide: Update ide_drive_get to be HBA agnostic
  qtest/bios-tables: Correct Q35 command line
  q35/ahci: Pick up -cdrom and -hda options

Jun Li (1):
  Modify qemu_opt_rename to realize renaming all items in opts

Kevin Wolf (2):
  make check-block: Use default cache modes
  qemu-iotests: Fix supported cache modes for 052

Markus Armbruster (8):
  block: Drop superfluous conditionals around qemu_opts_del()
  util: Emancipate id_wellformed() from QemuOpts
  drive_del-test: Merge of qdev-monitor-test, blockdev-test
  blockdev-test: Use single rather than double quotes in QMP
  blockdev-test: Clean up bogus drive_add argument
  blockdev-test: Simplify by using g_assert_cmpstr()
  blockdev-test: Factor out some common code into helpers
  blockdev-test: Test device_del after drive_del

Max Reitz (3):
  iotests: Use _img_info
  qapi: Add corrupt field to ImageInfoSpecificQCow2
  iotests: qemu-img info output for corrupt image

Richard W.M. Jones (1):
  ssh: Don't crash if either host or path is not specified.

Zhang Haoyu (1):
  snapshot: fix referencing wrong variable in while loop in do_delvm

 block.c                     |   9 +--
 block/qcow2.c               |   3 +
 block/ssh.c                 |  10 ++++
 block/vmdk.c                |   2 +-
 blockdev.c                  |  72 +++++++++++++++++++++--
 hw/alpha/dp264.c            |   2 +-
 hw/i386/pc.c                |   1 +
 hw/i386/pc_piix.c           |   2 +-
 hw/i386/pc_q35.c            |   7 ++-
 hw/ide/ahci.c               |  15 +++++
 hw/ide/ahci.h               |   2 +
 hw/ide/core.c               |  22 +++++--
 hw/mips/mips_fulong2e.c     |   2 +-
 hw/mips/mips_malta.c        |   2 +-
 hw/mips/mips_r4k.c          |   2 +-
 hw/ppc/mac_newworld.c       |   2 +-
 hw/ppc/mac_oldworld.c       |   2 +-
 hw/ppc/prep.c               |   2 +-
 hw/sparc64/sun4u.c          |   2 +-
 include/hw/boards.h         |   2 +
 include/qemu-common.h       |   3 +
 include/qemu/option.h       |   1 -
 include/sysemu/blockdev.h   |   5 ++
 qapi/block-core.json        |   6 +-
 qemu-img.c                  |   4 +-
 qemu-nbd.c                  |   4 +-
 savevm.c                    |  11 ++--
 tests/Makefile              |   5 +-
 tests/bios-tables-test.c    |  10 ++--
 tests/blockdev-test.c       |  59 -------------------
 tests/drive_del-test.c      | 137 ++++++++++++++++++++++++++++++++++++++++++++
 tests/qdev-monitor-test.c   |  77 -------------------------
 tests/qemu-iotests-quick.sh |   2 +-
 tests/qemu-iotests/052      |   5 +-
 tests/qemu-iotests/060      |   3 +
 tests/qemu-iotests/060.out  |   9 +++
 tests/qemu-iotests/065      |  12 ++--
 tests/qemu-iotests/067.out  |  10 ++--
 tests/qemu-iotests/070      |   2 +-
 tests/qemu-iotests/070.out  |   5 +-
 tests/qemu-iotests/082      |  12 ++--
 tests/qemu-iotests/082.out  |  62 ++++++--------------
 tests/qemu-iotests/089.out  |   2 +
 tests/qemu-iotests/095      |   4 +-
 tests/qemu-iotests/095.out  |  16 ++----
 tests/qemu-iotests/105      |  70 ++++++++++++++++++++++
 tests/qemu-iotests/105.out  |  21 +++++++
 tests/qemu-iotests/group    |   1 +
 util/Makefile.objs          |   1 +
 util/id.c                   |  28 +++++++++
 util/qemu-option.c          |  17 +-----
 vl.c                        |  18 +++++-
 52 files changed, 500 insertions(+), 285 deletions(-)
 delete mode 100644 tests/blockdev-test.c
 create mode 100644 tests/drive_del-test.c
 delete mode 100644 tests/qdev-monitor-test.c
 create mode 100755 tests/qemu-iotests/105
 create mode 100644 tests/qemu-iotests/105.out
 create mode 100644 util/id.c

-- 
1.9.3

Re: [Qemu-devel] [PULL 00/23] Block patches

[Peter Maydell]

On 4 October 2014 21:24, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> The following changes since commit b00a0ddb31a393b8386d30a9bef4d9bbb249e7ec:
>
>   Merge remote-tracking branch 'remotes/kraxel/tags/pull-input-20141002-1' into staging (2014-10-02 15:01:48 +0100)
>
> are available in the git repository at:
>
>
>   git://github.com/stefanha/qemu.git tags/block-pull-request
>
> for you to fetch changes up to 767c86d3e752dfc68ff5d018c3b0b63b333371b2:
>
>   blockdev-test: Test device_del after drive_del (2014-10-04 19:28:39 +0100)
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM

[Qemu-devel] [PULL 00/23] Block patches

[Stefan Hajnoczi]

The following changes since commit 576156ffed72ab4feb0b752979db86ff8759a2a1:

  Merge remote-tracking branch 'bonzini/iommu-for-anthony' into staging (2013-06-20 16:53:39 -0500)

are available in the git repository at:


  git://github.com/stefanha/qemu.git block

for you to fetch changes up to 96c51eb5e46af2312b33f745ad72acb20e799aea:

  vmdk: refuse to open higher version than supported (2013-06-24 10:25:43 +0200)

----------------------------------------------------------------
Fam Zheng (1):
      vmdk: refuse to open higher version than supported

Kevin Wolf (22):
      ide: Add handler to ide_cmd_table
      ide: Convert WIN_DSM to ide_cmd_table handler
      ide: Convert WIN_IDENTIFY to ide_cmd_table handler
      ide: Convert cmd_nop commands to ide_cmd_table handler
      ide: Convert verify commands to ide_cmd_table handler
      ide: Convert read/write multiple commands to ide_cmd_table handler
      ide: Convert PIO read/write commands to ide_cmd_table handler
      ide: Convert DMA read/write commands to ide_cmd_table handler
      ide: Convert READ NATIVE MAX ADDRESS to ide_cmd_table handler
      ide: Convert CHECK POWER MDOE to ide_cmd_table handler
      ide: Convert SET FEATURES to ide_cmd_table handler
      ide: Convert FLUSH CACHE to ide_cmd_table handler
      ide: Convert SEEK to ide_cmd_table handler
      ide: Convert ATAPI commands to ide_cmd_table handler
      ide: Convert CF-ATA commands to ide_cmd_table handler
      ide: Convert SMART commands to ide_cmd_table handler
      ide: Clean up ide_exec_cmd()
      Revert "block: Disable driver-specific options for 1.5"
      qcow2: Add refcount update reason to all callers
      qcow2: Options to enable discard for freed clusters
      qcow2: Batch discards
      block: Always enable discard on the protocol level

 block.c                  |    2 +-
 block/qcow2-cluster.c    |   41 +-
 block/qcow2-refcount.c   |  136 ++++-
 block/qcow2-snapshot.c   |    6 +-
 block/qcow2.c            |   30 +-
 block/qcow2.h            |   32 +-
 block/vmdk.c             |    9 +
 blockdev.c               |  118 +----
 hw/ide/core.c            | 1242 +++++++++++++++++++++++++---------------------
 tests/qemu-iotests/group |    2 +-
 10 files changed, 892 insertions(+), 726 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PULL 00/23] Block patches

[Stefan Hajnoczi]

I'm away from Friday to next Wednesday.  Sorry for bombing the mailing list.

The following changes since commit e280ff5e9159ed227a117339c1157143627cab96:

  spice-qemu-char: Drop hackish vmc_register on spice_chr_write (2013-03-27 10:26:50 -0500)

are available in the git repository at:

  git://github.com/stefanha/qemu.git block

for you to fetch changes up to 5d186eb03eb37b257e29a4731ca484362d5fc4e4:

  block: Fix direct use of protocols as driver for bdrv_open() (2013-03-28 11:58:40 +0100)

----------------------------------------------------------------
Kevin Wolf (22):
      qemu-iotests: More concurrent allocation scenarios
      qcow2: Fix "total clusters" number in bdrv_check
      qcow2: Remove bogus unlock of s->lock
      qcow2: Handle dependencies earlier
      qcow2: Improve check for overlapping allocations
      qcow2: Change handle_dependency to byte granularity
      qcow2: Decouple cluster allocation from cluster reuse code
      qcow2: Factor out handle_alloc()
      qcow2: handle_alloc(): Get rid of nb_clusters parameter
      qcow2: handle_alloc(): Get rid of keep_clusters parameter
      qcow2: Finalise interface of handle_alloc()
      qcow2: Clean up handle_alloc()
      qcow2: Factor out handle_copied()
      qcow2: handle_copied(): Get rid of nb_clusters parameter
      qcow2: handle_copied(): Get rid of keep_clusters parameter
      qcow2: handle_copied(): Implement non-zero host_offset
      qcow2: Prepare handle_alloc/copied() for byte granularity
      qcow2: Use byte granularity in qcow2_alloc_cluster_offset()
      qcow2: Allow requests with multiple l2metas
      qcow2: Move cluster gathering to a non-looping loop
      qcow2: Gather clusters in a looping loop
      block: Fix direct use of protocols as driver for bdrv_open()

Peter Lieven (1):
      vl.c: call bdrv_init_with_whitelist() before cmdline parsing

 block.c                    |  26 ++-
 block/qcow2-cluster.c      | 509 ++++++++++++++++++++++++++++++++-------------
 block/qcow2-refcount.c     |   4 +-
 block/qcow2.c              |  16 +-
 block/qcow2.h              |  29 +++
 tests/qemu-iotests/038.out |  10 +-
 tests/qemu-iotests/044.out |   4 +-
 tests/qemu-iotests/046     |  49 ++++-
 tests/qemu-iotests/046.out |  76 +++++++
 trace-events               |   2 +
 vl.c                       |   4 +-
 11 files changed, 553 insertions(+), 176 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PULL 00/23] Block patches

[Kevin Wolf]

The following changes since commit 0bfe006c5380c5f8a485a55ded3329fbbc224396:

  multiboot: Fix upper memory size in multiboot info (2011-01-23 22:44:13 +0100)

are available in the git repository at:
  git://repo.or.cz/qemu/kevin.git for-anthony

Aurelien Jarno (1):
      qcow2: fix unaligned access

Christoph Hellwig (3):
      ide: factor dma handling helpers
      ide: also reset io_buffer_index for writes
      ide: kill ide_dma_submit_check

Jes Sorensen (2):
      do_snapshot_blkdev() error on missing snapshot_file argument
      Make strtosz() return int64_t instead of ssize_t

Kevin Wolf (5):
      qemu-img snapshot: Use writeback caching
      qcow2: Add QcowCache
      qcow2: Use QcowCache
      qcow2: Batch flushes for COW
      Documentation: Add qemu-img check/rebase

Markus Armbruster (3):
      blockdev: Fix error message for invalid -drive CHS
      blockdev: Make drive_init() use error_report()
      blockdev: Fix drive_del not to crash when drive is not in use

Pierre Riteau (2):
      Avoid divide by zero when there is no block device to migrate
      Fix block migration when the device size is not a multiple of 1 MB

Stefan Hajnoczi (6):
      qed: Refuse to create images on block devices
      block: Use backing format driver during image creation
      scsi-disk: Allow overriding SCSI INQUIRY removable bit
      scsi: Allow scsi_bus_legacy_add_drive() to set removable bit
      usb-msd: Propagate removable bit to SCSI device
      docs: Document scsi-disk and usb-storage removable parameter

Stefan Weil (1):
      ide: Remove unneeded null pointer check

 Makefile.objs            |    2 +-
 block-migration.c        |   29 ++++-
 block.c                  |    8 +-
 block/qcow2-cache.c      |  314 ++++++++++++++++++++++++++++++++++++++++++++++
 block/qcow2-cluster.c    |  210 +++++++++++--------------------
 block/qcow2-refcount.c   |  260 ++++++++++++++++----------------------
 block/qcow2.c            |   48 +++++++-
 block/qcow2.h            |   32 ++++-
 block/qed.c              |    6 +
 blockdev.c               |   81 +++++++------
 cutils.c                 |    8 +-
 docs/qdev-device-use.txt |   13 ++-
 hw/ide/core.c            |  113 ++++++-----------
 hw/ide/internal.h        |    4 +-
 hw/ide/pci.c             |   13 +--
 hw/pci-hotplug.c         |    2 +-
 hw/scsi-bus.c            |    8 +-
 hw/scsi-disk.c           |    3 +
 hw/scsi.h                |    3 +-
 hw/usb-msd.c             |    4 +-
 monitor.c                |    2 +-
 qemu-common.h            |    4 +-
 qemu-img.c               |    4 +-
 qemu-img.texi            |   41 ++++++
 vl.c                     |    4 +-
 25 files changed, 764 insertions(+), 452 deletions(-)
 create mode 100644 block/qcow2-cache.c

[Qemu-devel] [PULL 00/23] Block patches

[Kevin Wolf]

The following changes since commit 8713f8ffb87a28c94b4f22b9a9ec16c55381187e:
  Andi Kleen (1):
        Don't declare XSAVE as supported

are available in the git repository at:

  git://repo.or.cz/qemu/kevin.git for-anthony

Christoph Hellwig (1):
      block: allow filenames with colons again for host devices

Kevin Wolf (6):
      qcow2: Fix error handling during metadata preallocation
      blkdebug: Fix set_state_opts definition
      blkdebug: Free QemuOpts after having read the config
      blkdebug: Initialize state as 1
      block: Fix early failure in multiwrite
      block: Handle multiwrite errors only when all requests have completed

MORITA Kazutaka (1):
      qemu-img: avoid calling exit(1) to release resources properly

Markus Armbruster (14):
      scsi: scsi_bus_legacy_handle_cmdline() can fail, fix callers
      ide: Make it explicit that ide_create_drive() can't fail
      blockdev: Remove drive_get_serial()
      blockdev: New drive_get_by_blockdev()
      blockdev: Clean up automatic drive deletion
      qdev: Decouple qdev_prop_drive from DriveInfo
      blockdev: drive_get_by_id() is no longer used, remove
      block: Catch attempt to attach multiple devices to a blockdev
      qemu-option: New qemu_opts_reset()
      savevm: Survive hot-unplug of snapshot device
      block: Clean up bdrv_snapshots()
      block: Fix virtual media change for if=none
      ide: Make PIIX and ISA IDE init functions return the qdev
      pc: Fix CMOS info for drives defined with -device

Ryan Harper (1):
      Don't reset bs->is_temporary in bdrv_open_common

 block.c              |  125 ++++++++++++++++++++++-----
 block.h              |    5 +
 block/blkdebug.c     |    7 ++-
 block/qcow2.c        |   15 ++--
 block_int.h          |    8 +-
 blockdev.c           |   45 ++++++----
 blockdev.h           |    7 +-
 hw/esp.c             |    3 +-
 hw/fdc.c             |   32 ++++---
 hw/ide.h             |   13 ++-
 hw/ide/core.c        |   18 ++--
 hw/ide/internal.h    |    2 +-
 hw/ide/isa.c         |    8 +-
 hw/ide/piix.c        |    6 +-
 hw/ide/qdev.c        |   22 ++++--
 hw/lsi53c895a.c      |    2 +-
 hw/pc.c              |   94 +++++++++++++--------
 hw/pc.h              |    3 +-
 hw/pc_piix.c         |   16 +++-
 hw/pci-hotplug.c     |   11 ++-
 hw/qdev-properties.c |   47 +++++++++--
 hw/qdev.h            |    7 +-
 hw/s390-virtio.c     |    2 +-
 hw/scsi-bus.c        |   20 +++--
 hw/scsi-disk.c       |   21 +++--
 hw/scsi-generic.c    |    7 +-
 hw/scsi.h            |    4 +-
 hw/usb-msd.c         |   30 +++++--
 hw/virtio-blk.c      |    3 +-
 hw/virtio-pci.c      |    4 +-
 qemu-img.c           |  237 +++++++++++++++++++++++++++++++++++++++-----------
 qemu-option.c        |    9 ++
 qemu-option.h        |    1 +
 savevm.c             |   31 +------
 34 files changed, 606 insertions(+), 259 deletions(-)