cgroups: support cgroups mounted in multiple places (v3)

(sorry for the extra traffic.)

With this patch, lxc works for me both with all cgroups mounted with
ns cgroup on /cgroup, and with libcgroup mounting all cgroups
separately.

To do this, instead of looking for one cgroup called 'lxc' or
otherwise taking the first cgroup we find, we actually create a
container in every mounted cgroup fs.  Right now it's done under the
root of each fs.  We may want to put that under lxc, or, better yet,
make that configurable.

Changelog:
  Michael H. Warfield: Handle the case where subsystem doesn't have '.'.
  Daniel Lezcano: clean up incorrect reentrant use of mntent helpers
  v3: use the rest of Daniel's cleanups

TODO: add a configurable directory name, 'lxc' by default, under which
      all lxc cgroups are created (i.e. /sys/fs/cgroup/lxc)

Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>
Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
Tested-by: Michael H. Warfield <mhw@WittsEnd.com>

diff --git a/src/lxc/cgroup.c b/src/lxc/cgroup.c
index a068a0168b7f4c96d8f77b4ad1008b220b3bec47..c4769f97bb00c11e6eca6822d31639a661078785 100644 (file)
--- a/src/lxc/cgroup.c
+++ b/src/lxc/cgroup.c
@@ -52,90 +52,49 @@ enum {
        CGROUP_CLONE_CHILDREN,
 };
 
-static int get_cgroup_mount(const char *mtab, char *mnt)
+static int get_cgroup_mount(const char *subsystem, char *mnt)
 {
-        struct mntent *mntent;
-        FILE *file = NULL;
-        int err = -1;
+       struct mntent *mntent;
+       FILE *file = NULL;
 
-        file = setmntent(mtab, "r");
-        if (!file) {
-                SYSERROR("failed to open %s", mtab);
+       file = setmntent(MTAB, "r");
+       if (!file) {
+               SYSERROR("failed to open %s", MTAB);
                return -1;
-        }
-
-        while ((mntent = getmntent(file))) {
+       }
 
-               /* there is a cgroup mounted named "lxc" */
-               if (!strcmp(mntent->mnt_fsname, "lxc") &&
-                   !strcmp(mntent->mnt_type, "cgroup")) {
-                       strcpy(mnt, mntent->mnt_dir);
-                       err = 0;
-                       break;
-               }
+       while ((mntent = getmntent(file))) {
 
-               /* fallback to the first non-lxc cgroup found */
-                if (!strcmp(mntent->mnt_type, "cgroup") && err) {
+               if (strcmp(mntent->mnt_type, "cgroup"))
+                       continue;
+               if (!subsystem || hasmntopt(mntent, subsystem)) {
                        strcpy(mnt, mntent->mnt_dir);
-                       err = 0;
+                       fclose(file);
+                       DEBUG("using cgroup mounted at '%s'", mnt);
+                       return 0;
                }
-        };
+       };
 
-       DEBUG("using cgroup mounted at '%s'", mnt);
+       DEBUG("Failed to find cgroup for %s\n", subsystem ? subsystem : "(NULL)");
 
-        fclose(file);
+       fclose(file);
 
-        return err;
+       return -1;
 }
 
-static int get_cgroup_flags(const char *mtab, int *flags)
+static int get_cgroup_flags(struct mntent *mntent)
 {
-        struct mntent *mntent;
-        FILE *file = NULL;
-        int err = -1;
-
-        file = setmntent(mtab, "r");
-        if (!file) {
-                SYSERROR("failed to open %s", mtab);
-               return -1;
-        }
+       int flags = 0;
 
-       *flags = 0;
 
-        while ((mntent = getmntent(file))) {
+       if (hasmntopt(mntent, "ns"))
+               flags |= CGROUP_NS_CGROUP;
 
-               /* there is a cgroup mounted named "lxc" */
-               if (!strcmp(mntent->mnt_fsname, "lxc") &&
-                   !strcmp(mntent->mnt_type, "cgroup")) {
+       if (hasmntopt(mntent, "clone_children"))
+               flags |= CGROUP_CLONE_CHILDREN;
 
-                       if (hasmntopt(mntent, "ns"))
-                               *flags |= CGROUP_NS_CGROUP;
-
-                       if (hasmntopt(mntent, "clone_children"))
-                               *flags |= CGROUP_CLONE_CHILDREN;
-
-                       err = 0;
-                       break;
-               }
-
-               /* fallback to the first non-lxc cgroup found */
-                if (!strcmp(mntent->mnt_type, "cgroup") && err) {
-
-                       if (hasmntopt(mntent, "ns"))
-                               *flags |= CGROUP_NS_CGROUP;
-
-                       if (hasmntopt(mntent, "clone_children"))
-                               *flags |= CGROUP_CLONE_CHILDREN;
-
-                       err = 0;
-               }
-        };
-
-       DEBUG("cgroup flags is 0x%x", *flags);
-
-        fclose(file);
-
-        return err;
+       DEBUG("cgroup %s has flags 0x%x", mntent->mnt_dir, flags);
+       return flags;
 }
 
 static int cgroup_rename_nsgroup(const char *mnt, const char *name, pid_t pid)
@@ -199,19 +158,19 @@ static int cgroup_attach(const char *path, pid_t pid)
        return ret;
 }
 
