On 25.06.2015 20:36, Dmitry Guryanov wrote:
> On 06/18/2015 12:28 PM, Nikolay Shirokovskiy wrote:
>> From: Nikolay Shirokovskiy <nshirokovskiy@xxxxxxxxxxxxx>
>>
>> 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@xxxxxxxxxxxxx>
>> ---
>> 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 = {
>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list