cgfsng: try to delete parent cgroups

Say we have

    lxc.uts.name = c1
    lxc.cgroup.dir = lxd/a/b/c

the path for the container's cgroup would be

    lxd/a/b/c/c1

When the container is shutdown we should not just try to delete "c1" we should
also try to delete "c", "b", "a", and "lxd". This is to ensure that we don't
leave empty cgroups around thereby increasing the chance that we run into
trouble with cgroup limits. The algorithm for this isn't too costly since we
can simply stop walking upwards at the first rmdir() failure.

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>

diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c
index fe3fd7062a442fe72e3166011aad6f1dd5595481..390923eca3235f0a4af1846054acff89dbbb9655 100644 (file)
--- a/src/lxc/cgroups/cgfsng.c
+++ b/src/lxc/cgroups/cgfsng.c
@@ -1272,35 +1272,91 @@ static int rmdir_wrapper(void *data)
        return cgroup_rmdir(path);
 }
 
-void recursive_destroy(char *path, struct lxc_conf *conf)
+int recursive_destroy(char *path, struct lxc_conf *conf)
 {
        int r;
+
        if (conf && !lxc_list_empty(&conf->id_map))
                r = userns_exec_1(conf, rmdir_wrapper, path, "rmdir_wrapper");
        else
                r = cgroup_rmdir(path);
-
        if (r < 0)
                ERROR("Error destroying %s", path);
+
+       return r;
 }
 
 static void cgfsng_destroy(void *hdata, struct lxc_conf *conf)
 {
+       int i;
+       char *clean_parent, *clean_fullcgpath;
+       char **fields;
+       size_t recurse_upwards = 0;
        struct cgfsng_handler_data *d = hdata;
 
        if (!d)
                return;
 
-       if (d->container_cgroup && hierarchies) {
-               int i;
-               for (i = 0; hierarchies[i]; i++) {
-                       struct hierarchy *h = hierarchies[i];
-                       if (h->fullcgpath) {
-                               recursive_destroy(h->fullcgpath, conf);
-                               free(h->fullcgpath);
-                               h->fullcgpath = NULL;
-                       }
+       if (!d->container_cgroup || !hierarchies)
+               return;
+
+       if (d->cgroup_meta.dir)
+               clean_parent = d->cgroup_meta.dir;
+       else
+               clean_parent = d->cgroup_pattern;
+       fields = lxc_normalize_path(clean_parent);
+       if (fields) {
+               recurse_upwards = lxc_array_len((void **)fields);
+               if (recurse_upwards > 0 && clean_parent == d->cgroup_pattern)
+                       recurse_upwards--;
+               lxc_free_array((void **)fields, free);
+       }
+
+       for (i = 0; hierarchies[i]; i++) {
+               int ret;
+               size_t j;
+               struct hierarchy *h = hierarchies[i];
+
+               if (!h->fullcgpath)
+                       continue;
+
+               clean_fullcgpath = lxc_deslashify(h->fullcgpath);
+               if (!clean_fullcgpath)
+                       clean_fullcgpath = h->fullcgpath;
+
+               /* Delete the container's cgroup */
+               ret = recursive_destroy(clean_fullcgpath, conf);
+               if (ret < 0)
+                       goto next;
+
+               if (h->fullcgpath == clean_fullcgpath)
+                       goto next;
+
+               /* Delete parent cgroups as specified in the containers config
+                * file. This takes care of not having useless empty cgroups
+                * around.
+                */
+               for (j = 0; j < recurse_upwards; j++) {
+                       char *s = clean_fullcgpath;
+
+                       s = strrchr(s, '/');
+                       if (!s)
+                               break;
+                       *s = '\0';
+
+                       /* If we fail to delete a cgroup we know that any parent
+                        * cgroup also cannot be removed.
+                        */
+                       ret = recursive_destroy(clean_fullcgpath, conf);
+                       if (ret < 0)
+                               break;
                }
+
+next:
+               if (h->fullcgpath != clean_fullcgpath)
+                       free(clean_fullcgpath);
+               free(h->fullcgpath);
+               h->fullcgpath = NULL;
        }
 
        free_handler_data(d);

diff --git a/src/lxc/confile.c b/src/lxc/confile.c
index 62337289e9b4a84e3aa117d507a270895e2ef673..e66bae314ed886089ac083f61d722a34c6c3d88c 100644 (file)
--- a/src/lxc/confile.c
+++ b/src/lxc/confile.c
@@ -1431,9 +1431,6 @@ static int set_config_cgroup_dir(const char *key, const char *value,
        if (lxc_config_value_empty(value))
                return clr_config_cgroup_dir(key, lxc_conf, NULL);
 
-       if (lxc_conf->cgroup_meta.dir)
-               clr_config_cgroup_dir(key, lxc_conf, NULL);
-
        return set_config_string_item(&lxc_conf->cgroup_meta.dir, value);
 }