From 38c98ba0d5084dcaa8507c5566eaa6e9b88d39c8 Mon Sep 17 00:00:00 2001 From: Jan Fajerski Date: Tue, 22 Dec 2020 14:29:02 +0100 Subject: [PATCH] ceph-volume: add some flexibility to bytes_to_extents For batch we want to report the projected sizes before creating any LVM structures, we use the device size for that. This means we pass this projected size to lvm/api.py::create_lv. This could result in situations where we request more extents for the new LV than are actually available, as LVM takes up some available space on the block device (plus alignment). We fix this by adjusting the extent count downwards if its less than 1% off the requested value. Fixes: https://tracker.ceph.com/issues/47758 Signed-off-by: Jan Fajerski (cherry picked from commit ece6e7eb9a92cb944b2f201a05a5acfb877c2dba) --- src/ceph-volume/ceph_volume/api/lvm.py | 26 ++++++++++++++++-- .../ceph_volume/tests/api/test_lvm.py | 27 +++++++++++++++++-- src/ceph-volume/ceph_volume/util/device.py | 2 +- 3 files changed, 50 insertions(+), 5 deletions(-) diff --git a/src/ceph-volume/ceph_volume/api/lvm.py b/src/ceph-volume/ceph_volume/api/lvm.py index 461e010506c2f..30362f1bd2759 100644 --- a/src/ceph-volume/ceph_volume/api/lvm.py +++ b/src/ceph-volume/ceph_volume/api/lvm.py @@ -588,9 +588,31 @@ class VolumeGroup(object): def bytes_to_extents(self, size): ''' - Return a how many extents we can fit into a size in bytes. + Return a how many free extents we can fit into a size in bytes. This has + some uncertainty involved. If size/extent_size is within 1% of the + actual free extents we will return the extent count, otherwise we'll + throw an error. + This accomodates for the size calculation in batch. We need to report + the OSD layout but have not yet created any LVM structures. We use the + disk size in batch if no VG is present and that will overshoot the + actual free_extent count due to LVM overhead. + ''' - return int(size / int(self.vg_extent_size)) + b_to_ext = int(size / int(self.vg_extent_size)) + if b_to_ext < int(self.vg_free_count): + # return bytes in extents if there is more space + return b_to_ext + elif b_to_ext / int(self.vg_free_count) - 1 < 0.01: + # return vg_fre_count if its less then 1% off + logger.info( + 'bytes_to_extents results in {} but only {} ' + 'are available, adjusting the latter'.format(b_to_ext, + self.vg_free_count)) + return int(self.vg_free_count) + # else raise an exception + raise RuntimeError('Can\'t convert {} to free extents, only {} ({} ' + 'bytes) are free'.format(size, self.vg_free_count, + self.free)) def slots_to_extents(self, slots): ''' diff --git a/src/ceph-volume/ceph_volume/tests/api/test_lvm.py b/src/ceph-volume/ceph_volume/tests/api/test_lvm.py index cd2f9d9cb42ef..fe7426f4fd506 100644 --- a/src/ceph-volume/ceph_volume/tests/api/test_lvm.py +++ b/src/ceph-volume/ceph_volume/tests/api/test_lvm.py @@ -194,10 +194,33 @@ class TestCreateLV(object): @patch('ceph_volume.api.lvm.get_first_lv') def test_uses_size(self, m_get_first_lv, m_call, m_run, monkeypatch): m_get_first_lv.return_value = self.foo_volume - api.create_lv('foo', 0, vg=self.foo_group, size=5368709120, tags={'ceph.type': 'data'}) - expected = ['lvcreate', '--yes', '-l', '1280', '-n', 'foo-0', 'foo_group'] + api.create_lv('foo', 0, vg=self.foo_group, size=419430400, tags={'ceph.type': 'data'}) + expected = ['lvcreate', '--yes', '-l', '100', '-n', 'foo-0', 'foo_group'] m_run.assert_called_with(expected) + @patch('ceph_volume.api.lvm.process.run') + @patch('ceph_volume.api.lvm.process.call') + @patch('ceph_volume.api.lvm.get_first_lv') + def test_uses_size_adjust_if_1percent_over(self, m_get_first_lv, m_call, m_run, monkeypatch): + foo_volume = api.Volume(lv_name='foo', lv_path='/path', vg_name='foo_group', lv_tags='') + foo_group = api.VolumeGroup(vg_name='foo_group', + vg_extent_size=4194304, + vg_extent_count=1000, + vg_free_count=1000) + m_get_first_lv.return_value = foo_volume + # 423624704 should be just under 1% off of the available size 419430400 + api.create_lv('foo', 0, vg=foo_group, size=4232052736, tags={'ceph.type': 'data'}) + expected = ['lvcreate', '--yes', '-l', '1000', '-n', 'foo-0', 'foo_group'] + m_run.assert_called_with(expected) + + @patch('ceph_volume.api.lvm.process.run') + @patch('ceph_volume.api.lvm.process.call') + @patch('ceph_volume.api.lvm.get_first_lv') + def test_uses_size_too_large(self, m_get_first_lv, m_call, m_run, monkeypatch): + m_get_first_lv.return_value = self.foo_volume + with pytest.raises(RuntimeError): + api.create_lv('foo', 0, vg=self.foo_group, size=5368709120, tags={'ceph.type': 'data'}) + @patch('ceph_volume.api.lvm.process.run') @patch('ceph_volume.api.lvm.process.call') @patch('ceph_volume.api.lvm.get_first_lv') diff --git a/src/ceph-volume/ceph_volume/util/device.py b/src/ceph-volume/ceph_volume/util/device.py index 193a72f3d008c..461775202a72e 100644 --- a/src/ceph-volume/ceph_volume/util/device.py +++ b/src/ceph-volume/ceph_volume/util/device.py @@ -454,7 +454,7 @@ class Device(object): # assuming 4M extents here extent_size = 4194304 vg_free = int(self.size / extent_size) * extent_size - if self.size % 4194304 == 0: + if self.size % extent_size == 0: # If the extent size divides size exactly, deduct on extent for # LVM metadata vg_free -= extent_size -- 2.39.5