[libvirt] [PATCH 1/8] qemu: bulk stats: extend internal collection API
Peter Krempa
pkrempa at redhat.com
Wed Sep 10 14:11:30 UTC 2014
On 09/10/14 15:55, Francesco Romani wrote:
> ----- Original Message -----
>> From: "Peter Krempa" <pkrempa at redhat.com>
>> To: "Francesco Romani" <fromani at redhat.com>, libvir-list at redhat.com
>> Sent: Wednesday, September 10, 2014 10:07:33 AM
>> Subject: Re: [libvirt] [PATCH 1/8] qemu: bulk stats: extend internal collection API
>
> [...]
>>>> Hmm this skips offline domains entirely if one of the stats groups needs
>>>> the monitor.
>>>>
>>>> I think we should rather skip individual stats groups, or better stats
>>>> fields that we can't provide.
>>>>
>>>> Any ideas?
>>>
>>> What about this (pseudo-C):
>>>
>>> unsigned int privflags = 0;
>>>
>>> if (needmon && qemuDomainObjBeginJob(driver, dom, QEMU_JOB_QUERY)
>>> needmon = false;
>>> /* auto disable monitoring, the remainder of the function should be
>>> unchanged */
>>> else
>>> privflags |= MONITOR_AVAILABLE;
>>>
>>> if ((needmon && virDomainObjIsActive(dom)) || !needmon) {
Drop this condition ...
>>> if (qemuDomainGetStats(conn, dom, stats, &tmp, privflags) < 0)
The individual stats groups will decide if they need the VM alive they
can skip it individually rather than omitting the vm completely.
>>> /* pass monitor availability down the chain. Individual workers
>>> will
>>> bail out immediately and silently if they need monitor but
>>> it is
>>> not available
>>> */
>>> goto endjob;
>>>
>>> if (tmp)
>>> tmpstats[nstats++] = tmp;
>>> }
>>>
>>>
>>> No other change should be needed to this patch, and with trivial changes
>>> all the others can be fixed.
>>
>> Also you can just grab the lock always and the workers will exit if the
>> VM is not alive. Having a domain job should be fine.
>
> As you prefer. For oVirt, we do care of all the stats group implemented, and always all of them,
> (actually I may have missed some bits, e.g. the pininfo as you pointed out elsewhere - going to fix),
> so for our needs we'll always need to enter the monitor.
>
> But other users of this API may beg to differ, and I believe is fair to assume that other consumers
> of this API could just use stats which doesn't require to enter the monitor: e.g. CPU/VCPU/interface.
In case you don't have any stat group that requires the monitor it is
imho fine not to get a job. If there is a group where we can enter it we
can just grab the monitor always ... see above.
>
> Hence, I was trying to be a good citizen and do not require monitor access unless it is actually
> needed; but if turns out it is OK to do a domain job anyway, I'll happily simplify my code :)
>
> Thanks and bests,
>
Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140910/1f85cbaa/attachment-0001.sig>
More information about the libvir-list
mailing list