[libvirt] [PATCH 1/8] qemu: bulk stats: extend internal collection API

[Francesco Romani]

----- 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) {
> >         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