Re: [PATCH v2 4/8] Adjust partition layout to support updates

Alyssa Ross
2025-11-13T16:01:03Z

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

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

> systemd-sysupdate has strict requirements on the partition layout:
>
> - The label of the active partition must match the template in the
> .transfer file. For instance, the root filesystem of Spectrum 0.0.0
> will be in a partition with label Spectrum_0.0.0.
> - The label of the inactive partition must either be that of the old
> version of Spectrum or "_empty". The former indicates an incomplete
> update.

Do you mean "the latter"?

> - The partition type UUID must conform to the Discoverable Partition
> Specification.
>
> After installing an image to a partition, systemd-sysupdate updates the
> label of the partition to match the image's version. However, it does
> not update the partition UUID. Therefore, use the partition label, not
> the partition UUID, to find the root filesystem and its verity metadata.

Seems a bit odd, considering I got this trick from
systemd-veritysetup-generator. Is that not compatible with
systemd-sysupdate?

> systemd-sysupdate will fail if the OS image does not fit in the
> partitions that the installer created. Therefor, make the partitions
> very large so that there is plenty of room for the OS to grow. This
> requires rewriting the code that calculates the partition sizes.
>
> Since the partition label includes the OS version, add an OS version
> number. Use 0.0.0 to indicate that Spectrum OS is still in very early
> development and should not be used. The version number can be
> overridden in the build configuration file.
>
> mkfs.ext4 is not able to produce images with files large enough to hold
> both the primary and backup copy of the root partition [1]. Reducing
> the sizes of partitions to be little greater than the size of the root
> filesystem image does not help. The produced file is still too large.
> Therefore, compress the image, which causes it to be small enough that
> mkfs.ext4 can handle it. This breaks the live image, so remove it.
> The live image will return once Spectrum switches to the GNOME OS
> installer [2].
>
> [1]: https://github.com/tytso/e2fsprogs/issues/254
> [2]: https://spectrum-os.org/lists/archives/spectrum-devel/87wm4dlkhz.fsf@alyssa.is/
>
> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
> ---
> Documentation/development/build-configuration.adoc | 11 ++++
> host/efi.nix | 5 +-
> host/initramfs/Makefile | 12 ++--
> host/initramfs/default.nix | 1 +
> host/initramfs/etc/init | 17 ++---
> host/initramfs/etc/probe | 20 +++---
> host/initramfs/shell.nix | 2 +
> host/rootfs/Makefile | 21 ++++---
> host/rootfs/default.nix | 1 +
> host/rootfs/shell.nix | 2 +
> img/app/Makefile | 2 +-
> img/app/default.nix | 1 +
> lib/config.default.nix | 1 +
> lib/config.nix | 3 +-
> lib/kcmdline-utils.mk | 5 ++
> release/checks/integration/try.c | 4 ++
> release/checks/no-roothash.nix | 2 +-
> release/combined/eosimages.nix | 14 +++--
> release/combined/grub.cfg.in | 5 --
> release/live/Makefile | 9 +--
> release/live/default.nix | 8 ++-
> release/live/shell.nix | 3 +-
> scripts/format-uuid.awk | 35 +++++++++++
> scripts/make-gpt.bash | 72 ++++++++++++++++++++++
> scripts/make-gpt.sh | 67 +-------------------
> scripts/make-live-image.sh | 43 +++++++++++++
> scripts/sfdisk-field.awk | 3 +-
> vm/sys/net/Makefile | 2 +-
> vm/sys/net/default.nix | 1 +
> 29 files changed, 248 insertions(+), 124 deletions(-)

Would you mind splitting up this patch in your next submission? This is
a lot to review all at once, and I doubt finding the rootfs from label
is very difficult to separate from changing the size of the partitions,
for example. I don't think I'm going to be able to do a full, thorough
review with there being this many different things going on at once.

> diff --git a/Documentation/development/build-configuration.adoc b/Documentation/development/build-configuration.adoc
> index 545aa8c05ac40a101b5ee280015cde7ec4f3a66f..0659d104efeeb8f483c24d8ea8d38a5d928d9358 100644
> --- a/Documentation/development/build-configuration.adoc
> +++ b/Documentation/development/build-configuration.adoc
> @@ -40,3 +40,14 @@ for supported configuration attributes and their default values.
> };
> }
> ----
> +
> +.config.nix to adjust the version of the OS
> +[example]
> +[source,nix]
> +----
> +{ default, ... }:
> +
> +{
> + version = "0.0.1";
> +}
> +----

