From f041f5f2598141464c5cdb71d8700e2eb3de8fcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Budai?= Date: Fri, 30 Aug 2024 16:01:20 +0200 Subject: [PATCH 01/13] osbuild: add org.osbuild.mkswap stage --- pkg/osbuild/mkswap_stage.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 pkg/osbuild/mkswap_stage.go diff --git a/pkg/osbuild/mkswap_stage.go b/pkg/osbuild/mkswap_stage.go new file mode 100644 index 000000000..d4cfdabd6 --- /dev/null +++ b/pkg/osbuild/mkswap_stage.go @@ -0,0 +1,16 @@ +package osbuild + +type MkswapStageOptions struct { + UUID string `json:"uuid"` + Label string `json:"label,omitempty"` +} + +func (MkswapStageOptions) isStageOptions() {} + +func NewMkswapStage(options *MkswapStageOptions, devices map[string]Device) *Stage { + return &Stage{ + Type: "org.osbuild.mkswap", + Options: options, + Devices: devices, + } +} From 37e5f50df5661f6692913f527e3ab4b2ded7051d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Budai?= Date: Fri, 30 Aug 2024 16:01:20 +0200 Subject: [PATCH 02/13] osbuild: add a test for the fstab stage I haven't been able to find one, so this commit adds a simple smoke test for NewFSTabStageOptions. --- pkg/osbuild/fstab_stage_test.go | 35 +++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/pkg/osbuild/fstab_stage_test.go b/pkg/osbuild/fstab_stage_test.go index e8b6fc223..ab29a6cd4 100644 --- a/pkg/osbuild/fstab_stage_test.go +++ b/pkg/osbuild/fstab_stage_test.go @@ -3,7 +3,10 @@ package osbuild import ( "testing" + "github.com/osbuild/images/internal/testdisk" + "github.com/osbuild/images/pkg/disk" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestNewFSTabStage(t *testing.T) { @@ -50,3 +53,35 @@ func TestAddFilesystem(t *testing.T) { } assert.Equal(t, len(filesystems), len(options.FileSystems)) } + +func TestNewFSTabStageOptions(t *testing.T) { + pt := testdisk.MakeFakePartitionTable("/", "/boot", "/boot/efi", "/home") + + opts, err := NewFSTabStageOptions(pt) + require.NoError(t, err) + + assert.Equal(t, &FSTabStageOptions{ + FileSystems: []*FSTabEntry{ + { + UUID: disk.RootPartitionUUID, + VFSType: "ext4", + Path: "/", + }, + { + UUID: disk.FilesystemDataUUID, + VFSType: "ext4", + Path: "/boot", + }, + { + UUID: disk.EFIFilesystemUUID, + VFSType: "vfat", + Path: "/boot/efi", + }, + { + UUID: disk.FilesystemDataUUID, + VFSType: "ext4", + Path: "/home", + }, + }, + }, opts) +} From b188ef55701666ea62fc446ee1a947696bd11e87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Budai?= Date: Fri, 30 Aug 2024 16:01:20 +0200 Subject: [PATCH 03/13] testdisk: support swap in fake partitions This is the first commit in the series of commits whose goal is to add swap support to disk library. In order to have everything related to swap covered with tests, we need to add support for swap to the fake partition tables used in tests. --- internal/testdisk/partition.go | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/internal/testdisk/partition.go b/internal/testdisk/partition.go index 34a0dbde9..92a988877 100644 --- a/internal/testdisk/partition.go +++ b/internal/testdisk/partition.go @@ -22,6 +22,8 @@ func MakeFakePartitionTable(mntPoints ...string) *disk.PartitionTable { case "/boot/efi": payload.UUID = disk.EFIFilesystemUUID payload.Type = "vfat" + case "swap": + payload.Type = "swap" default: payload.UUID = disk.FilesystemDataUUID } @@ -70,6 +72,15 @@ func MakeFakeBtrfsPartitionTable(mntPoints ...string) *disk.PartitionTable { }, }) size += 100 * common.MiB + case "swap": + pt.Partitions = append(pt.Partitions, disk.Partition{ + Start: size, + Size: 1 * common.GiB, + Payload: &disk.Filesystem{ + Type: "swap", + }, + }) + size += 1 * common.GiB default: name := mntPoint if name == "/" { @@ -104,6 +115,7 @@ func MakeFakeBtrfsPartitionTable(mntPoints ...string) *disk.PartitionTable { // MakeFakeLVMPartitionTable is similar to MakeFakePartitionTable but // creates a lvm-based partition table. +// Note that mntPoint "swap" is created as a LV-based swap filesystem. func MakeFakeLVMPartitionTable(mntPoints ...string) *disk.PartitionTable { var lvs []disk.LVMLogicalVolume pt := &disk.PartitionTable{ @@ -140,12 +152,17 @@ func MakeFakeLVMPartitionTable(mntPoints ...string) *disk.PartitionTable { if name == "/" { name = "lvroot" } + fsType := "xfs" + if mntPoint == "swap" { + fsType = "swap" + } + lvs = append( lvs, disk.LVMLogicalVolume{ Name: name, Payload: &disk.Filesystem{ - Type: "xfs", + Type: fsType, Mountpoint: mntPoint, }, }, From 7c83b6f57b678a85dcbc8e2c7f78635651ca920b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Budai?= Date: Fri, 30 Aug 2024 16:01:20 +0200 Subject: [PATCH 04/13] osbuild: do not create osbuild mounts for swap partitions If the fs type of a mountable is set to swap, it's a swap partition, and thus we cannot mount it when building a manifest. So let's not create an osbuild mount for it. --- pkg/osbuild/bootupd_stage.go | 11 +++++++++++ pkg/osbuild/bootupd_stage_test.go | 29 +++++++++++++++++++++++++---- pkg/osbuild/device.go | 7 +++++++ pkg/osbuild/device_test.go | 6 +++--- 4 files changed, 46 insertions(+), 7 deletions(-) diff --git a/pkg/osbuild/bootupd_stage.go b/pkg/osbuild/bootupd_stage.go index bee683fba..f8cecc31a 100644 --- a/pkg/osbuild/bootupd_stage.go +++ b/pkg/osbuild/bootupd_stage.go @@ -96,6 +96,10 @@ func genMountsForBootupd(source string, pt *disk.PartitionTable) ([]Mount, error if err != nil { return nil, err } + if mount == nil { + continue + + } mount.Partition = common.ToPtr(idx + 1) mounts = append(mounts, *mount) case *disk.Btrfs: @@ -104,6 +108,9 @@ func genMountsForBootupd(source string, pt *disk.PartitionTable) ([]Mount, error if err != nil { return nil, err } + if mount == nil { + continue + } mount.Partition = common.ToPtr(idx + 1) mounts = append(mounts, *mount) } @@ -118,6 +125,9 @@ func genMountsForBootupd(source string, pt *disk.PartitionTable) ([]Mount, error if err != nil { return nil, err } + if mount == nil { + continue + } mount.Source = lv.Name mounts = append(mounts, *mount) } @@ -147,6 +157,7 @@ func genDevicesForBootupd(filename, devName string, pt *disk.PartitionTable) (ma switch payload := part.Payload.(type) { case *disk.LVMVolumeGroup: for _, lv := range payload.LogicalVolumes { + // TODO: ignore swap LVs. It's harmless to create a device for them, but it's also pointless. // partitions start with "1", so add "1" partNum := idx + 1 devices[lv.Name] = *NewLVM2LVDevice(devName, &LVM2LVDeviceOptions{Volume: lv.Name, VGPartnum: common.ToPtr(partNum)}) diff --git a/pkg/osbuild/bootupd_stage_test.go b/pkg/osbuild/bootupd_stage_test.go index 4047e57af..ae20b2e3f 100644 --- a/pkg/osbuild/bootupd_stage_test.go +++ b/pkg/osbuild/bootupd_stage_test.go @@ -248,6 +248,19 @@ var fakePt = &disk.PartitionTable{ FSTabPassNo: 1, }, }, + + { + Size: 2 * common.GibiByte, + Type: disk.SwapPartitionGUID, + Payload: &disk.Filesystem{ + Type: "swap", + Label: "swap", + Mountpoint: "/", + FSTabOptions: "defaults", + FSTabFreq: 1, + FSTabPassNo: 1, + }, + }, }, } @@ -301,7 +314,7 @@ func TestGenBootupdDevicesMountsHappyBtrfs(t *testing.T) { UEFIVendor: "test", } - devices, mounts, err := osbuild.GenBootupdDevicesMounts(filename, testdisk.MakeFakeBtrfsPartitionTable("/", "/home", "/boot/efi", "/boot"), pf) + devices, mounts, err := osbuild.GenBootupdDevicesMounts(filename, testdisk.MakeFakeBtrfsPartitionTable("/", "/home", "/boot/efi", "/boot", "swap"), pf) require.Nil(t, err) assert.Equal(t, devices, map[string]osbuild.Device{ "disk": { @@ -319,7 +332,7 @@ func TestGenBootupdDevicesMountsHappyBtrfs(t *testing.T) { Source: "disk", Target: "/", Options: osbuild.BtrfsMountOptions{Subvol: "root", Compress: "zstd:1"}, - Partition: common.ToPtr(3), + Partition: common.ToPtr(4), }, { Name: "boot", @@ -341,7 +354,7 @@ func TestGenBootupdDevicesMountsHappyBtrfs(t *testing.T) { Source: "disk", Target: "/home", Options: osbuild.BtrfsMountOptions{Subvol: "/home", Compress: "zstd:1"}, - Partition: common.ToPtr(3), + Partition: common.ToPtr(4), }, }, mounts) } @@ -353,7 +366,7 @@ func TestGenBootupdDevicesMountsHappyLVM(t *testing.T) { UEFIVendor: "test", } - fakePt := testdisk.MakeFakeLVMPartitionTable("/", "/home", "/boot/efi", "/boot") + fakePt := testdisk.MakeFakeLVMPartitionTable("/", "/home", "/boot/efi", "/boot", "swap") devices, mounts, err := osbuild.GenBootupdDevicesMounts(filename, fakePt, pf) require.Nil(t, err) assert.Equal(t, devices, map[string]osbuild.Device{ @@ -380,6 +393,14 @@ func TestGenBootupdDevicesMountsHappyLVM(t *testing.T) { VGPartnum: common.ToPtr(3), }, }, + "lv-for-swap": { + Type: "org.osbuild.lvm2.lv", + Parent: "disk", + Options: &osbuild.LVM2LVDeviceOptions{ + Volume: "lv-for-swap", + VGPartnum: common.ToPtr(3), + }, + }, }) assert.Equal(t, []osbuild.Mount{ { diff --git a/pkg/osbuild/device.go b/pkg/osbuild/device.go index b6a9821ab..531bb4d38 100644 --- a/pkg/osbuild/device.go +++ b/pkg/osbuild/device.go @@ -244,6 +244,8 @@ func pathEscape(path string) string { // - source is the name of the device that the mount should be created from. // The name of the mount is derived from the mountpoint of the mountable, escaped with pathEscape. This shouldn't // create any conflicts, as the mountpoint is unique within the partition table. +// This function returns nil if the Mountable is swap, as it doesn't make sense to do anything with it +// during build. func genOsbuildMount(source string, mnt disk.Mountable) (*Mount, error) { mountpoint := mnt.GetMountpoint() name := pathEscape(mountpoint) @@ -261,6 +263,8 @@ func genOsbuildMount(source string, mnt disk.Mountable) (*Mount, error) { } else { return nil, fmt.Errorf("mounting bare btrfs partition is unsupported: %s", mountpoint) } + case "swap": + return nil, nil default: return nil, fmt.Errorf("unknown fs type " + t) } @@ -284,6 +288,9 @@ func GenMountsDevicesFromPT(filename string, pt *disk.PartitionTable) (string, [ if err != nil { return err } + if mount == nil { + return nil + } mountpoint := mnt.GetMountpoint() if mountpoint == "/" { diff --git a/pkg/osbuild/device_test.go b/pkg/osbuild/device_test.go index d61c2aa64..19598336c 100644 --- a/pkg/osbuild/device_test.go +++ b/pkg/osbuild/device_test.go @@ -189,7 +189,7 @@ func TestMountsDeviceFromPtNoRootErrors(t *testing.T) { func TestMountsDeviceFromPtHappy(t *testing.T) { filename := "fake-disk.img" - fakePt := testdisk.MakeFakePartitionTable("/", "/boot", "/boot/efi") + fakePt := testdisk.MakeFakePartitionTable("/", "/boot", "/boot/efi", "swap") fsRootMntName, mounts, devices, err := GenMountsDevicesFromPT(filename, fakePt) require.Nil(t, err) assert.Equal(t, fsRootMntName, "-") @@ -225,7 +225,7 @@ func TestMountsDeviceFromPtHappy(t *testing.T) { func TestMountsDeviceFromBrfs(t *testing.T) { filename := "fake-disk.img" - fakePt := testdisk.MakeFakeBtrfsPartitionTable("/", "/boot") + fakePt := testdisk.MakeFakeBtrfsPartitionTable("/", "/boot", "swap") fsRootMntName, mounts, devices, err := GenMountsDevicesFromPT(filename, fakePt) require.Nil(t, err) assert.Equal(t, "-", fsRootMntName) @@ -245,7 +245,7 @@ func TestMountsDeviceFromBrfs(t *testing.T) { Type: "org.osbuild.loopback", Options: &LoopbackDeviceOptions{ Filename: "fake-disk.img", - Start: 1 * common.GiB / 512, + Start: 2 * common.GiB / 512, Size: 9 * common.GiB / 512, }, }, From 77ceac4539764c8f46a13273caceca2b38084381 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Budai?= Date: Fri, 30 Aug 2024 16:01:20 +0200 Subject: [PATCH 05/13] disk: force path to be none for swap filesystem Swap has "none" as the mountpoint (target) in fstab. Let's just force it in the Filesystem.GetMountpoint method. Extend the fstab stage test to check it. --- pkg/disk/filesystem.go | 5 +++++ pkg/osbuild/fstab_stage_test.go | 6 +++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/pkg/disk/filesystem.go b/pkg/disk/filesystem.go index c5a3bd75b..0f483da99 100644 --- a/pkg/disk/filesystem.go +++ b/pkg/disk/filesystem.go @@ -52,6 +52,11 @@ func (fs *Filesystem) GetMountpoint() string { if fs == nil { return "" } + + if fs.Type == "swap" { + return "none" + } + return fs.Mountpoint } diff --git a/pkg/osbuild/fstab_stage_test.go b/pkg/osbuild/fstab_stage_test.go index ab29a6cd4..510b3bdcd 100644 --- a/pkg/osbuild/fstab_stage_test.go +++ b/pkg/osbuild/fstab_stage_test.go @@ -55,7 +55,7 @@ func TestAddFilesystem(t *testing.T) { } func TestNewFSTabStageOptions(t *testing.T) { - pt := testdisk.MakeFakePartitionTable("/", "/boot", "/boot/efi", "/home") + pt := testdisk.MakeFakePartitionTable("/", "/boot", "/boot/efi", "/home", "swap") opts, err := NewFSTabStageOptions(pt) require.NoError(t, err) @@ -82,6 +82,10 @@ func TestNewFSTabStageOptions(t *testing.T) { VFSType: "ext4", Path: "/home", }, + { + VFSType: "swap", + Path: "none", + }, }, }, opts) } From 62ea06e2320add075cd9251cff166a131923624f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Budai?= Date: Fri, 30 Aug 2024 16:01:20 +0200 Subject: [PATCH 06/13] [higher] osbuild: add LVM test for GenMkfsStages --- internal/testdisk/partition.go | 1 + pkg/osbuild/mkfs_stages_test.go | 81 +++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+) diff --git a/internal/testdisk/partition.go b/internal/testdisk/partition.go index 92a988877..a03f3707e 100644 --- a/internal/testdisk/partition.go +++ b/internal/testdisk/partition.go @@ -174,6 +174,7 @@ func MakeFakeLVMPartitionTable(mntPoints ...string) *disk.PartitionTable { Start: size, Size: 9 * common.GiB, Payload: &disk.LVMVolumeGroup{ + Name: "rootvg", LogicalVolumes: lvs, }, }) diff --git a/pkg/osbuild/mkfs_stages_test.go b/pkg/osbuild/mkfs_stages_test.go index b862975fe..2f480de5d 100644 --- a/pkg/osbuild/mkfs_stages_test.go +++ b/pkg/osbuild/mkfs_stages_test.go @@ -185,6 +185,87 @@ func TestGenMkfsStagesBtrfs(t *testing.T) { }, }, stages) } +func TestGenMkfsStagesLVM(t *testing.T) { + pt := testdisk.MakeFakeLVMPartitionTable("/", "/boot", "/boot/efi", "/home") + stages := GenMkfsStages(pt, "file.img") + assert.Equal(t, []*Stage{ + { + Type: "org.osbuild.mkfs.ext4", + Options: &MkfsExt4StageOptions{}, + Devices: map[string]Device{ + "device": { + Type: "org.osbuild.loopback", + Options: &LoopbackDeviceOptions{ + Filename: "file.img", + Size: common.GiB / disk.DefaultSectorSize, + Lock: true, + }, + }, + }, + }, + { + Type: "org.osbuild.mkfs.fat", + Options: &MkfsFATStageOptions{ + VolID: strings.ReplaceAll(disk.EFIFilesystemUUID, "-", ""), + }, + Devices: map[string]Device{ + "device": { + Type: "org.osbuild.loopback", + Options: &LoopbackDeviceOptions{ + Filename: "file.img", + Start: common.GiB / disk.DefaultSectorSize, + Size: 100 * common.MiB / disk.DefaultSectorSize, + Lock: true, + }, + }, + }, + }, + { + Type: "org.osbuild.mkfs.xfs", + Options: &MkfsXfsStageOptions{}, + Devices: map[string]Device{ + "rootvg": { + Type: "org.osbuild.loopback", + Options: &LoopbackDeviceOptions{ + Filename: "file.img", + Start: (common.GiB + 100*common.MiB) / disk.DefaultSectorSize, + Size: 9 * common.GiB / disk.DefaultSectorSize, + Lock: true, + }, + }, + "device": { + Type: "org.osbuild.lvm2.lv", + Parent: "rootvg", + Options: &LVM2LVDeviceOptions{ + Volume: "lv-for-/", + }, + }, + }, + }, + { + Type: "org.osbuild.mkfs.xfs", + Options: &MkfsXfsStageOptions{}, + Devices: map[string]Device{ + "rootvg": { + Type: "org.osbuild.loopback", + Options: &LoopbackDeviceOptions{ + Filename: "file.img", + Start: (common.GiB + 100*common.MiB) / disk.DefaultSectorSize, + Size: 9 * common.GiB / disk.DefaultSectorSize, + Lock: true, + }, + }, + "device": { + Type: "org.osbuild.lvm2.lv", + Parent: "rootvg", + Options: &LVM2LVDeviceOptions{ + Volume: "lv-for-/home", + }, + }, + }, + }, + }, stages) +} func TestGenMkfsStagesUnhappy(t *testing.T) { pt := &disk.PartitionTable{ From 1b888a1baff49d4f2f6edbf2693f973abdd2bf8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Budai?= Date: Sat, 31 Aug 2024 00:01:51 +0200 Subject: [PATCH 07/13] osbuild: handle swap when generating mkfs stages --- pkg/osbuild/mkfs_stage.go | 6 +++++ pkg/osbuild/mkfs_stages_test.go | 42 +++++++++++++++++++++++++++++++-- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/pkg/osbuild/mkfs_stage.go b/pkg/osbuild/mkfs_stage.go index b942406dd..25ebf328d 100644 --- a/pkg/osbuild/mkfs_stage.go +++ b/pkg/osbuild/mkfs_stage.go @@ -65,6 +65,12 @@ func GenMkfsStages(pt *disk.PartitionTable, filename string) []*Stage { Label: fsSpec.Label, } stage = NewMkfsExt4Stage(options, stageDevices) + case "swap": + options := &MkswapStageOptions{ + UUID: fsSpec.UUID, + Label: fsSpec.Label, + } + stage = NewMkswapStage(options, stageDevices) default: panic("unknown fs type " + t) } diff --git a/pkg/osbuild/mkfs_stages_test.go b/pkg/osbuild/mkfs_stages_test.go index 2f480de5d..49197d9e0 100644 --- a/pkg/osbuild/mkfs_stages_test.go +++ b/pkg/osbuild/mkfs_stages_test.go @@ -76,7 +76,7 @@ func TestNewMkfsStage(t *testing.T) { } func TestGenMkfsStages(t *testing.T) { - pt := testdisk.MakeFakePartitionTable("/", "/boot", "/boot/efi") + pt := testdisk.MakeFakePartitionTable("/", "/boot", "/boot/efi", "swap") stages := GenMkfsStages(pt, "file.img") assert.Equal(t, []*Stage{ { @@ -127,6 +127,21 @@ func TestGenMkfsStages(t *testing.T) { }, }, }, + + { + Type: "org.osbuild.mkswap", + Options: &MkswapStageOptions{}, + Devices: map[string]Device{ + "device": { + Type: "org.osbuild.loopback", + Options: &LoopbackDeviceOptions{ + Filename: "file.img", + Size: testdisk.FakePartitionSize / disk.DefaultSectorSize, + Lock: true, + }, + }, + }, + }, }, stages) } @@ -186,7 +201,7 @@ func TestGenMkfsStagesBtrfs(t *testing.T) { }, stages) } func TestGenMkfsStagesLVM(t *testing.T) { - pt := testdisk.MakeFakeLVMPartitionTable("/", "/boot", "/boot/efi", "/home") + pt := testdisk.MakeFakeLVMPartitionTable("/", "/boot", "/boot/efi", "/home", "swap") stages := GenMkfsStages(pt, "file.img") assert.Equal(t, []*Stage{ { @@ -264,6 +279,29 @@ func TestGenMkfsStagesLVM(t *testing.T) { }, }, }, + + { + Type: "org.osbuild.mkswap", + Options: &MkswapStageOptions{}, + Devices: map[string]Device{ + "rootvg": { + Type: "org.osbuild.loopback", + Options: &LoopbackDeviceOptions{ + Filename: "file.img", + Start: (common.GiB + 100*common.MiB) / disk.DefaultSectorSize, + Size: 9 * common.GiB / disk.DefaultSectorSize, + Lock: true, + }, + }, + "device": { + Type: "org.osbuild.lvm2.lv", + Parent: "rootvg", + Options: &LVM2LVDeviceOptions{ + Volume: "lv-for-swap", + }, + }, + }, + }, }, stages) } From e09a78fad60691ed63013888bd808c460361536d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Budai?= Date: Fri, 30 Aug 2024 16:07:55 +0200 Subject: [PATCH 08/13] disk: rename entityPath to entityPathForMountpoint The next commit will introduce a generalized version of entityPath. --- pkg/disk/disk_test.go | 28 ++++++++++++++-------------- pkg/disk/partition_table.go | 32 ++++++++++++++++---------------- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/pkg/disk/disk_test.go b/pkg/disk/disk_test.go index 96063e7a2..dbc78996e 100644 --- a/pkg/disk/disk_test.go +++ b/pkg/disk/disk_test.go @@ -424,7 +424,7 @@ func TestDisk_ForEachEntity(t *testing.T) { // returns nil if the blueprint was applied correctly, an error otherwise func blueprintApplied(pt *PartitionTable, bp []blueprint.FilesystemCustomization) error { for _, mnt := range bp { - path := entityPath(pt, mnt.Mountpoint) + path := entityPathForMountpoint(pt, mnt.Mountpoint) if path == nil { return fmt.Errorf("mountpoint %s not found", mnt.Mountpoint) } @@ -510,12 +510,12 @@ func TestCreatePartitionTableLVMify(t *testing.T) { mpt, err := NewPartitionTable(&pt, tbp, uint64(13*MiB), AutoLVMPartitioningMode, nil, rng) assert.NoError(err, "PT %q BP %q: Partition table generation failed: (%s)", ptName, bpName, err) - rootPath := entityPath(mpt, "/") + rootPath := entityPathForMountpoint(mpt, "/") if rootPath == nil { panic(fmt.Sprintf("PT %q BP %q: no root mountpoint", ptName, bpName)) } - bootPath := entityPath(mpt, "/boot") + bootPath := entityPathForMountpoint(mpt, "/boot") if tbp != nil && bootPath == nil { panic(fmt.Sprintf("PT %q BP %q: no boot mountpoint", ptName, bpName)) } @@ -548,12 +548,12 @@ func TestCreatePartitionTableBtrfsify(t *testing.T) { mpt, err := NewPartitionTable(&pt, tbp, uint64(13*MiB), BtrfsPartitioningMode, nil, rng) assert.NoError(err, "PT %q BP %q: Partition table generation failed: (%s)", ptName, bpName, err) - rootPath := entityPath(mpt, "/") + rootPath := entityPathForMountpoint(mpt, "/") if rootPath == nil { panic(fmt.Sprintf("PT %q BP %q: no root mountpoint", ptName, bpName)) } - bootPath := entityPath(mpt, "/boot") + bootPath := entityPathForMountpoint(mpt, "/boot") if tbp != nil && bootPath == nil { panic(fmt.Sprintf("PT %q BP %q: no boot mountpoint", ptName, bpName)) } @@ -586,12 +586,12 @@ func TestCreatePartitionTableLVMOnly(t *testing.T) { mpt, err := NewPartitionTable(&pt, tbp, uint64(13*MiB), LVMPartitioningMode, nil, rng) require.NoError(t, err, "PT %q BP %q: Partition table generation failed: (%s)", ptName, bpName, err) - rootPath := entityPath(mpt, "/") + rootPath := entityPathForMountpoint(mpt, "/") if rootPath == nil { panic(fmt.Sprintf("PT %q BP %q: no root mountpoint", ptName, bpName)) } - bootPath := entityPath(mpt, "/boot") + bootPath := entityPathForMountpoint(mpt, "/boot") if tbp != nil && bootPath == nil { panic(fmt.Sprintf("PT %q BP %q: no boot mountpoint", ptName, bpName)) } @@ -610,7 +610,7 @@ func TestCreatePartitionTableLVMOnly(t *testing.T) { // not on LVM; skipping continue } - mntPath := entityPath(mpt, mnt.Mountpoint) + mntPath := entityPathForMountpoint(mpt, mnt.Mountpoint) mntParent := mntPath[1] mntLV, ok := mntParent.(*LVMLogicalVolume) // the partition's parent should be the logical volume assert.True(ok, "PT %q BP %q: %s's parent (%+v) is not an LVM logical volume", ptName, bpName, mnt.Mountpoint, mntParent) @@ -738,7 +738,7 @@ func TestMinimumSizes(t *testing.T) { mpt, err := NewPartitionTable(&pt, tc.Blueprint, uint64(3*GiB), RawPartitioningMode, nil, rng) assert.NoError(err) for mnt, minSize := range tc.ExpectedMinSizes { - path := entityPath(mpt, mnt) + path := entityPathForMountpoint(mpt, mnt) assert.NotNil(path, "[%d] mountpoint %q not found", idx, mnt) parent := path[1] part, ok := parent.(*Partition) @@ -752,7 +752,7 @@ func TestMinimumSizes(t *testing.T) { mpt, err := NewPartitionTable(&pt, tc.Blueprint, uint64(3*GiB), AutoLVMPartitioningMode, nil, rng) assert.NoError(err) for mnt, minSize := range tc.ExpectedMinSizes { - path := entityPath(mpt, mnt) + path := entityPathForMountpoint(mpt, mnt) assert.NotNil(path, "[%d] mountpoint %q not found", idx, mnt) parent := path[1] part, ok := parent.(*LVMLogicalVolume) @@ -839,7 +839,7 @@ func TestLVMExtentAlignment(t *testing.T) { mpt, err := NewPartitionTable(&pt, tc.Blueprint, uint64(3*GiB), AutoLVMPartitioningMode, nil, rng) assert.NoError(err) for mnt, expSize := range tc.ExpectedSizes { - path := entityPath(mpt, mnt) + path := entityPathForMountpoint(mpt, mnt) assert.NotNil(path, "[%d] mountpoint %q not found", idx, mnt) parent := path[1] part, ok := parent.(*LVMLogicalVolume) @@ -870,7 +870,7 @@ func TestNewBootWithSizeLVMify(t *testing.T) { for idx, c := range custom { mnt, minSize := c.Mountpoint, c.MinSize - path := entityPath(mpt, mnt) + path := entityPathForMountpoint(mpt, mnt) assert.NotNil(path, "[%d] mountpoint %q not found", idx, mnt) parent := path[1] part, ok := parent.(*Partition) @@ -1208,7 +1208,7 @@ func TestMinimumSizesWithRequiredSizes(t *testing.T) { mpt, err := NewPartitionTable(&pt, tc.Blueprint, uint64(3*GiB), RawPartitioningMode, map[string]uint64{"/": 1 * GiB, "/usr": 3 * GiB}, rng) assert.NoError(err) for mnt, minSize := range tc.ExpectedMinSizes { - path := entityPath(mpt, mnt) + path := entityPathForMountpoint(mpt, mnt) assert.NotNil(path, "[%d] mountpoint %q not found", idx, mnt) parent := path[1] part, ok := parent.(*Partition) @@ -1222,7 +1222,7 @@ func TestMinimumSizesWithRequiredSizes(t *testing.T) { mpt, err := NewPartitionTable(&pt, tc.Blueprint, uint64(3*GiB), AutoLVMPartitioningMode, map[string]uint64{"/": 1 * GiB, "/usr": 3 * GiB}, rng) assert.NoError(err) for mnt, minSize := range tc.ExpectedMinSizes { - path := entityPath(mpt, mnt) + path := entityPathForMountpoint(mpt, mnt) assert.NotNil(path, "[%d] mountpoint %q not found", idx, mnt) parent := path[1] part, ok := parent.(*LVMLogicalVolume) diff --git a/pkg/disk/partition_table.go b/pkg/disk/partition_table.go index 7998f04bd..c2cb43e8c 100644 --- a/pkg/disk/partition_table.go +++ b/pkg/disk/partition_table.go @@ -225,7 +225,7 @@ func (pt *PartitionTable) SectorsToBytes(size uint64) uint64 { // Returns if the partition table contains a filesystem with the given // mount point. func (pt *PartitionTable) ContainsMountpoint(mountpoint string) bool { - return len(entityPath(pt, mountpoint)) > 0 + return len(entityPathForMountpoint(pt, mountpoint)) > 0 } // Generate all needed UUIDs for all the partiton and filesystems @@ -275,7 +275,7 @@ func (pt *PartitionTable) EnsureSize(s uint64) bool { } func (pt *PartitionTable) findDirectoryEntityPath(dir string) []Entity { - if path := entityPath(pt, dir); path != nil { + if path := entityPathForMountpoint(pt, dir); path != nil { return path } @@ -429,7 +429,7 @@ func (pt *PartitionTable) applyCustomization(mountpoints []blueprint.FilesystemC for _, mnt := range mountpoints { size := clampFSSize(mnt.Mountpoint, mnt.MinSize) - if path := entityPath(pt, mnt.Mountpoint); len(path) != 0 { + if path := entityPathForMountpoint(pt, mnt.Mountpoint); len(path) != 0 { size = alignEntityBranch(path, size) resizeEntityBranch(path, size) } else { @@ -465,7 +465,7 @@ func (pt *PartitionTable) relayout(size uint64) uint64 { var rootIdx = -1 for idx := range pt.Partitions { partition := &pt.Partitions[idx] - if len(entityPath(partition, "/")) != 0 { + if len(entityPathForMountpoint(partition, "/")) != 0 { // keep the root partition index to handle after all the other // partitions have been moved and resized rootIdx = idx @@ -510,7 +510,7 @@ func (pt *PartitionTable) relayout(size uint64) uint64 { } func (pt *PartitionTable) createFilesystem(mountpoint string, size uint64) error { - rootPath := entityPath(pt, "/") + rootPath := entityPathForMountpoint(pt, "/") if rootPath == nil { panic("no root mountpoint for PartitionTable") } @@ -539,13 +539,13 @@ func (pt *PartitionTable) createFilesystem(mountpoint string, size uint64) error return nil } -// entityPath stats at ent and searches for an Entity with a Mountpoint equal +// entityPathForMountpoint stats at ent and searches for an Entity with a Mountpoint equal // to the target. Returns a slice of all the Entities leading to the Mountable // in reverse order. If no Entity has the target as a Mountpoint, returns nil. // If a slice is returned, the last element is always the starting Entity ent // and the first element is always a Mountable with a Mountpoint equal to the // target. -func entityPath(ent Entity, target string) []Entity { +func entityPathForMountpoint(ent Entity, target string) []Entity { switch e := ent.(type) { case Mountable: if target == e.GetMountpoint() { @@ -554,7 +554,7 @@ func entityPath(ent Entity, target string) []Entity { case Container: for idx := uint(0); idx < e.GetItemCount(); idx++ { child := e.GetChild(idx) - path := entityPath(child, target) + path := entityPathForMountpoint(child, target) if path != nil { path = append(path, e) return path @@ -593,7 +593,7 @@ func (pt *PartitionTable) ForEachMountable(cb MountableCallback) error { // FindMountable returns the Mountable entity with the given mountpoint in the // PartitionTable. Returns nil if no Entity has the target as a Mountpoint. func (pt *PartitionTable) FindMountable(mountpoint string) Mountable { - path := entityPath(pt, mountpoint) + path := entityPathForMountpoint(pt, mountpoint) if len(path) == 0 { return nil @@ -684,13 +684,13 @@ func (pt *PartitionTable) GenUUID(rng *rand.Rand) { // it currently is not, it will wrap it in one func (pt *PartitionTable) ensureLVM() error { - rootPath := entityPath(pt, "/") + rootPath := entityPathForMountpoint(pt, "/") if rootPath == nil { panic("no root mountpoint for PartitionTable") } // we need a /boot partition to boot LVM, ensure one exists - bootPath := entityPath(pt, "/boot") + bootPath := entityPathForMountpoint(pt, "/boot") if bootPath == nil { _, err := pt.CreateMountpoint("/boot", 512*common.MiB) @@ -698,7 +698,7 @@ func (pt *PartitionTable) ensureLVM() error { return err } - rootPath = entityPath(pt, "/") + rootPath = entityPathForMountpoint(pt, "/") } parent := rootPath[1] // NB: entityPath has reversed order @@ -743,20 +743,20 @@ func (pt *PartitionTable) ensureLVM() error { // it currently is not, it will wrap it in one func (pt *PartitionTable) ensureBtrfs() error { - rootPath := entityPath(pt, "/") + rootPath := entityPathForMountpoint(pt, "/") if rootPath == nil { return fmt.Errorf("no root mountpoint for a partition table: %#v", pt) } // we need a /boot partition to boot btrfs, ensure one exists - bootPath := entityPath(pt, "/boot") + bootPath := entityPathForMountpoint(pt, "/boot") if bootPath == nil { _, err := pt.CreateMountpoint("/boot", 512*common.MiB) if err != nil { return fmt.Errorf("failed to create /boot partition when ensuring btrfs: %w", err) } - rootPath = entityPath(pt, "/") + rootPath = entityPathForMountpoint(pt, "/") } parent := rootPath[1] // NB: entityPath has reversed order @@ -881,7 +881,7 @@ func (pt *PartitionTable) GetBuildPackages() []string { // GetMountpointSize takes a mountpoint and returns the size of the entity this // mountpoint belongs to. func (pt *PartitionTable) GetMountpointSize(mountpoint string) (uint64, error) { - path := entityPath(pt, mountpoint) + path := entityPathForMountpoint(pt, mountpoint) if path == nil { return 0, fmt.Errorf("cannot find mountpoint %s", mountpoint) } From a8eebada7a90df0076e30d499edab952ba6d0f37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Budai?= Date: Fri, 30 Aug 2024 16:12:56 +0200 Subject: [PATCH 09/13] disk: create generalized entityPath This is basically the same method as entityPathForMountpoint - a DFS-search through a partition table tree. It takes a start and a function. Every visited node (entity) is passed in such function. If the function returns true, this means that the passed entity is the goal. entityPathForMountpoint is now implemented using the new generalized method. --- pkg/disk/partition_table.go | 43 +++++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/pkg/disk/partition_table.go b/pkg/disk/partition_table.go index c2cb43e8c..bc25695b2 100644 --- a/pkg/disk/partition_table.go +++ b/pkg/disk/partition_table.go @@ -539,6 +539,30 @@ func (pt *PartitionTable) createFilesystem(mountpoint string, size uint64) error return nil } +// entityPath starts at ent (usually a partition table) and traverses it until +// it finds an Entity for which isLeaf returns true. Returns a slice +// containing all entities from ent to the found leaf. The first element +// is the leaf, the last element is ent. If no leaf is found, returns nil. +func entityPath(ent Entity, isLeaf func(ent Entity) bool) []Entity { + if isLeaf(ent) { + return []Entity{ent} + } + + c, ok := ent.(Container) + if !ok { + return nil + } + + for idx := uint(0); idx < c.GetItemCount(); idx++ { + child := c.GetChild(idx) + if path := entityPath(child, isLeaf); path != nil { + return append(path, ent) + } + } + + return nil +} + // entityPathForMountpoint stats at ent and searches for an Entity with a Mountpoint equal // to the target. Returns a slice of all the Entities leading to the Mountable // in reverse order. If no Entity has the target as a Mountpoint, returns nil. @@ -546,22 +570,13 @@ func (pt *PartitionTable) createFilesystem(mountpoint string, size uint64) error // and the first element is always a Mountable with a Mountpoint equal to the // target. func entityPathForMountpoint(ent Entity, target string) []Entity { - switch e := ent.(type) { - case Mountable: - if target == e.GetMountpoint() { - return []Entity{ent} - } - case Container: - for idx := uint(0); idx < e.GetItemCount(); idx++ { - child := e.GetChild(idx) - path := entityPathForMountpoint(child, target) - if path != nil { - path = append(path, e) - return path - } + isTarget := func(ent Entity) bool { + if mnt, ok := ent.(Mountable); ok { + return mnt.GetMountpoint() == target } + return false } - return nil + return entityPath(ent, isTarget) } type MountableCallback func(mnt Mountable, path []Entity) error From 00b22b2af04b3d146023470dc92d7e1e218be3a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Budai?= Date: Fri, 30 Aug 2024 16:30:09 +0200 Subject: [PATCH 10/13] disk: refactor PartitionTable.createFilesystem This commit just shuffles some code around. This is mostly a preparatory commit for the next one. The only functional change is that if no suitable MountpointCreator is found, an error is now returned instead of a panic. --- pkg/disk/partition_table.go | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/pkg/disk/partition_table.go b/pkg/disk/partition_table.go index bc25695b2..0644f8831 100644 --- a/pkg/disk/partition_table.go +++ b/pkg/disk/partition_table.go @@ -515,28 +515,23 @@ func (pt *PartitionTable) createFilesystem(mountpoint string, size uint64) error panic("no root mountpoint for PartitionTable") } - var vc MountpointCreator - var entity Entity - var idx int - for idx, entity = range rootPath { - var ok bool - if vc, ok = entity.(MountpointCreator); ok { - break + for idx, entity := range rootPath { + vc, ok := entity.(MountpointCreator) + if !ok { + continue } - } - if vc == nil { - panic("could not find root volume container") + newVol, err := vc.CreateMountpoint(mountpoint, 0) + if err != nil { + return fmt.Errorf("failed creating volume: " + err.Error()) + } + vcPath := append([]Entity{newVol}, rootPath[idx:]...) + size = alignEntityBranch(vcPath, size) + resizeEntityBranch(vcPath, size) + return nil } - newVol, err := vc.CreateMountpoint(mountpoint, 0) - if err != nil { - return fmt.Errorf("failed creating volume: " + err.Error()) - } - vcPath := append([]Entity{newVol}, rootPath[idx:]...) - size = alignEntityBranch(vcPath, size) - resizeEntityBranch(vcPath, size) - return nil + return fmt.Errorf("could not find a suitable container for mountpoint %s", mountpoint) } // entityPath starts at ent (usually a partition table) and traverses it until From 5a7667a9a587636097a3804e116c55660583ec31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Budai?= Date: Fri, 30 Aug 2024 16:36:36 +0200 Subject: [PATCH 11/13] disk: fallback to higher-level containers when creating new filesystems Prior this commit, when the first-found mountpoint creator (a container nearest to the root mountpoint) failed to create a new mountpoint for some reason, the whole partition table customizing failed. This commit changes this behaviour: if a mountpoint creation fails, the code continues to traverse the path in the direction of the partition table root, and tries to find another creator and use that one instead. This is now mostly a pointless change because the only error can be returned from an LVM volume group that fails to allocate a unique name which is basically impossible. However, the following commits will use this feature to correctly allocate swap. --- pkg/disk/partition_table.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/disk/partition_table.go b/pkg/disk/partition_table.go index 0644f8831..458489916 100644 --- a/pkg/disk/partition_table.go +++ b/pkg/disk/partition_table.go @@ -1,6 +1,7 @@ package disk import ( + "errors" "fmt" "math/rand" "path/filepath" @@ -515,6 +516,7 @@ func (pt *PartitionTable) createFilesystem(mountpoint string, size uint64) error panic("no root mountpoint for PartitionTable") } + var errs []error for idx, entity := range rootPath { vc, ok := entity.(MountpointCreator) if !ok { @@ -523,7 +525,8 @@ func (pt *PartitionTable) createFilesystem(mountpoint string, size uint64) error newVol, err := vc.CreateMountpoint(mountpoint, 0) if err != nil { - return fmt.Errorf("failed creating volume: " + err.Error()) + errs = append(errs, fmt.Errorf("failed creating volume: %w", err)) + continue } vcPath := append([]Entity{newVol}, rootPath[idx:]...) size = alignEntityBranch(vcPath, size) @@ -531,7 +534,7 @@ func (pt *PartitionTable) createFilesystem(mountpoint string, size uint64) error return nil } - return fmt.Errorf("could not find a suitable container for mountpoint %s", mountpoint) + return fmt.Errorf("could not find a suitable container for mountpoint %s: %w", mountpoint, errors.Join(errs...)) } // entityPath starts at ent (usually a partition table) and traverses it until From d6cee9fbb80ca2f7267218a913be8a74a1d5b447 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Budai?= Date: Fri, 30 Aug 2024 16:43:20 +0200 Subject: [PATCH 12/13] disk: factor out createFilesystem method Prior this commit, both a partition table and a volume group contained code to create a new filesystem. Let's deduplicate this. --- pkg/disk/filesystem.go | 10 ++++++++++ pkg/disk/lvm.go | 11 +---------- pkg/disk/partition_table.go | 10 +--------- 3 files changed, 12 insertions(+), 19 deletions(-) diff --git a/pkg/disk/filesystem.go b/pkg/disk/filesystem.go index 0f483da99..353cf26db 100644 --- a/pkg/disk/filesystem.go +++ b/pkg/disk/filesystem.go @@ -98,3 +98,13 @@ func (fs *Filesystem) GenUUID(rng *rand.Rand) { fs.UUID = uuid.Must(newRandomUUIDFromReader(rng)).String() } } + +func createFilesystem(mountpoint string) *Filesystem { + return &Filesystem{ + Type: "xfs", + Mountpoint: mountpoint, + FSTabOptions: "defaults", + FSTabFreq: 0, + FSTabPassNo: 0, + } +} diff --git a/pkg/disk/lvm.go b/pkg/disk/lvm.go index eafc0ec40..e6057ce1a 100644 --- a/pkg/disk/lvm.go +++ b/pkg/disk/lvm.go @@ -72,16 +72,7 @@ func (vg *LVMVolumeGroup) GetChild(n uint) Entity { } func (vg *LVMVolumeGroup) CreateMountpoint(mountpoint string, size uint64) (Entity, error) { - - filesystem := Filesystem{ - Type: "xfs", - Mountpoint: mountpoint, - FSTabOptions: "defaults", - FSTabFreq: 0, - FSTabPassNo: 0, - } - - return vg.CreateLogicalVolume(mountpoint, size, &filesystem) + return vg.CreateLogicalVolume(mountpoint, size, createFilesystem(mountpoint)) } func (vg *LVMVolumeGroup) CreateLogicalVolume(lvName string, size uint64, payload Entity) (Entity, error) { diff --git a/pkg/disk/partition_table.go b/pkg/disk/partition_table.go index 458489916..22d7c043d 100644 --- a/pkg/disk/partition_table.go +++ b/pkg/disk/partition_table.go @@ -324,17 +324,9 @@ func (pt *PartitionTable) EnsureDirectorySizes(dirSizeMap map[string]uint64) { } func (pt *PartitionTable) CreateMountpoint(mountpoint string, size uint64) (Entity, error) { - filesystem := Filesystem{ - Type: "xfs", - Mountpoint: mountpoint, - FSTabOptions: "defaults", - FSTabFreq: 0, - FSTabPassNo: 0, - } - partition := Partition{ Size: size, - Payload: &filesystem, + Payload: createFilesystem(mountpoint), } n := len(pt.Partitions) From c545b4e693ed4e270abb61d926162ca95e529fd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Budai?= Date: Fri, 30 Aug 2024 16:51:16 +0200 Subject: [PATCH 13/13] {blueprint,disk}: add support for swap partitions This commit allows the filesystem customization in a blueprint to add a swap partition. This is done by adding a new field "type". It currently has only two values: unset and swap. The createFilesystem now creates a swap partition if type == swap. Otherwise, it still falls back to hardcoded "xfs". You can clearly see that should not be hard to change. ;) An interesting case is when / is on btrfs. Then, the library will try to create swap as a btrfs subvolume, which is bogus, so it returns an error. However, one of the previous commits changed the logic, so a higher-level container is tried in this case. So swap will be created as a raw partition as usual (or LV, if you have btrfs on LV). The blueprintApplied test helper can now work with swap, and it was also added into one of the test blueprints, so all the new code should be covered. --- pkg/blueprint/filesystem_customizations.go | 19 +++++++++++++-- pkg/disk/btrfs.go | 6 ++++- pkg/disk/disk.go | 10 ++++---- pkg/disk/disk_test.go | 28 ++++++++++++++++++---- pkg/disk/filesystem.go | 13 ++++++++-- pkg/disk/lvm.go | 5 ++-- pkg/disk/lvm_test.go | 11 +++++---- pkg/disk/partition_table.go | 19 +++++++++------ 8 files changed, 84 insertions(+), 27 deletions(-) diff --git a/pkg/blueprint/filesystem_customizations.go b/pkg/blueprint/filesystem_customizations.go index ad0ac1c49..d1d9b501f 100644 --- a/pkg/blueprint/filesystem_customizations.go +++ b/pkg/blueprint/filesystem_customizations.go @@ -9,9 +9,17 @@ import ( "github.com/osbuild/images/pkg/pathpolicy" ) +type FilesystemType string + +const ( + FilesystemTypeUnset FilesystemType = "" + FilesystemTypeSwap FilesystemType = "swap" +) + type FilesystemCustomization struct { - Mountpoint string `json:"mountpoint,omitempty" toml:"mountpoint,omitempty"` - MinSize uint64 `json:"minsize,omitempty" toml:"minsize,omitempty"` + Mountpoint string `json:"mountpoint,omitempty" toml:"mountpoint,omitempty"` + MinSize uint64 `json:"minsize,omitempty" toml:"minsize,omitempty"` + Type FilesystemType `json:"type,omitempty" toml:"type,omitempty"` } func (fsc *FilesystemCustomization) UnmarshalTOML(data interface{}) error { @@ -37,6 +45,13 @@ func (fsc *FilesystemCustomization) UnmarshalTOML(data interface{}) error { return fmt.Errorf("TOML unmarshal: minsize must be integer or string, got %v of type %T", d["minsize"], d["minsize"]) } + switch d["type"].(type) { + case string: + fsc.Type = FilesystemType(d["type"].(string)) + default: + return fmt.Errorf("TOML unmarshal: type must be string, got %v of type %T", d["type"], d["type"]) + } + return nil } diff --git a/pkg/disk/btrfs.go b/pkg/disk/btrfs.go index 80243bd4b..9b814d4bc 100644 --- a/pkg/disk/btrfs.go +++ b/pkg/disk/btrfs.go @@ -6,6 +6,7 @@ import ( "reflect" "github.com/google/uuid" + "github.com/osbuild/images/pkg/blueprint" ) const DefaultBtrfsCompression = "zstd:1" @@ -56,7 +57,10 @@ func (b *Btrfs) GetItemCount() uint { func (b *Btrfs) GetChild(n uint) Entity { return &b.Subvolumes[n] } -func (b *Btrfs) CreateMountpoint(mountpoint string, size uint64) (Entity, error) { +func (b *Btrfs) CreateMountpoint(mountpoint string, size uint64, fstype blueprint.FilesystemType) (Entity, error) { + if fstype == blueprint.FilesystemTypeSwap { + return nil, fmt.Errorf("btrfs subvolumes cannot be swap") + } name := mountpoint if name == "/" { name = "root" diff --git a/pkg/disk/disk.go b/pkg/disk/disk.go index 0c25faa6d..966af0677 100644 --- a/pkg/disk/disk.go +++ b/pkg/disk/disk.go @@ -27,6 +27,7 @@ import ( "slices" "github.com/google/uuid" + "github.com/osbuild/images/pkg/blueprint" ) const ( @@ -56,6 +57,8 @@ const ( // Extended Boot Loader Partition XBootLDRPartitionGUID = "BC13C2FF-59E6-4262-A352-B275FD6F7172" + + SwapPartitionGUID = "0657FD6D-A4AB-43C4-84E5-0933C84B4F4F" ) // Entity is the base interface for all disk-related entities. @@ -114,11 +117,10 @@ type Mountable interface { } // A MountpointCreator is a container that is able to create new volumes. -// -// CreateMountpoint creates a new mountpoint with the given size and -// returns the entity that represents the new mountpoint. type MountpointCreator interface { - CreateMountpoint(mountpoint string, size uint64) (Entity, error) + // CreateMountpoint creates a new mountpoint with the given size and type + // and returns the entity that represents the new mountpoint. + CreateMountpoint(mountpoint string, size uint64, fsType blueprint.FilesystemType) (Entity, error) // AlignUp will align the given bytes according to the // requirements of the container type. diff --git a/pkg/disk/disk_test.go b/pkg/disk/disk_test.go index dbc78996e..857067659 100644 --- a/pkg/disk/disk_test.go +++ b/pkg/disk/disk_test.go @@ -387,6 +387,10 @@ var testBlueprints = map[string][]blueprint.FilesystemCustomization{ Mountpoint: "/opt", MinSize: 7 * GiB, }, + { + Type: blueprint.FilesystemTypeSwap, + MinSize: 7 * GiB, + }, }, "small": { { @@ -423,10 +427,26 @@ func TestDisk_ForEachEntity(t *testing.T) { // blueprintApplied checks if the blueprint was applied correctly // returns nil if the blueprint was applied correctly, an error otherwise func blueprintApplied(pt *PartitionTable, bp []blueprint.FilesystemCustomization) error { + + // finds an entity in the partition table representing the desired custom mountpoint + // and returns an entity path to it + customMountpointPath := func(mnt blueprint.FilesystemCustomization) []Entity { + if mnt.Type == blueprint.FilesystemTypeSwap { + isSwap := func(e Entity) bool { + if fs, ok := e.(*Filesystem); ok { + return fs.Type == "swap" + } + return false + } + return entityPath(pt, isSwap) + } + return entityPathForMountpoint(pt, mnt.Mountpoint) + } + for _, mnt := range bp { - path := entityPathForMountpoint(pt, mnt.Mountpoint) + path := customMountpointPath(mnt) if path == nil { - return fmt.Errorf("mountpoint %s not found", mnt.Mountpoint) + return fmt.Errorf("mountpoint %s (type %s) not found", mnt.Mountpoint, mnt.Type) } for idx, ent := range path { if sz, ok := ent.(Sizeable); ok { @@ -606,8 +626,8 @@ func TestCreatePartitionTableLVMOnly(t *testing.T) { // check logical volume sizes against blueprint var lvsum uint64 for _, mnt := range tbp { - if mnt.Mountpoint == "/boot" { - // not on LVM; skipping + if mnt.Mountpoint == "/boot" || mnt.Type == blueprint.FilesystemTypeSwap { + // not on LVM or swap; skipping continue } mntPath := entityPathForMountpoint(mpt, mnt.Mountpoint) diff --git a/pkg/disk/filesystem.go b/pkg/disk/filesystem.go index 353cf26db..3887df1ea 100644 --- a/pkg/disk/filesystem.go +++ b/pkg/disk/filesystem.go @@ -5,6 +5,7 @@ import ( "reflect" "github.com/google/uuid" + "github.com/osbuild/images/pkg/blueprint" ) // Filesystem related functions @@ -99,9 +100,17 @@ func (fs *Filesystem) GenUUID(rng *rand.Rand) { } } -func createFilesystem(mountpoint string) *Filesystem { +func createFilesystem(mountpoint string, fstype blueprint.FilesystemType) *Filesystem { + var typ string + switch fstype { + case blueprint.FilesystemTypeSwap: + typ = "swap" + default: + typ = "xfs" + } + return &Filesystem{ - Type: "xfs", + Type: typ, Mountpoint: mountpoint, FSTabOptions: "defaults", FSTabFreq: 0, diff --git a/pkg/disk/lvm.go b/pkg/disk/lvm.go index e6057ce1a..e14a5c35e 100644 --- a/pkg/disk/lvm.go +++ b/pkg/disk/lvm.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/osbuild/images/internal/common" + "github.com/osbuild/images/pkg/blueprint" ) // Default physical extent size in bytes: logical volumes @@ -71,8 +72,8 @@ func (vg *LVMVolumeGroup) GetChild(n uint) Entity { return &vg.LogicalVolumes[n] } -func (vg *LVMVolumeGroup) CreateMountpoint(mountpoint string, size uint64) (Entity, error) { - return vg.CreateLogicalVolume(mountpoint, size, createFilesystem(mountpoint)) +func (vg *LVMVolumeGroup) CreateMountpoint(mountpoint string, size uint64, fstype blueprint.FilesystemType) (Entity, error) { + return vg.CreateLogicalVolume(mountpoint, size, createFilesystem(mountpoint, fstype)) } func (vg *LVMVolumeGroup) CreateLogicalVolume(lvName string, size uint64, payload Entity) (Entity, error) { diff --git a/pkg/disk/lvm_test.go b/pkg/disk/lvm_test.go index b1d9132d2..6e20f7d96 100644 --- a/pkg/disk/lvm_test.go +++ b/pkg/disk/lvm_test.go @@ -3,6 +3,7 @@ package disk import ( "testing" + "github.com/osbuild/images/pkg/blueprint" "github.com/stretchr/testify/assert" ) @@ -15,15 +16,15 @@ func TestLVMVCreateMountpoint(t *testing.T) { Description: "root volume group", } - entity, err := vg.CreateMountpoint("/", 0) + entity, err := vg.CreateMountpoint("/", 0, blueprint.FilesystemTypeUnset) assert.NoError(err) rootlv := entity.(*LVMLogicalVolume) assert.Equal("rootlv", rootlv.Name) - _, err = vg.CreateMountpoint("/home_test", 0) + _, err = vg.CreateMountpoint("/home_test", 0, blueprint.FilesystemTypeUnset) assert.NoError(err) - entity, err = vg.CreateMountpoint("/home/test", 0) + entity, err = vg.CreateMountpoint("/home/test", 0, blueprint.FilesystemTypeUnset) assert.NoError(err) dedup := entity.(*LVMLogicalVolume) @@ -31,11 +32,11 @@ func TestLVMVCreateMountpoint(t *testing.T) { // Lets collide it for i := 0; i < 98; i++ { - _, err = vg.CreateMountpoint("/home/test", 0) + _, err = vg.CreateMountpoint("/home/test", 0, blueprint.FilesystemTypeUnset) assert.NoError(err) } - _, err = vg.CreateMountpoint("/home/test", 0) + _, err = vg.CreateMountpoint("/home/test", 0, blueprint.FilesystemTypeUnset) assert.Error(err) } diff --git a/pkg/disk/partition_table.go b/pkg/disk/partition_table.go index 22d7c043d..5c6f6c1c7 100644 --- a/pkg/disk/partition_table.go +++ b/pkg/disk/partition_table.go @@ -323,10 +323,10 @@ func (pt *PartitionTable) EnsureDirectorySizes(dirSizeMap map[string]uint64) { } } -func (pt *PartitionTable) CreateMountpoint(mountpoint string, size uint64) (Entity, error) { +func (pt *PartitionTable) CreateMountpoint(mountpoint string, size uint64, fstype blueprint.FilesystemType) (Entity, error) { partition := Partition{ Size: size, - Payload: createFilesystem(mountpoint), + Payload: createFilesystem(mountpoint, fstype), } n := len(pt.Partitions) @@ -339,6 +339,11 @@ func (pt *PartitionTable) CreateMountpoint(mountpoint string, size uint64) (Enti default: partition.Type = FilesystemDataGUID } + + if fstype == blueprint.FilesystemTypeSwap { + partition.Type = SwapPartitionGUID + } + maxNo = 128 } else { maxNo = 4 @@ -428,7 +433,7 @@ func (pt *PartitionTable) applyCustomization(mountpoints []blueprint.FilesystemC } else { if !create { newMountpoints = append(newMountpoints, mnt) - } else if err := pt.createFilesystem(mnt.Mountpoint, size); err != nil { + } else if err := pt.createFilesystem(mnt.Mountpoint, size, mnt.Type); err != nil { return nil, err } } @@ -502,7 +507,7 @@ func (pt *PartitionTable) relayout(size uint64) uint64 { return start } -func (pt *PartitionTable) createFilesystem(mountpoint string, size uint64) error { +func (pt *PartitionTable) createFilesystem(mountpoint string, size uint64, fsType blueprint.FilesystemType) error { rootPath := entityPathForMountpoint(pt, "/") if rootPath == nil { panic("no root mountpoint for PartitionTable") @@ -515,7 +520,7 @@ func (pt *PartitionTable) createFilesystem(mountpoint string, size uint64) error continue } - newVol, err := vc.CreateMountpoint(mountpoint, 0) + newVol, err := vc.CreateMountpoint(mountpoint, 0, fsType) if err != nil { errs = append(errs, fmt.Errorf("failed creating volume: %w", err)) continue @@ -697,7 +702,7 @@ func (pt *PartitionTable) ensureLVM() error { // we need a /boot partition to boot LVM, ensure one exists bootPath := entityPathForMountpoint(pt, "/boot") if bootPath == nil { - _, err := pt.CreateMountpoint("/boot", 512*common.MiB) + _, err := pt.CreateMountpoint("/boot", 512*common.MiB, blueprint.FilesystemTypeUnset) if err != nil { return err @@ -756,7 +761,7 @@ func (pt *PartitionTable) ensureBtrfs() error { // we need a /boot partition to boot btrfs, ensure one exists bootPath := entityPathForMountpoint(pt, "/boot") if bootPath == nil { - _, err := pt.CreateMountpoint("/boot", 512*common.MiB) + _, err := pt.CreateMountpoint("/boot", 512*common.MiB, blueprint.FilesystemTypeUnset) if err != nil { return fmt.Errorf("failed to create /boot partition when ensuring btrfs: %w", err) }