[PATCH v2 07/11] virDomainIothreadMapping: Add support for naming certain virt queues

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(a)redhat.com&gt; --- 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