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
This commit is contained in:
Ján Tomko 2015-01-22 11:54:50 +01:00
parent b347c0c2a3
commit af1c98e406
6 changed files with 110 additions and 25 deletions

2
cfg.mk
View File

@ -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)$$)

View File

@ -1241,6 +1241,23 @@ nodeGetCPUCount(void)
#endif
}
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)
{

View File

@ -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);

View File

@ -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;

View File

@ -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",

View File

@ -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;
}
}