diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 3288b0b8388..7521d599e9e 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -4559,251 +4559,48 @@ 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 symlink names we found */ + /* Let's try to add in all names that are aliases of this unit */ 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. */ - /* 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; + other = manager_get_unit((*u)->manager, k); + if (!other) + return r; /* return previous failure */ - r = unit_name_replace_instance(k, (*u)->instance, &instance); - if (r < 0) - return r; + r = unit_merge(other, *u); + if (r < 0) + return r; - other = manager_get_unit((*u)->manager, instance); - } else - other = manager_get_unit((*u)->manager, k); - - free(k); - - if (other) { - r = unit_merge(other, *u); - if (r >= 0) { - *u = other; - return merge_by_names(u, names, NULL); - } - } - - return r; + *u = other; + return merge_by_names(u, names, NULL); } - if (id == k) + if (streq_ptr(id, k)) unit_choose_id(*u, id); - - free(k); - } - - return 0; -} - -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(path); - - 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; - } - - merged = u; - r = merge_by_names(&merged, symlink_names, id); - if (r < 0) - return r; - - if (merged != u) { - u->load_state = UNIT_MERGED; - return 0; - } - - 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); - - /* 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; - } - - 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); - else - u->source_mtime = 0; } return 0; } int unit_load_fragment(Unit *u) { + const char *fragment; + _cleanup_set_free_free_ Set *names = NULL; int r; - Iterator i; - const char *t; assert(u); assert(u->load_state == UNIT_STUB); @@ -4814,78 +4611,79 @@ int unit_load_fragment(Unit *u) { return 0; } - /* 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); + 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; + struct stat st; + + /* 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; + + r = free_and_strdup(&u->fragment_path, fragment); + 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; + } + } + + /* 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; + + 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. */ + + 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; + } + } + + merged = u; + r = merge_by_names(&merged, names, id); if (r < 0) return r; - /* 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; - } - } - } + if (merged != u) + u->load_state = UNIT_MERGED; return 0; } diff --git a/src/core/manager.c b/src/core/manager.c index c176309edf3..f8a7e7d64e4 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -674,6 +674,12 @@ 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); +} + static int manager_setup_run_queue(Manager *m) { int r; @@ -1357,7 +1363,7 @@ Manager* manager_free(Manager *m) { strv_free(m->client_environment); hashmap_free(m->cgroup_unit); - set_free_free(m->unit_path_cache); + manager_free_unit_name_maps(m); free(m->switch_root); free(m->switch_root_init); @@ -1461,56 +1467,6 @@ 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; @@ -1670,7 +1626,10 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) { if (r < 0) log_warning_errno(r, "Failed to reduce unit file paths, ignoring: %m"); - manager_build_unit_path_cache(m); + manager_free_unit_name_maps(m); + r = unit_file_build_name_map(&m->lookup_paths, &m->unit_id_map, &m->unit_name_map, &m->unit_path_cache); + if (r < 0) + return log_error_errno(r, "Failed to build name map: %m"); { /* This block is (optionally) done with the reloading counter bumped */ @@ -2888,8 +2847,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); + /* Release the path and unit name caches */ + manager_free_unit_name_maps(m); + // FIXME: once this happens, we cannot load any more units manager_check_finished(m); @@ -3566,7 +3526,10 @@ int manager_reload(Manager *m) { if (r < 0) log_warning_errno(r, "Failed to reduce unit file paths, ignoring: %m"); - manager_build_unit_path_cache(m); + manager_free_unit_name_maps(m); + r = unit_file_build_name_map(&m->lookup_paths, &m->unit_id_map, &m->unit_name_map, &m->unit_path_cache); + if (r < 0) + log_warning_errno(r, "Failed to build name map: %m"); /* First, enumerate what we can from kernel and suchlike */ manager_enumerate_perpetual(m); diff --git a/src/core/manager.h b/src/core/manager.h index 9879082fea7..3cb37b3bf49 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -221,6 +221,8 @@ struct Manager { UnitFileScope unit_file_scope; LookupPaths lookup_paths; + Hashmap *unit_id_map; + Hashmap *unit_name_map; Set *unit_path_cache; char **transient_environment; /* The environment, as determined from config files, kernel cmdline and environment generators */ diff --git a/src/core/unit.c b/src/core/unit.c index fa89bd4a4d4..7561f1e1a1b 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -949,6 +949,9 @@ 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); diff --git a/src/shared/unit-file.c b/src/shared/unit-file.c index cde38c472b8..90fa949d247 100644 --- a/src/shared/unit-file.c +++ b/src/shared/unit-file.c @@ -1,7 +1,14 @@ /* SPDX-License-Identifier: LGPL-2.1+ */ +#include "dirent-util.h" +#include "fd-util.h" +#include "fs-util.h" #include "macro.h" +#include "path-lookup.h" +#include "set.h" +#include "stat-util.h" #include "string-util.h" +#include "strv.h" #include "unit-file.h" bool unit_type_may_alias(UnitType type) { @@ -94,3 +101,349 @@ int unit_validate_alias_symlink_and_warn(const char *filename, const char *targe 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; +} + +int unit_file_build_name_map( + const LookupPaths *lp, + 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; + + 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; + + d = opendir(*dir); + if (!d) { + if (errno != ENOENT) + log_warning_errno(errno, "Failed to open \"%s\", ignoring: %m", *dir); + continue; + } + + FOREACH_DIRENT(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; + + 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 (!unit_name_is_valid(de->d_name, UNIT_NAME_ANY)) + 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; + + 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); + 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); + } + + *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 0; +} + +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 (unit_name_is_valid(unit_name, 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 (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", *t); + + 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_info("%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; +} diff --git a/src/shared/unit-file.h b/src/shared/unit-file.h index e57f472f4f7..52e17f7cfb1 100644 --- a/src/shared/unit-file.h +++ b/src/shared/unit-file.h @@ -3,10 +3,12 @@ #include +#include "hashmap.h" #include "unit-name.h" typedef enum UnitFileState UnitFileState; typedef enum UnitFileScope UnitFileScope; +typedef struct LookupPaths LookupPaths; enum UnitFileState { UNIT_FILE_ENABLED, @@ -37,3 +39,16 @@ 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, + 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); diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 97f3176cc55..31d364cefed 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -33,6 +33,7 @@ #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" @@ -165,12 +166,18 @@ 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); @@ -2582,38 +2589,24 @@ static int unit_find_paths( return log_error_errno(r, "Failed to get DropInPaths: %s", bus_error_message(&error, r)); } } else { - _cleanup_set_free_ Set *names = NULL; - _cleanup_free_ char *template = NULL; + const char *_path; + _cleanup_set_free_free_ Set *names = NULL; - names = set_new(NULL); - if (!names) - return log_oom(); + if (!cached_name_map) { + r = unit_file_build_name_map(lp, &cached_id_map, &cached_name_map, NULL); + if (r < 0) + return r; + } - r = unit_find_template_path(unit_name, lp, &path, &template); + r = unit_file_find_fragment(cached_id_map, cached_name_map, unit_name, &_path, &names); 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 (r < 0) - return log_error_errno(r, "Failed to add unit name: %m"); + if (_path) { + path = strdup(_path); + if (!path) + return log_oom(); + } if (ret_dropin_paths) { r = unit_file_find_dropin_paths(arg_root, lp->search_path, NULL, diff --git a/src/test/test-unit-file.c b/src/test/test-unit-file.c index 5e281b28d5a..c5144a1b7ea 100644 --- a/src/test/test-unit-file.c +++ b/src/test/test-unit-file.c @@ -24,10 +24,32 @@ static void test_unit_validate_alias_symlink_and_warn(void) { assert_se(unit_validate_alias_symlink_and_warn("/path/a.slice", "/other/b.slice") == -EINVAL); } +static void test_unit_file_build_name_map(void) { + _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; + + assert_se(lookup_paths_init(&lp, UNIT_FILE_SYSTEM, 0, NULL) >= 0); + + assert_se(unit_file_build_name_map(&lp, &unit_ids, &unit_names, NULL) == 0); + + 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); + } +} + int main(int argc, char **argv) { test_setup_logging(LOG_DEBUG); test_unit_validate_alias_symlink_and_warn(); + test_unit_file_build_name_map(); return 0; } diff --git a/test/TEST-15-DROPIN/test-dropin.sh b/test/TEST-15-DROPIN/test-dropin.sh index f7856807c45..2cef5a3c5b5 100755 --- a/test/TEST-15-DROPIN/test-dropin.sh +++ b/test/TEST-15-DROPIN/test-dropin.sh @@ -158,14 +158,14 @@ EOF systemctl show -p Names,Requires bar@0 systemctl show -p Names,Requires bar-alias@0 check_ok bar@0 Names bar@0 - check_ko bar@0 Names bar-alias@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_ko bar@0 Requires bar-alias-0-requires.device + check_ok bar@0 Requires bar-alias-0-requires.device check_ok bar@0 Requires bar-template-requires.device - check_ko bar@0 Requires bar-alias-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 @@ -181,15 +181,15 @@ EOF systemctl show -p Names,Requires bar@1 systemctl show -p Names,Requires bar-alias@1 check_ok bar@1 Names bar@1 - check_ko bar@1 Names bar-alias@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_ko bar@1 Requires bar-alias-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_ko bar@1 Requires bar-alias-template-requires.device + 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 @@ -241,14 +241,14 @@ EOF check_ko bar@3 Requires yup-template-requires.device check_ko bar@3 Requires yup-3-requires.device - check_ok bar-alias@3 After bar-template-after.device + check_ko bar-alias@3 After bar-template-after.device - check_ok bar-alias@3 Requires bar-3-requires.device + check_ko bar-alias@3 Requires bar-3-requires.device check_ok bar-alias@3 Requires bar-alias-3-requires.device - check_ok bar-alias@3 Requires bar-template-requires.device + check_ko bar-alias@3 Requires bar-template-requires.device check_ok bar-alias@3 Requires bar-alias-template-requires.device - check_ko bar-alias@3 Requires yup-template-requires.device - check_ko bar-alias@3 Requires yup-3-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} } @@ -267,14 +267,7 @@ test_alias_dropins () { rm /etc/systemd/system/b1.service clear_services a b - # 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). + # Check that dependencies don't vary. echo "*** test 2" create_services a x y mkdir -p /etc/systemd/system/a1.service.wants/ @@ -285,7 +278,7 @@ test_alias_dropins () { check_ok a1 Wants y.service systemctl start a check_ok a1 Wants x.service # see [2] - check_ko a1 Wants y.service + check_ok a1 Wants y.service systemctl stop a x y rm /etc/systemd/system/a1.service