Brick LVM: Handle space info as numeric types

Rather than relying on callers to cast values from the VG object
to float, always create them as floats up front.

This removes the nosuffix option used with the LVM commands in
Brick, as it does not appear to be used.

Also removes unneeded self.vg_pool_name variable.

Change-Id: Ic19ce206bc8069071e3c2d9e58ba5393119e9b4d

diff --git a/cinder/brick/local_dev/lvm.py b/cinder/brick/local_dev/lvm.py
index d6148b05afbb11b77a07a361d0e18431059a6e39..63caa3695a4c9e398a8d30b53964289fe6febee9 100644 (file)
--- a/cinder/brick/local_dev/lvm.py
+++ b/cinder/brick/local_dev/lvm.py
@@ -56,13 +56,13 @@ class LVM(executor.Executor):
         self.vg_name = vg_name
         self.pv_list = []
         self.lv_list = []
-        self.vg_size = 0
-        self.vg_free_space = 0
+        self.vg_size = 0.0
+        self.vg_free_space = 0.0
         self.vg_lv_count = 0
         self.vg_uuid = None
         self.vg_thin_pool = None
-        self.vg_thin_pool_size = 0
-        self.vg_thin_pool_free_space = 0
+        self.vg_thin_pool_size = 0.0
+        self.vg_thin_pool_free_space = 0.0
         self._supports_snapshot_lv_activation = None
         self._supports_lvchange_ignoreskipactivation = None
 
@@ -129,7 +129,7 @@ class LVM(executor.Executor):
 
         :param vg_name: the vg where the pool is placed
         :param thin_pool_name: the thin pool to gather info for
-        :returns: Free space, calculated after the data_percent value
+        :returns: Free space in GB (float), calculated using data_percent
 
         """
         cmd = ['env', 'LC_ALL=C', 'lvs', '--noheadings', '--unit=g',
@@ -139,7 +139,7 @@ class LVM(executor.Executor):
         # make sure to append the actual thin pool name
         cmd.append("/dev/%s/%s" % (vg_name, thin_pool_name))
 
-        free_space = 0
+        free_space = 0.0
 
         try:
             (out, err) = self._execute(*cmd,
@@ -240,20 +240,17 @@ class LVM(executor.Executor):
         return self._supports_lvchange_ignoreskipactivation
 
     @staticmethod
-    def get_all_volumes(root_helper, vg_name=None, no_suffix=True):
+    def get_all_volumes(root_helper, vg_name=None):
         """Static method to get all LV's on a system.
 
         :param root_helper: root_helper to use for execute
         :param vg_name: optional, gathers info for only the specified VG
-        :param no_suffix: optional, reports sizes in g with no suffix
         :returns: List of Dictionaries with LV info
 
         """
-        cmd = ['env', 'LC_ALL=C', 'lvs', '--noheadings', '--unit=g',
-               '-o', 'vg_name,name,size']
 
-        if no_suffix:
-            cmd.append('--nosuffix')
+        cmd = ['env', 'LC_ALL=C', 'lvs', '--noheadings', '--unit=g',
+               '-o', 'vg_name,name,size', '--nosuffix']
 
         if vg_name is not None:
             cmd.append(vg_name)
@@ -291,21 +288,19 @@ class LVM(executor.Executor):
                 return r
 
     @staticmethod
-    def get_all_physical_volumes(root_helper, vg_name=None, no_suffix=True):
+    def get_all_physical_volumes(root_helper, vg_name=None):
         """Static method to get all PVs on a system.
 
         :param root_helper: root_helper to use for execute
         :param vg_name: optional, gathers info for only the specified VG
