From 4c41ad9418058aefb2d2732b0b65da9c7cdf5151 Mon Sep 17 00:00:00 2001
From: Ruixin Bao <rubao@redhat.com>
Date: Tue, 21 Aug 2018 20:40:56 +0000
Subject: [PATCH] install: allow instantiated units to be enabled via presets
This patch implements https://github.com/systemd/systemd/issues/9421.
The .preset file now is able to take a rule in the format of:(e.g)
enable foo@.service bar0 bar1 bar2
In the above example, when preset-all is called, all three instances of
foo@bar0.service, foo@bar1.service and foo@bar2.service will be enabled.
When preset is called on a single service(e.g: foo@bar1.service), only
the mentioned one(foo@bar1.service) will be enabled.
Tests are added for future regression.
(cherry picked from commit 4c9565eea534cd233a913c8c21f7920dba229743)
Resolves: #1812972
---
src/shared/install.c | 155 ++++++++++++++++++++++++++++++-----
src/test/test-install-root.c | 57 +++++++++++++
2 files changed, 193 insertions(+), 19 deletions(-)
diff --git a/src/shared/install.c b/src/shared/install.c
index 77ae812878..1d4beaa83b 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -60,6 +60,7 @@ typedef enum {
typedef struct {
char *pattern;
PresetAction action;
+ char **instances;
} PresetRule;
typedef struct {
@@ -87,8 +88,10 @@ static inline void presets_freep(Presets *p) {
if (!p)
return;
- for (i = 0; i < p->n_rules; i++)
+ for (i = 0; i < p->n_rules; i++) {
free(p->rules[i].pattern);
+ strv_free(p->rules[i].instances);
+ }
free(p->rules);
p->n_rules = 0;
@@ -2755,6 +2758,39 @@ int unit_file_exists(UnitFileScope scope, const LookupPaths *paths, const char *
return 1;
}
+static int split_pattern_into_name_and_instances(const char *pattern, char **out_unit_name, char ***out_instances) {
+ _cleanup_strv_free_ char **instances = NULL;
+ _cleanup_free_ char *unit_name = NULL;
+ int r;
+
+ assert(pattern);
+ assert(out_instances);
+ assert(out_unit_name);
+
+ r = extract_first_word(&pattern, &unit_name, NULL, 0);
+ if (r < 0)
+ return r;
+
+ /* We handle the instances logic when unit name is extracted */
+ if (pattern) {
+ /* We only create instances when a rule of templated unit
+ * is seen. A rule like enable foo@.service a b c will
+ * result in an array of (a, b, c) as instance names */
+ if (!unit_name_is_valid(unit_name, UNIT_NAME_TEMPLATE))
+ return -EINVAL;
+
+ instances = strv_split(pattern, WHITESPACE);
+ if (!instances)
+ return -ENOMEM;
+
+ *out_instances = TAKE_PTR(instances);
+ }
+
+ *out_unit_name = TAKE_PTR(unit_name);
+
+ return 0;
+}
+
static int read_presets(UnitFileScope scope, const char *root_dir, Presets *presets) {
_cleanup_(presets_freep) Presets ps = {};
size_t n_allocated = 0;
@@ -2824,15 +2860,20 @@ static int read_presets(UnitFileScope scope, const char *root_dir, Presets *pres
parameter = first_word(l, "enable");
if (parameter) {
- char *pattern;
+ char *unit_name;
+ char **instances = NULL;
- pattern = strdup(parameter);
- if (!pattern)
- return -ENOMEM;
+ /* Unit_name will remain the same as parameter when no instances are specified */
+ r = split_pattern_into_name_and_instances(parameter, &unit_name, &instances);
+ if (r < 0) {
+ log_syntax(NULL, LOG_WARNING, *p, n, 0, "Couldn't parse line '%s'. Ignoring.", line);
+ continue;
+ }
rule = (PresetRule) {
- .pattern = pattern,
+ .pattern = unit_name,
.action = PRESET_ENABLE,
+ .instances = instances,
};
}
@@ -2868,15 +2909,71 @@ static int read_presets(UnitFileScope scope, const char *root_dir, Presets *pres
return 0;
}
-static int query_presets(const char *name, const Presets presets) {
+static int pattern_match_multiple_instances(
+ const PresetRule rule,
+ const char *unit_name,
+ char ***ret) {
+
+ _cleanup_free_ char *templated_name = NULL;
+ int r;
+
+ /* If no ret is needed or the rule itself does not have instances
+ * initalized, we return not matching */
+ if (!ret || !rule.instances)
+ return 0;
+
+ r = unit_name_template(unit_name, &templated_name);
+ if (r < 0)
+ return r;
+ if (!streq(rule.pattern, templated_name))
+ return 0;
+
+ /* Compose a list of specified instances when unit name is a template */
+ if (unit_name_is_valid(unit_name, UNIT_NAME_TEMPLATE)) {
+ _cleanup_free_ char *prefix = NULL;
+ _cleanup_strv_free_ char **out_strv = NULL;
+ char **iter;
+
+ r = unit_name_to_prefix(unit_name, &prefix);
+ if (r < 0)
+ return r;
+
+ STRV_FOREACH(iter, rule.instances) {
+ _cleanup_free_ char *name = NULL;
+ r = unit_name_build(prefix, *iter, ".service", &name);
+ if (r < 0)
+ return r;
+ r = strv_extend(&out_strv, name);
+ if (r < 0)
+ return r;
+ }
+
+ *ret = TAKE_PTR(out_strv);
+ return 1;
+ } else {
+ /* We now know the input unit name is an instance name */
+ _cleanup_free_ char *instance_name = NULL;
+
+ r = unit_name_to_instance(unit_name, &instance_name);
+ if (r < 0)
+ return r;
+
+ if (strv_find(rule.instances, instance_name))
+ return 1;
+ }
+ return 0;
+}
+
+static int query_presets(const char *name, const Presets presets, char ***instance_name_list) {
PresetAction action = PRESET_UNKNOWN;
size_t i;
-
+ char **s;
if (!unit_name_is_valid(name, UNIT_NAME_ANY))
return -EINVAL;
for (i = 0; i < presets.n_rules; i++)
- if (fnmatch(presets.rules[i].pattern, name, FNM_NOESCAPE) == 0) {
+ if (pattern_match_multiple_instances(presets.rules[i], name, instance_name_list) > 0 ||
+ fnmatch(presets.rules[i].pattern, name, FNM_NOESCAPE) == 0) {
action = presets.rules[i].action;
break;
}
@@ -2886,7 +2983,11 @@ static int query_presets(const char *name, const Presets presets) {
log_debug("Preset files don't specify rule for %s. Enabling.", name);
return 1;
case PRESET_ENABLE:
- log_debug("Preset files say enable %s.", name);
+ if (instance_name_list && *instance_name_list)
+ STRV_FOREACH(s, *instance_name_list)
+ log_debug("Preset files say enable %s.", *s);
+ else
+ log_debug("Preset files say enable %s.", name);
return 1;
case PRESET_DISABLE:
log_debug("Preset files say disable %s.", name);
@@ -2904,7 +3005,7 @@ int unit_file_query_preset(UnitFileScope scope, const char *root_dir, const char
if (r < 0)
return r;
- return query_presets(name, presets);
+ return query_presets(name, presets, NULL);
}
static int execute_preset(
@@ -2964,6 +3065,7 @@ static int preset_prepare_one(
size_t *n_changes) {
_cleanup_(install_context_done) InstallContext tmp = {};
+ _cleanup_strv_free_ char **instance_name_list = NULL;
UnitFileInstallInfo *i;
int r;
@@ -2979,19 +3081,34 @@ static int preset_prepare_one(
return 0;
}
- r = query_presets(name, presets);
+ r = query_presets(name, presets, &instance_name_list);
if (r < 0)
return r;
if (r > 0) {
- r = install_info_discover(scope, plus, paths, name, SEARCH_LOAD|SEARCH_FOLLOW_CONFIG_SYMLINKS,
- &i, changes, n_changes);
- if (r < 0)
- return r;
+ if (instance_name_list) {
+ char **s;
+ STRV_FOREACH(s, instance_name_list) {
+ r = install_info_discover(scope, plus, paths, *s, SEARCH_LOAD|SEARCH_FOLLOW_CONFIG_SYMLINKS,
+ &i, changes, n_changes);
+ if (r < 0)
+ return r;
+
+ r = install_info_may_process(i, paths, changes, n_changes);
+ if (r < 0)
+ return r;
+ }
+ } else {
+ r = install_info_discover(scope, plus, paths, name, SEARCH_LOAD|SEARCH_FOLLOW_CONFIG_SYMLINKS,
+ &i, changes, n_changes);
+ if (r < 0)
+ return r;
+
+ r = install_info_may_process(i, paths, changes, n_changes);
+ if (r < 0)
+ return r;
+ }
- r = install_info_may_process(i, paths, changes, n_changes);
- if (r < 0)
- return r;
} else
r = install_info_discover(scope, minus, paths, name, SEARCH_FOLLOW_CONFIG_SYMLINKS,
&i, changes, n_changes);
diff --git a/src/test/test-install-root.c b/src/test/test-install-root.c
index 15dd3c6966..dbbcfe4297 100644
--- a/src/test/test-install-root.c
+++ b/src/test/test-install-root.c
@@ -983,6 +983,62 @@ static void test_with_dropin_template(const char *root) {
assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "with-dropin-3@instance-2.service", &state) >= 0 && state == UNIT_FILE_ENABLED);
}
+static void test_preset_multiple_instances(const char *root) {
+ UnitFileChange *changes = NULL;
+ size_t n_changes = 0;
+ const char *p;
+ UnitFileState state;
+
+ /* Set up template service files and preset file */
+ p = strjoina(root, "/usr/lib/systemd/system/foo@.service");
+ assert_se(write_string_file(p,
+ "[Install]\n"
+ "DefaultInstance=def\n"
+ "WantedBy=multi-user.target\n", WRITE_STRING_FILE_CREATE) >= 0);
+
+ assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "foo@.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
+
+ p = strjoina(root, "/usr/lib/systemd/system-preset/test.preset");
+ assert_se(write_string_file(p,
+ "enable foo@.service bar0 bar1 bartest\n"
+ "enable emptylist@.service\n" /* This line ensures the old functionality for templated unit still works */
+ "disable *\n" , WRITE_STRING_FILE_CREATE) >= 0);
+
+ assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "foo@bar0.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
+
+ /* Preset a single instantiated unit specified in the list */
+ assert_se(unit_file_preset(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("foo@bar0.service"), UNIT_FILE_PRESET_FULL, &changes, &n_changes) >= 0);
+ assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "foo@bar0.service", &state) >= 0 && state == UNIT_FILE_ENABLED);
+ assert_se(n_changes == 1);
+ assert_se(changes[0].type == UNIT_FILE_SYMLINK);
+ p = strjoina(root, SYSTEM_CONFIG_UNIT_PATH"/multi-user.target.wants/foo@bar0.service");
+ assert_se(streq(changes[0].path, p));
+ unit_file_changes_free(changes, n_changes);
+ changes = NULL; n_changes = 0;
+
+ assert_se(unit_file_disable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("foo@bar0.service"), &changes, &n_changes) >= 0);
+ assert_se(n_changes == 1);
+ assert_se(changes[0].type == UNIT_FILE_UNLINK);
+ p = strjoina(root, SYSTEM_CONFIG_UNIT_PATH"/multi-user.target.wants/foo@bar0.service");
+ assert_se(streq(changes[0].path, p));
+ unit_file_changes_free(changes, n_changes);
+ changes = NULL; n_changes = 0;
+
+ /* Check for preset-all case, only instances on the list should be enabled, not including the default instance */
+ assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "foo@def.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
+ assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "foo@bar1.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
+ assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "foo@bartest.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
+
+ assert_se(unit_file_preset_all(UNIT_FILE_SYSTEM, 0, root, UNIT_FILE_PRESET_FULL, &changes, &n_changes) >= 0);
+ assert_se(n_changes > 0);
+
+ assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "foo@def.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
+ assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "foo@bar0.service", &state) >= 0 && state == UNIT_FILE_ENABLED);
+ assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "foo@bar1.service", &state) >= 0 && state == UNIT_FILE_ENABLED);
+ assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "foo@bartest.service", &state) >= 0 && state == UNIT_FILE_ENABLED);
+
+}
+
int main(int argc, char *argv[]) {
char root[] = "/tmp/rootXXXXXX";
const char *p;
@@ -1012,6 +1068,7 @@ int main(int argc, char *argv[]) {
test_indirect(root);
test_preset_and_list(root);
test_preset_order(root);
+ test_preset_multiple_instances(root);
test_revert(root);
test_static_instance(root);
test_with_dropin(root);