From 539d381434ccadcdc3f5d58c2705c35558a3a065 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 --- nova/tests/unit/virt/libvirt/fakelibvirt.py | 1 + nova/tests/unit/virt/libvirt/test_driver.py | 29 +++++++++++++++++++++ nova/tests/unit/virt/libvirt/test_guest.py | 6 +++++ nova/virt/libvirt/driver.py | 9 ++++--- nova/virt/libvirt/guest.py | 8 +++--- 5 files changed, 47 insertions(+), 6 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/fakelibvirt.py b/nova/tests/unit/virt/libvirt/fakelibvirt.py index 077f0f474283..d56af9e51897 100644 --- a/nova/tests/unit/virt/libvirt/fakelibvirt.py +++ b/nova/tests/unit/virt/libvirt/fakelibvirt.py @@ -69,6 +69,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 d49bfc8e72db..bcc2663420b0 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -11895,6 +11895,7 @@ class LibvirtConnTestCase(test.NoDBTestCase): 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=False) drvr.delete_instance_files = mock.Mock(return_value=None) drvr.get_info = mock.Mock(return_value= hardware.InstanceInfo(state=power_state.SHUTDOWN, id=-1) @@ -11917,6 +11918,7 @@ class LibvirtConnTestCase(test.NoDBTestCase): 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=False) drvr.delete_instance_files = mock.Mock(return_value=None) drvr.get_info = mock.Mock(return_value= hardware.InstanceInfo(state=power_state.SHUTDOWN, id=-1) @@ -11942,6 +11944,7 @@ class LibvirtConnTestCase(test.NoDBTestCase): 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=False) drvr.delete_instance_files = mock.Mock(return_value=None) drvr.get_info = mock.Mock(return_value= hardware.InstanceInfo(state=power_state.SHUTDOWN, id=-1) @@ -11957,6 +11960,32 @@ class LibvirtConnTestCase(test.NoDBTestCase): mock_domain.undefine.assert_called_once_with() 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 5e737ea50248..fe64f418f075 100644 --- a/nova/tests/unit/virt/libvirt/test_guest.py +++ b/nova/tests/unit/virt/libvirt/test_guest.py @@ -155,6 +155,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 d6a9be657cb9..391a26d1b2bf 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -903,7 +903,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() @@ -1241,7 +1242,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 @@ -1760,7 +1762,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 6c2b9e195444..0d44f76f0374 100644 --- a/nova/virt/libvirt/guest.py +++ b/nova/virt/libvirt/guest.py @@ -262,11 +262,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)