----- Original Message ----- > From: "Peter Krempa" <pkrempa@xxxxxxxxxx> > To: "Francesco Romani" <fromani@xxxxxxxxxx>, libvir-list@xxxxxxxxxx > Sent: Wednesday, September 10, 2014 10:07:33 AM > Subject: Re: [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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list