openstack
/
cinder
Code Issues Proposed changes

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 <vishvananda@gmail.com>
Co-authored-by: John Griffith <john.griffith@solidfire.com>

Closes-Bug: 1269964
Change-Id: I65e16b24367b4093a52c1c52d895fb58ef6a29ff
This commit is contained in:
Vishvananda Ishaya 2014-01-17 14:07:43 -07:00 committed by john-griffith
parent 9c1ad54e78
commit 51099632df
4 changed files with 98 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
cinder/brick/local_dev/lvm.py
View File

@ -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))

56
cinder/tests/brick/test_brick_lvm.py
View File

@ -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)

22
cinder/tests/test_volume.py
View File

@ -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)

12
cinder/volume/drivers/lvm.py
View File

@ -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'] =\