]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Fixes a problem in attach volume in EMC driver.
authorXing Yang <xing.yang@emc.com>
Sat, 22 Mar 2014 19:05:42 +0000 (15:05 -0400)
committerXing Yang <xing.yang@emc.com>
Wed, 26 Mar 2014 22:55:41 +0000 (18:55 -0400)
This patch fixes a problem in attach volume in EMC SMI-S driver.
The existing logic checks if a volume is already attached to any host,
but it doesn't check whether a volume is already attached to the specific
host that nova wants cinder to attach.  As a result, initialize_connection
could return success (thinking it is already attached), but nova will
fail to discover the LUN later and fail the attach.

This patch adds a check to see if a volume is already attached to a
specific host.  If not, it will do the attach.  The reason that the
volume being attached already could be due to a nova live-migration
use case.  Cinder doesn't support multiple attaches currently, but
allows a volume to be attached multiple times from nova during
live-migration.

Change-Id: I05a2f57cd8708d7fcbe902ec13665a9cfb44db07
Closes-Bug: #1295906

cinder/tests/test_emc_smis.py
cinder/volume/drivers/emc/emc_smis_common.py
cinder/volume/drivers/emc/emc_smis_iscsi.py

index aa4372b6dd83c48dc232a3ec50a75e32abe06748..9bbde8acc700c714b23e8bd89206a8e3ba26d2ff 100644 (file)
@@ -398,8 +398,12 @@ class FakeEcomConnection():
     def _assoc_hdwid(self):
         assocs = []
         assoc = {}
-        assoc['StorageID'] = self.data.initiator1
+        assoc['StorageID'] = self.data.connector['initiator']
         assocs.append(assoc)
+        for wwpn in self.data.connector['wwpns']:
+            assoc2 = {}
+            assoc2['StorageID'] = wwpn
+            assocs.append(assoc2)
         return assocs
 
     def _assoc_endpoint(self):
index 29ce057e6e3883370f7e4aec4d6d2ce1d4ceaf5a..736d068dc2b5ac7626a0d5631ad6b3e321f95bfa 100644 (file)
@@ -901,7 +901,7 @@ class EMCSMISCommon():
         LOG.info(_('Unmap volume: %(volume)s')
                  % {'volume': volumename})
 
