From: Michael Tremer <michael.tremer@ipfire.org>
Date: Tue, 10 Dec 2024 15:14:07 +0000 (+0000)
Subject: cgroups: Try to create some recursive dependency chain
X-Git-Tag: 0.9.30~733
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=95244c13d39332baa303c1575f42c5c8108c0bc9;p=pakfire.git

cgroups: Try to create some recursive dependency chain

This should help us to operate on cgroups since every group now has a
reference to its parent group.

Signed-off-by: Michael Tremer <michael.tremer@ipfire.org>
---

diff --git a/src/libpakfire/build.c b/src/libpakfire/build.c
index 6476f9f93..2cb498db9 100644
--- a/src/libpakfire/build.c
+++ b/src/libpakfire/build.c
@@ -1348,7 +1348,7 @@ static int pakfire_build_setup_cgroup(struct pakfire_build* build) {
 	}
 
 	// Create a new cgroup
-	r = pakfire_cgroup_create(&build->cgroup, build->ctx, path, 0);
+	r = pakfire_cgroup_create(&build->cgroup, build->ctx, NULL, path, 0);
 	if (r) {
 		ERROR(build->ctx, "Could not create cgroup for build %s: %m\n", build->_id);
 		goto ERROR;
diff --git a/src/libpakfire/cgroup.c b/src/libpakfire/cgroup.c
index 06758f7fc..45941a677 100644
--- a/src/libpakfire/cgroup.c
+++ b/src/libpakfire/cgroup.c
@@ -67,8 +67,8 @@ struct pakfire_cgroup {
 	// Flags
 	int flags;
 
-	// Store the path
-	char path[PATH_MAX];
+	// Store the name
+	char name[PATH_MAX];
 
 	// File descriptor to cgroup
 	int fd;
@@ -81,26 +81,7 @@ static const char* pakfire_cgroup_name(struct pakfire_cgroup* cgroup) {
 	if (!cgroup->parent)
 		return "(root)";
 
-	return cgroup->path;
-}
-
-static int pakfire_cgroup_parent(struct pakfire_cgroup* cgroup, struct pakfire_cgroup** parent) {
-	char path[PATH_MAX];
-	int r;
-
-	// If there is no path left, we have reached the root
-	if (!*cgroup->path)
-		return 0;
-
-	// Determine the path of the parent
-	r = pakfire_path_dirname(path, cgroup->path);
-	if (r < 0) {
-		ERROR(cgroup->ctx, "Could not determine path for parent cgroup: %s\n", strerror(-r));
-		return r;
-	}
-
-	// Open the cgroup
-	return pakfire_cgroup_create(parent, cgroup->ctx, path, 0);
+	return cgroup->name;
 }
 
 static int pakfire_cgroup_setup_devices(struct pakfire_cgroup* cgroup) {
@@ -161,8 +142,7 @@ static int pakfire_cgroup_open_root(struct pakfire_cgroup* cgroup) {
 
 	This function returns a negative value on error.
 */
-static int pakfire_cgroup_open(struct pakfire_cgroup* cgroup) {
-	char basename[NAME_MAX];
+static int __pakfire_cgroup_open(struct pakfire_cgroup* cgroup) {
 	int fd = -EBADF;
 	int r;
 
@@ -170,27 +150,22 @@ static int pakfire_cgroup_open(struct pakfire_cgroup* cgroup) {
 	if (!cgroup->parent)
 		return pakfire_cgroup_open_root(cgroup);
 
-	// Get the basename
-	r = pakfire_path_basename(basename, cgroup->path);
-	if (r < 0)
-		return r;
-
 	// Try to open the cgroup
-	fd = openat(cgroup->parent->fd, basename, O_DIRECTORY|O_PATH|O_CLOEXEC);
+	fd = openat(cgroup->parent->fd, cgroup->name, O_DIRECTORY|O_PATH|O_CLOEXEC);
 	if (fd < 0) {
 		switch (errno) {
 			case ENOENT:
-				r = mkdirat(cgroup->parent->fd, basename, 0755);
+				r = mkdirat(cgroup->parent->fd, cgroup->name, 0755);
 				if (r < 0) {
-					ERROR(cgroup->ctx, "Could not create cgroup '%s': %m\n", basename);
+					ERROR(cgroup->ctx, "Could not create cgroup '%s': %m\n", cgroup->name);
 					return -errno;
 				}
 
 				// Try opening it again
-				return pakfire_cgroup_open(cgroup);
+				return __pakfire_cgroup_open(cgroup);
 
 			default:
-				ERROR(cgroup->ctx, "Could not open cgroup '%s': %m\n", basename);
+				ERROR(cgroup->ctx, "Could not open cgroup '%s': %m\n", cgroup->name);
 				return -errno;
 		}
 	}
@@ -284,39 +259,24 @@ ERROR:
 	Enables a cgroup controller.
 */
 static int pakfire_cgroup_enable_controller(struct pakfire_cgroup* cgroup, const char* name) {
-	struct pakfire_cgroup* parent = NULL;
 	int r;
 
+	// Ensure this controller is enabled on the parent, too
+	if (cgroup->parent) {
+		r = pakfire_cgroup_enable_controller(cgroup->parent, name);
+		if (r < 0)
+			return r;
+	}
+
 	// Try to enable the controller
 	r = pakfire_cgroup_write(cgroup, "cgroup.subtree_control", "+%s", name);
 	if (r < 0) {
-		switch (-r) {
-			case ENOENT:
-				// Fetch the parent group
-				r = pakfire_cgroup_parent(cgroup, &parent);
-				if (r < 0)
-					return r;
-
-				// When we have reached root, we cannot continue
-				if (!parent)
-					return -ENOTSUP;
-
-				// Ensure it is enabled there
-				r = pakfire_cgroup_enable_controller(parent, name);
-				if (r < 0)
-					return 0;
-
-				// Once that was successful, try again
-				return pakfire_cgroup_enable_controller(cgroup, name);
-
-			default:
-				ERROR(cgroup->ctx, "Could not enable controller '%s': %s\n",
-					name, strerror(-r));
-				break;
-		}
+		ERROR(cgroup->ctx, "Could not enable controller '%s': %s\n",
+			name, strerror(-r));
+		return r;
 	}
 
-	return r;
+	return 0;
 }
 
 /*
@@ -343,8 +303,8 @@ static int pakfire_cgroup_enable_controllers(struct pakfire_cgroup* cgroup) {
 
 	If the cgroup doesn't exist, it will be created including any parent cgroups.
 */
-int pakfire_cgroup_create(struct pakfire_cgroup** cgroup,
-		struct pakfire_ctx* ctx, const char* path, int flags) {
+static int pakfire_cgroup_open(struct pakfire_cgroup** cgroup,
+		struct pakfire_ctx* ctx, struct pakfire_cgroup* parent, const char* name, int flags) {
 	struct pakfire_cgroup* c = NULL;
 	int r;
 
@@ -362,21 +322,20 @@ int pakfire_cgroup_create(struct pakfire_cgroup** cgroup,
 	// Initialize file descriptors
 	c->fd = c->devicesfd = -EBADF;
 
-	// Copy path
-	r = pakfire_string_set(c->path, path);
+	// Store the parent
+	if (parent)
+		c->parent = pakfire_cgroup_ref(parent);
+
+	// Store name
+	r = pakfire_string_set(c->name, name);
 	if (r < 0)
 		goto ERROR;
 
 	// Copy flags
 	c->flags = flags;
 
-	// Find the parent cgroup
-	r = pakfire_cgroup_parent(c, &c->parent);
-	if (r < 0)
-		goto ERROR;
-
 	// Open a file descriptor
-	c->fd = r = pakfire_cgroup_open(c);
+	c->fd = r = __pakfire_cgroup_open(c);
 	if (r < 0)
 		goto ERROR;
 
@@ -402,6 +361,76 @@ ERROR:
 	return r;
 }
 
+static int pakfire_cgroup_create_recursive(struct pakfire_cgroup** cgroup,
+		struct pakfire_ctx* ctx, struct pakfire_cgroup* parent, const char* path, int flags) {
+	struct pakfire_cgroup* child = NULL;
+	char buffer[PATH_MAX];
+	char* name = NULL;
+	char* p = NULL;
+	int r;
+
+	// Copy the path to the buffer
+	r = pakfire_string_set(buffer, path);
+	if (r < 0)
+		goto ERROR;
+
+	// Split the path by /
+	name = strtok_r(buffer, "/", &p);
+
+	// Walk through all elements of the path
+	while (name) {
+		r = pakfire_cgroup_open(&child, parent->ctx, parent, name, flags);
+		if (r < 0)
+			goto ERROR;
+
+		// Move on to the next round...
+		pakfire_cgroup_unref(parent);
+		parent = child; child = NULL;
+
+		name = strtok_r(NULL, "/", &p);
+	}
+
+	// Return the pointer
+	*cgroup = pakfire_cgroup_ref(parent);
+
+ERROR:
+	// XXX We may free the passed parent here if we did zero iterations of the while loop
+	if (parent)
+		pakfire_cgroup_unref(parent);
+
+	return r;
+}
+
+int pakfire_cgroup_create(struct pakfire_cgroup** cgroup,
+		struct pakfire_ctx* ctx, struct pakfire_cgroup* parent, const char* name, int flags) {
+	struct pakfire_cgroup* root = NULL;
+	int r;
+
+	// If parent is NULL, but name is set, we will create the root group and use it as parent
+	if (name && !parent) {
+		r = pakfire_cgroup_open(&root, ctx, NULL, NULL, 0);
+		if (r < 0)
+			return r;
+
+		parent = root;
+	}
+
+	// Recursively create the new cgroup
+	//r = pakfire_cgroup_create_recursive(cgroup, ctx, parent, name, flags);
+	r = pakfire_cgroup_open(cgroup, ctx, parent, name, flags);
+
+	// Cleanup
+	if (root)
+		pakfire_cgroup_unref(root);
+
+	return r;
+}
+
+int pakfire_cgroup_create_root(
+		struct pakfire_cgroup** cgroup, struct pakfire_ctx* ctx, int flags) {
+	return pakfire_cgroup_open(cgroup, ctx, NULL, NULL, flags);
+}
+
 static void pakfire_cgroup_free(struct pakfire_cgroup* cgroup) {
 	if (cgroup->fd >= 0)
 		close(cgroup->fd);
@@ -428,25 +457,6 @@ struct pakfire_cgroup* pakfire_cgroup_unref(struct pakfire_cgroup* cgroup) {
 	return NULL;
 }
 
-// Open a child cgroup
-int pakfire_cgroup_child(struct pakfire_cgroup** child,
-		struct pakfire_cgroup* cgroup, const char* path, int flags) {
-	char p[PATH_MAX];
-	int r;
-
-	// Check input
-	if (!path)
-		return -EINVAL;
-
-	// Join paths
-	r = pakfire_path_append(p, cgroup->path, path);
-	if (r < 0)
-		return r;
-
-	// Open the child group
-	return pakfire_cgroup_create(child, cgroup->ctx, p, flags);
-}
-
 /*
 	Immediately kills all processes in this cgroup
 */
@@ -479,19 +489,11 @@ int pakfire_cgroup_destroy(struct pakfire_cgroup* cgroup) {
 		cgroup->fd = -EBADF;
 	}
 
-	// Open the root directory
-	int fd = pakfire_cgroup_open_root(cgroup);
-	if (fd < 0)
-		return 1;
-
 	// Delete the directory
-	r = unlinkat(fd, cgroup->path, AT_REMOVEDIR);
+	r = unlinkat(cgroup->parent->fd, cgroup->name, AT_REMOVEDIR);
 	if (r)
 		ERROR(cgroup->ctx, "Could not destroy cgroup: %m\n");
 
-	// Close fd
-	close(fd);
-
 	return r;
 }
 
diff --git a/src/libpakfire/daemon.c b/src/libpakfire/daemon.c
index f7f2b099e..2363d4c4a 100644
--- a/src/libpakfire/daemon.c
+++ b/src/libpakfire/daemon.c
@@ -721,7 +721,7 @@ int pakfire_daemon_create(struct pakfire_daemon** daemon, struct pakfire_ctx* ct
 		goto ERROR;
 
 	// Create the cgroup
-	r = pakfire_cgroup_create(&d->cgroup, d->ctx, "pakfire-daemon", 0);
+	r = pakfire_cgroup_create(&d->cgroup, d->ctx, NULL, "pakfire-daemon", 0);
 	if (r < 0)
 		goto ERROR;
 
diff --git a/src/libpakfire/include/pakfire/cgroup.h b/src/libpakfire/include/pakfire/cgroup.h
index 05023312e..b4100a48b 100644
--- a/src/libpakfire/include/pakfire/cgroup.h
+++ b/src/libpakfire/include/pakfire/cgroup.h
@@ -196,7 +196,10 @@ struct pakfire_cgroup_stats {
 };
 
 int pakfire_cgroup_create(struct pakfire_cgroup** cgroup,
-	struct pakfire_ctx* ctx, const char* path, int flags);
+	struct pakfire_ctx* ctx, struct pakfire_cgroup* parent, const char* name, int flags);
+
+int pakfire_cgroup_create_root(
+	struct pakfire_cgroup** cgroup, struct pakfire_ctx* ctx, int flags);
 
 struct pakfire_cgroup* pakfire_cgroup_ref(struct pakfire_cgroup* cgroup);
 struct pakfire_cgroup* pakfire_cgroup_unref(struct pakfire_cgroup* cgroup);
diff --git a/src/libpakfire/jail.c b/src/libpakfire/jail.c
index 31cee655d..1260dd779 100644
--- a/src/libpakfire/jail.c
+++ b/src/libpakfire/jail.c
@@ -1506,9 +1506,9 @@ static int __pakfire_jail_exec(struct pakfire_jail* jail,
 		const char* uuid = pakfire_jail_uuid(jail);
 
 		// Create a temporary cgroup
-		r = pakfire_cgroup_child(&ctx.cgroup, jail->cgroup, uuid, 0);
-		if (r) {
-			ERROR(jail->ctx, "Could not create cgroup for jail: %m\n");
+		r = pakfire_cgroup_create(&ctx.cgroup, jail->ctx, jail->cgroup, uuid, 0);
+		if (r < 0) {
+			ERROR(jail->ctx, "Could not create cgroup for jail: %s\n", strerror(-r));
 			goto ERROR;
 		}
 
diff --git a/tests/libpakfire/cgroup.c b/tests/libpakfire/cgroup.c
index 3e99e0558..549f89990 100644
--- a/tests/libpakfire/cgroup.c
+++ b/tests/libpakfire/cgroup.c
@@ -30,7 +30,7 @@ static int test_create_and_destroy(const struct test* t) {
 	int r = EXIT_FAILURE;
 
 	// Open a new cgroup
-	ASSERT_SUCCESS(pakfire_cgroup_create(&cgroup, t->ctx, "pakfire-test", 0));
+	ASSERT_SUCCESS(pakfire_cgroup_create(&cgroup, t->ctx, NULL, "pakfire-test", 0));
 
 	// Destroy the cgroup again
 	ASSERT_SUCCESS(pakfire_cgroup_destroy(cgroup));
@@ -57,7 +57,7 @@ static int test_stats(const struct test* t) {
 	};
 
 	// Open the cgroup
-	ASSERT_SUCCESS(pakfire_cgroup_create(&cgroup, t->ctx, "pakfire-test", 0));
+	ASSERT_SUCCESS(pakfire_cgroup_create(&cgroup, t->ctx, NULL, "pakfire-test", 0));
 
 	// Create a new jail
 	ASSERT_SUCCESS(pakfire_jail_create(&jail, t->pakfire));