fix handling of templates instantiated in /usr/lib (#5263)

Fix handling of templates instantiated in /usr/lib.
All work to fix #5136.

diff --git a/src/shared/install.c b/src/shared/install.c
index 478abac8abfb8fb8f3cf5cbac974cf98361df5d6..f25ed685f64a45148aca0099952a0b9205ad28b7 100644 (file)
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -208,7 +208,7 @@ static int path_is_control(const LookupPaths *p, const char *path) {
                path_equal_ptr(parent, p->runtime_control);
 }
 
-static int path_is_config(const LookupPaths *p, const char *path) {
+static int path_is_config(const LookupPaths *p, const char *path, bool check_parent) {
         _cleanup_free_ char *parent = NULL;
 
         assert(p);
@@ -217,15 +217,19 @@ static int path_is_config(const LookupPaths *p, const char *path) {
         /* Note that we do *not* have generic checks for /etc or /run in place, since with
          * them we couldn't discern configuration from transient or generated units */
 
-        parent = dirname_malloc(path);
-        if (!parent)
-                return -ENOMEM;
+        if (check_parent) {
+                parent = dirname_malloc(path);
+                if (!parent)
+                        return -ENOMEM;
 
-        return path_equal_ptr(parent, p->persistent_config) ||
-               path_equal_ptr(parent, p->runtime_config);
+                path = parent;
+        }
+
+        return path_equal_ptr(path, p->persistent_config) ||
+               path_equal_ptr(path, p->runtime_config);
 }
 
-static int path_is_runtime(const LookupPaths *p, const char *path) {
+static int path_is_runtime(const LookupPaths *p, const char *path, bool check_parent) {
         _cleanup_free_ char *parent = NULL;
         const char *rpath;
 
@@ -239,16 +243,20 @@ static int path_is_runtime(const LookupPaths *p, const char *path) {
         if (rpath && path_startswith(rpath, "/run"))
                 return true;
 
-        parent = dirname_malloc(path);
-        if (!parent)
-                return -ENOMEM;
+        if (check_parent) {
+                parent = dirname_malloc(path);
+                if (!parent)
+                        return -ENOMEM;
 
-        return path_equal_ptr(parent, p->runtime_config) ||
-               path_equal_ptr(parent, p->generator) ||
-               path_equal_ptr(parent, p->generator_early) ||
-               path_equal_ptr(parent, p->generator_late) ||
-               path_equal_ptr(parent, p->transient) ||
-               path_equal_ptr(parent, p->runtime_control);
+                path = parent;
+        }
+
+        return path_equal_ptr(path, p->runtime_config) ||
+               path_equal_ptr(path, p->generator) ||
+               path_equal_ptr(path, p->generator_early) ||
+               path_equal_ptr(path, p->generator_late) ||
+               path_equal_ptr(path, p->transient) ||
+               path_equal_ptr(path, p->runtime_control);
 }
 
 static int path_is_vendor(const LookupPaths *p, const char *path) {
@@ -677,7 +685,6 @@ static int find_symlinks_fd(
                 int fd,
                 const char *path,
                 const char *config_path,
-                const LookupPaths *lp,
                 bool *same_name_link) {
 
         _cleanup_closedir_ DIR *d = NULL;
@@ -688,7 +695,6 @@ static int find_symlinks_fd(
         assert(fd >= 0);
         assert(path);
         assert(config_path);
-        assert(lp);
         assert(same_name_link);
 
         d = fdopendir(fd);
@@ -722,7 +728,7 @@ static int find_symlinks_fd(
                         }
 
                         /* This will close nfd, regardless whether it succeeds or not */
-                        q = find_symlinks_fd(root_dir, name, nfd, p, config_path, lp, same_name_link);
+                        q = find_symlinks_fd(root_dir, name, nfd, p, config_path, same_name_link);
                         if (q > 0)
                                 return 1;
                         if (r == 0)
@@ -800,7 +806,6 @@ static int find_symlinks(
                 const char *root_dir,
                 const char *name,
                 const char *config_path,
-                const LookupPaths *lp,
                 bool *same_name_link) {
 
         int fd;
@@ -817,44 +822,82 @@ static int find_symlinks(
         }
 
         /* This takes possession of fd and closes it */
-        return find_symlinks_fd(root_dir, name, fd, config_path, config_path, lp, same_name_link);
+        return find_symlinks_fd(root_dir, name, fd, config_path, config_path, same_name_link);
 }
 
 static int find_symlinks_in_scope(
-                UnitFileScope scope,
                 const LookupPaths *paths,
                 const char *name,
                 UnitFileState *state) {
 
-        bool same_name_link_runtime = false, same_name_link = false;
+        bool same_name_link_runtime = false, same_name_link_config = false;
+        bool enabled_in_runtime = false, enabled_at_all = false;
+        char **p;
         int r;
 
-        assert(scope >= 0);
-        assert(scope < _UNIT_FILE_SCOPE_MAX);
         assert(paths);
         assert(name);
 
-        /* First look in the persistent config path */
-        r = find_symlinks(paths->root_dir, name, paths->persistent_config, paths, &same_name_link);
-        if (r < 0)
-                return r;
-        if (r > 0) {
-                *state = UNIT_FILE_ENABLED;
-                return r;
+        STRV_FOREACH(p, paths->search_path)  {
+                bool same_name_link = false;
+
+                r = find_symlinks(paths->root_dir, name, *p, &same_name_link);
+                if (r < 0)
+                        return r;
+                if (r > 0) {
+                        /* We found symlinks in this dir? Yay! Let's see where precisely it is enabled. */
+
+                        r = path_is_config(paths, *p, false);
+                        if (r < 0)
+                                return r;
+                        if (r > 0) {
+                                /* This is the best outcome, let's return it immediately. */
+                                *state = UNIT_FILE_ENABLED;
+                                return 1;
+                        }
+
+                        r = path_is_runtime(paths, *p, false);
+                        if (r < 0)
+                                return r;
+                        if (r > 0)
+                                enabled_in_runtime = true;
+                        else
+                                enabled_at_all = true;
+
+                } else if (same_name_link) {
+
+                        r = path_is_config(paths, *p, false);
+                        if (r < 0)
+                                return r;
+                        if (r > 0)
+                                same_name_link_config = true;
+                        else {
+                                r = path_is_runtime(paths, *p, false);
+                                if (r < 0)
+                                        return r;
+                                if (r > 0)
+                                        same_name_link_runtime = true;
+                        }
+                }
         }
 
-        /* Then look in runtime config path */
-        r = find_symlinks(paths->root_dir, name, paths->runtime_config, paths, &same_name_link_runtime);
-        if (r < 0)
-                return r;
-        if (r > 0) {
+        if (enabled_in_runtime) {
                 *state = UNIT_FILE_ENABLED_RUNTIME;
-                return r;
+                return 1;
+        }
+
+        /* Here's a special rule: if the unit we are looking for is an instance, and it symlinked in the search path
+         * outside of runtime and configuration directory, then we consider it statically enabled. Note we do that only
+         * for instance, not for regular names, as those are merely aliases, while instances explicitly instantiate
+         * something, and hence are a much stronger concept. */
+        if (enabled_at_all && unit_name_is_valid(name, UNIT_NAME_INSTANCE)) {
+                *state = UNIT_FILE_STATIC;
+                return 1;
         }
 
         /* Hmm, we didn't find it, but maybe we found the same name
          * link? */
-        if (same_name_link) {
+        if (same_name_link_config) {
                 *state = UNIT_FILE_LINKED;
                 return 1;
         }
@@ -1354,7 +1397,8 @@ static int install_info_follow(
                 InstallContext *c,
                 UnitFileInstallInfo *i,
                 const char *root_dir,
-                SearchFlags flags) {
+                SearchFlags flags,
+                bool ignore_different_name) {
 
         assert(c);
         assert(i);
@@ -1367,7 +1411,7 @@ static int install_info_follow(
         /* If the basename doesn't match, the caller should add a
          * complete new entry for this. */
 
-        if (!streq(basename(i->symlink_target), i->name))
+        if (!ignore_different_name && !streq(basename(i->symlink_target), i->name))
                 return -EXDEV;
 
         free_and_replace(i->path, i->symlink_target);
@@ -1408,14 +1452,14 @@ static int install_info_traverse(
                         return -ELOOP;
 
                 if (!(flags & SEARCH_FOLLOW_CONFIG_SYMLINKS)) {
-                        r = path_is_config(paths, i->path);
+                        r = path_is_config(paths, i->path, true);
                         if (r < 0)
                                 return r;
                         if (r > 0)
                                 return -ELOOP;
                 }
 
-                r = install_info_follow(c, i, paths->root_dir, flags);
+                r = install_info_follow(c, i, paths->root_dir, flags, false);
                 if (r == -EXDEV) {
                         _cleanup_free_ char *buffer = NULL;
                         const char *bn;
@@ -1439,6 +1483,18 @@ static int install_info_traverse(
                                 if (r < 0)
                                         return r;
 
+                                if (streq(buffer, i->name)) {
+
+                                        /* We filled in the instance, and the target stayed the same? If so, then let's
+                                         * honour the link as it is. */
+
+                                        r = install_info_follow(c, i, paths->root_dir, flags, true);
+                                        if (r < 0)
+                                                return r;
+
+                                        continue;
+                                }
+
                                 bn = buffer;
                         }
 
@@ -2027,7 +2083,7 @@ static int path_shall_revert(const LookupPaths *paths, const char *path) {
 
         /* Checks whether the path is one where the drop-in directories shall be removed. */
 
-        r = path_is_config(paths, path);
+        r = path_is_config(paths, path, true);
         if (r != 0)
                 return r;
 
@@ -2135,7 +2191,7 @@ int unit_file_revert(
                                 if (errno != ENOENT)
                                         return -errno;
                         } else if (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)) {
-                                r = path_is_config(&paths, path);
+                                r = path_is_config(&paths, path, true);
                                 if (r < 0)
                                         return r;
                                 if (r > 0) {
@@ -2481,7 +2537,7 @@ static int unit_file_lookup_state(
         switch (i->type) {
 
         case UNIT_FILE_TYPE_MASKED:
-                r = path_is_runtime(paths, i->path);
+                r = path_is_runtime(paths, i->path, true);
                 if (r < 0)
                         return r;
 
@@ -2505,7 +2561,7 @@ static int unit_file_lookup_state(
                         break;
                 }
 
-                r = find_symlinks_in_scope(scope, paths, i->name, &state);
+                r = find_symlinks_in_scope(paths, i->name, &state);
                 if (r < 0)
                         return r;
                 if (r == 0) {

diff --git a/src/test/test-install-root.c b/src/test/test-install-root.c
index d0bc8004f3407ff49507071d0a0e9f747fceab29..575401cb91ba148307cb45a7e111c688f91b0467 100644 (file)
--- a/src/test/test-install-root.c
+++ b/src/test/test-install-root.c
@@ -736,6 +736,28 @@ static void test_preset_order(const char *root) {
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "prefix-2.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
 }
 
+static void test_static_instance(const char *root) {
+        UnitFileState state;
+        const char *p;
+
+        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "static-instance@.service", &state) == -ENOENT);
+        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "static-instance@foo.service", &state) == -ENOENT);
+
+        p = strjoina(root, "/usr/lib/systemd/system/static-instance@.service");
+        assert_se(write_string_file(p,
+                                    "[Install]\n"
+                                    "WantedBy=multi-user.target\n", WRITE_STRING_FILE_CREATE) >= 0);
+
+        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "static-instance@.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
+        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "static-instance@foo.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
+
+        p = strjoina(root, "/usr/lib/systemd/system/static-instance@foo.service");
+        assert_se(symlink("static-instance@.service", p) >= 0);
+
+        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "static-instance@.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
+        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "static-instance@foo.service", &state) >= 0 && state == UNIT_FILE_STATIC);
+}
+
 int main(int argc, char *argv[]) {
         char root[] = "/tmp/rootXXXXXX";
         const char *p;
@@ -766,6 +788,7 @@ int main(int argc, char *argv[]) {
         test_preset_and_list(root);
         test_preset_order(root);
         test_revert(root);
+        test_static_instance(root);
 
         assert_se(rm_rf(root, REMOVE_ROOT|REMOVE_PHYSICAL) >= 0);