Fixes KeyError: 'sr_uuid' when booting from volume on xenapi

This change fixes the capability for a XenServer/XCP hypervisor
to boot instances from an iscsi volume running on Cinder.

This patch takes into account that Cinder can be configured
with an iscsi back-end, instead of a xensm one, where keys like
sr_uuid are not being specified. It works through the magic of
determining the SR, and the vdi associated with the iscsi target
and lun.

This patch also does a bit of clean-up of code/comments that no
longer apply.

Addresses bug #1087308

Change-Id: Ie7d65eeb965b3468e4407981788725e2b43bff5e

4 files changed, 76 insertions, 81 deletions

diff --git a/nova/tests/xenapi/test_vm_utils.py b/nova/tests/xenapi/test_vm_utils.py
index 3a8c9a640..a12c0f268 100644
--- a/nova/tests/xenapi/test_vm_utils.py
+++ b/nova/tests/xenapi/test_vm_utils.py
@@ -10,6 +10,29 @@ from nova.virt.xenapi import volume_utils
 import unittest
 
 
+XENSM_TYPE = 'xensm'
+ISCSI_TYPE = 'iscsi'
+
+
+def get_fake_dev_params(sr_type):
+    fakes = {XENSM_TYPE: {'sr_uuid': 'falseSR',
+                          'name_label': 'fake_storage',
+                          'name_description': 'test purposes',
+                          'server': 'myserver',
+                          'serverpath': '/local/scratch/myname',
+                          'sr_type': 'nfs',
+                          'introduce_sr_keys': ['server',
+                                                'serverpath',
+                                                'sr_type'],
+                          'vdi_uuid': 'falseVDI'},
+             ISCSI_TYPE: {'volume_id': 'fake_volume_id',
+                          'target_lun': 1,
+                          'target_iqn': 'fake_iqn:volume-fake_volume_id',
+                          'target_portal': u'localhost:3260',
+                          'target_discovered': False}, }
+    return fakes[sr_type]
+
+
 class GetInstanceForVdisForSrTestCase(stubs.XenAPITestBase):
     def setUp(self):
         super(GetInstanceForVdisForSrTestCase, self).setUp()
@@ -50,15 +73,8 @@ class GetInstanceForVdisForSrTestCase(stubs.XenAPITestBase):
 
         self.assertEquals([], result)
 
-    def test_get_vdis_for_boot_from_vol(self):
-        dev_params = {'sr_uuid': 'falseSR',
-                      'name_label': 'fake_storage',
-                      'name_description': 'test purposes',
-                      'server': 'myserver',
-                      'serverpath': '/local/scratch/myname',
-                      'sr_type': 'nfs',
-                      'introduce_sr_keys': ['server', 'serverpath', 'sr_type'],
-                      'vdi_uuid': 'falseVDI'}
+    def test_get_vdis_for_boot_from_vol_with_sr_uuid(self):
+        dev_params = get_fake_dev_params(XENSM_TYPE)
         stubs.stubout_session(self.stubs, fake.SessionBase)
         driver = xenapi_conn.XenAPIDriver(False)
 
@@ -74,18 +90,20 @@ class GetInstanceForVdisForSrTestCase(stubs.XenAPITestBase):
             return None
 
         self.stubs.Set(volume_utils, 'introduce_sr', bad_introduce_sr)
-        dev_params = {'sr_uuid': 'falseSR',
-                      'name_label': 'fake_storage',
-                      'name_description': 'test purposes',
-                      'server': 'myserver',
-                      'serverpath': '/local/scratch/myname',
-                      'sr_type': 'nfs',
-                      'introduce_sr_keys': ['server', 'serverpath', 'sr_type'],
-                      'vdi_uuid': 'falseVDI'}
+        dev_params = get_fake_dev_params(XENSM_TYPE)
         self.assertRaises(exception.NovaException,
                           vm_utils.get_vdis_for_boot_from_vol,
                           driver._session, dev_params)
 
