Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Train ossa 2023 002 #26

Draft
wants to merge 8 commits into
base: jg-rebase-train
Choose a base branch
from
9 changes: 9 additions & 0 deletions nova/conf/compute.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""),
]

Expand Down
58 changes: 57 additions & 1 deletion nova/privsep/qemu.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
4 changes: 3 additions & 1 deletion nova/tests/unit/console/test_websocketproxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
24 changes: 24 additions & 0 deletions nova/tests/unit/privsep/test_qemu.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
30 changes: 5 additions & 25 deletions nova/tests/unit/virt/libvirt/test_driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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,
Expand All @@ -19613,10 +19595,9 @@ def test_get_instance_disk_info_parallels_ct(self, mock_get_domain,
"<target dir='/'/></filesystem>"
"</devices></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)
Expand All @@ -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)
Expand Down
Loading