On Wed, Nov 07, 2018 at 06:57:40PM -0500, John Ferlan wrote:
Add a test to fetch the GetMemoryStat output. This only gets data for v1 only right now since the v2 data from commit 61ff6021 is rather useless returning all 0's.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- tests/vircgrouptest.c | 61 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+)
diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index 310e1fb6a2..06c4a8ef5c 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -802,6 +802,64 @@ static int testCgroupGetMemoryUsage(const void *args ATTRIBUTE_UNUSED) return ret; }
+ +static int +testCgroupGetMemoryStat(const void *args ATTRIBUTE_UNUSED) +{ + virCgroupPtr cgroup = NULL; + int rv, ret = -1;
Please each variable on separate line. Once you need to change/remove any of the variable the diff is way better.
+ size_t i; + + const unsigned long long expected_values[] = { + 1336619008ULL, + 67100672ULL, + 145887232ULL, + 661872640ULL, + 627400704UL, + 3690496ULL + }; + const char* names[] = { + "cache", + "active_anon", + "inactive_anon", + "active_file", + "inactive_file", + "unevictable" + }; + unsigned long long values[ARRAY_CARDINALITY(expected_values)]; + + if ((rv = virCgroupNewPartition("/virtualmachines", true, + (1 << VIR_CGROUP_CONTROLLER_MEMORY), + &cgroup)) < 0) { + fprintf(stderr, "Could not create /virtualmachines cgroup: %d\n", -rv); + goto cleanup; + } + + if ((rv = virCgroupGetMemoryStat(cgroup, &values[0], + &values[1], &values[2], + &values[3], &values[4], + &values[5])) < 0) { + fprintf(stderr, "Could not retrieve GetMemoryStat for /virtualmachines cgroup: %d\n", -rv); + goto cleanup; + } + + for (i = 0; i < ARRAY_CARDINALITY(expected_values); i++) { + if (expected_values[i] != (values[i] << 10)) {
This feels wrong and it's just a lucky coincidence that it works with these values. It's basically the same operation as 'x * 1024'. I would rather change it into this: if ((expected_values[i] >> 10) != values[i]) { because we know that we do the same operation after reading these values from memory.stat file.
+ fprintf(stderr, + "Wrong value (%llu) for %s from virCgroupGetMemoryStat (expected %llu)\n", + values[i], names[i], expected_values[i]);
This would print wrong values, we need to print shifted values. Probably the best solution would be to have "expected_values" with the correct number from the start. Note: please keep the lines under 80 characters. Because it's a test I'm OK with both solutions, modifying "expected_values" in place or to have them correct from the start and I'll leave it up to you. There is no need to resend it. Reviewed-by: Pavel Hrdina <phrdina@redhat.com>