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

From: "Peter Krempa" <pkrempa(a)redhat.com&gt; To: "Francesco Romani" <fromani(a)redhat.com&gt;, libvir-list(a)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(a)redhat.com&gt; > --- > 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.

> +{ > + 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?

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