Not sure this is necessary. The paragraph above already says to refer
to lib/config.default.nix to see supported configuration options, so
readers can learn about version there. It's not like pkgsArgs where
it's quite complex to use so warrants additional handholding.

> diff --git a/host/initramfs/etc/probe b/host/initramfs/etc/probe
> index 4cbd00db52c1a7128b5c619a43d415675feaee0b..11a81c9be8f1adaef3cee17efdba1eb80e9fe3c7 100755
> --- a/host/initramfs/etc/probe
> +++ b/host/initramfs/etc/probe
> @@ -14,9 +14,13 @@ if -n {
> forx -pE module { ext4 loop }
> modprobe $module
> }
> - backtick -E uuid { lsblk -lnpo PARTUUID $mdev }
> + backtick uuid { lsblk -lnpo PARTUUID $mdev }
> + multisubstiute {
> + define mdev_ $mdev
> + importas -Si uuid
> + }
> if { mkdir -p /mnt/${uuid} }
> - if { mount $mdev /mnt/${uuid} }
> + if { mount $mdev_ /mnt/${uuid} }
> find /mnt/${uuid} -name *.img -exec
> losetup -Pf {}
> ;

I don't understand what this change does.

> @@ -24,11 +28,13 @@ if -n {
>
> # Check whether we now have all the partitions we need to boot.
>
> -importas -i rootfs_uuid ROOTFS_UUID
> -importas -i verity_uuid VERITY_UUID
> -
> -backtick -E rootfs_dev { findfs PARTUUID=${rootfs_uuid} }
> -backtick -E verity_dev { findfs PARTUUID=${verity_uuid} }
> +importas -i version x-spectrum-version
> +backtick rootfs_dev { findfs PARTLABEL=Spectrum_${version} }
> +backtick verity_dev { findfs PARTLABEL=Spectrum_${version}.verity }
> +multisubstitute {
> + importas -iS rootfs_dev
> + importas -iS verity_dev
> +}
>
> if { ln -s $rootfs_dev /dev/rootfs }
> if { ln -s $verity_dev /dev/verity }

Using multisubstitute is a good change but should be a separate patch
since it's equally applicable to the current code as it is to yours.

> diff --git a/lib/config.nix b/lib/config.nix
> index e437cdbe9aa22dd0f9c8d7052ac331c8fccf6ce6..01bcfa2bb2d5c412e212f5a60d9032e89c8a7442 100644
> --- a/lib/config.nix
> +++ b/lib/config.nix
> @@ -18,5 +18,4 @@ let
> inherit default;
> } else config;
> in
> -
> -default // callConfig config
> + default // callConfig config;

Looks like an accidental inclusion.

> diff --git a/lib/kcmdline-utils.mk b/lib/kcmdline-utils.mk
> new file mode 100644
> index 0000000000000000000000000000000000000000..5ed97c1a4b0c93d427fbb67f58736eee7fe09259
> --- /dev/null
> +++ b/lib/kcmdline-utils.mk
> @@ -0,0 +1,5 @@
> +# SPDX-License-Identifier: EUPL-1.2+
> +# SPDX-FileCopyrightText: 2021-2024 Alyssa Ross <hi@alyssa.is>
> +# SPDX-FileCopyrightText: 2025 Demi Marie Obenour <demiobenour@gmail.com>
> +
> +LIVE_IMAGE_DEPS = ../../scripts/format-uuid.awk ../../scripts/make-gpt.sh ../../scripts/make-gpt.bash ../../scripts/sfdisk-field.awk ../../scripts/make-live-image.sh ../../lib/kcmdline-utils.mk

Why is this called kcmdline-utils.mk? It doesn't seem to have anything
to do with a kernel command line.

