Fixes lp797017, which is broken as a result of a fragile method in the xenapi drivers that assumed there would only ever be one VBD attached to an instance.

3 files changed, 31 insertions, 14 deletions

diff --git a/nova/tests/test_xenapi.py b/nova/tests/test_xenapi.py
index d1c88287a..c0213250a 100644
--- a/nova/tests/test_xenapi.py
+++ b/nova/tests/test_xenapi.py
@@ -33,6 +33,7 @@ from nova import utils
 from nova.auth import manager
 from nova.compute import instance_types
 from nova.compute import power_state
+from nova import exception
 from nova.virt import xenapi_conn
 from nova.virt.xenapi import fake as xenapi_fake
 from nova.virt.xenapi import volume_utils
@@ -228,6 +229,23 @@ class XenAPIVMTestCase(test.TestCase):
         instance = self._create_instance()
         self.conn.get_diagnostics(instance)
 
+    def test_instance_snapshot_fails_with_no_primary_vdi(self):
+        def create_bad_vbd(vm_ref, vdi_ref):
+            vbd_rec = {'VM': vm_ref,
+               'VDI': vdi_ref,
+               'userdevice': 'fake',
+               'currently_attached': False}
+            vbd_ref = xenapi_fake._create_object('VBD', vbd_rec)
+            xenapi_fake.after_VBD_create(vbd_ref, vbd_rec)
+            return vbd_ref
+
+        self.stubs.Set(xenapi_fake, 'create_vbd', create_bad_vbd)
+        stubs.stubout_instance_snapshot(self.stubs)
+        instance = self._create_instance()
+
+        name = "MySnapshot"
+        self.assertRaises(exception.Error, self.conn.snapshot, instance, name)
+
     def test_instance_snapshot(self):
         stubs.stubout_instance_snapshot(self.stubs)
         instance = self._create_instance()
diff --git a/nova/virt/xenapi/fake.py b/nova/virt/xenapi/fake.py
index 113198689..d5ac39473 100644
--- a/nova/virt/xenapi/fake.py
+++ b/nova/virt/xenapi/fake.py
@@ -146,6 +146,7 @@ def create_vdi(name_label, read_only, sr_ref, sharable):
 def create_vbd(vm_ref, vdi_ref):
     vbd_rec = {'VM': vm_ref,
                'VDI': vdi_ref,
+               'userdevice': '0',
                'currently_attached': False}
     vbd_ref = _create_object('VBD', vbd_rec)
     after_VBD_create(vbd_ref, vbd_rec)
diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py
index f13b428d5..f91958c57 100644
--- a/nova/virt/xenapi/vm_utils.py
+++ b/nova/virt/xenapi/vm_utils.py
@@ -283,19 +283,16 @@ class VMHelper(HelperBase):
 
     @classmethod
     def get_vdi_for_vm_safely(cls, session, vm_ref):
-        vdi_refs = VMHelper.lookup_vm_vdis(session, vm_ref)
-        if vdi_refs is None:
-            raise Exception(_("No VDIs found for VM %s") % vm_ref)
-        else:
-            num_vdis = len(vdi_refs)
-            if num_vdis != 1:
-                raise Exception(
-                        _("Unexpected number of VDIs (%(num_vdis)s) found"
-                        " for VM %(vm_ref)s") % locals())
-
-        vdi_ref = vdi_refs[0]
-        vdi_rec = session.get_xenapi().VDI.get_record(vdi_ref)
-        return vdi_ref, vdi_rec
+        """Retrieves the primary VDI for a VM"""
+        vbd_refs = session.get_xenapi().VM.get_VBDs(vm_ref)
+        for vbd in vbd_refs:
+            vbd_rec = session.get_xenapi().VBD.get_record(vbd)
+            # Convention dictates the primary VDI will be userdevice 0
+            if vbd_rec['userdevice'] == '0':
+                vdi_rec = session.get_xenapi().VDI.get_record(vbd_rec['VDI'])
+                return vbd_rec['VDI'], vdi_rec
+        raise exception.Error(_("No primary VDI found for"
+                "%(vm_ref)s") % locals())
 
     @classmethod
     def create_snapshot(cls, session, instance_id, vm_ref, label):
@@ -875,7 +872,8 @@ def get_vdi_for_vm_safely(session, vm_ref):
     else:
         num_vdis = len(vdi_refs)
         if num_vdis != 1:
-            raise Exception(_("Unexpected number of VDIs (%(num_vdis)s) found"
+            raise exception.Exception(_("Unexpected number of VDIs"
+                    "(%(num_vdis)s) found"
                     " for VM %(vm_ref)s") % locals())
 
     vdi_ref = vdi_refs[0]