From 56c8d7444ac212c48443a0d30e42ea279140f96d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Fri, 16 Feb 2018 10:17:46 +0100
Subject: [PATCH] pid1: do not initialize join_controllers by default

We're moving towards unified cgroup hierarchy where this is not necessary.
This makes main.c a bit simpler.
---
 src/core/main.c             | 32 --------------------------------
 src/core/mount-setup.c      | 20 ++++++++++++++++----
 src/shared/conf-parser.c    | 11 +++++++++++
 src/test/test-conf-parser.c |  4 +++-
 4 files changed, 30 insertions(+), 37 deletions(-)

diff --git a/src/core/main.c b/src/core/main.c
index 44ebf4f1da..bdb7bbf221 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -1213,32 +1213,6 @@ static void test_usr(void) {
                     "Consult http://freedesktop.org/wiki/Software/systemd/separate-usr-is-broken for more information.");
 }
 
-static int initialize_join_controllers(void) {
-        /* By default, mount "cpu" + "cpuacct" together, and "net_cls"
-         * + "net_prio". We'd like to add "cpuset" to the mix, but
-         * "cpuset" doesn't really work for groups with no initialized
-         * attributes. */
-
-        arg_join_controllers = new(char**, 3);
-        if (!arg_join_controllers)
-                return -ENOMEM;
-
-        arg_join_controllers[0] = strv_new("cpu", "cpuacct", NULL);
-        if (!arg_join_controllers[0])
-                goto oom;
-
-        arg_join_controllers[1] = strv_new("net_cls", "net_prio", NULL);
-        if (!arg_join_controllers[1])
-                goto oom;
-
-        arg_join_controllers[2] = NULL;
-        return 0;
-
-oom:
-        arg_join_controllers = strv_free_free(arg_join_controllers);
-        return -ENOMEM;
-}
-
 static int enforce_syscall_archs(Set *archs) {
 #if HAVE_SECCOMP
         int r;
@@ -1993,12 +1967,6 @@ static int load_configuration(int argc, char **argv, const char **ret_error_mess
 
         assert(ret_error_message);
 
-        r = initialize_join_controllers();
-        if (r < 0) {
-                *ret_error_message = "Failed to initialize cgroup controller joining table";
-                return r;
-        }
-
         arg_default_tasks_max = system_tasks_max_scale(DEFAULT_TASKS_MAX_PERCENTAGE, 100U);
 
         r = parse_config_file();
diff --git a/src/core/mount-setup.c b/src/core/mount-setup.c
index a0c5f5aaae..ed78c945d1 100644
--- a/src/core/mount-setup.c
+++ b/src/core/mount-setup.c
@@ -253,6 +253,19 @@ int mount_cgroup_controllers(char ***join_controllers) {
 
         /* Mount all available cgroup controllers that are built into the kernel. */
 
+        if (!join_controllers)
+                /* The defaults:
+                 * mount "cpu" + "cpuacct" together, and "net_cls" + "net_prio".
+                 *
+                 * We'd like to add "cpuset" to the mix, but "cpuset" doesn't really
+                 * work for groups with no initialized attributes.
+                 */
+                join_controllers = (char**[]) {
+                        STRV_MAKE("cpu", "cpuacct"),
+                        STRV_MAKE("net_cls", "net_prio"),
+                        NULL,
+                };
+
         r = cg_kernel_controllers(&controllers);
         if (r < 0)
                 return log_error_errno(r, "Failed to enumerate cgroup controllers: %m");
@@ -271,10 +284,9 @@ int mount_cgroup_controllers(char ***join_controllers) {
                 if (!controller)
                         break;
 
-                if (join_controllers)
-                        for (k = join_controllers; *k; k++)
-                                if (strv_find(*k, controller))
-                                        break;
+                for (k = join_controllers; *k; k++)
+                        if (strv_find(*k, controller))
+                                break;
 
                 if (k && *k) {
                         char **i, **j;
diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c
index 5c033e0ec0..15cfe4e4f7 100644
--- a/src/shared/conf-parser.c
+++ b/src/shared/conf-parser.c
@@ -1120,6 +1120,17 @@ int config_parse_join_controllers(
         if (!isempty(rvalue))
                 log_syntax(unit, LOG_ERR, filename, line, 0, "Trailing garbage, ignoring.");
 
+        /* As a special case, return a single empty strv, to override the default */
+        if (!controllers) {
+                controllers = new(char**, 2);
+                if (!controllers)
+                        return log_oom();
+                controllers[0] = strv_new(NULL, NULL);
+                if (!controllers[0])
+                        return log_oom();
+                controllers[1] = NULL;
+        }
+
         strv_free_free(*ret);
         *ret = controllers;
         controllers = NULL;
diff --git a/src/test/test-conf-parser.c b/src/test/test-conf-parser.c
index 84789e3bb8..72c53ab6a0 100644
--- a/src/test/test-conf-parser.c
+++ b/src/test/test-conf-parser.c
@@ -244,7 +244,9 @@ static void test_config_parse_join_controllers(void) {
         /* Test special case of no mounted controllers */
         r = config_parse_join_controllers(NULL, "example.conf", 12, "Section", 10, "JoinControllers", 0, "", &c, NULL);
         assert_se(r == 0);
-        assert_se(c == NULL);
+        assert_se(c);
+        assert_se(strv_equal(c[0], STRV_MAKE_EMPTY));
+        assert_se(c[1] == NULL);
 
         /* Test merging of overlapping lists */
         r = config_parse_join_controllers(NULL, "example.conf", 13, "Section", 10, "JoinControllers", 0, "a,b b,c", &c, NULL);