Fix handling multiple WWPNs on preferred FC node

There was a bug in the code that checked to see if the
appropriate WWPN for a preferred node was available on the storage
host in intialize_connection.  It was assumed that there would only
ever be one preferred WWPN.  The code just checked to see if the list of
preferred WWPNs was in the list of available WWPNs.  This would only work
if all of the WWPNs were available on the storage host.  This resulted in
volumes not being able to be mounted when they should have been
available.

This fix changes the check of the preferred WWPNs to check each
item in the list until one is found.  The first one found is used
as the target WWPN.  If no match is found, the old default behavior
of selecting the first available WWPN is used.  I have added a
warning message for this case as it is quite possible that this
won't work in the user's environment as was the case that uncovered
this bug.

Note that this change is only relevant for systems that are not using
multipath as systems that use multipath don't choose one WWPN, they
just send the whole available list back.

Change-Id: I8d3239da24a8f9fbd8309ba5c90b05b29a068754
Close-bug: 1308298

diff --git a/cinder/tests/test_storwize_svc.py b/cinder/tests/test_storwize_svc.py
index 53a51317bc6f7021ca7835e215cd03e4436ced79..1b747854ad23987988515e65e0ae8e3af1fe8c27 100644 (file)
--- a/cinder/tests/test_storwize_svc.py
+++ b/cinder/tests/test_storwize_svc.py
@@ -2397,6 +2397,116 @@ class StorwizeSVCDriverTestCase(test.TestCase):
             self.driver.delete_volume(volume)
             self.assertNotIn(volume['id'], self.driver._vdiskcopyops)
 
+    def test_storwize_initiator_multiple_preferred_nodes_matching(self):
+        # Set the context.
+        ctxt = context.get_admin_context()
+
+        # Generate us a test volume
+        volume = self._generate_vol_info(None, None)
+        self.driver.create_volume(volume)
+
+        # Fibre Channel volume type
+        vol_type = volume_types.create(ctxt, 'FC', {'protocol': 'FC'})
+
+        volume['volume_type_id'] = vol_type['id']
+
+        # Make sure that the volumes have been created
+        self._assert_vol_exists(volume['name'], True)
+
+        #Set up one WWPN that won't match and one that will.
+        self.driver._state['storage_nodes']['1']['WWPN'] = ['123456789ABCDEF0',
+                                                            'AABBCCDDEEFF0010']
+
+        wwpns = ['ff00000000000000', 'ff00000000000001']
+        connector = {'host': 'storwize-svc-test', 'wwpns': wwpns}
+
+        with mock.patch.object(helpers.StorwizeHelpers,
+                               'get_conn_fc_wwpns') as get_mappings:
+            get_mappings.return_value = ['AABBCCDDEEFF0001',
+                                         'AABBCCDDEEFF0002',
+                                         'AABBCCDDEEFF0010',
+                                         'AABBCCDDEEFF0012']
+
+            # Initialize the connection
+            init_ret = self.driver.initialize_connection(volume, connector)
+
+            # Make sure we use the preferred WWPN.
+            self.assertEqual(init_ret['data']['target_wwn'],
+                             'AABBCCDDEEFF0010')
+
+    def test_storwize_initiator_multiple_preferred_nodes_no_matching(self):
+        # Set the context.
+        ctxt = context.get_admin_context()
+
+        # Generate us a test volume
+        volume = self._generate_vol_info(None, None)
+        self.driver.create_volume(volume)
+
+        # Fibre Channel volume type
+        vol_type = volume_types.create(ctxt, 'FC', {'protocol': 'FC'})
+
+        volume['volume_type_id'] = vol_type['id']
+
+        # Make sure that the volumes have been created
+        self._assert_vol_exists(volume['name'], True)
+
+        #Set up WWPNs that will not match what is available.
+        self.driver._state['storage_nodes']['1']['WWPN'] = ['123456789ABCDEF0',
+                                                            '123456789ABCDEF1']
+
+        wwpns = ['ff00000000000000', 'ff00000000000001']
+        connector = {'host': 'storwize-svc-test', 'wwpns': wwpns}
+
+        with mock.patch.object(helpers.StorwizeHelpers,
+                               'get_conn_fc_wwpns') as get_mappings:
+            get_mappings.return_value = ['AABBCCDDEEFF0001',
+                                         'AABBCCDDEEFF0002',
+                                         'AABBCCDDEEFF0010',
+                                         'AABBCCDDEEFF0012']
+
+            # Initialize the connection
+            init_ret = self.driver.initialize_connection(volume, connector)
+
+            # Make sure we use the first available WWPN.
+            self.assertEqual(init_ret['data']['target_wwn'],
+                             'AABBCCDDEEFF0001')
+
+    def test_storwize_initiator_single_preferred_node_matching(self):
+        # Set the context
+        ctxt = context.get_admin_context()
+
+        # Generate us a test volume
+        volume = self._generate_vol_info(None, None)
+        self.driver.create_volume(volume)
+
+        # Fibre Channel volume type
+        vol_type = volume_types.create(ctxt, 'FC', {'protocol': 'FC'})
+
+        volume['volume_type_id'] = vol_type['id']
+
+        # Make sure that the volumes have been created
+        self._assert_vol_exists(volume['name'], True)
+
+        #Set up one WWPN.
+        self.driver._state['storage_nodes']['1']['WWPN'] = ['AABBCCDDEEFF0012']
+
+        wwpns = ['ff00000000000000', 'ff00000000000001']
+        connector = {'host': 'storwize-svc-test', 'wwpns': wwpns}
+
+        with mock.patch.object(helpers.StorwizeHelpers,
+                               'get_conn_fc_wwpns') as get_mappings:
+            get_mappings.return_value = ['AABBCCDDEEFF0001',
+                                         'AABBCCDDEEFF0002',
+                                         'AABBCCDDEEFF0010',
+                                         'AABBCCDDEEFF0012']
+
+            # Initialize the connection
+            init_ret = self.driver.initialize_connection(volume, connector)
+
+            # Make sure we use the preferred WWPN.
+            self.assertEqual(init_ret['data']['target_wwn'],
+                             'AABBCCDDEEFF0012')
+
     def test_storwize_initiator_target_map(self):
         # Create two volumes to be used in mappings
         ctxt = context.get_admin_context()

