The AioContext pointer argument to co_aio_sleep_ns() is only used for

the sleep timer. It does not affect where the caller coroutine is

resumed.

Due to changes to coroutine and AIO APIs it is now possible to drop the

AioContext pointer argument. This is safe to do since no caller has

specific requirements for which AioContext the timer must run in.

This patch drops the AioContext pointer argument and renames the

function to simplify the API.

Reported-by: Paolo Bonzini <pbonzini@redhat.com>

Reported-by: Eric Blake <eblake@redhat.com>

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Eric Blake <eblake@redhat.com>

Message-id: 20171109102652.6360-1-stefanha@redhat.com

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

---

include/qemu/coroutine.h | 6 +-----

block/null.c | 3 +--

block/sheepdog.c | 3 +--

util/qemu-coroutine-sleep.c | 4 ++--

4 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h

index XXXXXXX..XXXXXXX 100644

--- a/include/qemu/coroutine.h

+++ b/include/qemu/coroutine.h

@@ -XXX,XX +XXX,XX @@ void qemu_co_rwlock_unlock(CoRwlock *lock);

/**

* Yield the coroutine for a given duration

- *

- * Behaves similarly to co_sleep_ns(), but the sleeping coroutine will be

- * resumed when using aio_poll().

*/

-void coroutine_fn co_aio_sleep_ns(AioContext *ctx, QEMUClockType type,

- int64_t ns);

+void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns);

/**

* Yield until a file descriptor becomes readable

diff --git a/block/null.c b/block/null.c

index XXXXXXX..XXXXXXX 100644

--- a/block/null.c

+++ b/block/null.c

@@ -XXX,XX +XXX,XX @@ static coroutine_fn int null_co_common(BlockDriverState *bs)

BDRVNullState *s = bs->opaque;

if (s->latency_ns) {

- co_aio_sleep_ns(bdrv_get_aio_context(bs), QEMU_CLOCK_REALTIME,

- s->latency_ns);

+ qemu_co_sleep_ns(QEMU_CLOCK_REALTIME, s->latency_ns);

}

return 0;

}

diff --git a/block/sheepdog.c b/block/sheepdog.c

index XXXXXXX..XXXXXXX 100644

--- a/block/sheepdog.c

+++ b/block/sheepdog.c

@@ -XXX,XX +XXX,XX @@ static coroutine_fn void reconnect_to_sdog(void *opaque)

if (s->fd < 0) {

DPRINTF("Wait for connection to be established\n");

error_report_err(local_err);

- co_aio_sleep_ns(bdrv_get_aio_context(s->bs), QEMU_CLOCK_REALTIME,

- 1000000000ULL);

+ qemu_co_sleep_ns(QEMU_CLOCK_REALTIME, 1000000000ULL);

}

};

diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c

index XXXXXXX..XXXXXXX 100644

--- a/util/qemu-coroutine-sleep.c

+++ b/util/qemu-coroutine-sleep.c

@@ -XXX,XX +XXX,XX @@ static void co_sleep_cb(void *opaque)

aio_co_wake(sleep_cb->co);

}

-void coroutine_fn co_aio_sleep_ns(AioContext *ctx, QEMUClockType type,

- int64_t ns)

+void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)

{

+ AioContext *ctx = qemu_get_current_aio_context();

CoSleepCB sleep_cb = {

.co = qemu_coroutine_self(),

};

--

2.14.3

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

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

Cc: John Snow <jsnow@redhat.com>

Cc: Keith Busch <keith.busch@intel.com>

Cc: Kevin Wolf <kwolf@redhat.com>

Cc: Max Reitz <mreitz@redhat.com>

Cc: Markus Armbruster <armbru@redhat.com>

Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>

Message-id: 2882e72d795e04cbe2120f569d551aef2467ac60.1511317952.git.maozy.fnst@cn.fujitsu.com

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

---

hw/block/nvme.c | 18 ++++++++++--------

1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/block/nvme.c

+++ b/hw/block/nvme.c

@@ -XXX,XX +XXX,XX @@ static const MemoryRegionOps nvme_cmb_ops = {

},

};

-static int nvme_init(PCIDevice *pci_dev)

+static void nvme_realize(PCIDevice *pci_dev, Error **errp)

{

NvmeCtrl *n = NVME(pci_dev);

NvmeIdCtrl *id = &n->id_ctrl;

@@ -XXX,XX +XXX,XX @@ static int nvme_init(PCIDevice *pci_dev)

Error *local_err = NULL;

if (!n->conf.blk) {

- return -1;

+ error_setg(errp, "drive property not set");

+ return;

}

bs_size = blk_getlength(n->conf.blk);

if (bs_size < 0) {

- return -1;

+ error_setg(errp, "could not get backing file size");

+ return;

}

blkconf_serial(&n->conf, &n->serial);

if (!n->serial) {

- return -1;

+ error_setg(errp, "serial property not set");

+ return;

}

blkconf_blocksizes(&n->conf);

blkconf_apply_backend_options(&n->conf, blk_is_read_only(n->conf.blk),

false, &local_err);

if (local_err) {

- error_report_err(local_err);

- return -1;

+ error_propagate(errp, local_err);

+ return;

}

pci_conf = pci_dev->config;

@@ -XXX,XX +XXX,XX @@ static int nvme_init(PCIDevice *pci_dev)

cpu_to_le64(n->ns_size >>

id_ns->lbaf[NVME_ID_NS_FLBAS_INDEX(ns->id_ns.flbas)].ds);

}

- return 0;

}

static void nvme_exit(PCIDevice *pci_dev)

@@ -XXX,XX +XXX,XX @@ static void nvme_class_init(ObjectClass *oc, void *data)

DeviceClass *dc = DEVICE_CLASS(oc);

PCIDeviceClass *pc = PCI_DEVICE_CLASS(oc);

- pc->init = nvme_init;

+ pc->realize = nvme_realize;

pc->exit = nvme_exit;

pc->class_id = PCI_CLASS_STORAGE_EXPRESS;

pc->vendor_id = PCI_VENDOR_ID_INTEL;

--

2.14.3

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

When the function no success value to transmit, it usually make the

function return void. It has turned out not to be a success, because

it means that the extra local_err variable and error_propagate() will

be needed. It leads to cumbersome code, therefore, transmit success/

failure in the return value is worth.

So fix the return type of blkconf_apply_backend_options(),

blkconf_geometry() and virtio_blk_data_plane_create() to avoid it.

Cc: John Snow <jsnow@redhat.com>

Cc: Kevin Wolf <kwolf@redhat.com>

Cc: Max Reitz <mreitz@redhat.com>

Cc: Stefan Hajnoczi <stefanha@redhat.com>

Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Message-id: ac0edc1fc70c4457e5cec94405eb7d1f89f9c2c1.1511317952.git.maozy.fnst@cn.fujitsu.com

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

---

hw/block/dataplane/virtio-blk.h | 2 +-

include/hw/block/block.h | 4 ++--

hw/block/block.c | 15 +++++++++------

hw/block/dataplane/virtio-blk.c | 12 +++++++-----

4 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.h b/hw/block/dataplane/virtio-blk.h

index XXXXXXX..XXXXXXX 100644

--- a/hw/block/dataplane/virtio-blk.h

+++ b/hw/block/dataplane/virtio-blk.h

@@ -XXX,XX +XXX,XX @@

typedef struct VirtIOBlockDataPlane VirtIOBlockDataPlane;

-void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,

+bool virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,

VirtIOBlockDataPlane **dataplane,

Error **errp);

void virtio_blk_data_plane_destroy(VirtIOBlockDataPlane *s);

diff --git a/include/hw/block/block.h b/include/hw/block/block.h

index XXXXXXX..XXXXXXX 100644

--- a/include/hw/block/block.h

+++ b/include/hw/block/block.h

@@ -XXX,XX +XXX,XX @@ static inline unsigned int get_physical_block_exp(BlockConf *conf)

/* Configuration helpers */

