From: Chris Down <chris@chrisdown.name>
Date: Wed, 19 Dec 2018 03:33:53 +0000 (+0000)
Subject: cgroup: Imply systemd.unified_cgroup_hierarchy=1 on cgroup_no_v1=all
X-Git-Tag: v240~3^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5f086dc7db539974535ad093abdd32d47fbb035d;p=thirdparty%2Fsystemd.git

cgroup: Imply systemd.unified_cgroup_hierarchy=1 on cgroup_no_v1=all

cgroup_no_v1=all doesn't make a whole lot of sense with legacy hierarchy
(where we use v1 hierarchy for everything), or hybrid hierarchy (where
we still use v1 hierarchy for resource control).

Right now we have to tell people to add both cgroup_no_v1=all and
systemd.unified_cgroup_hierarchy=1 to get the desired behaviour,
however in reality it's hard to imagine any situation where someone
passes cgroup_no_v1=all but *doesn't* want to use the unified cgroup
hierarchy.

Make it so that cgroup_no_v1=all produces intuitive behaviour in systemd
by default, although it can still be disabled by passing
systemd.unified_cgroup_hierarchy=0 explicitly.
---

diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c
index ec29a6f4c97..830a63c1850 100644
--- a/src/basic/cgroup-util.c
+++ b/src/basic/cgroup-util.c
@@ -2706,6 +2706,7 @@ bool cg_is_unified_wanted(void) {
         int r;
         bool b;
         const bool is_default = DEFAULT_HIERARCHY == CGROUP_UNIFIED_ALL;
+        _cleanup_free_ char *c = NULL;
 
         /* If we have a cached value, return that. */
         if (wanted >= 0)
@@ -2716,11 +2717,19 @@ bool cg_is_unified_wanted(void) {
         if (cg_unified_flush() >= 0)
                 return (wanted = unified_cache >= CGROUP_UNIFIED_ALL);
 
-        /* Otherwise, let's see what the kernel command line has to say.
-         * Since checking is expensive, cache a non-error result. */
+        /* If we were explicitly passed systemd.unified_cgroup_hierarchy,
+         * respect that. */
         r = proc_cmdline_get_bool("systemd.unified_cgroup_hierarchy", &b);
+        if (r > 0)
+                return (wanted = b);
+
+        /* If we passed cgroup_no_v1=all with no other instructions, it seems
+         * highly unlikely that we want to use hybrid or legacy hierarchy. */
+        r = proc_cmdline_get_key("cgroup_no_v1", 0, &c);
+        if (r > 0 && streq_ptr(c, "all"))
+                return (wanted = true);
 
-        return (wanted = r > 0 ? b : is_default);
+        return (wanted = is_default);
 }
 
 bool cg_is_legacy_wanted(void) {
diff --git a/src/test/test-cgroup-util.c b/src/test/test-cgroup-util.c
index 058cc44c42a..a3239d73f5e 100644
--- a/src/test/test-cgroup-util.c
+++ b/src/test/test-cgroup-util.c
@@ -369,6 +369,17 @@ static void test_is_wanted(void) {
                          "systemd.unified_cgroup_hierarchy=0 "
                          "systemd.legacy_systemd_cgroup_controller=0", 1) >= 0);
         test_is_wanted_print(false);
+
+        /* cgroup_no_v1=all implies unified cgroup hierarchy, unless otherwise
+         * explicitly specified. */
+        assert_se(setenv("SYSTEMD_PROC_CMDLINE",
+                         "cgroup_no_v1=all", 1) >= 0);
+        test_is_wanted_print(false);
+
+        assert_se(setenv("SYSTEMD_PROC_CMDLINE",
+                         "cgroup_no_v1=all "
+                         "systemd.unified_cgroup_hierarchy=0", 1) >= 0);
+        test_is_wanted_print(false);
 }
 
 static void test_cg_tests(void) {