----- Original Message -----
> From: "Peter Krempa" <pkrempa@xxxxxxxxxx>
> To: "Francesco Romani" <fromani@xxxxxxxxxx>, libvir-list@xxxxxxxxxx
> Sent: Friday, September 12, 2014 3:56:06 PM
> Subject: Re: [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@xxxxxxxxxx>
> > ---
> > 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.
Right, qemuDomainHelperGetBlockStats add little to none value, so I'll drop it.
I believe qemuMonitorGetAllBlockStatsInfo should stay, however:
I don't see JSON monitor being called directly anywhere. So I'll keep it.
[...]
> > +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.
Right, will fix to make it silently skip as the other groups.
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