The original implementation used QEMU_ADD_COUNT_PARAM which added the 'count' suffix, but 'cnt' was documented. Fix the documentation to conform with the original implementation. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/libvirt-domain.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 51fb79cddd..87110036ca 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -11615,11 +11615,11 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * * The typed parameter keys are in this format: * - * "iothread.cnt" - maximum number of IOThreads in the subsequent list - * as unsigned int. Each IOThread in the list will - * will use it's iothread_id value as the <id>. There - * may be fewer <id> entries than the iothread.cnt - * value if the polling values are not supported. + * "iothread.count" - maximum number of IOThreads in the subsequent list + * as unsigned int. Each IOThread in the list will + * will use it's iothread_id value as the <id>. There + * may be fewer <id> entries than the iothread.count + * value if the polling values are not supported. * "iothread.<id>.poll-max-ns" - maximum polling time in ns as an unsigned * long long. A 0 (zero) means polling is * disabled. -- 2.23.0

In commit 2ccb5335dc4 I've refactored how we fill the typed parameters for domain statistics. The commit introduced a regression in the formating of stats for IOthreads by using the array index to label the entries as it's common for all other types of statistics rather than the iothread IDs used for iothreads. Since only the design of iothread deviates from the common approach used in all other statistic types this was not caught. https://bugzilla.redhat.com/show_bug.cgi?id=1778014 Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 18bd0101e7..b5300241a8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -21208,13 +21208,16 @@ qemuDomainGetStatsIOThread(virQEMUDriverPtr driver, for (i = 0; i < niothreads; i++) { if (iothreads[i]->poll_valid) { if (virTypedParamListAddULLong(params, iothreads[i]->poll_max_ns, - "iothread.%zu.poll-max-ns", i) < 0) + "iothread.%u.poll-max-ns", + iothreads[i]->iothread_id) < 0) goto cleanup; if (virTypedParamListAddUInt(params, iothreads[i]->poll_grow, - "iothread.%zu.poll-grow", i) < 0) + "iothread.%u.poll-grow", + iothreads[i]->iothread_id) < 0) goto cleanup; if (virTypedParamListAddUInt(params, iothreads[i]->poll_shrink, - "iothread.%zu.poll-shrink", i) < 0) + "iothread.%u.poll-shrink", + iothreads[i]->iothread_id) < 0) goto cleanup; } } -- 2.23.0

The design of the stats fields returned for VIR_DOMAIN_STATS_IOTHREAD domain statistics groups deviates from the established pattern. In this instance it's impossible to infer which values of <id> for iothread.<id>... fields will be reported back because they have no connection to the iothread.count field. Introduce iothread.ids which will report a comma-separated list of <id>s reported in the subsequent array in the order they will be reported. virsh domstats upstream --iothread Domain: 'upstream' iothread.count=2 iothread.ids=7,5 iothread.7.poll-max-ns=32768 iothread.7.poll-grow=0 iothread.7.poll-shrink=0 iothread.5.poll-max-ns=32768 iothread.5.poll-grow=0 iothread.5.poll-shrink=0 Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/libvirt-domain.c | 2 ++ src/qemu/qemu_driver.c | 11 +++++++++++ 2 files changed, 13 insertions(+) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 87110036ca..e6d5697445 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -11620,6 +11620,8 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * will use it's iothread_id value as the <id>. There * may be fewer <id> entries than the iothread.count * value if the polling values are not supported. + * "iothread.ids" - a comma separated list of iotdread <id>s reported in the + * subsequent list reported as a string * "iothread.<id>.poll-max-ns" - maximum polling time in ns as an unsigned * long long. A 0 (zero) means polling is * disabled. diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b5300241a8..4ccc9d3d4e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -21189,6 +21189,8 @@ qemuDomainGetStatsIOThread(virQEMUDriverPtr driver, qemuMonitorIOThreadInfoPtr *iothreads = NULL; int niothreads; int ret = -1; + g_auto(virBuffer) iothridbuf = VIR_BUFFER_INITIALIZER; + g_autofree char *iothridstr = NULL; if (!HAVE_JOB(privflags) || !virDomainObjIsActive(dom)) return 0; @@ -21205,6 +21207,15 @@ qemuDomainGetStatsIOThread(virQEMUDriverPtr driver, if (virTypedParamListAddUInt(params, niothreads, "iothread.count") < 0) goto cleanup; + for (i = 0; i < niothreads; i++) + virBufferAsprintf(&iothridbuf, "%u,", iothreads[i]->iothread_id); + + virBufferTrim(&iothridbuf, ",", -1); + iothridstr = virBufferContentAndReset(&iothridbuf); + + if (virTypedParamListAddString(params, iothridstr, "iothread.ids") < 0) + goto cleanup; + for (i = 0; i < niothreads; i++) { if (iothreads[i]->poll_valid) { if (virTypedParamListAddULLong(params, iothreads[i]->poll_max_ns, -- 2.23.0

On Fri, Nov 29, 2019 at 10:46:47 +0100, Ján Tomko wrote: My grief is that there's no connection between the count field and the available ids later on. This is specifically irritating if the stats are not actually reported as then you have no way of figuring out which iothread ids were actually examined. Said this I don't think this patch is as important though so I can drop it. The order is probably not important at all. This is a easy way to format the string but mainly the stringified bitmaps are very unpleasant to parse if you don't have our internal functions so I tried to avoid that.