From 0397f8a002a24be5cd9239ae17a03964d4eec119 Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Fri, 1 Nov 2024 16:11:14 +0100 Subject: [PATCH 1/6] disk: EnsureRootFilesystem() helper function Add a function that takes a partition table and adds a root filesystem, but only if one does not already exist. A root filesystem is any mountable where the mountpoint is `/`. The function decides whether to create a plain partition, a logical volume, or a btrfs subvolume based on existing partitions and volumes in the partition table. This function is not currently used because our base partition tables always have root filesystems, but it will be part of the new custom partition table generator. --- pkg/disk/partition_table.go | 97 +++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/pkg/disk/partition_table.go b/pkg/disk/partition_table.go index 407074edb..42c3bfc35 100644 --- a/pkg/disk/partition_table.go +++ b/pkg/disk/partition_table.go @@ -894,3 +894,100 @@ func (pt *PartitionTable) GetMountpointSize(mountpoint string) (uint64, error) { panic(fmt.Sprintf("no sizeable of the entity path for mountpoint %s, this is a programming error", mountpoint)) } + +// EnsureRootFilesystem adds a root filesystem if the partition table doesn't +// already have one. +// +// When adding the root filesystem, add it to: +// +// - The first LVM Volume Group if one exists, otherwise +// - The first Btrfs volume if one exists, otherwise +// - At the end of the plain partitions. +// +// For LVM and Plain, the fsType argument must be a valid filesystem type. +func EnsureRootFilesystem(pt *PartitionTable, defaultFsType FSType) error { + // collect all labels and subvolume names to avoid conflicts + subvolNames := make(map[string]bool) + labels := make(map[string]bool) + var foundRoot bool + _ = pt.ForEachMountable(func(mnt Mountable, path []Entity) error { + if mnt.GetMountpoint() == "/" { + foundRoot = true + return nil + } + + labels[mnt.GetFSSpec().Label] = true + switch mountable := mnt.(type) { + case *BtrfsSubvolume: + subvolNames[mountable.Name] = true + } + return nil + }) + if foundRoot { + // nothing to do + return nil + } + + for _, part := range pt.Partitions { + switch payload := part.Payload.(type) { + case *LVMVolumeGroup: + if defaultFsType == FS_NONE { + return fmt.Errorf("error creating root logical volume: no default filesystem type") + } + + rootLabel, err := genUniqueString("root", labels) + if err != nil { + return fmt.Errorf("error creating root logical volume: %w", err) + } + rootfs := &Filesystem{ + Type: defaultFsType.String(), + Label: rootLabel, + Mountpoint: "/", + FSTabOptions: "defaults", + } + // Let the function autogenerate the name to avoid conflicts + // with LV names from customizations. + // Set the size to 0 and it will be adjusted by + // EnsureDirectorySizes() and relayout(). + if _, err := payload.CreateLogicalVolume("", 0, rootfs); err != nil { + return fmt.Errorf("error creating root logical volume: %w", err) + } + return nil + case *Btrfs: + rootName, err := genUniqueString("root", subvolNames) + if err != nil { + return fmt.Errorf("error creating root subvolume: %w", err) + } + rootsubvol := BtrfsSubvolume{ + Name: rootName, + Mountpoint: "/", + } + payload.Subvolumes = append(payload.Subvolumes, rootsubvol) + return nil + } + } + + // We're going to create a root partition, so we have to ensure the default type is set. + if defaultFsType == FS_NONE { + return fmt.Errorf("error creating root partition: no default filesystem type") + } + + // add a plain root partition at the end of the partition table + rootLabel, err := genUniqueString("root", labels) + if err != nil { + return fmt.Errorf("error creating root partition: %w", err) + } + rootpart := Partition{ + Type: FilesystemDataGUID, + Bootable: false, + Size: 0, // Set the size to 0 and it will be adjusted by EnsureDirectorySizes() and relayout() + Payload: &Filesystem{ + Type: defaultFsType.String(), + Label: rootLabel, + Mountpoint: "/", + FSTabOptions: "defaults", + }, + } + pt.Partitions = append(pt.Partitions, rootpart) + return nil +} From 5357ca259196b81e9215ae7def0f656fc7c710a4 Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Wed, 30 Oct 2024 18:31:41 +0100 Subject: [PATCH 2/6] disk: add tests for EnsureRootFilesystem() --- pkg/disk/partition_table.go | 5 +- pkg/disk/partition_table_test.go | 539 +++++++++++++++++++++++++++++++ 2 files changed, 541 insertions(+), 3 deletions(-) diff --git a/pkg/disk/partition_table.go b/pkg/disk/partition_table.go index 42c3bfc35..ff68ed3b0 100644 --- a/pkg/disk/partition_table.go +++ b/pkg/disk/partition_table.go @@ -978,9 +978,8 @@ func EnsureRootFilesystem(pt *PartitionTable, defaultFsType FSType) error { return fmt.Errorf("error creating root partition: %w", err) } rootpart := Partition{ - Type: FilesystemDataGUID, - Bootable: false, - Size: 0, // Set the size to 0 and it will be adjusted by EnsureDirectorySizes() and relayout() + Type: FilesystemDataGUID, + Size: 0, // Set the size to 0 and it will be adjusted by EnsureDirectorySizes() and relayout() Payload: &Filesystem{ Type: defaultFsType.String(), Label: rootLabel, diff --git a/pkg/disk/partition_table_test.go b/pkg/disk/partition_table_test.go index 89c80381b..8d4788258 100644 --- a/pkg/disk/partition_table_test.go +++ b/pkg/disk/partition_table_test.go @@ -105,3 +105,542 @@ func TestPartitionTable_GenerateUUIDs_VFAT(t *testing.T) { assert.Equal(t, "6e4ff95f", pt.Partitions[0].Payload.(*disk.Filesystem).UUID) } + +func TestEnsureRootFilesystem(t *testing.T) { + type testCase struct { + pt disk.PartitionTable + expected disk.PartitionTable + defaultFsType disk.FSType + } + + testCases := map[string]testCase{ + "empty-plain": { + pt: disk.PartitionTable{}, + defaultFsType: disk.FS_EXT4, + expected: disk.PartitionTable{ + Partitions: []disk.Partition{ + { + Start: 0, + Size: 0, + Type: disk.FilesystemDataGUID, + Bootable: false, + UUID: "", + Payload: &disk.Filesystem{ + Type: "ext4", + Label: "root", + Mountpoint: "/", + FSTabOptions: "defaults", + }, + }, + }, + }, + }, + "simple-plain": { + pt: disk.PartitionTable{ + Partitions: []disk.Partition{ + { + Payload: &disk.Filesystem{ + Type: "ext4", + Label: "home", + Mountpoint: "/home", + FSTabOptions: "defaults", + }, + }, + }, + }, + defaultFsType: disk.FS_EXT4, + expected: disk.PartitionTable{ + Partitions: []disk.Partition{ + { + Payload: &disk.Filesystem{ + Type: "ext4", + Label: "home", + Mountpoint: "/home", + FSTabOptions: "defaults", + }, + }, + { + Start: 0, + Size: 0, + Type: disk.FilesystemDataGUID, + Bootable: false, + UUID: "", + Payload: &disk.Filesystem{ + Type: "ext4", + Label: "root", + Mountpoint: "/", + FSTabOptions: "defaults", + }, + }, + }, + }, + }, + "simple-lvm": { + pt: disk.PartitionTable{ + Partitions: []disk.Partition{ + { + Payload: &disk.LVMVolumeGroup{ + Name: "testvg", + LogicalVolumes: []disk.LVMLogicalVolume{ + { + Name: "varloglv", + Payload: &disk.Filesystem{ + Label: "var-log", + Type: "xfs", + Mountpoint: "/var/log", + }, + }, + { + Name: "datalv", + Payload: &disk.Filesystem{ + Label: "data", + Mountpoint: "/data", + FSTabOptions: "defaults", + Type: "ext4", + }, + }, + }, + }, + }, + }, + }, + defaultFsType: disk.FS_EXT4, + expected: disk.PartitionTable{ + Partitions: []disk.Partition{ + { + Payload: &disk.LVMVolumeGroup{ + Name: "testvg", + LogicalVolumes: []disk.LVMLogicalVolume{ + { + Name: "varloglv", + Payload: &disk.Filesystem{ + Label: "var-log", + Type: "xfs", + Mountpoint: "/var/log", + }, + }, + { + Name: "datalv", + Payload: &disk.Filesystem{ + Label: "data", + Type: "ext4", + Mountpoint: "/data", + FSTabOptions: "defaults", + }, + }, + { + Name: "rootlv", + Payload: &disk.Filesystem{ + Label: "root", + Type: "ext4", + Mountpoint: "/", + FSTabOptions: "defaults", + }, + }, + }, + }, + }, + }, + }, + }, + "simple-btrfs": { + pt: disk.PartitionTable{ + Partitions: []disk.Partition{ + { + Payload: &disk.Btrfs{ + Subvolumes: []disk.BtrfsSubvolume{ + { + Name: "subvol/home", + Mountpoint: "/home", + }, + }, + }, + }, + }, + }, + // no default fs required + expected: disk.PartitionTable{ + Partitions: []disk.Partition{ + { + Payload: &disk.Btrfs{ + Subvolumes: []disk.BtrfsSubvolume{ + { + Name: "subvol/home", + Mountpoint: "/home", + }, + { + Name: "root", + Mountpoint: "/", + }, + }, + }, + }, + }, + }, + }, + "noop-lvm": { + pt: disk.PartitionTable{ + Partitions: []disk.Partition{ + { + Payload: &disk.LVMVolumeGroup{ + Name: "testvg", + LogicalVolumes: []disk.LVMLogicalVolume{ + { + Name: "varloglv", + Payload: &disk.Filesystem{ + Label: "var-log", + Type: "xfs", + Mountpoint: "/var/log", + }, + }, + { + Name: "datalv", + Payload: &disk.Filesystem{ + Label: "data", + Type: "ext4", + Mountpoint: "/data", + FSTabOptions: "defaults", + }, + }, + { + Name: "rootlv", + Payload: &disk.Filesystem{ + Label: "root", + Type: "ext4", + Mountpoint: "/", + FSTabOptions: "defaults", + }, + }, + }, + }, + }, + }, + }, + expected: disk.PartitionTable{ + Partitions: []disk.Partition{ + { + Payload: &disk.LVMVolumeGroup{ + Name: "testvg", + LogicalVolumes: []disk.LVMLogicalVolume{ + { + Name: "varloglv", + Payload: &disk.Filesystem{ + Label: "var-log", + Type: "xfs", + Mountpoint: "/var/log", + }, + }, + { + Name: "datalv", + Payload: &disk.Filesystem{ + Label: "data", + Type: "ext4", + Mountpoint: "/data", + FSTabOptions: "defaults", + }, + }, + { + Name: "rootlv", + Payload: &disk.Filesystem{ + Label: "root", + Type: "ext4", + Mountpoint: "/", + FSTabOptions: "defaults", + }, + }, + }, + }, + }, + }, + }, + }, + "noop-btrfs": { + pt: disk.PartitionTable{ + Partitions: []disk.Partition{ + { + Payload: &disk.Btrfs{ + Subvolumes: []disk.BtrfsSubvolume{ + { + Name: "subvol/home", + Mountpoint: "/home", + }, + { + Name: "root", + Mountpoint: "/", + }, + }, + }, + }, + }, + }, + expected: disk.PartitionTable{ + Partitions: []disk.Partition{ + { + Payload: &disk.Btrfs{ + Subvolumes: []disk.BtrfsSubvolume{ + { + Name: "subvol/home", + Mountpoint: "/home", + }, + { + Name: "root", + Mountpoint: "/", + }, + }, + }, + }, + }, + }, + }, + "plain-collision": { + pt: disk.PartitionTable{ + Partitions: []disk.Partition{ + { + Payload: &disk.Filesystem{ + Type: "ext4", + Label: "root", + Mountpoint: "/root", + FSTabOptions: "defaults", + }, + }, + }, + }, + defaultFsType: disk.FS_EXT4, + expected: disk.PartitionTable{ + Partitions: []disk.Partition{ + { + Payload: &disk.Filesystem{ + Type: "ext4", + Label: "root", + Mountpoint: "/root", + FSTabOptions: "defaults", + }, + }, + { + Start: 0, + Size: 0, + Type: disk.FilesystemDataGUID, + Bootable: false, + UUID: "", + Payload: &disk.Filesystem{ + Type: "ext4", + Label: "root00", + Mountpoint: "/", + FSTabOptions: "defaults", + }, + }, + }, + }, + }, + "lvm-collision": { + pt: disk.PartitionTable{ + Partitions: []disk.Partition{ + { + Payload: &disk.LVMVolumeGroup{ + Name: "testvg", + LogicalVolumes: []disk.LVMLogicalVolume{ + { + Name: "varloglv", + Payload: &disk.Filesystem{ + Label: "var-log", + Type: "xfs", + Mountpoint: "/var/log", + }, + }, + { + Name: "datalv", + Payload: &disk.Filesystem{ + Label: "data", + Type: "ext4", + Mountpoint: "/data", + FSTabOptions: "defaults", + }, + }, + { + Name: "rootlv", + Payload: &disk.Filesystem{ + Label: "root", + Type: "ext4", + Mountpoint: "/root", + FSTabOptions: "defaults", + }, + }, + }, + }, + }, + }, + }, + defaultFsType: disk.FS_XFS, + expected: disk.PartitionTable{ + Partitions: []disk.Partition{ + { + Payload: &disk.LVMVolumeGroup{ + Name: "testvg", + LogicalVolumes: []disk.LVMLogicalVolume{ + { + Name: "varloglv", + Payload: &disk.Filesystem{ + Label: "var-log", + Type: "xfs", + Mountpoint: "/var/log", + }, + }, + { + Name: "datalv", + Payload: &disk.Filesystem{ + Label: "data", + Type: "ext4", + Mountpoint: "/data", + FSTabOptions: "defaults", + }, + }, + { + Name: "rootlv", + Payload: &disk.Filesystem{ + Label: "root", + Type: "ext4", + Mountpoint: "/root", + FSTabOptions: "defaults", + }, + }, + { + Name: "rootlv00", + Payload: &disk.Filesystem{ + Label: "root00", + Type: "xfs", + Mountpoint: "/", + FSTabOptions: "defaults", + }, + }, + }, + }, + }, + }, + }, + }, + "btrfs-collision": { + pt: disk.PartitionTable{ + Partitions: []disk.Partition{ + { + Payload: &disk.Btrfs{ + Subvolumes: []disk.BtrfsSubvolume{ + { + Name: "subvol/home", + Mountpoint: "/home", + }, + { + Name: "root", + Mountpoint: "/root", + }, + }, + }, + }, + }, + }, + expected: disk.PartitionTable{ + Partitions: []disk.Partition{ + { + Payload: &disk.Btrfs{ + Subvolumes: []disk.BtrfsSubvolume{ + { + Name: "subvol/home", + Mountpoint: "/home", + }, + { + Name: "root", + Mountpoint: "/root", + }, + { + Name: "root00", + Mountpoint: "/", + }, + }, + }, + }, + }, + }, + }, + } + + for name := range testCases { + tc := testCases[name] + t.Run(name, func(t *testing.T) { + assert := assert.New(t) + pt := tc.pt + err := disk.EnsureRootFilesystem(&pt, tc.defaultFsType) + assert.NoError(err) + assert.Equal(tc.expected, pt) + }) + } +} + +func TestEnsureRootFilesystemErrors(t *testing.T) { + type testCase struct { + pt disk.PartitionTable + defaultFsType disk.FSType + errmsg string + } + + testCases := map[string]testCase{ + "err-empty": { + pt: disk.PartitionTable{}, + errmsg: "error creating root partition: no default filesystem type", + }, + "err-plain": { + pt: disk.PartitionTable{ + Partitions: []disk.Partition{ + { + Payload: &disk.Filesystem{ + Type: "ext4", + Label: "home", + Mountpoint: "/home", + FSTabOptions: "defaults", + }, + }, + }, + }, + errmsg: "error creating root partition: no default filesystem type", + }, + "err-lvm": { + pt: disk.PartitionTable{ + Partitions: []disk.Partition{ + { + Payload: &disk.LVMVolumeGroup{ + Name: "testvg", + LogicalVolumes: []disk.LVMLogicalVolume{ + { + Name: "varloglv", + Payload: &disk.Filesystem{ + Label: "var-log", + Type: "xfs", + Mountpoint: "/var/log", + }, + }, + { + Name: "datalv", + Payload: &disk.Filesystem{ + Label: "data", + Mountpoint: "/data", + FSTabOptions: "defaults", + Type: "ext4", + }, + }, + }, + }, + }, + }, + }, + errmsg: "error creating root logical volume: no default filesystem type", + }, + } + + for name := range testCases { + tc := testCases[name] + t.Run(name, func(t *testing.T) { + assert := assert.New(t) + pt := tc.pt + err := disk.EnsureRootFilesystem(&pt, tc.defaultFsType) + assert.EqualError(err, tc.errmsg) + }) + } +} From c88ba0deaaafc3ff8e1c1c9e8e6797e2ad4529ca Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Fri, 1 Nov 2024 16:16:02 +0100 Subject: [PATCH 3/6] disk: EnsureBootPartition() helper function Add a function that takes a partition table and adds a boot partition, but only if one does not already exist. A boot partition is any mountable where the mountpoint is `/boot`. Although the /boot mountpoint must always be on a plain partition, this is not validated. This function is not currently used because the NewPartitionTable() function handles this automatically during layout conversions (in ensureLVM() and ensureBtrfs()), but it will be part of the new custom partition table generator. --- pkg/disk/partition_table.go | 44 +++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/pkg/disk/partition_table.go b/pkg/disk/partition_table.go index ff68ed3b0..77adc87b7 100644 --- a/pkg/disk/partition_table.go +++ b/pkg/disk/partition_table.go @@ -990,3 +990,47 @@ func EnsureRootFilesystem(pt *PartitionTable, defaultFsType FSType) error { pt.Partitions = append(pt.Partitions, rootpart) return nil } + +// EnsureBootPartition creates a boot partition if one does not already exist. +// 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 EnsureBootPartition(pt *PartitionTable, bootFsType FSType) error { + // collect all labels to avoid conflicts + labels := make(map[string]bool) + var foundBoot bool + _ = pt.ForEachMountable(func(mnt Mountable, path []Entity) error { + if mnt.GetMountpoint() == "/boot" { + foundBoot = true + return nil + } + + labels[mnt.GetFSSpec().Label] = true + return nil + }) + if foundBoot { + // nothing to do + return nil + } + + if bootFsType == FS_NONE { + return fmt.Errorf("error creating boot partition: no filesystem type") + } + + bootLabel, err := genUniqueString("boot", labels) + if err != nil { + return fmt.Errorf("error creating boot partition: %w", err) + } + bootPart := Partition{ + Type: XBootLDRPartitionGUID, + Size: 512 * datasizes.MiB, + Payload: &Filesystem{ + Type: bootFsType.String(), + Label: bootLabel, + Mountpoint: "/boot", + FSTabOptions: "defaults", + }, + } + pt.Partitions = append(pt.Partitions, bootPart) + return nil +} From b3fd257c0a1df42536bcdd8e0c0bd176276bccfc Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Fri, 1 Nov 2024 16:30:44 +0100 Subject: [PATCH 4/6] disk: add test for EnsureBootPartition() --- pkg/disk/partition_table_test.go | 234 +++++++++++++++++++++++++++++++ 1 file changed, 234 insertions(+) diff --git a/pkg/disk/partition_table_test.go b/pkg/disk/partition_table_test.go index 8d4788258..77a5e9f6f 100644 --- a/pkg/disk/partition_table_test.go +++ b/pkg/disk/partition_table_test.go @@ -644,3 +644,237 @@ func TestEnsureRootFilesystemErrors(t *testing.T) { }) } } + +func TestEnsureBootPartition(t *testing.T) { + type testCase struct { + pt disk.PartitionTable + expected disk.PartitionTable + fsType disk.FSType + errmsg string + } + + testCases := map[string]testCase{ + "empty-plain": { + pt: disk.PartitionTable{}, + fsType: disk.FS_EXT4, + expected: disk.PartitionTable{ + Partitions: []disk.Partition{ + { + Start: 0, + Size: 512 * datasizes.MiB, + Type: disk.XBootLDRPartitionGUID, + Bootable: false, + UUID: "", + Payload: &disk.Filesystem{ + Type: "ext4", + Label: "boot", + Mountpoint: "/boot", + FSTabOptions: "defaults", + }, + }, + }, + }, + }, + "simple-plain": { + pt: disk.PartitionTable{ + Partitions: []disk.Partition{ + { + Payload: &disk.Filesystem{ + Type: "ext4", + Label: "home", + Mountpoint: "/home", + FSTabOptions: "defaults", + }, + }, + }, + }, + fsType: disk.FS_EXT4, + expected: disk.PartitionTable{ + Partitions: []disk.Partition{ + { + Payload: &disk.Filesystem{ + Type: "ext4", + Label: "home", + Mountpoint: "/home", + FSTabOptions: "defaults", + }, + }, + { + Start: 0, + Size: 512 * datasizes.MiB, + Type: disk.XBootLDRPartitionGUID, + Bootable: false, + UUID: "", + Payload: &disk.Filesystem{ + Type: "ext4", + Label: "boot", + Mountpoint: "/boot", + FSTabOptions: "defaults", + }, + }, + }, + }, + }, + "noop": { + pt: disk.PartitionTable{ + Partitions: []disk.Partition{ + { + Start: 0, + Size: 0, + Type: disk.XBootLDRPartitionGUID, + Bootable: false, + UUID: "", + Payload: &disk.Filesystem{ + Type: "ext4", + Label: "boot", + Mountpoint: "/boot", + FSTabOptions: "defaults", + }, + }, + { + Payload: &disk.LVMVolumeGroup{ + Name: "testvg", + LogicalVolumes: []disk.LVMLogicalVolume{ + { + Name: "varloglv", + Payload: &disk.Filesystem{ + Label: "var-log", + Type: "xfs", + Mountpoint: "/var/log", + }, + }, + { + Name: "datalv", + Payload: &disk.Filesystem{ + Label: "data", + Type: "ext4", + Mountpoint: "/data", + FSTabOptions: "defaults", + }, + }, + { + Name: "rootlv", + Payload: &disk.Filesystem{ + Label: "root", + Type: "ext4", + Mountpoint: "/", + FSTabOptions: "defaults", + }, + }, + }, + }, + }, + }, + }, + expected: disk.PartitionTable{ + Partitions: []disk.Partition{ + { + Start: 0, + Size: 0, + Type: disk.XBootLDRPartitionGUID, + Bootable: false, + UUID: "", + Payload: &disk.Filesystem{ + Type: "ext4", + Label: "boot", + Mountpoint: "/boot", + FSTabOptions: "defaults", + }, + }, + { + Payload: &disk.LVMVolumeGroup{ + Name: "testvg", + LogicalVolumes: []disk.LVMLogicalVolume{ + { + Name: "varloglv", + Payload: &disk.Filesystem{ + Label: "var-log", + Type: "xfs", + Mountpoint: "/var/log", + }, + }, + { + Name: "datalv", + Payload: &disk.Filesystem{ + Label: "data", + Type: "ext4", + Mountpoint: "/data", + FSTabOptions: "defaults", + }, + }, + { + Name: "rootlv", + Payload: &disk.Filesystem{ + Label: "root", + Type: "ext4", + Mountpoint: "/", + FSTabOptions: "defaults", + }, + }, + }, + }, + }, + }, + }, + }, + "label-collision": { + pt: disk.PartitionTable{ + Partitions: []disk.Partition{ + { + Payload: &disk.Filesystem{ + Type: "ext4", + Label: "boot", + Mountpoint: "/collections/footwear/boot", + FSTabOptions: "defaults", + }, + }, + }, + }, + fsType: disk.FS_EXT4, + expected: disk.PartitionTable{ + Partitions: []disk.Partition{ + { + Payload: &disk.Filesystem{ + Type: "ext4", + Label: "boot", + Mountpoint: "/collections/footwear/boot", + FSTabOptions: "defaults", + }, + }, + { + Start: 0, + Size: 512 * datasizes.MiB, + Type: disk.XBootLDRPartitionGUID, + Bootable: false, + UUID: "", + Payload: &disk.Filesystem{ + Type: "ext4", + Label: "boot00", + Mountpoint: "/boot", + FSTabOptions: "defaults", + }, + }, + }, + }, + }, + "err-nofs": { + pt: disk.PartitionTable{}, + errmsg: "error creating boot partition: no filesystem type", + }, + } + + for name := range testCases { + tc := testCases[name] + t.Run(name, func(t *testing.T) { + assert := assert.New(t) + pt := tc.pt + err := disk.EnsureBootPartition(&pt, tc.fsType) + if tc.errmsg == "" { + assert.NoError(err) + assert.Equal(tc.expected, pt) + } else { + assert.EqualError(err, tc.errmsg) + } + }) + } +} From 4123bd53dad3b3628b0bdfcf80e6daca2e96fefd Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Tue, 5 Nov 2024 17:43:28 +0100 Subject: [PATCH 5/6] disk: set the root and boot partition type correctly for dos Define a new const string: DosLinuxTypeID, which is used for Linux filesystems on dos/mbr partition tables. In the new helper functions, use the partition table type to determine which ID to use. This means that the functions will return an error when the partition table type is not set (or is set to an unknown value). Update the tests to match the new behaviour and to test the correct ID selection. --- pkg/disk/disk.go | 3 + pkg/disk/partition_table.go | 26 ++++- pkg/disk/partition_table_test.go | 157 +++++++++++++++++++++++++++++-- 3 files changed, 177 insertions(+), 9 deletions(-) diff --git a/pkg/disk/disk.go b/pkg/disk/disk.go index 6c0a3d664..5c26d18aa 100644 --- a/pkg/disk/disk.go +++ b/pkg/disk/disk.go @@ -60,6 +60,9 @@ const ( // DosFat16B used for the ESP-System partition DosFat16B = "06" + + // Partition type ID for any native Linux filesystem on dos + DosLinuxTypeID = "83" ) // FSType is the filesystem type enum. diff --git a/pkg/disk/partition_table.go b/pkg/disk/partition_table.go index 77adc87b7..add00d302 100644 --- a/pkg/disk/partition_table.go +++ b/pkg/disk/partition_table.go @@ -795,7 +795,7 @@ func (pt *PartitionTable) ensureBtrfs() error { if pt.Type == "gpt" { part.Type = FilesystemDataGUID } else { - part.Type = "83" + part.Type = DosLinuxTypeID } } else { @@ -977,8 +977,18 @@ func EnsureRootFilesystem(pt *PartitionTable, defaultFsType FSType) error { if err != nil { return fmt.Errorf("error creating root partition: %w", err) } + + var partType string + switch pt.Type { + case "dos": + partType = DosLinuxTypeID + case "gpt": + partType = FilesystemDataGUID + default: + return fmt.Errorf("error creating root partition: unknown or unsupported partition table type: %s", pt.Type) + } rootpart := Partition{ - Type: FilesystemDataGUID, + Type: partType, Size: 0, // Set the size to 0 and it will be adjusted by EnsureDirectorySizes() and relayout() Payload: &Filesystem{ Type: defaultFsType.String(), @@ -1021,8 +1031,18 @@ func EnsureBootPartition(pt *PartitionTable, bootFsType FSType) error { if err != nil { return fmt.Errorf("error creating boot partition: %w", err) } + + var partType string + switch pt.Type { + case "dos": + partType = DosLinuxTypeID + case "gpt": + partType = XBootLDRPartitionGUID + default: + return fmt.Errorf("error creating boot partition: unknown or unsupported partition table type: %s", pt.Type) + } bootPart := Partition{ - Type: XBootLDRPartitionGUID, + Type: partType, Size: 512 * datasizes.MiB, Payload: &Filesystem{ Type: bootFsType.String(), diff --git a/pkg/disk/partition_table_test.go b/pkg/disk/partition_table_test.go index 77a5e9f6f..ffd486c19 100644 --- a/pkg/disk/partition_table_test.go +++ b/pkg/disk/partition_table_test.go @@ -114,10 +114,11 @@ func TestEnsureRootFilesystem(t *testing.T) { } testCases := map[string]testCase{ - "empty-plain": { - pt: disk.PartitionTable{}, + "empty-plain-gpt": { + pt: disk.PartitionTable{Type: "gpt"}, defaultFsType: disk.FS_EXT4, expected: disk.PartitionTable{ + Type: "gpt", Partitions: []disk.Partition{ { Start: 0, @@ -135,8 +136,31 @@ func TestEnsureRootFilesystem(t *testing.T) { }, }, }, - "simple-plain": { + "empty-plain-dos": { + pt: disk.PartitionTable{Type: "dos"}, + defaultFsType: disk.FS_EXT4, + expected: disk.PartitionTable{ + Type: "dos", + Partitions: []disk.Partition{ + { + Start: 0, + Size: 0, + Type: disk.DosLinuxTypeID, + Bootable: false, + UUID: "", + Payload: &disk.Filesystem{ + Type: "ext4", + Label: "root", + Mountpoint: "/", + FSTabOptions: "defaults", + }, + }, + }, + }, + }, + "simple-plain-gpt": { pt: disk.PartitionTable{ + Type: "gpt", Partitions: []disk.Partition{ { Payload: &disk.Filesystem{ @@ -150,6 +174,7 @@ func TestEnsureRootFilesystem(t *testing.T) { }, defaultFsType: disk.FS_EXT4, expected: disk.PartitionTable{ + Type: "gpt", Partitions: []disk.Partition{ { Payload: &disk.Filesystem{ @@ -175,6 +200,48 @@ func TestEnsureRootFilesystem(t *testing.T) { }, }, }, + "simple-plain-dos": { + pt: disk.PartitionTable{ + Type: "dos", + Partitions: []disk.Partition{ + { + Payload: &disk.Filesystem{ + Type: "ext4", + Label: "home", + Mountpoint: "/home", + FSTabOptions: "defaults", + }, + }, + }, + }, + defaultFsType: disk.FS_EXT4, + expected: disk.PartitionTable{ + Type: "dos", + Partitions: []disk.Partition{ + { + Payload: &disk.Filesystem{ + Type: "ext4", + Label: "home", + Mountpoint: "/home", + FSTabOptions: "defaults", + }, + }, + { + Start: 0, + Size: 0, + Type: disk.DosLinuxTypeID, + Bootable: false, + UUID: "", + Payload: &disk.Filesystem{ + Type: "ext4", + Label: "root", + Mountpoint: "/", + FSTabOptions: "defaults", + }, + }, + }, + }, + }, "simple-lvm": { pt: disk.PartitionTable{ Partitions: []disk.Partition{ @@ -394,6 +461,7 @@ func TestEnsureRootFilesystem(t *testing.T) { }, "plain-collision": { pt: disk.PartitionTable{ + Type: "gpt", Partitions: []disk.Partition{ { Payload: &disk.Filesystem{ @@ -407,6 +475,7 @@ func TestEnsureRootFilesystem(t *testing.T) { }, defaultFsType: disk.FS_EXT4, expected: disk.PartitionTable{ + Type: "gpt", Partitions: []disk.Partition{ { Payload: &disk.Filesystem{ @@ -434,6 +503,7 @@ func TestEnsureRootFilesystem(t *testing.T) { }, "lvm-collision": { pt: disk.PartitionTable{ + Type: "gpt", Partitions: []disk.Partition{ { Payload: &disk.LVMVolumeGroup{ @@ -472,6 +542,7 @@ func TestEnsureRootFilesystem(t *testing.T) { }, defaultFsType: disk.FS_XFS, expected: disk.PartitionTable{ + Type: "gpt", Partitions: []disk.Partition{ { Payload: &disk.LVMVolumeGroup{ @@ -586,6 +657,11 @@ func TestEnsureRootFilesystemErrors(t *testing.T) { pt: disk.PartitionTable{}, errmsg: "error creating root partition: no default filesystem type", }, + "err-no-pt-type": { + pt: disk.PartitionTable{}, + defaultFsType: disk.FS_EXT4, + errmsg: "error creating root partition: unknown or unsupported partition table type: ", + }, "err-plain": { pt: disk.PartitionTable{ Partitions: []disk.Partition{ @@ -654,10 +730,11 @@ func TestEnsureBootPartition(t *testing.T) { } testCases := map[string]testCase{ - "empty-plain": { - pt: disk.PartitionTable{}, + "empty-plain-gpt": { + pt: disk.PartitionTable{Type: "gpt"}, fsType: disk.FS_EXT4, expected: disk.PartitionTable{ + Type: "gpt", Partitions: []disk.Partition{ { Start: 0, @@ -675,8 +752,31 @@ func TestEnsureBootPartition(t *testing.T) { }, }, }, - "simple-plain": { + "empty-plain-dos": { + pt: disk.PartitionTable{Type: "dos"}, + fsType: disk.FS_EXT4, + expected: disk.PartitionTable{ + Type: "dos", + Partitions: []disk.Partition{ + { + Start: 0, + Size: 512 * datasizes.MiB, + Type: disk.DosLinuxTypeID, + Bootable: false, + UUID: "", + Payload: &disk.Filesystem{ + Type: "ext4", + Label: "boot", + Mountpoint: "/boot", + FSTabOptions: "defaults", + }, + }, + }, + }, + }, + "simple-plain-gpt": { pt: disk.PartitionTable{ + Type: "gpt", Partitions: []disk.Partition{ { Payload: &disk.Filesystem{ @@ -690,6 +790,7 @@ func TestEnsureBootPartition(t *testing.T) { }, fsType: disk.FS_EXT4, expected: disk.PartitionTable{ + Type: "gpt", Partitions: []disk.Partition{ { Payload: &disk.Filesystem{ @@ -715,6 +816,48 @@ func TestEnsureBootPartition(t *testing.T) { }, }, }, + "simple-plain-dos": { + pt: disk.PartitionTable{ + Type: "dos", + Partitions: []disk.Partition{ + { + Payload: &disk.Filesystem{ + Type: "ext4", + Label: "home", + Mountpoint: "/home", + FSTabOptions: "defaults", + }, + }, + }, + }, + fsType: disk.FS_EXT4, + expected: disk.PartitionTable{ + Type: "dos", + Partitions: []disk.Partition{ + { + Payload: &disk.Filesystem{ + Type: "ext4", + Label: "home", + Mountpoint: "/home", + FSTabOptions: "defaults", + }, + }, + { + Start: 0, + Size: 512 * datasizes.MiB, + Type: disk.DosLinuxTypeID, + Bootable: false, + UUID: "", + Payload: &disk.Filesystem{ + Type: "ext4", + Label: "boot", + Mountpoint: "/boot", + FSTabOptions: "defaults", + }, + }, + }, + }, + }, "noop": { pt: disk.PartitionTable{ Partitions: []disk.Partition{ @@ -819,6 +962,7 @@ func TestEnsureBootPartition(t *testing.T) { }, "label-collision": { pt: disk.PartitionTable{ + Type: "gpt", Partitions: []disk.Partition{ { Payload: &disk.Filesystem{ @@ -832,6 +976,7 @@ func TestEnsureBootPartition(t *testing.T) { }, fsType: disk.FS_EXT4, expected: disk.PartitionTable{ + Type: "gpt", Partitions: []disk.Partition{ { Payload: &disk.Filesystem{ From 88d6a6a3f77cc1996d6ca5957bc8f09676e79f4e Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Tue, 5 Nov 2024 18:59:21 +0100 Subject: [PATCH 6/6] distro/fedora: use new const for all occurrences of Type: "83" --- pkg/distro/fedora/partition_tables.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/distro/fedora/partition_tables.go b/pkg/distro/fedora/partition_tables.go index 3984210fb..1f243ab6c 100644 --- a/pkg/distro/fedora/partition_tables.go +++ b/pkg/distro/fedora/partition_tables.go @@ -238,7 +238,7 @@ var minimalrawPartitionTables = distro.BasePartitionTableMap{ }, { Size: 1 * datasizes.GibiByte, - Type: "83", + Type: disk.DosLinuxTypeID, Payload: &disk.Filesystem{ Type: "ext4", Mountpoint: "/boot", @@ -250,7 +250,7 @@ var minimalrawPartitionTables = distro.BasePartitionTableMap{ }, { Size: 2 * datasizes.GibiByte, - Type: "83", + Type: disk.DosLinuxTypeID, Payload: &disk.Filesystem{ Type: "ext4", Label: "root", @@ -333,7 +333,7 @@ var iotBasePartitionTables = distro.BasePartitionTableMap{ }, { Size: 1 * datasizes.GibiByte, - Type: "83", + Type: disk.DosLinuxTypeID, Payload: &disk.Filesystem{ Type: "ext4", Mountpoint: "/boot", @@ -345,7 +345,7 @@ var iotBasePartitionTables = distro.BasePartitionTableMap{ }, { Size: 2569 * datasizes.MebiByte, - Type: "83", + Type: disk.DosLinuxTypeID, Payload: &disk.Filesystem{ Type: "ext4", Label: "root",