diff --git a/etc/nova/rootwrap.d/compute.filters b/etc/nova/rootwrap.d/compute.filters index ae65a0101a0f..d950034ac698 100644 --- a/etc/nova/rootwrap.d/compute.filters +++ b/etc/nova/rootwrap.d/compute.filters @@ -10,18 +10,6 @@ kpartx: CommandFilter, kpartx, root # nova/virt/xenapi/vm_utils.py: tune2fs, -j, partition_path tune2fs: CommandFilter, tune2fs, root -# nova/virt/disk/mount/api.py: 'mount', mapped_device -# nova/virt/disk/api.py: 'mount', '-o', 'bind', src, target -# nova/virt/xenapi/vm_utils.py: 'mount', '-t', 'ext2,ext3,ext4,reiserfs'.. -# nova/virt/configdrive.py: 'mount', device, mountdir -mount: CommandFilter, mount, root - -# nova/virt/disk/mount/api.py: 'umount', mapped_device -# nova/virt/disk/api.py: 'umount' target -# nova/virt/xenapi/vm_utils.py: 'umount', dev_path -# nova/virt/configdrive.py: 'umount', mountdir -umount: CommandFilter, umount, root - # nova/virt/disk/mount/nbd.py: 'qemu-nbd', '-c', device, image # nova/virt/disk/mount/nbd.py: 'qemu-nbd', '-d', device qemu-nbd: CommandFilter, qemu-nbd, root diff --git a/nova/privsep/fs.py b/nova/privsep/fs.py new file mode 100644 index 000000000000..b1b813094211 --- /dev/null +++ b/nova/privsep/fs.py @@ -0,0 +1,38 @@ +# Copyright 2016 Red Hat, Inc +# Copyright 2017 Rackspace Australia +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +""" +Helpers for filesystem related routines. +""" + +from oslo_concurrency import processutils + +import nova.privsep + + +@nova.privsep.sys_admin_pctxt.entrypoint +def mount(fstype, device, mountpoint, options): + mount_cmd = ['mount'] + if fstype: + mount_cmd.extend(['-t', fstype]) + if options is not None: + mount_cmd.extend(options) + mount_cmd.extend([device, mountpoint]) + return processutils.execute(*mount_cmd) + + +@nova.privsep.sys_admin_pctxt.entrypoint +def umount(mountpoint): + processutils.execute('umount', mountpoint, attempts=3, delay_on_retry=True) diff --git a/nova/privsep/path.py b/nova/privsep/path.py index b82a5f580d76..e2864b2faa0e 100644 --- a/nova/privsep/path.py +++ b/nova/privsep/path.py @@ -77,6 +77,13 @@ def utime(path): os.utime(path, None) +@nova.privsep.sys_admin_pctxt.entrypoint +def rmdir(path): + if not os.path.exists(path): + raise exception.FileNotFound(file_path=path) + os.rmdir(path) + + class path(object): @staticmethod @nova.privsep.sys_admin_pctxt.entrypoint diff --git a/nova/tests/unit/test_configdrive2.py b/nova/tests/unit/test_configdrive2.py index c06cfbca4228..ea1b2ef16599 100644 --- a/nova/tests/unit/test_configdrive2.py +++ b/nova/tests/unit/test_configdrive2.py @@ -65,10 +65,11 @@ class ConfigDriveTestCase(test.NoDBTestCase): fileutils.delete_if_exists(imagefile) @mock.patch.object(utils, 'mkfs', return_value=None) - @mock.patch.object(utils, 'execute', return_value=None) + @mock.patch('nova.privsep.fs.mount', return_value=('', '')) + @mock.patch('nova.privsep.fs.umount', return_value=None) @mock.patch.object(utils, 'trycmd', return_value=(None, None)) - def test_create_configdrive_vfat(self, mock_trycmd, - mock_execute, mock_mkfs): + def test_create_configdrive_vfat(self, mock_trycmd, mock_umount, + mock_mount, mock_mkfs): CONF.set_override('config_drive_format', 'vfat') imagefile = None try: @@ -79,13 +80,8 @@ class ConfigDriveTestCase(test.NoDBTestCase): mock_mkfs.assert_called_once_with('vfat', mock.ANY, label='config-2') - mock_trycmd.assert_called_once_with('mount', '-o', - mock.ANY, - mock.ANY, - mock.ANY, - run_as_root=True) - mock_execute.assert_called_once_with('umount', mock.ANY, - run_as_root=True) + mock_mount.assert_called_once() + mock_umount.assert_called_once() # NOTE(mikal): we can't check for a VFAT output here because the # filesystem creation stuff has been mocked out because it # requires root permissions diff --git a/nova/tests/unit/virt/disk/mount/test_nbd.py b/nova/tests/unit/virt/disk/mount/test_nbd.py index 104498600667..de9c6f83ec78 100644 --- a/nova/tests/unit/virt/disk/mount/test_nbd.py +++ b/nova/tests/unit/virt/disk/mount/test_nbd.py @@ -14,6 +14,7 @@ # under the License. +import mock import os import tempfile import time @@ -271,13 +272,10 @@ class NbdTestCase(test.NoDBTestCase): # No error logged, device consumed self.assertFalse(n.get_dev()) - def test_do_mount_need_to_specify_fs_type(self): + @mock.patch('nova.privsep.fs.mount', return_value=('', 'broken')) + def test_do_mount_need_to_specify_fs_type(self, mock_mount): # NOTE(mikal): Bug 1094373 saw a regression where we failed to # communicate a failed mount properly. - def fake_trycmd(*args, **kwargs): - return '', 'broken' - self.useFixture(fixtures.MonkeyPatch('nova.utils.trycmd', fake_trycmd)) - imgfile = tempfile.NamedTemporaryFile() self.addCleanup(imgfile.close) tempdir = self.useFixture(fixtures.TempDir()).path diff --git a/nova/tests/unit/virt/libvirt/volume/test_mount.py b/nova/tests/unit/virt/libvirt/volume/test_mount.py index d7cec4e4e9b4..76a3379c6b39 100644 --- a/nova/tests/unit/virt/libvirt/volume/test_mount.py +++ b/nova/tests/unit/virt/libvirt/volume/test_mount.py @@ -174,30 +174,9 @@ class HostMountStateTestCase(test.NoDBTestCase): def setUp(self): super(HostMountStateTestCase, self).setUp() - self.mounted = set() - - def fake_execute(cmd, *args, **kwargs): - if cmd == 'mount': - path = args[-1] - if path in self.mounted: - raise processutils.ProcessExecutionError('Already mounted') - self.mounted.add(path) - elif cmd == 'umount': - path = args[-1] - if path not in self.mounted: - raise processutils.ProcessExecutionError('Not mounted') - self.mounted.remove(path) - - def fake_ismount(path): - return path in self.mounted - - mock_execute = mock.MagicMock(side_effect=fake_execute) - - self.useFixture(fixtures.MonkeyPatch('nova.utils.execute', - mock_execute)) - self.useFixture(fixtures.MonkeyPatch('os.path.ismount', fake_ismount)) - - def test_init(self): + @mock.patch('os.path.ismount', + side_effect=[False, True, True, True, True]) + def test_init(self, mock_ismount): # Test that we initialise the state of MountManager correctly at # startup def fake_disk(disk): @@ -216,9 +195,6 @@ class HostMountStateTestCase(test.NoDBTestCase): mountpoint_a = '/mnt/a' mountpoint_b = '/mnt/b' - self.mounted.add(mountpoint_a) - self.mounted.add(mountpoint_b) - guests = map(mock_guest, [uuids.instance_a, uuids.instance_b], [ # Local file root disk and a volume on each of mountpoints a and b [ @@ -272,15 +248,13 @@ class HostMountStateTestCase(test.NoDBTestCase): instance=mock.sentinel.instance): m.umount(vol, mountpoint, instance) - @staticmethod - def _expected_sentinel_umount_calls(mountpoint=mock.sentinel.mountpoint): - return [mock.call('umount', mountpoint, - attempts=3, delay_on_retry=True, - run_as_root=True), - mock.call('rmdir', mountpoint)] - @mock.patch('oslo_utils.fileutils.ensure_tree') - def test_mount_umount(self, mock_ensure_tree): + @mock.patch('nova.privsep.fs.mount') + @mock.patch('nova.privsep.fs.umount') + @mock.patch('os.path.ismount', + side_effect=[False, True, True, True]) + def test_mount_umount(self, mock_ismount, mock_umount, mock_mount, + mock_ensure_tree): # Mount 2 different volumes from the same export. Test that we only # mount and umount once. m = self._get_clean_hostmountstate() @@ -289,47 +263,49 @@ class HostMountStateTestCase(test.NoDBTestCase): self._sentinel_mount(m, mock.sentinel.vol_a) mock_ensure_tree.assert_has_calls([ mock.call(mock.sentinel.mountpoint)]) - mount.utils.execute.assert_has_calls([ - mock.call('mount', '-t', mock.sentinel.fstype, - mock.sentinel.option1, mock.sentinel.option2, + mock_mount.assert_has_calls([ + mock.call(mock.sentinel.fstype, mock.sentinel.export, mock.sentinel.mountpoint, - run_as_root=True)]) + [mock.sentinel.option1, mock.sentinel.option2])]) # Mount vol_b from export. We shouldn't have mounted again self._sentinel_mount(m, mock.sentinel.vol_b) mock_ensure_tree.assert_has_calls([ mock.call(mock.sentinel.mountpoint)]) - mount.utils.execute.assert_has_calls([ - mock.call('mount', '-t', mock.sentinel.fstype, - mock.sentinel.option1, mock.sentinel.option2, + mock_mount.assert_has_calls([ + mock.call(mock.sentinel.fstype, mock.sentinel.export, mock.sentinel.mountpoint, - run_as_root=True)]) + [mock.sentinel.option1, mock.sentinel.option2])]) # Unmount vol_a. We shouldn't have unmounted self._sentinel_umount(m, mock.sentinel.vol_a) mock_ensure_tree.assert_has_calls([ mock.call(mock.sentinel.mountpoint)]) - mount.utils.execute.assert_has_calls([ - mock.call('mount', '-t', mock.sentinel.fstype, - mock.sentinel.option1, mock.sentinel.option2, + mock_mount.assert_has_calls([ + mock.call(mock.sentinel.fstype, mock.sentinel.export, mock.sentinel.mountpoint, - run_as_root=True)]) + [mock.sentinel.option1, mock.sentinel.option2])]) # Unmount vol_b. We should have umounted. self._sentinel_umount(m, mock.sentinel.vol_b) - expected_calls = [ - mock.call('mount', '-t', mock.sentinel.fstype, - mock.sentinel.option1, mock.sentinel.option2, - mock.sentinel.export, mock.sentinel.mountpoint, - run_as_root=True) - ] - expected_calls.extend(self._expected_sentinel_umount_calls()) mock_ensure_tree.assert_has_calls([ mock.call(mock.sentinel.mountpoint)]) - mount.utils.execute.assert_has_calls(expected_calls) + mock_mount.assert_has_calls([ + mock.call(mock.sentinel.fstype, + mock.sentinel.export, mock.sentinel.mountpoint, + [mock.sentinel.option1, mock.sentinel.option2])]) + mock_umount.assert_has_calls([ + mock.call(mock.sentinel.mountpoint)]) @mock.patch('oslo_utils.fileutils.ensure_tree') - def test_mount_umount_multi_attach(self, mock_ensure_tree): + @mock.patch('nova.privsep.fs.mount') + @mock.patch('nova.privsep.fs.umount') + @mock.patch('os.path.ismount', + side_effect=[False, True, True, True]) + @mock.patch('nova.privsep.path.rmdir') + def test_mount_umount_multi_attach(self, mock_rmdir, mock_ismount, + mock_umount, mock_mount, + mock_ensure_tree): # Mount a volume from a single export for 2 different instances. Test # that we only mount and umount once. m = self._get_clean_hostmountstate() @@ -341,48 +317,32 @@ class HostMountStateTestCase(test.NoDBTestCase): # Mount vol_a for instance_a self._sentinel_mount(m, mock.sentinel.vol_a, instance=instance_a) - mock_ensure_tree.assert_has_calls([ - mock.call(mock.sentinel.mountpoint)]) - mount.utils.execute.assert_has_calls([ - mock.call('mount', '-t', mock.sentinel.fstype, - mock.sentinel.option1, mock.sentinel.option2, - mock.sentinel.export, mock.sentinel.mountpoint, - run_as_root=True)]) + mock_mount.assert_has_calls([ + mock.call(mock.sentinel.fstype, mock.sentinel.export, + mock.sentinel.mountpoint, + [mock.sentinel.option1, mock.sentinel.option2])]) + mock_mount.reset_mock() # Mount vol_a for instance_b. We shouldn't have mounted again self._sentinel_mount(m, mock.sentinel.vol_a, instance=instance_b) - mock_ensure_tree.assert_has_calls([ - mock.call(mock.sentinel.mountpoint)]) - mount.utils.execute.assert_has_calls([ - mock.call('mount', '-t', mock.sentinel.fstype, - mock.sentinel.option1, mock.sentinel.option2, - mock.sentinel.export, mock.sentinel.mountpoint, - run_as_root=True)]) + mock_mount.assert_not_called() # Unmount vol_a for instance_a. We shouldn't have unmounted self._sentinel_umount(m, mock.sentinel.vol_a, instance=instance_a) - mock_ensure_tree.assert_has_calls([ - mock.call(mock.sentinel.mountpoint)]) - mount.utils.execute.assert_has_calls([ - mock.call('mount', '-t', mock.sentinel.fstype, - mock.sentinel.option1, mock.sentinel.option2, - mock.sentinel.export, mock.sentinel.mountpoint, - run_as_root=True)]) + mock_umount.assert_not_called() # Unmount vol_a for instance_b. We should have umounted. self._sentinel_umount(m, mock.sentinel.vol_a, instance=instance_b) - expected_calls = [ - mock.call('mount', '-t', mock.sentinel.fstype, - mock.sentinel.option1, mock.sentinel.option2, - mock.sentinel.export, mock.sentinel.mountpoint, - run_as_root=True)] - expected_calls.extend(self._expected_sentinel_umount_calls()) - mock_ensure_tree.assert_has_calls([ - mock.call(mock.sentinel.mountpoint)]) - mount.utils.execute.assert_has_calls(expected_calls) + mock_umount.assert_has_calls([mock.call(mock.sentinel.mountpoint)]) @mock.patch('oslo_utils.fileutils.ensure_tree') - def test_mount_concurrent(self, mock_ensure_tree): + @mock.patch('nova.privsep.fs.mount') + @mock.patch('nova.privsep.fs.umount') + @mock.patch('os.path.ismount', + side_effect=[False, False, True, False, False, True]) + @mock.patch('nova.privsep.path.rmdir') + def test_mount_concurrent(self, mock_rmdir, mock_ismount, mock_umount, + mock_mount, mock_ensure_tree): # This is 2 tests in 1, because the first test is the precondition # for the second. @@ -419,55 +379,71 @@ class HostMountStateTestCase(test.NoDBTestCase): ctl_c = TestThreadController(mount_c) ctl_d = TestThreadController(mount_d) - orig_execute = mount.utils.execute.side_effect - - def trap_mount_umount(cmd, *args, **kwargs): + def trap_mount(*args, **kwargs): # Conditionally wait at a waitpoint named after the command # we're executing - ctl = TestThreadController.current() - ctl.waitpoint(cmd) + TestThreadController.current().waitpoint('mount') - orig_execute(cmd, *args, **kwargs) + def trap_umount(*args, **kwargs): + # Conditionally wait at a waitpoint named after the command + # we're executing + TestThreadController.current().waitpoint('umount') - mount.utils.execute.side_effect = trap_mount_umount - - expected_calls = [] + mock_mount.side_effect = trap_mount + mock_umount.side_effect = trap_umount # Run the first thread until it's blocked while calling mount ctl_a.runto('mount') - expected_calls.extend([ - mock.call('mount', '-t', mock.sentinel.fstype, - mock.sentinel.option1, mock.sentinel.option2, - mock.sentinel.export, mock.sentinel.mountpoint, - run_as_root=True)]) - mock_ensure_tree.assert_has_calls([ mock.call(mock.sentinel.mountpoint)]) - self.assertEqual(expected_calls, mount.utils.execute.call_args_list) + mock_mount.assert_has_calls([ + mock.call(mock.sentinel.fstype, mock.sentinel.export, + mock.sentinel.mountpoint, + [mock.sentinel.option1, mock.sentinel.option2])]) # Start the second mount, and ensure it's got plenty of opportunity # to race. ctl_b.start() time.sleep(0.01) - - self.assertEqual(expected_calls, mount.utils.execute.call_args_list) + mock_ensure_tree.assert_has_calls([ + mock.call(mock.sentinel.mountpoint)]) + mock_mount.assert_has_calls([ + mock.call(mock.sentinel.fstype, mock.sentinel.export, + mock.sentinel.mountpoint, + [mock.sentinel.option1, mock.sentinel.option2])]) + mock_umount.assert_not_called() # Allow ctl_a to complete its mount ctl_a.runto('mounted') - self.assertEqual(expected_calls, mount.utils.execute.call_args_list) + mock_ensure_tree.assert_has_calls([ + mock.call(mock.sentinel.mountpoint)]) + mock_mount.assert_has_calls([ + mock.call(mock.sentinel.fstype, mock.sentinel.export, + mock.sentinel.mountpoint, + [mock.sentinel.option1, mock.sentinel.option2])]) + mock_umount.assert_not_called() # Allow ctl_b to finish. We should not have done a umount ctl_b.finish() - self.assertEqual(expected_calls, mount.utils.execute.call_args_list) + mock_ensure_tree.assert_has_calls([ + mock.call(mock.sentinel.mountpoint)]) + mock_mount.assert_has_calls([ + mock.call(mock.sentinel.fstype, mock.sentinel.export, + mock.sentinel.mountpoint, + [mock.sentinel.option1, mock.sentinel.option2])]) + mock_umount.assert_not_called() - # Allow ctl_a to start umounting + # Allow ctl_a to start umounting. We haven't executed rmdir yet, + # because we've blocked during umount ctl_a.runto('umount') - - expected_calls.extend(self._expected_sentinel_umount_calls()) - # We haven't executed rmdir yet, beause we've blocked during umount - rmdir = expected_calls.pop() - self.assertEqual(expected_calls, mount.utils.execute.call_args_list) - expected_calls.append(rmdir) + mock_ensure_tree.assert_has_calls([ + mock.call(mock.sentinel.mountpoint)]) + mock_mount.assert_has_calls([ + mock.call(mock.sentinel.fstype, mock.sentinel.export, + mock.sentinel.mountpoint, + [mock.sentinel.option1, mock.sentinel.option2])]) + mock_umount.assert_has_calls([mock.call(mock.sentinel.mountpoint)]) + mock_rmdir.assert_not_called() # While ctl_a is umounting, simultaneously start both ctl_c and # ctl_d, and ensure they have an opportunity to race @@ -481,17 +457,26 @@ class HostMountStateTestCase(test.NoDBTestCase): # We should have completed the previous umount, then remounted # exactly once - expected_calls.extend([ - mock.call('mount', '-t', mock.sentinel.fstype, - mock.sentinel.option1, mock.sentinel.option2, - mock.sentinel.export, mock.sentinel.mountpoint, - run_as_root=True)]) mock_ensure_tree.assert_has_calls([ mock.call(mock.sentinel.mountpoint)]) - self.assertEqual(expected_calls, mount.utils.execute.call_args_list) + mock_mount.assert_has_calls([ + mock.call(mock.sentinel.fstype, mock.sentinel.export, + mock.sentinel.mountpoint, + [mock.sentinel.option1, mock.sentinel.option2]), + mock.call(mock.sentinel.fstype, mock.sentinel.export, + mock.sentinel.mountpoint, + [mock.sentinel.option1, mock.sentinel.option2])]) + mock_umount.assert_has_calls([mock.call(mock.sentinel.mountpoint)]) @mock.patch('oslo_utils.fileutils.ensure_tree') - def test_mount_concurrent_no_interfere(self, mock_ensure_tree): + @mock.patch('nova.privsep.fs.mount') + @mock.patch('nova.privsep.fs.umount') + @mock.patch('os.path.ismount', + side_effect=[False, False, True, True, True, False]) + @mock.patch('nova.privsep.path.rmdir') + def test_mount_concurrent_no_interfere(self, mock_rmdir, mock_ismount, + mock_umount, mock_mount, + mock_ensure_tree): # Test that concurrent calls to mount volumes in different exports # run concurrently m = self._get_clean_hostmountstate() @@ -514,104 +499,77 @@ class HostMountStateTestCase(test.NoDBTestCase): ctl_a = TestThreadController(mount_a) ctl_b = TestThreadController(mount_b) - expected_calls = [] - ctl_a.runto('mounted') - expected_calls.extend([ - mock.call('mount', '-t', mock.sentinel.fstype, - mock.sentinel.option1, mock.sentinel.option2, - mock.sentinel.export, mock.sentinel.mountpoint_a, - run_as_root=True)]) - mock_ensure_tree.assert_has_calls([ - mock.call(mock.sentinel.mountpoint_a)]) - self.assertEqual(expected_calls, mount.utils.execute.call_args_list) + mock_mount.assert_has_calls([ + mock.call(mock.sentinel.fstype, mock.sentinel.export, + mock.sentinel.mountpoint_a, + [mock.sentinel.option1, mock.sentinel.option2])]) + mock_mount.reset_mock() ctl_b.finish() - expected_calls.extend([ - mock.call('mount', '-t', mock.sentinel.fstype, - mock.sentinel.option1, mock.sentinel.option2, - mock.sentinel.export, mock.sentinel.mountpoint_b, - run_as_root=True)]) - expected_calls.extend(self._expected_sentinel_umount_calls( - mock.sentinel.mountpoint_b)) - mock_ensure_tree.assert_has_calls([ - mock.call(mock.sentinel.mountpoint_b)]) - self.assertEqual(expected_calls, mount.utils.execute.call_args_list) + mock_mount.assert_has_calls([ + mock.call(mock.sentinel.fstype, mock.sentinel.export, + mock.sentinel.mountpoint_b, + [mock.sentinel.option1, mock.sentinel.option2])]) + mock_umount.assert_has_calls([mock.call(mock.sentinel.mountpoint_b)]) + mock_umount.reset_mock() ctl_a.finish() - expected_calls.extend(self._expected_sentinel_umount_calls( - mock.sentinel.mountpoint_a)) - self.assertEqual(expected_calls, mount.utils.execute.call_args_list) + mock_umount.assert_has_calls([mock.call(mock.sentinel.mountpoint_a)]) @mock.patch('oslo_utils.fileutils.ensure_tree') - def test_mount_after_failed_umount(self, mock_ensure_tree): + @mock.patch('nova.privsep.fs.mount') + @mock.patch('nova.privsep.fs.umount', + side_effect=processutils.ProcessExecutionError()) + @mock.patch('os.path.ismount', + side_effect=[False, True, True, True, False]) + @mock.patch('nova.privsep.path.rmdir') + def test_mount_after_failed_umount(self, mock_rmdir, mock_ismount, + mock_umount, mock_mount, + mock_ensure_tree): # Test that MountManager correctly tracks state when umount fails. # Test that when umount fails a subsequent mount doesn't try to # remount it. - # We've already got a fake execute (see setUp) which is ensuring mount, - # umount, and ismount work as expected. We don't want to mess with - # that, except that we want umount to raise an exception. We store the - # original here so we can call it if we're not unmounting, and so we - # can restore it when we no longer want the exception. - orig_execute = mount.utils.execute.side_effect - - def raise_on_umount(cmd, *args, **kwargs): - if cmd == 'umount': - raise mount.processutils.ProcessExecutionError() - orig_execute(cmd, *args, **kwargs) - - mount.utils.execute.side_effect = raise_on_umount - - expected_calls = [] - m = self._get_clean_hostmountstate() # Mount vol_a self._sentinel_mount(m, mock.sentinel.vol_a) - expected_calls.extend([ - mock.call('mount', '-t', mock.sentinel.fstype, - mock.sentinel.option1, mock.sentinel.option2, - mock.sentinel.export, mock.sentinel.mountpoint, - run_as_root=True)]) - mock_ensure_tree.assert_has_calls([ - mock.call(mock.sentinel.mountpoint)]) - self.assertEqual(expected_calls, mount.utils.execute.call_args_list) + mock_mount.assert_has_calls([ + mock.call(mock.sentinel.fstype, mock.sentinel.export, + mock.sentinel.mountpoint, + [mock.sentinel.option1, mock.sentinel.option2])]) + mock_mount.reset_mock() # Umount vol_a. The umount command will fail. self._sentinel_umount(m, mock.sentinel.vol_a) - expected_calls.extend(self._expected_sentinel_umount_calls()) + mock_umount.assert_has_calls([mock.call(mock.sentinel.mountpoint)]) # We should not have called rmdir, because umount failed - expected_calls.pop() - self.assertEqual(expected_calls, mount.utils.execute.call_args_list) + mock_rmdir.assert_not_called() # Mount vol_a again. We should not have called mount, because umount # failed. self._sentinel_mount(m, mock.sentinel.vol_a) - self.assertEqual(expected_calls, mount.utils.execute.call_args_list) + mock_mount.assert_not_called() # Prevent future failure of umount - mount.utils.execute.side_effect = orig_execute + mock_umount.side_effect = None # Umount vol_a successfully self._sentinel_umount(m, mock.sentinel.vol_a) - expected_calls.extend(self._expected_sentinel_umount_calls()) - self.assertEqual(expected_calls, mount.utils.execute.call_args_list) + mock_umount.assert_has_calls([mock.call(mock.sentinel.mountpoint)]) @mock.patch.object(mount.LOG, 'error') @mock.patch('oslo_utils.fileutils.ensure_tree') - def test_umount_log_failure(self, mock_ensure_tree, mock_LOG_error): - # Test that we log an error when umount fails - orig_execute = mount.utils.execute.side_effect - - def raise_on_umount(cmd, *args, **kwargs): - if cmd == 'umount': - raise mount.processutils.ProcessExecutionError( - None, None, None, 'umount', 'umount: device is busy.') - orig_execute(cmd, *args, **kwargs) - - mount.utils.execute.side_effect = raise_on_umount + @mock.patch('nova.privsep.fs.mount') + @mock.patch('os.path.ismount') + @mock.patch('nova.privsep.fs.umount') + def test_umount_log_failure(self, mock_umount, mock_ismount, mock_mount, + mock_ensure_tree, mock_LOG_error): + mock_umount.side_effect = mount.processutils.ProcessExecutionError( + None, None, None, 'umount', 'umount: device is busy.') + mock_ismount.side_effect = [False, True, True] m = self._get_clean_hostmountstate() diff --git a/nova/tests/unit/virt/libvirt/volume/test_nfs.py b/nova/tests/unit/virt/libvirt/volume/test_nfs.py index 0f9cb2b7b358..26f617072694 100644 --- a/nova/tests/unit/virt/libvirt/volume/test_nfs.py +++ b/nova/tests/unit/virt/libvirt/volume/test_nfs.py @@ -12,7 +12,6 @@ import os -import fixtures import mock from nova.tests.unit.virt.libvirt.volume import test_volume @@ -35,31 +34,13 @@ class LibvirtNFSVolumeDriverTestCase(test_volume.LibvirtVolumeBaseTestCase): m.host_up(self.fake_host) self.flags(nfs_mount_point_base=self.mnt_base, group='libvirt') - # Caution: this is also faked by the superclass - orig_execute = utils.execute - - mounted = [False] - - def fake_execute(*cmd, **kwargs): - orig_execute(*cmd, **kwargs) - - if cmd[0] == 'mount': - mounted[0] = True - - if cmd[0] == 'umount': - mounted[0] = False - - self.useFixture(fixtures.MonkeyPatch('nova.utils.execute', - fake_execute)) - - # Mock ismount to return the current mount state - # N.B. This is only valid for tests which mount and unmount a single - # directory. - self.useFixture(fixtures.MonkeyPatch('os.path.ismount', - lambda *args, **kwargs: mounted[0])) - @mock.patch('oslo_utils.fileutils.ensure_tree') - def test_libvirt_nfs_driver(self, mock_ensure_tree): + @mock.patch('nova.privsep.fs.mount') + @mock.patch('os.path.ismount', side_effect=[False, True, False]) + @mock.patch('nova.privsep.fs.umount') + @mock.patch('nova.privsep.path.rmdir') + def test_libvirt_nfs_driver(self, mock_rmdir, mock_umount, mock_ismount, + mock_mount, mock_ensure_tree): libvirt_driver = nfs.LibvirtNFSVolumeDriver(self.fake_host) export_string = '192.168.1.1:/nfs/share1' @@ -78,12 +59,11 @@ class LibvirtNFSVolumeDriverTestCase(test_volume.LibvirtVolumeBaseTestCase): device_path = os.path.join(export_mnt_base, connection_info['data']['name']) self.assertEqual(connection_info['data']['device_path'], device_path) - expected_commands = [ - ('mount', '-t', 'nfs', export_string, export_mnt_base), - ('umount', export_mnt_base), - ('rmdir', export_mnt_base)] mock_ensure_tree.assert_has_calls([mock.call(export_mnt_base)]) - self.assertEqual(expected_commands, self.executes) + mock_mount.assert_has_calls([mock.call('nfs', export_string, + export_mnt_base, [])]) + mock_umount.assert_has_calls([mock.call(export_mnt_base)]) + mock_rmdir.assert_has_calls([mock.call(export_mnt_base)]) def test_libvirt_nfs_driver_get_config(self): libvirt_driver = nfs.LibvirtNFSVolumeDriver(self.fake_host) @@ -102,7 +82,13 @@ class LibvirtNFSVolumeDriverTestCase(test_volume.LibvirtVolumeBaseTestCase): self.assertEqual('native', tree.find('./driver').get('io')) @mock.patch('oslo_utils.fileutils.ensure_tree') - def test_libvirt_nfs_driver_with_opts(self, mock_ensure_tree): + @mock.patch('nova.privsep.fs.mount') + @mock.patch('os.path.ismount', side_effect=[False, True, False]) + @mock.patch('nova.privsep.fs.umount') + @mock.patch('nova.privsep.path.rmdir') + def test_libvirt_nfs_driver_with_opts(self, mock_rmdir, mock_umount, + mock_ismount, mock_mount, + mock_ensure_tree): libvirt_driver = nfs.LibvirtNFSVolumeDriver(self.fake_host) export_string = '192.168.1.1:/nfs/share1' options = '-o intr,nfsvers=3' @@ -119,11 +105,9 @@ class LibvirtNFSVolumeDriverTestCase(test_volume.LibvirtVolumeBaseTestCase): libvirt_driver.disconnect_volume(connection_info, "vde", mock.sentinel.instance) - expected_commands = [ - ('mount', '-t', 'nfs', '-o', 'intr,nfsvers=3', - export_string, export_mnt_base), - ('umount', export_mnt_base), - ('rmdir', export_mnt_base) - ] mock_ensure_tree.assert_has_calls([mock.call(export_mnt_base)]) - self.assertEqual(expected_commands, self.executes) + mock_mount.assert_has_calls([mock.call('nfs', export_string, + export_mnt_base, + ['-o', 'intr,nfsvers=3'])]) + mock_umount.assert_has_calls([mock.call(export_mnt_base)]) + mock_rmdir.assert_has_calls([mock.call(export_mnt_base)]) diff --git a/nova/tests/unit/virt/libvirt/volume/test_remotefs.py b/nova/tests/unit/virt/libvirt/volume/test_remotefs.py index 188501e39b53..8cb2e0766037 100644 --- a/nova/tests/unit/virt/libvirt/volume/test_remotefs.py +++ b/nova/tests/unit/virt/libvirt/volume/test_remotefs.py @@ -17,20 +17,19 @@ import mock from oslo_concurrency import processutils from nova import test -from nova import utils from nova.virt.libvirt.volume import remotefs class RemoteFSTestCase(test.NoDBTestCase): """Remote filesystem operations test case.""" - @mock.patch.object(utils, 'execute') @mock.patch('oslo_utils.fileutils.ensure_tree') - def _test_mount_share(self, mock_ensure_tree, mock_execute, + @mock.patch('nova.privsep.fs.mount') + def _test_mount_share(self, mock_mount, mock_ensure_tree, already_mounted=False): if already_mounted: err_msg = 'Device or resource busy' - mock_execute.side_effect = [ + mock_mount.side_effect = [ None, processutils.ProcessExecutionError(err_msg)] remotefs.mount_share( @@ -39,11 +38,11 @@ class RemoteFSTestCase(test.NoDBTestCase): options=[mock.sentinel.mount_options]) mock_ensure_tree.assert_any_call(mock.sentinel.mount_path) - mock_execute.assert_any_call('mount', '-t', mock.sentinel.export_type, - mock.sentinel.mount_options, - mock.sentinel.export_path, - mock.sentinel.mount_path, - run_as_root=True) + mock_mount.assert_has_calls( + [mock.call(mock.sentinel.export_type, + mock.sentinel.export_path, + mock.sentinel.mount_path, + [mock.sentinel.mount_options])]) def test_mount_new_share(self): self._test_mount_share() @@ -51,14 +50,13 @@ class RemoteFSTestCase(test.NoDBTestCase): def test_mount_already_mounted_share(self): self._test_mount_share(already_mounted=True) - @mock.patch.object(utils, 'execute') - def test_unmount_share(self, mock_execute): + @mock.patch('nova.privsep.fs.umount') + def test_unmount_share(self, mock_umount): remotefs.unmount_share( mock.sentinel.mount_path, mock.sentinel.export_path) - mock_execute.assert_any_call('umount', mock.sentinel.mount_path, - run_as_root=True, attempts=3, - delay_on_retry=True) + mock_umount.assert_has_calls( + [mock.call(mock.sentinel.mount_path)]) @mock.patch('tempfile.mkdtemp', return_value='/tmp/Mercury') @mock.patch('nova.utils.execute') diff --git a/nova/tests/unit/virt/libvirt/volume/test_smbfs.py b/nova/tests/unit/virt/libvirt/volume/test_smbfs.py index 8e4742bc98a9..aaad0b5da0b9 100644 --- a/nova/tests/unit/virt/libvirt/volume/test_smbfs.py +++ b/nova/tests/unit/virt/libvirt/volume/test_smbfs.py @@ -30,7 +30,10 @@ class LibvirtSMBFSVolumeDriverTestCase(test_volume.LibvirtVolumeBaseTestCase): @mock.patch.object(libvirt_utils, 'is_mounted') @mock.patch('oslo_utils.fileutils.ensure_tree') - def test_libvirt_smbfs_driver(self, mock_ensure_tree, mock_is_mounted): + @mock.patch('nova.privsep.fs.mount') + @mock.patch('nova.privsep.fs.umount') + def test_libvirt_smbfs_driver(self, mock_umount, mock_mount, + mock_ensure_tree, mock_is_mounted): mock_is_mounted.return_value = False libvirt_driver = smbfs.LibvirtSMBFSVolumeDriver(self.fake_host) @@ -45,15 +48,16 @@ class LibvirtSMBFSVolumeDriverTestCase(test_volume.LibvirtVolumeBaseTestCase): libvirt_driver.disconnect_volume(connection_info, "vde", mock.sentinel.instance) - expected_commands = [ - ('mount', '-t', 'cifs', '-o', 'username=guest', - export_string, export_mnt_base), - ('umount', export_mnt_base)] mock_ensure_tree.assert_has_calls([mock.call(export_mnt_base)]) - self.assertEqual(expected_commands, self.executes) + mock_mount.assert_has_calls( + [mock.call('cifs', export_string, export_mnt_base, + ['-o', 'username=guest'])]) + mock_umount.assert_has_calls([mock.call(export_mnt_base)]) @mock.patch.object(libvirt_utils, 'is_mounted', return_value=True) - def test_libvirt_smbfs_driver_already_mounted(self, mock_is_mounted): + @mock.patch('nova.privsep.fs.umount') + def test_libvirt_smbfs_driver_already_mounted(self, mock_umount, + mock_is_mounted): libvirt_driver = smbfs.LibvirtSMBFSVolumeDriver(self.fake_host) export_string = '//192.168.1.1/volumes' export_mnt_base = os.path.join(self.mnt_base, @@ -66,9 +70,7 @@ class LibvirtSMBFSVolumeDriverTestCase(test_volume.LibvirtVolumeBaseTestCase): libvirt_driver.disconnect_volume(connection_info, "vde", mock.sentinel.instance) - expected_commands = [ - ('umount', export_mnt_base)] - self.assertEqual(expected_commands, self.executes) + mock_umount.assert_has_calls([mock.call(export_mnt_base)]) def test_libvirt_smbfs_driver_get_config(self): libvirt_driver = smbfs.LibvirtSMBFSVolumeDriver(self.fake_host) @@ -86,8 +88,10 @@ class LibvirtSMBFSVolumeDriverTestCase(test_volume.LibvirtVolumeBaseTestCase): @mock.patch.object(libvirt_utils, 'is_mounted') @mock.patch('oslo_utils.fileutils.ensure_tree') - def test_libvirt_smbfs_driver_with_opts(self, mock_ensure_tree, - mock_is_mounted): + @mock.patch('nova.privsep.fs.mount') + @mock.patch('nova.privsep.fs.umount') + def test_libvirt_smbfs_driver_with_opts(self, mock_umount, mock_mount, + mock_ensure_tree, mock_is_mounted): mock_is_mounted.return_value = False libvirt_driver = smbfs.LibvirtSMBFSVolumeDriver(self.fake_host) @@ -104,9 +108,8 @@ class LibvirtSMBFSVolumeDriverTestCase(test_volume.LibvirtVolumeBaseTestCase): libvirt_driver.disconnect_volume(connection_info, "vde", mock.sentinel.instance) - expected_commands = [ - ('mount', '-t', 'cifs', '-o', 'user=guest,uid=107,gid=105', - export_string, export_mnt_base), - ('umount', export_mnt_base)] mock_ensure_tree.assert_has_calls([mock.call(export_mnt_base)]) - self.assertEqual(expected_commands, self.executes) + mock_mount.assert_has_calls( + [mock.call('cifs', export_string, export_mnt_base, + ['-o', 'user=guest,uid=107,gid=105'])]) + mock_umount.assert_has_calls([mock.call(export_mnt_base)]) diff --git a/nova/tests/unit/virt/test_virt.py b/nova/tests/unit/virt/test_virt.py index 02db3acc6dd8..7e729fc4ec77 100644 --- a/nova/tests/unit/virt/test_virt.py +++ b/nova/tests/unit/virt/test_virt.py @@ -209,7 +209,8 @@ class TestVirtDisk(test.NoDBTestCase): self.assertEqual(disk_api.setup_container(image, container_dir), '/dev/fake') - def test_lxc_teardown_container(self): + @mock.patch('nova.privsep.fs.umount') + def test_lxc_teardown_container(self, mock_umount): def proc_mounts(mount_point): mount_points = { @@ -226,37 +227,40 @@ class TestVirtDisk(test.NoDBTestCase): expected_commands = [] disk_api.teardown_container('/mnt/loop/nopart') - expected_commands += [ - ('umount', '/dev/loop0'), - ('losetup', '--detach', '/dev/loop0'), - ] + expected_commands += [('losetup', '--detach', '/dev/loop0')] + mock_umount.assert_has_calls([mock.call('/dev/loop0')]) + mock_umount.reset_mock() disk_api.teardown_container('/mnt/loop/part') expected_commands += [ - ('umount', '/dev/mapper/loop0p1'), ('kpartx', '-d', '/dev/loop0'), ('losetup', '--detach', '/dev/loop0'), ] + mock_umount.assert_has_calls([mock.call('/dev/mapper/loop0p1')]) + mock_umount.reset_mock() disk_api.teardown_container('/mnt/nbd/nopart') expected_commands += [ ('blockdev', '--flushbufs', '/dev/nbd15'), - ('umount', '/dev/nbd15'), ('qemu-nbd', '-d', '/dev/nbd15'), ] + mock_umount.assert_has_calls([mock.call('/dev/nbd15')]) + mock_umount.reset_mock() disk_api.teardown_container('/mnt/nbd/part') expected_commands += [ ('blockdev', '--flushbufs', '/dev/nbd15'), - ('umount', '/dev/mapper/nbd15p1'), ('kpartx', '-d', '/dev/nbd15'), ('qemu-nbd', '-d', '/dev/nbd15'), ] + mock_umount.assert_has_calls([mock.call('/dev/mapper/nbd15p1')]) + mock_umount.reset_mock() # NOTE(thomasem): Not adding any commands in this case, because we're # not expecting an additional umount for LocalBlockImages. This is to # assert that no additional commands are run in this case. disk_api.teardown_container('/dev/volume-group/uuid_disk') + mock_umount.assert_not_called() self.assertEqual(self.executes, expected_commands) diff --git a/nova/tests/unit/virt/xenapi/test_xenapi.py b/nova/tests/unit/virt/xenapi/test_xenapi.py index 8fa2c9bc3e67..4ffdb7b856fd 100644 --- a/nova/tests/unit/virt/xenapi/test_xenapi.py +++ b/nova/tests/unit/virt/xenapi/test_xenapi.py @@ -953,8 +953,10 @@ class XenAPIVMTestCase(stubs.XenAPITestBase, @mock.patch.object(nova.privsep.path, 'makedirs') @mock.patch.object(nova.privsep.path, 'chown') @mock.patch.object(nova.privsep.path, 'chmod') - def test_spawn_netinject_file(self, chmod, chown, mkdir, write_file, - read_link): + @mock.patch.object(nova.privsep.fs, 'mount', return_value=(None, None)) + @mock.patch.object(nova.privsep.fs, 'umount') + def test_spawn_netinject_file(self, umount, mount, chmod, chown, mkdir, + write_file, read_link): self.flags(flat_injected=True) db_fakes.stub_out_db_instance_api(self, injected=True) diff --git a/nova/virt/configdrive.py b/nova/virt/configdrive.py index 1eb77cef9005..aef9784b82d1 100644 --- a/nova/virt/configdrive.py +++ b/nova/virt/configdrive.py @@ -25,6 +25,7 @@ import six import nova.conf from nova import exception from nova.objects import fields +import nova.privsep.fs from nova import utils from nova import version @@ -107,12 +108,9 @@ class ConfigDriveBuilder(object): with utils.tempdir() as mountdir: mounted = False try: - _, err = utils.trycmd( - 'mount', '-o', 'loop,uid=%d,gid=%d' % (os.getuid(), - os.getgid()), - path, - mountdir, - run_as_root=True) + _, err = nova.privsep.fs.mount( + None, path, mountdir, + ['-o', 'loop,uid=%d,gid=%d' % (os.getuid(), os.getgid())]) if err: raise exception.ConfigDriveMountFailed(operation='mount', error=err) @@ -127,7 +125,7 @@ class ConfigDriveBuilder(object): finally: if mounted: - utils.execute('umount', mountdir, run_as_root=True) + nova.privsep.fs.umount(mountdir) def make_drive(self, path): """Make the config drive. diff --git a/nova/virt/disk/mount/api.py b/nova/virt/disk/mount/api.py index 2e81d1b345c7..8e87051eadca 100644 --- a/nova/virt/disk/mount/api.py +++ b/nova/virt/disk/mount/api.py @@ -22,6 +22,7 @@ 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 @@ -248,8 +249,8 @@ class Mount(object): """Mount the device into the file system.""" LOG.debug("Mount %(dev)s on %(dir)s", {'dev': self.mapped_device, 'dir': self.mount_dir}) - _out, err = utils.trycmd('mount', self.mapped_device, self.mount_dir, - discard_warnings=True, run_as_root=True) + out, err = nova.privsep.fs.mount(None, self.mapped_device, + self.mount_dir) if err: self.error = _('Failed to mount filesystem: %s') % err LOG.debug(self.error) @@ -264,7 +265,7 @@ class Mount(object): return self.flush_dev() LOG.debug("Umount %s", self.mapped_device) - utils.execute('umount', self.mapped_device, run_as_root=True) + nova.privsep.fs.umount(self.mapped_device) self.mounted = False def flush_dev(self): diff --git a/nova/virt/libvirt/volume/mount.py b/nova/virt/libvirt/volume/mount.py index a90a26354774..a02fe460a385 100644 --- a/nova/virt/libvirt/volume/mount.py +++ b/nova/virt/libvirt/volume/mount.py @@ -25,7 +25,8 @@ import six import nova.conf from nova import exception from nova.i18n import _ -from nova import utils +import nova.privsep.fs +import nova.privsep.path CONF = nova.conf.CONF LOG = log.getLogger(__name__) @@ -292,20 +293,19 @@ class _HostMountState(object): 'mountpoint': mountpoint, 'options': options, 'gen': self.generation}) with self._get_locked(mountpoint) as mount: - if not os.path.ismount(mountpoint): + if os.path.ismount(mountpoint): + LOG.debug(('Mounting %(mountpoint)s generation %(gen)s, ' + 'mountpoint already mounted'), + {'mountpoint': mountpoint, 'gen': self.generation}) + else: LOG.debug('Mounting %(mountpoint)s generation %(gen)s', {'mountpoint': mountpoint, 'gen': self.generation}) fileutils.ensure_tree(mountpoint) - mount_cmd = ['mount', '-t', fstype] - if options is not None: - mount_cmd.extend(options) - mount_cmd.extend([export, mountpoint]) - try: - utils.execute(*mount_cmd, run_as_root=True) - except Exception: + nova.privsep.fs.mount(fstype, export, mountpoint, options) + except processutils.ProcessExecutionError(): # Check to see if mountpoint is mounted despite the error # eg it was already mounted if os.path.ismount(mountpoint): @@ -379,20 +379,13 @@ class _HostMountState(object): {'mountpoint': mountpoint, 'gen': self.generation}) try: - utils.execute('umount', mountpoint, run_as_root=True, - attempts=3, delay_on_retry=True) + nova.privsep.fs.umount(mountpoint) except processutils.ProcessExecutionError as ex: LOG.error("Couldn't unmount %(mountpoint)s: %(reason)s", {'mountpoint': mountpoint, 'reason': six.text_type(ex)}) if not os.path.ismount(mountpoint): - try: - utils.execute('rmdir', mountpoint) - except processutils.ProcessExecutionError as ex: - LOG.error("Couldn't remove directory %(mountpoint)s: " - "%(reason)s", - {'mountpoint': mountpoint, - 'reason': six.text_type(ex)}) + nova.privsep.path.rmdir(mountpoint) return False return True diff --git a/nova/virt/libvirt/volume/remotefs.py b/nova/virt/libvirt/volume/remotefs.py index 06f49dea3220..4397266eefb0 100644 --- a/nova/virt/libvirt/volume/remotefs.py +++ b/nova/virt/libvirt/volume/remotefs.py @@ -26,6 +26,7 @@ import six import nova.conf from nova.i18n import _ +import nova.privsep.fs from nova import utils LOG = logging.getLogger(__name__) @@ -44,13 +45,8 @@ def mount_share(mount_path, export_path, """ fileutils.ensure_tree(mount_path) - mount_cmd = ['mount', '-t', export_type] - if options is not None: - mount_cmd.extend(options) - mount_cmd.extend([export_path, mount_path]) - try: - utils.execute(*mount_cmd, run_as_root=True) + nova.privsep.fs.mount(export_type, export_path, mount_path, options) except processutils.ProcessExecutionError as exc: if 'Device or resource busy' in six.text_type(exc): LOG.warning("%s is already mounted", export_path) @@ -65,8 +61,7 @@ def unmount_share(mount_path, export_path): :param export_path: path of the remote export to be unmounted """ try: - utils.execute('umount', mount_path, run_as_root=True, - attempts=3, delay_on_retry=True) + nova.privsep.fs.umount(mount_path) except processutils.ProcessExecutionError as exc: if 'target is busy' in six.text_type(exc): LOG.debug("The share %s is still in use.", export_path) diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index a65aa337c5de..7e1bcf8eaae3 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -53,6 +53,7 @@ from nova.i18n import _ from nova.network import model as network_model from nova.objects import diagnostics from nova.objects import fields as obj_fields +import nova.privsep.fs from nova import utils from nova.virt import configdrive from nova.virt.disk import api as disk @@ -2440,12 +2441,11 @@ def _copy_partition(session, src_ref, dst_ref, partition, virtual_size): run_as_root=True) -def _mount_filesystem(dev_path, dir): - """mounts the device specified by dev_path in dir.""" +def _mount_filesystem(dev_path, mount_point): + """mounts the device specified by dev_path in mount_point.""" try: - _out, err = utils.execute('mount', - '-t', 'ext2,ext3,ext4,reiserfs', - dev_path, dir, run_as_root=True) + _out, err = nova.privsep.fs.mount('ext2,ext3,ext4,reiserfs', + dev_path, mount_point, None) except processutils.ProcessExecutionError as e: err = six.text_type(e) return err @@ -2478,7 +2478,7 @@ def _mounted_processing(device, key, net, metadata): disk.inject_data_into_fs(vfs, key, net, metadata, None, None) finally: - utils.execute('umount', dev_path, run_as_root=True) + nova.privsep.fs.umount(dev_path) else: LOG.info('Failed to mount filesystem (expected for ' 'non-linux instances): %s', err) diff --git a/releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml b/releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml index 3ef0a0e5f2a6..c84d5a607c21 100644 --- a/releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml +++ b/releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml @@ -3,7 +3,9 @@ upgrade: - | A sys-admin privsep daemon has been added and needs to be included in your rootwrap configuration. + - | + Calls to mount in the virt disk api no longer ignore the value of stderr. - | The following commands are no longer required to be listed in your rootwrap - configuration: cat; chown; cryptsetup; dd; mkdir; ploop; prl_disk_tool; - readlink; tee; touch. \ No newline at end of file + configuration: cat; chown; cryptsetup; dd; mkdir; mount; ploop; + prl_disk_tool; readlink; tee; touch; and umount.