-        device_info = self.find_device_number(volume)
+        device_info = self.find_device_number(volume, connector)
         device_number = device_info['hostlunid']
         if device_number is None:
             LOG.info(_("Volume %s is not mapped. No volume to unmap.")
@@ -931,7 +931,7 @@ class EMCSMISCommon():
         LOG.info(_('Initialize connection: %(volume)s')
                  % {'volume': volumename})
         self.conn = self._get_ecom_connection()
-        device_info = self.find_device_number(volume)
+        device_info = self.find_device_number(volume, connector)
         device_number = device_info['hostlunid']
         if device_number is not None:
             LOG.info(_("Volume %s is already mapped.")
@@ -939,7 +939,7 @@ class EMCSMISCommon():
         else:
             self._map_lun(volume, connector)
             # Find host lun id again after the volume is exported to the host
-            device_info = self.find_device_number(volume)
+            device_info = self.find_device_number(volume, connector)
 
         return device_info
 
@@ -1533,7 +1533,7 @@ class EMCSMISCommon():
         return out_device_number
 
     # Find a device number that a host can see for a volume
-    def find_device_number(self, volume):
+    def find_device_number(self, volume, connector):
         out_num_device_number = None
 
         volumename = volume['name']
@@ -1546,29 +1546,47 @@ class EMCSMISCommon():
             # VMAX LUN doesn't have this property
             pass
 
-        unitnames = self.conn.ReferenceNames(
-            vol_instance.path,
-            ResultClass='CIM_ProtocolControllerForUnit')
-
-        for unitname in unitnames:
-            controller = unitname['Antecedent']
-            classname = controller['CreationClassName']
-            index = classname.find('LunMaskingSCSIProtocolController')
-            if index > -1:  # VNX
-                # Get an instance of CIM_ProtocolControllerForUnit
-                unitinstance = self.conn.GetInstance(unitname,
-                                                     LocalOnly=False)
-                numDeviceNumber = int(unitinstance['DeviceNumber'], 16)
-                out_num_device_number = numDeviceNumber
-                break
-            else:
-                index = classname.find('Symm_LunMaskingView')
-                if index > -1:  # VMAX
+        indexVMAX = storage_system.find('SYMMETRIX')
+        if indexVMAX == -1:
+            # find out whether the volume is already attached to the host
+            ctrl = self._find_lunmasking_scsi_protocol_controller_for_vol(
+                vol_instance,
+                connector)
+
+            LOG.debug(_("LunMaskingSCSIProtocolController for "
+                      "volume %(vol)s and connector %(connector)s "
+                      "is %(ctrl)s.")
+                      % {'vol': vol_instance.path,
+                         'connector': connector,
+                         'ctrl': ctrl})
+
+        if indexVMAX > -1 or ctrl:
+            unitnames = self.conn.ReferenceNames(
+                vol_instance.path,
+                ResultClass='CIM_ProtocolControllerForUnit')
+
+            for unitname in unitnames:
+                controller = unitname['Antecedent']
+                classname = controller['CreationClassName']
+                index = classname.find('LunMaskingSCSIProtocolController')
+                if index > -1:  # VNX
+                    if ctrl['DeviceID'] != controller['DeviceID']:
+                        continue
+                    # Get an instance of CIM_ProtocolControllerForUnit
                     unitinstance = self.conn.GetInstance(unitname,
                                                          LocalOnly=False)
                     numDeviceNumber = int(unitinstance['DeviceNumber'], 16)
                     out_num_device_number = numDeviceNumber
                     break
+                else:
+                    index = classname.find('Symm_LunMaskingView')
+                    if index > -1:  # VMAX
+                        unitinstance = self.conn.GetInstance(unitname,
+                                                             LocalOnly=False)
+                        numDeviceNumber = int(unitinstance['DeviceNumber'],
+                                              16)
+                        out_num_device_number = numDeviceNumber
+                        break
 
         if out_num_device_number is None:
             LOG.info(_("Device number not found for volume "
index 9548d49c0bd8341943298de2c9e3c0676c852f9a..205daafd04ec5eed1852039c0bf8017feec89af5 100644 (file)
@@ -127,7 +127,7 @@ class EMCSMISISCSIDriver(driver.ISCSIDriver):
         """Initializes the connection and returns connection info.
 
         The iscsi driver returns a driver_volume_type of 'iscsi'.
-        the format of the driver data is defined in _get_iscsi_properties.
+        the format of the driver data is defined in smis_get_iscsi_properties.
         Example return value::
 
             {
@@ -136,14 +136,14 @@ class EMCSMISISCSIDriver(driver.ISCSIDriver):
                     'target_discovered': True,
                     'target_iqn': 'iqn.2010-10.org.openstack:volume-00000001',
                     'target_portal': '127.0.0.0.1:3260',
-                    'volume_id': 1,
+                    'volume_id': '12345678-1234-4321-1234-123456789012',
                 }
             }
 
         """
         self.common.initialize_connection(volume, connector)
 
-        iscsi_properties = self._get_iscsi_properties(volume)
+        iscsi_properties = self.smis_get_iscsi_properties(volume, connector)
         return {
             'driver_volume_type': 'iscsi',
             'data': iscsi_properties
@@ -163,7 +163,7 @@ class EMCSMISISCSIDriver(driver.ISCSIDriver):
 
         return targets
 
-    def _get_iscsi_properties(self, volume):
+    def smis_get_iscsi_properties(self, volume, connector):
         """Gets iscsi configuration.
 
         We ideally get saved information in the volume entity, but fall back
@@ -178,7 +178,7 @@ class EMCSMISISCSIDriver(driver.ISCSIDriver):
 
         :target_lun:    the lun of the iSCSI target
 
-        :volume_id:    the id of the volume (currently used by xen)
+        :volume_id:    the UUID of the volume
 
         :auth_method:, :auth_username:, :auth_password:
 
@@ -197,7 +197,7 @@ class EMCSMISISCSIDriver(driver.ISCSIDriver):
         LOG.debug(_("ISCSI Discovery: Found %s") % (location))
         properties['target_discovered'] = True
 
-        device_info = self.common.find_device_number(volume)
+        device_info = self.common.find_device_number(volume, connector)
         if device_info is None or device_info['hostlunid'] is None:
             exception_message = (_("Cannot find device number for volume %s")
                                  % volume['name'])
@@ -245,6 +245,8 @@ class EMCSMISISCSIDriver(driver.ISCSIDriver):
 
         properties['volume_id'] = volume['id']
 
+        LOG.debug(_("ISCSI properties: %s") % (properties))
+
         auth = volume['provider_auth']
         if auth:
             (auth_method, auth_username, auth_secret) = auth.split()
@@ -253,8 +255,6 @@ class EMCSMISISCSIDriver(driver.ISCSIDriver):
             properties['auth_username'] = auth_username
             properties['auth_password'] = auth_secret
 
-        LOG.debug(_("ISCSI properties: %s") % (properties))
-
         return properties
 
     def terminate_connection(self, volume, connector, **kwargs):