This patch adds cgroup_create_cgroup_from_parent() API. I've tested it

with a modified version of libcg_ba (basic acceptance tests), that I'll send
out later. I've also fixed a couple of bugs, I'll list them in order and
if desired, I'll split out the patches so that the enhancements can be
separate from the bug fixes

1. Fix cg_create_control_group() to succeed if the directory already exists
2. We can't always append a control_file, we need to open it "r+"
3. It's OK for writing values to a  control_file to fail, since some of
   them are read-only and some may not exist in hierarchies below the root
4. Skip over directories while trying to read control file's name value pairs
5. In cg_rd_ctlr_file we don't allocate any memory to value, directly fscanf
   into it, that's been fixed
6. There might be more fixes that might already be present in trunk, since
   I hope I've not redone a lot of the fixes, I had branched of trunk very
   recently

Changelog v2
------------
1. Refactor cgroup_free_controllers() and use it in
        cgroup_create_cgroup_from_parent()
2. Check for strdup failure
3. Check if dst == src

TODO:

1. Use cg_add_controller() API
2. Use cg_add_value_string() API

I'll send out patches for the TODOs later.

I've also updated the basic acceptance test (libcg_ba.cpp)

Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com>



git-svn-id: https://libcg.svn.sourceforge.net/svnroot/libcg/trunk@165 4f4bb910-9a46-0410-90c8-c897d4f1cd53

4 files changed, 196 insertions, 22 deletions

diff --git a/api.c b/api.c
index 2902ad1..3755957 100644
--- a/api.c
+++ b/api.c
@@ -39,6 +39,7 @@
 #include <fts.h>
 #include <ctype.h>
 #include <pwd.h>
+#include <libgen.h>
 
 #ifndef PACKAGE_VERSION
 #define PACKAGE_VERSION 0.01
@@ -366,6 +367,7 @@ int cgroup_attach_task_pid(struct cgroup *cgroup, pid_t tid)
 				return ECGROUPSUBSYSNOTMOUNTED;
 			}
 		}
