[libvirt] [PATCH v2 3/3] vz: add memory statistics

[Dmitry Guryanov]

On 06/18/2015 12:28 PM, Nikolay Shirokovskiy wrote:
> From: Nikolay Shirokovskiy <nshirokovskiy at parallels.com>
>
> Implemented counters:
>   VIR_DOMAIN_MEMORY_STAT_SWAP_IN
>   VIR_DOMAIN_MEMORY_STAT_SWAP_OUT
>   VIR_DOMAIN_MEMORY_STAT_MINOR_FAULT
>   VIR_DOMAIN_MEMORY_STAT_MAJOR_FAULT
>   VIR_DOMAIN_MEMORY_STAT_AVAILABLE
>   VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON
>   VIR_DOMAIN_MEMORY_STAT_UNUSED
>
> Comments.
>
> 1. Use vzDomObjFromDomainRef/virDomainObjEndAPI pair to get domain
> object as we use prlsdkGetStatsParam. See previous statistics
> comments.
>
> 2. Balloon statistics is not applicable to containers. Fault
> statistics for containers not provided in PCS6 yet.

At some reason it returns -1 for all counters on my server, need to 
check, why is it...

> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
> ---
>   src/vz/vz_driver.c |   73 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 files changed, 73 insertions(+), 0 deletions(-)
>
> diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
> index 4197569..8dae7c4 100644
> --- a/src/vz/vz_driver.c
> +++ b/src/vz/vz_driver.c
> @@ -1377,6 +1377,78 @@ vzDomainInterfaceStats(virDomainPtr domain,
>       return ret;
>   }
>   
> +static int
> +vzDomainMemoryStats(virDomainPtr domain,
> +                      virDomainMemoryStatPtr stats,
> +                      unsigned int nr_stats,
> +                      unsigned int flags)
> +{
> +    virDomainObjPtr dom = NULL;
> +    int ret = -1;
> +    long long v = 0, t = 0, u = 0;
> +    size_t i = 0;
> +
> +    virCheckFlags(0, -1);
> +    if (!(dom = vzDomObjFromDomainRef(domain)))
> +        return -1;
> +
> +#define PARALLELS_GET_COUNTER(NAME, VALUE)                          \
> +    if (prlsdkGetStatsParam(dom, NAME, &VALUE) < 0)                 \
> +        goto cleanup;                                               \
> +
> +#define PARALLELS_MEMORY_STAT_SET(TAG, VALUE)                       \
> +    if (i < nr_stats) {                                             \
> +        stats[i].tag = (TAG);                                       \
> +        stats[i].val = (VALUE);                                     \
> +        i++;                                                        \
> +    }
> +
> +#define PARALLELS_COUNTER_PROTECT(VALUE) VALUE == -1 : ?
> +
> +    i = 0;
> +
> +    // count to kb
> +    PARALLELS_GET_COUNTER("guest.ram.swap_in", v)
> +    if (v != -1)
> +        PARALLELS_MEMORY_STAT_SET(VIR_DOMAIN_MEMORY_STAT_SWAP_IN, v << 12)
> +
> +    PARALLELS_GET_COUNTER("guest.ram.swap_out", v)
> +    if (v != -1)
> +        PARALLELS_MEMORY_STAT_SET(VIR_DOMAIN_MEMORY_STAT_SWAP_OUT, v << 12)
> +
> +    PARALLELS_GET_COUNTER("guest.ram.minor_fault", v)
> +    if (v != -1)
> +        PARALLELS_MEMORY_STAT_SET(VIR_DOMAIN_MEMORY_STAT_MINOR_FAULT, v)
> +
> +    PARALLELS_GET_COUNTER("guest.ram.major_fault", v)
> +    if (v != -1)
> +        PARALLELS_MEMORY_STAT_SET(VIR_DOMAIN_MEMORY_STAT_MAJOR_FAULT, v)

To be honest, I don't think macros here improve code readability, look, 
how it would be without them:

if (prlsdkGetStatsParam(dom, "guest.ram.major_fault", &v) < 0)
     goto cleanup;

if (v != -1 && i < nr_stats) {
     stats[i].tag = VIR_DOMAIN_MEMORY_STAT_MAJOR_FAULT;
     stats[i].val = v;
     i++;
}


Only "goto cleanup" and "i++" are repeating information.

> +
> +    PARALLELS_GET_COUNTER("guest.ram.total", v)
> +    if (v != -1)
> +        PARALLELS_MEMORY_STAT_SET(VIR_DOMAIN_MEMORY_STAT_AVAILABLE, v << 10)
> +
> +    PARALLELS_GET_COUNTER("guest.ram.balloon_actual", v)
> +    if (v != -1)
> +        PARALLELS_MEMORY_STAT_SET(VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON, v << 10)
> +
> +    PARALLELS_GET_COUNTER("guest.ram.usage", u)
> +    PARALLELS_GET_COUNTER("guest.ram.total", t)
> +    if (u != -1 && t != -1)
> +        PARALLELS_MEMORY_STAT_SET(VIR_DOMAIN_MEMORY_STAT_UNUSED, (t - u) << 10)
> +
> +
> +#undef PARALLELS_GET_COUNTER
> +#undef PARALLELS_MEMORY_STAT_SET
> +
> +    ret = i;
> + cleanup:
> +    if (dom)
> +        virDomainObjEndAPI(&dom);
> +
> +    return ret;
> +}
> +
>   static virHypervisorDriver vzDriver = {
>       .name = "vz",
>       .connectOpen = vzConnectOpen,            /* 0.10.0 */
> @@ -1429,6 +1501,7 @@ static virHypervisorDriver vzDriver = {
>       .domainBlockStats = vzDomainBlockStats, /* 1.3.0 */
>       .domainBlockStatsFlags = vzDomainBlockStatsFlags, /* 1.3.0 */
>       .domainInterfaceStats = vzDomainInterfaceStats, /* 1.3.0 */
> +    .domainMemoryStats = vzDomainMemoryStats, /* 1.3.0 */
>   };
>   
>   static virConnectDriver vzConnectDriver = {


-- 
Dmitry Guryanov