Move loopback setup and removal to privsep.

Once more, again.

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

etc/nova/rootwrap.d/compute.filters

@@ -14,10 +14,6 @@ tune2fs: CommandFilter, tune2fs, root
 # nova/virt/disk/mount/nbd.py: 'qemu-nbd', '-d', device
 qemu-nbd: CommandFilter, qemu-nbd, root
 
-# nova/virt/disk/mount/loop.py: 'losetup', '--find', '--show', image
-# nova/virt/disk/mount/loop.py: 'losetup', '--detach', device
-losetup: CommandFilter, losetup, root
-
 # nova/virt/disk/vfs/localfs.py: 'blkid', '-o', 'value', '-s', 'TYPE', device
 blkid: CommandFilter, blkid, root
 

nova/privsep/fs.py

@@ -87,3 +87,13 @@ def clear(path, volume_size, shred=False):
         cmd.extend(['-n0', '-z'])
     cmd.extend(['-s%d' % volume_size, path])
     processutils.execute(*cmd)
+
+
+@nova.privsep.sys_admin_pctxt.entrypoint
+def loopsetup(path):
+    return processutils.execute('losetup', '--find', '--show', path)
+
+
+@nova.privsep.sys_admin_pctxt.entrypoint
+def loopremove(device):
+    return processutils.execute('losetup', '--detach', device, attempts=3)

nova/tests/unit/virt/disk/mount/test_loop.py

@@ -15,6 +15,7 @@
 
 
 import fixtures
+import mock
 
 from nova import test
 from nova.virt.disk.mount import loop
@@ -25,14 +26,6 @@ def _fake_noop(*args, **kwargs):
     return
 
 
-def _fake_trycmd_losetup_works(*args, **kwargs):
-    return '/dev/loop0', ''
-
-
-def _fake_trycmd_losetup_fails(*args, **kwards):
-    return '', 'doh'
-
-
 class LoopTestCase(test.NoDBTestCase):
     def setUp(self):
         super(LoopTestCase, self).setUp()
@@ -40,11 +33,11 @@ class LoopTestCase(test.NoDBTestCase):
         self.file = imgmodel.LocalFileImage("/some/file.qcow2",
                                             imgmodel.FORMAT_QCOW2)
 
-    def test_get_dev(self):
+    @mock.patch('nova.privsep.fs.loopsetup', return_value=('/dev/loop0', ''))
+    @mock.patch('nova.privsep.fs.loopremove')
+    def test_get_dev(self, mock_loopremove, mock_loopsetup):
         tempdir = self.useFixture(fixtures.TempDir()).path
         l = loop.LoopMount(self.file, tempdir)
-        self.useFixture(fixtures.MonkeyPatch('nova.utils.trycmd',
-                                             _fake_trycmd_losetup_works))
         self.useFixture(fixtures.MonkeyPatch('nova.utils.execute',
                                              _fake_noop))
 
@@ -60,11 +53,10 @@ class LoopTestCase(test.NoDBTestCase):
         self.assertEqual('', l.error)
         self.assertIsNone(l.device)
 
-    def test_inner_get_dev_fails(self):
+    @mock.patch('nova.privsep.fs.loopsetup', return_value=('', 'doh'))
+    def test_inner_get_dev_fails(self, mock_loopsetup):
         tempdir = self.useFixture(fixtures.TempDir()).path
         l = loop.LoopMount(self.file, tempdir)
-        self.useFixture(fixtures.MonkeyPatch('nova.utils.trycmd',
-                                             _fake_trycmd_losetup_fails))
 
         # No error logged, device consumed
         self.assertFalse(l._inner_get_dev())
@@ -77,12 +69,11 @@ class LoopTestCase(test.NoDBTestCase):
         self.assertFalse(l.linked)
         self.assertIsNone(l.device)
 
-    def test_get_dev_timeout(self):
+    @mock.patch('nova.privsep.fs.loopsetup', return_value=('', 'doh'))
+    def test_get_dev_timeout(self, mock_loopsetup):
         tempdir = self.useFixture(fixtures.TempDir()).path
         l = loop.LoopMount(self.file, tempdir)
         self.useFixture(fixtures.MonkeyPatch('time.sleep', _fake_noop))
-        self.useFixture(fixtures.MonkeyPatch('nova.utils.trycmd',
-                                             _fake_trycmd_losetup_fails))
         self.useFixture(fixtures.MonkeyPatch(('nova.virt.disk.mount.api.'
                                               'MAX_DEVICE_WAIT'), -10))
 
@@ -94,11 +85,10 @@ class LoopTestCase(test.NoDBTestCase):
         # Fail to get a device
         self.assertFalse(l.get_dev())
 
-    def test_unget_dev(self):
+    @mock.patch('nova.privsep.fs.loopremove')
+    def test_unget_dev(self, mock_loopremove):
         tempdir = self.useFixture(fixtures.TempDir()).path
         l = loop.LoopMount(self.file, tempdir)
-        self.useFixture(fixtures.MonkeyPatch('nova.utils.execute',
-                                             _fake_noop))
 
         # This just checks that a free of something we don't have doesn't
         # throw an exception

nova/tests/unit/virt/test_virt.py

@@ -182,6 +182,8 @@ class TestDiskImage(test.NoDBTestCase):
 class TestVirtDisk(test.NoDBTestCase):
     def setUp(self):
         super(TestVirtDisk, self).setUp()
+
+        # TODO(mikal): this can probably be removed post privsep cleanup.
         self.executes = []
 
         def fake_execute(*cmd, **kwargs):
@@ -209,8 +211,11 @@ class TestVirtDisk(test.NoDBTestCase):
         self.assertEqual(disk_api.setup_container(image, container_dir),
                          '/dev/fake')
 
+    @mock.patch('os.path.exists', return_value=True)
+    @mock.patch('nova.privsep.fs.loopremove')
     @mock.patch('nova.privsep.fs.umount')
-    def test_lxc_teardown_container(self, mock_umount):
+    def test_lxc_teardown_container(
+            self, mock_umount, mock_loopremove, mock_exist):
 
         def proc_mounts(mount_point):
             mount_points = {
@@ -221,21 +226,22 @@ class TestVirtDisk(test.NoDBTestCase):
             }
             return mount_points[mount_point]
 
-        self.stub_out('os.path.exists', lambda _: True)
         self.stub_out('nova.virt.disk.api._DiskImage._device_for_path',
                       proc_mounts)
         expected_commands = []
 
         disk_api.teardown_container('/mnt/loop/nopart')
-        expected_commands += [('losetup', '--detach', '/dev/loop0')]
+        mock_loopremove.assert_has_calls([mock.call('/dev/loop0')])
+        mock_loopremove.reset_mock()
         mock_umount.assert_has_calls([mock.call('/dev/loop0')])
         mock_umount.reset_mock()
 
         disk_api.teardown_container('/mnt/loop/part')
         expected_commands += [
-                              ('kpartx', '-d', '/dev/loop0'),
+                              ('kpartx', '-d', '/dev/loop0')
-                              ('losetup', '--detach', '/dev/loop0'),
                              ]
+        mock_loopremove.assert_has_calls([mock.call('/dev/loop0')])
+        mock_loopremove.reset_mock()
         mock_umount.assert_has_calls([mock.call('/dev/mapper/loop0p1')])
         mock_umount.reset_mock()
 
@@ -264,25 +270,22 @@ class TestVirtDisk(test.NoDBTestCase):
 
         self.assertEqual(self.executes, expected_commands)
 
-    def test_lxc_teardown_container_with_namespace_cleaned(self):
+    @mock.patch('os.path.exists', return_value=True)
+    @mock.patch('nova.virt.disk.api._DiskImage._device_for_path',
+                return_value=None)
+    @mock.patch('nova.privsep.fs.loopremove')
+    def test_lxc_teardown_container_with_namespace_cleaned(
+            self, mock_loopremove, mock_device_for_path, mock_exists):
 
-        def proc_mounts(mount_point):
-            return None
-
-        self.stub_out('os.path.exists', lambda _: True)
-        self.stub_out('nova.virt.disk.api._DiskImage._device_for_path',
-                      proc_mounts)
         expected_commands = []
 
         disk_api.teardown_container('/mnt/loop/nopart', '/dev/loop0')
-        expected_commands += [
+        mock_loopremove.assert_has_calls([mock.call('/dev/loop0')])
-                              ('losetup', '--detach', '/dev/loop0'),
+        mock_loopremove.reset_mock()
-                             ]
 
         disk_api.teardown_container('/mnt/loop/part', '/dev/loop0')
-        expected_commands += [
+        mock_loopremove.assert_has_calls([mock.call('/dev/loop0')])
-                              ('losetup', '--detach', '/dev/loop0'),
+        mock_loopremove.reset_mock()
-                             ]
 
         disk_api.teardown_container('/mnt/nbd/nopart', '/dev/nbd15')
         expected_commands += [

nova/virt/disk/api.py

@@ -447,8 +447,7 @@ def teardown_container(container_dir, container_root_device=None):
         if container_root_device:
             if 'loop' in container_root_device:
                 LOG.debug("Release loop device %s", container_root_device)
-                utils.execute('losetup', '--detach', container_root_device,
+                nova.privsep.fs.loopremove(container_root_device)
-                              run_as_root=True, attempts=3)
             elif 'nbd' in container_root_device:
                 LOG.debug('Release nbd device %s', container_root_device)
                 utils.execute('qemu-nbd', '-d', container_root_device,

nova/virt/disk/mount/loop.py

@@ -16,7 +16,7 @@
 from oslo_log import log as logging
 
 from nova.i18n import _
-from nova import utils
+import nova.privsep.fs
 from nova.virt.disk.mount import api
 
 LOG = logging.getLogger(__name__)
@@ -27,9 +27,7 @@ class LoopMount(api.Mount):
     mode = 'loop'
 
     def _inner_get_dev(self):
-        out, err = utils.trycmd('losetup', '--find', '--show',
+        out, err = nova.privsep.fs.loopsetup(self.image.path)
-                                self.image.path,
-                                run_as_root=True)
         if err:
             self.error = _('Could not attach image to loopback: %s') % err
             LOG.info('Loop mount error: %s', self.error)
@@ -57,7 +55,6 @@ class LoopMount(api.Mount):
         # thus leaking a loop device unless the losetup --detach is retried:
         # https://lkml.org/lkml/2012/9/28/62
         LOG.debug("Release loop device %s", self.device)
-        utils.execute('losetup', '--detach', self.device, run_as_root=True,
+        nova.privsep.fs.loopremove(self.device)
-                      attempts=3)
         self.linked = False
         self.device = None

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

@@ -10,6 +10,6 @@ upgrade:
     internal functionality using privsep.
   - |
     The following commands are no longer required to be listed in your rootwrap
-    configuration: cat; chown; cryptsetup; dd; lvcreate; lvremove; lvs; mkdir;
+    configuration: cat; chown; cryptsetup; dd; losetup; lvcreate; lvremove;
-    mount; nova-idmapshift; ploop; prl_disk_tool; readlink; shred; tee; touch;
+    lvs; mkdir; mount; nova-idmapshift; ploop; prl_disk_tool; readlink; shred;
-    umount; vgs; and xend.
+    tee; touch; umount; vgs; and xend.