From d090126c66c1df3e8f9b507b24db94a03e2fec7c Mon Sep 17 00:00:00 2001 From: Steve Baker Date: Fri, 29 Apr 2022 10:49:03 +1200 Subject: [PATCH] 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 | 10 ++++-- .../block_device/tests/test_lvm.py | 34 +++++++++++-------- .../notes/lvm-size-unit-c6b790b87d15c53d.yaml | 14 ++++++++ 3 files changed, 42 insertions(+), 16 deletions(-) create mode 100644 releasenotes/notes/lvm-size-unit-c6b790b87d15c53d.yaml diff --git a/diskimage_builder/block_device/level1/lvm.py b/diskimage_builder/block_device/level1/lvm.py index c1b6172c3..f86f09e3a 100644 --- a/diskimage_builder/block_device/level1/lvm.py +++ b/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: diff --git a/diskimage_builder/block_device/tests/test_lvm.py b/diskimage_builder/block_device/tests/test_lvm.py index 4f4253504..f39ba03d3 100644 --- a/diskimage_builder/block_device/tests/test_lvm.py +++ b/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_tmp', '-L', '100M', 'vg']), - mock.call(['lvcreate', '--name', 'lv_var', '-L', '500M', 'vg']), - mock.call(['lvcreate', '--name', 'lv_log', '-L', '100M', 'vg']), - mock.call(['lvcreate', '--name', 'lv_audit', '-L', '100M', 'vg']), - mock.call(['lvcreate', '--name', 'lv_home', '-L', '200M', 'vg'])] + mock.call(['lvcreate', '--name', 'lv_root', '-L', '1799356416B', + 'vg']), + mock.call(['lvcreate', '--name', 'lv_tmp', '-L', '96468992B', + 'vg']), + mock.call(['lvcreate', '--name', 'lv_var', '-L', '499122176B', + '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, diff --git a/releasenotes/notes/lvm-size-unit-c6b790b87d15c53d.yaml b/releasenotes/notes/lvm-size-unit-c6b790b87d15c53d.yaml new file mode 100644 index 000000000..39f307de0 --- /dev/null +++ b/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). \ No newline at end of file