Re: [PATCH v4 2/2] host/rootfs: Switch to systemd-udevd

[Demi Marie Obenour <demiobenour@gmail.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 8780 bytes --]

On 10/28/25 12:02, Alyssa Ross wrote:
> Demi Marie Obenour <demiobenour@gmail.com> writes:
> 
>> This replaces the mdevd uevent daemon with systemd-udevd, which supports
>> much more hardware and is therefore to be preferred for a full desktop
>> environment.  Various Wayland compositors integrate with systemd-udevd,
>> allowing them to discover devices as they appear rather than having to
>> only use devices plugged in when the compositor is started.
>> Additionally, systemd-udevd has quirks for various devices that are
>> needed to support the wide variety of hardware end-users have.
>>
>> The dependency on /dev/dri/card0 being present is eliminated, and
>> whatever devices the user has are now picked up by the compositor.  New
>> dependencies are added to ensure that udev coldplug has finished before
>> any non-trivial services are started.  systemd-udev-trigger.service runs
>> 'udevadm trigger' and has Before=sysinit.target, so anything that is not
>> an early boot service can assume 'udevadm trigger' has run.
>>
>> systemd-udevd unsets PATH before it runs programs.  Therefore, manually
>> set it to /usr/bin before invoking any scripts.
>>
>> systemd-udevd provides the 'udevadm wait' command to wait for a device
>> to appear and be processed.  This replaces all uses of /etc/mdev/wait,
>> so this script is removed.  /etc/mdev/listen and /run/wait only existed
>> to wake up /etc/mdev/wait, so they are also removed.
>>
>> The command-line options to 'udevadm trigger' are taken from
>> systemd-udev-trigger.service on my Arch machine.  Using the wrong
>> arguments leads to problems that take a very long time to debug.
>>
>> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
>> ---
>>  LICENSES/LGPL-2.1-or-later.txt                     | 502 +++++++++++++++++++++
>>  host/rootfs/Makefile                               |   2 +-
>>  host/rootfs/default.nix                            | 118 ++---
>>  host/rootfs/file-list.mk                           |  33 +-
>>  host/rootfs/image/etc/mdev.conf                    |   7 -
>>  host/rootfs/image/etc/mdev/listen                  |  11 -
>>  host/rootfs/image/etc/mdev/wait                    |  14 -
>>  host/rootfs/image/etc/s6-rc/card0/type.license     |   2 -
>>  host/rootfs/image/etc/s6-rc/card0/up               |   4 -
>>  host/rootfs/image/etc/s6-rc/kvm/timeout-up         |   1 -
>>  host/rootfs/image/etc/s6-rc/kvm/timeout-up.license |   2 -
>>  host/rootfs/image/etc/s6-rc/kvm/type               |   1 -
>>  host/rootfs/image/etc/s6-rc/kvm/type.license       |   2 -
>>  host/rootfs/image/etc/s6-rc/kvm/up                 |   4 -
>>  host/rootfs/image/etc/s6-rc/mdevd-coldplug/type    |   1 -
>>  host/rootfs/image/etc/s6-rc/mdevd-coldplug/up      |   4 -
>>  .../image/etc/s6-rc/mdevd/notification-fd.license  |   2 -
>>  host/rootfs/image/etc/s6-rc/mdevd/run              |   5 -
>>  .../contents.d/systemd-udevd-coldplug}             |   0
>>  .../dependencies.d/systemd-udevd}                  |   0
>>  .../s6-rc/{card0 => systemd-udevd-coldplug}/type   |   0
>>  .../type.license                                   |   0
>>  .../image/etc/s6-rc/systemd-udevd-coldplug/up      |   6 +
>>  .../s6-rc/{mdevd => systemd-udevd}/notification-fd |   0
>>  .../s6-rc/systemd-udevd/notification-fd.license    |   2 +
>>  host/rootfs/image/etc/s6-rc/systemd-udevd/run      |  15 +
>>  .../image/etc/s6-rc/{mdevd => systemd-udevd}/type  |   0
>>  .../s6-rc/{mdevd => systemd-udevd}/type.license    |   0
>>  .../contents.d/systemd-udevd-coldplug}             |   0
>>  .../contents.d/systemd-udevd-coldplug}             |   0
>>  .../weston/dependencies.d/systemd-udevd-coldplug   |   0
>>  host/rootfs/image/etc/s6-rc/weston/run             |   7 +-
>>  .../image/etc/udev/rules.d/99-spectrum.rules       |  15 +
>>  host/rootfs/image/usr/bin/run-vmm                  |   1 +
>>  host/rootfs/image/usr/bin/systemd-udevd            |   1 +
>>  .../{etc/mdev/net/add => usr/libexec/net-add}      |   0
>>  36 files changed, 604 insertions(+), 158 deletions(-)
> 
> Also looking good, and again if you'd prefer I just make my suggested
> changes myself when I apply it that's fine by me.

That would be great, especially because a change to net-add is needed.

