Skip to content

Commit

Permalink
bib: improve the "/" size calculation for LVM
Browse files Browse the repository at this point in the history
This commit improves the rootfs size calculation on LVM. The
issue is that when a user creates a disk customization that
adds LVM to e.g. add swap or a custom partition then the "/"
partition will also be moved to LVM. However the disk library
will not automatically calculate a size for "/" unless it gets
the `RequiredMinSizes` parameter (see gh#748). This leads to
a "0B" root partition. The naive fix was to just include a
hardcoded:
```
requiredDirectorySizes = map[string]uint64{
        "/":    1 * datasizes.GiB,
        "/usr": 2 * datasizes.GiB,
}
```
but of course that is not ideal because we calculate the
size of disk based on 2x the container size. We set this
for the rootfs so the entire disk is big enough. However
because LVM does not automatically expand "/" we still end
up with a potentially too small "/".

This commit tweaks the calculcation so that it sets the
requiredDirectorySizes for "/" to 2x container when
LVM is used and no "/" is defined.

This should be sufficient because the other cases are:
1. non-LVM is used, "/" is expanded automatically
2. LVM is used ensure "/" is at least rootfsMinSize

Note that a custom "/usr" is not supported in image mode so splitting
rootfsMinSize between / and /usr is not a concern.
  • Loading branch information
mvo5 committed Dec 6, 2024
1 parent a39b22e commit 367f4ca
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 7 deletions.
37 changes: 30 additions & 7 deletions bib/cmd/bootc-image-builder/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
"github.com/osbuild/images/pkg/customizations/anaconda"
"github.com/osbuild/images/pkg/customizations/kickstart"
"github.com/osbuild/images/pkg/customizations/users"
"github.com/osbuild/images/pkg/datasizes"
"github.com/osbuild/images/pkg/disk"
"github.com/osbuild/images/pkg/image"
"github.com/osbuild/images/pkg/manifest"
Expand Down Expand Up @@ -219,6 +218,35 @@ func genPartitionTable(c *ManifestConfig, customizations *blueprint.Customizatio
}
}

// calcRequiredDirectorySizes will calculate the minimum sizes for /
// for disk customizations. This is only relevant on LVM because on the
// other types we will expand the "/" to the size of the disk. But on
// LVM this is not done as we expect people to create VGs with extra space.
// This should be sufficient because the other cases are:
// 1. non-LVM is used, "/" is expanded automatically
// 2. LVM is used ensure "/" is at least rootfsMinSize
//
// Note that a custom "/usr" is not supported in image mode so splitting
// rootfsMinSize between / and /usr is not a concern.
func calcRequiredDirectorySizes(distCust *blueprint.DiskCustomization, rootfsMinSize uint64) map[string]uint64 {
lvmMounts := map[string]uint64{}
for _, part := range distCust.Partitions {
if part.Type == "lvm" {
for _, lv := range part.LogicalVolumes {
lvmMounts[lv.Mountpoint] = lv.MinSize
}
}
}
// no LVM used, nothing to do
if len(lvmMounts) == 0 {
return nil
}
// ensure rootfsMinSize is respected
return map[string]uint64{
"/": max(rootfsMinSize, lvmMounts["/"]),
}
}

func genPartitionTableDiskCust(c *ManifestConfig, diskCust *blueprint.DiskCustomization, rng *rand.Rand) (*disk.PartitionTable, error) {
if err := diskCust.ValidateLayoutConstraints(); err != nil {
return nil, fmt.Errorf("cannot use disk customization: %w", err)
Expand All @@ -235,18 +263,13 @@ func genPartitionTableDiskCust(c *ManifestConfig, diskCust *blueprint.DiskCustom
return nil, err
}

// XXX: copied from images, take from container instead?
requiredDirectorySizes := map[string]uint64{
"/": 1 * datasizes.GiB,
"/usr": 2 * datasizes.GiB,
}
partOptions := &disk.CustomPartitionTableOptions{
PartitionTableType: basept.Type,
// XXX: not setting/defaults will fail to boot with btrfs/lvm
BootMode: platform.BOOT_HYBRID,
DefaultFSType: defaultFSType,
// ensure sensible defaults for /,/usr
RequiredMinSizes: requiredDirectorySizes,
RequiredMinSizes: calcRequiredDirectorySizes(diskCust, c.RootfsMinsize),
}
return disk.NewCustomPartitionTable(diskCust, partOptions, rng)
}
Expand Down
37 changes: 37 additions & 0 deletions bib/cmd/bootc-image-builder/image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/osbuild/images/pkg/arch"
"github.com/osbuild/images/pkg/blueprint"
"github.com/osbuild/images/pkg/datasizes"
"github.com/osbuild/images/pkg/disk"
"github.com/osbuild/images/pkg/manifest"
"github.com/osbuild/images/pkg/runner"
Expand Down Expand Up @@ -448,3 +449,39 @@ func TestGenPartitionTableDiskCustomizationRunsValidateLayoutConstraints(t *test
_, err := bib.GenPartitionTable(cnf, cus, rng)
assert.EqualError(t, err, "cannot use disk customization: multiple LVM volume groups are not yet supported")
}

func TestGenPartitionTableDiskCustomizationLVMDirSize(t *testing.T) {
rng := bib.CreateRand()

cnf := &bib.ManifestConfig{
Architecture: arch.FromString("amd64"),
RootFSType: "xfs",
RootfsMinsize: 5 * datasizes.GiB,
}
cus := &blueprint.Customizations{
Disk: &blueprint.DiskCustomization{
Partitions: []blueprint.PartitionCustomization{
{
Type: "lvm",
VGCustomization: blueprint.VGCustomization{
LogicalVolumes: []blueprint.LVCustomization{
{
MinSize: 2 * datasizes.GiB,
FilesystemTypedCustomization: blueprint.FilesystemTypedCustomization{
Mountpoint: "/var/log",
FSType: "xfs",
},
},
},
},
},
},
},
}
pt, err := bib.GenPartitionTable(cnf, cus, rng)
assert.NoError(t, err)
vg := pt.Partitions[3].Payload.(*disk.LVMVolumeGroup)
assert.Equal(t, 2, len(vg.LogicalVolumes))
assert.Equal(t, vg.LogicalVolumes[1].Payload.(*disk.Filesystem).Mountpoint, "/")
assert.True(t, vg.LogicalVolumes[1].Size >= 5*datasizes.GiB)
}

0 comments on commit 367f4ca

Please sign in to comment.