Re: [PATCH v2 6/8] Support updates via systemd-sysupdate

Alyssa Ross
2025-11-20T14:56:32Z

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

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

> On 11/14/25 07:14, Alyssa Ross wrote:
>> Demi Marie Obenour <demiobenour@gmail.com> writes:
>>
>>> On 11/13/25 11:44, Alyssa Ross wrote:
>>>> Demi Marie Obenour <demiobenour@gmail.com> writes:
>>>>
>>>>> +
>>>>> + backtick -E update_vm_id_ {
>>>>> + backtick -E id_path { readlink /run/vm/by-name/sys.appvm-updates }
>>>>> + basename -- $id_path
>>>>> + }
>>>>> +
>>>>> + multisubstitute {
>>>>> + define fsdir /run/vm/by-id/${update_vm_id_}/fs
>>>>> + define update_vm_id ${update_vm_id_}
>>>>
>>>> Why?
>>>
>>> Avoiding serial substitution.
>>
>> But this is serial substitution. You're substituting update_vm_id_, and
>> then you're doing another substitution of update_vm_id without the
>> underscore. Why? Why not the following?
>>
>> backtick update_vm_id { ... }
>> multisubstitute {
>> define fsdir ...
>> importas -Siu update_vm_id
>> }
>
> "serial substitution" means that one substitutes into a string
> that has already been substituted into. See the multisubstitute
> documentation for why this is bad. I avoided the route you describe
> because I wanted to define fsdir and update_vm_id in the same call
> to multisubstitute.

Right — I now understand. I think going forward we might want to
consider having to substitute more than once as an indication that the
script is too complicated for execline, which I'm sure you'll be happy
to here. But we can do this way for now and do a full audit of our
execline scripts later when we get a chance.

>>>>> + }
>>>>> +
>>>>> + # $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.
>>>>> +
>>>>> + # Set up /etc with what the VM needs. The VM will overlay this
>>>>> + # on its own /etc.
>>>>> + if { rm -rf -- ${fsdir}/etc }
>>>>> + if { umask 022 mkdir -p -- ${fsdir}/updates ${fsdir}/etc/systemd }
>>>>> + if { cp -R -- /etc/updatevm/sysupdate.d /etc/updatevm/url-env ${fsdir}/etc }
>>>>> + if { cp -- /etc/systemd/import-pubring.gpg ${fsdir}/etc/systemd }
>>>>
>>>> Why copy rather than bind mount?
>>>
>>> Target does not exist and I didn't want to bind-mount all of /etc/systemd.
>>
>> You can touch a file and then bind mount, and still save a copy.
>
> This is a tiny file. I suspect the extra exec is more expensive than
> the copy.

My concern is the tmpfs space utilization — I know it's small but I'd
like to avoid files being stored on tmpfs at all unless there's a good
reason.

>>>>> @@ -17,5 +18,11 @@ let
>>>>> callConfig = config: if builtins.typeOf config == "lambda" then config {
>>>>> inherit default;
>>>>> } else config;
>>>>> + finalConfig = default // callConfig config;
>>>>> in
>>>>> - default // callConfig config;
>>>>> + finalConfig // {
>>>>> + update-signing-key = builtins.path {
>>>>> + name = "signing-key";
>>>>> + path = finalConfig.update-signing-key;
>>>>> + };
>>>>> + }
>>>>
>>>> What does this do?
>>>
>>> This ensures that the Nix store path doesn't depend on the name of
>>> the update signing key, only its contents.
>>
>> Interesting. Does that matter, though? It ends up being called
>> /etc/systemd/import-pubring.gpg in the image regardless.
>
> Otherwise renaming it would cause a pointless rebuild of a bunchof stuff.

Okay. Can we do this at the point of use? I'd rather not have a
special override for this one config value — it's another place to have
to remember to read to understand how config works.

>>>>> diff --git a/vm/app/updates.nix b/vm/app/updates.nix
>>>>> new file mode 100644
>>>>> index 0000000000000000000000000000000000000000..d2c1e5fcb35b37c7ed8a173f19b97894a36a7f0c
>>>>> --- /dev/null
>>>>> +++ b/vm/app/updates.nix
>>>>> @@ -0,0 +1,37 @@
>>>>> +# 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, config, curl, lib, src
>>>>> +, runCommand, systemd, writeScript
>>>>> +}:
>>>>> +
>>>>> +let
>>>>> + update-url = config.update-url;
>>>>> + mountpoint = "/run/virtiofs/virtiofs0";
>>>>> + sysupdate-path = "${systemd}/lib/systemd/systemd-sysupdate";
>>>>> + runner = writeScript "update-run-script"
>>>>> + ''
>>>>> + #!/usr/bin/execlineb -P
>>>>> + if { mount -toverlay -olowerdir=${mountpoint}/etc:/etc -- overlay /etc }
>>>>> + envfile ${mountpoint}/etc/url-env
>>>>
>>>> Seems like overkill to use an envfile for a single URL?
>>>
>>> It is indeed overkill, but I'm not aware of a simpler option.
>>> There is backtick + cat but that's two programs rather than one.
>>
>> I think the canonical way would be redirfd + withstdinas, but that's
>> also two programs, so if you want to avoid that, perhaps s6-envdir?
>> Reading it isn't any simpler but writing it at least doesn't require a
>> special tool.
>
> We need sed to generate the .transfer files anyway.

What does sed have to do with envfile/s6-envdir/etc?

>>>>> + importas -i update_url UPDATE_URL
>>>>> + if { ${sysupdate-path} update }
>>>>> + if { ${curl}/bin/curl -L --proto =http,https
>>>>> + -o ${mountpoint}/updates/SHA256SUMS.gpg ''${update_url}/SHA256SUMS.gpg }
>>>>> + # systemd-sysupdate recently went from needing SHA256SUMS.gpg to SHA256SUMS.sha256.asc.
>>>>> + # I (Demi) have no need if this is intentional or a bug. I also have no idea if this
>>>>> + # behavior will stay unchanged in the future. Therefore, create both files and let
>>>>> + # systemd-sysupdate ignore the one it isn't interested in.
>>>>> + if { ln -f ${mountpoint}/updates/SHA256SUMS.gpg ${mountpoint}/updates/SHA256SUMS.sha256.asc }
>>>>
>>>> Would be good to figure out why that happened. If we add a comment like
>>>> this it's very unlikely to ever get cleaned up.
>>>
>>> https://github.com/systemd/systemd/issues/39273
>>
>> "hwdb: drop trailing whitespace"?
>
> https://github.com/systemd/systemd/issues/39723
> ("systemd-sysupdate checks for SHA256SUMS.sha256.asc when fetching from file:///"),
> which was fixed in
> <https://github.com/systemd/systemd/commit/aa7574417b86ac0bb7ed492b7cfc872e9ace15d7>
> ("pull: fix SHA256SUMS fallback for file:// URLs").
> systemd v258.1 should have the fix. Does Spectrum use an older nixpkgs?

Yes, we're still on 258. I expect an update before this series is
applied though, because then we'll also pick up the systemd musl fix.

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