[1/2] cg: Add ability to dump custom cgroup properties

Submitted by Cyrill Gorcunov on April 27, 2016, 7:17 p.m.

Details

Message ID 1461784669-18635-2-git-send-email-gorcunov@openvz.org
State Rejected
Series "Custom cgroup props, v5"
Headers show

Commit Message

Cyrill Gorcunov April 27, 2016, 7:17 p.m.
We have some common predefined properties such as "cpuset.cpus" and etc gathered
in @cgp_predefined set, but there might be situation when only predefined ones
are not enough, so add ability to specify properties via --cgroup-props
and/or --cgroup-props-file options.

For example one may pass

  --cgroup-props "\"cpu\": [\"cpu.shares\", \"cpu.cfs_period_us\"]"

to dump custom properties for cpu controller.

The description is implemented in almost valid yaml, probably we will
need to support the various forms, but oneline is enough for now.

Depending if --cgroup-props-ignore-default option passed the
new properties either merged with predefined or substitute
them.

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
---
 criu/Makefile.crtools       |   1 +
 criu/cgroup-props.c         | 472 ++++++++++++++++++++++++++++++++++++++++++++
 criu/cgroup.c               | 102 +---------
 criu/cr-dump.c              |   8 +
 criu/crtools.c              |  23 +++
 criu/include/cgroup-props.h |  19 ++
 criu/include/cr_options.h   |   3 +
 criu/proc_parse.c           |  13 ++
 8 files changed, 547 insertions(+), 94 deletions(-)
 create mode 100644 criu/cgroup-props.c
 create mode 100644 criu/include/cgroup-props.h

Patch hide | download patch | download mbox

diff --git a/criu/Makefile.crtools b/criu/Makefile.crtools
index 1e452f4e16b1..657b08e9f0c9 100644
--- a/criu/Makefile.crtools
+++ b/criu/Makefile.crtools
@@ -5,6 +5,7 @@  obj-y			+= aio.o
 obj-y			+= bfd.o
 obj-y			+= bitmap.o
 obj-y			+= cgroup.o
+obj-y			+= cgroup-props.o
 obj-y			+= cr-check.o
 obj-y			+= cr-dedup.o
 obj-y			+= cr-dump.o
