diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_rest.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_rest.py index 8de2dd2be20..345a5a1843b 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_rest.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_rest.py @@ -2554,3 +2554,28 @@ class PowerMaxRestTest(test.TestCase): self.data.array, self.data.device_id, self.data.test_snapshot_snap_name) self.assertEqual('0', snap_id) + + def test_check_force(self): + extra_specs = {'pool_name': 'Diamond+DSS+SRP_1+000197800123', + 'slo': 'Diamond', + 'srp': 'SRP_1', + 'array': '000123456789', + 'interval': 3, + 'retries': 120} + self.assertEqual( + 'false', self.rest._check_force(extra_specs)) + self.assertEqual( + 'false', self.rest._check_force( + self.data.extra_specs, force_flag=False)) + self.assertEqual( + 'true', self.rest._check_force( + self.data.extra_specs, force_flag=True)) + extra_specs[utils.FORCE_VOL_EDIT] = True + self.assertEqual( + 'true', self.rest._check_force(extra_specs)) + self.assertEqual( + 'true', self.rest._check_force( + extra_specs, force_flag=False)) + self.assertEqual( + 'true', self.rest._check_force( + extra_specs, force_flag=True)) diff --git a/cinder/volume/drivers/dell_emc/powermax/rest.py b/cinder/volume/drivers/dell_emc/powermax/rest.py index 43a9dd50e87..2131bc2d5a3 100644 --- a/cinder/volume/drivers/dell_emc/powermax/rest.py +++ b/cinder/volume/drivers/dell_emc/powermax/rest.py @@ -1257,7 +1257,7 @@ class PowerMaxRest(object): if not isinstance(device_id, list): device_id = [device_id] - force_add = "true" if force else "false" + force_add = self._check_force(extra_specs, force) payload = ({"executionOption": "ASYNCHRONOUS", "editStorageGroupActionParam": { @@ -1282,8 +1282,7 @@ class PowerMaxRest(object): :param extra_specs: the extra specifications """ - force_vol_edit = ( - "true" if utils.FORCE_VOL_EDIT in extra_specs else "false") + force_vol_edit = self._check_force(extra_specs) if not isinstance(device_id, list): device_id = [device_id] payload = ({"executionOption": "ASYNCHRONOUS", @@ -1410,7 +1409,7 @@ class PowerMaxRest(object): :param extra_specs: extra specifications :param force: force flag (necessary on a detach) """ - force_flag = "true" if force else "false" + force_flag = self._check_force(extra_specs, force) payload = ({"executionOption": "ASYNCHRONOUS", "editStorageGroupActionParam": { "moveVolumeToStorageGroupParam": { @@ -2200,8 +2199,7 @@ class PowerMaxRest(object): """ action, operation, payload = '', '', {} copy = 'true' if copy else 'false' - force = ( - "true" if utils.FORCE_VOL_EDIT in extra_specs else "false") + force = self._check_force(extra_specs) if link: action = "Link" @@ -3562,3 +3560,18 @@ class PowerMaxRest(object): return (self.ucode_major_level >= utils.UCODE_5978 and self.ucode_minor_level >= utils.UCODE_5978_HICKORY) or ( self.ucode_major_level >= utils.UCODE_6079) + + @staticmethod + def _check_force(extra_specs, force_flag=False): + """Determine whether force should be used + + Returns 'true' if force_flag is True or FORCE_VOL_EDIT is set in + extra_specs, otherwise returns 'false'. + + :param extra_specs: extra specs dict + :param force_flag: force flag boolean + + :returns: str (true or false) + """ + return "true" if force_flag else ( + "true" if utils.FORCE_VOL_EDIT in extra_specs else "false") diff --git a/releasenotes/notes/bug-1981420-dell-powermax-fix-for-force-flag-9320910dfbf998d2.yaml b/releasenotes/notes/bug-1981420-dell-powermax-fix-for-force-flag-9320910dfbf998d2.yaml new file mode 100644 index 00000000000..aa626ac5cf4 --- /dev/null +++ b/releasenotes/notes/bug-1981420-dell-powermax-fix-for-force-flag-9320910dfbf998d2.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + `Dell PowerMax Driver Bug #1981420 `_: + Fixed issue faced while creating synchronous volume which was caused by + incorrect handling of the force flag. This is corrected by checking volume + type extra specs for the value of "force_vol_edit" parameter along with the + "force" parameter.