[PATCH RFC-v2 00/11] qemu: Add support for iothread to virtqueue mapping for 'virtio-scsi'
Unfortunately still RFC as the qemu support is still not merged. Once again there are a few patches that are applicable: qemu: Use modern header formatting in 'qemu_command.h' virXMLNodeGetSubelementList: Document return value semantics virDomainIothreadMappingDefParse: Fix usage of virXMLNodeGetSubelementList docs: formatdomain: Clarify configuration of iothread <-> virtqueue mapping The patches below are RFC: qemucapabilitiestest: Update 'caps_10.0.0_x86_64' to XXXXXX qemu: capabilities: Introduce QEMU_CAPS_VIRTIO_SCSI_IOTHREAD_MAPPING virDomainIothreadMapping: Add support for naming certain virt queues conf: Add support for iothread to queue mapping config for 'virtio-scsi' qemu: Implement support for iothread <-> virtqueue mapping for 'virtio-scsi' controllers qemuxmlconftest: Add 'iothreads-virtio-scsi-mapping' case NEWS: Mention multiple iothread support for 'virtio-scsi' controller Changes to v1: - pushed the non-RFC refactors - few new cleanups added - new docs patch emphasizing the auto-assignment (in example XML and description) in qemu and improved wording of docs for 'virtio-blk' miltiqueue mapping - changed code to use names for 'ctrl' and 'event' virtqueues of 'virtio-scsi' - used the improved docs also for virtio scsi - added NEWS NEWS.rst | 6 + docs/formatdomain.rst | 66 ++++++++++- src/conf/domain_conf.c | 32 ++++-- src/conf/domain_conf.h | 4 +- src/conf/domain_validate.c | 10 +- src/conf/schemas/domaincommon.rng | 9 +- src/hypervisor/domain_driver.c | 3 +- src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 86 ++++++++++++-- src/qemu/qemu_command.h | 95 ++++++++++------ src/qemu/qemu_domain.c | 10 +- src/qemu/qemu_validate.c | 105 ++++++++++++------ src/util/virxml.c | 2 + .../caps_10.0.0_x86_64.replies | 7 +- .../caps_10.0.0_x86_64.xml | 3 +- ...ads-virtio-scsi-mapping.x86_64-latest.args | 40 +++++++ ...eads-virtio-scsi-mapping.x86_64-latest.xml | 65 +++++++++++ .../iothreads-virtio-scsi-mapping.xml | 57 ++++++++++ tests/qemuxmlconftest.c | 1 + 20 files changed, 495 insertions(+), 109 deletions(-) create mode 100644 tests/qemuxmlconfdata/iothreads-virtio-scsi-mapping.x86_64-latest.args create mode 100644 tests/qemuxmlconfdata/iothreads-virtio-scsi-mapping.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/iothreads-virtio-scsi-mapping.xml -- 2.48.1
The file used intermixed style. Convert the last outliers to the new formatting style. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.h | 85 ++++++++++++++++++++++++----------------- 1 file changed, 50 insertions(+), 35 deletions(-) diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 76c514b5f7..7c891a6c98 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -40,31 +40,37 @@ VIR_ENUM_DECL(qemuVideo); VIR_ENUM_DECL(qemuSoundCodec); -virCommand *qemuBuildCommandLine(virDomainObj *vm, - const char *migrateURI, - virDomainMomentObj *snapshot, - virNetDevVPortProfileOp vmop, - size_t *nnicindexes, - int **nicindexes); +virCommand * +qemuBuildCommandLine(virDomainObj *vm, + const char *migrateURI, + virDomainMomentObj *snapshot, + virNetDevVPortProfileOp vmop, + size_t *nnicindexes, + int **nicindexes); /* Generate the object properties for pr-manager */ -virJSONValue *qemuBuildPRManagerInfoProps(virStorageSource *src); -virJSONValue *qemuBuildPRManagedManagerInfoProps(qemuDomainObjPrivate *priv); +virJSONValue * +qemuBuildPRManagerInfoProps(virStorageSource *src); +virJSONValue * +qemuBuildPRManagedManagerInfoProps(qemuDomainObjPrivate *priv); -virJSONValue *qemuBuildDBusVMStateInfoProps(virQEMUDriver *driver, - virDomainObj *vm); +virJSONValue * +qemuBuildDBusVMStateInfoProps(virQEMUDriver *driver, + virDomainObj *vm); /* Generate the object properties for a secret */ -int qemuBuildSecretInfoProps(qemuDomainSecretInfo *secinfo, - virJSONValue **propsret); +int +qemuBuildSecretInfoProps(qemuDomainSecretInfo *secinfo, + virJSONValue **propsret); /* Generate the object properties for a tls-creds-x509 */ -int qemuBuildTLSx509BackendProps(const char *tlspath, - bool isListen, - bool verifypeer, - const char *alias, - const char *secalias, - virJSONValue **propsret); +int +qemuBuildTLSx509BackendProps(const char *tlspath, + bool isListen, + bool verifypeer, + const char *alias, + const char *secalias, + virJSONValue **propsret); /* Open a UNIX socket for chardev FD passing */ int @@ -93,7 +99,8 @@ qemuBuildNicDevProps(virDomainDef *def, virDomainNetDef *net, virQEMUCaps *qemuCaps); -bool qemuDiskBusIsSD(int bus); +bool +qemuDiskBusIsSD(int bus); int qemuBuildStorageSourceAttachPrepareCommon(virStorageSource *src, @@ -134,15 +141,16 @@ qemuBuildControllerDevProps(const virDomainDef *domainDef, virQEMUCaps *qemuCaps, virJSONValue **devprops); -int qemuBuildMemoryBackendProps(virJSONValue **backendProps, - const char *alias, - virQEMUDriverConfig *cfg, - qemuDomainObjPrivate *priv, - const virDomainDef *def, - const virDomainMemoryDef *mem, - bool force, - bool systemMemory, - virBitmap **nodemaskRet); +int +qemuBuildMemoryBackendProps(virJSONValue **backendProps, + const char *alias, + virQEMUDriverConfig *cfg, + qemuDomainObjPrivate *priv, + const virDomainDef *def, + const virDomainMemoryDef *mem, + bool force, + bool systemMemory, + virBitmap **nodemaskRet); virJSONValue * qemuBuildMemoryDeviceProps(virQEMUDriverConfig *cfg, @@ -166,8 +174,9 @@ virJSONValue * qemuBuildRNGDevProps(const virDomainDef *def, virDomainRNGDef *dev, virQEMUCaps *qemuCaps); -int qemuBuildRNGBackendProps(virDomainRNGDef *rng, - virJSONValue **props); +int +qemuBuildRNGBackendProps(virDomainRNGDef *rng, + virJSONValue **props); /* Current, best practice */ virJSONValue * @@ -210,10 +219,12 @@ qemuBuildZPCIDevProps(virDomainDeviceInfo *dev); bool qemuDiskConfigBlkdeviotuneEnabled(const virDomainDiskDef *disk); -virJSONValue *qemuBuildHotpluggableCPUProps(const virDomainVcpuDef *vcpu) +virJSONValue * +qemuBuildHotpluggableCPUProps(const virDomainVcpuDef *vcpu) ATTRIBUTE_NONNULL(1); -virJSONValue *qemuBuildShmemBackendMemProps(virDomainShmemDef *shmem) +virJSONValue * +qemuBuildShmemBackendMemProps(virDomainShmemDef *shmem) ATTRIBUTE_NONNULL(1); bool @@ -254,6 +265,10 @@ qemuBuildTPMOpenBackendFDs(const char *tpmdev, int *cancelfd) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) G_NO_INLINE; -const char * qemuAudioDriverTypeToString(virDomainAudioType type); -virDomainAudioType qemuAudioDriverTypeFromString(const char *str); -int qemuVDPAConnect(const char *devicepath) G_NO_INLINE; +const char * +qemuAudioDriverTypeToString(virDomainAudioType type); +virDomainAudioType +qemuAudioDriverTypeFromString(const char *str); + +int +qemuVDPAConnect(const char *devicepath) G_NO_INLINE; -- 2.48.1
On a Monday in 2025, Peter Krempa wrote:
The file used intermixed style. Convert the last outliers to the new formatting style.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.h | 85 ++++++++++++++++++++++++----------------- 1 file changed, 50 insertions(+), 35 deletions(-)
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 76c514b5f7..7c891a6c98 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -40,31 +40,37 @@ VIR_ENUM_DECL(qemuVideo); VIR_ENUM_DECL(qemuSoundCodec);
-virCommand *qemuBuildCommandLine(virDomainObj *vm, - const char *migrateURI, - virDomainMomentObj *snapshot, - virNetDevVPortProfileOp vmop, - size_t *nnicindexes, - int **nicindexes); +virCommand * +qemuBuildCommandLine(virDomainObj *vm, + const char *migrateURI, + virDomainMomentObj *snapshot, + virNetDevVPortProfileOp vmop, + size_t *nnicindexes, + int **nicindexes);
/* Generate the object properties for pr-manager */ -virJSONValue *qemuBuildPRManagerInfoProps(virStorageSource *src); -virJSONValue *qemuBuildPRManagedManagerInfoProps(qemuDomainObjPrivate *priv); +virJSONValue * +qemuBuildPRManagerInfoProps(virStorageSource *src); +virJSONValue * +qemuBuildPRManagedManagerInfoProps(qemuDomainObjPrivate *priv);
-virJSONValue *qemuBuildDBusVMStateInfoProps(virQEMUDriver *driver, - virDomainObj *vm); +virJSONValue * +qemuBuildDBusVMStateInfoProps(virQEMUDriver *driver, + virDomainObj *vm);
/* Generate the object properties for a secret */ -int qemuBuildSecretInfoProps(qemuDomainSecretInfo *secinfo, - virJSONValue **propsret); +int +qemuBuildSecretInfoProps(qemuDomainSecretInfo *secinfo, + virJSONValue **propsret);
/* Generate the object properties for a tls-creds-x509 */ -int qemuBuildTLSx509BackendProps(const char *tlspath, - bool isListen, - bool verifypeer, - const char *alias, - const char *secalias, - virJSONValue **propsret); +int +qemuBuildTLSx509BackendProps(const char *tlspath, + bool isListen, + bool verifypeer, + const char *alias, + const char *secalias, + virJSONValue **propsret);
/* Open a UNIX socket for chardev FD passing */ int
Here, qemuOpenChrChardevUNIXSocket(const virDomainChrSourceDef *dev) G_NO_INLINE; still has the G_NO_INLINE attribute annotation on the same line
@@ -254,6 +265,10 @@ qemuBuildTPMOpenBackendFDs(const char *tpmdev, int *cancelfd) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) G_NO_INLINE;
-const char * qemuAudioDriverTypeToString(virDomainAudioType type); -virDomainAudioType qemuAudioDriverTypeFromString(const char *str); -int qemuVDPAConnect(const char *devicepath) G_NO_INLINE; +const char * +qemuAudioDriverTypeToString(virDomainAudioType type); +virDomainAudioType +qemuAudioDriverTypeFromString(const char *str); + +int +qemuVDPAConnect(const char *devicepath) G_NO_INLINE;
Same here. Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
The returned value is always non-NULL. Callers need to check the lenght of the returned array instead. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virxml.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/virxml.c b/src/util/virxml.c index 670cace4ab..d5c7ebaf22 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -909,6 +909,8 @@ virXMLNodeGetSubelement(xmlNodePtr node, * Find and return a sub-elements node of @node named @name in a GPtrArray * populated with the xmlNodePtr objects. Caller is responsible for freeing the * array but not the contained xmlNode objects. + * + * Note: The returned GPtrArray is non-NULL even if @node doesn't contain such sub-elements */ GPtrArray * virXMLNodeGetSubelementList(xmlNodePtr node, -- 2.48.1
On a Monday in 2025, Peter Krempa wrote:
The returned value is always non-NULL. Callers need to check the lenght
length
of the returned array instead.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virxml.c | 2 ++ 1 file changed, 2 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
virXMLNodeGetSubelementList always returns a non-NULL pointers thus we should check the lenght instead. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5630a469be..2b9955a1a0 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7950,19 +7950,21 @@ virDomainIothreadMappingDefParse(xmlNodePtr driverNode, if (!(iothreadsNode = virXMLNodeGetSubelement(driverNode, "iothreads"))) return 0; - if (!(iothreadNodes = virXMLNodeGetSubelementList(iothreadsNode, "iothread"))) + iothreadNodes = virXMLNodeGetSubelementList(iothreadsNode, "iothread"); + + if (iothreadNodes->len == 0) return 0; for (i = 0; i < iothreadNodes->len; i++) { xmlNodePtr iothNode = g_ptr_array_index(iothreadNodes, i); g_autoptr(virDomainIothreadMappingDef) iothdef = g_new0(virDomainIothreadMappingDef, 1); - g_autoptr(GPtrArray) queueNodes = NULL; + g_autoptr(GPtrArray) queueNodes = virXMLNodeGetSubelementList(iothNode, "queue"); if (virXMLPropUInt(iothNode, "id", 10, VIR_XML_PROP_REQUIRED, &iothdef->id) < 0) return -1; - if ((queueNodes = virXMLNodeGetSubelementList(iothNode, "queue"))) { + if (queueNodes->len > 0) { size_t q; iothdef->queues = g_new0(unsigned int, queueNodes->len); -- 2.48.1
On a Monday in 2025, Peter Krempa wrote:
virXMLNodeGetSubelementList always returns a non-NULL pointers thus we should check the lenght instead.
*length
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Add an example for the automatic/round-robin mapping of iothreads which users should preferrably use. Until now the example contained even the full mapping which could push users to use that instead. Mention that the queues are then automatically distributed among the iothreads. Also clarify the need to set 'queues' when mapping threads explicitly and how the queues are identified. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatdomain.rst | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index cbe378e61d..29f3c84780 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -3442,10 +3442,27 @@ paravirtualized driver is specified via the ``disk`` element. *Note:* ``iothread`` is mutually exclusive with ``iothreads``. - The optional ``iothreads`` sub-element allows specifying multiple IOThreads via the ``iothread`` sub-element with attribute ``id`` the disk will use - for I/O operations. Optionally the ``iothread`` element can have multiple - ``queue`` subelements specifying that given iothread should be used to - handle given queues. :since:`Since 10.0.0 (QEMU 9.0, virtio disks only)`. - Example:: + for I/O operations. The virt queues (see ``queues`` attribute below) are + automatically distributed among the configured iothreads. + + Optionally the ``iothread`` element can have multiple ``queue`` + subelements with mandatory ``id`` atribute specifying that the iothread + should be used to handle given virt queue. If queue mapping is present + the ``queues`` attribute of ``driver`` (see below) must be configured + and all configured virt queues must be included in the mapping. The + ``virtio-blk`` device exposes request virt queues ``0`` to ``N-1`` + where N is the number of queues configured for the device. + + :since:`Since 10.0.0 (QEMU 9.0, virtio disks only)`. + + Examples:: + + <driver name='qemu' queues='4> + <iothreads> + <iothread id='2'/> + <iothread id='3'/> + </iothreads> + </driver> <driver name='qemu' queues='3'> <iothreads> -- 2.48.1
On a Monday in 2025, Peter Krempa wrote:
Add an example for the automatic/round-robin mapping of iothreads which users should preferrably use. Until now the example contained even the full mapping which could push users to use that instead.
Mention that the queues are then automatically distributed among the iothreads.
Also clarify the need to set 'queues' when mapping threads explicitly and how the queues are identified.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatdomain.rst | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index cbe378e61d..29f3c84780 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -3442,10 +3442,27 @@ paravirtualized driver is specified via the ``disk`` element. *Note:* ``iothread`` is mutually exclusive with ``iothreads``. - The optional ``iothreads`` sub-element allows specifying multiple IOThreads via the ``iothread`` sub-element with attribute ``id`` the disk will use - for I/O operations. Optionally the ``iothread`` element can have multiple - ``queue`` subelements specifying that given iothread should be used to - handle given queues. :since:`Since 10.0.0 (QEMU 9.0, virtio disks only)`. - Example:: + for I/O operations. The virt queues (see ``queues`` attribute below) are + automatically distributed among the configured iothreads. + + Optionally the ``iothread`` element can have multiple ``queue`` + subelements with mandatory ``id`` atribute specifying that the iothread + should be used to handle given virt queue. If queue mapping is present + the ``queues`` attribute of ``driver`` (see below) must be configured
You already said "see below" in the paragraph above :)
+ and all configured virt queues must be included in the mapping. The + ``virtio-blk`` device exposes request virt queues ``0`` to ``N-1`` + where N is the number of queues configured for the device. + + :since:`Since 10.0.0 (QEMU 9.0, virtio disks only)`. + + Examples:: + + <driver name='qemu' queues='4>
Missing apostrophe. Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
+ <iothreads> + <iothread id='2'/> + <iothread id='3'/> + </iothreads> + </driver>
<driver name='qemu' queues='3'> <iothreads> -- 2.48.1
Notable changes: - 'virtio-scsi' supports 'iothread-vq-mapping' - 'nbd-server-start' command supports 'handshake-max-seconds' argument --- tests/qemucapabilitiesdata/caps_10.0.0_x86_64.replies | 7 ++++++- tests/qemucapabilitiesdata/caps_10.0.0_x86_64.xml | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/qemucapabilitiesdata/caps_10.0.0_x86_64.replies b/tests/qemucapabilitiesdata/caps_10.0.0_x86_64.replies index 4ac65390b4..264516201e 100644 --- a/tests/qemucapabilitiesdata/caps_10.0.0_x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_10.0.0_x86_64.replies @@ -20,7 +20,7 @@ "minor": 2, "major": 9 }, - "package": "v9.2.0-1967-gb69801dd6b" + "package": "v9.2.0-1979-gfdb6913c23" }, "id": "libvirt-2" } @@ -28064,6 +28064,11 @@ "description": "on/off", "type": "bool" }, + { + "name": "iothread-vq-mapping", + "description": "IOThread virtqueue mapping list [{\"iothread\":\"<id>\", \"vqs\":[1,2,3,...]},...]", + "type": "IOThreadVirtQueueMappingList" + }, { "default-value": 4294967295, "name": "num_queues", diff --git a/tests/qemucapabilitiesdata/caps_10.0.0_x86_64.xml b/tests/qemucapabilitiesdata/caps_10.0.0_x86_64.xml index 358e06b803..6694d7562f 100644 --- a/tests/qemucapabilitiesdata/caps_10.0.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_10.0.0_x86_64.xml @@ -214,7 +214,7 @@ <flag name='blockdev-set-active'/> <version>9002050</version> <microcodeVersion>43100285</microcodeVersion> - <package>v9.2.0-1967-gb69801dd6b</package> + <package>v9.2.0-1979-gfdb6913c23</package> <arch>x86_64</arch> <hostCPU type='kvm' model='base' migratability='yes'> <property name='avx-ne-convert' type='boolean' value='false'/> -- 2.48.1
The 'virtio-scsi' controller now supports iothread<->virtqueue mapping configuration. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_10.0.0_x86_64.xml | 1 + 3 files changed, 4 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 23b466c36e..19511e2333 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -728,6 +728,7 @@ VIR_ENUM_IMPL(virQEMUCaps, "machine.virt.aia", /* QEMU_CAPS_MACHINE_VIRT_AIA */ "virtio-mem-ccw", /* QEMU_CAPS_DEVICE_VIRTIO_MEM_CCW */ "blockdev-set-active", /* QEMU_CAPS_BLOCKDEV_SET_ACTIVE */ + "virtio-scsi.iothread-mapping", /* QEMU_CAPS_VIRTIO_SCSI_IOTHREAD_MAPPING */ ); @@ -1478,6 +1479,7 @@ static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsSpaprPCIHostBrid static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsVirtioSCSI[] = { { "acpi-index", QEMU_CAPS_ACPI_INDEX, NULL }, + { "iothread-vq-mapping", QEMU_CAPS_VIRTIO_SCSI_IOTHREAD_MAPPING, NULL }, }; static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsVfioPCI[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index ee71331a09..feca9bd185 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -707,6 +707,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_MACHINE_VIRT_AIA, /* -machine virt,aia=(none|aplic|aplic-imsic), RISC-V only */ QEMU_CAPS_DEVICE_VIRTIO_MEM_CCW, /* -device virtio-mem-ccw */ QEMU_CAPS_BLOCKDEV_SET_ACTIVE, /* blockdev-set-active QMP command supported */ + QEMU_CAPS_VIRTIO_SCSI_IOTHREAD_MAPPING, /* virtio-scsi supports per-virtqueue iothread mapping */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_10.0.0_x86_64.xml b/tests/qemucapabilitiesdata/caps_10.0.0_x86_64.xml index 6694d7562f..2ce6c5e62d 100644 --- a/tests/qemucapabilitiesdata/caps_10.0.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_10.0.0_x86_64.xml @@ -212,6 +212,7 @@ <flag name='netdev-stream-reconnect-miliseconds'/> <flag name='migrate-incoming.exit-on-error'/> <flag name='blockdev-set-active'/> + <flag name='virtio-scsi.iothread-mapping'/> <version>9002050</version> <microcodeVersion>43100285</microcodeVersion> <package>v9.2.0-1979-gfdb6913c23</package> -- 2.48.1
The 'virtio-scsi' device has two extra queues 'ctrl' and 'event'. Convert the existing code to treat the 'id' virtqueue attribute as a string so that we can use the names of the queues instead of making the user refer to them via magic numbers. This patch converts the code to store the strings instead and modifies the qemu code to pass a mapping hash table to lookup the numbers that qemu understands. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 14 ++++---- src/conf/domain_conf.h | 3 +- src/qemu/qemu_command.c | 64 +++++++++++++++++++++++++++++++----- src/qemu/qemu_command.h | 8 +++++ src/qemu/qemu_domain.c | 10 +++--- src/qemu/qemu_validate.c | 71 +++++++++++++++++++++++----------------- 6 files changed, 116 insertions(+), 54 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2b9955a1a0..bb82264758 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2373,7 +2373,7 @@ virDomainIothreadMappingDefFree(virDomainIothreadMappingDef *def) if (!def) return; - g_free(def->queues); + g_strfreev(def->queues); g_free(def); } @@ -7967,14 +7967,12 @@ virDomainIothreadMappingDefParse(xmlNodePtr driverNode, if (queueNodes->len > 0) { size_t q; - iothdef->queues = g_new0(unsigned int, queueNodes->len); - iothdef->nqueues = queueNodes->len; + iothdef->queues = g_new0(char *, queueNodes->len + 1); for (q = 0; q < queueNodes->len; q++) { xmlNodePtr queueNode = g_ptr_array_index(queueNodes, q); - if (virXMLPropUInt(queueNode, "id", 10, VIR_XML_PROP_REQUIRED, - &(iothdef->queues[q])) < 0) + if (!(iothdef->queues[q] = virXMLPropStringRequired(queueNode, "id"))) return -1; } } @@ -23182,10 +23180,10 @@ virDomainIothreadMappingDefFormat(virBuffer *buf, virBufferAsprintf(&iothreadAttrBuf, " id='%u'", iothDef->id); if (iothDef->queues) { - size_t q; + char **q; - for (q = 0; q < iothDef->nqueues; q++) - virBufferAsprintf(&iothreadChildBuf, "<queue id='%u'/>\n", iothDef->queues[q]); + for (q = iothDef->queues; *q; q++) + virBufferEscapeString(&iothreadChildBuf, "<queue id='%s'/>\n", *q); } virXMLFormatElement(&iothreadsChildBuf, "iothread", &iothreadAttrBuf, &iothreadChildBuf); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index d4fa79cb84..d56a5a22e0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -511,8 +511,7 @@ struct _virDomainIothreadMappingDef { unsigned int id; /* optional list of virtqueues the iothread should handle */ - unsigned int *queues; - size_t nqueues; + char **queues; }; typedef struct _virDomainIothreadMappingDef virDomainIothreadMappingDef; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0ad73af335..14d62b98e4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1574,7 +1574,8 @@ qemuBuildDriveStr(virDomainDiskDef *disk) static virJSONValue * -qemuBuildIothreadMappingProps(GSList *iothreads) +qemuBuildIothreadMappingProps(GSList *iothreads, + GHashTable *queuemap) { g_autoptr(virJSONValue) ret = virJSONValueNewArray(); GSList *n; @@ -1584,13 +1585,20 @@ qemuBuildIothreadMappingProps(GSList *iothreads) g_autoptr(virJSONValue) props = NULL; g_autoptr(virJSONValue) queues = NULL; g_autofree char *alias = g_strdup_printf("iothread%u", ioth->id); - size_t i; - if (ioth->nqueues > 0) { + if (ioth->queues) { + char **q; queues = virJSONValueNewArray(); - for (i = 0; i < ioth->nqueues; i++) { - g_autoptr(virJSONValue) vq = virJSONValueNewNumberUint(ioth->queues[i]); + for (q = ioth->queues; *q; q++) { + g_autoptr(virJSONValue) vq = NULL; + struct qemuVirtioIothreadMap *mapent; + + if (!(mapent = g_hash_table_lookup(queuemap, *q))) { + return NULL; + } + + vq = virJSONValueNewNumberUint(mapent->id); if (virJSONValueArrayAppend(queues, &vq)) return NULL; @@ -1612,6 +1620,43 @@ qemuBuildIothreadMappingProps(GSList *iothreads) } +static GHashTable * +qemuCommandGetVirtioIothreadMap(size_t nqueues, + const char **internal_queues) +{ + g_autoptr(GHashTable) ret = virHashNew(g_free); + size_t qid = 0; + const char **q; + size_t i; + + for (q = internal_queues; q && *q; q++) { + struct qemuVirtioIothreadMap *ent = g_new0(struct qemuVirtioIothreadMap, 1); + + ent->id = qid++; + + g_hash_table_insert(ret, g_strdup(*q), ent); + } + + + for (i = 0; i < nqueues; i++) { + struct qemuVirtioIothreadMap *ent = g_new0(struct qemuVirtioIothreadMap, 1); + + ent->id = qid++; + + g_hash_table_insert(ret, g_strdup_printf("%zu", i), ent); + } + + return g_steal_pointer(&ret); +} + + +GHashTable * +qemuCommandGetVirtioIothreadMapVirtioBlk(const virDomainDiskDef *disk) +{ + return qemuCommandGetVirtioIothreadMap(disk->queues, NULL); +} + + virJSONValue * qemuBuildDiskDeviceProps(const virDomainDef *def, virDomainDiskDef *disk, @@ -1686,9 +1731,12 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, if (disk->iothread > 0) iothread = g_strdup_printf("iothread%u", disk->iothread); - if (disk->iothreads && - !(iothreadMapping = qemuBuildIothreadMappingProps(disk->iothreads))) - return NULL; + if (disk->iothreads) { + g_autoptr(GHashTable) queuemap = qemuCommandGetVirtioIothreadMapVirtioBlk(disk); + + if (!(iothreadMapping = qemuBuildIothreadMappingProps(disk->iothreads, queuemap))) + return NULL; + } if (virStorageSourceGetActualType(disk->src) != VIR_STORAGE_TYPE_VHOST_USER && virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_BLK_SCSI)) { diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 7c891a6c98..636c2571d4 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -272,3 +272,11 @@ qemuAudioDriverTypeFromString(const char *str); int qemuVDPAConnect(const char *devicepath) G_NO_INLINE; + +struct qemuVirtioIothreadMap { + unsigned int id; + bool seen; +}; + +GHashTable * +qemuCommandGetVirtioIothreadMapVirtioBlk(const virDomainDiskDef *disk); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8be2181156..d16429b6a2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6646,7 +6646,6 @@ qemuDomainDiskChangeSupportedIothreads(virDomainDiskDef *disk, while (true) { virDomainIothreadMappingDef *old_def; virDomainIothreadMappingDef *new_def; - size_t i; /* match - both empty or both at the end */ if (!old && !new) @@ -6660,13 +6659,12 @@ qemuDomainDiskChangeSupportedIothreads(virDomainDiskDef *disk, new_def = new->data; if (old_def->id != new_def->id || - old_def->nqueues != new_def->nqueues) + !!old_def->queues != !!new_def->queues) goto fail; - for (i = 0; i < old_def->nqueues; i++) { - if (old_def->queues[i] != new_def->queues[i]) - goto fail; - } + if (old_def->queues && + !g_strv_equal((const char **) old_def->queues, (const char **) new_def->queues)) + goto fail; new = new->next; old = old->next; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index f3ef1be660..f8586a7dff 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2796,12 +2796,10 @@ qemuValidateDomainDeviceDefDiskSerial(const char *value) static int qemuDomainValidateIothreadMapping(const virDomainDef *def, GSList *iothreads, - size_t queues) + GHashTable *queueMap) { virDomainIothreadMappingDef *first_ioth; - g_autoptr(virBitmap) queueMap = NULL; g_autoptr(GHashTable) iothreadMap = virHashNew(NULL); - ssize_t unused; GSList *n; if (!iothreads) @@ -2810,13 +2808,11 @@ qemuDomainValidateIothreadMapping(const virDomainDef *def, first_ioth = iothreads->data; if (first_ioth->queues) { - if (queues == 0) { + if (!queueMap) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("'queue' count must be configured for explicit iothread to queue mapping")); return -1; } - - queueMap = virBitmapNew(queues); } /* we are validating that: @@ -2831,7 +2827,6 @@ qemuDomainValidateIothreadMapping(const virDomainDef *def, for (n = iothreads; n; n = n->next) { virDomainIothreadMappingDef *ioth = n->data; g_autofree char *alias = g_strdup_printf("iothread%u", ioth->id); - size_t i; if (g_hash_table_contains(iothreadMap, alias)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -2848,40 +2843,51 @@ qemuDomainValidateIothreadMapping(const virDomainDef *def, return -1; } - if (!!queueMap != !!ioth->queues) { + if (!!first_ioth->queues != !!ioth->queues) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("iothread to queue mapping must be provided for all iothreads or for none")); return -1; } - for (i = 0; i < ioth->nqueues; i++) { - bool hasMapping; + if (ioth->queues) { + char **q; - if (virBitmapGetBit(queueMap, ioth->queues[i], &hasMapping) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("iothread queue '%1$u' mapping out of range"), - ioth->queues[i]); - return -1; - } + for (q = ioth->queues; *q; q++) { + struct qemuVirtioIothreadMap *mapent; - if (hasMapping) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("iothread queue '%1$u' is already assigned"), - ioth->queues[i]); - return -1; - } + if (!(mapent = g_hash_table_lookup(queueMap, *q))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("iothread queue '%1$s' not available"), + *q); + return -1; + } - ignore_value(virBitmapSetBit(queueMap, ioth->queues[i])); + if (mapent->seen) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("iothread queue '%1$s' is already assigned"), + *q); + return -1; + } + mapent->seen = true; + } } } - if (queueMap) { - if ((unused = virBitmapNextClearBit(queueMap, -1)) >= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("missing iothread mapping for queue '%1$zd'"), - unused); - return -1; + if (queueMap && first_ioth->queues) { + GHashTableIter htitr; + char *name; + struct qemuVirtioIothreadMap *mapent; + + g_hash_table_iter_init(&htitr, queueMap); + + while (g_hash_table_iter_next(&htitr, (void **) &name, (void **) &mapent)) { + if (!mapent->seen) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("missing iothread mapping for queue '%1$s'"), + name); + return -1; + } } } @@ -2894,6 +2900,8 @@ qemuValidateDomainDeviceDefDiskIOThreads(const virDomainDef *def, const virDomainDiskDef *disk, virQEMUCaps *qemuCaps) { + g_autoptr(GHashTable) queueMap = NULL; + if (disk->iothread == 0 && !disk->iothreads) return 0; @@ -2925,7 +2933,10 @@ qemuValidateDomainDeviceDefDiskIOThreads(const virDomainDef *def, return -1; } - if (qemuDomainValidateIothreadMapping(def, disk->iothreads, disk->queues) < 0) + if (disk->queues > 0) + queueMap = qemuCommandGetVirtioIothreadMapVirtioBlk(disk); + + if (qemuDomainValidateIothreadMapping(def, disk->iothreads, queueMap) < 0) return -1; if (disk->iothread != 0 && -- 2.48.1
Upcoming qemu release will support configuring mapping iothreads to virtio queues for 'virtio-scsi' controllers in order to improve performance. Reuse the infrastructure we have from the same configuration for 'virti-blk' to implement the conf support for this feature. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatdomain.rst | 41 +++++++++++++++++++++++++++++++ src/conf/domain_conf.c | 10 +++++++- src/conf/domain_conf.h | 1 + src/conf/domain_validate.c | 10 +++++++- src/conf/schemas/domaincommon.rng | 9 ++++++- src/hypervisor/domain_driver.c | 3 ++- 6 files changed, 70 insertions(+), 4 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 29f3c84780..d049f28a65 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -4146,6 +4146,47 @@ An optional sub-element ``driver`` can specify the driver specific options: If a specific IOThread is desired for a specific SCSI ``disk``, then multiple controllers must be defined each having a specific ``iothread`` value. The ``iothread`` value must be within the range 1 to the domain iothreads value. +``iothreads`` + Supported for ``virtio-scsi`` controllers using ``address`` types ``pci`` and + ``ccw``. :since:`since 11.2.0 (QEMU 10.0).` Mutually exclusive with ``iothread``. + + The optional ``iothreads`` sub-element allows specifying multiple IOThreads + via the ``iothread`` sub-element with attribute ``id`` the ``virtio-scsi`` + controller will use for I/O operations. The virt queues (see ``queues`` + attribute of ``driver``) are automatically distributed among the configured + iothreads. + + Optionally the ``iothread`` element can have multiple ``queue`` subelements + with mandatory ``id`` atribute specifying that the iothread should be used + to handle given virt queue. If queue mapping is present the ``queues`` + attribute of ``driver`` must be configured and all configured virt queues + must be included in the mapping. The ``virtio-scsi`` controller exposes two + special queues named ``ctrl`` and ``event`` and request virt queues ``0`` to + ``N-1`` where N is the number of queues configured for the device. + + Example:: + + <driver queues='4> + <iothreads> + <iothread id='2'/> + <iothread id='3'/> + </iothreads> + </driver> + + <driver queues='3'> + <iothreads> + <iothread id='2'> + <queue id='ctrl'/> + <queue id='event'/> + <queue id='1'/> + </iothread> + <iothread id='3'> + <queue id='0'/> + <queue id='2'/> + </iothread> + </iothreads> + </driver> + virtio options For virtio controllers, `Virtio-related options`_ can also be set. ( :since:`Since 3.5.0` ) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bb82264758..def2ef7ae7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2557,6 +2557,8 @@ void virDomainControllerDefFree(virDomainControllerDef *def) if (!def) return; + g_slist_free_full(def->iothreads, (GDestroyNotify) virDomainIothreadMappingDefFree); + virDomainDeviceInfoClear(&def->info); g_free(def->virtio); @@ -8618,6 +8620,9 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt, &def->iothread) < 0) return NULL; + if (virDomainIothreadMappingDefParse(driver, &def->iothreads) < 0) + return NULL; + if (virDomainVirtioOptionsParseXML(driver, &def->virtio) < 0) return NULL; } @@ -23505,6 +23510,7 @@ virDomainControllerDriverFormat(virBuffer *buf, virDomainControllerDef *def) { g_auto(virBuffer) driverBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) driverChildBuf = VIR_BUFFER_INIT_CHILD(buf); if (def->queues) virBufferAsprintf(&driverBuf, " queues='%u'", def->queues); @@ -23523,9 +23529,11 @@ virDomainControllerDriverFormat(virBuffer *buf, if (def->iothread) virBufferAsprintf(&driverBuf, " iothread='%u'", def->iothread); + virDomainIothreadMappingDefFormat(&driverChildBuf, def->iothreads); + virDomainVirtioOptionsFormat(&driverBuf, def->virtio); - virXMLFormatElement(buf, "driver", &driverBuf, NULL); + virXMLFormatElement(buf, "driver", &driverBuf, &driverChildBuf); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index d56a5a22e0..20cb058473 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -763,6 +763,7 @@ struct _virDomainControllerDef { unsigned int max_sectors; virTristateSwitch ioeventfd; unsigned int iothread; /* unused = 0, > 0 specific thread # */ + GSList *iothreads; /* List of virDomainIothreadMappingDef */ union { virDomainVirtioSerialOpts vioserial; virDomainPCIControllerOpts pciopts; diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index ad3d17f0fd..f95316918d 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -1260,7 +1260,7 @@ virDomainControllerDefValidate(const virDomainControllerDef *controller) } } - if (controller->iothread != 0) { + if (controller->iothread != 0 || controller->iothreads) { if (controller->type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI || !(controller->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI || controller->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_TRANSITIONAL || @@ -1269,6 +1269,14 @@ virDomainControllerDefValidate(const virDomainControllerDef *controller) _("iothreads are supported only by 'virtio-scsi' controllers")); return -1; } + + /* configuring both <driver iothread='n'> and it's <iothreads> sub-element + * isn't supported */ + if (controller->iothread && controller->iothreads) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("controller driver 'iothread' attribute can't be used together with 'iothreads' subelement")); + return -1; + } } return 0; diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 824da9d066..7dea0f1b0c 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -2605,7 +2605,11 @@ <zeroOrMore> <element name="queue"> <attribute name="id"> - <ref name="unsignedInt"/> + <choice> + <value>ctrl</value> + <value>event</value> + <ref name="unsignedInt"/> + </choice> </attribute> </element> </zeroOrMore> @@ -3059,6 +3063,9 @@ <optional> <ref name="driverIOThread"/> </optional> + <optional> + <ref name="iothreadMapping"/> + </optional> <ref name="virtioOptions"/> </element> </optional> diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index b7499a376f..29ba358477 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -585,7 +585,8 @@ virDomainDriverDelIOThreadCheck(virDomainDef *def, } for (i = 0; i < def->ncontrollers; i++) { - if (def->controllers[i]->iothread == iothread_id) { + if (virDomainIothreadMappingDefHasIothread(def->controllers[i]->iothreads, iothread_id) || + def->controllers[i]->iothread == iothread_id) { virReportError(VIR_ERR_INVALID_ARG, _("cannot remove IOThread '%1$u' since it is being used by controller"), iothread_id); -- 2.48.1
Similarly to 'virtio-blk' users can map multiple iothreads and pin them appropriately for 'virtio-scsi' controllers to ensure the best performance. Implement the validation and command line generation based on the helpers we have for 'virtio-blk'. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 22 ++++++++++++++++++++++ src/qemu/qemu_command.h | 2 ++ src/qemu/qemu_validate.c | 34 +++++++++++++++++++++++++++------- 3 files changed, 51 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 14d62b98e4..fc0712f836 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1657,6 +1657,15 @@ qemuCommandGetVirtioIothreadMapVirtioBlk(const virDomainDiskDef *disk) } +GHashTable * +qemuCommandGetVirtioIothreadMapVirtioSCSI(const virDomainControllerDef *def) +{ + const char *internal_queues[] = { "ctrl", "event", NULL }; + + return qemuCommandGetVirtioIothreadMap(def->queues, internal_queues); +} + + virJSONValue * qemuBuildDiskDeviceProps(const virDomainDef *def, virDomainDiskDef *disk, @@ -2558,6 +2567,7 @@ qemuBuildControllerSCSIDevProps(virDomainControllerDef *def, virQEMUCaps *qemuCaps) { g_autoptr(virJSONValue) props = NULL; + g_autoptr(virJSONValue) iothreadsMapping = NULL; g_autofree char *iothread = NULL; const char *driver = NULL; @@ -2569,6 +2579,17 @@ qemuBuildControllerSCSIDevProps(virDomainControllerDef *def, qemuCaps))) return NULL; + if (def->iothreads) { + g_autoptr(GHashTable) queueMap = NULL; + + if (def->queues > 0) + queueMap = qemuCommandGetVirtioIothreadMapVirtioSCSI(def); + + if (!(iothreadsMapping = qemuBuildIothreadMappingProps(def->iothreads, + queueMap))) + return NULL; + } + if (def->iothread > 0) iothread = g_strdup_printf("iothread%u", def->iothread); @@ -2576,6 +2597,7 @@ qemuBuildControllerSCSIDevProps(virDomainControllerDef *def, "S:iothread", iothread, "s:id", def->info.alias, "p:num_queues", def->queues, + "A:iothread-vq-mapping", &iothreadsMapping, "p:cmd_per_lun", def->cmd_per_lun, "p:max_sectors", def->max_sectors, "T:ioeventfd", def->ioeventfd, diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 636c2571d4..d69fd793c4 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -280,3 +280,5 @@ struct qemuVirtioIothreadMap { GHashTable * qemuCommandGetVirtioIothreadMapVirtioBlk(const virDomainDiskDef *disk); +GHashTable * +qemuCommandGetVirtioIothreadMapVirtioSCSI(const virDomainControllerDef *def); diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index f8586a7dff..364bc59cba 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -3641,9 +3641,10 @@ qemuValidateDomainDeviceDefControllerIDE(const virDomainControllerDef *controlle */ static int qemuValidateCheckSCSIControllerIOThreads(const virDomainControllerDef *controller, - const virDomainDef *def) + const virDomainDef *def, + virQEMUCaps *qemuCaps) { - if (!controller->iothread) + if (controller->iothread == 0 && !controller->iothreads) return 0; if (controller->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && @@ -3654,8 +3655,24 @@ qemuValidateCheckSCSIControllerIOThreads(const virDomainControllerDef *controlle return -1; } - /* Can we find the controller iothread in the iothreadid list? */ - if (!virDomainIOThreadIDFind(def, controller->iothread)) { + if (controller->iothreads) { + g_autoptr(GHashTable) queueMap = NULL; + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_SCSI_IOTHREAD_MAPPING)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("IOThread mapping for virtio-scsi controllers is not available with this QEMU binary")); + return -1; + } + + if (controller->queues > 0) + queueMap = qemuCommandGetVirtioIothreadMapVirtioSCSI(controller); + + if (qemuDomainValidateIothreadMapping(def, controller->iothreads, queueMap) < 0) + return -1; + } + + if (controller->iothread > 0 && + !virDomainIOThreadIDFind(def, controller->iothread)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("controller iothread '%1$u' not defined in iothreadid"), controller->iothread); @@ -3668,13 +3685,15 @@ qemuValidateCheckSCSIControllerIOThreads(const virDomainControllerDef *controlle static int qemuValidateDomainDeviceDefControllerSCSI(const virDomainControllerDef *controller, - const virDomainDef *def) + const virDomainDef *def, + virQEMUCaps *qemuCaps) { switch ((virDomainControllerModelSCSI) controller->model) { case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI: case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_TRANSITIONAL: case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_NON_TRANSITIONAL: - if (qemuValidateCheckSCSIControllerIOThreads(controller, def) < 0) + if (qemuValidateCheckSCSIControllerIOThreads(controller, def, + qemuCaps) < 0) return -1; break; @@ -4348,7 +4367,8 @@ qemuValidateDomainDeviceDefController(const virDomainControllerDef *controller, break; case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: - ret = qemuValidateDomainDeviceDefControllerSCSI(controller, def); + ret = qemuValidateDomainDeviceDefControllerSCSI(controller, def, + qemuCaps); break; case VIR_DOMAIN_CONTROLLER_TYPE_PCI: -- 2.48.1
Test the XML and commandline for iothread<->virtqueue mapping for 'virtio-scsi' controllers. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- ...ads-virtio-scsi-mapping.x86_64-latest.args | 40 ++++++++++++ ...eads-virtio-scsi-mapping.x86_64-latest.xml | 65 +++++++++++++++++++ .../iothreads-virtio-scsi-mapping.xml | 57 ++++++++++++++++ tests/qemuxmlconftest.c | 1 + 4 files changed, 163 insertions(+) create mode 100644 tests/qemuxmlconfdata/iothreads-virtio-scsi-mapping.x86_64-latest.args create mode 100644 tests/qemuxmlconfdata/iothreads-virtio-scsi-mapping.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/iothreads-virtio-scsi-mapping.xml diff --git a/tests/qemuxmlconfdata/iothreads-virtio-scsi-mapping.x86_64-latest.args b/tests/qemuxmlconfdata/iothreads-virtio-scsi-mapping.x86_64-latest.args new file mode 100644 index 0000000000..adcc6f4e03 --- /dev/null +++ b/tests/qemuxmlconfdata/iothreads-virtio-scsi-mapping.x86_64-latest.args @@ -0,0 +1,40 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-QEMUGuest1/master-key.aes"}' \ +-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,acpi=off \ +-accel tcg \ +-cpu qemu64 \ +-m size=219136k \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ +-overcommit mem-lock=off \ +-smp 2,sockets=2,cores=1,threads=1 \ +-object '{"qom-type":"iothread","id":"iothread1"}' \ +-object '{"qom-type":"iothread","id":"iothread2"}' \ +-object '{"qom-type":"iothread","id":"iothread3"}' \ +-object '{"qom-type":"iothread","id":"iothread4"}' \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot strict=on \ +-device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ +-device '{"driver":"virtio-scsi-pci","id":"scsi0","num_queues":3,"iothread-vq-mapping":[{"iothread":"iothread2"},{"iothread":"iothread3"}],"bus":"pci.0","addr":"0xb"}' \ +-device '{"driver":"virtio-scsi-pci","id":"scsi1","num_queues":2,"iothread-vq-mapping":[{"iothread":"iothread1","vqs":[0,1]},{"iothread":"iothread4","vqs":[2]},{"iothread":"iothread2","vqs":[3]}],"bus":"pci.0","addr":"0xc"}' \ +-blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/iothrtest2.img","node-name":"libvirt-1-storage","read-only":false}' \ +-device '{"driver":"scsi-hd","bus":"scsi0.0","channel":0,"scsi-id":0,"lun":3,"device_id":"drive-scsi0-0-0-3","drive":"libvirt-1-storage","id":"scsi0-0-0-3","bootindex":1}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxmlconfdata/iothreads-virtio-scsi-mapping.x86_64-latest.xml b/tests/qemuxmlconfdata/iothreads-virtio-scsi-mapping.x86_64-latest.xml new file mode 100644 index 0000000000..bc6f9e5552 --- /dev/null +++ b/tests/qemuxmlconfdata/iothreads-virtio-scsi-mapping.x86_64-latest.xml @@ -0,0 +1,65 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>2</vcpu> + <iothreads>4</iothreads> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='raw'/> + <source file='/var/lib/libvirt/images/iothrtest2.img'/> + <target dev='sdc' bus='scsi'/> + <address type='drive' controller='0' bus='0' target='0' unit='3'/> + </disk> + <controller type='usb' index='0' model='piix3-uhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <controller type='scsi' index='0' model='virtio-scsi'> + <driver queues='3'> + <iothreads> + <iothread id='2'/> + <iothread id='3'/> + </iothreads> + </driver> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/> + </controller> + <controller type='scsi' index='1' model='virtio-scsi'> + <driver queues='2'> + <iothreads> + <iothread id='1'> + <queue id='ctrl'/> + <queue id='event'/> + </iothread> + <iothread id='4'> + <queue id='0'/> + </iothread> + <iothread id='2'> + <queue id='1'/> + </iothread> + </iothreads> + </driver> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0c' function='0x0'/> + </controller> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxmlconfdata/iothreads-virtio-scsi-mapping.xml b/tests/qemuxmlconfdata/iothreads-virtio-scsi-mapping.xml new file mode 100644 index 0000000000..e8fda484ee --- /dev/null +++ b/tests/qemuxmlconfdata/iothreads-virtio-scsi-mapping.xml @@ -0,0 +1,57 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>2</vcpu> + <iothreads>4</iothreads> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='raw'/> + <source file='/var/lib/libvirt/images/iothrtest2.img'/> + <target dev='sdc' bus='scsi'/> + <address type='drive' controller='0' bus='0' target='0' unit='3'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <controller type='scsi' index='0' model='virtio-scsi'> + <driver queues='3'> + <iothreads> + <iothread id='2'/> + <iothread id='3'/> + </iothreads> + </driver> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/> + </controller> + <controller type='scsi' index='1' model='virtio-scsi'> + <driver queues='2'> + <iothreads> + <iothread id='1'> + <queue id='ctrl'/> + <queue id='event'/> + </iothread> + <iothread id='4'> + <queue id='0'/> + </iothread> + <iothread id='2'> + <queue id='1'/> + </iothread> + </iothreads> + </driver> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0c' function='0x0'/> + </controller> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 8632434760..fd751c2c3a 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -2191,6 +2191,7 @@ mymain(void) DO_TEST_CAPS_LATEST("iothreads-disk"); DO_TEST_CAPS_VER("iothreads-virtio-scsi-pci", "5.2.0"); DO_TEST_CAPS_LATEST("iothreads-virtio-scsi-pci"); + DO_TEST_CAPS_LATEST("iothreads-virtio-scsi-mapping"); DO_TEST_CAPS_ARCH_LATEST("iothreads-virtio-scsi-ccw", "s390x"); DO_TEST_CAPS_LATEST("cpu-topology1"); -- 2.48.1
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- NEWS.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 9c940b1a81..37233121b6 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -17,6 +17,12 @@ v11.2.0 (unreleased) * **New features** + * qemu: Add support for multiple iothreads for ``virtio-scsi`` controller + + It's now possible to map multiple iotreads to the ``virtio-scsi`` controller + or even map them to specific virtqueues similarly to the ``virtio-blk`` + device allowing for better performance in certain scenarios. + * **Improvements** * **Bug fixes** -- 2.48.1
On Mon, Mar 03, 2025 at 16:30:38 +0100, Peter Krempa wrote:
Unfortunately still RFC as the qemu support is still not merged.
Once again there are a few patches that are applicable:
qemu: Use modern header formatting in 'qemu_command.h' virXMLNodeGetSubelementList: Document return value semantics virDomainIothreadMappingDefParse: Fix usage of virXMLNodeGetSubelementList docs: formatdomain: Clarify configuration of iothread <-> virtqueue mapping
The above are non-RFC, since I'll very likely be soon posting the proper version of this series if these'll be reviewed I can push them beforehand, without re-spaming them :) ...
The patches below are RFC:
qemucapabilitiestest: Update 'caps_10.0.0_x86_64' to XXXXXX
... the qemu support will be hopefully merged soon ;)
qemu: capabilities: Introduce QEMU_CAPS_VIRTIO_SCSI_IOTHREAD_MAPPING
participants (2)
-
Ján Tomko -
Peter Krempa