> diff --git a/release/checks/integration/try.c b/release/checks/integration/try.c
> index 4b874c0a7e9b48324497450fb5488e04576fd43b..c34b582230f75ff3374446468d2461a78c0099a6 100644
> --- a/release/checks/integration/try.c
> +++ b/release/checks/integration/try.c
> @@ -10,6 +10,10 @@ void test(struct config c)
> {
> struct vm *vm;
>
> + // Spectrum's live image doesn't work right now.
> + // Mark the test as skipped.
> + exit(77);
> +
> c.drives.img = getenv_or_die("COMBINED_PATH");
>
> vm = start_qemu(c);

We can just delete the test.

> diff --git a/release/combined/eosimages.nix b/release/combined/eosimages.nix
> index 0ac4c48374e7098a2b91f61fc07cebb2042ffbdc..ba44d9cd82d55d491293ed36cc0402db8ebd3ffe 100644
> --- a/release/combined/eosimages.nix
> +++ b/release/combined/eosimages.nix
> @@ -12,11 +12,15 @@ runCommand "eosimages.img" {
> unsafeDiscardReferences = { out = true; };
> dontFixup = true;
> } ''
> + set -o pipefail
> mkdir dir
> cd dir
> - ln -s $image $imageName
> - sha256sum $imageName > $imageName.sha256
> - tar -chf $NIX_BUILD_TOP/eosimages.tar *
> - tar2ext4 -i $NIX_BUILD_TOP/eosimages.tar -o $out
> - e2label $out eosimages
> + ln -s -- "$image" "$imageName"
> + sha256sum -- "$imageName" > "$imageName.sha256" &
> + pid=$!
> + gzip -9 < "$image" > "$imageName.gz"
> + sha256sum -- "$imageName.gz" > "$imageName.gz.sha256"
> + wait "$pid"
> + tar -ch -- "$imageName.gz" "$imageName.gz.sha256" "$imageName.sha256" | tar2ext4 -o "$out"
> + e2label "$out" eosimages
> '') (_: {})

My comments on this from last time still apply[1].

[1]: https://spectrum-os.org/lists/archives/spectrum-devel/87v7jyj5a3.fsf@alyssa.is

> diff --git a/scripts/format-uuid.awk b/scripts/format-uuid.awk
> new file mode 100644
> index 0000000000000000000000000000000000000000..a5349d68a4d29be5f750650236420c9b5a7257eb
> --- /dev/null
> +++ b/scripts/format-uuid.awk
> @@ -0,0 +1,35 @@
> +# SPDX-License-Identifier: EUPL-1.2+
> +# SPDX-FileCopyrightText: 2025 Demi Marie Obenour <demiobenour@gmail.com>
> +function format_uuid(arg) {
> + if (arg in found_so_far) {
> + fail("Duplicate UUID, try changing the image (by even 1 bit)");
> + }
> + found_so_far[arg] = 1;
> + print (substr(arg, 1, 8) "-" \
> + substr(arg, 9, 4) "-" \
> + substr(arg, 13, 4) "-" \
> + substr(arg, 17, 4) "-" \
> + substr(arg, 21, 12));
> +}
> +
> +function fail(msg) {
> + print msg > "/dev/stderr";
> + exit 1;
> +}
> +
> +BEGIN {
> + FS = "";
> + RS = "\n";
> + if ((getline) != 1)
> + fail("Empty input file");
> + roothash = $0;
> + if (roothash !~ /^[a-f0-9]{64}$/)
> + fail("Invalid root hash");
> + if (getline)
> + fail("Junk after root hash");
> + found_so_far[""] = "";
> + for (i = 1; i != 49; i += 16) {
> + format_uuid(substr($0, i, 32));
> + }
> + format_uuid(substr($0, 49, 16) substr($0, 1, 16));
> +}

So now we have two format-uuid scripts, one in sh and one in awk? Why?
What was wrong with the sh one?

