EMC VMAX - multi-attach failure to VMAX3 when SLO is omitted

An SLO of NONE is a valid FAST configuration so when the SLO was not included in the xml file we assigned NONE to it. The omission of SLO should mean Non FAST in VMAX3, similar to how VMAX2 is implemented. Change-Id: I6cafd45d0f2584c0cc2aba427546c153ca53a101 Closes-Bug: #1567577
2016-04-21 13:28:11 +01:00 · 2016-04-21 13:28:11 +01:00 · f37a80a505
commit f37a80a505
parent f2d6bea097
4 changed files with 161 additions and 44 deletions
--- a/cinder/tests/unit/volume/drivers/emc/test_emc_vmax.py
+++ b/cinder/tests/unit/volume/drivers/emc/test_emc_vmax.py
@ -522,6 +522,13 @@ class EMCVMAXCommonData(object):
                   'storagetype:array': u'1234567891011',
                   'isV3': True,
                   'portgroupname': u'OS-portgroup-PG'}
+    extra_specs_no_slo = {'storagetype:pool': 'SRP_1',
+                          'volume_backend_name': 'V3_BE',
+                          'storagetype:workload': None,
+                          'storagetype:slo': None,
+                          'storagetype:array': '1234567891011',
+                          'isV3': True,
+                          'portgroupname': 'OS-portgroup-PG'}
    remainingSLOCapacity = '123456789'
    SYNCHRONIZED = 4
    UNSYNCHRONIZED = 3
@ -1933,6 +1940,32 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase):
    def fake_is_v3(self, conn, serialNumber):
        return False

