Fix extent size issue when creating thin pool.

The LVM create thin pool rounds up extents, this
change rounds the free space down to an int to avoid
this issue and also cleans up some comments and
code around how PV's are set up and used.

Change-Id: I725aa819c21c38f464595d0146d6640ecd1e6440

diff --git a/cinder/brick/local_dev/lvm.py b/cinder/brick/local_dev/lvm.py
index 37953464d0464eee6fae13fe3aedcf0388b05f28..55dfb3f7a8c4939ca32d8f4c25bf141fddfc24b8 100644 (file)
--- a/cinder/brick/local_dev/lvm.py
+++ b/cinder/brick/local_dev/lvm.py
@@ -20,6 +20,7 @@ LVM class for performing LVM operations.
 """
 
 import math
+import re
 
 from itertools import izip
 
@@ -66,10 +67,11 @@ class LVM(object):
         self.pv_list = []
         self.lv_list = []
         self.vg_size = 0
-        self.vg_available_space = 0
+        self.vg_free_space = 0
         self.vg_lv_count = 0
         self.vg_uuid = None
         self._execute = executor
+        self.vg_thin_pool = None
 
         if create_vg and physical_volumes is not None:
             self.pv_list = physical_volumes
@@ -87,14 +89,21 @@ class LVM(object):
             LOG.error(_('Unable to locate Volume Group %s') % vg_name)
             raise VolumeGroupNotFound(vg_name=vg_name)
 
+        if lvm_type == 'thin':
+            pool_name = "%s-pool" % self.vg_name
+            if self.get_volume(pool_name) is None:
+                self.create_thin_pool(pool_name)
+            else:
+                self.vg_thin_pool = pool_name
+
     def _size_str(self, size_in_g):
         if '.00' in size_in_g:
             size_in_g = size_in_g.replace('.00', '')
 
         if int(size_in_g) == 0:
-            return '100M'
+            return '100m'
 
-        return '%sG' % size_in_g
+        return '%sg' % size_in_g
 
     def _vg_exists(self):
         """Simple check to see if VG exists.
@@ -155,7 +164,7 @@ class LVM(object):
         :returns: List of Dictionaries with LV info
 
         """
-        cmd = ['lvs', '--noheadings', '-o', 'vg_name,name,size']
+        cmd = ['lvs', '--noheadings', '--unit=g', '-o', 'vg_name,name,size']
         if vg_name is not None:
             cmd += [vg_name]
 
@@ -198,6 +207,7 @@ class LVM(object):
 
         """
         cmd = ['pvs', '--noheadings',
+               '--unit=g',
                '-o', 'vg_name,name,size,free',
                '--separator', ':']
         if vg_name is not None:
@@ -235,7 +245,8 @@ class LVM(object):
 
         """
         cmd = ['vgs', '--noheadings',
-               '-o', 'name,size,free,lv_count,uuid',
+               '--unit=g', '-o',
+               'name,size,free,lv_count,uuid',
                '--separator', ':']
         if vg_name is not None:
             cmd += [vg_name]
@@ -271,15 +282,21 @@ class LVM(object):
             raise VolumeGroupNotFound(vg_name=self.vg_name)
 
         self.vg_size = vg_list[0]['size']
-        self.vg_available_space = vg_list[0]['available']
+        self.vg_free_space = vg_list[0]['available']
         self.vg_lv_count = vg_list[0]['lv_count']
         self.vg_uuid = vg_list[0]['uuid']
+        if self.vg_thin_pool is not None:
+            self.vg_size = self.vg_size
 
         return vg_list[0]
 
     def create_thin_pool(self, name=None, size_str=0):
         """Creates a thin provisioning pool for this VG.
 
+        The syntax here is slightly different than the default
+        lvcreate -T, so we'll just write a custom cmd here
+        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
 
@@ -298,7 +315,16 @@ class LVM(object):
             self.update_volume_group_info()
             size_str = self.vg_size
 
-        self.create_volume(name, size_str, 'thin')
+        # 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]
+
+        putils.execute(*cmd,
+                       root_helper='sudo',
+                       run_as_root=True)
+        self.vg_thin_pool = pool_path
 
     def create_volume(self, name, size_str, lv_type='default', mirror_count=0):
         """Creates a logical volume on the object's VG.
@@ -310,12 +336,13 @@ class LVM(object):
 
         """
 
-        size = self._size_str(size_str)
+        size_str = self._size_str(size_str)
         cmd = ['lvcreate', '-n', name, self.vg_name]
         if lv_type == 'thin':
-            cmd += ['-T', '-V', size]
+            pool_path = '%s/%s' % (self.vg_name, self.vg_thin_pool)
+            cmd = ['lvcreate', '-T', '-V', size_str, '-n', name, pool_path]
         else:
-            cmd += ['-L', size]
+            cmd = ['lvcreate', '-n', name, self.vg_name, '-L', size_str]
 
         if mirror_count > 0:
             cmd += ['-m', mirror_count, '--nosync']

diff --git a/cinder/tests/brick/test_brick_lvm.py b/cinder/tests/brick/test_brick_lvm.py
index cb579b89b3656552d2369f90b2841c1bcc13bd3a..c245245e81c658cfee705a906557c696760442a9 100644 (file)
--- a/cinder/tests/brick/test_brick_lvm.py
+++ b/cinder/tests/brick/test_brick_lvm.py
@@ -62,8 +62,8 @@ class BrickLvmTestCase(test.TestCase):
             data = "  LVM version:     2.02.95(2) (2012-03-06)\n"
         elif 'vgs, --noheadings, -o uuid, fake-volumes' in cmd_string:
             data = "  kVxztV-dKpG-Rz7E-xtKY-jeju-QsYU-SLG6Z1\n"
-        elif 'vgs, --noheadings, -o, name,size,free,lv_count,uuid' in\
-                cmd_string:
+        elif 'vgs, --noheadings, --unit=g, -o, name,size,free,lv_count,uuid'\
+                in cmd_string:
             data = "  fake-volumes:10.00g:10.00g:0:"\
                    "kVxztV-dKpG-Rz7E-xtKY-jeju-QsYU-SLG6Z1\n"
             if 'fake-volumes' in cmd_string:
@@ -72,10 +72,8 @@ class BrickLvmTestCase(test.TestCase):
                     "lWyauW-dKpG-Rz7E-xtKY-jeju-QsYU-SLG7Z2\n"
             data += "  fake-volumes-3:10.00g:10.00g:0:"\
                     "mXzbuX-dKpG-Rz7E-xtKY-jeju-QsYU-SLG8Z3\n"
-        elif 'lvs, --noheadings, -o, vg_name,name,size' in cmd_string:
-            data = "  fake-volumes fake-1 1.00g\n"
-            data += "  fake-volumes fake-2 1.00g\n"
-        elif 'lvs, --noheadings, -o, vg_name,name,size' in cmd_string:
+        elif 'lvs, --noheadings, --unit=g, -o, vg_name,name,size'\
+                in cmd_string:
             data = "  fake-volumes fake-1 1.00g\n"
             data += "  fake-volumes fake-2 1.00g\n"
         elif 'pvs, --noheadings' and 'fake-volumes' in cmd_string: