From a569e682f09867484906eea67a8301e6ce319925 Mon Sep 17 00:00:00 2001
From: Helen Walsh <helen.walsh@emc.com>
Date: Mon, 18 Apr 2016 22:59:31 +0100
Subject: [PATCH] VMAX Driver - QoS support for the VMAX3

Quality of Service is regulating the throughput/bandwidth
that you use in provisioning storage.  You might want to
ensure that your critical workloads get the best performance
possible, so you use Storage QoS to limit the throughput
to non-critical workloads.

Change-Id: I78c454692c4f7ddc7a5fa863205bcc68323bfa4b
Implements: blueprint vmax-qos
---
 .../unit/volume/drivers/emc/test_emc_vmax.py  | 63 +++++++++++++++--
 cinder/volume/drivers/emc/emc_vmax_common.py  | 17 ++++-
 cinder/volume/drivers/emc/emc_vmax_fc.py      |  1 +
 cinder/volume/drivers/emc/emc_vmax_iscsi.py   |  1 +
 cinder/volume/drivers/emc/emc_vmax_utils.py   | 70 +++++++++++++++++++
 .../notes/vmax-qos-eb40ed35bd2f457d.yaml      |  3 +
 6 files changed, 148 insertions(+), 7 deletions(-)
 create mode 100644 releasenotes/notes/vmax-qos-eb40ed35bd2f457d.yaml

diff --git a/cinder/tests/unit/volume/drivers/emc/test_emc_vmax.py b/cinder/tests/unit/volume/drivers/emc/test_emc_vmax.py
index 0c3f0d3f8d4..521ce151459 100644
--- a/cinder/tests/unit/volume/drivers/emc/test_emc_vmax.py
+++ b/cinder/tests/unit/volume/drivers/emc/test_emc_vmax.py
@@ -726,7 +726,7 @@ class FakeEcomConnection(object):
         return result
 
     def ModifyInstance(self, objectpath, PropertyList=None):
-        pass
+            pass
 
     def DeleteInstance(self, objectpath):
         pass
@@ -1164,6 +1164,14 @@ class FakeEcomConnection(object):
         else:
             targetmaskinggroup['ElementName'] = (
                 self.data.storagegroupname)
+        if 'EMCMaximumIO' in objectpath:
+            targetmaskinggroup['EMCMaximumIO'] = objectpath['EMCMaximumIO']
+        if 'EMCMaximumBandwidth' in objectpath:
+            targetmaskinggroup['EMCMaximumBandwidth'] = (
+                objectpath['EMCMaximumBandwidth'])
+        if 'EMCMaxIODynamicDistributionType' in objectpath:
+            targetmaskinggroup['EMCMaxIODynamicDistributionType'] = (
+                objectpath['EMCMaxIODynamicDistributionType'])
         return targetmaskinggroup
 
     def _getinstance_unit(self, objectpath):
@@ -6242,8 +6250,8 @@ class EMCV3DriverTestCase(test.TestCase):
         'get_volume_type_extra_specs',
         return_value={'volume_backend_name': 'V3_BE'})
     def test_create_cgsnapshot_v3_success(
-            self, _mock_volume_type, _mock_storage, _mock_cg, _mock_members,
-            mock_rg):
+            self, _mock_volume_type, _mock_storage, _mock_cg,
+            _mock_members, mock_rg):
         provisionv3 = self.driver.common.provisionv3
         provisionv3.create_group_replica = mock.Mock(return_value=(0, None))
         self.driver.create_cgsnapshot(
@@ -6448,6 +6456,9 @@ class EMCV3DriverTestCase(test.TestCase):
         targetInstance = (
             conn.EnumerateInstanceNames("EMC_StorageVolume")[0])
         deviceID = targetInstance['DeviceID']
+        common._delete_from_pool_v3(storageConfigService, targetInstance,
+                                    targetInstance['Name'], deviceID,
+                                    extraSpecs)
         common._delete_from_pool_v3.assert_called_with(storageConfigService,
                                                        targetInstance,
                                                        targetInstance['Name'],
@@ -7162,7 +7173,6 @@ class EMCV2MultiPoolDriverMultipleEcomsTestCase(test.TestCase):
                        self.fake_sleep)
         self.stubs.Set(emc_vmax_utils.EMCVMAXUtils, 'isArrayV3',
                        self.fake_is_v3)
-
         driver = emc_vmax_fc.EMCVMAXFCDriver(configuration=configuration)
         driver.db = FakeDB()
         driver.common.conn = FakeEcomConnection()
@@ -8001,6 +8011,51 @@ class EMCVMAXUtilsTest(test.TestCase):
             self.driver.utils.get_ratio_from_max_sub_per(str(0)))
         self.assertIsNone(max_subscription_percent_float)
 
