Fix PV overhead calculation in LVMFactory._get_total_space

Resolves: RHEL-39981
This commit is contained in:
Vojtech Trefny 2026-05-18 11:02:57 +02:00
parent cbccb32ad7
commit 51da12199f
2 changed files with 205 additions and 1 deletions

View File

@ -0,0 +1,199 @@
From ccb0e61d6f76dbe2a5e19a6a397a287b7d480709 Mon Sep 17 00:00:00 2001
From: Vojtech Trefny <vtrefny@redhat.com>
Date: Tue, 28 Apr 2026 12:44:23 +0200
Subject: [PATCH 1/3] Fix PV overhead calculation in
LVMFactory._get_total_space
The previous fix (fdae8e91) had two issues:
1. It double-counted PV metadata: lvm_metadata_space already added the
PV metadata, and the new sum(pv.size - usable) also includes it
(since usable = align(size - metadata)).
2. When switching PV types (partition -> MD RAID), _configure() removes
old PVs before _get_total_space() is called, so self.vg.parents is
empty and the sum evaluates to 0 -- no overhead is accounted for.
Fix by replacing both the lvm_metadata_space line and the per-PV
overhead line with a single correct calculation: when parents exist,
use sum(pv.size - usable) which covers both metadata and alignment
loss; when parents are empty (PV type switch), fall back to the same
estimate used for new VGs (len(disks) * pe_size).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Resolves: RHEL-39981
---
blivet/devicefactory.py | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/blivet/devicefactory.py b/blivet/devicefactory.py
index 003254ea..e52b91b1 100644
--- a/blivet/devicefactory.py
+++ b/blivet/devicefactory.py
@@ -1411,11 +1411,11 @@ class LVMFactory(DeviceFactory):
if self.vg:
space += sum(p.size for p in self.vg.parents)
space -= self.vg.free_space
- # we need to account for the LVM metadata being placed somewhere
- space += self.vg.lvm_metadata_space
- # account for unusable space in the PV (difference between PV size and its usable
- # space), this is dues to PV metadata and data alignment to
- space += sum(pv.size - self.vg._get_pv_usable_space(pv) for pv in self.vg.parents)
+ # account for unusable space in the PV (PV metadata and data alignment)
+ if self.vg.parents:
+ space += sum(pv.size - self.vg._get_pv_usable_space(pv) for pv in self.vg.parents)
+ else:
+ space += len(self.disks) * self._pe_size
else:
# we need to account for the LVM metadata being placed on each disk
# (and thus taking up to one extent from each disk)
--
2.54.0
From f1bd28d097201737eb3cdd5e776803f1201d2cbb Mon Sep 17 00:00:00 2001
From: Vojtech Trefny <vtrefny@redhat.com>
Date: Tue, 28 Apr 2026 16:29:40 +0200
Subject: [PATCH 2/3] Use worst-case PV overhead in LVMFactory._get_total_space
When _get_total_space computes the target total PV space, it needs to
account for PV overhead (metadata + PE alignment loss). Previously it
used the current PV overhead as a correction, but after manage_size_sets
shrinks the PV partitions, PE alignment losses change unpredictably --
smaller PVs can have up to pe_size-1 more alignment loss per PV.
Use worst-case overhead (metadata + pe_size per PV) instead. The
pe_size term covers maximum PE alignment loss plus parted alignment
rounding. This guarantees the VG has enough usable space regardless
of how PE alignment shifts after PV resizing.
Maximum over-allocation is N * pe_size (e.g. 24 MiB for 6 PVs with
4 MiB PE), which is negligible relative to total VG size.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Resolves: RHEL-39981
---
blivet/devicefactory.py | 14 +++++++++++--
tests/unit_tests/devicefactory_test.py | 29 ++++++++++++++++++++++++++
2 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/blivet/devicefactory.py b/blivet/devicefactory.py
index e52b91b1..ec453293 100644
--- a/blivet/devicefactory.py
+++ b/blivet/devicefactory.py
@@ -1411,9 +1411,19 @@ class LVMFactory(DeviceFactory):
if self.vg:
space += sum(p.size for p in self.vg.parents)
space -= self.vg.free_space
- # account for unusable space in the PV (PV metadata and data alignment)
+ # Worst-case PV overhead: metadata + pe_size per PV.
+ # PE alignment losses change when manage_size_sets resizes PVs,
+ # so current overhead (already in sum - free) is not enough.
if self.vg.parents:
- space += sum(pv.size - self.vg._get_pv_usable_space(pv) for pv in self.vg.parents)
+ worst_case_overhead = sum(
+ self.vg._get_pv_metadata_space(pv) + self._pe_size
+ for pv in self.vg.parents
+ )
+ current_overhead = sum(
+ pv.size - self.vg._get_pv_usable_space(pv)
+ for pv in self.vg.parents
+ )
+ space += max(worst_case_overhead - current_overhead, Size(0))
else:
space += len(self.disks) * self._pe_size
else:
diff --git a/tests/unit_tests/devicefactory_test.py b/tests/unit_tests/devicefactory_test.py
index ff6bcb9e..cf9212bd 100644
--- a/tests/unit_tests/devicefactory_test.py
+++ b/tests/unit_tests/devicefactory_test.py
@@ -578,6 +578,35 @@ class LVMFactoryTestCase(DeviceFactoryTestCase):
device2 = self._factory_device(device_type, **kwargs)
self.assertEqual(device2.lvname, "name00")
+ @patch("blivet.formats.lvmpv.LVMPhysicalVolume.formattable", return_value=True)
+ @patch("blivet.formats.lvmpv.LVMPhysicalVolume.destroyable", return_value=True)
+ @patch("blivet.static_data.lvm_info.blockdev.lvm.lvs", return_value=[])
+ @patch("blivet.devices.lvm.LVMVolumeGroupDevice.type_external_dependencies", return_value=set())
+ @patch("blivet.devices.lvm.LVMLogicalVolumeBase.type_external_dependencies", return_value=set())
+ @patch("blivet.formats.fs.Ext4FS.formattable", return_value=True)
+ @patch("blivet.formats.fs.XFS.formattable", return_value=True)
+ def test_lvm_shrink_pv_overhead(self, *args): # pylint: disable=unused-argument
+ if self.device_type != devicefactory.DeviceTypes.LVM:
+ self.skipTest("only applies to plain LVM")
+
+ device_type = self.device_type
+
+ # Create a large LV filling most of the VG, then shrink it.
+ # _get_total_space must use worst-case PV overhead so that after
+ # manage_size_sets shrinks the PV partitions, the VG still has
+ # enough usable space (PE alignment losses change with PV size).
+ kwargs = {"disks": self.b.disks,
+ "size": Size("1500 MiB"),
+ "fstype": "ext4"}
+ device = self._factory_device(device_type, **kwargs)
+ vg = device.raw_device.container
+ self.assertGreaterEqual(vg.free_space, Size(0))
+
+ kwargs["device"] = device
+ kwargs["size"] = Size("200 MiB")
+ device = self._factory_device(device_type, **kwargs)
+ vg = device.raw_device.container
+ self.assertGreaterEqual(vg.free_space, Size(0))
class LVMThinPFactoryTestCase(LVMFactoryTestCase):
# TODO: check that the LV we get is a thin pool
--
2.54.0
From 589b961d235262f7131a5417dcc439bc3f3f6e65 Mon Sep 17 00:00:00 2001
From: Vojtech Trefny <vtrefny@redhat.com>
Date: Tue, 19 May 2026 12:05:29 +0200
Subject: [PATCH 3/3] Fix LVMFactoryTestCase.test_lvm_shrink_pv_overhead on
RHEL 9
Related: RHEL-39981
---
tests/unit_tests/devicefactory_test.py | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/tests/unit_tests/devicefactory_test.py b/tests/unit_tests/devicefactory_test.py
index cf9212bd..482da812 100644
--- a/tests/unit_tests/devicefactory_test.py
+++ b/tests/unit_tests/devicefactory_test.py
@@ -586,9 +586,6 @@ class LVMFactoryTestCase(DeviceFactoryTestCase):
@patch("blivet.formats.fs.Ext4FS.formattable", return_value=True)
@patch("blivet.formats.fs.XFS.formattable", return_value=True)
def test_lvm_shrink_pv_overhead(self, *args): # pylint: disable=unused-argument
- if self.device_type != devicefactory.DeviceTypes.LVM:
- self.skipTest("only applies to plain LVM")
-
device_type = self.device_type
# Create a large LV filling most of the VG, then shrink it.
@@ -640,6 +637,9 @@ class LVMThinPFactoryTestCase(LVMFactoryTestCase):
return delta
+ def test_lvm_shrink_pv_overhead(self, *args): # pylint: disable=unused-argument
+ self.skipTest("only applies to plain LVM")
+
class LVMVDOFactoryTestCase(LVMFactoryTestCase):
device_class = LVMLogicalVolumeDevice
@@ -788,6 +788,9 @@ class LVMVDOFactoryTestCase(LVMFactoryTestCase):
def test_lv_unique_name(self, *args): # pylint: disable=unused-argument,arguments-differ
super(LVMVDOFactoryTestCase, self).test_lv_unique_name()
+ def test_lvm_shrink_pv_overhead(self, *args): # pylint: disable=unused-argument
+ self.skipTest("only applies to plain LVM")
+
class MDFactoryTestCase(DeviceFactoryTestCase):
device_type = devicefactory.DEVICE_TYPE_MD
--
2.54.0

