Xen: Cleanup orphan volume connections on boot failure

If the boot process fails after the VDI creation but before a VBD is
created then the current cleanup methods will not work as they all rely
on lookups via VBD.

Change-Id: Id0d93ee60f75bf319baf7859b220ca325175128a
Closes-bug: #1329941

nova/tests/virt/xenapi/test_vmops.py

@@ -213,6 +213,8 @@ class SpawnTestCase(VMOpsTestBase):
         self.mox.StubOutWithMock(vm_utils, 'determine_disk_image_type')
         self.mox.StubOutWithMock(vm_utils, 'get_vdis_for_instance')
         self.mox.StubOutWithMock(vm_utils, 'safe_destroy_vdis')
+        self.mox.StubOutWithMock(self.vmops._volumeops,
+                                 'safe_cleanup_from_vdis')
         self.mox.StubOutWithMock(self.vmops, '_resize_up_vdis')
         self.mox.StubOutWithMock(vm_utils,
                                  'create_kernel_and_ramdisk')
@@ -370,6 +372,7 @@ class SpawnTestCase(VMOpsTestBase):
             vm_utils.destroy_kernel_ramdisk(self.vmops._session, instance,
                                             kernel_file, ramdisk_file)
             vm_utils.safe_destroy_vdis(self.vmops._session, ["fake_ref"])
+            self.vmops._volumeops.safe_cleanup_from_vdis(["fake_ref_2"])
 
         self.mox.ReplayAll()
         self.vmops.spawn(context, instance, image_meta, injected_files,

nova/tests/virt/xenapi/test_volume_utils.py

@@ -41,6 +41,36 @@ class SROps(stubs.XenAPITestBaseNoDB):
                                                       'sr_uuid'),
                          None)
 
+    def test_find_sr_from_vdi(self):
+        vdi_ref = 'fake-ref'
+
+        def fake_call_xenapi(method, *args):
+            self.assertEqual(method, 'VDI.get_SR')
+            self.assertEqual(args[0], vdi_ref)
+            return args[0]
+
+        session = mock.Mock()
+        session.call_xenapi.side_effect = fake_call_xenapi
+        self.assertEqual(volume_utils.find_sr_from_vdi(session, vdi_ref),
+                         vdi_ref)
+
+    def test_find_sr_from_vdi_exception(self):
+        vdi_ref = 'fake-ref'
+
+        class FakeException(Exception):
+            pass
+
+        def fake_call_xenapi(method, *args):
+            self.assertEqual(method, 'VDI.get_SR')
+            self.assertEqual(args[0], vdi_ref)
+            return args[0]
+
+        session = mock.Mock()
+        session.XenAPI.Failure = FakeException
+        session.call_xenapi.side_effect = FakeException
+        self.assertRaises(exception.StorageError,
+                volume_utils.find_sr_from_vdi, session, vdi_ref)
+
 
 class ISCSIParametersTestCase(stubs.XenAPITestBaseNoDB):
     def test_target_host(self):

nova/tests/virt/xenapi/test_volumeops.py

@@ -498,3 +498,52 @@ class FindBadVolumeTestCase(VolumeOpsTestBase):
                 self.assertRaises(FakeException,
                                   self.ops.find_bad_volumes, "vm_ref")
                 mock_scan.assert_called_once_with("sr_ref")