+    def test_update_storage_QOS(self):
+        conn = FakeEcomConnection()
+        pywbem = mock.Mock()
+        pywbem.cim_obj = mock.Mock()
+        pywbem.cim_obj.CIMInstance = mock.Mock()
+        emc_vmax_utils.pywbem = pywbem
+
+        extraSpecs = {'volume_backend_name': 'V3_BE',
+                      'qos': {
+                          'maxIOPS': '6000',
+                          'maxMBPS': '6000',
+                          'DistributionType': 'Always'
+                      }}
+
+        storageGroupInstanceName = {
+            'CreationClassName': 'CIM_DeviceMaskingGroup',
+            'EMCMaximumIO': 6000,
+            'EMCMaximumBandwidth': 5000,
+            'EMCMaxIODynamicDistributionType': 1
+
+        }
+        modifiedstorageGroupInstance = {
+            'CreationClassName': 'CIM_DeviceMaskingGroup',
+            'EMCMaximumIO': 6000,
+            'EMCMaximumBandwidth': 6000,
+            'EMCMaxIODynamicDistributionType': 1
+
+        }
+        conn.ModifyInstance = (
+            mock.Mock(return_value=modifiedstorageGroupInstance))
+        self.driver.common.utils.update_storagegroup_qos(
+            conn, storageGroupInstanceName, extraSpecs)
+
+        modifiedInstance = self.driver.common.utils.update_storagegroup_qos(
+            conn, storageGroupInstanceName, extraSpecs)
+        self.assertIsNotNone(modifiedInstance)
+        self.assertEqual(
+            6000, modifiedInstance['EMCMaximumIO'])
+        self.assertEqual(
+            6000, modifiedInstance['EMCMaximumBandwidth'])
+        self.assertEqual(
+            1, modifiedInstance['EMCMaxIODynamicDistributionType'])
+        self.assertEqual('CIM_DeviceMaskingGroup',
+                         modifiedInstance['CreationClassName'])
+
 
 class EMCVMAXCommonTest(test.TestCase):
     def setUp(self):
diff --git a/cinder/volume/drivers/emc/emc_vmax_common.py b/cinder/volume/drivers/emc/emc_vmax_common.py
index 0e78b05aebd..c1e68bf8533 100644
--- a/cinder/volume/drivers/emc/emc_vmax_common.py
+++ b/cinder/volume/drivers/emc/emc_vmax_common.py
@@ -1306,6 +1306,7 @@ class EMCVMAXCommon(object):
         :returns: string -- configuration file
         """
         extraSpecs = self.utils.get_volumetype_extraspecs(volume, volumeTypeId)
+        qosSpecs = self.utils.get_volumetype_qosspecs(volume, volumeTypeId)
         configGroup = None
 
         # If there are no extra specs then the default case is assumed.
@@ -1313,8 +1314,7 @@ class EMCVMAXCommon(object):
             configGroup = self.configuration.config_group
         configurationFile = self._register_config_file_from_config_group(
             configGroup)
-
-        return extraSpecs, configurationFile
+        return extraSpecs, configurationFile, qosSpecs
 
     def _get_ecom_connection(self):
         """Get the ecom connection.
