openstack
/
cinder
Code Issues Proposed changes

Enable use of an /etc/cinder/lvm.conf file 

During tempest and Rally runs we've noticed occasional
LVM command hangs (lvs, vgs and pvs), we've also gathered
enough data to show that we see very slow response times from
these commands almost all of the time.

It turns out that this seems to be an issue with us scanning
all devices during LVM operations, including devices that may
be Cinder Volumes that are attaching and detaching from the system.

Inspecting a run instrumented with strace shows a number of LVM
commands timing out due to the device being scanned being removed
during scan, and the LVM command in turn waiting until it times out
on the scan that's in process.

This patch just adds the ability to setup a lvm.conf file in
/etc/cinder.  The Cinder LVM code will now specifically set
the LVM_SYSTEM_DIR environment variable to that directory for
each of the LVM scan commands in brick/local_dev/lvm.py.
If the system doesn't have the file, we use the empty string
which tells LVM to use it's defaults.  This only affects LVM
commands in Cinder, the idea is to ensure we don't impact any
other LVM operations on the node outside of Cinder and that we
behave as we always have in the case of no lvm.conf file being
setup in /etc/cinder.  The presence of the file is auto-detected
on brick/localdev/lvm init.

We'll update the OpenStack Devstack deployment scripts to put this
together and fix things up there first. Until that's done and until
we auto-generate the conf (or document it well), this will be a
*partial* bugfix.

I considered adding a default lvm.conf file to cinder/etc/<sample>
that would be copied in on install, but decided against this to
avoid any possible issues with compatability issues between
platforms or versions.

To use, just copy the /etc/lvm/lvm.conf file to /etc/cinder and
modify the filter as appropriate, for example:
  To use loopback device only:
    filter = [ "a/loop/", "r/.*/" ]
  If you have a physical drive like /dev/sdb1
    filter = [ "a/dev/sdb1/", "r/.*/" ]

Finally, this patch also goes through and cleans up our cmd
variables in brick/localdev/lvm.  We had a mix of using a
cmd array, and strings; this causes inconsistencies and makes
it difficult to extend or modify commands.  Switch everything to
using an array and use extend to provide the correct prefix.

Need to update docs to include a recommendation to create an
/etc/cinder/lvm.conf file and set device filters appropriately.
Doc-Impact
Partial-Bug: #1373513

Change-Id: Ia2289197a6d7fcfc910ee3de48e0a2173881a1e6
This commit is contained in:
John Griffith 2015-01-20 16:31:57 -07:00
parent 107e564ea7
commit eb7bb3e08c
4 changed files with 68 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -19,6 +19,7 @@ LVM class for performing LVM operations.
import itertools
import math
import os
import re
import time
@ -37,10 +38,11 @@ LOG = logging.getLogger(__name__)
class LVM(executor.Executor):
"""LVM object to enable various LVM related operations."""
LVM_CMD_PREFIX = ['env', 'LC_ALL=C']
def __init__(self, vg_name, root_helper, create_vg=False,
physical_volumes=None, lvm_type='default',
executor=putils.execute):
executor=putils.execute, lvm_conf=None):
"""Initialize the LVM object.
@ -97,6 +99,10 @@ class LVM(executor.Executor):
self.activate_lv(self.vg_thin_pool)
self.pv_list = self.get_all_physical_volumes(root_helper, vg_name)
if lvm_conf and os.path.isfile(lvm_conf):
LVM.LVM_CMD_PREFIX = ['env',
'LC_ALL=C',
'LVM_SYSTEM_DIR=/etc/cinder']
def _vg_exists(self):
"""Simple check to see if VG exists.
@ -105,9 +111,11 @@ class LVM(executor.Executor):
"""
exists = False
(out, _err) = self._execute(
'env', 'LC_ALL=C', 'vgs', '--noheadings', '-o', 'name',
self.vg_name, root_helper=self._root_helper, run_as_root=True)
cmd = LVM.LVM_CMD_PREFIX + ['vgs', '--noheadings',
'-o', 'name', self.vg_name]
(out, _err) = self._execute(*cmd,
root_helper=self._root_helper,
run_as_root=True)
if out is not None:
volume_groups = out.split()
@ -121,8 +129,11 @@ class LVM(executor.Executor):
self._execute(*cmd, root_helper=self._root_helper, run_as_root=True)
def _get_vg_uuid(self):
(out, _err) = self._execute('env', 'LC_ALL=C', 'vgs', '--noheadings',
'-o uuid', self.vg_name)
cmd = LVM.LVM_CMD_PREFIX + ['vgs', '--noheadings',
'-o', 'uuid', self.vg_name]
(out, _err) = self._execute(*cmd,
root_helper=self._root_helper,
run_as_root=True)
if out is not None:
return out.split()
else:
@ -136,9 +147,10 @@ class LVM(executor.Executor):
:returns: Free space in GB (float), calculated using data_percent
"""
cmd = ['env', 'LC_ALL=C', 'lvs', '--noheadings', '--unit=g',
'-o', 'size,data_percent', '--separator', ':', '--nosuffix']
cmd = LVM.LVM_CMD_PREFIX +\
['lvs', '--noheadings', '--unit=g',
'-o', 'size,data_percent', '--separator',
':', '--nosuffix']
# NOTE(gfidente): data_percent only applies to some types of LV so we
# make sure to append the actual thin pool name
cmd.append("/dev/%s/%s" % (vg_name, thin_pool_name))
@ -174,7 +186,7 @@ class LVM(executor.Executor):
"""
cmd = ['env', 'LC_ALL=C', 'vgs', '--version']
cmd = LVM.LVM_CMD_PREFIX + ['vgs', '--version']
(out, _err) = putils.execute(*cmd,
root_helper=root_helper,
run_as_root=True)
@ -247,9 +259,8 @@ class LVM(executor.Executor):
"""
cmd = ['env', 'LC_ALL=C', 'lvs', '--noheadings', '--unit=g',
'-o', 'vg_name,name,size', '--nosuffix']
cmd = LVM.LVM_CMD_PREFIX + ['lvs', '--noheadings', '--unit=g',
'-o', 'vg_name,name,size', '--nosuffix']
if lv_name is not None and vg_name is not None:
cmd.append("%s/%s" % (vg_name, lv_name))
elif vg_name is not None:
@ -314,13 +325,11 @@ class LVM(executor.Executor):
"""
field_sep = '|'
cmd = ['env', 'LC_ALL=C', 'pvs', '--noheadings',
'--unit=g',
'-o', 'vg_name,name,size,free',
'--separator', field_sep,
'--nosuffix']
cmd = LVM.LVM_CMD_PREFIX + ['pvs', '--noheadings',
'--unit=g',
'-o', 'vg_name,name,size,free',
'--separator', field_sep,
'--nosuffix']
(out, _err) = putils.execute(*cmd,
root_helper=root_helper,
run_as_root=True)
@ -357,10 +366,11 @@ class LVM(executor.Executor):
:returns: List of Dictionaries with VG info
"""
cmd = ['env', 'LC_ALL=C', 'vgs', '--noheadings', '--unit=g',
'-o', 'name,size,free,lv_count,uuid', '--separator', ':',
'--nosuffix']
cmd = LVM.LVM_CMD_PREFIX + ['vgs', '--noheadings',
'--unit=g', '-o',
'name,size,free,lv_count,uuid',
'--separator', ':',
'--nosuffix']
if vg_name is not None:
cmd.append(vg_name)
@ -678,10 +688,11 @@ class LVM(executor.Executor):
run_as_root=True)
def lv_has_snapshot(self, name):
out, _err = self._execute(
'env', 'LC_ALL=C', 'lvdisplay', '--noheading',
'-C', '-o', 'Attr', '%s/%s' % (self.vg_name, name),
root_helper=self._root_helper, run_as_root=True)
cmd = LVM.LVM_CMD_PREFIX + ['lvdisplay', '--noheading', '-C', '-o',
'Attr', '%s/%s' % (self.vg_name, name)]
out, _err = self._execute(*cmd,
root_helper=self._root_helper,
run_as_root=True)
if out:
out = out.strip()
if (out[0] == 'o') or (out[0] == 'O'):

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

