[libvirt] [PATCH] Fix virCgroupGetPercpuStats with non-continuous present CPUs
Ján Tomko
jtomko at redhat.com
Thu Jan 22 11:26:38 UTC 2015
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
More information about the libvir-list
mailing list