----- Original Message -----
From: "Peter Krempa" <pkrempa@redhat.com> To: "Francesco Romani" <fromani@redhat.com>, libvir-list@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) { if (qemuDomainGetStats(conn, dom, stats, &tmp, privflags) < 0) /* 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. 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, -- Francesco Romani RedHat Engineering Virtualization R & D Phone: 8261328 IRC: fromani