Revert: Rework unit loading to take into account all aliases 73/236473/1
authorINSUN PYO <insun.pyo@samsung.com>
Tue, 9 Jun 2020 00:55:24 +0000 (09:55 +0900)
committerINSUN PYO <insun.pyo@samsung.com>
Wed, 17 Jun 2020 08:36:51 +0000 (17:36 +0900)
Revert below patches due to increasing unit loading time (UnitsLoadFinishTimestamp - UnitsLoadStartTimestamp)

Refer: https://github.com/systemd/systemd/pull/13119/commits
 - test-unit-file: allow printing of information about specific units
 - pid1: drop unit caches only based on mtime
 - analyze: add "unit-files" to dump the unit fragment map
 - core: restore initialization of u->source_mtime
 - pid1: use a cache for all unit aliases
 - shared/unit-file: add a function to validate unit alias symlinks
 - TEST-15-DROPIN: add test for details of unit aliasing

Change-Id: I1bff89f5851544cda7522bd3ceb398499dac57d4

src/analyze/analyze.c
src/core/load-fragment.c
src/core/manager.c
src/core/manager.h
src/core/unit.c
src/shared/unit-file.c
src/shared/unit-file.h
src/systemctl/systemctl.c
src/test/meson.build
src/test/test-unit-file.c [deleted file]
test/TEST-15-DROPIN/test-dropin.sh

index 991e61d..19c4ce9 100644 (file)
@@ -1546,53 +1546,6 @@ static int get_or_set_log_target(int argc, char *argv[], void *userdata) {
         return (argc == 1) ? get_log_target(argc, argv, userdata) : set_log_target(argc, argv, userdata);
 }
 
