From: INSUN PYO Date: Tue, 9 Jun 2020 00:55:24 +0000 (+0900) Subject: Revert: Rework unit loading to take into account all aliases X-Git-Tag: submit/tizen/20200617.084905~2 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=95b09c530e12cf7a26fcdcd83dab485ccc13e92a;p=platform%2Fupstream%2Fsystemd.git Revert: Rework unit loading to take into account all aliases 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 --- diff --git a/src/analyze/analyze.c b/src/analyze/analyze.c index 991e61d..19c4ce9 100644 --- a/src/analyze/analyze.c +++ b/src/analyze/analyze.c @@ -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 }, diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index b6bd38e..299b238 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -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; } diff --git a/src/core/manager.c b/src/core/manager.c index 91ed64b..0dd6675 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -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); diff --git a/src/core/manager.h b/src/core/manager.h index 05ae30c..2fbe05e 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -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 */ diff --git a/src/core/unit.c b/src/core/unit.c index 759a529..7c8cefe 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -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); diff --git a/src/shared/unit-file.c b/src/shared/unit-file.c index 28cd3c8..e99d304 100644 --- a/src/shared/unit-file.c +++ b/src/shared/unit-file.c @@ -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, diff --git a/src/shared/unit-file.h b/src/shared/unit-file.h index 98ba677..6925dae 100644 --- a/src/shared/unit-file.h +++ b/src/shared/unit-file.h @@ -3,13 +3,10 @@ #include -#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); diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 943c62e..6335238 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -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, diff --git a/src/test/meson.build b/src/test/meson.build index b0e074f..59325d3 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -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 index 6021164..0000000 --- a/src/test/test-unit-file.c +++ /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; -} diff --git a/test/TEST-15-DROPIN/test-dropin.sh b/test/TEST-15-DROPIN/test-dropin.sh index fe424c1..ef59218 100755 --- a/test/TEST-15-DROPIN/test-dropin.sh +++ b/test/TEST-15-DROPIN/test-dropin.sh @@ -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 <>/etc/systemd/system/yup@.service <