diff --git a/etc/nova/rootwrap.d/compute.filters b/etc/nova/rootwrap.d/compute.filters index 014ac19de436..2d2b2df34955 100644 --- a/etc/nova/rootwrap.d/compute.filters +++ b/etc/nova/rootwrap.d/compute.filters @@ -218,13 +218,6 @@ cp: CommandFilter, cp, root # nova/virt/xenapi/vm_utils.py: sync: CommandFilter, sync, root -# nova/virt/libvirt/imagebackend.py: -ploop: RegExpFilter, ploop, root, ploop, restore-descriptor, .* -prl_disk_tool: RegExpFilter, prl_disk_tool, root, prl_disk_tool, resize, --size, .*M$, --resize_partition, --hdd, .* - -# nova/virt/libvirt/utils.py: -ploop: RegExpFilter, ploop, root, ploop, init, -s, .*, -f, .*, -t, .*, .* - # nova/virt/libvirt/utils.py: 'xend', 'status' xend: CommandFilter, xend, root diff --git a/nova/privsep/libvirt.py b/nova/privsep/libvirt.py index 156ced432f6c..77eaaa3fdbec 100644 --- a/nova/privsep/libvirt.py +++ b/nova/privsep/libvirt.py @@ -20,8 +20,10 @@ libvirt specific routines. import binascii import errno import os +import stat from oslo_concurrency import processutils +from oslo_utils import units import nova.privsep @@ -89,6 +91,57 @@ def dmcrypt_delete_volume(target): processutils.execute('cryptsetup', 'remove', target) +@nova.privsep.sys_admin_pctxt.entrypoint +def ploop_init(size, disk_format, fs_type, disk_path): + """Initialize ploop disk, make it readable for non-root user + + :param disk_format: data allocation format (raw or expanded) + :param fs_type: filesystem (ext4, ext3, none) + :param disk_path: ploop image file + """ + processutils.execute('ploop', 'init', '-s', size, '-f', disk_format, '-t', + fs_type, disk_path, run_as_root=True, + check_exit_code=True) + + # Add read access for all users, because "ploop init" creates + # disk with rw rights only for root. OpenStack user should have access + # to the disk to request info via "qemu-img info" + # TODO(mikal): this is a faithful rendition of the pre-privsep code from + # the libvirt driver, but it seems undesirable to me. It would be good to + # create the loop file with the right owner or group such that we don't + # need to have it world readable. I don't have access to a system to test + # this on however. + st = os.stat(disk_path) + os.chmod(disk_path, st.st_mode | stat.S_IROTH) + + +@nova.privsep.sys_admin_pctxt.entrypoint +def ploop_resize(disk_path, size): + """Resize ploop disk + + :param disk_path: ploop image file + :param size: new size (in bytes) + """ + processutils.execute('prl_disk_tool', 'resize', + '--size', '%dM' % (size // units.Mi), + '--resize_partition', + '--hdd', disk_path, + run_as_root=True, check_exit_code=True) + + +@nova.privsep.sys_admin_pctxt.entrypoint +def ploop_restore_descriptor(image_dir, base_delta, fmt): + """Restore ploop disk descriptor XML + + :param image_dir: path to where descriptor XML is created + :param base_delta: ploop image file containing the data + :param fmt: ploop data allocation format (raw or expanded) + """ + processutils.execute('ploop', 'restore-descriptor', '-f', fmt, + image_dir, base_delta, + run_as_root=True, check_exit_code=True) + + @nova.privsep.sys_admin_pctxt.entrypoint def enable_hairpin(interface): """Enable hairpin mode for a libvirt guest.""" diff --git a/nova/tests/unit/virt/disk/test_api.py b/nova/tests/unit/virt/disk/test_api.py index 8b4edc7d4a5e..0b8ac060fd1a 100644 --- a/nova/tests/unit/virt/disk/test_api.py +++ b/nova/tests/unit/virt/disk/test_api.py @@ -154,22 +154,17 @@ class APITestCase(test.NoDBTestCase): @mock.patch.object(api, 'can_resize_image', autospec=True, return_value=True) - @mock.patch.object(utils, 'execute', autospec=True) - def test_extend_ploop(self, mock_execute, mock_can_resize_image): + @mock.patch('nova.privsep.libvirt.ploop_resize') + def test_extend_ploop(self, mock_ploop_resize, mock_can_resize_image): imgfile = tempfile.NamedTemporaryFile() self.addCleanup(imgfile.close) imgsize = 10 * units.Gi - imgsize_mb = str(imgsize // units.Mi) + 'M' image = imgmodel.LocalFileImage(imgfile, imgmodel.FORMAT_PLOOP) api.extend(image, imgsize) mock_can_resize_image.assert_called_once_with(image.path, imgsize) - mock_execute.assert_called_once_with('prl_disk_tool', 'resize', - '--size', imgsize_mb, - '--resize_partition', - '--hdd', imgfile, - run_as_root=True) + mock_ploop_resize.assert_called_once_with(imgfile, imgsize) @mock.patch.object(api, 'can_resize_image', autospec=True, return_value=True) diff --git a/nova/tests/unit/virt/libvirt/test_imagebackend.py b/nova/tests/unit/virt/libvirt/test_imagebackend.py index 2f0993bf3389..8d1e7582def0 100644 --- a/nova/tests/unit/virt/libvirt/test_imagebackend.py +++ b/nova/tests/unit/virt/libvirt/test_imagebackend.py @@ -1712,9 +1712,9 @@ class PloopTestCase(_ImageTestCase, test.NoDBTestCase): return_value=2048) @mock.patch.object(imagebackend.utils, 'synchronized') @mock.patch.object(fake_libvirt_utils, 'copy_image') - @mock.patch.object(imagebackend.utils, 'execute') + @mock.patch('nova.privsep.libvirt.ploop_restore_descriptor') @mock.patch.object(imagebackend.disk, 'extend') - def test_create_image(self, mock_extend, mock_execute, + def test_create_image(self, mock_extend, mock_ploop_restore_descriptor, mock_copy, mock_sync, mock_get): mock_sync.side_effect = lambda *a, **kw: self._fake_deco fn = mock.MagicMock() @@ -1724,9 +1724,9 @@ class PloopTestCase(_ImageTestCase, test.NoDBTestCase): image.create_image(fn, self.TEMPLATE_PATH, 2048, image_id=None) mock_copy.assert_called_once_with(self.TEMPLATE_PATH, img_path) - mock_execute.assert_called_once_with("ploop", "restore-descriptor", - "-f", "raw", - self.PATH, img_path) + mock_ploop_restore_descriptor.assert_called_once_with(self.PATH, + img_path, + "raw") self.assertTrue(mock_sync.called) fn.assert_called_once_with(target=self.TEMPLATE_PATH, image_id=None) mock_extend.assert_called_once_with( diff --git a/nova/tests/unit/virt/libvirt/test_utils.py b/nova/tests/unit/virt/libvirt/test_utils.py index 646a72c85991..2082d6e03ca2 100644 --- a/nova/tests/unit/virt/libvirt/test_utils.py +++ b/nova/tests/unit/virt/libvirt/test_utils.py @@ -351,18 +351,17 @@ ID TAG VM SIZE DATE VM CLOCK def test_create_ploop_image(self, fs_type, default_eph_format, expected_fs_type): - with mock.patch('nova.utils.execute') as mock_execute: + with test.nested(mock.patch('nova.utils.execute'), + mock.patch('nova.privsep.libvirt.ploop_init') + ) as (mock_execute, mock_ploop_init): self.flags(default_ephemeral_format=default_eph_format) libvirt_utils.create_ploop_image('expanded', '/some/path', '5G', fs_type) mock_execute.assert_has_calls([ - mock.call('mkdir', '-p', '/some/path'), - mock.call('ploop', 'init', '-s', '5G', - '-f', 'expanded', '-t', expected_fs_type, - '/some/path/root.hds', - run_as_root=True, check_exit_code=True), - mock.call('chmod', '-R', 'a+r', '/some/path', - run_as_root=True, check_exit_code=True)]) + mock.call('mkdir', '-p', '/some/path')]) + mock_ploop_init.assert_has_calls([ + mock.call('5G', 'expanded', expected_fs_type, + '/some/path/root.hds')]) def test_pick_disk_driver_name(self): type_map = {'kvm': ([True, 'qemu'], [False, 'qemu'], [None, 'qemu']), diff --git a/nova/virt/disk/api.py b/nova/virt/disk/api.py index 72643faf1857..956ea46243e8 100644 --- a/nova/virt/disk/api.py +++ b/nova/virt/disk/api.py @@ -33,11 +33,11 @@ if os.name != 'nt': from oslo_concurrency import processutils from oslo_log import log as logging from oslo_serialization import jsonutils -from oslo_utils import units import nova.conf from nova import exception from nova.i18n import _ +import nova.privsep.libvirt from nova import utils from nova.virt.disk.mount import api as mount from nova.virt.disk.vfs import api as vfs @@ -157,17 +157,11 @@ def extend(image, size): if not isinstance(image, imgmodel.LocalImage): return - if (image.format == imgmodel.FORMAT_PLOOP): - if not can_resize_image(image.path, size): - return - - utils.execute('prl_disk_tool', 'resize', - '--size', '%dM' % (size // units.Mi), - '--resize_partition', - '--hdd', image.path, run_as_root=True) + if not can_resize_image(image.path, size): return - if not can_resize_image(image.path, size): + if (image.format == imgmodel.FORMAT_PLOOP): + nova.privsep.libvirt.ploop_resize(image.path, size) return utils.execute('qemu-img', 'resize', image.path, size) diff --git a/nova/virt/libvirt/imagebackend.py b/nova/virt/libvirt/imagebackend.py index 06f129eee3d7..ceb40589ebb5 100644 --- a/nova/virt/libvirt/imagebackend.py +++ b/nova/virt/libvirt/imagebackend.py @@ -33,6 +33,7 @@ import nova.conf from nova import exception from nova.i18n import _ from nova import image +import nova.privsep.libvirt import nova.privsep.path from nova import utils from nova.virt.disk import api as disk @@ -1071,8 +1072,9 @@ class Ploop(Image): fileutils.ensure_tree(target) image_path = os.path.join(target, "root.hds") libvirt_utils.copy_image(base, image_path) - utils.execute('ploop', 'restore-descriptor', '-f', self.pcs_format, - target, image_path) + nova.privsep.libvirt.ploop_restore_descriptor(target, + image_path, + self.pcs_format) if size: self.resize_image(size) diff --git a/nova/virt/libvirt/utils.py b/nova/virt/libvirt/utils.py index 2ff986f23e21..1243ebbaa952 100644 --- a/nova/virt/libvirt/utils.py +++ b/nova/virt/libvirt/utils.py @@ -28,6 +28,7 @@ from oslo_log import log as logging import nova.conf from nova.i18n import _ from nova.objects import fields as obj_fields +import nova.privsep.libvirt from nova import utils from nova.virt.disk import api as disk from nova.virt import images @@ -105,13 +106,7 @@ def create_ploop_image(disk_format, path, size, fs_type): disk.FS_FORMAT_EXT4 utils.execute('mkdir', '-p', path) disk_path = os.path.join(path, 'root.hds') - utils.execute('ploop', 'init', '-s', size, '-f', disk_format, '-t', - fs_type, disk_path, run_as_root=True, check_exit_code=True) - # Add read access for all users, because "ploop init" creates - # disk with rw rights only for root. OpenStack user should have access - # to the disk to request info via "qemu-img info" - utils.execute('chmod', '-R', 'a+r', path, - run_as_root=True, check_exit_code=True) + nova.privsep.libvirt.ploop_init(size, disk_format, fs_type, disk_path) def pick_disk_driver_name(hypervisor_version, is_block_dev=False): diff --git a/releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml b/releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml index 386e9b1850f4..1f83e698fc69 100644 --- a/releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml +++ b/releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml @@ -5,4 +5,5 @@ upgrade: rootwrap configuration. - | The following commands are no longer required to be listed in your rootwrap - configuration: cat; chown; cryptsetup; readlink; tee; touch. \ No newline at end of file + configuration: cat; chown; cryptsetup; ploop; prl_disk_tool; readlink; + tee; touch. \ No newline at end of file