-int lxc_cgroup_create(const char *name, pid_t pid)
+/*
+ * create a cgroup for the container in a particular subsystem.
+ * XXX TODO we will of course want to use cgroup_path{subsystem}/lxc/name,
+ * not just cgroup_path{subsystem}/name.
+ */
+static int lxc_one_cgroup_create(const char *name,
+                                struct mntent *mntent, pid_t pid)
 {
-       char cgmnt[MAXPATHLEN];
        char cgname[MAXPATHLEN];
        char clonechild[MAXPATHLEN];
        int flags;
 
-       if (get_cgroup_mount(MTAB, cgmnt)) {
-               ERROR("cgroup is not mounted");
-               return -1;
-       }
-
-       snprintf(cgname, MAXPATHLEN, "%s/%s", cgmnt, name);
+       snprintf(cgname, MAXPATHLEN, "%s/%s", mntent->mnt_dir, name);
 
        /*
         * There is a previous cgroup, assume it is empty,
@@ -222,18 +181,16 @@ int lxc_cgroup_create(const char *name, pid_t pid)
                return -1;
        }
 
-       if (get_cgroup_flags(MTAB, &flags)) {
-               SYSERROR("failed to get cgroup flags");
-               return -1;
-       }
+       flags = get_cgroup_flags(mntent);
 
        /* We have the deprecated ns_cgroup subsystem */
        if (flags & CGROUP_NS_CGROUP) {
                WARN("using deprecated ns_cgroup");
-               return cgroup_rename_nsgroup(cgmnt, cgname, pid);
+               return cgroup_rename_nsgroup(mntent->mnt_dir, cgname, pid);
        }
 
-       snprintf(clonechild, MAXPATHLEN, "%s/cgroup.clone_children", cgmnt);
+       snprintf(clonechild, MAXPATHLEN, "%s/cgroup.clone_children",
+                mntent->mnt_dir);
 
        /* we check if the kernel has clone_children, at this point if there
         * no clone_children neither ns_cgroup, that means the cgroup is mounted
@@ -263,19 +220,49 @@ int lxc_cgroup_create(const char *name, pid_t pid)
                return -1;
        }
 
+       INFO("created cgroup '%s'", cgname);
+
        return 0;
 }
 
-int lxc_cgroup_destroy(const char *name)
+/*
+ * for each mounted cgroup, create a cgroup for the container
+ */
+int lxc_cgroup_create(const char *name, pid_t pid)
 {
-       char cgmnt[MAXPATHLEN];
-       char cgname[MAXPATHLEN];
+       struct mntent *mntent;
+       FILE *file = NULL;
+       int err = -1;
 
-       if (get_cgroup_mount(MTAB, cgmnt)) {
-               ERROR("cgroup is not mounted");
+       file = setmntent(MTAB, "r");
+       if (!file) {
+               SYSERROR("failed to open %s", MTAB);
                return -1;
        }
 
+       while ((mntent = getmntent(file))) {
+
+               DEBUG("checking '%s' (%s)", mntent->mnt_dir, mntent->mnt_type);
+
+               if (!strcmp(mntent->mnt_type, "cgroup")) {
+
+                       INFO("found cgroup mounted at '%s'", mntent->mnt_dir);
+                       err = lxc_one_cgroup_create(name, mntent, pid);
+                       if (err)
+                               goto out;
+               }
+       };
+
+out:
+       endmntent(file);
+       return err;
+}
+
+
+int lxc_one_cgroup_destroy(const char *cgmnt, const char *name)
+{
+       char cgname[MAXPATHLEN];
+
        snprintf(cgname, MAXPATHLEN, "%s/%s", cgmnt, name);
        if (rmdir(cgname)) {
                SYSERROR("failed to remove cgroup '%s'", cgname);
@@ -287,37 +274,80 @@ int lxc_cgroup_destroy(const char *name)
        return 0;
 }
 
-int lxc_cgroup_path_get(char **path, const char *name)
+/*
+ * for each mounted cgroup, destroy the cgroup for the container
+ */
+int lxc_cgroup_destroy(const char *name)
 {
-       static char        cgroup[MAXPATHLEN];
-       static const char* cgroup_cached = 0;
-       static char        buf[MAXPATHLEN];
+       struct mntent *mntent;
+       FILE *file = NULL;
+       int ret, err = -1;
+
+       file = setmntent(MTAB, "r");
+       if (!file) {
+               SYSERROR("failed to open %s", MTAB);
+               return -1;
+       }
 
-       if (!cgroup_cached) {
-               if (get_cgroup_mount(MTAB, cgroup)) {
-                       ERROR("cgroup is not mounted");
-                       return -1;
-               } else {
-                       cgroup_cached = cgroup;
+       while ((mntent = getmntent(file))) {
+               if (!strcmp(mntent->mnt_type, "cgroup")) {
+                       DEBUG("destroying %s %s\n", mntent->mnt_dir, name);
+                       ret = lxc_one_cgroup_destroy(mntent->mnt_dir, name);
+                       if (ret) {
+                               fclose(file);
+                               return ret;
+                       }
+                       err = 0;
                }
        }
 
-       snprintf(buf, MAXPATHLEN, "%s/%s", cgroup_cached, name);
-       *path = buf;
+       fclose(file);
+
+       return err;
+}
+/*
+ * lxc_cgroup_path_get: put into *path the pathname for
+ * %subsystem and cgroup %name.  If %subsystem is NULL, then
+ * the first mounted cgroup will be used (for nr_tasks)
+ */
+int lxc_cgroup_path_get(char **path, const char *subsystem, const char *name)
+{
+       static char        buf[MAXPATHLEN];
+       static char        retbuf[MAXPATHLEN];
+
+       /* what lxc_cgroup_set calls subsystem is actually the filename, i.e.
+          'devices.allow'.  So for our purposee we trim it */
+       if (subsystem) {
+               snprintf(retbuf, MAXPATHLEN, "%s", subsystem);
+               char *s = index(retbuf, '.');
+               if (s)
+                       *s = '\0';
+               DEBUG("%s: called for subsys %s name %s\n", __func__, retbuf, name);
+       }
+       if (get_cgroup_mount(subsystem ? retbuf : NULL, buf)) {
+               ERROR("cgroup is not mounted");
+               return -1;
+       }
+
+       snprintf(retbuf, MAXPATHLEN, "%s/%s", buf, name);
+
+       DEBUG("%s: returning %s for subsystem %s", __func__, retbuf, subsystem);
+
+       *path = retbuf;
        return 0;
 }
 
-int lxc_cgroup_set(const char *name, const char *subsystem, const char *value)
+int lxc_cgroup_set(const char *name, const char *filename, const char *value)
 {
        int fd, ret;
-       char *nsgroup;
+       char *dirpath;
        char path[MAXPATHLEN];
 
-       ret = lxc_cgroup_path_get(&nsgroup, name);
+       ret = lxc_cgroup_path_get(&dirpath, filename, name);
        if (ret)
                return -1;
 
-        snprintf(path, MAXPATHLEN, "%s/%s", nsgroup, subsystem);
+       snprintf(path, MAXPATHLEN, "%s/%s", dirpath, filename);
 
        fd = open(path, O_WRONLY);
        if (fd < 0) {
@@ -330,25 +360,25 @@ int lxc_cgroup_set(const char *name, const char *subsystem, const char *value)
                ERROR("write %s : %s", path, strerror(errno));
                goto out;
        }
-       
+
        ret = 0;
 out:
        close(fd);
        return ret;
 }
 
-int lxc_cgroup_get(const char *name, const char *subsystem,  
+int lxc_cgroup_get(const char *name, const char *filename,
                   char *value, size_t len)
 {
        int fd, ret = -1;
-       char *nsgroup;
+       char *dirpath;
        char path[MAXPATHLEN];
 
-       ret = lxc_cgroup_path_get(&nsgroup, name);
+       ret = lxc_cgroup_path_get(&dirpath, filename, name);
        if (ret)
                return -1;
 
-        snprintf(path, MAXPATHLEN, "%s/%s", nsgroup, subsystem);
+       snprintf(path, MAXPATHLEN, "%s/%s", dirpath, filename);
 
        fd = open(path, O_RDONLY);
        if (fd < 0) {
@@ -366,16 +396,16 @@ int lxc_cgroup_get(const char *name, const char *subsystem,
 
 int lxc_cgroup_nrtasks(const char *name)
 {
-       char *nsgroup;
+       char *dpath;
        char path[MAXPATHLEN];
        int pid, ret, count = 0;
        FILE *file;
 
-       ret = lxc_cgroup_path_get(&nsgroup, name);
+       ret = lxc_cgroup_path_get(&dpath, NULL, name);
        if (ret)
                return -1;
 
-        snprintf(path, MAXPATHLEN, "%s/tasks", nsgroup);
+       snprintf(path, MAXPATHLEN, "%s/tasks", dpath);
 
        file = fopen(path, "r");
        if (!file) {

diff --git a/src/lxc/cgroup.h b/src/lxc/cgroup.h
index 8607fa8041ebed5ed7cb8cf86bf3ed160b11c3e8..f6df8c5388483d46281c6fca50ac55ca9cd38ca7 100644 (file)
--- a/src/lxc/cgroup.h
+++ b/src/lxc/cgroup.h
@@ -28,6 +28,6 @@
 struct lxc_handler;
 int lxc_cgroup_create(const char *name, pid_t pid);
 int lxc_cgroup_destroy(const char *name);
-int lxc_cgroup_path_get(char **path, const char *name);
+int lxc_cgroup_path_get(char **path, const char *subsystem, const char *name);
 int lxc_cgroup_nrtasks(const char *name);
 #endif

diff --git a/src/lxc/freezer.c b/src/lxc/freezer.c
index 07eede5c65b19f451adf704bee7415502bd909d4..94984a3ef5018f78e0439a3668764b82017c82e0 100644 (file)
--- a/src/lxc/freezer.c
+++ b/src/lxc/freezer.c
@@ -45,7 +45,7 @@ static int freeze_unfreeze(const char *name, int freeze)
        char tmpf[32];
        int fd, ret;
        
-       ret = lxc_cgroup_path_get(&nsgroup, name);
+       ret = lxc_cgroup_path_get(&nsgroup, "freezer", name);
        if (ret)
                return -1;
 

diff --git a/src/lxc/lxc.h b/src/lxc/lxc.h
index a091baaebf7c7b8bcbcc26d858a7e1fef17582c3..e6af295c2ea102bcf532b8bfe4e2add815102be8 100644 (file)
--- a/src/lxc/lxc.h
+++ b/src/lxc/lxc.h
@@ -114,22 +114,22 @@ extern lxc_state_t lxc_state(const char *name);
  * Set a specified value for a specified subsystem. The specified
  * subsystem must be fully specified, eg. "cpu.shares"
  * @name      : the name of the container
- * @subsystem : the subsystem
+ * @filename : the cgroup attribute filename
  * @value     : the value to be set
  * Returns 0 on success, < 0 otherwise
  */
-extern int lxc_cgroup_set(const char *name, const char *subsystem, const char *value);
+extern int lxc_cgroup_set(const char *name, const char *filename, const char *value);
 
 /*
  * Get a specified value for a specified subsystem. The specified
  * subsystem must be fully specified, eg. "cpu.shares"
  * @name      : the name of the container
- * @subsystem : the subsystem
+ * @filename : the cgroup attribute filename
  * @value     : the value to be set
  * @len       : the len of the value variable
  * Returns the number of bytes read, < 0 on error
  */
-extern int lxc_cgroup_get(const char *name, const char *subsystem, 
+extern int lxc_cgroup_get(const char *name, const char *filename,
                          char *value, size_t len);
 
 /*

diff --git a/src/lxc/state.c b/src/lxc/state.c
index 672001179499c25f4589012ccb613f0ca50480f9..b435eba13ff218a0dc618b45db12041c83c5ab71 100644 (file)
--- a/src/lxc/state.c
+++ b/src/lxc/state.c
@@ -71,7 +71,7 @@ static int freezer_state(const char *name)
        FILE *file;
        int err;
 
-       err = lxc_cgroup_path_get(&nsgroup, name);
+       err = lxc_cgroup_path_get(&nsgroup, "freezer", name);
        if (err)
                return -1;