[libvirt] [PATCH 3/3] qemu: Report which iothread IDs are actually returned with VIR_DOMAIN_STATS_IOTHREAD

[Peter Krempa]

On Fri, Nov 29, 2019 at 10:46:47 +0100, Ján Tomko wrote:
> On Fri, Nov 29, 2019 at 09:59:28AM +0100, Peter Krempa wrote:
> > 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.
> 
> Knowing it upfront allows the users to construct the parameter name for
> the virTypedParamsGet APIs, but processing the returned parameters in
> this way seems inefficient - I'd expect any serious user to iterate over
> all the params and collect the iothread <id>s which are reported.

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.

> 
> > 
> > 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.
> > 
> 
> Why is the order important? Can it be returned as a stringified bitmap?

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.

> 
> > 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 at redhat.com>
> > ---
> > 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
> 
> s/iotdread/iothread/
> 
> Jano
> 
> > + *                      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
> > 
> > --
> > libvir-list mailing list
> > libvir-list at redhat.com
> > https://www.redhat.com/mailman/listinfo/libvir-list


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20191129/4b74e75a/attachment-0001.sig>