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

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"""
+    """Format a bytes amount with units GiB, MiB etc"""
-    step_unit = 1000.0
+    for x in ['GiB', 'MiB', 'KiB', 'B']:
-
+        unit = UNIT_BYTES_FORMAT[x]
-    for x in ['B', 'KB', 'MB', 'GB', 'TB']:
+        unit_num = num // unit
-        if num < step_unit:
+        if unit_num > 0:
-            return "%d%s" % (num, x)
+            break
-        num /= step_unit
+    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():

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('1000B', growvols.convert_bytes(1000))
-        self.assertEqual('2MB', growvols.convert_bytes(2000000))
+        self.assertEqual('1MiB', growvols.convert_bytes(2000000))
-        self.assertEqual('3GB', growvols.convert_bytes(3000000000))
+        self.assertEqual('2GiB', growvols.convert_bytes(3000000000))
-        self.assertEqual('4TB', growvols.convert_bytes(4000000000000))
+        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']),