From 4d968acc947fa5bc3296ebedee6b51b43786c63e Mon Sep 17 00:00:00 2001 From: michael-mcaleer Date: Wed, 17 Jul 2019 07:54:31 +0100 Subject: [PATCH] PowerMax Driver - SnapVX NoCopy Mode The PowerMax for Cinder driver now implements noCopy mode for links between SnapVX source and target. This change will improve space efficiency by using pointers instead of copied tracks when source and target volumes are linked. Change-Id: Idfc8de789a27f54d9211be3c1ea6778586f85672 --- .../dell_emc/powermax/powermax_data.py | 37 ++ .../dell_emc/powermax/test_powermax_common.py | 329 +++++++++--------- .../powermax/test_powermax_provision.py | 19 +- .../powermax/test_powermax_replication.py | 34 +- .../dell_emc/powermax/test_powermax_rest.py | 53 +-- .../drivers/dell_emc/powermax/common.py | 225 +++++------- cinder/volume/drivers/dell_emc/powermax/fc.py | 1 + .../volume/drivers/dell_emc/powermax/iscsi.py | 1 + .../drivers/dell_emc/powermax/provision.py | 26 +- .../volume/drivers/dell_emc/powermax/rest.py | 82 ++--- ...max-snapvx-link-mode-0050ac6b4a16c739.yaml | 7 + 11 files changed, 424 insertions(+), 390 deletions(-) create mode 100644 releasenotes/notes/powermax-snapvx-link-mode-0050ac6b4a16c739.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 acf18238958..9e407658f5e 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 @@ -1083,3 +1083,40 @@ class PowerMaxData(object): 'RestUserName': 'test', 'RestPassword': 'test', 'SSLVerify': 'True'}] + + snapshot_src_details = {'snapshotSrcs': [{ + 'snapshotName': 'temp-000AA-snapshot_for_clone', + 'generation': 0, 'state': 'Established', 'expired': False, + 'linkedDevices': [{'targetDevice': device_id2, 'state': 'Copied', + 'copy': True}]}, + {'snapshotName': 'temp-000AA-snapshot_for_clone', 'generation': 1, + 'state': 'Established', 'expired': False, + 'linkedDevices': [{'targetDevice': device_id3, 'state': 'Copied', + 'copy': True}]}], + 'snapshotLnks': []} + + snapshot_tgt_details = {"snapshotLnks": [{ + "linkSourceName": device_id2, "state": "Linked", "copy": False}]} + + snap_tgt_vol_details = {"timeFinderInfo": {"snapVXSession": [{ + "tgtSrcSnapshotGenInfo": { + "generation": 6, "expired": True, + "snapshotName": "temp-000AA-snapshot_for_clone"}}]}} + + snap_tgt_session = { + 'generation': 0, 'expired': False, 'copy_mode': False, + 'snap_name': 'temp-000AA-snapshot_for_clone', 'state': 'Copied', + 'source_vol_id': device_id, 'target_vol_id': device_id2} + + snap_tgt_session_cm_enabled = { + 'generation': 0, 'expired': False, 'copy_mode': True, + 'snap_name': 'temp-000AA-snapshot_for_clone', 'state': 'Copied', + 'source_vol_id': device_id, 'target_vol_id': device_id2} + + snap_src_sessions = [ + {'generation': 0, 'expired': False, 'copy_mode': False, + 'snap_name': 'temp-000AA-snapshot_for_clone', 'state': 'Copied', + 'source_vol_id': device_id, 'target_vol_id': device_id3}, + {'generation': 1, 'expired': False, 'copy_mode': False, + 'snap_name': 'temp-000AA-snapshot_for_clone', 'state': 'Copied', + 'source_vol_id': device_id, 'target_vol_id': device_id4}] diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_common.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_common.py index 840ae8a52cb..4701dc7bee3 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_common.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_common.py @@ -153,7 +153,8 @@ class PowerMaxCommonTest(test.TestCase): model_update = self.common.create_volume(self.data.test_volume) self.assertEqual(ref_model_update, model_update) - def test_create_volume_from_snapshot(self): + @mock.patch.object(common.PowerMaxCommon, '_clone_check') + def test_create_volume_from_snapshot(self, mck_clone_chk): ref_model_update = ({'provider_location': six.text_type( deepcopy(self.data.provider_location_snapshot))}) model_update = self.common.create_volume_from_snapshot( @@ -172,7 +173,8 @@ class PowerMaxCommonTest(test.TestCase): ast.literal_eval(ref_model_update['provider_location']), ast.literal_eval(model_update['provider_location'])) - def test_cloned_volume(self): + @mock.patch.object(common.PowerMaxCommon, '_clone_check') + def test_cloned_volume(self, mck_clone_chk): ref_model_update = ({'provider_location': six.text_type( self.data.provider_location_clone)}) model_update = self.common.create_cloned_volume( @@ -186,7 +188,8 @@ class PowerMaxCommonTest(test.TestCase): self.common.delete_volume(self.data.test_volume) mock_delete.assert_called_once_with(self.data.test_volume) - def test_create_snapshot(self): + @mock.patch.object(common.PowerMaxCommon, '_clone_check') + def test_create_snapshot(self, mck_clone_chk): ref_model_update = ({'provider_location': six.text_type( self.data.snap_location)}) model_update = self.common.create_snapshot( @@ -481,7 +484,8 @@ class PowerMaxCommonTest(test.TestCase): mck_extend.assert_called_with( array, device_id, new_size, ref_extra_specs, 10) - def test_extend_volume_failed_snap_src(self): + @mock.patch.object(common.PowerMaxCommon, '_sync_check') + def test_extend_volume_failed_snap_src(self, mck_sync): volume = self.data.test_volume new_size = self.data.test_volume.size with mock.patch.object(self.rest, 'is_vol_in_rep_session', @@ -497,7 +501,8 @@ class PowerMaxCommonTest(test.TestCase): self.assertRaises(exception.VolumeBackendAPIException, self.common.extend_volume, volume, new_size) - def test_extend_volume_failed_wrong_size(self): + @mock.patch.object(common.PowerMaxCommon, '_sync_check') + def test_extend_volume_failed_wrong_size(self, mck_sync): volume = self.data.test_volume new_size = 1 self.assertRaises(exception.VolumeBackendAPIException, @@ -705,7 +710,8 @@ class PowerMaxCommonTest(test.TestCase): volume, connector, extra_specs) self.assertEqual('NONE', masking_view_dict[utils.WORKLOAD]) - def test_create_cloned_volume(self): + @mock.patch.object(common.PowerMaxCommon, '_clone_check') + def test_create_cloned_volume(self, mck_clone_chk): volume = self.data.test_clone_volume source_volume = self.data.test_volume extra_specs = self.data.extra_specs @@ -714,7 +720,8 @@ class PowerMaxCommonTest(test.TestCase): volume, source_volume, extra_specs) self.assertEqual(ref_dict, clone_dict) - def test_create_cloned_volume_is_snapshot(self): + @mock.patch.object(common.PowerMaxCommon, '_clone_check') + def test_create_cloned_volume_is_snapshot(self, mck_clone_chk): volume = self.data.test_snapshot source_volume = self.data.test_volume extra_specs = self.data.extra_specs @@ -723,7 +730,8 @@ class PowerMaxCommonTest(test.TestCase): volume, source_volume, extra_specs, True, False) self.assertEqual(ref_dict, clone_dict) - def test_create_cloned_volume_from_snapshot(self): + @mock.patch.object(common.PowerMaxCommon, '_clone_check') + def test_create_cloned_volume_from_snapshot(self, mck_clone_chk): volume = self.data.test_clone_volume source_volume = self.data.test_snapshot extra_specs = self.data.extra_specs @@ -1085,8 +1093,9 @@ class PowerMaxCommonTest(test.TestCase): def test_get_port_group_from_masking_view(self): array = self.data.array maskingview_name = self.data.masking_view_name_f - with mock.patch.object( - self.rest, 'get_element_from_masking_view') as mock_get: + + with mock.patch.object(self.rest, + 'get_element_from_masking_view') as mock_get: self.common.get_port_group_from_masking_view( array, maskingview_name) mock_get.assert_called_once_with( @@ -1230,165 +1239,6 @@ class PowerMaxCommonTest(test.TestCase): array, target_device_id, clone_name, extra_specs) - @mock.patch.object(provision.PowerMaxProvision, 'delete_volume_snap') - @mock.patch.object(provision.PowerMaxProvision, - 'break_replication_relationship') - def test_sync_check_temp_snap(self, mock_break, mock_delete): - array = self.data.array - device_id = self.data.device_id - target = self.data.volume_details[1]['volumeId'] - extra_specs = self.data.extra_specs - snap_name = 'temp-1' - generation = '0' - with mock.patch.object(self.rest, 'get_volume_snap', - return_value=snap_name): - self.common._sync_check(array, device_id, extra_specs) - mock_break.assert_called_with( - array, target, device_id, snap_name, extra_specs, generation) - mock_delete.assert_called_with(array, snap_name, - device_id, restored=False, - generation=generation) - # Delete legacy temp snap - mock_delete.reset_mock() - snap_name2 = 'EMC_SMI_12345' - sessions = [{'source_vol': device_id, - 'snap_name': snap_name2, - 'target_vol_list': [], 'generation': 0}] - with mock.patch.object(self.rest, 'find_snap_vx_sessions', - return_value=sessions): - with mock.patch.object(self.rest, 'get_volume_snap', - return_value=snap_name2): - self.common._sync_check(array, device_id, extra_specs) - mock_delete.assert_called_once_with( - array, snap_name2, device_id, restored=False, generation=0) - - @mock.patch.object(provision.PowerMaxProvision, 'delete_volume_snap') - @mock.patch.object(provision.PowerMaxProvision, - 'break_replication_relationship') - def test_sync_check_not_temp_snap(self, mock_break, mock_delete): - array = self.data.array - device_id = self.data.device_id - target = self.data.volume_details[1]['volumeId'] - extra_specs = self.data.extra_specs - snap_name = 'OS-1' - sessions = [{'source_vol': device_id, - 'snap_name': snap_name, 'generation': 0, - 'target_vol_list': [(target, "Copied")]}] - with mock.patch.object(self.rest, 'find_snap_vx_sessions', - return_value=sessions): - self.common._sync_check(array, device_id, extra_specs) - mock_break.assert_called_with( - array, target, device_id, snap_name, extra_specs, 0) - mock_delete.assert_not_called() - - @mock.patch.object(provision.PowerMaxProvision, - 'break_replication_relationship') - def test_sync_check_no_sessions(self, mock_break): - array = self.data.array - device_id = self.data.device_id - extra_specs = self.data.extra_specs - with mock.patch.object(self.rest, 'find_snap_vx_sessions', - return_value=None): - self.common._sync_check(array, device_id, extra_specs) - mock_break.assert_not_called() - - def test_do_sync_check_repeat(self): - array = self.data.array - device_id = self.data.device_id - extra_specs = self.data.extra_specs - with mock.patch.object(self.common, - '_unlink_targets_and_delete_temp_snapvx', - side_effect=Exception): - with mock.patch.object(self.common, - '_unlink_targets_and_delete_temp_snapvx', - side_effect=None): - self.common._sync_check(array, device_id, extra_specs) - - def test_do_sync_check_repeat_and_fail_again(self): - array = self.data.array - device_id = self.data.device_id - extra_specs = self.data.extra_specs - with mock.patch.object(self.common, - '_unlink_targets_and_delete_temp_snapvx', - side_effect=Exception): - with mock.patch.object(self.common, - '_unlink_targets_and_delete_temp_snapvx', - side_effect=Exception): - self.assertRaises(exception.VolumeBackendAPIException, - self.common._sync_check, array, - device_id, extra_specs) - - @mock.patch.object(provision.PowerMaxProvision, 'delete_volume_snap') - @mock.patch.object(provision.PowerMaxProvision, - 'break_replication_relationship') - def test_clone_check_cinder_snap(self, mock_break, mock_delete): - array = self.data.array - device_id = self.data.device_id - target = self.data.volume_details[1]['volumeId'] - extra_specs = self.data.extra_specs - snap_name = 'OS-1' - sessions = [{'source_vol': device_id, - 'snap_name': snap_name, 'generation': 0, - 'target_vol_list': [(target, "Copied")]}] - - with mock.patch.object(self.rest, 'is_vol_in_rep_session', - return_value=(True, False, None)): - with mock.patch.object(self.rest, 'find_snap_vx_sessions', - return_value=sessions): - self.common._clone_check(array, device_id, extra_specs) - mock_delete.assert_not_called() - - mock_delete.reset_mock() - with mock.patch.object(self.rest, 'find_snap_vx_sessions', - return_value=sessions): - self.common._clone_check(array, device_id, extra_specs) - mock_break.assert_called_with( - array, target, device_id, snap_name, extra_specs, 0) - - @mock.patch.object(provision.PowerMaxProvision, 'delete_volume_snap') - @mock.patch.object(provision.PowerMaxProvision, - 'break_replication_relationship') - def test_clone_check_temp_snap(self, mock_break, mock_delete): - array = self.data.array - device_id = self.data.device_id - target = self.data.volume_details[1]['volumeId'] - extra_specs = self.data.extra_specs - temp_snap_name = 'temp-' + device_id + '-' + 'snapshot_for_clone' - sessions = [{'source_vol': device_id, - 'snap_name': temp_snap_name, 'generation': 0, - 'target_vol_list': [(target, "Copied")]}] - - with mock.patch.object(self.rest, 'find_snap_vx_sessions', - return_value=sessions): - self.common._clone_check(array, device_id, extra_specs) - mock_break.assert_called_with( - array, target, device_id, temp_snap_name, extra_specs, 0) - mock_delete.assert_not_called() - - sessions1 = [{'source_vol': device_id, - 'snap_name': temp_snap_name, 'generation': 0, - 'target_vol_list': [(target, "CopyInProg")]}] - mock_delete.reset_mock() - mock_break.reset_mock() - with mock.patch.object(self.rest, 'is_vol_in_rep_session', - return_value=(False, True, None)): - with mock.patch.object(self.rest, 'find_snap_vx_sessions', - return_value=sessions1): - self.common._clone_check(array, device_id, extra_specs) - mock_break.assert_not_called() - mock_delete.assert_not_called() - - @mock.patch.object(provision.PowerMaxProvision, - 'break_replication_relationship') - def test_clone_check_no_sessions(self, mock_break): - array = self.data.array - device_id = self.data.device_id - extra_specs = self.data.extra_specs - with mock.patch.object(self.rest, 'find_snap_vx_sessions', - return_value=None): - self.common._clone_check(array, device_id, extra_specs) - mock_break.assert_not_called() - def test_manage_existing_success(self): external_ref = {u'source-name': u'00002'} provider_location = {'device_id': u'00002', 'array': u'000197800123'} @@ -1471,7 +1321,8 @@ class PowerMaxCommonTest(test.TestCase): @mock.patch.object(common.PowerMaxCommon, '_remove_vol_and_cleanup_replication') - def test_unmanage_success(self, mock_rm): + @mock.patch.object(common.PowerMaxCommon, '_sync_check') + def test_unmanage_success(self, mck_sync, mock_rm): volume = self.data.test_volume with mock.patch.object(self.rest, 'rename_volume') as mock_rename: self.common.unmanage(volume) @@ -2755,7 +2606,8 @@ class PowerMaxCommonTest(test.TestCase): @mock.patch.object( common.PowerMaxCommon, 'get_remote_target_device', return_value=(None, None, None, None, None)) - def test_extend_legacy_replicated_vol_fail(self, mck_get_tgt): + @mock.patch.object(common.PowerMaxCommon, '_sync_check') + def test_extend_legacy_replicated_vol_fail(self, mck_sync, mck_get_tgt): volume = self.data.test_volume_group_member array = self.data.array @@ -2805,3 +2657,138 @@ class PowerMaxCommonTest(test.TestCase): self.common._sync_check(array, device_id, extra_specs, source_device_id='00123') mock_check.assert_not_called() + + def test_sync_check(self): + array = self.data.array + device_id = self.data.device_id + extra_specs = self.data.extra_specs + + with mock.patch.object(self.common, '_do_sync_check') as mck_sync: + self.common._sync_check(array, device_id, extra_specs, False, + self.data.device_id2) + mck_sync.assert_called_with(array, self.data.device_id2, + extra_specs, False) + mck_sync.reset_mock() + with mock.patch.object(self.common, '_get_target_source_device', + return_value=self.data.device_id3): + self.common._sync_check(array, device_id, extra_specs, True) + mck_sync.assert_called_with(array, self.data.device_id3, + extra_specs, True) + mck_sync.reset_mock() + self.common._sync_check(array, device_id, extra_specs) + mck_sync.assert_called_with(array, device_id, extra_specs, False) + + @mock.patch.object(common.PowerMaxCommon, + '_unlink_targets_and_delete_temp_snapvx') + @mock.patch.object(rest.PowerMaxRest, 'find_snap_vx_sessions', + return_value=(tpd.PowerMaxData.snap_src_sessions, + tpd.PowerMaxData.snap_tgt_session)) + @mock.patch.object(rest.PowerMaxRest, 'is_vol_in_rep_session', + return_value=(True, True, False)) + def test_do_sync_check(self, mck_rep, mck_find, mck_unlink): + + array = self.data.array + device_id = self.data.device_id + extra_specs = self.data.extra_specs + self.common._do_sync_check(array, device_id, extra_specs) + self.assertEqual(3, mck_unlink.call_count) + + @mock.patch.object(provision.PowerMaxProvision, 'delete_temp_volume_snap') + @mock.patch.object(provision.PowerMaxProvision, + 'break_replication_relationship') + def test_unlink_targets_and_delete_temp_snapvx(self, mck_break, mck_del): + array = self.data.array + extra_specs = self.data.extra_specs + session = self.data.snap_tgt_session_cm_enabled + snap_name = session['snap_name'] + source = session['source_vol_id'] + generation = session['generation'] + target = session['target_vol_id'] + + self.common._unlink_targets_and_delete_temp_snapvx( + session, array, extra_specs) + mck_break.assert_called_with(array, target, source, snap_name, + extra_specs, generation, True) + mck_del.assert_called_once_with(array, snap_name, source, generation) + + mck_break.reset_mock() + mck_del.reset_mock() + + session['copy_mode'] = False + session['expired'] = True + self.common._unlink_targets_and_delete_temp_snapvx( + session, array, extra_specs) + mck_break.assert_called_with(array, target, source, snap_name, + extra_specs, generation, False) + mck_del.assert_not_called() + + @mock.patch.object(rest.PowerMaxRest, 'find_snap_vx_sessions', + return_value=(None, tpd.PowerMaxData.snap_tgt_session)) + @mock.patch.object(rest.PowerMaxRest, 'is_vol_in_rep_session', + return_value=(True, False, False)) + def test_get_target_source_device(self, mck_rep, mck_find): + array = self.data.array + tgt_device = self.data.device_id2 + src_device = self.common._get_target_source_device(array, tgt_device) + self.assertEqual(src_device, self.data.device_id) + + @mock.patch.object(common.PowerMaxCommon, '_delete_valid_snapshot') + @mock.patch.object(rest.PowerMaxRest, 'find_snap_vx_sessions', + return_value=(tpd.PowerMaxData.snap_src_sessions, + tpd.PowerMaxData.snap_tgt_session)) + @mock.patch.object(rest.PowerMaxRest, 'is_vol_in_rep_session', + return_value=(True, True, False)) + def test_clone_check(self, mck_rep, mck_find, mck_del): + array = self.data.array + device_id = self.data.device_id + extra_specs = self.data.extra_specs + self.common.snapvx_unlink_limit = 3 + self.common._clone_check(array, device_id, extra_specs) + self.assertEqual(3, mck_del.call_count) + + @mock.patch.object(common.PowerMaxCommon, + '_unlink_targets_and_delete_temp_snapvx') + def test_delete_valid_snapshot(self, mck_unlink): + + array = self.data.array + extra_specs = self.data.extra_specs + + session = {'snap_name': 'EMC_SMI_TEST', 'expired': False} + self.common._delete_valid_snapshot(array, session, extra_specs) + mck_unlink.assert_called_with(session, array, extra_specs) + mck_unlink.reset_mock() + + session = {'snap_name': 'temp-000AA-snapshot_for_clone', + 'expired': True} + self.common._delete_valid_snapshot(array, session, extra_specs) + mck_unlink.assert_called_with(session, array, extra_specs) + mck_unlink.reset_mock() + + session = {'snap_name': 'temp-000AA-snapshot_for_clone', + 'expired': False} + self.common._delete_valid_snapshot(array, session, extra_specs) + mck_unlink.assert_not_called() + + def test_delete_valid_snapshot_exception(self): + + array = self.data.array + extra_specs = self.data.extra_specs + session = {'snap_name': 'temp-000AA-snapshot_for_clone', + 'expired': True} + + with mock.patch.object( + self.common, '_unlink_targets_and_delete_temp_snapvx', + side_effect=exception.VolumeBackendAPIException( + "404 temp-000AA-snapshot_for_clone does not exist") + ) as mck_unlink: + self.common._delete_valid_snapshot(array, session, extra_specs) + mck_unlink.assert_called_with(session, array, extra_specs) + + with mock.patch.object( + self.common, '_unlink_targets_and_delete_temp_snapvx', + side_effect=exception.VolumeBackendAPIException( + "500 internal server error")): + self.assertRaises( + exception.VolumeBackendAPIException, + self.common._unlink_targets_and_delete_temp_snapvx, + array, session, extra_specs) 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 537a834a564..ac8242b94d3 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 @@ -146,15 +146,16 @@ class PowerMaxProvisionTest(test.TestCase): target_device_id = self.data.device_id2 snap_name = self.data.snap_location['snap_name'] extra_specs = self.data.extra_specs + with mock.patch.object( - self.provision.rest, 'modify_volume_snap') as mock_modify: + self.provision, '_unlink_volume') as mock_unlink: self.provision.break_replication_relationship( array, target_device_id, source_device_id, snap_name, - extra_specs) - mock_modify.assert_called_once_with( + extra_specs, generation=6, loop=True) + mock_unlink.assert_called_once_with( array, source_device_id, target_device_id, snap_name, extra_specs, list_volume_pairs=None, - unlink=True, generation=0) + generation=6, loop=True) @mock.patch('oslo_service.loopingcall.FixedIntervalLoopingCall', new=test_utils.ZeroIntervalLoopingCall) @@ -168,6 +169,16 @@ class PowerMaxProvisionTest(test.TestCase): self.data.snap_location['snap_name'], self.data.extra_specs, list_volume_pairs=None, unlink=True, generation=0) + mock_mod.reset_mock() + self.provision._unlink_volume( + self.data.array, self.data.device_id, self.data.device_id2, + self.data.snap_location['snap_name'], self.data.extra_specs, + loop=False) + mock_mod.assert_called_once_with( + self.data.array, self.data.device_id, self.data.device_id2, + self.data.snap_location['snap_name'], self.data.extra_specs, + list_volume_pairs=None, unlink=True, generation=0) + @mock.patch('oslo_service.loopingcall.FixedIntervalLoopingCall', new=test_utils.ZeroIntervalLoopingCall) def test_unlink_volume_exception(self): diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_replication.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_replication.py index f045bad61ea..0e2caa5cd0c 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_replication.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_replication.py @@ -165,7 +165,8 @@ class PowerMaxReplicationTest(test.TestCase): self.common.create_volume, self.data.test_volume) - def test_create_cloned_replicated_volume(self): + @mock.patch.object(common.PowerMaxCommon, '_clone_check') + def test_create_cloned_replicated_volume(self, mck_clone): extra_specs = deepcopy(self.extra_specs) extra_specs[utils.PORTGROUPNAME] = self.data.port_group_name_f with mock.patch.object(self.common, '_replicate_volume', @@ -177,7 +178,8 @@ class PowerMaxReplicationTest(test.TestCase): self.data.test_clone_volume, self.data.test_clone_volume.name, volume_dict, extra_specs) - def test_create_replicated_volume_from_snap(self): + @mock.patch.object(common.PowerMaxCommon, '_clone_check') + def test_create_replicated_volume_from_snap(self, mck_clone): extra_specs = deepcopy(self.extra_specs) extra_specs[utils.PORTGROUPNAME] = self.data.port_group_name_f with mock.patch.object(self.common, '_replicate_volume', @@ -357,10 +359,11 @@ class PowerMaxReplicationTest(test.TestCase): self.data.test_volume, volume_name, provider_location, extra_specs, delete_src=False) + @mock.patch.object(common.PowerMaxCommon, '_sync_check') @mock.patch.object(masking.PowerMaxMasking, 'remove_and_reset_members') @mock.patch.object(rest.PowerMaxRest, 'get_array_model_info', return_value=('VMAX250F', False)) - def test_setup_volume_replication(self, mock_model, mock_rm): + def test_setup_volume_replication(self, mock_model, mock_rm, mck_sync): rep_status, rep_data, __ = self.common.setup_volume_replication( self.data.array, self.data.test_volume, self.data.device_id, self.extra_specs) @@ -368,12 +371,13 @@ class PowerMaxReplicationTest(test.TestCase): self.assertEqual({'array': self.data.remote_array, 'device_id': self.data.device_id}, rep_data) + @mock.patch.object(common.PowerMaxCommon, '_sync_check') @mock.patch.object(masking.PowerMaxMasking, 'remove_and_reset_members') @mock.patch.object(common.PowerMaxCommon, '_create_volume') @mock.patch.object(rest.PowerMaxRest, 'get_array_model_info', return_value=('VMAX250F', False)) def test_setup_volume_replication_target( - self, mock_model, mock_create, mock_rm): + self, mock_model, mock_create, mock_rm, mck_sync): rep_status, rep_data, __ = self.common.setup_volume_replication( self.data.array, self.data.test_volume, self.data.device_id, self.extra_specs, self.data.device_id2) @@ -532,7 +536,9 @@ class PowerMaxReplicationTest(test.TestCase): @mock.patch.object(masking.PowerMaxMasking, 'remove_vol_from_storage_group') @mock.patch.object(common.PowerMaxCommon, '_delete_from_srp') - def test_cleanup_replication_source(self, mock_del, mock_rm, mock_clean): + @mock.patch.object(common.PowerMaxCommon, '_sync_check') + def test_cleanup_replication_source( + self, mck_sync, mock_del, mock_rm, mock_clean): self.common._cleanup_replication_source( self.data.array, self.data.test_volume, 'vol1', {'device_id': self.data.device_id}, self.extra_specs) @@ -551,7 +557,8 @@ class PowerMaxReplicationTest(test.TestCase): self.assertRaises(exception.VolumeBackendAPIException, self.common.get_rdf_details, self.data.array) - def test_failover_host(self): + @mock.patch.object(common.PowerMaxCommon, '_sync_check') + def test_failover_host(self, mck_sync): volumes = [self.data.test_volume, self.data.test_clone_volume] with mock.patch.object(self.common, '_failover_replication', return_value=(None, {})) as mock_fo: @@ -595,8 +602,9 @@ class PowerMaxReplicationTest(test.TestCase): return_value=('VMAX250F', False)) @mock.patch.object(common.PowerMaxCommon, 'add_volume_to_replication_group') + @mock.patch.object(common.PowerMaxCommon, '_sync_check') @mock.patch.object(masking.PowerMaxMasking, 'remove_and_reset_members') - def test_enable_rdf(self, mock_remove, mock_add, mock_model): + def test_enable_rdf(self, mock_remove, mck_sync, mock_add, mock_model): rep_config = self.utils.get_replication_config( [self.replication_device]) self.common.enable_rdf( @@ -882,13 +890,14 @@ class PowerMaxReplicationTest(test.TestCase): self.data.failed_resource, self.data.device_id, 'name', self.data.remote_array, self.data.device_id2, extra_specs) + @mock.patch.object(common.PowerMaxCommon, '_sync_check') @mock.patch.object(rest.PowerMaxRest, 'get_array_model_info', return_value=('VMAX250F', False)) @mock.patch.object(common.PowerMaxCommon, '_add_volume_to_async_rdf_managed_grp') @mock.patch.object(masking.PowerMaxMasking, 'remove_and_reset_members') def test_setup_volume_replication_async( - self, mock_rm, mock_add, mock_model): + self, mock_rm, mock_add, mock_model, mck_sync): extra_specs = deepcopy(self.extra_specs) extra_specs['rep_mode'] = utils.REP_ASYNC rep_status, rep_data, __ = ( @@ -902,7 +911,8 @@ class PowerMaxReplicationTest(test.TestCase): @mock.patch.object(common.PowerMaxCommon, '_failover_replication', return_value=({}, {})) - def test_failover_host_async(self, mock_fg): + @mock.patch.object(common.PowerMaxCommon, '_sync_check') + def test_failover_host_async(self, mck_sync, mock_fg): volumes = [self.data.test_volume] extra_specs = deepcopy(self.extra_specs) extra_specs['rep_mode'] = utils.REP_ASYNC @@ -994,12 +1004,13 @@ class PowerMaxReplicationDebugTest(test.TestCase): self.extra_specs['interval'] = 1 self.extra_specs['rep_mode'] = 'Synchronous' + @mock.patch.object(common.PowerMaxCommon, '_sync_check') @mock.patch.object(masking.PowerMaxMasking, 'remove_and_reset_members') @mock.patch.object(common.PowerMaxCommon, '_create_volume') @mock.patch.object(rest.PowerMaxRest, 'get_array_model_info', return_value=('VMAX250F', False)) def test_setup_volume_replication_target_debug( - self, mock_model, mock_create, mock_rm): + self, mock_model, mock_create, mock_rm, mck_sync): rep_status, rep_data, rep_info_dict = ( self.common.setup_volume_replication( self.data.array, self.data.test_volume, self.data.device_id, @@ -1010,11 +1021,12 @@ class PowerMaxReplicationDebugTest(test.TestCase): self.assertEqual('VMAX250F', rep_info_dict['target_array_model']) mock_create.assert_not_called() + @mock.patch.object(common.PowerMaxCommon, '_sync_check') @mock.patch.object(masking.PowerMaxMasking, 'remove_and_reset_members') @mock.patch.object(rest.PowerMaxRest, 'get_array_model_info', return_value=('VMAX250F', False)) def test_setup_volume_replication_no_target_debug( - self, mock_model, mock_rm): + self, mock_model, mock_rm, mck_sync): rep_status, rep_data, rep_info_dict = ( self.common.setup_volume_replication( self.data.array, self.data.test_volume, self.data.device_id, 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 859c26e1550..954a6b0e845 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 @@ -1092,7 +1092,7 @@ class PowerMaxRestTest(test.TestCase): payload = {'deviceNameListSource': [{'name': source_id}], 'deviceNameListTarget': [ {'name': target_id}], - 'copy': 'true', 'action': "", + 'copy': 'false', 'action': "", 'star': 'false', 'force': 'false', 'exact': 'false', 'remote': 'false', 'symforce': 'false', 'generation': 0} @@ -1229,30 +1229,41 @@ class PowerMaxRestTest(test.TestCase): def test_find_snap_vx_sessions(self): array = self.data.array source_id = self.data.device_id - ref_sessions = [{'generation': '0', - 'snap_name': 'temp-1', - 'source_vol': self.data.device_id, - 'target_vol_list': - [(self.data.device_id2, 'Copied')]}, - {'generation': '0', - 'snap_name': 'temp-1', - 'source_vol': self.data.device_id, - 'target_vol_list': - [(self.data.device_id2, 'Copied')]}] - sessions = self.rest.find_snap_vx_sessions(array, source_id) - self.assertEqual(ref_sessions, sessions) + ref_sessions = [{'generation': 0, + 'snap_name': 'temp-000AA-snapshot_for_clone', + 'source_vol_id': self.data.device_id, + 'target_vol_id': self.data.device_id2, + 'expired': False, 'copy_mode': True, + 'state': 'Copied'}, + {'generation': 1, + 'snap_name': 'temp-000AA-snapshot_for_clone', + 'source_vol_id': self.data.device_id, + 'target_vol_id': self.data.device_id3, + 'expired': False, 'copy_mode': True, + 'state': 'Copied'}] - def test_find_snap_vx_sessions_tgt_only(self): + with mock.patch.object(self.rest, 'get_volume_snap_info', + return_value=self.data.snapshot_src_details): + src_list, __ = self.rest.find_snap_vx_sessions(array, source_id) + self.assertEqual(ref_sessions, src_list) + self.assertIsInstance(src_list, list) + + @mock.patch.object(rest.PowerMaxRest, '_get_private_volume', + return_value=tpd.PowerMaxData.snap_tgt_vol_details) + @mock.patch.object(rest.PowerMaxRest, 'get_volume_snap_info', + return_value=tpd.PowerMaxData.snapshot_tgt_details) + def test_find_snap_vx_sessions_tgt_only(self, mck_snap, mck_vol): array = self.data.array source_id = self.data.device_id - ref_sessions = [{'generation': '0', - 'snap_name': 'temp-1', - 'source_vol': self.data.device_id, - 'target_vol_list': - [(self.data.device_id2, 'Copied')]}] - sessions = self.rest.find_snap_vx_sessions( + ref_session = {'generation': 6, 'state': 'Linked', 'copy_mode': False, + 'snap_name': 'temp-000AA-snapshot_for_clone', + 'source_vol_id': self.data.device_id2, + 'target_vol_id': source_id, 'expired': True} + + __, snap_tgt = self.rest.find_snap_vx_sessions( array, source_id, tgt_only=True) - self.assertEqual(ref_sessions, sessions) + self.assertEqual(ref_session, snap_tgt) + self.assertIsInstance(snap_tgt, dict) def test_update_storagegroup_qos(self): sg_qos = {'srp': self.data.srp, 'num_of_vols': 2, 'cap_gb': 2, diff --git a/cinder/volume/drivers/dell_emc/powermax/common.py b/cinder/volume/drivers/dell_emc/powermax/common.py index ca89d82a22c..a6c9ce3a740 100644 --- a/cinder/volume/drivers/dell_emc/powermax/common.py +++ b/cinder/volume/drivers/dell_emc/powermax/common.py @@ -2214,6 +2214,7 @@ class PowerMaxCommon(object): self._delete_from_srp( array, target_device_id, clone_name, extra_specs) + @retry(retry_exc_tuple, interval=1, retries=3) def _sync_check(self, array, device_id, extra_specs, tgt_only=False, source_device_id=None): """Check if volume is part of a SnapVx sync process. @@ -2227,27 +2228,26 @@ class PowerMaxCommon(object): """ if not source_device_id and tgt_only: source_device_id = self._get_target_source_device( - array, device_id, tgt_only) + array, device_id) if source_device_id: - @coordination.synchronized("emc-source-{source_device_id}") - def do_unlink_and_delete_snap(source_device_id): + @coordination.synchronized("emc-source-{src_device_id}") + def do_unlink_and_delete_snap(src_device_id): # Check if source device exists on the array try: - self.rest.get_volume(array, source_device_id) + self.rest.get_volume(array, src_device_id) except exception.VolumeBackendAPIException: LOG.debug("Device %(device_id)s not found on array, no " "sync check required.", - {'device_id': source_device_id}) + {'device_id': src_device_id}) return self._do_sync_check( - array, device_id, extra_specs, tgt_only) + array, src_device_id, extra_specs, tgt_only) do_unlink_and_delete_snap(source_device_id) else: self._do_sync_check( array, device_id, extra_specs, tgt_only) - @retry(retry_exc_tuple, interval=2, retries=2) def _do_sync_check( self, array, device_id, extra_specs, tgt_only=False): """Check if volume is part of a SnapVx sync process. @@ -2258,92 +2258,82 @@ class PowerMaxCommon(object): :param extra_specs: extra specifications :param tgt_only: Flag to specify if it is a target """ - get_sessions = False snapvx_tgt, snapvx_src, __ = self.rest.is_vol_in_rep_session( array, device_id) - if snapvx_tgt: - get_sessions = True - elif snapvx_src and not tgt_only: - get_sessions = True - if get_sessions: - snap_vx_sessions = self.rest.find_snap_vx_sessions( - array, device_id, tgt_only) - if snap_vx_sessions: - snap_vx_sessions.sort( - key=lambda k: k['generation'], reverse=True) - for session in snap_vx_sessions: - try: - self._unlink_targets_and_delete_temp_snapvx( - session, array, extra_specs) - except Exception: - exception_message = _( - "Will retry one more time.") + get_sessions = True if snapvx_tgt or snapvx_src else False - LOG.warning(exception_message) - raise exception.VolumeBackendAPIException( - exception_message) + if get_sessions: + src_sessions, tgt_session = self.rest.find_snap_vx_sessions( + array, device_id, tgt_only) + if tgt_session: + self._unlink_targets_and_delete_temp_snapvx( + tgt_session, array, extra_specs) + if src_sessions and not tgt_only: + src_sessions.sort(key=lambda k: k['generation'], reverse=True) + for session in src_sessions: + self._unlink_targets_and_delete_temp_snapvx( + session, array, extra_specs) def _unlink_targets_and_delete_temp_snapvx( self, session, array, extra_specs): - """unlink targets and delete the temporary snapvx + """Unlink target and delete the temporary snapvx if valid candidate. :param session: the snapvx session :param array: the array serial number :param extra_specs: extra specifications """ - source = session['source_vol'] snap_name = session['snap_name'] - targets = session['target_vol_list'] + source = session['source_vol_id'] generation = session['generation'] - for target in targets: - LOG.debug("Unlinking source from target. Source: " - "%(volume)s, Target: %(target)s, " - "generation: %(generation)s.", - {'volume': source, 'target': target[0], - 'generation': generation}) + expired = session['expired'] + + target, cm_enabled = None, False + if session.get('target_vol_id'): + target = session['target_vol_id'] + cm_enabled = session['copy_mode'] + + if target: + loop = True if cm_enabled else False + LOG.debug( + "Unlinking source from target. Source: %(vol)s, Target: " + "%(tgt)s, Generation: %(gen)s.", {'vol': source, 'tgt': target, + 'gen': generation}) self.provision.break_replication_relationship( - array, target[0], source, snap_name, - extra_specs, generation) - # The snapshot name will only have 'temp' (or EMC_SMI for - # legacy volumes) if it is a temporary volume. - # Only then is it a candidate for deletion. - if 'temp' in snap_name or 'EMC_SMI' in snap_name: - LOG.debug("Deleting temporary snapshot. Source: " - "%(volume)s, snap name: %(snap_name)s, " - "generation: %(generation)s.", - {'volume': source, 'snap_name': snap_name, - 'generation': generation}) + array, target, source, snap_name, extra_specs, generation, + loop) + + # Candidates for deletion: + # 1. If legacy snapshot with 'EMC_SMI' in snapshot name + # 2. If snapVX snapshot with copy mode enabled + # 3. If snapVX snapshot with copy mode disabled and not expired + if ('EMC_SMI' in snap_name or cm_enabled or ( + not cm_enabled and not expired)): + LOG.debug( + "Deleting temporary snapshot. Source: %(vol)s, snap name: " + "%(name)s, generation: %(gen)s.", { + 'vol': source, 'name': snap_name, 'gen': generation}) self.provision.delete_temp_volume_snap( array, snap_name, source, generation) - def _get_target_source_device( - self, array, device_id, tgt_only=False): + def _get_target_source_device(self, array, device_id): """Get the source device id of the target. :param array: the array serial number :param device_id: volume instance - :param tgt_only: Flag - return only sessions where device is target return source_device_id """ - LOG.debug("Getting source device id from target %(target)s.", - {'target': device_id}) - get_sessions = False + LOG.debug("Getting the source device ID for target device %(tgt)s", + {'tgt': device_id}) source_device_id = None - snapvx_tgt, snapvx_src, __ = self.rest.is_vol_in_rep_session( + snapvx_tgt, __, __ = self.rest.is_vol_in_rep_session( array, device_id) if snapvx_tgt: - get_sessions = True - elif snapvx_src and not tgt_only: - get_sessions = True - if get_sessions: - snap_vx_sessions = self.rest.find_snap_vx_sessions( - array, device_id, tgt_only) - if snap_vx_sessions: - snap_vx_sessions.sort( - key=lambda k: k['generation'], reverse=True) - for session in snap_vx_sessions: - source_device_id = session['source_vol'] - break + __, tgt_session = self.rest.find_snap_vx_sessions( + array, device_id, tgt_only=True) + source_device_id = tgt_session['source_vol_id'] + LOG.debug("Target %(tgt)s source device %(src)s", + {'target': device_id, 'src': source_device_id}) + return source_device_id def _clone_check(self, array, device_id, extra_specs): @@ -2355,86 +2345,51 @@ class PowerMaxCommon(object): """ snapvx_tgt, snapvx_src, __ = self.rest.is_vol_in_rep_session( array, device_id) + if snapvx_src or snapvx_tgt: @coordination.synchronized("emc-source-{src_device_id}") def do_unlink_and_delete_snap(src_device_id): - snap_vx_sessions = self.rest.find_snap_vx_sessions( + src_sessions, tgt_session = self.rest.find_snap_vx_sessions( array, src_device_id) - if snap_vx_sessions: - snap_vx_sessions.sort( + count = 0 + if tgt_session and count < self.snapvx_unlink_limit: + self._delete_valid_snapshot(array, tgt_session, + extra_specs) + count += 1 + if src_sessions: + src_sessions.sort( key=lambda k: k['generation'], reverse=True) - self._break_relationship( - snap_vx_sessions, snapvx_tgt, src_device_id, array, - extra_specs) + for session in src_sessions: + if count < self.snapvx_unlink_limit: + self._delete_valid_snapshot(array, session, + extra_specs) + count += 1 + else: + break do_unlink_and_delete_snap(device_id) - def _break_relationship( - self, snap_vx_sessions, snapvx_tgt, snapvx_src, array, - extra_specs): - """Break relationship and cleanup - - :param snap_vx_sessions: the snapvx sessions - :param snapvx_tgt: the snapvx target - :param snapvx_src: the snapvx source - :param array: the serialnumber of the array - :param extra_specs: extra specifications - """ - count = 0 - for session in snap_vx_sessions: - snap_name = session['snap_name'] - targets = session['target_vol_list'] - # Only unlink a set number of targets - if count == self.snapvx_unlink_limit: - break - is_temp = False - is_temp = 'temp' in snap_name or 'EMC_SMI' in snap_name - if utils.CLONE_SNAPSHOT_NAME in snap_name: - is_temp = False - for target in targets: - if snapvx_src: - if not is_temp and target[1] == "Copied": - # Break the replication relationship - LOG.debug("Unlinking source from " - "target. Source: %(volume)s, " - "Target: %(target)s.", - {'volume': session['source_vol'], - 'target': target[0]}) - self.provision.break_replication_relationship( - array, target[0], session['source_vol'], - snap_name, extra_specs, - session['generation']) - count = count + 1 - elif snapvx_tgt: - # If our device is a target, we need to wait - # and then unlink - self._break_relationship_snapvx_tgt( - session, target, is_temp, array, extra_specs) - - def _break_relationship_snapvx_tgt( - self, session, target, is_temp, array, extra_specs): - """Break relationship of the snapvx target and cleanup + def _delete_valid_snapshot(self, array, session, extra_specs): + """Delete a snapshot if valid candidate for deletion. + :param array: the array serial :param session: the snapvx session - :param target: the snapvx target - :param is_temp: is the snapshot temporary - :param array: the serialnumber of the array :param extra_specs: extra specifications """ - LOG.debug("Unlinking source from " - "target. Source: %(volume)s, " - "Target: %(target)s.", - {'volume': session['source_vol'], - 'target': target[0]}) - self.provision.break_replication_relationship( - array, target[0], session['source_vol'], session['snap_name'], - extra_specs, session['generation']) - # For older styled temp snapshots for clone - # do a delete as well - if is_temp: - self.provision.delete_temp_volume_snap( - array, session['snap_name'], session['source_vol'], - session['generation']) + is_legacy = 'EMC_SMI' in session['snap_name'] + is_temp = utils.CLONE_SNAPSHOT_NAME in session['snap_name'] + is_expired = session['expired'] + is_valid = True if is_legacy or (is_temp and is_expired) else False + if is_valid: + try: + self._unlink_targets_and_delete_temp_snapvx( + session, array, extra_specs) + except exception.VolumeBackendAPIException as e: + # Ignore and continue as snapshot has been unlinked + # successfully with incorrect status code returned + if ('404' and session['snap_name'] and + 'does not exist' in six.text_type(e)): + pass def manage_existing(self, volume, external_ref): """Manages an existing PowerMax/VMAX Volume (import to Cinder). diff --git a/cinder/volume/drivers/dell_emc/powermax/fc.py b/cinder/volume/drivers/dell_emc/powermax/fc.py index 46fde077597..85496046197 100644 --- a/cinder/volume/drivers/dell_emc/powermax/fc.py +++ b/cinder/volume/drivers/dell_emc/powermax/fc.py @@ -113,6 +113,7 @@ class PowerMaxFCDriver(san.SanDriver, driver.FibreChannelDriver): - PowerMax OS Metro formatted volumes fix (bug #1829876) - Support for Metro ODE (bp/powermax-metro-ode) - Removal of san_rest_port from PowerMax cinder.conf config + - SnapVX noCopy mode enabled for all links """ 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 4294e6de2a3..47cdc2c976c 100644 --- a/cinder/volume/drivers/dell_emc/powermax/iscsi.py +++ b/cinder/volume/drivers/dell_emc/powermax/iscsi.py @@ -118,6 +118,7 @@ class PowerMaxISCSIDriver(san.SanISCSIDriver): - PowerMax OS Metro formatted volumes fix (bug #1829876) - Support for Metro ODE (bp/powermax-metro-ode) - Removal of san_rest_port from PowerMax cinder.conf config + - SnapVX noCopy mode enabled for all links """ VERSION = "4.1.0" diff --git a/cinder/volume/drivers/dell_emc/powermax/provision.py b/cinder/volume/drivers/dell_emc/powermax/provision.py index f877d296fbd..02c4b8c07a5 100644 --- a/cinder/volume/drivers/dell_emc/powermax/provision.py +++ b/cinder/volume/drivers/dell_emc/powermax/provision.py @@ -176,7 +176,7 @@ class PowerMaxProvision(object): def break_replication_relationship( self, array, target_device_id, source_device_id, snap_name, - extra_specs, generation=0): + extra_specs, generation=0, loop=True): """Unlink a snapshot from its target volume. :param array: the array serial number @@ -185,6 +185,7 @@ class PowerMaxProvision(object): :param snap_name: the name for the snap shot :param extra_specs: extra specifications :param generation: the generation number of the snapshot + :param loop: if looping call is required for handling retries """ @coordination.synchronized("emc-snapvx-{src_device_id}") def do_unlink_volume(src_device_id): @@ -194,13 +195,14 @@ class PowerMaxProvision(object): self._unlink_volume(array, src_device_id, target_device_id, snap_name, extra_specs, - list_volume_pairs=None, generation=generation) + list_volume_pairs=None, generation=generation, + loop=loop) do_unlink_volume(source_device_id) def _unlink_volume( self, array, source_device_id, target_device_id, snap_name, - extra_specs, list_volume_pairs=None, generation=0): + extra_specs, list_volume_pairs=None, generation=0, loop=True): """Unlink a target volume from its source volume. :param array: the array serial number @@ -210,6 +212,7 @@ class PowerMaxProvision(object): :param extra_specs: extra specifications :param list_volume_pairs: list of volume pairs, optional :param generation: the generation number of the snapshot + :param loop: if looping call is required for handling retries :return: return code """ def _unlink_vol(): @@ -237,11 +240,18 @@ class PowerMaxProvision(object): if kwargs['modify_vol_success']: raise loopingcall.LoopingCallDone() - kwargs = {'retries': 0, - 'modify_vol_success': False} - timer = loopingcall.FixedIntervalLoopingCall(_unlink_vol) - rc = timer.start(interval=UNLINK_INTERVAL).wait() - return rc + if not loop: + self.rest.modify_volume_snap( + array, source_device_id, target_device_id, snap_name, + extra_specs, unlink=True, + list_volume_pairs=list_volume_pairs, + generation=generation) + else: + kwargs = {'retries': 0, + 'modify_vol_success': False} + timer = loopingcall.FixedIntervalLoopingCall(_unlink_vol) + rc = timer.start(interval=UNLINK_INTERVAL).wait() + return rc def delete_volume_snap(self, array, snap_name, source_device_id, restored=False, generation=0): diff --git a/cinder/volume/drivers/dell_emc/powermax/rest.py b/cinder/volume/drivers/dell_emc/powermax/rest.py index 31d54e493f4..7b4438f60f0 100644 --- a/cinder/volume/drivers/dell_emc/powermax/rest.py +++ b/cinder/volume/drivers/dell_emc/powermax/rest.py @@ -1814,7 +1814,7 @@ class PowerMaxRest(object): tgt_list.append({'name': target_id}) payload = {"deviceNameListSource": src_list, "deviceNameListTarget": tgt_list, - "copy": 'true', "action": action, + "copy": 'false', "action": action, "star": 'false', "force": 'false', "exact": 'false', "remote": 'false', "symforce": 'false', "generation": generation} @@ -2094,46 +2094,48 @@ class PowerMaxRest(object): :param tgt_only: Flag - return only sessions where device is target :returns: list of snapshot dicts """ - snap_dict_list, sessions = [], [] - vol_details = self._get_private_volume(array, device_id) - snap_vx_info = vol_details['timeFinderInfo'] - is_snap_src = snap_vx_info['snapVXSrc'] - is_snap_tgt = snap_vx_info['snapVXTgt'] - if snap_vx_info.get('snapVXSession'): - sessions = snap_vx_info['snapVXSession'] - if is_snap_src and not tgt_only: - for session in sessions: - if session.get('srcSnapshotGenInfo'): - src_list = session['srcSnapshotGenInfo'] - for src in src_list: - snap_name = src['snapshotHeader']['snapshotName'] - generation = src['snapshotHeader']['generation'] - target_list, target_dict_list = [], [] - if src.get('lnkSnapshotGenInfo'): - target_dict_list = src['lnkSnapshotGenInfo'] - for tgt in target_dict_list: - target_tup = tgt['targetDevice'], tgt['state'] - target_list.append(target_tup) - link_info = {'target_vol_list': target_list, - 'snap_name': snap_name, - 'source_vol': device_id, - 'generation': generation} - snap_dict_list.append(link_info) - if is_snap_tgt: - for session in sessions: + snap_tgt_dict, snap_src_dict_list = dict(), list() + s_in = self.get_volume_snap_info(array, device_id) + + snap_src = ( + s_in['snapshotSrcs'] if s_in.get('snapshotSrcs') else list()) + snap_tgt = ( + s_in['snapshotLnks'][0] if s_in.get('snapshotLnks') else dict()) + + if snap_src and not tgt_only: + for session in snap_src: + snap_src_dict = dict() + + snap_src_dict['source_vol_id'] = device_id + snap_src_dict['generation'] = session['generation'] + snap_src_dict['snap_name'] = session['snapshotName'] + snap_src_dict['expired'] = session['expired'] + + if session.get('linkedDevices'): + snap_src_link = session['linkedDevices'][0] + snap_src_dict['target_vol_id'] = snap_src_link[ + 'targetDevice'] + snap_src_dict['copy_mode'] = snap_src_link['copy'] + snap_src_dict['state'] = snap_src_link['state'] + + snap_src_dict_list.append(snap_src_dict) + + if snap_tgt: + snap_tgt_dict['source_vol_id'] = snap_tgt['linkSourceName'] + snap_tgt_dict['target_vol_id'] = device_id + snap_tgt_dict['state'] = snap_tgt['state'] + snap_tgt_dict['copy_mode'] = snap_tgt['copy'] + + vol_info = self._get_private_volume(array, device_id) + vol_tf_sessions = vol_info['timeFinderInfo']['snapVXSession'] + for session in vol_tf_sessions: if session.get('tgtSrcSnapshotGenInfo'): - tgt = session['tgtSrcSnapshotGenInfo'] - snap_name = tgt['snapshotName'] - target_tup = tgt['targetDevice'], tgt['state'] - target_list = [target_tup] - source_vol = tgt['sourceDevice'] - generation = tgt['generation'] - link_info = {'target_vol_list': target_list, - 'snap_name': snap_name, - 'source_vol': source_vol, - 'generation': generation} - snap_dict_list.append(link_info) - return snap_dict_list + snap_tgt_link = session.get('tgtSrcSnapshotGenInfo') + snap_tgt_dict['snap_name'] = snap_tgt_link['snapshotName'] + snap_tgt_dict['expired'] = snap_tgt_link['expired'] + snap_tgt_dict['generation'] = snap_tgt_link['generation'] + + return snap_src_dict_list, snap_tgt_dict def get_rdf_group(self, array, rdf_number): """Get specific rdf group details. diff --git a/releasenotes/notes/powermax-snapvx-link-mode-0050ac6b4a16c739.yaml b/releasenotes/notes/powermax-snapvx-link-mode-0050ac6b4a16c739.yaml new file mode 100644 index 00000000000..bd118eee57b --- /dev/null +++ b/releasenotes/notes/powermax-snapvx-link-mode-0050ac6b4a16c739.yaml @@ -0,0 +1,7 @@ +--- +other: + - | + The PowerMax for Cinder driver now implements noCopy mode for links between + SnapVX source and target. This change will improve space efficiency by + using pointers instead of copied tracks when source and target volumes are + linked.