Re: [PATCH v4 05/20] host/rootfs: Add helper program for per-VM services

[Alyssa Ross <hi@alyssa.is>]

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

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

> The program handles cgroups and common substitutions.  This is better
> than repeating the boilerplate multiple times.
>
> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
> ---
>  host/rootfs/file-list.mk                 |  1 +
>  host/rootfs/image/usr/bin/vm-service-run | 34 ++++++++++++++++++++++++++++++++
>  2 files changed, 35 insertions(+)
>
> diff --git a/host/rootfs/file-list.mk b/host/rootfs/file-list.mk
> index e1280ab56d8797e40b9b1c584ab0daef3cda41d7..c707c01179238231f70164ffda5b7c940b34192a 100644
> --- a/host/rootfs/file-list.mk
> +++ b/host/rootfs/file-list.mk
> @@ -65,6 +65,7 @@ FILES = \
>  	image/usr/bin/spectrum-update \
>  	image/usr/bin/vm-console \
>  	image/usr/bin/vm-import \
> +	image/usr/bin/vm-service-run \
>  	image/usr/bin/vm-start \
>  	image/usr/bin/vm-stop \
>  	image/usr/bin/xdg-open \
> diff --git a/host/rootfs/image/usr/bin/vm-service-run b/host/rootfs/image/usr/bin/vm-service-run
> new file mode 100755
> index 0000000000000000000000000000000000000000..c788574cf95ec7fd0d93b656557f85fa80b31690
> --- /dev/null
> +++ b/host/rootfs/image/usr/bin/vm-service-run
> @@ -0,0 +1,34 @@
> +#!/usr/bin/execlineb -WS1
> +# SPDX-License-Identifier: EUPL-1.2+
> +# SPDX-FileCopyrightText: 2026 Demi Marie Obenour <demiobenour@gmail.com>
> +
> +# Substitute all of the needed environment variables
> +# into both this script and its arguments
> +# (the subsequent command to run).  This substitutes
> +# the *caller*'s arguments.
> +multisubstitute {
> +  importas -iS VM
> +  importas -iS "#"
> +  importas -iS "1"
> +  importas -iS WAYLAND_DISPLAY
> +}
> +
> +# Check that the VM name and service are reasonable.
> +# Then run the cgroup-setup program and the provided
> +# command line.  Avoid premature substitution by
> +# escaping ${#} and ${1} with backslashes.  Otherwise,
> +# they would be values for this script, not for the caller.
> +case -- "\\${#}@${VM}@\\${1}" {
> +  # s6-supervise might start passing extra arguments in the future, but
> +  # 0 arguments is not okay.
> +  "[1-9][0-9]*@[A-Za-z0-9_][A-Za-z0-9_.-]*@[A-Za-z_][A-Za-z0-9_-]*" {
> +    # The caller has been migrated from using -WS1 to using -Wp.
> +    # Pop the environment to preserve the original behavior.
> +    emptyenv -P cgroup-setup --leaf -- "\\${1}" $@
> +  }
> +}
> +fdmove -c 1 2
> +if { printf "Wrong VM name %s, \
> +parent argument count %s, or parent argument %s\n"
> +     $VM "\\$#" "\\${1}" }
> +exit 100

Not sure I get this.  This validation looks quite complicated and
prescriptive, and why are we substituting e.g. WAYLAND_DISPLAY in here
when not every service is going to need it?  I fear that this is the
sort of deduplication and validation that makes things harder to
understand and maintain…

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