PR#24: Backport core/device: ignore ID_PROCESSING udev property on enumerate - rpms/systemd

rpms / systemd

#24 Backport core/device: ignore ID_PROCESSING udev property on enumerate

Merged 2 days ago by ryantimwilson. Opened 2 days ago by ryantimwilson.

rpms/ ryantimwilson/systemd udev-fix-upstream into c10s-sig-hyperscale

Backport core/device: ignore ID_PROCESSING udev property on enumerate

Ryan Wilson • 2 days ago

10c3e1

0001-pid1-Do-not-update-state-for-device-units-being-proc.patch

file removed

-97

		`@@ -1,97 +0,0 @@`
		`- From 3a1ea280dd3c8c70b616467f4fd375cf2e4d31fa Mon Sep 17 00:00:00 2001`
		`- From: Ryan Wilson <ryantimwilson@meta4.com>`
		`- Date: Sat, 23 Nov 2024 20:30:31 -0800`
		`- Subject: [PATCH] pid1: Do not update state for device units being processed by`
		`- udev`
		`-`
		`- ---`
		`- src/core/device.c \| 40 ++++++++++++++++++++++++++++++++++++----`
		`- src/core/device.h \| 1 +`
		`- 2 files changed, 37 insertions(+), 4 deletions(-)`
		`-`
		`- diff --git a/src/core/device.c b/src/core/device.c`
		`- index d8567676a7..d13140759d 100644`
		`- --- a/src/core/device.c`
		`- +++ b/src/core/device.c`
		`- @@ -334,6 +334,11 @@ static int device_coldplug(Unit *u) {`
		`- static void device_catchup(Unit *u) {`
		`- Device *d = ASSERT_PTR(DEVICE(u));`
		`-`
		`- + if (d->enumerated_udev_processing) {`
		`- + log_unit_debug(u, "Device is being processed by systemd-udevd. Skipping updating state.");`
		`- + return;`
		`- + }`
		`- +`
		`- /* Second, let's update the state with the enumerated state */`
		`- device_update_found_one(d, d->enumerated_found, DEVICE_FOUND_MASK);`
		`- }`
		`- @@ -742,6 +747,14 @@ static bool device_is_ready(sd_device *dev) {`
		`-`
		`- assert(dev);`
		`-`
		`- + r = device_is_processed(dev);`
		`- + if (r < 0)`
		`- + log_device_warning_errno(dev, r, "Failed to check if device is processed, assuming device is processed: %m");`
		`- + if (r == 0) {`
		`- + log_device_debug(dev, "Device busy: device is currently being processed");`
		`- + return false;`
		`- + }`
		`- +`
		`- if (device_for_action(dev, SD_DEVICE_REMOVE))`
		`- return false;`
		`-`
		`- @@ -1051,16 +1064,35 @@ static void device_enumerate(Manager *m) {`
		`- _cleanup_set_free_ Set ready_units = NULL, not_ready_units = NULL;`
		`- Device *d;`
		`-`
		`- - if (device_is_processed(dev) <= 0)`
		`- + r = device_setup_units(m, dev, &ready_units, &not_ready_units);`
		`- + if (r < 0) {`
		`- + log_device_debug_errno(dev, r, "Failed to set up device units: %m");`
		`- continue;`
		`- + }`
		`-`
		`- - if (device_setup_units(m, dev, &ready_units, &not_ready_units) < 0)`
		`- + r = device_is_processed(dev);`
		`- + if (r < 0)`
		`- + log_device_warning_errno(dev, r, "Failed to determine if device is processed by systemd-udevd, assuming device is processed: %m");`
		`- + if (r == 0) {`
		`- + SET_FOREACH(d, ready_units) {`
		`- + log_unit_warning(UNIT(d), "Device unit is ready but currently processing in systemd-udevd.");`
		`- + d->enumerated_udev_processing = true;`
		`- + }`
		`- + SET_FOREACH(d, not_ready_units) {`
		`- + log_unit_debug(UNIT(d), "Device unit currently processing in systemd-udevd.");`
		`- + d->enumerated_udev_processing = true;`
		`- + }`
		`- continue;`
		`- + }`
		`-`
		`- - SET_FOREACH(d, ready_units)`
		`- + SET_FOREACH(d, ready_units) {`
		`- + log_unit_debug(UNIT(d), "Device unit found in systemd-udevd.");`
		`- device_update_found_one(d, DEVICE_FOUND_UDEV, DEVICE_FOUND_UDEV);`
		`- - SET_FOREACH(d, not_ready_units)`
		`- + }`
		`- + SET_FOREACH(d, not_ready_units) {`
		`- + log_unit_debug(UNIT(d), "Device unit not found in systemd-udevd.");`
		`- device_update_found_one(d, DEVICE_NOT_FOUND, DEVICE_FOUND_UDEV);`
		`- + }`
		`- }`
		`-`
		`- return;`
		`- diff --git a/src/core/device.h b/src/core/device.h`
		`- index 9dd6fb57c2..14d5547c7a 100644`
		`- --- a/src/core/device.h`
		`- +++ b/src/core/device.h`
		`- @@ -31,6 +31,7 @@ struct Device {`
		`- DeviceFound found, deserialized_found, enumerated_found;`
		`-`
		`- bool bind_mounts;`
		`- + bool enumerated_udev_processing;`
		`-`
		`- /* The SYSTEMD_WANTS udev property for this device the last time we saw it */`
		`- char **wants_property;`
		`- --`
		`- 2.43.5`
		`-`

