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

[Demi Marie Obenour <demiobenour@gmail.com>]

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

On 11/20/25 09:56, Alyssa Ross wrote:
> 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.

That seems like a good idea.  The multisubstitute command is there
for a reason, though :).

>>>>>> +  }
>>>>>> +
>>>>>> +  # $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?

The envfile and the .transfer files were both generated with sed,
so using an envfile didn't require any extra tools.  However, v3
doesn't use an envfile anymore, so this is moot.
>>>>>> +    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.

That's good, thanks!  v3 switches to SHA256SUMS.sha256.asc, as it
seems to be the "proper" filename.
-- 
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 --]