From aeacea16a30f85dfb307803a4b46a602cabc8eb5 Mon Sep 17 00:00:00 2001
From: Derek Higgins <derekh@redhat.com>
Date: Tue, 4 Sep 2012 11:50:36 +0100
Subject: [PATCH] Fixing call to hasManagedSaveImage

Fixes bug #1044090

hasManagedSaveImage is not implmented in the LXC libvirt driver, resulting
in the following error when a vm is deleted "Error from libvirt during saved
instance removal. Code=3 Error=this function is not supported by the
connection driver: virDomainHasManagedSaveImage"

This commit replaces the use of hasManagedSaveImage, managedSaveRemove and
undefine with undefineFlags which does the work of all 3 calls and is
implemented in versions of libvirt > 0.9.4. We also revert back to calling
undefine if undefineFlags raises an error.

Change-Id: Ib8d451aeff7767f835c3c1aab99ee4ab5e299852
---
 nova/tests/fakelibvirt.py   |  7 +++++++
 nova/tests/test_libvirt.py  | 29 +++++++++++++++++++++++++----
 nova/virt/libvirt/driver.py | 19 +++++++------------
 3 files changed, 39 insertions(+), 16 deletions(-)

diff --git a/nova/tests/fakelibvirt.py b/nova/tests/fakelibvirt.py
index 4e337aa5176..2dfc30970ca 100644
--- a/nova/tests/fakelibvirt.py
+++ b/nova/tests/fakelibvirt.py
@@ -69,6 +69,7 @@ def _reset():
 VIR_DOMAIN_CRASHED = 6
 
 VIR_DOMAIN_XML_SECURE = 1
+VIR_DOMAIN_UNDEFINE_MANAGED_SAVE = 1
 
 VIR_CPU_COMPARE_ERROR = -1
 VIR_CPU_COMPARE_INCOMPATIBLE = 0
@@ -276,6 +277,12 @@ def isActive(self):
     def undefine(self):
         self._connection._undefine(self)
 
+    def undefineFlags(self, flags):
+        self.undefine()
+        if flags & VIR_DOMAIN_UNDEFINE_MANAGED_SAVE:
+            if self.hasManagedSaveImage(0):
+                self.managedSaveRemove()
+
     def destroy(self):
         self._state = VIR_DOMAIN_SHUTOFF
         self._connection._mark_not_running(self)
diff --git a/nova/tests/test_libvirt.py b/nova/tests/test_libvirt.py
index e652e6d0b2f..a1b99388d12 100644
--- a/nova/tests/test_libvirt.py
+++ b/nova/tests/test_libvirt.py
@@ -2160,12 +2160,33 @@ def fake_lookup_by_name(instance_name):
         instance = db.instance_create(self.context, self.test_instance)
         conn.destroy(instance, {})
 
-    def test_destroy_saved(self):
-        """Ensure destroy calls managedSaveRemove for saved instance"""
+    def test_destroy(self):
+        """Ensure destroy calls virDomain.undefineFlags"""
+        mock = self.mox.CreateMock(libvirt.virDomain)
+        mock.destroy()
+        mock.undefineFlags(1).AndReturn(1)
+
+        self.mox.ReplayAll()
+
+        def fake_lookup_by_name(instance_name):
+            return mock
+
+        def fake_get_info(instance_name):
+            return {'state': power_state.SHUTDOWN}
+
+        conn = libvirt_driver.LibvirtDriver(False)
+        self.stubs.Set(conn, '_lookup_by_name', fake_lookup_by_name)
+        self.stubs.Set(conn, 'get_info', fake_get_info)
+        instance = {"name": "instancename", "id": "instanceid",
+                    "uuid": "875a8070-d0b9-4949-8b31-104d125c9a64"}
+        conn.destroy(instance, [])
+
+    def test_destroy_noflag(self):
+        """Ensure destroy calls virDomain.undefine
+        if mock.undefineFlags raises an error"""
         mock = self.mox.CreateMock(libvirt.virDomain)
         mock.destroy()
-        mock.hasManagedSaveImage(0).AndReturn(1)
-        mock.managedSaveRemove(0)
+        mock.undefineFlags(1).AndRaise(libvirt.libvirtError('Err'))
         mock.undefine()
 
         self.mox.ReplayAll()
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index 693590b385c..d7339d4da05 100644
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -483,18 +483,13 @@ def _cleanup(self, instance, network_info, block_device_info):
             virt_dom = None
         if virt_dom:
             try:
-                # NOTE(derekh): we can switch to undefineFlags and
-                # VIR_DOMAIN_UNDEFINE_MANAGED_SAVE once we require 0.9.4
-                if virt_dom.hasManagedSaveImage(0):
-                    virt_dom.managedSaveRemove(0)
-            except libvirt.libvirtError as e:
-                errcode = e.get_error_code()
-                LOG.error(_("Error from libvirt during saved instance "
-                              "removal. Code=%(errcode)s Error=%(e)s") %
-                            locals(), instance=instance)
-            try:
-                # NOTE(justinsb): We remove the domain definition.
-                virt_dom.undefine()
+                try:
+                    virt_dom.undefineFlags(
+                        libvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE)
+                except libvirt.libvirtError as e:
+                    LOG.debug(_("Error from libvirt during undefineFlags."
+                        " Retrying with undefine"), instance=instance)
+                    virt_dom.undefine()
             except libvirt.libvirtError as e:
                 errcode = e.get_error_code()
                 LOG.error(_("Error from libvirt during undefine. "