Re: [PATCH v5 11/13] Support updates via systemd-sysupdate

Demi Marie Obenour
2025-11-28T20:44:54Z

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

On 11/28/25 15:41, Alyssa Ross wrote:
> Demi Marie Obenour <demiobenour@gmail.com> writes:
>
>> On 11/28/25 08:47, Alyssa Ross wrote:
>>> Demi Marie Obenour <demiobenour@gmail.com> 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 <demiobenour@gmail.com>
>>>> +
>>>> +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 <hi@alyssa.is>
>>>> +# SPDX-FileCopyrightText: 2025 Demi Marie Obenour <demiobenour@gmail.com>
>>>> +
>>>> +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)

[-- 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 --]