>> diff --git a/host/rootfs/image/etc/s6-rc/systemd-udevd-coldplug/up b/host/rootfs/image/etc/s6-rc/systemd-udevd-coldplug/up
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..f4bfe58d1219218ba486050e4de0f949df8f2058
>> --- /dev/null
>> +++ b/host/rootfs/image/etc/s6-rc/systemd-udevd-coldplug/up
>> @@ -0,0 +1,6 @@
>> +# SPDX-License-Identifier: LGPL-2.1-or-later
>> +# SPDX-FileCopyrightText: systemd is free software; you can redistribute it and/or modify it
>> +#                         under the terms of the GNU Lesser General Public License as published by
>> +#                         the Free Software Foundation; either version 2.1 of the License, or
>> +#                         (at your option) any later version.
> 
> This is a statement of the license and is covered by the
> SPDX-License-Identifier metadata.  SPDX-FileCopyrightText is for
> identifying copyright owners.  For a project that doesn't provide a
> copyright notice, we'd typically attribute copyright to e.g. "The
> systemd Authors".
> 
> In this case though, I don't think there's anything copyrightable here,
> so CC0-1.0 and your name should be fine.

That makes sense.

>> +udevadm trigger --type=all --action=add --prioritized-subsystem=module,block,tpmrm,net,tty,input
> 
>> diff --git a/host/rootfs/image/etc/s6-rc/systemd-udevd/run b/host/rootfs/image/etc/s6-rc/systemd-udevd/run
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..98d1aa83153c99f32afb4d27a97cf39fcadb2968
>> --- /dev/null
>> +++ b/host/rootfs/image/etc/s6-rc/systemd-udevd/run
>> @@ -0,0 +1,15 @@
>> +#!/bin/execlineb -P
>> +# SPDX-License-Identifier: EUPL-1.2+
>> +# SPDX-FileCopyrightText: 2025 Demi Marie Obenour <demiobenour@gmail.com>
>> +s6-setlock /run/sd-notify-wrapper/systemd-udevd.lock
>> +# Save stdin to restore it later.
>> +fdmove -c 4 0
> 
> stdin will always be /dev/null, so instead of this dance you can just
> redirfd -r 0 /dev/null after the background.

Good point.

>> +s6-ipcserver-socketbinder -b0 -m -a 0600 /run/sd-notify-wrapper/systemd-udevd.sock
>> +background {
>> +   fdmove 1 3
>> +   sd-notify-adapter
>> +}
>> +fdclose 3
>> +fdmove 0 4
>> +export NOTIFY_SOCKET /run/sd-notify-wrapper/systemd-udevd.sock
>> +systemd-udevd
>> diff --git a/host/rootfs/image/etc/s6-rc/weston/run b/host/rootfs/image/etc/s6-rc/weston/run
>> index 7dca0dab095569c9e7d49df9d245533a7265283e..8276e7eed45ab8155e21a3e11f86c57c03d66cc4 100644
>> --- a/host/rootfs/image/etc/s6-rc/weston/run
>> +++ b/host/rootfs/image/etc/s6-rc/weston/run
>> @@ -4,9 +4,9 @@
>>  
>>  unexport WAYLAND_DISPLAY
>>  
>> -foreground {
>> -  umask 077
>> -  mkdir /run/user/0
>> +if {
>> +  umask 0022
>> +  mkdir -p -m 0700 /run/user/0
> 
> What's the point in continuing to umask when you explicitly provide the mode?

The mode only applies to the last directory (/run/user/0).  The umask
ensures that /run/user will be created with the correct permissions
(0022) if it doesn't exist already.

>>  }
>>  unexport ?
>>  
>> @@ -20,4 +20,5 @@ redirfd -r 0 /dev/tty1
>>  
>>  importas -i home HOME
>>  cd $home
>> +if { udevadm wait /dev/dri/card0 }
>>  weston
>> diff --git a/host/rootfs/image/etc/udev/rules.d/99-spectrum.rules b/host/rootfs/image/etc/udev/rules.d/99-spectrum.rules
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..c65829419de3516d6ec8e95e7928a69c308507aa
>> --- /dev/null
>> +++ b/host/rootfs/image/etc/udev/rules.d/99-spectrum.rules
>> @@ -0,0 +1,15 @@
>> +# SPDX-License-Identifier: EUPL-1.2+
>> +# SPDX-FileCopyrightText: 2025 Demi Marie Obenour <demiobenour@gmail.com>
>> +
>> +# systemd-udevd unsets PATH, so fix that.
> 
> Stray copy of comment.

Whoops!

>> +ACTION=="remove", GOTO="spectrum-end"
>> +ENV{PCI_CLASS}!="2????", GOTO="spectrum-end"
>> +
>> +# net-add unbinds and rebinds the driver, so avoid calling
>> +# it more than once.
>> +IMPORT{db}="SPECTRUM_DRIVER_ASSIGNED"
>> +ENV{SPECTRUM_DRIVER_ASSIGNED}=="yes", GOTO="spectrum-end"
>> +# systemd-udevd unsets PATH, so fix that.
>> +RUN+="/usr/bin/env PATH=/usr/bin /usr/libexec/net-add"
>> +ENV{SPECTRUM_DRIVER_ASSIGNED}="yes"
>> +LABEL="spectrum-end"
> 
> Again, I'd much rather this was taken care of in net-add.  I'll post a
> patch to make it re-entrant, I guess.

That would be great.
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]