+    def test_slo_empty_tag(self):
+        filename = 'cinder_emc_config_slo_empty_tag'
+        tempdir = tempfile.mkdtemp()
+        config_file = tempdir + '/' + filename
+        text_file = open(config_file, "w")
+        text_file.write("<?xml version='1.0' encoding='UTF-8'?>\n<EMC>\n"
+                        "<EcomServerIp>10.10.10.10</EcomServerIp>\n"
+                        "<EcomServerPort>5988</EcomServerPort>\n"
+                        "<EcomUserName>user</EcomUserName>\n"
+                        "<EcomPassword>password</EcomPassword>\n"
+                        "<PortGroups>\n"
+                        "<PortGroup>OS-PORTGROUP1-PG</PortGroup>\n"
+                        "</PortGroups>\n"
+                        "<Pool>SRP_1</Pool>\n"
+                        "<Slo></Slo>\n"
+                        "<Workload></Workload>\n"
+                        "</EMC>")
+        text_file.close()
+
+        arrayInfo = self.driver.utils.parse_file_to_get_array_map(config_file)
+        self.assertIsNone(arrayInfo[0]['SLO'])
+        self.assertIsNone(arrayInfo[0]['Workload'])
+        bExists = os.path.exists(config_file)
+        if bExists:
+            os.remove(config_file)
+
    def populate_masking_dict_setup(self):
        extraSpecs = {'storagetype:pool': u'gold_pool',
                      'volume_backend_name': 'GOLD_POOL_BE',
@ -5894,6 +5927,41 @@ class EMCV3DriverTestCase(test.TestCase):
        storagegroup['ElementName'] = 'no_masking_view'
        return storagegroup

+    def test_populate_masking_dict_no_slo(self):
+        extraSpecs = {'storagetype:pool': 'SRP_1',
+                      'volume_backend_name': 'V3_BE',
+                      'storagetype:workload': None,
+                      'storagetype:slo': None,
+                      'storagetype:array': '1234567891011',
+                      'isV3': True,
+                      'portgroupname': 'OS-portgroup-PG'}
+        # If fast is enabled it will uniquely determine the SG and MV
+        # on the host along with the protocol(iSCSI) e.g. I
+        maskingViewDict = self.driver.common._populate_masking_dict(
+            self.data.test_volume, self.data.connector, extraSpecs)
+        self.assertEqual(
+            'OS-fakehost-No_SLO-SG', maskingViewDict['sgGroupName'])
+        self.assertEqual(
+            'OS-fakehost-No_SLO-MV', maskingViewDict['maskingViewName'])
+
+    def test_populate_masking_dict_slo_NONE(self):
+        extraSpecs = {'storagetype:pool': 'SRP_1',
+                      'volume_backend_name': 'V3_BE',
+                      'storagetype:workload': 'NONE',
+                      'storagetype:slo': 'NONE',
+                      'storagetype:array': '1234567891011',
+                      'isV3': True,
+                      'portgroupname': 'OS-portgroup-PG'}
+        # If fast is enabled it will uniquely determine the SG and MV
+        # on the host along with the protocol(iSCSI) e.g. I
+        maskingViewDict = self.driver.common._populate_masking_dict(
+            self.data.test_volume, self.data.connector, extraSpecs)
+        self.assertEqual(
+            'OS-fakehost-SRP_1-NONE-NONE-SG', maskingViewDict['sgGroupName'])
+        self.assertEqual(
+            'OS-fakehost-SRP_1-NONE-NONE-MV',
+            maskingViewDict['maskingViewName'])
+
    def test_last_vol_in_SG_with_MV(self):
        conn = self.fake_ecom_connection()
        controllerConfigService = (
@ -5975,6 +6043,14 @@ class EMCV3DriverTestCase(test.TestCase):
            return_value = self.data.default_sg_instance_name)
        self.driver.create_volume(self.data.test_volume_v3)

+    @mock.patch.object(
+        emc_vmax_common.EMCVMAXCommon,
+        '_initial_setup',
+        return_value=(EMCVMAXCommonData.extra_specs_no_slo))
+    @mock.patch.object(
+        emc_vmax_common.EMCVMAXCommon,
+        '_get_or_create_storage_group_v3',
+        return_value=(EMCVMAXCommonData.default_sg_instance_name))
    @mock.patch.object(
        emc_vmax_common.EMCVMAXCommon,
        '_get_pool_and_storage_system',
@ -5984,7 +6060,24 @@ class EMCV3DriverTestCase(test.TestCase):
        'get_volume_type_extra_specs',
        return_value={'volume_backend_name': 'V3_BE'})
    def test_create_volume_v3_no_slo_success(
+            self, _mock_volume_type, mock_storage_system, mock_sg,
+            mock_initial_setup):
+        # This the no fast scenario
+        v3_vol = self.data.test_volume_v3
+        v3_vol['host'] = 'HostX@Backend#NONE+SRP_1+1234567891011'
+        self.driver.create_volume(v3_vol)
+
+    @mock.patch.object(
+        emc_vmax_common.EMCVMAXCommon,
+        '_get_pool_and_storage_system',
+        return_value=(None, EMCVMAXCommonData.storage_system))
+    @mock.patch.object(
+        volume_types,
+        'get_volume_type_extra_specs',
+        return_value={'volume_backend_name': 'V3_BE'})
+    def test_create_volume_v3_slo_NONE_success(
            self, _mock_volume_type, mock_storage_system):
+        # NONE is a valid SLO
        v3_vol = self.data.test_volume_v3
        v3_vol['host'] = 'HostX@Backend#NONE+SRP_1+1234567891011'
        instid = 'SYMMETRIX-+-000197200056-+-NONE:DSS-+-F-+-0-+-SR-+-SRP_1'
--- a/cinder/volume/drivers/emc/emc_vmax_common.py
+++ b/cinder/volume/drivers/emc/emc_vmax_common.py
@ -1841,12 +1841,17 @@ class EMCVMAXCommon(object):
            maskingViewDict['slo'] = slo
            maskingViewDict['workload'] = workload
            maskingViewDict['pool'] = uniqueName
-            prefix = (
-                ("OS-%(shortHostName)s-%(poolName)s-%(slo)s-%(workload)s"
-                 % {'shortHostName': shortHostName,
-                    'poolName': uniqueName,
-                    'slo': slo,
-                    'workload': workload}))
+            if slo:
+                prefix = (
+                    ("OS-%(shortHostName)s-%(poolName)s-%(slo)s-%(workload)s"
+                     % {'shortHostName': shortHostName,
+                        'poolName': uniqueName,
+                        'slo': slo,
+                        'workload': workload}))
+            else:
+                prefix = (
+                    ("OS-%(shortHostName)s-No_SLO"
+                     % {'shortHostName': shortHostName}))
        else:
            maskingViewDict['fastPolicy'] = extraSpecs[FASTPOLICY]
            if maskingViewDict['fastPolicy']:
@ -2940,15 +2945,27 @@ class EMCVMAXCommon(object):
        # Check to see if SLO and Workload are configured on the array.
        storagePoolCapability = self.provisionv3.get_storage_pool_capability(
            self.conn, poolInstanceName)
-        if storagePoolCapability:
-            self.provisionv3.get_storage_pool_setting(
-                self.conn, storagePoolCapability, extraSpecs[SLO],
-                extraSpecs[WORKLOAD])
-        else:
-            exceptionMessage = (_(
-                "Cannot determine storage pool settings."))
-            LOG.error(exceptionMessage)
-            raise exception.VolumeBackendAPIException(data=exceptionMessage)
+        if extraSpecs[SLO]:
+            if storagePoolCapability:
+                storagePoolSetting = self.provisionv3.get_storage_pool_setting(
+                    self.conn, storagePoolCapability, extraSpecs[SLO],
+                    extraSpecs[WORKLOAD])
+                if not storagePoolSetting:
+                    exceptionMessage = (_(
+                        "The array does not support the storage pool setting "
+                        "for SLO %(slo)s or workload %(workload)s. Please "
+                        "check the array for valid SLOs and workloads.")
+                        % {'slo': extraSpecs[SLO],
+                           'workload': extraSpecs[WORKLOAD]})
+                    LOG.error(exceptionMessage)
+                    raise exception.VolumeBackendAPIException(
+                        data=exceptionMessage)
+            else:
+                exceptionMessage = (_(
+                    "Cannot determine storage pool settings."))
+                LOG.error(exceptionMessage)
+                raise exception.VolumeBackendAPIException(
+                    data=exceptionMessage)

        LOG.debug("Create Volume: %(volume)s  Pool: %(pool)s "
                  "Storage System: %(storageSystem)s "
--- a/cinder/volume/drivers/emc/emc_vmax_provision_v3.py
+++ b/cinder/volume/drivers/emc/emc_vmax_provision_v3.py
@ -428,8 +428,8 @@ class EMCVMAXProvisionV3(object):

            foundStorageGroupInstanceName = self._find_new_storage_group(
                conn, job, groupName)
-
            return foundStorageGroupInstanceName
+
        return do_create_storage_group_v3()

    def get_storage_pool_capability(self, conn, poolInstanceName):
@ -781,28 +781,28 @@ class EMCVMAXProvisionV3(object):
        :returns: remainingCapacityGb
        """
        remainingCapacityGb = -1
-        storageConfigService = (
-            self.utils.find_storage_configuration_service(
-                conn, systemName))
+        if arrayInfo['SLO']:
+            storageConfigService = (
+                self.utils.find_storage_configuration_service(
+                    conn, systemName))

-        supportedSizeDict = (
-            self.get_volume_range(
-                conn, storageConfigService, srpPoolInstanceName,
-                arrayInfo['SLO'], arrayInfo['Workload'],
-                None))
-        try:
-            # Information source is V3.
-            if supportedSizeDict['EMCInformationSource'] == INFO_SRC_V3:
-                remainingCapacityGb = self.utils.convert_bits_to_gbs(
-                    supportedSizeDict['EMCRemainingSLOCapacity'])
-                LOG.debug("Received remaining SLO Capacity "
-                          "%(remainingCapacityGb)s GBs for SLO "
-                          "%(SLO)s and workload %(workload)s.",
-                          {'remainingCapacityGb': remainingCapacityGb,
-                           'SLO': arrayInfo['SLO'],
-                           'workload': arrayInfo['Workload']})
-        except KeyError:
-            pass
+            supportedSizeDict = (
+                self.get_volume_range(
+                    conn, storageConfigService, srpPoolInstanceName,
+                    arrayInfo['SLO'], arrayInfo['Workload'],
+                    None))
+            try:
+                if supportedSizeDict['EMCInformationSource'] == INFO_SRC_V3:
+                    remainingCapacityGb = self.utils.convert_bits_to_gbs(
+                        supportedSizeDict['EMCRemainingSLOCapacity'])
+                    LOG.debug("Received remaining SLO Capacity "
+                              "%(remainingCapacityGb)s GBs for SLO "
+                              "%(SLO)s and workload %(workload)s.",
+                              {'remainingCapacityGb': remainingCapacityGb,
+                               'SLO': arrayInfo['SLO'],
+                               'workload': arrayInfo['Workload']})
+            except KeyError:
+                pass
        return remainingCapacityGb

    def extend_volume_in_SG(
--- a/cinder/volume/drivers/emc/emc_vmax_utils.py
+++ b/cinder/volume/drivers/emc/emc_vmax_utils.py
@ -1543,6 +1543,11 @@ class EMCVMAXUtils(object):
        isValidSLO = False
        isValidWorkload = False

+        if not slo:
+            isValidSLO = True
+        if not workload:
+            isValidWorkload = True
+
        validSLOs = ['Bronze', 'Silver', 'Gold',
                     'Platinum', 'Diamond', 'Optimized',
                     'NONE']
@ -1579,10 +1584,13 @@ class EMCVMAXUtils(object):
        :param workload: the workload string e.g DSS
        :returns: storageGroupName
        """
-        storageGroupName = ("OS-%(poolName)s-%(slo)s-%(workload)s-SG"
-                            % {'poolName': poolName,
-                               'slo': slo,
-                               'workload': workload})
+        if slo and workload:
+            storageGroupName = ("OS-%(poolName)s-%(slo)s-%(workload)s-SG"
+                                % {'poolName': poolName,
+                                   'slo': slo,
+                                   'workload': workload})
+        else:
+            storageGroupName = ("OS-no_SLO-SG")
        return storageGroupName

    def _get_fast_settings_from_storage_group(self, storageGroupInstance):
@ -1884,12 +1892,11 @@ class EMCVMAXUtils(object):
        kwargs['EcomNoVerification'] = connargs['EcomNoVerification']

        slo = self._process_tag(element, 'SLO')
-        if slo is None:
-            slo = 'NONE'
        kwargs['SLO'] = slo
        workload = self._process_tag(element, 'Workload')
-        if workload is None:
+        if workload is None and slo:
            workload = 'NONE'
+
        kwargs['Workload'] = workload
        fastPolicy = self._process_tag(element, 'FastPolicy')
        kwargs['FastPolicy'] = fastPolicy