diff --git a/criu/cgroup-props.c b/criu/cgroup-props.c
new file mode 100644
index 000000000000..166509b5fef6
--- /dev/null
+++ b/criu/cgroup-props.c
@@ -0,0 +1,472 @@ 
+#include <stdio.h>
+#include <string.h>
+#include <ctype.h>
+#include <unistd.h>
+#include <fcntl.h>
+
+#include <sys/mman.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+
+#include "compiler.h"
+#include "cgroup-props.h"
+#include "cr_options.h"
+#include "config.h"
+#include "xmalloc.h"
+#include "string.h"
+#include "util.h"
+#include "list.h"
+#include "log.h"
+#include "bug.h"
+
+#undef	LOG_PREFIX
+#define LOG_PREFIX "cg-prop: "
+
+/*
+ * Predefined properties.
+ */
+static const char *cpu_props[] = {
+	"cpu.shares",
+	"cpu.cfs_period_us",
+	"cpu.cfs_quota_us",
+	"cpu.rt_period_us",
+	"cpu.rt_runtime_us",
+	"notify_on_release",
+};
+
+static const char *memory_props[] = {
+	/* limit_in_bytes and memsw.limit_in_bytes must be set in this order */
+	"memory.limit_in_bytes",
+	"memory.memsw.limit_in_bytes",
+	"memory.use_hierarchy",
+	"notify_on_release",
+};
+
+static const char *cpuset_props[] = {
+	/*
+	 * cpuset.cpus and cpuset.mems must be set before the process moves
+	 * into its cgroup; they are "initialized" below to whatever the root
+	 * values are in copy_special_cg_props so as not to cause ENOSPC when
+	 * values are restored via this code.
+	 */
+	"cpuset.cpus",
+	"cpuset.mems",
+	"cpuset.memory_migrate",
+	"cpuset.cpu_exclusive",
+	"cpuset.mem_exclusive",
+	"cpuset.mem_hardwall",
+	"cpuset.memory_spread_page",
+	"cpuset.memory_spread_slab",
+	"cpuset.sched_load_balance",
+	"cpuset.sched_relax_domain_level",
+	"notify_on_release",
+};
+
+static const char *blkio_props[] = {
+	"blkio.weight",
+	"notify_on_release",
+};
+
+static const char *freezer_props[] = {
+	"notify_on_release",
+};
+
+static const char *____criu_global_props____[] = {
+	"cgroup.clone_children",
+	"notify_on_release",
+	"cgroup.procs",
+	"tasks",
+};
+
+cgp_t cgp_global = {
+	.name		= "____criu_global_props____",
+	.nr_props	= ARRAY_SIZE(____criu_global_props____),
+	.props		= ____criu_global_props____,
+};
+
+typedef struct {
+	struct list_head	list;
+	cgp_t			cgp;
+} cgp_list_entry_t;
+
+static cgp_list_entry_t cgp_predefined[5];
+
+static struct list_head cgp_predefined_list = {
+	.next = &cgp_predefined[0].list,
+	.prev = &cgp_predefined[4].list,
+};
+
+static cgp_list_entry_t cgp_predefined[5] = {
+	{
+		.list.next	= &cgp_predefined[1].list,
+		.list.prev	= &cgp_predefined_list,
+		.cgp		= {
+			.name		= "cpu",
+			.nr_props	= ARRAY_SIZE(cpu_props),
+			.props		= cpu_props,
+		},
+	}, {
+		.list.next	= &cgp_predefined[2].list,
+		.list.prev	= &cgp_predefined[1].list,
+		.cgp		= {
+			.name		= "memory",
+			.nr_props	= ARRAY_SIZE(memory_props),
+			.props		= memory_props,
+		},
+	}, {
+		.list.next	= &cgp_predefined[3].list,
+		.list.prev	= &cgp_predefined[2].list,
+		.cgp		= {
+			.name		= "cpuset",
+			.nr_props	= ARRAY_SIZE(cpuset_props),
+			.props		= cpuset_props,
+		},
+	}, {
+		.list.next	= &cgp_predefined[4].list,
+		.list.prev	= &cgp_predefined[3].list,
+		.cgp		= {
+			.name		= "blkio",
+			.nr_props	= ARRAY_SIZE(blkio_props),
+			.props		= blkio_props,
+		},
+	}, {
+		.list.next	= &cgp_predefined_list,
+		.list.prev	= &cgp_predefined[4].list,
+		.cgp		= {
+			.name		= "freezer",
+			.nr_props	= ARRAY_SIZE(freezer_props),
+			.props		= freezer_props,
+		},
+	},
+};
+
+static LIST_HEAD(cgp_list);
+
+static void cgp_free(cgp_list_entry_t *p)
+{
+	size_t i;
+
+	if (p) {
+		for (i = 0; i < p->cgp.nr_props; i++)
+			xfree((void *)p->cgp.props[i]);
+		xfree((void *)p->cgp.name);
+		xfree((void *)p->cgp.props);
+		xfree(p);
+	}
+}
+
+static cgp_list_entry_t *cgp_get_predefined(const char *name)
+{
+	cgp_list_entry_t *p;
+
+	list_for_each_entry(p, &cgp_predefined_list, list) {
+		if (!strcmp(p->cgp.name, name))
+			return p;
+	}
+
+	return NULL;
+}
+
+static int cgp_subst_props(cgp_list_entry_t *t)
+{
+	cgp_list_entry_t *p, *n;
+
+	/* FIXME This is O^2, optimize! */
+	list_for_each_entry_safe(p, n, &cgp_predefined_list, list) {
+		if (strcmp(t->cgp.name, p->cgp.name))
+			continue;
+		pr_debug("Substitute \'%s\' controller props\n", t->cgp.name);
+		list_del(&p->list);
+		break;
+	}
+
+	return 0;
+}
+
+static int cgp_merge_props(cgp_list_entry_t *t)
+{
+	cgp_list_entry_t *p, *n;
+	size_t nr_props, i, j;
+
+	/* FIXME This is O^2, optimize! */
+	list_for_each_entry_safe(p, n, &cgp_predefined_list, list) {
+		if (strcmp(t->cgp.name, p->cgp.name))
+			continue;
+
+		pr_debug("Merging \'%s\' controller props\n", t->cgp.name);
+
+		nr_props = t->cgp.nr_props + p->cgp.nr_props;
+
+		if (xrealloc_safe(&t->cgp.props, nr_props * sizeof(char *)))
+			return -ENOMEM;
+
+		for (i = t->cgp.nr_props, j = 0; i < nr_props; i++, j++) {
+			t->cgp.props[i] = xstrdup(p->cgp.props[j]);
+			if (!t->cgp.props[i])
+				return -ENOMEM;
+			t->cgp.nr_props++;
+		}
+
+		list_del(&p->list);
+		break;
+	}
+
+	return 0;
+}
+
+static char *skip_spaces(char **stream, size_t *len)
+{
+	if (stream && *len) {
+		char *p = *stream;
+
+		while (p && *len && *p == ' ')
+			p++, (*len)--;
+		if (p != *stream)
+			*stream = p;
+		return p;
+	}
+
+	return NULL;
+}
+
+static char *eat_symbol(char **stream, size_t *len, char sym, bool skip_ws)
+{
+	char *p = skip_ws ? skip_spaces(stream, len) : (stream ? *stream : NULL);
+
+	if (!p || *p != sym || !*len)
+		return NULL;
+	(*stream) = p + 1;
+	(*len)--;
+	return p;
+}
+
+static char *get_quoted(char **stream, size_t *len, bool skip_ws)
+{
+	char *p = skip_ws ? skip_spaces(stream, len) : (stream ? *stream : NULL);
+	char *from = p + 1;
+	char *dst;
+
+	if (!p || *p != '\"')
+		return NULL;
+
+	for (p = from, (*len)--; (*len); p++, (*len)--) {
+		if (*p == '\"') {
+			if (p == from)
+				break;
+			dst = xmalloc(p - from + 1);
+			if (!dst)
+				break;
+
+			memcpy(dst, from, p - from);
+			dst[p - from] = '\0';
+
+			(*stream) = p + 1;
+			(*len)--;
+			return dst;
+		}
+	}
+
+	return NULL;
+}
+
+static int cgp_parse_stream(char *stream, size_t len)
+{
+	cgp_list_entry_t *cgp_entry;
+	int ret = 0;
+	char *p;
+
+	/*
+	 * We expect the following format here
+	 * (very simplified YAML!)
+	 *
+	 *  "cpu": ["cpu.shares", "cpu.cfs_period_us"]
+	 *  "memory": ["memory.limit_in_bytes", "memory.memsw.limit_in_bytes"]
+	 *
+	 *  and etc.
+	 */
+
+	while (len) {
+		/*
+		 * Controller name.
+		 */
+		p = get_quoted(&stream, &len, false);
+		if (!p) {
+			pr_err("Expected \'\"\' symbol for controller name\n");
+			goto err_parse;
+		}
+
+		pr_debug("Parsing controller %s\n", p);
+
+		cgp_entry = xzalloc(sizeof(*cgp_entry));
+		if (cgp_entry) {
+			INIT_LIST_HEAD(&cgp_entry->list);
+			cgp_entry->cgp.name = p;
+		} else {
+			pr_err("Can't allocate memory for controller %s\n", p);
+			xfree(p);
+			return -ENOMEM;
+		}
+
+		p = eat_symbol(&stream, &len, ':', true);
+		if (!p) {
+			pr_err("Expected \':\' symbol in controller's %s stream\n",
+			       cgp_entry->cgp.name);
+			goto err_parse;
+		}
+
+		p = eat_symbol(&stream, &len, '[', true);
+		if (!p) {
+			pr_err("Expected \'[\' symbol in controller's %s stream\n",
+			       cgp_entry->cgp.name);
+			goto err_parse;
+		}
+
+		while ((p = get_quoted(&stream, &len, true))) {
+			if (!p) {
+				pr_err("Expected property name for controller %s\n",
+				       cgp_entry->cgp.name);
+				goto err_parse;
+			}
+
+			if (xrealloc_safe(&cgp_entry->cgp.props,
+					  (cgp_entry->cgp.nr_props + 1) * sizeof(char *))) {
+				pr_err("Can't allocate property for controller %s\n",
+				       cgp_entry->cgp.name);
+				return -1;
+			}
+
+			cgp_entry->cgp.props[cgp_entry->cgp.nr_props++] = p;
+			pr_debug("\tProperty %s\n", p);
+
+			p = eat_symbol(&stream, &len, ',', true);
+			if (!p) {
+				if (stream[0] == ']') {
+					stream++, len--;
+					break;
+				}
+				pr_err("Expected ']' in controller's %s stream\n",
+				       cgp_entry->cgp.name);
+				goto err_parse;
+			}
+		}
+
+		p = eat_symbol(&stream, &len, '\n', true);
+		if (!p && len) {
+			pr_err("Expected \'\\n\' symbol in controller's %s stream\n",
+			       cgp_entry->cgp.name);
+			goto err_parse;
+		}
+
+		list_add(&cgp_entry->list, &cgp_list);
+
+		if (!opts.cgroup_props_ignore_default) {
+			if (cgp_merge_props(cgp_entry)) {
+				pr_err("Can't merge controller \'%s\'\n",
+				       cgp_entry->cgp.name);
+					goto out;
+			}
+		} else {
+			if (cgp_subst_props(cgp_entry)) {
+				pr_err("Can't substitute controller \'%s\'\n",
+				       cgp_entry->cgp.name);
+					goto out;
+			}
+		}
+
+		cgp_entry = NULL;
+	}
+
+	ret = 0;
+out:
+	return ret;
+
+err_parse:
+	cgp_free(cgp_entry);
+	ret = -EINVAL;
+	goto out;
+}
+
+static int cgp_parse_file(char *path)
+{
+	void *mem = MAP_FAILED;
+	int fd = -1, ret = -1;
+	struct stat st;
+
+	fd = open(path, O_RDONLY);
+	if (fd < 0) {
+		pr_perror("Can't open file %s\n", path);
+		goto err;
+	}
+
+	if (fstat(fd, &st)) {
+		pr_perror("Can't stat file %s\n", path);
+		goto err;
+	}
+
+	mem = mmap(NULL, st.st_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_FILE, fd, 0);
+	if (mem == MAP_FAILED) {
+		pr_perror("Can't mmap file %s\n", path);
+		goto err;
+	}
+
+	if (cgp_parse_stream(mem, st.st_size)) {
+		pr_err("Failed to parse file `%s'\n", path);
+		goto err;
+	}
+
+	ret = 0;
+err:
+	if (mem != MAP_FAILED)
+		munmap(mem, st.st_size);
+	close_safe(&fd);
+	return ret;
+}
+
+int cgp_init(char *stream, size_t len, char *path)
+{
+	int ret = 0;
+
+	if (stream && len) {
+		ret = cgp_parse_stream(stream, len);
+		if (ret)
+			goto err;
+	}
+
+	if (path)
+		ret = cgp_parse_file(path);
+err:
+	return ret;
+}
+
+bool cgp_should_skip_controller(const char *name)
+{
+	/*
+	 * FIXME Implement!
+	 */
+	return false;
+}
+
+const cgp_t *cgp_get_props(const char *name)
+{
+	cgp_list_entry_t *p;
+
+	p = cgp_get_predefined(name);
+	if (p)
+		return &p->cgp;
+
+	list_for_each_entry(p, &cgp_list, list) {
+		if (!strcmp(p->cgp.name, name))
+			return &p->cgp;
+	}
+
+	return NULL;
+}
+
+void cgp_fini(void)
+{
+	cgp_list_entry_t *p, *t;
+
+	list_for_each_entry_safe(p, t, &cgp_list, list)
+		cgp_free(p);
+	INIT_LIST_HEAD(&cgp_list);
+}
diff --git a/criu/cgroup.c b/criu/cgroup.c
index a706ff6f76ce..1bf52851ea17 100644
--- a/criu/cgroup.c
+++ b/criu/cgroup.c
@@ -10,6 +10,7 @@ 
 #include "list.h"
 #include "xmalloc.h"
 #include "cgroup.h"
