diff --git a/nova/tests/fixtures/libvirt.py b/nova/tests/fixtures/libvirt.py index 631952b256b6..827f739b282b 100644 --- a/nova/tests/fixtures/libvirt.py +++ b/nova/tests/fixtures/libvirt.py @@ -101,6 +101,7 @@ VIR_DOMAIN_EVENT_SUSPENDED_POSTCOPY = 7 VIR_DOMAIN_UNDEFINE_MANAGED_SAVE = 1 VIR_DOMAIN_UNDEFINE_NVRAM = 4 VIR_DOMAIN_UNDEFINE_KEEP_TPM = 64 +VIR_DOMAIN_UNDEFINE_KEEP_NVRAM = 8 VIR_DOMAIN_AFFECT_CURRENT = 0 VIR_DOMAIN_AFFECT_LIVE = 1 diff --git a/nova/tests/unit/virt/libvirt/test_config.py b/nova/tests/unit/virt/libvirt/test_config.py index 3bd460bd4208..df955f036ebf 100644 --- a/nova/tests/unit/virt/libvirt/test_config.py +++ b/nova/tests/unit/virt/libvirt/test_config.py @@ -2838,6 +2838,7 @@ class LibvirtConfigGuestTest(LibvirtConfigBaseTest): obj.os_mach_type = "pc-q35-5.1" obj.os_loader = '/tmp/OVMF_CODE.secboot.fd' obj.os_loader_type = 'pflash' + obj.os_nvram = '/foo/bar/instance-00000012_VARS.fd' obj.os_loader_secure = True obj.os_loader_stateless = True xml = obj.to_xml() @@ -2852,6 +2853,7 @@ class LibvirtConfigGuestTest(LibvirtConfigBaseTest): hvm /tmp/OVMF_CODE.secboot.fd + /foo/bar/instance-00000012_VARS.fd """, # noqa: E501 xml, diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index a3f8340c66ad..f2bfd368f043 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -19229,10 +19229,11 @@ class LibvirtConnTestCase(test.NoDBTestCase, fake_guest = mock.Mock() mock_get.return_value = fake_guest - drvr._undefine_domain(instance, keep_vtpm=True) + drvr._undefine_domain(instance, keep_vtpm=True, keep_nvram=False) fake_guest.delete_configuration.assert_called_once_with( keep_vtpm=False, + keep_nvram=False, ) # Check that it truly forces it to False and doesn't do a `not` or @@ -19242,6 +19243,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, fake_guest.delete_configuration.assert_called_once_with( keep_vtpm=False, + keep_nvram=False, ) @mock.patch.object(host.Host, "get_guest") @@ -19252,9 +19254,11 @@ class LibvirtConnTestCase(test.NoDBTestCase, fake_guest = mock.Mock() mock_get.return_value = fake_guest - drvr._undefine_domain(instance, keep_vtpm=True) + drvr._undefine_domain(instance, keep_vtpm=True, keep_nvram=False) - fake_guest.delete_configuration.assert_called_once_with(keep_vtpm=True) + fake_guest.delete_configuration.assert_called_once_with( + keep_vtpm=True, + keep_nvram=False) # Check that it does not force keep_vtpm to true, just because it is # supported. @@ -19263,8 +19267,27 @@ class LibvirtConnTestCase(test.NoDBTestCase, fake_guest.delete_configuration.assert_called_once_with( keep_vtpm=False, + keep_nvram=False, ) + @mock.patch.object(host.Host, "get_guest") + def test_undefine_domain_passes_keep_nvram_if_supported(self, mock_get): + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) + instance = objects.Instance(**self.test_instance) + fake_guest = mock.Mock() + mock_get.return_value = fake_guest + drvr._undefine_domain(instance, keep_nvram=True) + fake_guest.delete_configuration.assert_called_once_with( + keep_vtpm=False, keep_nvram=True) + # Check that it does not force keep_nvram to true, just because it is + # supported. + fake_guest.reset_mock() + drvr._undefine_domain(instance, keep_nvram=False) + fake_guest.delete_configuration.assert_called_once_with( + keep_vtpm=False, + keep_nvram=False, + ) + @mock.patch.object(host.Host, "list_instance_domains") @mock.patch.object(objects.BlockDeviceMappingList, "bdms_by_instance_uuid") @mock.patch.object(objects.InstanceList, "get_by_filters") @@ -19564,7 +19587,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, disk_actual_size = 3687091200 disk_actual_size_blocks = disk_actual_size / 512 expected_over_committed_disk_size = disk_virtual_size -\ - disk_actual_size + disk_actual_size mock_getsize.return_value = disk_virtual_size mock_stat.return_value = mock.Mock(st_blocks=disk_actual_size_blocks) @@ -21836,7 +21859,10 @@ class LibvirtConnTestCase(test.NoDBTestCase, mock_delete_files.assert_called_once_with(fake_inst) # vTPM secret should not be deleted until instance is deleted. mock_delete_vtpm.assert_not_called() - mock_undefine.assert_called_once_with(fake_inst, keep_vtpm=False) + mock_undefine.assert_called_once_with( + fake_inst, + keep_vtpm=False, + keep_nvram=False) @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._undefine_domain') @mock.patch('nova.crypto.delete_vtpm_secret') @@ -21862,7 +21888,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, mock_get_mapping.assert_called_once_with(None) mock_delete_files.assert_not_called() mock_delete_vtpm.assert_not_called() - mock_undefine.assert_called_once_with(fake_inst, keep_vtpm=True) + mock_undefine.assert_called_once_with(fake_inst, keep_vtpm=True, + keep_nvram=True) @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._undefine_domain') @mock.patch('nova.crypto.delete_vtpm_secret') @@ -21887,7 +21914,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, drvr.cleanup('ctxt', fake_inst, 'netinfo') # vTPM secret should not be deleted until instance is deleted. mock_delete_vtpm.assert_not_called() - mock_undefine.assert_called_once_with(fake_inst, keep_vtpm=False) + mock_undefine.assert_called_once_with(fake_inst, keep_vtpm=False, + keep_nvram=False) @mock.patch.object(libvirt_driver.LibvirtDriver, 'delete_instance_files', return_value=True) diff --git a/nova/tests/unit/virt/libvirt/test_guest.py b/nova/tests/unit/virt/libvirt/test_guest.py index 359013c54ea6..3034ff0a9d29 100644 --- a/nova/tests/unit/virt/libvirt/test_guest.py +++ b/nova/tests/unit/virt/libvirt/test_guest.py @@ -145,6 +145,12 @@ class GuestTestCase(test.NoDBTestCase): fakelibvirt.VIR_DOMAIN_UNDEFINE_NVRAM | fakelibvirt.VIR_DOMAIN_UNDEFINE_KEEP_TPM) + def test_delete_configuration_keep_nvram(self): + self.guest.delete_configuration(keep_nvram=True) + self.domain.undefineFlags.assert_called_once_with( + fakelibvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE | + fakelibvirt.VIR_DOMAIN_UNDEFINE_KEEP_NVRAM) + def test_delete_configuration_exception(self): self.domain.undefineFlags.side_effect = fakelibvirt.libvirtError( 'oops') diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index dd6ca2f4b437..1e276c66971c 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -1625,7 +1625,7 @@ class LibvirtDriver(driver.ComputeDriver): self.cleanup(context, instance, network_info, block_device_info, destroy_disks, destroy_secrets=destroy_secrets) - def _delete_guest_configuration(self, guest, keep_vtpm): + def _delete_guest_configuration(self, guest, keep_vtpm, keep_nvram): """Wrapper around guest.delete_configuration which incorporates version checks for the additional arguments. @@ -1644,13 +1644,14 @@ class LibvirtDriver(driver.ComputeDriver): ) keep_vtpm = False - guest.delete_configuration(keep_vtpm=keep_vtpm) + guest.delete_configuration(keep_vtpm=keep_vtpm, keep_nvram=keep_nvram) - def _undefine_domain(self, instance, keep_vtpm=False): + def _undefine_domain(self, instance, keep_vtpm=False, keep_nvram=False): try: guest = self._host.get_guest(instance) try: - self._delete_guest_configuration(guest, keep_vtpm=keep_vtpm) + self._delete_guest_configuration(guest, keep_vtpm=keep_vtpm, + keep_nvram=keep_nvram) except libvirt.libvirtError as e: with excutils.save_and_reraise_exception() as ctxt: errcode = e.get_error_code() @@ -1737,9 +1738,10 @@ class LibvirtDriver(driver.ComputeDriver): :param destroy_vifs: if plugged vifs should be unplugged :param cleanup_instance_dir: If the instance dir should be removed :param cleanup_instance_disks: If the instance disks should be removed. - Also removes ephemeral encryption secrets, if present. - :param destroy_secrets: If the cinder volume encryption libvirt secrets - should be deleted. + Also removes ephemeral encryption secrets, if present, as well as + vTPM and NVRAM data. + :param destroy_secrets: If the cinder volume encryption secrets should + be deleted. """ # zero the data on backend pmem device vpmems = self._get_vpmems(instance) @@ -1813,7 +1815,8 @@ class LibvirtDriver(driver.ComputeDriver): self._cleanup_ephemeral_encryption_secrets( context, instance, block_device_info) - self._undefine_domain(instance, keep_vtpm=not cleanup_instance_disks) + self._undefine_domain(instance, keep_vtpm=not cleanup_instance_disks, + keep_nvram=not cleanup_instance_disks) def _cleanup_ephemeral_encryption_secrets( self, context, instance, block_device_info @@ -2388,7 +2391,8 @@ class LibvirtDriver(driver.ComputeDriver): # undefine it. If any part of this block fails, the domain is # re-defined regardless. if guest.has_persistent_configuration(): - self._delete_guest_configuration(guest, keep_vtpm=True) + self._delete_guest_configuration(guest, keep_vtpm=True, + keep_nvram=True) try: dev.copy(conf.to_xml(), reuse_ext=True) @@ -3517,7 +3521,8 @@ class LibvirtDriver(driver.ComputeDriver): # If any part of this block fails, the domain is # re-defined regardless. if guest.has_persistent_configuration(): - self._delete_guest_configuration(guest, keep_vtpm=True) + self._delete_guest_configuration(guest, keep_vtpm=True, + keep_nvram=True) # NOTE (rmk): Establish a temporary mirror of our root disk and # issue an abort once we have a complete copy. diff --git a/nova/virt/libvirt/guest.py b/nova/virt/libvirt/guest.py index 6d7eb969df91..065181d89e66 100644 --- a/nova/virt/libvirt/guest.py +++ b/nova/virt/libvirt/guest.py @@ -295,12 +295,15 @@ class Guest(object): yield VCPUInfo( id=vcpu[0], cpu=vcpu[3], state=vcpu[1], time=vcpu[2]) - def delete_configuration(self, keep_vtpm=False): + def delete_configuration(self, keep_vtpm=False, keep_nvram=False): """Undefines a domain from hypervisor. :param keep_vtpm: If true, the vTPM data will be preserved. Otherwise, it will be deleted. Defaults to false (that is, deleting the vTPM data). + :param keep_nvram: If true, the NVRAM data will be preserved. + Otherwise, it will be deleted. Defaults to false (that is, deleting + the NVRAM data). Calling this with `keep_vtpm` set to True should, eventually, be followed up with a call where it is set to False (after re-defining @@ -312,9 +315,12 @@ class Guest(object): """ try: flags = libvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE - flags |= libvirt.VIR_DOMAIN_UNDEFINE_NVRAM if keep_vtpm: flags |= libvirt.VIR_DOMAIN_UNDEFINE_KEEP_TPM + if keep_nvram: + flags |= libvirt.VIR_DOMAIN_UNDEFINE_KEEP_NVRAM + else: + flags |= libvirt.VIR_DOMAIN_UNDEFINE_NVRAM self._domain.undefineFlags(flags) except libvirt.libvirtError: LOG.debug("Error from libvirt during undefineFlags for guest " diff --git a/releasenotes/notes/preserve-nvram-ab6d3d2fe923301f.yaml b/releasenotes/notes/preserve-nvram-ab6d3d2fe923301f.yaml new file mode 100644 index 000000000000..1dc0a594e9a9 --- /dev/null +++ b/releasenotes/notes/preserve-nvram-ab6d3d2fe923301f.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + NVRAM variable store is preserved during stop/start, hard reboot, and live + migration by passing the corresponding flag to libvirt. + + See https://bugs.launchpad.net/nova/+bug/1633447 for more details.