mantle/kola/qemu: Add test for DPS UUID

[Johan-Liebert1]

#4380 makes sure that partitions are created according to the Discoverable Partition Specification. This adds test for the same

[gemini-code-assist]

Code Review

This pull request adds a new test to verify that partitions are created with the correct Discoverable Partition Specification (DPS) UUIDs. The changes look good overall, but I have a couple of suggestions to improve robustness and avoid a potential bug related to loop variable scoping. One suggestion is to handle unsupported architectures more gracefully during partition UUID lookup. The other is to refactor a for...range loop to avoid taking the address of a loop variable, which is a common pitfall in Go.

[gemini-code-assist]

You are taking the address of a loop variable child inside the for...range loop (e.g., &child). In Go, the loop variable is reused for each iteration. If dpsFatalFail were ever changed to not be fatal, this would lead to a subtle bug where it would always operate on the last element of the slice.

While it's not a bug currently because dpsFatalFail calls c.Fatalf, it's a dangerous pattern. It's better to iterate using an index to get a stable pointer to the slice element.

for i := range lsblk.Blockdevices[0].Children {
		child := &lsblk.Blockdevices[0].Children[i]
		if child.Label == nil {
			continue
		}

		if child.PartType == nil {
			c.Fatalf("'%s' has no parttype", child.Name)
		}

		switch *child.Label {
		case "boot":
			if strings.ToUpper(*child.PartType) != XBootldr {
				dpsFatalFail(c, child, XBootldr)
			}

		case "root":
			rootFound = true
			if strings.ToUpper(*child.PartType) != RootPartition {
				dpsFatalFail(c, child, RootPartition)
			}

		case "EFI-SYSTEM":
			espFound = true
			if strings.ToUpper(*child.PartType) != ESP {
				dpsFatalFail(c, child, ESP)
			}
		}
	}

[Johan-Liebert1]

This was fixed in Go 1.22 iirc and we use Go 1.24

[Johan-Liebert1]

Also, please correct me if I'm misremembering it, but this would only happen in async code

[gemini-code-assist]

Initializing RootPartition this way will result in an empty string if runtime.GOARCH is not found in the map, which can cause confusing test failures. It's better to fail fast for unsupported architectures. A common pattern for this is to use an anonymous function that panics if the key is not found, ensuring the program doesn't continue in an invalid state.

var RootPartition = func() string {
	uuid, ok := map[string]string{
		// Root partition for 64-bit x86/AMD64
		"amd64": "4F68BCE3-E8CD-4DB1-96E7-FBCAF984B709",
		// Root partition for 64-bit ARM/AArch64 architecture
		"arm64": "B921B045-1DF0-41C3-AF44-4C6F280D3FAE",
		// Root partition for 64-bit PowerPC Big Endian
		"ppc64": "912ADE1D-A839-4913-8964-A10EEE08FBD2",
		// Root partition for 64-bit PowerPC Little Endian
		"ppc64le": "C31C45E6-3F39-412E-80FB-4809C4980599",
		// Root partition for s390x architecture
		"s390x": "5EEAD9A9-FE09-4A1E-A1D7-520D00531306",
	}[runtime.GOARCH]
	if !ok {
		panic(fmt.Sprintf("no root partition UUID defined for architecture %s", runtime.GOARCH))
	}
	return uuid
}()

[jbtrystram]

Looks generally sane to me, but i don't think this have to be a cosa test and could be an external test.

[Johan-Liebert1]

Thanks for the review @jbtrystram. Where do we add external tests?

[jbtrystram]

Where do we add external tests?

https://github.com/coreos/fedora-coreos-config/tree/testing-devel/tests/kola

I think that can even be a non-exclusive test since you are simply reading the partitions UUIDs :)

[dustymabe]

Looks generally sane to me, but i don't think this have to be a cosa test and could be an external test.

agree. It would be nice to have this be an external test if it can be. Those are easier to manage because we don't have to re-build COSA anytime the test changes.

[jbtrystram]

Looks generally sane to me, but i don't think this have to be a cosa test and could be an external test.

agree. It would be nice to have this be an external test if it can be. Those are easier to manage because we don't have to re-build COSA anytime the test changes.

Yeah. also for this kind of checks, bash can be easier to reason about and more straightforward to maintain than go code