openstack
/
nova
Code Issues Proposed changes

libvirt: Use SATA bus for cdrom devices when using Q35 machine type 

The Q35 machine type no longer provides an IDE bus and will need to use
a SATA bus to attach legacy devices such as cdroms. More details can be
found in the following related bug:

Don't assume the guest machine type to be of 'pc'
https://bugs.launchpad.net/nova/+bug/1780138

This change now ensures the blockinfo.get_disk_bus_for_device_type
method will now return "sata" as the bus type when the Q35 machine type
is used for cdrom devices on QEMU or KVM hosts that are not PPC, S390 or
AArch64 based.

To enable this the _get_machine_type method has been extracted from the
Libvirt driver into the Libvirt utils module. This method has also been
simplified through the removal of the caps parameter, replaced with
calls to the get_arch utility method and additional extraction of
architecture specific defaults into the existing
get_default_machine_type utility method.

Related-bug: 1780138
Closes-bug: 1831538
Change-Id: Id97f4baddcf2caff91599773d9b5de5181b7fdf6
(cherry picked from commit 527c452a6f)
This commit is contained in:
Lee Yarwood 2019-06-04 11:57:18 +01:00
parent 0cb6106b83
commit db40cc44cb
7 changed files with 97 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
nova/tests/functional/libvirt/base.py
View File

@ -17,6 +17,8 @@ import copy
import fixtures
import mock
from nova.objects import fields as obj_fields
from nova.tests import fixtures as nova_fixtures
from nova.tests.functional import fixtures as func_fixtures
from nova.tests.functional import test_servers as base
@ -52,6 +54,13 @@ class ServersTestBase(base.ServersTestBase):
_p = mock.patch('nova.virt.libvirt.host.Host.get_connection')
self.mock_conn = _p.start()
self.addCleanup(_p.stop)
# As above, mock the 'get_arch' function as we may need to provide
# different host architectures during some tests.
_a = mock.patch('nova.virt.libvirt.utils.get_arch')
self.mock_arch = _a.start()
# Default to X86_64
self.mock_arch.return_value = obj_fields.Architecture.X86_64
self.addCleanup(_a.stop)
def _setup_compute_service(self):
# NOTE(stephenfin): We don't start the compute service here as we wish

4
nova/tests/unit/virt/libvirt/fake_libvirt_utils.py
View File

@ -179,6 +179,10 @@ def cpu_features_to_traits(features):
return libvirt_utils.cpu_features_to_traits(features)
def get_machine_type(image_meta):
return libvirt_utils.get_machine_type(image_meta)
def get_default_machine_type(arch):
return libvirt_utils.get_default_machine_type(arch)

23
nova/tests/unit/virt/libvirt/test_blockinfo.py
View File

@ -814,6 +814,29 @@ class LibvirtBlockInfoTest(test.NoDBTestCase):
self.assertRaises(exception.NovaException,
blockinfo.get_disk_bus_for_disk_dev, 'inv', 'val')
@mock.patch('nova.virt.libvirt.utils.get_machine_type')
@mock.patch('nova.virt.libvirt.utils.get_arch')
def test_get_disk_bus_for_device_type_cdrom_with_q35_get_arch(self,
mock_get_arch, mock_get_machine_type):
instance = objects.Instance(**self.test_instance)
mock_get_machine_type.return_value = 'pc-q35-rhel8.0.0'
mock_get_arch.return_value = obj_fields.Architecture.X86_64
image_meta = {'properties': {}}
image_meta = objects.ImageMeta.from_dict(image_meta)
bus = blockinfo.get_disk_bus_for_device_type(instance, 'kvm',
image_meta,
device_type='cdrom')
self.assertEqual('sata', bus)
def test_get_disk_bus_for_device_type_cdrom_with_q35_image_meta(self):
instance = objects.Instance(**self.test_instance)
image_meta = {'properties': {'hw_machine_type': 'pc-q35-rhel8.0.0'}}
image_meta = objects.ImageMeta.from_dict(image_meta)
bus = blockinfo.get_disk_bus_for_device_type(instance, 'kvm',
image_meta,
device_type='cdrom')
self.assertEqual('sata', bus)
def test_get_config_drive_type_default(self):
config_drive_type = blockinfo.get_config_drive_type()
self.assertEqual('cdrom', config_drive_type)

26
nova/tests/unit/virt/libvirt/test_driver.py
View File

