On 09/12/14 13:48, Francesco Romani wrote:
> This patch implements the VIR_DOMAIN_STATS_BALLOON
> group of statistics.
>
> Signed-off-by: Francesco Romani <fromani@xxxxxxxxxx>
> ---
> include/libvirt/libvirt.h.in | 1 +
> src/libvirt.c | 6 ++++
> src/qemu/qemu_driver.c | 73 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 80 insertions(+)
>
Just one small nit:
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 8665c6c..c005442 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 b22f9aa..3fa86ab 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -21569,6 +21569,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 9014976..f677884 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -2525,6 +2525,48 @@ 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 int
> +qemuDomainGetBalloonMemory(virQEMUDriverPtr driver, virDomainObjPtr vm,
> + unsigned long long *memory)
> +{
> + 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)) {
If qemu supports the BALLOON_EVENT, you get the right data even if you
can't acquire the job ...
> + *memory = vm->def->mem.cur_balloon;
> + } else {
> + int rv;
> + unsigned long long balloon;
> +
> + qemuDomainObjEnterMonitor(driver, vm);
> + rv = qemuMonitorGetBalloonInfo(priv->mon, &balloon);
> + qemuDomainObjExitMonitor(driver, vm);
> +
> + if (rv < 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 (rv > 0) {
> + *memory = balloon;
> + } else {
> + /* Balloon not supported, so maxmem is always the allocation */
> + return -1;
> + }
> + }
> + return 0;
> +}
> +
> +
> static int qemuDomainGetInfo(virDomainPtr dom,
> virDomainInfoPtr info)
> {
> @@ -17315,6 +17357,36 @@ qemuDomainGetStatsCpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
> return 0;
> }
>
> +static int
> +qemuDomainGetStatsBalloon(virQEMUDriverPtr driver,
> + virDomainObjPtr dom,
> + virDomainStatsRecordPtr record,
> + int *maxparams,
> + unsigned int privflags)
> +{
> + if (HAVE_MONITOR(privflags) && virDomainObjIsActive(dom)) {
> + unsigned long long cur_balloon = 0;
> + int err = 0;
> +
> + err = qemuDomainGetBalloonMemory(driver, dom, &cur_balloon);
... but you'd skip it now. As the helper is separate anyways, you may
inline the code here and have better control over what happens according
to the presence of the monitor.
> +
> + if (!err && virTypedParamsAddULLong(&record->params,
> + &record->nparams,
> + maxparams,
> + "balloon.current",
> + cur_balloon) < 0)
> + return -1;
> + }
> +
> + if (virTypedParamsAddULLong(&record->params,
> + &record->nparams,
> + maxparams,
> + "balloon.maximum",
> + dom->def->mem.max_balloon) < 0)
> + return -1;
> +
> + return 0;
> +}
>
> typedef int
> (*qemuDomainGetStatsFunc)(virQEMUDriverPtr driver,
> @@ -17332,6 +17404,7 @@ struct qemuDomainGetStatsWorker {
> static struct qemuDomainGetStatsWorker qemuDomainGetStatsWorkers[] = {
> { qemuDomainGetStatsState, VIR_DOMAIN_STATS_STATE, false },
> { qemuDomainGetStatsCpu, VIR_DOMAIN_STATS_CPU_TOTAL, false },
> + { qemuDomainGetStatsBalloon, VIR_DOMAIN_STATS_BALLOON, true },
> { NULL, 0, false }
> };
>
>
ACK, current version looks fine, but if you wan't to do it a bit more
flexible you can follow up with another version.
Peter
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list