-        :param no_suffix: optional, reports sizes in g with no suffix
         :returns: List of Dictionaries with PV info
 
         """
         cmd = ['env', 'LC_ALL=C', 'pvs', '--noheadings',
                '--unit=g',
                '-o', 'vg_name,name,size,free',
-               '--separator', ':']
-        if no_suffix:
-            cmd.append('--nosuffix')
+               '--separator', ':',
+               '--nosuffix']
 
         (out, err) = putils.execute(*cmd,
                                     root_helper=root_helper,
@@ -335,20 +330,17 @@ class LVM(executor.Executor):
         return self.pv_list
 
     @staticmethod
-    def get_all_volume_groups(root_helper, vg_name=None, no_suffix=True):
+    def get_all_volume_groups(root_helper, vg_name=None):
         """Static method to get all VGs on a system.
 
         :param root_helper: root_helper to use for execute
         :param vg_name: optional, gathers info for only the specified VG
-        :param no_suffix: optional, reports sizes in g with no suffix
         :returns: List of Dictionaries with VG info
 
         """
         cmd = ['env', 'LC_ALL=C', 'vgs', '--noheadings', '--unit=g',
-               '-o', 'name,size,free,lv_count,uuid', '--separator', ':']
-
-        if no_suffix:
-            cmd.append('--nosuffix')
+               '-o', 'name,size,free,lv_count,uuid', '--separator', ':',
+               '--nosuffix']
 
         if vg_name is not None:
             cmd.append(vg_name)
@@ -363,9 +355,9 @@ class LVM(executor.Executor):
             for vg in vgs:
                 fields = vg.split(':')
                 vg_list.append({'name': fields[0],
-                                'size': fields[1],
-                                'available': fields[2],
-                                'lv_count': fields[3],
+                                'size': float(fields[1]),
+                                'available': float(fields[2]),
+                                'lv_count': int(fields[3]),
                                 'uuid': fields[4]})
 
         return vg_list
@@ -385,9 +377,9 @@ class LVM(executor.Executor):
             LOG.error(_('Unable to find VG: %s') % self.vg_name)
             raise exception.VolumeGroupNotFound(vg_name=self.vg_name)
 
-        self.vg_size = vg_list[0]['size']
-        self.vg_free_space = vg_list[0]['available']
-        self.vg_lv_count = vg_list[0]['lv_count']
+        self.vg_size = float(vg_list[0]['size'])
+        self.vg_free_space = float(vg_list[0]['available'])
+        self.vg_lv_count = int(vg_list[0]['lv_count'])
         self.vg_uuid = vg_list[0]['uuid']
 
         if self.vg_thin_pool is not None:
@@ -440,12 +432,12 @@ class LVM(executor.Executor):
         if name is None:
             name = '%s-pool' % self.vg_name
 
-        self.vg_pool_name = '%s/%s' % (self.vg_name, name)
+        vg_pool_name = '%s/%s' % (self.vg_name, name)
 
         if not size_str:
             size_str = self._calculate_thin_pool_size()
 
-        cmd = ['lvcreate', '-T', '-L', size_str, self.vg_pool_name]
+        cmd = ['lvcreate', '-T', '-L', size_str, vg_pool_name]
 
         self._execute(*cmd,
                       root_helper=self._root_helper,

diff --git a/cinder/tests/brick/fake_lvm.py b/cinder/tests/brick/fake_lvm.py
index d6eec28f55c36daea405e3de820565c35e76e6e1..cd2d6171b39bccf3db5e6bf563edfbf0de6dd5d7 100644 (file)
--- a/cinder/tests/brick/fake_lvm.py
+++ b/cinder/tests/brick/fake_lvm.py
@@ -27,7 +27,7 @@ class FakeBrickLVM(object):
     def supports_thin_provisioning():
         return False
 
-    def get_all_volumes(vg_name=None, no_suffix=True):
+    def get_all_volumes(vg_name=None):
         if vg_name is not None:
             return [vg_name]
         return ['cinder-volumes', 'fake-vg-1']
@@ -38,13 +38,13 @@ class FakeBrickLVM(object):
     def get_volume(self, name):
         return ['name']
 
-    def get_all_physical_volumes(vg_name=None, no_suffix=True):
+    def get_all_physical_volumes(vg_name=None):
         return []
 
     def get_physical_volumes(self):
         return []
 
-    def get_all_volume_groups(vg_name=None, no_suffix=True):
+    def get_all_volume_groups(vg_name=None):
         return ['cinder-volumes', 'fake-vg']
 
     def update_volume_group_info(self):

diff --git a/cinder/tests/brick/test_brick_lvm.py b/cinder/tests/brick/test_brick_lvm.py
index 5aef2f1695f47697bffe858265183bcbcb44b9e0..5fb62a21b9eef9b4c44694ebaf1ea116f0c5d001 100644 (file)
--- a/cinder/tests/brick/test_brick_lvm.py
+++ b/cinder/tests/brick/test_brick_lvm.py
@@ -102,9 +102,8 @@ class BrickLvmTestCase(test.TestCase):
             data += "  fake-vg:/dev/sdb:10.00:1.00\n"
             data += "  fake-vg:/dev/sdc:10.00:8.99\n"
             data += "  fake-vg-2:/dev/sdd:10.00:9.99\n"
-        elif 'env, LC_ALL=C, lvs, --noheadings, --unit=g, -o, ' \
-             'size,data_percent, ' \
-             '--separator, :' in cmd_string:
+        elif 'env, LC_ALL=C, lvs, --noheadings, --unit=g' \
+             ', -o, size,data_percent, --separator, :' in cmd_string:
             data = "  9:12\n"
         elif 'lvcreate, -T, -L, ' in cmd_string:
             pass

diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py
index d5434ab4fe27feefca400bb6028808769f5835f4..9a1223df7dbfb7c9fb2bc245784078f9e3931d13 100644 (file)
--- a/cinder/volume/drivers/lvm.py
+++ b/cinder/volume/drivers/lvm.py
@@ -379,11 +379,11 @@ class LVMVolumeDriver(driver.VolumeDriver):
             data['free_capacity_gb'] =\
                 self.vg.vg_mirror_free_space(self.configuration.lvm_mirrors)
         elif self.configuration.lvm_type == 'thin':
-            data['total_capacity_gb'] = float(self.vg.vg_thin_pool_size)
-            data['free_capacity_gb'] = float(self.vg.vg_thin_pool_free_space)
+            data['total_capacity_gb'] = self.vg.vg_thin_pool_size
+            data['free_capacity_gb'] = self.vg.vg_thin_pool_free_space
         else:
-            data['total_capacity_gb'] = float(self.vg.vg_size)
-            data['free_capacity_gb'] = float(self.vg.vg_free_space)
+            data['total_capacity_gb'] = self.vg.vg_size
+            data['free_capacity_gb'] = self.vg.vg_free_space
         data['reserved_percentage'] = self.configuration.reserved_percentage
         data['QoS_support'] = False
         data['location_info'] =\

diff --git a/cinder/volume/utils.py b/cinder/volume/utils.py
index 4a5c0ea13c704ef287455f602122357aa97236e3..d19e270fae795d5266719e9441f5205eafb32f16 100644 (file)
--- a/cinder/volume/utils.py
+++ b/cinder/volume/utils.py
@@ -188,19 +188,19 @@ def supports_thin_provisioning():
         utils.get_root_helper())
 
 
-def get_all_volumes(vg_name=None, no_suffix=True):
+def get_all_volumes(vg_name=None):
     return brick_lvm.LVM.get_all_volumes(
         utils.get_root_helper(),
-        vg_name, no_suffix)
+        vg_name)
 
 
-def get_all_physical_volumes(vg_name=None, no_suffix=True):
+def get_all_physical_volumes(vg_name=None):
     return brick_lvm.LVM.get_all_physical_volumes(
         utils.get_root_helper(),
-        vg_name, no_suffix)
+        vg_name)
 
 
-def get_all_volume_groups(vg_name=None, no_suffix=True):
+def get_all_volume_groups(vg_name=None):
     return brick_lvm.LVM.get_all_volume_groups(
         utils.get_root_helper(),
-        vg_name, no_suffix)
+        vg_name)