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

diff --git a/cinder/brick/local_dev/lvm.py b/cinder/brick/local_dev/lvm.py
index b4d832e5589aafeddbeab4f755add9406a75637b..6b5c0a7b575058e1a53eaba2fa87dc5613cd43bf 100644 (file)
--- 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 d790dae5fbccd4a7e8abbada5404ea61898f2ec6..ddfeb42cbac34902b3d9a26bce51f3c018321e2b 100644 (file)
--- 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 e6a534b2779ee20967518ed7484d5dfbeca6a40e..f535bb20463bd610f7b5144d03f5acc408bfc249 100644 (file)
--- 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 c4187eaec4e922c08d8782c6c3cc0dfbb8f3ccaf..5ba8e506fbcb7f53007619112e1fbf10a8c28638 100644 (file)
--- 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 <lv>'
 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 (file)
index 0000000..e5d2bb7
--- /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.