PR#3: Don't trigger udev if socket is not accessible - rpms/udisks2

rpms / udisks2

#3 Don't trigger udev if socket is not accessible

Merged 4 years ago by tbzatek. Opened 4 years ago by jlebon.

rpms/ jlebon/udisks2 pr/fix-udisks2-post into master

Don't trigger udev if socket is not accessible

Jonathan Lebon • 4 years ago

f651544

udisks2.spec

file modified

+10 -3

		`@@ -59,7 +59,7 @@`
		`Name: udisks2`
		`Summary: Disk Manager`
		`Version: 2.8.4`
		`- Release: 2%{?dist}`
		`+ Release: 3%{?dist}`
		`License: GPLv2+`
		`URL: https://github.com/storaged-project/udisks`
		`Source0: https://github.com/storaged-project/udisks/releases/download/udisks-%{version}/udisks-%{version}.tar.bz2`
		`@@ -307,8 +307,12 @@`
		`%post -n %{name}`
		`%systemd_post udisks2.service`
		`%systemd_post clean-mount-point@.service`
		`- udevadm control --reload`
		`- udevadm trigger`
		`+ # skip retriggering if udevd isn't even accessible, e.g. containers or`
		`+ # rpm-ostree-based systems`
		`+ if [ -S /run/udev/control ]; then`
		`+ udevadm control --reload`
		`+ udevadm trigger`
		`+ fi`

		`%preun -n %{name}`
		`%systemd_preun udisks2.service`
		`@@ -430,6 +434,9 @@`
		`%endif`

		`%changelog`
		`+ * Tue Oct 01 2019 Jonathan Lebon <jonathan@jlebon.com> - 2.8.4-3`
		`+ - Don't trigger udev if socket is not accessible`
		`+`
		`* Sat Jul 27 2019 Fedora Release Engineering <releng@fedoraproject.org> - 2.8.4-2`
		`- Rebuilt for https://fedoraproject.org/wiki/Fedora_31_Mass_Rebuild`

jlebon commented 4 years ago

In container environments or rpm-ostree systems, scriptlets are not
directly run on a host system. Thus, there's no point in asking udev to
reload and retrigger: the host udev wouldn't see the new rules anyway.

Let's just generalize this by checking if the udev socket even exists.
If not, then we can't talk to udevd and we should just skip the reload
& trigger bits.

Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1753786

jlebon commented 4 years ago

I've successfully tested this in traditional Fedora cloud (where we do trigger a reload) and in Fedora CoreOS (where we don't).

tbzatek commented 4 years ago

Well yes, looks good, no objections before merging. Though I still think this should be ideally dealt with on udev side, e.g. detecting an environment it's called from and bypassing operation in such case.

walters commented 4 years ago

Yep, LGTM

jlebon commented 4 years ago

Well yes, looks good, no objections before merging. Though I still think this should be ideally dealt with on udev side, e.g. detecting an environment it's called from and bypassing operation in such case.

I think both make sense: checking for it here, and handling it gracefully on the udev side.

Note BTW while testing this that this did not reproduce on f30. So It may be that it used to handle it gracefully, and regressed between f30 and f31.