Merge pull request #8271 from poettering/unit-prefix-search

Search unit .d/ drop-in files also in all "dash prefixes" of a unit

man/systemd.unit.xml

@@ -189,15 +189,24 @@
     suffix is <filename>.requires/</filename> in this case.</para>
 
     <para>Along with a unit file <filename>foo.service</filename>, a "drop-in" directory
-    <filename>foo.service.d/</filename> may exist. All files with the suffix
-    <literal>.conf</literal> from this directory will be parsed after the file itself is
-    parsed. This is useful to alter or add configuration settings for a unit, without having to
-    modify unit files. Each drop-in file must have appropriate section headers. Note that for
-    instantiated units, this logic will first look for the instance <literal>.d/</literal>
-    subdirectory and read its <literal>.conf</literal> files, followed by the template
-    <literal>.d/</literal> subdirectory and the <literal>.conf</literal> files there.</para>
+    <filename>foo.service.d/</filename> may exist. All files with the suffix <literal>.conf</literal> from this
+    directory will be parsed after the unit file itself is parsed. This is useful to alter or add configuration
+    settings for a unit, without having to modify unit files. Drop-in files must contain appropriate section
+    headers. For instantiated units, this logic will first look for the instance <literal>.d/</literal> subdirectory
+    (e.g. <literal>foo@bar.service.d/</literal>) and read its <literal>.conf</literal> files, followed by the template
+    <literal>.d/</literal> subdirectory (e.g. <literal>foo@.service.d/</literal>) and the <literal>.conf</literal>
+    files there. Moreover for units names containing dashes (<literal>-</literal>), the set of directories generated by
+    truncating the unit name after all dashes is searched too. Specifically, for a unit name
+    <filename>foo-bar-baz.service</filename> not only the the regular drop-in directory
+    <filename>foo-bar-baz.service.d/</filename> is searched but also both <filename>foo-bar-.service.d/</filename> and
+    <filename>foo-.service.d/</filename>. This is useful for defining common drop-ins for a set of related units, whose
+    names begin with a common prefix. This scheme is particularly useful for mount, automount and slice units, whose
+    systematic naming structure is built around dashes as component separators. Note that equally named drop-in files
+    further down the prefix hierarchy override those further up,
+    i.e. <filename>foo-bar-.service.d/10-override.conf</filename> overrides
+    <filename>foo-.service.d/10-override.conf</filename>.</para>
 
-    <para>In addition to <filename>/etc/systemd/system</filename>, the drop-in <literal>.d</literal>
+    <para>In addition to <filename>/etc/systemd/system</filename>, the drop-in <literal>.d/</literal>
     directories for system services can be placed in <filename>/usr/lib/systemd/system</filename> or
     <filename>/run/systemd/system</filename> directories. Drop-in files in <filename>/etc</filename>
     take precedence over those in <filename>/run</filename> which in turn take precedence over those

src/basic/unit-name.c

