Fix cgroups when all are mounted on /sys/fs/cgroup

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@redhat.com>

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;

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,10 +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")) {
+            if (allinone)
+                return fmemopen((void *)procmountsallinone,
+                                strlen(procmountsallinone), mode);
+            else
                 return fmemopen((void *)procmounts, strlen(procmounts), mode);
         } else {
             errno = EACCES;
@@ -390,6 +421,10 @@ FILE *fopen(const char *path, const char *mode)
     }
     if (STREQ(path, "/proc/cgroups")) {
         if (STREQ(mode, "r")) {
+            if (allinone)
+                return fmemopen((void *)proccgroupsallinone,
+                                strlen(proccgroupsallinone), mode);
+            else
                 return fmemopen((void *)proccgroups, strlen(proccgroups), mode);
         } else {
             errno = EACCES;
@@ -398,6 +433,10 @@ FILE *fopen(const char *path, const char *mode)
     }
     if (STREQ(path, "/proc/self/cgroup")) {
         if (STREQ(mode, "r")) {
+            if (allinone)
+                return fmemopen((void *)procselfcgroupsallinone,
+                                strlen(procselfcgroupsallinone), mode);
+            else
                 return fmemopen((void *)procselfcgroups, strlen(procselfcgroups), mode);
         } else {
             errno = EACCES;

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