[lxc-devel] [lxc/master] cgfsng: defer to cgfs if needed subsystems are not available

hallyn on Github lxc-bot at linuxcontainers.org
Fri Apr 8 21:32:41 UTC 2016


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 669 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20160408/5d807f50/attachment.bin>
-------------- next part --------------
From 457ca9aa85c3682c2746f593090df2e1e6664d34 Mon Sep 17 00:00:00 2001
From: Serge Hallyn <serge.hallyn at ubuntu.com>
Date: Fri, 8 Apr 2016 16:18:04 -0500
Subject: [PATCH] cgfsng: defer to cgfs if needed subsystems are not available

This requires us to check that at cgfsng_ops_init, rather than
cgfs_init.  Cache the hierarchy and cgroup.use info globally
rather than putting it into the per-container info, as cgmanager
does.  This is ok as both cgroup.use and the list of usable
hierarchies are in fact global to a lxc run.

Closes #952

Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
---
 src/lxc/cgfsng.c | 196 ++++++++++++++++++++++---------------------------------
 1 file changed, 78 insertions(+), 118 deletions(-)

diff --git a/src/lxc/cgfsng.c b/src/lxc/cgfsng.c
index cf75319..fad0be4 100644
--- a/src/lxc/cgfsng.c
+++ b/src/lxc/cgfsng.c
@@ -72,10 +72,6 @@ struct hierarchy {
 
 /*
  * The cgroup data which is attached to the lxc_handler.
- * @hierarchies - a NULL-terminated array of struct hierarchy, one per
- *   hierarchy.  No duplicates.  First sufficient, writeable mounted
- *   hierarchy wins
- * @cgroup_use - a copy of the lxc.cgroup.use
  * @cgroup_pattern - a copy of the lxc.cgroup.pattern
  * @container_cgroup - if not null, the cgroup which was created for
  *   the container.  For each hierarchy, it is created under the
@@ -84,13 +80,23 @@ struct hierarchy {
  * @name - the container name
  */
 struct cgfsng_handler_data {
-	struct hierarchy **hierarchies;
-	char *cgroup_use;
 	char *cgroup_pattern;
 	char *container_cgroup; // cgroup we created for the container
 	char *name; // container name
 };
 
+/*
+ * @hierarchies - a NULL-terminated array of struct hierarchy, one per
+ *   hierarchy.  No duplicates.  First sufficient, writeable mounted
+ *   hierarchy wins
+ */
+struct hierarchy **hierarchies;
+
+/*
+ * @cgroup_use - a copy of the lxc.cgroup.use
+ */
+char *cgroup_use;
+
 static void free_string_list(char **clist)
 {
 	if (clist) {
@@ -218,25 +224,8 @@ static void must_append_controller(char **klist, char **nlist, char ***clist, ch
 	(*clist)[newentry] = copy;
 }
 
-static void free_hierarchies(struct hierarchy **hlist)
-{
-	if (hlist) {
-		int i;
-
-		for (i = 0; hlist[i]; i++) {
-			free(hlist[i]->mountpoint);
-			free(hlist[i]->base_cgroup);
-			free(hlist[i]->fullcgpath);
-			free_string_list(hlist[i]->controllers);
-		}
-		free(hlist);
-	}
-}
-
 static void free_handler_data(struct cgfsng_handler_data *d)
 {
-	free_hierarchies(d->hierarchies);
-	free(d->cgroup_use);
 	free(d->cgroup_pattern);
 	free(d->container_cgroup);
 	free(d->name);
@@ -247,15 +236,15 @@ static void free_handler_data(struct cgfsng_handler_data *d)
  * Given a handler's cgroup data, return the struct hierarchy for the
  * controller @c, or NULL if there is none.
  */
-struct hierarchy *get_hierarchy(struct cgfsng_handler_data *d, const char *c)
+struct hierarchy *get_hierarchy(const char *c)
 {
 	int i;
 
-	if (!d || !d->hierarchies)
+	if (!hierarchies)
 		return NULL;
-	for (i = 0; d->hierarchies[i]; i++) {
-		if (string_in_list(d->hierarchies[i]->controllers, c))
-			return d->hierarchies[i];
+	for (i = 0; hierarchies[i]; i++) {
+		if (string_in_list(hierarchies[i]->controllers, c))
+			return hierarchies[i];
 	}
 	return NULL;
 }
@@ -422,10 +411,10 @@ static bool controller_found(struct hierarchy **hlist, char *entry)
  * found.  The required list is systemd, freezer, and anything in
  * lxc.cgroup.use.
  */
-static bool all_controllers_found(struct cgfsng_handler_data *d)
+static bool all_controllers_found(void)
 {
 	char *p, *saveptr = NULL;
-	struct hierarchy ** hlist = d->hierarchies;
+	struct hierarchy ** hlist = hierarchies;
 
 	if (!controller_found(hlist, "name=systemd")) {
 		ERROR("no systemd controller mountpoint found");
@@ -436,9 +425,9 @@ static bool all_controllers_found(struct cgfsng_handler_data *d)
 		return false;
 	}
 
-	if (!d->cgroup_use)
+	if (!cgroup_use)
 		return true;
-	for (p = strtok_r(d->cgroup_use, ",", &saveptr); p;
+	for (p = strtok_r(cgroup_use, ",", &saveptr); p;
 			p = strtok_r(NULL, ",", &saveptr)) {
 		if (!controller_found(hlist, p)) {
 			ERROR("no %s controller mountpoint found", p);
@@ -508,8 +497,7 @@ static bool is_cgroupfs(char *line)
 }
 
 /* Add a controller to our list of hierarchies */
-static void add_controller(struct cgfsng_handler_data *d, char **clist,
-			   char *mountpoint, char *base_cgroup)
+static void add_controller(char **clist, char *mountpoint, char *base_cgroup)
 {
 	struct hierarchy *new;
 	int newentry;
@@ -520,8 +508,8 @@ static void add_controller(struct cgfsng_handler_data *d, char **clist,
 	new->base_cgroup = base_cgroup;
 	new->fullcgpath = NULL;
 
-	newentry = append_null_to_list((void ***)&d->hierarchies);
-	d->hierarchies[newentry] = new;
+	newentry = append_null_to_list((void ***)&hierarchies);
+	hierarchies[newentry] = new;
 }
 
 /*
@@ -732,16 +720,16 @@ static void print_init_debuginfo(struct cgfsng_handler_data *d)
 
 	printf("Cgroup information:\n");
 	printf("  container name: %s\n", d->name);
-	printf("  lxc.cgroup.use: %s\n", d->cgroup_use ? d->cgroup_use : "(none)");
+	printf("  lxc.cgroup.use: %s\n", cgroup_use ? cgroup_use : "(none)");
 	printf("  lxc.cgroup.pattern: %s\n", d->cgroup_pattern);
 	printf("  cgroup: %s\n", d->container_cgroup ? d->container_cgroup : "(none)");
-	if (!d->hierarchies) {
+	if (!hierarchies) {
 		printf("  No hierarchies found.\n");
 		return;
 	}
 	printf("  Hierarchies:\n");
-	for (i = 0; d->hierarchies[i]; i++) {
-		struct hierarchy *h = d->hierarchies[i];
+	for (i = 0; hierarchies[i]; i++) {
+		struct hierarchy *h = hierarchies[i];
 		int j;
 		printf("  %d: base_cgroup %s\n", i, h->base_cgroup);
 		printf("      mountpoint %s\n", h->mountpoint);
@@ -769,7 +757,7 @@ static void print_basecg_debuginfo(char *basecginfo, char **klist, char **nlist)
  * At startup, parse_hierarchies finds all the info we need about
  * cgroup mountpoints and current cgroups, and stores it in @d.
  */
-static bool parse_hierarchies(struct cgfsng_handler_data *d)
+static bool parse_hierarchies(void)
 {
 	FILE *f;
 	char * line = NULL, *basecginfo;
@@ -808,7 +796,7 @@ static bool parse_hierarchies(struct cgfsng_handler_data *d)
 		if (!controller_list)
 			continue;
 
-		if (controller_list_is_dup(d->hierarchies, controller_list)) {
+		if (controller_list_is_dup(hierarchies, controller_list)) {
 			free(controller_list);
 			continue;
 		}
@@ -835,7 +823,7 @@ static bool parse_hierarchies(struct cgfsng_handler_data *d)
 			free(base_cgroup);
 			continue;
 		}
-		add_controller(d, controller_list, mountpoint, base_cgroup);
+		add_controller(controller_list, mountpoint, base_cgroup);
 	}
 
 	free_string_list(klist);
@@ -846,35 +834,39 @@ static bool parse_hierarchies(struct cgfsng_handler_data *d)
 	fclose(f);
 	free(line);
 
-	print_init_debuginfo(d);
-
 	/* verify that all controllers in cgroup.use and all crucial
 	 * controllers are accounted for
 	 */
-	if (!all_controllers_found(d))
+	if (!all_controllers_found())
 		return false;
 
 	return true;
 }
 
+static bool collect_hierarchy_info(void)
+{
+	const char *tmp;
+	errno = 0;
+	tmp = lxc_global_config_value("lxc.cgroup.use");
+	if (!cgroup_use && errno != 0) { // lxc.cgroup.use can be NULL
+		SYSERROR("cgfsng: error reading list of cgroups to use");
+		return false;
+	}
+	cgroup_use = must_copy_string(tmp);
+
+	return parse_hierarchies();
+}
+
 static void *cgfsng_init(const char *name)
 {
 	struct cgfsng_handler_data *d;
-	const char *cgroup_use, *cgroup_pattern;
+	const char *cgroup_pattern;
 
 	d = must_alloc(sizeof(*d));
 	memset(d, 0, sizeof(*d));
 
 	d->name = must_copy_string(name);
 
-	errno = 0;
-	cgroup_use = lxc_global_config_value("lxc.cgroup.use");
-	if (!cgroup_use && errno != 0) { // lxc.cgroup.use can be NULL
-		SYSERROR("Error reading list of cgroups to use");
-		goto out_free;
-	}
-	d->cgroup_use = must_copy_string(cgroup_use);
-
 	cgroup_pattern = lxc_global_config_value("lxc.cgroup.pattern");
 	if (!cgroup_pattern) { // lxc.cgroup.pattern is only NULL on error
 		ERROR("Error getting cgroup pattern");
@@ -882,9 +874,6 @@ static void *cgfsng_init(const char *name)
 	}
 	d->cgroup_pattern = must_copy_string(cgroup_pattern);
 
-	if (!parse_hierarchies(d))
-		goto out_free;
-
 	print_init_debuginfo(d);
 
 	return d;
@@ -1006,10 +995,10 @@ static void cgfsng_destroy(void *hdata, struct lxc_conf *conf)
 	if (!d)
 		return;
 
-	if (d->container_cgroup && d->hierarchies) {
+	if (d->container_cgroup && hierarchies) {
 		int i;
-		for (i = 0; d->hierarchies[i]; i++) {
-			struct hierarchy *h = d->hierarchies[i];
+		for (i = 0; hierarchies[i]; i++) {
+			struct hierarchy *h = hierarchies[i];
 			if (h->fullcgpath) {
 				recursive_destroy(h->fullcgpath, conf);
 				free(h->fullcgpath);
@@ -1023,6 +1012,8 @@ static void cgfsng_destroy(void *hdata, struct lxc_conf *conf)
 
 struct cgroup_ops *cgfsng_ops_init(void)
 {
+	if (!collect_hierarchy_info())
+		return NULL;
 	return &cgfsng_ops;
 }
 
@@ -1080,14 +1071,14 @@ static inline bool cgfsng_create(void *hdata)
 	}
 	if (idx)
 		snprintf(offset, 5, "-%d", idx);
-	for (i = 0; d->hierarchies[i]; i++) {
-		if (!create_path_for_hierarchy(d->hierarchies[i], cgname)) {
+	for (i = 0; hierarchies[i]; i++) {
+		if (!create_path_for_hierarchy(hierarchies[i], cgname)) {
 			int j;
-			SYSERROR("Failed to create %s: %s", d->hierarchies[i]->fullcgpath, strerror(errno));
-			free(d->hierarchies[i]->fullcgpath);
-			d->hierarchies[i]->fullcgpath = NULL;
+			SYSERROR("Failed to create %s: %s", hierarchies[i]->fullcgpath, strerror(errno));
+			free(hierarchies[i]->fullcgpath);
+			hierarchies[i]->fullcgpath = NULL;
 			for (j = 0; j < i; j++)
-				remove_path_for_hierarchy(d->hierarchies[j], cgname);
+				remove_path_for_hierarchy(hierarchies[j], cgname);
 			idx++;
 			goto again;
 		}
@@ -1110,7 +1101,6 @@ static const char *cgfsng_canonical_path(void *hdata)
 
 static bool cgfsng_enter(void *hdata, pid_t pid)
 {
-	struct cgfsng_handler_data *d = hdata;
 	char pidstr[25];
 	int i, len;
 
@@ -1118,8 +1108,8 @@ static bool cgfsng_enter(void *hdata, pid_t pid)
 	if (len < 0 || len > 25)
 		return false;
 
-	for (i = 0; d->hierarchies[i]; i++) {
-		char *fullpath = must_make_path(d->hierarchies[i]->fullcgpath,
+	for (i = 0; hierarchies[i]; i++) {
+		char *fullpath = must_make_path(hierarchies[i]->fullcgpath,
 						"cgroup.procs", NULL);
 		if (lxc_write_to_file(fullpath, pidstr, len, false) != 0) {
 			SYSERROR("Failed to enter %s", fullpath);
@@ -1148,7 +1138,6 @@ struct chown_data {
 static int chown_cgroup_wrapper(void *data)
 {
 	struct chown_data *arg = data;
-	struct cgfsng_handler_data *d = arg->d;
 	uid_t destuid;
 	int i;
 
@@ -1161,8 +1150,8 @@ static int chown_cgroup_wrapper(void *data)
 
 	destuid = get_ns_uid(arg->origuid);
 
-	for (i = 0; d->hierarchies[i]; i++) {
-		char *fullpath, *path = d->hierarchies[i]->fullcgpath;
+	for (i = 0; hierarchies[i]; i++) {
+		char *fullpath, *path = hierarchies[i]->fullcgpath;
 
 		if (chown(path, destuid, 0) < 0) {
 			SYSERROR("Error chowning %s to %d", path, (int) destuid);
@@ -1334,9 +1323,9 @@ static bool cgfsng_mount(void *hdata, const char *root, int type)
 			root) < 0)
 		goto  bad;
 
-	for (i = 0; d->hierarchies[i]; i++) {
+	for (i = 0; hierarchies[i]; i++) {
 		char *controllerpath, *path2;
-		struct hierarchy *h = d->hierarchies[i];
+		struct hierarchy *h = hierarchies[i];
 		char *controller = strrchr(h->mountpoint, '/');
 		int r;
 
@@ -1431,9 +1420,9 @@ static int cgfsng_nrtasks(void *hdata) {
 	char *path;
 	int count;
 
-	if (!d || !d->container_cgroup || !d->hierarchies)
+	if (!d || !d->container_cgroup || !hierarchies)
 		return -1;
-	path = must_make_path(d->hierarchies[0]->fullcgpath, NULL);
+	path = must_make_path(hierarchies[0]->fullcgpath, NULL);
 	count = recursive_count_nrtasks(path);
 	free(path);
 	return count;
@@ -1455,9 +1444,9 @@ static bool cgfsng_escape()
 		return false;
 	}
 
-	for (i = 0; d->hierarchies[i]; i++) {
-		char *fullpath = must_make_path(d->hierarchies[i]->mountpoint,
-						d->hierarchies[i]->base_cgroup,
+	for (i = 0; hierarchies[i]; i++) {
+		char *fullpath = must_make_path(hierarchies[i]->mountpoint,
+						hierarchies[i]->base_cgroup,
 						"cgroup.procs", NULL);
 		if (lxc_write_to_file(fullpath, "0", 2, false) != 0) {
 			SYSERROR("Failed to escape to %s", fullpath);
@@ -1478,11 +1467,10 @@ static bool cgfsng_escape()
 
 static bool cgfsng_unfreeze(void *hdata)
 {
-	struct cgfsng_handler_data *d = hdata;
 	char *fullpath;
-	struct hierarchy *h = get_hierarchy(d, "freezer");
+	struct hierarchy *h = get_hierarchy("freezer");
 
-	if (!d || !h)
+	if (!h)
 		return false;
 	fullpath = must_make_path(h->fullcgpath, "freezer.state", NULL);
 	if (lxc_write_to_file(fullpath, THAWED, THAWED_LEN, false) != 0) {
@@ -1495,12 +1483,7 @@ static bool cgfsng_unfreeze(void *hdata)
 
 static const char *cgfsng_get_cgroup(void *hdata, const char *subsystem)
 {
-	struct cgfsng_handler_data *d = hdata;
-	struct hierarchy *h;
-	if (!d)
-		return NULL;
-
-	h = get_hierarchy(d, subsystem);
+	struct hierarchy *h = get_hierarchy(subsystem);
 	if (!h)
 		return NULL;
 
@@ -1527,7 +1510,6 @@ static char *build_full_cgpath_from_monitorpath(struct hierarchy *h,
 
 static bool cgfsng_attach(const char *name, const char *lxcpath, pid_t pid)
 {
-	struct cgfsng_handler_data *d;
 	char pidstr[25];
 	int i, len;
 
@@ -1535,13 +1517,9 @@ static bool cgfsng_attach(const char *name, const char *lxcpath, pid_t pid)
 	if (len < 0 || len > 25)
 		return false;
 
-	d = cgfsng_init(name);
-	if (!d)
-		return false;
-
-	for (i = 0; d->hierarchies[i]; i++) {
+	for (i = 0; hierarchies[i]; i++) {
 		char *path, *fullpath;
-		struct hierarchy *h = d->hierarchies[i];
+		struct hierarchy *h = hierarchies[i];
 
 		path = lxc_cmd_get_cgroup_path(name, lxcpath, h->controllers[0]);
 		if (!path) // not running
@@ -1552,13 +1530,11 @@ static bool cgfsng_attach(const char *name, const char *lxcpath, pid_t pid)
 		if (lxc_write_to_file(fullpath, pidstr, len, false) != 0) {
 			SYSERROR("Failed to attach %d to %s", (int)pid, fullpath);
 			free(fullpath);
-			free_handler_data(d);
 			return false;
 		}
 		free(fullpath);
 	}
 
-	free_handler_data(d);
 	return true;
 }
 
@@ -1570,7 +1546,6 @@ static bool cgfsng_attach(const char *name, const char *lxcpath, pid_t pid)
 static int cgfsng_get(const char *filename, char *value, size_t len, const char *name, const char *lxcpath)
 {
 	char *subsystem, *p, *path;
-	struct cgfsng_handler_data *d;
 	struct hierarchy *h;
 	int ret = -1;
 
@@ -1583,20 +1558,13 @@ static int cgfsng_get(const char *filename, char *value, size_t len, const char
 	if (!path) // not running
 		return -1;
 
-	d = cgfsng_init(name);
-	if (!d) {
-		free(path);
-		return false;
-	}
-
-	h = get_hierarchy(d, subsystem);
+	h = get_hierarchy(subsystem);
 	if (h) {
 		char *fullpath = build_full_cgpath_from_monitorpath(h, path, filename);
 		ret = lxc_read_from_file(fullpath, value, len);
 		free(fullpath);
 	}
 
-	free_handler_data(d);
 	free(path);
 
 	return ret;
@@ -1610,7 +1578,6 @@ static int cgfsng_get(const char *filename, char *value, size_t len, const char
 static int cgfsng_set(const char *filename, const char *value, const char *name, const char *lxcpath)
 {
 	char *subsystem, *p, *path;
-	struct cgfsng_handler_data *d;
 	struct hierarchy *h;
 	int ret = -1;
 
@@ -1623,20 +1590,13 @@ static int cgfsng_set(const char *filename, const char *value, const char *name,
 	if (!path) // not running
 		return -1;
 
-	d = cgfsng_init(name);
-	if (!d) {
-		free(path);
-		return false;
-	}
-
-	h = get_hierarchy(d, subsystem);
+	h = get_hierarchy(subsystem);
 	if (h) {
 		char *fullpath = build_full_cgpath_from_monitorpath(h, path, filename);
 		ret = lxc_write_to_file(fullpath, value, strlen(value), false);
 		free(fullpath);
 	}
 
-	free_handler_data(d);
 	free(path);
 
 	return ret;
@@ -1657,7 +1617,7 @@ static int lxc_cgroup_set_data(const char *filename, const char *value, struct c
 	if ((p = strchr(subsystem, '.')) != NULL)
 		*p = '\0';
 
-	h = get_hierarchy(d, subsystem);
+	h = get_hierarchy(subsystem);
 	if (h) {
 		char *fullpath = must_make_path(h->fullcgpath, filename, NULL);
 		ret = lxc_write_to_file(fullpath, value, strlen(value), false);
@@ -1685,7 +1645,7 @@ static bool cgfsng_setup_limits(void *hdata, struct lxc_list *cgroup_settings,
 	}
 
 	if (do_devices) {
-		h = get_hierarchy(d, "devices");
+		h = get_hierarchy("devices");
 		if (!h) {
 			ERROR("No devices cgroup setup for %s", d->name);
 			return false;


More information about the lxc-devel mailing list