-static bool strv_fnmatch_strv_or_empty(char* const* patterns, char **strv, int flags) {
-        char **s;
-        STRV_FOREACH(s, strv)
-                if (strv_fnmatch_or_empty(patterns, *s, flags))
-                        return true;
-
-        return false;
-}
-
-static int do_unit_files(int argc, char *argv[], void *userdata) {
-        _cleanup_(lookup_paths_free) LookupPaths lp = {};
-        _cleanup_hashmap_free_ Hashmap *unit_ids = NULL;
-        _cleanup_hashmap_free_ Hashmap *unit_names = NULL;
-        char **patterns = strv_skip(argv, 1);
-        Iterator i;
-        const char *k, *dst;
-        char **v;
-        int r;
-
-        r = lookup_paths_init(&lp, arg_scope, 0, NULL);
-        if (r < 0)
-                return log_error_errno(r, "lookup_paths_init() failed: %m");
-
-        r = unit_file_build_name_map(&lp, NULL, &unit_ids, &unit_names, NULL);
-        if (r < 0)
-                return log_error_errno(r, "unit_file_build_name_map() failed: %m");
-
-        HASHMAP_FOREACH_KEY(dst, k, unit_ids, i) {
-                if (!strv_fnmatch_or_empty(patterns, k, FNM_NOESCAPE) &&
-                    !strv_fnmatch_or_empty(patterns, dst, FNM_NOESCAPE))
-                        continue;
-
-                printf("ids: %s → %s\n", k, dst);
-        }
-
-        HASHMAP_FOREACH_KEY(v, k, unit_names, i) {
-                if (!strv_fnmatch_or_empty(patterns, k, FNM_NOESCAPE) &&
-                    !strv_fnmatch_strv_or_empty(patterns, v, FNM_NOESCAPE))
-                        continue;
-
-                _cleanup_free_ char *j = strv_join(v, ", ");
-                printf("aliases: %s ← %s\n", k, j);
-        }
-
-        return 0;
-}
-
 static int dump_unit_paths(int argc, char *argv[], void *userdata) {
         _cleanup_(lookup_paths_free) LookupPaths paths = {};
         int r;
@@ -2250,7 +2203,6 @@ static int help(int argc, char *argv[], void *userdata) {
                "  dot [UNIT...]            Output dependency graph in %s format\n"
                "  dump                     Output state serialization of service manager\n"
                "  cat-config               Show configuration file and drop-ins\n"
-               "  unit-files               List files and symlinks for units\n"
                "  unit-paths               List load directories for units\n"
                "  exit-status [STATUS...]  List exit status definitions\n"
                "  syscall-filter [NAME...] Print list of syscalls in seccomp filter\n"
@@ -2486,7 +2438,6 @@ static int run(int argc, char *argv[]) {
                 { "service-watchdogs", VERB_ANY, 2,        0,            service_watchdogs      },
                 { "dump",              VERB_ANY, 1,        0,            dump                   },
                 { "cat-config",        2,        VERB_ANY, 0,            cat_config             },
-                { "unit-files",        VERB_ANY, VERB_ANY, 0,            do_unit_files          },
                 { "unit-paths",        1,        1,        0,            dump_unit_paths        },
                 { "exit-status",       VERB_ANY, VERB_ANY, 0,            dump_exit_status       },
                 { "syscall-filter",    VERB_ANY, VERB_ANY, 0,            dump_syscall_filters   },
index b6bd38e..299b238 100644 (file)
@@ -4754,113 +4754,237 @@ int config_parse_ip_filter_bpf_progs(
         return 0;
 }
 
+#define FOLLOW_MAX 8
+
+static int open_follow(char **filename, FILE **_f, Set *names, char **_final) {
+        char *id = NULL;
+        unsigned c = 0;
+        int fd, r;
+        FILE *f;
+
+        assert(filename);
+        assert(*filename);
+        assert(_f);
+        assert(names);
+
+        /* This will update the filename pointer if the loaded file is
+         * reached by a symlink. The old string will be freed. */
+
+        for (;;) {
+                char *target, *name;
+
+                if (c++ >= FOLLOW_MAX)
+                        return -ELOOP;
+
+                path_simplify(*filename, false);
+
+                /* Add the file name we are currently looking at to
+                 * the names of this unit, but only if it is a valid
+                 * unit name. */
+                name = basename(*filename);
+                if (unit_name_is_valid(name, UNIT_NAME_ANY)) {
+
+                        id = set_get(names, name);
+                        if (!id) {
+                                id = strdup(name);
+                                if (!id)
+                                        return -ENOMEM;
+
+                                r = set_consume(names, id);
+                                if (r < 0)
+                                        return r;
+                        }
+                }
+
+                /* Try to open the file name, but don't if its a symlink */
+                fd = open(*filename, O_RDONLY|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW);
+                if (fd >= 0)
+                        break;
+
+                if (errno != ELOOP)
+                        return -errno;
+
+                /* Hmm, so this is a symlink. Let's read the name, and follow it manually */
+                r = readlink_and_make_absolute(*filename, &target);
+                if (r < 0)
+                        return r;
+
+                free_and_replace(*filename, target);
+        }
+
+        f = fdopen(fd, "r");
+        if (!f) {
+                safe_close(fd);
+                return -errno;
+        }
+
+        *_f = f;
+        *_final = id;
+
+        return 0;
+}
+
 static int merge_by_names(Unit **u, Set *names, const char *id) {
         char *k;
         int r;
 
         assert(u);
         assert(*u);
+        assert(names);
 
-        /* Let's try to add in all names that are aliases of this unit */
+        /* Let's try to add in all symlink names we found */
         while ((k = set_steal_first(names))) {
-                _cleanup_free_ _unused_ char *free_k = k;
 
-                /* First try to merge in the other name into our unit */
+                /* First try to merge in the other name into our
+                 * unit */
                 r = unit_merge_by_name(*u, k);
                 if (r < 0) {
                         Unit *other;
 
-                        /* Hmm, we couldn't merge the other unit into ours? Then let's try it the other way
-                         * round. */
+                        /* Hmm, we couldn't merge the other unit into
+                         * ours? Then let's try it the other way
+                         * round */
 
-                        other = manager_get_unit((*u)->manager, k);
-                        if (!other)
-                                return r; /* return previous failure */
+                        /* If the symlink name we are looking at is unit template, then
+                           we must search for instance of this template */
+                        if (unit_name_is_valid(k, UNIT_NAME_TEMPLATE) && (*u)->instance) {
+                                _cleanup_free_ char *instance = NULL;
 
-                        r = unit_merge(other, *u);
-                        if (r < 0)
-                                return r;
+                                r = unit_name_replace_instance(k, (*u)->instance, &instance);
+                                if (r < 0)
+                                        return r;
+
+                                other = manager_get_unit((*u)->manager, instance);
+                        } else
+                                other = manager_get_unit((*u)->manager, k);
 
-                        *u = other;
-                        return merge_by_names(u, names, NULL);
+                        free(k);
+
+                        if (other) {
+                                r = unit_merge(other, *u);
+                                if (r >= 0) {
+                                        *u = other;
+                                        return merge_by_names(u, names, NULL);
+                                }
+                        }
+
+                        return r;
                 }
 
-                if (streq_ptr(id, k))
+                if (id == k)
                         unit_choose_id(*u, id);
+
+                free(k);
         }
 
         return 0;
 }
 
-int unit_load_fragment(Unit *u) {
-        const char *fragment;
-        _cleanup_set_free_free_ Set *names = NULL;
+static int load_from_path(Unit *u, const char *path) {
+        _cleanup_set_free_free_ Set *symlink_names = NULL;
+        _cleanup_fclose_ FILE *f = NULL;
+        _cleanup_free_ char *filename = NULL;
+        char *id = NULL;
+        Unit *merged;
         struct stat st;
         int r;
 
         assert(u);
-        assert(u->load_state == UNIT_STUB);
-        assert(u->id);
+        assert(path);
 
-        if (u->transient) {
-                u->load_state = UNIT_LOADED;
+        symlink_names = set_new(&string_hash_ops);
+        if (!symlink_names)
+                return -ENOMEM;
+
+        if (path_is_absolute(path)) {
+
+                filename = strdup(path);
+                if (!filename)
+                        return -ENOMEM;
+
+                r = open_follow(&filename, &f, symlink_names, &id);
+                if (r < 0) {
+                        filename = mfree(filename);
+                        if (r != -ENOENT)
+                                return r;
+                }
+
+        } else  {
+                char **p;
+
+                STRV_FOREACH(p, u->manager->lookup_paths.search_path) {
+
+                        /* Instead of opening the path right away, we manually
+                         * follow all symlinks and add their name to our unit
+                         * name set while doing so */
+                        filename = path_make_absolute(path, *p);
+                        if (!filename)
+                                return -ENOMEM;
+
+                        if (u->manager->unit_path_cache &&
+                            !set_get(u->manager->unit_path_cache, filename))
+                                r = -ENOENT;
+                        else
+                                r = open_follow(&filename, &f, symlink_names, &id);
+                        if (r >= 0)
+                                break;
+
+                        /* ENOENT means that the file is missing or is a dangling symlink.
+                         * ENOTDIR means that one of paths we expect to be is a directory
+                         * is not a directory, we should just ignore that.
+                         * EACCES means that the directory or file permissions are wrong.
+                         */
+                        if (r == -EACCES)
+                                log_debug_errno(r, "Cannot access \"%s\": %m", filename);
+                        else if (!IN_SET(r, -ENOENT, -ENOTDIR))
+                                return r;
+
+                        filename = mfree(filename);
+                        /* Empty the symlink names for the next run */
+                        set_clear_free(symlink_names);
+                }
+        }
+
+        if (!filename)
+                /* Hmm, no suitable file found? */
                 return 0;
+
+        if (!unit_type_may_alias(u->type) && set_size(symlink_names) > 1) {
+                log_unit_warning(u, "Unit type of %s does not support alias names, refusing loading via symlink.", u->id);
+                return -ELOOP;
         }
 
-        /* Possibly rebuild the fragment map to catch new units */
-        r = unit_file_build_name_map(&u->manager->lookup_paths,
-                                     &u->manager->unit_cache_mtime,
-                                     &u->manager->unit_id_map,
-                                     &u->manager->unit_name_map,
-                                     &u->manager->unit_path_cache);
+        merged = u;
+        r = merge_by_names(&merged, symlink_names, id);
         if (r < 0)
-                log_error_errno(r, "Failed to rebuild name map: %m");
-
-        r = unit_file_find_fragment(u->manager->unit_id_map,
-                                    u->manager->unit_name_map,
-                                    u->id,
-                                    &fragment,
-                                    &names);
-        if (r < 0 && r != -ENOENT)
                 return r;
 
-        if (fragment) {
-                /* Open the file, check if this is a mask, otherwise read. */
-                _cleanup_fclose_ FILE *f = NULL;
+        if (merged != u) {
+                u->load_state = UNIT_MERGED;
+                return 0;
+        }
 
-                /* Try to open the file name. A symlink is OK, for example for linked files or masks. We
-                 * expect that all symlinks within the lookup paths have been already resolved, but we don't
-                 * verify this here. */
-                f = fopen(fragment, "re");
-                if (!f)
-                        return log_unit_notice_errno(u, errno, "Failed to open %s: %m", fragment);
+        if (fstat(fileno(f), &st) < 0)
+                return -errno;
 
-                if (fstat(fileno(f), &st) < 0)
-                        return -errno;
+        if (null_or_empty(&st)) {
+                u->load_state = UNIT_MASKED;
+                u->fragment_mtime = 0;
+        } else {
+                u->load_state = UNIT_LOADED;
+                u->fragment_mtime = timespec_load(&st.st_mtim);
 
-                r = free_and_strdup(&u->fragment_path, fragment);
+                /* Now, parse the file contents */
+                r = config_parse(u->id, filename, f,
+                                 UNIT_VTABLE(u)->sections,
+                                 config_item_perf_lookup, load_fragment_gperf_lookup,
+                                 CONFIG_PARSE_ALLOW_INCLUDE, u);
                 if (r < 0)
                         return r;
-
-                if (null_or_empty(&st)) {
-                        u->load_state = UNIT_MASKED;
-                        u->fragment_mtime = 0;
-                } else {
-                        u->load_state = UNIT_LOADED;
-                        u->fragment_mtime = timespec_load(&st.st_mtim);
-
-                        /* Now, parse the file contents */
-                        r = config_parse(u->id, fragment, f,
-                                         UNIT_VTABLE(u)->sections,
-                                         config_item_perf_lookup, load_fragment_gperf_lookup,
-                                         CONFIG_PARSE_ALLOW_INCLUDE, u);
-                        if (r == -ENOEXEC)
-                                log_unit_notice_errno(u, r, "Unit configuration has fatal error, unit will not be started.");
-                        if (r < 0)
-                                return r;
-                }
         }
 
+        free_and_replace(u->fragment_path, filename);
+
         if (u->source_path) {
                 if (stat(u->source_path, &st) >= 0)
                         u->source_mtime = timespec_load(&st.st_mtim);
@@ -4868,33 +4992,95 @@ int unit_load_fragment(Unit *u) {
                         u->source_mtime = 0;
         }
 
-        /* We do the merge dance here because for some unit types, the unit might have aliases which are not
-         * declared in the file system. In particular, this is true (and frequent) for device and swap units.
-         */
-        Unit *merged;
-        const char *id = u->id;
-        _cleanup_free_ char *free_id = NULL;
+        return 0;
+}
 
-        if (fragment) {
-                id = basename(fragment);
-                if (unit_name_is_valid(id, UNIT_NAME_TEMPLATE)) {
-                        assert(u->instance); /* If we're not trying to use a template for non-instanced unit,
-                                              * this must be set. */
+int unit_load_fragment(Unit *u) {
+        int r;
+        Iterator i;
+        const char *t;
 
-                        r = unit_name_replace_instance(id, u->instance, &free_id);
-                        if (r < 0)
-                                return log_debug_errno(r, "Failed to build id (%s + %s): %m", id, u->instance);
-                        id = free_id;
-                }
+        assert(u);
+        assert(u->load_state == UNIT_STUB);
+        assert(u->id);
+
+        if (u->transient) {
+                u->load_state = UNIT_LOADED;
+                return 0;
         }
 
-        merged = u;
-        r = merge_by_names(&merged, names, id);
+        /* First, try to find the unit under its id. We always look
+         * for unit files in the default directories, to make it easy
+         * to override things by placing things in /etc/systemd/system */
+        r = load_from_path(u, u->id);
         if (r < 0)
                 return r;
 
-        if (merged != u)
-                u->load_state = UNIT_MERGED;
+        /* Try to find an alias we can load this with */
+        if (u->load_state == UNIT_STUB) {
+                SET_FOREACH(t, u->names, i) {
+
+                        if (t == u->id)
+                                continue;
+
+                        r = load_from_path(u, t);
+                        if (r < 0)
+                                return r;
+
+                        if (u->load_state != UNIT_STUB)
+                                break;
+                }
+        }
+
+        /* And now, try looking for it under the suggested (originally linked) path */
+        if (u->load_state == UNIT_STUB && u->fragment_path) {
+
+                r = load_from_path(u, u->fragment_path);
+                if (r < 0)
+                        return r;
+
+                if (u->load_state == UNIT_STUB)
+                        /* Hmm, this didn't work? Then let's get rid
+                         * of the fragment path stored for us, so that
+                         * we don't point to an invalid location. */
+                        u->fragment_path = mfree(u->fragment_path);
+        }
+
+        /* Look for a template */
+        if (u->load_state == UNIT_STUB && u->instance) {
+                _cleanup_free_ char *k = NULL;
+
+                r = unit_name_template(u->id, &k);
+                if (r < 0)
+                        return r;
+
+                r = load_from_path(u, k);
+                if (r < 0) {
+                        if (r == -ENOEXEC)
+                                log_unit_notice(u, "Unit configuration has fatal error, unit will not be started.");
+                        return r;
+                }
+
+                if (u->load_state == UNIT_STUB) {
+                        SET_FOREACH(t, u->names, i) {
+                                _cleanup_free_ char *z = NULL;
+
+                                if (t == u->id)
+                                        continue;
+
+                                r = unit_name_template(t, &z);
+                                if (r < 0)
+                                        return r;
+
+                                r = load_from_path(u, z);
+                                if (r < 0)
+                                        return r;
+
+                                if (u->load_state != UNIT_STUB)
+                                        break;
+                        }
+                }
+        }
 
         return 0;
 }
index 91ed64b..0dd6675 100644 (file)
@@ -689,13 +689,6 @@ static int manager_setup_prefix(Manager *m) {
         return 0;
 }
 
-static void manager_free_unit_name_maps(Manager *m) {
-        m->unit_id_map = hashmap_free(m->unit_id_map);
-        m->unit_name_map = hashmap_free(m->unit_name_map);
-        m->unit_path_cache = set_free_free(m->unit_path_cache);
-        m->unit_cache_mtime =  0;
-}
-
 static int manager_setup_run_queue(Manager *m) {
         int r;
 
@@ -1409,7 +1402,7 @@ Manager* manager_free(Manager *m) {
         strv_free(m->client_environment);
 
         hashmap_free(m->cgroup_unit);
-        manager_free_unit_name_maps(m);
+        set_free_free(m->unit_path_cache);
 
         free(m->switch_root);
         free(m->switch_root_init);
@@ -1513,6 +1506,56 @@ static void manager_catchup(Manager *m) {
         }
 }
 
+static void manager_build_unit_path_cache(Manager *m) {
+        char **i;
+        int r;
+
+        assert(m);
+
+        set_free_free(m->unit_path_cache);
+
+        m->unit_path_cache = set_new(&path_hash_ops);
+        if (!m->unit_path_cache) {
+                r = -ENOMEM;
+                goto fail;
+        }
+
+        /* This simply builds a list of files we know exist, so that
+         * we don't always have to go to disk */
+
+        STRV_FOREACH(i, m->lookup_paths.search_path) {
+                _cleanup_closedir_ DIR *d = NULL;
+                struct dirent *de;
+
+                d = opendir(*i);
+                if (!d) {
+                        if (errno != ENOENT)
+                                log_warning_errno(errno, "Failed to open directory %s, ignoring: %m", *i);
+                        continue;
+                }
+
+                FOREACH_DIRENT(de, d, r = -errno; goto fail) {
+                        char *p;
+
+                        p = path_join(*i, de->d_name);
+                        if (!p) {
+                                r = -ENOMEM;
+                                goto fail;
+                        }
+
+                        r = set_consume(m->unit_path_cache, p);
+                        if (r < 0)
+                                goto fail;
+                }
+        }
+
+        return;
+
+fail:
+        log_warning_errno(r, "Failed to build unit path cache, proceeding without: %m");
+        m->unit_path_cache = set_free_free(m->unit_path_cache);
+}
+
 static void manager_distribute_fds(Manager *m, FDSet *fds) {
         Iterator i;
         Unit *u;
@@ -1673,6 +1716,8 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) {
 
         lookup_paths_log(&m->lookup_paths);
 
+        manager_build_unit_path_cache(m);
+
         {
                 /* This block is (optionally) done with the reloading counter bumped */
                 _cleanup_(manager_reloading_stopp) Manager *reloading = NULL;
@@ -2901,6 +2946,9 @@ int manager_loop(Manager *m) {
         assert(m);
         assert(m->objective == MANAGER_OK); /* Ensure manager_startup() has been called */
 
+        /* Release the path cache */
+        m->unit_path_cache = set_free_free(m->unit_path_cache);
+
         manager_check_finished(m);
 
         /* There might still be some zombies hanging around from before we were exec()'ed. Let's reap them. */
@@ -3594,8 +3642,7 @@ int manager_reload(Manager *m) {
 
         lookup_paths_log(&m->lookup_paths);
 
-        /* We flushed out generated files, for which we don't watch mtime, so we should flush the old map. */
-        manager_free_unit_name_maps(m);
+        manager_build_unit_path_cache(m);
 
         /* First, enumerate what we can from kernel and suchlike */
         manager_enumerate_perpetual(m);
index 05ae30c..2fbe05e 100644 (file)
@@ -223,10 +223,7 @@ struct Manager {
 
         UnitFileScope unit_file_scope;
         LookupPaths lookup_paths;
-        Hashmap *unit_id_map;
-        Hashmap *unit_name_map;
         Set *unit_path_cache;
-        usec_t unit_cache_mtime;
 
         char **transient_environment;  /* The environment, as determined from config files, kernel cmdline and environment generators */
         char **client_environment;     /* Environment variables created by clients through the bus API */
index 759a529..7c8cefe 100644 (file)
@@ -960,9 +960,6 @@ int unit_merge_by_name(Unit *u, const char *name) {
         Unit *other;
         int r;
 
-        /* Either add name to u, or if a unit with name already exists, merge it with u.
-         * If name is a template, do the same for name@instance, where instance is u's instance. */
-
         assert(u);
         assert(name);
 
index 28cd3c8..e99d304 100644 (file)
@@ -1,15 +1,9 @@
 /* SPDX-License-Identifier: LGPL-2.1+ */
 
-#include "dirent-util.h"
-#include "fd-util.h"
-#include "fs-util.h"
+#include "util.h"
 #include "macro.h"
-#include "path-lookup.h"
-#include "set.h"
 #include "special.h"
-#include "stat-util.h"
 #include "string-util.h"
-#include "strv.h"
 #include "unit-file.h"
 
 bool unit_type_may_alias(UnitType type) {
@@ -31,489 +25,6 @@ bool unit_type_may_template(UnitType type) {
                       UNIT_PATH);
 }
 
-int unit_validate_alias_symlink_and_warn(const char *filename, const char *target) {
-        const char *src, *dst;
-        _cleanup_free_ char *src_instance = NULL, *dst_instance = NULL;
-        UnitType src_unit_type, dst_unit_type;
-        int src_name_type, dst_name_type;
-
-        /* Check if the *alias* symlink is valid. This applies to symlinks like
-         * /etc/systemd/system/dbus.service → dbus-broker.service, but not to .wants or .requires symlinks
-         * and such. Neither does this apply to symlinks which *link* units, i.e. symlinks to outside of the
-         * unit lookup path.
-         *
-         * -EINVAL is returned if the something is wrong with the source filename or the source unit type is
-         *         not allowed to symlink,
-         * -EXDEV if the target filename is not a valid unit name or doesn't match the source.
-         */
-
-        src = basename(filename);
-        dst = basename(target);
-
-        /* src checks */
-
-        src_name_type = unit_name_to_instance(src, &src_instance);
-        if (src_name_type < 0)
-                return log_notice_errno(src_name_type,
-                                        "%s: not a valid unit name \"%s\": %m", filename, src);
-
-        src_unit_type = unit_name_to_type(src);
-        assert(src_unit_type >= 0); /* unit_name_to_instance() checked the suffix already */
-
-        if (!unit_type_may_alias(src_unit_type))
-                return log_notice_errno(SYNTHETIC_ERRNO(EINVAL),
-                                        "%s: symlinks are not allowed for units of this type, rejecting.",
-                                        filename);
-
-        if (src_name_type != UNIT_NAME_PLAIN &&
-            !unit_type_may_template(src_unit_type))
-                return log_notice_errno(SYNTHETIC_ERRNO(EINVAL),
-                                        "%s: templates not allowed for %s units, rejecting.",
-                                        filename, unit_type_to_string(src_unit_type));
-
-        /* dst checks */
-
-        dst_name_type = unit_name_to_instance(dst, &dst_instance);
-        if (dst_name_type < 0)
-                return log_notice_errno(dst_name_type == -EINVAL ? SYNTHETIC_ERRNO(EXDEV) : dst_name_type,
-                                        "%s points to \"%s\" which is not a valid unit name: %m",
-                                        filename, dst);
-
-        if (!(dst_name_type == src_name_type ||
-              (src_name_type == UNIT_NAME_INSTANCE && dst_name_type == UNIT_NAME_TEMPLATE)))
-                return log_notice_errno(SYNTHETIC_ERRNO(EXDEV),
-                                        "%s: symlink target name type \"%s\" does not match source, rejecting.",
-                                        filename, dst);
-
-        if (dst_name_type == UNIT_NAME_INSTANCE) {
-                assert(src_instance);
-                assert(dst_instance);
-                if (!streq(src_instance, dst_instance))
-                        return log_notice_errno(SYNTHETIC_ERRNO(EXDEV),
-                                                "%s: unit symlink target \"%s\" instance name doesn't match, rejecting.",
-                                                filename, dst);
-        }
-
-        dst_unit_type = unit_name_to_type(dst);
-        if (dst_unit_type != src_unit_type)
-                return log_notice_errno(SYNTHETIC_ERRNO(EXDEV),
-                                        "%s: symlink target \"%s\" has incompatible suffix, rejecting.",
-                                        filename, dst);
-
-        return 0;
-}
-
-#define FOLLOW_MAX 8
-
-static int unit_ids_map_get(
-                Hashmap *unit_ids_map,
-                const char *unit_name,
-                const char **ret_fragment_path) {
-
-        /* Resolve recursively until we hit an absolute path, i.e. a non-aliased unit.
-         *
-         * We distinguish the case where unit_name was not found in the hashmap at all, and the case where
-         * some symlink was broken.
-         *
-         * If a symlink target points to an instance name, then we also check for the template. */
-
-        const char *id = NULL;
-        int r;
-
-        for (unsigned n = 0; n < FOLLOW_MAX; n++) {
-                const char *t = hashmap_get(unit_ids_map, id ?: unit_name);
-                if (!t) {
-                        _cleanup_free_ char *template = NULL;
-
-                        if (!id)
-                                return -ENOENT;
-
-                        r = unit_name_template(id, &template);
-                        if (r == -EINVAL)
-                                return -ENXIO; /* we failed to find the symlink target */
-                        if (r < 0)
-                                return log_error_errno(r, "Failed to determine template name for %s: %m", id);
-
-                        t = hashmap_get(unit_ids_map, template);
-                        if (!t)
-                                return -ENXIO;
-
-                        /* We successfully switched from instanced name to a template, let's continue */
-                }
-
-                if (path_is_absolute(t)) {
-                        if (ret_fragment_path)
-                                *ret_fragment_path = t;
-                        return 0;
-                }
-
-                id = t;
-        }
-
-        return -ELOOP;
-}
-
-static bool lookup_paths_mtime_exclude(const LookupPaths *lp, const char *path) {
-        /* Paths that are under our exclusive control. Users shall not alter those directly. */
-
-        return streq_ptr(path, lp->generator) ||
-               streq_ptr(path, lp->generator_early) ||
-               streq_ptr(path, lp->generator_late) ||
-               streq_ptr(path, lp->transient) ||
-               streq_ptr(path, lp->persistent_control) ||
-               streq_ptr(path, lp->runtime_control);
-}
-
-static bool lookup_paths_mtime_good(const LookupPaths *lp, usec_t mtime) {
-        char **dir;
-
-        STRV_FOREACH(dir, (char**) lp->search_path) {
-                struct stat st;
-
-                if (lookup_paths_mtime_exclude(lp, *dir))
-                        continue;
-
-                /* Determine the latest lookup path modification time */
-                if (stat(*dir, &st) < 0) {
-                        if (errno == ENOENT)
-                                continue;
-
-                        log_debug_errno(errno, "Failed to stat %s, ignoring: %m", *dir);
-                        continue;
-                }
-
-                if (timespec_load(&st.st_mtim) > mtime) {
-                        log_debug_errno(errno, "Unit dir %s has changed, need to update cache.", *dir);
-                        return false;
-                }
-        }
-
-        return true;
-}
-
-int unit_file_build_name_map(
-                const LookupPaths *lp,
-                usec_t *cache_mtime,
-                Hashmap **ret_unit_ids_map,
-                Hashmap **ret_unit_names_map,
-                Set **ret_path_cache) {
-
-        /* Build two mappings: any name → main unit (i.e. the end result of symlink resolution), unit name →
-         * all aliases (i.e. the entry for a given key is a a list of all names which point to this key). The
-         * key is included in the value iff we saw a file or symlink with that name. In other words, if we
-         * have a key, but it is not present in the value for itself, there was an alias pointing to it, but
-         * the unit itself is not loadable.
-         *
-         * At the same, build a cache of paths where to find units.
-         */
-
-        _cleanup_hashmap_free_ Hashmap *ids = NULL, *names = NULL;
-        _cleanup_set_free_free_ Set *paths = NULL;
-        char **dir;
-        int r;
-        usec_t mtime = 0;
-
-        /* Before doing anything, check if the mtime that was passed is still valid. If
-         * yes, do nothing. If *cache_time == 0, always build the cache. */
-        if (cache_mtime && *cache_mtime > 0 && lookup_paths_mtime_good(lp, *cache_mtime))
-                return 0;
-
-        if (ret_path_cache) {
-                paths = set_new(&path_hash_ops);
-                if (!paths)
-                        return log_oom();
-        }
-
-        STRV_FOREACH(dir, (char**) lp->search_path) {
-                struct dirent *de;
-                _cleanup_closedir_ DIR *d = NULL;
-                struct stat st;
-
-                d = opendir(*dir);
-                if (!d) {
-                        if (errno != ENOENT)
-                                log_warning_errno(errno, "Failed to open \"%s\", ignoring: %m", *dir);
-                        continue;
-                }
-
-                /* Determine the latest lookup path modification time */
-                if (fstat(dirfd(d), &st) < 0)
-                        return log_error_errno(errno, "Failed to fstat %s: %m", *dir);
-
-                if (!lookup_paths_mtime_exclude(lp, *dir))
-                        mtime = MAX(mtime, timespec_load(&st.st_mtim));
-
-                FOREACH_DIRENT_ALL(de, d, log_warning_errno(errno, "Failed to read \"%s\", ignoring: %m", *dir)) {
-                        char *filename;
-                        _cleanup_free_ char *_filename_free = NULL, *simplified = NULL;
-                        const char *suffix, *dst = NULL;
-                        bool valid_unit_name;
-
-                        valid_unit_name = unit_name_is_valid(de->d_name, UNIT_NAME_ANY);
-
-                        /* We only care about valid units and dirs with certain suffixes, let's ignore the
-                         * rest. */
-                        if (!valid_unit_name &&
-                            !ENDSWITH_SET(de->d_name, ".wants", ".requires", ".d"))
-                                continue;
-
-                        filename = path_join(*dir, de->d_name);
-                        if (!filename)
-                                return log_oom();
-
-                        if (ret_path_cache) {
-                                r = set_consume(paths, filename);
-                                if (r < 0)
-                                        return log_oom();
-                                /* We will still use filename below. This is safe because we know the set
-                                 * holds a reference. */
-                        } else
-                                _filename_free = filename; /* Make sure we free the filename. */
-
-                        if (!valid_unit_name)
-                                continue;
-                        assert_se(suffix = strrchr(de->d_name, '.'));
-
-                        /* search_path is ordered by priority (highest first). If the name is already mapped
-                         * to something (incl. itself), it means that we have already seen it, and we should
-                         * ignore it here. */
-                        if (hashmap_contains(ids, de->d_name))
-                                continue;
-
-                        dirent_ensure_type(d, de);
-                        if (de->d_type == DT_LNK) {
-                                /* We don't explicitly check for alias loops here. unit_ids_map_get() which
-                                 * limits the number of hops should be used to access the map. */
-
-                                _cleanup_free_ char *target = NULL, *target_abs = NULL;
-
-                                r = readlinkat_malloc(dirfd(d), de->d_name, &target);
-                                if (r < 0) {
-                                        log_warning_errno(r, "Failed to read symlink %s/%s, ignoring: %m",
-                                                          *dir, de->d_name);
-                                        continue;
-                                }
-
-                                if (!path_is_absolute(target)) {
-                                        target_abs = path_join(*dir, target);
-                                        if (!target_abs)
-                                                return log_oom();
-
-                                        free_and_replace(target, target_abs);
-                                }
-
-                                /* Get rid of "." and ".." components in target path */
-                                r = chase_symlinks(target, lp->root_dir, CHASE_NOFOLLOW | CHASE_NONEXISTENT, &simplified, NULL);
-                                if (r < 0) {
-                                        log_warning_errno(r, "Failed to resolve symlink %s pointing to %s, ignoring: %m",
-                                                          filename, target);
-                                        continue;
-                                }
-
-                                /* Check if the symlink goes outside of our search path.
-                                 * If yes, it's a linked unit file or mask, and we don't care about the target name.
-                                 * Let's just store the link destination directly.
-                                 * If not, let's verify that it's a good symlink. */
-                                char *tail = path_startswith_strv(simplified, lp->search_path);
-                                if (tail) {
-                                        bool self_alias;
-
-                                        dst = basename(simplified);
-                                        self_alias = streq(dst, de->d_name);
-
-                                        if (is_path(tail))
-                                                log_full(self_alias ? LOG_DEBUG : LOG_WARNING,
-                                                         "Suspicious symlink %s→%s, treating as alias.",
-                                                         filename, simplified);
-
-                                        r = unit_validate_alias_symlink_and_warn(filename, simplified);
-                                        if (r < 0)
-                                                continue;
-
-                                        if (self_alias) {
-                                                /* A self-alias that has no effect */
-                                                log_debug("%s: self-alias: %s/%s → %s, ignoring.",
-                                                          __func__, *dir, de->d_name, dst);
-                                                continue;
-                                        }
-
-                                        log_debug("%s: alias: %s/%s → %s", __func__, *dir, de->d_name, dst);
-                                } else {
-                                        dst  = simplified;
-
-                                        log_debug("%s: linked unit file: %s/%s → %s", __func__, *dir, de->d_name, dst);
-                                }
-
-                        } else {
-                                dst = filename;
-                                log_debug("%s: normal unit file: %s", __func__, dst);
-                        }
-
-                        r = hashmap_put_strdup(&ids, de->d_name, dst);
-                        if (r < 0)
-                                return log_warning_errno(r, "Failed to add entry to hashmap (%s→%s): %m",
-                                                         de->d_name, dst);
-                }
-        }
-
-        /* Let's also put the names in the reverse db. */
-        Iterator it;
-        const char *dummy, *src;
-        HASHMAP_FOREACH_KEY(dummy, src, ids, it) {
-                const char *dst;
-
-                r = unit_ids_map_get(ids, src, &dst);
-                if (r < 0)
-                        continue;
-
-                if (null_or_empty_path(dst) != 0)
-                        continue;
-
-                /* Do not treat instance symlinks that point to the template as aliases */
-                if (unit_name_is_valid(basename(dst), UNIT_NAME_TEMPLATE) &&
-                    unit_name_is_valid(src, UNIT_NAME_INSTANCE))
-                        continue;
-
-                r = string_strv_hashmap_put(&names, basename(dst), src);
-                if (r < 0)
-                        return log_warning_errno(r, "Failed to add entry to hashmap (%s→%s): %m",
-                                                 basename(dst), src);
-        }
-
-        if (cache_mtime)
-                *cache_mtime = mtime;
-        *ret_unit_ids_map = TAKE_PTR(ids);
-        *ret_unit_names_map = TAKE_PTR(names);
-        if (ret_path_cache)
-                *ret_path_cache = TAKE_PTR(paths);
-
-        return 1;
-}
-
-int unit_file_find_fragment(
-                Hashmap *unit_ids_map,
-                Hashmap *unit_name_map,
-                const char *unit_name,
-                const char **ret_fragment_path,
-                Set **ret_names) {
-
-        const char *fragment = NULL;
-        _cleanup_free_ char *template = NULL, *instance = NULL;
-        _cleanup_set_free_free_ Set *names = NULL;
-        char **t, **nnn;
-        int r, name_type;
-
-        /* Finds a fragment path, and returns the set of names:
-         * if we have …/foo.service and …/foo-alias.service→foo.service,
-         * and …/foo@.service and …/foo-alias@.service→foo@.service,
-         * and …/foo@inst.service,
-         * this should return:
-         * foo.service → …/foo.service, {foo.service, foo-alias.service},
-         * foo-alias.service → …/foo.service, {foo.service, foo-alias.service},
-         * foo@.service → …/foo@.service, {foo@.service, foo-alias@.service},
-         * foo-alias@.service → …/foo@.service, {foo@.service, foo-alias@.service},
-         * foo@bar.service → …/foo@.service, {foo@bar.service, foo-alias@bar.service},
-         * foo-alias@bar.service → …/foo@.service, {foo@bar.service, foo-alias@bar.service},
-         * foo-alias@inst.service → …/foo@inst.service, {foo@inst.service, foo-alias@inst.service}.
-         */
-
-        name_type = unit_name_to_instance(unit_name, &instance);
-        if (name_type < 0)
-                return name_type;
-
-        names = set_new(&string_hash_ops);
-        if (!names)
-                return -ENOMEM;
-
-        /* The unit always has its own name if it's not a template. */
-        if (IN_SET(name_type, UNIT_NAME_PLAIN, UNIT_NAME_INSTANCE)) {
-                r = set_put_strdup(names, unit_name);
-                if (r < 0)
-                        return r;
-        }
-
-        /* First try to load fragment under the original name */
-        r = unit_ids_map_get(unit_ids_map, unit_name, &fragment);
-        if (r < 0 && !IN_SET(r, -ENOENT, -ENXIO))
-                return log_debug_errno(r, "Cannot load unit %s: %m", unit_name);
-
-        if (fragment) {
-                /* Add any aliases of the original name to the set of names */
-                nnn = hashmap_get(unit_name_map, basename(fragment));
-                STRV_FOREACH(t, nnn) {
-                        if (name_type == UNIT_NAME_INSTANCE && unit_name_is_valid(*t, UNIT_NAME_TEMPLATE)) {
-                                char *inst;
-
-                                r = unit_name_replace_instance(*t, instance, &inst);
-                                if (r < 0)
-                                        return log_debug_errno(r, "Cannot build instance name %s+%s: %m", *t, instance);
-
-                                if (!streq(unit_name, inst))
-                                        log_debug("%s: %s has alias %s", __func__, unit_name, inst);
-
-                                log_info("%s: %s+%s → %s", __func__, *t, instance, inst);
-                                r = set_consume(names, inst);
-                        } else {
-                                if (!streq(unit_name, *t))
-                                        log_debug("%s: %s has alias %s", __func__, unit_name, *t);
-
-                                r = set_put_strdup(names, *t);
-                        }
-                        if (r < 0)
-                                return r;
-                }
-        }
-
-        if (!fragment && name_type == UNIT_NAME_INSTANCE) {
-                /* Look for a fragment under the template name */
-
-                r = unit_name_template(unit_name, &template);
-                if (r < 0)
-                        return log_error_errno(r, "Failed to determine template name: %m");
-
-                r = unit_ids_map_get(unit_ids_map, template, &fragment);
-                if (r < 0 && !IN_SET(r, -ENOENT, -ENXIO))
-                        return log_debug_errno(r, "Cannot load template %s: %m", template);
-
-                if (fragment) {
-                        /* Add any aliases of the original name to the set of names */
-                        nnn = hashmap_get(unit_name_map, basename(fragment));
-                        STRV_FOREACH(t, nnn) {
-                                _cleanup_free_ char *inst = NULL;
-                                const char *inst_fragment = NULL;
-
-                                r = unit_name_replace_instance(*t, instance, &inst);
-                                if (r < 0)
-                                        return log_debug_errno(r, "Cannot build instance name %s+%s: %m", template, instance);
-
-                                /* Exclude any aliases that point in some other direction. */
-                                r = unit_ids_map_get(unit_ids_map, inst, &inst_fragment);
-                                if (r < 0 && !IN_SET(r, -ENOENT, -ENXIO))
-                                        return log_debug_errno(r, "Cannot find instance fragment %s: %m", inst);
-
-                                if (inst_fragment &&
-                                    !streq(basename(inst_fragment), basename(fragment))) {
-                                        log_debug("Instance %s has fragment %s and is not an alias of %s.",
-                                                  inst, inst_fragment, unit_name);
-                                        continue;
-                                }
-
-                                if (!streq(unit_name, inst))
-                                        log_debug("%s: %s has alias %s", __func__, unit_name, inst);
-                                r = set_consume(names, TAKE_PTR(inst));
-                                if (r < 0)
-                                        return r;
-                        }
-                }
-        }
-
-        *ret_fragment_path = fragment;
-        *ret_names = TAKE_PTR(names);
-
-        // FIXME: if instance, consider any unit names with different template name
-        return 0;
-}
-
 static const char * const rlmap[] = {
         "emergency", SPECIAL_EMERGENCY_TARGET,
         "-b",        SPECIAL_EMERGENCY_TARGET,
index 98ba677..6925dae 100644 (file)
@@ -3,13 +3,10 @@
 
 #include <stdbool.h>
 
-#include "hashmap.h"
-#include "time-util.h"
 #include "unit-name.h"
 
 typedef enum UnitFileState UnitFileState;
 typedef enum UnitFileScope UnitFileScope;
-typedef struct LookupPaths LookupPaths;
 
 enum UnitFileState {
         UNIT_FILE_ENABLED,
@@ -39,20 +36,4 @@ enum UnitFileScope {
 bool unit_type_may_alias(UnitType type) _const_;
 bool unit_type_may_template(UnitType type) _const_;
 
-int unit_validate_alias_symlink_and_warn(const char *filename, const char *target);
-
-int unit_file_build_name_map(
-                const LookupPaths *lp,
-                usec_t *ret_time,
-                Hashmap **ret_unit_ids_map,
-                Hashmap **ret_unit_names_map,
-                Set **ret_path_cache);
-
-int unit_file_find_fragment(
-                Hashmap *unit_ids_map,
-                Hashmap *unit_name_map,
-                const char *unit_name,
-                const char **ret_fragment_path,
-                Set **names);
-
 const char* runlevel_to_target(const char *rl);
index 943c62e..6335238 100644 (file)
@@ -30,7 +30,6 @@
 #include "cgroup-util.h"
 #include "copy.h"
 #include "cpu-set-util.h"
-#include "dirent-util.h"
 #include "dropin.h"
 #include "efivars.h"
 #include "env-util.h"
@@ -164,18 +163,12 @@ static bool arg_jobs_before = false;
 static bool arg_jobs_after = false;
 static char **arg_clean_what = NULL;
 
-/* This is a global cache that will be constructed on first use. */
-static Hashmap *cached_id_map = NULL;
-static Hashmap *cached_name_map = NULL;
-
 STATIC_DESTRUCTOR_REGISTER(arg_wall, strv_freep);
 STATIC_DESTRUCTOR_REGISTER(arg_root, freep);
 STATIC_DESTRUCTOR_REGISTER(arg_types, strv_freep);
 STATIC_DESTRUCTOR_REGISTER(arg_states, strv_freep);
 STATIC_DESTRUCTOR_REGISTER(arg_properties, strv_freep);
 STATIC_DESTRUCTOR_REGISTER(arg_clean_what, strv_freep);
-STATIC_DESTRUCTOR_REGISTER(cached_id_map, hashmap_freep);
-STATIC_DESTRUCTOR_REGISTER(cached_name_map, hashmap_freep);
 
 static int daemon_reload(int argc, char *argv[], void* userdata);
 static int trivial_method(int argc, char *argv[], void *userdata);
@@ -2655,24 +2648,38 @@ static int unit_find_paths(
                                 return log_error_errno(r, "Failed to get DropInPaths: %s", bus_error_message(&error, r));
                 }
         } else {
-                const char *_path;
-                _cleanup_set_free_free_ Set *names = NULL;
+                _cleanup_set_free_ Set *names = NULL;
+                _cleanup_free_ char *template = NULL;
 
-                if (!cached_name_map) {
-                        r = unit_file_build_name_map(lp, NULL, &cached_id_map, &cached_name_map, NULL);
-                        if (r < 0)
-                                return r;
-                }
+                names = set_new(NULL);
+                if (!names)
+                        return log_oom();
 
-                r = unit_file_find_fragment(cached_id_map, cached_name_map, unit_name, &_path, &names);
+                r = unit_find_template_path(unit_name, lp, &path, &template);
                 if (r < 0)
                         return r;
+                if (r > 0) {
+                        if (null_or_empty_path(path))
+                                /* The template is masked. Let's cut the process short. */
+                                return -ERFKILL;
+
+                        /* We found the unit file. If we followed symlinks, this name might be
+                         * different then the unit_name with started with. Look for dropins matching
+                         * that "final" name. */
+                        r = set_put(names, basename(path));
+                } else if (!template)
+                        /* No unit file, let's look for dropins matching the original name.
+                         * systemd has fairly complicated rules (based on unit type and provenience),
+                         * which units are allowed not to have the main unit file. We err on the
+                         * side of including too many files, and always try to load dropins. */
+                        r = set_put(names, unit_name);
+                else
+                        /* The cases where we allow a unit to exist without the main file are
+                         * never valid for templates. Don't try to load dropins in this case. */
+                        goto not_found;
 
-                if (_path) {
-                        path = strdup(_path);
-                        if (!path)
-                                return log_oom();
-                }
+                if (r < 0)
+                        return log_error_errno(r, "Failed to add unit name: %m");
 
                 if (ret_dropin_paths) {
                         r = unit_file_find_dropin_paths(arg_root, lp->search_path, NULL,
index b0e074f..59325d3 100644 (file)
@@ -135,10 +135,6 @@ tests += [
          [],
          'ENABLE_EFI'],
 
-        [['src/test/test-unit-file.c'],
-         [],
-         []],
-
         [['src/test/test-unit-name.c'],
          [libcore,
           libshared],
diff --git a/src/test/test-unit-file.c b/src/test/test-unit-file.c
deleted file mode 100644 (file)
index 6021164..0000000
+++ /dev/null
@@ -1,104 +0,0 @@
-/* SPDX-License-Identifier: LGPL-2.1+ */
-
-#include "path-lookup.h"
-#include "set.h"
-#include "special.h"
-#include "strv.h"
-#include "tests.h"
-#include "unit-file.h"
-
-static void test_unit_validate_alias_symlink_and_warn(void) {
-        log_info("/* %s */", __func__);
-
-        assert_se(unit_validate_alias_symlink_and_warn("/path/a.service", "/other/b.service") == 0);
-        assert_se(unit_validate_alias_symlink_and_warn("/path/a.service", "/other/b.socket") == -EXDEV);
-        assert_se(unit_validate_alias_symlink_and_warn("/path/a.service", "/other/b.foobar") == -EXDEV);
-        assert_se(unit_validate_alias_symlink_and_warn("/path/a@.service", "/other/b@.service") == 0);
-        assert_se(unit_validate_alias_symlink_and_warn("/path/a@.service", "/other/b@.socket") == -EXDEV);
-        assert_se(unit_validate_alias_symlink_and_warn("/path/a@XXX.service", "/other/b@YYY.service") == -EXDEV);
-        assert_se(unit_validate_alias_symlink_and_warn("/path/a@XXX.service", "/other/b@YYY.socket") == -EXDEV);
-        assert_se(unit_validate_alias_symlink_and_warn("/path/a@.service", "/other/b@YYY.service") == -EXDEV);
-        assert_se(unit_validate_alias_symlink_and_warn("/path/a@XXX.service", "/other/b@XXX.service") == 0);
-        assert_se(unit_validate_alias_symlink_and_warn("/path/a@XXX.service", "/other/b@.service") == 0);
-        assert_se(unit_validate_alias_symlink_and_warn("/path/a@.service", "/other/b.service") == -EXDEV);
-        assert_se(unit_validate_alias_symlink_and_warn("/path/a.service", "/other/b@.service") == -EXDEV);
-        assert_se(unit_validate_alias_symlink_and_warn("/path/a@.slice", "/other/b.slice") == -EINVAL);
-        assert_se(unit_validate_alias_symlink_and_warn("/path/a.slice", "/other/b.slice") == -EINVAL);
-}
-
-static void test_unit_file_build_name_map(char **ids) {
-        _cleanup_(lookup_paths_free) LookupPaths lp = {};
-        _cleanup_hashmap_free_ Hashmap *unit_ids = NULL;
-        _cleanup_hashmap_free_ Hashmap *unit_names = NULL;
-        Iterator i;
-        const char *k, *dst;
-        char **v;
-        usec_t mtime = 0;
-        int r;
-
-        assert_se(lookup_paths_init(&lp, UNIT_FILE_SYSTEM, 0, NULL) >= 0);
-
-        assert_se(unit_file_build_name_map(&lp, &mtime, &unit_ids, &unit_names, NULL) == 1);
-
-        HASHMAP_FOREACH_KEY(dst, k, unit_ids, i)
-                log_info("ids: %s → %s", k, dst);
-
-        HASHMAP_FOREACH_KEY(v, k, unit_names, i) {
-                _cleanup_free_ char *j = strv_join(v, ", ");
-                log_info("aliases: %s ← %s", k, j);
-        }
-
-        char buf[FORMAT_TIMESTAMP_MAX];
-        log_debug("Last modification time: %s", format_timestamp(buf, sizeof buf, mtime));
-
-        r = unit_file_build_name_map(&lp, &mtime, &unit_ids, &unit_names, NULL);
-        assert_se(IN_SET(r, 0, 1));
-        if (r == 0)
-                log_debug("Cache rebuild skipped based on mtime.");
-
-        char **id;
-        STRV_FOREACH(id, ids) {
-                 const char *fragment, *name;
-                 Iterator it;
-                 _cleanup_set_free_free_ Set *names = NULL;
-                 log_info("*** %s ***", *id);
-                 r = unit_file_find_fragment(unit_ids,
-                                             unit_names,
-                                             *id,
-                                             &fragment,
-                                             &names);
-                 assert(r == 0);
-                 log_info("fragment: %s", fragment);
-                 log_info("names:");
-                 SET_FOREACH(name, names, it)
-                         log_info("    %s", name);
-        }
-}
-
-static void test_runlevel_to_target(void) {
-        log_info("/* %s */", __func__);
-
-        in_initrd_force(false);
-        assert_se(streq_ptr(runlevel_to_target(NULL), NULL));
-        assert_se(streq_ptr(runlevel_to_target("unknown-runlevel"), NULL));
-        assert_se(streq_ptr(runlevel_to_target("rd.unknown-runlevel"), NULL));
-        assert_se(streq_ptr(runlevel_to_target("3"), SPECIAL_MULTI_USER_TARGET));
-        assert_se(streq_ptr(runlevel_to_target("rd.rescue"), NULL));
-
-        in_initrd_force(true);
-        assert_se(streq_ptr(runlevel_to_target(NULL), NULL));
-        assert_se(streq_ptr(runlevel_to_target("unknown-runlevel"), NULL));
-        assert_se(streq_ptr(runlevel_to_target("rd.unknown-runlevel"), NULL));
-        assert_se(streq_ptr(runlevel_to_target("3"), NULL));
-        assert_se(streq_ptr(runlevel_to_target("rd.rescue"), SPECIAL_RESCUE_TARGET));
-}
-
-int main(int argc, char **argv) {
-        test_setup_logging(LOG_DEBUG);
-
-        test_unit_validate_alias_symlink_and_warn();
-        test_unit_file_build_name_map(strv_skip(argv, 1));
-        test_runlevel_to_target();
-
-        return 0;
-}
index fe424c1..ef59218 100755 (executable)
@@ -122,148 +122,10 @@ test_template_dropins () {
 
     create_services foo bar@ yup@
 
-    # Declare some deps to check if the body was loaded
-    cat >>/etc/systemd/system/bar@.service <<EOF
-[Unit]
-After=bar-template-after.device
-EOF
-
-    cat >>/etc/systemd/system/yup@.service <<EOF
-[Unit]
-After=yup-template-after.device
-EOF
-
     ln -s /etc/systemd/system/bar@.service /etc/systemd/system/foo.service.wants/bar@1.service
     check_ok foo Wants bar@1.service
 
-    echo "*** test bar-alias@.service→bar@.service, but instance symlinks point to yup@.service ***"
-    ln -s bar@.service  /etc/systemd/system/bar-alias@.service
-    ln -s bar@1.service /etc/systemd/system/bar-alias@1.service
-    ln -s yup@.service  /etc/systemd/system/bar-alias@2.service
-    ln -s yup@3.service /etc/systemd/system/bar-alias@3.service
-
-    # create some dropin deps
-    mkdir -p /etc/systemd/system/bar@{,0,1,2,3}.service.requires/
-    mkdir -p /etc/systemd/system/yup@{,0,1,2,3}.service.requires/
-    mkdir -p /etc/systemd/system/bar-alias@{,0,1,2,3}.service.requires/
-
-    ln -s ../bar-template-requires.device /etc/systemd/system/bar@.service.requires/
-    ln -s ../bar-0-requires.device /etc/systemd/system/bar@0.service.requires/
-    ln -s ../bar-1-requires.device /etc/systemd/system/bar@1.service.requires/
-    ln -s ../bar-2-requires.device /etc/systemd/system/bar@2.service.requires/
-    ln -s ../bar-3-requires.device /etc/systemd/system/bar@3.service.requires/
-
-    ln -s ../yup-template-requires.device /etc/systemd/system/yup@.service.requires/
-    ln -s ../yup-0-requires.device /etc/systemd/system/yup@0.service.requires/
-    ln -s ../yup-1-requires.device /etc/systemd/system/yup@1.service.requires/
-    ln -s ../yup-2-requires.device /etc/systemd/system/yup@2.service.requires/
-    ln -s ../yup-3-requires.device /etc/systemd/system/yup@3.service.requires/
-
-    ln -s ../bar-alias-template-requires.device /etc/systemd/system/bar-alias@.service.requires/
-    ln -s ../bar-alias-0-requires.device /etc/systemd/system/bar-alias@0.service.requires/
-    ln -s ../bar-alias-1-requires.device /etc/systemd/system/bar-alias@1.service.requires/
-    ln -s ../bar-alias-2-requires.device /etc/systemd/system/bar-alias@2.service.requires/
-    ln -s ../bar-alias-3-requires.device /etc/systemd/system/bar-alias@3.service.requires/
-
-    systemctl daemon-reload
-
-    echo '*** bar@0 is aliased by bar-alias@0 ***'
-    systemctl show -p Names,Requires bar@0
-    systemctl show -p Names,Requires bar-alias@0
-    check_ok bar@0 Names bar@0
-    check_ok bar@0 Names bar-alias@0
-
-    check_ok bar@0 After bar-template-after.device
-
-    check_ok bar@0 Requires bar-0-requires.device
-    check_ok bar@0 Requires bar-alias-0-requires.device
-    check_ok bar@0 Requires bar-template-requires.device
-    check_ok bar@0 Requires bar-alias-template-requires.device
-    check_ko bar@0 Requires yup-template-requires.device
-
-    check_ok bar-alias@0 After bar-template-after.device
-
-    check_ok bar-alias@0 Requires bar-0-requires.device
-    check_ok bar-alias@0 Requires bar-alias-0-requires.device
-    check_ok bar-alias@0 Requires bar-template-requires.device
-    check_ok bar-alias@0 Requires bar-alias-template-requires.device
-    check_ko bar-alias@0 Requires yup-template-requires.device
-    check_ko bar-alias@0 Requires yup-0-requires.device
-
-    echo '*** bar@1 is aliased by bar-alias@1 ***'
-    systemctl show -p Names,Requires bar@1
-    systemctl show -p Names,Requires bar-alias@1
-    check_ok bar@1 Names bar@1
-    check_ok bar@1 Names bar-alias@1
-
-    check_ok bar@1 After bar-template-after.device
-
-    check_ok bar@1 Requires bar-1-requires.device
-    check_ok bar@1 Requires bar-alias-1-requires.device
-    check_ok bar@1 Requires bar-template-requires.device
-    # See https://github.com/systemd/systemd/pull/13119#discussion_r308145418
-    check_ok bar@1 Requires bar-alias-template-requires.device
-    check_ko bar@1 Requires yup-template-requires.device
-    check_ko bar@1 Requires yup-1-requires.device
-
-    check_ok bar-alias@1 After bar-template-after.device
-
-    check_ok bar-alias@1 Requires bar-1-requires.device
-    check_ok bar-alias@1 Requires bar-alias-1-requires.device
-    check_ok bar-alias@1 Requires bar-template-requires.device
-    check_ok bar-alias@1 Requires bar-alias-template-requires.device
-    check_ko bar-alias@1 Requires yup-template-requires.device
-    check_ko bar-alias@1 Requires yup-1-requires.device
-
-    echo '*** bar-alias@2 aliases yup@2, bar@2 is independent ***'
-    systemctl show -p Names,Requires bar@2
-    systemctl show -p Names,Requires bar-alias@2
-    check_ok bar@2 Names bar@2
-    check_ko bar@2 Names bar-alias@2
-
-    check_ok bar@2 After bar-template-after.device
-
-    check_ok bar@2 Requires bar-2-requires.device
-    check_ko bar@2 Requires bar-alias-2-requires.device
-    check_ok bar@2 Requires bar-template-requires.device
-    check_ko bar@2 Requires bar-alias-template-requires.device
-    check_ko bar@2 Requires yup-template-requires.device
-    check_ko bar@2 Requires yup-2-requires.device
-
-    check_ko bar-alias@2 After bar-template-after.device
-
-    check_ko bar-alias@2 Requires bar-2-requires.device
-    check_ok bar-alias@2 Requires bar-alias-2-requires.device
-    check_ko bar-alias@2 Requires bar-template-requires.device
-    check_ok bar-alias@2 Requires bar-alias-template-requires.device
-    check_ok bar-alias@2 Requires yup-template-requires.device
-    check_ok bar-alias@2 Requires yup-2-requires.device
-
-    echo '*** bar-alias@3 aliases yup@3, bar@3 is independent ***'
-    systemctl show -p Names,Requires bar@3
-    systemctl show -p Names,Requires bar-alias@3
-    check_ok bar@3 Names bar@3
-    check_ko bar@3 Names bar-alias@3
-
-    check_ok bar@3 After bar-template-after.device
-
-    check_ok bar@3 Requires bar-3-requires.device
-    check_ko bar@3 Requires bar-alias-3-requires.device
-    check_ok bar@3 Requires bar-template-requires.device
-    check_ko bar@3 Requires bar-alias-template-requires.device
-    check_ko bar@3 Requires yup-template-requires.device
-    check_ko bar@3 Requires yup-3-requires.device
-
-    check_ko bar-alias@3 After bar-template-after.device
-
-    check_ko bar-alias@3 Requires bar-3-requires.device
-    check_ok bar-alias@3 Requires bar-alias-3-requires.device
-    check_ko bar-alias@3 Requires bar-template-requires.device
-    check_ok bar-alias@3 Requires bar-alias-template-requires.device
-    check_ok bar-alias@3 Requires yup-template-requires.device
-    check_ok bar-alias@3 Requires yup-3-requires.device
-
-    clear_services foo {bar,yup,bar-alias}@{,1,2,3}
+    clear_services foo bar@ yup@
 }
 
 test_alias_dropins () {
@@ -280,7 +142,14 @@ test_alias_dropins () {
     rm /etc/systemd/system/b1.service
     clear_services a b
 
-    # Check that dependencies don't vary.
+    # A weird behavior: the dependencies for 'a' may vary. It can be
+    # changed by loading an alias...
+    #
+    # [1] 'a1' is loaded and then "renamed" into 'a'. 'a1' is therefore
+    # part of the names set so all its specific dropins are loaded.
+    #
+    # [2] 'a' is already loaded. 'a1' is simply only merged into 'a' so
+    # none of its dropins are loaded ('y' is missing from the deps).
     echo "*** test 2"
     create_services a x y
     mkdir -p /etc/systemd/system/a1.service.wants/
@@ -291,7 +160,7 @@ test_alias_dropins () {
     check_ok a1 Wants y.service
     systemctl start a
     check_ok a1 Wants x.service # see [2]
-    check_ok a1 Wants y.service
+    check_ko a1 Wants y.service
     systemctl stop a x y
     rm /etc/systemd/system/a1.service