----- Original Message -----
From: "Peter Krempa" <pkrempa(a)redhat.com>
To: "Francesco Romani" <fromani(a)redhat.com>, 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>
> ---
> 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