From f764e5b520a61ee6c0d203119e674d05d25f6151 Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Mon, 30 Sep 2024 13:42:03 -0700 Subject: [PATCH] Reduce LVM extent usage With 4k disks, it appears that we can encounter cases where logical block sizes being larger begins to chip away at space at the beginning *and* end of an LVM Physical Volume (PV). With GPT, a similar issue occurs, larger logical block sizes increases the number of bytes used at the beginning and end of disk for metadata storage. For the PV, the signature is on the second logical block, and there is also reference to a potential backup at the end of the volume. Which means the overall available space goes from just being impacted by 1kB to at least 8kB. This can then begin to impact overall alignment which is in 4MB blocks on top of the base device. The attempt here is to dial the sizing back just a little bit, so we avoid assumptions regarding volume sizing ever possibly conflicting or cases where we somehow end up asking to grow a volume on a physical volume group which exceeds the number of extents LVM has calculated. Example failure this intends to avoid similar errors on a 4k disk when growvols is triggered: Exception: Running command failed: cmd "lvextend -L+952065064960B /dev/mapper/vg-lv_thinpool /dev/sdc6" Which involves Standard Error ouptut along the lines of: "Insufficient free space: 226990 extents needed, but only 226988 available" With this change, we will apply logic to *both* the extent size of the thinpool, if any, and all logical volumes which effectively reduces the leveraged volumes by 8MB or two LVM extents, because growvols tries to convert the user's preference of size or percentage to bytes, which doesn't compensate for the additional lost extents on a 4k block device. Change-Id: I303d504f3c822fd534f3e3642d85873ba30d3f68 --- .../growvols/static/usr/local/sbin/growvols | 34 +++++++++++-- .../elements/growvols/tests/test_growvols.py | 51 ++++++++++++++----- 2 files changed, 69 insertions(+), 16 deletions(-) diff --git a/diskimage_builder/elements/growvols/static/usr/local/sbin/growvols b/diskimage_builder/elements/growvols/static/usr/local/sbin/growvols index 497a17674..91b4235fc 100755 --- a/diskimage_builder/elements/growvols/static/usr/local/sbin/growvols +++ b/diskimage_builder/elements/growvols/static/usr/local/sbin/growvols @@ -191,6 +191,31 @@ def convert_bytes(num): return "%d%s" % (unit_num, x) +def shrink_bytes_for_alignment(num): + """Shrink the amount of bytes to line up with LVM. + + The purpose of this method is to gently shrink the requested volume + size of bytes to ensure that the size of the bytes range is in alignment + with the LVM extent size, and that the bytes value used is modeled in + the concept of extents where *two* extents have also been removed from + the value to account for any possible structural alignment differences + due to varying block sizes. In other words, reduce the size slightly to + account for the differences. + + :param: num as in the number of bytes desired. + :returns: The closest value accounting for extent sizing *minus* two LVM + extents. + """ + result = ((num // PHYSICAL_EXTENT_BYTES) - 2) * PHYSICAL_EXTENT_BYTES + if result <= 0: + raise Exception( + 'Not enough space available to shrink to alignment. ' + 'Requires more than %s, requested: %s' % ( + convert_bytes(PHYSICAL_EXTENT_BYTES * 2), + convert_bytes(num))) + return result + + def execute(cmd): """Run a command""" LOG.info('Running: %s', printable_cmd(cmd)) @@ -542,7 +567,8 @@ def main(argv): devname = find_next_device_name(devices, disk, partnum) thin_pool, thin_pool_name = find_thin_pool(devices, group) if thin_pool: - # reserve for the size of the metadata volume + # reserve for the size of the metadata volume, which + # is *allocated* from the underlying storage as well. size_bytes -= POOL_METADATA_SIZE # reserve for the size of the spare metadata volume, # used for metadata check and repair @@ -598,18 +624,18 @@ def main(argv): convert_bytes(POOL_METADATA_SIZE), thin_pool))) commands.append(Command([ 'lvextend', - '-L+%sB' % size_bytes, + '-L+%sB' % shrink_bytes_for_alignment(size_bytes), thin_pool, dev_path ], 'Add %s to thin pool %s' % (convert_bytes(size_bytes), - thin_pool))) + thin_pool))) for volume_path, size_bytes in grow_vols.items(): if size_bytes > 0: extend_args = [ 'lvextend', '--size', - '+%sB' % size_bytes, + '+%sB' % shrink_bytes_for_alignment(size_bytes), volume_path ] if not thin_pool: diff --git a/diskimage_builder/elements/growvols/tests/test_growvols.py b/diskimage_builder/elements/growvols/tests/test_growvols.py index 794c800d2..5676de69d 100644 --- a/diskimage_builder/elements/growvols/tests/test_growvols.py +++ b/diskimage_builder/elements/growvols/tests/test_growvols.py @@ -279,6 +279,33 @@ class TestGrowvols(base.BaseTestCase): self.assertEqual('2GiB', growvols.convert_bytes(3000000000)) self.assertEqual('3725GiB', growvols.convert_bytes(4000000000000)) + def test_shrink_bytes_for_alignment(self): + peb = growvols.PHYSICAL_EXTENT_BYTES + half_peb = peb // 2 + + # shrink 3 extents to 1 + self.assertEqual(peb, growvols.shrink_bytes_for_alignment(peb * 3)) + + # shrink 4.5 extents to 2 (round down to whole extent minus 2 extents) + self.assertEqual(peb * 2, + growvols.shrink_bytes_for_alignment( + peb * 4 + half_peb)) + + # error shrinking zero + e = self.assertRaises(Exception, + growvols.shrink_bytes_for_alignment, 0) + self.assertIn('Requires more than 8MiB, requested: 0B', str(e)) + + # error shrinking 1 extent + e = self.assertRaises(Exception, + growvols.shrink_bytes_for_alignment, peb) + self.assertIn('Requires more than 8MiB, requested: 4MiB', str(e)) + + # error shrinking 2 extents + e = self.assertRaises(Exception, + growvols.shrink_bytes_for_alignment, peb * 2) + self.assertIn('Requires more than 8MiB, requested: 8MiB', str(e)) + @mock.patch('subprocess.Popen') def test_execute(self, mock_popen): mock_process = mock.Mock() @@ -728,7 +755,7 @@ class TestGrowvols(base.BaseTestCase): mock.call(['partprobe']), mock.call(['pvcreate', '-ff', '--yes', '/dev/sda5']), mock.call(['vgextend', 'vg', '/dev/sda5']), - mock.call(['lvextend', '--size', '+209404821504B', + mock.call(['lvextend', '--size', '+209396432896B', '/dev/mapper/vg-lv_root', '/dev/sda5']), mock.call(['xfs_growfs', '/dev/mapper/vg-lv_root']) ]) @@ -759,11 +786,11 @@ class TestGrowvols(base.BaseTestCase): mock.call(['partprobe']), mock.call(['pvcreate', '-ff', '--yes', '/dev/sda5']), mock.call(['vgextend', 'vg', '/dev/sda5']), - mock.call(['lvextend', '--size', '+41880125440B', + mock.call(['lvextend', '--size', '+41871736832B', '/dev/mapper/vg-lv_home', '/dev/sda5']), - mock.call(['lvextend', '--size', '+83760250880B', + mock.call(['lvextend', '--size', '+83751862272B', '/dev/mapper/vg-lv_var', '/dev/sda5']), - mock.call(['lvextend', '--size', '+83764445184B', + mock.call(['lvextend', '--size', '+83756056576B', '/dev/mapper/vg-lv_root', '/dev/sda5']), mock.call(['xfs_growfs', '/dev/mapper/vg-lv_home']), mock.call(['xfs_growfs', '/dev/mapper/vg-lv_var']), @@ -832,13 +859,13 @@ class TestGrowvols(base.BaseTestCase): mock.call(['vgextend', 'vg', '/dev/sda5']), mock.call(['lvextend', '--poolmetadatasize', '+1073741824B', 'vg/lv_thinpool']), - mock.call(['lvextend', '-L+207253143552B', + mock.call(['lvextend', '-L+207244754944B', '/dev/mapper/vg-lv_thinpool', '/dev/sda5']), - mock.call(['lvextend', '--size', '+41448112128B', + mock.call(['lvextend', '--size', '+41439723520B', '/dev/mapper/vg-lv_home']), - mock.call(['lvextend', '--size', '+82900418560B', + mock.call(['lvextend', '--size', '+82892029952B', '/dev/mapper/vg-lv_var']), - mock.call(['lvextend', '--size', '+82904612864B', + mock.call(['lvextend', '--size', '+82896224256B', '/dev/mapper/vg-lv_root']), mock.call(['xfs_growfs', '/dev/mapper/vg-lv_home']), mock.call(['xfs_growfs', '/dev/mapper/vg-lv_var']), @@ -881,13 +908,13 @@ class TestGrowvols(base.BaseTestCase): mock.call(['vgextend', 'vg', '/dev/mapper/mpatha6']), mock.call(['lvextend', '--poolmetadatasize', '+1073741824B', 'vg/lv_thinpool']), - mock.call(['lvextend', '-L+207253143552B', + mock.call(['lvextend', '-L+207244754944B', '/dev/mapper/vg-lv_thinpool', '/dev/mapper/mpatha6']), - mock.call(['lvextend', '--size', '+41448112128B', + mock.call(['lvextend', '--size', '+41439723520B', '/dev/mapper/vg-lv_home']), - mock.call(['lvextend', '--size', '+82900418560B', + mock.call(['lvextend', '--size', '+82892029952B', '/dev/mapper/vg-lv_var']), - mock.call(['lvextend', '--size', '+82904612864B', + mock.call(['lvextend', '--size', '+82896224256B', '/dev/mapper/vg-lv_root']), mock.call(['xfs_growfs', '/dev/mapper/vg-lv_home']), mock.call(['xfs_growfs', '/dev/mapper/vg-lv_var']),