From 0a4acba0fcf651f3df5ea48c8054e067777d0528 Mon Sep 17 00:00:00 2001 From: Helen Walsh Date: Wed, 12 Apr 2017 17:21:53 +0100 Subject: [PATCH] VMAX driver - Pre-zoned port group fix If there are more than one masking view on the same compute node with different portgroups, then code did not always pick the correct masking view to extract the port group from. This can happen when a system is pre-zoned FC. Change-Id: I6787f9415d97ce5988984f3aeac05c02c5217aac Closes-Bug: #1682176 --- .../unit/volume/drivers/dell_emc/test_vmax.py | 140 +++++++----------- cinder/volume/drivers/dell_emc/vmax/common.py | 59 ++------ cinder/volume/drivers/dell_emc/vmax/fc.py | 5 +- .../volume/drivers/dell_emc/vmax/masking.py | 9 ++ cinder/volume/drivers/dell_emc/vmax/utils.py | 40 ----- 5 files changed, 81 insertions(+), 172 deletions(-) diff --git a/cinder/tests/unit/volume/drivers/dell_emc/test_vmax.py b/cinder/tests/unit/volume/drivers/dell_emc/test_vmax.py index 94e8fede534..58735365fe8 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/test_vmax.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/test_vmax.py @@ -297,6 +297,10 @@ class VMAXCommonData(object): 'SYMMETRIX+000195900551+OS-fakehost-gold-I-MV') lunmaskctrl_name = ( 'OS-fakehost-gold-I-MV') + mv_instance_name = { + 'CreationClassName': 'Symm_LunMaskingView', + 'ElementName': 'OS-fakehost-SRP_1-Bronze-DSS-I-Mv', + 'SystemName': 'SYMMETRIX+000195900551'} rdf_group = 'test_rdf' srdf_group_instance = ( @@ -4473,6 +4477,10 @@ class VMAXISCSIDriverFastTestCase(test.TestCase): self.data.test_volume, self.data.connector) + @mock.patch.object( + common.VMAXCommon, + 'get_target_wwns_from_masking_view', + return_value=[{'Name': '5000090000000000'}]) @mock.patch.object( masking.VMAXMasking, 'get_initiator_group_from_masking_view', @@ -4491,7 +4499,7 @@ class VMAXISCSIDriverFastTestCase(test.TestCase): return_value={'volume_backend_name': 'ISCSIFAST'}) def test_detach_fast_success( self, mock_volume_type, mock_storage_group, - mock_ig, mock_igc): + mock_ig, mock_igc, mock_tw): self.driver.terminate_connection( self.data.test_volume, self.data.connector) @@ -5661,15 +5669,16 @@ class VMAXFCDriverFastTestCase(test.TestCase): def test_map_fast_success(self, _mock_volume_type, mock_maskingview, mock_is_same_host): common = self.driver.common - common.get_target_wwns = mock.Mock( + common.get_target_wwns_list = mock.Mock( return_value=VMAXCommonData.target_wwns) self.driver.common._get_correct_port_group = mock.Mock( return_value=self.data.port_group) data = self.driver.initialize_connection( self.data.test_volume, self.data.connector) # Test the no lookup service, pre-zoned case. - common.get_target_wwns.assert_called_once_with( - VMAXCommonData.storage_system, VMAXCommonData.connector) + common.get_target_wwns_list.assert_called_once_with( + VMAXCommonData.storage_system, self.data.test_volume, + VMAXCommonData.connector) for init, target in data['data']['initiator_target_map'].items(): self.assertIn(init[::-1], target) @@ -5716,12 +5725,13 @@ class VMAXFCDriverFastTestCase(test.TestCase): def test_detach_fast_success(self, mock_volume_type, mock_maskingview, mock_ig, mock_igc, mock_mv, mock_check_ig): common = self.driver.common - common.get_target_wwns = mock.Mock( + common.get_target_wwns_list = mock.Mock( return_value=VMAXCommonData.target_wwns) data = self.driver.terminate_connection(self.data.test_volume, self.data.connector) - common.get_target_wwns.assert_called_once_with( - VMAXCommonData.storage_system, VMAXCommonData.connector) + common.get_target_wwns_list.assert_called_once_with( + VMAXCommonData.storage_system, self.data.test_volume, + VMAXCommonData.connector) numTargetWwns = len(VMAXCommonData.target_wwns) self.assertEqual(numTargetWwns, len(data['data'])) @@ -6725,7 +6735,7 @@ class EMCV3DriverTestCase(test.TestCase): self, _mock_volume_type, mock_maskingview, mock_is_same_host, mock_element_name): common = self.driver.common - common.get_target_wwns = mock.Mock( + common.get_target_wwns_list = mock.Mock( return_value=VMAXCommonData.target_wwns) self.driver.common._initial_setup = mock.Mock( return_value=self.default_extraspec()) @@ -6734,8 +6744,9 @@ class EMCV3DriverTestCase(test.TestCase): data = self.driver.initialize_connection( self.data.test_volume_v3, self.data.connector) # Test the no lookup service, pre-zoned case. - common.get_target_wwns.assert_called_once_with( - VMAXCommonData.storage_system, VMAXCommonData.connector) + common.get_target_wwns_list.assert_called_once_with( + VMAXCommonData.storage_system, self.data.test_volume_v3, + VMAXCommonData.connector) for init, target in data['data']['initiator_target_map'].items(): self.assertIn(init[::-1], target) @@ -6755,6 +6766,10 @@ class EMCV3DriverTestCase(test.TestCase): self.data.test_volume, self.data.connector) + @mock.patch.object( + masking.VMAXMasking, + 'get_port_group_from_masking_view', + return_value='myPortGroup') @mock.patch.object( masking.VMAXMasking, 'remove_and_reset_members') @@ -6781,23 +6796,25 @@ class EMCV3DriverTestCase(test.TestCase): @mock.patch.object( masking.VMAXMasking, 'get_masking_view_from_storage_group', - return_value=VMAXCommonData.lunmaskctrl_name) + return_value=[VMAXCommonData.mv_instance_name]) @mock.patch.object( volume_types, 'get_volume_type_extra_specs', return_value={'volume_backend_name': 'V3_BE'}) def test_detach_v3_success(self, mock_volume_type, mock_maskingview, mock_ig, mock_igc, mock_mv, mock_check_ig, - mock_element_name, mock_remove): + mock_element_name, mock_remove, mock_pg): common = self.driver.common - with mock.patch.object(common, 'get_target_wwns', + with mock.patch.object(common, 'get_target_wwns_list', return_value=VMAXCommonData.target_wwns): with mock.patch.object(common, '_initial_setup', return_value=self.default_extraspec()): data = self.driver.terminate_connection( self.data.test_volume_v3, self.data.connector) - common.get_target_wwns.assert_called_once_with( - VMAXCommonData.storage_system, VMAXCommonData.connector) + common.get_target_wwns_list.assert_called_once_with( + VMAXCommonData.storage_system, + self.data.test_volume_v3, + VMAXCommonData.connector) numTargetWwns = len(VMAXCommonData.target_wwns) self.assertEqual(numTargetWwns, len(data['data'])) @@ -8333,7 +8350,7 @@ class VMAXFCTest(test.TestCase): return_value='testMV') common.get_masking_views_by_port_group = mock.Mock( return_value=[]) - common.get_target_wwns = mock.Mock( + common.get_target_wwns_list = mock.Mock( return_value=VMAXCommonData.target_wwns) initiatorGroupInstanceName = ( self.driver.common.masking._get_initiator_group_from_masking_view( @@ -8344,8 +8361,9 @@ class VMAXFCTest(test.TestCase): return_value=initiatorGroupInstanceName): data = self.driver.terminate_connection(self.data.test_volume_v3, self.data.connector) - common.get_target_wwns.assert_called_once_with( - VMAXCommonData.storage_system, VMAXCommonData.connector) + common.get_target_wwns_list.assert_called_once_with( + VMAXCommonData.storage_system, self.data.test_volume_v3, + VMAXCommonData.connector) numTargetWwns = len(VMAXCommonData.target_wwns) self.assertEqual(numTargetWwns, len(data['data'])) @@ -8355,7 +8373,7 @@ class VMAXFCTest(test.TestCase): return_value=None) @mock.patch.object( common.VMAXCommon, - 'get_target_wwns', + 'get_target_wwns_list', return_value=VMAXCommonData.target_wwns) @mock.patch.object( common.VMAXCommon, @@ -8375,8 +8393,9 @@ class VMAXFCTest(test.TestCase): common.conn = FakeEcomConnection() data = self.driver.terminate_connection(self.data.test_volume_v3, self.data.connector) - common.get_target_wwns.assert_called_once_with( - VMAXCommonData.storage_system, VMAXCommonData.connector) + common.get_target_wwns_list.assert_called_once_with( + VMAXCommonData.storage_system, self.data.test_volume_v3, + VMAXCommonData.connector) numTargetWwns = len(VMAXCommonData.target_wwns) self.assertEqual(numTargetWwns, len(data['data'])) @@ -8544,30 +8563,6 @@ class VMAXUtilsTest(test.TestCase): self.driver = driver self.driver.utils = utils.VMAXUtils(object) - def test_get_target_endpoints(self): - conn = FakeEcomConnection() - hardwareid = 123456789012345 - result = self.driver.utils.get_target_endpoints(conn, hardwareid) - self.assertEqual( - ([{'Name': '5000090000000000'}]), result) - - def test_get_protocol_controller(self): - conn = FakeEcomConnection() - hardwareid = 123456789012345 - result = self.driver.utils.get_protocol_controller(conn, hardwareid) - self.assertEqual( - ({'CreationClassName': 'Symm_LunMaskingView', - 'ElementName': 'OS-fakehost-gold-I-MV'}), result) - - def test_get_protocol_controller_exception(self): - conn = FakeEcomConnection() - conn.AssociatorNames = mock.Mock(return_value=[]) - hardwareid = 123456789012345 - self.assertRaises( - exception.VolumeBackendAPIException, - self.driver.utils.get_protocol_controller, - conn, hardwareid) - def test_set_target_element_supplier_in_rsd(self): conn = FakeEcomConnection() extraSpecs = self.data.extra_specs @@ -8832,57 +8827,30 @@ class VMAXCommonTest(test.TestCase): sourceInstance, cloneName, extraSpecs) self.assertIsNotNone(duplicateVolumeInstance) - def test_get_target_wwn(self): + @mock.patch.object( + common.VMAXCommon, + 'get_target_wwns_from_masking_view', + return_value=["5000090000000000"]) + def test_get_target_wwn_list(self, mock_tw): common = self.driver.common common.conn = FakeEcomConnection() - targetWwns = common.get_target_wwns( - VMAXCommonData.storage_system, VMAXCommonData.connector) + targetWwns = common.get_target_wwns_list( + VMAXCommonData.storage_system, + VMAXCommonData.test_volume_v3, VMAXCommonData.connector) self.assertListEqual(["5000090000000000"], targetWwns) @mock.patch.object( - utils.VMAXUtils, - 'get_target_endpoints', - return_value=None) - def test_get_target_wwn_all_invalid(self, mock_target_ep): + common.VMAXCommon, + 'get_target_wwns_from_masking_view', + return_value=[]) + def test_get_target_wwn_list_empty(self, mock_tw): common = self.driver.common common.conn = FakeEcomConnection() self.assertRaises( exception.VolumeBackendAPIException, - common.get_target_wwns, VMAXCommonData.storage_system, - VMAXCommonData.connector) - - def test_get_target_wwn_one_invalid(self): - common = self.driver.common - common.conn = FakeEcomConnection() - targetEndpoints = [{'CreationClassName': 'EMC_FCSCSIProtocolEndpoint', - 'Name': '5000090000000000'}] - hardwareInstanceNames = ( - [{'CreationClassName': 'EMC_StorageHardwareID'}] * 3) - e = exception.VolumeBackendAPIException('Get target endpoint ex') - with mock.patch.object(common, '_find_storage_hardwareids', - return_value=hardwareInstanceNames): - with mock.patch.object(common.utils, 'get_target_endpoints', - side_effect=[e, None, targetEndpoints]): - targetWwns = common.get_target_wwns( - VMAXCommonData.storage_system, - VMAXCommonData.connector) - self.assertListEqual(["5000090000000000"], targetWwns) - - def test_get_target_wwn_all_invalid_endpoints(self): - common = self.driver.common - common.conn = FakeEcomConnection() - hardwareInstanceNames = ( - [{'CreationClassName': 'EMC_StorageHardwareID'}] * 3) - e = exception.VolumeBackendAPIException('Get target endpoint ex') - with mock.patch.object(common, '_find_storage_hardwareids', - return_value=hardwareInstanceNames): - with mock.patch.object(common.utils, 'get_target_endpoints', - side_effect=[e, None, None]): - self.assertRaises( - exception.VolumeBackendAPIException, - common.get_target_wwns, VMAXCommonData.storage_system, - VMAXCommonData.connector) + common.get_target_wwns_list, VMAXCommonData.storage_system, + VMAXCommonData.test_volume_v3, VMAXCommonData.connector) def test_cleanup_target(self): common = self.driver.common diff --git a/cinder/volume/drivers/dell_emc/vmax/common.py b/cinder/volume/drivers/dell_emc/vmax/common.py index 8e93fca3f32..3d7fccfdfa0 100644 --- a/cinder/volume/drivers/dell_emc/vmax/common.py +++ b/cinder/volume/drivers/dell_emc/vmax/common.py @@ -2008,59 +2008,32 @@ class VMAXCommon(object): LOG.debug("Device info: %(data)s.", {'data': data}) return data, isLiveMigration, source_data - def get_target_wwns(self, storageSystem, connector): - """Find target WWNs. + def get_target_wwns_list(self, storage_system, volume, connector): + """Find target WWN list. :param storageSystem: the storage system name :param connector: the connector dict :returns: list -- targetWwns, the target WWN list - :raises VolumeBackendAPIException: + :raises: VolumeBackendAPIException """ targetWwns = set() + try: + fc_targets = self.get_target_wwns_from_masking_view( + storage_system, volume, connector) + except Exception: + exception_message = _("Unable to get fc targets.") + raise exception.VolumeBackendAPIException( + data=exception_message) - storageHardwareService = self.utils.find_storage_hardwareid_service( - self.conn, storageSystem) - - hardwareIdInstances = self._find_storage_hardwareids( - connector, storageHardwareService) - - LOG.debug( - "EMCGetTargetEndpoints: Service: %(service)s, " - "Storage HardwareIDs: %(hardwareIds)s.", - {'service': storageHardwareService, - 'hardwareIds': hardwareIdInstances}) - - for hardwareIdInstance in hardwareIdInstances: - LOG.debug("HardwareID instance is: %(hardwareIdInstance)s.", - {'hardwareIdInstance': hardwareIdInstance}) - try: - targetEndpoints = ( - self.utils.get_target_endpoints( - self.conn, hardwareIdInstance)) - if not targetEndpoints: - LOG.warning( - "Unable to get target endpoints for hardwareId " - "%(instance)s.", - {'instance': hardwareIdInstance}) - continue - except Exception: - LOG.warning( - "Unable to get target endpoints for hardwareId " - "%(instance)s.", - {'instance': hardwareIdInstance}, exc_info=True) - continue - - LOG.debug("There are %(len)lu endpoints.", - {'len': len(targetEndpoints)}) - for targetendpoint in targetEndpoints: - wwn = targetendpoint['Name'] - # Add target wwn to the list if it is not already there. - targetWwns.add(wwn) - break + LOG.debug("There are %(len)lu endpoints.", {'len': len(fc_targets)}) + for fc_target in fc_targets: + wwn = fc_target + # Add target wwn to the list if it is not already there. + targetWwns.add(wwn) if not targetWwns: exception_message = (_( - "Unable to get target endpoints for any hardwareIds.")) + "Unable to get target endpoints.")) raise exception.VolumeBackendAPIException(data=exception_message) LOG.debug("Target WWNs: %(targetWwns)s.", diff --git a/cinder/volume/drivers/dell_emc/vmax/fc.py b/cinder/volume/drivers/dell_emc/vmax/fc.py index 4e8dc910bec..7800294a980 100644 --- a/cinder/volume/drivers/dell_emc/vmax/fc.py +++ b/cinder/volume/drivers/dell_emc/vmax/fc.py @@ -76,7 +76,6 @@ class VMAXFCDriver(driver.FibreChannelDriver): - Support for compression on All Flash - Volume replication 2.1 (bp add-vmax-replication) - rename and restructure driver (bp vmax-rename-dell-emc) - """ VERSION = "2.5.0" @@ -357,8 +356,8 @@ class VMAXFCDriver(driver.FibreChannelDriver): for initiator in map_d['initiator_port_wwn_list']: init_targ_map[initiator] = map_d['target_port_wwn_list'] else: # No lookup service, pre-zoned case. - target_wwns = self.common.get_target_wwns(storage_system, - connector) + target_wwns = self.common.get_target_wwns_list( + storage_system, volume, connector) for initiator in initiator_wwns: init_targ_map[initiator] = target_wwns diff --git a/cinder/volume/drivers/dell_emc/vmax/masking.py b/cinder/volume/drivers/dell_emc/vmax/masking.py index 0bddb4fdcc7..3264e72e9cc 100644 --- a/cinder/volume/drivers/dell_emc/vmax/masking.py +++ b/cinder/volume/drivers/dell_emc/vmax/masking.py @@ -2497,6 +2497,15 @@ class VMAXMasking(object): conn, sgInstanceName) # Get initiator group from masking view. for mvInstanceName in mvInstanceNames: + host = self.utils.get_host_short_name(connector['host']) + mvInstance = conn.GetInstance(mvInstanceName) + if host not in mvInstance['ElementName']: + LOG.info( + "Check 1: Connector host %(connHost)s " + "does not match mv host %(mvHost)s. Skipping...", + {'connHost': host, + 'mvHost': mvInstance['ElementName']}) + continue LOG.debug("Found masking view associated with SG " "%(storageGroup)s: %(maskingview)s", {'maskingview': mvInstanceName, diff --git a/cinder/volume/drivers/dell_emc/vmax/utils.py b/cinder/volume/drivers/dell_emc/vmax/utils.py index 227a8eb3992..d5013e29613 100644 --- a/cinder/volume/drivers/dell_emc/vmax/utils.py +++ b/cinder/volume/drivers/dell_emc/vmax/utils.py @@ -2348,46 +2348,6 @@ class VMAXUtils(object): foundIpAddress = cimProperties.value return foundIpAddress - def get_target_endpoints(self, conn, hardwareId): - """Given the hardwareId get the target endpoints. - - :param conn: the connection to the ecom server - :param hardwareId: the hardware Id - :returns: targetEndpoints - :raises: VolumeBackendAPIException - """ - protocolControllerInstanceName = self.get_protocol_controller( - conn, hardwareId) - - targetEndpoints = conn.AssociatorNames( - protocolControllerInstanceName, - ResultClass='EMC_FCSCSIProtocolEndpoint') - - return targetEndpoints - - def get_protocol_controller(self, conn, hardwareinstancename): - """Get the front end protocol endpoints of a hardware instance - - :param conn: the ecom connection - :param hardwareinstancename: the hardware instance name - :returns: protocolControllerInstanceName - :raises: VolumeBackendAPIException - """ - protocolControllerInstanceName = None - protocol_controllers = conn.AssociatorNames( - hardwareinstancename, - ResultClass='EMC_FrontEndSCSIProtocolController') - if len(protocol_controllers) > 0: - protocolControllerInstanceName = protocol_controllers[0] - if protocolControllerInstanceName is None: - exceptionMessage = (_( - "Unable to get target endpoints for hardwareId " - "%(hardwareIdInstance)s.") - % {'hardwareIdInstance': hardwareinstancename}) - LOG.error(exceptionMessage) - raise exception.VolumeBackendAPIException(data=exceptionMessage) - return protocolControllerInstanceName - def get_replication_setting_data(self, conn, repServiceInstanceName, replication_type, extraSpecs): """Get the replication setting data