void blkconf_serial(BlockConf *conf, char **serial);

-void blkconf_geometry(BlockConf *conf, int *trans,

+bool blkconf_geometry(BlockConf *conf, int *trans,

unsigned cyls_max, unsigned heads_max, unsigned secs_max,

Error **errp);

void blkconf_blocksizes(BlockConf *conf);

-void blkconf_apply_backend_options(BlockConf *conf, bool readonly,

+bool blkconf_apply_backend_options(BlockConf *conf, bool readonly,

bool resizable, Error **errp);

/* Hard disk geometry */

diff --git a/hw/block/block.c b/hw/block/block.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/block/block.c

+++ b/hw/block/block.c

@@ -XXX,XX +XXX,XX @@ void blkconf_blocksizes(BlockConf *conf)

}

}

-void blkconf_apply_backend_options(BlockConf *conf, bool readonly,

+bool blkconf_apply_backend_options(BlockConf *conf, bool readonly,

bool resizable, Error **errp)

{

BlockBackend *blk = conf->blk;

@@ -XXX,XX +XXX,XX @@ void blkconf_apply_backend_options(BlockConf *conf, bool readonly,

ret = blk_set_perm(blk, perm, shared_perm, errp);

if (ret < 0) {

- return;

+ return false;

}

switch (conf->wce) {

@@ -XXX,XX +XXX,XX @@ void blkconf_apply_backend_options(BlockConf *conf, bool readonly,

blk_set_enable_write_cache(blk, wce);

blk_set_on_error(blk, rerror, werror);

+

+ return true;

}

-void blkconf_geometry(BlockConf *conf, int *ptrans,

+bool blkconf_geometry(BlockConf *conf, int *ptrans,

unsigned cyls_max, unsigned heads_max, unsigned secs_max,

Error **errp)

{

@@ -XXX,XX +XXX,XX @@ void blkconf_geometry(BlockConf *conf, int *ptrans,

if (conf->cyls || conf->heads || conf->secs) {

if (conf->cyls < 1 || conf->cyls > cyls_max) {

error_setg(errp, "cyls must be between 1 and %u", cyls_max);

- return;

+ return false;

}

if (conf->heads < 1 || conf->heads > heads_max) {

error_setg(errp, "heads must be between 1 and %u", heads_max);

- return;

+ return false;

}

if (conf->secs < 1 || conf->secs > secs_max) {

error_setg(errp, "secs must be between 1 and %u", secs_max);

- return;

+ return false;

}

}

+ return true;

}

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/block/dataplane/virtio-blk.c

+++ b/hw/block/dataplane/virtio-blk.c

@@ -XXX,XX +XXX,XX @@ static void notify_guest_bh(void *opaque)

}

/* Context: QEMU global mutex held */

-void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,

+bool virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,

VirtIOBlockDataPlane **dataplane,

Error **errp)

{

@@ -XXX,XX +XXX,XX @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,

error_setg(errp,

"device is incompatible with iothread "

"(transport does not support notifiers)");

- return;

+ return false;

}

if (!virtio_device_ioeventfd_enabled(vdev)) {

error_setg(errp, "ioeventfd is required for iothread");

- return;

+ return false;

}

/* If dataplane is (re-)enabled while the guest is running there could

@@ -XXX,XX +XXX,XX @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,

*/

if (blk_op_is_blocked(conf->conf.blk, BLOCK_OP_TYPE_DATAPLANE, errp)) {

error_prepend(errp, "cannot start virtio-blk dataplane: ");

- return;

+ return false;

}

}

/* Don't try if transport does not support notifiers. */

if (!virtio_device_ioeventfd_enabled(vdev)) {

- return;

+ return false;

}

s = g_new0(VirtIOBlockDataPlane, 1);

@@ -XXX,XX +XXX,XX @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,

s->batch_notify_vqs = bitmap_new(conf->num_queues);

*dataplane = s;

+

+ return true;

}

/* Context: QEMU global mutex held */

--

2.14.3

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

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 XXXXXXX..XXXXXXX 100644

--- a/hw/block/fdc.c

+++ b/hw/block/fdc.c

@@ -XXX,XX +XXX,XX @@ static void fd_revalidate(FDrive *drv)

static void fd_change_cb(void *opaque, bool load, Error **errp)

{

FDrive *drive = opaque;

- Error *local_err = NULL;

if (!load) {

blk_set_perm(drive->blk, 0, BLK_PERM_ALL, &error_abort);

} else {

- blkconf_apply_backend_options(drive->conf,

- blk_is_read_only(drive->blk), false,

- &local_err);

- if (local_err) {

- error_propagate(errp, local_err);

+ if (!blkconf_apply_backend_options(drive->conf,

+ blk_is_read_only(drive->blk), false,

+ errp)) {

return;

}

}

@@ -XXX,XX +XXX,XX @@ static void floppy_drive_realize(DeviceState *qdev, Error **errp)

FloppyDrive *dev = FLOPPY_DRIVE(qdev);

FloppyBus *bus = FLOPPY_BUS(qdev->parent_bus);

FDrive *drive;

- Error *local_err = NULL;

int ret;

if (dev->unit == -1) {

@@ -XXX,XX +XXX,XX @@ static void floppy_drive_realize(DeviceState *qdev, Error **errp)

dev->conf.rerror = BLOCKDEV_ON_ERROR_AUTO;

dev->conf.werror = BLOCKDEV_ON_ERROR_AUTO;

- blkconf_apply_backend_options(&dev->conf, blk_is_read_only(dev->conf.blk),

- false, &local_err);

- if (local_err) {

- error_propagate(errp, local_err);

+ if (!blkconf_apply_backend_options(&dev->conf,

+ blk_is_read_only(dev->conf.blk),

+ false, errp)) {

return;

}

diff --git a/hw/block/nvme.c b/hw/block/nvme.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/block/nvme.c

+++ b/hw/block/nvme.c

@@ -XXX,XX +XXX,XX @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)

int i;

int64_t bs_size;

uint8_t *pci_conf;

- Error *local_err = NULL;

if (!n->conf.blk) {

error_setg(errp, "drive property not set");

@@ -XXX,XX +XXX,XX @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)

return;

}

blkconf_blocksizes(&n->conf);

- blkconf_apply_backend_options(&n->conf, blk_is_read_only(n->conf.blk),

- false, &local_err);

- if (local_err) {

- error_propagate(errp, local_err);

+ if (!blkconf_apply_backend_options(&n->conf, blk_is_read_only(n->conf.blk),

+ false, errp)) {

return;

}

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/block/virtio-blk.c

+++ b/hw/block/virtio-blk.c

@@ -XXX,XX +XXX,XX @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)

VirtIODevice *vdev = VIRTIO_DEVICE(dev);

VirtIOBlock *s = VIRTIO_BLK(dev);

VirtIOBlkConf *conf = &s->conf;

- Error *err = NULL;

unsigned i;

if (!conf->conf.blk) {

@@ -XXX,XX +XXX,XX @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)

}

blkconf_serial(&conf->conf, &conf->serial);

- blkconf_apply_backend_options(&conf->conf,

- blk_is_read_only(conf->conf.blk), true,

- &err);

- if (err) {

- error_propagate(errp, err);

+ if (!blkconf_apply_backend_options(&conf->conf,

+ blk_is_read_only(conf->conf.blk), true,

+ errp)) {

return;

}

s->original_wce = blk_enable_write_cache(conf->conf.blk);

- blkconf_geometry(&conf->conf, NULL, 65535, 255, 255, &err);

- if (err) {

- error_propagate(errp, err);

+ if (!blkconf_geometry(&conf->conf, NULL, 65535, 255, 255, errp)) {

return;

}

+

blkconf_blocksizes(&conf->conf);

virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,

@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644

--- a/hw/ide/qdev.c

+++ b/hw/ide/qdev.c

@@ -XXX,XX +XXX,XX @@ static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp)

{

IDEBus *bus = DO_UPCAST(IDEBus, qbus, dev->qdev.parent_bus);

IDEState *s = bus->ifs + dev->unit;

- Error *err = NULL;

int ret;

if (!dev->conf.blk) {

@@ -XXX,XX +XXX,XX @@ static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp)

blkconf_serial(&dev->conf, &dev->serial);

if (kind != IDE_CD) {

- blkconf_geometry(&dev->conf, &dev->chs_trans, 65535, 16, 255, &err);

- if (err) {

- error_propagate(errp, err);

+ if (!blkconf_geometry(&dev->conf, &dev->chs_trans, 65535, 16, 255,

+ errp)) {

return;

}

}

- blkconf_apply_backend_options(&dev->conf, kind == IDE_CD, kind != IDE_CD,

- &err);

- if (err) {

- error_propagate(errp, err);

+ if (!blkconf_apply_backend_options(&dev->conf, kind == IDE_CD,

+ kind != IDE_CD, errp)) {

return;

}

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/scsi/scsi-disk.c

+++ b/hw/scsi/scsi-disk.c

@@ -XXX,XX +XXX,XX @@ static void scsi_disk_unit_attention_reported(SCSIDevice *dev)

static void scsi_realize(SCSIDevice *dev, Error **errp)

{

SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);

- Error *err = NULL;

if (!s->qdev.conf.blk) {

error_setg(errp, "drive property not set");

@@ -XXX,XX +XXX,XX @@ static void scsi_realize(SCSIDevice *dev, Error **errp)

}

if (dev->type == TYPE_DISK) {

- blkconf_geometry(&dev->conf, NULL, 65535, 255, 255, &err);

- if (err) {

- error_propagate(errp, err);

+ if (!blkconf_geometry(&dev->conf, NULL, 65535, 255, 255, errp)) {

return;

}

}

- blkconf_apply_backend_options(&dev->conf,

- blk_is_read_only(s->qdev.conf.blk),

- dev->type == TYPE_DISK, &err);

- if (err) {

- error_propagate(errp, err);

+ if (!blkconf_apply_backend_options(&dev->conf,

+ blk_is_read_only(s->qdev.conf.blk),

+ dev->type == TYPE_DISK, errp)) {

return;

}

diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/usb/dev-storage.c

+++ b/hw/usb/dev-storage.c

@@ -XXX,XX +XXX,XX @@ static void usb_msd_realize_storage(USBDevice *dev, Error **errp)

MSDState *s = USB_STORAGE_DEV(dev);

BlockBackend *blk = s->conf.blk;

SCSIDevice *scsi_dev;

- Error *err = NULL;

if (!blk) {

error_setg(errp, "drive property not set");

@@ -XXX,XX +XXX,XX @@ static void usb_msd_realize_storage(USBDevice *dev, Error **errp)

blkconf_serial(&s->conf, &dev->serial);

blkconf_blocksizes(&s->conf);

- blkconf_apply_backend_options(&s->conf, blk_is_read_only(blk), true, &err);

- if (err) {

- error_propagate(errp, err);

+ if (!blkconf_apply_backend_options(&s->conf, blk_is_read_only(blk), true,

+ errp)) {

return;

}

@@ -XXX,XX +XXX,XX @@ static void usb_msd_realize_storage(USBDevice *dev, Error **errp)

&usb_msd_scsi_info_storage, NULL);

scsi_dev = scsi_bus_legacy_add_drive(&s->bus, blk, 0, !!s->removable,

s->conf.bootindex, dev->serial,

- &err);

+ errp);

blk_unref(blk);

if (!scsi_dev) {

- error_propagate(errp, err);

return;

}

usb_msd_handle_reset(dev);

--

2.14.3

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

The function name of usb_msd_{realize,unrealize}_*,

usb_msd_class_initfn_* are unusual. Rename it to

usb_msd_*_{realize,unrealize}, usb_msd_class_*_initfn.

Cc: Gerd Hoffmann <kraxel@redhat.com>

Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Message-id: 11e6003433abce35f3f4970e1acc71ee92dbcf51.1511317952.git.maozy.fnst@cn.fujitsu.com

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

---

hw/usb/dev-storage.c | 20 ++++++++++----------

1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/usb/dev-storage.c

+++ b/hw/usb/dev-storage.c

@@ -XXX,XX +XXX,XX @@ static void usb_msd_unrealize_storage(USBDevice *dev, Error **errp)

object_unref(OBJECT(&s->bus));

}

-static void usb_msd_realize_storage(USBDevice *dev, Error **errp)

+static void usb_msd_storage_realize(USBDevice *dev, Error **errp)

{

MSDState *s = USB_STORAGE_DEV(dev);

BlockBackend *blk = s->conf.blk;

@@ -XXX,XX +XXX,XX @@ static void usb_msd_realize_storage(USBDevice *dev, Error **errp)

s->scsi_dev = scsi_dev;

}

-static void usb_msd_unrealize_bot(USBDevice *dev, Error **errp)

+static void usb_msd_bot_unrealize(USBDevice *dev, Error **errp)

{

MSDState *s = USB_STORAGE_DEV(dev);

object_unref(OBJECT(&s->bus));

}

-static void usb_msd_realize_bot(USBDevice *dev, Error **errp)

+static void usb_msd_bot_realize(USBDevice *dev, Error **errp)

{

MSDState *s = USB_STORAGE_DEV(dev);

DeviceState *d = DEVICE(dev);

@@ -XXX,XX +XXX,XX @@ static void usb_msd_class_initfn_common(ObjectClass *klass, void *data)

dc->vmsd = &vmstate_usb_msd;

}

-static void usb_msd_class_initfn_storage(ObjectClass *klass, void *data)

+static void usb_msd_class_storage_initfn(ObjectClass *klass, void *data)

{

DeviceClass *dc = DEVICE_CLASS(klass);

USBDeviceClass *uc = USB_DEVICE_CLASS(klass);

- uc->realize = usb_msd_realize_storage;

+ uc->realize = usb_msd_storage_realize;

uc->unrealize = usb_msd_unrealize_storage;

dc->props = msd_properties;

}

@@ -XXX,XX +XXX,XX @@ static void usb_msd_instance_init(Object *obj)

object_property_set_int(obj, -1, "bootindex", NULL);

}

-static void usb_msd_class_initfn_bot(ObjectClass *klass, void *data)

+static void usb_msd_class_bot_initfn(ObjectClass *klass, void *data)

{

USBDeviceClass *uc = USB_DEVICE_CLASS(klass);

- uc->realize = usb_msd_realize_bot;

- uc->unrealize = usb_msd_unrealize_bot;

+ uc->realize = usb_msd_bot_realize;

+ uc->unrealize = usb_msd_bot_unrealize;

uc->attached_settable = true;

}

static const TypeInfo msd_info = {

.name = "usb-storage",

.parent = TYPE_USB_STORAGE,

- .class_init = usb_msd_class_initfn_storage,

+ .class_init = usb_msd_class_storage_initfn,

.instance_init = usb_msd_instance_init,

};

static const TypeInfo bot_info = {

.name = "usb-bot",

.parent = TYPE_USB_STORAGE,

- .class_init = usb_msd_class_initfn_bot,

+ .class_init = usb_msd_class_bot_initfn,

};

static void usb_msd_register_types(void)

--

2.14.3

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 XXXXXXX..XXXXXXX 100644

--- a/hw/core/qdev-properties-system.c

+++ b/hw/core/qdev-properties-system.c

@@ -XXX,XX +XXX,XX @@

#include "qapi/visitor.h"

#include "chardev/char-fe.h"

#include "sysemu/tpm_backend.h"

-#include "sysemu/iothread.h"

static void get_pointer(Object *obj, Visitor *v, Property *prop,

char *(*print)(void *ptr),

--

2.14.3

bdrv_unref() requires the AioContext lock because bdrv_flush() uses

BDRV_POLL_WHILE(), which assumes the AioContext is currently held. If

BDRV_POLL_WHILE() runs without AioContext held the

pthread_mutex_unlock() call in aio_context_release() fails.

This patch moves bdrv_unref() into the AioContext locked region to solve

the following pthread_mutex_unlock() failure:

#0 0x00007f566181969b in raise () at /lib64/libc.so.6

#1 0x00007f566181b3b1 in abort () at /lib64/libc.so.6

#2 0x00005592cd590458 in error_exit (err=<optimized out>, msg=msg@entry=0x5592cdaf6d60 <__func__.23977> "qemu_mutex_unlock") at util/qemu-thread-posix.c:36

#3 0x00005592cd96e738 in qemu_mutex_unlock (mutex=mutex@entry=0x5592ce9505e0) at util/qemu-thread-posix.c:96

#4 0x00005592cd969b69 in aio_context_release (ctx=ctx@entry=0x5592ce950580) at util/async.c:507

#5 0x00005592cd8ead78 in bdrv_flush (bs=bs@entry=0x5592cfa87210) at block/io.c:2478

#6 0x00005592cd89df30 in bdrv_close (bs=0x5592cfa87210) at block.c:3207

#7 0x00005592cd89df30 in bdrv_delete (bs=0x5592cfa87210) at block.c:3395

#8 0x00005592cd89df30 in bdrv_unref (bs=0x5592cfa87210) at block.c:4418

#9 0x00005592cd6b7f86 in qmp_transaction (dev_list=<optimized out>, has_props=<optimized out>, props=<optimized out>, errp=errp@entry=0x7ffe4a1fc9d8) at blockdev.c:2308

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 XXXXXXX..XXXXXXX 100644

--- a/blockdev.c

+++ b/blockdev.c

@@ -XXX,XX +XXX,XX @@ static void external_snapshot_clean(BlkActionState *common)

DO_UPCAST(ExternalSnapshotState, common, common);

if (state->aio_context) {

bdrv_drained_end(state->old_bs);

- aio_context_release(state->aio_context);

bdrv_unref(state->new_bs);

+ aio_context_release(state->aio_context);

}

}

--

2.14.3

It is not necessary to hold AioContext across transactions anymore since

bdrv_drained_begin/end() is used to keep the nodes quiesced. In fact,

using the AioContext lock for this purpose was always buggy.

This patch reduces the scope of AioContext locked regions. This is not

just a cleanup but also fixes hangs that occur in BDRV_POLL_WHILE()

because it is unware of recursive locking and does not release the

AioContext the necessary number of times to allow progress to be made.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Reviewed-by: Eric Blake <eblake@redhat.com>

Message-id: 20171206144550.22295-3-stefanha@redhat.com

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

---

blockdev.c | 71 ++++++++++++++++++++++++++++++++++++++++++--------------------

1 file changed, 48 insertions(+), 23 deletions(-)

diff --git a/blockdev.c b/blockdev.c

index XXXXXXX..XXXXXXX 100644

--- a/blockdev.c

+++ b/blockdev.c

@@ -XXX,XX +XXX,XX @@ typedef struct ExternalSnapshotState {

BlkActionState common;

BlockDriverState *old_bs;

BlockDriverState *new_bs;

- AioContext *aio_context;

bool overlay_appended;

} ExternalSnapshotState;

@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,

ExternalSnapshotState *state =

DO_UPCAST(ExternalSnapshotState, common, common);

TransactionAction *action = common->action;

+ AioContext *aio_context;

/* 'blockdev-snapshot' and 'blockdev-snapshot-sync' have similar

* purpose but a different set of parameters */

@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,

return;

}

- /* Acquire AioContext now so any threads operating on old_bs stop */

- state->aio_context = bdrv_get_aio_context(state->old_bs);

- aio_context_acquire(state->aio_context);

+ aio_context = bdrv_get_aio_context(state->old_bs);

+ aio_context_acquire(aio_context);

+

+ /* Paired with .clean() */

bdrv_drained_begin(state->old_bs);

if (!bdrv_is_inserted(state->old_bs)) {

error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);

- return;

+ goto out;

}

if (bdrv_op_is_blocked(state->old_bs,

BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT, errp)) {

- return;

+ goto out;

}

if (!bdrv_is_read_only(state->old_bs)) {

if (bdrv_flush(state->old_bs)) {

error_setg(errp, QERR_IO_ERROR);

- return;

+ goto out;

}

}

if (!bdrv_is_first_non_filter(state->old_bs)) {

error_setg(errp, QERR_FEATURE_DISABLED, "snapshot");

- return;

+ goto out;

}

if (action->type == TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC) {

@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,

if (node_name && !snapshot_node_name) {

error_setg(errp, "New snapshot node name missing");

- return;

+ goto out;

}

if (snapshot_node_name &&

bdrv_lookup_bs(snapshot_node_name, snapshot_node_name, NULL)) {

error_setg(errp, "New snapshot node name already in use");

- return;

+ goto out;

}

flags = state->old_bs->open_flags;

@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,

int64_t size = bdrv_getlength(state->old_bs);

if (size < 0) {

error_setg_errno(errp, -size, "bdrv_getlength failed");

- return;

+ goto out;

}

bdrv_img_create(new_image_file, format,

state->old_bs->filename,

@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,

NULL, size, flags, false, &local_err);

if (local_err) {

error_propagate(errp, local_err);

- return;

+ goto out;

}

}

@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,

errp);

/* We will manually add the backing_hd field to the bs later */

if (!state->new_bs) {

- return;

+ goto out;

}

if (bdrv_has_blk(state->new_bs)) {

error_setg(errp, "The snapshot is already in use");

- return;

+ goto out;

}

if (bdrv_op_is_blocked(state->new_bs, BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT,

errp)) {

- return;

+ goto out;

}

if (state->new_bs->backing != NULL) {

error_setg(errp, "The snapshot already has a backing image");

- return;

+ goto out;

}

if (!state->new_bs->drv->supports_backing) {

error_setg(errp, "The snapshot does not support backing images");

- return;

+ goto out;

}

- bdrv_set_aio_context(state->new_bs, state->aio_context);

+ bdrv_set_aio_context(state->new_bs, aio_context);

/* This removes our old bs and adds the new bs. This is an operation that

* can fail, so we need to do it in .prepare; undoing it for abort is

@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,

bdrv_append(state->new_bs, state->old_bs, &local_err);

if (local_err) {

error_propagate(errp, local_err);

- return;

+ goto out;

}

state->overlay_appended = true;

+

+out:

+ aio_context_release(aio_context);

}

static void external_snapshot_commit(BlkActionState *common)

{

ExternalSnapshotState *state =

DO_UPCAST(ExternalSnapshotState, common, common);

+ AioContext *aio_context;

+

+ aio_context = bdrv_get_aio_context(state->old_bs);

+ aio_context_acquire(aio_context);

/* We don't need (or want) to use the transactional

* bdrv_reopen_multiple() across all the entries at once, because we

@@ -XXX,XX +XXX,XX @@ static void external_snapshot_commit(BlkActionState *common)

bdrv_reopen(state->old_bs, state->old_bs->open_flags & ~BDRV_O_RDWR,

NULL);

}

+

+ aio_context_release(aio_context);

}

static void external_snapshot_abort(BlkActionState *common)

@@ -XXX,XX +XXX,XX @@ static void external_snapshot_abort(BlkActionState *common)

DO_UPCAST(ExternalSnapshotState, common, common);

if (state->new_bs) {

if (state->overlay_appended) {

+ AioContext *aio_context;

+

+ aio_context = bdrv_get_aio_context(state->old_bs);

+ aio_context_acquire(aio_context);

+

bdrv_ref(state->old_bs); /* we can't let bdrv_set_backind_hd()

close state->old_bs; we need it */

bdrv_set_backing_hd(state->new_bs, NULL, &error_abort);

bdrv_replace_node(state->new_bs, state->old_bs, &error_abort);

bdrv_unref(state->old_bs); /* bdrv_replace_node() ref'ed old_bs */

+

+ aio_context_release(aio_context);

}

}

}

@@ -XXX,XX +XXX,XX @@ static void external_snapshot_clean(BlkActionState *common)

{

ExternalSnapshotState *state =

DO_UPCAST(ExternalSnapshotState, common, common);

- if (state->aio_context) {

- bdrv_drained_end(state->old_bs);

- bdrv_unref(state->new_bs);

- aio_context_release(state->aio_context);

+ AioContext *aio_context;

+

+ if (!state->old_bs) {

+ return;

}

+

+ aio_context = bdrv_get_aio_context(state->old_bs);

+ aio_context_acquire(aio_context);

+

+ bdrv_drained_end(state->old_bs);

+ bdrv_unref(state->new_bs);

+

+ aio_context_release(aio_context);

}

typedef struct DriveBackupState {

--

2.14.3

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 XXXXXXX..XXXXXXX 100644

--- a/blockdev.c

+++ b/blockdev.c

@@ -XXX,XX +XXX,XX @@ static void external_snapshot_clean(BlkActionState *common)

typedef struct DriveBackupState {

BlkActionState common;

BlockDriverState *bs;

- AioContext *aio_context;

BlockJob *job;

} DriveBackupState;

@@ -XXX,XX +XXX,XX @@ static void drive_backup_prepare(BlkActionState *common, Error **errp)

DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);

BlockDriverState *bs;

DriveBackup *backup;

+ AioContext *aio_context;

Error *local_err = NULL;

assert(common->action->type == TRANSACTION_ACTION_KIND_DRIVE_BACKUP);

@@ -XXX,XX +XXX,XX @@ static void drive_backup_prepare(BlkActionState *common, Error **errp)

return;

}

- /* AioContext is released in .clean() */

- state->aio_context = bdrv_get_aio_context(bs);

- aio_context_acquire(state->aio_context);

+ aio_context = bdrv_get_aio_context(bs);

+ aio_context_acquire(aio_context);

+

+ /* Paired with .clean() */

bdrv_drained_begin(bs);

+

state->bs = bs;

state->job = do_drive_backup(backup, common->block_job_txn, &local_err);

if (local_err) {

error_propagate(errp, local_err);

- return;

+ goto out;

}

+

+out:

+ aio_context_release(aio_context);

}

static void drive_backup_commit(BlkActionState *common)

{

DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);

+ AioContext *aio_context;

+

+ aio_context = bdrv_get_aio_context(state->bs);

+ aio_context_acquire(aio_context);

+

assert(state->job);

block_job_start(state->job);

+

+ aio_context_release(aio_context);

}

static void drive_backup_abort(BlkActionState *common)

@@ -XXX,XX +XXX,XX @@ static void drive_backup_abort(BlkActionState *common)

DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);

if (state->job) {

+ AioContext *aio_context;

+

+ aio_context = bdrv_get_aio_context(state->bs);

+ aio_context_acquire(aio_context);

+

block_job_cancel_sync(state->job);

+

+ aio_context_release(aio_context);

}

}

static void drive_backup_clean(BlkActionState *common)

{

DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);

+ AioContext *aio_context;

- if (state->aio_context) {

- bdrv_drained_end(state->bs);

- aio_context_release(state->aio_context);

+ if (!state->bs) {

+ return;

}

+

+ aio_context = bdrv_get_aio_context(state->bs);

+ aio_context_acquire(aio_context);

+

+ bdrv_drained_end(state->bs);

+

+ aio_context_release(aio_context);

}

typedef struct BlockdevBackupState {

--

2.14.3

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 XXXXXXX..XXXXXXX 100644

--- a/blockdev.c

+++ b/blockdev.c

@@ -XXX,XX +XXX,XX @@ typedef struct BlockdevBackupState {

BlkActionState common;

BlockDriverState *bs;

BlockJob *job;

- AioContext *aio_context;

} BlockdevBackupState;

static BlockJob *do_blockdev_backup(BlockdevBackup *backup, BlockJobTxn *txn,

@@ -XXX,XX +XXX,XX @@ static void blockdev_backup_prepare(BlkActionState *common, Error **errp)

BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);

BlockdevBackup *backup;

BlockDriverState *bs, *target;

+ AioContext *aio_context;

Error *local_err = NULL;

assert(common->action->type == TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP);

@@ -XXX,XX +XXX,XX @@ static void blockdev_backup_prepare(BlkActionState *common, Error **errp)

return;

}

