Re: [PATCH v2 3/3] host/rootfs: Simplify s6-rc dependencies

[Alyssa Ross <hi@alyssa.is>]

[-- Attachment #1: Type: text/plain, Size: 7001 bytes --]

Demi Marie Obenour <demiobenour@gmail.com> writes:

> This moves almost all of the s6-rc dependencies into named, commented
> bundles.  This makes the system much easier to understand.
>
> Most of the explanation is in the bundle files themselves.
>
> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
> ---
>  host/rootfs/Makefile                                  |  8 ++++----
>  host/rootfs/image/etc/s6-rc/basic/contents            | 18 ++++++++++++++++++
>  host/rootfs/image/etc/s6-rc/{vmm-env => basic}/type   |  0
>  .../image/etc/s6-rc/{vmm-env => basic}/type.license   |  0
>  host/rootfs/image/etc/s6-rc/ok-all/contents           | 19 ++++++++++++++++---
>  .../s6-rc/sys-vmms/dependencies.d/{vmm-env => vm-env} |  0
>  host/rootfs/image/etc/s6-rc/systemd-udevd/run         |  9 ++++++---
>  host/rootfs/image/etc/s6-rc/vm-env/contents           | 13 ++++++++++---
>  host/rootfs/image/etc/s6-rc/vmm-env/contents          |  9 ---------
>  .../dependencies.d/{systemd-udevd-coldplug => basic}  |  0
>  10 files changed, 54 insertions(+), 22 deletions(-)

Hmm, we just moved away from using dependencies files to dependencies.d
directories, but now we're going the other way with contents?  That's
not to say we couldn't have a README file in each contents.d or
something serving much the same purpose, though.

> diff --git a/host/rootfs/image/etc/s6-rc/basic/contents b/host/rootfs/image/etc/s6-rc/basic/contents
> new file mode 100644
> index 0000000000000000000000000000000000000000..54f58577ba9f125e7fc08dcdd69394e8a88ab622
> --- /dev/null
> +++ b/host/rootfs/image/etc/s6-rc/basic/contents
> @@ -0,0 +1,18 @@
> +# SPDX-License-Identifier: CC0-1.0
> +# SPDX-FileCopyrightText: 2021 Alyssa Ross <hi@alyssa.is>
> +# SPDX-FileCopyrightText: 2025 Demi Marie Obenour <demiobenour@gmail.com>
> +
> +# This file contains all atomics and bundles needed for basic system
> +# initialization.  Everything that is not meant to run in early boot
> +# should depend on this.
> +
> +# This ensures that core files are written to the right place.
> +core
> +
> +# This ensures that static nodes are created.
> +static-nodes
> +
> +# systemd runs systemd-udev-trigger.service before sysinit.target.
> +# Therefore, anything using libudev that isn't an early boot service
> +# might expect udev coldplug to have finished.
> +systemd-udevd-coldplug

I'm not convinced by the "basic" bundle.  I worry that it's likely to
end up accumulating stuff that isn't actually needed by everything that
depends on it, and that things will end up being slow because they're
waiting for things they don't actually need.  I'm not sure where core
should go, really, because anything /could/ crash at any time.
static-nodes is presumably going away anyway with udev, so this is
really just the coldplug bundle.

> diff --git a/host/rootfs/image/etc/s6-rc/ok-all/contents b/host/rootfs/image/etc/s6-rc/ok-all/contents
> index c76a5af336c7f1c3f4b81bf1f6244a53e0399fe8..edef03d2b6eae4fb0b92ac828436cd4572ca7acc 100644
> --- a/host/rootfs/image/etc/s6-rc/ok-all/contents
> +++ b/host/rootfs/image/etc/s6-rc/ok-all/contents
> @@ -1,6 +1,19 @@
>  # SPDX-License-Identifier: CC0-1.0
>  # SPDX-FileCopyrightText: 2021 Alyssa Ross <hi@alyssa.is>
> -#
> -systemd-udevd-coldplug
> +# SPDX-FileCopyrightText: 2025 Demi Marie Obenour <demiobenour@gmail.com>
> +
> +# This file contains the services that need to be started even
> +# if nothing else depends on them.  It should not include services
> +# that are only needed by other services.  For instance, it would
> +# not be appropriate for this file to reference systemd-udevd.
> +# However, services should be included in this file whenever they
> +# are directly needed by the user, even if they depend on another
> +# service in this files.  For instance, sys-vmms depends on weston,
> +# but it is still included.
> +
> +# VMMs that should be started at boot.  In the future
> +# it might make sense to start them only as needed.
>  sys-vmms
> -vm-env

See below — I think the name of "vm-env" hasn't captured what it does
very well.

> diff --git a/host/rootfs/image/etc/s6-rc/systemd-udevd/run b/host/rootfs/image/etc/s6-rc/systemd-udevd/run
> index 2501680b69eb0060b651146dffb6b3a99640c6fb..f05804a6b3903a767f8e1400cf0153045dd3781f 100644
> --- a/host/rootfs/image/etc/s6-rc/systemd-udevd/run
> +++ b/host/rootfs/image/etc/s6-rc/systemd-udevd/run
> @@ -1,11 +1,14 @@
>  #!/bin/execlineb -P
>  # SPDX-License-Identifier: EUPL-1.2+
>  # SPDX-FileCopyrightText: 2025 Demi Marie Obenour <demiobenour@gmail.com>
> -if { rm -f /run/sd-notify-wrapper/systemd-udevd }
> +if { mkdir -p /run/sd-notify-wrapper }
> +s6-setlock -d 4 /run/sd-notify-wrapper/systemd-udevd.lock
> +if { rm -f /run/sd-notify-wrapper/systemd-udevd.sock }
>  background {
> -   s6-ipcserver-socketbinder -b0 -m -a 0600 /run/sd-notify-wrapper/systemd-udevd
> +   s6-ipcserver-socketbinder -b0 -m -a 0600 /run/sd-notify-wrapper/systemd-udevd.sock
>     fdmove 1 3
>     sd-notify-adapter
>  }
> -export NOTIFY_SOCKET /run/sd-notify-wrapper/systemd-udevd
> +fdclose 3
> +export NOTIFY_SOCKET /run/sd-notify-wrapper/systemd-udevd.sock
>  exec -a systemd-udevd udevadm

Are these changes supposed to be here?

> diff --git a/host/rootfs/image/etc/s6-rc/vm-env/contents b/host/rootfs/image/etc/s6-rc/vm-env/contents
> index ed8b5c410adfc17f5bbc33932ac79d97bd7b3115..81bfd422d22bde582fe198e0778c1c93d0d0f329 100644
> --- a/host/rootfs/image/etc/s6-rc/vm-env/contents
> +++ b/host/rootfs/image/etc/s6-rc/vm-env/contents
> @@ -1,6 +1,13 @@
>  # SPDX-License-Identifier: CC0-1.0
>  # SPDX-FileCopyrightText: 2021 Alyssa Ross <hi@alyssa.is>
> -#
> -static-nodes
> -systemd-udevd-coldplug
> +# SPDX-FileCopyrightText: 2025 Demi Marie Obenour <demiobenour@gmail.com>
> +
> +# This file contains s6 atomics and bundles that
> +# need to be started before starting any VMs.
> +
> +# Basic initialization
> +basic
> +# KVM being available
> +kvm
> +# Wayland compositor ready
>  weston
> diff --git a/host/rootfs/image/etc/s6-rc/vmm-env/contents b/host/rootfs/image/etc/s6-rc/vmm-env/contents
> deleted file mode 100644
> index 9a1aea63abd1b5c374e3ac9a9671e4c9341f8d16..0000000000000000000000000000000000000000
> --- a/host/rootfs/image/etc/s6-rc/vmm-env/contents
> +++ /dev/null
> @@ -1,9 +0,0 @@
> -# SPDX-License-Identifier: CC0-1.0
> -# SPDX-FileCopyrightText: 2021 Alyssa Ross <hi@alyssa.is>
> -#
> -core
> -kvm
> -# systemd runs systemd-udev-trigger.service before sysinit.target.
> -# Therefore, anything using libudev that isn't an early boot service
> -# might expect udev coldplug to have finished.
> -systemd-udevd-coldplug

I think it probably makes sense to keep a distinction between "user can
now start VMs" (vm-env) and "it is now possible to start a VMM, possibly
automatically" (vmm-env).  We can of course try to find better names,
though.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]