View File

@ -23,7 +23,7 @@ Version: 3.6.0
#%%global prerelease .b2
# prerelease, if defined, should be something like .a1, .b1, .b2.dev1, or .c2
Release: 30%{?prerelease}%{?dist}
Release: 31%{?prerelease}%{?dist}
Epoch: 1
License: LGPLv2+
%global realname blivet
@ -70,6 +70,7 @@ Patch36: 0037-Wipe-end-partition-before-creating-it-as-well-as-the-start.patch
Patch37: 0038-Add-a-pre-wipe-fixup-function-for-LVM-logical-volume.patch
Patch38: 0039-iSCSI-dont-crash-when-LUN-ID-256.patch
Patch39: 0040-Account-for-unusable-space-in-the-PV-in-LVMFactory.patch
Patch40: 0041-Fix-PV-overhead-calculation-in-LVMFactory-_get_total_space.patch
# Versions of required components (done so we make sure the buildrequires
# match the requires versions of things).
@ -233,6 +234,10 @@ configuration.
%endif
%changelog
* Mon May 18 2026 Vojtech Trefny <vtrefny@redhat.com> - 3.6.0-31
- Fix PV overhead calculation in LVMFactory._get_total_space
Resolves: RHEL-39981
* Thu Apr 16 2026 Vojtech Trefny <vtrefny@redhat.com> - 3.6.0-30
- Account for unusable space in the PV in LVMFactory
Resolves: RHEL-39981