diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_data.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_data.py index 73e5f8a962e..106a8cc39d3 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_data.py +++ b/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 diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_masking.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_masking.py index 291578ce53c..40ce442760d 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_masking.py +++ b/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', diff --git a/cinder/volume/drivers/dell_emc/powermax/masking.py b/cinder/volume/drivers/dell_emc/powermax/masking.py index 5808974e61a..820321e2b64 100644 --- a/cinder/volume/drivers/dell_emc/powermax/masking.py +++ b/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) diff --git a/releasenotes/notes/powermax-bug-1930290-4f598329a6ced006.yaml b/releasenotes/notes/powermax-bug-1930290-4f598329a6ced006.yaml new file mode 100644 index 00000000000..c911d8f6699 --- /dev/null +++ b/releasenotes/notes/powermax-bug-1930290-4f598329a6ced006.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + PowerMax driver `bug #1930290 + `_: This fixes the + QoS conflict issue on a child storage group by not setting QoS on a + parent storage group.