[libvirt] [PATCH] Fix virCgroupGetPercpuStats with non-continuous present CPUs

Per-cpu stats are only shown for present CPUs in the cgroups, but we were only parsing the largest CPU number from /sys/devices/system/cpu/present and looking for stats even for non-present CPUs. This resulted in: internal error: cpuacct parse error --- cfg.mk | 2 +- src/nodeinfo.c | 17 +++++++++++++++++ src/nodeinfo.h | 1 + src/util/vircgroup.c | 24 ++++++++++++++++++------ tests/vircgroupmock.c | 44 ++++++++++++++++++++++++++++++++++++++------ tests/vircgrouptest.c | 47 +++++++++++++++++++++++++++++++++++------------ 6 files changed, 110 insertions(+), 25 deletions(-) diff --git a/cfg.mk b/cfg.mk index 21f83c3..70612f8 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1095,7 +1095,7 @@ exclude_file_name_regexp--sc_prohibit_asprintf = \ ^(bootstrap.conf$$|src/util/virstring\.[ch]$$|tests/vircgroupmock\.c$$) exclude_file_name_regexp--sc_prohibit_strdup = \ - ^(docs/|examples/|src/util/virstring\.c|tests/virnetserverclientmock.c$$) + ^(docs/|examples/|src/util/virstring\.c|tests/vir(netserverclient|cgroup)mock.c$$) exclude_file_name_regexp--sc_prohibit_close = \ (\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/virfile\.c|src/libvirt-stream\.c|tests/vir(cgroup|pci)mock\.c)$$) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 3c22ebc..3a27c22 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -1242,6 +1242,23 @@ nodeGetCPUCount(void) } virBitmapPtr +nodeGetPresentCPUBitmap(void) +{ + int max_present; + + if ((max_present = nodeGetCPUCount()) < 0) + return NULL; + +#ifdef __linux__ + if (virFileExists(SYSFS_SYSTEM_PATH "/cpu/present")) + return linuxParseCPUmap(max_present, SYSFS_SYSTEM_PATH "/cpu/present"); +#endif + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("non-continuous host cpu numbers not implemented on this platform")); + return NULL; +} + +virBitmapPtr nodeGetCPUBitmap(int *max_id ATTRIBUTE_UNUSED) { #ifdef __linux__ diff --git a/src/nodeinfo.h b/src/nodeinfo.h index a993c63..047bd5c 100644 --- a/src/nodeinfo.h +++ b/src/nodeinfo.h @@ -43,6 +43,7 @@ int nodeGetCellsFreeMemory(unsigned long long *freeMems, int nodeGetMemory(unsigned long long *mem, unsigned long long *freeMem); +virBitmapPtr nodeGetPresentCPUBitmap(void); virBitmapPtr nodeGetCPUBitmap(int *max_id); int nodeGetCPUCount(void); diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index fe34290..e65617a 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -2985,7 +2985,8 @@ static int virCgroupGetPercpuVcpuSum(virCgroupPtr group, unsigned int nvcpupids, unsigned long long *sum_cpu_time, - unsigned int num) + size_t nsum, + virBitmapPtr cpumap) { int ret = -1; size_t i; @@ -2995,7 +2996,7 @@ virCgroupGetPercpuVcpuSum(virCgroupPtr group, for (i = 0; i < nvcpupids; i++) { char *pos; unsigned long long tmp; - size_t j; + ssize_t j; if (virCgroupNewVcpu(group, i, false, &group_vcpu) < 0) goto cleanup; @@ -3004,7 +3005,9 @@ virCgroupGetPercpuVcpuSum(virCgroupPtr group, goto cleanup; pos = buf; - for (j = 0; j < num; j++) { + for (j = virBitmapNextSetBit(cpumap, -1); + j >= 0 && j < nsum; + j = virBitmapNextSetBit(cpumap, j)) { if (virStrToLong_ull(pos, &pos, 10, &tmp) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cpuacct parse error")); @@ -3042,6 +3045,7 @@ virCgroupGetPercpuStats(virCgroupPtr group, virTypedParameterPtr ent; int param_idx; unsigned long long cpu_time; + virBitmapPtr cpumap = NULL; /* return the number of supported params */ if (nparams == 0 && ncpus != 0) { @@ -3052,9 +3056,11 @@ virCgroupGetPercpuStats(virCgroupPtr group, } /* To parse account file, we need to know how many cpus are present. */ - if ((total_cpus = nodeGetCPUCount()) < 0) + if (!(cpumap = nodeGetPresentCPUBitmap())) return rv; + total_cpus = virBitmapSize(cpumap); + if (ncpus == 0) return total_cpus; @@ -3077,7 +3083,11 @@ virCgroupGetPercpuStats(virCgroupPtr group, need_cpus = MIN(total_cpus, start_cpu + ncpus); for (i = 0; i < need_cpus; i++) { - if (virStrToLong_ull(pos, &pos, 10, &cpu_time) < 0) { + bool present; + ignore_value(virBitmapGetBit(cpumap, i, &present)); + if (!present) { + cpu_time = 0; + } else if (virStrToLong_ull(pos, &pos, 10, &cpu_time) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cpuacct parse error")); goto cleanup; @@ -3097,7 +3107,8 @@ virCgroupGetPercpuStats(virCgroupPtr group, if (VIR_ALLOC_N(sum_cpu_time, need_cpus) < 0) goto cleanup; - if (virCgroupGetPercpuVcpuSum(group, nvcpupids, sum_cpu_time, need_cpus) < 0) + if (virCgroupGetPercpuVcpuSum(group, nvcpupids, sum_cpu_time, need_cpus, + cpumap) < 0) goto cleanup; for (i = start_cpu; i < need_cpus; i++) { @@ -3113,6 +3124,7 @@ virCgroupGetPercpuStats(virCgroupPtr group, rv = param_idx + 1; cleanup: + virBitmapFree(cpumap); VIR_FREE(sum_cpu_time); VIR_FREE(buf); return rv; diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c index 6e0a0e7..f2fee7d 100644 --- a/tests/vircgroupmock.c +++ b/tests/vircgroupmock.c @@ -51,6 +51,8 @@ const char *fakedevicedir1 = FAKEDEVDIR1; # define SYSFS_PREFIX "/not/really/sys/fs/cgroup/" +# define SYSFS_CPU_PRESENT "/sys/devices/system/cpu/present" +# define SYSFS_CPU_PRESENT_MOCKED "devices_system_cpu_present" /* * The plan: @@ -215,7 +217,15 @@ static int make_controller(const char *path, mode_t mode) "user 216687025\n" "system 43421396\n"); MAKE_FILE("cpuacct.usage", "2787788855799582\n"); - MAKE_FILE("cpuacct.usage_percpu", "1413142688153030 1374646168910542\n"); + MAKE_FILE("cpuacct.usage_percpu", + "7059492996 0 0 0 0 0 0 0 4180532496 0 0 0 0 0 0 0 " + "1957541268 0 0 0 0 0 0 0 2065932204 0 0 0 0 0 0 0 " + "18228689414 0 0 0 0 0 0 0 4245525148 0 0 0 0 0 0 0 " + "2911161568 0 0 0 0 0 0 0 1407758136 0 0 0 0 0 0 0 " + "1836807700 0 0 0 0 0 0 0 1065296618 0 0 0 0 0 0 0 " + "2046213266 0 0 0 0 0 0 0 747889778 0 0 0 0 0 0 0 " + "709566900 0 0 0 0 0 0 0 444777342 0 0 0 0 0 0 0 " + "5683512916 0 0 0 0 0 0 0 635751356 0 0 0 0 0 0 0\n"); } else if (STRPREFIX(controller, "cpuset")) { MAKE_FILE("cpuset.cpu_exclusive", "1\n"); if (STREQ(controller, "cpuset")) @@ -431,6 +441,9 @@ static void init_sysfs(void) MAKE_CONTROLLER("blkio"); MAKE_CONTROLLER("memory"); MAKE_CONTROLLER("freezer"); + + if (make_file(fakesysfsdir, SYSFS_CPU_PRESENT_MOCKED, "8-23,48-159\n") < 0) + abort(); } @@ -623,21 +636,27 @@ int __xstat(int ver, const char *path, struct stat *sb) int stat(const char *path, struct stat *sb) { + char *newpath = NULL; int ret; init_syms(); - if (STRPREFIX(path, SYSFS_PREFIX)) { + if (STREQ(path, SYSFS_CPU_PRESENT)) { + init_sysfs(); + if (asprintf(&newpath, "%s/%s", + fakesysfsdir, + SYSFS_CPU_PRESENT_MOCKED) < 0) { + errno = ENOMEM; + return -1; + } + } else if (STRPREFIX(path, SYSFS_PREFIX)) { init_sysfs(); - char *newpath; if (asprintf(&newpath, "%s/%s", fakesysfsdir, path + strlen(SYSFS_PREFIX)) < 0) { errno = ENOMEM; return -1; } - ret = realstat(newpath, sb); - free(newpath); } else if (STRPREFIX(path, fakedevicedir0)) { sb->st_mode = S_IFBLK; sb->st_rdev = makedev(8, 0); @@ -647,8 +666,11 @@ int stat(const char *path, struct stat *sb) sb->st_rdev = makedev(9, 0); return 0; } else { - ret = realstat(path, sb); + if (!(newpath = strdup(path))) + return -1; } + ret = realstat(newpath, sb); + free(newpath); return ret; } @@ -682,6 +704,16 @@ int open(const char *path, int flags, ...) init_syms(); + if (STREQ(path, SYSFS_CPU_PRESENT)) { + init_sysfs(); + if (asprintf(&newpath, "%s/%s", + fakesysfsdir, + SYSFS_CPU_PRESENT_MOCKED) < 0) { + errno = ENOMEM; + return -1; + } + } + if (STRPREFIX(path, SYSFS_PREFIX)) { init_sysfs(); if (asprintf(&newpath, "%s/%s", diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index 35ac0c0..b65ea3f 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -538,12 +538,35 @@ static int testCgroupGetPercpuStats(const void *args ATTRIBUTE_UNUSED) virCgroupPtr cgroup = NULL; size_t i; int rv, ret = -1; - virTypedParameter params[2]; + virTypedParameterPtr params = NULL; +# define EXPECTED_NCPUS 160 - // TODO: mock nodeGetCPUCount() as well & check 2nd cpu, too unsigned long long expected[] = { - 1413142688153030ULL + 0, 0, 0, 0, 0, 0, 0, 0, + 7059492996, 0, 0, 0, 0, 0, 0, 0, + 4180532496, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, + 1957541268, 0, 0, 0, 0, 0, 0, 0, + 2065932204, 0, 0, 0, 0, 0, 0, 0, + 18228689414, 0, 0, 0, 0, 0, 0, 0, + 4245525148, 0, 0, 0, 0, 0, 0, 0, + 2911161568, 0, 0, 0, 0, 0, 0, 0, + 1407758136, 0, 0, 0, 0, 0, 0, 0, + 1836807700, 0, 0, 0, 0, 0, 0, 0, + 1065296618, 0, 0, 0, 0, 0, 0, 0, + 2046213266, 0, 0, 0, 0, 0, 0, 0, + 747889778, 0, 0, 0, 0, 0, 0, 0, + 709566900, 0, 0, 0, 0, 0, 0, 0, + 444777342, 0, 0, 0, 0, 0, 0, 0, + 5683512916, 0, 0, 0, 0, 0, 0, 0, + 635751356, 0, 0, 0, 0, 0, 0, 0, }; + verify(ARRAY_CARDINALITY(expected) == EXPECTED_NCPUS); + + if (VIR_ALLOC_N(params, EXPECTED_NCPUS) < 0) + goto cleanup; if ((rv = virCgroupNewPartition("/virtualmachines", true, (1 << VIR_CGROUP_CONTROLLER_CPU) | @@ -553,37 +576,37 @@ static int testCgroupGetPercpuStats(const void *args ATTRIBUTE_UNUSED) goto cleanup; } - if (nodeGetCPUCount() < 1) { + if (nodeGetCPUCount() != EXPECTED_NCPUS) { fprintf(stderr, "Unexpected: nodeGetCPUCount() yields: %d\n", nodeGetCPUCount()); goto cleanup; } if ((rv = virCgroupGetPercpuStats(cgroup, params, - 2, 0, 1, 0)) < 0) { + 1, 0, EXPECTED_NCPUS, 0)) < 0) { fprintf(stderr, "Failed call to virCgroupGetPercpuStats for /virtualmachines cgroup: %d\n", -rv); goto cleanup; } - for (i = 0; i < ARRAY_CARDINALITY(expected); i++) { + for (i = 0; i < EXPECTED_NCPUS; i++) { if (!STREQ(params[i].field, VIR_DOMAIN_CPU_STATS_CPUTIME)) { fprintf(stderr, - "Wrong parameter name value from virCgroupGetPercpuStats (is: %s)\n", - params[i].field); + "Wrong parameter name value from virCgroupGetPercpuStats at %zu (is: %s)\n", + i, params[i].field); goto cleanup; } if (params[i].type != VIR_TYPED_PARAM_ULLONG) { fprintf(stderr, - "Wrong parameter value type from virCgroupGetPercpuStats (is: %d)\n", - params[i].type); + "Wrong parameter value type from virCgroupGetPercpuStats at %zu (is: %d)\n", + i, params[i].type); goto cleanup; } if (params[i].value.ul != expected[i]) { fprintf(stderr, - "Wrong value from virCgroupGetMemoryUsage (expected %llu)\n", - params[i].value.ul); + "Wrong value from virCgroupGetMemoryUsage at %zu (expected %llu)\n", + i, params[i].value.ul); goto cleanup; } } -- 2.0.4

On 22.01.2015 12:26, Ján Tomko wrote:
Per-cpu stats are only shown for present CPUs in the cgroups, but we were only parsing the largest CPU number from /sys/devices/system/cpu/present and looking for stats even for non-present CPUs. This resulted in: internal error: cpuacct parse error --- cfg.mk | 2 +- src/nodeinfo.c | 17 +++++++++++++++++ src/nodeinfo.h | 1 + src/util/vircgroup.c | 24 ++++++++++++++++++------ tests/vircgroupmock.c | 44 ++++++++++++++++++++++++++++++++++++++------ tests/vircgrouptest.c | 47 +++++++++++++++++++++++++++++++++++------------ 6 files changed, 110 insertions(+), 25 deletions(-)
diff --git a/cfg.mk b/cfg.mk index 21f83c3..70612f8 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1095,7 +1095,7 @@ exclude_file_name_regexp--sc_prohibit_asprintf = \ ^(bootstrap.conf$$|src/util/virstring\.[ch]$$|tests/vircgroupmock\.c$$)
exclude_file_name_regexp--sc_prohibit_strdup = \ - ^(docs/|examples/|src/util/virstring\.c|tests/virnetserverclientmock.c$$) + ^(docs/|examples/|src/util/virstring\.c|tests/vir(netserverclient|cgroup)mock.c$$)
exclude_file_name_regexp--sc_prohibit_close = \ (\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/virfile\.c|src/libvirt-stream\.c|tests/vir(cgroup|pci)mock\.c)$$) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 3c22ebc..3a27c22 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -1242,6 +1242,23 @@ nodeGetCPUCount(void) }
virBitmapPtr +nodeGetPresentCPUBitmap(void) +{ + int max_present; + + if ((max_present = nodeGetCPUCount()) < 0) + return NULL; + +#ifdef __linux__ + if (virFileExists(SYSFS_SYSTEM_PATH "/cpu/present")) + return linuxParseCPUmap(max_present, SYSFS_SYSTEM_PATH "/cpu/present"); +#endif + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("non-continuous host cpu numbers not implemented on this platform")); + return NULL; +} + +virBitmapPtr nodeGetCPUBitmap(int *max_id ATTRIBUTE_UNUSED) {
From my understanding the nodeGetPresentCPUBitmap() is no different than nodeGetCPUBitmap(). The latter can do everything that the former and more. Can we just use already existing function then?
Otherwise looking good. Michal

On 01/22/2015 03:21 PM, Michal Privoznik wrote:
On 22.01.2015 12:26, Ján Tomko wrote:
Per-cpu stats are only shown for present CPUs in the cgroups, but we were only parsing the largest CPU number from /sys/devices/system/cpu/present and looking for stats even for non-present CPUs. This resulted in: internal error: cpuacct parse error --- cfg.mk | 2 +- src/nodeinfo.c | 17 +++++++++++++++++ src/nodeinfo.h | 1 + src/util/vircgroup.c | 24 ++++++++++++++++++------ tests/vircgroupmock.c | 44 ++++++++++++++++++++++++++++++++++++++------ tests/vircgrouptest.c | 47 +++++++++++++++++++++++++++++++++++------------ 6 files changed, 110 insertions(+), 25 deletions(-)
diff --git a/cfg.mk b/cfg.mk index 21f83c3..70612f8 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1095,7 +1095,7 @@ exclude_file_name_regexp--sc_prohibit_asprintf = \ ^(bootstrap.conf$$|src/util/virstring\.[ch]$$|tests/vircgroupmock\.c$$)
exclude_file_name_regexp--sc_prohibit_strdup = \ - ^(docs/|examples/|src/util/virstring\.c|tests/virnetserverclientmock.c$$) + ^(docs/|examples/|src/util/virstring\.c|tests/vir(netserverclient|cgroup)mock.c$$)
exclude_file_name_regexp--sc_prohibit_close = \ (\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/virfile\.c|src/libvirt-stream\.c|tests/vir(cgroup|pci)mock\.c)$$) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 3c22ebc..3a27c22 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -1242,6 +1242,23 @@ nodeGetCPUCount(void) }
virBitmapPtr +nodeGetPresentCPUBitmap(void) +{ + int max_present; + + if ((max_present = nodeGetCPUCount()) < 0) + return NULL; + +#ifdef __linux__ + if (virFileExists(SYSFS_SYSTEM_PATH "/cpu/present")) + return linuxParseCPUmap(max_present, SYSFS_SYSTEM_PATH "/cpu/present"); +#endif + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("non-continuous host cpu numbers not implemented on this platform")); + return NULL; +} + +virBitmapPtr nodeGetCPUBitmap(int *max_id ATTRIBUTE_UNUSED) {
From my understanding the nodeGetPresentCPUBitmap() is no different than nodeGetCPUBitmap(). The latter can do everything that the former and more. Can we just use already existing function then?
This functions checks the present CPUs, nodeGetCPUBitmap checks the online CPUs. I don't think splitting their common part into another function would be more readable - IMO the fact that both use linuxParseCPUmap should be enough. Jan
Otherwise looking good.
Michal
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On 22.01.2015 15:52, Ján Tomko wrote:
On 01/22/2015 03:21 PM, Michal Privoznik wrote:
On 22.01.2015 12:26, Ján Tomko wrote:
Per-cpu stats are only shown for present CPUs in the cgroups, but we were only parsing the largest CPU number from /sys/devices/system/cpu/present and looking for stats even for non-present CPUs. This resulted in: internal error: cpuacct parse error --- cfg.mk | 2 +- src/nodeinfo.c | 17 +++++++++++++++++ src/nodeinfo.h | 1 + src/util/vircgroup.c | 24 ++++++++++++++++++------ tests/vircgroupmock.c | 44 ++++++++++++++++++++++++++++++++++++++------ tests/vircgrouptest.c | 47 +++++++++++++++++++++++++++++++++++------------ 6 files changed, 110 insertions(+), 25 deletions(-)
diff --git a/cfg.mk b/cfg.mk index 21f83c3..70612f8 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1095,7 +1095,7 @@ exclude_file_name_regexp--sc_prohibit_asprintf = \ ^(bootstrap.conf$$|src/util/virstring\.[ch]$$|tests/vircgroupmock\.c$$)
exclude_file_name_regexp--sc_prohibit_strdup = \ - ^(docs/|examples/|src/util/virstring\.c|tests/virnetserverclientmock.c$$) + ^(docs/|examples/|src/util/virstring\.c|tests/vir(netserverclient|cgroup)mock.c$$)
exclude_file_name_regexp--sc_prohibit_close = \ (\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/virfile\.c|src/libvirt-stream\.c|tests/vir(cgroup|pci)mock\.c)$$) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 3c22ebc..3a27c22 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -1242,6 +1242,23 @@ nodeGetCPUCount(void) }
virBitmapPtr +nodeGetPresentCPUBitmap(void) +{ + int max_present; + + if ((max_present = nodeGetCPUCount()) < 0) + return NULL; + +#ifdef __linux__ + if (virFileExists(SYSFS_SYSTEM_PATH "/cpu/present")) + return linuxParseCPUmap(max_present, SYSFS_SYSTEM_PATH "/cpu/present"); +#endif + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("non-continuous host cpu numbers not implemented on this platform")); + return NULL; +} + +virBitmapPtr nodeGetCPUBitmap(int *max_id ATTRIBUTE_UNUSED) {
From my understanding the nodeGetPresentCPUBitmap() is no different than nodeGetCPUBitmap(). The latter can do everything that the former and more. Can we just use already existing function then?
This functions checks the present CPUs, nodeGetCPUBitmap checks the online CPUs.
I don't think splitting their common part into another function would be more readable - IMO the fact that both use linuxParseCPUmap should be enough.
Okay, ACK then. Michal

On 01/22/2015 04:26 AM, Ján Tomko wrote:
Per-cpu stats are only shown for present CPUs in the cgroups, but we were only parsing the largest CPU number from /sys/devices/system/cpu/present and looking for stats even for non-present CPUs. This resulted in: internal error: cpuacct parse error --- cfg.mk | 2 +- src/nodeinfo.c | 17 +++++++++++++++++ src/nodeinfo.h | 1 + src/util/vircgroup.c | 24 ++++++++++++++++++------ tests/vircgroupmock.c | 44 ++++++++++++++++++++++++++++++++++++++------ tests/vircgrouptest.c | 47 +++++++++++++++++++++++++++++++++++------------ 6 files changed, 110 insertions(+), 25 deletions(-)
+++ b/tests/vircgroupmock.c @@ -51,6 +51,8 @@ const char *fakedevicedir1 = FAKEDEVDIR1;
# define SYSFS_PREFIX "/not/really/sys/fs/cgroup/" +# define SYSFS_CPU_PRESENT "/sys/devices/system/cpu/present" +# define SYSFS_CPU_PRESENT_MOCKED "devices_system_cpu_present"
This mock file is not working on RHEL 5:
+++ b/tests/vircgrouptest.c @@ -538,12 +538,35 @@ static int testCgroupGetPercpuStats(const void *args ATTRIBUTE_UNUSED) virCgroupPtr cgroup = NULL; size_t i; int rv, ret = -1; - virTypedParameter params[2]; + virTypedParameterPtr params = NULL; +# define EXPECTED_NCPUS 160
@@ -553,37 +576,37 @@ static int testCgroupGetPercpuStats(const void *args ATTRIBUTE_UNUSED) goto cleanup; }
- if (nodeGetCPUCount() < 1) { + if (nodeGetCPUCount() != EXPECTED_NCPUS) {
On Fedora 21, I see the mocking kicking in during linuxParseCPUmax(): nodeGetCPUCount () at nodeinfo.c:1209 1209 { (gdb) n 1216 char *cpupath = NULL; (gdb) 1219 if (virFileExists(SYSFS_SYSTEM_PATH "/cpu/present")) { (gdb) 1220 ncpu = linuxParseCPUmax(SYSFS_SYSTEM_PATH "/cpu/present"); (gdb) 1237 VIR_FREE(cpupath); (gdb) p ncpu $1 = 160 but in RHEL 5, we aren't mocking the virFileExists() gate, and I see: nodeGetCPUCount () at nodeinfo.c:1209 1209 { (gdb) n 1216 char *cpupath = NULL; (gdb) 1219 if (virFileExists(SYSFS_SYSTEM_PATH "/cpu/present")) { (gdb) 1221 } else if (virFileExists(SYSFS_SYSTEM_PATH "/cpu/cpu0")) { (gdb) 1225 VIR_FREE(cpupath); (gdb) 1224 ncpu++; (gdb) 1225 VIR_FREE(cpupath); (gdb) 1226 if (virAsprintf(&cpupath, "%s/cpu/cpu%d", (gdb) fin Run till exit from #0 nodeGetCPUCount () at nodeinfo.c:1226 0x0804ae19 in testCgroupGetPercpuStats (args=0x0) at vircgrouptest.c:659 659 if (nodeGetCPUCount() != EXPECTED_NCPUS) { Value returned is $1 = 3 I didn't investigate further, but we need to improve the mock code so that virFileExists(SYSFS_SYSTEM_PATH "/cpu/present") works even when run on an older kernel where it is not natively present. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (3)
-
Eric Blake
-
Ján Tomko
-
Michal Privoznik