+    def test_get_vdis_for_boot_from_iscsi_vol_missing_sr_uuid(self):
+        dev_params = get_fake_dev_params(ISCSI_TYPE)
+        stubs.stubout_session(self.stubs, fake.SessionBase)
+        driver = xenapi_conn.XenAPIDriver(False)
+
+        result = vm_utils.get_vdis_for_boot_from_vol(driver._session,
+                                                     dev_params)
+        self.assertNotEquals(result['root']['uuid'], None)
+
 
 class VMRefOrRaiseVMFoundTestCase(unittest.TestCase):
 
diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py
index be1a0f4ef..4e2b5e977 100644
--- a/nova/virt/xenapi/vm_utils.py
+++ b/nova/virt/xenapi/vm_utils.py
@@ -462,35 +462,29 @@ def create_vdi(session, sr_ref, instance, name_label, disk_type, virtual_size,
 
 def get_vdis_for_boot_from_vol(session, dev_params):
     vdis = {}
-    sr_uuid = dev_params['sr_uuid']
-    sr_ref = volume_utils.find_sr_by_uuid(session,
-                                          sr_uuid)
+    sr_uuid, label, sr_params = volume_utils.parse_sr_info(dev_params)
+    sr_ref = volume_utils.find_sr_by_uuid(session, sr_uuid)
     # Try introducing SR if it is not present
     if not sr_ref:
-        if 'name_label' not in dev_params:
-            label = 'tempSR-%s' % dev_params['volume_id']
-        else:
-            label = dev_params['name_label']
-
-        if 'name_description' not in dev_params:
-            desc = ''
-        else:
-            desc = dev_params.get('name_description')
-        sr_params = {}
-        for k in dev_params['introduce_sr_keys']:
-            sr_params[k] = dev_params[k]
-
-        sr_params['name_description'] = desc
-        sr_ref = volume_utils.introduce_sr(session, sr_uuid, label,
-                                           sr_params)
+        sr_ref = volume_utils.introduce_sr(session, sr_uuid, label, sr_params)
 
     if sr_ref is None:
         raise exception.NovaException(_('SR not present and could not be '
                                         'introduced'))
     else:
-        session.call_xenapi("SR.scan", sr_ref)
-        return {'root': dict(uuid=dev_params['vdi_uuid'],
-                file=None, osvol=True)}
+        if 'vdi_uuid' in dev_params:
+            session.call_xenapi("SR.scan", sr_ref)
+            vdis = {'root': dict(uuid=dev_params['vdi_uuid'],
+                    file=None, osvol=True)}
+        else:
+            try:
+                vdi_ref = volume_utils.introduce_vdi(session, sr_ref)
+                vdi_rec = session.call_xenapi("VDI.get_record", vdi_ref)
+                vdis = {'root': dict(uuid=vdi_rec['uuid'],
+                                     file=None, osvol=True)}
+            except volume_utils.StorageError, exc:
+                LOG.exception(exc)
+                volume_utils.forget_sr(session, sr_uuid)
     return vdis
 
 
@@ -523,8 +517,7 @@ def get_vdis_for_instance(context, session, instance, name_label, image,
             bdm_root_dev = block_device_info['block_device_mapping'][0]
             dev_params = bdm_root_dev['connection_info']['data']
             LOG.debug(dev_params)
-            return get_vdis_for_boot_from_vol(session,
-                                             dev_params)
+            return get_vdis_for_boot_from_vol(session, dev_params)
     return _create_image(context, session, instance, name_label, image,
                         image_type)
 
diff --git a/nova/virt/xenapi/volume_utils.py b/nova/virt/xenapi/volume_utils.py
index 17c4c3300..b632401ac 100644
--- a/nova/virt/xenapi/volume_utils.py
+++ b/nova/virt/xenapi/volume_utils.py
@@ -67,8 +67,9 @@ def create_sr(session, label, params):
 
 def introduce_sr(session, sr_uuid, label, params):
     LOG.debug(_("introducing sr within volume_utils"))
-    type = params['sr_type']
-    del params['sr_type']
+    # If the sr_type is missing, we assume we are
+    # using the default iscsi back-end
+    type = params.pop('sr_type', 'iscsi')
     LOG.debug(_('type is = %s') % type)
     if 'name_description' in params:
         desc = params['name_description']
@@ -283,18 +284,29 @@ def get_device_number(mountpoint):
     return device_number
 
 
+def parse_sr_info(connection_data, description=''):
+    label = connection_data.pop('name_label',
+                                'tempSR-%s' % connection_data.get('volume_id'))
+    params = {}
+    if 'sr_uuid' not in connection_data:
+        params = parse_volume_info(connection_data)
+        # This magic label sounds a lot like 'False Disc' in leet-speak
+        uuid = "FA15E-D15C-" + str(params['id'])
+    else:
+        uuid = connection_data['sr_uuid']
+        for k in connection_data.get('introduce_sr_keys', {}):
+            params[k] = connection_data[k]
+    params['name_description'] = connection_data.get('name_description',
+                                                     description)
+
+    return (uuid, label, params)
+
+
 def parse_volume_info(connection_data):
     """
     Parse device_path and mountpoint as they can be used by XenAPI.
     In particular, the mountpoint (e.g. /dev/sdc) must be translated
     into a numeric literal.
-    FIXME(armando):
-    As for device_path, currently cannot be used as it is,
-    because it does not contain target information. As for interim
-    solution, target details are passed either via Flags or obtained
-    by iscsiadm. Long-term solution is to add a few more fields to the
-    db in the iscsi_target table with the necessary info and modify
-    the iscsi driver to set them.
     """
     volume_id = connection_data['volume_id']
     target_portal = connection_data['target_portal']
@@ -369,12 +381,3 @@ def _get_target_port(iscsi_string):
         return iscsi_string[iscsi_string.find(':') + 1:]
     elif iscsi_string is None or CONF.target_port:
         return CONF.target_port
-
-
-def _get_iqn(iscsi_string, id):
-    """Retrieve target IQN"""
-    if iscsi_string:
-        return iscsi_string
-    elif iscsi_string is None or CONF.iqn_prefix:
-        volume_id = _get_volume_id(id)
-        return '%s:%s' % (CONF.iqn_prefix, volume_id)
diff --git a/nova/virt/xenapi/volumeops.py b/nova/virt/xenapi/volumeops.py
index ae21518d8..d17adeba6 100644
--- a/nova/virt/xenapi/volumeops.py
+++ b/nova/virt/xenapi/volumeops.py
@@ -130,28 +130,9 @@ class VolumeOps(object):
     def connect_volume(self, connection_data, dev_number, instance_name,
                        vm_ref):
 
-        if 'name_label' not in connection_data:
-            label = 'tempSR-%s' % connection_data['volume_id']
-        else:
-            label = connection_data['name_label']
-            del connection_data['name_label']
-
-        if 'name_description' not in connection_data:
-            desc = 'Disk-for:%s' % instance_name
-        else:
-            desc = connection_data['name_description']
-
-        sr_params = {}
-        if u'sr_uuid' not in connection_data:
-            sr_params = volume_utils.parse_volume_info(connection_data)
-            uuid = "FA15E-D15C-" + str(sr_params['id'])
-            sr_params['sr_type'] = 'iscsi'
-        else:
-            uuid = connection_data['sr_uuid']
-            for k in connection_data['introduce_sr_keys']:
-                sr_params[k] = connection_data[k]
-
-        sr_params['name_description'] = desc
+        description = 'Disk-for:%s' % instance_name
+        uuid, label, sr_params = volume_utils.parse_sr_info(connection_data,
+                                                            description)
 
         # Introduce SR
         try: