diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_provision.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_provision.py index edd5feb7523..d75dae750a3 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_provision.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_provision.py @@ -442,15 +442,39 @@ class PowerMaxProvisionTest(test.TestCase): array = self.data.array snap_name = self.data.group_snapshot_name source_group_name = self.data.storagegroup_name_source - extra_specs = self.data.extra_specs - src_dev_ids = [self.data.device_id] with mock.patch.object( self.provision, 'delete_group_replica') as mock_delete_replica: self.provision.delete_group_replica( - array, snap_name, source_group_name, src_dev_ids, extra_specs) + array, snap_name, source_group_name) mock_delete_replica.assert_called_once_with( - array, snap_name, source_group_name, src_dev_ids, extra_specs) + array, snap_name, source_group_name) + + @mock.patch.object(rest.PowerMaxRest, + 'get_storagegroup_snap_generation_list', + side_effect=[['0', '3', '1', '2'], + ['0', '1'], ['0'], list()]) + def test_delete_group_replica_side_effect(self, mock_list): + array = self.data.array + snap_name = self.data.group_snapshot_name + source_group_name = self.data.storagegroup_name_source + with mock.patch.object( + self.rest, 'delete_storagegroup_snap') as mock_del: + self.provision.delete_group_replica( + array, snap_name, source_group_name) + self.assertEqual(4, mock_del.call_count) + mock_del.reset_mock() + self.provision.delete_group_replica( + array, snap_name, source_group_name) + self.assertEqual(2, mock_del.call_count) + mock_del.reset_mock() + self.provision.delete_group_replica( + array, snap_name, source_group_name) + self.assertEqual(1, mock_del.call_count) + mock_del.reset_mock() + self.provision.delete_group_replica( + array, snap_name, source_group_name) + mock_del.assert_not_called() def test_link_and_break_replica(self): array = self.data.array 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 599b042481b..f1bdba2cbe4 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 @@ -1497,6 +1497,27 @@ class PowerMaxRestTest(test.TestCase): mock_create.assert_called_once_with( array, source_group, snap_name, extra_specs) + def test_delete_storagegroup_snap(self): + array = self.data.array + source_group = self.data.storagegroup_name_source + snap_name = self.data.group_snapshot_name + with mock.patch.object( + self.rest, 'delete_storagegroup_snap') as mock_delete: + self.rest.delete_storagegroup_snap( + array, source_group, snap_name, '0') + mock_delete.assert_called_once_with( + array, source_group, snap_name, '0') + + @mock.patch.object(rest.PowerMaxRest, 'get_resource', + return_value={'generations': ['0', '1']}) + def test_get_storagegroup_snap_generation_list(self, mock_list): + array = self.data.array + source_group = self.data.storagegroup_name_source + snap_name = self.data.group_snapshot_name + ret_list = self.rest.get_storagegroup_snap_generation_list( + array, source_group, snap_name) + self.assertEqual(['0', '1'], ret_list) + def test_get_storagegroup_rdf_details(self): details = self.rest.get_storagegroup_rdf_details( self.data.array, self.data.test_vol_grp_name, diff --git a/cinder/volume/drivers/dell_emc/powermax/common.py b/cinder/volume/drivers/dell_emc/powermax/common.py index d93bbd618cf..8ef82fd1b0f 100644 --- a/cinder/volume/drivers/dell_emc/powermax/common.py +++ b/cinder/volume/drivers/dell_emc/powermax/common.py @@ -4468,16 +4468,10 @@ class PowerMaxCommon(object): {'group_id': source_group.id}) raise exception.VolumeBackendAPIException( message=exception_message) - # Check if the snapshot exists - if 'snapVXSnapshots' in volume_group: - if snap_name in volume_group['snapVXSnapshots']: - src_devs = self._get_snap_src_dev_list(array, snapshots) - self.provision.delete_group_replica( - array, snap_name, vol_grp_name, src_devs, extra_specs) - else: - # Snapshot has been already deleted, return successfully - LOG.error("Cannot find group snapshot %(snapId)s.", - {'snapId': group_snapshot.id}) + + self.provision.delete_group_replica( + array, snap_name, vol_grp_name) + model_update = {'status': fields.GroupSnapshotStatus.DELETED} for snapshot in snapshots: snapshots_model_update.append( @@ -4855,12 +4849,9 @@ class PowerMaxCommon(object): # Delete the snapshot if required if rollback_dict.get("snap_name"): try: - src_dev_ids = [ - a for a, b in rollback_dict['list_volume_pairs']] self.provision.delete_group_replica( array, rollback_dict["snap_name"], - rollback_dict["source_group_name"], - src_dev_ids, rollback_dict['interval_retries_dict']) + rollback_dict["source_group_name"]) except Exception as e: LOG.debug("Failed to delete group snapshot. Attempting " "further rollback. Exception received: %(e)s.", diff --git a/cinder/volume/drivers/dell_emc/powermax/fc.py b/cinder/volume/drivers/dell_emc/powermax/fc.py index 22d8f6bc27c..faa0d02e4c5 100644 --- a/cinder/volume/drivers/dell_emc/powermax/fc.py +++ b/cinder/volume/drivers/dell_emc/powermax/fc.py @@ -109,9 +109,10 @@ class PowerMaxFCDriver(san.SanDriver, driver.FibreChannelDriver): - Support for storage-assisted in-use retype (bp/powermax-storage-assisted-inuse-retype) 4.0.1 - PowerMax OS Metro formatted volumes fix (bug #1829876) + 4.0.2 - Volume group delete failure (bug #1853589) """ - VERSION = "4.0.1" + VERSION = "4.0.2" # ThirdPartySystems wiki CI_WIKI_NAME = "EMC_VMAX_CI" diff --git a/cinder/volume/drivers/dell_emc/powermax/iscsi.py b/cinder/volume/drivers/dell_emc/powermax/iscsi.py index f4db8544dcc..85a48583ec9 100644 --- a/cinder/volume/drivers/dell_emc/powermax/iscsi.py +++ b/cinder/volume/drivers/dell_emc/powermax/iscsi.py @@ -114,9 +114,10 @@ class PowerMaxISCSIDriver(san.SanISCSIDriver): - Support for storage-assisted in-use retype (bp/powermax-storage-assisted-inuse-retype) 4.0.1 - PowerMax OS Metro formatted volumes fix (bug #1829876) + 4.0.2 - Volume group delete failure (bug #1853589) """ - VERSION = "4.0.1" + VERSION = "4.0.2" # ThirdPartySystems wiki CI_WIKI_NAME = "EMC_VMAX_CI" diff --git a/cinder/volume/drivers/dell_emc/powermax/provision.py b/cinder/volume/drivers/dell_emc/powermax/provision.py index 2c934537dbc..1afaec2f2c4 100644 --- a/cinder/volume/drivers/dell_emc/powermax/provision.py +++ b/cinder/volume/drivers/dell_emc/powermax/provision.py @@ -382,8 +382,9 @@ class PowerMaxProvision(object): self._unlink_volume(array, "", "", snap_name, extra_specs, list_volume_pairs=list_device_pairs, generation=generation) - self.delete_volume_snap(array, snap_name, source_devices, - restored=False, generation=generation) + if source_devices: + self.delete_volume_snap(array, snap_name, source_devices, + restored=False, generation=generation) def extend_volume(self, array, device_id, new_size, extra_specs, rdf_group=None): @@ -691,8 +692,7 @@ class PowerMaxProvision(object): self.rest.create_storagegroup_snap( array, source_group, snap_name, extra_specs) - def delete_group_replica(self, array, snap_name, source_group_name, - src_dev_ids, extra_specs): + def delete_group_replica(self, array, snap_name, source_group_name): """Delete the snapshot. :param array: the array serial number @@ -701,12 +701,19 @@ class PowerMaxProvision(object): :param src_dev_ids: the list of source device ids :param extra_specs: extra specifications """ - # Delete snapvx snapshot LOG.debug("Deleting Snap Vx snapshot: source group: %(srcGroup)s " "snapshot: %(snap_name)s.", {'srcGroup': source_group_name, 'snap_name': snap_name}) - self.delete_volume_snap_check_for_links( - array, snap_name, src_dev_ids, extra_specs) + gen_list = self.rest.get_storagegroup_snap_generation_list( + array, source_group_name, snap_name) + if gen_list: + gen_list.sort(reverse=True) + for gen in gen_list: + self.rest.delete_storagegroup_snap( + array, source_group_name, snap_name, gen) + else: + LOG.debug("Unable to get generation number(s) for: %(srcGroup)s.", + {'srcGroup': source_group_name}) def link_and_break_replica(self, array, source_group_name, target_group_name, snap_name, extra_specs, diff --git a/cinder/volume/drivers/dell_emc/powermax/rest.py b/cinder/volume/drivers/dell_emc/powermax/rest.py index eab835acc37..47f2917337e 100644 --- a/cinder/volume/drivers/dell_emc/powermax/rest.py +++ b/cinder/volume/drivers/dell_emc/powermax/rest.py @@ -2441,6 +2441,43 @@ class PowerMaxRest(object): self.wait_for_job('Create storage group snapVx', status_code, job, extra_specs) + def delete_storagegroup_snap(self, array, source_group, + snap_name, generation='0'): + """Delete a snapVx snapshot of a storage group. + + :param array: the array serial number + :param source_group: the source group name + :param snap_name: the name of the snapshot + :param generation: the generation number of the SnapVX + """ + resource_name = ("%(sg_name)s/snapshot/%(snap_name)s" + "/generation/%(generation)s" + % {'sg_name': source_group, 'snap_name': snap_name, + 'generation': generation}) + + self.delete_resource( + array, REPLICATION, 'storagegroup', resource_name=resource_name) + + def get_storagegroup_snap_generation_list( + self, array, source_group, snap_name): + """Get a snapshot and its generation count information for an sg. + + The most recent snapshot will have a gen number of 0. The oldest + snapshot will have a gen number = genCount - 1 (i.e. if there are 4 + generations of particular snapshot, the oldest will have a gen num of + 3). + + :param array: name of the array -- str + :param source_group: name of the storage group -- str + :param snap_name: the name of the snapshot -- str + :returns: generation numbers -- list + """ + resource_name = ("%(sg_name)s/snapshot/%(snap_name)s/generation" + % {'sg_name': source_group, 'snap_name': snap_name}) + response = self.get_resource(array, REPLICATION, 'storagegroup', + resource_name=resource_name) + return response.get('generations', list()) if response else list() + def get_storagegroup_rdf_details(self, array, storagegroup_name, rdf_group_num): """Get the remote replication details of a storage group. diff --git a/releasenotes/notes/powermax-bug-1853589-f6c7164177da0496.yaml b/releasenotes/notes/powermax-bug-1853589-f6c7164177da0496.yaml new file mode 100644 index 00000000000..8902d33c4b5 --- /dev/null +++ b/releasenotes/notes/powermax-bug-1853589-f6c7164177da0496.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + PowerMax driver - fix to eliminate 'cannot use the device for the function + because it is in a Copy Session' when attempting to delete a volume group + that previously had a group snapshot created on and deleted from it.