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

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

[-- 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 --]