From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Some users in Ubuntu/Debian seem to have a setup where all the cgroup controllers are mounted on /sys/fs/cgroup rather than any /sys/fs/cgroup/<controller> name. In the loop which detects which controllers are present for a mount point we were modifying 'mnt_dir' field in the 'struct mntent' var, but not always restoring the original value. This caused detection to break in the all-in-one mount setup. Fix that logic bug and add test case coverage for this mount setup. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/util/vircgroup.c | 3 ++- tests/vircgroupmock.c | 42 ++++++++++++++++++++++++++++++++++++--- tests/vircgrouptest.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 4 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 16458a3..a260356 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -342,10 +342,11 @@ virCgroupDetectMounts(virCgroupPtr group) entry.mnt_dir); goto error; } - *tmp2 = '\0'; + /* If it is a co-mount it has a filename like "cpu,cpuacct" * and we must identify the symlink path */ if (strchr(tmp2 + 1, ',')) { + *tmp2 = '\0'; if (virAsprintf(&linksrc, "%s/%s", entry.mnt_dir, typestr) < 0) goto error; diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c index f1a5700..bf52843 100644 --- a/tests/vircgroupmock.c +++ b/tests/vircgroupmock.c @@ -103,6 +103,27 @@ const char *proccgroups = "blkio 8 4 1\n"; +const char *procmountsallinone = + "rootfs / rootfs rw 0 0\n" + "sysfs /sys sysfs rw,nosuid,nodev,noexec,relatime 0 0\n" + "proc /proc proc rw,nosuid,nodev,noexec,relatime 0 0\n" + "udev /dev devtmpfs rw,relatime,size=16458560k,nr_inodes=4114640,mode=755 0 0\n" + "devpts /dev/pts devpts rw,nosuid,noexec,relatime,gid=5,mode=620,ptmxmode=000 0 0\n" + "nfsd /proc/fs/nfsd nfsd rw,relatime 0 0\n" + "cgroup /not/really/sys/fs/cgroup cgroup rw,relatime,blkio,devices,memory,cpuacct,cpu,cpuset 0 0\n"; + +const char *procselfcgroupsallinone = + "6:blkio,devices,memory,cpuacct,cpu,cpuset:/"; + +const char *proccgroupsallinone = + "#subsys_name hierarchy num_cgroups enabled\n" + "cpuset 6 1 1\n" + "cpu 6 1 1\n" + "cpuacct 6 1 1\n" + "memory 6 1 1\n" + "devices 6 1 1\n" + "blkio 6 1 1\n"; + static int make_file(const char *path, const char *name, const char *value) @@ -378,11 +399,20 @@ static void init_sysfs(void) FILE *fopen(const char *path, const char *mode) { + const char *mock; + bool allinone = false; init_syms(); + mock = getenv("VIR_CGROUP_MOCK_MODE"); + if (mock && STREQ(mock, "allinone")) + allinone = true; + if (STREQ(path, "/proc/mounts")) { if (STREQ(mode, "r")) { - return fmemopen((void *)procmounts, strlen(procmounts), mode); + if (allinone) + return fmemopen((void *)procmountsallinone, strlen(procmountsallinone), mode); + else + return fmemopen((void *)procmounts, strlen(procmounts), mode); } else { errno = EACCES; return NULL; @@ -390,7 +420,10 @@ FILE *fopen(const char *path, const char *mode) } if (STREQ(path, "/proc/cgroups")) { if (STREQ(mode, "r")) { - return fmemopen((void *)proccgroups, strlen(proccgroups), mode); + if (allinone) + return fmemopen((void *)proccgroupsallinone, strlen(proccgroupsallinone), mode); + else + return fmemopen((void *)proccgroups, strlen(proccgroups), mode); } else { errno = EACCES; return NULL; @@ -398,7 +431,10 @@ FILE *fopen(const char *path, const char *mode) } if (STREQ(path, "/proc/self/cgroup")) { if (STREQ(mode, "r")) { - return fmemopen((void *)procselfcgroups, strlen(procselfcgroups), mode); + if (allinone) + return fmemopen((void *)procselfcgroupsallinone, strlen(procselfcgroupsallinone), mode); + else + return fmemopen((void *)procselfcgroups, strlen(procselfcgroups), mode); } else { errno = EACCES; return NULL; diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index 4bdd4c9..f12587c 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -99,6 +99,16 @@ const char *mountsFull[VIR_CGROUP_CONTROLLER_LAST] = { [VIR_CGROUP_CONTROLLER_BLKIO] = "/not/really/sys/fs/cgroup/blkio", [VIR_CGROUP_CONTROLLER_SYSTEMD] = "/not/really/sys/fs/cgroup/systemd", }; +const char *mountsAllInOne[VIR_CGROUP_CONTROLLER_LAST] = { + [VIR_CGROUP_CONTROLLER_CPU] = "/not/really/sys/fs/cgroup", + [VIR_CGROUP_CONTROLLER_CPUACCT] = "/not/really/sys/fs/cgroup", + [VIR_CGROUP_CONTROLLER_CPUSET] = "/not/really/sys/fs/cgroup", + [VIR_CGROUP_CONTROLLER_MEMORY] = "/not/really/sys/fs/cgroup", + [VIR_CGROUP_CONTROLLER_DEVICES] = "/not/really/sys/fs/cgroup", + [VIR_CGROUP_CONTROLLER_FREEZER] = NULL, + [VIR_CGROUP_CONTROLLER_BLKIO] = "/not/really/sys/fs/cgroup", + [VIR_CGROUP_CONTROLLER_SYSTEMD] = NULL, +}; const char *links[VIR_CGROUP_CONTROLLER_LAST] = { [VIR_CGROUP_CONTROLLER_CPU] = "/not/really/sys/fs/cgroup/cpu", @@ -108,6 +118,18 @@ const char *links[VIR_CGROUP_CONTROLLER_LAST] = { [VIR_CGROUP_CONTROLLER_DEVICES] = NULL, [VIR_CGROUP_CONTROLLER_FREEZER] = NULL, [VIR_CGROUP_CONTROLLER_BLKIO] = NULL, + [VIR_CGROUP_CONTROLLER_SYSTEMD] = NULL, +}; + +const char *linksAllInOne[VIR_CGROUP_CONTROLLER_LAST] = { + [VIR_CGROUP_CONTROLLER_CPU] = NULL, + [VIR_CGROUP_CONTROLLER_CPUACCT] = NULL, + [VIR_CGROUP_CONTROLLER_CPUSET] = NULL, + [VIR_CGROUP_CONTROLLER_MEMORY] = NULL, + [VIR_CGROUP_CONTROLLER_DEVICES] = NULL, + [VIR_CGROUP_CONTROLLER_FREEZER] = NULL, + [VIR_CGROUP_CONTROLLER_BLKIO] = NULL, + [VIR_CGROUP_CONTROLLER_SYSTEMD] = NULL, }; @@ -417,6 +439,34 @@ cleanup: return ret; } +static int testCgroupNewForSelfAllInOne(const void *args ATTRIBUTE_UNUSED) +{ + virCgroupPtr cgroup = NULL; + int ret = -1; + const char *placement[VIR_CGROUP_CONTROLLER_LAST] = { + [VIR_CGROUP_CONTROLLER_CPU] = "/", + [VIR_CGROUP_CONTROLLER_CPUACCT] = "/", + [VIR_CGROUP_CONTROLLER_CPUSET] = "/", + [VIR_CGROUP_CONTROLLER_MEMORY] = "/", + [VIR_CGROUP_CONTROLLER_DEVICES] = "/", + [VIR_CGROUP_CONTROLLER_FREEZER] = NULL, + [VIR_CGROUP_CONTROLLER_BLKIO] = "/", + }; + + if (virCgroupNewSelf(&cgroup) < 0) { + fprintf(stderr, "Cannot create cgroup for self\n"); + goto cleanup; + } + + ret = validateCgroup(cgroup, "", mountsAllInOne, linksAllInOne, placement); + +cleanup: + virCgroupFree(&cgroup); + return ret; +} + + + # define FAKESYSFSDIRTEMPLATE abs_builddir "/fakesysfsdir-XXXXXX" static int @@ -455,6 +505,11 @@ mymain(void) if (virtTestRun("New cgroup for domain partition escaped", 1, testCgroupNewForPartitionDomainEscaped, NULL) < 0) ret = -1; + setenv("VIR_CGROUP_MOCK_MODE", "allinone", 1); + if (virtTestRun("New cgroup for self (allinone)", 1, testCgroupNewForSelfAllInOne, NULL) < 0) + ret = -1; + unsetenv("VIR_CGROUP_MOCK_MODE"); + if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) virFileDeleteTree(fakesysfsdir); -- 1.8.3.1