From 871768e94938a6a01555da0d02d29449a6c1585f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 4 Feb 2017 20:50:44 -0500 Subject: [PATCH] core: when loading .wants and .requires, follow the same logic as .d conf dropins Essentially, instead of sequentially adding deps based on all symlinks encountered in .wants and .requires dirs for each name and each unit file load path, iteratate over the load paths and unit names gathering symlinks, then order them based on priority, filter our masked entries, and then iterate over the final list, adding dependencies. This patch doesn't change the logic too much, except of course the added ability to mask entires in .wants and .requires. Adding additional filtering on the symlinks is left for later patches. Fixes #1169. Fixes #4830. Example log errors: Feb 04 22:13:28 systemd[1462]: foo.service: Wants dependency on empty_file.service is masked by /home/zbyszek/.config/systemd/user/foo.service.wants/empty_file.service, ignoring Feb 04 22:13:28 systemd[1462]: foo.service: Wants dependency on masked.service is masked by /home/zbyszek/.config/systemd/user/foo.service.wants/masked.service, ignoring --- src/basic/conf-files.c | 3 -- src/basic/dirent-util.c | 3 ++ src/core/load-dropin.c | 78 ++++++++++++++++++++++++++------------- src/core/load-dropin.h | 10 ++--- src/shared/dropin.c | 11 ++++-- src/shared/dropin.h | 26 +++++++------ src/systemctl/systemctl.c | 3 +- 7 files changed, 85 insertions(+), 49 deletions(-) diff --git a/src/basic/conf-files.c b/src/basic/conf-files.c index c0c22610d77c1..b5780194df5df 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 59067121b74f0..6b9d26773ea78 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 fc07151d37ec0..ed6ab4c2fd4b1 100644 --- a/src/core/load-dropin.c +++ b/src/core/load-dropin.c @@ -22,50 +22,78 @@ #include "load-dropin.h" #include "load-fragment.h" #include "log.h" +#include "stat-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 int process_deps(Unit *u, UnitDependency dependency, const char *dir_suffix) { + _cleanup_strv_free_ char **paths = NULL; + char **p; int r; - assert(u); - - r = unit_add_dependency_by_name(u, dependency, entry, filepath, true); + 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; + + 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; + } + + /* TODO: add more checks on the symlink name and target here */ + + 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 */ - - SET_FOREACH(t, u->names, i) { - char **p; + /* Load dependencies from .wants and .requires directories */ + r = process_deps(u, UNIT_WANTS, ".wants"); + if (r < 0) + return r; - 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 319827dfb9980..5828a223cee7c 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 06cf3de6202d5..3944ccec44f13 100644 --- a/src/shared/dropin.c +++ b/src/shared/dropin.c @@ -173,7 +173,7 @@ static int iterate_dir( return 0; } -int unit_file_process_dir( +static int unit_file_process_dir( const char *original_root, Set *unit_path_cache, const char *unit_path, @@ -221,6 +221,8 @@ 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) { @@ -235,16 +237,17 @@ int unit_file_find_dropin_paths( char **p; STRV_FOREACH(p, lookup_path) - unit_file_process_dir(original_root, unit_path_cache, *p, t, ".d", + unit_file_process_dir(original_root, unit_path_cache, + *p, t, dir_suffix, _UNIT_DEPENDENCY_INVALID, NULL, NULL, &strv); } if (strv_isempty(strv)) return 0; - r = conf_files_list_strv(&ans, ".conf", NULL, (const char**) strv); + r = conf_files_list_strv(&ans, file_suffix, NULL, (const char**) strv); 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 761b25088601c..22cd2fa22968f 100644 --- a/src/shared/dropin.h +++ b/src/shared/dropin.h @@ -44,20 +44,24 @@ typedef int (*dependency_consumer_t)(UnitDependency dependency, 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 2964b4e6b28b0..5cb07739d4399 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; }