mirror of https://gitee.com/openkylin/libvirt.git
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
parent
b347c0c2a3
commit
af1c98e406
2
cfg.mk
2
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)$$)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
{
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue