PowerMax Driver - QoS should not be set on parent storage group

The parent storage group is just a hold all for the child
storage groups which contain attributes like service level and QoS.
Setting QoS on the parent storage group can cause problems as
having a higher I/O limit on the child than the parent is not
allowed and this is not the intention.  The fix is to set
QoS on the child storage groups only.

Closes-Bug: #1930290
Change-Id: Ib2c1a44086da1fa3c64611344221c9307be22296

cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_data.py

@@ -475,6 +475,11 @@ class PowerMaxData(object):
     extra_specs_tags = deepcopy(extra_specs)
     extra_specs_tags.update({utils.STORAGE_GROUP_TAGS: sg_tags})
 
+    extra_specs_qos = deepcopy(extra_specs)
+    qos_dict = {
+        'total_iops_sec': '4000',
+        'DistributionType': 'Always'}
+    extra_specs_qos['qos'] = qos_dict
     rep_extra_specs_mgmt = deepcopy(rep_extra_specs)
     rep_extra_specs_mgmt['srp'] = srp
     rep_extra_specs_mgmt['mgmt_sg_name'] = rdf_managed_async_grp

cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_masking.py

@@ -298,6 +298,30 @@ class PowerMaxMaskingTest(test.TestCase):
         self.assertEqual(2, mock_get_sg.call_count)
         self.assertEqual(1, mock_sg.call_count)
 
+    @mock.patch.object(
+        rest.PowerMaxRest, 'update_storagegroup_qos')
+    @mock.patch.object(
+        rest.PowerMaxRest, 'get_storage_group',
+        return_value=tpd.PowerMaxData.storagegroup_name_i)
+    def test_get_or_create_storage_group_is_parent_qos(
+            self, mock_sg, mock_update_sg):
+        self.driver.masking._get_or_create_storage_group(
+            self.data.array, self.maskingviewdict,
+            self.data.storagegroup_name_i, self.data.extra_specs_qos, True)
+        mock_update_sg.assert_not_called()
+
+    @mock.patch.object(
+        rest.PowerMaxRest, 'update_storagegroup_qos')
+    @mock.patch.object(
+        rest.PowerMaxRest, 'get_storage_group',
+        return_value=tpd.PowerMaxData.storagegroup_name_i)
+    def test_get_or_create_storage_group_is_child_qos(
+            self, mock_sg, mock_update_sg):
+        self.driver.masking._get_or_create_storage_group(
+            self.data.array, self.maskingviewdict,
+            self.data.storagegroup_name_i, self.data.extra_specs_qos, False)
+        mock_update_sg.assert_called_once()
+
     @mock.patch.object(masking.PowerMaxMasking, '_move_vol_from_default_sg',
                        return_value=None)
     @mock.patch.object(masking.PowerMaxMasking, '_get_or_create_storage_group',

cinder/volume/drivers/dell_emc/powermax/masking.py

@@ -461,14 +461,13 @@ class PowerMaxMasking(object):
                       'volume_name': masking_view_dict[utils.VOL_NAME]})
             LOG.error(msg)
 
-        # If qos exists, update storage group to reflect qos parameters
-        if 'qos' in extra_specs:
-            self.rest.update_storagegroup_qos(
-                serial_number, storagegroup_name, extra_specs)
-
-        # If storagetype:storagegrouptags exist update storage group
-        # to add tags
         if not parent:
+            # If qos exists, update storage group to reflect qos parameters
+            if 'qos' in extra_specs:
+                self.rest.update_storagegroup_qos(
+                    serial_number, storagegroup_name, extra_specs)
+            # If storagetype:storagegrouptags exist update storage group
+            # to add tags
             self._add_tags_to_storage_group(
                 serial_number, storagegroup, extra_specs)
 

releasenotes/notes/powermax-bug-1930290-4f598329a6ced006.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    PowerMax driver `bug #1930290
+    <https://bugs.launchpad.net/cinder/+bug/1930290>`_: This fixes the
+    QoS conflict issue on a child storage group by not setting QoS on a
+    parent storage group.