Move shred to privsep.

The same pattern once again, this time for shred.

Change-Id: Ib6cf64d18f2ebde34030cc5b6a142af1dbf75c90
blueprint: hurrah-for-privsep

etc/nova/rootwrap.d/compute.filters

@@ -182,9 +182,6 @@ xenstore-read: CommandFilter, xenstore-read, root
 # nova/virt/libvirt/utils.py:
 rbd: CommandFilter, rbd, root
 
-# nova/virt/libvirt/utils.py: 'shred', '-n3', '-s%d' % volume_size, path
-shred: CommandFilter, shred, root
-
 # nova/virt/libvirt/volume/volume.py: 'cp', '/dev/stdin', delete_control..
 cp: CommandFilter, cp, root
 

nova/privsep/fs.py

@@ -76,3 +76,14 @@ def lvremove(path):
 @nova.privsep.sys_admin_pctxt.entrypoint
 def blockdev_size(path):
     return processutils.execute('blockdev', '--getsize64', path)
+
+
+@nova.privsep.sys_admin_pctxt.entrypoint
+def clear(path, volume_size, shred=False):
+    cmd = ['shred']
+    if shred:
+        cmd.extend(['-n3'])
+    else:
+        cmd.extend(['-n0', '-z'])
+    cmd.extend(['-s%d' % volume_size, path])
+    processutils.execute(*cmd)

nova/tests/unit/virt/libvirt/storage/test_lvm.py

@@ -16,6 +16,7 @@
 import mock
 from oslo_concurrency import processutils
 from oslo_config import cfg
+from oslo_utils import units
 
 from nova import exception
 from nova import test
@@ -56,8 +57,8 @@ class LvmTestCase(test.NoDBTestCase):
         self.assertRaises(processutils.ProcessExecutionError,
                           lvm.get_volume_size, '/dev/foo')
 
-    @mock.patch('nova.utils.execute')
-    def test_lvm_clear(self, mock_execute):
+    @mock.patch('nova.privsep.fs.clear')
+    def test_lvm_clear(self, mock_clear):
         def fake_lvm_size(path):
             return lvm_size
 
@@ -65,53 +66,49 @@ class LvmTestCase(test.NoDBTestCase):
                       fake_lvm_size)
 
         # Test zeroing volumes works
+        CONF.set_override('volume_clear', 'zero', 'libvirt')
         lvm_size = 1024
         lvm.clear_volume('/dev/v1')
-        mock_execute.assert_has_calls(
-            [mock.call('shred', '-n0', '-z', '-s1024', '/dev/v1',
-                       run_as_root=True)])
-        mock_execute.reset_mock()
+        mock_clear.assert_has_calls([
+            mock.call('/dev/v1', 1024, shred=False)])
+        mock_clear.reset_mock()
 
         # Test volume_clear_size limits the size
         lvm_size = 10485761
         CONF.set_override('volume_clear_size', '1', 'libvirt')
         lvm.clear_volume('/dev/v7')
-        mock_execute.assert_has_calls(
-            [mock.call('shred', '-n0', '-z', '-s1048576', '/dev/v7',
-                       run_as_root=True)])
-        mock_execute.reset_mock()
+        mock_clear.assert_has_calls(
+            [mock.call('/dev/v7', 1048576, shred=False)])
+        mock_clear.reset_mock()
 
         CONF.set_override('volume_clear_size', '2', 'libvirt')
         lvm_size = 1048576
         lvm.clear_volume('/dev/v9')
-        mock_execute.assert_has_calls(
-            [mock.call('shred', '-n0', '-z', '-s1048576', '/dev/v9',
-                       run_as_root=True)])
-        mock_execute.reset_mock()
+        mock_clear.assert_has_calls(
+            [mock.call('/dev/v9', 1048576, shred=False)])
+        mock_clear.reset_mock()
 
         # Test volume_clear=shred
         CONF.set_override('volume_clear', 'shred', 'libvirt')
         CONF.set_override('volume_clear_size', '0', 'libvirt')
         lvm_size = 1048576
         lvm.clear_volume('/dev/va')
-        mock_execute.assert_has_calls(
-            [mock.call('shred', '-n3', '-s1048576', '/dev/va',
-                       run_as_root=True)])
-        mock_execute.reset_mock()
+        mock_clear.assert_has_calls([
+            mock.call('/dev/va', 1048576, shred=True)])
+        mock_clear.reset_mock()
 
         CONF.set_override('volume_clear', 'shred', 'libvirt')
         CONF.set_override('volume_clear_size', '1', 'libvirt')
         lvm_size = 10485761
         lvm.clear_volume('/dev/vb')
-        mock_execute.assert_has_calls(
-            [mock.call('shred', '-n3', '-s1048576', '/dev/vb',
-                       run_as_root=True)])
-        mock_execute.reset_mock()
+        mock_clear.assert_has_calls([
+            mock.call('/dev/vb', 1 * units.Mi, shred=True)])
+        mock_clear.reset_mock()
 
         # Test volume_clear=none does nothing
         CONF.set_override('volume_clear', 'none', 'libvirt')
         lvm.clear_volume('/dev/vc')
-        mock_execute.assert_not_called()
+        mock_clear.assert_not_called()
 
     @mock.patch('nova.privsep.fs.blockdev_size',
                 side_effect=processutils.ProcessExecutionError(

nova/virt/libvirt/storage/lvm.py

@@ -30,7 +30,6 @@ import nova.conf
 from nova import exception
 from nova.i18n import _
 import nova.privsep.fs
-from nova import utils
 
 CONF = nova.conf.CONF
 LOG = logging.getLogger(__name__)
@@ -161,9 +160,7 @@ def clear_volume(path):
 
     :param path: logical volume path
     """
-    volume_clear = CONF.libvirt.volume_clear
-
-    if volume_clear == 'none':
+    if CONF.libvirt.volume_clear == 'none':
         return
 
     volume_clear_size = int(CONF.libvirt.volume_clear_size) * units.Mi
@@ -171,19 +168,14 @@ def clear_volume(path):
     try:
         volume_size = get_volume_size(path)
     except exception.VolumeBDMPathNotFound:
-        LOG.warning('ignoring missing logical volume %(path)s', {'path': path})
+        LOG.warning('Ignoring missing logical volume %(path)s', {'path': path})
         return
 
     if volume_clear_size != 0 and volume_clear_size < volume_size:
         volume_size = volume_clear_size
 
-    cmd = ['shred']
-    if volume_clear == 'zero':
-        cmd.extend(['-n0', '-z'])
-    else:
-        cmd.extend(['-n3'])
-    cmd.extend(['-s%d' % volume_size, path])
-    utils.execute(*cmd, run_as_root=True)
+    nova.privsep.fs.clear(path, volume_size,
+                          shred=(CONF.libvirt.volume_clear == 'shred'))
 
 
 def remove_volumes(paths):

releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml

@@ -8,4 +8,4 @@ upgrade:
   - |
     The following commands are no longer required to be listed in your rootwrap
     configuration: cat; chown; cryptsetup; dd; lvcreate; lvremove; lvs; mkdir;
-    mount; ploop; prl_disk_tool; readlink; tee; touch; umount; and vgs.
+    mount; ploop; prl_disk_tool; readlink; shred; tee; touch; umount; and vgs.