Merge "Parse block device lvm lvs size attributes"

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).