Ensure all cgroup partitions have a suffix of ".partition"

If the partition named passed in the XML does not already have
a suffix, ensure it gets a '.partition' added to each component.
The exceptions are /machine, /user and /system which do not need
to have a suffix, since they are fixed partitions at the top
level.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 0084aead74e4f75a585ed996a7c243f523bb5163..297408dc72f38c1e95f082df0faf12edaf2cdc9b 100644 (file)
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -49,6 +49,7 @@
 #include "virfile.h"
 #include "virhash.h"
 #include "virhashcode.h"
+#include "virstring.h"
 
 #define CGROUP_MAX_VAL 512
 
@@ -1091,6 +1092,47 @@ cleanup:
 
 
 #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R
+static char *virCgroupSetPartitionSuffix(const char *path)
+{
+    char **tokens = virStringSplit(path, "/", 0);
+    size_t i;
+    char *ret = NULL;
+
+    if (!tokens)
+        return NULL;
+
+    for (i = 0 ; tokens[i] != NULL ; i++) {
+        /* Whitelist the 3 top level fixed dirs
+         * NB i == 0 is "", since we have leading '/'
+         */
+        if (i == 1 &&
+            (STREQ(tokens[i], "machine") ||
+             STREQ(tokens[i], "system") ||
+             STREQ(tokens[i], "user"))) {
+            continue;
+        }
+        /* If there is no suffix set already, then
+         * add ".partition"
+         */
+        if (STRNEQ(tokens[i], "") &&
+            !strchr(tokens[i], '.')) {
+            if (VIR_REALLOC_N(tokens[i],
+                              strlen(tokens[i]) + strlen(".partition") + 1) < 0) {
+                virReportOOMError();
+                goto cleanup;
+            }
+            strcat(tokens[i], ".partition");
+        }
+    }
+
+    if (!(ret = virStringJoin((const char **)tokens, "/")))
+        goto cleanup;
+
+cleanup:
+    virStringFreeList(tokens);
+    return ret;
+}
+
 /**
  * virCgroupNewPartition:
  * @path: path for the partition
@@ -1110,19 +1152,28 @@ int virCgroupNewPartition(const char *path,
     int rc;
     char *parentPath = NULL;
     virCgroupPtr parent = NULL;
+    char *newpath;
     VIR_DEBUG("path=%s create=%d controllers=%x",
               path, create, controllers);
 
     if (path[0] != '/')
         return -EINVAL;
 
-    rc = virCgroupNew(path, NULL, controllers, group);
+    /* XXX convert all cgroups APIs to use error report
+     * APIs instead of returning errno */
+    if (!(newpath = virCgroupSetPartitionSuffix(path))) {
+        virResetLastError();
+        rc = -ENOMEM;
+        goto cleanup;
+    }
+
+    rc = virCgroupNew(newpath, NULL, controllers, group);
     if (rc != 0)
         goto cleanup;
 
