diff --git a/internal/disk/btrfs.go b/internal/disk/btrfs.go index a4f0e658399..5f48c7cc22c 100644 --- a/internal/disk/btrfs.go +++ b/internal/disk/btrfs.go @@ -67,6 +67,10 @@ func (b *Btrfs) CreateMountpoint(mountpoint string, size uint64) (Entity, error) return &b.Subvolumes[len(b.Subvolumes)-1], nil } +func (b *Btrfs) AlignUp(size uint64) uint64 { + return size // No extra alignment necessary for subvolumes +} + func (b *Btrfs) GenUUID(rng *rand.Rand) { if b.UUID == "" { b.UUID = uuid.Must(newRandomUUIDFromReader(rng)).String() diff --git a/internal/disk/disk.go b/internal/disk/disk.go index 85680feb3cf..14fa14d4439 100644 --- a/internal/disk/disk.go +++ b/internal/disk/disk.go @@ -120,6 +120,10 @@ type Mountable interface { // returns the entity that represents the new mountpoint. type MountpointCreator interface { CreateMountpoint(mountpoint string, size uint64) (Entity, error) + + // AlignUp will align the given bytes according to the + // requirements of the container type. + AlignUp(size uint64) uint64 } // A UniqueEntity is an entity that can be uniquely identified via a UUID. diff --git a/internal/disk/disk_test.go b/internal/disk/disk_test.go index 7e98ced6268..e77c5c47e7d 100644 --- a/internal/disk/disk_test.go +++ b/internal/disk/disk_test.go @@ -617,6 +617,92 @@ func TestMinimumSizes(t *testing.T) { } } +func TestLVMExtentAlignment(t *testing.T) { + assert := assert.New(t) + + // math/rand is good enough in this case + /* #nosec G404 */ + rng := rand.New(rand.NewSource(13)) + pt := testPartitionTables["plain"] + + type testCase struct { + Blueprint []blueprint.FilesystemCustomization + ExpectedSizes map[string]uint64 + } + + testCases := []testCase{ + { + Blueprint: []blueprint.FilesystemCustomization{ + { + Mountpoint: "/var", + MinSize: 1*GiB + 1, + }, + }, + ExpectedSizes: map[string]uint64{ + "/var": 1*GiB + LVMDefaultExtentSize, + }, + }, + { + // lots of mount points in /var + // https://bugzilla.redhat.com/show_bug.cgi?id=2141738 + Blueprint: []blueprint.FilesystemCustomization{ + { + Mountpoint: "/", + MinSize: 32000000000, + }, + { + Mountpoint: "/var", + MinSize: 4096000000, + }, + { + Mountpoint: "/var/log", + MinSize: 4096000000, + }, + }, + ExpectedSizes: map[string]uint64{ + "/": 32002539520, + "/var": 3908 * MiB, + "/var/log": 3908 * MiB, + }, + }, + { + Blueprint: []blueprint.FilesystemCustomization{ + { + Mountpoint: "/", + MinSize: 32 * GiB, + }, + { + Mountpoint: "/var", + MinSize: 4 * GiB, + }, + { + Mountpoint: "/var/log", + MinSize: 4 * GiB, + }, + }, + ExpectedSizes: map[string]uint64{ + "/": 32 * GiB, + "/var": 4 * GiB, + "/var/log": 4 * GiB, + }, + }, + } + + for idx, tc := range testCases { + mpt, err := NewPartitionTable(&pt, tc.Blueprint, uint64(3*GiB), true, rng) + assert.NoError(err) + for mnt, expSize := range tc.ExpectedSizes { + path := entityPath(mpt, mnt) + assert.NotNil(path, "[%d] mountpoint %q not found", idx, mnt) + parent := path[1] + part, ok := parent.(*LVMLogicalVolume) + assert.True(ok, "[%d] %q parent (%v) is not an LVM logical volume", idx, mnt, parent) + assert.Equal(part.GetSize(), expSize, + "[%d] %q size %d should be equal to %d", idx, mnt, part.GetSize(), expSize) + } + } +} + func TestNewBootWithSizeLVMify(t *testing.T) { pt := testPartitionTables["plain-noboot"] assert := assert.New(t) diff --git a/internal/disk/lvm.go b/internal/disk/lvm.go index 539e4665830..809f5e3f952 100644 --- a/internal/disk/lvm.go +++ b/internal/disk/lvm.go @@ -3,8 +3,14 @@ package disk import ( "fmt" "strings" + + "github.com/osbuild/osbuild-composer/internal/common" ) +// Default physical extent size in bytes: logical volumes +// created inside the VG will be aligned to this. +const LVMDefaultExtentSize = 4 * common.MebiByte + type LVMVolumeGroup struct { Name string Description string @@ -105,7 +111,7 @@ func (vg *LVMVolumeGroup) CreateLogicalVolume(lvName string, size uint64, payloa lv := LVMLogicalVolume{ Name: name, - Size: size, + Size: vg.AlignUp(size), Payload: payload, } @@ -114,6 +120,15 @@ func (vg *LVMVolumeGroup) CreateLogicalVolume(lvName string, size uint64, payloa return &vg.LogicalVolumes[len(vg.LogicalVolumes)-1], nil } +func (vg *LVMVolumeGroup) AlignUp(size uint64) uint64 { + + if size%LVMDefaultExtentSize != 0 { + size += LVMDefaultExtentSize - size%LVMDefaultExtentSize + } + + return size +} + func (vg *LVMVolumeGroup) MetadataSize() uint64 { if vg == nil { return 0 diff --git a/internal/disk/partition_table.go b/internal/disk/partition_table.go index 91719deb0e3..27be1173e46 100644 --- a/internal/disk/partition_table.go +++ b/internal/disk/partition_table.go @@ -326,8 +326,9 @@ func (pt *PartitionTable) applyCustomization(mountpoints []blueprint.FilesystemC newMountpoints := []blueprint.FilesystemCustomization{} for _, mnt := range mountpoints { - size := pt.AlignUp(clampFSSize(mnt.Mountpoint, mnt.MinSize)) + size := clampFSSize(mnt.Mountpoint, mnt.MinSize) if path := entityPath(pt, mnt.Mountpoint); len(path) != 0 { + size = alignEntityBranch(path, size) resizeEntityBranch(path, size) } else { if !create { @@ -427,6 +428,7 @@ func (pt *PartitionTable) createFilesystem(mountpoint string, size uint64) error return fmt.Errorf("failed creating volume: " + err.Error()) } vcPath := append([]Entity{newVol}, rootPath[idx:]...) + size = alignEntityBranch(vcPath, size) resizeEntityBranch(vcPath, size) return nil } @@ -509,6 +511,20 @@ func clampFSSize(mountpoint string, size uint64) uint64 { return size } +func alignEntityBranch(path []Entity, size uint64) uint64 { + if len(path) == 0 { + return size + } + + element := path[0] + + if c, ok := element.(MountpointCreator); ok { + size = c.AlignUp(size) + } + + return alignEntityBranch(path[1:], size) +} + // resizeEntityBranch resizes the first entity in the specified path to be at // least the specified size and then grows every entity up the path to the // PartitionTable accordingly. @@ -579,18 +595,18 @@ func (pt *PartitionTable) ensureLVM() error { } else if part, ok := parent.(*Partition); ok { filesystem := part.Payload - part.Payload = &LVMVolumeGroup{ + vg := &LVMVolumeGroup{ Name: "rootvg", Description: "created via lvm2 and osbuild", - LogicalVolumes: []LVMLogicalVolume{ - { - Size: part.Size, - Name: "rootlv", - Payload: filesystem, - }, - }, } + _, err := vg.CreateLogicalVolume("root", part.Size, filesystem) + if err != nil { + panic(fmt.Sprintf("Could not create LV: %v", err)) + } + + part.Payload = vg + // reset it so it will be grown later part.Size = 0