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 <derekh@redhat.com>
Change-Id: I46baa952b6c3a1a4c5cf2660931f317cafb5757d
Closes-Bug: #1567807

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

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()

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')

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.

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(
+            flags = libvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE
-                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)