Add validation and formatting of the commandline. Note that this is based on Stefan's RFC series which only adds the qemu interface: https://lists.gnu.org/archive/html/qemu-devel/2023-01/msg04090.html --- src/qemu/qemu_command.c | 45 +++++++ src/qemu/qemu_validate.c | 117 ++++++++++++++++-- .../iothreads-disk.x86_64-latest.args | 4 +- 3 files changed, 153 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index cc476addce..d9bb984101 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1746,6 +1746,45 @@ qemuBuildDriveStr(virDomainDiskDef *disk) } +static virJSONValue * +qemuBuildDiskDeviceIothreadMappingProps(GSList *iothreads) +{ + g_autoptr(virJSONValue) ret = virJSONValueNewArray(); + GSList *n; + + for (n = iothreads; n; n = n->next) { + virDomainDiskIothreadDef *ioth = n->data; + 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) { + queues = virJSONValueNewArray(); + + for (i = 0; i < ioth->nqueues; i++) { + g_autoptr(virJSONValue) vq = virJSONValueNewNumberUint(ioth->queues[i]); + + if (virJSONValueArrayAppend(queues, &vq)) + return NULL; + } + } + + if (virJSONValueObjectAdd(&props, + "s:iothread", alias, + "A:vqs", &queues, + NULL) < 0) + return NULL; + + + if (virJSONValueArrayAppend(ret, &props)) + return NULL; + } + + return g_steal_pointer(&ret); +} + + virJSONValue * qemuBuildDiskDeviceProps(const virDomainDef *def, virDomainDiskDef *disk, @@ -1804,11 +1843,16 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, case VIR_DOMAIN_DISK_BUS_VIRTIO: { virTristateSwitch scsi = VIR_TRISTATE_SWITCH_ABSENT; + g_autoptr(virJSONValue) iothreadMapping = NULL; g_autofree char *iothread = NULL; if (disk->iothread > 0) iothread = g_strdup_printf("iothread%u", disk->iothread); + if (disk->iothreads && + !(iothreadMapping = qemuBuildDiskDeviceIothreadMappingProps(disk->iothreads))) + return NULL; + if (virStorageSourceGetActualType(disk->src) != VIR_STORAGE_TYPE_VHOST_USER && virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_BLK_SCSI)) { /* if sg_io is true but the scsi option isn't supported, @@ -1832,6 +1876,7 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, "T:scsi", scsi, "p:num-queues", disk->queues, "p:queue-size", disk->queue_size, + "A:iothread-vq-mapping", &iothreadMapping, NULL) < 0) return NULL; } diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index c877aa73d4..79096a2fcf 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2822,12 +2822,24 @@ qemuValidateDomainDeviceDefDiskSerial(const char *value) } -static bool +static int qemuValidateDomainDeviceDefDiskIOThreads(const virDomainDef *def, - const virDomainDiskDef *disk) + const virDomainDiskDef *disk, + virQEMUCaps *qemuCaps) { + if (disk->iothread == 0 && !disk->iothreads) + return 0; + switch ((virDomainDiskBus)disk->bus) { case VIR_DOMAIN_DISK_BUS_VIRTIO: + if (disk->iothreads) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_BLK_IOTHREAD_MAPPING)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("IOThread mapping for disk '%s' is not available with this QEMU binary"), + disk->dst); + return -1; + } + } break; case VIR_DOMAIN_DISK_BUS_IDE: @@ -2843,18 +2855,101 @@ qemuValidateDomainDeviceDefDiskIOThreads(const virDomainDef *def, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("IOThreads not available for bus %s target %s"), virDomainDiskBusTypeToString(disk->bus), disk->dst); - return false; + return -1; } - /* Can we find the disk iothread in the iothreadid list? */ - if (!virDomainIOThreadIDFind(def, disk->iothread)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Disk iothread '%u' not defined in iothreadid"), - disk->iothread); - return false; + if (disk->iothreads) { + virDomainDiskIothreadDef *first_ioth = disk->iothreads->data; + g_autoptr(virBitmap) queueMap = NULL; + g_autoptr(GHashTable) iothreads = virHashNew(NULL); + ssize_t unused; + GSList *n; + + if (first_ioth->queues) { + if (disk->queues == 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("disk 'queue' count must be configured for explicit iothread to queue mapping")); + return -1; + } + + queueMap = virBitmapNew(disk->queues); + } + + /* we are validating that: + * - there are no duplicate iothreads + * - there are only valid iothreads + * - if queue mapping is provided + * - queue is in range + * - it must be provided for all assigned iothreads + * - it must be provided for all queues + * - queue must be assigned only once + */ + for (n = disk->iothreads; n; n = n->next) { + virDomainDiskIothreadDef *ioth = n->data; + g_autofree char *alias = g_strdup_printf("iothread%u", ioth->id); + size_t i; + + if (g_hash_table_contains(iothreads, alias)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Duplicate mapping for iothread '%u'"), ioth->id); + return -1; + } + + g_hash_table_insert(iothreads, g_steal_pointer(&alias), NULL); + + if (!virDomainIOThreadIDFind(def, ioth->id)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Disk iothread '%u' not defined in iothreadid"), + ioth->id); + return -1; + } + + if (!!queueMap != !!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 (virBitmapGetBit(queueMap, ioth->queues[i], &hasMapping) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk iothread queue '%u' mapping out of range"), + ioth->queues[i]); + return -1; + } + + if (hasMapping) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk iothread queue '%u' is already assigned"), + ioth->queues[i]); + return -1; + } + + ignore_value(virBitmapSetBit(queueMap, ioth->queues[i])); + + } + } + + if (queueMap) { + if ((unused = virBitmapNextClearBit(queueMap, -1)) >= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("missing iothread mapping for queue '%zd'"), + unused); + return -1; + } + } + } else { + if (!virDomainIOThreadIDFind(def, disk->iothread)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Disk iothread '%u' not defined in iothreadid"), + disk->iothread); + return -1; + } } - return true; + return 0; } @@ -3147,7 +3242,7 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk, qemuValidateDomainDeviceDefDiskSerial(disk->serial) < 0) return -1; - if (disk->iothread && !qemuValidateDomainDeviceDefDiskIOThreads(def, disk)) + if (qemuValidateDomainDeviceDefDiskIOThreads(def, disk, qemuCaps)) return -1; return 0; diff --git a/tests/qemuxml2argvdata/iothreads-disk.x86_64-latest.args b/tests/qemuxml2argvdata/iothreads-disk.x86_64-latest.args index d1953327a7..05729acde5 100644 --- a/tests/qemuxml2argvdata/iothreads-disk.x86_64-latest.args +++ b/tests/qemuxml2argvdata/iothreads-disk.x86_64-latest.args @@ -37,10 +37,10 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ -device '{"driver":"virtio-blk-pci","iothread":"iothread1","bus":"pci.0","addr":"0x4","drive":"libvirt-3-format","id":"virtio-disk1","bootindex":1}' \ -blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/iothrtest2.img","node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}' \ -blockdev '{"node-name":"libvirt-2-format","read-only":false,"driver":"raw","file":"libvirt-2-storage"}' \ --device '{"driver":"virtio-blk-pci","num-queues":4,"bus":"pci.0","addr":"0x2","drive":"libvirt-2-format","id":"virtio-disk2"}' \ +-device '{"driver":"virtio-blk-pci","num-queues":4,"iothread-vq-mapping":[{"iothread":"iothread2","vqs":[1,3]},{"iothread":"iothread3","vqs":[0,2]}],"bus":"pci.0","addr":"0x2","drive":"libvirt-2-format","id":"virtio-disk2"}' \ -blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/iothrtest3.img","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ -blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}' \ --device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x3","drive":"libvirt-1-format","id":"virtio-disk3"}' \ +-device '{"driver":"virtio-blk-pci","iothread-vq-mapping":[{"iothread":"iothread4"},{"iothread":"iothread1"}],"bus":"pci.0","addr":"0x3","drive":"libvirt-1-format","id":"virtio-disk3"}' \ -audiodev '{"id":"audio1","driver":"none"}' \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ -msg timestamp=on -- 2.39.2