From 033fe6506af5881db77c5ec1bb1f135bd6093406 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 28 Jun 2022 06:20:19 +0900 Subject: [PATCH 1/4] core/device: start units specified in SYSTEMD_WANTS if it is not running Otherwise, e.g. --- KERNEL=="foo", ACTION!="remove", ENV{SYSTEMD_WANTS}+="test.service" --- then the service will not triggered on CHANGE uevents for the device even if the service is already stopped. --- src/core/device.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/core/device.c b/src/core/device.c index fcde8a420e..90d6324d96 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -457,18 +457,24 @@ static int device_add_udev_wants(Unit *u, sd_device *dev) { if (d->state != DEVICE_DEAD) /* So here's a special hack, to compensate for the fact that the udev database's reload cycles are not * synchronized with our own reload cycles: when we detect that the SYSTEMD_WANTS property of a device - * changes while the device unit is already up, let's manually trigger any new units listed in it not - * seen before. This typically happens during the boot-time switch root transition, as udev devices - * will generally already be up in the initrd, but SYSTEMD_WANTS properties get then added through udev - * rules only available on the host system, and thus only when the initial udev coldplug trigger runs. + * changes while the device unit is already up, let's skip to trigger units that were already listed + * and are active, and start units otherwise. This typically happens during the boot-time switch root + * transition, as udev devices will generally already be up in the initrd, but SYSTEMD_WANTS properties + * get then added through udev rules only available on the host system, and thus only when the initial + * udev coldplug trigger runs. * * We do this only if the device has been up already when we parse this, as otherwise the usual * dependency logic that is run from the dead → plugged transition will trigger these deps. */ STRV_FOREACH(i, added) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - if (strv_contains(d->wants_property, *i)) /* Was this unit already listed before? */ - continue; + if (strv_contains(d->wants_property, *i)) { + Unit *v; + + v = manager_get_unit(u->manager, *i); + if (v && UNIT_IS_ACTIVE_OR_RELOADING(unit_active_state(v))) + continue; /* The unit was already listed and is running. */ + } r = manager_add_job_by_name(u->manager, JOB_START, *i, JOB_FAIL, NULL, &error, NULL); if (r < 0) From 156ba52b43f4ce795fbc1662f85e2fb4a45dd00a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 28 Jun 2022 06:11:53 +0900 Subject: [PATCH 2/4] core/unit: try to submit stop_when_unneeded queue on removing dependencies Fixes #23410. --- src/core/unit.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/core/unit.c b/src/core/unit.c index 180dccc2b2..d933bfc0cc 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -5165,6 +5165,9 @@ void unit_remove_dependencies(Unit *u, UnitDependencyMask mask) { unit_add_to_gc_queue(other); + /* The unit 'other' may not be wanted by the unit 'u'. */ + unit_submit_to_stop_when_unneeded_queue(other); + done = false; break; } From 386427cfcfbec2b6ee62c97689263be53822cfcf Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 28 Jun 2022 06:31:49 +0900 Subject: [PATCH 3/4] test: disable echo earlier --- test/units/assert.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/units/assert.sh b/test/units/assert.sh index 66357ab688..2f4d93ab8c 100644 --- a/test/units/assert.sh +++ b/test/units/assert.sh @@ -4,10 +4,10 @@ # utility functions for shell tests assert_true() {( - local rc - set +ex + local rc + "$@" rc=$? if [[ $rc -ne 0 ]]; then @@ -47,10 +47,10 @@ assert_not_in() {( )} assert_rc() {( - local rc exp="${1?}" - set +ex + local rc exp="${1?}" + shift "$@" rc=$? From 9fd1f8eea9dfea920274ebbf7349c02e9268a18a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 28 Jun 2022 06:33:38 +0900 Subject: [PATCH 4/4] test: add udev tests for SYSTEMD_WANTS For issue #23410. --- test/units/testsuite-17.07.sh | 205 ++++++++++++++++++++++++++++++++++ 1 file changed, 205 insertions(+) create mode 100755 test/units/testsuite-17.07.sh diff --git a/test/units/testsuite-17.07.sh b/test/units/testsuite-17.07.sh new file mode 100755 index 0000000000..549107af10 --- /dev/null +++ b/test/units/testsuite-17.07.sh @@ -0,0 +1,205 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: LGPL-2.1-or-later +set -ex +set -o pipefail + +# shellcheck source=test/units/assert.sh +. "$(dirname "$0")"/assert.sh + +wait_service_active() {( + set +ex + for (( i = 0; i < 20; i++ )); do + if (( i != 0 )); then sleep 0.5; fi + if systemctl --quiet is-active "${1?}"; then + return 0 + fi + done + return 1 +)} + +wait_service_inactive() {( + set +ex + for (( i = 0; i < 20; i++ )); do + if (( i != 0 )); then sleep 0.5; fi + systemctl --quiet is-active "${1?}" + if [[ "$?" == "3" ]]; then + return 0 + fi + done + return 1 +)} + +mkdir -p /run/systemd/system +cat >/run/systemd/system/both.service </run/systemd/system/on-add.service </run/systemd/system/on-change.service </run/udev/rules.d/50-testsuite.rules </run/systemd/system/both.service </run/systemd/system/on-add.service </run/systemd/system/on-change.service <