From b79fdd1a49780bc57cb135fba4ace65402552706 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Mon, 25 Nov 2024 15:29:30 +0100 Subject: [PATCH 1/2] disk: unexport `AddBootPartition` and `AddPartitionsForBootMode` Those two are not used outside of `disk` so let's unexport them to keep a smaller API. Followups, c.f. https://github.com/osbuild/images/pull/1041#discussion_r1848291704 --- pkg/disk/export_test.go | 6 ++++-- pkg/disk/partition_table.go | 12 ++++++------ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/pkg/disk/export_test.go b/pkg/disk/export_test.go index f81043df13..329e85fe6c 100644 --- a/pkg/disk/export_test.go +++ b/pkg/disk/export_test.go @@ -1,8 +1,10 @@ package disk var ( - PayloadEntityMap = payloadEntityMap - EntityPath = entityPath + PayloadEntityMap = payloadEntityMap + EntityPath = entityPath + AddBootPartition = addBootPartition + AddPartitionsForBootMode = addPartitionsForBootMode ) func FindDirectoryEntityPath(pt *PartitionTable, path string) []Entity { diff --git a/pkg/disk/partition_table.go b/pkg/disk/partition_table.go index 7c0cd01c2f..109b796d44 100644 --- a/pkg/disk/partition_table.go +++ b/pkg/disk/partition_table.go @@ -996,10 +996,10 @@ func EnsureRootFilesystem(pt *PartitionTable, defaultFsType FSType) error { return nil } -// AddBootPartition creates a boot partition. The function will append the boot +// addBootPartition creates a boot partition. The function will append the boot // partition to the end of the existing partition table therefore it is best to // call this function early to put boot near the front (as is conventional). -func AddBootPartition(pt *PartitionTable, bootFsType FSType) error { +func addBootPartition(pt *PartitionTable, bootFsType FSType) error { if bootFsType == FS_NONE { return fmt.Errorf("error creating boot partition: no filesystem type") } @@ -1034,7 +1034,7 @@ func AddBootPartition(pt *PartitionTable, bootFsType FSType) error { return nil } -// AddPartitionsForBootMode creates partitions to satisfy the boot mode requirements: +// addPartitionsForBootMode creates partitions to satisfy the boot mode requirements: // - BIOS/legacy: adds a 1 MiB BIOS boot partition. // - UEFI: adds a 200 MiB EFI system partition. // - Hybrid: adds both. @@ -1042,7 +1042,7 @@ func AddBootPartition(pt *PartitionTable, bootFsType FSType) error { // The function will append the new partitions to the end of the existing // partition table therefore it is best to call this function early to put them // near the front (as is conventional). -func AddPartitionsForBootMode(pt *PartitionTable, bootMode platform.BootMode) error { +func addPartitionsForBootMode(pt *PartitionTable, bootMode platform.BootMode) error { switch bootMode { case platform.BOOT_LEGACY: // add BIOS boot partition @@ -1185,7 +1185,7 @@ func NewCustomPartitionTable(customizations *blueprint.DiskCustomization, option } // TODO: switch to ensure ESP in case customizations already include it - if err := AddPartitionsForBootMode(pt, options.BootMode); err != nil { + if err := addPartitionsForBootMode(pt, options.BootMode); err != nil { return nil, fmt.Errorf("%s %w", errPrefix, err) } @@ -1203,7 +1203,7 @@ func NewCustomPartitionTable(customizations *blueprint.DiskCustomization, option if needsBoot(customizations) { // we need a /boot partition to boot LVM or Btrfs, create boot // partition if it does not already exist - if err := AddBootPartition(pt, bootFsType); err != nil { + if err := addBootPartition(pt, bootFsType); err != nil { return nil, fmt.Errorf("%s %w", errPrefix, err) } } From bf0b53d8c9219d749c6ff7e707c7373c067ade07 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Mon, 25 Nov 2024 15:49:17 +0100 Subject: [PATCH 2/2] disk: extract `maybeAddBootPartition()` helper This commit extracts a new `maybeAddBootPartition()` helper to make the `NewCustomPartitionTable()` a little bit more linear to read. Followup, c.f. https://github.com/osbuild/images/pull/1041#discussion_r1848298181 --- pkg/disk/partition_table.go | 48 ++++++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/pkg/disk/partition_table.go b/pkg/disk/partition_table.go index 109b796d44..978b34d7fb 100644 --- a/pkg/disk/partition_table.go +++ b/pkg/disk/partition_table.go @@ -1155,6 +1155,28 @@ func (options *CustomPartitionTableOptions) getfstype(fstype string) (string, er return options.DefaultFSType.String(), nil } +func maybeAddBootPartition(pt *PartitionTable, disk *blueprint.DiskCustomization, defaultFSType FSType) error { + // The boot type will be the default only if it's a supported filesystem + // type for /boot (ext4 or xfs). Otherwise, we default to xfs. + // FS_NONE also falls back to xfs. + var bootFsType FSType + switch defaultFSType { + case FS_EXT4, FS_XFS: + bootFsType = defaultFSType + default: + bootFsType = FS_XFS + } + + if needsBoot(disk) { + // we need a /boot partition to boot LVM or Btrfs, create boot + // partition if it does not already exist + if err := addBootPartition(pt, bootFsType); err != nil { + return err + } + } + return nil +} + // NewCustomPartitionTable creates a partition table based almost entirely on the disk customizations from a blueprint. func NewCustomPartitionTable(customizations *blueprint.DiskCustomization, options *CustomPartitionTableOptions, rng *rand.Rand) (*PartitionTable, error) { if options == nil { @@ -1184,30 +1206,18 @@ func NewCustomPartitionTable(customizations *blueprint.DiskCustomization, option return nil, fmt.Errorf("%s invalid partition table type enum value: %d", errPrefix, options.PartitionTableType) } + // add any partition(s) that are needed for booting (like /boot/efi) + // if needed + // // TODO: switch to ensure ESP in case customizations already include it if err := addPartitionsForBootMode(pt, options.BootMode); err != nil { return nil, fmt.Errorf("%s %w", errPrefix, err) } - - // The boot type will be the default only if it's a supported filesystem - // type for /boot (ext4 or xfs). Otherwise, we default to xfs. - // FS_NONE also falls back to xfs. - var bootFsType FSType - switch options.DefaultFSType { - case FS_EXT4, FS_XFS: - bootFsType = options.DefaultFSType - default: - bootFsType = FS_XFS - } - - if needsBoot(customizations) { - // we need a /boot partition to boot LVM or Btrfs, create boot - // partition if it does not already exist - if err := addBootPartition(pt, bootFsType); err != nil { - return nil, fmt.Errorf("%s %w", errPrefix, err) - } + // add the /boot partition (if it is needed) + if err := maybeAddBootPartition(pt, customizations, options.DefaultFSType); err != nil { + return nil, fmt.Errorf("%s %w", errPrefix, err) } - + // add user customized partitions for _, part := range customizations.Partitions { switch part.Type { case "plain", "":