From fef14351672416bce465b25eb7b1314f13f9eb14 Mon Sep 17 00:00:00 2001 From: Michael Still Date: Tue, 12 Dec 2017 03:28:41 +1100 Subject: [PATCH] Move makefs to privsep Change-Id: I388d31d5e9c1cff10bc534ba69be899e67681ce6 blueprint: hurrah-for-privsep --- nova/privsep/fs.py | 31 ++++++++++++++++ nova/tests/unit/privsep/test_fs.py | 39 ++++++++++++++++++++ nova/tests/unit/test_configdrive2.py | 2 +- nova/tests/unit/test_utils.py | 39 -------------------- nova/tests/unit/virt/libvirt/test_driver.py | 35 ++++++++---------- nova/tests/unit/virt/xenapi/test_vm_utils.py | 10 ++--- nova/tests/unit/virt/xenapi/test_xenapi.py | 3 +- nova/utils.py | 25 ------------- nova/virt/configdrive.py | 2 +- nova/virt/disk/api.py | 5 ++- nova/virt/libvirt/driver.py | 2 +- nova/virt/xenapi/vm_utils.py | 3 +- 12 files changed, 100 insertions(+), 96 deletions(-) diff --git a/nova/privsep/fs.py b/nova/privsep/fs.py index b5c9604d6ea7..1191660a71f2 100644 --- a/nova/privsep/fs.py +++ b/nova/privsep/fs.py @@ -220,3 +220,34 @@ def ext_journal_disable(device): @nova.privsep.sys_admin_pctxt.entrypoint def ext_journal_enable(device): processutils.execute('tune2fs', '-j', device) + + +@nova.privsep.sys_admin_pctxt.entrypoint +def mkfs(fs, path, label=None): + unprivileged_mkfs(fs, path, label=None) + + +# NOTE(mikal): this method is deliberately not wrapped in a privsep entrypoint +def unprivileged_mkfs(fs, path, label=None): + """Format a file or block device + + :param fs: Filesystem type (examples include 'swap', 'ext3', 'ext4' + 'btrfs', etc.) + :param path: Path to file or block device to format + :param label: Volume label to use + """ + if fs == 'swap': + args = ['mkswap'] + else: + args = ['mkfs', '-t', fs] + # add -F to force no interactive execute on non-block device. + if fs in ('ext3', 'ext4', 'ntfs'): + args.extend(['-F']) + if label: + if fs in ('msdos', 'vfat'): + label_opt = '-n' + else: + label_opt = '-L' + args.extend([label_opt, label]) + args.append(path) + processutils.execute(*args) diff --git a/nova/tests/unit/privsep/test_fs.py b/nova/tests/unit/privsep/test_fs.py index babcbe5f292d..7affb2f01c87 100644 --- a/nova/tests/unit/privsep/test_fs.py +++ b/nova/tests/unit/privsep/test_fs.py @@ -32,3 +32,42 @@ class PrivsepFilesystemHelpersTestCase(test.NoDBTestCase): self.assertEqual(2, len(partitions)) self.assertEqual((1, 2, 10, "ext3", "", "boot"), partitions[0]) self.assertEqual((2, 20, 10, "", "bob", ""), partitions[1]) + + +class MkfsTestCase(test.NoDBTestCase): + @mock.patch('oslo_concurrency.processutils.execute') + def test_mkfs_ext4(self, mock_execute): + nova.privsep.fs.unprivileged_mkfs('ext4', '/my/block/dev') + mock_execute.assert_called_once_with('mkfs', '-t', 'ext4', '-F', + '/my/block/dev') + + @mock.patch('oslo_concurrency.processutils.execute') + def test_mkfs_msdos(self, mock_execute): + nova.privsep.fs.unprivileged_mkfs('msdos', '/my/msdos/block/dev') + mock_execute.assert_called_once_with('mkfs', '-t', 'msdos', + '/my/msdos/block/dev') + + @mock.patch('oslo_concurrency.processutils.execute') + def test_mkfs_swap(self, mock_execute): + nova.privsep.fs.unprivileged_mkfs('swap', '/my/swap/block/dev') + mock_execute.assert_called_once_with('mkswap', '/my/swap/block/dev') + + @mock.patch('oslo_concurrency.processutils.execute') + def test_mkfs_ext4_withlabel(self, mock_execute): + nova.privsep.fs.unprivileged_mkfs('ext4', '/my/block/dev', 'ext4-vol') + mock_execute.assert_called_once_with( + 'mkfs', '-t', 'ext4', '-F', '-L', 'ext4-vol', '/my/block/dev') + + @mock.patch('oslo_concurrency.processutils.execute') + def test_mkfs_msdos_withlabel(self, mock_execute): + nova.privsep.fs.unprivileged_mkfs( + 'msdos', '/my/msdos/block/dev', 'msdos-vol') + mock_execute.assert_called_once_with( + 'mkfs', '-t', 'msdos', '-n', 'msdos-vol', '/my/msdos/block/dev') + + @mock.patch('oslo_concurrency.processutils.execute') + def test_mkfs_swap_withlabel(self, mock_execute): + nova.privsep.fs.unprivileged_mkfs( + 'swap', '/my/swap/block/dev', 'swap-vol') + mock_execute.assert_called_once_with( + 'mkswap', '-L', 'swap-vol', '/my/swap/block/dev') diff --git a/nova/tests/unit/test_configdrive2.py b/nova/tests/unit/test_configdrive2.py index ea1b2ef16599..b0ff80423d9f 100644 --- a/nova/tests/unit/test_configdrive2.py +++ b/nova/tests/unit/test_configdrive2.py @@ -64,7 +64,7 @@ class ConfigDriveTestCase(test.NoDBTestCase): if imagefile: fileutils.delete_if_exists(imagefile) - @mock.patch.object(utils, 'mkfs', return_value=None) + @mock.patch('nova.privsep.fs.unprivileged_mkfs', 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)) diff --git a/nova/tests/unit/test_utils.py b/nova/tests/unit/test_utils.py index 2328e9616896..98f08b4f2661 100644 --- a/nova/tests/unit/test_utils.py +++ b/nova/tests/unit/test_utils.py @@ -677,45 +677,6 @@ class AuditPeriodTest(test.NoDBTestCase): year=2011)) -class MkfsTestCase(test.NoDBTestCase): - - @mock.patch('nova.utils.execute') - def test_mkfs_ext4(self, mock_execute): - utils.mkfs('ext4', '/my/block/dev') - mock_execute.assert_called_once_with('mkfs', '-t', 'ext4', '-F', - '/my/block/dev', run_as_root=False) - - @mock.patch('nova.utils.execute') - def test_mkfs_msdos(self, mock_execute): - utils.mkfs('msdos', '/my/msdos/block/dev') - mock_execute.assert_called_once_with('mkfs', '-t', 'msdos', - '/my/msdos/block/dev', run_as_root=False) - - @mock.patch('nova.utils.execute') - def test_mkfs_swap(self, mock_execute): - utils.mkfs('swap', '/my/swap/block/dev') - mock_execute.assert_called_once_with('mkswap', '/my/swap/block/dev', - run_as_root=False) - - @mock.patch('nova.utils.execute') - def test_mkfs_ext4_withlabel(self, mock_execute): - utils.mkfs('ext4', '/my/block/dev', 'ext4-vol') - mock_execute.assert_called_once_with('mkfs', '-t', 'ext4', '-F', - '-L', 'ext4-vol', '/my/block/dev', run_as_root=False) - - @mock.patch('nova.utils.execute') - def test_mkfs_msdos_withlabel(self, mock_execute): - utils.mkfs('msdos', '/my/msdos/block/dev', 'msdos-vol') - mock_execute.assert_called_once_with('mkfs', '-t', 'msdos', - '-n', 'msdos-vol', '/my/msdos/block/dev', run_as_root=False) - - @mock.patch('nova.utils.execute') - def test_mkfs_swap_withlabel(self, mock_execute): - utils.mkfs('swap', '/my/swap/block/dev', 'swap-vol') - mock_execute.assert_called_once_with('mkswap', '-L', 'swap-vol', - '/my/swap/block/dev', run_as_root=False) - - class MetadataToDictTestCase(test.NoDBTestCase): def test_metadata_to_dict(self): self.assertEqual(utils.metadata_to_dict( diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 3cd945cbc85b..92e07b96f0a1 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -11952,15 +11952,14 @@ class LibvirtConnTestCase(test.NoDBTestCase, fake_backend.disks['disk'].create_snap.assert_called_once_with( libvirt_utils.RESIZE_SNAPSHOT_NAME) - @mock.patch.object(utils, 'execute') - def test_create_ephemeral_specified_fs(self, mock_exec): + @mock.patch('nova.privsep.fs.mkfs') + def test_create_ephemeral_specified_fs(self, fake_mkfs): self.flags(default_ephemeral_format='ext3') drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) drvr._create_ephemeral('/dev/something', 20, 'myVol', 'linux', is_block_dev=True, specified_fs='ext4') - mock_exec.assert_called_once_with('mkfs', '-t', 'ext4', '-F', '-L', - 'myVol', '/dev/something', - run_as_root=True) + fake_mkfs.assert_has_calls([mock.call('ext4', '/dev/something', + 'myVol')]) @mock.patch('nova.privsep.path.utime') def test_create_ephemeral_specified_fs_not_valid(self, mock_utime): @@ -11995,24 +11994,22 @@ class LibvirtConnTestCase(test.NoDBTestCase, context, instance, disk_info['mapping'], block_device_info=block_device_info) - def test_create_ephemeral_default(self): + @mock.patch('nova.privsep.fs.mkfs') + def test_create_ephemeral_default(self, fake_mkfs): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) - self.mox.StubOutWithMock(utils, 'execute') - utils.execute('mkfs', '-t', 'ext4', '-F', '-L', 'myVol', - '/dev/something', run_as_root=True) - self.mox.ReplayAll() drvr._create_ephemeral('/dev/something', 20, 'myVol', 'linux', is_block_dev=True) + fake_mkfs.assert_has_calls([mock.call('ext4', '/dev/something', + 'myVol')]) - def test_create_ephemeral_with_conf(self): + @mock.patch('nova.privsep.fs.mkfs') + def test_create_ephemeral_with_conf(self, fake_mkfs): CONF.set_override('default_ephemeral_format', 'ext4') drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) - self.mox.StubOutWithMock(utils, 'execute') - utils.execute('mkfs', '-t', 'ext4', '-F', '-L', 'myVol', - '/dev/something', run_as_root=True) - self.mox.ReplayAll() drvr._create_ephemeral('/dev/something', 20, 'myVol', 'linux', is_block_dev=True) + fake_mkfs.assert_has_calls([mock.call('ext4', '/dev/something', + 'myVol')]) def test_create_ephemeral_with_arbitrary(self): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) @@ -12048,13 +12045,11 @@ class LibvirtConnTestCase(test.NoDBTestCase, '/dev/something', '20G', 'fs_format') - def test_create_swap_default(self): + @mock.patch('nova.privsep.fs.unprivileged_mkfs') + def test_create_swap_default(self, fake_mkfs): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) - self.mox.StubOutWithMock(utils, 'execute') - utils.execute('mkswap', '/dev/something', run_as_root=False) - self.mox.ReplayAll() - drvr._create_swap('/dev/something', 1) + fake_mkfs.assert_has_calls([mock.call('swap', '/dev/something')]) def test_ensure_console_log_for_instance_pass(self): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) diff --git a/nova/tests/unit/virt/xenapi/test_vm_utils.py b/nova/tests/unit/virt/xenapi/test_vm_utils.py index 3ac280153d23..d349a1c6980e 100644 --- a/nova/tests/unit/virt/xenapi/test_vm_utils.py +++ b/nova/tests/unit/virt/xenapi/test_vm_utils.py @@ -995,8 +995,8 @@ class VDIOtherConfigTestCase(VMUtilsTestBase): class GenerateDiskTestCase(VMUtilsTestBase): @mock.patch.object(vm_utils, 'vdi_attached') - @mock.patch.object(vm_utils.utils, 'mkfs', - side_effect = test.TestingException()) + @mock.patch('nova.privsep.fs.mkfs', + side_effect = test.TestingException()) @mock.patch.object(vm_utils, '_get_dom0_ref', return_value='dom0_ref') @mock.patch.object(vm_utils, 'safe_find_sr', return_value='sr_ref') @mock.patch.object(vm_utils, 'create_vdi', return_value='vdi_ref') @@ -1021,7 +1021,7 @@ class GenerateDiskTestCase(VMUtilsTestBase): bootable=False) @mock.patch.object(vm_utils, 'vdi_attached') - @mock.patch.object(vm_utils.utils, 'mkfs') + @mock.patch('nova.privsep.fs.mkfs') @mock.patch.object(vm_utils, '_get_dom0_ref', return_value='dom0_ref') @mock.patch.object(vm_utils, 'safe_find_sr', return_value='sr_ref') @mock.patch.object(vm_utils, 'create_vdi', return_value='vdi_ref') @@ -1054,7 +1054,7 @@ class GenerateDiskTestCase(VMUtilsTestBase): bootable=False) @mock.patch.object(vm_utils, 'vdi_attached') - @mock.patch.object(vm_utils.utils, 'mkfs') + @mock.patch('nova.privsep.fs.mkfs') @mock.patch.object(vm_utils, '_get_dom0_ref', return_value='dom0_ref') @mock.patch.object(vm_utils, 'safe_find_sr', return_value='sr_ref') @mock.patch.object(vm_utils, 'create_vdi', return_value='vdi_ref') @@ -1082,7 +1082,7 @@ class GenerateDiskTestCase(VMUtilsTestBase): mock_attached_here.assert_any_call(session, 'vdi_ref', read_only=False) mock_mkfs.assert_called_with('ext4', '/dev/fake_devp1', - 'ephemeral-1', run_as_root=True) + 'ephemeral-1') mock_create_vbd.assert_called_with(session, 'vm_ref', 'vdi_ref', '2', bootable=False) diff --git a/nova/tests/unit/virt/xenapi/test_xenapi.py b/nova/tests/unit/virt/xenapi/test_xenapi.py index 903182b04a25..c597ba8b6a1d 100644 --- a/nova/tests/unit/virt/xenapi/test_xenapi.py +++ b/nova/tests/unit/virt/xenapi/test_xenapi.py @@ -904,7 +904,8 @@ class XenAPIVMTestCase(stubs.XenAPITestBase, os_type="linux", architecture="x86-64") self.check_vm_params_for_linux() - def test_spawn_vhd_glance_windows(self): + @mock.patch('nova.privsep.fs.mkfs') + def test_spawn_vhd_glance_windows(self, fake_mkfs): self._test_spawn(IMAGE_VHD, None, None, os_type="windows", architecture="i386", instance_type_id=5) diff --git a/nova/utils.py b/nova/utils.py index 9d835fcd7222..fb37941c727d 100644 --- a/nova/utils.py +++ b/nova/utils.py @@ -723,31 +723,6 @@ class UndoManager(object): self._rollback() -def mkfs(fs, path, label=None, run_as_root=False): - """Format a file or block device - - :param fs: Filesystem type (examples include 'swap', 'ext3', 'ext4' - 'btrfs', etc.) - :param path: Path to file or block device to format - :param label: Volume label to use - """ - if fs == 'swap': - args = ['mkswap'] - else: - args = ['mkfs', '-t', fs] - # add -F to force no interactive execute on non-block device. - if fs in ('ext3', 'ext4', 'ntfs'): - args.extend(['-F']) - if label: - if fs in ('msdos', 'vfat'): - label_opt = '-n' - else: - label_opt = '-L' - args.extend([label_opt, label]) - args.append(path) - execute(*args, run_as_root=run_as_root) - - def metadata_to_dict(metadata, include_deleted=False): result = {} for item in metadata: diff --git a/nova/virt/configdrive.py b/nova/virt/configdrive.py index aef9784b82d1..40a3dcae62ad 100644 --- a/nova/virt/configdrive.py +++ b/nova/virt/configdrive.py @@ -103,7 +103,7 @@ class ConfigDriveBuilder(object): with open(path, 'wb') as f: f.truncate(CONFIGDRIVESIZE_BYTES) - utils.mkfs('vfat', path, label='config-2') + nova.privsep.fs.unprivileged_mkfs('vfat', path, label='config-2') with utils.tempdir() as mountdir: mounted = False diff --git a/nova/virt/disk/api.py b/nova/virt/disk/api.py index 04f8b93a855f..31b0ab5e751f 100644 --- a/nova/virt/disk/api.py +++ b/nova/virt/disk/api.py @@ -117,7 +117,10 @@ def mkfs(os_type, fs_label, target, run_as_root=True, specified_fs=None): specified_fs = _DEFAULT_FS_BY_OSTYPE.get(os_type, _DEFAULT_FILE_SYSTEM) - utils.mkfs(specified_fs, target, fs_label, run_as_root=run_as_root) + if run_as_root: + nova.privsep.fs.mkfs(specified_fs, target, fs_label) + else: + nova.privsep.fs.unprivileged_mkfs(specified_fs, target, fs_label) def resize2fs(image, check_exit_code=False, run_as_root=False): diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index ca9256392bc4..ab641b24a6f1 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -3177,7 +3177,7 @@ class LibvirtDriver(driver.ComputeDriver): def _create_swap(target, swap_mb, context=None): """Create a swap file of specified size.""" libvirt_utils.create_image('raw', target, '%dM' % swap_mb) - utils.mkfs('swap', target) + nova.privsep.fs.unprivileged_mkfs('swap', target) @staticmethod def _get_console_log_path(instance): diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index 0852f072df4f..a0212c3306a6 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -1038,8 +1038,7 @@ def _generate_disk(session, instance, vm_ref, userdevice, name_label, if fs_type is not None and not mkfs_in_dom0: with vdi_attached(session, vdi_ref, read_only=False) as dev: partition_path = utils.make_dev_path(dev, partition=1) - utils.mkfs(fs_type, partition_path, fs_label, - run_as_root=True) + nova.privsep.fs.mkfs(fs_type, partition_path, fs_label) # 4. Create VBD between instance VM and VDI if vm_ref: