From cd39a9b2f19e5a3fff86dd6908ea8d76db6f0e55 Mon Sep 17 00:00:00 2001 From: Simon O'Donovan Date: Mon, 13 May 2019 15:14:42 +0100 Subject: [PATCH] PowerMax driver - Rapid TDEV Deallocation Changes made to adding & removing volumes from RDF enabled storage groups when using new 91 endpoints force flag is needed in established groups. Volume delete workflow no longer calls deallocation function seperately, this is now included in volume delete process. Implements: blueprint powermax-tdev-deallocation Change-Id: I0d2c129c172c7c2e4126e31a9b07de6178036dfc --- .../dell_emc/powermax/powermax_data.py | 2 + .../dell_emc/powermax/test_powermax_rest.py | 31 ++++++-- .../drivers/dell_emc/powermax/common.py | 11 ++- cinder/volume/drivers/dell_emc/powermax/fc.py | 1 + .../volume/drivers/dell_emc/powermax/iscsi.py | 1 + .../drivers/dell_emc/powermax/masking.py | 5 +- .../volume/drivers/dell_emc/powermax/rest.py | 74 ++++++++++++++----- .../volume/drivers/dell_emc/powermax/utils.py | 2 + ...ax-tdev-deallocation-90bda0f95ab0b271.yaml | 5 ++ 9 files changed, 103 insertions(+), 29 deletions(-) create mode 100644 releasenotes/notes/powermax-tdev-deallocation-90bda0f95ab0b271.yaml 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 560124044d1..944435af480 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 @@ -752,6 +752,8 @@ class PowerMaxData(object): headroom = {'headroom': [{'headroomCapacity': 20348.29}]} + ucode_5978_foxtail = {'ucode': '5978.435.435'} + p_vol_rest_response_single = { 'id': 'f3aab01c-a5a8-4fb4-af2b-16ae1c46dc9e_0', 'count': 1, 'expirationTime': 1521650650793, 'maxPageSize': 1000, 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 6e5e1d28c31..6a7401c2bc5 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 @@ -561,20 +561,39 @@ class PowerMaxRestTest(test.TestCase): mock_modify.assert_called_once_with( self.data.array, device_id, extend_vol_payload) - def test_delete_volume(self): + def test_legacy_delete_volume(self): device_id = self.data.device_id vb_except = exception.VolumeBackendAPIException - with mock.patch.object(self.rest, 'delete_resource') as mock_delete, \ + with mock.patch.object(self.rest, 'delete_resource') as mock_delete, ( mock.patch.object( self.rest, '_modify_volume', - side_effect=[None, None, None, vb_except]) as mock_modify: - for x in range(0, 2): + side_effect=[None, None, None, vb_except])) as mock_modify: + for _ in range(0, 2): self.rest.delete_volume(self.data.array, device_id) mod_call_count = mock_modify.call_count self.assertEqual(4, mod_call_count) mock_delete.assert_called_once_with( self.data.array, 'sloprovisioning', 'volume', device_id) + def test_delete_volume(self): + device_id = self.data.device_id + ucode_5978_foxtail = tpd.PowerMaxData.ucode_5978_foxtail + with mock.patch.object( + self.rest, 'delete_resource') as mock_delete, ( + mock.patch.object( + self.rest, '_modify_volume')) as mock_modify, ( + mock.patch.object( + self.rest, 'get_array_detail', + return_value=ucode_5978_foxtail))as mock_det: + + self.rest.delete_volume(self.data.array, device_id) + detail_call_count = mock_det.call_count + mod_call_count = mock_modify.call_count + self.assertEqual(1, detail_call_count) + self.assertEqual(1, mod_call_count) + mock_delete.assert_called_once_with( + self.data.array, 'sloprovisioning', 'volume', device_id) + def test_rename_volume(self): device_id = self.data.device_id payload = {'editVolumeActionParam': { @@ -1369,14 +1388,14 @@ class PowerMaxRestTest(test.TestCase): ref_payload2 = {'establish': 'true', 'rdfType': 'RDF1'} payload3 = self.rest.get_metro_payload_info( self.data.array, ref_payload2, self.data.rdf_group_no, {}) - ref_payload3 = {'rdfType': 'NA', 'format': 'true'} + ref_payload3 = {'rdfType': 'RDF1', 'format': 'true'} self.assertEqual(ref_payload3, payload3) def test_modify_rdf_device_pair(self): resource_name = '70/volume/00001' common_opts = {'force': 'false', 'symForce': 'false', 'star': 'false', 'hop2': 'false', 'bypass': 'false'} - suspend_payload = {'action': 'Suspend', + suspend_payload = {'action': 'SUSPEND', 'executionOption': 'ASYNCHRONOUS', 'suspend': common_opts} diff --git a/cinder/volume/drivers/dell_emc/powermax/common.py b/cinder/volume/drivers/dell_emc/powermax/common.py index 5db9017763a..1ba15c64ce4 100644 --- a/cinder/volume/drivers/dell_emc/powermax/common.py +++ b/cinder/volume/drivers/dell_emc/powermax/common.py @@ -1883,8 +1883,12 @@ class PowerMaxCommon(object): """ # Cleanup remote replication if self.utils.is_replication_enabled(extra_specs): + # Add force to allow volume removal from RDF enabled storage groups + extra_specs['force_vol_remove'] = True + self.cleanup_lun_replication(volume, volume_name, device_id, extra_specs) + # Remove from any storage groups self.masking.remove_and_reset_members( array, volume, device_id, volume_name, extra_specs, False) @@ -3486,7 +3490,8 @@ class PowerMaxCommon(object): try: self.provision.get_or_create_group(array, group_name, extra_specs) self.masking.add_volume_to_storage_group( - array, device_id, group_name, volume_name, extra_specs) + array, device_id, group_name, volume_name, extra_specs, + force=True) # Add remote volume self.provision.get_or_create_group( remote_array, group_name, extra_specs) @@ -3622,6 +3627,7 @@ class PowerMaxCommon(object): array, device_id, target_device, rdf_group, rep_extra_specs, metro_grp) # Remove the volume from the metro_grp + rep_extra_specs['force_vol_remove'] = True self.masking.remove_volume_from_sg(array, device_id, 'metro_vol', metro_grp, rep_extra_specs) # Resume I/O on the RDF links for any remaining volumes @@ -4089,7 +4095,8 @@ class PowerMaxCommon(object): message=exception_message) self.masking.add_volume_to_storage_group( - array, device_id, storagegroup_name, volume_name, extra_specs) + array, device_id, storagegroup_name, volume_name, extra_specs, + force=True) return storagegroup_name diff --git a/cinder/volume/drivers/dell_emc/powermax/fc.py b/cinder/volume/drivers/dell_emc/powermax/fc.py index 577e8e1ef53..4783b441ad2 100644 --- a/cinder/volume/drivers/dell_emc/powermax/fc.py +++ b/cinder/volume/drivers/dell_emc/powermax/fc.py @@ -109,6 +109,7 @@ class PowerMaxFCDriver(san.SanDriver, driver.FibreChannelDriver): - Support for storage-assisted in-use retype (bp/powermax-storage-assisted-inuse-retype) 4.1.0 - Changing from 90 to 91 rest endpoints + - Support for Rapid TDEV Delete (bp powermax-tdev-deallocation) """ VERSION = "4.1.0" diff --git a/cinder/volume/drivers/dell_emc/powermax/iscsi.py b/cinder/volume/drivers/dell_emc/powermax/iscsi.py index 45be71f095e..434de4e3aef 100644 --- a/cinder/volume/drivers/dell_emc/powermax/iscsi.py +++ b/cinder/volume/drivers/dell_emc/powermax/iscsi.py @@ -114,6 +114,7 @@ class PowerMaxISCSIDriver(san.SanISCSIDriver): - Support for storage-assisted in-use retype (bp/powermax-storage-assisted-inuse-retype) 4.1.0 - Changing from 90 to 91 rest endpoints + - Support for Rapid TDEV Delete (bp powermax-tdev-deallocation) """ VERSION = "4.1.0" diff --git a/cinder/volume/drivers/dell_emc/powermax/masking.py b/cinder/volume/drivers/dell_emc/powermax/masking.py index dbdc8927296..e848c0ecf72 100644 --- a/cinder/volume/drivers/dell_emc/powermax/masking.py +++ b/cinder/volume/drivers/dell_emc/powermax/masking.py @@ -632,7 +632,7 @@ class PowerMaxMasking(object): def add_volume_to_storage_group( self, serial_number, device_id, storagegroup_name, - volume_name, extra_specs): + volume_name, extra_specs, force=False): """Add a volume to a storage group. :param serial_number: array serial number @@ -640,6 +640,7 @@ class PowerMaxMasking(object): :param storagegroup_name: storage group name :param volume_name: volume name :param extra_specs: extra specifications + :param force: add force argument to call """ start_time = time.time() @@ -655,7 +656,7 @@ class PowerMaxMasking(object): 'sg_name': storagegroup_name}) else: self.rest.add_vol_to_sg(serial_number, sg_name, - device_id, extra_specs) + device_id, extra_specs, force) do_add_volume_to_sg(storagegroup_name, serial_number) LOG.debug("Add volume to storagegroup took: %(delta)s H:MM:SS.", diff --git a/cinder/volume/drivers/dell_emc/powermax/rest.py b/cinder/volume/drivers/dell_emc/powermax/rest.py index 1ae12a4a977..ad106486c2d 100644 --- a/cinder/volume/drivers/dell_emc/powermax/rest.py +++ b/cinder/volume/drivers/dell_emc/powermax/rest.py @@ -931,21 +931,28 @@ class PowerMaxRest(object): found_device_id = device_id return found_device_id - def add_vol_to_sg(self, array, storagegroup_name, device_id, extra_specs): + def add_vol_to_sg(self, array, storagegroup_name, device_id, extra_specs, + force=False): """Add a volume to a storage group. :param array: the array serial number :param storagegroup_name: storage group name :param device_id: the device id :param extra_specs: extra specifications + :param force: add force argument to call """ if not isinstance(device_id, list): device_id = [device_id] + + force_add = "true" if force else "false" + payload = ({"executionOption": "ASYNCHRONOUS", "editStorageGroupActionParam": { "expandStorageGroupParam": { "addSpecificVolumeParam": { - "volumeId": device_id}}}}) + "volumeId": device_id, + "remoteSymmSGInfoParam": { + "force": force_add}}}}}) status_code, job = self.modify_storage_group( array, storagegroup_name, payload) @@ -961,12 +968,17 @@ class PowerMaxRest(object): :param device_id: the device id :param extra_specs: the extra specifications """ + + force_vol_remove = ("true" if "force_vol_remove" in extra_specs + else "false") if not isinstance(device_id, list): device_id = [device_id] payload = ({"executionOption": "ASYNCHRONOUS", "editStorageGroupActionParam": { "removeVolumeParam": { - "volumeId": device_id}}}) + "volumeId": device_id, + "remoteSymmSGInfoParam": { + "force": force_vol_remove}}}}) status_code, job = self.modify_storage_group( array, storagegroup_name, payload) @@ -1267,23 +1279,47 @@ class PowerMaxRest(object): self._modify_volume(array, device_id, rename_vol_payload) def delete_volume(self, array, device_id): - """Deallocate or delete a volume. + """Delete a volume. :param array: the array serial number :param device_id: volume device id """ - # Deallocate volume. Can fail if there are no tracks allocated. - payload = {"editVolumeActionParam": { - "freeVolumeParam": {"free_volume": 'true'}}} - try: - self._modify_volume(array, device_id, payload) - # Rename volume, removing the OS- - self.rename_volume(array, device_id, None) - except Exception as e: - LOG.warning('Deallocate volume failed with %(e)s.' - 'Attempting delete.', {'e': e}) - # Try to delete the volume if deallocate failed. - self.delete_resource(array, SLOPROVISIONING, "volume", device_id) + array_details = self.get_array_detail(array) + ucode_major_level = 0 + ucode_minor_level = 0 + + if array_details: + split_ucode_level = array_details['ucode'].split('.') + ucode_level = [int(level) for level in split_ucode_level] + ucode_major_level = ucode_level[0] + ucode_minor_level = ucode_level[1] + + if ((ucode_major_level >= utils.UCODE_5978) + and (ucode_minor_level > utils.UCODE_5978_ELMSR)): + # Use Rapid TDEV Deallocation to delete after ELMSR + try: + # Rename volume, removing the OS- + self.rename_volume(array, device_id, None) + self.delete_resource(array, SLOPROVISIONING, + "volume", device_id) + except Exception as e: + LOG.warning('Delete volume failed with %(e)s.', {'e': e}) + raise + else: + # Pre-Foxtail, deallocation and delete are separate calls + payload = {"editVolumeActionParam": { + "freeVolumeParam": {"free_volume": 'true'}}} + try: + # Rename volume, removing the OS- + self.rename_volume(array, device_id, None) + self._modify_volume(array, device_id, payload) + pass + except Exception as e: + LOG.warning('Deallocate volume failed with %(e)s.' + 'Attempting delete.', {'e': e}) + # Try to delete the volume if deallocate failed. + self.delete_resource(array, SLOPROVISIONING, + "volume", device_id) def find_mv_connections_for_vol(self, array, maskingview, device_id): """Find the host_lun_id for a volume in a masking view. @@ -2324,7 +2360,7 @@ class PowerMaxRest(object): # Need to format subsequent volumes payload['format'] = 'true' payload.pop('establish') - payload['rdfType'] = 'NA' + payload['rdfType'] = 'RDF1' return payload def modify_rdf_device_pair( @@ -2347,7 +2383,7 @@ class PowerMaxRest(object): and extra_specs[utils.REP_MODE] == utils.REP_ASYNC): common_opts.update({"immediate": 'false', "consExempt": 'true'}) - payload = {"action": "Suspend", + payload = {"action": "SUSPEND", "executionOption": "ASYNCHRONOUS", "suspend": common_opts} @@ -2379,7 +2415,7 @@ class PowerMaxRest(object): resource_name = ("%(rdf_num)s/volume/%(device_id)s" % {'rdf_num': rdf_group, 'device_id': device_id}) self.delete_resource(array, REPLICATION, 'rdf_group', resource_name, - private="/private", params=params) + params=params) def get_storage_group_rep(self, array, storage_group_name): """Given a name, return storage group details wrt replication. diff --git a/cinder/volume/drivers/dell_emc/powermax/utils.py b/cinder/volume/drivers/dell_emc/powermax/utils.py index 329e39ee84f..0ad7f56d7b0 100644 --- a/cinder/volume/drivers/dell_emc/powermax/utils.py +++ b/cinder/volume/drivers/dell_emc/powermax/utils.py @@ -41,6 +41,8 @@ VMAX_AFA_MODELS = ['VMAX250F', 'VMAX450F', 'VMAX850F', 'VMAX950F'] MAX_SRP_LENGTH = 16 TRUNCATE_5 = 5 TRUNCATE_27 = 27 +UCODE_5978_ELMSR = 221 +UCODE_5978 = 5978 ARRAY = 'array' SLO = 'slo' diff --git a/releasenotes/notes/powermax-tdev-deallocation-90bda0f95ab0b271.yaml b/releasenotes/notes/powermax-tdev-deallocation-90bda0f95ab0b271.yaml new file mode 100644 index 00000000000..66bef22eb28 --- /dev/null +++ b/releasenotes/notes/powermax-tdev-deallocation-90bda0f95ab0b271.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + PowerMax driver - Volume deallocate and volume delete functionality + have been combined into a single workflow.