LVM: Create thin pools of adequate size

Thin pools in LVM are quite different from volume groups or logical
volumes and their differences must be taken into account when providing
thin LVM support in Cinder.

When you create a thin pool, LVM actually creates 4 block devices.  You
can see this after thin pool creation with the following command:

    $ dmsetup ls

    volumes--1-volumes--1--pool       (253:4)
    volumes--1-volumes--1--pool-tpool (253:3)
    volumes--1-volumes--1--pool_tdata (253:2)
    volumes--1-volumes--1--pool_tmeta (253:1)

In the above command, a thin pool named 'volumes-1-pool' was created in
the 'volumes-1' volume group.  Despite this, the 'lvs' command will only
show one logical volume for the thin pool, which can be misleading if
you aren't aware of how thin pools are implemented.

When you create a thin pool, you specify on the command line a size for
the pool.  LVM will interpret this size as the amount of space requested
to store data blocks only.  In order to allow volume sharing and
snapshots, some amount of metadata must be reserved in addition to the
data request.  This amount is calculated by LVM internally and varies
depending on volume size and chunksize.  This is why one cannot simply
allocate 100% of a volume group to a thin pool - there must be some
remaining space for metadata or you will not be able to create volumes
and snapshots that are pool-backed.

This patch allocates 95% of a volume group's free space to the thin
pool.  By doing this, we allow LVM to successfully allocate a region for
metadata.  Additionally, any free space remaining will by dynamically
used by either data or metadata if capacity should become scarce.

The 95/5 split seems like a sane default.  This split can easily (and
probably should) be made user-configurable in the future if the user
expects an abnormal amount of volume sharing.

Change-Id: Id461445780c1574db316ede0c0194736e71640d0
Closes-Bug: #1245909

diff --git a/cinder/brick/local_dev/lvm.py b/cinder/brick/local_dev/lvm.py
index 872e4fe19122810ecfc1f0d65a951357c6a7ce07..433251e2d60c700719547f87448e26eb57a94d56 100644 (file)
--- a/cinder/brick/local_dev/lvm.py
+++ b/cinder/brick/local_dev/lvm.py
@@ -356,7 +356,27 @@ class LVM(executor.Executor):
                 if lv['name'] == self.vg_thin_pool:
                     self.vg_thin_pool_size = lv['size']
 
-    def create_thin_pool(self, name=None, size_str=0):
+    def _calculate_thin_pool_size(self):
+        """Calculates the correct size for a thin pool.
+
+        Ideally we would use 100% of the containing volume group and be done.
+        But the 100%VG notation to lvcreate is not implemented and thus cannot
+        be used.  See https://bugzilla.redhat.com/show_bug.cgi?id=998347
+
+        Further, some amount of free space must remain in the volume group for
+        metadata for the contained logical volumes.  The exact amount depends
+        on how much volume sharing you expect.
+
+        :returns: An lvcreate-ready string for the number of calculated bytes.
+        """
+
+        # make sure volume group information is current
+        self.update_volume_group_info()
+
+        # leave 5% free for metadata
+        return "%sg" % (float(self.vg_free_space) * 0.95)
+
+    def create_thin_pool(self, name=None, size_str=None):
         """Creates a thin provisioning pool for this VG.
 
         The syntax here is slightly different than the default
@@ -365,6 +385,7 @@ class LVM(executor.Executor):
 
         :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
 
         """
 
@@ -377,20 +398,19 @@ class LVM(executor.Executor):
         if name is None:
             name = '%s-pool' % self.vg_name
 
-        if size_str == 0:
-            self.update_volume_group_info()
-            size_str = self.vg_size
+        self.vg_pool_name = '%s/%s' % (self.vg_name, name)
 
-        # NOTE(jdg): lvcreate will round up extents
-        # to avoid issues, let's chop the size off to an int
-        size_str = re.sub(r'\.\d*', '', size_str)
-        pool_path = '%s/%s' % (self.vg_name, name)
-        cmd = ['lvcreate', '-T', '-L', size_str, pool_path]
+        if not size_str:
+            size_str = self._calculate_thin_pool_size()
+
+        cmd = ['lvcreate', '-T', '-L', size_str, self.vg_pool_name]
 
         self._execute(*cmd,
                       root_helper=self._root_helper,
                       run_as_root=True)
+
         self.vg_thin_pool = name
+        return size_str
 
     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/cinder/tests/brick/test_brick_lvm.py b/cinder/tests/brick/test_brick_lvm.py
index 6802e908aee6f83c2ff8fbab1c76e16b2905cf2e..beb60ecd93c30ee2561059a84f1f5f92eb90ed97 100644 (file)
--- a/cinder/tests/brick/test_brick_lvm.py
+++ b/cinder/tests/brick/test_brick_lvm.py
@@ -214,6 +214,17 @@ class BrickLvmTestCase(test.TestCase):
 
         self.vg._supports_lvchange_ignoreskipactivation = None
 
+    def test_thin_pool_creation(self):
+
+        # The size of fake-volumes 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))
+
     def test_volume_create_after_thin_creation(self):
         """Test self.vg.vg_thin_pool is set to pool_name