+
 		for (i = 0; i < cgroup->index; i++) {
 			if (!cg_build_path(cgroup->name, path,
 					cgroup->controller[i]->name))
@@ -436,6 +438,12 @@ static int cg_create_control_group(char *path)
 	error = mkdir(path, S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH);
 	if (error) {
 		switch(errno) {
+		case EEXIST:
+			/*
+			 * If the directory already exists, it really should
+			 * not be an error
+			 */
+			return 0;
 		case EPERM:
 			return ECGROUPNOTOWNER;
 		default:
@@ -457,7 +465,7 @@ static int cg_set_control_value(char *path, char *val)
 	if (!cg_test_mounted_fs())
 		return ECGROUPNOTMOUNTED;
 
-	control_file = fopen(path, "a");
+	control_file = fopen(path, "r+");
 
 	if (!control_file) {
 		if (errno == EPERM) {
@@ -529,7 +537,7 @@ int cgroup_modify_cgroup(struct cgroup *cgroup)
 			continue;
 		strcpy(path, base);
 		for (j = 0; j < cgroup->controller[i]->index;
-					j++, strcpy(path, base)) {
+				j++, strcpy(path, base)) {
 			strcat(path, cgroup->controller[i]->values[j]->name);
 			error = cg_set_control_value(path,
 				cgroup->controller[i]->values[j]->value);
@@ -543,6 +551,80 @@ err:
 
 }
 
+/**
+ * @dst: Destination controller
+ * @src: Source controller from which values will be copied to dst
+ *
+ * Create a duplicate copy of values under the specified controller
+ */
+int cgroup_copy_controller_values(struct cgroup_controller *dst,
+					struct cgroup_controller *src)
+{
+	int i, ret = 0;
+
+	if (!dst || !src)
+		return ECGFAIL;
+
+	strncpy(dst->name, src->name, FILENAME_MAX);
+	for (i = 0; i < src->index; i++, dst->index++) {
+		struct control_value *src_val = src->values[i];
+		struct control_value *dst_val;
+
+		dst->values[i] = calloc(1, sizeof(struct control_value));
+		if (!dst->values[i]) {
+			ret = ECGFAIL;
+			goto err;
+		}
+
+		dst_val = dst->values[i];
+		strncpy(dst_val->value, src_val->value, CG_VALUE_MAX);
+		strncpy(dst_val->name, src_val->name, FILENAME_MAX);
+	}
+err:
+	return ret;
+}
+
+/**
+ * @dst: Destination control group
+ * @src: Source from which values will be copied to dst
+ *
+ * Create a duplicate copy of src in dst. This will be useful for those who
+ * that intend to create new instances based on an existing control group
+ */
+int cgroup_copy_cgroup(struct cgroup *dst, struct cgroup *src)
+{
+	int ret = 0, i;
+
+	if (!dst || !src)
+		return ECGROUPNOTEXIST;
+
+	/*
+	 * Should we just use the restrict keyword instead?
+	 */
+	if (dst == src)
+		return ECGFAIL;
+
+	cgroup_free_controllers(dst);
+
+	for (i = 0; i < src->index; i++, dst->index++) {
+		struct cgroup_controller *src_ctlr = src->controller[i];
+		struct cgroup_controller *dst_ctlr;
+
+		dst->controller[i] = calloc(1, sizeof(struct cgroup_controller));
+		if (!dst->controller[i]) {
+			ret = ECGFAIL;
+			goto err;
+		}
+
+		dst_ctlr = dst->controller[i];
+		ret = cgroup_copy_controller_values(dst_ctlr, src_ctlr);
+		if (ret)
+			goto err;
+	}
+err:
+	return ret;
+}
+
 /** cgroup_create_cgroup creates a new control group.
  * struct cgroup *cgroup: The control group to be created
  *
@@ -584,7 +666,6 @@ int cgroup_create_cgroup(struct cgroup *cgroup, int ignore_ownership)
 				cgroup->controller[k]->name))
 			continue;
 
-		dbg("path is %s\n", path);
 		error = cg_create_control_group(path);
 		if (error)
 			goto err;
@@ -605,9 +686,12 @@ int cgroup_create_cgroup(struct cgroup *cgroup, int ignore_ownership)
 				cgroup->controller[k]->values[j]->value);
 			/*
 			 * Should we undo, what we've done in the loops above?
+			 * An error should not be treated as fatal, since we have
+			 * several read-only files and several files that
+			 * are only conditionally created in the child.
 			 */
 			if (error)
-				goto err;
+				continue;
 		}
 
 		if (!ignore_ownership) {
@@ -627,6 +711,53 @@ err:
 	return error;
 }
 
+/**
+ * @cgroup: cgroup data structure to be filled with parent values and then
+ *          passed down for creation
+ * @ignore_ownership: Ignore doing a chown on the newly created cgroup
+ */
+int cgroup_create_cgroup_from_parent(struct cgroup *cgroup, int ignore_ownership)
+{
+	char *parent;
+	struct cgroup *parent_cgroup;
+	int ret = ECGFAIL;
+	char *dir_name, *orig_dir_name;
+
+	dir_name = strdup(cgroup->name);
+	if (!dir_name)
+		return ECGFAIL;
+
+	orig_dir_name = dir_name;
+	dir_name = dirname(dir_name);
+
+	/*
+	 * Ugly: but we expect cgroup_get_cgroup() to do the right thing
+	 */
+	if (asprintf(&parent, "%s", dir_name) < 0) {
+		free(orig_dir_name);
+		return ret;
+	}
+	parent_cgroup = cgroup_new_cgroup(parent);
+	if (!parent_cgroup)
+		goto err_nomem;
+
+	if (cgroup_get_cgroup(parent_cgroup) == NULL)
+		goto err_parent;
+
+	ret = cgroup_copy_cgroup(cgroup, parent_cgroup);
+	if (ret)
+		goto err_parent;
+
+	ret = cgroup_create_cgroup(cgroup, ignore_ownership);
+
+err_parent:
+	cgroup_free(&parent_cgroup);
+err_nomem:
+	free(parent);
+	free(orig_dir_name);
+	return ret;
+}
+
 /** cgroup_delete cgroup deletes a control group.
  *  struct cgroup *cgroup takes the group which is to be deleted.
  *
@@ -726,6 +857,10 @@ static char *cg_rd_ctrl_file(char *subsys, char *cgroup, char *file)
 	if (!ctrl_file)
 		return NULL;
 
+	value = malloc(CG_VALUE_MAX);
+	if (!value)
+		return NULL;
+
 	fscanf(ctrl_file, "%as", &value);
 
 	fclose(ctrl_file);
@@ -741,7 +876,7 @@ static int cgroup_fill_cgc(struct dirent *ctrl_dir, struct cgroup *cgroup,
 {
 	char *ctrl_name;
 	char *ctrl_file;
-	char *ctrl_value;
+	char *ctrl_value = NULL;
 	char *d_name;
 	char path[FILENAME_MAX+1];
 	char *buffer;
@@ -799,10 +934,12 @@ static int cgroup_fill_cgc(struct dirent *ctrl_dir, struct cgroup *cgroup,
 		if (cgroup_add_value_string(cgc, ctrl_dir->d_name,
 				ctrl_value)) {
 			error = ECGFAIL;
+			goto fill_error;
 		}
-		free(ctrl_value);
 	}
 fill_error:
+	if (ctrl_value)
+		free(ctrl_value);
 	free(d_name);
 	return error;
 }
@@ -864,7 +1001,7 @@ struct cgroup *cgroup_get_cgroup(struct cgroup *cgroup)
 		 * Get the uid and gid information
 		 */
 
-		control_path = malloc(strlen(path)+strlen("tasks") + 1);
+		control_path = malloc(strlen(path) + strlen("tasks") + 1);
 		strcpy(control_path, path);
 
 		if (!control_path)
@@ -892,16 +1029,23 @@ struct cgroup *cgroup_get_cgroup(struct cgroup *cgroup)
 			/* error = ECGROUPSTRUCTERROR; */
 			goto unlock_error;
 		}
+
 		while ((ctrl_dir = readdir(dir)) != NULL) {
+			/*
+			 * Skip over non regular files
+			 */
+			if (ctrl_dir->d_type != DT_REG)
+				continue;
+
 			error = cgroup_fill_cgc(ctrl_dir, cgroup, cgc, i);
 			if (error == ECGFAIL) {
 				closedir(dir);
 				goto unlock_error;
 			}
-
 		}
 		closedir(dir);
 	}
+
 	/* Check if the group really exists or not */
 	if (!cgroup->index)
 		goto unlock_error;
@@ -1134,8 +1278,8 @@ static int cg_parse_rules_config_file(struct cgroup_rules_data *cgrldp,
 					(match_gid && !strcmp(user, "%"))) {
 					match_gid = 1;
 
-					cgroup = calloc(1,
-							sizeof(struct cgroup));
+					cgroup = (struct cgroup *)
+						calloc(1, sizeof(struct cgroup));
 					if (!cgroup) {
 						ret = ECGOTHER;
 						goto out;
diff --git a/libcgroup.h b/libcgroup.h
index 8eeb1a7..dc7e23f 100644
--- a/libcgroup.h
+++ b/libcgroup.h
@@ -172,6 +172,8 @@ int cgroup_create_cgroup(struct cgroup *cgroup, int ignore_ownership);
 int cgroup_delete_cgroup(struct cgroup *cgroup, int ignore_migration);
 int cgroup_attach_task_pid(struct cgroup *cgroup, pid_t tid);
 struct cgroup *cgroup_get_cgroup(struct cgroup *cgroup);
+int cgroup_create_cgroup_from_parent(struct cgroup *cgroup, int ignore_ownership);
+int cgroup_copy_cgroup(struct cgroup *dst, struct cgroup *src);
 
 /* Changes the cgroup of calling application based on rule file */
 int cgroup_change_cgroup_uid_gid(uid_t uid, gid_t gid, pid_t pid);
@@ -183,6 +185,7 @@ struct cgroup *cgroup_new_cgroup(const char *name);
 struct cgroup_controller *cgroup_add_controller(struct cgroup *cgroup,
 						const char *name);
 void cgroup_free(struct cgroup **cgroup);
+void cgroup_free_controllers(struct cgroup *cgroup);
 int cgroup_add_value_string(struct cgroup_controller *controller,
 				const char *name, const char *value);
 int cgroup_add_value_int64(struct cgroup_controller *controller,
diff --git a/tests/libcg_ba.cpp b/tests/libcg_ba.cpp
index f06ba9b..99a9184 100644
--- a/tests/libcg_ba.cpp
+++ b/tests/libcg_ba.cpp
@@ -36,6 +36,7 @@ public:
 	struct cgroup *makenode(const string &name, const string &task_uid,
 			const string &task_gid, const string &control_uid,
 			const string &control_gid);
+	struct cgroup *makenodefromparent(const string &name);
 };
 
 cg::cg(void)
@@ -57,7 +58,7 @@ struct cgroup *cg::makenode(const string &name, const string &task_uid,
 	gid_t tgid, cgid;
 	char *cgroup_name;
 	struct cgroup *ccg;
-	struct cgroup_controller *cpu, *memory, *cpuacct;
+	struct cgroup_controller *cpu, *cpuacct;
 	struct passwd *passwd;
 	struct group *grp;
 	int ret;
@@ -88,11 +89,10 @@ struct cgroup *cg::makenode(const string &name, const string &task_uid,
 	memset(cgroup_name, '\0', name.length());
 	strncpy(cgroup_name, name.c_str(), name.length());
 
-	ccg = cgroup_new_cgroup(cgroup_name, tuid, tgid, cuid, cgid);
+	ccg = cgroup_new_cgroup(cgroup_name);
+	cgroup_set_uid_gid(ccg, tuid, tgid, cuid, cgid);
 	cpu = cgroup_add_controller(ccg, "cpu");
 	cgroup_add_value_uint64(cpu, "cpu.shares", 2048);
-	memory = cgroup_add_controller(ccg, "memory");
-	cgroup_add_value_uint64(memory, "memory.limit_in_bytes", 102400);
 	cpuacct = cgroup_add_controller(ccg, "cpuacct");
 	cgroup_add_value_uint64(cpuacct, "cpuacct.usage", 0);
 
@@ -107,6 +107,27 @@ struct cgroup *cg::makenode(const string &name, const string &task_uid,
 	return ccg;
 }
 
+struct cgroup *cg::makenodefromparent(const string &name)
+{
+	char *cgroup_name;
+	struct cgroup *ccg;
+	int ret;
+
+	cgroup_name = (char *) malloc(name.length());
+	memset(cgroup_name, '\0', name.length());
+	strcpy(cgroup_name, name.c_str());
+
+	ccg = cgroup_new_cgroup(cgroup_name);
+	ret = cgroup_create_cgroup_from_parent(ccg, 1);
+	if (ret) {
+		cout << "cg create group failed " << errno << endl;
+		ret = cgroup_delete_cgroup(ccg, 1);
+		if (ret)
+			cout << "cg delete group failed " << errno << endl;
+	}
+	return ccg;
+}
+
 } // namespace
 
 using namespace cgtest;
@@ -114,10 +135,12 @@ int main(int argc, char *argv[])
 {
 	try {
 		cg *app = new cg();
-		struct cgroup *ccg;
+		struct cgroup *ccg, *ccg_child;
 		ccg = app->makenode("database", "root", "root", "balbir",
 					"balbir");
+		ccg_child = app->makenodefromparent("mysql");
 		cgroup_free(&ccg);
+		cgroup_free(&ccg_child);
 		delete app;
 	} catch (exception &e) {
 		cout << e.what() << endl;
diff --git a/wrapper.c b/wrapper.c
index c53a874..24a641d 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -70,9 +70,18 @@ struct cgroup_controller *cgroup_add_controller(struct cgroup *cgroup,
 	return controller;
 }
 
-void cgroup_free(struct cgroup **cgroup)
+void cgroup_free_controllers(struct cgroup *cgroup)
 {
 	int i, j;
+	for (i = 0; i < cgroup->index; i++) {
+		for (j = 0; j < cgroup->controller[i]->index; j++)
+			free(cgroup->controller[i]->values[j]);
+		free(cgroup->controller[i]);
+	}
+}
+
+void cgroup_free(struct cgroup **cgroup)
+{
 	struct cgroup *cg = *cgroup;
 
 	/*
@@ -81,12 +90,7 @@ void cgroup_free(struct cgroup **cgroup)
 	if (!cg)
 		return;
 
-	for (i = 0; i < cg->index; i++) {
-		for (j = 0; j < cg->controller[i]->index; j++)
-			free(cg->controller[i]->values[j]);
-		free(cg->controller[i]);
-	}
-
+	cgroup_free_controllers(cg);
 	free(cg);
 	*cgroup = NULL;
 }