From: DaanDeMeyer Date: Tue, 6 Jan 2026 20:29:04 +0000 (+0100) Subject: Stop running auxiliary programs in systemd scopes X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=634b0ef4c7ee7ccd3196b2f6cc1e1549ba316260;p=thirdparty%2Fmkosi.git Stop running auxiliary programs in systemd scopes Similar to the same change made in systemd-vmspawn, let's stop running virtiofsd, systemd-journal-remote and swtpm in scopes. Nobody ever makes use of the features this provides and it simplifies our code quite a bit. This also means we drop the UnitProperties setting, which was effectively unused anyway. This allows us to get rid of the --suspend setting in mkosi-sandbox, which only really existed to allow waiting for systemd-run to finish its setup before registering the machine. Because registering a machine means it needs a cgroup, we allow systemd-machined to create the scope itself if needed. --- diff --git a/mkosi/__init__.py b/mkosi/__init__.py index d23332750..e90041f85 100644 --- a/mkosi/__init__.py +++ b/mkosi/__init__.py @@ -4355,9 +4355,6 @@ def run_shell(args: Args, config: Config) -> None: "--set-credential=journal.forward_to_socket:/run/host/journal/socket", ] - for p in config.unit_properties: - cmdline += ["--property", p] - if args.verb == Verb.boot: # Add nspawn options first since systemd-nspawn ignores all options after the first argument. argv = args.cmdline diff --git a/mkosi/config.py b/mkosi/config.py index 09e0e25c1..5e5574c1c 100644 --- a/mkosi/config.py +++ b/mkosi/config.py @@ -2138,7 +2138,6 @@ class Config: runtime_network: Network runtime_build_sources: bool bind_user: bool - unit_properties: list[str] ssh_key: Optional[Path] ssh_certificate: Optional[Path] machine: Optional[str] @@ -4059,15 +4058,6 @@ SETTINGS: list[ConfigSetting[Any]] = [ help="Bind current user from host into container or virtual machine", scope=SettingScope.main, ), - ConfigSetting( - dest="unit_properties", - long="--unit-property", - metavar="PROPERTY", - section="Runtime", - parse=config_make_list_parser(delimiter=" ", unescape=True), - help="Set properties on the scopes spawned by systemd-nspawn or systemd-run", - scope=SettingScope.main, - ), ConfigSetting( dest="ssh_key", metavar="PATH", @@ -5772,7 +5762,6 @@ def summary(config: Config) -> str: Runtime Network: {config.runtime_network} Runtime Build Sources: {config.runtime_build_sources} Bind User: {yes_no(config.bind_user)} - Unit Properties: {line_join_list(config.unit_properties)} SSH Signing Key: {none_to_none(config.ssh_key)} SSH Certificate: {none_to_none(config.ssh_certificate)} Machine: {config.machine_or_name()} diff --git a/mkosi/qemu.py b/mkosi/qemu.py index a433d5742..6617677bf 100644 --- a/mkosi/qemu.py +++ b/mkosi/qemu.py @@ -16,7 +16,6 @@ import queue import random import resource import shutil -import signal import socket import struct import subprocess @@ -45,7 +44,6 @@ from mkosi.config import ( format_bytes, swtpm_setup_version, systemd_pty_forward, - systemd_tool_version, want_selinux_relabel, yes_no, ) @@ -298,10 +296,6 @@ def start_swtpm(config: Config) -> Iterator[Path]: cmdline, pass_fds=(sock.fileno(),), sandbox=config.sandbox(options=["--bind", state, workdir(Path(state))]), - setup=scope_cmd( - name=f"mkosi-swtpm-{config.machine_or_name()}", - description=f"swtpm for {config.machine_or_name()}", - ), ) as proc: yield path proc.terminate() @@ -338,7 +332,6 @@ def start_virtiofsd( directory: Path, *, uidmap: bool = True, - name: Optional[str] = None, selinux: bool = False, ) -> Iterator[Path]: virtiofsd = find_virtiofsd(root=config.tools(), extra=config.extra_search_paths) @@ -393,36 +386,32 @@ def start_virtiofsd( # We want RuntimeBuildSources= and RuntimeTrees= to do the right thing even when running mkosi vm # as root without the source directories necessarily being owned by root. We achieve this by running # virtiofsd as the owner of the source directory and then mapping that uid to root. - if not name: - name = f"{config.machine_or_name()}-{systemd_escape(config, directory, path=True)}" - else: - name = systemd_escape(config, name) - - name = f"mkosi-virtiofsd-{name}" - description = f"virtiofsd for machine {config.machine_or_name()} for {directory}" - scope = [] - if st: - scope = scope_cmd(name=name, description=description, user=st.st_uid, group=st.st_gid) - elif not uidmap and (os.getuid() == 0 or unshare_version() >= "2.38"): - scope = scope_cmd(name=name, description=description) with spawn( cmdline, pass_fds=(sock.fileno(),), - user=st.st_uid if st and not scope else None, - group=st.st_gid if st and not scope else None, - # If we're booting from virtiofs and unshare is too old, we don't set up a scope so we can use - # our own function to become root in the subuid range. + user=st.st_uid if st else None, + group=st.st_gid if st else None, + # If we're booting from virtiofs and unshare is too old, we use our own function to become root + # in the subuid range if needed. # TODO: Drop this as soon as we drop CentOS Stream 9 support and can rely on newer unshare # features. - preexec=become_root_in_subuid_range if not scope and not uidmap else None, + preexec=( + become_root_in_subuid_range + if not uidmap and os.getuid() != 0 and unshare_version() < "2.38" + else None + ), sandbox=config.sandbox( options=[ "--bind", directory, workdir(directory), *(["--become-root"] if uidmap else []), ], ), - setup=scope + (become_root_in_subuid_range_cmd() if scope and not uidmap else []), + setup=( + become_root_in_subuid_range_cmd() + if not uidmap and os.getuid() != 0 and unshare_version() >= "2.38" + else [] + ), ) as proc: # fmt: skip yield path proc.terminate() @@ -502,12 +491,6 @@ def start_journal_remote(config: Config, sockfd: int) -> Iterator[None]: user = d.stat().st_uid if os.getuid() == 0 else None group = d.stat().st_gid if os.getuid() == 0 else None - scope = scope_cmd( - name=f"mkosi-journal-remote-{config.machine_or_name()}", - description=f"mkosi systemd-journal-remote for {config.machine_or_name()}", - user=user, - group=group, - ) with spawn( [ @@ -523,9 +506,8 @@ def start_journal_remote(config: Config, sockfd: int) -> Iterator[None]: "--pack-fds", ], ), - setup=scope, - user=user if not scope else None, - group=group if not scope else None, + user=user, + group=group, ) as proc: # fmt: skip yield proc.terminate() @@ -905,49 +887,6 @@ def finalize_credentials(config: Config, stack: contextlib.ExitStack) -> Path: return d -def scope_cmd( - name: str, - description: str, - user: Optional[int] = None, - group: Optional[int] = None, - properties: Sequence[str] = (), - environment: bool = True, -) -> list[str]: - if not find_binary("systemd-run"): - return [] - - if os.getuid() != 0 and "DBUS_SESSION_BUS_ADDRESS" in os.environ and "XDG_RUNTIME_DIR" in os.environ: - env = { - "DBUS_SESSION_BUS_ADDRESS": os.environ["DBUS_SESSION_BUS_ADDRESS"], - "XDG_RUNTIME_DIR": os.environ["XDG_RUNTIME_DIR"], - } - elif os.getuid() == 0: - if "DBUS_SYSTEM_ADDRESS" in os.environ: - env = {"DBUS_SYSTEM_ADDRESS": os.environ["DBUS_SYSTEM_ADDRESS"]} - elif Path("/run/dbus/system_bus_socket").exists(): - env = {"DBUS_SYSTEM_ADDRESS": "/run/dbus/system_bus_socket"} - else: - return [] - else: - return [] - - return [ - "env", - *(f"{k}={v}" for k, v in env.items() if environment), - "systemd-run", - "--system" if os.getuid() == 0 else "--user", - *(["--quiet"] if not ARG_DEBUG.get() else []), - "--unit", name, - "--description", description, - "--scope", - "--collect", - *(["--expand-environment=no"] if systemd_tool_version("systemd-run") >= 254 else []), - *(["--uid", str(user)] if user is not None else []), - *(["--gid", str(group)] if group is not None else []), - *([f"--property={p}" for p in properties]), - ] # fmt: skip - - def finalize_register(config: Config) -> bool: if config.register == ConfigFeature.disabled: return False @@ -983,6 +922,7 @@ def register_machine(config: Config, pid: int, fname: Path, cid: Optional[int]) **({"vSockCid": cid} if cid is not None else {}), **({"sshAddress": f"vsock/{cid}"} if cid is not None else {}), **({"sshPrivateKeyPath": f"{config.ssh_key}"} if config.ssh_key else {}), + **({"allocateUnit": True}), } ), ], @@ -1253,7 +1193,6 @@ def run_qemu(args: Args, config: Config) -> None: start_virtiofsd( config, fname, - name=config.machine_or_name(), uidmap=False, selinux=bool(want_selinux_relabel(config, fname, fatal=False)), ), @@ -1456,13 +1395,7 @@ def run_qemu(args: Args, config: Config) -> None: network=True, devices=True, relaxed=True, - options=["--same-dir", "--suspend"], - ), - setup=scope_cmd( - name=name, - description=f"mkosi Virtual Machine {name}", - properties=config.unit_properties, - environment=False, + options=["--same-dir"], ), ) as proc: # We have to close these before we wait for qemu otherwise we'll deadlock as qemu will never @@ -1470,8 +1403,6 @@ def run_qemu(args: Args, config: Config) -> None: for fd in qemu_device_fds.values(): os.close(fd) - os.waitid(os.P_PID, proc.pid, os.WEXITED | os.WSTOPPED | os.WNOWAIT) - register_machine(config, proc.pid, fname, cid) # Fork and threads don't mix well when using preexec functions, so start the thread to handle @@ -1481,8 +1412,6 @@ def run_qemu(args: Args, config: Config) -> None: AsyncioThread(functools.partial(vsock_notify_handler, sock=vsock)) ) - proc.send_signal(signal.SIGCONT) - if notify and (status := int({k: v for k, v in notify.process()}.get("EXIT_STATUS", "0"))) != 0: raise subprocess.CalledProcessError(status, cmdline) diff --git a/mkosi/resources/man/mkosi-sandbox.1.md b/mkosi/resources/man/mkosi-sandbox.1.md index 950b8f368..5db03e4be 100644 --- a/mkosi/resources/man/mkosi-sandbox.1.md +++ b/mkosi/resources/man/mkosi-sandbox.1.md @@ -131,11 +131,6 @@ from `mkosi.sandbox` is not supported and may break in future versions. `--unshare-ipc` : Specifying this option makes `mkosi-sandbox` unshare an IPC namespace if possible. -`--suspend` -: Make the `mkosi-sandbox` process suspend itself with `SIGSTOP` just before it calls `execve()`. - This is useful to wait until all setup logic has completed before continuing execution in the parent - process invoking `mkosi-sandbox` by using `waitid()` with the `WNOWAIT` AND `WSTOPPED` flags. - `--pack-fds` : Pack inherited file descriptors together starting at file descriptor number 3 and set `$LISTEN_FDS` to the number of packed file descriptors and `$LISTEN_PID` to the current process diff --git a/mkosi/resources/man/mkosi.1.md b/mkosi/resources/man/mkosi.1.md index 16550cc96..8f9fed2b5 100644 --- a/mkosi/resources/man/mkosi.1.md +++ b/mkosi/resources/man/mkosi.1.md @@ -1992,12 +1992,6 @@ boolean argument: either `1`, `yes`, or `true` to enable, or `0`, `no`, : Bind the home directory of the current user into the container/vm. Takes a boolean. Disabled by default. -`UnitProperties=`, `--unit-property=` -: Configure systemd unit properties to add to the systemd scopes - allocated when using `mkosi boot` or `mkosi vm`. These are passed - directly to the `--property=` options of **systemd-nspawn** and - **systemd-run** respectively. - `SshKey=`, `--ssh-key=` : Path to the X.509 private key in PEM format to use to connect to a virtual machine started with `mkosi vm` and built with the `Ssh=` diff --git a/mkosi/run.py b/mkosi/run.py index c0acd565c..984fa2bc1 100644 --- a/mkosi/run.py +++ b/mkosi/run.py @@ -322,10 +322,6 @@ def spawn( if log: log_process_failure(sbx, cmd, returncode) if ARG_DEBUG_SHELL.get(): - # --suspend will freeze the debug shell with no way to unfreeze it so strip it from the - # sandbox if it's there. - prefix = [s for s in prefix if s != "--suspend"] - sbx = [s for s in sbx if s != "--suspend"] subprocess.run( [*setup, *prefix, "bash"], check=False, diff --git a/mkosi/sandbox.py b/mkosi/sandbox.py index 377c4e025..a6c4422f2 100755 --- a/mkosi/sandbox.py +++ b/mkosi/sandbox.py @@ -1057,7 +1057,6 @@ mkosi-sandbox [OPTIONS...] COMMAND [ARGUMENTS...] --suppress-sync Make sync() syscalls in the sandbox a noop --unshare-net Unshare the network namespace if possible --unshare-ipc Unshare the IPC namespace if possible - --suspend Stop process before execve() See the mkosi-sandbox(1) man page for details.\ """ @@ -1083,7 +1082,7 @@ def main(argv: list[str] = sys.argv[1:]) -> None: upperdir = "" workdir = "" chdir = None - become_root = suppress_chown = suppress_sync = unshare_net = unshare_ipc = suspend = pack_fds = False + become_root = suppress_chown = suppress_sync = unshare_net = unshare_ipc = pack_fds = False try: ttyname = os.ttyname(2) if os.isatty(2) else "" @@ -1163,8 +1162,6 @@ def main(argv: list[str] = sys.argv[1:]) -> None: unshare_net = True elif arg == "--unshare-ipc": unshare_ipc = True - elif arg == "--suspend": - suspend = True elif arg == "--pack-fds": pack_fds = True elif arg.startswith("-"): @@ -1288,9 +1285,6 @@ def main(argv: list[str] = sys.argv[1:]) -> None: os.environ["LISTEN_FDS"] = str(nfds) os.environ["LISTEN_PID"] = str(os.getpid()) - if suspend: - os.kill(os.getpid(), SIGSTOP) - if is_main(): try: os.execvp(argv[0], argv) diff --git a/tests/test_json.py b/tests/test_json.py index e6deffbc2..a17fdcfae 100644 --- a/tests/test_json.py +++ b/tests/test_json.py @@ -404,9 +404,6 @@ def test_config() -> None: } ], "UnifiedKernelImages": "auto", - "UnitProperties": [ - "PROPERTY=VALUE" - ], "UseSubvolumes": "auto", "VSock": "enabled", "VSockCID": -2, @@ -603,7 +600,6 @@ def test_config() -> None: ) ], unified_kernel_images=UnifiedKernelImage.auto, - unit_properties=["PROPERTY=VALUE"], use_subvolumes=ConfigFeature.auto, verity_certificate_source=CertificateSource(type=CertificateSourceType.file), verity_certificate=Path("/path/to/cert"),