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

@@ -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
+        if LVM.supports_full_pool_create:
-        return "%sg" % (self.vg_free_space * 0.95)
+            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 "<vg-name>-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_args = self._calculate_thin_pool_size()
-            size_str = 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.

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())
+        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))
 
     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)