> diff --git a/scripts/make-gpt.bash b/scripts/make-gpt.bash
> new file mode 100644
> index 0000000000000000000000000000000000000000..f9d53817e3cc4342cac5d4c832cf4aa129880399
> --- /dev/null
> +++ b/scripts/make-gpt.bash
> @@ -0,0 +1,72 @@
> +#!/usr/bin/bash --
> +# SPDX-FileCopyrightText: 2021-2023 Alyssa Ross <hi@alyssa.is>
> +# SPDX-FileCopyrightText: 2022 Unikie
> +# SPDX-License-Identifier: EUPL-1.2+
> +#
> +# usage: make-gpt.sh GPT_PATH PATH:PARTTYPE[:PARTUUID[:PARTLABEL]]...
> +
> +set -xeuo pipefail
> +ONE_MiB=1048576
> +
> +# Prints the number of 1MiB blocks required to store the file named
> +# $1. We use 1MiB blocks because that's what sfdisk uses for
> +# alignment. It would be possible to get a slightly smaller image
> +# using actual normal-sized 512-byte blocks, but it's probably not
> +# worth it to configure sfdisk to do that.
> +sizeMiB() {
> + wc -c "$1" | awk -v ONE_MiB=$ONE_MiB \
> + '{printf "%d\n", ($1 + ONE_MiB - 1) / ONE_MiB}'
> +}
> +
> +# Copies from path $3 into partition number $2 in partition table $1.
> +fillPartition() {
> + start="$(sfdisk -J "$1" | jq -r --argjson index "$2" \
> + '.partitiontable.partitions[$index].start * 512')"
> +
> + # GNU cat will use copy_file_range(2) if possible, whereas dd
> + # will always do a userspace copy, which is significantly slower.
> + lseek -S 1 "$start" cat "$3" 1<>"$1"
> +}
> +
> +# Prints the partition path from a PATH:PARTTYPE[:PARTUUID[:PARTLABEL]] string.
> +partitionPath() {
> + awk -F: '{print $1}' <<EOF
> +$1
> +EOF
> +}
> +
> +scriptsDir="$(dirname "$0")"
> +
> +out="$1"
> +shift
> +
> +table="label: gpt"
> +
> +# Keep 1MiB free at the start, and 1MiB free at the end.
> +gptBytes=$((ONE_MiB * 2))
> +for partition; do
> + if [[ "$partition" =~ :([1-9][0-9]*)MiB$ ]]; then
> + sizeMiB=${BASH_REMATCH[1]}
> + partition=${partition%:*}
> + else
> + partitionPath=$(partitionPath "$partition")
> + sizeMiB=$(sizeMiB "$partitionPath")
> + fi

Would be a lot simpler to just multiply by 1024 * 1024 in whatever runs
this script, wouldn't it?

> diff --git a/scripts/make-gpt.sh b/scripts/make-gpt.sh
> index 96f0d2c8494c093558c0e32e7e920b569bb078ef..665057da8281d2b5282081e4999098fbaa29e6ca 100755
> --- a/scripts/make-gpt.sh
> +++ b/scripts/make-gpt.sh
> @@ -1,65 +1,4 @@
> -#!/bin/sh -eu
> -#
> -# SPDX-FileCopyrightText: 2021-2023 Alyssa Ross <hi@alyssa.is>
> -# SPDX-FileCopyrightText: 2022 Unikie
> +#!/bin/sh --
> # SPDX-License-Identifier: EUPL-1.2+
> -#
> -# usage: make-gpt.sh GPT_PATH PATH:PARTTYPE[:PARTUUID[:PARTLABEL]]...
> -
> -ONE_MiB=1048576
> -
> -# Prints the number of 1MiB blocks required to store the file named
> -# $1. We use 1MiB blocks because that's what sfdisk uses for
> -# alignment. It would be possible to get a slightly smaller image
> -# using actual normal-sized 512-byte blocks, but it's probably not
> -# worth it to configure sfdisk to do that.
> -sizeMiB() {
> - wc -c "$1" | awk -v ONE_MiB=$ONE_MiB \
> - '{printf "%d\n", ($1 + ONE_MiB - 1) / ONE_MiB}'
> -}
> -
> -# Copies from path $3 into partition number $2 in partition table $1.
> -fillPartition() {
> - start="$(sfdisk -J "$1" | jq -r --argjson index "$2" \
> - '.partitiontable.partitions[$index].start * 512')"
> -
> - # GNU cat will use copy_file_range(2) if possible, whereas dd
> - # will always do a userspace copy, which is significantly slower.
> - lseek -S 1 "$start" cat "$3" 1<>"$1"
> -}
> -
> -# Prints the partition path from a PATH:PARTTYPE[:PARTUUID[:PARTLABEL]] string.
> -partitionPath() {
> - awk -F: '{print $1}' <<EOF
> -$1
> -EOF
> -}
> -
> -scriptsDir="$(dirname "$0")"
> -
> -out="$1"
> -shift
> -
> -nl='
> -'
> -table="label: gpt"
> -
> -# Keep 1MiB free at the start, and 1MiB free at the end.
> -gptBytes=$((ONE_MiB * 2))
> -for partition; do
> - sizeMiB="$(sizeMiB "$(partitionPath "$partition")")"
> - table="$table${nl}size=${sizeMiB}MiB,$(awk -f "$scriptsDir/sfdisk-field.awk" -v partition="$partition")"
> - gptBytes="$((gptBytes + sizeMiB * ONE_MiB))"
> -done
> -
> -rm -f "$out"
> -truncate -s "$gptBytes" "$out"
> -sfdisk --no-reread --no-tell-kernel "$out" <<EOF
> -$table
> -EOF
> -
> -n=0
> -for partition; do
> - fillPartition "$out" "$n" "$(partitionPath "$partition")"
> - n="$((n + 1))"
> -done
> +# SPDX-FileCopyrightText: 2025 Demi Marie Obenour <demiobenour@gmail.com>
> +exec bash -- "${0%.sh}.bash" "$@"

