Re: [PATCH v4 06/14] Support generating multiple partition UUIDs

Demi Marie Obenour
2025-11-26T18:27:12Z

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

On 11/25/25 08:02, Alyssa Ross wrote:
> Demi Marie Obenour <demiobenour@gmail.com> writes:
>
>> Generate 4 partition UUIDs instead of just 2. Port
>> scripts/format-uuid.sh to awk to make this much easier.
>
> Would have been nice to see the awk port first with unchanged behaviour,
> so it was then easier to see what behaviour was actually getting changed
> here.
>
>> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
>> ---
>> Changes since v2:
>> - Split into separate commit.
>> ---
>> host/initramfs/Makefile | 8 +++++---
>> host/rootfs/Makefile | 6 ++++--
>> release/live/Makefile | 8 +++++---
>> release/live/default.nix | 2 +-
>> scripts/format-uuid.awk | 35 +++++++++++++++++++++++++++++++++++
>> scripts/format-uuid.sh | 19 -------------------
>> 6 files changed, 50 insertions(+), 28 deletions(-)
>
> No blockers in here, but there are improvements I'd like to see made at
> some point, even if that's after this has been applied.
>
>> diff --git a/host/initramfs/Makefile b/host/initramfs/Makefile
>> index fd8cbb6c3e775ed27d0a524bf167cb4d3940d799..27a26b46a8110d35ee02a63b12931d6b9c2742e5 100644
>> --- a/host/initramfs/Makefile
>> +++ b/host/initramfs/Makefile
>> @@ -35,10 +35,12 @@ build/mountpoints:
>> cd build/mountpoints && mkdir -p $(MOUNTPOINTS)
>> find build/mountpoints -mindepth 1 -exec touch -d @0 {} ';'
>>
>> -build/live.img: ../../scripts/format-uuid.sh ../../scripts/make-gpt.sh ../../scripts/sfdisk-field.awk $(ROOT_FS_IMAGES)
>> +build/live.img: ../../scripts/format-uuid.awk ../../scripts/make-gpt.sh ../../scripts/sfdisk-field.awk build/boot.fat $(ROOT_FS_IMAGES)
>> + uuids=$$(awk -f ../../scripts/format-uuid.awk < $(ROOT_FS_VERITY_ROOTHASH)) && \
>> + set -euo pipefail -- $$uuids && \
>
> Nice trick — I like it. There's no need to set -eo pipefail on Make
> commands that don't use ; or | though. If we did that consistently our
> Makefiles would be unreadable because it'd be hard to see past all the
> sets to the actual functionality, and seeing this here when it's not set
> on every command makes me wonder what's special about this one, which
> turns out to be nothing.

Will change in v5.