-    if (STRNEQ(path, "/")) {
+    if (STRNEQ(newpath, "/")) {
         char *tmp;
-        if (!(parentPath = strdup(path))) {
+        if (!(parentPath = strdup(newpath))) {
             rc = -ENOMEM;
             goto cleanup;
         }

diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c
index 9c2590f2012c406a668b5381b34fea68d2cbc10b..b51106aaccfe923c540f39f836a058379ce3c029 100644 (file)
--- a/tests/vircgrouptest.c
+++ b/tests/vircgrouptest.c
@@ -246,22 +246,22 @@ static int testCgroupNewForPartition(const void *args ATTRIBUTE_UNUSED)
     int ret = -1;
     int rv;
     const char *placementSmall[VIR_CGROUP_CONTROLLER_LAST] = {
-        [VIR_CGROUP_CONTROLLER_CPU] = "/virtualmachines",
-        [VIR_CGROUP_CONTROLLER_CPUACCT] = "/virtualmachines",
+        [VIR_CGROUP_CONTROLLER_CPU] = "/virtualmachines.partition",
+        [VIR_CGROUP_CONTROLLER_CPUACCT] = "/virtualmachines.partition",
         [VIR_CGROUP_CONTROLLER_CPUSET] = NULL,
-        [VIR_CGROUP_CONTROLLER_MEMORY] = "/virtualmachines",
+        [VIR_CGROUP_CONTROLLER_MEMORY] = "/virtualmachines.partition",
         [VIR_CGROUP_CONTROLLER_DEVICES] = NULL,
         [VIR_CGROUP_CONTROLLER_FREEZER] = NULL,
         [VIR_CGROUP_CONTROLLER_BLKIO] = NULL,
     };
     const char *placementFull[VIR_CGROUP_CONTROLLER_LAST] = {
-        [VIR_CGROUP_CONTROLLER_CPU] = "/virtualmachines",
-        [VIR_CGROUP_CONTROLLER_CPUACCT] = "/virtualmachines",
-        [VIR_CGROUP_CONTROLLER_CPUSET] = "/virtualmachines",
-        [VIR_CGROUP_CONTROLLER_MEMORY] = "/virtualmachines",
+        [VIR_CGROUP_CONTROLLER_CPU] = "/virtualmachines.partition",
+        [VIR_CGROUP_CONTROLLER_CPUACCT] = "/virtualmachines.partition",
+        [VIR_CGROUP_CONTROLLER_CPUSET] = "/virtualmachines.partition",
+        [VIR_CGROUP_CONTROLLER_MEMORY] = "/virtualmachines.partition",
         [VIR_CGROUP_CONTROLLER_DEVICES] = NULL,
-        [VIR_CGROUP_CONTROLLER_FREEZER] = "/virtualmachines",
-        [VIR_CGROUP_CONTROLLER_BLKIO] = "/virtualmachines",
+        [VIR_CGROUP_CONTROLLER_FREEZER] = "/virtualmachines.partition",
+        [VIR_CGROUP_CONTROLLER_BLKIO] = "/virtualmachines.partition",
     };
 
     if ((rv = virCgroupNewPartition("/virtualmachines", false, -1, &cgroup)) != -ENOENT) {
@@ -294,14 +294,14 @@ static int testCgroupNewForPartition(const void *args ATTRIBUTE_UNUSED)
         fprintf(stderr, "Cannot create /virtualmachines cgroup: %d\n", -rv);
         goto cleanup;
     }
-    ret = validateCgroup(cgroup, "/virtualmachines", mountsSmall, links, placementSmall);
+    ret = validateCgroup(cgroup, "/virtualmachines.partition", mountsSmall, links, placementSmall);
     virCgroupFree(&cgroup);
 
     if ((rv = virCgroupNewPartition("/virtualmachines", true, -1, &cgroup)) != 0) {
         fprintf(stderr, "Cannot create /virtualmachines cgroup: %d\n", -rv);
         goto cleanup;
     }
-    ret = validateCgroup(cgroup, "/virtualmachines", mountsFull, links, placementFull);
+    ret = validateCgroup(cgroup, "/virtualmachines.partition", mountsFull, links, placementFull);
 
 cleanup:
     virCgroupFree(&cgroup);
@@ -315,38 +315,90 @@ static int testCgroupNewForPartitionNested(const void *args ATTRIBUTE_UNUSED)
     int ret = -1;
     int rv;
     const char *placementFull[VIR_CGROUP_CONTROLLER_LAST] = {
-        [VIR_CGROUP_CONTROLLER_CPU] = "/users/berrange",
-        [VIR_CGROUP_CONTROLLER_CPUACCT] = "/users/berrange",
-        [VIR_CGROUP_CONTROLLER_CPUSET] = "/users/berrange",
-        [VIR_CGROUP_CONTROLLER_MEMORY] = "/users/berrange",
+        [VIR_CGROUP_CONTROLLER_CPU] = "/deployment.partition/production.partition",
+        [VIR_CGROUP_CONTROLLER_CPUACCT] = "/deployment.partition/production.partition",
+        [VIR_CGROUP_CONTROLLER_CPUSET] = "/deployment.partition/production.partition",
+        [VIR_CGROUP_CONTROLLER_MEMORY] = "/deployment.partition/production.partition",
         [VIR_CGROUP_CONTROLLER_DEVICES] = NULL,
-        [VIR_CGROUP_CONTROLLER_FREEZER] = "/users/berrange",
-        [VIR_CGROUP_CONTROLLER_BLKIO] = "/users/berrange",
+        [VIR_CGROUP_CONTROLLER_FREEZER] = "/deployment.partition/production.partition",
+        [VIR_CGROUP_CONTROLLER_BLKIO] = "/deployment.partition/production.partition",
     };
 
-    if ((rv = virCgroupNewPartition("/users/berrange", false, -1, &cgroup)) != -ENOENT) {
-        fprintf(stderr, "Unexpected found /users/berrange cgroup: %d\n", -rv);
+    if ((rv = virCgroupNewPartition("/deployment/production", false, -1, &cgroup)) != -ENOENT) {
+        fprintf(stderr, "Unexpected found /deployment/production cgroup: %d\n", -rv);
         goto cleanup;
     }
 
-    /* Should not work, since we require /users to be pre-created */
-    if ((rv = virCgroupNewPartition("/users/berrange", true, -1, &cgroup)) != -ENOENT) {
-        fprintf(stderr, "Unexpected created /users/berrange cgroup: %d\n", -rv);
+    /* Should not work, since we require /deployment to be pre-created */
+    if ((rv = virCgroupNewPartition("/deployment/production", true, -1, &cgroup)) != -ENOENT) {
+        fprintf(stderr, "Unexpected created /deployment/production cgroup: %d\n", -rv);
         goto cleanup;
     }
 
-    if ((rv = virCgroupNewPartition("/users", true, -1, &cgroup)) != 0) {
-        fprintf(stderr, "Failed to create /users cgroup: %d\n", -rv);
+    if ((rv = virCgroupNewPartition("/deployment", true, -1, &cgroup)) != 0) {
+        fprintf(stderr, "Failed to create /deployment cgroup: %d\n", -rv);
         goto cleanup;
     }
 
     /* Should now work */
-    if ((rv = virCgroupNewPartition("/users/berrange", true, -1, &cgroup)) != 0) {
-        fprintf(stderr, "Failed to create /users/berrange cgroup: %d\n", -rv);
+    if ((rv = virCgroupNewPartition("/deployment/production", true, -1, &cgroup)) != 0) {
+        fprintf(stderr, "Failed to create /deployment/production cgroup: %d\n", -rv);
         goto cleanup;
     }
 
-    ret = validateCgroup(cgroup, "/users/berrange", mountsFull, links, placementFull);
+    ret = validateCgroup(cgroup, "/deployment.partition/production.partition",
+                         mountsFull, links, placementFull);
+
+cleanup:
+    virCgroupFree(&cgroup);
+    return ret;
+}
+
+
+static int testCgroupNewForPartitionNestedDeep(const void *args ATTRIBUTE_UNUSED)
+{
+    virCgroupPtr cgroup = NULL;
+    int ret = -1;
+    int rv;
+    const char *placementFull[VIR_CGROUP_CONTROLLER_LAST] = {
+        [VIR_CGROUP_CONTROLLER_CPU] = "/user/berrange.user/production.partition",
+        [VIR_CGROUP_CONTROLLER_CPUACCT] = "/user/berrange.user/production.partition",
+        [VIR_CGROUP_CONTROLLER_CPUSET] = "/user/berrange.user/production.partition",
+        [VIR_CGROUP_CONTROLLER_MEMORY] = "/user/berrange.user/production.partition",
+        [VIR_CGROUP_CONTROLLER_DEVICES] = NULL,
+        [VIR_CGROUP_CONTROLLER_FREEZER] = "/user/berrange.user/production.partition",
+        [VIR_CGROUP_CONTROLLER_BLKIO] = "/user/berrange.user/production.partition",
+    };
+
+    if ((rv = virCgroupNewPartition("/user/berrange.user/production", false, -1, &cgroup)) != -ENOENT) {
+        fprintf(stderr, "Unexpected found /user/berrange.user/production cgroup: %d\n", -rv);
+        goto cleanup;
+    }
+
+    /* Should not work, since we require /user/berrange.user to be pre-created */
+    if ((rv = virCgroupNewPartition("/user/berrange.user/production", true, -1, &cgroup)) != -ENOENT) {
+        fprintf(stderr, "Unexpected created /user/berrange.user/production cgroup: %d\n", -rv);
+        goto cleanup;
+    }
+
+    if ((rv = virCgroupNewPartition("/user", true, -1, &cgroup)) != 0) {
+        fprintf(stderr, "Failed to create /user/berrange.user cgroup: %d\n", -rv);
+        goto cleanup;
+    }
+
+    if ((rv = virCgroupNewPartition("/user/berrange.user", true, -1, &cgroup)) != 0) {
+        fprintf(stderr, "Failed to create /user/berrange.user cgroup: %d\n", -rv);
+        goto cleanup;
+    }
+
+    /* Should now work */
+    if ((rv = virCgroupNewPartition("/user/berrange.user/production", true, -1, &cgroup)) != 0) {
+        fprintf(stderr, "Failed to create /user/berrange.user/production cgroup: %d\n", -rv);
+        goto cleanup;
+    }
+
+    ret = validateCgroup(cgroup, "/user/berrange.user/production.partition",
+                         mountsFull, links, placementFull);
 
 cleanup:
     virCgroupFree(&cgroup);
@@ -362,13 +414,13 @@ static int testCgroupNewForPartitionDomain(const void *args ATTRIBUTE_UNUSED)
     int ret = -1;
     int rv;
     const char *placement[VIR_CGROUP_CONTROLLER_LAST] = {
-        [VIR_CGROUP_CONTROLLER_CPU] = "/production/foo.libvirt-lxc",
-        [VIR_CGROUP_CONTROLLER_CPUACCT] = "/production/foo.libvirt-lxc",
-        [VIR_CGROUP_CONTROLLER_CPUSET] = "/production/foo.libvirt-lxc",
-        [VIR_CGROUP_CONTROLLER_MEMORY] = "/production/foo.libvirt-lxc",
+        [VIR_CGROUP_CONTROLLER_CPU] = "/production.partition/foo.libvirt-lxc",
+        [VIR_CGROUP_CONTROLLER_CPUACCT] = "/production.partition/foo.libvirt-lxc",
+        [VIR_CGROUP_CONTROLLER_CPUSET] = "/production.partition/foo.libvirt-lxc",
+        [VIR_CGROUP_CONTROLLER_MEMORY] = "/production.partition/foo.libvirt-lxc",
         [VIR_CGROUP_CONTROLLER_DEVICES] = NULL,
-        [VIR_CGROUP_CONTROLLER_FREEZER] = "/production/foo.libvirt-lxc",
-        [VIR_CGROUP_CONTROLLER_BLKIO] = "/production/foo.libvirt-lxc",
+        [VIR_CGROUP_CONTROLLER_FREEZER] = "/production.partition/foo.libvirt-lxc",
+        [VIR_CGROUP_CONTROLLER_BLKIO] = "/production.partition/foo.libvirt-lxc",
     };
 
     if ((rv = virCgroupNewPartition("/production", true, -1, &partitioncgroup)) != 0) {
@@ -381,7 +433,7 @@ static int testCgroupNewForPartitionDomain(const void *args ATTRIBUTE_UNUSED)
         goto cleanup;
     }
 
-    ret = validateCgroup(domaincgroup, "/production/foo.libvirt-lxc", mountsFull, links, placement);
+    ret = validateCgroup(domaincgroup, "/production.partition/foo.libvirt-lxc", mountsFull, links, placement);
 
 cleanup:
     virCgroupFree(&partitioncgroup);
@@ -424,6 +476,9 @@ mymain(void)
     if (virtTestRun("New cgroup for partition nested", 1, testCgroupNewForPartitionNested, NULL) < 0)
         ret = -1;
 
+    if (virtTestRun("New cgroup for partition nested deeply", 1, testCgroupNewForPartitionNestedDeep, NULL) < 0)
+        ret = -1;
+
     if (virtTestRun("New cgroup for domain partition", 1, testCgroupNewForPartitionDomain, NULL) < 0)
         ret = -1;