From: Chris Down <chris@chrisdown.name>
Date: Mon, 18 Dec 2017 07:53:29 +0000 (+0000)
Subject: condition: Create AssertControlGroupController (#7630)
X-Git-Tag: v237~208
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=e16647c39d195804711a006667d5bce49c0ef73d;p=thirdparty%2Fsystemd.git

condition: Create AssertControlGroupController (#7630)

Up until now, the behaviour in systemd has (mostly) been to silently
ignore failures to action unit directives that refer to an unavailble
controller. The addition of AssertControlGroupController and its
conditional counterpart allow explicit specification of the desired
behaviour when such a situation occurs.

As for how this can happen, it is possible that a particular controller
is not available in the cgroup hierarchy. One possible reason for this
is that, in the running kernel, the controller simply doesn't exist --
for example, the CPU controller in cgroup v2 has only recently been
merged and was out of tree until then. Another possibility is that the
controller exists, but has been forcibly disabled by `cgroup_disable=`
on the kernel command line.

In future this will also support whatever comes out of issue #7624,
`DefaultXAccounting=never`, or similar.
---

diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml
index 445c26f3e50..005fdea73cd 100644
--- a/man/systemd.unit.xml
+++ b/man/systemd.unit.xml
@@ -939,6 +939,7 @@
         <term><varname>ConditionFileIsExecutable=</varname></term>
         <term><varname>ConditionUser=</varname></term>
         <term><varname>ConditionGroup=</varname></term>
+        <term><varname>ConditionControlGroupController=</varname></term>
 
         <!-- We do not document ConditionNull=
              here, as it is not particularly
@@ -1166,6 +1167,18 @@
         auxiliary groups match the specified group or GID. This setting
         does not have a special value <literal>@system</literal>.</para>
 
+        <para><varname>ConditionControlGroupController=</varname> takes a
+        cgroup controller name (eg. <option>cpu</option>), verifying that it is
+        available for use on the system. For example, a particular controller
+        may not be available if it was disabled on the kernel command line with
+        <literal>cgroup_disable=</literal><replaceable>controller</replaceable>.
+        Multiple controllers may be passed with a space separating them; in
+        this case the condition will only pass if all listed controllers are
+        available for use. Controllers unknown to systemd are ignored. Valid
+        controllers are <option>cpu</option>, <option>cpuacct</option>,
+        <option>io</option>, <option>blkio</option>, <option>memory</option>,
+        <option>devices</option>, and <option>pids</option>.</para>
+
         <para>If multiple conditions are specified, the unit will be
         executed if all of them apply (i.e. a logical AND is applied).
         Condition checks can be prefixed with a pipe symbol (|) in
@@ -1204,6 +1217,7 @@
         <term><varname>AssertFileIsExecutable=</varname></term>
         <term><varname>AssertUser=</varname></term>
         <term><varname>AssertGroup=</varname></term>
+        <term><varname>AssertControlGroupController=</varname></term>
 
         <listitem><para>Similar to the <varname>ConditionArchitecture=</varname>,
         <varname>ConditionVirtualization=</varname>, …, condition settings described above, these settings add
diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4
index 240f3317780..5604312bc5e 100644
--- a/src/core/load-fragment-gperf.gperf.m4
+++ b/src/core/load-fragment-gperf.gperf.m4
@@ -249,6 +249,7 @@ Unit.ConditionHost,              config_parse_unit_condition_string, CONDITION_H
 Unit.ConditionACPower,           config_parse_unit_condition_string, CONDITION_AC_POWER,            offsetof(Unit, conditions)
 Unit.ConditionUser,              config_parse_unit_condition_string, CONDITION_USER,                offsetof(Unit, conditions)
 Unit.ConditionGroup,             config_parse_unit_condition_string, CONDITION_GROUP,               offsetof(Unit, conditions)
+Unit.ConditionControlGroupController,  config_parse_unit_condition_string, CONDITION_CONTROL_GROUP_CONTROLLER,   offsetof(Unit, conditions)
 Unit.ConditionNull,              config_parse_unit_condition_null,   0,                             offsetof(Unit, conditions)
 Unit.AssertPathExists,           config_parse_unit_condition_path,   CONDITION_PATH_EXISTS,         offsetof(Unit, asserts)
 Unit.AssertPathExistsGlob,       config_parse_unit_condition_path,   CONDITION_PATH_EXISTS_GLOB,    offsetof(Unit, asserts)
@@ -270,6 +271,7 @@ Unit.AssertHost,                 config_parse_unit_condition_string, CONDITION_H
 Unit.AssertACPower,              config_parse_unit_condition_string, CONDITION_AC_POWER,            offsetof(Unit, asserts)
 Unit.AssertUser,                 config_parse_unit_condition_string, CONDITION_USER,                offsetof(Unit, asserts)
 Unit.AssertGroup,                config_parse_unit_condition_string, CONDITION_GROUP,               offsetof(Unit, asserts)
+Unit.AssertControlGroupController,     config_parse_unit_condition_string, CONDITION_CONTROL_GROUP_CONTROLLER,   offsetof(Unit, asserts)
 Unit.AssertNull,                 config_parse_unit_condition_null,   0,                             offsetof(Unit, asserts)
 Unit.CollectMode,                config_parse_collect_mode,          0,                             offsetof(Unit, collect_mode)
 m4_dnl
diff --git a/src/shared/condition.c b/src/shared/condition.c
index 3f32dfb7b67..d4bbaf3c65d 100644
--- a/src/shared/condition.c
+++ b/src/shared/condition.c
@@ -36,6 +36,7 @@
 #include "architecture.h"
 #include "audit-util.h"
 #include "cap-list.h"
+#include "cgroup-util.h"
 #include "condition.h"
 #include "extract-word.h"
 #include "fd-util.h"
@@ -177,6 +178,30 @@ static int condition_test_user(Condition *c) {
         return id == getuid() || id == geteuid();
 }
 
+static int condition_test_control_group_controller(Condition *c) {
+        int r;
+        CGroupMask system_mask, wanted_mask = 0;
+
+        assert(c);
+        assert(c->parameter);
+        assert(c->type == CONDITION_CONTROL_GROUP_CONTROLLER);
+
+        r = cg_mask_supported(&system_mask);
+        if (r < 0)
+                return log_debug_errno(r, "Failed to determine supported controllers: %m");
+
+        r = cg_mask_from_string(c->parameter, &wanted_mask);
+        if (r < 0 || wanted_mask <= 0) {
+                /* This won't catch the case that we have an unknown controller
+                 * mixed in with valid ones -- these are only assessed on the
+                 * validity of the valid controllers found. */
+                log_debug("Failed to parse cgroup string: %s", c->parameter);
+                return 1;
+        }
+
+        return (system_mask & wanted_mask) == wanted_mask;
+}
+
 static int condition_test_group(Condition *c) {
         gid_t id;
         int r;
@@ -537,6 +562,7 @@ int condition_test(Condition *c) {
                 [CONDITION_FIRST_BOOT] = condition_test_first_boot,
                 [CONDITION_USER] = condition_test_user,
                 [CONDITION_GROUP] = condition_test_group,
+                [CONDITION_CONTROL_GROUP_CONTROLLER] = condition_test_control_group_controller,
                 [CONDITION_NULL] = condition_test_null,
         };
 
@@ -602,6 +628,7 @@ static const char* const condition_type_table[_CONDITION_TYPE_MAX] = {
         [CONDITION_FILE_IS_EXECUTABLE] = "ConditionFileIsExecutable",
         [CONDITION_USER] = "ConditionUser",
         [CONDITION_GROUP] = "ConditionGroup",
+        [CONDITION_CONTROL_GROUP_CONTROLLER] = "ConditionControlGroupController",
         [CONDITION_NULL] = "ConditionNull"
 };
 
@@ -628,6 +655,7 @@ static const char* const assert_type_table[_CONDITION_TYPE_MAX] = {
         [CONDITION_FILE_IS_EXECUTABLE] = "AssertFileIsExecutable",
         [CONDITION_USER] = "AssertUser",
         [CONDITION_GROUP] = "AssertGroup",
+        [CONDITION_CONTROL_GROUP_CONTROLLER] = "AssertControlGroupController",
         [CONDITION_NULL] = "AssertNull"
 };
 
diff --git a/src/shared/condition.h b/src/shared/condition.h
index 534906b6d6d..715866be704 100644
--- a/src/shared/condition.h
+++ b/src/shared/condition.h
@@ -53,6 +53,8 @@ typedef enum ConditionType {
         CONDITION_USER,
         CONDITION_GROUP,
 
+        CONDITION_CONTROL_GROUP_CONTROLLER,
+
         _CONDITION_TYPE_MAX,
         _CONDITION_TYPE_INVALID = -1
 } ConditionType;
diff --git a/src/test/test-condition.c b/src/test/test-condition.c
index d43db3a7cd6..bf455aac891 100644
--- a/src/test/test-condition.c
+++ b/src/test/test-condition.c
@@ -29,12 +29,14 @@
 #include "architecture.h"
 #include "audit-util.h"
 #include "condition.h"
+#include "cgroup-util.h"
 #include "hostname-util.h"
 #include "id128-util.h"
 #include "ima-util.h"
 #include "log.h"
 #include "macro.h"
 #include "selinux-util.h"
+#include "set.h"
 #include "smack-util.h"
 #include "strv.h"
 #include "virt.h"
@@ -125,6 +127,77 @@ static void test_condition_test_path(void) {
         condition_free(condition);
 }
 
+static int test_condition_test_control_group_controller(void) {
+        Condition *condition;
+        CGroupMask system_mask;
+        CGroupController controller;
+        _cleanup_free_ char *controller_name = NULL;
+        int r;
+
+        r = cg_unified_flush();
+        if (r < 0) {
+                log_notice_errno(r, "Skipping ConditionControlGroupController tests: %m");
+                return EXIT_TEST_SKIP;
+        }
+
+        /* Invalid controllers are ignored */
+        condition = condition_new(CONDITION_CONTROL_GROUP_CONTROLLER, "thisisnotarealcontroller", false, false);
+        assert_se(condition);
+        assert_se(condition_test(condition));
+        condition_free(condition);
+
+        condition = condition_new(CONDITION_CONTROL_GROUP_CONTROLLER, "thisisnotarealcontroller", false, true);
+        assert_se(condition);
+        assert_se(!condition_test(condition));
+        condition_free(condition);
+
+        assert_se(cg_mask_supported(&system_mask) >= 0);
+
+        /* Individual valid controllers one by one */
+        for (controller = 0; controller < _CGROUP_CONTROLLER_MAX; controller++) {
+                const char *local_controller_name = cgroup_controller_to_string(controller);
+                log_info("chosen controller is '%s'", local_controller_name);
+                if (system_mask & CGROUP_CONTROLLER_TO_MASK(controller)) {
+                        log_info("this controller is available");
+                        condition = condition_new(CONDITION_CONTROL_GROUP_CONTROLLER, local_controller_name, false, false);
+                        assert_se(condition);
+                        assert_se(condition_test(condition));
+                        condition_free(condition);
+
+                        condition = condition_new(CONDITION_CONTROL_GROUP_CONTROLLER, local_controller_name, false, true);
+                        assert_se(condition);
+                        assert_se(!condition_test(condition));
+                        condition_free(condition);
+                } else {
+                        log_info("this controller is unavailable");
+                        condition = condition_new(CONDITION_CONTROL_GROUP_CONTROLLER, local_controller_name, false, false);
+                        assert_se(condition);
+                        assert_se(!condition_test(condition));
+                        condition_free(condition);
+
+                        condition = condition_new(CONDITION_CONTROL_GROUP_CONTROLLER, local_controller_name, false, true);
+                        assert_se(condition);
+                        assert_se(condition_test(condition));
+                        condition_free(condition);
+                }
+        }
+
+        /* Multiple valid controllers at the same time */
+        assert_se(cg_mask_to_string(system_mask, &controller_name) >= 0);
+
+        condition = condition_new(CONDITION_CONTROL_GROUP_CONTROLLER, controller_name, false, false);
+        assert_se(condition);
+        assert_se(condition_test(condition));
+        condition_free(condition);
+
+        condition = condition_new(CONDITION_CONTROL_GROUP_CONTROLLER, controller_name, false, true);
+        assert_se(condition);
+        assert_se(!condition_test(condition));
+        condition_free(condition);
+
+        return EXIT_SUCCESS;
+}
+
 static void test_condition_test_ac_power(void) {
         Condition *condition;
 
@@ -488,6 +561,7 @@ int main(int argc, char *argv[]) {
         test_condition_test_virtualization();
         test_condition_test_user();
         test_condition_test_group();
+        test_condition_test_control_group_controller();
 
         return 0;
 }