From bf6c5ba86509c1e162b1f9d23c7cab603abce914 Mon Sep 17 00:00:00 2001 From: Kevin Zhao Date: Thu, 5 Jan 2017 21:32:41 +0000 Subject: [PATCH] libvirt: fix nova can't delete the instance with nvram Currently libvirt needs a flag when deleting an VM with a nvram file, without which nova can't delete an instance booted with UEFI. Add deletion flag for NVRAM. Also add a test case. Co-authored-by: Derek Higgins Change-Id: I46baa952b6c3a1a4c5cf2660931f317cafb5757d Closes-Bug: #1567807 (cherry picked from commit 539d381434ccadcdc3f5d58c2705c35558a3a065) --- nova/tests/unit/virt/libvirt/fakelibvirt.py | 1 + nova/tests/unit/virt/libvirt/test_driver.py | 26 +++++++++++++++++++++ nova/tests/unit/virt/libvirt/test_guest.py | 6 +++++ nova/virt/libvirt/driver.py | 9 ++++--- nova/virt/libvirt/guest.py | 8 ++++--- 5 files changed, 44 insertions(+), 6 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/fakelibvirt.py b/nova/tests/unit/virt/libvirt/fakelibvirt.py index 0261223a422b..75cd4ef9abb7 100644 --- a/nova/tests/unit/virt/libvirt/fakelibvirt.py +++ b/nova/tests/unit/virt/libvirt/fakelibvirt.py @@ -68,6 +68,7 @@ VIR_DOMAIN_EVENT_SHUTDOWN = 6 VIR_DOMAIN_EVENT_PMSUSPENDED = 7 VIR_DOMAIN_UNDEFINE_MANAGED_SAVE = 1 +VIR_DOMAIN_UNDEFINE_NVRAM = 4 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 fff91a4cdcc4..7798584d29d4 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -11843,6 +11843,32 @@ class LibvirtConnTestCase(test.NoDBTestCase): drvr.destroy(self.context, instance, []) mock_save.assert_called_once_with() + @mock.patch.object(objects.Instance, 'save') + def test_destroy_removes_nvram(self, mock_save): + 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, id=-1) + ) + + instance = objects.Instance(self.context, **self.test_instance) + 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 + 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() + def test_destroy_timed_out(self): mock = self.mox.CreateMock(fakelibvirt.virDomain) mock.ID() diff --git a/nova/tests/unit/virt/libvirt/test_guest.py b/nova/tests/unit/virt/libvirt/test_guest.py index 085df4664cae..5f9ba58d74f1 100644 --- a/nova/tests/unit/virt/libvirt/test_guest.py +++ b/nova/tests/unit/virt/libvirt/test_guest.py @@ -161,6 +161,12 @@ class GuestTestCase(test.NoDBTestCase): self.domain.undefineFlags.assert_called_once_with( fakelibvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE) + def test_delete_configuration_with_nvram(self): + self.guest.delete_configuration(support_uefi=True) + self.domain.undefineFlags.assert_called_once_with( + fakelibvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE | + fakelibvirt.VIR_DOMAIN_UNDEFINE_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 ee0ab5bf66a8..7ee58f4adc4e 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -865,7 +865,8 @@ class LibvirtDriver(driver.ComputeDriver): try: guest = self._host.get_guest(instance) try: - guest.delete_configuration() + support_uefi = self._has_uefi_support() + guest.delete_configuration(support_uefi) except libvirt.libvirtError as e: with excutils.save_and_reraise_exception(): errcode = e.get_error_code() @@ -1203,7 +1204,8 @@ 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() + support_uefi = self._has_uefi_support() + guest.delete_configuration(support_uefi) # Start copy with VIR_DOMAIN_REBASE_REUSE_EXT flag to # allow writing to existing external volume file @@ -1704,7 +1706,8 @@ 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() + support_uefi = self._has_uefi_support() + guest.delete_configuration(support_uefi) # 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 5a31dfd12f03..0edc058eb91f 100644 --- a/nova/virt/libvirt/guest.py +++ b/nova/virt/libvirt/guest.py @@ -255,11 +255,13 @@ class Guest(object): yield VCPUInfo( id=vcpu[0], cpu=vcpu[3], state=vcpu[1], time=vcpu[2]) - def delete_configuration(self): + def delete_configuration(self, support_uefi=False): """Undefines a domain from hypervisor.""" try: - self._domain.undefineFlags( - libvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE) + flags = libvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE + if support_uefi: + flags |= libvirt.VIR_DOMAIN_UNDEFINE_NVRAM + self._domain.undefineFlags(flags) except libvirt.libvirtError: LOG.debug("Error from libvirt during undefineFlags. %d" "Retrying with undefine", self.id)