[libvirt] [PATCHv3 3/8] qemu: bulk stats: implement balloon group

[Francesco Romani]

----- Original Message -----
> From: "Peter Krempa" <pkrempa at redhat.com>
> To: "Francesco Romani" <fromani at redhat.com>, libvir-list at redhat.com
> Sent: Tuesday, September 9, 2014 1:45:57 PM
> Subject: Re: [libvirt] [PATCHv3 3/8] qemu: bulk stats: implement balloon	group
> 
> On 09/08/14 15:05, Francesco Romani wrote:
> > This patch implements the VIR_DOMAIN_STATS_BALLOON
> > group of statistics.
> > 
> > Signed-off-by: Francesco Romani <fromani at redhat.com>
> > ---
> >  include/libvirt/libvirt.h.in |  1 +
> >  src/libvirt.c                |  6 ++++
> >  src/qemu/qemu_driver.c       | 70
> >  ++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 77 insertions(+)
> > 
> > diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> > index e6ed803..1e4e428 100644
> > --- a/include/libvirt/libvirt.h.in
> > +++ b/include/libvirt/libvirt.h.in
> > @@ -2512,6 +2512,7 @@ struct _virDomainStatsRecord {
> >  typedef enum {
> >      VIR_DOMAIN_STATS_STATE = (1 << 0), /* return domain state */
> >      VIR_DOMAIN_STATS_CPU_TOTAL = (1 << 1), /* return domain CPU info */
> > +    VIR_DOMAIN_STATS_BALLOON = (1 << 2), /* return domain balloon info */
> >  } virDomainStatsTypes;
> >  
> >  typedef enum {
> > diff --git a/src/libvirt.c b/src/libvirt.c
> > index 4d504ff..f21eb39 100644
> > --- a/src/libvirt.c
> > +++ b/src/libvirt.c
> > @@ -21562,6 +21562,12 @@ virConnectGetDomainCapabilities(virConnectPtr
> > conn,
> >   * "cpu.user" - user cpu time spent as unsigned long long.
> >   * "cpu.system" - system cpu time spent as unsigned long long.
> >   *
> > + * VIR_DOMAIN_STATS_BALLOON: Return memory balloon device information.
> > + * The typed parameter keys are in this format:
> > + * "balloon.current" - the memory in kiB currently used
> > + *                     as unsigned long long.
> > + * "balloon.maximum" - the maximum memory in kiB allowed
> > + *                     as unsigned long long.
> >   *
> >   * Using 0 for @stats returns all stats groups supported by the given
> >   * hypervisor.
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index cfc5941..4f8ccac 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -2520,6 +2520,47 @@ static int qemuDomainSendKey(virDomainPtr domain,
> >      return ret;
> >  }
> >  
> > +
> > +/*
> > + * FIXME: this code is a stripped down version of what is done into
> > + * qemuDomainGetInfo. Due to the different handling of jobs, it is not
> > + * trivial to extract a common helper function.
> > + */
> > +static void
> > +qemuDomainGetBalloonMemory(virQEMUDriverPtr driver, virDomainObjPtr vm,
> > +                           unsigned long *memory)
> 
> Use unsigned long long here. Unsigned long is 32 bit on 32bit systems.

Will fix
 
> > +{
> > +    qemuDomainObjPrivatePtr priv = vm->privateData;
> > +
> > +    if (vm->def->memballoon &&
> > +        vm->def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_NONE) {
> > +        *memory = vm->def->mem.max_balloon;
> > +    } else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BALLOON_EVENT)) {
> > +        *memory = vm->def->mem.cur_balloon;
> > +    } else {
> > +        int err;
> > +        unsigned long long balloon;
> 
> Note this ...
> 
> > +
> > +        qemuDomainObjEnterMonitor(driver, vm);
> > +        err = qemuMonitorGetBalloonInfo(priv->mon, &balloon);
> > +        qemuDomainObjExitMonitor(driver, vm);
> > +
> > +        if (err < 0) {
> > +            /* We couldn't get current memory allocation but that's not
> > +             * a show stopper; we wouldn't get it if there was a job
> > +             * active either
> > +             */
> > +            *memory = vm->def->mem.cur_balloon;
> > +        } else if (err == 0) {
> > +            /* Balloon not supported, so maxmem is always the allocation
> > */
> 
> Should we in such case drop the balloon stat from the output?

That would be ok for me. Just let me know :)

> 
> > +            *memory = vm->def->mem.max_balloon;
> > +        } else {
> > +            *memory = balloon;
> 
> Here it'd break.

Good catch, will fix.

Bests,

-- 
Francesco Romani
RedHat Engineering Virtualization R & D
Phone: 8261328
IRC: fromani