[rhel7-branch 3/5] devicelibs.lvm: refactor _getConfigArgs()/lvm()
wgwoods
installerbot-noreply at redhat.com
Thu Oct 8 17:17:42 UTC 2015
From: Will Woods <wwoods at redhat.com>
Every place in devicelibs.lvm that executes lvm uses _getConfigArgs()[1] to
help build the argument list. So it'd be cleaner and simpler if we made
every call to the lvm binary go through lvm(), and then just called
_getConfigArgs() from there.
This also means we can pass the caller-requested lvm command+arguments to
_getConfigArgs(), so it can examine them and add appropriate config options
for us.
For example: right now the caller has to explicitly request
read_only_locking if it wants read-only locks. The current code does that
for every call to the informational commands (lvs, pvs, vgs, etc.).
This change moves that logic into _getConfigArgs(), which now just
automatically does read-only locking whenever we call one of the info
commands. Much simpler!
Mechanically, this patch should only make the following changes:
* make _getConfigArgs() do read_only_locking automatically when needed
* make lvm() call _getConfigArgs() for every lvm command
* remove all other calls to _getConfigArgs()
* remove all calls to the `lvm` binary that don't use lvm():
* add (capture=False, ignore_errors=False) kwargs to lvm
* replace `util.capture_output(["lvm"] + args)` with
`lvm(args, capture=True, ignore_errors=True)`
This should result in no actual behavioral changes.
[1] except thinsnapshotcreate(), but that was an oversight (which is
therefore fixed by this commit)
(cherry picked from commit 6c24bb25fddd3333592ae37bc3379e25f08fc3f5)
Related: rhbz#1269144
---
blivet/devicelibs/lvm.py | 106 +++++++++++++++++++++--------------------------
1 file changed, 48 insertions(+), 58 deletions(-)
diff --git a/blivet/devicelibs/lvm.py b/blivet/devicelibs/lvm.py
index 00c5ec2..4382993 100644
--- a/blivet/devicelibs/lvm.py
+++ b/blivet/devicelibs/lvm.py
@@ -74,11 +74,15 @@ def has_lvm():
config_args_data = { "filterRejects": [], # regular expressions to reject.
"filterAccepts": [] } # regexp to accept
-def _getConfigArgs(**kwargs):
+def _getConfigArgs(args):
"""lvm command accepts lvm.conf type arguments preceded by --config. """
- config_args = []
- read_only_locking = kwargs.get("read_only_locking", False)
+ # These commands are read-only, so we can run them with read-only locking.
+ # (not an exhaustive list, but these are the only ones used here)
+ READONLY_COMMANDS = ('lvs', 'pvs', 'vgs')
+
+ cmd = args[0]
+ config_args = []
filter_string = ""
rejects = config_args_data["filterRejects"]
@@ -98,7 +102,7 @@ def _getConfigArgs(**kwargs):
# "preferred_names", "filter", "cache_dir", "write_cache_state",
# "types", "sysfs_scan", "md_component_detection". see man lvm.conf.
config_string = " devices { %s } " % (devices_string) # strings can be added
- if read_only_locking:
+ if cmd in READONLY_COMMANDS:
config_string += "global {locking_type=4} "
if config_string:
config_args = ["--config", config_string]
@@ -224,14 +228,24 @@ def strip_lvm_warnings(buf):
"""
return [l for l in buf.splitlines() if l and not l.lstrip().startswith("WARNING:")]
-def lvm(args):
- ret = util.run_program(["lvm"] + args)
- if ret:
- raise LVMError("running lvm " + " ".join(args) + " failed")
+def lvm(args, capture=False, ignore_errors=False):
+ """ Runs lvm with specified arguments.
+
+ :param bool capture: if True, return the output of the command.
+ :param bool ignore_errors: if True, do not raise LVMError on failure
+ :returns: the output of the command or None
+ :rtype: str or NoneType
+ :raises: LVMError if command fails
+ """
+ argv = ["lvm"] + args + _getConfigArgs(args)
+ (ret, out) = util.run_program_and_capture_output(argv)
+ if ret and not ignore_errors:
+ raise LVMError("running "+ " ".join(argv) + " failed")
+ if capture:
+ return out
def pvcreate(device, data_alignment=None):
- args = ["pvcreate"] + \
- _getConfigArgs()
+ args = ["pvcreate"]
if data_alignment is not None:
args.extend(["--dataalignment", "%dk" % data_alignment.convertTo(spec="KiB")])
@@ -243,10 +257,9 @@ def pvcreate(device, data_alignment=None):
raise LVMError("pvcreate failed for %s: %s" % (device, msg))
def pvresize(device, size):
- args = ["pvresize"] + \
- ["--setphysicalvolumesize", ("%dm" % size.convertTo(spec="mib"))] + \
- _getConfigArgs() + \
- [device]
+ args = ["pvresize",
+ "--setphysicalvolumesize", ("%dm" % size.convertTo(spec="mib")),
+ device]
try:
lvm(args)
@@ -254,9 +267,7 @@ def pvresize(device, size):
raise LVMError("pvresize failed for %s: %s" % (device, msg))
def pvremove(device):
- args = ["pvremove", "--force", "--force", "--yes"] + \
- _getConfigArgs() + \
- [device]
+ args = ["pvremove", "--force", "--force", "--yes", device]
try:
lvm(args)
@@ -264,9 +275,7 @@ def pvremove(device):
raise LVMError("pvremove failed for %s: %s" % (device, msg))
def pvscan(device):
- args = ["pvscan", "--cache",] + \
- _getConfigArgs() + \
- [device]
+ args = ["pvscan", "--cache", device]
try:
lvm(args)
@@ -279,7 +288,7 @@ def pvmove(source, dest=None):
:param str source: pv device path to move extents off of
:keyword str dest: pv device path to move the extents onto
"""
- args = ["pvmove"] + _getConfigArgs() + [source]
+ args = ["pvmove", source]
if dest:
args.append(dest)
@@ -324,13 +333,12 @@ def pvinfo(device=None):
"--unit=k", "--nosuffix", "--nameprefixes",
"--unquoted", "--noheadings",
"-opv_name,pv_uuid,pe_start,vg_name,vg_uuid,vg_size,vg_free,"
- "vg_extent_size,vg_extent_count,vg_free_count,pv_count"] + \
- _getConfigArgs(read_only_locking=True)
+ "vg_extent_size,vg_extent_count,vg_free_count,pv_count"]
if device:
args.append(device)
- buf = util.capture_output(["lvm"] + args)
+ buf = lvm(args, capture=True, ignore_errors=True)
pvs = {}
for line in buf.splitlines():
info = parse_lvm_vars(line)
@@ -346,7 +354,6 @@ def vgcreate(vg_name, pv_list, pe_size):
argv = ["vgcreate"]
if pe_size:
argv.extend(["-s", "%sk" % pe_size.convertTo(spec="KiB")])
- argv.extend(_getConfigArgs())
argv.append(vg_name)
argv.extend(pv_list)
@@ -356,9 +363,7 @@ def vgcreate(vg_name, pv_list, pe_size):
raise LVMError("vgcreate failed for %s: %s" % (vg_name, msg))
def vgremove(vg_name):
- args = ["vgremove", "--force"] + \
- _getConfigArgs() +\
- [vg_name]
+ args = ["vgremove", "--force", vg_name]
try:
lvm(args)
@@ -366,9 +371,7 @@ def vgremove(vg_name):
raise LVMError("vgremove failed for %s: %s" % (vg_name, msg))
def vgactivate(vg_name):
- args = ["vgchange", "-a", "y"] + \
- _getConfigArgs() + \
- [vg_name]
+ args = ["vgchange", "-a", "y", vg_name]
try:
lvm(args)
@@ -376,9 +379,7 @@ def vgactivate(vg_name):
raise LVMError("vgactivate failed for %s: %s" % (vg_name, msg))
def vgdeactivate(vg_name):
- args = ["vgchange", "-a", "n"] + \
- _getConfigArgs() + \
- [vg_name]
+ args = ["vgchange", "-a", "n", vg_name]
try:
lvm(args)
@@ -400,7 +401,6 @@ def vgreduce(vg_name, pv, missing=False):
it from the VG. You must do that first by calling :func:`.pvmove`.
"""
args = ["vgreduce"]
- args.extend(_getConfigArgs())
if missing:
args.extend(["--removemissing", "--force", vg_name])
else:
@@ -417,7 +417,7 @@ def vgextend(vg_name, pv):
:param str vg_name: the name of the VG
:param str pv: device path of PV to add
"""
- args = ["vgextend"] + _getConfigArgs() + [vg_name, pv]
+ args = ["vgextend", vg_name, pv]
try:
lvm(args)
@@ -432,10 +432,10 @@ def vginfo(vg_name):
"""
args = ["vgs", "--noheadings", "--nosuffix", "--nameprefixes", "--unquoted",
"--units", "m",
- "-o", "uuid,size,free,extent_size,extent_count,free_count,pv_count"] + \
- _getConfigArgs(read_only_locking=True) + [vg_name]
+ "-o", "uuid,size,free,extent_size,extent_count,free_count,pv_count",
+ vg_name]
- buf = util.capture_output(["lvm"] + args)
+ buf = lvm(args, capture=True, ignore_errors=True)
info = parse_lvm_vars(buf)
if len(info.keys()) != 7:
raise LVMError(_("vginfo failed for %s") % vg_name)
@@ -454,12 +454,11 @@ def lvs(vg_name=None):
args = ["lvs",
"-a", "--unit", "k", "--nosuffix", "--nameprefixes",
"--unquoted", "--noheadings",
- "-ovg_name,lv_name,lv_uuid,lv_size,lv_attr,segtype"] + \
- _getConfigArgs(read_only_locking=True)
+ "-ovg_name,lv_name,lv_uuid,lv_size,lv_attr,segtype"]
if vg_name:
args.append(vg_name)
- buf = util.capture_output(["lvm"] + args)
+ buf = lvm(args, capture=True, ignore_errors=True)
logvols = {}
for line in buf.splitlines():
info = parse_lvm_vars(line)
@@ -474,10 +473,9 @@ def lvs(vg_name=None):
def lvorigin(vg_name, lv_name):
args = ["lvs", "--noheadings", "-o", "origin"] + \
- _getConfigArgs(read_only_locking=True) + \
["%s/%s" % (vg_name, lv_name)]
- buf = util.capture_output(["lvm"] + args)
+ buf = lvm(args, capture=True, ignore_errors=True)
lines = strip_lvm_warnings(buf)
try:
origin = lines[0].strip()
@@ -493,7 +491,6 @@ def lvcreate(vg_name, lv_name, size, pvs=None):
["-L", "%dm" % size.convertTo(spec="mib")] + \
["-n", lv_name] + \
["-y"] + \
- _getConfigArgs() + \
[vg_name] + pvs
try:
@@ -506,7 +503,7 @@ def lvremove(vg_name, lv_name, force=False):
if force:
args.extend(["--force", "--yes"])
- args += _getConfigArgs() + ["%s/%s" % (vg_name, lv_name)]
+ args += ["%s/%s" % (vg_name, lv_name)]
try:
lvm(args)
@@ -516,7 +513,6 @@ def lvremove(vg_name, lv_name, force=False):
def lvresize(vg_name, lv_name, size):
args = ["lvresize"] + \
["--force", "-L", "%dm" % size.convertTo(spec="mib")] + \
- _getConfigArgs() + \
["%s/%s" % (vg_name, lv_name)]
try:
@@ -530,7 +526,7 @@ def lvactivate(vg_name, lv_name, ignore_skip=False):
if ignore_skip:
args.append("-K")
- args += _getConfigArgs() + ["%s/%s" % (vg_name, lv_name)]
+ args += ["%s/%s" % (vg_name, lv_name)]
try:
lvm(args)
@@ -539,7 +535,6 @@ def lvactivate(vg_name, lv_name, ignore_skip=False):
def lvdeactivate(vg_name, lv_name):
args = ["lvchange", "-a", "n"] + \
- _getConfigArgs() + \
["%s/%s" % (vg_name, lv_name)]
try:
@@ -556,8 +551,7 @@ def lvsnapshotmerge(vg_name, lv_name):
how merge is handled by lvm.
"""
- args = ["lvconvert", "--merge", "%s/%s" % (vg_name, lv_name)]
- args += _getConfigArgs() + [lv_name]
+ args = ["lvconvert", "--merge", "%s/%s" % (vg_name, lv_name), lv_name]
try:
lvm(args)
@@ -572,7 +566,7 @@ def lvsnapshotcreate(vg_name, snap_name, size, origin_name):
:param str origin_name: the name of the origin logical volume
"""
args = ["lvcreate", "-s", "-L", "%dm" % size.convertTo(spec="MiB"),
- "-n", snap_name] + _getConfigArgs() + ["%s/%s" % (vg_name, origin_name)]
+ "-n", snap_name, "%s/%s" % (vg_name, origin_name)]
try:
lvm(args)
@@ -594,8 +588,6 @@ def thinpoolcreate(vg_name, lv_name, size, metadatasize=None, chunksize=None, pr
if profile:
args += ["--profile=%s" % profile]
- args += _getConfigArgs()
-
try:
lvm(args)
except LVMError as msg:
@@ -604,8 +596,7 @@ def thinpoolcreate(vg_name, lv_name, size, metadatasize=None, chunksize=None, pr
def thinlvcreate(vg_name, pool_name, lv_name, size):
args = ["lvcreate", "--thinpool", "%s/%s" % (vg_name, pool_name),
"--virtualsize", "%dm" % size.convertTo(spec="MiB"),
- "-n", lv_name] + \
- _getConfigArgs()
+ "-n", lv_name]
try:
lvm(args)
@@ -629,10 +620,9 @@ def thinsnapshotcreate(vg_name, snap_name, origin_name, pool_name=None):
def thinlvpoolname(vg_name, lv_name):
args = ["lvs", "--noheadings", "-o", "pool_lv"] + \
- _getConfigArgs(read_only_locking=True) + \
["%s/%s" % (vg_name, lv_name)]
- buf = util.capture_output(["lvm"] + args)
+ buf = lvm(args, capture=True, ignore_errors=True)
lines = strip_lvm_warnings(buf)
try:
pool = lines[0].strip()
--
To view this commit on github, visit https://github.com/rhinstaller/blivet/commit/fe08b0c48e3a2360b11f7aac17020e8756e1755a
More information about the anaconda-patches
mailing list