Merge "Avoid unplugging VBDs for rescue instances"

2 files changed, 22 insertions, 30 deletions

diff --git a/nova/tests/test_xenapi.py b/nova/tests/test_xenapi.py
index f7d3b7903..bc8a4c8c1 100644
--- a/nova/tests/test_xenapi.py
+++ b/nova/tests/test_xenapi.py
@@ -694,11 +694,6 @@ class XenAPIVMTestCase(test.TestCase):
                               str(3 * 1024))
 
     def test_rescue(self):
-        def _find_rescue_vbd_ref(*args):
-            return vbd
-
-        self.stubs.Set(vmops.VMOps, '_find_rescue_vbd_ref',
-                _find_rescue_vbd_ref)
         instance = self._create_instance()
         session = xenapi_conn.XenAPISession('test_url', 'root', 'test_pass')
         vm = vm_utils.VMHelper.lookup(session, instance.name)
diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py
index ba3d5bd3b..bd83ebf95 100644
--- a/nova/virt/xenapi/vmops.py
+++ b/nova/virt/xenapi/vmops.py
@@ -1058,8 +1058,11 @@ class VMOps(object):
         except self.XenAPI.Failure, exc:
             LOG.exception(exc)
 
-    def _find_rescue_vbd_ref(self, vm_ref, rescue_vm_ref):
-        """Find and return the rescue VM's vbd_ref."""
+    def _find_root_vdi_ref(self, vm_ref):
+        """Find and return the root vdi ref for a VM."""
+        if not vm_ref:
+            return None
+
         vbd_refs = self._session.call_xenapi("VM.get_VBDs", vm_ref)
 
         if len(vbd_refs) == 0:
@@ -1072,10 +1075,7 @@ class VMOps(object):
             # with the root fs coming second
             vbd_ref = vbd_refs[1]
 
-        vdi_ref = self._session.call_xenapi("VBD.get_record", vbd_ref)["VDI"]
-
-        return VMHelper.create_vbd(self._session, rescue_vm_ref, vdi_ref,
-                                   1, bootable=False)
+        return self._session.call_xenapi("VBD.get_record", vbd_ref)["VDI"]
 
     def _shutdown_rescue(self, rescue_vm_ref):
         """Shutdown a rescue instance."""
@@ -1101,20 +1101,14 @@ class VMOps(object):
         vdi_refs = VMHelper.lookup_vm_vdis(self._session, vm_ref)
         self._safe_destroy_vdis(vdi_refs)
 
-    def _destroy_rescue_vdis(self, rescue_vm_ref):
+    def _destroy_rescue_vdis(self, rescue_vm_ref, except_vdi_ref):
         """Destroys all VDIs associated with a rescued VM."""
         vdi_refs = VMHelper.lookup_vm_vdis(self._session, rescue_vm_ref)
+        if except_vdi_ref:
+            vdi_refs = [vdi_ref for vdi_ref in vdi_refs
+                        if vdi_ref != except_vdi_ref]
         self._safe_destroy_vdis(vdi_refs)
 
-    def _destroy_rescue_vbds(self, rescue_vm_ref):
-        """Destroys all VBDs tied to a rescue VM."""
-        vbd_refs = self._session.call_xenapi("VM.get_VBDs", rescue_vm_ref)
-        for vbd_ref in vbd_refs:
-            vbd_rec = self._session.call_xenapi("VBD.get_record", vbd_ref)
-            if vbd_rec.get("userdevice", None) == "1":  # VBD is always 1
-                VMHelper.unplug_vbd(self._session, vbd_ref)
-                VMHelper.destroy_vbd(self._session, vbd_ref)
-
     def _destroy_kernel_ramdisk_plugin_call(self, kernel, ramdisk):
         args = {}
         if kernel:
@@ -1165,11 +1159,11 @@ class VMOps(object):
 
         LOG.debug(_("Instance %(instance_uuid)s VM destroyed") % locals())
 
-    def _destroy_rescue_instance(self, rescue_vm_ref):
+    def _destroy_rescue_instance(self, rescue_vm_ref, original_vm_ref):
         """Destroy a rescue instance."""
-        self._destroy_rescue_vbds(rescue_vm_ref)
+        vdi_ref = self._find_root_vdi_ref(original_vm_ref)
         self._shutdown_rescue(rescue_vm_ref)
-        self._destroy_rescue_vdis(rescue_vm_ref)
+        self._destroy_rescue_vdis(rescue_vm_ref, vdi_ref)
 
         self._session.call_xenapi("VM.destroy", rescue_vm_ref)
 
@@ -1183,12 +1177,13 @@ class VMOps(object):
         instance_uuid = instance['uuid']
         LOG.info(_("Destroying VM for Instance %(instance_uuid)s") % locals())
 
+        vm_ref = VMHelper.lookup(self._session, instance.name)
+
         rescue_vm_ref = VMHelper.lookup(self._session,
                                         "%s-rescue" % instance.name)
         if rescue_vm_ref:
-            self._destroy_rescue_instance(rescue_vm_ref)
+            self._destroy_rescue_instance(rescue_vm_ref, vm_ref)
 
-        vm_ref = VMHelper.lookup(self._session, instance.name)
         return self._destroy(instance, vm_ref, network_info, shutdown=True)
 
     def _destroy(self, instance, vm_ref, network_info=None, shutdown=True,
@@ -1260,8 +1255,10 @@ class VMOps(object):
         instance._rescue = True
         self.spawn_rescue(context, instance, image_meta, network_info)
         rescue_vm_ref = VMHelper.lookup(self._session, instance.name)
-        rescue_vbd_ref = self._find_rescue_vbd_ref(vm_ref, rescue_vm_ref)
+        vdi_ref = self._find_root_vdi_ref(vm_ref)
 
+        rescue_vbd_ref = VMHelper.create_vbd(self._session, rescue_vm_ref,
+                                             vdi_ref, 1, bootable=False)
         self._session.call_xenapi('VBD.plug', rescue_vbd_ref)
 
     def unrescue(self, instance):
@@ -1281,7 +1278,7 @@ class VMOps(object):
         original_vm_ref = VMHelper.lookup(self._session, instance.name)
         instance._rescue = False
 
-        self._destroy_rescue_instance(rescue_vm_ref)
+        self._destroy_rescue_instance(rescue_vm_ref, original_vm_ref)
         self._release_bootlock(original_vm_ref)
         self._start(instance, original_vm_ref)
 
@@ -1360,11 +1357,11 @@ class VMOps(object):
         for vm in rescue_vms:
             rescue_vm_ref = vm["vm_ref"]
 
-            self._destroy_rescue_instance(rescue_vm_ref)
-
             original_name = vm["name"].split("-rescue", 1)[0]
             original_vm_ref = VMHelper.lookup(self._session, original_name)
 
+            self._destroy_rescue_instance(rescue_vm_ref, original_vm_ref)
+
             self._release_bootlock(original_vm_ref)
             self._session.call_xenapi("VM.start", original_vm_ref, False,
                                       False)