35332.patch

file added

+117

		`@@ -0,0 +1,117 @@`
		`+ From ca0cef28691be7ea22274c2fa7d95a2725da1ba5 Mon Sep 17 00:00:00 2001`
		`+ From: Yu Watanabe <watanabe.yu+github@gmail.com>`
		`+ Date: Sun, 24 Nov 2024 13:19:27 +0900`
		`+ Subject: [PATCH 1/2] core/device: ignore ID_PROCESSING udev property on`
		`+ enumerate`
		`+`
		`+ This partially reverts the commit 405be62f05d76f1845f347737b5972158c79dd3e`
		`+ "tree-wide: refuse enumerated device with ID_PROCESSING=1".`
		`+`
		`+ Otherwise, when systemd-udev-trigger.service is started just before`
		`+ daemon-reexec, which can be easily happen on update, then udev database`
		`+ files for many devices may have ID_PROCESSING=1 property, thus devices may`
		`+ not enumerated on daemon-reexec. That causes many units especially mount`
		`+ units deactivated after daemon-reexec.`
		`+`
		`+ Fixes #35329.`
		`+ ---`
		`+ src/core/device.c \| 3 ---`
		`+ 1 file changed, 3 deletions(-)`
		`+`
		`+ diff --git a/src/core/device.c b/src/core/device.c`
		`+ index 03a730f6240c1..a8921e91c344e 100644`
		`+ --- a/src/core/device.c`
		`+ +++ b/src/core/device.c`
		`+ @@ -1048,9 +1048,6 @@ static void device_enumerate(Manager *m) {`
		`+ _cleanup_set_free_ Set ready_units = NULL, not_ready_units = NULL;`
		`+ Device *d;`
		`+`
		`+ - if (device_is_processed(dev) <= 0)`
		`+ - continue;`
		`+ -`
		`+ if (device_setup_units(m, dev, &ready_units, &not_ready_units) < 0)`
		`+ continue;`
		`+`
		`+`
		`+ From 388151b72103c10d63bac1e845d6a151619990ae Mon Sep 17 00:00:00 2001`
		`+ From: Yu Watanabe <watanabe.yu+github@gmail.com>`
		`+ Date: Sun, 24 Nov 2024 13:57:37 +0900`
		`+ Subject: [PATCH 2/2] test: add reproducer for issue #35329`
		`+`
		`+ Without the previous commit, the test case will fail.`
		`+ ---`
		`+ .../TEST-17-UDEV.device_is_processing.sh \| 65 +++++++++++++++++++`
		`+ 1 file changed, 65 insertions(+)`
		`+ create mode 100755 test/units/TEST-17-UDEV.device_is_processing.sh`
		`+`
		`+ diff --git a/test/units/TEST-17-UDEV.device_is_processing.sh b/test/units/TEST-17-UDEV.device_is_processing.sh`
		`+ new file mode 100755`
		`+ index 0000000000000..9b39b43238e44`
		`+ --- /dev/null`
		`+ +++ b/test/units/TEST-17-UDEV.device_is_processing.sh`
		`+ @@ -0,0 +1,65 @@`
		`+ +#!/usr/bin/env bash`
		`+ +# SPDX-License-Identifier: LGPL-2.1-or-later`
		`+ +set -ex`
		`+ +set -o pipefail`
		`+ +`
		`+ +# This is a reproducer of issue #35329,`
		`+ +# which is a regression caused by 405be62f05d76f1845f347737b5972158c79dd3e.`
		`+ +`
		`+ +IFNAME=udevtestnetif`
		`+ +`
		`+ +udevadm settle`
		`+ +`
		`+ +mkdir -p /run/udev/udev.conf.d/`
		`+ +cat >/run/udev/udev.conf.d/timeout.conf <<EOF`
		`+ +event_timeout=1h`
		`+ +EOF`
		`+ +`
		`+ +mkdir -p /run/udev/rules.d/`
		`+ +cat >/run/udev/rules.d/99-testsuite.rules <<EOF`
		`+ +SUBSYSTEM=="net", ACTION=="change", KERNEL=="${IFNAME}", OPTIONS="log_level=debug", RUN+="/usr/bin/sleep 1000"`
		`+ +EOF`
		`+ +`
		`+ +systemctl restart systemd-udevd.service`
		`+ +`
		`+ +ip link add "$IFNAME" type dummy`
		`+ +IFINDEX=$(ip -json link show "$IFNAME" \| jq '.[].ifindex')`
		`+ +udevadm wait --timeout 10 "/sys/class/net/${IFNAME}"`
		`+ +# Check if the database file is created.`
		`+ +[[ -e "/run/udev/data/n${IFINDEX}" ]]`
		`+ +`
		`+ +systemd-run \`
		`+ + -p After="sys-subsystem-net-devices-${IFNAME}.device" \`
		`+ + -p BindsTo="sys-subsystem-net-devices-${IFNAME}.device" \`
		`+ + -u testsleep.service \`
		`+ + sleep 1h`
		`+ +`
		`+ +timeout 10 bash -c 'until systemctl is-active testsleep.service; do sleep .5; done'`
		`+ +`
		`+ +udevadm trigger "/sys/class/net/${IFNAME}"`
		`+ +timeout 30 bash -c "until grep -F 'ID_PROCESSING=1' /run/udev/data/n${IFINDEX}; do sleep .5; done"`
		`+ +`
		`+ +for _ in {1..3}; do`
		`+ + systemctl daemon-reexec`
		`+ + systemctl is-active testsleep.service`
		`+ +done`
		`+ +`
		`+ +for _ in {1..3}; do`
		`+ + systemctl daemon-reload`
		`+ + systemctl is-active testsleep.service`
		`+ +done`
		`+ +`
		`+ +# Check if the reexec and reload have finished during processing the event.`
		`+ +grep -F 'ID_PROCESSING=1' "/run/udev/data/n${IFINDEX}"`
		`+ +`
		`+ +# cleanup`
		`+ +systemctl stop testsleep.service`
		`+ +rm -f /run/udev/udev.conf.d/timeout.conf`
		`+ +rm -f /run/udev/rules.d/99-testsuite.rules`
		`+ +# Forcibly kills sleep command invoked by the udev rule before restarting,`
		`+ +# otherwise systemctl restart below will takes longer.`
		`+ +killall -KILL sleep`
		`+ +systemctl restart systemd-udevd.service`
		`+ +ip link del "$IFNAME"`
		`+ +`
		`+ +exit 0`

systemd.spec

file modified

+3 -3

		`@@ -129,6 +129,9 @@`
		`# Soft-disable tmpfiles --purge until a good use case comes up.`
		`Patch0492: 0001-tmpfiles-make-purge-hard-to-mis-use.patch`

		`+ # core/device: ignore ID_PROCESSING udev property on enumerate`
		`+ Patch0493: https://github.com/systemd/systemd/pull/35332.patch`
		`+`
		`# Meta specific backports (900-1000)`

		`%if 0%{?facebook}`
		`@@ -151,9 +154,6 @@`
		`# pam_systemd: Make pam_systemd 256 backwards compatible to logind 255`
		`Patch0905: 0001-pam_systemd-Make-pam_systemd-256-backwards-compatibl.patch`

		`- # pid1: Do not updated state for device units being processed by udev`
		`- Patch0906: 0001-pid1-Do-not-update-state-for-device-units-being-proc.patch`
		`-`
		`%endif`

		`%ifarch %{ix86} x86_64 aarch64 riscv64`