@ -6195,10 +6195,13 @@ class LibvirtConnTestCase(test.NoDBTestCase,
image_meta, disk_info)
self.assertNotEqual(cfg.os_cmdline, "")
@mock.patch('nova.virt.libvirt.utils.get_arch',
return_value=fields.Architecture.ARMV7)
@mock.patch.object(libvirt_driver.LibvirtDriver,
"_get_guest_storage_config")
@mock.patch.object(libvirt_driver.LibvirtDriver, "_has_numa_support")
def test_get_guest_config_armv7(self, mock_numa, mock_storage):
def test_get_guest_config_armv7(self, mock_numa, mock_storage,
mock_get_arch):
def get_host_capabilities_stub(self):
cpu = vconfig.LibvirtConfigGuestCPU()
cpu.arch = fields.Architecture.ARMV7
@ -6227,12 +6230,14 @@ class LibvirtConnTestCase(test.NoDBTestCase,
image_meta, disk_info)
self.assertEqual(cfg.os_mach_type, "virt")
@mock.patch('nova.virt.libvirt.utils.get_arch',
return_value=fields.Architecture.AARCH64)
@mock.patch.object(libvirt_driver.LibvirtDriver,
"_get_guest_storage_config")
@mock.patch.object(libvirt_driver.LibvirtDriver, "_has_numa_support")
@mock.patch('os.path.exists', return_value=True)
def test_get_guest_config_aarch64(self, mock_path_exists,
mock_numa, mock_storage):
mock_numa, mock_storage, mock_get_arch):
def get_host_capabilities_stub(self):
cpu = vconfig.LibvirtConfigGuestCPU()
cpu.arch = fields.Architecture.AARCH64
@ -6279,12 +6284,15 @@ class LibvirtConnTestCase(test.NoDBTestCase,
self.assertEqual(TEST_AMOUNT_OF_PCIE_SLOTS, num_ports)
@mock.patch('nova.virt.libvirt.utils.get_arch',
return_value=fields.Architecture.AARCH64)
@mock.patch.object(libvirt_driver.LibvirtDriver,
"_get_guest_storage_config")
@mock.patch.object(libvirt_driver.LibvirtDriver, "_has_numa_support")
@mock.patch('os.path.exists', return_value=True)
def test_get_guest_config_aarch64_with_graphics(self, mock_path_exists,
mock_numa, mock_storage):
mock_numa, mock_storage,
mock_get_arch):
def get_host_capabilities_stub(self):
cpu = vconfig.LibvirtConfigGuestCPU()
cpu.arch = fields.Architecture.AARCH64
@ -6320,18 +6328,14 @@ class LibvirtConnTestCase(test.NoDBTestCase,
self.assertTrue(usbhost_exists)
self.assertTrue(keyboard_exists)
def test_get_guest_config_machine_type_s390(self):
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
caps = vconfig.LibvirtConfigCaps()
caps.host = vconfig.LibvirtConfigCapsHost()
caps.host.cpu = vconfig.LibvirtConfigGuestCPU()
@mock.patch('nova.virt.libvirt.utils.get_arch')
def test_get_guest_config_machine_type_s390(self, mock_get_arch):
image_meta = objects.ImageMeta.from_dict(self.test_image_meta)
host_cpu_archs = (fields.Architecture.S390, fields.Architecture.S390X)
for host_cpu_arch in host_cpu_archs:
caps.host.cpu.arch = host_cpu_arch
os_mach_type = drvr._get_machine_type(image_meta, caps)
mock_get_arch.return_value = host_cpu_arch
os_mach_type = libvirt_utils.get_machine_type(image_meta)
self.assertEqual('s390-ccw-virtio', os_mach_type)
def test_get_guest_config_machine_type_through_image_meta(self):

22
nova/virt/libvirt/blockinfo.py
View File

@ -222,14 +222,14 @@ def get_disk_bus_for_device_type(instance,
device_type="disk"):
"""Determine the best disk bus to use for a device type.
Considering the currently configured virtualization
type, return the optimal disk_bus to use for a given
device type. For example, for a disk on KVM it will
return 'virtio', while for a CDROM it will return 'ide'
on x86_64 and 'scsi' on ppc64.
Considering the currently configured virtualization type, return the
optimal disk_bus to use for a given device type. For example, for a disk
on KVM it will return 'virtio', while for a CDROM, it will return 'ide'
for the 'pc' machine type on x86_64, 'sata' for the 'q35' machine type on
x86_64 and 'scsi' on ppc64.
Returns the disk_bus, or returns None if the device
type is not supported for this virtualization
Returns the disk_bus, or returns None if the device type is not supported
for this virtualization
"""
# Prefer a disk bus set against the image first of all
@ -268,6 +268,14 @@ def get_disk_bus_for_device_type(instance,
obj_fields.Architecture.S390X,
obj_fields.Architecture.AARCH64):
return "scsi"
machine_type = libvirt_utils.get_machine_type(image_meta)
# NOTE(lyarwood): We can't be any more explicit here as QEMU
# provides a version of the Q35 machine type per release.
# Additionally downstream distributions can also provide their own.
if machine_type and 'q35' in machine_type:
# NOTE(lyarwood): The Q35 machine type does not provide an IDE
# bus and as such we must use a SATA bus for cdroms.
return "sata"
else:
return "ide"
elif device_type == "disk":

32
nova/virt/libvirt/driver.py
View File

@ -4249,36 +4249,6 @@ class LibvirtDriver(driver.ComputeDriver):
return meta
def _get_machine_type(self, image_meta, caps):
# The guest machine type can be set as an image metadata
# property, or otherwise based on architecture-specific
# defaults.
mach_type = None
if image_meta.properties.get('hw_machine_type') is not None:
mach_type = image_meta.properties.hw_machine_type
else:
# NOTE(kchamart): For ARMv7 and AArch64, use the 'virt'
# board as the default machine type. It is the recommended
# board, which is designed to be used with virtual machines.
# The 'virt' board is more flexible, supports PCI, 'virtio',
# has decent RAM limits, etc.
if caps.host.cpu.arch in (fields.Architecture.ARMV7,
fields.Architecture.AARCH64):
mach_type = "virt"
if caps.host.cpu.arch in (fields.Architecture.S390,
fields.Architecture.S390X):
mach_type = 's390-ccw-virtio'
# If set in the config, use that as the default.
mach_type = (
libvirt_utils.get_default_machine_type(caps.host.cpu.arch)
or mach_type
)
return mach_type
@staticmethod
def _create_idmaps(klass, map_strings):
idmaps = []
@ -4952,7 +4922,7 @@ class LibvirtDriver(driver.ComputeDriver):
guest.os_loader_type = "pflash"
else:
raise exception.UEFINotSupported()
guest.os_mach_type = self._get_machine_type(image_meta, caps)
guest.os_mach_type = libvirt_utils.get_machine_type(image_meta)
if image_meta.properties.get('hw_boot_menu') is None:
guest.os_bootmenu = strutils.bool_from_string(
flavor.extra_specs.get('hw:boot_menu', 'no'))

31
nova/virt/libvirt/utils.py
View File

@ -541,6 +541,19 @@ def get_cpu_model_from_arch(arch):
return mode
def get_machine_type(image_meta):
"""The guest machine type can be set as an image metadata property, or
otherwise based on architecture-specific defaults. If no defaults are
found then None will be returned. This will ultimately lead to QEMU using
its own default which is currently the 'pc' machine type.
"""
if image_meta.properties.get('hw_machine_type') is not None:
return image_meta.properties.hw_machine_type
# If set in the config, use that as the default.
return get_default_machine_type(get_arch(image_meta))
def machine_type_mappings():
mappings = {}
for mapping in CONF.libvirt.hw_machine_type or {}:
@ -553,7 +566,23 @@ def machine_type_mappings():
def get_default_machine_type(arch):
return machine_type_mappings().get(arch)
# NOTE(lyarwood): Values defined in [libvirt]/hw_machine_type take
# precedence here if available for the provided arch.
machine_type = machine_type_mappings().get(arch)
if machine_type:
return machine_type
# NOTE(kchamart): For ARMv7 and AArch64, use the 'virt' board as the
# default machine type. It is the recommended board, which is designed
# to be used with virtual machines. The 'virt' board is more flexible,
# supports PCI, 'virtio', has decent RAM limits, etc.
if arch in (obj_fields.Architecture.ARMV7,
obj_fields.Architecture.AARCH64):
return "virt"
if arch in (obj_fields.Architecture.S390,
obj_fields.Architecture.S390X):
return "s390-ccw-virtio"
return None
def mdev_name2uuid(mdev_name):