diff --git a/nova/tests/fixtures/libvirt.py b/nova/tests/fixtures/libvirt.py index 9818526f95dd..492fe0ba43da 100644 --- a/nova/tests/fixtures/libvirt.py +++ b/nova/tests/fixtures/libvirt.py @@ -99,6 +99,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_AFFECT_CURRENT = 0 VIR_DOMAIN_AFFECT_LIVE = 1 diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 0b6057e3a544..a3568d3d2d4d 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -1763,6 +1763,39 @@ class LibvirtConnTestCase(test.NoDBTestCase, mock_which.assert_not_called() + @mock.patch.object(libvirt_driver.LibvirtDriver, + '_register_all_undefined_instance_details', + new=mock.Mock()) + @mock.patch.object(host.Host, 'has_min_version', return_value=True) + def test_keep_tpm_supported(self, mock_version): + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + drvr.init_host('dummyhost') + self.assertTrue( + drvr._may_keep_vtpm, + "LibvirtDriver did not correctly detect libvirt version " + "supporting KEEP_TPM" + ) + + @mock.patch.object(libvirt_driver.LibvirtDriver, + '_register_all_undefined_instance_details', + new=mock.Mock()) + @mock.patch.object(host.Host, 'has_min_version') + def test_keep_tpm_unsupported(self, mock_version): + def version_check(lv_ver=None, **kwargs): + if lv_ver == libvirt_driver.MIN_VERSION_INT_FOR_KEEP_TPM: + return False + return True + + mock_version.side_effect = version_check + + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + drvr.init_host('dummyhost') + self.assertFalse( + drvr._may_keep_vtpm, + "LibvirtDriver did not correctly detect libvirt version which " + "does not support KEEP_TPM" + ) + def test__check_multipath_misconfiguration(self): self.flags(volume_use_multipath=False, volume_enforce_multipath=True, group='libvirt') @@ -19475,6 +19508,51 @@ class LibvirtConnTestCase(test.NoDBTestCase, # ensure no raise for no such domain drvr._undefine_domain(instance) + @mock.patch.object(host.Host, "get_guest") + def test_undefine_domain_disarms_keep_vtpm_if_not_supported( + self, mock_get): + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) + drvr._may_keep_vtpm = False # normally set by init_host + instance = objects.Instance(**self.test_instance) + fake_guest = mock.Mock() + mock_get.return_value = fake_guest + + drvr._undefine_domain(instance, keep_vtpm=True) + + fake_guest.delete_configuration.assert_called_once_with( + keep_vtpm=False, + ) + + # Check that it truly forces it to False and doesn't do a `not` or + # something weird :-). + fake_guest.reset_mock() + drvr._undefine_domain(instance, keep_vtpm=False) + + fake_guest.delete_configuration.assert_called_once_with( + keep_vtpm=False, + ) + + @mock.patch.object(host.Host, "get_guest") + def test_undefine_domain_passes_keep_vtpm_if_supported(self, mock_get): + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) + drvr._may_keep_vtpm = True # normally set by init_host + instance = objects.Instance(**self.test_instance) + fake_guest = mock.Mock() + mock_get.return_value = fake_guest + + drvr._undefine_domain(instance, keep_vtpm=True) + + fake_guest.delete_configuration.assert_called_once_with(keep_vtpm=True) + + # Check that it does not force keep_vtpm to true, just because it is + # supported. + fake_guest.reset_mock() + drvr._undefine_domain(instance, keep_vtpm=False) + + fake_guest.delete_configuration.assert_called_once_with( + keep_vtpm=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") @@ -22113,7 +22191,33 @@ 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) + mock_undefine.assert_called_once_with(fake_inst, keep_vtpm=False) + + @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._undefine_domain') + @mock.patch('nova.crypto.delete_vtpm_secret') + @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.delete_instance_files') + @mock.patch('nova.virt.driver.block_device_info_get_mapping') + @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._unplug_vifs') + @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_vpmems', + new=mock.Mock(return_value=None)) + def test_cleanup_preserves_tpm_if_not_destroying_disks( + self, mock_unplug, mock_get_mapping, mock_delete_files, + mock_delete_vtpm, mock_undefine, + ): + """Test with default parameters.""" + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI()) + fake_inst = objects.Instance(**self.test_instance) + mock_get_mapping.return_value = [] + mock_delete_files.return_value = True + + with mock.patch.object(fake_inst, 'save'): + drvr.cleanup('ctxt', fake_inst, 'netinfo', destroy_disks=False) + + mock_unplug.assert_called_once_with(fake_inst, 'netinfo', True) + 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.patch('nova.virt.libvirt.driver.LibvirtDriver._undefine_domain') @mock.patch('nova.crypto.delete_vtpm_secret') @@ -22138,7 +22242,7 @@ 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) + mock_undefine.assert_called_once_with(fake_inst, keep_vtpm=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 6d9eb6ede505..359013c54ea6 100644 --- a/nova/tests/unit/virt/libvirt/test_guest.py +++ b/nova/tests/unit/virt/libvirt/test_guest.py @@ -138,6 +138,13 @@ class GuestTestCase(test.NoDBTestCase): fakelibvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE | fakelibvirt.VIR_DOMAIN_UNDEFINE_NVRAM) + def test_delete_configuration_with_keep_vtpm_true(self): + self.guest.delete_configuration(keep_vtpm=True) + self.domain.undefineFlags.assert_called_once_with( + fakelibvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE | + fakelibvirt.VIR_DOMAIN_UNDEFINE_NVRAM | + fakelibvirt.VIR_DOMAIN_UNDEFINE_KEEP_TPM) + 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 a8f291c32054..3c5de7aecd21 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -252,6 +252,9 @@ MIN_VIRTIO_SOUND_QEMU_VERSION = (8, 2, 0) # Minimum version of Qemu that supports multifd migration with post-copy MIN_MULTIFD_WITH_POSTCOPY_QEMU_VERSION = (10, 1, 0) +# Minimum version to preserve vTPM data +MIN_VERSION_INT_FOR_KEEP_TPM = (8, 9, 0) + REGISTER_IMAGE_PROPERTY_DEFAULTS = [ 'hw_machine_type', 'hw_cdrom_bus', @@ -582,6 +585,10 @@ class LibvirtDriver(driver.ComputeDriver): # See also nova.virt.libvirt.cpu.api.API.core(). self.cpu_api = libvirt_cpu.API() + # Cache the availability of the VIR_DOMAIN_UNDEFINE_KEEP_TPM flag in + # this libvirt version. This is set in init_host. + self._may_keep_vtpm = False + def _discover_vpmems(self, vpmem_conf=None): """Discover vpmems on host and configuration. @@ -903,6 +910,12 @@ class LibvirtDriver(driver.ComputeDriver): self._check_vtpm_support() + # Cache the availability of the VIR_DOMAIN_UNDEFINE_KEEP_TPM flag in + # this libvirt version. + self._may_keep_vtpm = self._host.has_min_version( + MIN_VERSION_INT_FOR_KEEP_TPM, + ) + self._check_multipath() # Even if we already checked the whitelist at startup, this driver @@ -1639,11 +1652,32 @@ class LibvirtDriver(driver.ComputeDriver): self.cleanup(context, instance, network_info, block_device_info, destroy_disks, destroy_secrets=destroy_secrets) - def _undefine_domain(self, instance): + def _delete_guest_configuration(self, guest, keep_vtpm): + """Wrapper around guest.delete_configuration which incorporates version + checks for the additional arguments. + + :param guest: The domain to undefine. + :param keep_vtpm: If set, the vTPM data (if any) is not deleted during + undefine. + + This flag may be ignored if libvirt is too old to support + preserving vTPM data (see bug #2118888). + """ + if keep_vtpm and not self._may_keep_vtpm: + LOG.warning( + "Temporary undefine operation is deleting vTPM contents. " + "Please upgrade libvirt to >= 8.9.0 to avoid this.", + instance=guest.uuid, + ) + keep_vtpm = False + + guest.delete_configuration(keep_vtpm=keep_vtpm) + + def _undefine_domain(self, instance, keep_vtpm=False): try: guest = self._host.get_guest(instance) try: - guest.delete_configuration() + self._delete_guest_configuration(guest, keep_vtpm=keep_vtpm) except libvirt.libvirtError as e: with excutils.save_and_reraise_exception() as ctxt: errcode = e.get_error_code() @@ -1809,7 +1843,7 @@ class LibvirtDriver(driver.ComputeDriver): self._cleanup_ephemeral_encryption_secrets( context, instance, block_device_info) - self._undefine_domain(instance) + self._undefine_domain(instance, keep_vtpm=not cleanup_instance_disks) def _cleanup_ephemeral_encryption_secrets( self, context, instance, block_device_info @@ -2384,7 +2418,7 @@ class LibvirtDriver(driver.ComputeDriver): # undefine it. If any part of this block fails, the domain is # re-defined regardless. if guest.has_persistent_configuration(): - guest.delete_configuration() + self._delete_guest_configuration(guest, keep_vtpm=True) try: dev.copy(conf.to_xml(), reuse_ext=True) @@ -3509,7 +3543,7 @@ class LibvirtDriver(driver.ComputeDriver): # If any part of this block fails, the domain is # re-defined regardless. if guest.has_persistent_configuration(): - guest.delete_configuration() + self._delete_guest_configuration(guest, keep_vtpm=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 eef6c21a01f7..7ecddb1c7de0 100644 --- a/nova/virt/libvirt/guest.py +++ b/nova/virt/libvirt/guest.py @@ -289,11 +289,26 @@ class Guest: yield VCPUInfo( id=vcpu[0], cpu=vcpu[3], state=vcpu[1], time=vcpu[2]) - def delete_configuration(self): - """Undefines a domain from hypervisor.""" + def delete_configuration(self, keep_vtpm=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). + + 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 + the VM in libvirt with the same UUID), to prevent orphaning the vTPM + data in libvirt's data directory. + + It is the caller's responsibility to ensure that keep_vtpm is only set + to true on libvirt versions which support it, that is >= 8.9.0. + """ try: flags = libvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE flags |= libvirt.VIR_DOMAIN_UNDEFINE_NVRAM + if keep_vtpm: + flags |= libvirt.VIR_DOMAIN_UNDEFINE_KEEP_TPM self._domain.undefineFlags(flags) except libvirt.libvirtError: LOG.debug("Error from libvirt during undefineFlags for guest "