Add support for selecting a partition table type

cmd/otk/osbuild-gen-partition-table/main_test.go

@@ -343,7 +343,7 @@ func TestGenPartitionTableIntegrationPPC(t *testing.T) {
 				Name:     "ppc-boot",
 				Bootable: true,
 				Size:     "4 MiB",
-				PartType: "41",
+				PartType: disk.DosPRePID,
 				PartUUID: "",
 			},
 			{
@@ -373,7 +373,7 @@ func TestGenPartitionTableIntegrationPPC(t *testing.T) {
 							Bootable: true,
 							Start:    1048576,
 							Size:     4194304,
-							Type:     "41",
+							Type:     disk.DosPRePID,
 						},
 						{
 							Start: 5242880,

pkg/blueprint/disk_customizations.go

@@ -14,13 +14,15 @@ import (
 )
 
 type DiskCustomization struct {
-	// TODO: Add partition table type: gpt or dos
+	// Type of the partition table: gpt or dos.
+	// Optional, the default depends on the distro and image type.
+	Type       string
 	MinSize    uint64
 	Partitions []PartitionCustomization
 }
 
 type diskCustomizationMarshaler struct {
-	// TODO: Add partition table type: gpt or dos
+	Type       string                   `json:"type,omitempty" toml:"type,omitempty"`
 	MinSize    datasizes.Size           `json:"minsize,omitempty" toml:"minsize,omitempty"`
 	Partitions []PartitionCustomization `json:"partitions,omitempty" toml:"partitions,omitempty"`
 }
@@ -30,6 +32,7 @@ func (dc *DiskCustomization) UnmarshalJSON(data []byte) error {
 	if err := json.Unmarshal(data, &dcm); err != nil {
 		return err
 	}
+	dc.Type = dcm.Type
 	dc.MinSize = dcm.MinSize.Uint64()
 	dc.Partitions = dcm.Partitions
 
@@ -345,6 +348,21 @@ func (p *DiskCustomization) Validate() error {
 		return nil
 	}
 
+	switch p.Type {
+	case "gpt", "":
+	case "dos":
+		// dos/mbr only supports 4 partitions
+		// Unfortunately, at this stage it's unknown whether we will need extra
+		// partitions (bios boot, root, esp), so this check is just to catch
+		// obvious invalid customizations early. The final partition table is
+		// checked after it's created.
+		if len(p.Partitions) > 4 {
+			return fmt.Errorf("invalid partitioning customizations: \"dos\" partition table type only supports up to 4 partitions: got %d", len(p.Partitions))
+		}
+	default:
+		return fmt.Errorf("unknown partition table type: %s (valid: gpt, dos)", p.Type)
+	}
+
 	mountpoints := make(map[string]bool)
 	vgnames := make(map[string]bool)
 	var errs []error

pkg/blueprint/disk_customizations_test.go

@@ -904,6 +904,72 @@ func TestPartitioningValidation(t *testing.T) {
 			},
 			expectedMsg: "invalid partitioning customizations:\nmountpoint for swap logical volume with name \"swappylv\" in volume group \"badvg\" must be empty",
 		},
+		"gpt": {
+			partitioning: &blueprint.DiskCustomization{
+				Type: "gpt",
+			},
+		},
+		"dos": {
+			partitioning: &blueprint.DiskCustomization{
+				Type: "dos",
+			},
+		},
+		"unhappy-badtype": {
+			partitioning: &blueprint.DiskCustomization{
+				Type: "toucan",
+			},
+			expectedMsg: "unknown partition table type: toucan (valid: gpt, dos)",
+		},
+		"unhappy-too-many-parts": {
+			partitioning: &blueprint.DiskCustomization{
+				Type: "dos",
+				Partitions: []blueprint.PartitionCustomization{
+					{
+						Type: "plain",
+						FilesystemTypedCustomization: blueprint.FilesystemTypedCustomization{
+							FSType:     "xfs",
+							Mountpoint: "/1",
+						},
+					},
+					{
+						Type: "plain",
+						FilesystemTypedCustomization: blueprint.FilesystemTypedCustomization{
+							FSType:     "xfs",
+							Mountpoint: "/2",
+						},
+					},
+					{
+						Type: "plain",
+						FilesystemTypedCustomization: blueprint.FilesystemTypedCustomization{
+							FSType:     "xfs",
+							Mountpoint: "/3",
+						},
+					},
+					{
+						Type: "plain",
+						FilesystemTypedCustomization: blueprint.FilesystemTypedCustomization{
+							FSType:     "ext4",
+							Mountpoint: "/4",
+						},
+					},
+					{
+						Type: "plain",
+						FilesystemTypedCustomization: blueprint.FilesystemTypedCustomization{
+							FSType:     "ext4",
+							Mountpoint: "/5",
+						},
+					},
+					{
+						Type: "plain",
+						FilesystemTypedCustomization: blueprint.FilesystemTypedCustomization{
+							FSType:     "ext4",
+							Mountpoint: "/6",
+						},
+					},
+				},
+			},
+			expectedMsg: `invalid partitioning customizations: "dos" partition table type only supports up to 4 partitions: got 6`,
+		},
 	}
 
 	for name := range testCases {
@@ -1737,6 +1803,15 @@ func TestDiskCustomizationUnmarshalJSON(t *testing.T) {
 				MinSize: 1 * datasizes.GiB,
 			},
 		},
+		"type": {
+			inputJSON: `{
+				"type": "gpt"
+			}`,
+			inputTOML: `type = "gpt"`,
+			expected: &blueprint.DiskCustomization{
+				Type: "gpt",
+			},
+		},
 	}
 
 	for name := range testCases {

pkg/disk/disk.go

@@ -67,14 +67,23 @@ const (
 	// Partition type ID for any native Linux filesystem on dos
 	DosLinuxTypeID = "83"
 
-	// Partition type ID for BIOS boot partition on dos
-	DosBIOSBootID = "ef02"
+	// Partition type ID for LVM on dos
+	DosLVMTypeID = "8e"
+
+	// Partition type ID for BIOS boot partition on dos.
+	// Type ID is for 'empty'.
+	// TODO: drop this completely when we convert the bios BOOT space to a
+	// partitionless gap/offset.
+	DosBIOSBootID = "00"
 
 	// Partition type ID for ESP on dos
-	DosESPID = "ef00"
+	DosESPID = "ef"
 
 	// Partition type ID for swap
 	DosSwapID = "82"
+
+	// Partition type ID for PRep on dos
+	DosPRePID = "41"
 )
 
 // pt type -> type -> ID mapping for convenience
@@ -84,7 +93,7 @@ var idMap = map[PartitionTableType]map[string]string{
 		"boot": DosLinuxTypeID,
 		"data": DosLinuxTypeID,
 		"esp":  DosESPID,
-		"lvm":  DosLinuxTypeID,
+		"lvm":  DosLVMTypeID,
 		"swap": DosSwapID,
 	},
 	PT_GPT: {

pkg/disk/partition.go

@@ -87,15 +87,15 @@ func (p *Partition) IsBIOSBoot() bool {
 		return false
 	}
 
-	return p.Type == BIOSBootPartitionGUID
+	return p.Type == BIOSBootPartitionGUID || p.Type == DosBIOSBootID
 }
 
 func (p *Partition) IsPReP() bool {
 	if p == nil {
 		return false
 	}
 
-	return p.Type == "41" || p.Type == PRePartitionGUID
+	return p.Type == DosPRePID || p.Type == PRePartitionGUID
 }
 
 func (p *Partition) MarshalJSON() ([]byte, error) {

pkg/disk/partition_table.go

@@ -1232,15 +1232,24 @@ func NewCustomPartitionTable(customizations *blueprint.DiskCustomization, option
 
 	pt := &PartitionTable{}
 
-	// TODO: Handle partition table type in customizations
-	switch options.PartitionTableType {
-	case PT_GPT, PT_DOS:
-		pt.Type = options.PartitionTableType
-	case PT_NONE:
-		// default to "gpt"
+	switch customizations.Type {
+	case "dos":
+		pt.Type = PT_DOS
+	case "gpt":
 		pt.Type = PT_GPT
+	case "":
+		// partition table type not specified, determine the default
+		switch options.PartitionTableType {
+		case PT_GPT, PT_DOS:
+			pt.Type = options.PartitionTableType
+		case PT_NONE:
+			// default to "gpt"
+			pt.Type = PT_GPT
+		default:
+			return nil, fmt.Errorf("%s invalid partition table type enum value: %d", errPrefix, options.PartitionTableType)
+		}
 	default:
-		return nil, fmt.Errorf("%s invalid partition table type enum value: %d", errPrefix, options.PartitionTableType)
+		return nil, fmt.Errorf("%s invalid partition table type: %s", errPrefix, customizations.Type)
 	}
 
 	// add any partition(s) that are needed for booting (like /boot/efi)
@@ -1266,7 +1275,9 @@ func NewCustomPartitionTable(customizations *blueprint.DiskCustomization, option
 				return nil, fmt.Errorf("%s %w", errPrefix, err)
 			}
 		case "btrfs":
-			addBtrfsPartition(pt, part)
+			if err := addBtrfsPartition(pt, part); err != nil {
+				return nil, fmt.Errorf("%s %w", errPrefix, err)
+			}
 		default:
 			return nil, fmt.Errorf("%s invalid partition type: %s", errPrefix, part.Type)
 		}
@@ -1283,6 +1294,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
 }
 
@@ -1387,8 +1408,12 @@ func addLVMPartition(pt *PartitionTable, partition blueprint.PartitionCustomizat
 	}
 
 	// create partition for volume group
+	partType, err := getPartitionTypeIDfor(pt.Type, "lvm")
+	if err != nil {
+		return fmt.Errorf("error creating lvm partition %q: %w", vgname, err)
+	}
 	newpart := Partition{
-		Type:     LVMPartitionGUID,
+		Type:     partType,
 		Size:     partition.MinSize,
 		Bootable: false,
 		Payload:  newvg,
@@ -1397,7 +1422,7 @@ func addLVMPartition(pt *PartitionTable, partition blueprint.PartitionCustomizat
 	return nil
 }
 
-func addBtrfsPartition(pt *PartitionTable, partition blueprint.PartitionCustomization) {
+func addBtrfsPartition(pt *PartitionTable, partition blueprint.PartitionCustomization) error {
 	subvols := make([]BtrfsSubvolume, len(partition.Subvolumes))
 	for idx, subvol := range partition.Subvolumes {
 		newsubvol := BtrfsSubvolume{
@@ -1412,14 +1437,19 @@ func addBtrfsPartition(pt *PartitionTable, partition blueprint.PartitionCustomiz
 	}
 
 	// create partition for btrfs volume
+	partType, err := getPartitionTypeIDfor(pt.Type, "data")
+	if err != nil {
+		return fmt.Errorf("error creating btrfs partition: %w", err)
+	}
 	newpart := Partition{
-		Type:     FilesystemDataGUID,
+		Type:     partType,
 		Bootable: false,
 		Payload:  newvol,
 		Size:     partition.MinSize,
 	}
 
 	pt.Partitions = append(pt.Partitions, newpart)
+	return nil
 }
 
 // Determine if a boot partition is needed based on the customizations. A boot