From eb7bb3e08c1b8e1008ba447b842b635821e2097e Mon Sep 17 00:00:00 2001 From: John Griffith Date: Tue, 20 Jan 2015 16:31:57 -0700 Subject: [PATCH] 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/ 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 --- cinder/brick/local_dev/lvm.py | 67 ++++++++++++++++------------ cinder/tests/brick/test_brick_lvm.py | 2 +- cinder/volume/drivers/lvm.py | 24 +++++++++- etc/cinder/rootwrap.d/volume.filters | 6 +++ 4 files changed, 68 insertions(+), 31 deletions(-) diff --git a/cinder/brick/local_dev/lvm.py b/cinder/brick/local_dev/lvm.py index 315c82068..b0ceff71d 100644 --- a/cinder/brick/local_dev/lvm.py +++ b/cinder/brick/local_dev/lvm.py @@ -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'): diff --git a/cinder/tests/brick/test_brick_lvm.py b/cinder/tests/brick/test_brick_lvm.py index cd8ad48cb..1e84032f7 100644 --- a/cinder/tests/brick/test_brick_lvm.py +++ b/cinder/tests/brick/test_brick_lvm.py @@ -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, ' \ diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py index 4954e34b1..f5a4b1e2d 100644 --- a/cinder/volume/drivers/lvm.py +++ b/cinder/volume/drivers/lvm.py @@ -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']), diff --git a/etc/cinder/rootwrap.d/volume.filters b/etc/cinder/rootwrap.d/volume.filters index a90f8ded1..5fc7d7daa 100644 --- a/etc/cinder/rootwrap.d/volume.filters +++ b/etc/cinder/rootwrap.d/volume.filters @@ -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 -- 2.45.2