+#include "cgroup-props.h"
 #include "cr_options.h"
 #include "pstree.h"
 #include "proc_parse.h"
@@ -25,73 +26,6 @@ 
 #include "images/cgroup.pb-c.h"
 
 /*
- * These string arrays have the names of all the properties that will be
- * restored. To add a property for a cgroup type, add it to the
- * corresponding char array above the NULL terminator. If you are adding
- * a new cgroup family all together, you must also edit get_known_properties()
- * Currently the code only supports properties with 1 value
- */
-
-static const char *cpu_props[] = {
-	"cpu.shares",
-	"cpu.cfs_period_us",
-	"cpu.cfs_quota_us",
-	"cpu.rt_period_us",
-	"cpu.rt_runtime_us",
-	"notify_on_release",
-	NULL
-};
-
-static const char *memory_props[] = {
-	/* limit_in_bytes and memsw.limit_in_bytes must be set in this order */
-	"memory.limit_in_bytes",
-	"memory.memsw.limit_in_bytes",
-	"memory.use_hierarchy",
-	"notify_on_release",
-	NULL
-};
-
-static const char *cpuset_props[] = {
-	/*
-	 * cpuset.cpus and cpuset.mems must be set before the process moves
-	 * into its cgroup; they are "initialized" below to whatever the root
-	 * values are in copy_special_cg_props so as not to cause ENOSPC when
-	 * values are restored via this code.
-	 */
-	"cpuset.cpus",
-	"cpuset.mems",
-	"cpuset.memory_migrate",
-	"cpuset.cpu_exclusive",
-	"cpuset.mem_exclusive",
-	"cpuset.mem_hardwall",
-	"cpuset.memory_spread_page",
-	"cpuset.memory_spread_slab",
-	"cpuset.sched_load_balance",
-	"cpuset.sched_relax_domain_level",
-	"notify_on_release",
-	NULL
-};
-
-static const char *blkio_props[] = {
-	"blkio.weight",
-	"notify_on_release",
-	NULL
-};
-
-static const char *freezer_props[] = {
-	"notify_on_release",
-	NULL
-};
-
-static const char *global_props[] = {
-	"cgroup.clone_children",
-	"notify_on_release",
-	"cgroup.procs",
-	"tasks",
-	NULL
-};
-
-/*
  * This structure describes set of controller groups
  * a task lives in. The cg_ctl entries are stored in
  * the @ctls list sorted by the .name field and then
@@ -421,33 +355,14 @@  static void free_all_cgroup_props(struct cgroup_dir *ncd)
 	ncd->n_properties = 0;
 }
 
-static const char **get_known_properties(char *controller)
-{
-	const char **prop_arr = NULL;
-
-	if (!strcmp(controller, "cpu"))
-		prop_arr = cpu_props;
-	else if (!strcmp(controller, "memory"))
-		prop_arr = memory_props;
-	else if (!strcmp(controller, "cpuset"))
-		prop_arr = cpuset_props;
-	else if (!strcmp(controller, "blkio"))
-		prop_arr = blkio_props;
-	else if (!strcmp(controller, "freezer"))
-		prop_arr = freezer_props;
-
-	return prop_arr;
-}
-
-static int dump_cg_props_array(const char *fpath, struct cgroup_dir *ncd,
-			       const char **prop_arr)
+static int dump_cg_props_array(const char *fpath, struct cgroup_dir *ncd, const cgp_t *cgp)
 {
 	int j;
 	char buf[PATH_MAX];
 	struct cgroup_prop *prop;
 
-	for (j = 0; prop_arr != NULL && prop_arr[j] != NULL; ++j) {
-		if (snprintf(buf, PATH_MAX, "%s/%s", fpath, prop_arr[j]) >= PATH_MAX) {
+	for (j = 0; cgp && j < cgp->nr_props; j++) {
+		if (snprintf(buf, PATH_MAX, "%s/%s", fpath, cgp->props[j]) >= PATH_MAX) {
 			pr_err("snprintf output was truncated\n");
 			return -1;
 		}
@@ -457,7 +372,7 @@  static int dump_cg_props_array(const char *fpath, struct cgroup_dir *ncd,
 			continue;
 		}
 
-		prop = create_cgroup_prop(prop_arr[j]);
+		prop = create_cgroup_prop(cgp->props[j]);
 		if (!prop) {
 			free_all_cgroup_props(ncd);
 			return -1;
@@ -483,15 +398,14 @@  static int add_cgroup_properties(const char *fpath, struct cgroup_dir *ncd,
 	int i;
 
 	for (i = 0; i < controller->n_controllers; ++i) {
+		const cgp_t *cgp = cgp_get_props(controller->controllers[i]);
 
-		const char **prop_arr = get_known_properties(controller->controllers[i]);
-
-		if (dump_cg_props_array(fpath, ncd, prop_arr) < 0) {
+		if (dump_cg_props_array(fpath, ncd, cgp) < 0) {
 			pr_err("dumping known properties failed");
 			return -1;
 		}
 
-		if (dump_cg_props_array(fpath, ncd, global_props) < 0) {
+		if (dump_cg_props_array(fpath, ncd, &cgp_global) < 0) {
 			pr_err("dumping global properties failed");
 			return -1;
 		}
diff --git a/criu/cr-dump.c b/criu/cr-dump.c
index 5ac9fd041e4e..b5531e43d115 100644
--- a/criu/cr-dump.c
+++ b/criu/cr-dump.c
@@ -61,6 +61,7 @@ 
 #include "cpu.h"
 #include "elf.h"
 #include "cgroup.h"
+#include "cgroup-props.h"
 #include "file-lock.h"
 #include "page-xfer.h"
 #include "kerndat.h"
@@ -1554,6 +1555,7 @@  static int cr_dump_finish(int ret)
 		ret = -1;
 
 	cr_plugin_fini(CR_PLUGIN_STAGE__DUMP, ret);
+	cgp_fini();
 
 	if (!ret) {
 		/*
@@ -1653,6 +1655,12 @@  int cr_dump_tasks(pid_t pid)
 	if (vdso_init())
 		goto err;
 
+	if (cgp_init(opts.cgroup_props,
+		     opts.cgroup_props ?
+		     strlen(opts.cgroup_props) : 0,
+		     opts.cgroup_props_file))
+		goto err;
+
 	if (parse_cg_info())
 		goto err;
 
diff --git a/criu/crtools.c b/criu/crtools.c
index 7a0f9778db74..eb6f68b398b2 100644
--- a/criu/crtools.c
+++ b/criu/crtools.c
@@ -38,6 +38,7 @@ 
 #include "mount.h"
 #include "namespaces.h"
 #include "cgroup.h"
+#include "cgroup-props.h"
 #include "cpu.h"
 #include "action-scripts.h"
 #include "irmap.h"
@@ -320,6 +321,9 @@  int main(int argc, char *argv[], char *envp[])
 		{ "extra",			no_argument,		0, 1077	},
 		{ "experimental",		no_argument,		0, 1078	},
 		{ "all",			no_argument,		0, 1079	},
+		{ "cgroup-props",		required_argument,	0, 1080	},
+		{ "cgroup-props-file",		required_argument,	0, 1081	},
+		{ "cgroup-props-ignore-default",no_argument,		0, 1082	},
 		{ },
 	};
 
@@ -622,6 +626,15 @@  int main(int argc, char *argv[], char *envp[])
 			opts.check_extra_features = true;
 			opts.check_experimental_features = true;
 			break;
+		case 1080:
+			opts.cgroup_props = optarg;
+			break;
+		case 1081:
+			opts.cgroup_props_file = optarg;
+			break;
+		case 1082:
+			opts.cgroup_props_ignore_default = true;
+			break;
 		case 'V':
 			pr_msg("Version: %s\n", CRIU_VERSION);
 			if (strcmp(CRIU_GITID, "0"))
@@ -873,6 +886,16 @@  usage:
 "                        change the root cgroup the controller will be\n"
 "                        installed into. No controller means that root is the\n"
 "                        default for all controllers not specified.\n"
+"  --cgroup-props STRING\n"
+"                        define cgroup controllers and properties\n"
+"                        to be checkpointed, which are described\n"
+"                        via STRING using simplified YAML format.\n"
+"  --cgroup-props-file FILE\n"
+"                        same as --cgroup-props but taking descrition\n"
+"                        from the path specified.\n"
+"  --cgroup-props-ignore-default\n"
+"                        when parse custom properties do not merged them with\n"
+"                        default ones but simply substitute appropriate controller.\n"
 "  --skip-mnt PATH       ignore this mountpoint when dumping the mount namespace.\n"
 "  --enable-fs FSNAMES   a comma separated list of filesystem names or \"all\".\n"
 "                        force criu to (try to) dump/restore these filesystem's\n"
diff --git a/criu/include/cgroup-props.h b/criu/include/cgroup-props.h
new file mode 100644
index 000000000000..e6c96bae94f8
--- /dev/null
+++ b/criu/include/cgroup-props.h
@@ -0,0 +1,19 @@ 
+#ifndef __CR_CGROUP_PROPS_H__
+#define __CR_CGROUP_PROPS_H__
+
+#include <stdbool.h>
+
+typedef struct {
+	const char	*name;
+	size_t		nr_props;
+	const char	**props;
+} cgp_t;
+
+extern cgp_t cgp_global;
+extern const cgp_t *cgp_get_props(const char *name);
+extern bool cgp_should_skip_controller(const char *name);
+
+extern int cgp_init(char *stream, size_t len, char *path);
+extern void cgp_fini(void);
+
+#endif /* __CR_CGROUP_PROPS_H__ */
diff --git a/criu/include/cr_options.h b/criu/include/cr_options.h
index 551ca7045b84..844bf2ca461c 100644
--- a/criu/include/cr_options.h
+++ b/criu/include/cr_options.h
@@ -96,6 +96,9 @@  struct cr_options {
 	char			**exec_cmd;
 	unsigned int		manage_cgroups;
 	char			*new_global_cg_root;
+	char			*cgroup_props;
+	char			*cgroup_props_file;
+	bool			cgroup_props_ignore_default;
 	struct list_head	new_cgroup_roots;
 	bool			autodetect_ext_mounts;
 	bool			enable_external_sharing;
diff --git a/criu/proc_parse.c b/criu/proc_parse.c
index 174b54044794..c9b132be1786 100644
--- a/criu/proc_parse.c
+++ b/criu/proc_parse.c
@@ -33,6 +33,7 @@ 
 #include "namespaces.h"
 #include "files-reg.h"
 #include "cgroup.h"
+#include "cgroup-props.h"
 
 #include "protobuf.h"
 #include "images/fdinfo.pb-c.h"
@@ -2185,6 +2186,18 @@  int parse_cgroup_file(FILE *f, struct list_head *retl, unsigned int *n)
 		if (e)
 			*e = '\0';
 
+		/*
+		 * Controllers and their props might be
+		 * configured the way some of them are
+		 * not taken into the image for migration
+		 * sake or container specifics.
+		 */
+		if (cgp_should_skip_controller(name)) {
+			pr_debug("cg-prop: Skipping controller %s\n", name);
+			xfree(ncc);
+			continue;
+		}
+
 		ncc->name = xstrdup(name);
 		ncc->path = xstrdup(path);
 		ncc->cgns_prefix = 0;

Comments

Cyrill Gorcunov April 27, 2016, 8:26 p.m.
On Wed, Apr 27, 2016 at 10:17:48PM +0300, Cyrill Gorcunov wrote:
> We have some common predefined properties such as "cpuset.cpus" and etc gathered
> in @cgp_predefined set, but there might be situation when only predefined ones
> are not enough, so add ability to specify properties via --cgroup-props
> and/or --cgroup-props-file options.
> 
> For example one may pass
> 
>   --cgroup-props "\"cpu\": [\"cpu.shares\", \"cpu.cfs_period_us\"]"
> 
> to dump custom properties for cpu controller.
> 
> The description is implemented in almost valid yaml, probably we will
> need to support the various forms, but oneline is enough for now.
> 
> Depending if --cgroup-props-ignore-default option passed the
> new properties either merged with predefined or substitute
> them.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>

You know, guys, maybe it would be more convenient to define
strategy inside --cgroup-props, like

 --cgroup-props cpu: [strategy: merge, properties: [cpu.shares, cpu.cfs_period_us]]
 --cgroup-props cpu: [strategy: replace, properties: [cpu.shares, cpu.cfs_period_us]]

?
Tycho Andersen April 28, 2016, 3:04 p.m.
On Wed, Apr 27, 2016 at 10:17:48PM +0300, Cyrill Gorcunov wrote:
>
> +static int cgp_merge_props(cgp_list_entry_t *t)
> +{
> +	cgp_list_entry_t *p, *n;
> +	size_t nr_props, i, j;
> +
> +	/* FIXME This is O^2, optimize! */
> +	list_for_each_entry_safe(p, n, &cgp_predefined_list, list) {
> +		if (strcmp(t->cgp.name, p->cgp.name))
> +			continue;
> +
> +		pr_debug("Merging \'%s\' controller props\n", t->cgp.name);
> +
> +		nr_props = t->cgp.nr_props + p->cgp.nr_props;
> +
> +		if (xrealloc_safe(&t->cgp.props, nr_props * sizeof(char *)))
> +			return -ENOMEM;
> +
> +		for (i = t->cgp.nr_props, j = 0; i < nr_props; i++, j++) {
> +			t->cgp.props[i] = xstrdup(p->cgp.props[j]);
> +			if (!t->cgp.props[i])
> +				return -ENOMEM;
> +			t->cgp.nr_props++;
> +		}

here it might make sense to dedup so that we don't save a predefined
property twice if the user supplies it as well, but it's a minor
point.

Acked-by: Tycho Andersen <tycho.andersen@canonical.com>

> +
> +		list_del(&p->list);
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static char *skip_spaces(char **stream, size_t *len)
> +{
> +	if (stream && *len) {
> +		char *p = *stream;
> +
> +		while (p && *len && *p == ' ')
> +			p++, (*len)--;
> +		if (p != *stream)
> +			*stream = p;
> +		return p;
> +	}
> +
> +	return NULL;
> +}
> +
> +static char *eat_symbol(char **stream, size_t *len, char sym, bool skip_ws)
> +{
> +	char *p = skip_ws ? skip_spaces(stream, len) : (stream ? *stream : NULL);
> +
> +	if (!p || *p != sym || !*len)
> +		return NULL;
> +	(*stream) = p + 1;
> +	(*len)--;
> +	return p;
> +}
> +
> +static char *get_quoted(char **stream, size_t *len, bool skip_ws)
> +{
> +	char *p = skip_ws ? skip_spaces(stream, len) : (stream ? *stream : NULL);
> +	char *from = p + 1;
> +	char *dst;
> +
> +	if (!p || *p != '\"')
> +		return NULL;
> +
> +	for (p = from, (*len)--; (*len); p++, (*len)--) {
> +		if (*p == '\"') {
> +			if (p == from)
> +				break;
> +			dst = xmalloc(p - from + 1);
> +			if (!dst)
> +				break;
> +
> +			memcpy(dst, from, p - from);
> +			dst[p - from] = '\0';
> +
> +			(*stream) = p + 1;
> +			(*len)--;
> +			return dst;
> +		}
> +	}
> +
> +	return NULL;
> +}
> +
> +static int cgp_parse_stream(char *stream, size_t len)
> +{
> +	cgp_list_entry_t *cgp_entry;
> +	int ret = 0;
> +	char *p;
> +
> +	/*
> +	 * We expect the following format here
> +	 * (very simplified YAML!)
> +	 *
> +	 *  "cpu": ["cpu.shares", "cpu.cfs_period_us"]
> +	 *  "memory": ["memory.limit_in_bytes", "memory.memsw.limit_in_bytes"]
> +	 *
> +	 *  and etc.
> +	 */
> +
> +	while (len) {
> +		/*
> +		 * Controller name.
> +		 */
> +		p = get_quoted(&stream, &len, false);
> +		if (!p) {
> +			pr_err("Expected \'\"\' symbol for controller name\n");
> +			goto err_parse;
> +		}
> +
> +		pr_debug("Parsing controller %s\n", p);
> +
> +		cgp_entry = xzalloc(sizeof(*cgp_entry));
> +		if (cgp_entry) {
> +			INIT_LIST_HEAD(&cgp_entry->list);
> +			cgp_entry->cgp.name = p;
> +		} else {
> +			pr_err("Can't allocate memory for controller %s\n", p);
> +			xfree(p);
> +			return -ENOMEM;
> +		}
> +
> +		p = eat_symbol(&stream, &len, ':', true);
> +		if (!p) {
> +			pr_err("Expected \':\' symbol in controller's %s stream\n",
> +			       cgp_entry->cgp.name);
> +			goto err_parse;
> +		}
> +
> +		p = eat_symbol(&stream, &len, '[', true);
> +		if (!p) {
> +			pr_err("Expected \'[\' symbol in controller's %s stream\n",
> +			       cgp_entry->cgp.name);
> +			goto err_parse;
> +		}
> +
> +		while ((p = get_quoted(&stream, &len, true))) {
> +			if (!p) {
> +				pr_err("Expected property name for controller %s\n",
> +				       cgp_entry->cgp.name);
> +				goto err_parse;
> +			}
> +
> +			if (xrealloc_safe(&cgp_entry->cgp.props,
> +					  (cgp_entry->cgp.nr_props + 1) * sizeof(char *))) {
> +				pr_err("Can't allocate property for controller %s\n",
> +				       cgp_entry->cgp.name);
> +				return -1;
> +			}
> +
> +			cgp_entry->cgp.props[cgp_entry->cgp.nr_props++] = p;
> +			pr_debug("\tProperty %s\n", p);
> +
> +			p = eat_symbol(&stream, &len, ',', true);
> +			if (!p) {
> +				if (stream[0] == ']') {
> +					stream++, len--;
> +					break;
> +				}
> +				pr_err("Expected ']' in controller's %s stream\n",
> +				       cgp_entry->cgp.name);
> +				goto err_parse;
> +			}
> +		}
> +
> +		p = eat_symbol(&stream, &len, '\n', true);
> +		if (!p && len) {
> +			pr_err("Expected \'\\n\' symbol in controller's %s stream\n",
> +			       cgp_entry->cgp.name);
> +			goto err_parse;
> +		}
> +
> +		list_add(&cgp_entry->list, &cgp_list);
> +
> +		if (!opts.cgroup_props_ignore_default) {
> +			if (cgp_merge_props(cgp_entry)) {
> +				pr_err("Can't merge controller \'%s\'\n",
> +				       cgp_entry->cgp.name);
> +					goto out;
> +			}
> +		} else {
> +			if (cgp_subst_props(cgp_entry)) {
> +				pr_err("Can't substitute controller \'%s\'\n",
> +				       cgp_entry->cgp.name);
> +					goto out;
> +			}
> +		}
> +
> +		cgp_entry = NULL;
> +	}
> +
> +	ret = 0;
> +out:
> +	return ret;
> +
> +err_parse:
> +	cgp_free(cgp_entry);
> +	ret = -EINVAL;
> +	goto out;
> +}
> +
> +static int cgp_parse_file(char *path)
> +{
> +	void *mem = MAP_FAILED;
> +	int fd = -1, ret = -1;
> +	struct stat st;
> +
> +	fd = open(path, O_RDONLY);
> +	if (fd < 0) {
> +		pr_perror("Can't open file %s\n", path);
> +		goto err;
> +	}
> +
> +	if (fstat(fd, &st)) {
> +		pr_perror("Can't stat file %s\n", path);
> +		goto err;
> +	}
> +
> +	mem = mmap(NULL, st.st_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_FILE, fd, 0);
> +	if (mem == MAP_FAILED) {
> +		pr_perror("Can't mmap file %s\n", path);
> +		goto err;
> +	}
> +
> +	if (cgp_parse_stream(mem, st.st_size)) {
> +		pr_err("Failed to parse file `%s'\n", path);
> +		goto err;
> +	}
> +
> +	ret = 0;
> +err:
> +	if (mem != MAP_FAILED)
> +		munmap(mem, st.st_size);
> +	close_safe(&fd);
> +	return ret;
> +}
> +
> +int cgp_init(char *stream, size_t len, char *path)
> +{
> +	int ret = 0;
> +
> +	if (stream && len) {
> +		ret = cgp_parse_stream(stream, len);
> +		if (ret)
> +			goto err;
> +	}
> +
> +	if (path)
> +		ret = cgp_parse_file(path);
> +err:
> +	return ret;
> +}
> +
> +bool cgp_should_skip_controller(const char *name)
> +{
> +	/*
> +	 * FIXME Implement!
> +	 */
> +	return false;
> +}
> +
> +const cgp_t *cgp_get_props(const char *name)
> +{
> +	cgp_list_entry_t *p;
> +
> +	p = cgp_get_predefined(name);
> +	if (p)
> +		return &p->cgp;
> +
> +	list_for_each_entry(p, &cgp_list, list) {
> +		if (!strcmp(p->cgp.name, name))
> +			return &p->cgp;
> +	}
> +
> +	return NULL;
> +}
> +
> +void cgp_fini(void)
> +{
> +	cgp_list_entry_t *p, *t;
> +
> +	list_for_each_entry_safe(p, t, &cgp_list, list)
> +		cgp_free(p);
> +	INIT_LIST_HEAD(&cgp_list);
> +}
> diff --git a/criu/cgroup.c b/criu/cgroup.c
> index a706ff6f76ce..1bf52851ea17 100644
> --- a/criu/cgroup.c
> +++ b/criu/cgroup.c
> @@ -10,6 +10,7 @@
>  #include "list.h"
>  #include "xmalloc.h"
>  #include "cgroup.h"
> +#include "cgroup-props.h"
>  #include "cr_options.h"
>  #include "pstree.h"
>  #include "proc_parse.h"
> @@ -25,73 +26,6 @@
>  #include "images/cgroup.pb-c.h"
>  
>  /*
> - * These string arrays have the names of all the properties that will be
> - * restored. To add a property for a cgroup type, add it to the
> - * corresponding char array above the NULL terminator. If you are adding
> - * a new cgroup family all together, you must also edit get_known_properties()
> - * Currently the code only supports properties with 1 value
> - */
> -
> -static const char *cpu_props[] = {
> -	"cpu.shares",
> -	"cpu.cfs_period_us",
> -	"cpu.cfs_quota_us",
> -	"cpu.rt_period_us",
> -	"cpu.rt_runtime_us",
> -	"notify_on_release",
> -	NULL
> -};
> -
> -static const char *memory_props[] = {
> -	/* limit_in_bytes and memsw.limit_in_bytes must be set in this order */
> -	"memory.limit_in_bytes",
> -	"memory.memsw.limit_in_bytes",
> -	"memory.use_hierarchy",
> -	"notify_on_release",
> -	NULL
> -};
> -
> -static const char *cpuset_props[] = {
> -	/*
> -	 * cpuset.cpus and cpuset.mems must be set before the process moves
> -	 * into its cgroup; they are "initialized" below to whatever the root
> -	 * values are in copy_special_cg_props so as not to cause ENOSPC when
> -	 * values are restored via this code.
> -	 */
> -	"cpuset.cpus",
> -	"cpuset.mems",
> -	"cpuset.memory_migrate",
> -	"cpuset.cpu_exclusive",
> -	"cpuset.mem_exclusive",
> -	"cpuset.mem_hardwall",
> -	"cpuset.memory_spread_page",
> -	"cpuset.memory_spread_slab",
> -	"cpuset.sched_load_balance",
> -	"cpuset.sched_relax_domain_level",
> -	"notify_on_release",
> -	NULL
> -};
> -
> -static const char *blkio_props[] = {
> -	"blkio.weight",
> -	"notify_on_release",
> -	NULL
> -};
> -
> -static const char *freezer_props[] = {
> -	"notify_on_release",
> -	NULL
> -};
> -
> -static const char *global_props[] = {
> -	"cgroup.clone_children",
> -	"notify_on_release",
> -	"cgroup.procs",
> -	"tasks",
> -	NULL
> -};
> -
> -/*
>   * This structure describes set of controller groups
>   * a task lives in. The cg_ctl entries are stored in
>   * the @ctls list sorted by the .name field and then
> @@ -421,33 +355,14 @@ static void free_all_cgroup_props(struct cgroup_dir *ncd)
>  	ncd->n_properties = 0;
>  }
>  
> -static const char **get_known_properties(char *controller)
> -{
> -	const char **prop_arr = NULL;
> -
> -	if (!strcmp(controller, "cpu"))
> -		prop_arr = cpu_props;
> -	else if (!strcmp(controller, "memory"))
> -		prop_arr = memory_props;
> -	else if (!strcmp(controller, "cpuset"))
> -		prop_arr = cpuset_props;
> -	else if (!strcmp(controller, "blkio"))
> -		prop_arr = blkio_props;
> -	else if (!strcmp(controller, "freezer"))
> -		prop_arr = freezer_props;
> -
> -	return prop_arr;
> -}
> -
> -static int dump_cg_props_array(const char *fpath, struct cgroup_dir *ncd,
> -			       const char **prop_arr)
> +static int dump_cg_props_array(const char *fpath, struct cgroup_dir *ncd, const cgp_t *cgp)
>  {
>  	int j;
>  	char buf[PATH_MAX];
>  	struct cgroup_prop *prop;
>  
> -	for (j = 0; prop_arr != NULL && prop_arr[j] != NULL; ++j) {
> -		if (snprintf(buf, PATH_MAX, "%s/%s", fpath, prop_arr[j]) >= PATH_MAX) {
> +	for (j = 0; cgp && j < cgp->nr_props; j++) {
> +		if (snprintf(buf, PATH_MAX, "%s/%s", fpath, cgp->props[j]) >= PATH_MAX) {
>  			pr_err("snprintf output was truncated\n");
>  			return -1;
>  		}
> @@ -457,7 +372,7 @@ static int dump_cg_props_array(const char *fpath, struct cgroup_dir *ncd,
>  			continue;
>  		}
>  
> -		prop = create_cgroup_prop(prop_arr[j]);
> +		prop = create_cgroup_prop(cgp->props[j]);
>  		if (!prop) {
>  			free_all_cgroup_props(ncd);
>  			return -1;
> @@ -483,15 +398,14 @@ static int add_cgroup_properties(const char *fpath, struct cgroup_dir *ncd,
>  	int i;
>  
>  	for (i = 0; i < controller->n_controllers; ++i) {
> +		const cgp_t *cgp = cgp_get_props(controller->controllers[i]);
>  
> -		const char **prop_arr = get_known_properties(controller->controllers[i]);
> -
> -		if (dump_cg_props_array(fpath, ncd, prop_arr) < 0) {
> +		if (dump_cg_props_array(fpath, ncd, cgp) < 0) {
>  			pr_err("dumping known properties failed");
>  			return -1;
>  		}
>  
> -		if (dump_cg_props_array(fpath, ncd, global_props) < 0) {
> +		if (dump_cg_props_array(fpath, ncd, &cgp_global) < 0) {
>  			pr_err("dumping global properties failed");
>  			return -1;
>  		}
> diff --git a/criu/cr-dump.c b/criu/cr-dump.c
> index 5ac9fd041e4e..b5531e43d115 100644
> --- a/criu/cr-dump.c
> +++ b/criu/cr-dump.c
> @@ -61,6 +61,7 @@
>  #include "cpu.h"
>  #include "elf.h"
>  #include "cgroup.h"
> +#include "cgroup-props.h"
>  #include "file-lock.h"
>  #include "page-xfer.h"
>  #include "kerndat.h"
> @@ -1554,6 +1555,7 @@ static int cr_dump_finish(int ret)
>  		ret = -1;
>  
>  	cr_plugin_fini(CR_PLUGIN_STAGE__DUMP, ret);
> +	cgp_fini();
>  
>  	if (!ret) {
>  		/*
> @@ -1653,6 +1655,12 @@ int cr_dump_tasks(pid_t pid)
>  	if (vdso_init())
>  		goto err;
>  
> +	if (cgp_init(opts.cgroup_props,
> +		     opts.cgroup_props ?
> +		     strlen(opts.cgroup_props) : 0,
> +		     opts.cgroup_props_file))
> +		goto err;
> +
>  	if (parse_cg_info())
>  		goto err;
>  
> diff --git a/criu/crtools.c b/criu/crtools.c
> index 7a0f9778db74..eb6f68b398b2 100644
> --- a/criu/crtools.c
> +++ b/criu/crtools.c
> @@ -38,6 +38,7 @@
>  #include "mount.h"
>  #include "namespaces.h"
>  #include "cgroup.h"
> +#include "cgroup-props.h"
>  #include "cpu.h"
>  #include "action-scripts.h"
>  #include "irmap.h"
> @@ -320,6 +321,9 @@ int main(int argc, char *argv[], char *envp[])
>  		{ "extra",			no_argument,		0, 1077	},
>  		{ "experimental",		no_argument,		0, 1078	},
>  		{ "all",			no_argument,		0, 1079	},
> +		{ "cgroup-props",		required_argument,	0, 1080	},
> +		{ "cgroup-props-file",		required_argument,	0, 1081	},
> +		{ "cgroup-props-ignore-default",no_argument,		0, 1082	},
>  		{ },
>  	};
>  
> @@ -622,6 +626,15 @@ int main(int argc, char *argv[], char *envp[])
>  			opts.check_extra_features = true;
>  			opts.check_experimental_features = true;
>  			break;
> +		case 1080:
> +			opts.cgroup_props = optarg;
> +			break;
> +		case 1081:
> +			opts.cgroup_props_file = optarg;
> +			break;
> +		case 1082:
> +			opts.cgroup_props_ignore_default = true;
> +			break;
>  		case 'V':
>  			pr_msg("Version: %s\n", CRIU_VERSION);
>  			if (strcmp(CRIU_GITID, "0"))
> @@ -873,6 +886,16 @@ usage:
>  "                        change the root cgroup the controller will be\n"
>  "                        installed into. No controller means that root is the\n"
>  "                        default for all controllers not specified.\n"
> +"  --cgroup-props STRING\n"
> +"                        define cgroup controllers and properties\n"
> +"                        to be checkpointed, which are described\n"
> +"                        via STRING using simplified YAML format.\n"
> +"  --cgroup-props-file FILE\n"
> +"                        same as --cgroup-props but taking descrition\n"
> +"                        from the path specified.\n"
> +"  --cgroup-props-ignore-default\n"
> +"                        when parse custom properties do not merged them with\n"
> +"                        default ones but simply substitute appropriate controller.\n"
>  "  --skip-mnt PATH       ignore this mountpoint when dumping the mount namespace.\n"
>  "  --enable-fs FSNAMES   a comma separated list of filesystem names or \"all\".\n"
>  "                        force criu to (try to) dump/restore these filesystem's\n"
> diff --git a/criu/include/cgroup-props.h b/criu/include/cgroup-props.h
> new file mode 100644
> index 000000000000..e6c96bae94f8
> --- /dev/null
> +++ b/criu/include/cgroup-props.h
> @@ -0,0 +1,19 @@
> +#ifndef __CR_CGROUP_PROPS_H__
> +#define __CR_CGROUP_PROPS_H__
> +
> +#include <stdbool.h>
> +
> +typedef struct {
> +	const char	*name;
> +	size_t		nr_props;
> +	const char	**props;
> +} cgp_t;
> +
> +extern cgp_t cgp_global;
> +extern const cgp_t *cgp_get_props(const char *name);
> +extern bool cgp_should_skip_controller(const char *name);
> +
> +extern int cgp_init(char *stream, size_t len, char *path);
> +extern void cgp_fini(void);
> +
> +#endif /* __CR_CGROUP_PROPS_H__ */
> diff --git a/criu/include/cr_options.h b/criu/include/cr_options.h
> index 551ca7045b84..844bf2ca461c 100644
> --- a/criu/include/cr_options.h
> +++ b/criu/include/cr_options.h
> @@ -96,6 +96,9 @@ struct cr_options {
>  	char			**exec_cmd;
>  	unsigned int		manage_cgroups;
>  	char			*new_global_cg_root;
> +	char			*cgroup_props;
> +	char			*cgroup_props_file;
> +	bool			cgroup_props_ignore_default;
>  	struct list_head	new_cgroup_roots;
>  	bool			autodetect_ext_mounts;
>  	bool			enable_external_sharing;
> diff --git a/criu/proc_parse.c b/criu/proc_parse.c
> index 174b54044794..c9b132be1786 100644
> --- a/criu/proc_parse.c
> +++ b/criu/proc_parse.c
> @@ -33,6 +33,7 @@
>  #include "namespaces.h"
>  #include "files-reg.h"
>  #include "cgroup.h"
> +#include "cgroup-props.h"
>  
>  #include "protobuf.h"
>  #include "images/fdinfo.pb-c.h"
> @@ -2185,6 +2186,18 @@ int parse_cgroup_file(FILE *f, struct list_head *retl, unsigned int *n)
>  		if (e)
>  			*e = '\0';
>  
> +		/*
> +		 * Controllers and their props might be
> +		 * configured the way some of them are
> +		 * not taken into the image for migration
> +		 * sake or container specifics.
> +		 */
> +		if (cgp_should_skip_controller(name)) {
> +			pr_debug("cg-prop: Skipping controller %s\n", name);
> +			xfree(ncc);
> +			continue;
> +		}
> +
>  		ncc->name = xstrdup(name);
>  		ncc->path = xstrdup(path);
>  		ncc->cgns_prefix = 0;
> -- 
> 2.5.5
>
Tycho Andersen April 28, 2016, 3:04 p.m.
On Wed, Apr 27, 2016 at 11:26:27PM +0300, Cyrill Gorcunov wrote:
> On Wed, Apr 27, 2016 at 10:17:48PM +0300, Cyrill Gorcunov wrote:
> > We have some common predefined properties such as "cpuset.cpus" and etc gathered
> > in @cgp_predefined set, but there might be situation when only predefined ones
> > are not enough, so add ability to specify properties via --cgroup-props
> > and/or --cgroup-props-file options.
> > 
> > For example one may pass
> > 
> >   --cgroup-props "\"cpu\": [\"cpu.shares\", \"cpu.cfs_period_us\"]"
> > 
> > to dump custom properties for cpu controller.
> > 
> > The description is implemented in almost valid yaml, probably we will
> > need to support the various forms, but oneline is enough for now.
> > 
> > Depending if --cgroup-props-ignore-default option passed the
> > new properties either merged with predefined or substitute
> > them.
> > 
> > Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
> 
> You know, guys, maybe it would be more convenient to define
> strategy inside --cgroup-props, like
> 
>  --cgroup-props cpu: [strategy: merge, properties: [cpu.shares, cpu.cfs_period_us]]
>  --cgroup-props cpu: [strategy: replace, properties: [cpu.shares, cpu.cfs_period_us]]

Either would be fine with me, I have no strong opinions :)

Tycho
Cyrill Gorcunov April 28, 2016, 3:08 p.m.
On Thu, Apr 28, 2016 at 09:04:48AM -0600, Tycho Andersen wrote:
> > 
> > You know, guys, maybe it would be more convenient to define
> > strategy inside --cgroup-props, like
> > 
> >  --cgroup-props cpu: [strategy: merge, properties: [cpu.shares, cpu.cfs_period_us]]
> >  --cgroup-props cpu: [strategy: replace, properties: [cpu.shares, cpu.cfs_period_us]]
> 
> Either would be fine with me, I have no strong opinions :)

Since this varian allows to specify how to hande each controller
i guess it's more flexible, but I don't have strong opinion here either.
Andrew, Pavel?
Cyrill Gorcunov April 28, 2016, 3:09 p.m.
On Thu, Apr 28, 2016 at 09:04:18AM -0600, Tycho Andersen wrote:
> On Wed, Apr 27, 2016 at 10:17:48PM +0300, Cyrill Gorcunov wrote:
> >
> > +static int cgp_merge_props(cgp_list_entry_t *t)
> > +{
> > +	cgp_list_entry_t *p, *n;
> > +	size_t nr_props, i, j;
> > +
> > +	/* FIXME This is O^2, optimize! */
> > +	list_for_each_entry_safe(p, n, &cgp_predefined_list, list) {
> > +		if (strcmp(t->cgp.name, p->cgp.name))
> > +			continue;
> > +
> > +		pr_debug("Merging \'%s\' controller props\n", t->cgp.name);
> > +
> > +		nr_props = t->cgp.nr_props + p->cgp.nr_props;
> > +
> > +		if (xrealloc_safe(&t->cgp.props, nr_props * sizeof(char *)))
> > +			return -ENOMEM;
> > +
> > +		for (i = t->cgp.nr_props, j = 0; i < nr_props; i++, j++) {
> > +			t->cgp.props[i] = xstrdup(p->cgp.props[j]);
> > +			if (!t->cgp.props[i])
> > +				return -ENOMEM;
> > +			t->cgp.nr_props++;
> > +		}
> 
> here it might make sense to dedup so that we don't save a predefined
> property twice if the user supplies it as well, but it's a minor
> point.
> 
> Acked-by: Tycho Andersen <tycho.andersen@canonical.com>

Thanks!

There are ways to optimize ;)

	Cyrill