[libvirt] [PATCH v2] test_driver: implement virDomainMemoryStats

[Erik Skultety]

On Fri, Aug 02, 2019 at 12:07:19AM +0200, Ilias Stamatis wrote:
> Signed-off-by: Ilias Stamatis <stamatis.iliass at gmail.com>
> ---
>  src/test/test_driver.c | 54 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 54 insertions(+)
>
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index aae9875194..0e0acf2baf 100755
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -7367,6 +7367,59 @@ testDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags)
>      return 0;
>  }
>
> +
> +static int
> +testDomainMemoryStats(virDomainPtr dom,
> +                      virDomainMemoryStatPtr stats,
> +                      unsigned int nr_stats,
> +                      unsigned int flags)
> +{
> +    virDomainObjPtr vm = NULL;
> +    int cur_memory;
> +    int ret = -1;
> +
> +    virCheckFlags(0, -1);
> +
> +    if (!(vm = testDomObjFromDomain(dom)))
> +        return -1;
> +
> +    if (virDomainObjCheckActive(vm) < 0)
> +        goto cleanup;
> +
> +    cur_memory = vm->def->mem.cur_balloon;
> +    ret = 0;
> +
> +#define STATS_SET_PARAM(name, value) \
> +    if (ret < nr_stats) { \
> +        stats[ret].tag = name; \
> +        stats[ret].val = value; \
> +        ret++; \
> +    }
> +
> +    if (virDomainDefHasMemballoon(vm->def)) {
> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON, cur_memory);
> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_SWAP_IN, 0);
> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_SWAP_OUT, 0);
> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_MAJOR_FAULT, 0);
> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_MINOR_FAULT, 0);
> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_UNUSED, cur_memory / 2);
> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_AVAILABLE, cur_memory);
> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_USABLE, cur_memory / 2);
> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE, 627319920);
> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_DISK_CACHES, cur_memory / 10);

Let's divide by 4 or 8 ^here, compiler is usually smart enough with such
divisions and prefers bitwise operations instead.

> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_HUGETLB_PGALLOC, 0);
> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_HUGETLB_PGFAIL, 0);
> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_RSS, cur_memory);

I'd divide here by 2 ^here as well, since it's a bit odd that the residential
memory of the process equals it's current balloon and yet it still has some
usable memory.

It would also be beneficial if we introduced a default memballoon device in the
test config, otherwise this API returns nothing.

Since we're still in the freeze, I suggest you re-spin the series with a patch
introducing the memballoon device and adjusting according to my comments above,
still, you can consider my:

Reviewed-by: Erik Skultety <eskultet at redhat.com>