@ -73,7 +73,7 @@ class BrickLvmTestCase(test.TestCase):
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-vg' 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, ' \

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

@ -52,6 +52,12 @@ volume_opts = [
cfg.StrOpt('lvm_type',
default='default',
help='Type of LVM volumes to deploy; (default or thin)'),
cfg.StrOpt('lvm_conf_file',
default='/etc/cinder/lvm.conf',
help='LVM conf file to use for the LVM driver in Cinder; '
'this setting is ignored if the specified file does '
'not exist (You can also specify \'None\' to not use '
'a conf file even if one exists).')
]
CONF = cfg.CONF
@ -229,11 +235,18 @@ class LVMVolumeDriver(driver.VolumeDriver):
"""Verify that requirements are in place to use LVM driver."""
if self.vg is None:
root_helper = utils.get_root_helper()
lvm_conf_file = self.configuration.lvm_conf_file
if lvm_conf_file.lower() == 'none':
lvm_conf_file = None
try:
self.vg = lvm.LVM(self.configuration.volume_group,
root_helper,
lvm_type=self.configuration.lvm_type,
executor=self._execute)
executor=self._execute,
lvm_conf=lvm_conf_file)
except brick_exception.VolumeGroupNotFound:
message = (_("Volume Group %s does not exist") %
self.configuration.volume_group)
@ -519,9 +532,16 @@ class LVMVolumeDriver(driver.VolumeDriver):
return false_ret
helper = utils.get_root_helper()
lvm_conf_file = self.configuration.lvm_conf_file
if lvm_conf_file.lower() == 'none':
lvm_conf_file = None
dest_vg_ref = lvm.LVM(dest_vg, helper,
lvm_type=lvm_type,
executor=self._execute)
executor=self._execute,
lvm_conf=lvm_conf_file)
self.remove_export(ctxt, volume)
self._create_volume(volume['name'],
self._sizestr(volume['size']),

6
etc/cinder/rootwrap.d/volume.filters
View File

@ -14,6 +14,12 @@ vgs: EnvFilter, env, root, LC_ALL=C, vgs
lvs: EnvFilter, env, root, LC_ALL=C, lvs
lvdisplay: EnvFilter, env, root, LC_ALL=C, lvdisplay
# LVM conf var
pvs_lvmconf: EnvFilter, env, root, LVM_SYSTEM_DIR=, LC_ALL=C, pvs
vgs_lvmconf: EnvFilter, env, root, LVM_SYSTEM_DIR=, LC_ALL=C, vgs
lvs_lvmconf: EnvFilter, env, root, LVM_SYSTEM_DIR=, LC_ALL=C, lvs
lvdisplay_lvmconf: EnvFilter, env, root, LVM_SYSTEM_DIR=, LC_ALL=C, lvdisplay
# cinder/volumes/drivers/srb.py: 'pvresize', '--setphysicalvolumesize', sizestr, pvname
pvresize: CommandFilter, pvresize, root