diff --git a/cinder/volume/drivers/ibm/storwize_svc/__init__.py b/cinder/volume/drivers/ibm/storwize_svc/__init__.py
index 463e080aee350b9e07acc0ce991225f44a7abdcd..58362ac8bcdec95e7c9a01bd0a9e5108b5738e61 100644 (file)
--- a/cinder/volume/drivers/ibm/storwize_svc/__init__.py
+++ b/cinder/volume/drivers/ibm/storwize_svc/__init__.py
@@ -409,9 +409,21 @@ class StorwizeSVCDriver(san.SanDriver):
                     LOG.error(msg)
                     raise exception.VolumeBackendAPIException(data=msg)
                 if not vol_opts['multipath']:
-                    if preferred_node_entry['WWPN'] in conn_wwpns:
-                        properties['target_wwn'] = preferred_node_entry['WWPN']
+                    # preferred_node_entry can have a list of WWPNs while only
+                    # one WWPN may be available on the storage host.  Here we
+                    # walk through the nodes until we find one that works,
+                    # default to the first WWPN otherwise.
+                    for WWPN in preferred_node_entry['WWPN']:
+                        if WWPN in conn_wwpns:
+                            properties['target_wwn'] = WWPN
+                            break
                     else:
+                        LOG.warning(_('Unable to find a preferred node match '
+                                    'for node %(node)s in the list of '
+                                    'available WWPNs on %(host)s. '
+                                    'Using first available.') %
+                                    {'node': preferred_node,
+                                     'host': host_name})
                         properties['target_wwn'] = conn_wwpns[0]
                 else:
                     properties['target_wwn'] = conn_wwpns