On 11/28/25 15:41, Alyssa Ross wrote: > Demi Marie Obenour writes: > >> On 11/28/25 08:47, Alyssa Ross wrote: >>> Demi Marie Obenour writes: >>> >>>> diff --git a/host/rootfs/image/usr/bin/spectrum-update b/host/rootfs/image/usr/bin/spectrum-update >>>> new file mode 100755 >>>> index 0000000000000000000000000000000000000000..613b43570d0538fce20296ccb1de2a6364e0df55 >>>> --- /dev/null >>>> +++ b/host/rootfs/image/usr/bin/spectrum-update >>>> @@ -0,0 +1,92 @@ >>>> +#!/bin/execlineb -WS1 >>>> +# SPDX-License-Identifier: EUPL-1.2+ >>>> +# SPDX-FileCopyrightText: 2025 Demi Marie Obenour >>>> + >>>> +if { mkdir -p -m 0700 /run/updater } >>>> + >>>> +# Take a global lock to avoid races. >>>> +s6-setlock /run/update-lock >>>> + >>>> +foreground { redirfd -w 2 /dev/null rmdir -- $1 } >>>> +if { umask 0077 mkdir -p -- $1 } >>>> +cd $1 >>>> +foreground { >>>> + # If this exists already that is okay. >>>> + foreground { redirfd -w 2 /dev/null btrfs subvolume create -- shared } >>>> + >>>> + # Delete any stale temporary files. Delete any existing signature >>>> + # files. If the VM is still running (it should not be), the VM might >>>> + # have write access to the directory. However, updates-dir-check is >>>> + # safe against that. >>>> + if { updates-dir-check cleanup shared } >>>> + >>>> + if { >>>> + foreground { >>>> + # TODO: suppress only "subvolume does not exist" errors. >>>> + redirfd -w 2 /dev/null >>>> + btrfs subvolume delete snapshot >>>> + } >>>> + rm -f snapshot >>>> + } >>>> + >>>> + backtick -E update_vm_id { >>>> + backtick -E id_path { readlink /run/vm/by-name/sys.appvm-systemd-sysupdate } >>>> + basename -- $id_path >>>> + } >>>> + >>>> + # $fsdir is read-only to the guest, but read-write to the host. >>>> + # Directories bind-mounted into it are read-write to the guest. >>>> + # See etc/s6-linux-init/run-image/service/vhost-user-fs/template/run >>>> + # for details. >>>> + >>> >>> This still refers to a non-existent variable. Whoops! >>>> + # Set up /etc with what the VM needs. The VM will overlay this >>>> + # on its own /etc. >>>> + # >>>> + # In the future, this should use a bind mount instead of copying >>>> + # into a tmpfs. However, this would significantly complicate the >>>> + # cleanup code. Deleting fs/etc would require undoing the bind >>>> + # mounts instead of rm -rf. Once this code is in a separate mount >>>> + # namespace, the copies should be replaced by bind mounts. >>>> + if { >>>> + if { rm -rf -- /run/vm/by-id/${update_vm_id}/fs/etc } >>>> + umask 022 >>>> + if { mkdir -p -- /run/vm/by-id/${update_vm_id}/fs/updates /run/vm/by-id/${update_vm_id}/fs/etc/systemd } >>>> + if { cp -R -- /etc/vm-sysupdate.d /etc/update-url /run/vm/by-id/${update_vm_id}/fs/etc } >>>> + cp -- /etc/systemd/import-pubring.gpg /run/vm/by-id/${update_vm_id}/fs/etc/systemd >>>> + } >>>> + >>>> + # If the directory is already mounted, unmount it. This prevents a >>>> + # confusing error from mount. >>>> + foreground { redirfd -w 2 /dev/null umount -- /run/vm/by-id/${update_vm_id}/fs/updates } >>>> + >>>> + # Share the update directory with the VM. >>>> + if { mount --bind -- shared /run/vm/by-id/${update_vm_id}/fs/updates } >>>> + >>>> + # Start the update VM. >>>> + if { vm-start $update_vm_id } >>>> + >>>> + # Wait for the VM to exit. >>>> + # TODO: This is racy. If the update finishes before this code runs, >>>> + # the s6-svwait call will fail. >>>> + if { s6-svwait -D /run/service/vmm/instance/${update_vm_id} } >>>> + >>>> + # Remove the bind mount. >>>> + if { umount -- /run/vm/by-id/${update_vm_id}/fs/updates } >>>> + >>>> + # Ensure that the VM cannot change the directory >>>> + # while systemd-sysupdate is using it. >>>> + if { btrfs subvolume snapshot -- shared snapshot } >>>> + >>>> + # Validate the update directory. Delete any stale temporary files. >>>> + # Check that a signature file was downloaded. >>>> + if { updates-dir-check check snapshot } >>>> + >>>> + unshare --mount >>>> + if { mount --bind -o ro -- snapshot /run/updater } >>>> + >>>> + /usr/lib/systemd/systemd-sysupdate update >>> >>> Why not just make a readonly snapshot? >>> (btrfs subvolume snapshot -r) >> >> The checker will delete any temporary files it comes across, so it >> needs write access. A snapshot is much heavier than a bind mount >> and isn't automatically cleaned up. > > Okay, but why do we need to block systemd-sysupdate from writing to this > directory, but not to anywhere else on the system? We don't. I'm fine with a writable mount. Either should work. >>>> diff --git a/vm/app/systemd-sysupdate/default.nix b/vm/app/systemd-sysupdate/default.nix >>>> new file mode 100644 >>>> index 0000000000000000000000000000000000000000..69be0bab500ea2ea6cb3b6d71edbf1a3e7bddbba >>>> --- /dev/null >>>> +++ b/vm/app/systemd-sysupdate/default.nix >>>> @@ -0,0 +1,26 @@ >>>> +# SPDX-License-Identifier: MIT >>>> +# SPDX-FileCopyrightText: 2023 Alyssa Ross >>>> +# SPDX-FileCopyrightText: 2025 Demi Marie Obenour >>>> + >>>> +import ../../../lib/call-package.nix ( >>>> +{ callSpectrumPackage, curl, lib, src >>>> +, runCommand, systemd, writeScript >>>> +}: >>>> + >>>> +let >>>> + downloadUpdate = builtins.path { >>>> + name = "download-update"; >>>> + path = ./download-update; >>>> + }; >>> >>> builtins.path is overkill here surely, as opposed to just writing >>> ${./download-update} below? >> >> ${./download-update} includes the working directory in the Nix store >> hash, which means that renaming your source tree forces an unnecessary >> rebuild. builtins.path is the standard way to avoid this. > > Oh, I didn't realise it included the whole directory rather than just > the file name. Very justified then. Thanks! Also, changing the file name shouldn't cause a rebuild if the contents don't change. -- Sincerely, Demi Marie Obenour (she/her/hers)