From: John Griffith <john.griffith@solidfire.com>
Date: Tue, 20 Jan 2015 23:31:57 +0000 (-0700)
Subject: Enable use of an /etc/cinder/lvm.conf file
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=eb7bb3e08c1b8e1008ba447b842b635821e2097e;p=openstack-build%2Fcinder-build.git
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
---
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