From 45e1c7d5ca2d736699f7b0565b06d25cdf245f01 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 24 Oct 2017 12:18:27 +0200 Subject: [PATCH 01/17] virt: trivial whitespace fixes --- src/basic/virt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/basic/virt.c b/src/basic/virt.c index d5665edc1b..bacaabc5c0 100644 --- a/src/basic/virt.c +++ b/src/basic/virt.c @@ -216,6 +216,7 @@ static int detect_vm_dmi(void) { } static int detect_vm_xen(void) { + /* Check for Dom0 will be executed later in detect_vm_xen_dom0 Thats why we dont check the content of /proc/xen/capabilities here. */ if (access("/proc/xen/capabilities", F_OK) < 0) { @@ -224,8 +225,7 @@ static int detect_vm_xen(void) { } log_debug("Virtualization XEN found (/proc/xen/capabilities exists)"); - return VIRTUALIZATION_XEN; - + return VIRTUALIZATION_XEN; } static bool detect_vm_xen_dom0(void) { From e37fae1ea0c491e7c5ac1d364a80e2f846fbc312 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 24 Oct 2017 12:26:36 +0200 Subject: [PATCH 02/17] core: include a bad /var/run symlink in the "tainted" string --- src/core/dbus-manager.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index f87b52a266..c9da05335f 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -139,8 +139,10 @@ static int property_get_tainted( void *userdata, sd_bus_error *error) { - char buf[sizeof("split-usr:cgroups-missing:local-hwclock:")] = "", *e = buf; + char buf[sizeof("split-usr:cgroups-missing:local-hwclock:var-run-bad:")] = "", *e = buf; + _cleanup_free_ char *destination = NULL; Manager *m = userdata; + int r; assert(bus); assert(reply); @@ -155,6 +157,10 @@ static int property_get_tainted( if (clock_is_localtime(NULL) > 0) e = stpcpy(e, "local-hwclock:"); + r = readlink_malloc("/var/run", &destination); + if (r < 0 || !PATH_IN_SET(destination, "../run", "/run")) + e = stpcpy(e, "var-run-bad:"); + /* remove the last ':' */ if (e != buf) e[-1] = 0; From f33a319f9924196555084452f30c5ed4c0799f24 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 25 Oct 2017 20:40:04 +0200 Subject: [PATCH 03/17] mkosi: configure mkosi.cache/ and mkosi.builddir/ explicitly This way these dirs will be created automatically if they are missing, thus always guaranteeing optimal speedy behaviour. (Well at least, after https://github.com/systemd/mkosi/pull/181 is merged) --- .mkosi/mkosi.fedora | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.mkosi/mkosi.fedora b/.mkosi/mkosi.fedora index 4cfdcb0b36..f54cbe08f3 100644 --- a/.mkosi/mkosi.fedora +++ b/.mkosi/mkosi.fedora @@ -74,3 +74,6 @@ BuildPackages= Packages= libidn2 + +BuildDirectory=mkosi.builddir +Cache=mkosi.cache From 8da0592c5db5a1b9a8306433256ca80915a34043 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 25 Oct 2017 20:42:38 +0200 Subject: [PATCH 04/17] mkosi: fix build script to use right sysvinit path On Fedora /etc/init.d is a symlink to /etc/rc.d/init.d. Our build scripts default to /etc/init.d since that is the LSB default. Let's make sure the build script thus follows the symlink correctly and configures to path explicitly, since otherwise our build artifacts in $DESTDIR are incompatible with the setup we actually need for Fedora. --- mkosi.build | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mkosi.build b/mkosi.build index 456217fa5d..71251920ed 100755 --- a/mkosi.build +++ b/mkosi.build @@ -26,7 +26,9 @@ export LC_CTYPE=en_US.UTF-8 -[ -f "$BUILDDIR"/build.ninja ] || meson "$BUILDDIR" +sysvinit_path=`realpath /etc/init.d` + +[ -f "$BUILDDIR"/build.ninja ] || meson "$BUILDDIR" -D "sysvinit-path=$sysvinit_path" ninja -C "$BUILDDIR" all [ "$WITH_TESTS" = 0 ] || ninja -C "$BUILDDIR" test || ( RET="$?" ; cat "$BUILDDIR"/meson-logs/testlog.txt ; exit "$RET" ) ninja -C "$BUILDDIR" install From 8f50e86a868e73e05b7587e2d2bbf4d61dd433cf Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 25 Oct 2017 20:44:25 +0200 Subject: [PATCH 05/17] gpt-auto-generator: make sure "r" is always set --- src/gpt-auto-generator/gpt-auto-generator.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/gpt-auto-generator/gpt-auto-generator.c b/src/gpt-auto-generator/gpt-auto-generator.c index f9a0a14d9e..2e1ce334bf 100644 --- a/src/gpt-auto-generator/gpt-auto-generator.c +++ b/src/gpt-auto-generator/gpt-auto-generator.c @@ -688,7 +688,7 @@ static int add_mounts(void) { } int main(int argc, char *argv[]) { - int r = 0, k; + int r, k; if (argc > 1 && argc != 4) { log_error("This program takes three or no arguments."); @@ -720,6 +720,8 @@ int main(int argc, char *argv[]) { if (arg_root_enabled) r = add_root_mount(); + else + r = 0; if (!in_initrd()) { k = add_mounts(); From eef85c4a3f8054d29383a176f6cebd1ef3a15b9a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 25 Oct 2017 20:46:01 +0200 Subject: [PATCH 06/17] core: track why unit dependencies came to be This replaces the dependencies Set* objects by Hashmap* objects, where the key is the depending Unit, and the value is a bitmask encoding why the specific dependency was created. The bitmask contains a number of different, defined bits, that indicate why dependencies exist, for example whether they are created due to explicitly configured deps in files, by udev rules or implicitly. Note that memory usage is not increased by this change, even though we store more information, as we manage to encode the bit mask inside the value pointer each Hashmap entry contains. Why this all? When we know how a dependency came to be, we can update dependencies correctly when a configuration source changes but others are left unaltered. Specifically: 1. We can fix UDEV_WANTS dependency generation: so far we kept adding dependencies configured that way, but if a device lost such a dependency we couldn't them again as there was no scheme for removing of dependencies in place. 2. We can implement "pin-pointed" reload of unit files. If we know what dependencies were created as result of configuration in a unit file, then we know what to flush out when we want to reload it. 3. It's useful for debugging: "systemd-analyze dump" now shows this information, helping substantially with understanding how systemd's dependency tree came to be the way it came to be. --- src/core/automount.c | 29 ++- src/core/cgroup.c | 9 +- src/core/dbus-unit.c | 7 +- src/core/device.c | 17 +- src/core/job.c | 29 ++- src/core/load-dropin.c | 2 +- src/core/load-fragment.c | 12 +- src/core/manager.c | 10 +- src/core/mount.c | 71 +++--- src/core/path.c | 42 ++-- src/core/scope.c | 3 +- src/core/service.c | 17 +- src/core/slice.c | 2 +- src/core/socket.c | 29 +-- src/core/swap.c | 14 +- src/core/target.c | 19 +- src/core/timer.c | 40 ++-- src/core/transaction.c | 28 ++- src/core/unit.c | 496 +++++++++++++++++++++++++-------------- src/core/unit.h | 69 +++++- 20 files changed, 598 insertions(+), 347 deletions(-) diff --git a/src/core/automount.c b/src/core/automount.c index 9b0d1ca429..befa3131ee 100644 --- a/src/core/automount.c +++ b/src/core/automount.c @@ -130,7 +130,20 @@ static void automount_done(Unit *u) { a->expire_event_source = sd_event_source_unref(a->expire_event_source); } -static int automount_add_mount_links(Automount *a) { +static int automount_add_trigger_dependencies(Automount *a) { + Unit *x; + int r; + + assert(a); + + r = unit_load_related_unit(UNIT(a), ".mount", &x); + if (r < 0) + return r; + + return unit_add_two_dependencies(UNIT(a), UNIT_BEFORE, UNIT_TRIGGERS, x, true, UNIT_DEPENDENCY_IMPLICIT); +} + +static int automount_add_mount_dependencies(Automount *a) { _cleanup_free_ char *parent = NULL; assert(a); @@ -139,7 +152,7 @@ static int automount_add_mount_links(Automount *a) { if (!parent) return -ENOMEM; - return unit_require_mounts_for(UNIT(a), parent); + return unit_require_mounts_for(UNIT(a), parent, UNIT_DEPENDENCY_IMPLICIT); } static int automount_add_default_dependencies(Automount *a) { @@ -153,7 +166,7 @@ static int automount_add_default_dependencies(Automount *a) { if (!MANAGER_IS_SYSTEM(UNIT(a)->manager)) return 0; - r = unit_add_two_dependencies_by_name(UNIT(a), UNIT_BEFORE, UNIT_CONFLICTS, SPECIAL_UMOUNT_TARGET, NULL, true); + r = unit_add_two_dependencies_by_name(UNIT(a), UNIT_BEFORE, UNIT_CONFLICTS, SPECIAL_UMOUNT_TARGET, NULL, true, UNIT_DEPENDENCY_DEFAULT); if (r < 0) return r; @@ -215,21 +228,15 @@ static int automount_load(Unit *u) { return r; if (u->load_state == UNIT_LOADED) { - Unit *x; - r = automount_set_where(a); if (r < 0) return r; - r = unit_load_related_unit(u, ".mount", &x); + r = automount_add_trigger_dependencies(a); if (r < 0) return r; - r = unit_add_two_dependencies(u, UNIT_BEFORE, UNIT_TRIGGERS, x, true); - if (r < 0) - return r; - - r = automount_add_mount_links(a); + r = automount_add_mount_dependencies(a); if (r < 0) return r; diff --git a/src/core/cgroup.c b/src/core/cgroup.c index e9f9447118..dff7d1dddc 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -1095,10 +1095,11 @@ CGroupMask unit_get_members_mask(Unit *u) { u->cgroup_members_mask = 0; if (u->type == UNIT_SLICE) { + void *v; Unit *member; Iterator i; - SET_FOREACH(member, u->dependencies[UNIT_BEFORE], i) { + HASHMAP_FOREACH_KEY(v, member, u->dependencies[UNIT_BEFORE], i) { if (member == u) continue; @@ -1575,8 +1576,9 @@ static void unit_add_siblings_to_cgroup_realize_queue(Unit *u) { while ((slice = UNIT_DEREF(u->slice))) { Iterator i; Unit *m; + void *v; - SET_FOREACH(m, slice->dependencies[UNIT_BEFORE], i) { + HASHMAP_FOREACH_KEY(v, m, u->dependencies[UNIT_BEFORE], i) { if (m == u) continue; @@ -2426,8 +2428,9 @@ void unit_invalidate_cgroup_bpf(Unit *u) { if (u->type == UNIT_SLICE) { Unit *member; Iterator i; + void *v; - SET_FOREACH(member, u->dependencies[UNIT_BEFORE], i) { + HASHMAP_FOREACH_KEY(v, member, u->dependencies[UNIT_BEFORE], i) { if (member == u) continue; diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index a9a68826b4..8fc02ab744 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -100,9 +100,10 @@ static int property_get_dependencies( void *userdata, sd_bus_error *error) { - Set *s = *(Set**) userdata; + Hashmap *h = *(Hashmap**) userdata; Iterator j; Unit *u; + void *v; int r; assert(bus); @@ -112,7 +113,7 @@ static int property_get_dependencies( if (r < 0) return r; - SET_FOREACH(u, s, j) { + HASHMAP_FOREACH_KEY(v, u, h, j) { r = sd_bus_message_append(reply, "s", u->id); if (r < 0) return r; @@ -1395,7 +1396,7 @@ static int bus_unit_set_transient_property( if (mode != UNIT_CHECK) { _cleanup_free_ char *label = NULL; - r = unit_add_dependency_by_name(u, d, other, NULL, true); + r = unit_add_dependency_by_name(u, d, other, NULL, true, UNIT_DEPENDENCY_FILE); if (r < 0) return r; diff --git a/src/core/device.c b/src/core/device.c index 3915b2637d..63933baf85 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -279,7 +279,7 @@ static int device_add_udev_wants(Unit *u, struct udev_device *dev) { if (r < 0) return log_unit_error_errno(u, r, "Failed to mangle unit name \"%s\": %m", word); - r = unit_add_dependency_by_name(u, UNIT_WANTS, k, NULL, true); + r = unit_add_dependency_by_name(u, UNIT_WANTS, k, NULL, true, UNIT_DEPENDENCY_UDEV); if (r < 0) return log_unit_error_errno(u, r, "Failed to add wants dependency: %m"); } @@ -303,13 +303,16 @@ static bool device_is_bound_by_mounts(Unit *d, struct udev_device *dev) { static int device_upgrade_mount_deps(Unit *u) { Unit *other; Iterator i; + void *v; int r; - SET_FOREACH(other, u->dependencies[UNIT_REQUIRED_BY], i) { + /* Let's upgrade Requires= to BindsTo= on us. (Used when SYSTEMD_MOUNT_DEVICE_BOUND is set) */ + + HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_REQUIRED_BY], i) { if (other->type != UNIT_MOUNT) continue; - r = unit_add_dependency(other, UNIT_BINDS_TO, u, true); + r = unit_add_dependency(other, UNIT_BINDS_TO, u, true, UNIT_DEPENDENCY_UDEV); if (r < 0) return r; } @@ -380,11 +383,9 @@ static int device_setup_unit(Manager *m, struct udev_device *dev, const char *pa (void) device_add_udev_wants(u, dev); } - /* So the user wants the mount units to be bound to the device but a - * mount unit might has been seen by systemd before the device appears - * on its radar. In this case the device unit is partially initialized - * and includes the deps on the mount unit but at that time the "bind - * mounts" flag wasn't not present. Fix this up now. */ + /* So the user wants the mount units to be bound to the device but a mount unit might has been seen by systemd + * before the device appears on its radar. In this case the device unit is partially initialized and includes + * the deps on the mount unit but at that time the "bind mounts" flag wasn't not present. Fix this up now. */ if (dev && device_is_bound_by_mounts(u, dev)) device_upgrade_mount_deps(u); diff --git a/src/core/job.c b/src/core/job.c index d441453839..606b6422ab 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -437,6 +437,7 @@ int job_type_merge_and_collapse(JobType *a, JobType b, Unit *u) { static bool job_is_runnable(Job *j) { Iterator i; Unit *other; + void *v; assert(j); assert(j->installed); @@ -459,13 +460,12 @@ static bool job_is_runnable(Job *j) { return true; if (IN_SET(j->type, JOB_START, JOB_VERIFY_ACTIVE, JOB_RELOAD)) { - /* Immediate result is that the job is or might be * started. In this case let's wait for the * dependencies, regardless whether they are * starting or stopping something. */ - SET_FOREACH(other, j->unit->dependencies[UNIT_AFTER], i) + HASHMAP_FOREACH_KEY(v, other, j->unit->dependencies[UNIT_AFTER], i) if (other->job) return false; } @@ -473,7 +473,7 @@ static bool job_is_runnable(Job *j) { /* Also, if something else is being stopped and we should * change state after it, then let's wait. */ - SET_FOREACH(other, j->unit->dependencies[UNIT_BEFORE], i) + HASHMAP_FOREACH_KEY(v, other, j->unit->dependencies[UNIT_BEFORE], i) if (other->job && IN_SET(other->job->type, JOB_STOP, JOB_RESTART)) return false; @@ -832,10 +832,11 @@ static void job_emit_status_message(Unit *u, JobType t, JobResult result) { static void job_fail_dependencies(Unit *u, UnitDependency d) { Unit *other; Iterator i; + void *v; assert(u); - SET_FOREACH(other, u->dependencies[d], i) { + HASHMAP_FOREACH_KEY(v, other, u->dependencies[d], i) { Job *j = other->job; if (!j) @@ -852,6 +853,7 @@ int job_finish_and_invalidate(Job *j, JobResult result, bool recursive, bool alr Unit *other; JobType t; Iterator i; + void *v; assert(j); assert(j->installed); @@ -919,12 +921,12 @@ int job_finish_and_invalidate(Job *j, JobResult result, bool recursive, bool alr finish: /* Try to start the next jobs that can be started */ - SET_FOREACH(other, u->dependencies[UNIT_AFTER], i) + HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_AFTER], i) if (other->job) { job_add_to_run_queue(other->job); job_add_to_gc_queue(other->job); } - SET_FOREACH(other, u->dependencies[UNIT_BEFORE], i) + HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_BEFORE], i) if (other->job) { job_add_to_run_queue(other->job); job_add_to_gc_queue(other->job); @@ -1273,6 +1275,7 @@ int job_get_timeout(Job *j, usec_t *timeout) { bool job_check_gc(Job *j) { Unit *other; Iterator i; + void *v; assert(j); @@ -1301,7 +1304,7 @@ bool job_check_gc(Job *j) { /* If a job is ordered after ours, and is to be started, then it needs to wait for us, regardless if we stop or * start, hence let's not GC in that case. */ - SET_FOREACH(other, j->unit->dependencies[UNIT_BEFORE], i) { + HASHMAP_FOREACH_KEY(v, other, j->unit->dependencies[UNIT_BEFORE], i) { if (!other->job) continue; @@ -1314,7 +1317,7 @@ bool job_check_gc(Job *j) { /* If we are going down, but something else is ordered After= us, then it needs to wait for us */ if (IN_SET(j->type, JOB_STOP, JOB_RESTART)) - SET_FOREACH(other, j->unit->dependencies[UNIT_AFTER], i) { + HASHMAP_FOREACH_KEY(v, other, j->unit->dependencies[UNIT_AFTER], i) { if (!other->job) continue; @@ -1392,6 +1395,7 @@ int job_get_before(Job *j, Job*** ret) { size_t n = 0, n_allocated = 0; Unit *other = NULL; Iterator i; + void *v; /* Returns a list of all pending jobs that need to finish before this job may be started. */ @@ -1405,7 +1409,7 @@ int job_get_before(Job *j, Job*** ret) { if (IN_SET(j->type, JOB_START, JOB_VERIFY_ACTIVE, JOB_RELOAD)) { - SET_FOREACH(other, j->unit->dependencies[UNIT_AFTER], i) { + HASHMAP_FOREACH_KEY(v, other, j->unit->dependencies[UNIT_AFTER], i) { if (!other->job) continue; @@ -1415,7 +1419,7 @@ int job_get_before(Job *j, Job*** ret) { } } - SET_FOREACH(other, j->unit->dependencies[UNIT_BEFORE], i) { + HASHMAP_FOREACH_KEY(v, other, j->unit->dependencies[UNIT_BEFORE], i) { if (!other->job) continue; @@ -1439,6 +1443,7 @@ int job_get_after(Job *j, Job*** ret) { _cleanup_free_ Job** list = NULL; size_t n = 0, n_allocated = 0; Unit *other = NULL; + void *v; Iterator i; assert(j); @@ -1446,7 +1451,7 @@ int job_get_after(Job *j, Job*** ret) { /* Returns a list of all pending jobs that are waiting for this job to finish. */ - SET_FOREACH(other, j->unit->dependencies[UNIT_BEFORE], i) { + HASHMAP_FOREACH_KEY(v, other, j->unit->dependencies[UNIT_BEFORE], i) { if (!other->job) continue; @@ -1463,7 +1468,7 @@ int job_get_after(Job *j, Job*** ret) { if (IN_SET(j->type, JOB_STOP, JOB_RESTART)) { - SET_FOREACH(other, j->unit->dependencies[UNIT_AFTER], i) { + HASHMAP_FOREACH_KEY(v, other, j->unit->dependencies[UNIT_AFTER], i) { if (!other->job) continue; diff --git a/src/core/load-dropin.c b/src/core/load-dropin.c index 00f09ce60a..948d1bc248 100644 --- a/src/core/load-dropin.c +++ b/src/core/load-dropin.c @@ -115,7 +115,7 @@ static int process_deps(Unit *u, UnitDependency dependency, const char *dir_suff 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); + r = unit_add_dependency_by_name(u, dependency, entry, *p, true, UNIT_DEPENDENCY_FILE); if (r < 0) log_unit_error_errno(u, r, "cannot add %s dependency on %s, ignoring: %m", unit_dependency_to_string(dependency), entry); diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index a4e39a84ea..125fef6d49 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -142,7 +142,7 @@ int config_parse_unit_deps( continue; } - r = unit_add_dependency_by_name(u, d, k, NULL, true); + r = unit_add_dependency_by_name(u, d, k, NULL, true, UNIT_DEPENDENCY_FILE); if (r < 0) log_syntax(unit, LOG_ERR, filename, line, r, "Failed to add dependency on %s, ignoring: %m", k); } @@ -1538,7 +1538,7 @@ int config_parse_trigger_unit( assert(rvalue); assert(data); - if (!set_isempty(u->dependencies[UNIT_TRIGGERS])) { + if (!hashmap_isempty(u->dependencies[UNIT_TRIGGERS])) { log_syntax(unit, LOG_ERR, filename, line, 0, "Multiple units to trigger specified, ignoring: %s", rvalue); return 0; } @@ -1560,7 +1560,7 @@ int config_parse_trigger_unit( return 0; } - r = unit_add_two_dependencies_by_name(u, UNIT_BEFORE, UNIT_TRIGGERS, p, NULL, true); + r = unit_add_two_dependencies_by_name(u, UNIT_BEFORE, UNIT_TRIGGERS, p, NULL, true, UNIT_DEPENDENCY_FILE); if (r < 0) { log_syntax(unit, LOG_ERR, filename, line, r, "Failed to add trigger on %s, ignoring: %m", p); return 0; @@ -1760,11 +1760,11 @@ int config_parse_service_sockets( continue; } - r = unit_add_two_dependencies_by_name(UNIT(s), UNIT_WANTS, UNIT_AFTER, k, NULL, true); + r = unit_add_two_dependencies_by_name(UNIT(s), UNIT_WANTS, UNIT_AFTER, k, NULL, true, UNIT_DEPENDENCY_FILE); if (r < 0) log_syntax(unit, LOG_ERR, filename, line, r, "Failed to add dependency on %s, ignoring: %m", k); - r = unit_add_dependency_by_name(UNIT(s), UNIT_TRIGGERED_BY, k, NULL, true); + r = unit_add_dependency_by_name(UNIT(s), UNIT_TRIGGERED_BY, k, NULL, true, UNIT_DEPENDENCY_FILE); if (r < 0) log_syntax(unit, LOG_ERR, filename, line, r, "Failed to add dependency on %s, ignoring: %m", k); } @@ -2569,7 +2569,7 @@ int config_parse_unit_requires_mounts_for( continue; } - r = unit_require_mounts_for(u, resolved); + r = unit_require_mounts_for(u, resolved, UNIT_DEPENDENCY_FILE); if (r < 0) { log_syntax(unit, LOG_ERR, filename, line, r, "Failed to add required mount \"%s\", ignoring: %m", resolved); continue; diff --git a/src/core/manager.c b/src/core/manager.c index d501182f22..c89dadc108 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -967,21 +967,23 @@ enum { }; static void unit_gc_mark_good(Unit *u, unsigned gc_marker) { - Iterator i; Unit *other; + Iterator i; + void *v; u->gc_marker = gc_marker + GC_OFFSET_GOOD; /* Recursively mark referenced units as GOOD as well */ - SET_FOREACH(other, u->dependencies[UNIT_REFERENCES], i) + HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_REFERENCES], i) if (other->gc_marker == gc_marker + GC_OFFSET_UNSURE) unit_gc_mark_good(other, gc_marker); } static void unit_gc_sweep(Unit *u, unsigned gc_marker) { - Iterator i; Unit *other; bool is_bad; + Iterator i; + void *v; assert(u); @@ -999,7 +1001,7 @@ static void unit_gc_sweep(Unit *u, unsigned gc_marker) { is_bad = true; - SET_FOREACH(other, u->dependencies[UNIT_REFERENCED_BY], i) { + HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_REFERENCED_BY], i) { unit_gc_sweep(other, gc_marker); if (other->gc_marker == gc_marker + GC_OFFSET_GOOD) diff --git a/src/core/mount.c b/src/core/mount.c index 9b582072c6..343e17fa10 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -280,8 +280,7 @@ _pure_ static MountParameters* get_mount_parameters(Mount *m) { return get_mount_parameters_fragment(m); } -static int mount_add_mount_links(Mount *m) { - _cleanup_free_ char *parent = NULL; +static int mount_add_mount_dependencies(Mount *m) { const char *fstype; MountParameters *pm; Unit *other; @@ -292,33 +291,32 @@ static int mount_add_mount_links(Mount *m) { assert(m); if (!path_equal(m->where, "/")) { - /* Adds in links to other mount points that might lie further - * up in the hierarchy */ + _cleanup_free_ char *parent = NULL; + + /* Adds in links to other mount points that might lie further up in the hierarchy */ parent = dirname_malloc(m->where); if (!parent) return -ENOMEM; - r = unit_require_mounts_for(UNIT(m), parent); + r = unit_require_mounts_for(UNIT(m), parent, UNIT_DEPENDENCY_IMPLICIT); if (r < 0) return r; } - /* Adds in links to other mount points that might be needed - * for the source path (if this is a bind mount or a loop mount) to be - * available. */ + /* Adds in dependencies to other mount points that might be needed for the source path (if this is a bind mount + * or a loop mount) to be available. */ pm = get_mount_parameters_fragment(m); if (pm && pm->what && path_is_absolute(pm->what) && (mount_is_bind(pm) || mount_is_loop(pm) || !mount_is_network(pm))) { - r = unit_require_mounts_for(UNIT(m), pm->what); + r = unit_require_mounts_for(UNIT(m), pm->what, UNIT_DEPENDENCY_FILE); if (r < 0) return r; } - /* Adds in links to other units that use this path or paths - * further down in the hierarchy */ + /* Adds in dependencies to other units that use this path or paths further down in the hierarchy */ s = manager_get_units_requiring_mounts_for(UNIT(m)->manager, m->where); SET_FOREACH(other, s, i) { @@ -328,13 +326,13 @@ static int mount_add_mount_links(Mount *m) { if (other == UNIT(m)) continue; - r = unit_add_dependency(other, UNIT_AFTER, UNIT(m), true); + r = unit_add_dependency(other, UNIT_AFTER, UNIT(m), true, UNIT_DEPENDENCY_PATH); if (r < 0) return r; if (UNIT(m)->fragment_path) { /* If we have fragment configuration, then make this dependency required */ - r = unit_add_dependency(other, UNIT_REQUIRES, UNIT(m), true); + r = unit_add_dependency(other, UNIT_REQUIRES, UNIT(m), true, UNIT_DEPENDENCY_PATH); if (r < 0) return r; } @@ -343,7 +341,7 @@ static int mount_add_mount_links(Mount *m) { /* If this is a tmpfs mount then we have to unmount it before we try to deactivate swaps */ fstype = mount_get_fstype(m); if (streq(fstype, "tmpfs")) { - r = unit_add_dependency_by_name(UNIT(m), UNIT_AFTER, SPECIAL_SWAP_TARGET, NULL, true); + r = unit_add_dependency_by_name(UNIT(m), UNIT_AFTER, SPECIAL_SWAP_TARGET, NULL, true, UNIT_DEPENDENCY_IMPLICIT); if (r < 0) return r; } @@ -351,9 +349,10 @@ static int mount_add_mount_links(Mount *m) { return 0; } -static int mount_add_device_links(Mount *m) { - MountParameters *p; +static int mount_add_device_dependencies(Mount *m) { bool device_wants_mount = false; + UnitDependencyMask mask; + MountParameters *p; UnitDependency dep; int r; @@ -391,16 +390,19 @@ static int mount_add_device_links(Mount *m) { * automatically stopped when the device disappears suddenly. */ dep = mount_is_bound_to_device(m) ? UNIT_BINDS_TO : UNIT_REQUIRES; - r = unit_add_node_link(UNIT(m), p->what, device_wants_mount, dep); + mask = m->from_fragment ? UNIT_DEPENDENCY_FILE : UNIT_DEPENDENCY_MOUNTINFO_IMPLICIT; + + r = unit_add_node_dependency(UNIT(m), p->what, device_wants_mount, dep, mask); if (r < 0) return r; return 0; } -static int mount_add_quota_links(Mount *m) { - int r; +static int mount_add_quota_dependencies(Mount *m) { + UnitDependencyMask mask; MountParameters *p; + int r; assert(m); @@ -414,11 +416,13 @@ static int mount_add_quota_links(Mount *m) { if (!needs_quota(p)) return 0; - r = unit_add_two_dependencies_by_name(UNIT(m), UNIT_BEFORE, UNIT_WANTS, SPECIAL_QUOTACHECK_SERVICE, NULL, true); + mask = m->from_fragment ? UNIT_DEPENDENCY_FILE : UNIT_DEPENDENCY_MOUNTINFO_IMPLICIT; + + r = unit_add_two_dependencies_by_name(UNIT(m), UNIT_BEFORE, UNIT_WANTS, SPECIAL_QUOTACHECK_SERVICE, NULL, true, mask); if (r < 0) return r; - r = unit_add_two_dependencies_by_name(UNIT(m), UNIT_BEFORE, UNIT_WANTS, SPECIAL_QUOTAON_SERVICE, NULL, true); + r = unit_add_two_dependencies_by_name(UNIT(m), UNIT_BEFORE, UNIT_WANTS, SPECIAL_QUOTAON_SERVICE, NULL, true, mask); if (r < 0) return r; @@ -457,9 +461,10 @@ static bool mount_is_extrinsic(Mount *m) { } static int mount_add_default_dependencies(Mount *m) { + UnitDependencyMask mask; + int r; MountParameters *p; const char *after; - int r; assert(m); @@ -476,6 +481,8 @@ static int mount_add_default_dependencies(Mount *m) { if (!p) return 0; + mask = m->from_fragment ? UNIT_DEPENDENCY_FILE : UNIT_DEPENDENCY_MOUNTINFO_DEFAULT; + if (mount_is_network(p)) { /* We order ourselves after network.target. This is * primarily useful at shutdown: services that take @@ -483,7 +490,7 @@ static int mount_add_default_dependencies(Mount *m) { * network.target, so that they are shut down only * after this mount unit is stopped. */ - r = unit_add_dependency_by_name(UNIT(m), UNIT_AFTER, SPECIAL_NETWORK_TARGET, NULL, true); + r = unit_add_dependency_by_name(UNIT(m), UNIT_AFTER, SPECIAL_NETWORK_TARGET, NULL, true, mask); if (r < 0) return r; @@ -494,7 +501,7 @@ static int mount_add_default_dependencies(Mount *m) { * whose purpose it is to delay this until the network * is "up". */ - r = unit_add_two_dependencies_by_name(UNIT(m), UNIT_WANTS, UNIT_AFTER, SPECIAL_NETWORK_ONLINE_TARGET, NULL, true); + r = unit_add_two_dependencies_by_name(UNIT(m), UNIT_WANTS, UNIT_AFTER, SPECIAL_NETWORK_ONLINE_TARGET, NULL, true, mask); if (r < 0) return r; @@ -502,11 +509,11 @@ static int mount_add_default_dependencies(Mount *m) { } else after = SPECIAL_LOCAL_FS_PRE_TARGET; - r = unit_add_dependency_by_name(UNIT(m), UNIT_AFTER, after, NULL, true); + r = unit_add_dependency_by_name(UNIT(m), UNIT_AFTER, after, NULL, true, mask); if (r < 0) return r; - r = unit_add_two_dependencies_by_name(UNIT(m), UNIT_BEFORE, UNIT_CONFLICTS, SPECIAL_UMOUNT_TARGET, NULL, true); + r = unit_add_two_dependencies_by_name(UNIT(m), UNIT_BEFORE, UNIT_CONFLICTS, SPECIAL_UMOUNT_TARGET, NULL, true, mask); if (r < 0) return r; @@ -577,15 +584,15 @@ static int mount_add_extras(Mount *m) { return r; } - r = mount_add_device_links(m); + r = mount_add_device_dependencies(m); if (r < 0) return r; - r = mount_add_mount_links(m); + r = mount_add_mount_dependencies(m); if (r < 0) return r; - r = mount_add_quota_links(m); + r = mount_add_quota_dependencies(m); if (r < 0) return r; @@ -1453,11 +1460,11 @@ static int mount_setup_new_unit( int r; target = mount_is_network(p) ? SPECIAL_REMOTE_FS_TARGET : SPECIAL_LOCAL_FS_TARGET; - r = unit_add_dependency_by_name(u, UNIT_BEFORE, target, NULL, true); + r = unit_add_dependency_by_name(u, UNIT_BEFORE, target, NULL, true, UNIT_DEPENDENCY_MOUNTINFO_IMPLICIT); if (r < 0) return r; - r = unit_add_dependency_by_name(u, UNIT_CONFLICTS, SPECIAL_UMOUNT_TARGET, NULL, true); + r = unit_add_dependency_by_name(u, UNIT_CONFLICTS, SPECIAL_UMOUNT_TARGET, NULL, true, UNIT_DEPENDENCY_MOUNTINFO_IMPLICIT); if (r < 0) return r; } @@ -1515,7 +1522,7 @@ static int mount_setup_existing_unit( * in the dependency "Set*" objects who created a * dependency), we can only add deps, never lose them, * until the next full daemon-reload. */ - unit_add_dependency_by_name(u, UNIT_BEFORE, SPECIAL_REMOTE_FS_TARGET, NULL, true); + unit_add_dependency_by_name(u, UNIT_BEFORE, SPECIAL_REMOTE_FS_TARGET, NULL, true, UNIT_DEPENDENCY_MOUNTINFO_IMPLICIT); load_extras = true; } diff --git a/src/core/path.c b/src/core/path.c index 44831f5803..6b0768505a 100644 --- a/src/core/path.c +++ b/src/core/path.c @@ -277,14 +277,14 @@ static void path_done(Unit *u) { path_free_specs(p); } -static int path_add_mount_links(Path *p) { +static int path_add_mount_dependencies(Path *p) { PathSpec *s; int r; assert(p); LIST_FOREACH(spec, s, p->specs) { - r = unit_require_mounts_for(UNIT(p), s->path); + r = unit_require_mounts_for(UNIT(p), s->path, UNIT_DEPENDENCY_FILE); if (r < 0) return r; } @@ -314,17 +314,33 @@ static int path_add_default_dependencies(Path *p) { if (!UNIT(p)->default_dependencies) return 0; - r = unit_add_dependency_by_name(UNIT(p), UNIT_BEFORE, SPECIAL_PATHS_TARGET, NULL, true); + r = unit_add_dependency_by_name(UNIT(p), UNIT_BEFORE, SPECIAL_PATHS_TARGET, NULL, true, UNIT_DEPENDENCY_DEFAULT); if (r < 0) return r; if (MANAGER_IS_SYSTEM(UNIT(p)->manager)) { - r = unit_add_two_dependencies_by_name(UNIT(p), UNIT_AFTER, UNIT_REQUIRES, SPECIAL_SYSINIT_TARGET, NULL, true); + r = unit_add_two_dependencies_by_name(UNIT(p), UNIT_AFTER, UNIT_REQUIRES, SPECIAL_SYSINIT_TARGET, NULL, true, UNIT_DEPENDENCY_DEFAULT); if (r < 0) return r; } - return unit_add_two_dependencies_by_name(UNIT(p), UNIT_BEFORE, UNIT_CONFLICTS, SPECIAL_SHUTDOWN_TARGET, NULL, true); + return unit_add_two_dependencies_by_name(UNIT(p), UNIT_BEFORE, UNIT_CONFLICTS, SPECIAL_SHUTDOWN_TARGET, NULL, true, UNIT_DEPENDENCY_DEFAULT); +} + +static int path_add_trigger_dependencies(Path *p) { + Unit *x; + int r; + + assert(p); + + if (!hashmap_isempty(UNIT(p)->dependencies[UNIT_TRIGGERS])) + return 0; + + r = unit_load_related_unit(UNIT(p), ".service", &x); + if (r < 0) + return r; + + return unit_add_two_dependencies(UNIT(p), UNIT_BEFORE, UNIT_TRIGGERS, x, true, UNIT_DEPENDENCY_IMPLICIT); } static int path_load(Unit *u) { @@ -340,19 +356,11 @@ static int path_load(Unit *u) { if (u->load_state == UNIT_LOADED) { - if (set_isempty(u->dependencies[UNIT_TRIGGERS])) { - Unit *x; + r = path_add_trigger_dependencies(p); + if (r < 0) + return r; - r = unit_load_related_unit(u, ".service", &x); - if (r < 0) - return r; - - r = unit_add_two_dependencies(u, UNIT_BEFORE, UNIT_TRIGGERS, x, true); - if (r < 0) - return r; - } - - r = path_add_mount_links(p); + r = path_add_mount_dependencies(p); if (r < 0) return r; diff --git a/src/core/scope.c b/src/core/scope.c index 7e789ed147..073b216ea2 100644 --- a/src/core/scope.c +++ b/src/core/scope.c @@ -124,7 +124,8 @@ static int scope_add_default_dependencies(Scope *s) { r = unit_add_two_dependencies_by_name( UNIT(s), UNIT_BEFORE, UNIT_CONFLICTS, - SPECIAL_SHUTDOWN_TARGET, NULL, true); + SPECIAL_SHUTDOWN_TARGET, NULL, true, + UNIT_DEPENDENCY_DEFAULT); if (r < 0) return r; diff --git a/src/core/service.c b/src/core/service.c index 78f315ad18..f42f1effb7 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -562,7 +562,7 @@ static int service_add_default_dependencies(Service *s) { * require it, so that we fail if we can't acquire * it. */ - r = unit_add_two_dependencies_by_name(UNIT(s), UNIT_AFTER, UNIT_REQUIRES, SPECIAL_SYSINIT_TARGET, NULL, true); + r = unit_add_two_dependencies_by_name(UNIT(s), UNIT_AFTER, UNIT_REQUIRES, SPECIAL_SYSINIT_TARGET, NULL, true, UNIT_DEPENDENCY_DEFAULT); if (r < 0) return r; } else { @@ -570,7 +570,7 @@ static int service_add_default_dependencies(Service *s) { /* In the --user instance there's no sysinit.target, * in that case require basic.target instead. */ - r = unit_add_dependency_by_name(UNIT(s), UNIT_REQUIRES, SPECIAL_BASIC_TARGET, NULL, true); + r = unit_add_dependency_by_name(UNIT(s), UNIT_REQUIRES, SPECIAL_BASIC_TARGET, NULL, true, UNIT_DEPENDENCY_DEFAULT); if (r < 0) return r; } @@ -578,12 +578,12 @@ static int service_add_default_dependencies(Service *s) { /* Second, if the rest of the base system is in the same * transaction, order us after it, but do not pull it in or * even require it. */ - r = unit_add_dependency_by_name(UNIT(s), UNIT_AFTER, SPECIAL_BASIC_TARGET, NULL, true); + r = unit_add_dependency_by_name(UNIT(s), UNIT_AFTER, SPECIAL_BASIC_TARGET, NULL, true, UNIT_DEPENDENCY_DEFAULT); if (r < 0) return r; /* Third, add us in for normal shutdown. */ - return unit_add_two_dependencies_by_name(UNIT(s), UNIT_BEFORE, UNIT_CONFLICTS, SPECIAL_SHUTDOWN_TARGET, NULL, true); + return unit_add_two_dependencies_by_name(UNIT(s), UNIT_BEFORE, UNIT_CONFLICTS, SPECIAL_SHUTDOWN_TARGET, NULL, true, UNIT_DEPENDENCY_DEFAULT); } static void service_fix_output(Service *s) { @@ -612,12 +612,12 @@ static int service_setup_bus_name(Service *s) { if (!s->bus_name) return 0; - r = unit_add_dependency_by_name(UNIT(s), UNIT_REQUIRES, SPECIAL_DBUS_SOCKET, NULL, true); + r = unit_add_dependency_by_name(UNIT(s), UNIT_REQUIRES, SPECIAL_DBUS_SOCKET, NULL, true, UNIT_DEPENDENCY_FILE); if (r < 0) return log_unit_error_errno(UNIT(s), r, "Failed to add dependency on " SPECIAL_DBUS_SOCKET ": %m"); /* We always want to be ordered against dbus.socket if both are in the transaction. */ - r = unit_add_dependency_by_name(UNIT(s), UNIT_AFTER, SPECIAL_DBUS_SOCKET, NULL, true); + r = unit_add_dependency_by_name(UNIT(s), UNIT_AFTER, SPECIAL_DBUS_SOCKET, NULL, true, UNIT_DEPENDENCY_FILE); if (r < 0) return log_unit_error_errno(UNIT(s), r, "Failed to add dependency on " SPECIAL_DBUS_SOCKET ": %m"); @@ -1103,11 +1103,12 @@ static int service_collect_fds(Service *s, rn_socket_fds = 1; } else { Iterator i; + void *v; Unit *u; /* Pass all our configured sockets for singleton services */ - SET_FOREACH(u, UNIT(s)->dependencies[UNIT_TRIGGERED_BY], i) { + HASHMAP_FOREACH_KEY(v, u, UNIT(s)->dependencies[UNIT_TRIGGERED_BY], i) { _cleanup_free_ int *cfds = NULL; Socket *sock; int cn_fds; @@ -3617,7 +3618,7 @@ int service_set_socket_fd(Service *s, int fd, Socket *sock, bool selinux_context return r; } - r = unit_add_two_dependencies(UNIT(sock), UNIT_BEFORE, UNIT_TRIGGERS, UNIT(s), false); + r = unit_add_two_dependencies(UNIT(sock), UNIT_BEFORE, UNIT_TRIGGERS, UNIT(s), false, UNIT_DEPENDENCY_IMPLICIT); if (r < 0) return r; diff --git a/src/core/slice.c b/src/core/slice.c index b15f751c82..e4faa1cdbd 100644 --- a/src/core/slice.c +++ b/src/core/slice.c @@ -97,7 +97,7 @@ static int slice_add_default_dependencies(Slice *s) { r = unit_add_two_dependencies_by_name( UNIT(s), UNIT_BEFORE, UNIT_CONFLICTS, - SPECIAL_SHUTDOWN_TARGET, NULL, true); + SPECIAL_SHUTDOWN_TARGET, NULL, true, UNIT_DEPENDENCY_DEFAULT); if (r < 0) return r; diff --git a/src/core/socket.c b/src/core/socket.c index c0f4030302..0686de1d1b 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -250,7 +250,7 @@ int socket_instantiate_service(Socket *s) { unit_ref_set(&s->service, u); - return unit_add_two_dependencies(UNIT(s), UNIT_BEFORE, UNIT_TRIGGERS, u, false); + return unit_add_two_dependencies(UNIT(s), UNIT_BEFORE, UNIT_TRIGGERS, u, false, UNIT_DEPENDENCY_IMPLICIT); } static bool have_non_accept_socket(Socket *s) { @@ -273,7 +273,7 @@ static bool have_non_accept_socket(Socket *s) { return false; } -static int socket_add_mount_links(Socket *s) { +static int socket_add_mount_dependencies(Socket *s) { SocketPort *p; int r; @@ -290,7 +290,7 @@ static int socket_add_mount_links(Socket *s) { if (!path) continue; - r = unit_require_mounts_for(UNIT(s), path); + r = unit_require_mounts_for(UNIT(s), path, UNIT_DEPENDENCY_FILE); if (r < 0) return r; } @@ -298,7 +298,7 @@ static int socket_add_mount_links(Socket *s) { return 0; } -static int socket_add_device_link(Socket *s) { +static int socket_add_device_dependencies(Socket *s) { char *t; assert(s); @@ -307,7 +307,7 @@ static int socket_add_device_link(Socket *s) { return 0; t = strjoina("/sys/subsystem/net/devices/", s->bind_to_device); - return unit_add_node_link(UNIT(s), t, false, UNIT_BINDS_TO); + return unit_add_node_dependency(UNIT(s), t, false, UNIT_BINDS_TO, UNIT_DEPENDENCY_FILE); } static int socket_add_default_dependencies(Socket *s) { @@ -317,17 +317,17 @@ static int socket_add_default_dependencies(Socket *s) { if (!UNIT(s)->default_dependencies) return 0; - r = unit_add_dependency_by_name(UNIT(s), UNIT_BEFORE, SPECIAL_SOCKETS_TARGET, NULL, true); + r = unit_add_dependency_by_name(UNIT(s), UNIT_BEFORE, SPECIAL_SOCKETS_TARGET, NULL, true, UNIT_DEPENDENCY_DEFAULT); if (r < 0) return r; if (MANAGER_IS_SYSTEM(UNIT(s)->manager)) { - r = unit_add_two_dependencies_by_name(UNIT(s), UNIT_AFTER, UNIT_REQUIRES, SPECIAL_SYSINIT_TARGET, NULL, true); + r = unit_add_two_dependencies_by_name(UNIT(s), UNIT_AFTER, UNIT_REQUIRES, SPECIAL_SYSINIT_TARGET, NULL, true, UNIT_DEPENDENCY_DEFAULT); if (r < 0) return r; } - return unit_add_two_dependencies_by_name(UNIT(s), UNIT_BEFORE, UNIT_CONFLICTS, SPECIAL_SHUTDOWN_TARGET, NULL, true); + return unit_add_two_dependencies_by_name(UNIT(s), UNIT_BEFORE, UNIT_CONFLICTS, SPECIAL_SHUTDOWN_TARGET, NULL, true, UNIT_DEPENDENCY_DEFAULT); } _pure_ static bool socket_has_exec(Socket *s) { @@ -378,16 +378,16 @@ static int socket_add_extras(Socket *s) { unit_ref_set(&s->service, x); } - r = unit_add_two_dependencies(u, UNIT_BEFORE, UNIT_TRIGGERS, UNIT_DEREF(s->service), true); + r = unit_add_two_dependencies(u, UNIT_BEFORE, UNIT_TRIGGERS, UNIT_DEREF(s->service), true, UNIT_DEPENDENCY_IMPLICIT); if (r < 0) return r; } - r = socket_add_mount_links(s); + r = socket_add_mount_dependencies(s); if (r < 0) return r; - r = socket_add_device_link(s); + r = socket_add_device_dependencies(s); if (r < 0) return r; @@ -2261,13 +2261,14 @@ static void socket_enter_running(Socket *s, int cfd) { } if (cfd < 0) { - Iterator i; - Unit *other; bool pending = false; + Unit *other; + Iterator i; + void *v; /* If there's already a start pending don't bother to * do anything */ - SET_FOREACH(other, UNIT(s)->dependencies[UNIT_TRIGGERS], i) + HASHMAP_FOREACH_KEY(v, other, UNIT(s)->dependencies[UNIT_TRIGGERS], i) if (unit_active_or_pending(other)) { pending = true; break; diff --git a/src/core/swap.c b/src/core/swap.c index f475755572..42d91ba3f7 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -197,7 +197,7 @@ static int swap_arm_timer(Swap *s, usec_t usec) { return 0; } -static int swap_add_device_links(Swap *s) { +static int swap_add_device_dependencies(Swap *s) { assert(s); if (!s->what) @@ -207,12 +207,12 @@ static int swap_add_device_links(Swap *s) { return 0; if (is_device_path(s->what)) - return unit_add_node_link(UNIT(s), s->what, MANAGER_IS_SYSTEM(UNIT(s)->manager), UNIT_BINDS_TO); + return unit_add_node_dependency(UNIT(s), s->what, MANAGER_IS_SYSTEM(UNIT(s)->manager), UNIT_BINDS_TO, UNIT_DEPENDENCY_FILE); else /* File based swap devices need to be ordered after * systemd-remount-fs.service, since they might need a * writable file system. */ - return unit_add_dependency_by_name(UNIT(s), UNIT_AFTER, SPECIAL_REMOUNT_FS_SERVICE, NULL, true); + return unit_add_dependency_by_name(UNIT(s), UNIT_AFTER, SPECIAL_REMOUNT_FS_SERVICE, NULL, true, UNIT_DEPENDENCY_FILE); } static int swap_add_default_dependencies(Swap *s) { @@ -231,11 +231,11 @@ static int swap_add_default_dependencies(Swap *s) { /* swap units generated for the swap dev links are missing the * ordering dep against the swap target. */ - r = unit_add_dependency_by_name(UNIT(s), UNIT_BEFORE, SPECIAL_SWAP_TARGET, NULL, true); + r = unit_add_dependency_by_name(UNIT(s), UNIT_BEFORE, SPECIAL_SWAP_TARGET, NULL, true, UNIT_DEPENDENCY_DEFAULT); if (r < 0) return r; - return unit_add_two_dependencies_by_name(UNIT(s), UNIT_BEFORE, UNIT_CONFLICTS, SPECIAL_UMOUNT_TARGET, NULL, true); + return unit_add_two_dependencies_by_name(UNIT(s), UNIT_BEFORE, UNIT_CONFLICTS, SPECIAL_UMOUNT_TARGET, NULL, true, UNIT_DEPENDENCY_DEFAULT); } static int swap_verify(Swap *s) { @@ -323,11 +323,11 @@ static int swap_load(Unit *u) { return r; } - r = unit_require_mounts_for(UNIT(s), s->what); + r = unit_require_mounts_for(UNIT(s), s->what, UNIT_DEPENDENCY_IMPLICIT); if (r < 0) return r; - r = swap_add_device_links(s); + r = swap_add_device_dependencies(s); if (r < 0) return r; diff --git a/src/core/target.c b/src/core/target.c index 2a58dd394d..7083813459 100644 --- a/src/core/target.c +++ b/src/core/target.c @@ -56,8 +56,6 @@ static int target_add_default_dependencies(Target *t) { UNIT_PART_OF }; - Iterator i; - Unit *other; int r; unsigned k; @@ -66,23 +64,26 @@ static int target_add_default_dependencies(Target *t) { if (!UNIT(t)->default_dependencies) return 0; - /* Imply ordering for requirement dependencies on target - * units. Note that when the user created a contradicting - * ordering manually we won't add anything in here to make - * sure we don't create a loop. */ + /* Imply ordering for requirement dependencies on target units. Note that when the user created a contradicting + * ordering manually we won't add anything in here to make sure we don't create a loop. */ - for (k = 0; k < ELEMENTSOF(deps); k++) - SET_FOREACH(other, UNIT(t)->dependencies[deps[k]], i) { + for (k = 0; k < ELEMENTSOF(deps); k++) { + Unit *other; + Iterator i; + void *v; + + HASHMAP_FOREACH_KEY(v, other, UNIT(t)->dependencies[deps[k]], i) { r = unit_add_default_target_dependency(other, UNIT(t)); if (r < 0) return r; } + } if (unit_has_name(UNIT(t), SPECIAL_SHUTDOWN_TARGET)) return 0; /* Make sure targets are unloaded on shutdown */ - return unit_add_two_dependencies_by_name(UNIT(t), UNIT_BEFORE, UNIT_CONFLICTS, SPECIAL_SHUTDOWN_TARGET, NULL, true); + return unit_add_two_dependencies_by_name(UNIT(t), UNIT_BEFORE, UNIT_CONFLICTS, SPECIAL_SHUTDOWN_TARGET, NULL, true, UNIT_DEPENDENCY_DEFAULT); } static int target_load(Unit *u) { diff --git a/src/core/timer.c b/src/core/timer.c index 9ea5f322d8..5f50576999 100644 --- a/src/core/timer.c +++ b/src/core/timer.c @@ -105,18 +105,18 @@ static int timer_add_default_dependencies(Timer *t) { if (!UNIT(t)->default_dependencies) return 0; - r = unit_add_dependency_by_name(UNIT(t), UNIT_BEFORE, SPECIAL_TIMERS_TARGET, NULL, true); + r = unit_add_dependency_by_name(UNIT(t), UNIT_BEFORE, SPECIAL_TIMERS_TARGET, NULL, true, UNIT_DEPENDENCY_DEFAULT); if (r < 0) return r; if (MANAGER_IS_SYSTEM(UNIT(t)->manager)) { - r = unit_add_two_dependencies_by_name(UNIT(t), UNIT_AFTER, UNIT_REQUIRES, SPECIAL_SYSINIT_TARGET, NULL, true); + r = unit_add_two_dependencies_by_name(UNIT(t), UNIT_AFTER, UNIT_REQUIRES, SPECIAL_SYSINIT_TARGET, NULL, true, UNIT_DEPENDENCY_DEFAULT); if (r < 0) return r; LIST_FOREACH(value, v, t->values) { if (v->base == TIMER_CALENDAR) { - r = unit_add_dependency_by_name(UNIT(t), UNIT_AFTER, SPECIAL_TIME_SYNC_TARGET, NULL, true); + r = unit_add_dependency_by_name(UNIT(t), UNIT_AFTER, SPECIAL_TIME_SYNC_TARGET, NULL, true, UNIT_DEPENDENCY_DEFAULT); if (r < 0) return r; break; @@ -124,7 +124,23 @@ static int timer_add_default_dependencies(Timer *t) { } } - return unit_add_two_dependencies_by_name(UNIT(t), UNIT_BEFORE, UNIT_CONFLICTS, SPECIAL_SHUTDOWN_TARGET, NULL, true); + return unit_add_two_dependencies_by_name(UNIT(t), UNIT_BEFORE, UNIT_CONFLICTS, SPECIAL_SHUTDOWN_TARGET, NULL, true, UNIT_DEPENDENCY_DEFAULT); +} + +static int timer_add_trigger_dependencies(Timer *t) { + Unit *x; + int r; + + assert(t); + + if (!hashmap_isempty(UNIT(t)->dependencies[UNIT_TRIGGERS])) + return 0; + + r = unit_load_related_unit(UNIT(t), ".service", &x); + if (r < 0) + return r; + + return unit_add_two_dependencies(UNIT(t), UNIT_BEFORE, UNIT_TRIGGERS, x, true, UNIT_DEPENDENCY_IMPLICIT); } static int timer_setup_persistent(Timer *t) { @@ -137,7 +153,7 @@ static int timer_setup_persistent(Timer *t) { if (MANAGER_IS_SYSTEM(UNIT(t)->manager)) { - r = unit_require_mounts_for(UNIT(t), "/var/lib/systemd/timers"); + r = unit_require_mounts_for(UNIT(t), "/var/lib/systemd/timers", UNIT_DEPENDENCY_FILE); if (r < 0) return r; @@ -179,17 +195,9 @@ static int timer_load(Unit *u) { if (u->load_state == UNIT_LOADED) { - if (set_isempty(u->dependencies[UNIT_TRIGGERS])) { - Unit *x; - - r = unit_load_related_unit(u, ".service", &x); - if (r < 0) - return r; - - r = unit_add_two_dependencies(u, UNIT_BEFORE, UNIT_TRIGGERS, x, true); - if (r < 0) - return r; - } + r = timer_add_trigger_dependencies(t); + if (r < 0) + return r; r = timer_setup_persistent(t); if (r < 0) diff --git a/src/core/transaction.c b/src/core/transaction.c index 46c2fa452e..52014c634f 100644 --- a/src/core/transaction.c +++ b/src/core/transaction.c @@ -361,6 +361,7 @@ static char* merge_unit_ids(const char* unit_log_field, char **pairs) { static int transaction_verify_order_one(Transaction *tr, Job *j, Job *from, unsigned generation, sd_bus_error *e) { Iterator i; Unit *u; + void *v; int r; assert(tr); @@ -452,7 +453,7 @@ static int transaction_verify_order_one(Transaction *tr, Job *j, Job *from, unsi /* We assume that the dependencies are bidirectional, and * hence can ignore UNIT_AFTER */ - SET_FOREACH(u, j->unit->dependencies[UNIT_BEFORE], i) { + HASHMAP_FOREACH_KEY(v, u, j->unit->dependencies[UNIT_BEFORE], i) { Job *o; /* Is there a job for this unit? */ @@ -860,14 +861,15 @@ static void transaction_unlink_job(Transaction *tr, Job *j, bool delete_dependen void transaction_add_propagate_reload_jobs(Transaction *tr, Unit *unit, Job *by, bool ignore_order, sd_bus_error *e) { Iterator i; - Unit *dep; JobType nt; + Unit *dep; + void *v; int r; assert(tr); assert(unit); - SET_FOREACH(dep, unit->dependencies[UNIT_PROPAGATES_RELOAD_TO], i) { + HASHMAP_FOREACH_KEY(v, dep, unit->dependencies[UNIT_PROPAGATES_RELOAD_TO], i) { nt = job_type_collapse(JOB_TRY_RELOAD, dep); if (nt == JOB_NOP) continue; @@ -892,11 +894,13 @@ int transaction_add_job_and_dependencies( bool ignore_requirements, bool ignore_order, sd_bus_error *e) { - Job *ret; + + bool is_new; Iterator i; Unit *dep; + Job *ret; + void *v; int r; - bool is_new; assert(tr); assert(type < _JOB_TYPE_MAX); @@ -969,7 +973,7 @@ int transaction_add_job_and_dependencies( /* Finally, recursively add in all dependencies. */ if (IN_SET(type, JOB_START, JOB_RESTART)) { - SET_FOREACH(dep, ret->unit->dependencies[UNIT_REQUIRES], i) { + HASHMAP_FOREACH_KEY(v, dep, ret->unit->dependencies[UNIT_REQUIRES], i) { r = transaction_add_job_and_dependencies(tr, JOB_START, dep, ret, true, false, false, ignore_order, e); if (r < 0) { if (r != -EBADR) /* job type not applicable */ @@ -979,7 +983,7 @@ int transaction_add_job_and_dependencies( } } - SET_FOREACH(dep, ret->unit->dependencies[UNIT_BINDS_TO], i) { + HASHMAP_FOREACH_KEY(v, dep, ret->unit->dependencies[UNIT_BINDS_TO], i) { r = transaction_add_job_and_dependencies(tr, JOB_START, dep, ret, true, false, false, ignore_order, e); if (r < 0) { if (r != -EBADR) /* job type not applicable */ @@ -989,7 +993,7 @@ int transaction_add_job_and_dependencies( } } - SET_FOREACH(dep, ret->unit->dependencies[UNIT_WANTS], i) { + HASHMAP_FOREACH_KEY(v, dep, ret->unit->dependencies[UNIT_WANTS], i) { r = transaction_add_job_and_dependencies(tr, JOB_START, dep, ret, false, false, false, ignore_order, e); if (r < 0) { /* unit masked, job type not applicable and unit not found are not considered as errors. */ @@ -1001,7 +1005,7 @@ int transaction_add_job_and_dependencies( } } - SET_FOREACH(dep, ret->unit->dependencies[UNIT_REQUISITE], i) { + HASHMAP_FOREACH_KEY(v, dep, ret->unit->dependencies[UNIT_REQUISITE], i) { r = transaction_add_job_and_dependencies(tr, JOB_VERIFY_ACTIVE, dep, ret, true, false, false, ignore_order, e); if (r < 0) { if (r != -EBADR) /* job type not applicable */ @@ -1011,7 +1015,7 @@ int transaction_add_job_and_dependencies( } } - SET_FOREACH(dep, ret->unit->dependencies[UNIT_CONFLICTS], i) { + HASHMAP_FOREACH_KEY(v, dep, ret->unit->dependencies[UNIT_CONFLICTS], i) { r = transaction_add_job_and_dependencies(tr, JOB_STOP, dep, ret, true, true, false, ignore_order, e); if (r < 0) { if (r != -EBADR) /* job type not applicable */ @@ -1021,7 +1025,7 @@ int transaction_add_job_and_dependencies( } } - SET_FOREACH(dep, ret->unit->dependencies[UNIT_CONFLICTED_BY], i) { + HASHMAP_FOREACH_KEY(v, dep, ret->unit->dependencies[UNIT_CONFLICTED_BY], i) { r = transaction_add_job_and_dependencies(tr, JOB_STOP, dep, ret, false, false, false, ignore_order, e); if (r < 0) { log_unit_warning(dep, @@ -1050,7 +1054,7 @@ int transaction_add_job_and_dependencies( ptype = type == JOB_RESTART ? JOB_TRY_RESTART : type; for (j = 0; j < ELEMENTSOF(propagate_deps); j++) - SET_FOREACH(dep, ret->unit->dependencies[propagate_deps[j]], i) { + HASHMAP_FOREACH_KEY(v, dep, ret->unit->dependencies[propagate_deps[j]], i) { JobType nt; nt = job_type_collapse(ptype, dep); diff --git a/src/core/unit.c b/src/core/unit.c index f6b281b285..76902cca90 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -421,25 +421,25 @@ void unit_add_to_dbus_queue(Unit *u) { u->in_dbus_queue = true; } -static void bidi_set_free(Unit *u, Set *s) { - Iterator i; +static void bidi_set_free(Unit *u, Hashmap *h) { Unit *other; + Iterator i; + void *v; assert(u); - /* Frees the set and makes sure we are dropped from the - * inverse pointers */ + /* Frees the hashmap and makes sure we are dropped from the inverse pointers */ - SET_FOREACH(other, s, i) { + HASHMAP_FOREACH_KEY(v, other, h, i) { UnitDependency d; for (d = 0; d < _UNIT_DEPENDENCY_MAX; d++) - set_remove(other->dependencies[d], u); + hashmap_remove(other->dependencies[d], u); unit_add_to_gc_queue(other); } - set_free(s); + hashmap_free(h); } static void unit_remove_transient(Unit *u) { @@ -474,30 +474,37 @@ static void unit_remove_transient(Unit *u) { } static void unit_free_requires_mounts_for(Unit *u) { - char **j; + assert(u); - STRV_FOREACH(j, u->requires_mounts_for) { - char s[strlen(*j) + 1]; + for (;;) { + _cleanup_free_ char *path; - PATH_FOREACH_PREFIX_MORE(s, *j) { - char *y; - Set *x; + path = hashmap_steal_first_key(u->requires_mounts_for); + if (!path) + break; + else { + char s[strlen(path) + 1]; - x = hashmap_get2(u->manager->units_requiring_mounts_for, s, (void**) &y); - if (!x) - continue; + PATH_FOREACH_PREFIX_MORE(s, path) { + char *y; + Set *x; - set_remove(x, u); + x = hashmap_get2(u->manager->units_requiring_mounts_for, s, (void**) &y); + if (!x) + continue; - if (set_isempty(x)) { - hashmap_remove(u->manager->units_requiring_mounts_for, y); - free(y); - set_free(x); + (void) set_remove(x, u); + + if (set_isempty(x)) { + (void) hashmap_remove(u->manager->units_requiring_mounts_for, y); + free(y); + set_free(x); + } } } } - u->requires_mounts_for = strv_free(u->requires_mounts_for); + u->requires_mounts_for = hashmap_free(u->requires_mounts_for); } static void unit_done(Unit *u) { @@ -651,20 +658,33 @@ const char* unit_sub_state_to_string(Unit *u) { return UNIT_VTABLE(u)->sub_state_to_string(u); } -static int complete_move(Set **s, Set **other) { - int r; +static int set_complete_move(Set **s, Set **other) { + assert(s); + assert(other); + if (!other) + return 0; + + if (*s) + return set_move(*s, *other); + else { + *s = *other; + *other = NULL; + } + + return 0; +} + +static int hashmap_complete_move(Hashmap **s, Hashmap **other) { assert(s); assert(other); if (!*other) return 0; - if (*s) { - r = set_move(*s, *other); - if (r < 0) - return r; - } else { + if (*s) + return hashmap_move(*s, *other); + else { *s = *other; *other = NULL; } @@ -680,7 +700,7 @@ static int merge_names(Unit *u, Unit *other) { assert(u); assert(other); - r = complete_move(&u->names, &other->names); + r = set_complete_move(&u->names, &other->names); if (r < 0) return r; @@ -710,48 +730,73 @@ static int reserve_dependencies(Unit *u, Unit *other, UnitDependency d) { return 0; /* merge_dependencies() will skip a u-on-u dependency */ - n_reserve = set_size(other->dependencies[d]) - !!set_get(other->dependencies[d], u); + n_reserve = hashmap_size(other->dependencies[d]) - !!hashmap_get(other->dependencies[d], u); - return set_reserve(u->dependencies[d], n_reserve); + return hashmap_reserve(u->dependencies[d], n_reserve); } static void merge_dependencies(Unit *u, Unit *other, const char *other_id, UnitDependency d) { Iterator i; Unit *back; + void *v; int r; + /* Merges all dependencies of type 'd' of the unit 'other' into the deps of the unit 'u' */ + assert(u); assert(other); assert(d < _UNIT_DEPENDENCY_MAX); - /* Fix backwards pointers */ - SET_FOREACH(back, other->dependencies[d], i) { + /* Fix backwards pointers. Let's iterate through all dependendent units of the other unit. */ + HASHMAP_FOREACH_KEY(v, back, other->dependencies[d], i) { UnitDependency k; + /* Let's now iterate through the dependencies of that dependencies of the other units, looking for + * pointers back, and let's fix them up, to instead point to 'u'. */ + for (k = 0; k < _UNIT_DEPENDENCY_MAX; k++) { - /* Do not add dependencies between u and itself */ if (back == u) { - if (set_remove(back->dependencies[k], other)) + /* Do not add dependencies between u and itself. */ + if (hashmap_remove(back->dependencies[k], other)) maybe_warn_about_dependency(u, other_id, k); } else { - r = set_remove_and_put(back->dependencies[k], other, u); - if (r == -EEXIST) - set_remove(back->dependencies[k], other); - else - assert(r >= 0 || r == -ENOENT); + UnitDependencyInfo di_u, di_other, di_merged; + + /* Let's drop this dependency between "back" and "other", and let's create it between + * "back" and "u" instead. Let's merge the bit masks of the dependency we are moving, + * and any such dependency which might already exist */ + + di_other.data = hashmap_get(back->dependencies[k], other); + if (!di_other.data) + continue; /* dependency isn't set, let's try the next one */ + + di_u.data = hashmap_get(back->dependencies[k], u); + + di_merged = (UnitDependencyInfo) { + .origin_mask = di_u.origin_mask | di_other.origin_mask, + .destination_mask = di_u.destination_mask | di_other.destination_mask, + }; + + r = hashmap_remove_and_replace(back->dependencies[k], other, u, di_merged.data); + if (r < 0) + log_warning_errno(r, "Failed to remove/replace: back=%s other=%s u=%s: %m", back->id, other_id, u->id); + assert(r >= 0); + + /* assert_se(hashmap_remove_and_replace(back->dependencies[k], other, u, di_merged.data) >= 0); */ } } + } /* Also do not move dependencies on u to itself */ - back = set_remove(other->dependencies[d], u); + back = hashmap_remove(other->dependencies[d], u); if (back) maybe_warn_about_dependency(u, other_id, d); /* The move cannot fail. The caller must have performed a reservation. */ - assert_se(complete_move(&u->dependencies[d], &other->dependencies[d]) == 0); + assert_se(hashmap_complete_move(&u->dependencies[d], &other->dependencies[d]) == 0); - other->dependencies[d] = set_free(other->dependencies[d]); + other->dependencies[d] = hashmap_free(other->dependencies[d]); } int unit_merge(Unit *u, Unit *other) { @@ -876,19 +921,19 @@ int unit_add_exec_dependencies(Unit *u, ExecContext *c) { assert(c); if (c->working_directory) { - r = unit_require_mounts_for(u, c->working_directory); + r = unit_require_mounts_for(u, c->working_directory, UNIT_DEPENDENCY_FILE); if (r < 0) return r; } if (c->root_directory) { - r = unit_require_mounts_for(u, c->root_directory); + r = unit_require_mounts_for(u, c->root_directory, UNIT_DEPENDENCY_FILE); if (r < 0) return r; } if (c->root_image) { - r = unit_require_mounts_for(u, c->root_image); + r = unit_require_mounts_for(u, c->root_image, UNIT_DEPENDENCY_FILE); if (r < 0) return r; } @@ -904,7 +949,7 @@ int unit_add_exec_dependencies(Unit *u, ExecContext *c) { if (!p) return -ENOMEM; - r = unit_require_mounts_for(u, p); + r = unit_require_mounts_for(u, p, UNIT_DEPENDENCY_FILE); if (r < 0) return r; } @@ -917,12 +962,12 @@ int unit_add_exec_dependencies(Unit *u, ExecContext *c) { const char *p; FOREACH_STRING(p, "/tmp", "/var/tmp") { - r = unit_require_mounts_for(u, p); + r = unit_require_mounts_for(u, p, UNIT_DEPENDENCY_FILE); if (r < 0) return r; } - r = unit_add_dependency_by_name(u, UNIT_AFTER, SPECIAL_TMPFILES_SETUP_SERVICE, NULL, true); + r = unit_add_dependency_by_name(u, UNIT_AFTER, SPECIAL_TMPFILES_SETUP_SERVICE, NULL, true, UNIT_DEPENDENCY_FILE); if (r < 0) return r; } @@ -940,7 +985,7 @@ int unit_add_exec_dependencies(Unit *u, ExecContext *c) { /* If syslog or kernel logging is requested, make sure our own * logging daemon is run first. */ - r = unit_add_dependency_by_name(u, UNIT_AFTER, SPECIAL_JOURNALD_SOCKET, NULL, true); + r = unit_add_dependency_by_name(u, UNIT_AFTER, SPECIAL_JOURNALD_SOCKET, NULL, true, UNIT_DEPENDENCY_FILE); if (r < 0) return r; @@ -956,6 +1001,48 @@ const char *unit_description(Unit *u) { return strna(u->id); } +static void print_unit_dependency_mask(FILE *f, const char *kind, UnitDependencyMask mask, bool *space) { + const struct { + UnitDependencyMask mask; + const char *name; + } table[] = { + { UNIT_DEPENDENCY_FILE, "file" }, + { UNIT_DEPENDENCY_IMPLICIT, "implicit" }, + { UNIT_DEPENDENCY_DEFAULT, "default" }, + { UNIT_DEPENDENCY_UDEV, "udev" }, + { UNIT_DEPENDENCY_PATH, "path" }, + { UNIT_DEPENDENCY_MOUNTINFO_IMPLICIT, "mountinfo-implicit" }, + { UNIT_DEPENDENCY_MOUNTINFO_DEFAULT, "mountinfo-default" }, + { UNIT_DEPENDENCY_PROC_SWAP, "proc-swap" }, + }; + size_t i; + + assert(f); + assert(kind); + assert(space); + + for (i = 0; i < ELEMENTSOF(table); i++) { + + if (mask == 0) + break; + + if ((mask & table[i].mask) == table[i].mask) { + if (*space) + fputc(' ', f); + else + *space = true; + + fputs(kind, f); + fputs("-", f); + fputs(table[i].name, f); + + mask &= ~table[i].mask; + } + } + + assert(mask == 0); +} + void unit_dump(Unit *u, FILE *f, const char *prefix) { char *t, **j; UnitDependency d; @@ -1084,20 +1171,35 @@ void unit_dump(Unit *u, FILE *f, const char *prefix) { prefix, yes_no(u->assert_result)); for (d = 0; d < _UNIT_DEPENDENCY_MAX; d++) { + UnitDependencyInfo di; Unit *other; - SET_FOREACH(other, u->dependencies[d], i) - fprintf(f, "%s\t%s: %s\n", prefix, unit_dependency_to_string(d), other->id); + HASHMAP_FOREACH_KEY(di.data, other, u->dependencies[d], i) { + bool space = false; + + fprintf(f, "%s\t%s: %s (", prefix, unit_dependency_to_string(d), other->id); + + print_unit_dependency_mask(f, "origin", di.origin_mask, &space); + print_unit_dependency_mask(f, "destination", di.destination_mask, &space); + + fputs(")\n", f); + } } - if (!strv_isempty(u->requires_mounts_for)) { - fprintf(f, - "%s\tRequiresMountsFor:", prefix); + if (!hashmap_isempty(u->requires_mounts_for)) { + UnitDependencyInfo di; + const char *path; - STRV_FOREACH(j, u->requires_mounts_for) - fprintf(f, " %s", *j); + HASHMAP_FOREACH_KEY(di.data, path, u->requires_mounts_for, i) { + bool space = false; - fputs("\n", f); + fprintf(f, "%s\tRequiresMountsFor: %s (", prefix, path); + + print_unit_dependency_mask(f, "origin", di.origin_mask, &space); + print_unit_dependency_mask(f, "destination", di.destination_mask, &space); + + fputs(")\n", f); + } } if (u->load_state == UNIT_LOADED) { @@ -1198,10 +1300,10 @@ int unit_add_default_target_dependency(Unit *u, Unit *target) { return 0; /* Don't create loops */ - if (set_get(target->dependencies[UNIT_BEFORE], u)) + if (hashmap_get(target->dependencies[UNIT_BEFORE], u)) return 0; - return unit_add_dependency(target, UNIT_AFTER, u, true); + return unit_add_dependency(target, UNIT_AFTER, u, true, UNIT_DEPENDENCY_DEFAULT); } static int unit_add_target_dependencies(Unit *u) { @@ -1213,48 +1315,59 @@ static int unit_add_target_dependencies(Unit *u) { UNIT_BOUND_BY }; - Unit *target; - Iterator i; unsigned k; int r = 0; assert(u); - for (k = 0; k < ELEMENTSOF(deps); k++) - SET_FOREACH(target, u->dependencies[deps[k]], i) { + for (k = 0; k < ELEMENTSOF(deps); k++) { + Unit *target; + Iterator i; + void *v; + + HASHMAP_FOREACH_KEY(v, target, u->dependencies[deps[k]], i) { r = unit_add_default_target_dependency(u, target); if (r < 0) return r; } + } return r; } static int unit_add_slice_dependencies(Unit *u) { + UnitDependencyMask mask; assert(u); if (!UNIT_HAS_CGROUP_CONTEXT(u)) return 0; + /* Slice units are implicitly ordered against their parent slices (as this relationship is encoded in the + name), while all other units are ordered based on configuration (as in their case Slice= configures the + relationship). */ + mask = u->type == UNIT_SLICE ? UNIT_DEPENDENCY_IMPLICIT : UNIT_DEPENDENCY_FILE; + if (UNIT_ISSET(u->slice)) - return unit_add_two_dependencies(u, UNIT_AFTER, UNIT_REQUIRES, UNIT_DEREF(u->slice), true); + return unit_add_two_dependencies(u, UNIT_AFTER, UNIT_REQUIRES, UNIT_DEREF(u->slice), true, mask); if (unit_has_name(u, SPECIAL_ROOT_SLICE)) return 0; - return unit_add_two_dependencies_by_name(u, UNIT_AFTER, UNIT_REQUIRES, SPECIAL_ROOT_SLICE, NULL, true); + return unit_add_two_dependencies_by_name(u, UNIT_AFTER, UNIT_REQUIRES, SPECIAL_ROOT_SLICE, NULL, true, mask); } static int unit_add_mount_dependencies(Unit *u) { - char **i; + UnitDependencyInfo di; + const char *path; + Iterator i; int r; assert(u); - STRV_FOREACH(i, u->requires_mounts_for) { - char prefix[strlen(*i) + 1]; + HASHMAP_FOREACH_KEY(di.data, path, u->requires_mounts_for, i) { + char prefix[strlen(path) + 1]; - PATH_FOREACH_PREFIX_MORE(prefix, *i) { + PATH_FOREACH_PREFIX_MORE(prefix, path) { _cleanup_free_ char *p = NULL; Unit *m; @@ -1278,12 +1391,12 @@ static int unit_add_mount_dependencies(Unit *u) { if (m->load_state != UNIT_LOADED) continue; - r = unit_add_dependency(u, UNIT_AFTER, m, true); + r = unit_add_dependency(u, UNIT_AFTER, m, true, di.origin_mask); if (r < 0) return r; if (m->fragment_path) { - r = unit_add_dependency(u, UNIT_REQUIRES, m, true); + r = unit_add_dependency(u, UNIT_REQUIRES, m, true, di.origin_mask); if (r < 0) return r; } @@ -1369,7 +1482,7 @@ int unit_load(Unit *u) { if (r < 0) goto fail; - if (u->on_failure_job_mode == JOB_ISOLATE && set_size(u->dependencies[UNIT_ON_FAILURE]) > 1) { + if (u->on_failure_job_mode == JOB_ISOLATE && hashmap_size(u->dependencies[UNIT_ON_FAILURE]) > 1) { log_unit_error(u, "More than one OnFailure= dependencies specified but OnFailureJobMode=isolate set. Refusing."); r = -EINVAL; goto fail; @@ -1584,6 +1697,7 @@ bool unit_shall_confirm_spawn(Unit *u) { static bool unit_verify_deps(Unit *u) { Unit *other; Iterator j; + void *v; assert(u); @@ -1592,9 +1706,9 @@ static bool unit_verify_deps(Unit *u) { * processing, but do not have any effect afterwards. We don't check BindsTo= dependencies that are not used in * conjunction with After= as for them any such check would make things entirely racy. */ - SET_FOREACH(other, u->dependencies[UNIT_BINDS_TO], j) { + HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_BINDS_TO], j) { - if (!set_contains(u->dependencies[UNIT_AFTER], other)) + if (!hashmap_contains(u->dependencies[UNIT_AFTER], other)) continue; if (!UNIT_IS_ACTIVE_OR_RELOADING(unit_active_state(other))) { @@ -1797,7 +1911,7 @@ bool unit_can_reload(Unit *u) { if (UNIT_VTABLE(u)->can_reload) return UNIT_VTABLE(u)->can_reload(u); - if (!set_isempty(u->dependencies[UNIT_PROPAGATES_RELOAD_TO])) + if (!hashmap_isempty(u->dependencies[UNIT_PROPAGATES_RELOAD_TO])) return true; return UNIT_VTABLE(u)->reload; @@ -1814,8 +1928,6 @@ static void unit_check_unneeded(Unit *u) { UNIT_BOUND_BY, }; - Unit *other; - Iterator i; unsigned j; int r; @@ -1830,10 +1942,15 @@ static void unit_check_unneeded(Unit *u) { if (!UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(u))) return; - for (j = 0; j < ELEMENTSOF(needed_dependencies); j++) - SET_FOREACH(other, u->dependencies[needed_dependencies[j]], i) + for (j = 0; j < ELEMENTSOF(needed_dependencies); j++) { + Unit *other; + Iterator i; + void *v; + + HASHMAP_FOREACH_KEY(v, other, u->dependencies[needed_dependencies[j]], i) if (unit_active_or_pending(other)) return; + } /* If stopping a unit fails continuously we might enter a stop * loop here, hence stop acting on the service being @@ -1856,6 +1973,7 @@ static void unit_check_binds_to(Unit *u) { bool stop = false; Unit *other; Iterator i; + void *v; int r; assert(u); @@ -1866,7 +1984,7 @@ static void unit_check_binds_to(Unit *u) { if (unit_active_state(u) != UNIT_ACTIVE) return; - SET_FOREACH(other, u->dependencies[UNIT_BINDS_TO], i) { + HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_BINDS_TO], i) { if (other->job) continue; @@ -1904,65 +2022,68 @@ static void unit_check_binds_to(Unit *u) { static void retroactively_start_dependencies(Unit *u) { Iterator i; Unit *other; + void *v; assert(u); assert(UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(u))); - SET_FOREACH(other, u->dependencies[UNIT_REQUIRES], i) - if (!set_get(u->dependencies[UNIT_AFTER], other) && + HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_REQUIRES], i) + if (!hashmap_get(u->dependencies[UNIT_AFTER], other) && !UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(other))) manager_add_job(u->manager, JOB_START, other, JOB_REPLACE, NULL, NULL); - SET_FOREACH(other, u->dependencies[UNIT_BINDS_TO], i) - if (!set_get(u->dependencies[UNIT_AFTER], other) && + HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_BINDS_TO], i) + if (!hashmap_get(u->dependencies[UNIT_AFTER], other) && !UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(other))) manager_add_job(u->manager, JOB_START, other, JOB_REPLACE, NULL, NULL); - SET_FOREACH(other, u->dependencies[UNIT_WANTS], i) - if (!set_get(u->dependencies[UNIT_AFTER], other) && + HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_WANTS], i) + if (!hashmap_get(u->dependencies[UNIT_AFTER], other) && !UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(other))) manager_add_job(u->manager, JOB_START, other, JOB_FAIL, NULL, NULL); - SET_FOREACH(other, u->dependencies[UNIT_CONFLICTS], i) + HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_CONFLICTS], i) if (!UNIT_IS_INACTIVE_OR_DEACTIVATING(unit_active_state(other))) manager_add_job(u->manager, JOB_STOP, other, JOB_REPLACE, NULL, NULL); - SET_FOREACH(other, u->dependencies[UNIT_CONFLICTED_BY], i) + HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_CONFLICTED_BY], i) if (!UNIT_IS_INACTIVE_OR_DEACTIVATING(unit_active_state(other))) manager_add_job(u->manager, JOB_STOP, other, JOB_REPLACE, NULL, NULL); } static void retroactively_stop_dependencies(Unit *u) { - Iterator i; Unit *other; + Iterator i; + void *v; assert(u); assert(UNIT_IS_INACTIVE_OR_DEACTIVATING(unit_active_state(u))); /* Pull down units which are bound to us recursively if enabled */ - SET_FOREACH(other, u->dependencies[UNIT_BOUND_BY], i) + HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_BOUND_BY], i) if (!UNIT_IS_INACTIVE_OR_DEACTIVATING(unit_active_state(other))) manager_add_job(u->manager, JOB_STOP, other, JOB_REPLACE, NULL, NULL); } static void check_unneeded_dependencies(Unit *u) { - Iterator i; Unit *other; + Iterator i; + void *v; assert(u); assert(UNIT_IS_INACTIVE_OR_DEACTIVATING(unit_active_state(u))); /* Garbage collect services that might not be needed anymore, if enabled */ - SET_FOREACH(other, u->dependencies[UNIT_REQUIRES], i) + HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_REQUIRES], i) if (!UNIT_IS_INACTIVE_OR_DEACTIVATING(unit_active_state(other))) unit_check_unneeded(other); - SET_FOREACH(other, u->dependencies[UNIT_WANTS], i) + HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_WANTS], i) if (!UNIT_IS_INACTIVE_OR_DEACTIVATING(unit_active_state(other))) unit_check_unneeded(other); - SET_FOREACH(other, u->dependencies[UNIT_REQUISITE], i) + HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_REQUISITE], i) if (!UNIT_IS_INACTIVE_OR_DEACTIVATING(unit_active_state(other))) unit_check_unneeded(other); - SET_FOREACH(other, u->dependencies[UNIT_BINDS_TO], i) + HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_BINDS_TO], i) if (!UNIT_IS_INACTIVE_OR_DEACTIVATING(unit_active_state(other))) unit_check_unneeded(other); } @@ -1970,15 +2091,16 @@ static void check_unneeded_dependencies(Unit *u) { void unit_start_on_failure(Unit *u) { Unit *other; Iterator i; + void *v; assert(u); - if (set_size(u->dependencies[UNIT_ON_FAILURE]) <= 0) + if (hashmap_size(u->dependencies[UNIT_ON_FAILURE]) <= 0) return; log_unit_info(u, "Triggering OnFailure= dependencies."); - SET_FOREACH(other, u->dependencies[UNIT_ON_FAILURE], i) { + HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_ON_FAILURE], i) { int r; r = manager_add_job(u->manager, JOB_START, other, u->on_failure_job_mode, NULL, NULL); @@ -1990,10 +2112,11 @@ void unit_start_on_failure(Unit *u) { void unit_trigger_notify(Unit *u) { Unit *other; Iterator i; + void *v; assert(u); - SET_FOREACH(other, u->dependencies[UNIT_TRIGGERED_BY], i) + HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_TRIGGERED_BY], i) if (UNIT_VTABLE(other)->trigger_notify) UNIT_VTABLE(other)->trigger_notify(other, u); } @@ -2465,7 +2588,59 @@ static void maybe_warn_about_dependency(Unit *u, const char *other, UnitDependen log_unit_warning(u, "Dependency %s=%s dropped, merged into %s", unit_dependency_to_string(dependency), strna(other), u->id); } -int unit_add_dependency(Unit *u, UnitDependency d, Unit *other, bool add_reference) { +static int unit_add_dependency_hashmap( + Hashmap **h, + Unit *other, + UnitDependencyMask origin_mask, + UnitDependencyMask destination_mask) { + + UnitDependencyInfo info; + int r; + + assert(h); + assert(other); + assert(origin_mask < _UNIT_DEPENDENCY_MASK_FULL); + assert(destination_mask < _UNIT_DEPENDENCY_MASK_FULL); + assert(origin_mask > 0 || destination_mask > 0); + + r = hashmap_ensure_allocated(h, NULL); + if (r < 0) + return r; + + assert_cc(sizeof(void*) == sizeof(info)); + + info.data = hashmap_get(*h, other); + if (info.data) { + /* Entry already exists. Add in our mask. */ + + if ((info.origin_mask & origin_mask) == info.origin_mask && + (info.destination_mask & destination_mask) == info.destination_mask) + return 0; /* NOP */ + + info.origin_mask |= origin_mask; + info.destination_mask |= destination_mask; + + r = hashmap_update(*h, other, info.data); + } else { + info = (UnitDependencyInfo) { + .origin_mask = origin_mask, + .destination_mask = destination_mask, + }; + + r = hashmap_put(*h, other, info.data); + } + if (r < 0) + return r; + + return 1; +} + +int unit_add_dependency( + Unit *u, + UnitDependency d, + Unit *other, + bool add_reference, + UnitDependencyMask mask) { static const UnitDependency inverse_table[_UNIT_DEPENDENCY_MAX] = { [UNIT_REQUIRES] = UNIT_REQUIRED_BY, @@ -2491,8 +2666,8 @@ int unit_add_dependency(Unit *u, UnitDependency d, Unit *other, bool add_referen [UNIT_RELOAD_PROPAGATED_FROM] = UNIT_PROPAGATES_RELOAD_TO, [UNIT_JOINS_NAMESPACE_OF] = UNIT_JOINS_NAMESPACE_OF, }; - int r, q = 0, v = 0, w = 0; - Unit *orig_u = u, *orig_other = other; + Unit *original_u = u, *original_other = other; + int r; assert(u); assert(d >= 0 && d < _UNIT_DEPENDENCY_MAX); @@ -2504,85 +2679,50 @@ int unit_add_dependency(Unit *u, UnitDependency d, Unit *other, bool add_referen /* We won't allow dependencies on ourselves. We will not * consider them an error however. */ if (u == other) { - maybe_warn_about_dependency(orig_u, orig_other->id, d); + maybe_warn_about_dependency(original_u, original_other->id, d); return 0; } - if (d == UNIT_BEFORE && other->type == UNIT_DEVICE) { + if ((d == UNIT_BEFORE && other->type == UNIT_DEVICE) || + (d == UNIT_AFTER && u->type == UNIT_DEVICE)) { log_unit_warning(u, "Dependency Before=%s ignored (.device units cannot be delayed)", other->id); return 0; } - r = set_ensure_allocated(&u->dependencies[d], NULL); + r = unit_add_dependency_hashmap(u->dependencies + d, other, mask, 0); if (r < 0) return r; - if (inverse_table[d] != _UNIT_DEPENDENCY_INVALID) { - r = set_ensure_allocated(&other->dependencies[inverse_table[d]], NULL); - if (r < 0) - return r; - } - - if (add_reference) { - r = set_ensure_allocated(&u->dependencies[UNIT_REFERENCES], NULL); - if (r < 0) - return r; - - r = set_ensure_allocated(&other->dependencies[UNIT_REFERENCED_BY], NULL); - if (r < 0) - return r; - } - - q = set_put(u->dependencies[d], other); - if (q < 0) - return q; - if (inverse_table[d] != _UNIT_DEPENDENCY_INVALID && inverse_table[d] != d) { - v = set_put(other->dependencies[inverse_table[d]], u); - if (v < 0) { - r = v; - goto fail; - } + r = unit_add_dependency_hashmap(other->dependencies + inverse_table[d], u, 0, mask); + if (r < 0) + return r; } if (add_reference) { - w = set_put(u->dependencies[UNIT_REFERENCES], other); - if (w < 0) { - r = w; - goto fail; - } - - r = set_put(other->dependencies[UNIT_REFERENCED_BY], u); + r = unit_add_dependency_hashmap(u->dependencies + UNIT_REFERENCES, other, mask, 0); if (r < 0) - goto fail; + return r; + + r = unit_add_dependency_hashmap(other->dependencies + UNIT_REFERENCED_BY, u, 0, mask); + if (r < 0) + return r; } unit_add_to_dbus_queue(u); return 0; - -fail: - if (q > 0) - set_remove(u->dependencies[d], other); - - if (v > 0) - set_remove(other->dependencies[inverse_table[d]], u); - - if (w > 0) - set_remove(u->dependencies[UNIT_REFERENCES], other); - - return r; } -int unit_add_two_dependencies(Unit *u, UnitDependency d, UnitDependency e, Unit *other, bool add_reference) { +int unit_add_two_dependencies(Unit *u, UnitDependency d, UnitDependency e, Unit *other, bool add_reference, UnitDependencyMask mask) { int r; assert(u); - r = unit_add_dependency(u, d, other, add_reference); + r = unit_add_dependency(u, d, other, add_reference, mask); if (r < 0) return r; - return unit_add_dependency(u, e, other, add_reference); + return unit_add_dependency(u, e, other, add_reference, mask); } static int resolve_template(Unit *u, const char *name, const char*path, char **buf, const char **ret) { @@ -2620,7 +2760,7 @@ static int resolve_template(Unit *u, const char *name, const char*path, char **b return 0; } -int unit_add_dependency_by_name(Unit *u, UnitDependency d, const char *name, const char *path, bool add_reference) { +int unit_add_dependency_by_name(Unit *u, UnitDependency d, const char *name, const char *path, bool add_reference, UnitDependencyMask mask) { _cleanup_free_ char *buf = NULL; Unit *other; int r; @@ -2636,10 +2776,10 @@ int unit_add_dependency_by_name(Unit *u, UnitDependency d, const char *name, con if (r < 0) return r; - return unit_add_dependency(u, d, other, add_reference); + return unit_add_dependency(u, d, other, add_reference, mask); } -int unit_add_two_dependencies_by_name(Unit *u, UnitDependency d, UnitDependency e, const char *name, const char *path, bool add_reference) { +int unit_add_two_dependencies_by_name(Unit *u, UnitDependency d, UnitDependency e, const char *name, const char *path, bool add_reference, UnitDependencyMask mask) { _cleanup_free_ char *buf = NULL; Unit *other; int r; @@ -2655,7 +2795,7 @@ int unit_add_two_dependencies_by_name(Unit *u, UnitDependency d, UnitDependency if (r < 0) return r; - return unit_add_two_dependencies(u, d, e, other, add_reference); + return unit_add_two_dependencies(u, d, e, other, add_reference, mask); } int set_unit_path(const char *p) { @@ -3365,7 +3505,7 @@ void unit_deserialize_skip(FILE *f) { } -int unit_add_node_link(Unit *u, const char *what, bool wants, UnitDependency dep) { +int unit_add_node_dependency(Unit *u, const char *what, bool wants, UnitDependency dep, UnitDependencyMask mask) { Unit *device; _cleanup_free_ char *e = NULL; int r; @@ -3397,12 +3537,12 @@ int unit_add_node_link(Unit *u, const char *what, bool wants, UnitDependency dep r = unit_add_two_dependencies(u, UNIT_AFTER, MANAGER_IS_SYSTEM(u->manager) ? dep : UNIT_WANTS, - device, true); + device, true, mask); if (r < 0) return r; if (wants) { - r = unit_add_dependency(device, UNIT_WANTS, u, false); + r = unit_add_dependency(device, UNIT_WANTS, u, false, mask); if (r < 0) return r; } @@ -4222,23 +4362,26 @@ int unit_kill_context( return wait_for_exit; } -int unit_require_mounts_for(Unit *u, const char *path) { +int unit_require_mounts_for(Unit *u, const char *path, UnitDependencyMask mask) { char prefix[strlen(path) + 1], *p; + UnitDependencyInfo di; int r; assert(u); assert(path); - /* Registers a unit for requiring a certain path and all its - * prefixes. We keep a simple array of these paths in the - * unit, since its usually short. However, we build a prefix - * table for all possible prefixes so that new appearing mount - * units can easily determine which units to make themselves a - * dependency of. */ + /* Registers a unit for requiring a certain path and all its prefixes. We keep a hashtable of these paths in + * the unit (from the path to the UnitDependencyInfo structure indicating how to the dependency came to + * be). However, we build a prefix table for all possible prefixes so that new appearing mount units can easily + * determine which units to make themselves a dependency of. */ if (!path_is_absolute(path)) return -EINVAL; + r = hashmap_ensure_allocated(&u->requires_mounts_for, &string_hash_ops); + if (r < 0) + return r; + p = strdup(path); if (!p) return -ENOMEM; @@ -4250,14 +4393,20 @@ int unit_require_mounts_for(Unit *u, const char *path) { return -EPERM; } - if (strv_contains(u->requires_mounts_for, p)) { + if (hashmap_contains(u->requires_mounts_for, p)) { free(p); return 0; } - r = strv_consume(&u->requires_mounts_for, p); - if (r < 0) + di = (UnitDependencyInfo) { + .origin_mask = mask + }; + + r = hashmap_put(u->requires_mounts_for, p, di.data); + if (r < 0) { + free(p); return r; + } PATH_FOREACH_PREFIX_MORE(prefix, p) { Set *x; @@ -4299,8 +4448,9 @@ int unit_require_mounts_for(Unit *u, const char *path) { int unit_setup_exec_runtime(Unit *u) { ExecRuntime **rt; size_t offset; - Iterator i; Unit *other; + Iterator i; + void *v; offset = UNIT_VTABLE(u)->exec_runtime_offset; assert(offset > 0); @@ -4311,7 +4461,7 @@ int unit_setup_exec_runtime(Unit *u) { return 0; /* Try to get it from somebody else */ - SET_FOREACH(other, u->dependencies[UNIT_JOINS_NAMESPACE_OF], i) { + HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_JOINS_NAMESPACE_OF], i) { *rt = unit_get_exec_runtime(other); if (*rt) { diff --git a/src/core/unit.h b/src/core/unit.h index 8c5c92ecd9..5b9dce3d0b 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -61,6 +61,53 @@ static inline bool UNIT_IS_INACTIVE_OR_FAILED(UnitActiveState t) { return IN_SET(t, UNIT_INACTIVE, UNIT_FAILED); } +/* Stores the 'reason' a dependency was created as a bit mask, i.e. due to which configuration source it came to be. We + * use this so that we can selectively flush out parts of dependencies again. Note that the same dependency might be + * created as a result of multiple "reasons", hence the bitmask. */ +typedef enum UnitDependencyMask { + /* Configured directly by the unit file, .wants/.requries symlink or drop-in, or as an immediate result of a + * non-dependency option configured that way. */ + UNIT_DEPENDENCY_FILE = 1 << 0, + + /* As unconditional implicit dependency (not affected by unit configuration — except by the unit name and + * type) */ + UNIT_DEPENDENCY_IMPLICIT = 1 << 1, + + /* A dependency effected by DefaultDependencies=yes. Note that dependencies marked this way are conceptually + * just a subset of UNIT_DEPENDENCY_FILE, as DefaultDependencies= is itself a unit file setting that can only + * be set in unit files. We make this two separate bits only to help debugging how dependencies came to be. */ + UNIT_DEPENDENCY_DEFAULT = 1 << 2, + + /* A dependency created from udev rules */ + UNIT_DEPENDENCY_UDEV = 1 << 3, + + /* A dependency created because of some unit's RequiresMountsFor= setting */ + UNIT_DEPENDENCY_PATH = 1 << 4, + + /* A dependency created because of data read from /proc/self/mountinfo and no other configuration source */ + UNIT_DEPENDENCY_MOUNTINFO_IMPLICIT = 1 << 5, + + /* A dependency created because of data read from /proc/self/mountinfo, but conditionalized by + * DefaultDependencies= and thus also involving configuration from UNIT_DEPENDENCY_FILE sources */ + UNIT_DEPENDENCY_MOUNTINFO_DEFAULT = 1 << 6, + + /* A dependency created because of data read from /proc/swaps and no other configuration source */ + UNIT_DEPENDENCY_PROC_SWAP = 1 << 7, + + _UNIT_DEPENDENCY_MASK_FULL = (1 << 8) - 1, +} UnitDependencyMask; + +/* The Unit's dependencies[] hashmaps use this structure as value. It has the same size as a void pointer, and thus can + * be stored directly as hashmap value, without any indirection. Note that this stores two masks, as both the origin + * and the destination of a dependency might have created it. */ +typedef union UnitDependencyInfo { + void *data; + struct { + UnitDependencyMask origin_mask:16; + UnitDependencyMask destination_mask:16; + } _packed_; +} UnitDependencyInfo; + #include "job.h" struct UnitRef { @@ -89,9 +136,13 @@ struct Unit { char *instance; Set *names; - Set *dependencies[_UNIT_DEPENDENCY_MAX]; - char **requires_mounts_for; + /* For each dependency type we maintain a Hashmap whose key is the Unit* object, and the value encodes why the + * dependency exists, using the UnitDependencyInfo type */ + Hashmap *dependencies[_UNIT_DEPENDENCY_MAX]; + + /* Similar, for RequiresMountsFor= path dependencies. The key is the path, the value the UnitDependencyInfo type */ + Hashmap *requires_mounts_for; char *description; char **documentation; @@ -492,7 +543,7 @@ extern const UnitVTable * const unit_vtable[_UNIT_TYPE_MAX]; #define UNIT_HAS_CGROUP_CONTEXT(u) (UNIT_VTABLE(u)->cgroup_context_offset > 0) #define UNIT_HAS_KILL_CONTEXT(u) (UNIT_VTABLE(u)->kill_context_offset > 0) -#define UNIT_TRIGGER(u) ((Unit*) set_first((u)->dependencies[UNIT_TRIGGERS])) +#define UNIT_TRIGGER(u) ((Unit*) hashmap_first_key((u)->dependencies[UNIT_TRIGGERS])) DEFINE_CAST(SERVICE, Service); DEFINE_CAST(SOCKET, Socket); @@ -512,11 +563,11 @@ void unit_free(Unit *u); int unit_new_for_name(Manager *m, size_t size, const char *name, Unit **ret); int unit_add_name(Unit *u, const char *name); -int unit_add_dependency(Unit *u, UnitDependency d, Unit *other, bool add_reference); -int unit_add_two_dependencies(Unit *u, UnitDependency d, UnitDependency e, Unit *other, bool add_reference); +int unit_add_dependency(Unit *u, UnitDependency d, Unit *other, bool add_reference, UnitDependencyMask mask); +int unit_add_two_dependencies(Unit *u, UnitDependency d, UnitDependency e, Unit *other, bool add_reference, UnitDependencyMask mask); -int unit_add_dependency_by_name(Unit *u, UnitDependency d, const char *name, const char *filename, bool add_reference); -int unit_add_two_dependencies_by_name(Unit *u, UnitDependency d, UnitDependency e, const char *name, const char *path, bool add_reference); +int unit_add_dependency_by_name(Unit *u, UnitDependency d, const char *name, const char *filename, bool add_reference, UnitDependencyMask mask); +int unit_add_two_dependencies_by_name(Unit *u, UnitDependency d, UnitDependency e, const char *name, const char *path, bool add_reference, UnitDependencyMask mask); int unit_add_exec_dependencies(Unit *u, ExecContext *c); @@ -596,7 +647,7 @@ int unit_serialize_item_escaped(Unit *u, FILE *f, const char *key, const char *v int unit_serialize_item_fd(Unit *u, FILE *f, FDSet *fds, const char *key, int fd); void unit_serialize_item_format(Unit *u, FILE *f, const char *key, const char *value, ...) _printf_(4,5); -int unit_add_node_link(Unit *u, const char *what, bool wants, UnitDependency d); +int unit_add_node_dependency(Unit *u, const char *what, bool wants, UnitDependency d, UnitDependencyMask mask); int unit_coldplug(Unit *u); @@ -651,7 +702,7 @@ int unit_kill_context(Unit *u, KillContext *c, KillOperation k, pid_t main_pid, int unit_make_transient(Unit *u); -int unit_require_mounts_for(Unit *u, const char *path); +int unit_require_mounts_for(Unit *u, const char *path, UnitDependencyMask mask); bool unit_type_supported(UnitType t); From 40b1a32ca854b5ce29029d358c924e1b18f4a1d0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 25 Oct 2017 21:29:24 +0200 Subject: [PATCH 07/17] device: rework device_is_bound_by_mounts() a bit Let's log when we can't parse the udev property, and always use the most precise, correct types. --- src/core/device.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/core/device.c b/src/core/device.c index 63933baf85..ad101a4197 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -285,19 +285,24 @@ static int device_add_udev_wants(Unit *u, struct udev_device *dev) { } } -static bool device_is_bound_by_mounts(Unit *d, struct udev_device *dev) { +static bool device_is_bound_by_mounts(Device *d, struct udev_device *dev) { const char *bound_by; - int r = false; + int r; assert(d); assert(dev); bound_by = udev_device_get_property_value(dev, "SYSTEMD_MOUNT_DEVICE_BOUND"); - if (bound_by) - r = parse_boolean(bound_by) > 0; + if (bound_by) { + r = parse_boolean(bound_by); + if (r < 0) + log_warning_errno(r, "Failed to parse SYSTEMD_MOUNT_DEVICE_BOUND='%s' udev property of %s, ignoring: %m", bound_by, strna(d->sysfs)); - DEVICE(d)->bind_mounts = r; - return r; + d->bind_mounts = r > 0; + } else + d->bind_mounts = false; + + return d->bind_mounts; } static int device_upgrade_mount_deps(Unit *u) { @@ -386,7 +391,7 @@ static int device_setup_unit(Manager *m, struct udev_device *dev, const char *pa /* So the user wants the mount units to be bound to the device but a mount unit might has been seen by systemd * before the device appears on its radar. In this case the device unit is partially initialized and includes * the deps on the mount unit but at that time the "bind mounts" flag wasn't not present. Fix this up now. */ - if (dev && device_is_bound_by_mounts(u, dev)) + if (dev && device_is_bound_by_mounts(DEVICE(u), dev)) device_upgrade_mount_deps(u); /* Note that this won't dispatch the load queue, the caller From 2651d037518bbcfab06d6d070b86fcf667942e97 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 26 Oct 2017 12:22:38 +0200 Subject: [PATCH 08/17] man: extend documentation on the unit name escaping logic --- man/systemd-escape.xml | 15 +++++++++----- man/systemd.unit.xml | 46 +++++++++++++++++++++++++----------------- 2 files changed, 38 insertions(+), 23 deletions(-) diff --git a/man/systemd-escape.xml b/man/systemd-escape.xml index fb20d2d94f..a2f2b9242f 100644 --- a/man/systemd-escape.xml +++ b/man/systemd-escape.xml @@ -73,6 +73,9 @@ special mode of escaping is applied instead, which assumes the string is already escaped but will escape everything that appears obviously non-escaped. + + For details on the escaping and unescaping algorithms see the relevant section in + systemd.unit5. @@ -107,11 +110,12 @@ - When escaping or unescaping a string, assume - it refers to a file system path. This eliminates leading, - trailing or duplicate / characters - and rejects . and .. - path components. + When escaping or unescaping a string, assume it refers to a file system path. This eliminates + leading, trailing or duplicate / characters and rejects . and + .. path components. This is particularly useful for generating strings suitable for + unescaping with the %f specifier in unit files, see + systemd.unit5. + @@ -172,6 +176,7 @@ systemd-nspawn@My\x20Container\x201.service systemd-nspawn@containerb.service sy See Also systemd1, + systemd.unit5, systemctl1 diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml index b9991bee76..d6feaa1817 100644 --- a/man/systemd.unit.xml +++ b/man/systemd.unit.xml @@ -213,22 +213,6 @@ socket-based activation which make dependencies implicit, resulting in a both simpler and more flexible system. - Some unit names reflect paths existing in the file system - namespace. Example: a device unit - dev-sda.device refers to a device with the - device node /dev/sda in the - file system namespace. If this applies, a special way to escape - the path name is used, so that the result is usable as part of a - filename. Basically, given a path, "/" is replaced by "-", and all - other characters which are not ASCII alphanumerics are replaced by - C-style "\x2d" escapes (except that "_" is never replaced and "." - is only replaced when it would be the first character in the - escaped path). The root directory "/" is encoded as single dash, - while otherwise the initial and ending "/" are removed from all - paths during transformation. This escaping is reversible. Properly - escaped paths can be generated using the - systemd-escape1 - command. Optionally, units may be instantiated from a template file at runtime. This allows creation of @@ -265,6 +249,32 @@ + + String Escaping for Inclusion in Unit Names + + Sometimes it is useful to convert arbitrary strings into unit names. To facilitate this, a method of string + escaping is used, in order to map strings containing arbitrary byte values (except NUL) into valid unit names and + their restricted character set. A common special case are unit names that reflect paths to objects in the file + system hierarchy. Example: a device unit dev-sda.device refers to a device with the device + node /dev/sda in the file system. + + The escaping algorithm operates as follows: given a string, any / character is replaced by + -, and all other characters which are not ASCII alphanumerics or _ are + replaced by C-style \x2d escapes. In addition, . is replaced with such a + C-style escape when it would appear as the first character in the escaped string. + + When the input qualifies as absolute file system path, this algorithm is extended slightly: the path to the + root directory / is encoded as single dash -. In addition, any leading, + trailing or duplicate / characters are removed from the string before transformation. Example: + /foo//bar/baz/ becomes foo-bar-baz. + + This escaping is fully reversible, as long as it is known whether the escaped string was a path (the + unescaping results are different for paths and non-path strings). The + systemd-escape1 command may be + used to apply and reverse escaping on arbitrary strings. Use systemd-escape --path to escape + path strings, and systemd-escape without otherwise. + + Implicit Dependencies @@ -1241,7 +1251,7 @@ %N Unescaped full unit name - Same as %n, but with escaping undone + Same as %n, but with escaping undone. This undoes the escaping used when generating unit names from arbitrary strings (see above). %p @@ -1266,7 +1276,7 @@ %f Unescaped filename - This is either the unescaped instance name (if applicable) with / prepended (if applicable), or the unescaped prefix name prepended with /. + This is either the unescaped instance name (if applicable) with / prepended (if applicable), or the unescaped prefix name prepended with /. This implements unescaping according to the rules for escaping absolute file system paths discussed above. %t From c999cf385aee08295dc204d98585cb4001d08ade Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 26 Oct 2017 16:39:35 +0200 Subject: [PATCH 09/17] core: add internal API to remove dependencies again, based on dependency mask let's make use of the dependency mask, and add internal API to remove dependencies ago, based on bits in the dependency mask. --- src/core/unit.c | 69 ++++++++++++++++++++++++++++++++++++++++++ src/core/unit.h | 2 ++ src/test/test-engine.c | 27 +++++++++++++++++ 3 files changed, 98 insertions(+) diff --git a/src/core/unit.c b/src/core/unit.c index 76902cca90..7d95f9db0b 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -4831,3 +4831,72 @@ int unit_fork_helper_process(Unit *u, pid_t *ret) { *ret = pid; return 1; } + +static void unit_update_dependency_mask(Unit *u, UnitDependency d, Unit *other, UnitDependencyInfo di) { + assert(u); + assert(d >= 0); + assert(d < _UNIT_DEPENDENCY_MAX); + assert(other); + + if (di.origin_mask == 0 && di.destination_mask == 0) { + /* No bit set anymore, let's drop the whole entry */ + assert_se(hashmap_remove(u->dependencies[d], other)); + log_unit_debug(u, "%s lost dependency %s=%s", u->id, unit_dependency_to_string(d), other->id); + } else + /* Mask was reduced, let's update the entry */ + assert_se(hashmap_update(u->dependencies[d], other, di.data) == 0); +} + +void unit_remove_dependencies(Unit *u, UnitDependencyMask mask) { + UnitDependency d; + + assert(u); + + /* Removes all dependencies u has on other units marked for ownership by 'mask'. */ + + if (mask == 0) + return; + + for (d = 0; d < _UNIT_DEPENDENCY_MAX; d++) { + bool done; + + do { + UnitDependencyInfo di; + Unit *other; + Iterator i; + + done = true; + + HASHMAP_FOREACH_KEY(di.data, other, u->dependencies[d], i) { + UnitDependency q; + + if ((di.origin_mask & ~mask) == di.origin_mask) + continue; + di.origin_mask &= ~mask; + unit_update_dependency_mask(u, d, other, di); + + /* We updated the dependency from our unit to the other unit now. But most dependencies + * imply a reverse dependency. Hence, let's delete that one too. For that we go through + * all dependency types on the other unit and delete all those which point to us and + * have the right mask set. */ + + for (q = 0; q < _UNIT_DEPENDENCY_MAX; q++) { + UnitDependencyInfo dj; + + dj.data = hashmap_get(other->dependencies[q], u); + if ((dj.destination_mask & ~mask) == dj.destination_mask) + continue; + dj.destination_mask &= ~mask; + + unit_update_dependency_mask(other, q, u, dj); + } + + unit_add_to_gc_queue(other); + + done = false; + break; + } + + } while (!done); + } +} diff --git a/src/core/unit.h b/src/core/unit.h index 5b9dce3d0b..5e5e791bc4 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -740,6 +740,8 @@ void unit_set_exec_params(Unit *s, ExecParameters *p); int unit_fork_helper_process(Unit *u, pid_t *ret); +void unit_remove_dependencies(Unit *u, UnitDependencyMask mask); + /* Macros which append UNIT= or USER_UNIT= to the message */ #define log_unit_full(unit, level, error, ...) \ diff --git a/src/test/test-engine.c b/src/test/test-engine.c index 55249fdce2..a2e68bf5d9 100644 --- a/src/test/test-engine.c +++ b/src/test/test-engine.c @@ -115,6 +115,33 @@ int main(int argc, char *argv[]) { assert_se(manager_add_job(m, JOB_START, h, JOB_FAIL, NULL, &j) == 0); manager_dump_jobs(m, stdout, "\t"); + assert_se(!hashmap_get(a->dependencies[UNIT_PROPAGATES_RELOAD_TO], b)); + assert_se(!hashmap_get(b->dependencies[UNIT_RELOAD_PROPAGATED_FROM], a)); + assert_se(!hashmap_get(a->dependencies[UNIT_PROPAGATES_RELOAD_TO], c)); + assert_se(!hashmap_get(c->dependencies[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(hashmap_get(a->dependencies[UNIT_PROPAGATES_RELOAD_TO], b)); + assert_se(hashmap_get(b->dependencies[UNIT_RELOAD_PROPAGATED_FROM], a)); + assert_se(hashmap_get(a->dependencies[UNIT_PROPAGATES_RELOAD_TO], c)); + assert_se(hashmap_get(c->dependencies[UNIT_RELOAD_PROPAGATED_FROM], a)); + + unit_remove_dependencies(a, UNIT_DEPENDENCY_UDEV); + + assert_se(!hashmap_get(a->dependencies[UNIT_PROPAGATES_RELOAD_TO], b)); + assert_se(!hashmap_get(b->dependencies[UNIT_RELOAD_PROPAGATED_FROM], a)); + assert_se(hashmap_get(a->dependencies[UNIT_PROPAGATES_RELOAD_TO], c)); + assert_se(hashmap_get(c->dependencies[UNIT_RELOAD_PROPAGATED_FROM], a)); + + unit_remove_dependencies(a, UNIT_DEPENDENCY_PROC_SWAP); + + assert_se(!hashmap_get(a->dependencies[UNIT_PROPAGATES_RELOAD_TO], b)); + assert_se(!hashmap_get(b->dependencies[UNIT_RELOAD_PROPAGATED_FROM], a)); + assert_se(!hashmap_get(a->dependencies[UNIT_PROPAGATES_RELOAD_TO], c)); + assert_se(!hashmap_get(c->dependencies[UNIT_RELOAD_PROPAGATED_FROM], a)); + manager_free(m); return 0; From de04054349713eb10757ad1de447f41941942447 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 26 Oct 2017 16:40:35 +0200 Subject: [PATCH 10/17] device: Let's simplify device_add_udev_wants() a bit Let's drop use of one variable and make the rest more explicit. --- src/core/device.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/core/device.c b/src/core/device.c index ad101a4197..565292605a 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -256,18 +256,22 @@ static int device_update_description(Unit *u, struct udev_device *dev, const cha } static int device_add_udev_wants(Unit *u, struct udev_device *dev) { - const char *wants, *property, *p; + const char *wants, *property; int r; assert(u); assert(dev); property = MANAGER_IS_USER(u->manager) ? "SYSTEMD_USER_WANTS" : "SYSTEMD_WANTS"; + wants = udev_device_get_property_value(dev, property); - for (p = wants;;) { + if (!wants) + return 0; + + for (;;) { _cleanup_free_ char *word = NULL, *k = NULL; - r = extract_first_word(&p, &word, NULL, EXTRACT_QUOTES); + r = extract_first_word(&wants, &word, NULL, EXTRACT_QUOTES); if (r == 0) return 0; if (r == -ENOMEM) @@ -281,7 +285,7 @@ static int device_add_udev_wants(Unit *u, struct udev_device *dev) { r = unit_add_dependency_by_name(u, UNIT_WANTS, k, NULL, true, UNIT_DEPENDENCY_UDEV); if (r < 0) - return log_unit_error_errno(u, r, "Failed to add wants dependency: %m"); + return log_unit_error_errno(u, r, "Failed to add Wants= dependency: %m"); } } From 38b9b72ec10fa8875b1b7c93bc7ca6b8a2d5486e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 26 Oct 2017 16:41:06 +0200 Subject: [PATCH 11/17] core: remove SYSTEMD_WANTS udev property configured dependencies at the right moment Previously dependencies configured with SYSTEMD_WANTS would be collected on a device unit as long as it was loaded. let's fix that, and remove dependencies again when SYTEMD_WANTS changes. --- src/core/device.c | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/src/core/device.c b/src/core/device.c index 565292605a..bfab494149 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -349,23 +349,26 @@ static int device_setup_unit(Manager *m, struct udev_device *dev, const char *pa return log_error_errno(r, "Failed to generate unit name from device path: %m"); u = manager_get_unit(m, e); + if (u) { + /* The device unit can still be present even if the device was unplugged: a mount unit can reference it hence + * preventing the GC to have garbaged it. That's desired since the device unit may have a dependency on the + * mount unit which was added during the loading of the later. */ + if (dev && DEVICE(u)->state == DEVICE_PLUGGED) { - /* The device unit can still be present even if the device was - * unplugged: a mount unit can reference it hence preventing - * the GC to have garbaged it. That's desired since the device - * unit may have a dependency on the mount unit which was - * added during the loading of the later. */ - if (dev && u && DEVICE(u)->state == DEVICE_PLUGGED) { - /* This unit is in plugged state: we're sure it's - * attached to a device. */ - if (!path_equal(DEVICE(u)->sysfs, sysfs)) { - log_unit_debug(u, "Dev %s appeared twice with different sysfs paths %s and %s", - e, DEVICE(u)->sysfs, sysfs); - return -EEXIST; + /* This unit is in plugged state: we're sure it's attached to a device. */ + if (!path_equal(DEVICE(u)->sysfs, sysfs)) { + log_unit_debug(u, "Dev %s appeared twice with different sysfs paths %s and %s", + e, DEVICE(u)->sysfs, sysfs); + return -EEXIST; + } } - } - if (!u) { + delete = false; + + /* Let's remove all dependencies generated due to udev properties. We'll readd whatever is configured + * now below. */ + unit_remove_dependencies(u, UNIT_DEPENDENCY_UDEV); + } else { delete = true; r = unit_new_for_name(m, sizeof(Device), e, &u); @@ -373,8 +376,7 @@ static int device_setup_unit(Manager *m, struct udev_device *dev, const char *pa goto fail; unit_add_to_load_queue(u); - } else - delete = false; + } /* If this was created via some dependency and has not * actually been seen yet ->sysfs will not be @@ -398,8 +400,7 @@ static int device_setup_unit(Manager *m, struct udev_device *dev, const char *pa if (dev && device_is_bound_by_mounts(DEVICE(u), dev)) device_upgrade_mount_deps(u); - /* Note that this won't dispatch the load queue, the caller - * has to do that if needed and appropriate */ + /* Note that this won't dispatch the load queue, the caller has to do that if needed and appropriate */ unit_add_to_dbus_queue(u); return 0; From 3ac62a0ee4db7a1f3653a0f2188ce4e1ad3360c7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 26 Oct 2017 16:43:31 +0200 Subject: [PATCH 12/17] test: add test case for adding/removing dependencies via udev rules --- test/TEST-17-UDEV-WANTS/Makefile | 4 ++ test/TEST-17-UDEV-WANTS/test.sh | 49 ++++++++++++++++++ test/TEST-17-UDEV-WANTS/testsuite.sh | 76 ++++++++++++++++++++++++++++ 3 files changed, 129 insertions(+) create mode 100644 test/TEST-17-UDEV-WANTS/Makefile create mode 100755 test/TEST-17-UDEV-WANTS/test.sh create mode 100755 test/TEST-17-UDEV-WANTS/testsuite.sh diff --git a/test/TEST-17-UDEV-WANTS/Makefile b/test/TEST-17-UDEV-WANTS/Makefile new file mode 100644 index 0000000000..b895de8bcb --- /dev/null +++ b/test/TEST-17-UDEV-WANTS/Makefile @@ -0,0 +1,4 @@ +include ../Makefile.guess + +all setup clean run: + @basedir=../.. TEST_BASE_DIR=../ BUILD_DIR=$(BUILD_DIR) ./test.sh --$@ diff --git a/test/TEST-17-UDEV-WANTS/test.sh b/test/TEST-17-UDEV-WANTS/test.sh new file mode 100755 index 0000000000..7beef7f398 --- /dev/null +++ b/test/TEST-17-UDEV-WANTS/test.sh @@ -0,0 +1,49 @@ +#!/bin/bash +# -*- mode: shell-script; indent-tabs-mode: nil; sh-basic-offset: 4; -*- +# ex: ts=8 sw=4 sts=4 et filetype=sh +set -e +TEST_DESCRIPTION="UDEV SYSTEMD_WANTS property" +TEST_NO_NSPAWN=1 + +. $TEST_BASE_DIR/test-functions +QEMU_TIMEOUT=180 + +test_setup() { + create_empty_image + mkdir -p $TESTDIR/root + mount ${LOOPDEV}p1 $TESTDIR/root + + ( + LOG_LEVEL=5 + eval $(udevadm info --export --query=env --name=${LOOPDEV}p2) + + setup_basic_environment + + # 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 + + # setup the testsuite service + cat >$initdir/etc/systemd/system/testsuite.service < /run/udev/rules.d/50-testsuite.rules < /run/udev/rules.d/50-testsuite.rules < /testok + +exit 0 From dcebc9bae4dcc3e844f01558c6127fc0d8745c8e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 26 Oct 2017 17:12:44 +0200 Subject: [PATCH 13/17] core: when a unit template is specified in SYSTEMD_WANTS=, instantiate it with sysfs path This should make cases like the user's setup in #7109 a lot easier to handle, as in that case we'll do the right escaping automatically. --- man/systemd.device.xml | 65 +++++++++++++++++++----------------------- src/core/device.c | 25 +++++++++++++--- 2 files changed, 51 insertions(+), 39 deletions(-) diff --git a/man/systemd.device.xml b/man/systemd.device.xml index 6edf1090d0..e121e151bc 100644 --- a/man/systemd.device.xml +++ b/man/systemd.device.xml @@ -112,35 +112,36 @@ The udev Database - The settings of device units may either be configured via - unit files, or directly from the udev database (which is - recommended). The following udev device properties are understood - by systemd: + Unit settings of device units may either be configured via unit files, or directly from the udev + database. The following udev device properties are understood by the service manager: SYSTEMD_WANTS= SYSTEMD_USER_WANTS= - Adds dependencies of type - Wants from the device unit to all listed - units. The first form is used by the system systemd instance, - the second by user systemd instances. Those settings may be - used to activate arbitrary units when a specific device - becomes available. + Adds dependencies of type Wants= from the device unit to the specified + units. SYSTEMD_WANTS= is read by the system service manager, + SYSTEMD_USER_WANTS= by user service manager instances. These properties may be used to + activate arbitrary units when a specific device becomes available. - Note that this and the other tags are not taken into - account unless the device is tagged with the - systemd string in the udev database, - because otherwise the device is not exposed as a systemd unit - (see above). + Note that this and the other udev device properties are not taken into account unless the device is + tagged with the systemd tag in the udev database, because otherwise the device is not + exposed as a systemd unit (see above). - Note that systemd will only act on - Wants dependencies when a device first - becomes active. It will not act on them if they are added to - devices that are already active. Use - SYSTEMD_READY= (see below) to influence on - which udev event to trigger the dependencies. - + Note that systemd will only act on Wants= dependencies when a device first becomes + active. It will not act on them if they are added to devices that are already active. Use + SYSTEMD_READY= (see below) to configure when a udev device shall be considered active, and + thus when to trigger the dependencies. + + + + The specified property value should be a space-separated list of valid unit names. If a unit template + name is specified (that is, a unit name containing an @ character indicating a unit name to + use for multiple instantiation, but with an empty instance name following the @), it will be + automatically instantiated by the device's sysfs path (that is: the path is escaped and + inserted as instance name into the template unit name). This is useful in order to instantiate a specific + template unit once for each device that appears and matches specific properties. @@ -152,20 +153,14 @@ SYSTEMD_READY= - If set to 0, systemd will consider this device - unplugged even if it shows up in the udev tree. If this - property is unset or set to 1, the device will be considered - plugged if it is visible in the udev tree. This property has - no influence on the behavior when a device disappears from the - udev tree. + If set to 0, systemd will consider this device unplugged even if it shows up in the udev + tree. If this property is unset or set to 1, the device will be considered plugged if it is visible in the udev + tree. - This option is useful to support devices that initially - show up in an uninitialized state in the tree, and for which a - changed event is generated the moment they - are fully set up. Note that SYSTEMD_WANTS= - (see above) is not acted on as long as - SYSTEMD_READY=0 is set for a - device. + This option is useful for devices that initially show up in an uninitialized state in the tree, and for + which a changed event is generated the moment they are fully set up. Note that + SYSTEMD_WANTS= (see above) is not acted on as long as SYSTEMD_READY=0 is + set for a device. diff --git a/src/core/device.c b/src/core/device.c index bfab494149..a0150751cd 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -277,11 +277,28 @@ static int device_add_udev_wants(Unit *u, struct udev_device *dev) { if (r == -ENOMEM) return log_oom(); if (r < 0) - return log_unit_error_errno(u, r, "Failed to add parse %s: %m", property); + return log_unit_error_errno(u, r, "Failed to parse property %s with value %s: %m", property, wants); - r = unit_name_mangle(word, UNIT_NAME_NOGLOB, &k); - if (r < 0) - return log_unit_error_errno(u, r, "Failed to mangle unit name \"%s\": %m", word); + if (unit_name_is_valid(word, UNIT_NAME_TEMPLATE) && DEVICE(u)->sysfs) { + _cleanup_free_ char *escaped = NULL; + + /* If the unit name is specified as template, then automatically fill in the sysfs path of the + * device as instance name, properly escaped. */ + + r = unit_name_path_escape(DEVICE(u)->sysfs, &escaped); + if (r < 0) + return log_unit_error_errno(u, r, "Failed to escape %s: %m", DEVICE(u)->sysfs); + + r = unit_name_replace_instance(word, escaped, &k); + if (r < 0) + return log_unit_error_errno(u, r, "Failed to build %s instance of template %s: %m", escaped, word); + } else { + /* If this is not a template, then let's mangle it so, that it becomes a valid unit name. */ + + r = unit_name_mangle(word, UNIT_NAME_NOGLOB, &k); + if (r < 0) + return log_unit_error_errno(u, r, "Failed to mangle unit name \"%s\": %m", word); + } r = unit_add_dependency_by_name(u, UNIT_WANTS, k, NULL, true, UNIT_DEPENDENCY_UDEV); if (r < 0) From 3e3852b3c6c61506963112fd218a86b673fc61e6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 26 Oct 2017 17:24:55 +0200 Subject: [PATCH 14/17] core: make "tmpfs" dependencies on swapfs a "default" dep, not an "implicit" There should be a way to turn this logic of, and DefaultDependencies= appears to be the right option for that, hence let's downgrade this dependency type from "implicit" to "default, and thus honour DefaultDependencies=. This also drops mount_get_fstype() as we only have a single user needing this now. A follow-up for #7076. --- src/core/dbus-mount.c | 8 +++++++- src/core/mount.c | 31 +++++++------------------------ src/core/mount.h | 2 -- units/tmp.mount | 1 + 4 files changed, 15 insertions(+), 27 deletions(-) diff --git a/src/core/dbus-mount.c b/src/core/dbus-mount.c index 1f9c254c39..c683b36c4c 100644 --- a/src/core/dbus-mount.c +++ b/src/core/dbus-mount.c @@ -87,13 +87,19 @@ static int property_get_type( void *userdata, sd_bus_error *error) { + const char *fstype = NULL; Mount *m = userdata; assert(bus); assert(reply); assert(m); - return sd_bus_message_append(reply, "s", mount_get_fstype(m)); + if (m->from_proc_self_mountinfo && m->parameters_proc_self_mountinfo.fstype) + fstype = m->parameters_proc_self_mountinfo.fstype; + else if (m->from_fragment && m->parameters_fragment.fstype) + fstype = m->parameters_fragment.fstype; + + return sd_bus_message_append(reply, "s", fstype); } static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_result, mount_result, MountResult); diff --git a/src/core/mount.c b/src/core/mount.c index 343e17fa10..214b46f670 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -156,21 +156,6 @@ static bool needs_quota(const MountParameters *p) { "usrquota\0" "grpquota\0" "quota\0" "usrjquota\0" "grpjquota\0"); } -const char *mount_get_fstype(const Mount *m) { - const char *type = NULL; - - assert(m); - - if (m->from_proc_self_mountinfo && m->parameters_proc_self_mountinfo.fstype) - type = m->parameters_proc_self_mountinfo.fstype; - else if (m->from_fragment && m->parameters_fragment.fstype) - type = m->parameters_fragment.fstype; - else - type = ""; - - return type; -} - static void mount_init(Unit *u) { Mount *m = MOUNT(u); @@ -281,7 +266,6 @@ _pure_ static MountParameters* get_mount_parameters(Mount *m) { } static int mount_add_mount_dependencies(Mount *m) { - const char *fstype; MountParameters *pm; Unit *other; Iterator i; @@ -338,14 +322,6 @@ static int mount_add_mount_dependencies(Mount *m) { } } - /* If this is a tmpfs mount then we have to unmount it before we try to deactivate swaps */ - fstype = mount_get_fstype(m); - if (streq(fstype, "tmpfs")) { - r = unit_add_dependency_by_name(UNIT(m), UNIT_AFTER, SPECIAL_SWAP_TARGET, NULL, true, UNIT_DEPENDENCY_IMPLICIT); - if (r < 0) - return r; - } - return 0; } @@ -517,6 +493,13 @@ static int mount_add_default_dependencies(Mount *m) { if (r < 0) return r; + /* If this is a tmpfs mount then we have to unmount it before we try to deactivate swaps */ + if (streq(p->fstype, "tmpfs")) { + r = unit_add_dependency_by_name(UNIT(m), UNIT_AFTER, SPECIAL_SWAP_TARGET, NULL, true, mask); + if (r < 0) + return r; + } + return 0; } diff --git a/src/core/mount.h b/src/core/mount.h index f37094e39f..f81e4217df 100644 --- a/src/core/mount.h +++ b/src/core/mount.h @@ -110,5 +110,3 @@ MountExecCommand mount_exec_command_from_string(const char *s) _pure_; const char* mount_result_to_string(MountResult i) _const_; MountResult mount_result_from_string(const char *s) _pure_; - -const char *mount_get_fstype(const Mount *m); diff --git a/units/tmp.mount b/units/tmp.mount index a057fa1cf9..3a333d22ec 100644 --- a/units/tmp.mount +++ b/units/tmp.mount @@ -13,6 +13,7 @@ ConditionPathIsSymbolicLink=!/tmp DefaultDependencies=no Conflicts=umount.target Before=local-fs.target umount.target +After=swap.target [Mount] What=tmpfs From ecd48322c2fd11ccdeed971c45c611f001d0d7b8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 26 Oct 2017 17:26:27 +0200 Subject: [PATCH 15/17] core: sd-bus can handle NULL strings nicely, let's use it No need to set an empty string here, sd-bus serializes NULL as empty string anway. --- src/core/dbus-mount.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/core/dbus-mount.c b/src/core/dbus-mount.c index c683b36c4c..69a04fe2ae 100644 --- a/src/core/dbus-mount.c +++ b/src/core/dbus-mount.c @@ -36,7 +36,7 @@ static int property_get_what( sd_bus_error *error) { Mount *m = userdata; - const char *d; + const char *d = NULL; assert(bus); assert(reply); @@ -46,8 +46,6 @@ static int property_get_what( d = m->parameters_proc_self_mountinfo.what; else if (m->from_fragment && m->parameters_fragment.what) d = m->parameters_fragment.what; - else - d = ""; return sd_bus_message_append(reply, "s", d); } @@ -62,7 +60,7 @@ static int property_get_options( sd_bus_error *error) { Mount *m = userdata; - const char *d; + const char *d = NULL; assert(bus); assert(reply); @@ -72,8 +70,6 @@ static int property_get_options( d = m->parameters_proc_self_mountinfo.options; else if (m->from_fragment && m->parameters_fragment.options) d = m->parameters_fragment.options; - else - d = ""; return sd_bus_message_append(reply, "s", d); } From 17b6f896b49a4c6044f5e803e19d3a427f7f615d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 24 Oct 2017 12:18:17 +0200 Subject: [PATCH 16/17] update TODO --- TODO | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/TODO b/TODO index 4cf7eec7cd..63b8635a01 100644 --- a/TODO +++ b/TODO @@ -24,6 +24,48 @@ Janitorial Clean-ups: Features: +* let's log the "tainted" string at boot + +* Add NetworkNamespacePath= to specify a path to a network namespace + +* Add StandardInputData= and StandardInputText= for putting together data to + pass to a service through stdin + +* Add StandardInputPath=, StandardOutputPath=, StandardErrorPath= to connect a + service to a specific file. Be smart, and if the specified path refers to an + AF_UNIX socket, connect() to it. + +* maybe use SOURCE_DATE_EPOCH (i.e. the env var the reproducible builds folks + introduced) as the RTC epoch, instead of the mtime of NEWS. + +* Introduce GCMode= as unit file property or so, for tweaking the GC + logic. Specifically, there should be a way to tell systemd to collect + individual units even on failure. Then, make systemd-run --wait use this, so + that failed transient units in that case don't stick around. + +* add a way to lock down cgroup migration: a boolean, which when set for a unit + makes sure the processes in it can never migrate out of it + +* complain if a unit starts up and there are already processes in its cgroup + +* blog about fd store and restartable services + +* document Environment=SYSTEMD_LOG_LEVEL=debug drop-in in debugging document + +* add a way to remove fds from the fdstore by name, and make logind use it + +* in the long run: permit a system with /etc/machine-id linked to /dev/null, to + make it lose its identity, i.e. be anonymous. For this we'd have to patch + through the whole tree to make all code deal with the case where no machine + ID is available. + +* optionally, collect cgroup resource data, and store it in per-unit RRD files, + suitable for processing with rrdtool. Add bus API to access this data, and + possibly implement a CPULoad property based on it. + +* In journalctl add a way how "-o verbose" and suchlike can be tweaked to show + only a specific set of properties + * export UID ranges nspawns's --private-user and DynamicUser= uses in the systemd.pc pkg-config file, the same way we already expose the system user boundary there From 74b1731c756f2c8c9eb9577465250eb22f697506 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 12 Nov 2017 14:25:58 +0100 Subject: [PATCH 17/17] core/mount: fstype may be NULL --- src/core/mount.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/mount.c b/src/core/mount.c index 214b46f670..e2c480a51d 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -494,7 +494,7 @@ static int mount_add_default_dependencies(Mount *m) { return r; /* If this is a tmpfs mount then we have to unmount it before we try to deactivate swaps */ - if (streq(p->fstype, "tmpfs")) { + if (streq_ptr(p->fstype, "tmpfs")) { r = unit_add_dependency_by_name(UNIT(m), UNIT_AFTER, SPECIAL_SWAP_TARGET, NULL, true, mask); if (r < 0) return r;