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

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}]

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)

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)
+                extra_specs, generation=6, loop=True)
-            mock_modify.assert_called_once_with(
+            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):

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,

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',
+        ref_sessions = [{'generation': 0,
-                         'snap_name': 'temp-1',
+                         'snap_name': 'temp-000AA-snapshot_for_clone',
-                         'source_vol': self.data.device_id,
+                         'source_vol_id': self.data.device_id,
-                         'target_vol_list':
+                         'target_vol_id': self.data.device_id2,
-                             [(self.data.device_id2, 'Copied')]},
+                         'expired': False, 'copy_mode': True,
-                        {'generation': '0',
+                         'state': 'Copied'},
-                         'snap_name': 'temp-1',
+                        {'generation': 1,
-                         'source_vol': self.data.device_id,
+                         'snap_name': 'temp-000AA-snapshot_for_clone',
-                         'target_vol_list':
+                         'source_vol_id': self.data.device_id,
-                             [(self.data.device_id2, 'Copied')]}]
+                         'target_vol_id': self.data.device_id3,
-        sessions = self.rest.find_snap_vx_sessions(array, source_id)
+                         'expired': False, 'copy_mode': True,
-        self.assertEqual(ref_sessions, sessions)
+                         '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',
+        ref_session = {'generation': 6, 'state': 'Linked', 'copy_mode': False,
-                         'snap_name': 'temp-1',
+                       'snap_name': 'temp-000AA-snapshot_for_clone',
-                         'source_vol': self.data.device_id,
+                       'source_vol_id': self.data.device_id2,
-                         'target_vol_list':
+                       'target_vol_id': source_id, 'expired': True}
-                             [(self.data.device_id2, 'Copied')]}]
+
-        sessions = self.rest.find_snap_vx_sessions(
+        __, 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,

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}")
+            @coordination.synchronized("emc-source-{src_device_id}")
-            def do_unlink_and_delete_snap(source_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 if snapvx_tgt or snapvx_src else False
-            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.")
 
-                        LOG.warning(exception_message)
+        if get_sessions:
-                        raise exception.VolumeBackendAPIException(
+            src_sessions, tgt_session = self.rest.find_snap_vx_sessions(
-                            exception_message)
+                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:
+        expired = session['expired']
-            LOG.debug("Unlinking source from target. Source: "
+
-                      "%(volume)s, Target: %(target)s, "
+        target, cm_enabled = None, False
-                      "generation: %(generation)s.",
+        if session.get('target_vol_id'):
-                      {'volume': source, 'target': target[0],
+            target = session['target_vol_id']
-                       'generation': generation})
+            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,
+                array, target, source, snap_name, extra_specs, generation,
-                extra_specs, generation)
+                loop)
-        # The snapshot name will only have 'temp' (or EMC_SMI for
+
-        # legacy volumes) if it is a temporary volume.
+        # Candidates for deletion:
-        # Only then is it a candidate for deletion.
+        # 1. If legacy snapshot with 'EMC_SMI' in snapshot name
-        if 'temp' in snap_name or 'EMC_SMI' in snap_name:
+        # 2. If snapVX snapshot with copy mode enabled
-            LOG.debug("Deleting temporary snapshot. Source: "
+        # 3. If snapVX snapshot with copy mode disabled and not expired
-                      "%(volume)s, snap name: %(snap_name)s, "
+        if ('EMC_SMI' in snap_name or cm_enabled or (
-                      "generation: %(generation)s.",
+                not cm_enabled and not expired)):
-                      {'volume': source, 'snap_name': snap_name,
+            LOG.debug(
-                       'generation': generation})
+                "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(
+    def _get_target_source_device(self, array, device_id):
-            self, array, device_id, tgt_only=False):
         """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.",
+        LOG.debug("Getting the source device ID for target device %(tgt)s",
-                  {'target': device_id})
+                  {'tgt': device_id})
-        get_sessions = False
         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
+            __, tgt_session = self.rest.find_snap_vx_sessions(
-        elif snapvx_src and not tgt_only:
+                array, device_id, tgt_only=True)
-            get_sessions = True
+            source_device_id = tgt_session['source_vol_id']
-        if get_sessions:
+            LOG.debug("Target %(tgt)s source device %(src)s",
-            snap_vx_sessions = self.rest.find_snap_vx_sessions(
+                      {'target': device_id, 'src': source_device_id})
-                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
         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:
+                count = 0
-                    snap_vx_sessions.sort(
+                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(
+                    for session in src_sessions:
-                        snap_vx_sessions, snapvx_tgt, src_device_id, array,
+                        if count < self.snapvx_unlink_limit:
-                        extra_specs)
+                            self._delete_valid_snapshot(array, session,
+                                                        extra_specs)
+                            count += 1
+                        else:
+                            break
 
             do_unlink_and_delete_snap(device_id)
 
-    def _break_relationship(
+    def _delete_valid_snapshot(self, array, session, extra_specs):
-            self, snap_vx_sessions, snapvx_tgt, snapvx_src, array,
+        """Delete a snapshot if valid candidate for deletion.
-            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
 
