diff --git a/nova/tests/fixtures/libvirt.py b/nova/tests/fixtures/libvirt.py index d328316037..a5e616f61d 100644 --- a/nova/tests/fixtures/libvirt.py +++ b/nova/tests/fixtures/libvirt.py @@ -17,7 +17,6 @@ import os import sys import textwrap import time -from unittest import mock import fixtures from lxml import etree @@ -1189,7 +1188,16 @@ class Domain(object): os_loader = tree.find('./os/loader') if os_loader is not None: + os['loader'] = os_loader.text + os['loader_type'] = os_loader.get('type') + os['loader_readonly'] = os_loader.get('readonly') os['loader_stateless'] = os_loader.get('stateless') + os['loader_secure'] = os_loader.get('secure') + + os_nvram = tree.find('./os/nvram') + if os_nvram is not None: + os['nvram'] = os_nvram.text + os['nvram_template'] = os_nvram.get('template') os_kernel = tree.find('./os/kernel') if os_kernel is not None: @@ -1554,10 +1562,35 @@ class Domain(object): pass def XMLDesc(self, flags): - loader = '' + loader_elems = [''] + if self._def['os'].get('loader_type'): + loader_elems.append( + "type='%s'" % self._def['os'].get('loader_type')) + if self._def['os'].get('loader_readonly'): + loader_elems.append( + "readonly='%s'" % self._def['os'].get('loader_readonly')) + if self._def['os'].get('loader_secure'): + loader_elems.append( + "secure='%s'" % self._def['os'].get('loader_secure')) if self._def['os'].get('loader_stateless'): - loader = ('' % - self._def['os'].get('loader_stateless')) + loader_elems.append( + "stateless='%s'" % self._def['os'].get('loader_stateless')) + + loader = '' + if self._def['os'].get('loader'): + loader = '%s' % ( + ' '.join(loader_elems), self._def['os'].get('loader')) + elif loader_elems: + loader = '' % ' '.join(loader_elems) + + nvram = '' + nvram_template = self._def['os'].get('nvram_template') + if nvram_template: + if not self._def['os'].get('nvram'): + nvram = "" % nvram_template + else: + nvram = "%s" % ( + nvram_template, self._def['os'].get('nvram')) disks = '' for disk in self._def['devices']['disks']: @@ -1749,6 +1782,7 @@ class Domain(object): hvm %(loader)s + %(nvram)s @@ -1800,6 +1834,7 @@ class Domain(object): 'iothreads': iothreads, 'arch': self._def['os']['arch'], 'loader': loader, + 'nvram': nvram, 'disks': disks, 'filesystems': filesystems, 'nics': nics, @@ -2710,91 +2745,6 @@ class LibvirtFixture(fixtures.Fixture): self.useFixture(fixtures.MonkeyPatch('os.path.exists', fake_exists)) - # ...and on all machine types - fake_loaders = [ - { - 'description': 'UEFI firmware for x86_64', - 'interface-types': ['uefi'], - 'mapping': { - 'device': 'flash', - 'executable': { - 'filename': '/usr/share/OVMF/OVMF_CODE.fd', - 'format': 'raw', - }, - 'nvram-template': { - 'filename': '/usr/share/OVMF/OVMF_VARS.fd', - 'format': 'raw', - }, - }, - 'targets': [ - { - 'architecture': 'x86_64', - 'machines': ['pc-i440fx-*', 'pc-q35-*'], - }, - ], - 'features': ['acpi-s3', 'amd-sev', 'verbose-dynamic'], - 'tags': [], - }, - { - 'description': 'UEFI firmware for x86_64, with SB+SMM', - 'interface-types': ['uefi'], - 'mapping': { - 'device': 'flash', - 'executable': { - 'filename': '/usr/share/OVMF/OVMF_CODE.secboot.fd', - 'format': 'raw', - }, - 'nvram-template': { - 'filename': '/usr/share/OVMF/OVMF_VARS.secboot.fd', - 'format': 'raw', - }, - }, - 'targets': [ - { - 'architecture': 'x86_64', - 'machines': ['pc-q35-*'], - }, - ], - 'features': [ - 'acpi-s3', - 'amd-sev', - 'enrolled-keys', - 'requires-smm', - 'secure-boot', - 'verbose-dynamic', - ], - 'tags': [], - }, - { - 'description': 'UEFI firmware for aarch64', - 'interface-types': ['uefi'], - 'mapping': { - 'device': 'flash', - 'executable': { - 'filename': '/usr/share/AAVMF/AAVMF_CODE.fd', - 'format': 'raw', - }, - 'nvram-template': { - 'filename': '/usr/share/AAVMF/AAVMF_VARS.fd', - 'format': 'raw', - } - }, - 'targets': [ - { - 'architecture': 'aarch64', - 'machines': ['virt-*'], - } - ], - 'features': ['verbose-static'], - "tags": [], - }, - ] - self.useFixture( - fixtures.MockPatch( - 'nova.virt.libvirt.host.Host.loaders', - new_callable=mock.PropertyMock, - return_value=fake_loaders)) - disable_event_thread(self) if self.stub_os_vif: diff --git a/nova/tests/functional/libvirt/test_uefi.py b/nova/tests/functional/libvirt/test_uefi.py index 87ca8ed7a4..e509addf50 100644 --- a/nova/tests/functional/libvirt/test_uefi.py +++ b/nova/tests/functional/libvirt/test_uefi.py @@ -14,8 +14,12 @@ # under the License. import datetime +import os import re +import textwrap +from unittest import mock +import fixtures from lxml import etree from oslo_log import log as logging from oslo_utils.fixture import uuidsentinel as uuids @@ -68,14 +72,16 @@ class UEFIServersTest(base.ServersTestBase): tree = etree.fromstring(xml) self.assertXmlEqual( """ - + + + + hvm - /usr/share/OVMF/OVMF_CODE.fd - + - """, # noqa: E501 + """, etree.tostring(tree.find('./os'), encoding='unicode')) return orig_create(xml, host) @@ -130,14 +136,16 @@ class UEFIServersTest(base.ServersTestBase): tree = etree.fromstring(xml) self.assertXmlEqual( """ - + + + + hvm - /usr/share/OVMF/OVMF_CODE.secboot.fd - + - """, # noqa: E501 + """, etree.tostring(tree.find('./os'), encoding='unicode')) return orig_create(xml, host) @@ -193,13 +201,16 @@ class UEFIServersTest(base.ServersTestBase): tree = etree.fromstring(xml) self.assertXmlEqual( """ - + + + + hvm - /usr/share/OVMF/OVMF_CODE.fd + - """, # noqa: E501 + """, etree.tostring(tree.find('./os'), encoding='unicode')) return orig_create(xml, host) @@ -255,9 +266,12 @@ class UEFIServersTest(base.ServersTestBase): tree = etree.fromstring(xml) self.assertXmlEqual( """ - + + + + hvm - /usr/share/OVMF/OVMF_CODE.secboot.fd + @@ -309,3 +323,351 @@ class UEFIServersTest(base.ServersTestBase): # ensure our instance's system_metadata field is correct self.assertInstanceHasUEFI(server, secure_boot=True, stateless=True) + + +class UEFIServersFirmwareTest(base.ServersTestBase): + + def test_hard_reboot(self): + orig_path_exists = os.path.exists + + code_exists = True + nvram_template_exists = True + + def fake_path_exists(path): + if path == '/usr/share/OVMF/OVMF_CODE.fd': + return code_exists + elif path == '/usr/share/OVMF/OVMF_VARS.fd': + return nvram_template_exists + else: + return orig_path_exists(path) + + self.useFixture(fixtures.MonkeyPatch( + 'os.path.exists', fake_path_exists)) + + orig_create = nova.virt.libvirt.guest.Guest.create + + auto_select = True + secure = 'yes' + + def fake_create(cls, xml, host): + xml = re.sub('type arch.*machine', + 'type machine', xml) + tree = etree.fromstring(xml) + if not auto_select: + self.assertXmlEqual( + """ + + hvm + /usr/share/OVMF/OVMF_CODE.fd + /path/to/nvram + + + + """ % secure, # noqa: E501 + etree.tostring(tree.find('./os'), encoding='unicode')) + else: + self.assertXmlEqual( + """ + + + + + hvm + + + + + """ % (secure, secure), + etree.tostring(tree.find('./os'), encoding='unicode')) + # NOTE(tkajinam): Simulate edit by libvirt + tree.replace(tree.find('./os'), etree.fromstring( + textwrap.dedent(""" + + + + + hvm + /usr/share/OVMF/OVMF_CODE.fd + /path/to/nvram + + + + """ % (secure, secure), # noqa: E501 + ))) + xml = etree.tostring(tree, encoding='unicode', + pretty_print=True) + + return orig_create(xml, host) + + self.stub_out('nova.virt.libvirt.guest.Guest.create', fake_create) + + compute = self.start_compute() + + # ensure we are reporting the correct trait + traits = self._get_provider_traits(self.compute_rp_uuids[compute]) + self.assertIn('COMPUTE_SECURITY_UEFI_SECURE_BOOT', traits) + + # create a server with UEFI and secure boot + timestamp = datetime.datetime( + 2021, 1, 2, 3, 4, 5, tzinfo=datetime.timezone.utc + ) + uefi_image = { + 'id': uuids.uefi_image, + 'name': 'uefi_image', + 'created_at': timestamp, + 'updated_at': timestamp, + 'deleted_at': None, + 'deleted': False, + 'status': 'active', + 'is_public': False, + 'container_format': 'ova', + 'disk_format': 'vhd', + 'size': 74185822, + 'min_ram': 0, + 'min_disk': 0, + 'protected': False, + 'visibility': 'public', + 'tags': [], + 'properties': { + 'hw_machine_type': 'q35', + 'hw_firmware_type': 'uefi', + 'os_secure_boot': 'optional', + } + } + self.glance.create(None, uefi_image) + + # Initial creation + server = self._create_server(image_uuid=uuids.uefi_image) + self._stop_server(server) + + # if the domain exists, use the loaders which were already selected + auto_select = False + self._start_server(server) + self._stop_server(server) + + # if code file does not exist, ignore the existing loader + code_exists = False + auto_select = True + self._start_server(server) + self._stop_server(server) + code_exists = True + + # if nvram template file does not exist, ignore the existing loader + nvram_template_exists = False + auto_select = True + self._start_server(server) + self._stop_server(server) + nvram_template_exists = True + + # the host lost secure boot support and the secure flag has been + # changed from true to false. + self.computes[compute].driver._host._supports_secure_boot = False + secure = 'no' + # if secure boot flag is changed, ignore the existing loader + auto_select = True + self._start_server(server) + self._stop_server(server) + + # if the domain exists, use the loaders which were already selected + auto_select = False + self._start_server(server) + self._stop_server(server) + + # the host regain secure boot support and the secure flag has been + # changed from false to true. + self.computes[compute].driver._host._supports_secure_boot = True + secure = 'yes' + # if secure boot flag is changed, ignore the existing loader + auto_select = True + self._start_server(server) + + def test_rebuild(self): + orig_path_exists = os.path.exists + + def fake_path_exists(path): + if path == '/usr/share/OVMF/OVMF_CODE.fd': + return True + elif path == '/usr/share/OVMF/OVMF_VARS.fd': + return True + else: + return orig_path_exists(path) + + self.useFixture(fixtures.MonkeyPatch( + 'os.path.exists', fake_path_exists)) + + orig_create = nova.virt.libvirt.guest.Guest.create + + def fake_create(cls, xml, host): + xml = re.sub('type arch.*machine', + 'type machine', xml) + tree = etree.fromstring(xml) + self.assertXmlEqual( + """ + + + + + hvm + + + + + """, + etree.tostring(tree.find('./os'), encoding='unicode')) + # NOTE(tkajinam): Simulate edit by libvirt + tree.replace(tree.find('./os'), etree.fromstring( + textwrap.dedent(""" + + + + + hvm + /usr/share/OVMF/OVMF_CODE.fd + /path/to/nvram + + + + """, # noqa: E501 + ))) + xml = etree.tostring(tree, encoding='unicode', pretty_print=True) + + return orig_create(xml, host) + + self.stub_out('nova.virt.libvirt.guest.Guest.create', fake_create) + + compute = self.start_compute() + + # ensure we are reporting the correct trait + traits = self._get_provider_traits(self.compute_rp_uuids[compute]) + self.assertIn('COMPUTE_SECURITY_UEFI_SECURE_BOOT', traits) + + # create a server with UEFI + timestamp = datetime.datetime( + 2021, 1, 2, 3, 4, 5, tzinfo=datetime.timezone.utc + ) + uefi_image = { + 'id': uuids.uefi_image, + 'name': 'uefi_image', + 'created_at': timestamp, + 'updated_at': timestamp, + 'deleted_at': None, + 'deleted': False, + 'status': 'active', + 'is_public': False, + 'container_format': 'ova', + 'disk_format': 'vhd', + 'size': 74185822, + 'min_ram': 0, + 'min_disk': 0, + 'protected': False, + 'visibility': 'public', + 'tags': [], + 'properties': { + 'hw_machine_type': 'q35', + 'hw_firmware_type': 'uefi', + } + } + self.glance.create(None, uefi_image) + + # Initial creation + server = self._create_server(image_uuid=uuids.uefi_image) + + # In rebuild, the previous xml is destroyed thus firmware is again + # auto-selected. + self._rebuild_server(server, uuids.uefi_image) + + def test_resize(self): + orig_path_exists = os.path.exists + + def fake_path_exists(path): + if path == '/usr/share/OVMF/OVMF_CODE.fd': + return True + elif path == '/usr/share/OVMF/OVMF_VARS.fd': + return True + else: + return orig_path_exists(path) + + self.useFixture(fixtures.MonkeyPatch( + 'os.path.exists', fake_path_exists)) + + orig_create = nova.virt.libvirt.guest.Guest.create + + def fake_create(cls, xml, host): + xml = re.sub('type arch.*machine', + 'type machine', xml) + tree = etree.fromstring(xml) + self.assertXmlEqual( + """ + + + + + hvm + + + + + """, + etree.tostring(tree.find('./os'), encoding='unicode')) + # NOTE(tkajinam): Simulate edit by libvirt + tree.replace(tree.find('./os'), etree.fromstring( + textwrap.dedent(""" + + + + + hvm + /usr/share/OVMF/OVMF_CODE.fd + /path/to/nvram + + + + """, # noqa: E501 + ))) + xml = etree.tostring(tree, encoding='unicode', pretty_print=True) + + return orig_create(xml, host) + + self.stub_out('nova.virt.libvirt.guest.Guest.create', fake_create) + + self.start_compute('host1') + self.start_compute('host2') + + # create a server with UEFI + timestamp = datetime.datetime( + 2021, 1, 2, 3, 4, 5, tzinfo=datetime.timezone.utc + ) + uefi_image = { + 'id': uuids.uefi_image, + 'name': 'uefi_image', + 'created_at': timestamp, + 'updated_at': timestamp, + 'deleted_at': None, + 'deleted': False, + 'status': 'active', + 'is_public': False, + 'container_format': 'ova', + 'disk_format': 'vhd', + 'size': 74185822, + 'min_ram': 0, + 'min_disk': 0, + 'protected': False, + 'visibility': 'public', + 'tags': [], + 'properties': { + 'hw_machine_type': 'q35', + 'hw_firmware_type': 'uefi', + } + } + self.glance.create(None, uefi_image) + + # Initial creation + server = self._create_server(image_uuid=uuids.uefi_image) + + # In cold-migration, the previous xml is destroyed so firmware should + # be auto-selected. + with mock.patch( + 'nova.virt.libvirt.driver.LibvirtDriver' + '.migrate_disk_and_power_off', return_value='{}', + ): + self._resize_server(server, self.api.get_flavors()[1]['id']) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 104502e36d..8ab5614fc1 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -6074,6 +6074,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, def test_get_guest_config_with_uefi(self): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + drvr._host._supports_amd_sev = False + drvr._host._supports_uefi = True image_meta = objects.ImageMeta.from_dict({ "disk_format": "raw", @@ -6084,12 +6086,145 @@ class LibvirtConnTestCase(test.NoDBTestCase, CONF.libvirt.virt_type, instance_ref, image_meta) cfg = drvr._get_guest_config( instance_ref, [], image_meta, disk_info) - # these paths are derived from the FakeLibvirtFixture - self.assertEqual('/usr/share/OVMF/OVMF_CODE.fd', cfg.os_loader) - self.assertEqual('/usr/share/OVMF/OVMF_VARS.fd', cfg.os_nvram_template) + + self.assertEqual('efi', cfg.os_firmware) + self.assertIsNone(cfg.os_loader_type) + self.assertIsNone(cfg.os_loader_readonly) + self.assertFalse(cfg.os_loader_secure) + self.assertIsNone(cfg.os_loader_stateless) + self.assertIsNone(cfg.os_loader) + self.assertIsNone(cfg.os_nvram) + self.assertIsNone(cfg.os_nvram_template) + + @mock.patch('nova.virt.libvirt.driver.os.path.exists') + def test_get_guest_config_with_uefi_old_guest(self, mock_exists): + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + drvr._host._supports_amd_sev = False + drvr._host._supports_uefi = True + + image_meta = objects.ImageMeta.from_dict({ + "disk_format": "raw", + "properties": {"hw_firmware_type": "uefi"}}) + instance_ref = objects.Instance(**self.test_instance) + + loader = 'LOADER' + nvram = 'NVRAM' + nvram_template = 'NVRAM_TEMPLATE' + + old_guest = vconfig.LibvirtConfigGuest() + old_guest.os_loader_type = 'pflash' + old_guest.os_loader_readonly = True + old_guest.os_loader_secure = False + old_guest.os_loader = loader + old_guest.os_nvram = nvram + old_guest.os_nvram_template = nvram_template + + mock_exists.return_value = True + + disk_info = blockinfo.get_disk_info( + CONF.libvirt.virt_type, instance_ref, image_meta) + cfg = drvr._get_guest_config( + instance_ref, [], image_meta, disk_info, old_guest=old_guest) + + self.assertIsNone(cfg.os_firmware) + self.assertEqual('pflash', cfg.os_loader_type) + self.assertTrue(cfg.os_loader_readonly) + self.assertFalse(cfg.os_loader_secure) + self.assertIsNone(cfg.os_loader_stateless) + self.assertEqual(loader, cfg.os_loader) + self.assertEqual(nvram, cfg.os_nvram) + self.assertEqual(nvram_template, cfg.os_nvram_template) + + @mock.patch('nova.virt.libvirt.driver.os.path.exists') + def test_get_guest_config_with_uefi_old_guest_loader_not_found( + self, mock_exists): + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + drvr._host._supports_amd_sev = False + drvr._host._supports_uefi = True + + image_meta = objects.ImageMeta.from_dict({ + "disk_format": "raw", + "properties": {"hw_firmware_type": "uefi"}}) + instance_ref = objects.Instance(**self.test_instance) + + loader = 'LOADER' + nvram = 'NVRAM' + nvram_template = 'NVRAM_TEMPLATE' + + old_guest = vconfig.LibvirtConfigGuest() + old_guest.os_loader_type = 'pflash' + old_guest.os_loader_readonly = True + old_guest.os_loader_secure = False + old_guest.os_loader = loader + old_guest.os_nvram = nvram + old_guest.os_nvram_template = nvram_template + + def mock_func(path): + return path != 'LOADER' + + mock_exists.side_effect = mock_func + + disk_info = blockinfo.get_disk_info( + CONF.libvirt.virt_type, instance_ref, image_meta) + cfg = drvr._get_guest_config( + instance_ref, [], image_meta, disk_info, old_guest=old_guest) + + self.assertEqual('efi', cfg.os_firmware) + self.assertIsNone(cfg.os_loader_type) + self.assertIsNone(cfg.os_loader_readonly) + self.assertFalse(cfg.os_loader_secure) + self.assertIsNone(cfg.os_loader_stateless) + self.assertIsNone(cfg.os_loader) + self.assertIsNone(cfg.os_nvram) + self.assertIsNone(cfg.os_nvram_template) + + @mock.patch('nova.virt.libvirt.driver.os.path.exists') + def test_get_guest_config_with_uefi_old_guest_nvram_not_found( + self, mock_exists): + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + drvr._host._supports_amd_sev = False + drvr._host._supports_uefi = True + + image_meta = objects.ImageMeta.from_dict({ + "disk_format": "raw", + "properties": {"hw_firmware_type": "uefi"}}) + instance_ref = objects.Instance(**self.test_instance) + + loader = 'LOADER' + nvram = 'NVRAM' + nvram_template = 'NVRAM_TEMPLATE' + + old_guest = vconfig.LibvirtConfigGuest() + old_guest.os_loader_type = 'pflash' + old_guest.os_loader_readonly = True + old_guest.os_loader_secure = False + old_guest.os_loader = loader + old_guest.os_nvram = nvram + old_guest.os_nvram_template = nvram_template + + def mock_func(path): + return path != 'NVRAM_TEMPLATE' + + mock_exists.side_effect = mock_func + + disk_info = blockinfo.get_disk_info( + CONF.libvirt.virt_type, instance_ref, image_meta) + cfg = drvr._get_guest_config( + instance_ref, [], image_meta, disk_info, old_guest=old_guest) + + self.assertEqual('efi', cfg.os_firmware) + self.assertIsNone(cfg.os_loader_type) + self.assertIsNone(cfg.os_loader_readonly) + self.assertFalse(cfg.os_loader_secure) + self.assertIsNone(cfg.os_loader_stateless) + self.assertIsNone(cfg.os_loader) + self.assertIsNone(cfg.os_nvram) + self.assertIsNone(cfg.os_nvram_template) def test_get_guest_config_with_uefi_and_stateless_firmware(self): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + drvr._host._supports_amd_sev = False + drvr._host._supports_uefi = True image_meta = objects.ImageMeta.from_dict({ "disk_format": "raw", @@ -6104,54 +6239,22 @@ class LibvirtConnTestCase(test.NoDBTestCase, CONF.libvirt.virt_type, instance_ref, image_meta) cfg = drvr._get_guest_config( instance_ref, [], image_meta, disk_info) - # these paths are derived from the FakeLibvirtFixture - self.assertEqual('/usr/share/OVMF/OVMF_CODE.fd', cfg.os_loader) + + self.assertEqual('efi', cfg.os_firmware) + self.assertIsNone(cfg.os_loader_type) + self.assertIsNone(cfg.os_loader_readonly) + self.assertFalse(cfg.os_loader_secure) self.assertTrue(cfg.os_loader_stateless) + self.assertIsNone(cfg.os_loader) + self.assertIsNone(cfg.os_nvram) self.assertIsNone(cfg.os_nvram_template) - def test_get_guest_config_with_secure_boot_and_smm_required(self): - drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) - # uefi only used with secure boot - drvr._host._supports_uefi = True - # smm only used with secure boot - drvr._host._supports_secure_boot = True - - # NOTE(imranh2): Current way of gathering firmwares is inflexible - # nova/tests/fixtures/libvirt.py FakeLoaders has requires-smm - # defined. do the following to make sure we get this programtically - # in the future we should test firmwares that both do and don't - # require smm but the current way firmware is selected doesn't - # make it possible to do so. - loader, nvram_template, requires_smm = drvr._host.get_loader( - 'x86_64', 'q35', True) - - image_meta = objects.ImageMeta.from_dict({ - 'disk_format': 'raw', - # secure boot requires UEFI - 'properties': { - 'hw_firmware_type': 'uefi', - 'hw_machine_type': 'q35', - 'os_secure_boot': 'required', - }, - }) - instance_ref = objects.Instance(**self.test_instance) - - disk_info = blockinfo.get_disk_info( - CONF.libvirt.virt_type, instance_ref, image_meta) - - cfg = drvr._get_guest_config( - instance_ref, [], image_meta, disk_info) - # if we require it make sure it's there - if requires_smm: - self.assertTrue(any(isinstance(feature, - vconfig.LibvirtConfigGuestFeatureSMM) - for feature in cfg.features)) - @ddt.data(True, False) def test_get_guest_config_with_secure_boot_required( self, host_has_support, ): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + drvr._host._supports_amd_sev = False drvr._host._supports_uefi = True drvr._host._supports_secure_boot = host_has_support @@ -6172,12 +6275,13 @@ class LibvirtConnTestCase(test.NoDBTestCase, # if the host supports it, we should get the feature cfg = drvr._get_guest_config( instance_ref, [], image_meta, disk_info) - # these paths are derived from the FakeLibvirtFixture - self.assertEqual( - '/usr/share/OVMF/OVMF_CODE.secboot.fd', cfg.os_loader) - self.assertEqual( - '/usr/share/OVMF/OVMF_VARS.secboot.fd', cfg.os_nvram_template) + self.assertEqual('efi', cfg.os_firmware) + self.assertIsNone(cfg.os_loader_type) + self.assertIsNone(cfg.os_loader_readonly) self.assertTrue(cfg.os_loader_secure) + self.assertIsNone(cfg.os_loader) + self.assertIsNone(cfg.os_nvram) + self.assertIsNone(cfg.os_nvram_template) else: # if not, we should see an exception self.assertRaises( @@ -6190,6 +6294,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, self, host_has_support, ): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + drvr._host._supports_amd_sev = False drvr._host._supports_uefi = True drvr._host._supports_secure_boot = host_has_support @@ -6209,20 +6314,119 @@ class LibvirtConnTestCase(test.NoDBTestCase, cfg = drvr._get_guest_config( instance_ref, [], image_meta, disk_info) if host_has_support: - # if the host supports it we should get the feature - self.assertEqual( - '/usr/share/OVMF/OVMF_CODE.secboot.fd', cfg.os_loader) - self.assertEqual( - '/usr/share/OVMF/OVMF_VARS.secboot.fd', cfg.os_nvram_template) self.assertTrue(cfg.os_loader_secure) else: - # if not, silently ignore - self.assertEqual( - '/usr/share/OVMF/OVMF_CODE.fd', cfg.os_loader) - self.assertEqual( - '/usr/share/OVMF/OVMF_VARS.fd', cfg.os_nvram_template) self.assertFalse(cfg.os_loader_secure) + self.assertEqual('efi', cfg.os_firmware) + self.assertIsNone(cfg.os_loader_type) + self.assertIsNone(cfg.os_loader_readonly) + self.assertIsNone(cfg.os_loader) + self.assertIsNone(cfg.os_nvram) + self.assertIsNone(cfg.os_nvram_template) + self.assertNotIn(vconfig.LibvirtConfigGuestFeatureSMM(), cfg.features) + + @ddt.data(True, False) + @mock.patch('nova.virt.libvirt.driver.os.path.exists') + def test_get_guest_config_with_secure_boot_old_guest( + self, mock_exists, smm): + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + drvr._host._supports_amd_sev = False + drvr._host._supports_uefi = True + drvr._host._supports_secure_boot = True + + image_meta = objects.ImageMeta.from_dict({ + 'disk_format': 'raw', + # secure boot requires UEFI + 'properties': { + 'hw_firmware_type': 'uefi', + 'hw_machine_type': 'q35', + 'os_secure_boot': 'required', + }, + }) + instance_ref = objects.Instance(**self.test_instance) + + loader = 'LOADER' + nvram = 'NVRAM' + nvram_template = 'NVRAM_TEMPLATE' + + old_guest = vconfig.LibvirtConfigGuest() + old_guest.os_loader_type = 'pflash' + old_guest.os_loader_readonly = True + old_guest.os_loader_secure = True + old_guest.os_loader = loader + old_guest.os_nvram = nvram + old_guest.os_nvram_template = nvram_template + if smm: + old_guest.features.append(vconfig.LibvirtConfigGuestFeatureSMM()) + + disk_info = blockinfo.get_disk_info( + CONF.libvirt.virt_type, instance_ref, image_meta) + cfg = drvr._get_guest_config( + instance_ref, [], image_meta, disk_info, old_guest=old_guest) + self.assertEqual('pflash', cfg.os_loader_type) + self.assertTrue(cfg.os_loader_readonly) + self.assertTrue(cfg.os_loader_secure) + self.assertIsNone(cfg.os_loader_stateless) + self.assertEqual(loader, cfg.os_loader) + self.assertEqual(nvram, cfg.os_nvram) + self.assertEqual(nvram_template, cfg.os_nvram_template) + if smm: + self.assertIn(vconfig.LibvirtConfigGuestFeatureSMM(), + cfg.features) + else: + self.assertNotIn(vconfig.LibvirtConfigGuestFeatureSMM(), + cfg.features) + + @mock.patch('nova.virt.libvirt.driver.os.path.exists') + def test_get_guest_config_with_uefi_old_guest_sb_changed( + self, mock_exists): + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + drvr._host._supports_amd_sev = False + drvr._host._supports_uefi = True + drvr._host._supports_secure_boot = False + + image_meta = objects.ImageMeta.from_dict({ + "disk_format": "raw", + "properties": { + 'hw_firmware_type': 'uefi', + 'hw_machine_type': 'q35', + 'os_secure_boot': 'optional', + } + }) + instance_ref = objects.Instance(**self.test_instance) + + loader = 'LOADER' + nvram = 'NVRAM' + nvram_template = 'NVRAM_TEMPLATE' + + # Secure boot was enabled previously + old_guest = vconfig.LibvirtConfigGuest() + old_guest.os_loader_type = 'pflash' + old_guest.os_loader_readonly = True + old_guest.os_loader_secure = True + old_guest.os_loader = loader + old_guest.os_nvram = nvram + old_guest.os_nvram_template = nvram_template + old_guest.features.append(vconfig.LibvirtConfigGuestFeatureSMM()) + + mock_exists.return_value = True + + disk_info = blockinfo.get_disk_info( + CONF.libvirt.virt_type, instance_ref, image_meta) + cfg = drvr._get_guest_config( + instance_ref, [], image_meta, disk_info, old_guest=old_guest) + + self.assertEqual('efi', cfg.os_firmware) + self.assertIsNone(cfg.os_loader_type) + self.assertIsNone(cfg.os_loader_readonly) + self.assertFalse(cfg.os_loader_secure) + self.assertIsNone(cfg.os_loader_stateless) + self.assertIsNone(cfg.os_loader) + self.assertIsNone(cfg.os_nvram) + self.assertIsNone(cfg.os_nvram_template) + self.assertNotIn(vconfig.LibvirtConfigGuestFeatureSMM(), cfg.features) + def test_get_guest_config_with_block_device(self): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) @@ -18517,9 +18721,13 @@ class LibvirtConnTestCase(test.NoDBTestCase, @mock.patch('nova.virt.libvirt.LibvirtDriver.destroy') @mock.patch('nova.virt.libvirt.LibvirtDriver.' '_get_all_assigned_mediated_devices') - def test_hard_reboot(self, mock_get_mdev, mock_destroy, mock_get_disk_info, - mock_get_guest_xml, mock_create_guest_with_network, - mock_get_info, mock_metadata, mock_save): + @mock.patch('nova.virt.libvirt.LibvirtDriver.' + '_get_existing_guest_config') + def test_hard_reboot( + self, mock_get_guest, mock_get_mdev, mock_destroy, mock_get_disk_info, + mock_get_guest_xml, mock_create_guest_with_network, + mock_get_info, mock_metadata, mock_save + ): self.context.auth_token = True # any non-None value will suffice instance = objects.Instance(**self.test_instance) network_info = _fake_network_info(self) @@ -18535,6 +18743,10 @@ class LibvirtConnTestCase(test.NoDBTestCase, "" "") + guest = vconfig.LibvirtConfigGuest() + guest.parse_dom(etree.fromstring(dummyxml)) + mock_get_guest.return_value = guest + mock_get_mdev.return_value = {uuids.mdev1: uuids.inst1} drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) @@ -18585,7 +18797,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, mock_get_guest_xml.assert_called_once_with(self.context, instance, network_info, mock.ANY, mock.ANY, block_device_info=block_device_info, mdevs=[uuids.mdev1], - accel_info=accel_info, share_info=share_info) + accel_info=accel_info, share_info=share_info, + old_guest=guest) mock_create_guest_with_network.assert_called_once_with( self.context, dummyxml, instance, network_info, block_device_info, vifs_already_plugged=True, @@ -18605,8 +18818,10 @@ class LibvirtConnTestCase(test.NoDBTestCase, @mock.patch('nova.virt.libvirt.LibvirtDriver.destroy') @mock.patch('nova.virt.libvirt.LibvirtDriver.' '_get_all_assigned_mediated_devices') + @mock.patch('nova.virt.libvirt.LibvirtDriver.' + '_get_existing_guest_config') def test_hard_reboot_with_share_info( - self, mock_get_mdev, mock_destroy, mock_get_disk_info, + self, mock_get_guest, mock_get_mdev, mock_destroy, mock_get_disk_info, mock_get_guest_xml, mock_create_guest_with_network, mock_get_info, mock_attach, mock_metadata, mock_save ): @@ -18625,6 +18840,10 @@ class LibvirtConnTestCase(test.NoDBTestCase, "" "") + guest = vconfig.LibvirtConfigGuest() + guest.parse_dom(etree.fromstring(dummyxml)) + mock_get_guest.return_value = guest + mock_get_mdev.return_value = {uuids.mdev1: uuids.inst1} drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) @@ -18690,7 +18909,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, mock_get_guest_xml.assert_called_once_with(self.context, instance, network_info, mock.ANY, mock.ANY, block_device_info=block_device_info, mdevs=[uuids.mdev1], - accel_info=accel_info, share_info=share_info) + accel_info=accel_info, share_info=share_info, + old_guest=guest) mock_create_guest_with_network.assert_called_once_with( self.context, dummyxml, instance, network_info, block_device_info, vifs_already_plugged=True, @@ -25499,7 +25719,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin): def fake_to_xml(self, context, instance, network_info, disk_info, image_meta=None, rescue=None, - block_device_info=None, mdevs=None): + block_device_info=None, mdevs=None, old_guest=None): return "" self.stub_out('nova.virt.libvirt.driver.LibvirtDriver._get_guest_xml', @@ -25527,16 +25747,21 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin): f = open(libvirt_xml_path, 'w') f.close() - with mock.patch.object( - self.drvr, '_get_all_assigned_mediated_devices', - return_value={} - ) as mock_get_a_mdevs: + with test.nested( + mock.patch.object( + self.drvr, '_get_all_assigned_mediated_devices', + return_value={}), + mock.patch.object( + self.drvr, '_get_existing_guest_config', + return_value=None), + ) as (mock_get_a_mdevs, mock_get_guest): self.drvr.finish_revert_migration( self.context, instance, network_model.NetworkInfo(), objects.Migration(), power_on=power_on) self.assertTrue(self.fake_create_guest_called) mock_get_a_mdevs.assert_called_once_with(mock.ANY) + mock_get_guest.assert_called_once_with(mock.ANY) def test_finish_revert_migration_power_on(self): self._test_finish_revert_migration(True) @@ -25603,7 +25828,8 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin): def test_finish_revert_migration_preserves_disk_bus(self): def fake_get_guest_xml(context, instance, network_info, disk_info, - image_meta, block_device_info=None, mdevs=None): + image_meta, block_device_info=None, mdevs=None, + old_guest=None): self.assertEqual('ide', disk_info['disk_bus']) image_meta = {"disk_format": "raw", @@ -27511,6 +27737,8 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin): @mock.patch('nova.objects.block_device.BlockDeviceMapping.save', new=mock.Mock()) @mock.patch('nova.objects.image_meta.ImageMeta.from_image_ref') + @mock.patch('nova.virt.libvirt.LibvirtDriver.' + '_get_existing_guest_config') @mock.patch('nova.virt.libvirt.LibvirtDriver.' '_get_all_assigned_mediated_devices') # NOTE(mdbooth): The following 4 mocks are required to execute @@ -27523,8 +27751,9 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin): def _test_rescue( self, instance, mock_instance_metadata, mock_supports_direct_io, mock_build_device_metadata, mock_set_host_enabled, mock_get_mdev, - mock_get_image_meta_by_ref, image_meta_dict=None, exists=None, - instance_image_meta_dict=None, block_device_info=None, share_info=None + mock_get_guest, mock_get_image_meta_by_ref, image_meta_dict=None, + exists=None, instance_image_meta_dict=None, block_device_info=None, + share_info=None ): self.flags(instances_path=self.useFixture(fixtures.TempDir()).path) @@ -27532,6 +27761,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin): mock_supports_direct_io.return_value = True mock_get_mdev.return_value = {uuids.mdev1: uuids.inst1} + mock_get_guest.return_value = None backend = self.useFixture( nova_fixtures.LibvirtImageBackendFixture(exists=exists)) @@ -27840,6 +28070,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin): mock.patch.object(drvr, '_destroy'), mock.patch.object(drvr, '_get_guest_xml'), mock.patch.object(drvr, '_create_image'), + mock.patch.object(drvr, '_get_existing_guest_config'), mock.patch.object(drvr, '_get_existing_domain_xml'), mock.patch.object(libvirt_utils, 'get_instance_path'), mock.patch('nova.virt.libvirt.blockinfo.get_disk_info'), @@ -27848,10 +28079,12 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin): mock.patch('builtins.open', new_callable=mock.mock_open), ) as ( mock_create, mock_destroy, mock_get_guest_xml, mock_create_image, - mock_get_existing_xml, mock_inst_path, mock_get_disk_info, - mock_image_get, mock_from_dict, mock_open + mock_get_existing_guest, mock_get_existing_xml, + mock_inst_path, mock_get_disk_info, mock_image_get, mock_from_dict, + mock_open ): self.flags(virt_type='kvm', group='libvirt') + mock_get_existing_guest.return_value = None mock_image_get.return_value = mock.sentinel.bdm_image_meta_dict mock_from_dict.return_value = mock.sentinel.bdm_image_meta mock_get_disk_info.return_value = disk_info @@ -27880,7 +28113,8 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin): mock_get_guest_xml.assert_called_once_with( self.context, instance, network_info, disk_info, mock.sentinel.bdm_image_meta, rescue=mock.ANY, mdevs=mock.ANY, - block_device_info=block_device_info, share_info=share_info) + block_device_info=block_device_info, share_info=share_info, + old_guest=None) def test_rescue_stable_device_bfv(self): """Assert the disk layout when rescuing BFV instances""" @@ -29401,6 +29635,8 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin): @mock.patch.object(libvirt_driver.LibvirtDriver, '_build_device_metadata') @mock.patch.object(objects.Instance, 'save') + @mock.patch.object(libvirt_driver.LibvirtDriver, + '_get_existing_guest_config') @mock.patch.object( libvirt_driver.LibvirtDriver, '_get_all_assigned_mediated_devices', new=mock.Mock(return_value={})) @@ -29420,7 +29656,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin): 'oslo_service.loopingcall.FixedIntervalLoopingCall', new=mock.Mock()) def _test_hard_reboot_allocate_missing_mdevs( self, mock_get_xml, mock_image_meta, mock_allocate_mdevs, - mock_db, mock_build_metadata): + mock_get_guest, mock_db, mock_build_metadata): mock_compute = mock.Mock() mock_compute.reportclient.get_allocations_for_consumer.return_value = ( mock.sentinel.allocations) @@ -29433,6 +29669,9 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin): image_ref=uuids.image, flavor=objects.Flavor(extra_specs={'resources:VGPU': 1})) + old_guest = mock.Mock() + mock_get_guest.return_value = old_guest + share_info = objects.ShareMappingList() mock_build_metadata.return_value = objects.InstanceDeviceMetadata() drvr._hard_reboot( @@ -29442,6 +29681,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin): (mock_compute.reportclient.get_allocations_for_consumer. assert_called_once_with(ctxt, instance.uuid)) mock_allocate_mdevs.assert_called_once_with(mock.sentinel.allocations) + mock_get_guest.assert_called_once_with(instance) mock_get_xml.assert_called_once_with( ctxt, instance, @@ -29452,6 +29692,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin): mdevs=mock_allocate_mdevs.return_value, accel_info=None, share_info=share_info, + old_guest=old_guest, ) return ctxt, mock_get_xml, instance diff --git a/nova/tests/unit/virt/libvirt/test_host.py b/nova/tests/unit/virt/libvirt/test_host.py index 62a2f4f4b4..b55628783f 100644 --- a/nova/tests/unit/virt/libvirt/test_host.py +++ b/nova/tests/unit/virt/libvirt/test_host.py @@ -21,7 +21,6 @@ import ddt import eventlet from eventlet import tpool from lxml import etree -from oslo_serialization import jsonutils from oslo_utils.fixture import uuidsentinel as uuids from oslo_utils import uuidutils from oslo_utils import versionutils @@ -2054,133 +2053,6 @@ Active: 8381604 kB mock_libversion.return_value = 7008000 self.assertFalse(self.host.supports_remote_managed_ports) - @mock.patch.object(host.Host, 'loaders', new_callable=mock.PropertyMock) - @mock.patch.object(host.Host, 'get_canonical_machine_type') - def test_get_loader(self, mock_get_mtype, mock_loaders): - loaders = [ - { - 'description': 'Sample descriptor', - 'interface-types': ['uefi'], - 'mapping': { - 'device': 'flash', - 'mode': 'split', - 'executable': { - 'filename': '/usr/share/edk2/ovmf/OVMF_CODE.fd', - 'format': 'raw', - }, - 'nvram-template': { - 'filename': '/usr/share/edk2/ovmf/OVMF_VARS.fd', - 'format': 'raw', - }, - }, - 'targets': [ - { - 'architecture': 'x86_64', - 'machines': ['pc-q35-*'], # exclude pc-i440fx-* - }, - ], - 'features': ['acpi-s3', 'amd-sev', 'verbose-dynamic'], - 'tags': [], - }, - # NOTE(tkajinam): The following loaders are not supported and - # should be ignored. https://bugs.launchpad.net/nova/+bug/2122288 - { - 'description': 'Sample descriptor for stateless mode', - 'interface-types': ['uefi'], - 'mapping': { - 'device': 'flash', - 'mode': 'stateless', - 'executable': { - 'filename': '/usr/share/edk2/ovmf/OVMF_CODE_SL.fd', - 'format': 'raw' - } - }, - 'targets': [ - { - 'architecture': 'x86_64', - 'machines': ['pc-q35-*'], - }, - ], - 'features': ['amd-sev', 'verbose-dynamic'], - 'tags': [], - }, - { - 'description': 'Sample descriptor for memory device', - 'interface-types': ['uefi'], - 'mapping': { - 'device': 'memory', - 'filename': '/usr/share/edk2/ovmf/OVMF_MEM.fd' - }, - 'targets': [ - { - 'architecture': 'x86_64', - 'machines': ['pc-q35-*'], - } - ], - 'features': ['amd-sev', 'verbose-dynamic'], - 'tags': [], - }, - ] - - def fake_get_mtype(arch, machine): - return { - 'x86_64': { - 'pc': 'pc-i440fx-5.1', - 'q35': 'pc-q35-5.1', - }, - 'aarch64': { - 'virt': 'virt-5.1', - }, - }[arch][machine] - - mock_get_mtype.side_effect = fake_get_mtype - mock_loaders.return_value = loaders - - # this should pass because we're not reporting the secure-boot feature - # which is what we don't want - loader = self.host.get_loader('x86_64', 'q35', has_secure_boot=False) - self.assertIsNotNone(loader) - - # while it should fail here since we want it now - self.assertRaises( - exception.UEFINotSupported, - self.host.get_loader, - 'x86_64', 'q35', has_secure_boot=True) - - # it should also fail for an unsupported architecture - self.assertRaises( - exception.UEFINotSupported, - self.host.get_loader, - 'aarch64', 'virt', has_secure_boot=False) - - # or an unsupported machine type - self.assertRaises( - exception.UEFINotSupported, - self.host.get_loader, - 'x86_64', 'pc', has_secure_boot=False) - - # add the secure-boot feature flag - loaders[0]['features'].append('secure-boot') - - # this should pass because we're reporting the secure-boot feature - # which is what we want - loader = self.host.get_loader('x86_64', 'q35', has_secure_boot=True) - self.assertIsNotNone(loader) - - # check that SMM bool is false as we don't need it - self.assertFalse(loader[2]) - - # check that we get SMM bool correctly (True) when required - loaders[0]['features'].append('requires-smm') - loader = self.host.get_loader('x86_64', 'q35', has_secure_boot=True) - self.assertTrue(loader[2]) - - # while it should fail here since we don't want it now - self.assertRaises( - exception.UEFINotSupported, - self.host.get_loader, - 'x86_64', 'q35', has_secure_boot=False) - vc = fakelibvirt.virConnect @@ -2447,51 +2319,3 @@ class LibvirtTpoolProxyTestCase(test.NoDBTestCase): self.assertEqual(1, len(dev_names)) for name in dev_names: self.assertIsInstance(name, str) - - -class LoadersTestCase(test.NoDBTestCase): - - def test_loaders(self): - loader = { - 'description': 'Sample descriptor', - 'interface-types': ['uefi'], - 'mapping': { - 'device': 'flash', - 'executable': { - 'filename': '/usr/share/edk2/ovmf/OVMF_CODE.fd', - 'format': 'raw', - }, - 'nvram-template': { - 'filename': '/usr/share/edk2/ovmf/OVMF_VARS.fd', - 'format': 'raw', - }, - }, - 'targets': [ - { - 'architecture': 'x86_64', - 'machines': ['pc-i440fx-*', 'pc-q35-*'], - }, - ], - 'features': ['acpi-s3', 'amd-sev', 'verbose-dynamic'], - 'tags': [], - } - - m = mock.mock_open(read_data=jsonutils.dumps(loader).encode('utf-8')) - with test.nested( - mock.patch.object( - os.path, 'exists', - side_effect=lambda path: path == '/usr/share/qemu/firmware'), - mock.patch('glob.glob', return_value=['10_fake.json']), - mock.patch('builtins.open', m, create=True), - ) as (mock_exists, mock_glob, mock_open): - loaders = host._get_loaders() - - self.assertEqual(loaders, [loader]) - - mock_exists.assert_has_calls([ - mock.call('/usr/share/qemu/firmware'), - mock.call('/etc/qemu/firmware'), - ]) - mock_glob.assert_called_once_with( - '/usr/share/qemu/firmware/*.json') - mock_open.assert_called_once_with('10_fake.json', 'rb') diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 6e8a5b4306..1f9ef61f4a 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -4141,6 +4141,9 @@ class LibvirtDriver(driver.ComputeDriver): # need to remember the existing mdevs for reusing them. mdevs = self._get_all_assigned_mediated_devices(instance) mdevs = list(mdevs.keys()) + + old_guest = self._get_existing_guest_config(instance) + # NOTE(mdbooth): In addition to performing a hard reboot of the domain, # the hard reboot operation is relied upon by operators to be an # automated attempt to fix as many things as possible about a @@ -4186,7 +4189,8 @@ class LibvirtDriver(driver.ComputeDriver): instance.image_meta, block_device_info=block_device_info, mdevs=mdevs, accel_info=accel_info, - share_info=share_info) + share_info=share_info, + old_guest=old_guest) # NOTE(mdbooth): context.auth_token will not be set when we call # _hard_reboot from resume_state_on_host_boot() @@ -4650,6 +4654,9 @@ class LibvirtDriver(driver.ComputeDriver): # remember the existing mdevs for reusing them. mdevs = self._get_all_assigned_mediated_devices(instance) mdevs = list(mdevs.keys()) + + old_guest = self._get_existing_guest_config(instance) + self._create_image(context, instance, disk_info['mapping'], injection_info=injection_info, suffix='.rescue', disk_images=rescue_images) @@ -4659,7 +4666,7 @@ class LibvirtDriver(driver.ComputeDriver): image_meta, rescue=rescue_images, mdevs=mdevs, block_device_info=block_device_info, - share_info=share_info) + share_info=share_info, old_guest=old_guest) self._destroy(instance) self._create_guest( context, xml, instance, post_xml_callback=gen_confdrive, @@ -7111,12 +7118,55 @@ class LibvirtDriver(driver.ComputeDriver): return supported_events + def _copy_guest_firmware_elements( + self, + old_guest: vconfig.LibvirtConfigGuest, + guest: vconfig.LibvirtConfigGuest, + ) -> None: + loader = old_guest.os_loader + nvram_template = old_guest.os_nvram_template + + if guest.os_loader_secure != old_guest.os_loader_secure: + LOG.warning('Secure boot support was changed ' + 'after this instance had been created. ' + 'Re-selecting the firmware files.') + # TODO(tkajinam): VIR_DOMAIN_START_RESET_NVRAM should + # be added to guest.start if the hard_reboot method is modified + # so that it keeps NVRAM file. + elif loader and not os.path.exists(loader): + # NOTE(tkajinam): Loader does not exist in this host + LOG.debug('The previous loader file %s does not ' + 'exist. Force re-selection of firmware.', + loader) + elif nvram_template and not os.path.exists(nvram_template): + LOG.debug('The previous nvram template file %s does ' + 'not exist. Force re-selection of firmware.', + nvram_template) + else: + # Disable firmware re-selection + guest.os_firmware = None + + guest.os_loader = old_guest.os_loader + guest.os_loader_type = old_guest.os_loader_type + guest.os_loader_readonly = \ + old_guest.os_loader_readonly + guest.os_nvram = old_guest.os_nvram + guest.os_nvram_template = old_guest.os_nvram_template + + # if the feature set says we need SMM then enable it + for f in old_guest.features: + if f == vconfig.LibvirtConfigGuestFeatureSMM(): + guest.features.append( + vconfig.LibvirtConfigGuestFeatureSMM()) + break + def _configure_guest_by_virt_type( self, guest: vconfig.LibvirtConfigGuest, instance: 'objects.Instance', image_meta: 'objects.ImageMeta', flavor: 'objects.Flavor', + old_guest: ty.Optional[vconfig.LibvirtConfigGuest] = None, ) -> None: if CONF.libvirt.virt_type in ("kvm", "qemu"): caps = self._host.get_capabilities() @@ -7182,30 +7232,17 @@ class LibvirtDriver(driver.ComputeDriver): else: guest.os_loader_secure = False - try: - loader, nvram_template, requires_smm = ( - self._host.get_loader( - arch, mach_type, - has_secure_boot=guest.os_loader_secure)) - except exception.UEFINotSupported as exc: - if guest.os_loader_secure: - # we raise a specific exception if we requested secure - # boot and couldn't get that - raise exception.SecureBootNotSupported() from exc - raise - - guest.os_loader = loader - guest.os_loader_type = 'pflash' - guest.os_loader_readonly = True + guest.os_firmware = 'efi' if hw_firmware_stateless: guest.os_loader_stateless = True - else: - guest.os_nvram_template = nvram_template - # if the feature set says we need SMM then enable it - if requires_smm: - guest.features.append( - vconfig.LibvirtConfigGuestFeatureSMM()) + if old_guest: + LOG.debug('The domain already exists. Loading ' + 'the firmware files previously selected.') + self._copy_guest_firmware_elements(old_guest, guest) + else: + LOG.debug('The domain does not exist. Firmware files ' + 'will be selected by libvirt.') # NOTE(lyarwood): If the machine type isn't recorded in the stashed # image metadata then record it through the system metadata table. @@ -7537,7 +7574,7 @@ class LibvirtDriver(driver.ComputeDriver): def _get_guest_config(self, instance, network_info, image_meta, disk_info, rescue=None, block_device_info=None, context=None, mdevs=None, accel_info=None, - share_info=None): + share_info=None, old_guest=None): """Get config data for parameters. :param rescue: optional dictionary that should contain the key @@ -7606,7 +7643,8 @@ class LibvirtDriver(driver.ComputeDriver): me_config = self._get_mem_encryption_config(flavor, image_meta) - self._configure_guest_by_virt_type(guest, instance, image_meta, flavor) + self._configure_guest_by_virt_type( + guest, instance, image_meta, flavor, old_guest) if CONF.libvirt.virt_type != 'lxc': self._conf_non_lxc( guest, root_device_name, rescue, instance, inst_path, @@ -8098,11 +8136,26 @@ class LibvirtDriver(driver.ComputeDriver): ioapic = vconfig.LibvirtConfigGuestFeatureIOAPIC() guest.add_feature(ioapic) + def _get_existing_guest_config( + self, + instance: 'objects.Instance', + ) -> ty.Optional[vconfig.LibvirtConfigGuest]: + guest_config = None + try: + guest = self._host.get_guest(instance) + xml = guest.get_xml_desc() + xml_doc = etree.fromstring(xml) + guest_config = vconfig.LibvirtConfigGuest() + guest_config.parse_dom(xml_doc) + except exception.InstanceNotFound: + pass + return guest_config + def _get_guest_xml(self, context, instance, network_info, disk_info, image_meta, rescue=None, block_device_info=None, mdevs=None, accel_info=None, - share_info=None): + share_info=None, old_guest=None): # NOTE(danms): Stringifying a NetworkInfo will take a lock. Do # this ahead of time so that we don't acquire it while also # holding the logging lock. @@ -8122,7 +8175,8 @@ class LibvirtDriver(driver.ComputeDriver): LOG.debug(strutils.mask_password(msg), instance=instance) conf = self._get_guest_config(instance, network_info, image_meta, disk_info, rescue, block_device_info, - context, mdevs, accel_info, share_info) + context, mdevs, accel_info, share_info, + old_guest) xml = conf.to_xml() LOG.debug('End _get_guest_xml xml=%(xml)s', @@ -12931,10 +12985,12 @@ class LibvirtDriver(driver.ComputeDriver): # the new XML mdevs = list(self._get_all_assigned_mediated_devices(instance)) + old_guest = self._get_existing_guest_config(instance) + xml = self._get_guest_xml(context, instance, network_info, disk_info, instance.image_meta, block_device_info=block_device_info, - mdevs=mdevs) + mdevs=mdevs, old_guest=old_guest) self._create_guest_with_network( context, xml, instance, network_info, block_device_info, power_on=power_on) diff --git a/nova/virt/libvirt/host.py b/nova/virt/libvirt/host.py index 5266481a45..9faf1a20ae 100644 --- a/nova/virt/libvirt/host.py +++ b/nova/virt/libvirt/host.py @@ -30,8 +30,6 @@ the other libvirt related classes from collections.abc import Callable from collections.abc import Mapping from collections import defaultdict -import fnmatch -import glob import inspect import operator import os @@ -41,7 +39,6 @@ import typing as ty from lxml import etree from oslo_log import log as logging -from oslo_serialization import jsonutils from oslo_utils import excutils from oslo_utils import strutils from oslo_utils import units @@ -81,39 +78,9 @@ HV_DRIVER_QEMU = "QEMU" SEV_KERNEL_PARAM_FILE = '/sys/module/kvm_amd/parameters/%s' -# These are taken from the spec -# https://github.com/qemu/qemu/blob/v5.2.0/docs/interop/firmware.json -QEMU_FIRMWARE_DESCRIPTOR_PATHS = [ - '/usr/share/qemu/firmware', - '/etc/qemu/firmware', - # we intentionally ignore '$XDG_CONFIG_HOME/qemu/firmware' -] - MIN_QEMU_SEV_ES_VERSION = (8, 0, 0) -def _get_loaders(): - if not any( - os.path.exists(path) for path in QEMU_FIRMWARE_DESCRIPTOR_PATHS - ): - msg = _("Failed to locate firmware descriptor files") - raise exception.InternalError(msg) - - _loaders = [] - - for path in QEMU_FIRMWARE_DESCRIPTOR_PATHS: - if not os.path.exists(path): - continue - - for spec_path in sorted(glob.glob(f'{path}/*.json')): - with open(spec_path, 'rb') as fh: - spec = jsonutils.load(fh) - - _loaders.append(spec) - - return _loaders - - class LibvirtEventHandler: def __init__(self, conn_event_handler=None, lifecycle_event_handler=None): self._lifecycle_event_handler = lifecycle_event_handler @@ -354,8 +321,6 @@ class Host(object): self._libvirt_proxy_classes = self._get_libvirt_proxy_classes(libvirt) self._libvirt_proxy = self._wrap_libvirt_proxy(libvirt) - self._loaders: list[dict] | None = None - # A number of features are conditional on support in the hardware, # kernel, QEMU, and/or libvirt. These are determined on demand and # memoized by various properties below @@ -2170,75 +2135,3 @@ class Host(object): is meant to be checked elsewhere. """ return self.has_min_version(lv_ver=(7, 9, 0)) - - @property - def loaders(self) -> list[dict]: - """Retrieve details of loader configuration for the host. - - Inspect the firmware metadata files provided by QEMU [1] to retrieve - information about the firmware supported by this host. Note that most - distros only publish this information for UEFI loaders currently. - - This should be removed when libvirt correctly supports switching - between loaders with or without secure boot enabled [2]. - - [1] https://github.com/qemu/qemu/blob/v5.2.0/docs/interop/firmware.json - [2] https://bugzilla.redhat.com/show_bug.cgi?id=1906500 - - :returns: An ordered list of loader configuration dictionaries. - """ - if self._loaders is not None: - return self._loaders - - self._loaders = _get_loaders() - return self._loaders - - def get_loader( - self, - arch: str, - machine: str, - has_secure_boot: bool, - ) -> tuple[str, str, bool]: - """Get loader for the specified architecture and machine type. - - :returns: A the bootloader executable path and the NVRAM - template path and a bool indicating if we need to enable SMM. - """ - - machine = self.get_canonical_machine_type(arch, machine) - - for loader in self.loaders: - try: - for target in loader['targets']: - if arch != target['architecture']: - continue - - for machine_glob in target['machines']: - # the 'machines' attribute supports glob patterns (e.g. - # 'pc-q35-*') so we need to resolve these - if fnmatch.fnmatch(machine, machine_glob): - break - else: - continue - - # if we've got this far, we have a match on the target - break - else: - continue - - # if we request secure boot then we should get it and vice - # versa - if has_secure_boot != ('secure-boot' in loader['features']): - continue - - return ( - loader['mapping']['executable']['filename'], - loader['mapping']['nvram-template']['filename'], - 'requires-smm' in loader['features'], - ) - except KeyError: - # This indicates that the description structure is new and nova - # does not how to handle it - continue - - raise exception.UEFINotSupported() diff --git a/releasenotes/notes/bp-libvirt-firmware-auto-selection-2d58b075816898b7.yaml b/releasenotes/notes/bp-libvirt-firmware-auto-selection-2d58b075816898b7.yaml new file mode 100644 index 0000000000..39ca89df57 --- /dev/null +++ b/releasenotes/notes/bp-libvirt-firmware-auto-selection-2d58b075816898b7.yaml @@ -0,0 +1,16 @@ +--- +features: + - | + Now the libvirt virt driver uses firmware auto-selection by libvirt, which + is capable to select the appropriate firmware files according to + the requested features. This built-in auto-selection extends the existing + firmware selection capability within nova, and checks a few more feature + flags such as amd-sev and also supports new firmware types such as rom + firmware. + +upgrades: + - | + Existing UEFI guests may start using a new firmware file after operations + like rebuild or resize (including cold-migration) which generates libvirt + domain XML from scratch, due to the extended logic to select + the appropriate files.