From dacd6c80380423e101552995f67081b2c07370f5 Mon Sep 17 00:00:00 2001
From: Helen Walsh <helen.walsh@emc.com>
Date: Tue, 31 Jan 2017 23:23:04 +0000
Subject: [PATCH] VMAX driver - ignore service level and workload in xml

When multi_pool_support is enabled in cinder.conf then
the driver should ignore any ServiceLevel entries in the
emc xml file. This was not happening because of an
incorrect check while fetching and populating the various
service level/workload combinations from VMAX. With the
fix, if any ServiceLevel or Workload tags are present in
the emc xml file, they will be ignored.

Change-Id: I45a8660aee2e4e4623aedc52c99ddeb51396980e
Closes-Bug: #1659831
---
 .../unit/volume/drivers/dell_emc/test_vmax.py | 65 +++++++++++++++++--
 cinder/volume/drivers/dell_emc/vmax/common.py |  6 +-
 2 files changed, 65 insertions(+), 6 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 041342e3bc2..6de1b3a5a73 100644
--- a/cinder/tests/unit/volume/drivers/dell_emc/test_vmax.py
+++ b/cinder/tests/unit/volume/drivers/dell_emc/test_vmax.py
@@ -347,6 +347,9 @@ class VMAXCommonData(object):
     fake_host_2_v3 = 'HostY@Backend#SRP_1+1234567891011'
     fake_host_3_v3 = 'HostX@Backend#Bronze+DSS+SRP_1+1234567891011'
     fake_host_4_v3 = 'HostX@Backend#Silver+None+SRP_1+1234567891011'
+    poolInstanceName = {
+        'InstanceID': 'SRP_1',
+        'CreationClassName': 'Symm_StorageSystem'}
 
     unit_creationclass = 'CIM_ProtocolControllerForUnit'
     storage_type = 'gold'
@@ -1771,11 +1774,18 @@ class FakeEcomConnection(object):
 
     def _enum_storagesettings(self):
         storagesettings = []
-        storagesetting = {}
-        storagesetting['CreationClassName'] = 'CIM_StoragePoolSetting'
-        storagesetting['InstanceID'] = ('SYMMETRIX-+-000197200056-+-SBronze:'
-                                        'DSS-+-F-+-0-+-SR-+-SRP_1')
-        storagesettings.append(storagesetting)
+        storagesetting_bronze = {}
+        storagesetting_bronze['CreationClassName'] = 'CIM_StoragePoolSetting'
+        storagesetting_bronze['InstanceID'] = (
+            'SYMMETRIX-+-000197200056-+-SBronze:'
+            'DSS-+-F-+-0-+-SR-+-SRP_1')
+        storagesettings.append(storagesetting_bronze)
+        storagesetting_silver = {}
+        storagesetting_silver['CreationClassName'] = 'CIM_StoragePoolSetting'
+        storagesetting_silver['InstanceID'] = (
+            'SYMMETRIX-+-000197200056-+-SSilver:'
+            'DSS-+-F-+-0-+-SR-+-SRP_1')
+        storagesettings.append(storagesetting_silver)
         return storagesettings
 
     def _enum_targetMaskingGroup(self):
@@ -7044,6 +7054,15 @@ class EMCV3MultiPoolDriverTestCase(test.TestCase):
                  'SLO': u'Bronze',
                  'Workload': u'DSS'}]
 
+    def array_info_list_without_slo(self):
+        return [{'EcomServerIp': u'1.1.1.1',
+                 'EcomServerPort': 10,
+                 'EcomUserName': u'user',
+                 'EcomPassword': u'pass',
+                 'PoolName': u'SRP_1',
+                 'PortGroup': u'OS-portgroup-PG',
+                 'SerialNumber': 1234567891011}]
+
     def multiple_array_info_list(self):
         return [{'EcomServerIp': u'1.1.1.1',
                  'EcomServerPort': 10,
@@ -7476,6 +7495,42 @@ class EMCV3MultiPoolDriverTestCase(test.TestCase):
                          pools[1]['location_info'])
         self._cleanup_pool_info()
 
+    @mock.patch.object(
+        common.VMAXCommon,
+        '_find_pool_in_array',
+        return_value=(VMAXCommonData.poolInstanceName,
+                      VMAXCommonData.storage_system))
+    def test_get_slo_workload_combinations_with_slo(self, mock_pool):
+        self.driver.common.multiPoolSupportEnabled = True
+        final_array_info_list = (
+            self.driver.common._get_slo_workload_combinations(
+                self.default_array_info_list()))
+        bCheckForSilver = False
+        for array_info in final_array_info_list:
+            # Check if 'Silver' is present in the final list
+            if array_info['SLO'] == 'Silver':
+                bCheckForSilver = True
+        self.assertTrue(bCheckForSilver)
+        self._cleanup_pool_info()
+
+    @mock.patch.object(
+        common.VMAXCommon,
+        '_find_pool_in_array',
+        return_value=(VMAXCommonData.poolInstanceName,
+                      VMAXCommonData.storage_system))
+    def test_get_slo_workload_combinations_without_slo(self, mock_pool):
+        self.driver.common.multiPoolSupportEnabled = True
+        final_array_info_list = (
+            self.driver.common._get_slo_workload_combinations(
+                self.array_info_list_without_slo()))
+        bCheckForSilver = False
+        for array_info in final_array_info_list:
+            # Check if 'Silver' is present in the final list
+            if array_info['SLO'] == 'Silver':
+                bCheckForSilver = True
+        self.assertTrue(bCheckForSilver)
+        self._cleanup_pool_info()
+
     def _cleanup(self, tempdir, config_file_path):
         bExists = os.path.exists(config_file_path)
         if bExists:
diff --git a/cinder/volume/drivers/dell_emc/vmax/common.py b/cinder/volume/drivers/dell_emc/vmax/common.py
index 8a95e84d1e7..8f8f7e9e049 100644
--- a/cinder/volume/drivers/dell_emc/vmax/common.py
+++ b/cinder/volume/drivers/dell_emc/vmax/common.py
@@ -264,8 +264,12 @@ class VMAXCommon(object):
                 # only strings
                 temparrayInfo = arrayInfoList[0].copy()
                 slo, workload = sloWorkload.split(':')
-                if temparrayInfo['SLO'] is None:
+                # Check if we got SLO and workload from the set (from array)
+                # The previous check was done by mistake against the value
+                # from XML file
+                if slo:
                     temparrayInfo['SLO'] = slo
+                if workload:
                     temparrayInfo['Workload'] = workload
                 finalArrayInfoList.append(temparrayInfo)
         except Exception: