diff --git a/etc/nova/rootwrap.d/compute.filters b/etc/nova/rootwrap.d/compute.filters index 0260670caf5c..3d733acba011 100644 --- a/etc/nova/rootwrap.d/compute.filters +++ b/etc/nova/rootwrap.d/compute.filters @@ -2,10 +2,6 @@ # This file should be owned by (and only-writeable by) the root user [Filters] -# nova/virt/disk/mount/api.py: 'kpartx', '-a', device -# nova/virt/disk/mount/api.py: 'kpartx', '-d', device -kpartx: CommandFilter, kpartx, root - # nova/virt/xenapi/vm_utils.py: tune2fs, -O ^has_journal, part_path # nova/virt/xenapi/vm_utils.py: tune2fs, -j, partition_path tune2fs: CommandFilter, tune2fs, root diff --git a/nova/privsep/fs.py b/nova/privsep/fs.py index c97ed1afa4dd..89fdcf6c9a30 100644 --- a/nova/privsep/fs.py +++ b/nova/privsep/fs.py @@ -107,3 +107,13 @@ def nbd_connect(device, image): @nova.privsep.sys_admin_pctxt.entrypoint def nbd_disconnect(device): return processutils.execute('qemu-nbd', '-d', device) + + +@nova.privsep.sys_admin_pctxt.entrypoint +def create_device_maps(device): + return processutils.execute('kpartx', '-a', device) + + +@nova.privsep.sys_admin_pctxt.entrypoint +def remove_device_maps(device): + return processutils.execute('kpartx', '-d', device) diff --git a/nova/tests/unit/virt/disk/mount/test_api.py b/nova/tests/unit/virt/disk/mount/test_api.py index de86d41d1445..baaa81ae7d93 100644 --- a/nova/tests/unit/virt/disk/mount/test_api.py +++ b/nova/tests/unit/virt/disk/mount/test_api.py @@ -36,13 +36,6 @@ class MountTestCase(test.NoDBTestCase): mount.map_dev() return mount - @mock.patch('nova.utils.trycmd') - def _test_map_dev_with_trycmd(self, partition, trycmd): - trycmd.return_value = [None, None] - mount = self._test_map_dev(partition) - self.assertEqual(1, trycmd.call_count) # don't care about args - return mount - def _exists_effect(self, data): def exists_effect(filename): try: @@ -72,36 +65,42 @@ class MountTestCase(test.NoDBTestCase): self.assertFalse(mount.mapped) @mock.patch('os.path.exists') - def test_map_dev_good(self, exists): - exists.side_effect = self._exists_effect({ + @mock.patch('nova.privsep.fs.create_device_maps', + return_value=(None, None)) + def test_map_dev_good(self, mock_create_maps, mock_exists): + mock_exists.side_effect = self._exists_effect({ ORIG_DEVICE: True, AUTOMAP_PARTITION: False, MAP_PARTITION: [False, True]}) - mount = self._test_map_dev_with_trycmd(PARTITION) - self._check_calls(exists, [ORIG_DEVICE, AUTOMAP_PARTITION], 2) + mount = self._test_map_dev(PARTITION) + self._check_calls(mock_exists, [ORIG_DEVICE, AUTOMAP_PARTITION], 2) self.assertEqual("", mount.error) self.assertTrue(mount.mapped) @mock.patch('os.path.exists') - def test_map_dev_error(self, exists): - exists.side_effect = self._exists_effect({ + @mock.patch('nova.privsep.fs.create_device_maps', + return_value=(None, None)) + def test_map_dev_error(self, mock_create_maps, mock_exists): + mock_exists.side_effect = self._exists_effect({ ORIG_DEVICE: True, AUTOMAP_PARTITION: False, MAP_PARTITION: False}) - mount = self._test_map_dev_with_trycmd(PARTITION) - self._check_calls(exists, [ORIG_DEVICE, AUTOMAP_PARTITION], + mount = self._test_map_dev(PARTITION) + self._check_calls(mock_exists, [ORIG_DEVICE, AUTOMAP_PARTITION], api.MAX_FILE_CHECKS + 1) self.assertNotEqual("", mount.error) self.assertFalse(mount.mapped) @mock.patch('os.path.exists') - def test_map_dev_error_then_pass(self, exists): - exists.side_effect = self._exists_effect({ + @mock.patch('nova.privsep.fs.create_device_maps', + return_value=(None, None)) + def test_map_dev_error_then_pass(self, mock_create_maps, mock_exists): + mock_exists.side_effect = self._exists_effect({ ORIG_DEVICE: True, AUTOMAP_PARTITION: False, MAP_PARTITION: [False, False, True]}) - mount = self._test_map_dev_with_trycmd(PARTITION) - self._check_calls(exists, [ORIG_DEVICE, AUTOMAP_PARTITION], 3) + mount = self._test_map_dev(PARTITION) + self._check_calls(mock_exists, [ORIG_DEVICE, AUTOMAP_PARTITION], 3) self.assertEqual("", mount.error) self.assertTrue(mount.mapped) diff --git a/nova/tests/unit/virt/test_virt.py b/nova/tests/unit/virt/test_virt.py index 6743c0dac64f..44b273d1673c 100644 --- a/nova/tests/unit/virt/test_virt.py +++ b/nova/tests/unit/virt/test_virt.py @@ -215,9 +215,10 @@ class TestVirtDisk(test.NoDBTestCase): @mock.patch('nova.privsep.fs.loopremove') @mock.patch('nova.privsep.fs.umount') @mock.patch('nova.privsep.fs.nbd_disconnect') + @mock.patch('nova.privsep.fs.remove_device_maps') def test_lxc_teardown_container( - self, mock_nbd_disconnect, mock_umount, mock_loopremove, - mock_exist): + self, mock_remove_maps, mock_nbd_disconnect, mock_umount, + mock_loopremove, mock_exist): def proc_mounts(mount_point): mount_points = { @@ -239,13 +240,12 @@ class TestVirtDisk(test.NoDBTestCase): mock_umount.reset_mock() disk_api.teardown_container('/mnt/loop/part') - expected_commands += [ - ('kpartx', '-d', '/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() + mock_remove_maps.assert_has_calls([mock.call('/dev/loop0')]) + mock_remove_maps.reset_mock() disk_api.teardown_container('/mnt/nbd/nopart') expected_commands += [ @@ -259,12 +259,13 @@ class TestVirtDisk(test.NoDBTestCase): disk_api.teardown_container('/mnt/nbd/part') expected_commands += [ ('blockdev', '--flushbufs', '/dev/nbd15'), - ('kpartx', '-d', '/dev/nbd15'), ] mock_nbd_disconnect.assert_has_calls([mock.call('/dev/nbd15')]) mock_umount.assert_has_calls([mock.call('/dev/mapper/nbd15p1')]) mock_nbd_disconnect.reset_mock() mock_umount.reset_mock() + mock_remove_maps.assert_has_calls([mock.call('/dev/nbd15')]) + mock_remove_maps.reset_mock() # NOTE(thomasem): Not adding any commands in this case, because we're # not expecting an additional umount for LocalBlockImages. This is to diff --git a/nova/virt/disk/mount/api.py b/nova/virt/disk/mount/api.py index 8e87051eadca..97ac6d2e2fba 100644 --- a/nova/virt/disk/mount/api.py +++ b/nova/virt/disk/mount/api.py @@ -23,7 +23,6 @@ from oslo_utils import importutils from nova import exception from nova.i18n import _ import nova.privsep.fs -from nova import utils from nova.virt.image import model as imgmodel LOG = logging.getLogger(__name__) @@ -200,8 +199,7 @@ class Mount(object): # Note kpartx can output warnings to stderr and succeed # Also it can output failures to stderr and "succeed" # So we just go on the existence of the mapped device - _out, err = utils.trycmd('kpartx', '-a', self.device, - run_as_root=True, discard_warnings=True) + _out, err = nova.privsep.fs.create_device_maps(self.device) @loopingcall.RetryDecorator( max_retry_count=MAX_FILE_CHECKS - 1, @@ -241,7 +239,7 @@ class Mount(object): return LOG.debug("Unmap dev %s", self.device) if self.partition and not self.automapped: - utils.execute('kpartx', '-d', self.device, run_as_root=True) + nova.privsep.fs.remove_device_maps(self.device) self.mapped = False self.automapped = False diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index 7e1bcf8eaae3..9f7f3fd2ba94 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -956,9 +956,7 @@ def _make_partition(session, dev, partition_start, partition_end): partition_path = utils.make_dev_path(dev, partition=1) if session.is_local_connection: # Need to refresh the partitions - utils.trycmd('kpartx', '-a', dev_path, - run_as_root=True, - discard_warnings=True) + nova.privsep.fs.create_device_maps(dev_path) # Sometimes the partition gets created under /dev/mapper, depending # on the setup in dom0. diff --git a/releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml b/releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml index 3bd98ebb7ab4..4f59f1c43122 100644 --- a/releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml +++ b/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; losetup; lvcreate; lvremove; - lvs; mkdir; mount; nova-idmapshift; ploop; prl_disk_tool; qemu-nbd; - readlink; shred; tee; touch; umount; vgs; and xend. + configuration: cat; chown; cryptsetup; dd; kpartx; losetup; lvcreate; + lvremove; lvs; mkdir; mount; nova-idmapshift; ploop; prl_disk_tool; + qemu-nbd; readlink; shred; tee; touch; umount; vgs; and xend.