Skip to content

Commit

Permalink
disk: error on PTs with type dos and len(partitions) > 4
Browse files Browse the repository at this point in the history
Return an error when a partition table is created with type dos and more
than 4 partitions.

This check can only happen after the partition table is created,
otherwise we would have to predict all the partitions that get created
automatically, which would duplicate a lot of the logic in
NewCustomPartitionTable().

Update tests:
- Convert the plain+swap (valid) test to GPT since now it's invalid as
  DOS.
- Add a fail test for a DOS partition table with too many partitions.
  • Loading branch information
achilleas-k authored and supakeen committed Dec 10, 2024
1 parent 8e2b2b9 commit b923fd8
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 12 deletions.
10 changes: 10 additions & 0 deletions pkg/disk/partition_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -1292,6 +1292,16 @@ func NewCustomPartitionTable(customizations *blueprint.DiskCustomization, option
pt.relayout(customizations.MinSize)
pt.GenerateUUIDs(rng)

// One thing not caught by the customization validation is if a final "dos"
// partition table has more than 4 partitions. This is not possible to
// predict with customizations alone because it depends on the boot type
// (which comes from the image type) which controls automatic partition
// creation. We should therefore always check the final partition table for
// this rule.
if pt.Type == PT_DOS && len(pt.Partitions) > 4 {
return nil, fmt.Errorf("%s invalid partition table: \"dos\" partition table type only supports up to 4 partitions: got %d after creating the partition table with all necessary partitions", errPrefix, len(pt.Partitions))
}

return pt, nil
}

Expand Down
51 changes: 39 additions & 12 deletions pkg/disk/partition_table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1285,24 +1285,24 @@ func TestNewCustomPartitionTable(t *testing.T) {
options: &disk.CustomPartitionTableOptions{
DefaultFSType: disk.FS_XFS,
BootMode: platform.BOOT_HYBRID,
PartitionTableType: disk.PT_DOS,
PartitionTableType: disk.PT_GPT,
},
expected: &disk.PartitionTable{
Type: disk.PT_DOS,
Size: 227 * datasizes.MiB,
Type: disk.PT_GPT,
Size: 227*datasizes.MiB + datasizes.MiB, // last part + footer
UUID: "0194fdc2-fa2f-4cc0-81d3-ff12045b73c8",
Partitions: []disk.Partition{
{
Start: 1 * datasizes.MiB, // header
Size: 1 * datasizes.MiB,
Bootable: true,
Type: disk.DosBIOSBootID,
Type: disk.BIOSBootPartitionGUID,
UUID: disk.BIOSBootPartitionUUID,
},
{
Start: 2 * datasizes.MiB,
Size: 200 * datasizes.MiB,
Type: disk.DosESPID,
Type: disk.EFISystemPartitionGUID,
UUID: disk.EFISystemPartitionUUID,
Payload: &disk.Filesystem{
Type: "vfat",
Expand All @@ -1317,9 +1317,9 @@ func TestNewCustomPartitionTable(t *testing.T) {
{
Start: 202 * datasizes.MiB,
Size: 20 * datasizes.MiB,
Type: disk.DosLinuxTypeID,
Type: disk.FilesystemDataGUID,
Bootable: false,
UUID: "", // partitions on dos PTs don't have UUIDs
UUID: "e2d3d0d0-de6b-48f9-b44c-e85ff044c6b1",
Payload: &disk.Filesystem{
Type: "ext4",
Label: "data",
Expand All @@ -1333,8 +1333,8 @@ func TestNewCustomPartitionTable(t *testing.T) {
{
Start: 222 * datasizes.MiB,
Size: 5 * datasizes.MiB,
Type: disk.DosSwapID,
UUID: "", // partitions on dos PTs don't have UUIDs
Type: disk.SwapPartitionGUID,
UUID: "f83b8e88-3bbf-457a-ab99-c5b252c7429c",
Bootable: false,
Payload: &disk.Swap{
Label: "swap",
Expand All @@ -1344,9 +1344,9 @@ func TestNewCustomPartitionTable(t *testing.T) {
},
{
Start: 227 * datasizes.MiB,
Size: 0,
Type: disk.DosLinuxTypeID,
UUID: "", // partitions on dos PTs don't have UUIDs
Size: 1*datasizes.MiB - (disk.DefaultSectorSize + (128 * 128)), // grows by 1 grain size (1 MiB) minus the unaligned size of the header to fit the gpt footer
Type: disk.FilesystemDataGUID,
UUID: "32f3a8ae-b79e-4856-b659-c18f0dcecc77",
Bootable: false,
Payload: &disk.Filesystem{
Type: "xfs",
Expand Down Expand Up @@ -2276,6 +2276,33 @@ func TestNewCustomPartitionTableErrors(t *testing.T) {
},
errmsg: `error generating partition table: unknown partition table type: toucan (valid: gpt, dos)`,
},
"dos-too-many-parts": {
customizations: &blueprint.DiskCustomization{
Partitions: []blueprint.PartitionCustomization{
{
MinSize: 20 * datasizes.MiB,
FilesystemTypedCustomization: blueprint.FilesystemTypedCustomization{
Mountpoint: "/data",
Label: "data",
FSType: "ext4",
},
},
{
MinSize: 5 * datasizes.MiB,
FilesystemTypedCustomization: blueprint.FilesystemTypedCustomization{
Label: "swap",
FSType: "swap",
},
},
},
},
options: &disk.CustomPartitionTableOptions{
DefaultFSType: disk.FS_XFS,
BootMode: platform.BOOT_HYBRID,
PartitionTableType: disk.PT_DOS,
},
errmsg: `error generating partition table: invalid partition table: "dos" partition table type only supports up to 4 partitions: got 5 after creating the partition table with all necessary partitions`,
},
}

// we don't care about the rng for error tests
Expand Down

0 comments on commit b923fd8

Please sign in to comment.