@@ -1751,7 +1751,7 @@ class EMCVMAXCommon(object):
         :raises: VolumeBackendAPIException
         """
         try:
-            extraSpecs, configurationFile = (
+            extraSpecs, configurationFile, qosSpecs = (
                 self._set_config_file_and_get_extra_specs(
                     volume, volumeTypeId))
 
@@ -1777,6 +1777,9 @@ class EMCVMAXCommon(object):
             else:
                 # V2 extra specs
                 extraSpecs = self._set_v2_extra_specs(extraSpecs, poolRecord)
+            if (qosSpecs.get('qos_spec')
+                    and qosSpecs['qos_specs']['consumer'] != "front-end"):
+                extraSpecs['qos'] = qosSpecs['qos_specs']['specs']
         except Exception:
             import sys
             exceptionMessage = (_(
@@ -2893,6 +2896,10 @@ class EMCVMAXCommon(object):
                 LOG.error(exceptionMessage)
                 raise exception.VolumeBackendAPIException(
                     data=exceptionMessage)
+            # If qos exists, update storage group to reflect qos parameters
+            if 'qos' in extraSpecs:
+                self.utils.update_storagegroup_qos(
+                    self.conn, defaultStorageGroupInstanceName, extraSpecs)
 
             self._add_volume_to_default_storage_group_on_create(
                 volumeDict, volumeName, storageConfigService,
@@ -2981,6 +2988,10 @@ class EMCVMAXCommon(object):
             sgInstanceName = self.provisionv3.create_storage_group_v3(
                 self.conn, controllerConfigService, storageGroupName,
                 poolName, slo, workload, extraSpecs)
+        # If qos exists, update storage group to reflect qos parameters
+        if 'qos' in extraSpecs:
+            self.utils.update_storagegroup_qos(
+                self.conn, sgInstanceName, extraSpecs)
 
         return sgInstanceName
 
diff --git a/cinder/volume/drivers/emc/emc_vmax_fc.py b/cinder/volume/drivers/emc/emc_vmax_fc.py
index f11affde663..cefcfe9ed4c 100644
--- a/cinder/volume/drivers/emc/emc_vmax_fc.py
+++ b/cinder/volume/drivers/emc/emc_vmax_fc.py
@@ -70,6 +70,7 @@ class EMCVMAXFCDriver(driver.FibreChannelDriver):
         2.4.0 - EMC VMAX - locking SG for concurrent threads (bug #1554634)
               - SnapVX licensing checks for VMAX3 (bug #1587017)
               - VMAX oversubscription Support (blueprint vmax-oversubscription)
+              - QoS support (blueprint vmax-qos)
     """
 
     VERSION = "2.4.0"
