On 25.06.2015 20:36, Dmitry Guryanov wrote:
On 06/18/2015 12:28 PM, Nikolay Shirokovskiy wrote:
> From: Nikolay Shirokovskiy <nshirokovskiy(a)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...
You need an recent build of PCS6 update10 and update guest tools for VMs too.
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)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.
But with macros it is easy to see actual structure:
1. you get some counter
2. check if it is present (!= -1)
3. save to libvirt structure optionally converting to different units
Without conversion this could be just 1 line. May be introduce conversion macros.
Then we could get just:
PARALLELS_GET_COUNTER("guest.ram.swap_out", VIR_DOMAIN_MEMORY_STAT_SWAP_OUT,
BYTES_TO_PAGES)
PARALLELS_GET_COUNTER("guest.ram.minor_fault",
VIR_DOMAIN_MEMORY_STAT_MINOR_FAULT, NO_CONVERSION)
Without macros it is entangled with counters vector capacity checks( i < nr_stats),
incrementing counters. It looks like low-level but there is no wish to
factor it out to type.
> +
> + 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 = {