openstack/cinder
Code Issues Proposed changes

Add LC_ALL=C to lvcreate, lvextend and pvresize

Browse Source 
In various locales decimal mark is considered to be ',' instead of '.'.
This creates a problem when Cinder executes lvcreate, lvextend or
pvresize because it always passes numerals in 1.00g form and these
commands expect 1,00g when started with LC_NUMERIC set to one of such
locales. This commit adds LC_ALL=C env variable to all executions of
lvcreate, lvextends and pvresize and updates rootwrap's volume.filters
accordingly to make sure that parameters are interpreted in a correct
way.

Depends-On: Ie25194997b94664ec14a0f94d09c167f4fad3b4d
Change-Id: Ice7ae67f649f75cbbf2702f5f732d489ebe08aa8
Closes-Bug: 1488433
This commit is contained in:
Michal Dulko 2015-08-25 12:12:57 +02:00 committed by Michał Dulko
parent 7da1684f3c
commit 81645a9ca6
5 changed files with 62 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -507,7 +507,8 @@ class LVM(executor.Executor):
if not size_str: if not size_str:
size_str = self._calculate_thin_pool_size() size_str = self._calculate_thin_pool_size()
cmd = ['lvcreate', '-T', '-L', size_str, vg_pool_name] cmd = LVM.LVM_CMD_PREFIX + ['lvcreate', '-T', '-L', size_str,
vg_pool_name]
LOG.debug("Creating thin pool '%(pool)s' with size %(size)s of " LOG.debug("Creating thin pool '%(pool)s' with size %(size)s of "
"total %(free)sg", {'pool': vg_pool_name, "total %(free)sg", {'pool': vg_pool_name,
'size': size_str, 'size': size_str,
@ -532,9 +533,11 @@ class LVM(executor.Executor):
if lv_type == 'thin': if lv_type == 'thin':
pool_path = '%s/%s' % (self.vg_name, self.vg_thin_pool) pool_path = '%s/%s' % (self.vg_name, self.vg_thin_pool)
cmd = ['lvcreate', '-T', '-V', size_str, '-n', name, pool_path] cmd = LVM.LVM_CMD_PREFIX + ['lvcreate', '-T', '-V', size_str, '-n',
name, pool_path]
else: else:
cmd = ['lvcreate', '-n', name, self.vg_name, '-L', size_str] cmd = LVM.LVM_CMD_PREFIX + ['lvcreate', '-n', name, self.vg_name,
'-L', size_str]
if mirror_count > 0: if mirror_count > 0:
cmd.extend(['-m', mirror_count, '--nosync', cmd.extend(['-m', mirror_count, '--nosync',
@ -571,8 +574,8 @@ class LVM(executor.Executor):
LOG.error(_LE("Trying to create snapshot by non-existent LV: %s"), LOG.error(_LE("Trying to create snapshot by non-existent LV: %s"),
source_lv_name) source_lv_name)
raise exception.VolumeDeviceNotFound(device=source_lv_name) raise exception.VolumeDeviceNotFound(device=source_lv_name)
cmd = ['lvcreate', '--name', name, cmd = LVM.LVM_CMD_PREFIX + ['lvcreate', '--name', name, '--snapshot',
'--snapshot', '%s/%s' % (self.vg_name, source_lv_name)] '%s/%s' % (self.vg_name, source_lv_name)]
if lv_type != 'thin': if lv_type != 'thin':
size = source_lvref['size'] size = source_lvref['size']
cmd.extend(['-L', '%sg' % (size)]) cmd.extend(['-L', '%sg' % (size)])
@ -730,9 +733,9 @@ class LVM(executor.Executor):
if self.lv_has_snapshot(lv_name): if self.lv_has_snapshot(lv_name):
self.deactivate_lv(lv_name) self.deactivate_lv(lv_name)
try: try:
self._execute('lvextend', '-L', new_size, cmd = LVM.LVM_CMD_PREFIX + ['lvextend', '-L', new_size,
'%s/%s' % (self.vg_name, lv_name), '%s/%s' % (self.vg_name, lv_name)]
root_helper=self._root_helper, self._execute(*cmd, root_helper=self._root_helper,
run_as_root=True) run_as_root=True)
except putils.ProcessExecutionError as err: except putils.ProcessExecutionError as err:
LOG.exception(_LE('Error extending Volume')) LOG.exception(_LE('Error extending Volume'))

54
cinder/tests/unit/test_srb.py
View File

@ -87,6 +87,8 @@ class SRBLvmTestCase(test_brick_lvm.BrickLvmTestCase):
with mock.patch.object(self.vg, '_execute') as executor: with mock.patch.object(self.vg, '_execute') as executor:
self.vg.pv_resize('fake-pv', '50G') self.vg.pv_resize('fake-pv', '50G')
executor.assert_called_once_with( executor.assert_called_once_with(
'env',
'LC_ALL=C',
'pvresize', 'pvresize',
'--setphysicalvolumesize', '--setphysicalvolumesize',
'50G', 'fake-pv', '50G', 'fake-pv',
@ -116,7 +118,7 @@ class SRBLvmTestCase(test_brick_lvm.BrickLvmTestCase):
executor = mock.MagicMock() executor = mock.MagicMock()
self.thin_vg._execute = executor self.thin_vg._execute = executor
self.thin_vg.extend_thin_pool() self.thin_vg.extend_thin_pool()
executor.assert_called_once_with('lvextend', executor.assert_called_once_with('env', 'LC_ALL=C', 'lvextend',
'-L', '9.5g', '-L', '9.5g',
'fake-vg/fake-vg-pool', 'fake-vg/fake-vg-pool',
root_helper=self.vg._root_helper, root_helper=self.vg._root_helper,
@ -532,14 +534,14 @@ class SRBDriverTestCase(test.TestCase):
return 'lvcreate, -T, -L, ' in cmd_string return 'lvcreate, -T, -L, ' in cmd_string
def act(cmd): def act(cmd):
vgname = cmd[4].split('/')[0] vgname = cmd[6].split('/')[0]
lvname = cmd[4].split('/')[1] lvname = cmd[6].split('/')[1]
if cmd[3][-1] == 'g': if cmd[5][-1] == 'g':
lv_size = int(float(cmd[3][0:-1]) * units.Gi) lv_size = int(float(cmd[5][0:-1]) * units.Gi)
elif cmd[3][-1] == 'B': elif cmd[5][-1] == 'B':
lv_size = int(cmd[3][0:-1]) lv_size = int(cmd[5][0:-1])
else: else:
lv_size = int(cmd[3]) lv_size = int(cmd[5])
self._volumes[vgname]['vgs'][vgname]['lvs'][lvname] = lv_size self._volumes[vgname]['vgs'][vgname]['lvs'][lvname] = lv_size
return check, act return check, act
@ -551,19 +553,19 @@ class SRBDriverTestCase(test.TestCase):
def act(cmd): def act(cmd):
cmd_string = ', '.join(cmd) cmd_string = ', '.join(cmd)
vgname = cmd[6].split('/')[0] vgname = cmd[8].split('/')[0]
poolname = cmd[6].split('/')[1] poolname = cmd[8].split('/')[1]
lvname = cmd[5] lvname = cmd[7]
if poolname not in self._volumes[vgname]['vgs'][vgname]['lvs']: if poolname not in self._volumes[vgname]['vgs'][vgname]['lvs']:
raise AssertionError('thin-lv creation attempted before ' raise AssertionError('thin-lv creation attempted before '
'thin-pool creation: %s' 'thin-pool creation: %s'
% cmd_string) % cmd_string)
if cmd[3][-1] == 'g': if cmd[5][-1] == 'g':
lv_size = int(float(cmd[3][0:-1]) * units.Gi) lv_size = int(float(cmd[5][0:-1]) * units.Gi)
elif cmd[3][-1] == 'B': elif cmd[5][-1] == 'B':
lv_size = int(cmd[3][0:-1]) lv_size = int(cmd[5][0:-1])
else: else:
lv_size = int(cmd[3]) lv_size = int(cmd[5])
self._volumes[vgname]['vgs'][vgname]['lvs'][lvname] = lv_size self._volumes[vgname]['vgs'][vgname]['lvs'][lvname] = lv_size
return check, act return check, act
@ -575,9 +577,9 @@ class SRBDriverTestCase(test.TestCase):
def act(cmd): def act(cmd):
cmd_string = ', '.join(cmd) cmd_string = ', '.join(cmd)
vgname = cmd[4].split('/')[0] vgname = cmd[6].split('/')[0]
lvname = cmd[4].split('/')[1] lvname = cmd[6].split('/')[1]
snapname = cmd[2] snapname = cmd[4]
if lvname not in self._volumes[vgname]['vgs'][vgname]['lvs']: if lvname not in self._volumes[vgname]['vgs'][vgname]['lvs']:
raise AssertionError('snap creation attempted on non-existent ' raise AssertionError('snap creation attempted on non-existent '
'thin-lv: %s' % cmd_string) 'thin-lv: %s' % cmd_string)
@ -648,14 +650,14 @@ class SRBDriverTestCase(test.TestCase):
def act(cmd): def act(cmd):
cmd_string = ', '.join(cmd) cmd_string = ', '.join(cmd)
vgname = cmd[3].split('/')[0] vgname = cmd[5].split('/')[0]
lvname = cmd[3].split('/')[1] lvname = cmd[5].split('/')[1]
if cmd[2][-1] == 'g': if cmd[4][-1] == 'g':
size = int(float(cmd[2][0:-1]) * units.Gi) size = int(float(cmd[4][0:-1]) * units.Gi)
elif cmd[2][-1] == 'B': elif cmd[4][-1] == 'B':
size = int(cmd[2][0:-1]) size = int(cmd[4][0:-1])
else: else:
size = int(cmd[2]) size = int(cmd[4])
if vgname not in self._volumes: if vgname not in self._volumes:
raise AssertionError('Cannot extend inexistant volume' raise AssertionError('Cannot extend inexistant volume'
': %s' % cmd_string) ': %s' % cmd_string)

15
cinder/volume/drivers/srb.py
View File

@ -177,10 +177,10 @@ class LVM(lvm.LVM):
:raises: putils.ProcessExecutionError :raises: putils.ProcessExecutionError
""" """
try: try:
self._execute('pvresize', cmd = lvm.LVM.LVM_CMD_PREFIX + ['pvresize',
'--setphysicalvolumesize', new_size_str, '--setphysicalvolumesize',
pv_name, new_size_str, pv_name]
root_helper=self._root_helper, self._execute(*cmd, root_helper=self._root_helper,
run_as_root=True) run_as_root=True)
except putils.ProcessExecutionError as err: except putils.ProcessExecutionError as err:
LOG.exception(_LE('Error resizing Physical Volume')) LOG.exception(_LE('Error resizing Physical Volume'))
@ -203,9 +203,10 @@ class LVM(lvm.LVM):
new_size_str = self._calculate_thin_pool_size() new_size_str = self._calculate_thin_pool_size()
try: try:
self._execute('lvextend', cmd = lvm.LVM.LVM_CMD_PREFIX + ['lvextend', '-L', new_size_str,
'-L', new_size_str, "%s/%s-pool" % (self.vg_name,
"%s/%s-pool" % (self.vg_name, self.vg_name), self.vg_name)]
self._execute(*cmd,
root_helper=self._root_helper, root_helper=self._root_helper,
run_as_root=True) run_as_root=True)
except putils.ProcessExecutionError as err: except putils.ProcessExecutionError as err:

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

@ -29,7 +29,8 @@ lvdisplay_lvmconf: EnvFilter, env, root, LVM_SYSTEM_DIR=, LC_ALL=C, lvdisplay
scsi_id: CommandFilter, /lib/udev/scsi_id, root scsi_id: CommandFilter, /lib/udev/scsi_id, root
# cinder/volumes/drivers/srb.py: 'pvresize', '--setphysicalvolumesize', sizestr, pvname # cinder/volumes/drivers/srb.py: 'pvresize', '--setphysicalvolumesize', sizestr, pvname
pvresize: CommandFilter, pvresize, root pvresize: EnvFilter, env, root, LC_ALL=C, pvresize
pvresize_lvmconf: EnvFilter, env, root, LVM_SYSTEM_DIR=, LC_ALL=C, pvresize
# cinder/brick/local_dev/lvm.py: 'vgcreate', vg_name, pv_list # cinder/brick/local_dev/lvm.py: 'vgcreate', vg_name, pv_list
vgcreate: CommandFilter, vgcreate, root vgcreate: CommandFilter, vgcreate, root
@ -41,9 +42,10 @@ vgremove: CommandFilter, vgremove, root
# cinder/volumes/drivers/srb.py: 'vgchange', '-ay', vgname # cinder/volumes/drivers/srb.py: 'vgchange', '-ay', vgname
vgchange: CommandFilter, vgchange, root vgchange: CommandFilter, vgchange, root
# cinder/volume/driver.py: 'lvcreate', '-L', sizestr, '-n', volume_name,.. # cinder/brick/local_dev/lvm.py: 'lvcreate', '-L', sizestr, '-n', volume_name,..
# cinder/volume/driver.py: 'lvcreate', '-L', ... # cinder/brick/local_dev/lvm.py: 'lvcreate', '-L', ...
lvcreate: CommandFilter, lvcreate, root lvcreate: EnvFilter, env, root, LC_ALL=C, lvcreate
lvcreate_lvmconf: EnvFilter, env, root, LVM_SYSTEM_DIR=, LC_ALL=C, lvcreate
# cinder/volume/driver.py: 'dd', 'if=%s' % srcstr, 'of=%s' % deststr,... # cinder/volume/driver.py: 'dd', 'if=%s' % srcstr, 'of=%s' % deststr,...
dd: CommandFilter, dd, root dd: CommandFilter, dd, root
@ -54,9 +56,10 @@ lvremove: CommandFilter, lvremove, root
# cinder/volume/driver.py: 'lvrename', '%(vg)s', '%(orig)s' '(new)s'... # cinder/volume/driver.py: 'lvrename', '%(vg)s', '%(orig)s' '(new)s'...
lvrename: CommandFilter, lvrename, root lvrename: CommandFilter, lvrename, root
# cinder/volume/driver.py: 'lvextend', '-L' '%(new_size)s', '%(lv_name)s' ... # cinder/brick/local_dev/lvm.py: 'lvextend', '-L' '%(new_size)s', '%(lv_name)s' ...
# cinder/volume/driver.py: 'lvextend', '-L' '%(new_size)s', '%(thin_pool)s' ... # cinder/brick/local_dev/lvm.py: 'lvextend', '-L' '%(new_size)s', '%(thin_pool)s' ...
lvextend: CommandFilter, lvextend, root lvextend: EnvFilter, env, root, LC_ALL=C, lvextend
lvextend_lvmconf: EnvFilter, env, root, LVM_SYSTEM_DIR=, LC_ALL=C, lvextend
# cinder/brick/local_dev/lvm.py: 'lvchange -a y -K <lv>' # cinder/brick/local_dev/lvm.py: 'lvchange -a y -K <lv>'
lvchange: CommandFilter, lvchange, root lvchange: CommandFilter, lvchange, root

5
releasenotes/notes/1220b8a67602b8e7-update_rootwrap_volume_filters.yaml Normal file
View File

@ -0,0 +1,5 @@
---
upgrade:
- It is required to copy new rootwrap.d/volume.filters file into /etc/cinder/rootwrap.d directory.
fixes:
- Fixed bug causing snapshot creation to fail on systems with LC_NUMERIC set to locale using ',' as decimal separator.