+        :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 "
+        is_legacy = 'EMC_SMI' in session['snap_name']
-                  "target. Source: %(volume)s, "
+        is_temp = utils.CLONE_SNAPSHOT_NAME in session['snap_name']
-                  "Target: %(target)s.",
+        is_expired = session['expired']
-                  {'volume': session['source_vol'],
+        is_valid = True if is_legacy or (is_temp and is_expired) else False
-                   'target': target[0]})
+        if is_valid:
-        self.provision.break_replication_relationship(
+            try:
-            array, target[0], session['source_vol'], session['snap_name'],
+                self._unlink_targets_and_delete_temp_snapvx(
-            extra_specs, session['generation'])
+                    session, array, extra_specs)
-        # For older styled temp snapshots for clone
+            except exception.VolumeBackendAPIException as e:
-        # do a delete as well
+                # Ignore and continue as snapshot has been unlinked
-        if is_temp:
+                # successfully with incorrect status code returned
-            self.provision.delete_temp_volume_snap(
+                if ('404' and session['snap_name'] and
-                array, session['snap_name'], session['source_vol'],
+                        'does not exist' in six.text_type(e)):
-                session['generation'])
+                    pass
 
     def manage_existing(self, volume, external_ref):
         """Manages an existing PowerMax/VMAX Volume (import to Cinder).

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"

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"

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,
+        if not loop:
-                  'modify_vol_success': False}
+            self.rest.modify_volume_snap(
-        timer = loopingcall.FixedIntervalLoopingCall(_unlink_vol)
+                array, source_device_id, target_device_id, snap_name,
-        rc = timer.start(interval=UNLINK_INTERVAL).wait()
+                extra_specs, unlink=True,
-        return rc
+                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):

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 = [], []
+        snap_tgt_dict, snap_src_dict_list = dict(), list()
-        vol_details = self._get_private_volume(array, device_id)
+        s_in = self.get_volume_snap_info(array, device_id)
-        snap_vx_info = vol_details['timeFinderInfo']
+
-        is_snap_src = snap_vx_info['snapVXSrc']
+        snap_src = (
-        is_snap_tgt = snap_vx_info['snapVXTgt']
+            s_in['snapshotSrcs'] if s_in.get('snapshotSrcs') else list())
-        if snap_vx_info.get('snapVXSession'):
+        snap_tgt = (
-            sessions = snap_vx_info['snapVXSession']
+            s_in['snapshotLnks'][0] if s_in.get('snapshotLnks') else dict())
-        if is_snap_src and not tgt_only:
+
-            for session in sessions:
+        if snap_src and not tgt_only:
-                if session.get('srcSnapshotGenInfo'):
+            for session in snap_src:
-                    src_list = session['srcSnapshotGenInfo']
+                snap_src_dict = dict()
-                    for src in src_list:
+
-                        snap_name = src['snapshotHeader']['snapshotName']
+                snap_src_dict['source_vol_id'] = device_id
-                        generation = src['snapshotHeader']['generation']
+                snap_src_dict['generation'] = session['generation']
-                        target_list, target_dict_list = [], []
+                snap_src_dict['snap_name'] = session['snapshotName']
-                        if src.get('lnkSnapshotGenInfo'):
+                snap_src_dict['expired'] = session['expired']
-                            target_dict_list = src['lnkSnapshotGenInfo']
+
-                        for tgt in target_dict_list:
+                if session.get('linkedDevices'):
-                            target_tup = tgt['targetDevice'], tgt['state']
+                    snap_src_link = session['linkedDevices'][0]
-                            target_list.append(target_tup)
+                    snap_src_dict['target_vol_id'] = snap_src_link[
-                        link_info = {'target_vol_list': target_list,
+                        'targetDevice']
-                                     'snap_name': snap_name,
+                    snap_src_dict['copy_mode'] = snap_src_link['copy']
-                                     'source_vol': device_id,
+                    snap_src_dict['state'] = snap_src_link['state']
-                                     'generation': generation}
+
-                        snap_dict_list.append(link_info)
+                snap_src_dict_list.append(snap_src_dict)
-        if is_snap_tgt:
+
-            for session in sessions:
+        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_tgt_link = session.get('tgtSrcSnapshotGenInfo')
-                    snap_name = tgt['snapshotName']
+                    snap_tgt_dict['snap_name'] = snap_tgt_link['snapshotName']
-                    target_tup = tgt['targetDevice'], tgt['state']
+                    snap_tgt_dict['expired'] = snap_tgt_link['expired']
-                    target_list = [target_tup]
+                    snap_tgt_dict['generation'] = snap_tgt_link['generation']
-                    source_vol = tgt['sourceDevice']
+
-                    generation = tgt['generation']
+        return snap_src_dict_list, snap_tgt_dict
-                    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
 
     def get_rdf_group(self, array, rdf_number):
         """Get specific rdf group details.

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.