From 81645a9ca68ad7ec4a5986925b835d28df078b4c Mon Sep 17 00:00:00 2001 From: Michal Dulko Date: Tue, 25 Aug 2015 12:12:57 +0200 Subject: [PATCH] Add LC_ALL=C to lvcreate, lvextend and pvresize 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 --- cinder/brick/local_dev/lvm.py | 19 ++++--- cinder/tests/unit/test_srb.py | 54 ++++++++++--------- cinder/volume/drivers/srb.py | 15 +++--- etc/cinder/rootwrap.d/volume.filters | 17 +++--- ...02b8e7-update_rootwrap_volume_filters.yaml | 5 ++ 5 files changed, 62 insertions(+), 48 deletions(-) create mode 100644 releasenotes/notes/1220b8a67602b8e7-update_rootwrap_volume_filters.yaml diff --git a/cinder/brick/local_dev/lvm.py b/cinder/brick/local_dev/lvm.py index b4d832e55..6b5c0a7b5 100644 --- a/cinder/brick/local_dev/lvm.py +++ b/cinder/brick/local_dev/lvm.py @@ -507,7 +507,8 @@ class LVM(executor.Executor): if not size_str: 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 " "total %(free)sg", {'pool': vg_pool_name, 'size': size_str, @@ -532,9 +533,11 @@ class LVM(executor.Executor): if lv_type == 'thin': 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: - 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: 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"), source_lv_name) raise exception.VolumeDeviceNotFound(device=source_lv_name) - cmd = ['lvcreate', '--name', name, - '--snapshot', '%s/%s' % (self.vg_name, source_lv_name)] + cmd = LVM.LVM_CMD_PREFIX + ['lvcreate', '--name', name, '--snapshot', + '%s/%s' % (self.vg_name, source_lv_name)] if lv_type != 'thin': size = source_lvref['size'] cmd.extend(['-L', '%sg' % (size)]) @@ -730,9 +733,9 @@ class LVM(executor.Executor): if self.lv_has_snapshot(lv_name): self.deactivate_lv(lv_name) try: - self._execute('lvextend', '-L', new_size, - '%s/%s' % (self.vg_name, lv_name), - root_helper=self._root_helper, + cmd = LVM.LVM_CMD_PREFIX + ['lvextend', '-L', new_size, + '%s/%s' % (self.vg_name, lv_name)] + self._execute(*cmd, root_helper=self._root_helper, run_as_root=True) except putils.ProcessExecutionError as err: LOG.exception(_LE('Error extending Volume')) diff --git a/cinder/tests/unit/test_srb.py b/cinder/tests/unit/test_srb.py index d790dae5f..ddfeb42cb 100644 --- a/cinder/tests/unit/test_srb.py +++ b/cinder/tests/unit/test_srb.py @@ -87,6 +87,8 @@ class SRBLvmTestCase(test_brick_lvm.BrickLvmTestCase): with mock.patch.object(self.vg, '_execute') as executor: self.vg.pv_resize('fake-pv', '50G') executor.assert_called_once_with( + 'env', + 'LC_ALL=C', 'pvresize', '--setphysicalvolumesize', '50G', 'fake-pv', @@ -116,7 +118,7 @@ class SRBLvmTestCase(test_brick_lvm.BrickLvmTestCase): executor = mock.MagicMock() self.thin_vg._execute = executor 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', 'fake-vg/fake-vg-pool', root_helper=self.vg._root_helper, @@ -532,14 +534,14 @@ class SRBDriverTestCase(test.TestCase): return 'lvcreate, -T, -L, ' in cmd_string def act(cmd): - vgname = cmd[4].split('/')[0] - lvname = cmd[4].split('/')[1] - if cmd[3][-1] == 'g': - lv_size = int(float(cmd[3][0:-1]) * units.Gi) - elif cmd[3][-1] == 'B': - lv_size = int(cmd[3][0:-1]) + vgname = cmd[6].split('/')[0] + lvname = cmd[6].split('/')[1] + if cmd[5][-1] == 'g': + lv_size = int(float(cmd[5][0:-1]) * units.Gi) + elif cmd[5][-1] == 'B': + lv_size = int(cmd[5][0:-1]) else: - lv_size = int(cmd[3]) + lv_size = int(cmd[5]) self._volumes[vgname]['vgs'][vgname]['lvs'][lvname] = lv_size return check, act @@ -551,19 +553,19 @@ class SRBDriverTestCase(test.TestCase): def act(cmd): cmd_string = ', '.join(cmd) - vgname = cmd[6].split('/')[0] - poolname = cmd[6].split('/')[1] - lvname = cmd[5] + vgname = cmd[8].split('/')[0] + poolname = cmd[8].split('/')[1] + lvname = cmd[7] if poolname not in self._volumes[vgname]['vgs'][vgname]['lvs']: raise AssertionError('thin-lv creation attempted before ' 'thin-pool creation: %s' % cmd_string) - if cmd[3][-1] == 'g': - lv_size = int(float(cmd[3][0:-1]) * units.Gi) - elif cmd[3][-1] == 'B': - lv_size = int(cmd[3][0:-1]) + if cmd[5][-1] == 'g': + lv_size = int(float(cmd[5][0:-1]) * units.Gi) + elif cmd[5][-1] == 'B': + lv_size = int(cmd[5][0:-1]) else: - lv_size = int(cmd[3]) + lv_size = int(cmd[5]) self._volumes[vgname]['vgs'][vgname]['lvs'][lvname] = lv_size return check, act @@ -575,9 +577,9 @@ class SRBDriverTestCase(test.TestCase): def act(cmd): cmd_string = ', '.join(cmd) - vgname = cmd[4].split('/')[0] - lvname = cmd[4].split('/')[1] - snapname = cmd[2] + vgname = cmd[6].split('/')[0] + lvname = cmd[6].split('/')[1] + snapname = cmd[4] if lvname not in self._volumes[vgname]['vgs'][vgname]['lvs']: raise AssertionError('snap creation attempted on non-existent ' 'thin-lv: %s' % cmd_string) @@ -648,14 +650,14 @@ class SRBDriverTestCase(test.TestCase): def act(cmd): cmd_string = ', '.join(cmd) - vgname = cmd[3].split('/')[0] - lvname = cmd[3].split('/')[1] - if cmd[2][-1] == 'g': - size = int(float(cmd[2][0:-1]) * units.Gi) - elif cmd[2][-1] == 'B': - size = int(cmd[2][0:-1]) + vgname = cmd[5].split('/')[0] + lvname = cmd[5].split('/')[1] + if cmd[4][-1] == 'g': + size = int(float(cmd[4][0:-1]) * units.Gi) + elif cmd[4][-1] == 'B': + size = int(cmd[4][0:-1]) else: - size = int(cmd[2]) + size = int(cmd[4]) if vgname not in self._volumes: raise AssertionError('Cannot extend inexistant volume' ': %s' % cmd_string) diff --git a/cinder/volume/drivers/srb.py b/cinder/volume/drivers/srb.py index e6a534b27..f535bb204 100644 --- a/cinder/volume/drivers/srb.py +++ b/cinder/volume/drivers/srb.py @@ -177,10 +177,10 @@ class LVM(lvm.LVM): :raises: putils.ProcessExecutionError """ try: - self._execute('pvresize', - '--setphysicalvolumesize', new_size_str, - pv_name, - root_helper=self._root_helper, + cmd = lvm.LVM.LVM_CMD_PREFIX + ['pvresize', + '--setphysicalvolumesize', + new_size_str, pv_name] + self._execute(*cmd, root_helper=self._root_helper, run_as_root=True) except putils.ProcessExecutionError as err: LOG.exception(_LE('Error resizing Physical Volume')) @@ -203,9 +203,10 @@ class LVM(lvm.LVM): new_size_str = self._calculate_thin_pool_size() try: - self._execute('lvextend', - '-L', new_size_str, - "%s/%s-pool" % (self.vg_name, self.vg_name), + cmd = lvm.LVM.LVM_CMD_PREFIX + ['lvextend', '-L', new_size_str, + "%s/%s-pool" % (self.vg_name, + self.vg_name)] + self._execute(*cmd, root_helper=self._root_helper, run_as_root=True) except putils.ProcessExecutionError as err: diff --git a/etc/cinder/rootwrap.d/volume.filters b/etc/cinder/rootwrap.d/volume.filters index c4187eaec..5ba8e506f 100644 --- a/etc/cinder/rootwrap.d/volume.filters +++ b/etc/cinder/rootwrap.d/volume.filters @@ -29,7 +29,8 @@ lvdisplay_lvmconf: EnvFilter, env, root, LVM_SYSTEM_DIR=, LC_ALL=C, lvdisplay scsi_id: CommandFilter, /lib/udev/scsi_id, root # 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 vgcreate: CommandFilter, vgcreate, root @@ -41,9 +42,10 @@ vgremove: CommandFilter, vgremove, root # cinder/volumes/drivers/srb.py: 'vgchange', '-ay', vgname vgchange: CommandFilter, vgchange, root -# cinder/volume/driver.py: 'lvcreate', '-L', sizestr, '-n', volume_name,.. -# cinder/volume/driver.py: 'lvcreate', '-L', ... -lvcreate: CommandFilter, lvcreate, root +# cinder/brick/local_dev/lvm.py: 'lvcreate', '-L', sizestr, '-n', volume_name,.. +# cinder/brick/local_dev/lvm.py: 'lvcreate', '-L', ... +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,... dd: CommandFilter, dd, root @@ -54,9 +56,10 @@ lvremove: CommandFilter, lvremove, root # cinder/volume/driver.py: 'lvrename', '%(vg)s', '%(orig)s' '(new)s'... lvrename: CommandFilter, lvrename, root -# cinder/volume/driver.py: 'lvextend', '-L' '%(new_size)s', '%(lv_name)s' ... -# cinder/volume/driver.py: 'lvextend', '-L' '%(new_size)s', '%(thin_pool)s' ... -lvextend: CommandFilter, lvextend, root +# cinder/brick/local_dev/lvm.py: 'lvextend', '-L' '%(new_size)s', '%(lv_name)s' ... +# cinder/brick/local_dev/lvm.py: 'lvextend', '-L' '%(new_size)s', '%(thin_pool)s' ... +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 ' lvchange: CommandFilter, lvchange, root diff --git a/releasenotes/notes/1220b8a67602b8e7-update_rootwrap_volume_filters.yaml b/releasenotes/notes/1220b8a67602b8e7-update_rootwrap_volume_filters.yaml new file mode 100644 index 000000000..e5d2bb749 --- /dev/null +++ b/releasenotes/notes/1220b8a67602b8e7-update_rootwrap_volume_filters.yaml @@ -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. -- 2.45.2