From cf9915e363924c49a9039543c85c8fdb5322527b Mon Sep 17 00:00:00 2001 From: Eric Harney Date: Fri, 3 Jan 2014 11:19:42 -0500 Subject: [PATCH] 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 --- cinder/brick/local_dev/lvm.py | 54 ++++++++++++---------------- cinder/tests/brick/fake_lvm.py | 6 ++-- cinder/tests/brick/test_brick_lvm.py | 5 ++- cinder/volume/drivers/lvm.py | 8 ++--- cinder/volume/utils.py | 12 +++---- 5 files changed, 38 insertions(+), 47 deletions(-) diff --git a/cinder/brick/local_dev/lvm.py b/cinder/brick/local_dev/lvm.py index d6148b05a..63caa3695 100644 --- 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 d6eec28f5..cd2d6171b 100644 --- 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 5aef2f169..5fb62a21b 100644 --- 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 d5434ab4f..9a1223df7 100644 --- 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 4a5c0ea13..d19e270fa 100644 --- 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) -- 2.45.2