From 00ca1262872f42991ba3132f662c6a531556ecbb Mon Sep 17 00:00:00 2001 From: Steve Baker Date: Mon, 19 Dec 2022 13:40:04 +1300 Subject: [PATCH] Grow thin pool metadata by 1GiB An LVM thin pool has an associated metadata volume, and it can be assumed that the size of this volume on the image is minimized for distribution. This change grows the metadata volume by 1GiB, which is recommended[1] as a reasonable default. This fixes a specific issue with the metadata volume being exausted when growing into a 2TB drive. Other minor changes include: - Human readable printed values have switched to GiB, MiB, KiB, B - Growth percentage volumes are adjusted down to not over-provision the thin volume [1] https://access.redhat.com/solutions/6318131 Change-Id: I1dd6dd932bb5f5d9adac9b78a026569165bd4ea9 Resolves: rhbz#2149586 --- .../growvols/static/usr/local/sbin/growvols | 45 ++++++++++++++----- .../elements/growvols/tests/test_growvols.py | 20 +++++---- 2 files changed, 44 insertions(+), 21 deletions(-) diff --git a/diskimage_builder/elements/growvols/static/usr/local/sbin/growvols b/diskimage_builder/elements/growvols/static/usr/local/sbin/growvols index 8c0249e1e..0e5d4686d 100755 --- a/diskimage_builder/elements/growvols/static/usr/local/sbin/growvols +++ b/diskimage_builder/elements/growvols/static/usr/local/sbin/growvols @@ -36,12 +36,22 @@ UNITS = ['%'] UNITS.extend(UNIT_BYTES.keys()) AMOUNT_UNIT_RE = re.compile('^([0-9]+)(%s)$' % '|'.join(UNITS)) +UNIT_BYTES_FORMAT = { + 'B': 1, + 'KiB': 1024, + 'MiB': 1048576, + 'GiB': 1073741824 +} + # Only create growth partition if there is at least 1GiB available MIN_DISK_SPACE_BYTES = UNIT_BYTES['GiB'] # Default LVM physical extent size is 4MiB PHYSICAL_EXTENT_BYTES = 4 * UNIT_BYTES['MiB'] +# Grow the thin pool metadata size to 1GiB +POOL_METADATA_SIZE = UNIT_BYTES['GiB'] + class Command(object): """ An object to represent a command to run with associated comment """ @@ -172,13 +182,13 @@ def printable_cmd(cmd): def convert_bytes(num): - """Format a bytes amount with units MB, GB etc""" - step_unit = 1000.0 - - for x in ['B', 'KB', 'MB', 'GB', 'TB']: - if num < step_unit: - return "%d%s" % (num, x) - num /= step_unit + """Format a bytes amount with units GiB, MiB etc""" + for x in ['GiB', 'MiB', 'KiB', 'B']: + unit = UNIT_BYTES_FORMAT[x] + unit_num = num // unit + if unit_num > 0: + break + return "%d%s" % (unit_num, x) def execute(cmd): @@ -499,9 +509,14 @@ def main(argv): group = find_group(opts) partnum = find_next_partnum(devices, disk_name) devname = find_next_device_name(devices, disk_name, partnum) + thin_pool = find_thin_pool(devices, group) + if thin_pool: + # total size available, reduced by POOL_METADATA_SIZE + # rounded down to whole extent + size_bytes -= POOL_METADATA_SIZE + size_bytes -= size_bytes % PHYSICAL_EXTENT_BYTES dev_path = '/dev/%s' % devname grow_vols = find_grow_vols(opts, devices, group, size_bytes) - thin_pool = find_thin_pool(devices, group) commands = [] @@ -528,14 +543,20 @@ def main(argv): ], 'Add physical volume %s to group %s' % (devname, group))) if thin_pool: - # total size available, rounded down to whole extents - pool_size = size_bytes - size_bytes % PHYSICAL_EXTENT_BYTES commands.append(Command([ 'lvextend', - '-L+%sB' % pool_size, + '--poolmetadatasize', + '+%sB' % POOL_METADATA_SIZE, thin_pool, dev_path - ], 'Add %s to thin pool %s' % (convert_bytes(pool_size), + ], 'Add %s to thin pool metadata %s' % ( + convert_bytes(POOL_METADATA_SIZE), thin_pool))) + commands.append(Command([ + 'lvextend', + '-L+%sB' % size_bytes, + thin_pool, + dev_path + ], 'Add %s to thin pool %s' % (convert_bytes(size_bytes), thin_pool))) for volume_path, size_bytes in grow_vols.items(): diff --git a/diskimage_builder/elements/growvols/tests/test_growvols.py b/diskimage_builder/elements/growvols/tests/test_growvols.py index 7c2deb910..db1f87c7d 100644 --- a/diskimage_builder/elements/growvols/tests/test_growvols.py +++ b/diskimage_builder/elements/growvols/tests/test_growvols.py @@ -147,10 +147,10 @@ class TestGrowvols(base.BaseTestCase): def test_convert_bytes(self): self.assertEqual('100B', growvols.convert_bytes(100)) - self.assertEqual('1KB', growvols.convert_bytes(1000)) - self.assertEqual('2MB', growvols.convert_bytes(2000000)) - self.assertEqual('3GB', growvols.convert_bytes(3000000000)) - self.assertEqual('4TB', growvols.convert_bytes(4000000000000)) + self.assertEqual('1000B', growvols.convert_bytes(1000)) + self.assertEqual('1MiB', growvols.convert_bytes(2000000)) + self.assertEqual('2GiB', growvols.convert_bytes(3000000000)) + self.assertEqual('3725GiB', growvols.convert_bytes(4000000000000)) @mock.patch('subprocess.Popen') def test_execute(self, mock_popen): @@ -582,7 +582,7 @@ class TestGrowvols(base.BaseTestCase): SGDISK_LARGEST, VGS, LVS_THIN, - '', '', '', '', '', '', '', '', '', '', '' + '', '', '', '', '', '', '', '', '', '', '', '' ] growvols.main(['growvols', '--yes', '--group', 'vg', '/home=20%', 'fs_var=40%']) @@ -599,13 +599,15 @@ class TestGrowvols(base.BaseTestCase): mock.call(['partprobe']), mock.call(['pvcreate', '/dev/sda5']), mock.call(['vgextend', 'vg', '/dev/sda5']), - mock.call(['lvextend', '-L+209404821504B', + mock.call(['lvextend', '--poolmetadatasize', '+1073741824B', '/dev/mapper/vg-lv_thinpool', '/dev/sda5']), - mock.call(['lvextend', '--size', '+41880125440B', + mock.call(['lvextend', '-L+208331079680B', + '/dev/mapper/vg-lv_thinpool', '/dev/sda5']), + mock.call(['lvextend', '--size', '+41666215936B', '/dev/mapper/vg-lv_home']), - mock.call(['lvextend', '--size', '+83760250880B', + mock.call(['lvextend', '--size', '+83332431872B', '/dev/mapper/vg-lv_var']), - mock.call(['lvextend', '--size', '+83764445184B', + mock.call(['lvextend', '--size', '+83332431872B', '/dev/mapper/vg-lv_root']), mock.call(['xfs_growfs', '/dev/mapper/vg-lv_home']), mock.call(['xfs_growfs', '/dev/mapper/vg-lv_var']),