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

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(
+        common.get_target_wwns_list.assert_called_once_with(
-            VMAXCommonData.storage_system, VMAXCommonData.connector)
+            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(
+        common.get_target_wwns_list.assert_called_once_with(
-            VMAXCommonData.storage_system, VMAXCommonData.connector)
+            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(
+        common.get_target_wwns_list.assert_called_once_with(
-            VMAXCommonData.storage_system, VMAXCommonData.connector)
+            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(
+                common.get_target_wwns_list.assert_called_once_with(
-                    VMAXCommonData.storage_system, VMAXCommonData.connector)
+                    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(
+        common.get_target_wwns_list.assert_called_once_with(
-            VMAXCommonData.storage_system, VMAXCommonData.connector)
+            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(
+        common.get_target_wwns_list.assert_called_once_with(
-            VMAXCommonData.storage_system, VMAXCommonData.connector)
+            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(
+        targetWwns = common.get_target_wwns_list(
-            VMAXCommonData.storage_system, VMAXCommonData.connector)
+            VMAXCommonData.storage_system,
+            VMAXCommonData.test_volume_v3, VMAXCommonData.connector)
         self.assertListEqual(["5000090000000000"], targetWwns)
 
     @mock.patch.object(
-        utils.VMAXUtils,
+        common.VMAXCommon,
-        'get_target_endpoints',
+        'get_target_wwns_from_masking_view',
-        return_value=None)
+        return_value=[])
-    def test_get_target_wwn_all_invalid(self, mock_target_ep):
+    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,
+            common.get_target_wwns_list, VMAXCommonData.storage_system,
-            VMAXCommonData.connector)
+            VMAXCommonData.test_volume_v3, 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)
 
     def test_cleanup_target(self):
         common = self.driver.common

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):
+    def get_target_wwns_list(self, storage_system, volume, connector):
-        """Find target WWNs.
+        """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(
+        LOG.debug("There are %(len)lu endpoints.", {'len': len(fc_targets)})
-            self.conn, storageSystem)
+        for fc_target in fc_targets:
-
+            wwn = fc_target
-        hardwareIdInstances = self._find_storage_hardwareids(
+            # Add target wwn to the list if it is not already there.
-            connector, storageHardwareService)
+            targetWwns.add(wwn)
-
-        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
 
         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.",

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,
+            target_wwns = self.common.get_target_wwns_list(
-                                                      connector)
+                storage_system, volume, connector)
             for initiator in initiator_wwns:
                 init_targ_map[initiator] = target_wwns
 

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,

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