Parse block device lvm lvs size attributes

The block device lvm lvs `size` attribute was passed directly to
lvcreate, so using units M, G means base 2. All other block device
size values are parsed with accepted conventions of M, B being base 10
and MiB, GiB being base 2.

lvm lvs `size` attributes are now parsed the same as other size
attributes. This improves consistency and makes it practical to
calculate volume sizes to fill the partition size. This means existing
size values will now create slightly smaller volumes. Previous sizes
can be restored by changing the unit to MiB, GiB, or increasing the
value for a base 10 unit.

The impact on this change should be minimal, the only known uses of lvm
volumes (TripleO, and element block-device-efi-lvm) uses extents
percentage instead of size. The smaller sizes can always be increased
after deployment.

Requested sizes will also be rounded down to align with physical
extents (4MiB). Previously specifying a value which did not align on
4MiB would consume an extra extent which could unexpectedly consume
more than the partition size.

Change-Id: Ia109cc5105071d82cc895d8d9cb85bc47da20a7a

diskimage_builder/block_device/level1/lvm.py

@@ -19,7 +19,9 @@ from diskimage_builder.block_device.exception \
 from diskimage_builder.block_device.plugin import NodeBase
 from diskimage_builder.block_device.plugin import PluginBase
 from diskimage_builder.block_device.utils import exec_sudo
+from diskimage_builder.block_device.utils import parse_abs_size_spec
 
+PHYSICAL_EXTENT_BYTES = parse_abs_size_spec('4MiB')
 
 logger = logging.getLogger(__name__)
 
@@ -180,7 +182,8 @@ class LvsNode(NodeBase):
         :param state: global state pointer
         :param base: the parent volume group
         :param options: options passed to lvcreate
-        :param size: size of the LV, in MB (this or extents must be provided)
+        :param size: size of the LV, using the supported unit types
+                     (MB, MiB, etc)
         :param extents: size of the LV in extents
         """
         super(LvsNode, self).__init__(name, state)
@@ -193,7 +196,10 @@ class LvsNode(NodeBase):
         cmd = ["lvcreate", ]
         cmd.extend(['--name', self.name])
         if self.size:
-            cmd.extend(['-L', self.size])
+            size = parse_abs_size_spec(self.size)
+            # ensuire size aligns with physical extents
+            size = size - size % PHYSICAL_EXTENT_BYTES
+            cmd.extend(['-L', '%dB' % size])
         elif self.extents:
             cmd.extend(['-l', self.extents])
         if self.options:

diskimage_builder/block_device/tests/test_lvm.py

@@ -104,12 +104,18 @@ class TestLVM(tc.TestGraphGeneration):
             mock.call(['vgcreate', 'vg',
                        '/dev/fake/root', '/dev/fake/data', '--force']),
             # create a bunch of lv's on vg
-            mock.call(['lvcreate', '--name', 'lv_root', '-L', '1800M', 'vg']),
+            mock.call(['lvcreate', '--name', 'lv_root', '-L', '1799356416B',
-            mock.call(['lvcreate', '--name', 'lv_tmp', '-L', '100M', 'vg']),
+                       'vg']),
-            mock.call(['lvcreate', '--name', 'lv_var', '-L', '500M', 'vg']),
+            mock.call(['lvcreate', '--name', 'lv_tmp', '-L', '96468992B',
-            mock.call(['lvcreate', '--name', 'lv_log', '-L', '100M', 'vg']),
+                       'vg']),
-            mock.call(['lvcreate', '--name', 'lv_audit', '-L', '100M', 'vg']),
+            mock.call(['lvcreate', '--name', 'lv_var', '-L', '499122176B',
-            mock.call(['lvcreate', '--name', 'lv_home', '-L', '200M', 'vg'])]
+                       'vg']),
+            mock.call(['lvcreate', '--name', 'lv_log', '-L', '96468992B',
+                       'vg']),
+            mock.call(['lvcreate', '--name', 'lv_audit', '-L', '96468992B',
+                       'vg']),
+            mock.call(['lvcreate', '--name', 'lv_home', '-L', '197132288B',
+                       'vg'])]
 
         self.assertEqual(mock_exec_sudo.call_count, len(cmd_sequence))
         mock_exec_sudo.assert_has_calls(cmd_sequence)
@@ -216,17 +222,17 @@ class TestLVM(tc.TestGraphGeneration):
                            '/dev/fake/data', '--force']),
                 # create a bunch of lv's on vg
                 mock.call(['lvcreate', '--name', 'lv_root',
-                           '-L', '1800M', 'vg1']),
+                           '-L', '1799356416B', 'vg1']),
                 mock.call(['lvcreate', '--name', 'lv_tmp',
-                           '-L', '100M', 'vg1']),
+                           '-L', '96468992B', 'vg1']),
                 mock.call(['lvcreate', '--name', 'lv_var',
-                           '-L', '500M', 'vg2']),
+                           '-L', '499122176B', 'vg2']),
                 mock.call(['lvcreate', '--name', 'lv_log',
-                           '-L', '100M', 'vg2']),
+                           '-L', '96468992B', 'vg2']),
                 mock.call(['lvcreate', '--name', 'lv_audit',
-                           '-L', '100M', 'vg2']),
+                           '-L', '96468992B', 'vg2']),
                 mock.call(['lvcreate', '--name', 'lv_home',
-                           '-L', '200M', 'vg2'])]
+                           '-L', '197132288B', 'vg2'])]
 
             self.assertListEqual(mock_exec_sudo.call_args_list,
                                  cmd_sequence)
@@ -389,9 +395,9 @@ class TestLVM(tc.TestGraphGeneration):
                            '/dev/fake/data1', '/dev/fake/data2', '--force']),
                 # create root and data volume
                 mock.call(['lvcreate', '--name', 'lv_root',
-                           '-L', '1800M', 'vg_root']),
+                           '-L', '1799356416B', 'vg_root']),
                 mock.call(['lvcreate', '--name', 'lv_data',
-                           '-L', '2G', 'vg_data'])
+                           '-L', '1996488704B', 'vg_data'])
             ]
 
             self.assertListEqual(mock_exec_sudo.call_args_list,

releasenotes/notes/lvm-size-unit-c6b790b87d15c53d.yaml

@@ -0,0 +1,14 @@
+---
+other:
+  - |
+    The block device lvm lvs `size` attribute was passed directly to lvcreate,
+    so using units M, G means base 2. All other block device size values are
+    parsed with accepted conventions of M, B being base 10 and MiB, GiB being
+    base 2. lvm lvs `size` attributes are now parsed the same as other size
+    attributes. This improves consistency and makes it practical to calculate
+    volume sizes to fill the partition size. This means existing size values
+    will now create slightly smaller volumes. Previous sizes can be restored by
+    changing the unit to MiB, GiB, or increasing the value for a base 10 unit.
+
+    Requested sizes will also be rounded down to align with physical extents
+    (4MiB).