Not sure what the purpose of this is. We don't need to keep old paths
working.

> diff --git a/scripts/make-live-image.sh b/scripts/make-live-image.sh
> new file mode 100755
> index 0000000000000000000000000000000000000000..6608cc35b7a15178adf5ff3d3917b5243c5da6cd
> --- /dev/null
> +++ b/scripts/make-live-image.sh
> @@ -0,0 +1,43 @@
> +#!/bin/sh --
> +# SPDX-License-Identifier: EUPL-1.2+
> +# SPDX-FileCopyrightText: 2021-2024 Alyssa Ross <hi@alyssa.is>
> +# SPDX-FileCopyrightText: 2025 Demi Marie Obenour <demiobenour@gmail.com>
> +set -euo pipefail
> +case $0 in
> +(/*) dir=${0%/*}/;;
> +(*/*) dir=./${0%/*};;
> +(*) dir=.;;
> +esac
> +usage () {
> + echo 'Usage: make-live-image.sh [release|live] OUTPUT_FILE' >&2
> + exit 1
> +}
> +if [ "$#" != 2 ]; then usage; fi
> +file_type=$1 output_file=$2
> +for i in "$ROOT_FS" "$ROOT_FS_VERITY" "$ROOT_FS_VERITY_ROOTHASH" "$VERSION"; do
> + # Some characters not special to the shell can't be handled by this code.
> + case $i in
> + (-*|*[!A-Za-z0-9._/+@-]*) printf 'Forbidden characters in "%s"\n' "$i" >&2; exit 1;;
> + esac
> +done
> +root_hashes=$(LC_ALL=C awk -f "${dir}/format-uuid.awk" < "$ROOT_FS_VERITY_ROOTHASH")
> +# The awk script produces output that is meant for field splitting
> +# and has no characters special for globbing.
> +# shellcheck disable=SC2086
> +set -- $root_hashes
> +case $file_type in
> +(release)
> + "$dir/make-gpt.sh" "$output_file.tmp" \
> + build/boot.fat:c12a7328-f81f-11d2-ba4b-00a0c93ec93b \
> + "$ROOT_FS_VERITY:verity:$1:Spectrum_$VERSION.verity:1024MiB" \
> + "$ROOT_FS:root:$2:Spectrum_$VERSION:20480MiB" \
> + "/dev/null:verity:$3:_empty:1024MiB" \
> + "/dev/null:root:$4:_empty:20480MiB"
> + ;;
> +(live)
> + "$dir/make-gpt.sh" "$output_file.tmp" \
> + "$ROOT_FS_VERITY:verity:$1:Spectrum_$VERSION.verity" \
> + "$ROOT_FS:root:$2:Spectrum_$VERSION";;
> +(*) usage;;
> +esac
> +mv -- "$output_file.tmp" "$output_file"

If we need separate modes for each caller anyway, I don't think there's
having them in a shared script is a win over having them local to the
place they're used.

> diff --git a/scripts/sfdisk-field.awk b/scripts/sfdisk-field.awk
> index e13c86d2fb11a066eebd043808e659b08dbd269c..72eec9a0a770563d32da14440fe2552eb2e39b68 100644
> --- a/scripts/sfdisk-field.awk
> +++ b/scripts/sfdisk-field.awk
> @@ -24,6 +24,7 @@ BEGIN {
> arch = _arch
> }
>
> + comma = ""
> for (n in fields) {
> if (n <= skip)
> continue
> @@ -33,6 +34,6 @@ BEGIN {
> fields[n] = uuid
> }
>
> - printf "%s=%s,", keys[n - skip], fields[n]
> + printf ",%s%s=%s", comma, keys[n - skip], fields[n]
> }
> }

I don't understand. The comma variable is always empty?

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