>> bash ../../scripts/make-gpt.sh $@.tmp \
>> - $(ROOT_FS_VERITY):verity:$$(../../scripts/format-uuid.sh "$$(dd if=$(ROOT_FS_VERITY_ROOTHASH) bs=32 skip=1 count=1 status=none)") \
>> - $(ROOT_FS):root:$$(../../scripts/format-uuid.sh "$$(head -c 32 $(ROOT_FS_VERITY_ROOTHASH))")
>> + $(ROOT_FS_VERITY):verity:$$3 \
>> + $(ROOT_FS):root:$$1
>> mv $@.tmp $@
>>
>> build/loop.tar: build/live.img
>> diff --git a/host/rootfs/Makefile b/host/rootfs/Makefile
>> index fcfbc3e437fdb108252ba77d4d4e8f4f636ffd78..f02bb76371f000e3f65bb7c2a7f217d437845481 100644
>> --- a/host/rootfs/Makefile
>> +++ b/host/rootfs/Makefile
>> @@ -90,9 +90,11 @@ clean:
>> .PHONY: clean
>>
>> build/live.img: ../../scripts/format-uuid.sh ../../scripts/make-gpt.sh ../../scripts/sfdisk-field.awk $(ROOT_FS_DIR)/verity-timestamp $(ROOT_FS)
>> + uuids=$$(awk -f ../../scripts/format-uuid.awk < $(ROOT_FS_VERITY_ROOTHASH)) && \
>> + set -euo pipefail -- $$uuids && \
>> bash ../../scripts/make-gpt.sh $@.tmp \
>> - $(ROOT_FS)/rootfs.verity.superblock:verity:$$(../../scripts/format-uuid.sh "$$(dd if=$(ROOT_FS_VERITY_ROOTHASH) bs=32 skip=1 count=1 status=none)") \
>> - $(ROOT_FS)/rootfs:root:$$(../../scripts/format-uuid.sh "$$(head -c 32 $(ROOT_FS_VERITY_ROOTHASH)")
>> + $(ROOT_FS_VERITY):verity:$$3 \
>> + $(ROOT_FS):root:$$1
>> mv $@.tmp $@
>>
>> debug:
>> diff --git a/release/live/Makefile b/release/live/Makefile
>> index a79947c57d562677760bc669c66320953a2b0d2d..78361a48512a37514ba0e57e0cc8b0ec3a71664b 100644
>> --- a/release/live/Makefile
>> +++ b/release/live/Makefile
>> @@ -9,11 +9,13 @@ DTBS ?= build/empty
>>
>> dest = build/live.img
>>
>> -$(dest): ../../scripts/format-uuid.sh ../../scripts/make-gpt.sh ../../scripts/sfdisk-field.awk build/boot.fat $(ROOT_FS_IMAGES)
>> +$(dest): ../../scripts/format-uuid.awk ../../scripts/make-gpt.sh ../../scripts/sfdisk-field.awk build/boot.fat $(ROOT_FS_IMAGES)
>> + uuids=$$(awk -f ../../scripts/format-uuid.awk < $(ROOT_FS_VERITY_ROOTHASH)) && \
>> + set -euo pipefail -- $$uuids && \
>> bash ../../scripts/make-gpt.sh $@.tmp \
>> build/boot.fat:c12a7328-f81f-11d2-ba4b-00a0c93ec93b \
>> - $(ROOT_FS_VERITY):verity:$$(../../scripts/format-uuid.sh "$$(dd if=$(ROOT_FS_VERITY_ROOTHASH) bs=32 skip=1 count=1 status=none)") \
>> - $(ROOT_FS):root:$$(../../scripts/format-uuid.sh "$$(head -c 32 $(ROOT_FS_VERITY_ROOTHASH))")
>> + $(ROOT_FS_VERITY):verity:$$3 \
>> + $(ROOT_FS):root:$$1
>> mv $@.tmp $@
>>
>> build/empty:
>> diff --git a/release/live/default.nix b/release/live/default.nix
>> index ac2d7a55fd4fe0c02108309ecea20e368000af0d..98cb4862e239e3ad9ddbd7b5ace5716f57df683b 100644
>> --- a/release/live/default.nix
>> +++ b/release/live/default.nix
>> @@ -29,7 +29,7 @@ stdenv.mkDerivation {
>> fileset = lib.fileset.intersection src (lib.fileset.unions [
>> ./.
>> ../../lib/common.mk
>> - ../../scripts/format-uuid.sh
>> + ../../scripts/format-uuid.awk
>> ../../scripts/make-gpt.sh
>> ../../scripts/sfdisk-field.awk
>> ]);
>> 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)");
>> + }
>
> This doesn't really feel like the right place for this check? If
> duplicate UUIDs are broken that should be detected when assembling the
> partition table.

Will drop in v5.

>> + 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";
>
> RS is \n by default.

Will fix in v5.

>> + if ((getline) != 1)
>
> No need for the inner parens.

Will fix in v5.

>> + fail("Empty input file");
>> + roothash = $0;
>> + if (roothash !~ /^[a-f0-9]{64}$/)
>> + fail("Invalid root hash");
>> + if (getline)
>> + fail("Junk after root hash");
>
> This will only ever be used by our build system. It'll be readily
> apparent if it's providing junk even without these validations, and
> without them the script would be shorter and easier to fit in my brain.

Will fix in v5.

>> + found_so_far[""] = "";
>
> What does this do? Won't this just cause the script to falsely claim an
> empty string is a duplicate UUID if it ever ends up being passed to
> format_uuid()?

That can't happen :).

>> + for (i = 1; i != 49; i += 16) {
>> + format_uuid(substr($0, i, 32));
>> + }
>
> Either unrolling the loop, or using a modulo inside the loop and getting
> rid of the format_uuid() call afterwards, would have helped me figure
> out what this does a lot quicker, but I see now that we're generating
> the two UUIDs we had before, and then two additional UUIDs offset into
> the string.

I did this because it is easy to implement.

> I guess this is something to do with having multiple copies of the OS
> installed, but I wish I understood how the offset ones will be used
> without having to go hunting in later patches and come back here
> afterwards.
>
>> + format_uuid(substr($0, 49, 16) substr($0, 1, 16));
>> +}

These GUIDs aren't booted ever. They are used for the blank backup
partitions. They just need to be deterministic and not collide with
anything a user might use. The verity hash was a convenient source
for this.

A fixed GUID would work too.
--
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 --]