diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index b4439a91d83d..92fbdbb69c1b 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -15822,8 +15822,11 @@ class LibvirtConnTestCase(test.NoDBTestCase, mock_domain.undefine.assert_called_once_with() mock_save.assert_called_once_with() + @mock.patch('nova.utils.get_image_from_system_metadata') @mock.patch.object(objects.Instance, 'save') - def test_destroy_removes_nvram(self, mock_save): + def test_destroy_removes_nvram_host_support_uefi(self, + mock_save, + mock_image): mock_domain = mock.Mock(fakelibvirt.virDomain) mock_domain.ID.return_value = 123 @@ -15835,15 +15838,47 @@ class LibvirtConnTestCase(test.NoDBTestCase, state=power_state.SHUTDOWN, internal_id=-1)) instance = objects.Instance(self.context, **self.test_instance) + mock_image.return_value = {"properties": { + "hw_firmware_type": "bios"}} drvr.destroy(self.context, instance, []) self.assertEqual(1, mock_domain.ID.call_count) mock_domain.destroy.assert_called_once_with() - # undefineFlags should now be called with 5 as uefi us supported + # NVRAM flag should not called only host support uefi mock_domain.undefineFlags.assert_called_once_with( + fakelibvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE + ) + mock_domain.undefine.assert_not_called() + mock_save.assert_called_once_with() + + @mock.patch('nova.utils.get_image_from_system_metadata') + @mock.patch.object(objects.Instance, 'save') + def test_destroy_removes_nvram_host_and_guest_support_uefi(self, + mock_save, + mock_image): + mock_domain = mock.Mock(fakelibvirt.virDomain) + mock_domain.ID.return_value = 123 + + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) + drvr._host._get_domain = mock.Mock(return_value=mock_domain) + drvr._has_uefi_support = mock.Mock(return_value=True) + drvr.delete_instance_files = mock.Mock(return_value=None) + drvr.get_info = mock.Mock(return_value=hardware.InstanceInfo( + state=power_state.SHUTDOWN, internal_id=-1)) + + instance = objects.Instance(self.context, **self.test_instance) + mock_image.return_value = {"properties": { + "hw_firmware_type": "uefi"}} + drvr.destroy(self.context, instance, []) + + self.assertEqual(1, mock_domain.ID.call_count) + mock_domain.destroy.assert_called_once_with() + # undefineFlags should now be called with 5 as uefi supported + # by both host and guest + mock_domain.undefineFlags.assert_has_calls([mock.call( fakelibvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE | fakelibvirt.VIR_DOMAIN_UNDEFINE_NVRAM - ) + )]) mock_domain.undefine.assert_not_called() mock_save.assert_called_once_with() @@ -18730,6 +18765,9 @@ class LibvirtConnTestCase(test.NoDBTestCase, def _test_swap_volume(self, mock_is_job_complete, source_type, resize=False, fail=False): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI()) + image_meta = objects.ImageMeta.from_dict(self.test_image_meta) + hw_firmware_type = image_meta.properties.get( + 'hw_firmware_type') mock_dom = mock.MagicMock() guest = libvirt_guest.Guest(mock_dom) @@ -18769,10 +18807,11 @@ class LibvirtConnTestCase(test.NoDBTestCase, mock_conf = mock.MagicMock(source_type=source_type, source_path=dstfile) if not fail: - drvr._swap_volume(guest, srcfile, mock_conf, 1) + drvr._swap_volume(guest, srcfile, mock_conf, 1, + hw_firmware_type) else: self.assertRaises(expected_exception, drvr._swap_volume, guest, - srcfile, mock_conf, 1) + srcfile, mock_conf, 1, hw_firmware_type) # Verify we read the original persistent config. expected_call_count = 1 @@ -18829,6 +18868,9 @@ class LibvirtConnTestCase(test.NoDBTestCase, swap_volume, disconnect_volume): conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI()) instance = objects.Instance(**self.test_instance) + image_meta = objects.ImageMeta.from_dict(self.test_image_meta) + hw_firmware_type = image_meta.properties.get( + 'hw_firmware_type') old_connection_info = {'driver_volume_type': 'fake', 'serial': 'old-volume-id', 'data': {'device_path': '/fake-old-volume', @@ -18861,7 +18903,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, connect_volume.assert_called_once_with(self.context, new_connection_info, instance) - swap_volume.assert_called_once_with(guest, 'vdb', conf, 1) + swap_volume.assert_called_once_with(guest, 'vdb', + conf, 1, hw_firmware_type) disconnect_volume.assert_called_once_with(self.context, old_connection_info, instance) diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 5a891f3a51b1..c85d2cb932ca 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -1314,7 +1314,10 @@ class LibvirtDriver(driver.ComputeDriver): try: guest = self._host.get_guest(instance) try: - support_uefi = self._has_uefi_support() + hw_firmware_type = instance.image_meta.properties.get( + 'hw_firmware_type') + support_uefi = (self._has_uefi_support() and + hw_firmware_type == fields.FirmwareType.UEFI) guest.delete_configuration(support_uefi) except libvirt.libvirtError as e: with excutils.save_and_reraise_exception() as ctxt: @@ -1842,7 +1845,8 @@ class LibvirtDriver(driver.ComputeDriver): self._disconnect_volume(context, connection_info, instance, encryption=encryption) - def _swap_volume(self, guest, disk_path, conf, resize_to): + def _swap_volume(self, guest, disk_path, + conf, resize_to, hw_firmware_type): """Swap existing disk with a new block device.""" dev = guest.get_block_device(disk_path) @@ -1863,7 +1867,8 @@ class LibvirtDriver(driver.ComputeDriver): # If any part of this block fails, the domain is # re-defined regardless. if guest.has_persistent_configuration(): - support_uefi = self._has_uefi_support() + support_uefi = (self._has_uefi_support() and + hw_firmware_type == fields.FirmwareType.UEFI) guest.delete_configuration(support_uefi) try: @@ -1933,8 +1938,12 @@ class LibvirtDriver(driver.ComputeDriver): self._disconnect_volume(context, new_connection_info, instance) raise NotImplementedError(_("Swap only supports host devices")) + hw_firmware_type = instance.image_meta.properties.get( + 'hw_firmware_type') + try: - self._swap_volume(guest, disk_dev, conf, resize_to) + self._swap_volume(guest, disk_dev, conf, + resize_to, hw_firmware_type) except exception.VolumeRebaseFailed: with excutils.save_and_reraise_exception(): self._disconnect_volume(context, new_connection_info, instance) @@ -2538,7 +2547,10 @@ class LibvirtDriver(driver.ComputeDriver): # If any part of this block fails, the domain is # re-defined regardless. if guest.has_persistent_configuration(): - support_uefi = self._has_uefi_support() + hw_firmware_type = image_meta.properties.get( + 'hw_firmware_type') + support_uefi = (self._has_uefi_support() and + hw_firmware_type == fields.FirmwareType.UEFI) guest.delete_configuration(support_uefi) # NOTE (rmk): Establish a temporary mirror of our root disk and diff --git a/releasenotes/notes/bug-1845628-3152e73a1e4856b2.yaml b/releasenotes/notes/bug-1845628-3152e73a1e4856b2.yaml new file mode 100644 index 000000000000..aca19dd4bc3e --- /dev/null +++ b/releasenotes/notes/bug-1845628-3152e73a1e4856b2.yaml @@ -0,0 +1,13 @@ +--- +fixes: + - | + Prior to this release Nova determined if ``UEFI`` support should be + enable solely by checking host support as reported in `bug 1845628`_. + + Nova now correctly check for guest os support via the ``hw_firmware_type`` + image metadata property when spawning new instance and only + enables ``UEFI`` if the guest and host support it. + Guest deletion has also been updated to correctly clean up based on + the ``UEFI`` or ``BIOS`` configuration of the vm. + + .. _bug 1845628: https://bugs.launchpad.net/nova/+bug/1845628