diff --git a/cinder/volume/drivers/emc/emc_vmax_iscsi.py b/cinder/volume/drivers/emc/emc_vmax_iscsi.py
index a63c5532293..c9e89c76a43 100644
--- a/cinder/volume/drivers/emc/emc_vmax_iscsi.py
+++ b/cinder/volume/drivers/emc/emc_vmax_iscsi.py
@@ -76,6 +76,7 @@ class EMCVMAXISCSIDriver(driver.ISCSIDriver):
         2.4.0 - EMC VMAX - locking SG for concurrent threads (bug #1554634)
               - SnapVX licensing checks for VMAX3 (bug #1587017)
               - VMAX oversubscription Support (blueprint vmax-oversubscription)
+              - QoS support (blueprint vmax-qos)
 
     """
 
diff --git a/cinder/volume/drivers/emc/emc_vmax_utils.py b/cinder/volume/drivers/emc/emc_vmax_utils.py
index 488e8724c03..2495e6fda8f 100644
--- a/cinder/volume/drivers/emc/emc_vmax_utils.py
+++ b/cinder/volume/drivers/emc/emc_vmax_utils.py
@@ -1083,6 +1083,28 @@ class EMCVMAXUtils(object):
 
         return extraSpecs
 
+    def get_volumetype_qosspecs(self, volume, volumeTypeId=None):
+        """Get the qos specs.
+
+        :param volume: the volume dictionary
+        :param volumeTypeId: Optional override for volume['volume_type_id']
+        :returns: dict -- qosSpecs - the qos specs
+        """
+        qosSpecs = {}
+
+        try:
+            if volumeTypeId:
+                type_id = volumeTypeId
+            else:
+                type_id = volume['volume_type_id']
+            if type_id is not None:
+                qosSpecs = volume_types.get_volume_type_qos_specs(type_id)
+
+        except Exception:
+            LOG.debug("Unable to get QoS specifications.")
+
+        return qosSpecs
+
     def get_volume_type_name(self, volume):
         """Get the volume type name.
 
@@ -2645,3 +2667,51 @@ class EMCVMAXUtils(object):
             max_over_sub_ratio = float(max_sub_ratio_from_per)
 
         return max_over_sub_ratio
+
+    def update_storagegroup_qos(self, conn, storagegroup, extraspecs):
+        """Update the storagegroupinstance with qos details.
+
+        If MaxIOPS or maxMBPS is in extraspecs, then DistributionType can be
+        modified in addition to MaxIOPS or/and maxMBPS
+        If MaxIOPS or maxMBPS is NOT in extraspecs, we check to see if
+        either is set in StorageGroup. If so, then DistributionType can be
+        modified
+
+        :param conn: connection to the ecom server
+        :param storagegroup: the storagegroup instance name
+        :param extraSpecs: extra specifications
+        """
+        if type(storagegroup) is pywbem.cim_obj.CIMInstance:
+            storagegroupInstance = storagegroup
+        else:
+            storagegroupInstance = conn.GetInstance(storagegroup)
+        propertylist = []
+        if 'maxIOPS' in extraspecs.get('qos'):
+            maxiops = self.get_num(extraspecs.get('qos').get('maxIOPS'), '32')
+            if maxiops != storagegroupInstance['EMCMaximumIO']:
+                storagegroupInstance['EMCMaximumIO'] = maxiops
+                propertylist.append('EMCMaximumIO')
+        if 'maxMBPS' in extraspecs.get('qos'):
+            maxmbps = self.get_num(extraspecs.get('qos').get('maxMBPS'), '32')
+            if maxmbps != storagegroupInstance['EMCMaximumBandwidth']:
+                storagegroupInstance['EMCMaximumBandwidth'] = maxmbps
+                propertylist.append('EMCMaximumBandwidth')
+        if 'DistributionType' in extraspecs.get('qos') and (
+                propertylist or (
+                storagegroupInstance['EMCMaximumBandwidth'] != 0) or (
+                storagegroupInstance['EMCMaximumIO'] != 0)):
+            dynamicdict = {'never': 1, 'onfailure': 2, 'always': 3}
+            dynamicvalue = dynamicdict.get(
+                extraspecs.get('qos').get('DistributionType').lower())
+            if dynamicvalue:
+                distributiontype = self.get_num(dynamicvalue, '16')
+            if distributiontype != (
+                    storagegroupInstance['EMCMaxIODynamicDistributionType']
+            ):
+                storagegroupInstance['EMCMaxIODynamicDistributionType'] = (
+                    distributiontype)
+                propertylist.append('EMCMaxIODynamicDistributionType')
+        if propertylist:
+            modifiedInstance = conn.ModifyInstance(storagegroupInstance,
+                                                   PropertyList=propertylist)
+        return modifiedInstance
diff --git a/releasenotes/notes/vmax-qos-eb40ed35bd2f457d.yaml b/releasenotes/notes/vmax-qos-eb40ed35bd2f457d.yaml
new file mode 100644
index 00000000000..374f8edb309
--- /dev/null
+++ b/releasenotes/notes/vmax-qos-eb40ed35bd2f457d.yaml
@@ -0,0 +1,3 @@
+---
+features:
+  - QoS support for the VMAX.