XenAPI: Fix volume detach

fixes bug 1101229

The code-cleanup and code-move changes from this patch:
https://review.openstack.org/#/c/19219/
introduced a problem: the volume_utils.find_sr_from_vbd method was
called after the vbd has already been destroyed. This change fixes the
issue by moving the sr discovery before the vbd destruction, and tests
the call order by using side effects.

Change-Id: Ide4f8ac810f98bb192909f5f0408affc940e7446

2 files changed, 15 insertions, 3 deletions

diff --git a/nova/tests/virt/xenapi/test_volumeops.py b/nova/tests/virt/xenapi/test_volumeops.py
index 7cc5c70da..844ae8459 100644
--- a/nova/tests/virt/xenapi/test_volumeops.py
+++ b/nova/tests/virt/xenapi/test_volumeops.py
@@ -21,6 +21,13 @@ from nova.virt.xenapi import volumeops
 
 class VolumeAttachTestCase(test.TestCase):
     def test_detach_volume_call(self):
+        registered_calls = []
+
+        def regcall(label):
+            def side_effect(*args, **kwargs):
+                registered_calls.append(label)
+            return side_effect
+
         ops = volumeops.VolumeOps('session')
         self.mox.StubOutWithMock(volumeops.vm_utils, 'vm_ref_or_raise')
         self.mox.StubOutWithMock(volumeops.vm_utils, 'find_vbd_by_number')
@@ -45,10 +52,12 @@ class VolumeAttachTestCase(test.TestCase):
 
         volumeops.vm_utils.unplug_vbd('session', 'vbdref')
 
-        volumeops.vm_utils.destroy_vbd('session', 'vbdref')
+        volumeops.vm_utils.destroy_vbd('session', 'vbdref').WithSideEffects(
+            regcall('destroy_vbd'))
 
         volumeops.volume_utils.find_sr_from_vbd(
-            'session', 'vbdref').AndReturn('srref')
+            'session', 'vbdref').WithSideEffects(
+                regcall('find_sr_from_vbd')).AndReturn('srref')
 
         volumeops.volume_utils.purge_sr('session', 'srref')
 
@@ -58,6 +67,9 @@ class VolumeAttachTestCase(test.TestCase):
             dict(driver_volume_type='iscsi', data='conn_data'),
             'instance_1', 'mountpoint')
 
+        self.assertEquals(
+            ['find_sr_from_vbd', 'destroy_vbd'], registered_calls)
+
     def test_attach_volume_call(self):
         ops = volumeops.VolumeOps('session')
         self.mox.StubOutWithMock(ops, '_connect_volume')
diff --git a/nova/virt/xenapi/volumeops.py b/nova/virt/xenapi/volumeops.py
index 5f79b6c3a..c2d717cfd 100644
--- a/nova/virt/xenapi/volumeops.py
+++ b/nova/virt/xenapi/volumeops.py
@@ -125,6 +125,7 @@ class VolumeOps(object):
         try:
             vbd_ref = vm_utils.find_vbd_by_number(self._session, vm_ref,
                                                   device_number)
+            sr_ref = volume_utils.find_sr_from_vbd(self._session, vbd_ref)
         except volume_utils.StorageError, exc:
             LOG.exception(exc)
             raise Exception(_('Unable to locate volume %s') % mountpoint)
@@ -143,7 +144,6 @@ class VolumeOps(object):
 
         # Forget SR only if no other volumes on this host are using it
         try:
-            sr_ref = volume_utils.find_sr_from_vbd(self._session, vbd_ref)
             volume_utils.purge_sr(self._session, sr_ref)
         except volume_utils.StorageError, exc:
             LOG.exception(exc)