From f971def3c23d780aab99d988bd71b94931d74be6 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Thu, 12 May 2022 00:05:04 +0200 Subject: [PATCH 1/2] core: Return 1 from unit_add_dependency() on success To allow checking if adding dependency was skipped or not. --- src/core/unit.c | 2 +- src/test/test-engine.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/unit.c b/src/core/unit.c index b0756bc6f4..fd95e02153 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -3146,7 +3146,7 @@ int unit_add_dependency( if (!noop) unit_add_to_dbus_queue(u); - return 0; + return 1; } int unit_add_two_dependencies(Unit *u, UnitDependency d, UnitDependency e, Unit *other, bool add_reference, UnitDependencyMask mask) { diff --git a/src/test/test-engine.c b/src/test/test-engine.c index ef055360fb..d430076056 100644 --- a/src/test/test-engine.c +++ b/src/test/test-engine.c @@ -189,8 +189,8 @@ int main(int argc, char *argv[]) { assert_se(!hashmap_get(unit_get_dependencies(a, UNIT_PROPAGATES_RELOAD_TO), c)); assert_se(!hashmap_get(unit_get_dependencies(c, UNIT_RELOAD_PROPAGATED_FROM), a)); - assert_se(unit_add_dependency(a, UNIT_PROPAGATES_RELOAD_TO, b, true, UNIT_DEPENDENCY_UDEV) == 0); - assert_se(unit_add_dependency(a, UNIT_PROPAGATES_RELOAD_TO, c, true, UNIT_DEPENDENCY_PROC_SWAP) == 0); + assert_se(unit_add_dependency(a, UNIT_PROPAGATES_RELOAD_TO, b, true, UNIT_DEPENDENCY_UDEV) >= 0); + assert_se(unit_add_dependency(a, UNIT_PROPAGATES_RELOAD_TO, c, true, UNIT_DEPENDENCY_PROC_SWAP) >= 0); assert_se( hashmap_get(unit_get_dependencies(a, UNIT_PROPAGATES_RELOAD_TO), b)); assert_se( hashmap_get(unit_get_dependencies(b, UNIT_RELOAD_PROPAGATED_FROM), a)); From 556b6f4e6fd7121920d27707668828cb7ae42380 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Wed, 11 May 2022 23:48:07 +0200 Subject: [PATCH 2/2] core: Add trace logging to mount_add_device_dependencies() To help debug missing implicit device deps in https://github.com/systemd/systemd/issues/13775#issuecomment-1122969810. --- src/core/mount.c | 38 +++++++++++++++++++++++++++++++------- src/core/unit.h | 8 ++++++++ 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/src/core/mount.c b/src/core/mount.c index 20b4bb6d2b..dcfbf970d7 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -354,26 +354,40 @@ static int mount_add_device_dependencies(Mount *m) { assert(m); + log_unit_trace(UNIT(m), "Processing implicit device dependencies"); + p = get_mount_parameters(m); - if (!p) + if (!p) { + log_unit_trace(UNIT(m), "Missing mount parameters, skipping implicit device dependencies"); return 0; + } - if (!p->what) + if (!p->what) { + log_unit_trace(UNIT(m), "Missing mount source, skipping implicit device dependencies"); return 0; + } - if (mount_is_bind(p)) + if (mount_is_bind(p)) { + log_unit_trace(UNIT(m), "Mount unit is a bind mount, skipping implicit device dependencies"); return 0; + } - if (!is_device_path(p->what)) + if (!is_device_path(p->what)) { + log_unit_trace(UNIT(m), "Mount source is not a device path, skipping implicit device dependencies"); return 0; + } /* /dev/root is a really weird thing, it's not a real device, but just a path the kernel exports for * the root file system specified on the kernel command line. Ignore it here. */ - if (PATH_IN_SET(p->what, "/dev/root", "/dev/nfs")) + if (PATH_IN_SET(p->what, "/dev/root", "/dev/nfs")) { + log_unit_trace(UNIT(m), "Mount source is in /dev/root or /dev/nfs, skipping implicit device dependencies"); return 0; + } - if (path_equal(m->where, "/")) + if (path_equal(m->where, "/")) { + log_unit_trace(UNIT(m), "Mount destination is '/', skipping implicit device dependencies"); return 0; + } /* Mount units from /proc/self/mountinfo are not bound to devices by default since they're subject to * races when mounts are established by other tools with different backing devices than what we @@ -388,13 +402,23 @@ static int mount_add_device_dependencies(Mount *m) { r = unit_add_node_dependency(UNIT(m), p->what, dep, mask); if (r < 0) return r; + if (r > 0) + log_unit_trace(UNIT(m), "Added %s dependency on %s", unit_dependency_to_string(dep), p->what); + if (mount_propagate_stop(m)) { r = unit_add_node_dependency(UNIT(m), p->what, UNIT_STOP_PROPAGATED_FROM, mask); if (r < 0) return r; + if (r > 0) + log_unit_trace(UNIT(m), "Added %s dependency on %s", + unit_dependency_to_string(UNIT_STOP_PROPAGATED_FROM), p->what); } - return unit_add_blockdev_dependency(UNIT(m), p->what, mask); + r = unit_add_blockdev_dependency(UNIT(m), p->what, mask); + if (r > 0) + log_unit_trace(UNIT(m), "Added %s dependency on %s", unit_dependency_to_string(UNIT_AFTER), p->what); + + return r; } static int mount_add_quota_dependencies(Mount *m) { diff --git a/src/core/unit.h b/src/core/unit.h index 20c511c108..c6a2fadf52 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -1025,6 +1025,14 @@ Condition *unit_find_failed_condition(Unit *u); #define log_unit_warning_errno(unit, error, ...) log_unit_full_errno(unit, LOG_WARNING, error, __VA_ARGS__) #define log_unit_error_errno(unit, error, ...) log_unit_full_errno(unit, LOG_ERR, error, __VA_ARGS__) +#if LOG_TRACE +# define log_unit_trace(...) log_unit_debug(__VA_ARGS__) +# define log_unit_trace_errno(...) log_unit_debug_errno(__VA_ARGS__) +#else +# define log_unit_trace(...) do {} while (0) +# define log_unit_trace_errno(e, ...) (-ERRNO_VALUE(e)) +#endif + #define log_unit_struct_errno(unit, level, error, ...) \ ({ \ const Unit *_u = (unit); \