On Wed, Jul 13, 2016 at 01:42:16PM +0300, Derbyshev Dmitriy wrote:
> From: Derbyshev Dmitry <dderbyshev@xxxxxxxxxxxxx>
>
> To collect all balloon statistics for all guests it was necessary to make
> several libvirt requests. Now it's possible to get all balloon statiscs via
> single connectGetAllDomainStats call.
>
> Signed-off-by: Derbyshev Dmitry <dderbyshev@xxxxxxxxxxxxx>
> ---
> src/qemu/qemu_driver.c | 39 +++++++++++++++++++++++++++++++++++++--
> tools/virsh.pod | 12 +++++++++++-
> 2 files changed, 48 insertions(+), 3 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 6fa8d01..70f3faa 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -18511,15 +18511,29 @@ qemuDomainGetStatsCpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
> return 0;
> }
>
> +
> +#define STORE_MEM_RECORD(TAG, NAME) { \
> + if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_ ##TAG) \
> + if (virTypedParamsAddULLong(&record->params, \
> + &record->nparams, \
> + maxparams, \
> + "balloon." NAME, \
> + stats[i].val) < 0) \
> + return -1; \
> +}
This isn't a correct way how to write a MACRO. For starters, the syntax isn't
the same as for functions. If you need to ensure that it is a separate block
the correct way is to do something like this:
#define MACRO() \
do { \
#body of the macro \
} while (0)
But in this case it's not required to use the do-while construct.
Next point to this MACRO is that it uses other variables not passed as
parameters. It's OK to do that but only inside a function and right before its
usage and also you should #undef it right away. For example:
#define MACRO()
MACRO()
MACRO()
MACRO()
#undef MACRO
> +
> static int
> -qemuDomainGetStatsBalloon(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
> +qemuDomainGetStatsBalloon(virQEMUDriverPtr driver,
> virDomainObjPtr dom,
> virDomainStatsRecordPtr record,
> int *maxparams,
> - unsigned int privflags ATTRIBUTE_UNUSED)
> + unsigned int privflags)
> {
> qemuDomainObjPrivatePtr priv = dom->privateData;
> + virDomainMemoryStatStruct stats[VIR_DOMAIN_MEMORY_STAT_NR];
> + int nr_stats;
> unsigned long long cur_balloon = 0;
> + size_t i;
> int err = 0;
>
> if (!virDomainDefHasMemballoon(dom->def)) {
> @@ -18544,8 +18558,29 @@ qemuDomainGetStatsBalloon(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
> virDomainDefGetMemoryTotal(dom->def)) < 0)
> return -1;
>
> + if (err || !HAVE_JOB(privflags))
> + return 0;
There is no need to check the *err* variable, it's used only to indicate whether
the "balloon.current" should be reported or not. I would change the condition
to this:
if (!HAVE_JOB(privflags) || !virDomainObjIsActive(dom))
return 0;
> +
> + nr_stats = qemuDomainMemoryStatsInternal(driver, dom, stats,
> + VIR_DOMAIN_MEMORY_STAT_NR);
> + if (nr_stats < 0)
> + return 0;
> +
> + for (i = 0; i < nr_stats; i++) {
> + STORE_MEM_RECORD(SWAP_IN, "swap_in")
> + STORE_MEM_RECORD(SWAP_OUT, "swap_out")
> + STORE_MEM_RECORD(MAJOR_FAULT, "major_fault")
> + STORE_MEM_RECORD(MINOR_FAULT, "minor_fault")
> + STORE_MEM_RECORD(UNUSED, "unused")
> + STORE_MEM_RECORD(AVAILABLE, "available")
> + STORE_MEM_RECORD(RSS, "rss")
> + STORE_MEM_RECORD(LAST_UPDATE, "last-update")
> + STORE_MEM_RECORD(USABLE, "usable")
> + }
> +
> return 0;
> }
> +#undef STORE_MEM_RECORD
>
>
> static int
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index 6af94d1..cda1874 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -899,7 +899,17 @@ I<--cpu-total> returns:
>
> I<--balloon> returns:
> "balloon.current" - the memory in kiB currently used,
> -"balloon.maximum" - the maximum memory in kiB allowed
> +"balloon.maximum" - the maximum memory in kiB allowed,
> +"balloon.swap_in" - the amount of data read from swap space (in kB),
> +"balloon.swap_out" - the amount of memory written out to swap space (in kB),
> +"balloon.major_fault" - the number of page faults then disk IO was required,
> +"balloon.minor_fault" - the number of other page faults,
> +"balloon.unused" - the amount of memory left unused by the system (in kB),
> +"balloon.available" - the amount of usable memory as seen by the domain (in kB),
> +"balloon.rss" - Resident Set Size of running domain's process (in kB),
> +"balloon.usable" - the amount of memory which can be reclaimed by balloon
> +without causing host swapping (in KB),
> +"balloon.last-update" - timestamp of the last update of statistics (in seconds),
>
> I<--vcpu> returns:
> "vcpu.current" - current number of online virtual CPUs,
> --
> 1.9.5.msysgit.0
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list