From 9bfc5b0bf58ecb7de5904cbd41db6de7330187e3 Mon Sep 17 00:00:00 2001 From: "Jay S. Bryant" Date: Fri, 18 Apr 2014 17:07:15 -0500 Subject: [PATCH] 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 --- cinder/tests/test_storwize_svc.py | 110 ++++++++++++++++++ .../drivers/ibm/storwize_svc/__init__.py | 16 ++- 2 files changed, 124 insertions(+), 2 deletions(-) diff --git a/cinder/tests/test_storwize_svc.py b/cinder/tests/test_storwize_svc.py index 53a51317b..1b747854a 100644 --- 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 463e080ae..58362ac8b 100644 --- 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 -- 2.45.2