@@ -241,25 +241,45 @@ int unit_name_change_suffix(const char *n, const char *suffix, char **ret) {
 }
 
 int unit_name_build(const char *prefix, const char *instance, const char *suffix, char **ret) {
-        char *s;
+        UnitType type;
 
         assert(prefix);
         assert(suffix);
         assert(ret);
 
+        if (isempty(suffix))
+                return -EINVAL;
+        if (suffix[0] != '.')
+                return -EINVAL;
+
+        type = unit_type_from_string(suffix + 1);
+        if (type < 0)
+                return -EINVAL;
+
+        return unit_name_build_from_type(prefix, instance, type, ret);
+}
+
+int unit_name_build_from_type(const char *prefix, const char *instance, UnitType type, char **ret) {
+        const char *ut;
+        char *s;
+
+        assert(prefix);
+        assert(type >= 0);
+        assert(type < _UNIT_TYPE_MAX);
+        assert(ret);
+
         if (!unit_prefix_is_valid(prefix))
                 return -EINVAL;
 
         if (instance && !unit_instance_is_valid(instance))
                 return -EINVAL;
 
-        if (!unit_suffix_is_valid(suffix))
-                return -EINVAL;
+        ut = unit_type_to_string(type);
 
         if (!instance)
-                s = strappend(prefix, suffix);
+                s = strjoin(prefix, ".", ut);
         else
-                s = strjoin(prefix, "@", instance, suffix);
+                s = strjoin(prefix, "@", instance, ".", ut);
         if (!s)
                 return -ENOMEM;
 

src/basic/unit-name.h

@@ -40,6 +40,7 @@ UnitType unit_name_to_type(const char *n) _pure_;
 int unit_name_change_suffix(const char *n, const char *suffix, char **ret);
 
 int unit_name_build(const char *prefix, const char *instance, const char *suffix, char **ret);
+int unit_name_build_from_type(const char *prefix, const char *instance, UnitType, char **ret);
 
 char *unit_name_escape(const char *f);
 int unit_name_unescape(const char *f, char **ret);

src/core/load-dropin.c

@@ -18,26 +18,21 @@
 #include "unit.h"
 
 static int unit_name_compatible(const char *a, const char *b) {
-        _cleanup_free_ char *prefix = NULL;
+        _cleanup_free_ char *template = NULL;
         int r;
 
-        /* the straightforward case: the symlink name matches the target */
+        /* The straightforward case: the symlink name matches the target */
         if (streq(a, b))
                 return 1;
 
-        r = unit_name_template(a, &prefix);
+        r = unit_name_template(a, &template);
         if (r == -EINVAL)
-                /* not a template */
-                return 0;
+                return 0; /* Not a template */
         if (r < 0)
-                /* oom, or some other failure. Just skip the warning. */
-                return r;
+                return r; /* OOM, or some other failure. Just skip the warning. */
 
-        /* an instance name points to a target that is just the template name */
-        if (streq(prefix, b))
-                return 1;
-
-        return 0;
+        /* An instance name points to a target that is just the template name */
+        return streq(template, b);
 }
 
 static int process_deps(Unit *u, UnitDependency dependency, const char *dir_suffix) {
@@ -56,8 +51,8 @@ static int process_deps(Unit *u, UnitDependency dependency, const char *dir_suff
                 return r;
 
         STRV_FOREACH(p, paths) {
-                const char *entry;
                 _cleanup_free_ char *target = NULL;
+                const char *entry;
 
                 entry = basename(*p);
 

src/core/load-dropin.h

@@ -13,6 +13,8 @@
 /* Read service data supplementary drop-in directories */
 
 static inline int unit_find_dropin_paths(Unit *u, char ***paths) {
+        assert(u);
+
         return unit_file_find_dropin_conf_paths(NULL,
                                                 u->manager->lookup_paths.search_path,
                                                 u->manager->unit_path_cache,

src/shared/dropin.c

@@ -115,14 +115,16 @@ static int unit_file_find_dir(
         assert(path);
 
         r = chase_symlinks(path, original_root, 0, &chased);
-        /* Ignore -ENOENT, after all most units won't have a drop-in dir.
-         * Also ignore -ENAMETOOLONG, users are not even able to create
-         * the drop-in dir in such case. This mostly happens for device units with long /sys path.
-         * */
-        if (IN_SET(r, -ENOENT, -ENAMETOOLONG))
+        if (r == -ENOENT) /* Ignore -ENOENT, after all most units won't have a drop-in dir. */
                 return 0;
+        if (r == -ENAMETOOLONG) {
+                /* Also, ignore -ENAMETOOLONG but log about it. After all, users are not even able to create the
+                 * drop-in dir in such case. This mostly happens for device units with an overly long /sys path. */
+                log_debug_errno(r, "Path '%s' too long, couldn't canonicalize, ignoring.", path);
+                return 0;
+        }
         if (r < 0)
-                return log_full_errno(LOG_WARNING, r, "Failed to canonicalize path %s: %m", path);
+                return log_warning_errno(r, "Failed to canonicalize path '%s': %m", path);
 
         r = strv_push(dirs, chased);
         if (r < 0)
@@ -140,7 +142,12 @@ static int unit_file_find_dirs(
                 const char *suffix,
                 char ***dirs) {
 
+        _cleanup_free_ char *prefix = NULL, *instance = NULL, *built = NULL;
+        bool is_instance, chopped;
+        const char *dash;
+        UnitType type;
         char *path;
+        size_t n;
         int r;
 
         assert(unit_path);
@@ -148,26 +155,76 @@ static int unit_file_find_dirs(
         assert(suffix);
 
         path = strjoina(unit_path, "/", name, suffix);
-
         if (!unit_path_cache || set_get(unit_path_cache, path)) {
                 r = unit_file_find_dir(original_root, path, dirs);
                 if (r < 0)
                         return r;
         }
 
-        if (unit_name_is_valid(name, UNIT_NAME_INSTANCE)) {
-                /* Also try the template dir */
-
+        is_instance = unit_name_is_valid(name, UNIT_NAME_INSTANCE);
+        if (is_instance) { /* Also try the template dir */
                 _cleanup_free_ char *template = NULL;
 
                 r = unit_name_template(name, &template);
                 if (r < 0)
                         return log_error_errno(r, "Failed to generate template from unit name: %m");
 
-                return unit_file_find_dirs(original_root, unit_path_cache, unit_path, template, suffix, dirs);
+                r = unit_file_find_dirs(original_root, unit_path_cache, unit_path, template, suffix, dirs);
+                if (r < 0)
+                        return r;
         }
 
-        return 0;
+        /* Let's see if there's a "-" prefix for this unit name. If so, let's invoke ourselves for it. This will then
+         * recursively do the same for all our prefixes. i.e. this means given "foo-bar-waldo.service" we'll also
+         * search "foo-bar-.service" and "foo-.service".
+         *
+         * Note the order in which we do it: we traverse up adding drop-ins on each step. This means the more specific
+         * drop-ins may override the more generic drop-ins, which is the intended behaviour. */
+
+        r = unit_name_to_prefix(name, &prefix);
+        if (r < 0)
+                return log_error_errno(r, "Failed to derive unit name prefix from unit name: %m");
+
+        chopped = false;
+        for (;;) {
+                dash = strrchr(prefix, '-');
+                if (!dash) /* No dash? if so we are done */
+                        return 0;
+
+                n = (size_t) (dash - prefix);
+                if (n == 0) /* Leading dash? If so, we are done */
+                        return 0;
+
+                if (prefix[n+1] != 0 || chopped) {
+                        prefix[n+1] = 0;
+                        break;
+                }
+
+                /* Trailing dash? If so, chop it off and try again, but not more than once. */
+                prefix[n] = 0;
+                chopped = true;
+        }
+
+        if (!unit_prefix_is_valid(prefix))
+                return 0;
+
+        type = unit_name_to_type(name);
+        if (type < 0) {
+                log_error("Failed to to derive unit type from unit name: %m");
+                return -EINVAL;
+        }
+
+        if (is_instance) {
+                r = unit_name_to_instance(name, &instance);
+                if (r < 0)
+                        return log_error_errno(r, "Failed to derive unit name instance from unit name: %m");
+        }
+
+        r = unit_name_build_from_type(prefix, instance, type, &built);
+        if (r < 0)
+                return log_error_errno(r, "Failed to build prefix unit name: %m");
+
+        return unit_file_find_dirs(original_root, unit_path_cache, unit_path, built, suffix, dirs);
 }
 
 int unit_file_find_dropin_paths(
@@ -180,15 +237,15 @@ int unit_file_find_dropin_paths(
                 char ***ret) {
 
         _cleanup_strv_free_ char **dirs = NULL;
-        Iterator i;
         char *t, **p;
+        Iterator i;
         int r;
 
         assert(ret);
 
         SET_FOREACH(t, names, i)
                 STRV_FOREACH(p, lookup_path)
-                        unit_file_find_dirs(original_root, unit_path_cache, *p, t, dir_suffix, &dirs);
+                        (void) unit_file_find_dirs(original_root, unit_path_cache, *p, t, dir_suffix, &dirs);
 
         if (strv_isempty(dirs)) {
                 *ret = NULL;

src/shared/dropin.h

@@ -36,6 +36,7 @@ static inline int unit_file_find_dropin_conf_paths(
                 Set *unit_path_cache,
                 Set *names,
                 char ***paths) {
+
         return unit_file_find_dropin_paths(original_root,
                                            lookup_path,
                                            unit_path_cache,

src/systemctl/systemctl.c

@@ -4021,8 +4021,9 @@ static void print_status_info(
                 char ** dropin;
 
                 STRV_FOREACH(dropin, i->dropin_paths) {
-                        if (! dir || last) {
-                                printf(dir ? "        " : "  Drop-In: ");
+                        if (!dir || last) {
+                                printf(dir ? "           " :
+                                             "  Drop-In: ");
 
                                 dir = mfree(dir);
 
@@ -4032,7 +4033,8 @@ static void print_status_info(
                                         return;
                                 }
 
-                                printf("%s\n           %s", dir,
+                                printf("%s\n"
+                                       "           %s", dir,
                                        special_glyph(TREE_RIGHT));
                         }
 

src/test/test-engine.c

@@ -19,12 +19,16 @@ int main(int argc, char *argv[]) {
         _cleanup_(rm_rf_physical_and_freep) char *runtime_dir = NULL;
         _cleanup_(sd_bus_error_free) sd_bus_error err = SD_BUS_ERROR_NULL;
         _cleanup_(manager_freep) Manager *m = NULL;
-        Unit *a = NULL, *b = NULL, *c = NULL, *d = NULL, *e = NULL, *g = NULL, *h = NULL;
+        Unit *a = NULL, *b = NULL, *c = NULL, *d = NULL, *e = NULL, *g = NULL, *h = NULL, *unit_with_multiple_dashes = NULL;
         FILE *serial = NULL;
         FDSet *fdset = NULL;
         Job *j;
         int r;
 
+        log_set_max_level(LOG_DEBUG);
+        log_parse_environment();
+        log_open();
+
         r = enter_cgroup_subroot();
         if (r == -ENOMEDIUM) {
                 log_notice_errno(r, "Skipping test: cgroupfs not available");
@@ -130,5 +134,10 @@ int main(int argc, char *argv[]) {
         assert_se(!hashmap_get(a->dependencies[UNIT_PROPAGATES_RELOAD_TO], c));
         assert_se(!hashmap_get(c->dependencies[UNIT_RELOAD_PROPAGATED_FROM], a));
 
+        assert_se(manager_load_unit(m, "unit-with-multiple-dashes.service", NULL, NULL, &unit_with_multiple_dashes) >= 0);
+
+        assert_se(strv_equal(unit_with_multiple_dashes->documentation, STRV_MAKE("man:test", "man:override2", "man:override3")));
+        assert_se(streq_ptr(unit_with_multiple_dashes->description, "override4"));
+
         return 0;
 }

src/test/test-unit-name.c

@@ -449,6 +449,26 @@ static void test_unit_name_path_unescape(void) {
         test_unit_name_path_unescape_one("", NULL, -EINVAL);
 }
 
+static void test_unit_name_to_prefix_one(const char *input, int ret, const char *output) {
+        _cleanup_free_ char *k = NULL;
+
+        assert_se(unit_name_to_prefix(input, &k) == ret);
+        assert_se(streq_ptr(k, output));
+}
+
+static void test_unit_name_to_prefix(void) {
+        test_unit_name_to_prefix_one("foobar.service", 0, "foobar");
+        test_unit_name_to_prefix_one("", -EINVAL, NULL);
+        test_unit_name_to_prefix_one("foobar", -EINVAL, NULL);
+        test_unit_name_to_prefix_one(".service", -EINVAL, NULL);
+        test_unit_name_to_prefix_one("quux.quux", -EINVAL, NULL);
+        test_unit_name_to_prefix_one("quux.mount", 0, "quux");
+        test_unit_name_to_prefix_one("quux-quux.mount", 0, "quux-quux");
+        test_unit_name_to_prefix_one("quux@bar.mount", 0, "quux");
+        test_unit_name_to_prefix_one("quux-@.mount", 0, "quux-");
+        test_unit_name_to_prefix_one("@.mount", -EINVAL, NULL);
+}
+
 int main(int argc, char* argv[]) {
         _cleanup_(rm_rf_physical_and_freep) char *runtime_dir = NULL;
         int r, rc = 0;
@@ -482,6 +502,7 @@ int main(int argc, char* argv[]) {
         test_unit_name_escape();
         test_unit_name_template();
         test_unit_name_path_unescape();
+        test_unit_name_to_prefix();
 
         return rc;
 }

test/meson.build

@@ -158,13 +158,18 @@ test_data_files = '''
         test-path/path-unit.path
         test-path/paths.target
         test-path/sysinit.target
-        testsuite.target
-        timers.target
-        unstoppable.service
         test-umount/empty.mountinfo
+        test-umount/example.swaps
         test-umount/garbled.mountinfo
         test-umount/rhbug-1554943.mountinfo
-        test-umount/example.swaps
+        testsuite.target
+        timers.target
+        unit-with-.service.d/20-override.conf
+        unit-with-multiple-.service.d/20-override.conf
+        unit-with-multiple-.service.d/30-override.conf
+        unit-with-multiple-dashes.service
+        unit-with-multiple-dashes.service.d/10-override.conf
+        unstoppable.service
 '''.split()
 
 if conf.get('ENABLE_RESOLVE') == 1

test/unit-.service.d/10-override.conf

@@ -0,0 +1,2 @@
+[Unit]
+Description=override0

test/unit-with-.service.d/20-override.conf

@@ -0,0 +1,2 @@
+[Unit]
+Documentation=man:override1

test/unit-with-multiple-.service.d/20-override.conf

@@ -0,0 +1,2 @@
+[Unit]
+Documentation=man:override2

test/unit-with-multiple-.service.d/30-override.conf

@@ -0,0 +1,2 @@
+[Unit]
+Documentation=man:override3

test/unit-with-multiple-dashes.service

@@ -0,0 +1,6 @@
+[Unit]
+Description=A unit with multiple dashes
+Documentation=man:test
+
+[Service]
+ExecStart=/bin/true

test/unit-with-multiple-dashes.service.d/10-override.conf

@@ -0,0 +1,2 @@
+[Unit]
+Description=override4