Re: [libvirt] [PATCHv4 6/8] qemu: bulk stats: implement block group

From: "Peter Krempa" <pkrempa(a)redhat.com&gt; To: "Francesco Romani" <fromani(a)redhat.com&gt;, libvir-list(a)redhat.com Sent: Friday, September 12, 2014 3:56:06 PM Subject: Re: [libvirt] [PATCHv4 6/8] qemu: bulk stats: implement block group On 09/12/14 13:48, Francesco Romani wrote: > This patch implements the VIR_DOMAIN_STATS_BLOCK > group of statistics. > > To do so, an helper function to get the block stats > of all the disks of a domain is added. > > Signed-off-by: Francesco Romani <fromani(a)redhat.com&gt; > --- > include/libvirt/libvirt.h.in | 1 + > src/libvirt.c | 20 +++++++ > src/qemu/qemu_driver.c | 96 ++++++++++++++++++++++++++++++ > src/qemu/qemu_monitor.c | 26 +++++++++ > src/qemu/qemu_monitor.h | 20 +++++++ > src/qemu/qemu_monitor_json.c | 136 > +++++++++++++++++++++++++++++-------------- > src/qemu/qemu_monitor_json.h | 4 ++ > 7 files changed, 260 insertions(+), 43 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 7e5d707..4644f4a 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -9687,6 +9687,31 @@ qemuDomainBlockStats(virDomainPtr dom, > return ret; > } > > + > +/* > + * Returns at most the first `nstats' stats, then stops. > + * Returns the number of stats filled. > + */ > +static int > +qemuDomainHelperGetBlockStats(virQEMUDriverPtr driver, > + virDomainObjPtr vm, > + qemuBlockStatsPtr stats, > + int nstats) > +{ > + int ret; > + qemuDomainObjPrivatePtr priv = vm->privateData; > + > + qemuDomainObjEnterMonitor(driver, vm); > + > + ret = qemuMonitorGetAllBlockStatsInfo(priv->mon, NULL, > + stats, nstats); Humm, is it worth doing this helper? This pretty much can be inlined as it has only one caller.

> +static int > +qemuDomainGetStatsBlock(virQEMUDriverPtr driver, > + virDomainObjPtr dom, > + virDomainStatsRecordPtr record, > + int *maxparams, > + unsigned int privflags) > +{ > + size_t i; > + int ret = -1; > + int nstats = 0; > + qemuBlockStatsPtr stats = NULL; > + > + if (!HAVE_MONITOR(privflags) || !virDomainObjIsActive(dom)) > + return 0; /* it's ok, just go ahead silently */ > + > + if (VIR_ALLOC_N(stats, dom->def->ndisks) < 0) > + return -1; > + > + nstats = qemuDomainHelperGetBlockStats(driver, dom, stats, > + dom->def->ndisks); > + if (nstats < 0) Are we erroring out on block stats failure? Other statistics gatherers just skip it if it's not available.