[lxc-devel] [lxc/master] cgfsng: bugfixes + improvements for lxc.cgroup.dir
brauner on Github
lxc-bot at linuxcontainers.org
Fri Aug 25 09:58:44 UTC 2017
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 364 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20170825/6080c323/attachment.bin>
-------------- next part --------------
From 6b89f639d46ad52dfbdd6a7bda27e1bcdff5fa53 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Fri, 25 Aug 2017 09:52:14 +0200
Subject: [PATCH 1/3] cgfsng: non-functional changes
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/cgroups/cgfsng.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c
index fc658faf2..e137b33c0 100644
--- a/src/lxc/cgroups/cgfsng.c
+++ b/src/lxc/cgroups/cgfsng.c
@@ -1199,6 +1199,7 @@ static void *cgfsng_init(struct lxc_handler *handler)
static int cgroup_rmdir(char *dirname)
{
+ int ret;
struct dirent *direntp;
DIR *dir;
int r = 0;
@@ -1208,8 +1209,8 @@ static int cgroup_rmdir(char *dirname)
return -1;
while ((direntp = readdir(dir))) {
- struct stat mystat;
char *pathname;
+ struct stat mystat;
if (!direntp)
break;
@@ -1220,32 +1221,40 @@ static int cgroup_rmdir(char *dirname)
pathname = must_make_path(dirname, direntp->d_name, NULL);
- if (lstat(pathname, &mystat)) {
+ ret = lstat(pathname, &mystat);
+ if (ret < 0) {
if (!r)
- WARN("failed to stat %s", pathname);
+ WARN("Failed to stat %s", pathname);
r = -1;
goto next;
}
if (!S_ISDIR(mystat.st_mode))
goto next;
- if (cgroup_rmdir(pathname) < 0)
+
+ ret = cgroup_rmdir(pathname);
+ if (ret < 0)
r = -1;
next:
free(pathname);
}
- if (rmdir(dirname) < 0) {
+ ret = rmdir(dirname);
+ if (ret < 0) {
if (!r)
- WARN("failed to delete %s: %s", dirname, strerror(errno));
+ WARN("Failed to delete \"%s\": %s", dirname,
+ strerror(errno));
r = -1;
}
- if (closedir(dir) < 0) {
+ ret = closedir(dir);
+ if (ret < 0) {
if (!r)
- WARN("failed to delete %s: %s", dirname, strerror(errno));
+ WARN("Failed to delete \"%s\": %s", dirname,
+ strerror(errno));
r = -1;
}
+
return r;
}
From 631fc3b8075eac258a000de6327f089a96169054 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Fri, 25 Aug 2017 11:51:05 +0200
Subject: [PATCH 2/3] cgfsng: add container name to lxc.cgroup.dir value
Say we have
lxc.uts.name = c1
lxc.cgroup.dir = lxd
the actual path should be
lxd/c1
Right now it would just be
lxd
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/cgroups/cgfsng.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c
index e137b33c0..fe3fd7062 100644
--- a/src/lxc/cgroups/cgfsng.c
+++ b/src/lxc/cgroups/cgfsng.c
@@ -1345,11 +1345,11 @@ static void remove_path_for_hierarchy(struct hierarchy *h, char *cgname)
*/
static inline bool cgfsng_create(void *hdata)
{
- struct cgfsng_handler_data *d = hdata;
- char *tmp, *cgname, *offset;
int i;
- int idx = 0;
size_t len;
+ char *cgname, *offset, *tmp;
+ int idx = 0;
+ struct cgfsng_handler_data *d = hdata;
if (!d)
return false;
@@ -1360,7 +1360,7 @@ static inline bool cgfsng_create(void *hdata)
}
if (d->cgroup_meta.dir)
- tmp = strdup(d->cgroup_meta.dir);
+ tmp = lxc_string_join("/", (const char *[]){d->cgroup_meta.dir, d->name, NULL}, false);
else
tmp = lxc_string_replace("%n", d->name, d->cgroup_pattern);
if (!tmp) {
From 0c5945aaadee5833cae5e927e5eba0cd5e17f5a9 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Fri, 25 Aug 2017 11:53:55 +0200
Subject: [PATCH 3/3] 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 at ubuntu.com>
---
src/lxc/cgroups/cgfsng.c | 71 ++++++++++++++++++++++++++++++++++++++++++++----
src/lxc/confile.c | 3 --
2 files changed, 65 insertions(+), 9 deletions(-)
diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c
index fe3fd7062..8c7c8afbf 100644
--- a/src/lxc/cgroups/cgfsng.c
+++ b/src/lxc/cgroups/cgfsng.c
@@ -1272,16 +1272,18 @@ 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)
@@ -1293,13 +1295,70 @@ static void cgfsng_destroy(void *hdata, struct lxc_conf *conf)
if (d->container_cgroup && hierarchies) {
int i;
+ char *parent;
+ size_t recurse_upwards = 0;
+
+ if (d->cgroup_meta.dir)
+ parent = d->cgroup_meta.dir;
+ else
+ parent = d->cgroup_pattern;
+
+ /* skip slashes at beginning */
+ while (*parent == '/')
+ parent++;
+
+ /* Count the number of parent cgroups. */
+ while ((parent = strchr(parent, '/'))) {
+ parent++;
+
+ /* skip extraneous slashes */
+ while (*parent == '/')
+ parent++;
+
+ recurse_upwards++;
+ }
+
for (i = 0; hierarchies[i]; i++) {
+ int j, ret;
struct hierarchy *h = hierarchies[i];
- if (h->fullcgpath) {
- recursive_destroy(h->fullcgpath, conf);
- free(h->fullcgpath);
- h->fullcgpath = NULL;
+
+ if (!h->fullcgpath)
+ continue;
+
+ /* Delete the container's cgroup */
+ ret = recursive_destroy(h->fullcgpath, conf);
+ if (ret < 0)
+ 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 = h->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(h->fullcgpath, conf);
+ if (ret < 0)
+ break;
+
+ if (s == h->fullcgpath)
+ break;
+ s--;
+ while (*s == '/')
+ s--;
}
+
+next:
+ free(h->fullcgpath);
+ h->fullcgpath = NULL;
}
}
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);
}
More information about the lxc-devel
mailing list