From 51099632dfc979c57133a69e4c68cdad3fe47008 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Fri, 17 Jan 2014 14:07:43 -0700 Subject: [PATCH] Fix up calculating space info for mirrored volumes The status reporting in the lvm driver for cinder volume incorrectly reports free capacity. This is due to a couple of issues: a) the number of mirrors isn't taken into account b) there are some pathological cases where a simple division by total mirrors isn't sufficient because all mirrors must be on separate physical volumes. Co-authored-by: Vishvananda Ishaya Co-authored-by: John Griffith Closes-Bug: 1269964 Change-Id: I65e16b24367b4093a52c1c52d895fb58ef6a29ff --- cinder/brick/local_dev/lvm.py | 48 ++++++++++++++++++------ cinder/tests/brick/test_brick_lvm.py | 56 +++++++++++++++------------- cinder/tests/test_volume.py | 22 +++++++++++ cinder/volume/drivers/lvm.py | 12 ++++-- 4 files changed, 98 insertions(+), 40 deletions(-) diff --git a/cinder/brick/local_dev/lvm.py b/cinder/brick/local_dev/lvm.py index 5ff54630a..b5ba09b02 100644 --- a/cinder/brick/local_dev/lvm.py +++ b/cinder/brick/local_dev/lvm.py @@ -92,6 +92,7 @@ class LVM(executor.Executor): self.vg_thin_pool = pool_name self.activate_lv(self.vg_thin_pool) + self.pv_list = self.get_all_physical_volumes(root_helper, vg_name) def _vg_exists(self): """Simple check to see if VG exists. @@ -306,23 +307,21 @@ class LVM(executor.Executor): if no_suffix: cmd.append('--nosuffix') - if vg_name is not None: - cmd.append(vg_name) - (out, err) = putils.execute(*cmd, root_helper=root_helper, run_as_root=True) - pv_list = [] - if out is not None: - pvs = out.split() - for pv in pvs: - fields = pv.split(':') - pv_list.append({'vg': fields[0], - 'name': fields[1], - 'size': fields[2], - 'available': fields[3]}) + pvs = out.split() + if vg_name is not None: + pvs = [pv for pv in pvs if vg_name == pv.split(':')[0]] + pv_list = [] + for pv in pvs: + fields = pv.split(':') + pv_list.append({'vg': fields[0], + 'name': fields[1], + 'size': float(fields[2]), + 'available': float(fields[3])}) return pv_list def get_physical_volumes(self): @@ -625,3 +624,28 @@ class LVM(executor.Executor): LOG.error(_('StdOut :%s') % err.stdout) LOG.error(_('StdErr :%s') % err.stderr) raise + + def vg_mirror_free_space(self, mirror_count): + free_capacity = 0.0 + + disks = [] + for pv in self.pv_list: + disks.append(float(pv['available'])) + + while True: + disks = sorted([a for a in disks if a > 0.0], reverse=True) + if len(disks) <= mirror_count: + break + # consume the smallest disk + disk = disks[-1] + disks = disks[:-1] + # match extents for each mirror on the largest disks + for index in list(range(mirror_count)): + disks[index] -= disk + free_capacity += disk + + return free_capacity + + def vg_mirror_size(self, mirror_count): + return (float(self.vg_free_space) / + (mirror_count + 1)) diff --git a/cinder/tests/brick/test_brick_lvm.py b/cinder/tests/brick/test_brick_lvm.py index 03fde16d0..5aef2f169 100644 --- a/cinder/tests/brick/test_brick_lvm.py +++ b/cinder/tests/brick/test_brick_lvm.py @@ -35,7 +35,7 @@ class BrickLvmTestCase(test.TestCase): def setUp(self): self._mox = mox.Mox() self.configuration = mox.MockObject(conf.Configuration) - self.configuration.volume_group_name = 'fake-volumes' + self.configuration.volume_group_name = 'fake-vg' super(BrickLvmTestCase, self).setUp() #Stub processutils.execute for static methods @@ -66,44 +66,42 @@ class BrickLvmTestCase(test.TestCase): if ('env, LC_ALL=C, vgs, --noheadings, --unit=g, -o, name' == cmd_string): - data = " fake-volumes\n" + data = " fake-vg\n" data += " some-other-vg\n" - elif ('env, LC_ALL=C, vgs, --noheadings, -o, name, fake-volumes' == + elif ('env, LC_ALL=C, vgs, --noheadings, -o, name, fake-vg' == cmd_string): - data = " fake-volumes\n" + data = " fake-vg\n" elif 'env, LC_ALL=C, vgs, --version' in cmd_string: data = " LVM version: 2.02.95(2) (2012-03-06)\n" - elif ('env, LC_ALL=C, vgs, --noheadings, -o uuid, fake-volumes' in + elif ('env, LC_ALL=C, vgs, --noheadings, -o uuid, fake-vg' in cmd_string): data = " kVxztV-dKpG-Rz7E-xtKY-jeju-QsYU-SLG6Z1\n" elif 'env, LC_ALL=C, vgs, --noheadings, --unit=g, ' \ '-o, name,size,free,lv_count,uuid, ' \ '--separator, :, --nosuffix' in cmd_string: - data = " fake-volumes:10.00:10.00:0:"\ + data = " fake-vg:10.00:10.00:0:"\ "kVxztV-dKpG-Rz7E-xtKY-jeju-QsYU-SLG6Z1\n" - if 'fake-volumes' in cmd_string: + if 'fake-vg' in cmd_string: return (data, "") - data += " fake-volumes-2:10.00:10.00:0:"\ + data += " fake-vg-2:10.00:10.00:0:"\ "lWyauW-dKpG-Rz7E-xtKY-jeju-QsYU-SLG7Z2\n" - data += " fake-volumes-3:10.00:10.00:0:"\ + data += " fake-vg-3:10.00:10.00:0:"\ "mXzbuX-dKpG-Rz7E-xtKY-jeju-QsYU-SLG8Z3\n" elif ('env, LC_ALL=C, 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" + data = " fake-vg fake-1 1.00g\n" + data += " fake-vg fake-2 1.00g\n" elif ('env, LC_ALL=C, lvdisplay, --noheading, -C, -o, Attr' in cmd_string): if 'test-volumes' in cmd_string: data = ' wi-a-' else: data = ' owi-a-' - elif 'env, LC_ALL=C, pvs, --noheadings' in cmd_string \ - and 'fake-volumes' in cmd_string: - data = " fake-volumes:/dev/sda:10.00g:8.99g\n" elif 'env, LC_ALL=C, pvs, --noheadings' in cmd_string: - data = " fake-volumes:/dev/sda:10.00g:8.99g\n" - data += " fake-volumes-2:/dev/sdb:10.00g:8.99g\n" - data += " fake-volumes-3:/dev/sdc:10.00g:8.99g\n" + data = " fake-vg:/dev/sda:10.00:1.00\n" + 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: @@ -129,23 +127,28 @@ class BrickLvmTestCase(test.TestCase): self.assertEqual(out[0]['name'], 'fake-1') self.assertEqual(out[0]['size'], '1.00g') - self.assertEqual(out[0]['vg'], 'fake-volumes') + self.assertEqual(out[0]['vg'], 'fake-vg') def test_get_volume(self): self.assertEqual(self.vg.get_volume('fake-1')['name'], 'fake-1') def test_get_all_physical_volumes(self): - pvs = self.vg.get_all_physical_volumes('sudo') + # Filtered VG version + pvs = self.vg.get_all_physical_volumes('sudo', 'fake-vg') self.assertEqual(len(pvs), 3) + # Non-Filtered, all VG's + pvs = self.vg.get_all_physical_volumes('sudo') + self.assertEqual(len(pvs), 4) + def test_get_physical_volumes(self): pvs = self.vg.get_physical_volumes() - self.assertEqual(len(pvs), 1) + self.assertEqual(len(pvs), 3) def test_get_volume_groups(self): self.assertEqual(len(self.vg.get_all_volume_groups('sudo')), 3) self.assertEqual(len(self.vg.get_all_volume_groups('sudo', - 'fake-volumes')), 1) + 'fake-vg')), 1) def test_thin_support(self): # lvm.supports_thin() is a static method and doesn't @@ -227,7 +230,7 @@ class BrickLvmTestCase(test.TestCase): def test_thin_pool_creation(self): - # The size of fake-volumes volume group is 10g, so the calculated thin + # 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()) @@ -237,7 +240,7 @@ class BrickLvmTestCase(test.TestCase): self.assertEqual(size, self.vg.create_thin_pool(size_str=size)) def test_thin_pool_free_space(self): - # The size of fake-volumes-pool is 9g and the allocated data sums up to + # The size of fake-vg-pool is 9g and the allocated data sums up to # 12% so the calculated free space should be 7.92 self.assertEqual(float("7.92"), self.vg._get_thin_pool_free_space("fake-vg", @@ -263,7 +266,7 @@ class BrickLvmTestCase(test.TestCase): self.assertEqual(self.vg.vg_thin_pool, pool_name) def test_lv_has_snapshot(self): - self.assertTrue(self.vg.lv_has_snapshot('fake-volumes')) + self.assertTrue(self.vg.lv_has_snapshot('fake-vg')) self.assertFalse(self.vg.lv_has_snapshot('test-volumes')) def test_activate_lv(self): @@ -271,7 +274,7 @@ class BrickLvmTestCase(test.TestCase): self.vg._supports_lvchange_ignoreskipactivation = True self.vg._execute('lvchange', '-a', 'y', '--yes', '-K', - 'fake-volumes/my-lv', + 'fake-vg/my-lv', root_helper='sudo', run_as_root=True) self._mox.ReplayAll() @@ -279,3 +282,6 @@ class BrickLvmTestCase(test.TestCase): self.vg.activate_lv('my-lv') self._mox.VerifyAll() + + def test_get_mirrored_available_capacity(self): + self.assertEqual(self.vg.vg_mirror_free_space(1), 2.0) diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py index 1a43feacc..00afad0cd 100644 --- a/cinder/tests/test_volume.py +++ b/cinder/tests/test_volume.py @@ -2683,6 +2683,13 @@ class LVMISCSIVolumeDriverTestCase(DriverTestCase): # host to test the check of dest VG existence. return [{'name': 'cinder-volumes-2'}] + def _fake_get_all_physical_volumes(obj, root_helper, vg_name): + return [{}] + + self.stubs.Set(brick_lvm.LVM, + 'get_all_physical_volumes', + _fake_get_all_physical_volumes) + self.stubs.Set(self.volume.driver, '_execute', fake_execute) self.stubs.Set(volutils, 'copy_volume', @@ -2876,6 +2883,9 @@ class ISCSITestCase(DriverTestCase): def test_get_volume_stats(self): + def _fake_get_all_physical_volumes(obj, root_helper, vg_name): + return [{}] + def _fake_get_all_volume_groups(obj, vg_name=None, no_suffix=True): return [{'name': 'cinder-volumes', 'size': '5.52', @@ -2886,6 +2896,11 @@ class ISCSITestCase(DriverTestCase): self.stubs.Set(brick_lvm.LVM, 'get_all_volume_groups', _fake_get_all_volume_groups) + + self.stubs.Set(brick_lvm.LVM, + 'get_all_physical_volumes', + _fake_get_all_physical_volumes) + self.volume.driver.vg = brick_lvm.LVM('cinder-volumes', 'sudo') self.volume.driver._update_volume_stats() @@ -2924,6 +2939,9 @@ class ISERTestCase(ISCSITestCase): self.configuration.iser_port = 3260 def test_get_volume_stats(self): + def _fake_get_all_physical_volumes(obj, root_helper, vg_name): + return [{}] + def _fake_get_all_volume_groups(obj, vg_name=None, no_suffix=True): return [{'name': 'cinder-volumes', 'size': '5.52', @@ -2931,6 +2949,10 @@ class ISERTestCase(ISCSITestCase): 'lv_count': '2', 'uuid': 'vR1JU3-FAKE-C4A9-PQFh-Mctm-9FwA-Xwzc1m'}] + self.stubs.Set(brick_lvm.LVM, + 'get_all_physical_volumes', + _fake_get_all_physical_volumes) + self.stubs.Set(brick_lvm.LVM, 'get_all_volume_groups', _fake_get_all_volume_groups) diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py index 7c4ac314f..0d7369146 100644 --- a/cinder/volume/drivers/lvm.py +++ b/cinder/volume/drivers/lvm.py @@ -371,11 +371,17 @@ class LVMVolumeDriver(driver.VolumeDriver): data["driver_version"] = self.VERSION data["storage_protocol"] = self.protocol - data['total_capacity_gb'] = float(self.vg.vg_size) - data['free_capacity_gb'] = float(self.vg.vg_free_space) - if self.configuration.lvm_type == 'thin': + if self.configuration.lvm_mirrors > 0: + data['total_capacity_gb'] =\ + self.vg.vg_mirror_size(self.configuration.lvm_mirrors) + 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) + else: + data['total_capacity_gb'] = float(self.vg.vg_size) + data['free_capacity_gb'] = float(self.vg.vg_free_space) data['reserved_percentage'] = self.configuration.reserved_percentage data['QoS_support'] = False data['location_info'] =\ -- 2.45.2