From: Christian Brauner <christian.brauner@ubuntu.com>
Date: Fri, 25 Aug 2017 09:53:55 +0000 (+0200)
Subject: cgfsng: try to delete parent cgroups
X-Git-Tag: lxc-2.1.0~14^2~13
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=92c590ae1ea40bc094603ab49c20b785cc88bb1d;p=thirdparty%2Flxc.git

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 fe3fd7062..390923eca 100644
--- 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 62337289e..e66bae314 100644
--- 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);
 }