Skip to content

Commit

Permalink
disk: align LVM2 volumes to the extent size
Browse files Browse the repository at this point in the history
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]: https://github.com/lvmteam/lvm2/blob/868665766491d9d42b8acedaf07cafc7118d165c/lib/metadata/metadata.c#L1072
  • Loading branch information
gicmo committed Nov 17, 2022
1 parent 1da3fd0 commit 2a1ad27
Show file tree
Hide file tree
Showing 5 changed files with 135 additions and 10 deletions.
4 changes: 4 additions & 0 deletions internal/disk/btrfs.go
Expand Up @@ -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()
Expand Down
4 changes: 4 additions & 0 deletions internal/disk/disk.go
Expand Up @@ -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.
Expand Down
86 changes: 86 additions & 0 deletions internal/disk/disk_test.go
Expand Up @@ -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)
Expand Down
17 changes: 16 additions & 1 deletion internal/disk/lvm.go
Expand Up @@ -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
Expand Down Expand Up @@ -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,
}

Expand All @@ -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
Expand Down
34 changes: 25 additions & 9 deletions internal/disk/partition_table.go
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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

Expand Down

0 comments on commit 2a1ad27

Please sign in to comment.