From dc9bbb978aa111db805250fec84c61b84320185f Mon Sep 17 00:00:00 2001 From: Eric Harney Date: Tue, 26 Apr 2016 16:16:43 -0500 Subject: [PATCH] LVM: Create thin pool with 100%FREE For LVM versions new enough to support this, this will ensure VG space is used efficiently. This also removes size specification for create_thin_pool since it does not apply to the flow where Cinder uses this. Change-Id: Ia9107280c418b6c568887ab5d459ad6386898313 --- os_brick/local_dev/lvm.py | 37 ++++++++++++++++------ os_brick/tests/local_dev/test_brick_lvm.py | 22 ++++++------- 2 files changed, 38 insertions(+), 21 deletions(-) diff --git a/os_brick/local_dev/lvm.py b/os_brick/local_dev/lvm.py index 3b239d0cf..2b80c2d89 100644 --- a/os_brick/local_dev/lvm.py +++ b/os_brick/local_dev/lvm.py @@ -259,6 +259,19 @@ class LVM(executor.Executor): return self._supports_lvchange_ignoreskipactivation + @property + def supports_full_pool_create(self): + """Property indicating whether 100% pool creation is supported. + + Check for LVM version >= 2.02.115. + Ref: https://bugzilla.redhat.com/show_bug.cgi?id=998347 + """ + + if self.get_lvm_version(self._root_helper) >= (2, 2, 115): + return True + else: + return False + @staticmethod def get_lv_info(root_helper, vg_name=None, lv_name=None): """Retrieve info about LVs (all, in a VG, or a single LV). @@ -478,10 +491,13 @@ class LVM(executor.Executor): # make sure volume group information is current self.update_volume_group_info() - # leave 5% free for metadata - return "%sg" % (self.vg_free_space * 0.95) + if LVM.supports_full_pool_create: + return ["-l", "100%FREE"] - def create_thin_pool(self, name=None, size_str=None): + # leave 5% free for metadata + return ["-L", "%sg" % (self.vg_free_space * 0.95)] + + def create_thin_pool(self, name=None): """Creates a thin provisioning pool for this VG. The syntax here is slightly different than the default @@ -489,7 +505,6 @@ class LVM(executor.Executor): and do it. :param name: Name to use for pool, default is "-pool" - :param size_str: Size to allocate for pool, default is entire VG :returns: The size string passed to the lvcreate command """ @@ -505,14 +520,15 @@ class LVM(executor.Executor): vg_pool_name = '%s/%s' % (self.vg_name, name) - if not size_str: - size_str = self._calculate_thin_pool_size() + size_args = self._calculate_thin_pool_size() + + cmd = LVM.LVM_CMD_PREFIX + ['lvcreate', '-T'] + cmd.extend(size_args) + cmd.append(vg_pool_name) - cmd = LVM.LVM_CMD_PREFIX + ['lvcreate', '-T', '-L', size_str, - vg_pool_name] LOG.debug("Creating thin pool '%(pool)s' with size %(size)s of " "total %(free)sg", {'pool': vg_pool_name, - 'size': size_str, + 'size': size_args, 'free': self.vg_free_space}) self._execute(*cmd, @@ -520,7 +536,8 @@ class LVM(executor.Executor): run_as_root=True) self.vg_thin_pool = name - return size_str + + return def create_volume(self, name, size_str, lv_type='default', mirror_count=0): """Creates a logical volume on the object's VG. diff --git a/os_brick/tests/local_dev/test_brick_lvm.py b/os_brick/tests/local_dev/test_brick_lvm.py index 2a462d889..f097e84b9 100644 --- a/os_brick/tests/local_dev/test_brick_lvm.py +++ b/os_brick/tests/local_dev/test_brick_lvm.py @@ -50,7 +50,11 @@ class BrickLvmTestCase(base.TestCase): def fake_customised_lvm_version(obj, *cmd, **kwargs): return (" LVM version: 2.02.100(2)-RHEL6 (2013-09-12)\n", "") + def fake_f23_lvm_version(obj, *cmd, **kwargs): + return (" LVM version: 2.02.132(2) (2015-09-22)\n", "") + def fake_execute(obj, *cmd, **kwargs): + # TODO(eharney): remove this and move to per-test mocked execute calls cmd_string = ', '.join(cmd) data = "\n" @@ -133,6 +137,8 @@ class BrickLvmTestCase(base.TestCase): data = " 9:12\n" elif 'lvcreate, -T, -L, ' in cmd_string: pass + elif 'lvcreate, -T, -l, 100%FREE' in cmd_string: + pass elif 'lvcreate, -T, -V, ' in cmd_string: pass elif 'lvcreate, -n, ' in cmd_string: @@ -277,22 +283,16 @@ class BrickLvmTestCase(base.TestCase): self.vg._supports_lvchange_ignoreskipactivation = None - def test_thin_pool_creation(self): - + def test_thin_pool_creation_manual(self): # The size of fake-vg volume group is 10g, so the calculated thin # pool size should be 9.5g (95% of 10g). - self.assertEqual("9.5g", self.vg.create_thin_pool()) - - # Passing a size parameter should result in a thin pool of that exact - # size. - for size in ("1g", "1.2g", "1.75g"): - self.assertEqual(size, self.vg.create_thin_pool(size_str=size)) + self.vg.create_thin_pool() def test_thin_pool_provisioned_capacity(self): self.vg.vg_thin_pool = "test-prov-cap-pool-unit" self.vg.vg_name = 'test-prov-cap-vg-unit' self.assertEqual( - "9.5g", + None, self.vg.create_thin_pool(name=self.vg.vg_thin_pool)) self.assertEqual("9.50", self.vg.vg_thin_pool_size) self.assertEqual(7.6, self.vg.vg_thin_pool_free_space) @@ -301,7 +301,7 @@ class BrickLvmTestCase(base.TestCase): self.vg.vg_thin_pool = "test-prov-cap-pool-no-unit" self.vg.vg_name = 'test-prov-cap-vg-no-unit' self.assertEqual( - "9.5g", + None, self.vg.create_thin_pool(name=self.vg.vg_thin_pool)) self.assertEqual("9.50", self.vg.vg_thin_pool_size) self.assertEqual(7.6, self.vg.vg_thin_pool_free_space) @@ -328,7 +328,7 @@ class BrickLvmTestCase(base.TestCase): self.assertEqual(pool_path, cmd[-1]) self.vg._executor = executor - self.vg.create_thin_pool(pool_name, "1G") + self.vg.create_thin_pool(pool_name) self.vg.create_volume("test", "1G", lv_type='thin') self.assertEqual(pool_name, self.vg.vg_thin_pool)