disk: align LVM2 volumes to the extent size

When the size of a logical volume is not aligned to the extent size of
the volume group, LVM2 will automatically align it by rounding up[1]:
	Rounding up size to full physical extent 29.80 GiB
	Rounding up size to full physical extent <3.82 GiB

Since we don't take that into account when we create a new volume or
set the size of an existing one, the size for the whole volume group
will be short by that amount and thus the creation of the last volume
will fail:
  	Volume group <uuid> has insufficient free space (975 extents): 977 required.

To fix this a new `AlignUp` method is added to the `MountpointCreator`
creator interface. It will align a given size to the requirements of
the implementing container, like e.g. `LVMVolumeGroup`. It is then
used by a new `alignEntityBranch` which takes a size and walks the
entity path, calling `AlignUp` for all entities that implement said
`MountpointCreator` interface; thus the resulting size should fullfil
the alignment requirement for all elements in the path.
NB: `PartitionTable` already had an `AlignUp` method.

Add a corresponding test.

[1]: 8686657664/lib/metadata/metadata.c (L1072)

Co-authored-by: Achilleas Koutsou <achilleas@koutsou.net>

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()

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.

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)

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

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