+
+
+class CleanupFromVDIsTestCase(VolumeOpsTestBase):
+    def _check_find_purge_calls(self, find_sr_from_vdi, purge_sr, vdi_refs,
+            sr_refs):
+        find_sr_calls = [mock.call(self.ops._session, vdi_ref) for vdi_ref
+                in vdi_refs]
+        find_sr_from_vdi.assert_has_calls(find_sr_calls)
+        purge_sr_calls = [mock.call(self.ops._session, sr_ref) for sr_ref
+                in sr_refs]
+        purge_sr.assert_has_calls(purge_sr_calls)
+
+    @mock.patch.object(volume_utils, 'find_sr_from_vdi')
+    @mock.patch.object(volume_utils, 'purge_sr')
+    def test_safe_cleanup_from_vdis(self, purge_sr, find_sr_from_vdi):
+        vdi_refs = ['vdi_ref1', 'vdi_ref2']
+        sr_refs = ['sr_ref1', 'sr_ref2']
+        find_sr_from_vdi.side_effect = sr_refs
+        self.ops.safe_cleanup_from_vdis(vdi_refs)
+
+        self._check_find_purge_calls(find_sr_from_vdi, purge_sr, vdi_refs,
+                sr_refs)
+
+    @mock.patch.object(volume_utils, 'find_sr_from_vdi',
+            side_effect=[exception.StorageError(reason=''), 'sr_ref2'])
+    @mock.patch.object(volume_utils, 'purge_sr')
+    def test_safe_cleanup_from_vdis_handles_find_sr_exception(self, purge_sr,
+            find_sr_from_vdi):
+        vdi_refs = ['vdi_ref1', 'vdi_ref2']
+        sr_refs = ['sr_ref2']
+        find_sr_from_vdi.side_effect = [exception.StorageError(reason=''),
+                sr_refs[0]]
+        self.ops.safe_cleanup_from_vdis(vdi_refs)
+
+        self._check_find_purge_calls(find_sr_from_vdi, purge_sr, vdi_refs,
+                sr_refs)
+
+    @mock.patch.object(volume_utils, 'find_sr_from_vdi')
+    @mock.patch.object(volume_utils, 'purge_sr')
+    def test_safe_cleanup_from_vdis_handles_purge_sr_exception(self, purge_sr,
+            find_sr_from_vdi):
+        vdi_refs = ['vdi_ref1', 'vdi_ref2']
+        sr_refs = ['sr_ref1', 'sr_ref2']
+        find_sr_from_vdi.side_effect = sr_refs
+        purge_sr.side_effects = [test.TestingException, None]
+        self.ops.safe_cleanup_from_vdis(vdi_refs)
+
+        self._check_find_purge_calls(find_sr_from_vdi, purge_sr, vdi_refs,
+                sr_refs)

nova/virt/xenapi/vmops.py

@@ -341,6 +341,9 @@ class VMOps(object):
                 vdi_refs = [vdi['ref'] for vdi in vdis.values()
                         if not vdi.get('osvol')]
                 vm_utils.safe_destroy_vdis(self._session, vdi_refs)
+                vol_vdi_refs = [vdi['ref'] for vdi in vdis.values()
+                        if vdi.get('osvol')]
+                self._volumeops.safe_cleanup_from_vdis(vol_vdi_refs)
 
             undo_mgr.undo_with(undo_create_disks)
             return vdis

nova/virt/xenapi/volume_utils.py

@@ -301,6 +301,17 @@ def find_sr_from_vbd(session, vbd_ref):
     return sr_ref
 
 
+def find_sr_from_vdi(session, vdi_ref):
+    """Find the SR reference from the VDI reference."""
+    try:
+        sr_ref = session.call_xenapi("VDI.get_SR", vdi_ref)
+    except session.XenAPI.Failure as exc:
+        LOG.exception(exc)
+        raise exception.StorageError(
+                reason=_('Unable to find SR from VDI %s') % vdi_ref)
+    return sr_ref
+
+
 def find_vbd_by_number(session, vm_ref, dev_number):
     """Get the VBD reference from the device number."""
     vbd_refs = session.VM.get_VBDs(vm_ref)

nova/virt/xenapi/volumeops.py

@@ -204,3 +204,20 @@ class VolumeOps(object):
                     raise
 
         return bad_devices
+
+    def safe_cleanup_from_vdis(self, vdi_refs):
+        # A helper method to detach volumes that are not associated with an
+        # instance
+
+        for vdi_ref in vdi_refs:
+            try:
+                sr_ref = volume_utils.find_sr_from_vdi(self._session, vdi_ref)
+            except exception.StorageError as exc:
+                LOG.debug(exc.format_message())
+                continue
+            try:
+                # Forget (i.e. disconnect) SR only if not in use
+                volume_utils.purge_sr(self._session, sr_ref)
+            except Exception:
+                LOG.debug('Ignoring error while purging sr: %s' % sr_ref,
+                        exc_info=True)