diff --git a/nova/conf/compute.py b/nova/conf/compute.py index 6713f61d47b..f3d138a7a13 100644 --- a/nova/conf/compute.py +++ b/nova/conf/compute.py @@ -956,6 +956,15 @@ * Any integer >= 1 represents the maximum allowed. A value of 0 will cause the ``nova-compute`` service to fail to start, as 0 disk devices is an invalid configuration that would prevent instances from being able to boot. +"""), + cfg.ListOpt('vmdk_allowed_types', + default=['streamOptimized', 'monolithicSparse'], + help=""" +A list of strings describing allowed VMDK "create-type" subformats +that will be allowed. This is recommended to only include +single-file-with-sparse-header variants to avoid potential host file +exposure due to processing named extents. If this list is empty, then no +form of VMDK image will be allowed. """), ] diff --git a/nova/privsep/qemu.py b/nova/privsep/qemu.py index 5880fb1775e..529c24faf31 100644 --- a/nova/privsep/qemu.py +++ b/nova/privsep/qemu.py @@ -16,14 +16,22 @@ Helpers for qemu tasks. """ +import os + from oslo_concurrency import processutils from oslo_log import log as logging +from oslo_utils import units +from nova import exception +from nova.i18n import _ import nova.privsep.utils - LOG = logging.getLogger(__name__) +QEMU_IMG_LIMITS = processutils.ProcessLimits( + cpu_time=30, + address_space=1 * units.Gi) + @nova.privsep.sys_admin_pctxt.entrypoint def convert_image(source, dest, in_format, out_format, instances_path, @@ -71,3 +79,51 @@ def unprivileged_convert_image(source, dest, in_format, out_format, cmd = cmd + (source, dest) processutils.execute(*cmd) + + +@nova.privsep.sys_admin_pctxt.entrypoint +def privileged_qemu_img_info(path, format=None): + """Return an oject containing the parsed output from qemu-img info + + This is a privileged call to qemu-img info using the sys_admin_pctxt + entrypoint allowing host block devices etc to be accessed. + """ + return unprivileged_qemu_img_info(path, format=format) + + +def unprivileged_qemu_img_info(path, format=None): + """Return an object containing the parsed output from qemu-img info.""" + try: + # The following check is about ploop images that reside within + # directories and always have DiskDescriptor.xml file beside them + if (os.path.isdir(path) and + os.path.exists(os.path.join(path, "DiskDescriptor.xml"))): + path = os.path.join(path, "root.hds") + + cmd = ( + 'env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'info', path, + '--force-share', '--output=json', + ) + if format is not None: + cmd = cmd + ('-f', format) + out, err = processutils.execute(*cmd, prlimit=QEMU_IMG_LIMITS) + except processutils.ProcessExecutionError as exp: + if exp.exit_code == -9: + # this means we hit prlimits, make the exception more specific + msg = (_("qemu-img aborted by prlimits when inspecting " + "%(path)s : %(exp)s") % {'path': path, 'exp': exp}) + elif exp.exit_code == 1 and 'No such file or directory' in exp.stderr: + # The os.path.exists check above can race so this is a simple + # best effort at catching that type of failure and raising a more + # specific error. + raise exception.DiskNotFound(location=path) + else: + msg = (_("qemu-img failed to execute on %(path)s : %(exp)s") % + {'path': path, 'exp': exp}) + raise exception.InvalidDiskInfo(reason=msg) + + if not out: + msg = (_("Failed to run qemu-img info on %(path)s : %(error)s") % + {'path': path, 'error': err}) + raise exception.InvalidDiskInfo(reason=msg) + return out diff --git a/nova/tests/unit/console/test_websocketproxy.py b/nova/tests/unit/console/test_websocketproxy.py index 46a51dbf7c3..b7d2cd3e843 100644 --- a/nova/tests/unit/console/test_websocketproxy.py +++ b/nova/tests/unit/console/test_websocketproxy.py @@ -675,7 +675,9 @@ def test_reject_open_redirect(self, url='//example.com/%2F..'): # now the same url but with extra leading '/' characters removed. if expected_cpython in errmsg: location = result[3].decode() - location = location.removeprefix('Location: ').rstrip('\r\n') + if location.startswith('Location: '): + location = location[len('Location: '):] + location = location.rstrip('\r\n') self.assertTrue( location.startswith('/example.com/%2F..'), msg='Redirect location is not the expected sanitized URL', diff --git a/nova/tests/unit/privsep/test_qemu.py b/nova/tests/unit/privsep/test_qemu.py index 5fbc1789837..85c48aa4ae2 100644 --- a/nova/tests/unit/privsep/test_qemu.py +++ b/nova/tests/unit/privsep/test_qemu.py @@ -52,3 +52,27 @@ def test_convert_image(self): def test_convert_image_unprivileged(self): self._test_convert_image(nova.privsep.qemu.unprivileged_convert_image) + + @mock.patch('oslo_concurrency.processutils.execute') + @mock.patch('os.path.isdir') + def _test_qemu_img_info(self, method, mock_isdir, mock_execute): + mock_isdir.return_value = False + mock_execute.return_value = (mock.sentinel.out, None) + expected_cmd = ( + 'env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'info', + mock.sentinel.path, '--force-share', '--output=json', '-f', + mock.sentinel.format) + + # Assert that the output from processutils is returned + self.assertEqual( + mock.sentinel.out, + method(mock.sentinel.path, format=mock.sentinel.format)) + # Assert that the expected command is used + mock_execute.assert_called_once_with( + *expected_cmd, prlimit=nova.privsep.qemu.QEMU_IMG_LIMITS) + + def test_privileged_qemu_img_info(self): + self._test_qemu_img_info(nova.privsep.qemu.privileged_qemu_img_info) + + def test_unprivileged_qemu_img_info(self): + self._test_qemu_img_info(nova.privsep.qemu.unprivileged_qemu_img_info) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 93b41583042..b153dbc7703 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -105,7 +105,6 @@ from nova.virt import firewall as base_firewall from nova.virt import hardware from nova.virt.image import model as imgmodel -from nova.virt import images from nova.virt.libvirt import blockinfo from nova.virt.libvirt import config as vconfig from nova.virt.libvirt import designer @@ -1207,23 +1206,6 @@ def test_next_min_qemu_version_ok(self, mock_warning, mock_get_libversion): break self.assertFalse(version_arg_found) - # NOTE(sdague): python2.7 and python3.5 have different behaviors - # when it comes to comparing against the sentinel, so - # has_min_version is needed to pass python3.5. - @mock.patch.object(nova.virt.libvirt.host.Host, "has_min_version", - return_value=True) - @mock.patch.object(fakelibvirt.Connection, 'getVersion', - return_value=mock.sentinel.qemu_version) - def test_qemu_image_version(self, mock_get_libversion, min_ver): - """Test that init_host sets qemu image version - - A sentinel is used here so that we aren't chasing this value - against minimums that get raised over time. - """ - drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) - drvr.init_host("dummyhost") - self.assertEqual(images.QEMU_VERSION, mock.sentinel.qemu_version) - @mock.patch.object(fields.Architecture, "from_host", return_value=fields.Architecture.PPC64) def test_min_version_ppc_ok(self, mock_arch): @@ -19595,10 +19577,10 @@ def test_prepare_domain_for_snapshot_live_snapshots(self): @mock.patch('os.path.exists') @mock.patch('os.path.getsize') @mock.patch('os.path.isdir') - @mock.patch('oslo_concurrency.processutils.execute') + @mock.patch('nova.virt.images.qemu_img_info') @mock.patch.object(host.Host, '_get_domain') def test_get_instance_disk_info_parallels_ct(self, mock_get_domain, - mock_execute, + mock_qemu_img_info, mock_isdir, mock_getsize, mock_exists, @@ -19613,10 +19595,9 @@ def test_get_instance_disk_info_parallels_ct(self, mock_get_domain, "" "") - ret = ("image: /test/disk/root.hds\n" - "file format: parallels\n" - "virtual size: 20G (21474836480 bytes)\n" - "disk size: 789M\n") + mock_qemu_img_info.return_value = mock.Mock( + virtual_size=21474836480, image="/test/disk/root.hds", + file_format="ploop", size=827327254, backing_file=None) self.flags(virt_type='parallels', group='libvirt') instance = objects.Instance(**self.test_instance) @@ -19636,7 +19617,6 @@ def getsize_sideeffect(*args, **kwargs): mock_getsize.side_effect = getsize_sideeffect mock_exists.return_value = True mock_isdir.return_value = True - mock_execute.return_value = (ret, '') info = drvr.get_instance_disk_info(instance) info = jsonutils.loads(info) diff --git a/nova/tests/unit/virt/libvirt/test_utils.py b/nova/tests/unit/virt/libvirt/test_utils.py index 2888078cdef..c53dfd23930 100644 --- a/nova/tests/unit/virt/libvirt/test_utils.py +++ b/nova/tests/unit/virt/libvirt/test_utils.py @@ -32,11 +32,11 @@ from nova import objects from nova.objects import fields as obj_fields import nova.privsep.fs +import nova.privsep.qemu from nova import test from nova.tests import fixtures as nova_fixtures from nova.tests.unit import fake_instance from nova.tests.unit.virt.libvirt import fakelibvirt -from nova.virt.disk import api as disk from nova.virt import images from nova.virt.libvirt import guest as libvirt_guest from nova.virt.libvirt import utils as libvirt_utils @@ -92,244 +92,6 @@ def test_disk_type_ploop(self, mock_isdir, mock_exists): mock_exists.assert_called_once_with("%s/DiskDescriptor.xml" % path) self.assertEqual('ploop', d_type) - @mock.patch('os.path.exists', return_value=True) - @mock.patch('oslo_concurrency.processutils.execute') - def test_disk_backing(self, mock_execute, mock_exists): - path = '/myhome/disk.config' - template_output = """image: %(path)s -file format: raw -virtual size: 2K (2048 bytes) -cluster_size: 65536 -disk size: 96K -""" - output = template_output % ({ - 'path': path, - }) - mock_execute.return_value = (output, '') - d_backing = libvirt_utils.get_disk_backing_file(path) - mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', - 'qemu-img', 'info', path, - prlimit=images.QEMU_IMG_LIMITS) - mock_exists.assert_called_once_with(path) - self.assertIsNone(d_backing) - - def _test_disk_size(self, mock_execute, path, expected_size): - d_size = libvirt_utils.get_disk_size(path) - self.assertEqual(expected_size, d_size) - mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', - 'qemu-img', 'info', path, - prlimit=images.QEMU_IMG_LIMITS) - - @mock.patch('os.path.exists', return_value=True) - def test_disk_size(self, mock_exists): - path = '/myhome/disk.config' - template_output = """image: %(path)s -file format: raw -virtual size: %(v_size)s (%(vsize_b)s bytes) -cluster_size: 65536 -disk size: 96K -""" - for i in range(0, 128): - bytes = i * 65336 - kbytes = bytes / 1024 - mbytes = kbytes / 1024 - output = template_output % ({ - 'v_size': "%sM" % (mbytes), - 'vsize_b': i, - 'path': path, - }) - with mock.patch('oslo_concurrency.processutils.execute', - return_value=(output, '')) as mock_execute: - self._test_disk_size(mock_execute, path, i) - output = template_output % ({ - 'v_size': "%sK" % (kbytes), - 'vsize_b': i, - 'path': path, - }) - with mock.patch('oslo_concurrency.processutils.execute', - return_value=(output, '')) as mock_execute: - self._test_disk_size(mock_execute, path, i) - - @mock.patch('os.path.exists', return_value=True) - @mock.patch('oslo_concurrency.processutils.execute') - def test_qemu_info_canon(self, mock_execute, mock_exists): - path = "disk.config" - example_output = """image: disk.config -file format: raw -virtual size: 64M (67108864 bytes) -cluster_size: 65536 -disk size: 96K -blah BLAH: bb -""" - mock_execute.return_value = (example_output, '') - image_info = images.qemu_img_info(path) - mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', - 'qemu-img', 'info', path, - prlimit=images.QEMU_IMG_LIMITS) - mock_exists.assert_called_once_with(path) - self.assertEqual('disk.config', image_info.image) - self.assertEqual('raw', image_info.file_format) - self.assertEqual(67108864, image_info.virtual_size) - self.assertEqual(98304, image_info.disk_size) - self.assertEqual(65536, image_info.cluster_size) - - @mock.patch('os.path.exists', return_value=True) - @mock.patch('oslo_concurrency.processutils.execute') - def test_qemu_info_canon_qemu_2_10(self, mock_execute, mock_exists): - images.QEMU_VERSION = images.QEMU_VERSION_REQ_SHARED - path = "disk.config" - example_output = """image: disk.config -file format: raw -virtual size: 64M (67108864 bytes) -cluster_size: 65536 -disk size: 96K -blah BLAH: bb -""" - mock_execute.return_value = (example_output, '') - image_info = images.qemu_img_info(path) - mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', - 'qemu-img', 'info', path, - '--force-share', - prlimit=images.QEMU_IMG_LIMITS) - mock_exists.assert_called_once_with(path) - self.assertEqual('disk.config', image_info.image) - self.assertEqual('raw', image_info.file_format) - self.assertEqual(67108864, image_info.virtual_size) - self.assertEqual(98304, image_info.disk_size) - self.assertEqual(65536, image_info.cluster_size) - - @mock.patch('os.path.exists', return_value=True) - @mock.patch('oslo_concurrency.processutils.execute') - def test_qemu_info_canon2(self, mock_execute, mock_exists): - path = "disk.config" - example_output = """image: disk.config -file format: QCOW2 -virtual size: 67108844 -cluster_size: 65536 -disk size: 963434 -backing file: /var/lib/nova/a328c7998805951a_2 -""" - mock_execute.return_value = (example_output, '') - image_info = images.qemu_img_info(path) - mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', - 'qemu-img', 'info', path, - prlimit=images.QEMU_IMG_LIMITS) - mock_exists.assert_called_once_with(path) - self.assertEqual('disk.config', image_info.image) - self.assertEqual('qcow2', image_info.file_format) - self.assertEqual(67108844, image_info.virtual_size) - self.assertEqual(963434, image_info.disk_size) - self.assertEqual(65536, image_info.cluster_size) - self.assertEqual('/var/lib/nova/a328c7998805951a_2', - image_info.backing_file) - - @mock.patch('os.path.exists', return_value=True) - @mock.patch('os.path.isdir', return_value=True) - @mock.patch('oslo_concurrency.processutils.execute') - def test_qemu_info_ploop(self, mock_execute, mock_isdir, mock_exists): - path = "/var/lib/nova" - example_output = """image: root.hds -file format: parallels -virtual size: 3.0G (3221225472 bytes) -disk size: 706M -""" - mock_execute.return_value = (example_output, '') - image_info = images.qemu_img_info(path) - mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', - 'qemu-img', 'info', - os.path.join(path, 'root.hds'), - prlimit=images.QEMU_IMG_LIMITS) - mock_isdir.assert_called_once_with(path) - self.assertEqual(2, mock_exists.call_count) - self.assertEqual(path, mock_exists.call_args_list[0][0][0]) - self.assertEqual(os.path.join(path, 'DiskDescriptor.xml'), - mock_exists.call_args_list[1][0][0]) - self.assertEqual('root.hds', image_info.image) - self.assertEqual('parallels', image_info.file_format) - self.assertEqual(3221225472, image_info.virtual_size) - self.assertEqual(740294656, image_info.disk_size) - - @mock.patch('os.path.exists', return_value=True) - @mock.patch('oslo_concurrency.processutils.execute') - def test_qemu_backing_file_actual(self, - mock_execute, mock_exists): - path = "disk.config" - example_output = """image: disk.config -file format: raw -virtual size: 64M (67108864 bytes) -cluster_size: 65536 -disk size: 96K -Snapshot list: -ID TAG VM SIZE DATE VM CLOCK -1 d9a9784a500742a7bb95627bb3aace38 0 2012-08-20 10:52:46 00:00:00.000 -backing file: /var/lib/nova/a328c7998805951a_2 (actual path: /b/3a988059e51a_2) -""" - mock_execute.return_value = (example_output, '') - image_info = images.qemu_img_info(path) - mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', - 'qemu-img', 'info', path, - prlimit=images.QEMU_IMG_LIMITS) - mock_exists.assert_called_once_with(path) - self.assertEqual('disk.config', image_info.image) - self.assertEqual('raw', image_info.file_format) - self.assertEqual(67108864, image_info.virtual_size) - self.assertEqual(98304, image_info.disk_size) - self.assertEqual(1, len(image_info.snapshots)) - self.assertEqual('/b/3a988059e51a_2', - image_info.backing_file) - - @mock.patch('os.path.exists', return_value=True) - @mock.patch('oslo_concurrency.processutils.execute') - def test_qemu_info_convert(self, mock_execute, mock_exists): - path = "disk.config" - example_output = """image: disk.config -file format: raw -virtual size: 64M -disk size: 96K -Snapshot list: -ID TAG VM SIZE DATE VM CLOCK -1 d9a9784a500742a7bb95627bb3aace38 0 2012-08-20 10:52:46 00:00:00.000 -3 d9a9784a500742a7bb95627bb3aace38 0 2012-08-20 10:52:46 00:00:00.000 -4 d9a9784a500742a7bb95627bb3aace38 0 2012-08-20 10:52:46 00:00:00.000 -junk stuff: bbb -""" - mock_execute.return_value = (example_output, '') - image_info = images.qemu_img_info(path) - mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', - 'qemu-img', 'info', path, - prlimit=images.QEMU_IMG_LIMITS) - mock_exists.assert_called_once_with(path) - self.assertEqual('disk.config', image_info.image) - self.assertEqual('raw', image_info.file_format) - self.assertEqual(67108864, image_info.virtual_size) - self.assertEqual(98304, image_info.disk_size) - - @mock.patch('os.path.exists', return_value=True) - @mock.patch('oslo_concurrency.processutils.execute') - def test_qemu_info_snaps(self, mock_execute, mock_exists): - path = "disk.config" - example_output = """image: disk.config -file format: raw -virtual size: 64M (67108864 bytes) -disk size: 96K -Snapshot list: -ID TAG VM SIZE DATE VM CLOCK -1 d9a9784a500742a7bb95627bb3aace38 0 2012-08-20 10:52:46 00:00:00.000 -3 d9a9784a500742a7bb95627bb3aace38 0 2012-08-20 10:52:46 00:00:00.000 -4 d9a9784a500742a7bb95627bb3aace38 0 2012-08-20 10:52:46 00:00:00.000 -""" - mock_execute.return_value = (example_output, '') - image_info = images.qemu_img_info(path) - mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', - 'qemu-img', 'info', path, - prlimit=images.QEMU_IMG_LIMITS) - mock_exists.assert_called_once_with(path) - self.assertEqual('disk.config', image_info.image) - self.assertEqual('raw', image_info.file_format) - self.assertEqual(67108864, image_info.virtual_size) - self.assertEqual(98304, image_info.disk_size) - self.assertEqual(3, len(image_info.snapshots)) - def test_valid_hostname_normal(self): self.assertTrue(libvirt_utils.is_valid_hostname("hello.world.com")) @@ -466,22 +228,6 @@ def xend_probe_side_effect(): libvirt_utils.pick_disk_driver_name(version)) mock_execute.reset_mock() - @mock.patch('os.path.exists', return_value=True) - @mock.patch('oslo_concurrency.processutils.execute') - def test_get_disk_size(self, mock_execute, mock_exists): - path = '/some/path' - example_output = """image: 00000001 -file format: raw -virtual size: 4.4M (4592640 bytes) -disk size: 4.4M -""" - mock_execute.return_value = (example_output, '') - self.assertEqual(4592640, disk.get_disk_size('/some/path')) - mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', - 'qemu-img', 'info', path, - prlimit=images.QEMU_IMG_LIMITS) - mock_exists.assert_called_once_with(path) - def test_copy_image(self): dst_fd, dst_path = tempfile.mkstemp() try: @@ -740,31 +486,6 @@ class FakeImgInfo(object): del self.executes - def test_get_disk_backing_file(self): - with_actual_path = False - - def fake_execute(*args, **kwargs): - if with_actual_path: - return ("some: output\n" - "backing file: /foo/bar/baz (actual path: /a/b/c)\n" - "...: ...\n"), '' - else: - return ("some: output\n" - "backing file: /foo/bar/baz\n" - "...: ...\n"), '' - - def return_true(*args, **kwargs): - return True - - self.stub_out('oslo_concurrency.processutils.execute', fake_execute) - self.stub_out('os.path.exists', return_true) - - out = libvirt_utils.get_disk_backing_file('') - self.assertEqual(out, 'baz') - with_actual_path = True - out = libvirt_utils.get_disk_backing_file('') - self.assertEqual(out, 'c') - def test_get_instance_path_at_destination(self): instance = fake_instance.fake_instance_obj(None, name='fake_inst', uuid=uuids.instance) diff --git a/nova/tests/unit/virt/test_images.py b/nova/tests/unit/virt/test_images.py index 9dacc828add..c1115272a74 100644 --- a/nova/tests/unit/virt/test_images.py +++ b/nova/tests/unit/virt/test_images.py @@ -16,6 +16,8 @@ import mock from oslo_concurrency import processutils +from oslo_serialization import jsonutils +from oslo_utils import imageutils import six from nova.compute import utils as compute_utils @@ -41,13 +43,12 @@ def test_qemu_info_with_errors(self, path_exists, mock_exec): '/fake/path') @mock.patch.object(os.path, 'exists', return_value=True) - @mock.patch('oslo_concurrency.processutils.execute', - return_value=('stdout', None)) + @mock.patch('nova.privsep.qemu.unprivileged_qemu_img_info', + return_value={}) def test_qemu_info_with_no_errors(self, path_exists, utils_execute): image_info = images.qemu_img_info('/fake/path') self.assertTrue(image_info) - self.assertTrue(str(image_info)) @mock.patch.object(compute_utils, 'disk_ops_semaphore') @mock.patch('nova.privsep.utils.supports_direct_io', return_value=True) @@ -129,3 +130,47 @@ def test_convert_image_without_direct_io_support(self, mock_execute, '-O', 'out_format', '-f', 'in_format', 'source', 'dest') mock_disk_op_sema.__enter__.assert_called_once() self.assertTupleEqual(expected, mock_execute.call_args[0]) + + def test_convert_image_vmdk_allowed_list_checking(self): + info = {'format': 'vmdk', + 'format-specific': { + 'type': 'vmdk', + 'data': { + 'create-type': 'monolithicFlat', + }}} + + # If the format is not in the allowed list, we should get an error + self.assertRaises(exception.ImageUnacceptable, + images.check_vmdk_image, 'foo', + imageutils.QemuImgInfo(jsonutils.dumps(info), + format='json')) + + # With the format in the allowed list, no error + self.flags(vmdk_allowed_types=['streamOptimized', 'monolithicFlat', + 'monolithicSparse'], + group='compute') + images.check_vmdk_image('foo', + imageutils.QemuImgInfo(jsonutils.dumps(info), + format='json')) + + # With an empty list, allow nothing + self.flags(vmdk_allowed_types=[], group='compute') + self.assertRaises(exception.ImageUnacceptable, + images.check_vmdk_image, 'foo', + imageutils.QemuImgInfo(jsonutils.dumps(info), + format='json')) + + @mock.patch.object(images, 'fetch') + @mock.patch('nova.privsep.qemu.unprivileged_qemu_img_info') + def test_fetch_checks_vmdk_rules(self, mock_info, mock_fetch): + info = {'format': 'vmdk', + 'format-specific': { + 'type': 'vmdk', + 'data': { + 'create-type': 'monolithicFlat', + }}} + mock_info.return_value = jsonutils.dumps(info) + with mock.patch('os.path.exists', return_value=True): + e = self.assertRaises(exception.ImageUnacceptable, + images.fetch_to_raw, None, 'foo', 'anypath') + self.assertIn('Invalid VMDK create-type specified', str(e)) diff --git a/nova/virt/images.py b/nova/virt/images.py index d01214beee0..df7ffbd05bf 100644 --- a/nova/virt/images.py +++ b/nova/virt/images.py @@ -19,14 +19,12 @@ Handling of VM disk images. """ -import operator import os from oslo_concurrency import processutils from oslo_log import log as logging from oslo_utils import fileutils from oslo_utils import imageutils -from oslo_utils import units from nova.compute import utils as compute_utils import nova.conf @@ -40,15 +38,6 @@ CONF = nova.conf.CONF IMAGE_API = image.API() -QEMU_IMG_LIMITS = processutils.ProcessLimits( - cpu_time=30, - address_space=1 * units.Gi) - -# This is set by the libvirt driver on startup. The version is used to -# determine what flags need to be set on the command line. -QEMU_VERSION = None -QEMU_VERSION_REQ_SHARED = 2010000 - def qemu_img_info(path, format=None): """Return an object containing the parsed output from qemu-img info.""" @@ -57,42 +46,19 @@ def qemu_img_info(path, format=None): if not os.path.exists(path) and CONF.libvirt.images_type != 'rbd': raise exception.DiskNotFound(location=path) - try: - # The following check is about ploop images that reside within - # directories and always have DiskDescriptor.xml file beside them - if (os.path.isdir(path) and - os.path.exists(os.path.join(path, "DiskDescriptor.xml"))): - path = os.path.join(path, "root.hds") - - cmd = ('env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'info', path) - if format is not None: - cmd = cmd + ('-f', format) - # Check to see if the qemu version is >= 2.10 because if so, we need - # to add the --force-share flag. - if QEMU_VERSION and operator.ge(QEMU_VERSION, QEMU_VERSION_REQ_SHARED): - cmd = cmd + ('--force-share',) - out, err = processutils.execute(*cmd, prlimit=QEMU_IMG_LIMITS) - except processutils.ProcessExecutionError as exp: - if exp.exit_code == -9: - # this means we hit prlimits, make the exception more specific - msg = (_("qemu-img aborted by prlimits when inspecting " - "%(path)s : %(exp)s") % {'path': path, 'exp': exp}) - elif exp.exit_code == 1 and 'No such file or directory' in exp.stderr: - # The os.path.exists check above can race so this is a simple - # best effort at catching that type of failure and raising a more - # specific error. - raise exception.DiskNotFound(location=path) - else: - msg = (_("qemu-img failed to execute on %(path)s : %(exp)s") % - {'path': path, 'exp': exp}) - raise exception.InvalidDiskInfo(reason=msg) + info = nova.privsep.qemu.unprivileged_qemu_img_info(path, format=format) + return imageutils.QemuImgInfo(info, format='json') - if not out: - msg = (_("Failed to run qemu-img info on %(path)s : %(error)s") % - {'path': path, 'error': err}) - raise exception.InvalidDiskInfo(reason=msg) - return imageutils.QemuImgInfo(out) +def privileged_qemu_img_info(path, format=None, output_format='json'): + """Return an object containing the parsed output from qemu-img info.""" + # TODO(mikal): this code should not be referring to a libvirt specific + # flag. + if not os.path.exists(path) and CONF.libvirt.images_type != 'rbd': + raise exception.DiskNotFound(location=path) + + info = nova.privsep.qemu.privileged_qemu_img_info(path, format=format) + return imageutils.QemuImgInfo(info, format='json') def convert_image(source, dest, in_format, out_format, run_as_root=False, @@ -148,6 +114,34 @@ def get_info(context, image_href): return IMAGE_API.get(context, image_href) +def check_vmdk_image(image_id, data): + # Check some rules about VMDK files. Specifically we want to make + # sure that the "create-type" of the image is one that we allow. + # Some types of VMDK files can reference files outside the disk + # image and we do not want to allow those for obvious reasons. + + types = CONF.compute.vmdk_allowed_types + + if not len(types): + LOG.warning('Refusing to allow VMDK image as vmdk_allowed_' + 'types is empty') + msg = _('Invalid VMDK create-type specified') + raise exception.ImageUnacceptable(image_id=image_id, reason=msg) + + try: + create_type = data.format_specific['data']['create-type'] + except KeyError: + msg = _('Unable to determine VMDK create-type') + raise exception.ImageUnacceptable(image_id=image_id, reason=msg) + + if create_type not in CONF.compute.vmdk_allowed_types: + LOG.warning('Refusing to process VMDK file with create-type of %r ' + 'which is not in allowed set of: %s', create_type, + ','.join(CONF.compute.vmdk_allowed_types)) + msg = _('Invalid VMDK create-type specified') + raise exception.ImageUnacceptable(image_id=image_id, reason=msg) + + def fetch_to_raw(context, image_href, path, trusted_certs=None): path_tmp = "%s.part" % path fetch(context, image_href, path_tmp, trusted_certs) @@ -167,6 +161,9 @@ def fetch_to_raw(context, image_href, path, trusted_certs=None): reason=(_("fmt=%(fmt)s backed by: %(backing_file)s") % {'fmt': fmt, 'backing_file': backing_file})) + if fmt == 'vmdk': + check_vmdk_image(image_href, data) + if fmt != "raw" and CONF.force_raw_images: staged = "%s.converted" % path LOG.debug("%s was %s, converting to raw", image_href, fmt) diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index da988294531..fc6833fc9f6 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -686,11 +686,7 @@ def init_host(self, host): libvirt_utils.version_to_string(MIN_LIBVIRT_VERSION)) if CONF.libvirt.virt_type in ("qemu", "kvm"): - if self._host.has_min_version(hv_ver=MIN_QEMU_VERSION): - # "qemu-img info" calls are version dependent, so we need to - # store the version in the images module. - images.QEMU_VERSION = self._host.get_connection().getVersion() - else: + if not self._host.has_min_version(hv_ver=MIN_QEMU_VERSION): raise exception.InternalError( _('Nova requires QEMU version %s or greater.') % libvirt_utils.version_to_string(MIN_QEMU_VERSION)) diff --git a/tox.ini b/tox.ini index 150d98d0d1b..87d67577ce6 100644 --- a/tox.ini +++ b/tox.ini @@ -38,12 +38,16 @@ passenv = [testenv:py27] commands = +# hack to bring in required backports, higher than upper constraints + pip install 'git+https://github.com/stackhpc/oslo.utils.git@c0de0b493e1c56ec985dca65becdf7d721c0b571' stestr run {posargs} env TEST_OSPROFILER=1 stestr run --combine --no-discover 'nova.tests.unit.test_profiler' stestr slowest [testenv:py36] commands = +# hack to bring in required backports, instead of pip install 'oslo.utils===4.1.2' + pip install 'git+https://github.com/stackhpc/oslo.utils.git@c0de0b493e1c56ec985dca65becdf7d721c0b571' stestr run {posargs} env TEST_OSPROFILER=1 stestr run --combine --no-discover 'nova.tests.unit.test_profiler'