- /* AioContext is released in .clean() */

- state->aio_context = bdrv_get_aio_context(bs);

- if (state->aio_context != bdrv_get_aio_context(target)) {

- state->aio_context = NULL;

+ aio_context = bdrv_get_aio_context(bs);

+ if (aio_context != bdrv_get_aio_context(target)) {

error_setg(errp, "Backup between two IO threads is not implemented");

return;

}

- aio_context_acquire(state->aio_context);

+ aio_context_acquire(aio_context);

state->bs = bs;

+

+ /* Paired with .clean() */

bdrv_drained_begin(state->bs);

state->job = do_blockdev_backup(backup, common->block_job_txn, &local_err);

if (local_err) {

error_propagate(errp, local_err);

- return;

+ goto out;

}

+

+out:

+ aio_context_release(aio_context);

}

static void blockdev_backup_commit(BlkActionState *common)

{

BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);

+ AioContext *aio_context;

+

+ aio_context = bdrv_get_aio_context(state->bs);

+ aio_context_acquire(aio_context);

+

assert(state->job);

block_job_start(state->job);

+

+ aio_context_release(aio_context);

}

static void blockdev_backup_abort(BlkActionState *common)

@@ -XXX,XX +XXX,XX @@ static void blockdev_backup_abort(BlkActionState *common)

BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);

if (state->job) {

+ AioContext *aio_context;

+

+ aio_context = bdrv_get_aio_context(state->bs);

+ aio_context_acquire(aio_context);

+

block_job_cancel_sync(state->job);

+

+ aio_context_release(aio_context);

}

}

static void blockdev_backup_clean(BlkActionState *common)

{

BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);

+ AioContext *aio_context;

- if (state->aio_context) {

- bdrv_drained_end(state->bs);

- aio_context_release(state->aio_context);

+ if (!state->bs) {

+ return;

}

+

+ aio_context = bdrv_get_aio_context(state->bs);

+ aio_context_acquire(aio_context);

+

+ bdrv_drained_end(state->bs);

+