From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from atuin.qyliss.net (localhost [IPv6:::1]) by atuin.qyliss.net (Postfix) with ESMTP id 3B3A825480; Fri, 14 Nov 2025 12:14:38 +0000 (UTC) Received: by atuin.qyliss.net (Postfix, from userid 993) id 4664D253F6; Fri, 14 Nov 2025 12:14:36 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-26) on atuin.qyliss.net X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,DMARC_MISSING,RCVD_IN_DNSWL_LOW,SPF_HELO_PASS autolearn=unavailable autolearn_force=no version=4.0.1 Received: from fhigh-b5-smtp.messagingengine.com (fhigh-b5-smtp.messagingengine.com [202.12.124.156]) by atuin.qyliss.net (Postfix) with ESMTPS id 74088253F5 for ; Fri, 14 Nov 2025 12:14:32 +0000 (UTC) Received: from phl-compute-03.internal (phl-compute-03.internal [10.202.2.43]) by mailfhigh.stl.internal (Postfix) with ESMTP id 580287A016B; Fri, 14 Nov 2025 07:14:30 -0500 (EST) Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-03.internal (MEProxy); Fri, 14 Nov 2025 07:14:30 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=alyssa.is; h=cc :cc:content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm2; t=1763122470; x=1763208870; bh=w4wRoYGqOm ebkIAKaQoWZqWbC+fUQK/zKrTeu4QhrUA=; b=KRRFFefBLNavF6MAwKODRf4OTe z3zazQpHdM9Nx6iGe4HYJHLvgetVZ9/yOUTniNGAjQfZZptbK8ZdyptmHn3+ZO6a KgZAGsrVaUhhEqMdUbz0qky+rgxNZMxTkyeyajVV1eb3O6Rni37NuAny0VfRnvio bAWww6/GqYA6Gi26k6GVx5IysK2sFTdxCiSgXY4XLEor4dGnrsyWteY0hW22O6lA 2fu1mSVJnMXBM+b7st3XvEkIIDEhuP7gh/nLdibVgcaBAO907h9bCcQ1CJ7S+McS mXV19x6GMklOPDJJ8Y8cQRwZEkzzOHOGATl3qTDD70N6R/06vot9O5Ys+IDg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; t= 1763122470; x=1763208870; bh=w4wRoYGqOmebkIAKaQoWZqWbC+fUQK/zKrT eu4QhrUA=; b=NSUmR+9EF+YWR73cU1Mw3UNim8Xsziq7TtEDvV+AZs4ALD2NEJI J3xNoJrWaaS0WsOE6RFyEKSx+pdWVaej79viRAfpxXylKpYY5nci/WLNgnFYTR8W wQ836CVcj9Quz9tQJDkiWzfTBrXysIxyebNxiV3gU+eEQkufn/67V8KejBjbcWTd 7n5jqMqzZI0r+b+YJCDwlE3C8lctg0uYuG0KXH4pq/nb4+68uyn9dmtbCMBls7Kk dMZ3fjwf6Ov2l53BByaa9me8C+yw7FGXkQxiHHuQuhVv3JYvkKrCbdDLXWn8gqPn LrvC33pWSIbTd9REYja6Txml+HKJV0olgoA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeffedrtdeggddvtdeljeelucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfurfetoffkrfgpnffqhgenuceu rghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmnecujf gurhephffvvefujghffffkgggtsehgtderredttddtnecuhfhrohhmpeetlhihshhsrgcu tfhoshhsuceohhhisegrlhihshhsrgdrihhsqeenucggtffrrghtthgvrhhnpefftedvie eltdektedugffgleeggfdtteelvedtveduheevveffveffledvffejvdenucffohhmrghi nhepghhithhhuhgsrdgtohhmpdhprhhovhhiuggvrhhsrdhnvghtnecuvehluhhsthgvrh fuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhephhhisegrlhihshhsrgdrihhs pdhnsggprhgtphhtthhopedvpdhmohguvgepshhmthhpohhuthdprhgtphhtthhopeguvg hmihhosggvnhhouhhrsehgmhgrihhlrdgtohhmpdhrtghpthhtohepuggvvhgvlhesshhp vggtthhruhhmqdhoshdrohhrgh X-ME-Proxy: Feedback-ID: i12284293:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 14 Nov 2025 07:14:29 -0500 (EST) Received: by fw12.qyliss.net (Postfix, from userid 1000) id C38EC19CE3FE; Fri, 14 Nov 2025 13:14:28 +0100 (CET) From: Alyssa Ross To: Demi Marie Obenour Subject: Re: [PATCH v2 6/8] Support updates via systemd-sysupdate In-Reply-To: <154426ab-f42b-4c5e-9f0e-8a91abbe7596@gmail.com> References: <20251112-updates-v2-0-88d96bf81b79@gmail.com> <20251112-updates-v2-6-88d96bf81b79@gmail.com> <87tsyxc26t.fsf@alyssa.is> <154426ab-f42b-4c5e-9f0e-8a91abbe7596@gmail.com> Date: Fri, 14 Nov 2025 13:14:26 +0100 Message-ID: <877bvslskd.fsf@alyssa.is> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Message-ID-Hash: OTBEXO3AAGMHVRBXQ4BVGPONYKKLDB22 X-Message-ID-Hash: OTBEXO3AAGMHVRBXQ4BVGPONYKKLDB22 X-MailFrom: hi@alyssa.is X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-devel.spectrum-os.org-0; header-match-devel.spectrum-os.org-1; header-match-devel.spectrum-os.org-2; header-match-devel.spectrum-os.org-3; header-match-devel.spectrum-os.org-4; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Spectrum OS Development X-Mailman-Version: 3.3.9 Precedence: list List-Id: Patches and low-level development discussion Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Demi Marie Obenour writes: > On 11/13/25 11:44, Alyssa Ross wrote: >> Demi Marie Obenour writes: >>=20 >>> +# 9. Call systemd-sysupdate to run the actual update. >>> + >>> +if { mkdir -p -m 0700 /run/updater } >>> +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= } >>> + >>=20 >> Wouldn't it break if there's already stuff in it? > > No, it works fine in this case. I checked :). > >> I'd do >>=20 >> foreground { redirfd -w 2 /dev/null btrfs subvolume delete -- shared } >> if { btrfs subvolume create -- shared } >>=20 >> and then you know you've got an empty subvolume. > > An empty subvolume isn't good: it means that systemd-sysupdate will > redownload an update even when it isn't needed. When would the update have already been downloaded but not applied? Only if there's an error in actually installing it? I'd feel happier knowing we were always starting with a clean state. >>> + # Snapshot directory may have files or directories with untrusted na= mes. >>> + # Redirect its output to /dev/null to avoid printing them to the con= sole. >>> + ifelse -n { redirfd -w 2 /dev/null rm -rf -- snapshot } { >>> + foreground { redirfd -w 2 echo "Cannot remove snapshot directory" } >>> + exit 1 >>> + } >>=20 >> Why not btrfs subvolume delete? It's faster and won't print names. > > It doesn't distinguish "subvolume doesn't exist" from "problem > deleting subvolume". A better solution is to call `rm -f` if `btrfs > subvolume delete` failed. That ignores "does not exist" errors, > but not other errors. Right. I had assumed btrfs subvolume create would cause an error if it already existed, but you've said it doesn't. >>> + >>> + 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_} >>=20 >> 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 } >>> + } >>> + >>> + # $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 ${fsdi= r}/etc } >>> + if { cp -- /etc/systemd/import-pubring.gpg ${fsdir}/etc/systemd } >>=20 >> 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. >>> + >>> + # If the directory is already mounted, unmount it. This prevents a >>> + # confusing error from mount. >>> + foreground { redirfd -w 2 /dev/null umount -- ${fsdir}/updates } >>> + >>> + # Share the update directory with the VM. >>> + if { mount --bind -- shared ${fsdir}/updates } >>> + >>> + # Start the update VM. >>> + if { vm-start $update_vm_id } >>> + >>> + # Wait for the VM to exit. >>> + if { s6-svwait -D ${svcdir} } >>> + >>=20 >> It might be more robust to use a transient VM, like we use for >> AppImages, so that nothing can restart it. Transient VMs are still >> developing though, so it's also fine to say we'll do it this way for now >> and adapt it later. This would also save all the filesystem resetting >> you're needing to do here. > > The path to the update directory is user-provided. It's not from > the VM's persistent storage. Ideally (at some point) it isn't user provided, and is the VM's transient disk-backed storage, IMO. >>> @@ -17,5 +18,11 @@ let >>> callConfig =3D config: if builtins.typeOf config =3D=3D "lambda" the= n config { >>> inherit default; >>> } else config; >>> + finalConfig =3D default // callConfig config; >>> in >>> - default // callConfig config; >>> + finalConfig // { >>> + update-signing-key =3D builtins.path { >>> + name =3D "signing-key"; >>> + path =3D finalConfig.update-signing-key; >>> + }; >>> + } >>=20 >> 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. >>> diff --git a/vm/app/updates.nix b/vm/app/updates.nix >>> new file mode 100644 >>> index 0000000000000000000000000000000000000000..d2c1e5fcb35b37c7ed8a173= f19b97894a36a7f0c >>> --- /dev/null >>> +++ b/vm/app/updates.nix >>> @@ -0,0 +1,37 @@ >>> +# SPDX-License-Identifier: MIT >>> +# SPDX-FileCopyrightText: 2023 Alyssa Ross >>> +# SPDX-FileCopyrightText: 2025 Demi Marie Obenour >>> + >>> +import ../../lib/call-package.nix ( >>> +{ callSpectrumPackage, config, curl, lib, src >>> +, runCommand, systemd, writeScript >>> +}: >>> + >>> +let >>> + update-url =3D config.update-url; >>> + mountpoint =3D "/run/virtiofs/virtiofs0"; >>> + sysupdate-path =3D "${systemd}/lib/systemd/systemd-sysupdate"; >>> + runner =3D writeScript "update-run-script" >>> + '' >>> + #!/usr/bin/execlineb -P >>> + if { mount -toverlay -olowerdir=3D${mountpoint}/etc:/etc -- overla= y /etc } >>> + envfile ${mountpoint}/etc/url-env >>=20 >> 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. >>> + importas -i update_url UPDATE_URL >>> + if { ${sysupdate-path} update } >>> + if { ${curl}/bin/curl -L --proto =3Dhttp,https >>> + -o ${mountpoint}/updates/SHA256SUMS.gpg ''${update_url}/SHA256S= UMS.gpg } >>> + # systemd-sysupdate recently went from needing SHA256SUMS.gpg to S= HA256SUMS.sha256.asc. >>> + # I (Demi) have no need if this is intentional or a bug. I also h= ave no idea if this >>> + # behavior will stay unchanged in the future. Therefore, create b= oth files and let >>> + # systemd-sysupdate ignore the one it isn't interested in. >>> + if { ln -f ${mountpoint}/updates/SHA256SUMS.gpg ${mountpoint}/upda= tes/SHA256SUMS.sha256.asc } >>=20 >> 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"? >>> + ${curl}/bin/curl -L --proto =3Dhttp,https >>> + -o ${mountpoint}/updates/SHA256SUMS ''${update_url}/SHA256SUMS >>> + ''; >>> +in >>> + >>> +callSpectrumPackage ../make-vm.nix {} { >>> + providers.net =3D [ "sys.netvm" ]; >>> + type =3D "nix"; >>> + run =3D "${runner}"; >>=20 >> Might as well inline this. > > I chose to keep it separate to improve readability. Okay. I'd find it more readable inlined but it's your code. :) --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEARYKAB0WIQQGoGac7QfI+H5ZtFCZddwkt31pFQUCaRcdIgAKCRCZddwkt31p FfcvAQCIGsq/PzbcdAakrggOto4bIcjpg35NIWQYoS8OKyusLQEAjQPyanvggBBu Zvex1o68ZyOJdO8e4xQW1KfDBV+yigU= =iUzU -----END PGP SIGNATURE----- --=-=-=--