From 307a5f60ebe0506a5b90f323fdcabb4333405484 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 29 Apr 2019 17:40:14 +0200 Subject: block: Add qdev_prop_drive_iothread property type Some qdev block devices have support for iothreads and take care of the AioContext they are running in, but most devices don't know about any of this. For the latter category, the qdev drive property must make sure that their BlockBackend is in the main AioContext. Unfortunately, while the current code just does the same thing for devices that do support iothreads, this is not correct and it would show as soon as we actually try to keep a consistent AioContext assignment across all nodes and users of a block graph subtree: If a node is already in a non-default AioContext because of one of its users, attaching a new device should still be possible if that device can work in the same AioContext. Switching the node back to the main context first and only then into the device AioContext causes failure (because the existing user wouldn't allow the switch to the main context). So devices that support iothreads need a different kind of drive property that leaves the node in its current AioContext, but by using this type, the device promises to check later that it can work with this context. This patch adds the qdev infrastructure that allows devices to signal that they handle iothreads and qdev should leave the AioContext alone. Signed-off-by: Kevin Wolf --- include/hw/block/block.h | 7 +++++-- include/hw/qdev-properties.h | 3 +++ 2 files changed, 8 insertions(+), 2 deletions(-) (limited to 'include/hw') diff --git a/include/hw/block/block.h b/include/hw/block/block.h index d06f25aa0f..607539057a 100644 --- a/include/hw/block/block.h +++ b/include/hw/block/block.h @@ -45,8 +45,7 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf) return exp; } -#define DEFINE_BLOCK_PROPERTIES(_state, _conf) \ - DEFINE_PROP_DRIVE("drive", _state, _conf.blk), \ +#define DEFINE_BLOCK_PROPERTIES_BASE(_state, _conf) \ DEFINE_PROP_BLOCKSIZE("logical_block_size", _state, \ _conf.logical_block_size), \ DEFINE_PROP_BLOCKSIZE("physical_block_size", _state, \ @@ -59,6 +58,10 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf) ON_OFF_AUTO_AUTO), \ DEFINE_PROP_BOOL("share-rw", _state, _conf.share_rw, false) +#define DEFINE_BLOCK_PROPERTIES(_state, _conf) \ + DEFINE_PROP_DRIVE("drive", _state, _conf.blk), \ + DEFINE_BLOCK_PROPERTIES_BASE(_state, _conf) + #define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \ DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0), \ DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0), \ diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h index b6758c852e..1eae5ab056 100644 --- a/include/hw/qdev-properties.h +++ b/include/hw/qdev-properties.h @@ -28,6 +28,7 @@ extern const PropertyInfo qdev_prop_blockdev_on_error; extern const PropertyInfo qdev_prop_bios_chs_trans; extern const PropertyInfo qdev_prop_fdc_drive_type; extern const PropertyInfo qdev_prop_drive; +extern const PropertyInfo qdev_prop_drive_iothread; extern const PropertyInfo qdev_prop_netdev; extern const PropertyInfo qdev_prop_pci_devfn; extern const PropertyInfo qdev_prop_blocksize; @@ -198,6 +199,8 @@ extern const PropertyInfo qdev_prop_pcie_link_width; DEFINE_PROP(_n, _s, _f, qdev_prop_netdev, NICPeers) #define DEFINE_PROP_DRIVE(_n, _s, _f) \ DEFINE_PROP(_n, _s, _f, qdev_prop_drive, BlockBackend *) +#define DEFINE_PROP_DRIVE_IOTHREAD(_n, _s, _f) \ + DEFINE_PROP(_n, _s, _f, qdev_prop_drive_iothread, BlockBackend *) #define DEFINE_PROP_MACADDR(_n, _s, _f) \ DEFINE_PROP(_n, _s, _f, qdev_prop_macaddr, MACAddr) #define DEFINE_PROP_ON_OFF_AUTO(_n, _s, _f, _d) \ -- cgit 1.4.1 From 4f71fb436a6fbec7a7b2360cdba741deeb24be67 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 26 Apr 2019 19:29:47 +0200 Subject: scsi-disk: Use qdev_prop_drive_iothread This makes use of qdev_prop_drive_iothread for scsi-disk so that the disk can be attached to a node that is already in the target AioContext. We need to check that the HBA actually supports iothreads, otherwise scsi-disk must make sure that the node is already in the main AioContext. This changes the error message for conflicting iothread settings. Previously, virtio-scsi produced the error message, now it comes from blk_set_aio_context(). Update a test case accordingly. Signed-off-by: Kevin Wolf --- hw/scsi/scsi-disk.c | 22 +++++++++++++++------- hw/scsi/virtio-scsi.c | 15 ++++++++------- include/hw/scsi/scsi.h | 1 + tests/qemu-iotests/240.out | 2 +- 4 files changed, 25 insertions(+), 15 deletions(-) (limited to 'include/hw') diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index 91c5a8b1ac..7b89ac798b 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -2336,6 +2336,13 @@ static void scsi_realize(SCSIDevice *dev, Error **errp) return; } + if (blk_get_aio_context(s->qdev.conf.blk) != qemu_get_aio_context() && + !s->qdev.hba_supports_iothread) + { + error_setg(errp, "HBA does not support iothreads"); + return; + } + if (dev->type == TYPE_DISK) { if (!blkconf_geometry(&dev->conf, NULL, 65535, 255, 255, errp)) { return; @@ -2929,13 +2936,14 @@ static const TypeInfo scsi_disk_base_info = { .abstract = true, }; -#define DEFINE_SCSI_DISK_PROPERTIES() \ - DEFINE_BLOCK_PROPERTIES(SCSIDiskState, qdev.conf), \ - DEFINE_BLOCK_ERROR_PROPERTIES(SCSIDiskState, qdev.conf), \ - DEFINE_PROP_STRING("ver", SCSIDiskState, version), \ - DEFINE_PROP_STRING("serial", SCSIDiskState, serial), \ - DEFINE_PROP_STRING("vendor", SCSIDiskState, vendor), \ - DEFINE_PROP_STRING("product", SCSIDiskState, product), \ +#define DEFINE_SCSI_DISK_PROPERTIES() \ + DEFINE_PROP_DRIVE_IOTHREAD("drive", SCSIDiskState, qdev.conf.blk), \ + DEFINE_BLOCK_PROPERTIES_BASE(SCSIDiskState, qdev.conf), \ + DEFINE_BLOCK_ERROR_PROPERTIES(SCSIDiskState, qdev.conf), \ + DEFINE_PROP_STRING("ver", SCSIDiskState, version), \ + DEFINE_PROP_STRING("serial", SCSIDiskState, serial), \ + DEFINE_PROP_STRING("vendor", SCSIDiskState, vendor), \ + DEFINE_PROP_STRING("product", SCSIDiskState, product), \ DEFINE_PROP_STRING("device_id", SCSIDiskState, device_id) diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index 01c2b85f90..2994f0738f 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -789,6 +789,13 @@ static void virtio_scsi_change(SCSIBus *bus, SCSIDevice *dev, SCSISense sense) } } +static void virtio_scsi_pre_hotplug(HotplugHandler *hotplug_dev, + DeviceState *dev, Error **errp) +{ + SCSIDevice *sd = SCSI_DEVICE(dev); + sd->hba_supports_iothread = true; +} + static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev, Error **errp) { @@ -798,16 +805,9 @@ static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev, int ret; if (s->ctx && !s->dataplane_fenced) { - AioContext *ctx; if (blk_op_is_blocked(sd->conf.blk, BLOCK_OP_TYPE_DATAPLANE, errp)) { return; } - ctx = blk_get_aio_context(sd->conf.blk); - if (ctx != s->ctx && ctx != qemu_get_aio_context()) { - error_setg(errp, "Cannot attach a blockdev that is using " - "a different iothread"); - return; - } virtio_scsi_acquire(s); ret = blk_set_aio_context(sd->conf.blk, s->ctx, errp); virtio_scsi_release(s); @@ -990,6 +990,7 @@ static void virtio_scsi_class_init(ObjectClass *klass, void *data) vdc->reset = virtio_scsi_reset; vdc->start_ioeventfd = virtio_scsi_dataplane_start; vdc->stop_ioeventfd = virtio_scsi_dataplane_stop; + hc->pre_plug = virtio_scsi_pre_hotplug; hc->plug = virtio_scsi_hotplug; hc->unplug = virtio_scsi_hotunplug; } diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h index acef25faa4..426566a5c6 100644 --- a/include/hw/scsi/scsi.h +++ b/include/hw/scsi/scsi.h @@ -88,6 +88,7 @@ struct SCSIDevice int scsi_version; int default_scsi_version; bool needs_vpd_bl_emulation; + bool hba_supports_iothread; }; extern const VMStateDescription vmstate_scsi_device; diff --git a/tests/qemu-iotests/240.out b/tests/qemu-iotests/240.out index 84e0a43ce5..d00df50297 100644 --- a/tests/qemu-iotests/240.out +++ b/tests/qemu-iotests/240.out @@ -43,7 +43,7 @@ QMP_VERSION {"return": {}} {"return": {}} {"return": {}} -{"error": {"class": "GenericError", "desc": "Cannot attach a blockdev that is using a different iothread"}} +{"error": {"class": "GenericError", "desc": "Cannot change iothread of active block backend"}} {"return": {}} {"return": {}} {"return": {}} -- cgit 1.4.1