diff --git a/src/basic/conf-files.c b/src/basic/conf-files.c index c0c22610d77..b5780194df5 100644 --- a/src/basic/conf-files.c +++ b/src/basic/conf-files.c @@ -43,7 +43,6 @@ static int files_add(Hashmap *h, const char *root, const char *path, const char int r; assert(path); - assert(suffix); dirpath = prefix_roota(root, path); @@ -94,7 +93,6 @@ static int conf_files_list_strv_internal(char ***strv, const char *suffix, const int r; assert(strv); - assert(suffix); /* This alters the dirs string array */ if (!path_strv_resolve_uniq(dirs, root)) @@ -126,7 +124,6 @@ int conf_files_list_strv(char ***strv, const char *suffix, const char *root, con _cleanup_strv_free_ char **copy = NULL; assert(strv); - assert(suffix); copy = strv_copy((char**) dirs); if (!copy) diff --git a/src/basic/dirent-util.c b/src/basic/dirent-util.c index 59067121b74..6b9d26773ea 100644 --- a/src/basic/dirent-util.c +++ b/src/basic/dirent-util.c @@ -70,5 +70,8 @@ bool dirent_is_file_with_suffix(const struct dirent *de, const char *suffix) { if (de->d_name[0] == '.') return false; + if (!suffix) + return true; + return endswith(de->d_name, suffix); } diff --git a/src/core/load-dropin.c b/src/core/load-dropin.c index fc07151d37e..ff3636149a8 100644 --- a/src/core/load-dropin.c +++ b/src/core/load-dropin.c @@ -19,53 +19,121 @@ #include "conf-parser.h" +#include "fs-util.h" #include "load-dropin.h" #include "load-fragment.h" #include "log.h" +#include "stat-util.h" +#include "string-util.h" #include "strv.h" #include "unit-name.h" #include "unit.h" -static int add_dependency_consumer( - UnitDependency dependency, - const char *entry, - const char* filepath, - void *arg) { - Unit *u = arg; +static bool unit_name_compatible(const char *a, const char *b) { + _cleanup_free_ char *prefix = NULL; int r; - assert(u); + /* the straightforward case: the symlink name matches the target */ + if (streq(a, b)) + return true; - r = unit_add_dependency_by_name(u, dependency, entry, filepath, true); + r = unit_name_template(a, &prefix); + if (r < 0) { + log_oom(); + return true; + } + + /* an instance name points to a target that is just the template name */ + if (streq(prefix, b)) + return true; + + return false; +} + +static int process_deps(Unit *u, UnitDependency dependency, const char *dir_suffix) { + _cleanup_strv_free_ char **paths = NULL; + char **p; + int r; + + r = unit_file_find_dropin_paths(NULL, + u->manager->lookup_paths.search_path, + u->manager->unit_path_cache, + dir_suffix, + NULL, + u->names, + &paths); if (r < 0) - log_error_errno(r, "Cannot add dependency %s to %s, ignoring: %m", entry, u->id); + return r; + + STRV_FOREACH(p, paths) { + const char *entry; + _cleanup_free_ char *target = NULL; + + entry = basename(*p); + + if (null_or_empty_path(*p) > 0) { + /* an error usually means an invalid symlink, which is not a mask */ + log_unit_debug(u, "%s dependency on %s is masked by %s, ignoring.", + unit_dependency_to_string(dependency), entry, *p); + continue; + } + + r = is_symlink(*p); + if (r < 0) { + log_unit_warning_errno(u, r, "%s dropin %s unreadable, ignoring: %m", + unit_dependency_to_string(dependency), *p); + continue; + } + if (r == 0) { + log_unit_warning(u, "%s dependency dropin %s is not a symlink, ignoring.", + unit_dependency_to_string(dependency), *p); + continue; + } + + if (!unit_name_is_valid(entry, UNIT_NAME_ANY)) { + log_unit_warning(u, "%s dependency dropin %s is not a valid unit name, ignoring.", + unit_dependency_to_string(dependency), *p); + continue; + } + + r = readlink_malloc(*p, &target); + if (r < 0) { + log_unit_warning_errno(u, r, "readlink(\"%s\") failed, ignoring: %m", *p); + continue; + } + + /* We don't treat this as an error, especially because we didn't check this for a + * long time. Nevertheless, we warn, because such mismatch can be mighty confusing. */ + if (!unit_name_compatible(entry, basename(target))) + log_unit_warning(u, "%s dependency dropin %s target %s has different name", + unit_dependency_to_string(dependency), *p, target); + + r = unit_add_dependency_by_name(u, dependency, entry, *p, true); + if (r < 0) + log_unit_error_errno(u, r, "cannot add %s dependency on %s, ignoring: %m", + unit_dependency_to_string(dependency), entry); + } return 0; } int unit_load_dropin(Unit *u) { _cleanup_strv_free_ char **l = NULL; - Iterator i; - char *t, **f; + char **f; int r; assert(u); - /* Load dependencies from supplementary drop-in directories */ + /* Load dependencies from .wants and .requires directories */ + r = process_deps(u, UNIT_WANTS, ".wants"); + if (r < 0) + return r; - SET_FOREACH(t, u->names, i) { - char **p; - - STRV_FOREACH(p, u->manager->lookup_paths.search_path) { - unit_file_process_dir(NULL, u->manager->unit_path_cache, *p, t, - ".wants", UNIT_WANTS, - add_dependency_consumer, u, NULL); - unit_file_process_dir(NULL, u->manager->unit_path_cache, *p, t, - ".requires", UNIT_REQUIRES, - add_dependency_consumer, u, NULL); - } - } + r = process_deps(u, UNIT_REQUIRES, ".requires"); + if (r < 0) + return r; + /* Load .conf dropins */ r = unit_find_dropin_paths(u, &l); if (r <= 0) return 0; diff --git a/src/core/load-dropin.h b/src/core/load-dropin.h index 319827dfb99..5828a223cee 100644 --- a/src/core/load-dropin.h +++ b/src/core/load-dropin.h @@ -25,11 +25,11 @@ /* Read service data supplementary drop-in directories */ static inline int unit_find_dropin_paths(Unit *u, char ***paths) { - return unit_file_find_dropin_paths(NULL, - u->manager->lookup_paths.search_path, - u->manager->unit_path_cache, - u->names, - paths); + return unit_file_find_dropin_conf_paths(NULL, + u->manager->lookup_paths.search_path, + u->manager->unit_path_cache, + u->names, + paths); } int unit_load_dropin(Unit *u); diff --git a/src/shared/dropin.c b/src/shared/dropin.c index 06cf3de6202..3917eb8f236 100644 --- a/src/shared/dropin.c +++ b/src/shared/dropin.c @@ -117,17 +117,12 @@ int write_drop_in_format(const char *dir, const char *unit, unsigned level, return write_drop_in(dir, unit, level, name, p); } -static int iterate_dir( - const char *path, +static int unit_file_find_dir( const char *original_root, - UnitDependency dependency, - dependency_consumer_t consumer, - void *arg, - char ***strv) { + const char *path, + char ***dirs) { _cleanup_free_ char *chased = NULL; - _cleanup_closedir_ DIR *d = NULL; - struct dirent *de; int r; assert(path); @@ -137,52 +132,21 @@ static int iterate_dir( return log_full_errno(r == -ENOENT ? LOG_DEBUG : LOG_WARNING, r, "Failed to canonicalize path %s: %m", path); - /* The config directories are special, since the order of the - * drop-ins matters */ - if (dependency < 0) { - r = strv_push(strv, chased); - if (r < 0) - return log_oom(); - - chased = NULL; - return 0; - } - - assert(consumer); - - d = opendir(chased); - if (!d) { - if (errno == ENOENT) - return 0; - - return log_warning_errno(errno, "Failed to open directory %s: %m", path); - } - - FOREACH_DIRENT(de, d, return log_warning_errno(errno, "Failed to read directory %s: %m", path)) { - _cleanup_free_ char *f = NULL; - - f = strjoin(path, "/", de->d_name); - if (!f) - return log_oom(); - - r = consumer(dependency, de->d_name, f, arg); - if (r < 0) - return r; - } + r = strv_push(dirs, chased); + if (r < 0) + return log_oom(); + chased = NULL; return 0; } -int unit_file_process_dir( +static int unit_file_find_dirs( const char *original_root, Set *unit_path_cache, const char *unit_path, const char *name, const char *suffix, - UnitDependency dependency, - dependency_consumer_t consumer, - void *arg, - char ***strv) { + char ***dirs) { _cleanup_free_ char *path = NULL; int r; @@ -195,8 +159,11 @@ int unit_file_process_dir( if (!path) return log_oom(); - if (!unit_path_cache || set_get(unit_path_cache, path)) - (void) iterate_dir(path, original_root, dependency, consumer, arg, strv); + if (!unit_path_cache || set_get(unit_path_cache, path)) { + r = unit_file_find_dir(original_root, path, dirs); + if (r < 0) + return r; + } if (unit_name_is_valid(name, UNIT_NAME_INSTANCE)) { _cleanup_free_ char *template = NULL, *p = NULL; @@ -210,8 +177,11 @@ int unit_file_process_dir( if (!p) return log_oom(); - if (!unit_path_cache || set_get(unit_path_cache, p)) - (void) iterate_dir(p, original_root, dependency, consumer, arg, strv); + if (!unit_path_cache || set_get(unit_path_cache, p)) { + r = unit_file_find_dir(original_root, p, dirs); + if (r < 0) + return r; + } } return 0; @@ -221,30 +191,28 @@ int unit_file_find_dropin_paths( const char *original_root, char **lookup_path, Set *unit_path_cache, + const char *dir_suffix, + const char *file_suffix, Set *names, char ***paths) { - _cleanup_strv_free_ char **strv = NULL, **ans = NULL; + _cleanup_strv_free_ char **dirs = NULL, **ans = NULL; Iterator i; - char *t; + char *t, **p; int r; assert(paths); - SET_FOREACH(t, names, i) { - char **p; - + SET_FOREACH(t, names, i) STRV_FOREACH(p, lookup_path) - unit_file_process_dir(original_root, unit_path_cache, *p, t, ".d", - _UNIT_DEPENDENCY_INVALID, NULL, NULL, &strv); - } + unit_file_find_dirs(original_root, unit_path_cache, *p, t, dir_suffix, &dirs); - if (strv_isempty(strv)) + if (strv_isempty(dirs)) return 0; - r = conf_files_list_strv(&ans, ".conf", NULL, (const char**) strv); + r = conf_files_list_strv(&ans, file_suffix, NULL, (const char**) dirs); if (r < 0) - return log_warning_errno(r, "Failed to get list of configuration files: %m"); + return log_warning_errno(r, "Failed to sort the list of configuration files: %m"); *paths = ans; ans = NULL; diff --git a/src/shared/dropin.h b/src/shared/dropin.h index 761b2508860..a2b8cdce613 100644 --- a/src/shared/dropin.h +++ b/src/shared/dropin.h @@ -33,31 +33,24 @@ int write_drop_in(const char *dir, const char *unit, unsigned level, int write_drop_in_format(const char *dir, const char *unit, unsigned level, const char *name, const char *format, ...) _printf_(5, 6); -/** - * This callback will be called for each directory entry @entry, - * with @filepath being the full path to the entry. - * - * If return value is negative, loop will be aborted. - */ -typedef int (*dependency_consumer_t)(UnitDependency dependency, - const char *entry, - const char* filepath, - void *arg); - -int unit_file_process_dir( - const char *original_root, - Set * unit_path_cache, - const char *unit_path, - const char *name, - const char *suffix, - UnitDependency dependency, - dependency_consumer_t consumer, - void *arg, - char ***strv); - int unit_file_find_dropin_paths( const char *original_root, char **lookup_path, Set *unit_path_cache, + const char *dir_suffix, + const char *file_suffix, Set *names, char ***paths); + +static inline int unit_file_find_dropin_conf_paths( + const char *original_root, + char **lookup_path, + Set *unit_path_cache, + Set *names, + char ***paths) { + return unit_file_find_dropin_paths(original_root, + lookup_path, + unit_path_cache, + ".d", ".conf", + names, paths); +} diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 2964b4e6b28..5cb07739d43 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -2601,7 +2601,8 @@ static int unit_find_paths( return log_error_errno(r, "Failed to add unit name: %m"); if (dropin_paths) { - r = unit_file_find_dropin_paths(arg_root, lp->search_path, NULL, names, &dropins); + r = unit_file_find_dropin_conf_paths(arg_root, lp->search_path, + NULL, names, &dropins); if (r < 0) return r; } diff --git a/test/TEST-15-DROPIN/Makefile b/test/TEST-15-DROPIN/Makefile new file mode 120000 index 00000000000..e9f93b1104c --- /dev/null +++ b/test/TEST-15-DROPIN/Makefile @@ -0,0 +1 @@ +../TEST-01-BASIC/Makefile \ No newline at end of file diff --git a/test/TEST-15-DROPIN/test-dropin.sh b/test/TEST-15-DROPIN/test-dropin.sh new file mode 100755 index 00000000000..9d8af99ac48 --- /dev/null +++ b/test/TEST-15-DROPIN/test-dropin.sh @@ -0,0 +1,274 @@ +#! /bin/bash + +set -e +set -x + +_clear_service () { + systemctl stop $1.service 2>/dev/null || : + rm -f /{etc,run,usr/lib}/systemd/system/$1.service + rm -fr /{etc,run,usr/lib}/systemd/system/$1.service.d + rm -fr /{etc,run,usr/lib}/systemd/system/$1.service.{wants,requires} +} + +clear_services () { + for u in $*; do + _clear_service $u + done + systemctl daemon-reload +} + +create_service () { + clear_services $1 + + cat >/etc/systemd/system/$1.service</usr/lib/systemd/system/a.service.d/wants-b.conf</dev/null + + # mask some services that we do not want to run in these tests + ln -s /dev/null $initdir/etc/systemd/system/systemd-hwdb-update.service + ln -s /dev/null $initdir/etc/systemd/system/systemd-journal-catalog-update.service + ln -s /dev/null $initdir/etc/systemd/system/systemd-networkd.service + ln -s /dev/null $initdir/etc/systemd/system/systemd-networkd.socket + ln -s /dev/null $initdir/etc/systemd/system/systemd-resolved.service + + # import the test scripts in the rootfs and plug them in systemd + cp testsuite.service $initdir/etc/systemd/system/ + cp test-dropin.sh $initdir/ + setup_testsuite + + # create dedicated rootfs for nspawn (located in $TESTDIR/nspawn-root) + setup_nspawn_root +} + +test_cleanup() { + return 0 +} + +do_test "$@" diff --git a/test/TEST-15-DROPIN/testsuite.service b/test/TEST-15-DROPIN/testsuite.service new file mode 100644 index 00000000000..d9790c26102 --- /dev/null +++ b/test/TEST-15-DROPIN/testsuite.service @@ -0,0 +1,7 @@ +[Unit] +Description=Testsuite service +After=multi-user.target + +[Service] +ExecStart=/test-dropin.sh +Type=oneshot