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 365052a9163..bd854accc4f 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 @@ -93,6 +93,8 @@ class PowerMaxData(object): next_gen_ucode = 5978 gvg_group_id = 'test-gvg' sg_tags = 'production,test' + snap_id = 118749976833 + snap_id_2 = 118749976833 # connector info wwpn1 = '123456789012345' @@ -870,7 +872,7 @@ class PowerMaxData(object): {'snapshotHeader': { 'snapshotName': 'temp-1', 'device': device_id, - 'generation': '0'}, + 'snapid': snap_id}, 'lnkSnapshotGenInfo': [ {'targetDevice': device_id2, 'state': 'Copied'}]}]}, @@ -878,7 +880,7 @@ class PowerMaxData(object): 'snapshotName': 'temp-1', 'targetDevice': device_id2, 'sourceDevice': device_id, - 'generation': '0', + 'snapid': snap_id_2, 'state': 'Copied'}}], 'snapVXSrc': 'true', 'snapVXTgt': 'true'}, @@ -906,14 +908,15 @@ class PowerMaxData(object): # replication volume_snap_vx = {'snapshotLnks': [], 'snapshotSrcs': [ - {'generation': 0, + {'snap_id': snap_id, 'linkedDevices': [ {'targetDevice': device_id2, 'percentageCopied': 100, 'state': 'Copied', 'copy': True, 'defined': True, - 'linked': True}], + 'linked': True, + 'snap_id': snap_id}], 'snapshotName': test_snapshot_snap_name, 'state': 'Established'}]} capabilities = {'symmetrixCapability': [{'rdfCapable': True, @@ -1068,7 +1071,8 @@ class PowerMaxData(object): {'snapshotHeader': { 'timestamp': 1512763278000, 'expired': False, 'secured': False, 'snapshotName': 'testSnap1', - 'device': '00001', 'generation': 0, 'timeToLive': 0 + 'device': '00001', 'snapid': snap_id, + 'timeToLive': 0, 'generation': 0 }}]}]}}] priv_vol_func_response_multi = [ @@ -1094,7 +1098,8 @@ class PowerMaxData(object): {'snapshotHeader': { 'timestamp': 1512763278000, 'expired': False, 'secured': False, 'snapshotName': 'testSnap1', - 'device': '00001', 'generation': 0, 'timeToLive': 0 + 'device': '00001', 'snapid': snap_id, + 'timeToLive': 0, 'generation': 0 }}]}]}}, {'volumeHeader': { 'private': False, 'capGB': 200.0, 'capMB': 204800.0, @@ -1118,7 +1123,8 @@ class PowerMaxData(object): {'snapshotHeader': { 'timestamp': 1512763278000, 'expired': False, 'secured': False, 'snapshotName': 'testSnap2', - 'device': '00002', 'generation': 0, 'timeToLive': 0 + 'device': '00002', 'snapid': snap_id, + 'timeToLive': 0, 'generation': 0 }}]}]}}, {'volumeHeader': { 'private': False, 'capGB': 300.0, 'capMB': 307200.0, @@ -1142,7 +1148,8 @@ class PowerMaxData(object): {'snapshotHeader': { 'timestamp': 1512763278000, 'expired': False, 'secured': False, 'snapshotName': 'testSnap3', - 'device': '00003', 'generation': 0, 'timeToLive': 0 + 'device': '00003', 'snapid': snap_id, + 'timeToLive': 0, 'generation': 0 }}]}]}}, {'volumeHeader': { 'private': False, 'capGB': 400.0, 'capMB': 409600.0, @@ -1166,7 +1173,8 @@ class PowerMaxData(object): {'snapshotHeader': { 'timestamp': 1512763278000, 'expired': False, 'secured': False, 'snapshotName': 'testSnap4', - 'device': '00004', 'generation': 0, 'timeToLive': 0 + 'device': '00004', 'snapid': snap_id, + 'timeToLive': 0, 'generation': 0 }}]}]}}] priv_vol_func_response_multi_invalid = [ @@ -1340,19 +1348,21 @@ class PowerMaxData(object): 'RestServerPort': '8443', 'RestUserName': 'test', 'RestPassword': 'test', - 'SSLVerify': '/path/to/cert'}, + 'SSLVerify': '/path/to/cert', + 'SerialNumber': array}, {'RestServerIp': '10.10.10.12', 'RestServerPort': '8443', 'RestUserName': 'test', 'RestPassword': 'test', - 'SSLVerify': 'True'}] + 'SSLVerify': 'True', + 'SerialNumber': array}] snapshot_src_details = {'snapshotSrcs': [{ 'snapshotName': 'temp-000AA-snapshot_for_clone', - 'generation': 0, 'state': 'Established', 'expired': False, + 'snap_id': snap_id, 'state': 'Established', 'expired': False, 'linkedDevices': [{'targetDevice': device_id2, 'state': 'Copied', 'copy': True}]}, - {'snapshotName': 'temp-000AA-snapshot_for_clone', 'generation': 1, + {'snapshotName': 'temp-000AA-snapshot_for_clone', 'snap_id': snap_id_2, 'state': 'Established', 'expired': False, 'linkedDevices': [{'targetDevice': device_id3, 'state': 'Copied', 'copy': True}]}], @@ -1363,24 +1373,24 @@ class PowerMaxData(object): snap_tgt_vol_details = {"timeFinderInfo": {"snapVXSession": [{ "tgtSrcSnapshotGenInfo": { - "generation": 6, "expired": True, + "snapid": snap_id, "expired": True, "snapshotName": "temp-000AA-snapshot_for_clone"}}]}} snap_tgt_session = { - 'generation': 0, 'expired': False, 'copy_mode': False, + 'snapid': snap_id, '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, + 'snapid': snap_id, '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, + {'snapid': snap_id, '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, + {'snapid': snap_id_2, 'expired': False, 'copy_mode': False, 'snap_name': 'temp-000AA-snapshot_for_clone', 'state': 'Copied', 'source_vol_id': device_id, 'target_vol_id': device_id4}] @@ -1449,7 +1459,7 @@ class PowerMaxData(object): priv_snap_response = { 'deviceName': snap_device_label, 'snapshotLnks': [], 'snapshotSrcs': [ - {'generation': 0, + {'snap_id': snap_id, 'linkedDevices': [ {'targetDevice': device_id2, 'percentageCopied': 100, 'state': 'Copied', 'copy': True, 'defined': True, @@ -1620,3 +1630,7 @@ class PowerMaxData(object): legacy_mvs = [legacy_mv1, legacy_mv2] legacy_not_shared_mv = 'OS-myhostA-SRP_1-Diamond-NONE-MV' legacy_not_shared_sg = 'OS-myhostA-SRP_1-Diamond-NONE-SG' + snapshot_metadata = {'SnapshotLabel': test_snapshot_snap_name, + 'SourceDeviceID': device_id, + 'SourceDeviceLabel': device_label, + 'SnapIdList': [snap_id]} diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_fake_objects.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_fake_objects.py index a02ca6264d2..5f5eca0dbe4 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_fake_objects.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_fake_objects.py @@ -229,7 +229,7 @@ class FakeRequestsSession(object): def _replication_sg(self, url): return_object = None - if 'generation' in url: + if 'snapid' in url: return_object = self.data.group_snap_vx elif 'rdf_group' in url: for sg in self.data.sg_rdf_details: 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 569d3b5154c..ac862bc0d9c 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 @@ -64,6 +64,7 @@ class PowerMaxCommonTest(test.TestCase): self.utils = self.common.utils self.utils.get_volumetype_extra_specs = ( mock.Mock(return_value=self.data.vol_type_extra_specs)) + self.rest.is_snap_id = True @mock.patch.object(rest.PowerMaxRest, 'get_array_ucode_version', return_value=tpd.PowerMaxData.next_gen_ucode) @@ -396,21 +397,25 @@ class PowerMaxCommonTest(test.TestCase): snapshot, self.data.test_volume) self.assertEqual(ref_model_update, model_update) - def test_delete_snapshot(self): + @mock.patch.object( + common.PowerMaxCommon, '_parse_snap_info', + return_value=(tpd.PowerMaxData.device_id, + tpd.PowerMaxData.snap_location['snap_name'], + [tpd.PowerMaxData.snap_id])) + def test_delete_snapshot(self, mock_parse): snap_name = self.data.snap_location['snap_name'] sourcedevice_id = self.data.snap_location['source_id'] - generation = 0 with mock.patch.object( self.provision, 'delete_volume_snap') as mock_delete_snap: self.common.delete_snapshot( self.data.test_snapshot, self.data.test_volume) mock_delete_snap.assert_called_once_with( self.data.array, snap_name, [sourcedevice_id], - restored=False, generation=generation) + self.data.snap_id, restored=False) def test_delete_snapshot_not_found(self): with mock.patch.object(self.common, '_parse_snap_info', - return_value=(None, 'Something')): + return_value=(None, 'Something', None)): with mock.patch.object( self.provision, 'delete_volume_snap') as mock_delete_snap: self.common.delete_snapshot(self.data.test_snapshot, @@ -1144,7 +1149,7 @@ class PowerMaxCommonTest(test.TestCase): def test_parse_snap_info_found(self): ref_device_id = self.data.device_id ref_snap_name = self.data.snap_location['snap_name'] - sourcedevice_id, foundsnap_name = self.common._parse_snap_info( + sourcedevice_id, foundsnap_name, __ = self.common._parse_snap_info( self.data.array, self.data.test_snapshot) self.assertEqual(ref_device_id, sourcedevice_id) self.assertEqual(ref_snap_name, foundsnap_name) @@ -1153,22 +1158,22 @@ class PowerMaxCommonTest(test.TestCase): ref_snap_name = None with mock.patch.object(self.rest, 'get_volume_snap', return_value=None): - __, foundsnap_name = self.common._parse_snap_info( + __, foundsnap_name, __ = self.common._parse_snap_info( self.data.array, self.data.test_snapshot) self.assertIsNone(ref_snap_name, foundsnap_name) def test_parse_snap_info_exception(self): with mock.patch.object( - self.rest, 'get_volume_snap', + self.rest, 'get_volume_snaps', side_effect=exception.VolumeBackendAPIException): - __, foundsnap_name = self.common._parse_snap_info( + __, foundsnap_name, __ = self.common._parse_snap_info( self.data.array, self.data.test_snapshot) self.assertIsNone(foundsnap_name) def test_parse_snap_info_provider_location_not_string(self): snapshot = fake_snapshot.fake_snapshot_obj( context='ctxt', provider_loaction={'not': 'string'}) - sourcedevice_id, foundsnap_name = self.common._parse_snap_info( + sourcedevice_id, foundsnap_name, __ = self.common._parse_snap_info( self.data.array, snapshot) self.assertIsNone(foundsnap_name) @@ -1747,10 +1752,12 @@ class PowerMaxCommonTest(test.TestCase): clone_name, snap_name, self.data.extra_specs, target_volume=clone_volume) + @mock.patch.object(rest.PowerMaxRest, 'get_snap_id', + return_value=tpd.PowerMaxData.snap_id) @mock.patch.object( masking.PowerMaxMasking, 'remove_and_reset_members') - def test_cleanup_target_sync_present(self, mock_remove): + def test_cleanup_target_sync_present(self, mock_remove, mock_snaps): array = self.data.array clone_volume = self.data.test_clone_volume source_device_id = self.data.device_id @@ -1758,7 +1765,6 @@ class PowerMaxCommonTest(test.TestCase): snap_name = self.data.failed_resource clone_name = clone_volume.name extra_specs = self.data.extra_specs - generation = 0 with mock.patch.object(self.rest, 'get_sync_session', return_value='session'): with mock.patch.object( @@ -1768,10 +1774,13 @@ class PowerMaxCommonTest(test.TestCase): clone_name, snap_name, extra_specs) mock_break.assert_called_with( array, target_device_id, source_device_id, - snap_name, extra_specs, generation) + snap_name, extra_specs, self.data.snap_id) + @mock.patch.object(rest.PowerMaxRest, 'get_volume_snaps', + return_value=[{'snap_name': 'snap_name', + 'snap_id': tpd.PowerMaxData.snap_id}]) @mock.patch.object(masking.PowerMaxMasking, 'remove_volume_from_sg') - def test_cleanup_target_no_sync(self, mock_remove): + def test_cleanup_target_no_sync(self, mock_remove, mock_snaps): array = self.data.array clone_volume = self.data.test_clone_volume source_device_id = self.data.device_id @@ -2646,8 +2655,9 @@ class PowerMaxCommonTest(test.TestCase): self.data.test_snapshot) self.assertEqual(2, size) - @mock.patch.object(rest.PowerMaxRest, 'get_volume_snap', - return_value={'snap_name': 'snap_name'}) + @mock.patch.object(rest.PowerMaxRest, 'get_volume_snaps', + return_value=[{'snap_name': 'snap_name', + 'snap_id': tpd.PowerMaxData.snap_id}]) @mock.patch.object( common.PowerMaxCommon, 'get_snapshot_metadata', return_value={'snap-meta-key-1': 'snap-meta-value-1', @@ -2732,29 +2742,35 @@ class PowerMaxCommonTest(test.TestCase): self.common.unmanage_snapshot, self.data.test_snapshot_manage) + @mock.patch.object( + common.PowerMaxCommon, '_parse_snap_info', return_value=( + tpd.PowerMaxData.device_id, + tpd.PowerMaxData.snap_location['snap_name'], + [tpd.PowerMaxData.snap_id])) @mock.patch.object(provision.PowerMaxProvision, 'delete_volume_snap') @mock.patch.object(provision.PowerMaxProvision, 'is_restore_complete', return_value=True) @mock.patch.object(common.PowerMaxCommon, '_clone_check') @mock.patch.object(provision.PowerMaxProvision, 'revert_volume_snapshot') def test_revert_to_snapshot(self, mock_revert, mock_clone, - mock_complete, mock_delete): + mock_complete, mock_delete, mock_parse): volume = self.data.test_volume snapshot = self.data.test_snapshot array = self.data.array device_id = self.data.device_id snap_name = self.data.snap_location['snap_name'] + snap_id = self.data.snap_id extra_specs = deepcopy(self.data.extra_specs_intervals_set) extra_specs['storagetype:portgroupname'] = ( self.data.port_group_name_f) self.common.revert_to_snapshot(volume, snapshot) mock_revert.assert_called_once_with( - array, device_id, snap_name, extra_specs) + array, device_id, snap_name, snap_id, extra_specs) mock_clone.assert_called_once_with(array, device_id, extra_specs) mock_complete.assert_called_once_with(array, device_id, - snap_name, extra_specs) + snap_name, snap_id, extra_specs) mock_delete.assert_called_once_with(array, snap_name, device_id, - restored=True, generation=0) + self.data.snap_id, restored=True) @mock.patch.object(utils.PowerMaxUtils, 'is_replication_enabled', return_value=True) @@ -2837,9 +2853,9 @@ class PowerMaxCommonTest(test.TestCase): 'reason_not_safe': None, 'cinder_id': None, 'extra_info': { 'generation': 0, 'secured': False, 'timeToLive': 'N/A', - 'timestamp': mock.ANY}, + 'timestamp': mock.ANY, 'snap_id': self.data.snap_id}, 'source_reference': {'source-id': '00001'}}] - self.assertEqual(snap_list, expected_response) + self.assertEqual(expected_response, snap_list) def test_get_manageable_snapshots_filters_set(self): marker, limit, offset = 'testSnap2', 2, 1 @@ -2853,14 +2869,16 @@ class PowerMaxCommonTest(test.TestCase): {'reference': {'source-name': 'testSnap3'}, 'safe_to_manage': True, 'size': 300, 'reason_not_safe': None, 'cinder_id': None, 'extra_info': { - 'generation': 0, 'secured': False, 'timeToLive': 'N/A', - 'timestamp': mock.ANY}, + 'snap_id': self.data.snap_id, 'secured': False, + 'timeToLive': 'N/A', 'timestamp': mock.ANY, + 'generation': 0}, 'source_reference': {'source-id': '00003'}}, {'reference': {'source-name': 'testSnap4'}, 'safe_to_manage': True, 'size': 400, 'reason_not_safe': None, 'cinder_id': None, 'extra_info': { - 'generation': 0, 'secured': False, 'timeToLive': 'N/A', - 'timestamp': mock.ANY}, + 'snap_id': self.data.snap_id, 'secured': False, + 'timeToLive': 'N/A', 'timestamp': mock.ANY, + 'generation': 0}, 'source_reference': {'source-id': '00004'}}] self.assertEqual(vols_lists, expected_response) @@ -3203,14 +3221,14 @@ class PowerMaxCommonTest(test.TestCase): session = self.data.snap_tgt_session_cm_enabled snap_name = session['snap_name'] source = session['source_vol_id'] - generation = session['generation'] + snap_id = session['snapid'] 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) + extra_specs, snap_id, True) + mck_del.assert_called_once_with(array, snap_name, source, snap_id) mck_break.reset_mock() mck_del.reset_mock() @@ -3220,7 +3238,7 @@ class PowerMaxCommonTest(test.TestCase): 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) + extra_specs, snap_id, False) mck_del.assert_not_called() @mock.patch.object(rest.PowerMaxRest, 'find_snap_vx_sessions', @@ -3385,7 +3403,9 @@ class PowerMaxCommonTest(test.TestCase): snap_name = self.data.test_snapshot_snap_name ref_metadata = {'SnapshotLabel': snap_name, 'SourceDeviceID': device_id, - 'SourceDeviceLabel': device_label} + 'SourceDeviceLabel': device_label, + 'SnapIdList': six.text_type(self.data.snap_id), + 'is_snap_id': True} act_metadata = self.common.get_snapshot_metadata( array, device_id, snap_name) diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_metadata.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_metadata.py index fc79a1d2245..42a7d65da11 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_metadata.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_metadata.py @@ -151,7 +151,7 @@ class PowerMaxVolumeMetadataDebugTest(test.TestCase): def test_capture_snapshot_info(self, mock_uvim): self.volume_metadata.capture_snapshot_info( self.data.test_volume, self.data.extra_specs, 'createSnapshot', - 'ss-test-vol') + self.data.snapshot_metadata) mock_uvim.assert_called_once() @mock.patch.object( 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 b3a4f4493ed..c94bfaf7838 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 @@ -165,11 +165,11 @@ class PowerMaxProvisionTest(test.TestCase): self.provision, '_unlink_volume') as mock_unlink: self.provision.unlink_snapvx_tgt_volume( array, target_device_id, source_device_id, snap_name, - extra_specs, generation=6, loop=True) + extra_specs, self.data.snap_id, loop=True) mock_unlink.assert_called_once_with( array, source_device_id, target_device_id, - snap_name, extra_specs, list_volume_pairs=None, - generation=6, loop=True) + snap_name, extra_specs, snap_id=self.data.snap_id, + list_volume_pairs=None, loop=True) @mock.patch('oslo_service.loopingcall.FixedIntervalLoopingCall', new=test_utils.ZeroIntervalLoopingCall) @@ -177,21 +177,22 @@ class PowerMaxProvisionTest(test.TestCase): with mock.patch.object(self.rest, 'modify_volume_snap') as mock_mod: 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) + self.data.snap_location['snap_name'], self.data.extra_specs, + snap_id=self.data.snap_id) 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) + snap_id=self.data.snap_id, list_volume_pairs=None, unlink=True) 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) + snap_id=self.data.snap_id, 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) + snap_id=self.data.snap_id, list_volume_pairs=None, unlink=True) @mock.patch('oslo_service.loopingcall.FixedIntervalLoopingCall', new=test_utils.ZeroIntervalLoopingCall) @@ -202,31 +203,33 @@ class PowerMaxProvisionTest(test.TestCase): ) as mock_mod: 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) + self.data.snap_location['snap_name'], self.data.extra_specs, + self.data.snap_id) self.assertEqual(2, mock_mod.call_count) def test_delete_volume_snap(self): array = self.data.array source_device_id = self.data.device_id snap_name = self.data.snap_location['snap_name'] - generation = 0 with mock.patch.object(self.provision.rest, 'delete_volume_snap'): self.provision.delete_volume_snap( - array, snap_name, source_device_id) + array, snap_name, source_device_id, snap_id=self.data.snap_id) self.provision.rest.delete_volume_snap.assert_called_once_with( - array, snap_name, source_device_id, False, generation) + array, snap_name, source_device_id, snap_id=self.data.snap_id, + restored=False) def test_delete_volume_snap_restore(self): array = self.data.array source_device_id = self.data.device_id snap_name = self.data.snap_location['snap_name'] restored = True - generation = 0 with mock.patch.object(self.provision.rest, 'delete_volume_snap'): self.provision.delete_volume_snap( - array, snap_name, source_device_id, restored) + array, snap_name, source_device_id, snap_id=self.data.snap_id, + restored=restored) self.provision.rest.delete_volume_snap.assert_called_once_with( - array, snap_name, source_device_id, True, generation) + array, snap_name, source_device_id, snap_id=self.data.snap_id, + restored=True) @mock.patch('oslo_service.loopingcall.FixedIntervalLoopingCall', new=test_utils.ZeroIntervalLoopingCall) @@ -234,37 +237,40 @@ class PowerMaxProvisionTest(test.TestCase): array = self.data.array source_device_id = self.data.device_id snap_name = self.data.snap_location['snap_name'] + snap_id = self.data.snap_id extra_specs = self.data.extra_specs with mock.patch.object( self.provision, '_is_restore_complete', return_value=True): isrestored = self.provision.is_restore_complete( - array, source_device_id, snap_name, extra_specs) + array, source_device_id, snap_name, snap_id, extra_specs) self.assertTrue(isrestored) with mock.patch.object( self.provision, '_is_restore_complete', side_effect=exception.CinderException): self.assertRaises(exception.VolumeBackendAPIException, self.provision.is_restore_complete, - array, source_device_id, snap_name, extra_specs) + array, source_device_id, snap_name, snap_id, + extra_specs) def test_is_restore_complete(self): array = self.data.array source_device_id = self.data.device_id snap_name = self.data.snap_location['snap_name'] + snap_id = self.data.snap_id snap_details = { 'linkedDevices': [{'targetDevice': source_device_id, 'state': 'Restored'}]} with mock.patch.object(self.provision.rest, 'get_volume_snap', return_value=snap_details): isrestored = self.provision._is_restore_complete( - array, source_device_id, snap_name) + array, source_device_id, snap_name, snap_id) self.assertTrue(isrestored) snap_details['linkedDevices'][0]['state'] = 'Restoring' with mock.patch.object(self.provision.rest, 'get_volume_snap', return_value=snap_details): isrestored = self.provision._is_restore_complete( - array, source_device_id, snap_name) + array, source_device_id, snap_name, snap_id) self.assertFalse(isrestored) def test_revert_volume_snapshot(self): @@ -272,13 +278,14 @@ class PowerMaxProvisionTest(test.TestCase): source_device_id = self.data.device_id snap_name = self.data.snap_location['snap_name'] extra_specs = self.data.extra_specs + snap_id = self.data.snap_id with mock.patch.object( self.provision.rest, 'modify_volume_snap', return_value=None): self.provision.revert_volume_snapshot( - array, source_device_id, snap_name, extra_specs) + array, source_device_id, snap_name, snap_id, extra_specs) self.provision.rest.modify_volume_snap.assert_called_once_with( array, source_device_id, "", snap_name, - extra_specs, restore=True) + extra_specs, snap_id=snap_id, restore=True) def test_extend_volume(self): array = self.data.array @@ -459,9 +466,14 @@ class PowerMaxProvisionTest(test.TestCase): array, snap_name, source_group_name) @mock.patch.object(rest.PowerMaxRest, - 'get_storagegroup_snap_generation_list', - side_effect=[['0', '3', '1', '2'], - ['0', '1'], ['0'], list()]) + 'get_storage_group_snap_id_list', + side_effect=[[tpd.PowerMaxData.snap_id, + tpd.PowerMaxData.snap_id_2, + tpd.PowerMaxData.snap_id, + tpd.PowerMaxData.snap_id_2], + [tpd.PowerMaxData.snap_id, + tpd.PowerMaxData.snap_id_2], + [tpd.PowerMaxData.snap_id], list()]) def test_delete_group_replica_side_effect(self, mock_list): array = self.data.array snap_name = self.data.group_snapshot_name @@ -532,14 +544,14 @@ class PowerMaxProvisionTest(test.TestCase): def test_delete_volume_snap_check_for_links(self, mock_unlink, mock_tgts): self.provision.delete_volume_snap_check_for_links( self.data.array, self.data.test_snapshot_snap_name, - self.data.device_id, self.data.extra_specs) + self.data.device_id, self.data.extra_specs, self.data.snap_id) mock_unlink.assert_called_once_with( self.data.array, "", "", self.data.test_snapshot_snap_name, - self.data.extra_specs, list_volume_pairs=[ - (self.data.device_id, tpd.PowerMaxData.device_id2)], - generation=0) + self.data.extra_specs, snap_id=self.data.snap_id, + list_volume_pairs=[ + (self.data.device_id, tpd.PowerMaxData.device_id2)]) mock_unlink.reset_mock() self.provision.delete_volume_snap_check_for_links( self.data.array, self.data.test_snapshot_snap_name, - self.data.device_id, self.data.extra_specs) + self.data.device_id, self.data.extra_specs, self.data.snap_id) self.assertEqual(2, mock_unlink.call_count) 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 e88885be897..fa8684728da 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 @@ -47,6 +47,7 @@ class PowerMaxRestTest(test.TestCase): self.driver = driver self.common = self.driver.common self.rest = self.common.rest + self.rest.is_snap_id = True self.utils = self.common.utils def test_rest_request_no_response(self): @@ -725,19 +726,15 @@ class PowerMaxRestTest(test.TestCase): def test_delete_volume(self): device_id = self.data.device_id - ucode_5978_foxtail = tpd.PowerMaxData.ucode_5978_foxtail + self.rest.ucode_major_level = utils.UCODE_5978 + self.rest.ucode_minor_level = utils.UCODE_5978_HICKORY with mock.patch.object( self.rest, 'delete_resource') as mock_delete, ( mock.patch.object( - self.rest, '_modify_volume')) as mock_modify, ( - mock.patch.object( - self.rest, 'get_array_detail', - return_value=ucode_5978_foxtail))as mock_det: + self.rest, '_modify_volume')) as mock_modify: self.rest.delete_volume(self.data.array, device_id) - detail_call_count = mock_det.call_count mod_call_count = mock_modify.call_count - self.assertEqual(1, detail_call_count) self.assertEqual(1, mod_call_count) mock_delete.assert_called_once_with( self.data.array, 'sloprovisioning', 'volume', device_id) @@ -1243,58 +1240,62 @@ class PowerMaxRestTest(test.TestCase): 'copy': 'false', 'action': "", 'star': 'false', 'force': 'false', 'exact': 'false', 'remote': 'false', - 'symforce': 'false', 'generation': 0} + 'symforce': 'false', 'snap_id': self.data.snap_id} payload_restore = {'deviceNameListSource': [{'name': source_id}], 'deviceNameListTarget': [{'name': source_id}], 'action': 'Restore', - 'star': 'false', 'force': 'false'} + 'star': 'false', 'force': 'false', + 'snap_id': self.data.snap_id} with mock.patch.object( self.rest, 'modify_resource', return_value=(202, self.data.job_list[0])) as mock_modify: # link payload['action'] = 'Link' self.rest.modify_volume_snap( - array, source_id, target_id, snap_name, extra_specs, link=True) + array, source_id, target_id, snap_name, extra_specs, + self.data.snap_id, link=True) mock_modify.assert_called_once_with( array, 'replication', 'snapshot', payload, resource_name=snap_name, private='/private') # unlink mock_modify.reset_mock() payload['action'] = 'Unlink' - self.rest.modify_volume_snap(array, source_id, target_id, - snap_name, extra_specs, unlink=True) + self.rest.modify_volume_snap( + array, source_id, target_id, snap_name, extra_specs, + self.data.snap_id, unlink=True) mock_modify.assert_called_once_with( array, 'replication', 'snapshot', payload, resource_name=snap_name, private='/private') # restore mock_modify.reset_mock() payload['action'] = 'Restore' - self.rest.modify_volume_snap(array, source_id, "", snap_name, - extra_specs, unlink=False, - restore=True) + self.rest.modify_volume_snap( + array, source_id, "", snap_name, extra_specs, + self.data.snap_id, unlink=False, restore=True) mock_modify.assert_called_once_with( array, 'replication', 'snapshot', payload_restore, resource_name=snap_name, private='/private') # link or unlink, list of volumes mock_modify.reset_mock() payload['action'] = 'Link' - self.rest.modify_volume_snap(array, "", "", snap_name, extra_specs, - unlink=False, link=True, - list_volume_pairs=[(source_id, + self.rest.modify_volume_snap( + array, "", "", snap_name, extra_specs, self.data.snap_id, + unlink=False, link=True, list_volume_pairs=[(source_id, target_id)]) mock_modify.assert_called_once_with( array, 'replication', 'snapshot', payload, resource_name=snap_name, private='/private') # none selected mock_modify.reset_mock() - self.rest.modify_volume_snap(array, source_id, target_id, - snap_name, extra_specs) + self.rest.modify_volume_snap( + array, source_id, target_id, snap_name, extra_specs, + self.data.snap_id) mock_modify.assert_not_called() # copy mode is True payload['copy'] = 'true' self.rest.modify_volume_snap( - array, source_id, target_id, snap_name, extra_specs, link=True, - copy=True) + array, source_id, target_id, snap_name, extra_specs, + self.data.snap_id, link=True, copy=True) mock_modify.assert_called_once_with( array, 'replication', 'snapshot', payload, resource_name=snap_name, private='/private') @@ -1304,11 +1305,10 @@ class PowerMaxRestTest(test.TestCase): snap_name = self.data.volume_snap_vx['snapshotSrcs'][0]['snapshotName'] source_device_id = self.data.device_id payload = {'deviceNameListSource': [{'name': source_device_id}], - 'generation': 0} - generation = 0 + 'snap_id': self.data.snap_id} with mock.patch.object(self.rest, 'delete_resource') as mock_delete: self.rest.delete_volume_snap( - array, snap_name, source_device_id, generation) + array, snap_name, source_device_id, self.data.snap_id) mock_delete.assert_called_once_with( array, 'replication', 'snapshot', snap_name, payload=payload, private='/private') @@ -1318,10 +1318,11 @@ class PowerMaxRestTest(test.TestCase): snap_name = self.data.volume_snap_vx['snapshotSrcs'][0]['snapshotName'] source_device_id = self.data.device_id payload = {'deviceNameListSource': [{'name': source_device_id}], - 'restore': True, 'generation': 0} + 'restore': True, 'snap_id': self.data.snap_id} with mock.patch.object(self.rest, 'delete_resource') as mock_delete: self.rest.delete_volume_snap( - array, snap_name, source_device_id, restored=True) + array, snap_name, source_device_id, self.data.snap_id, + restored=True) mock_delete.assert_called_once_with( array, 'replication', 'snapshot', snap_name, payload=payload, private='/private') @@ -1335,23 +1336,27 @@ class PowerMaxRestTest(test.TestCase): def test_get_volume_snap(self): array = self.data.array + snap_id = self.data.snap_id snap_name = self.data.volume_snap_vx['snapshotSrcs'][0]['snapshotName'] device_id = self.data.device_id ref_snap = self.data.volume_snap_vx['snapshotSrcs'][0] - snap = self.rest.get_volume_snap(array, device_id, snap_name) + snap = self.rest.get_volume_snap(array, device_id, snap_name, snap_id) self.assertEqual(ref_snap, snap) def test_get_volume_snap_none(self): array = self.data.array + snap_id = self.data.snap_id snap_name = self.data.volume_snap_vx['snapshotSrcs'][0]['snapshotName'] device_id = self.data.device_id with mock.patch.object(self.rest, 'get_volume_snap_info', return_value=None): - snap = self.rest.get_volume_snap(array, device_id, snap_name) + snap = self.rest.get_volume_snap( + array, device_id, snap_name, snap_id) self.assertIsNone(snap) with mock.patch.object(self.rest, 'get_volume_snap_info', return_value={'snapshotSrcs': []}): - snap = self.rest.get_volume_snap(array, device_id, snap_name) + snap = self.rest.get_volume_snap( + array, device_id, snap_name, snap_id) self.assertIsNone(snap) def test_get_snap_linked_device_dict_list(self): @@ -1360,8 +1365,8 @@ class PowerMaxRestTest(test.TestCase): device_id = self.data.device_id snap_list = [{'linked_vols': [ {'target_device': device_id, 'state': 'Copied'}], - 'snap_name': snap_name, 'generation': '0'}] - ref_snap_list = [{'generation': '0', 'linked_vols': [ + 'snap_name': snap_name, 'snapid': self.data.snap_id}] + ref_snap_list = [{'snapid': self.data.snap_id, 'linked_vols': [ {'state': 'Copied', 'target_device': '00001'}]}] with mock.patch.object(self.rest, '_find_snap_vx_source_sessions', return_value=snap_list): @@ -1372,26 +1377,25 @@ class PowerMaxRestTest(test.TestCase): def test_get_sync_session(self): array = self.data.array source_id = self.data.device_id - generation = 0 target_id = self.data.volume_snap_vx[ 'snapshotSrcs'][0]['linkedDevices'][0]['targetDevice'] snap_name = self.data.volume_snap_vx['snapshotSrcs'][0]['snapshotName'] ref_sync = self.data.volume_snap_vx[ 'snapshotSrcs'][0]['linkedDevices'][0] sync = self.rest.get_sync_session( - array, source_id, snap_name, target_id, generation) + array, source_id, snap_name, target_id, self.data.snap_id) self.assertEqual(ref_sync, sync) def test_find_snap_vx_sessions(self): array = self.data.array source_id = self.data.device_id - ref_sessions = [{'generation': 0, + ref_sessions = [{'snapid': self.data.snap_id, '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, + {'snapid': self.data.snap_id_2, 'snap_name': 'temp-000AA-snapshot_for_clone', 'source_vol_id': self.data.device_id, 'target_vol_id': self.data.device_id3, @@ -1411,7 +1415,8 @@ class PowerMaxRestTest(test.TestCase): def test_find_snap_vx_sessions_tgt_only(self, mck_snap, mck_vol): array = self.data.array source_id = self.data.device_id - ref_session = {'generation': 6, 'state': 'Linked', 'copy_mode': False, + ref_session = {'snapid': self.data.snap_id, '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} @@ -1592,14 +1597,15 @@ class PowerMaxRestTest(test.TestCase): array, source_group, snap_name, '0') @mock.patch.object(rest.PowerMaxRest, 'get_resource', - return_value={'generations': ['0', '1']}) - def test_get_storagegroup_snap_generation_list(self, mock_list): + return_value={'snapids': [tpd.PowerMaxData.snap_id, + tpd.PowerMaxData.snap_id_2]}) + def test_get_storagegroup_snap_id_list(self, mock_list): array = self.data.array source_group = self.data.storagegroup_name_source snap_name = self.data.group_snapshot_name - ret_list = self.rest.get_storagegroup_snap_generation_list( + ret_list = self.rest.get_storage_group_snap_id_list( array, source_group, snap_name) - self.assertEqual(['0', '1'], ret_list) + self.assertEqual([self.data.snap_id, self.data.snap_id_2], ret_list) def test_get_storagegroup_rdf_details(self): details = self.rest.get_storagegroup_rdf_details( @@ -1650,8 +1656,8 @@ class PowerMaxRestTest(test.TestCase): new_snap_backend_name = self.data.managed_snap_id self.rest.modify_volume_snap( array, source_id, source_id, old_snap_backend_name, - self.data.extra_specs, link=False, unlink=False, - rename=True, new_snap_name=new_snap_backend_name) + self.data.extra_specs, self.data.snap_id, link=False, + unlink=False, rename=True, new_snap_name=new_snap_backend_name) mock_modify.assert_called_once() def test_get_private_volume_list_pass(self): @@ -1733,6 +1739,7 @@ class PowerMaxRestTest(test.TestCase): 'RestServerPort': '8443', 'RestUserName': 'user_test', 'RestPassword': 'pass_test', + 'SerialNumber': self.data.array, 'SSLVerify': True, } self.rest.set_rest_credentials(array_info) diff --git a/cinder/volume/drivers/dell_emc/powermax/common.py b/cinder/volume/drivers/dell_emc/powermax/common.py index ccf0f9393e3..abd452894c8 100644 --- a/cinder/volume/drivers/dell_emc/powermax/common.py +++ b/cinder/volume/drivers/dell_emc/powermax/common.py @@ -710,15 +710,17 @@ class PowerMaxCommon(object): model_update = { 'provider_location': six.text_type(snapshot_dict)} + snapshot_metadata = self.get_snapshot_metadata( + extra_specs.get('array'), snapshot_dict.get('source_id'), + snapshot_dict.get('snap_name')) model_update = self.update_metadata( - model_update, snapshot.metadata, self.get_snapshot_metadata( - extra_specs['array'], snapshot_dict['source_id'], - snapshot_dict['snap_name'])) + model_update, snapshot.metadata, snapshot_metadata) if snapshot.metadata: model_update['metadata'].update(snapshot.metadata) - + snapshot_metadata.update( + {'snap_display_name': snapshot_dict.get('snap_name')}) self.volume_metadata.capture_snapshot_info( - volume, extra_specs, 'createSnapshot', snapshot_dict['snap_name']) + volume, extra_specs, 'createSnapshot', snapshot_metadata) return model_update @@ -731,7 +733,7 @@ class PowerMaxCommon(object): LOG.info("Delete Snapshot: %(snapshotName)s.", {'snapshotName': snapshot.name}) extra_specs = self._initial_setup(volume) - sourcedevice_id, snap_name = self._parse_snap_info( + sourcedevice_id, snap_name, snap_id_list = self._parse_snap_info( extra_specs[utils.ARRAY], snapshot) if not sourcedevice_id and not snap_name: # Check if legacy snapshot @@ -745,9 +747,10 @@ class PowerMaxCommon(object): LOG.info("No snapshot found on the array") else: # Ensure snap has not been recently deleted - self.provision.delete_volume_snap_check_for_links( - extra_specs[utils.ARRAY], snap_name, - sourcedevice_id, extra_specs) + for snap_id in snap_id_list: + self.provision.delete_volume_snap_check_for_links( + extra_specs[utils.ARRAY], snap_name, + sourcedevice_id, extra_specs, snap_id) LOG.info("Leaving delete_snapshot: %(ssname)s.", {'ssname': snap_name}) @@ -1909,7 +1912,7 @@ class PowerMaxCommon(object): message=exception_message) if from_snapvx: - source_device_id, snap_name = self._parse_snap_info( + source_device_id, snap_name, __ = self._parse_snap_info( array, source_volume) else: source_device_id = self._find_device_on_array( @@ -1946,10 +1949,13 @@ class PowerMaxCommon(object): :param array: the array serial number :param snapshot: the snapshot object - :returns: sourcedevice_id, foundsnap_name + :returns: sourcedevice_id -- str + foundsnap_name -- str + found_snap_id_list -- list """ foundsnap_name = None sourcedevice_id = None + found_snap_id_list = list() volume_name = snapshot.id loc = snapshot.provider_location @@ -1962,19 +1968,21 @@ class PowerMaxCommon(object): except KeyError: LOG.info("Error retrieving snapshot details. Assuming " "legacy structure of snapshot...") - return None, None - # Ensure snapvx is on the array. + return None, None, None try: - snap_details = self.rest.get_volume_snap( + snap_detail_list = self.rest.get_volume_snaps( array, sourcedevice_id, snap_name) - if snap_details: + for snap_details in snap_detail_list: foundsnap_name = snap_name + found_snap_id_list.append(snap_details.get( + 'snap_id') if self.rest.is_snap_id else ( + snap_details.get('generation'))) except Exception as e: LOG.info("Exception in retrieving snapshot: %(e)s.", {'e': e}) foundsnap_name = None - if foundsnap_name is None or sourcedevice_id is None: + if not foundsnap_name or not sourcedevice_id or not found_snap_id_list: LOG.debug("Error retrieving snapshot details. " "Snapshot name: %(snap)s", {'snap': volume_name}) @@ -1982,9 +1990,10 @@ class PowerMaxCommon(object): LOG.debug("Source volume: %(volume_name)s Snap name: " "%(foundsnap_name)s.", {'volume_name': sourcedevice_id, - 'foundsnap_name': foundsnap_name}) + 'foundsnap_name': foundsnap_name, + 'snap_ids': found_snap_id_list}) - return sourcedevice_id, foundsnap_name + return sourcedevice_id, foundsnap_name, found_snap_id_list def _create_snapshot(self, array, snapshot, source_device_id, extra_specs): @@ -2859,24 +2868,24 @@ class PowerMaxCommon(object): def _cleanup_target( self, array, target_device_id, source_device_id, - clone_name, snap_name, extra_specs, generation=0, - target_volume=None): + clone_name, snap_name, extra_specs, target_volume=None): """Cleanup target volume on failed clone/ snapshot creation. :param array: the array serial number :param target_device_id: the target device ID :param source_device_id: the source device ID :param clone_name: the name of the clone volume + :param snap_name: the snapVX name :param extra_specs: the extra specifications - :param generation: the generation number of the snapshot :param target_volume: the target volume object """ + snap_id = self.rest.get_snap_id(array, source_device_id, snap_name) snap_session = self.rest.get_sync_session( - array, source_device_id, snap_name, target_device_id, generation) + array, source_device_id, snap_name, target_device_id, snap_id) if snap_session: self.provision.unlink_snapvx_tgt_volume( array, target_device_id, source_device_id, - snap_name, extra_specs, generation) + snap_name, extra_specs, snap_id) self._remove_vol_and_cleanup_replication( array, target_device_id, clone_name, extra_specs, target_volume) self._delete_from_srp( @@ -2937,7 +2946,8 @@ class PowerMaxCommon(object): 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) + if not self.rest.is_snap_id: + src_sessions.sort(key=lambda k: k['snapid'], reverse=True) for session in src_sessions: self._unlink_targets_and_delete_temp_snapvx( session, array, extra_specs) @@ -2952,7 +2962,7 @@ class PowerMaxCommon(object): """ snap_name = session.get('snap_name') source = session.get('source_vol_id') - generation = session.get('generation') + snap_id = session.get('snapid') expired = session.get('expired') target, cm_enabled = None, False @@ -2964,24 +2974,24 @@ class PowerMaxCommon(object): 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}) + "%(tgt)s, Snap id: %(snapid)s.", {'vol': source, 'tgt': target, + 'snapid': snap_id}) self.provision.unlink_snapvx_tgt_volume( - array, target, source, snap_name, extra_specs, generation, + array, target, source, snap_name, extra_specs, snap_id, 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 (snap_name and 'EMC_SMI' in snap_name or cm_enabled or ( + 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}) + "%(name)s, snap id: %(snapid)s.", { + 'vol': source, 'name': snap_name, 'snapid': snap_id}) self.provision.delete_temp_volume_snap( - array, snap_name, source, generation) + array, snap_name, source, snap_id) def _get_target_source_device(self, array, device_id): """Get the source device id of the target. @@ -3027,8 +3037,6 @@ class PowerMaxCommon(object): array, tgt_session, extra_specs, force_unlink) count += 1 if src_sessions: - src_sessions.sort( - key=lambda k: k['generation'], reverse=True) for session in src_sessions: if count < self.snapvx_unlink_limit: self._delete_valid_snapshot( @@ -3406,26 +3414,15 @@ class PowerMaxCommon(object): LOG.error(exception_message) raise exception.VolumeBackendAPIException( message=exception_message) - - if not self.rest.get_volume_snap(array, device_id, snap_name): - exception_message = ( - _("Snapshot %(snap_name)s is not associated with specified " - "volume %(device_id)s, it is not possible to manage a " - "snapshot that is not associated with the specified " - "volume.") - % {'device_id': device_id, 'snap_name': snap_name}) - LOG.error(exception_message) - raise exception.VolumeBackendAPIException( - message=exception_message) - + snap_id = self.rest.get_snap_id(array, device_id, snap_name) snap_backend_name = self.utils.modify_snapshot_prefix( snap_name, manage=True) try: self.rest.modify_volume_snap( array, device_id, device_id, snap_name, - extra_specs, rename=True, new_snap_name=snap_backend_name) - + extra_specs, snap_id=snap_id, rename=True, + new_snap_name=snap_backend_name) except Exception as e: exception_message = ( _("There was an issue managing %(snap_name)s, it was not " @@ -3439,15 +3436,19 @@ class PowerMaxCommon(object): model_update = { 'display_name': snap_display_name, 'provider_location': six.text_type(prov_loc)} + snapshot_metadata = self.get_snapshot_metadata( + array, device_id, snap_backend_name) model_update = self.update_metadata( - model_update, snapshot.metadata, self.get_snapshot_metadata( - array, device_id, snap_backend_name)) + model_update, snapshot.metadata, snapshot_metadata) LOG.info("Managing SnapVX Snapshot %(snap_name)s of source " "volume %(device_id)s, OpenStack Snapshot display name: " "%(snap_display_name)s", { 'snap_name': snap_name, 'device_id': device_id, 'snap_display_name': snap_display_name}) + snapshot_metadata.update({'snap_display_name': snap_display_name}) + self.volume_metadata.capture_snapshot_info( + volume, extra_specs, 'manageSnapshot', snapshot_metadata) return model_update @@ -3474,7 +3475,17 @@ class PowerMaxCommon(object): volume = snapshot.volume extra_specs = self._initial_setup(volume) array = extra_specs[utils.ARRAY] - device_id, snap_name = self._parse_snap_info(array, snapshot) + device_id, snap_name, snap_id_list = self._parse_snap_info( + array, snapshot) + + if len(snap_id_list) != 1: + exception_message = (_( + "It is not possible to unmanage snapshot because there " + "are either multiple or no snapshots associated with " + "%(snap_name)s.") % {'snap_name': snap_name}) + LOG.error(exception_message) + raise exception.VolumeBackendAPIException( + message=exception_message) if self.utils.is_volume_failed_over(volume): exception_message = ( @@ -3493,7 +3504,8 @@ class PowerMaxCommon(object): try: self.rest.modify_volume_snap( array, device_id, device_id, snap_name, extra_specs, - rename=True, new_snap_name=new_snap_backend_name) + snap_id=snap_id_list[0], rename=True, + new_snap_name=new_snap_backend_name) except Exception as e: exception_message = ( _("There was an issue unmanaging Snapshot, it " @@ -3646,7 +3658,8 @@ class PowerMaxCommon(object): if not volumes: LOG.info("There were no volumes found on the backend " "PowerMax/VMAX. You need to create some volumes " - "before snashot can be created and managed into Cinder.") + "before a snapshot can be created and managed into " + "Cinder.") return manageable_snaps for device in volumes: @@ -3757,8 +3770,9 @@ class PowerMaxCommon(object): math.ceil(device['volumeHeader']['capGB'])), 'reason_not_safe': None, 'cinder_id': None, 'extra_info': { - 'generation': snap_info['generation'], - 'secured': snap_info['secured'], + 'generation': snap_info.get('generation'), + 'snap_id': snap_info.get('snapid'), + 'secured': snap_info.get('secured'), 'timeToLive': human_ttl_timestamp, 'timestamp': human_timestamp}, 'source_reference': {'source-id': snap_info['device']}} @@ -5559,7 +5573,7 @@ class PowerMaxCommon(object): """ src_dev_ids = [] for snap in snapshots: - src_dev_id, snap_name = self._parse_snap_info(array, snap) + src_dev_id, snap_name, __ = self._parse_snap_info(array, snap) if snap_name: src_dev_ids.append(src_dev_id) return src_dev_ids @@ -5908,7 +5922,7 @@ class PowerMaxCommon(object): if not source_vols: for snap in snapshots: if snap.id == volume.snapshot_id: - src_dev_id, __ = self._parse_snap_info( + src_dev_id, __, __ = self._parse_snap_info( extra_specs[utils.ARRAY], snap) vol_size = snap.volume_size else: @@ -6311,30 +6325,41 @@ class PowerMaxCommon(object): LOG.error(exception_message) raise exception.VolumeDriverException(message=exception_message) array = extra_specs[utils.ARRAY] - sourcedevice_id, snap_name = self._parse_snap_info( + sourcedevice_id, snap_name, snap_id_list = self._parse_snap_info( array, snapshot) if not sourcedevice_id or not snap_name: LOG.error("No snapshot found on the array") exception_message = (_( "Failed to revert the volume to the snapshot")) raise exception.VolumeDriverException(message=exception_message) + if len(snap_id_list) != 1: + exception_message = (_( + "It is not possible to revert snapshot because there are " + "either multiple or no snapshots associated with " + "%(snap_name)s.") % {'snap_name': snap_name}) + LOG.error(exception_message) + raise exception.VolumeBackendAPIException( + message=exception_message) + else: + snap_id = snap_id_list[0] self._clone_check(array, sourcedevice_id, extra_specs) try: LOG.info("Reverting device: %(deviceid)s " "to snapshot: %(snapname)s.", {'deviceid': sourcedevice_id, 'snapname': snap_name}) self.provision.revert_volume_snapshot( - array, sourcedevice_id, snap_name, extra_specs) + array, sourcedevice_id, snap_name, snap_id, extra_specs) # Once the restore is done, we need to check if it is complete restore_complete = self.provision.is_restore_complete( - array, sourcedevice_id, snap_name, extra_specs) + array, sourcedevice_id, snap_name, snap_id, extra_specs) if not restore_complete: LOG.debug("Restore couldn't complete in the specified " "time interval. The terminate restore may fail") LOG.debug("Terminating restore session") # This may throw an exception if restore_complete is False self.provision.delete_volume_snap( - array, snap_name, sourcedevice_id, restored=True, generation=0) + array, snap_name, sourcedevice_id, snap_id, + restored=True) # Revert volume to snapshot is successful if termination was # successful - possible even if restore_complete was False # when we checked last. @@ -6449,12 +6474,22 @@ class PowerMaxCommon(object): :param snap_name: the snapshot name :returns: dict -- volume metadata """ + snap_id_list = list() snap_info = self.rest.get_volume_snap_info(array, device_id) - device_name = snap_info['deviceName'] + device_name = snap_info.get('deviceName') + snapshot_src_list = snap_info.get('snapshotSrcs') + for snapshot_src in snapshot_src_list: + if snap_name == snapshot_src.get('snapshotName'): + snap_id_list.append(snapshot_src.get( + 'snap_id') if self.rest.is_snap_id else snapshot_src.get( + 'generation')) device_label = device_name.split(':')[1] metadata = {'SnapshotLabel': snap_name, 'SourceDeviceID': device_id, - 'SourceDeviceLabel': device_label} + 'SourceDeviceLabel': device_label, + 'SnapIdList': ', '.join( + six.text_type(v) for v in snap_id_list), + 'is_snap_id': self.rest.is_snap_id} return metadata diff --git a/cinder/volume/drivers/dell_emc/powermax/fc.py b/cinder/volume/drivers/dell_emc/powermax/fc.py index ae45aa3114f..9aa3c73d14d 100644 --- a/cinder/volume/drivers/dell_emc/powermax/fc.py +++ b/cinder/volume/drivers/dell_emc/powermax/fc.py @@ -127,6 +127,7 @@ class PowerMaxFCDriver(san.SanDriver, driver.FibreChannelDriver): - Support for Port Group and Port load balancing (bp powermax-port-load-balance) - Fix to enable legacy volumes to live migrate (#1867163) + - Use of snap id instead of generation (bp powermax-snapset-ids) """ VERSION = "4.3.0" diff --git a/cinder/volume/drivers/dell_emc/powermax/iscsi.py b/cinder/volume/drivers/dell_emc/powermax/iscsi.py index 7fcf165f61a..5989bb08ae2 100644 --- a/cinder/volume/drivers/dell_emc/powermax/iscsi.py +++ b/cinder/volume/drivers/dell_emc/powermax/iscsi.py @@ -133,6 +133,7 @@ class PowerMaxISCSIDriver(san.SanISCSIDriver): - Support for Port Group and Port load balancing (bp powermax-port-load-balance) - Fix to enable legacy volumes to live migrate (#1867163) + - Use of snap id instead of generation (bp powermax-snapset-ids) """ VERSION = "4.3.0" diff --git a/cinder/volume/drivers/dell_emc/powermax/metadata.py b/cinder/volume/drivers/dell_emc/powermax/metadata.py index afbbde7aa2b..935b62d0a40 100644 --- a/cinder/volume/drivers/dell_emc/powermax/metadata.py +++ b/cinder/volume/drivers/dell_emc/powermax/metadata.py @@ -387,17 +387,28 @@ class PowerMaxVolumeMetadata(object): @debug_required def capture_snapshot_info( - self, source, extra_specs, successful_operation, last_ss_name): + self, source, extra_specs, successful_operation, + snapshot_metadata): """Captures snapshot info in volume metadata :param source: the source volume object :param extra_specs: extra specifications :param successful_operation: snapshot operation - :param last_ss_name: the last snapshot name + :param snapshot_metadata: snapshot metadata """ + last_ss_name, snapshot_label, source_device_id = None, None, None + source_device_label, snap_ids, is_snap_id = None, None, None if isinstance(source, volume.Volume): - if 'create' in successful_operation: + if 'create' or 'manage' in successful_operation: snapshot_count = six.text_type(len(source.snapshots)) + if snapshot_metadata: + last_ss_name = snapshot_metadata.get('snap_display_name') + snapshot_label = snapshot_metadata.get('SnapshotLabel') + source_device_id = snapshot_metadata.get('SourceDeviceID') + source_device_label = snapshot_metadata.get( + 'SourceDeviceLabel') + snap_ids = snapshot_metadata.get('SnapIdList') + is_snap_id = snapshot_metadata.get('is_snap_id') else: snapshot_count = six.text_type(len(source.snapshots) - 1) default_sg = ( @@ -414,7 +425,12 @@ class PowerMaxVolumeMetadata(object): self.utils.get_volume_element_name(source.id)), openstack_name=source.display_name, snapshot_count=snapshot_count, - last_ss_name=last_ss_name) + last_ss_name=last_ss_name, + snapshot_label=snapshot_label, + is_snap_id=is_snap_id, + snap_ids_or_gens=snap_ids, + source_device_id=source_device_id, + source_device_label=source_device_label) volume_metadata = self.update_volume_info_metadata( datadict, self.version_dict) self.print_pretty_table(volume_metadata) diff --git a/cinder/volume/drivers/dell_emc/powermax/provision.py b/cinder/volume/drivers/dell_emc/powermax/provision.py index 0547a6ebd3f..ab9ee415532 100644 --- a/cinder/volume/drivers/dell_emc/powermax/provision.py +++ b/cinder/volume/drivers/dell_emc/powermax/provision.py @@ -184,7 +184,7 @@ class PowerMaxProvision(object): def unlink_snapvx_tgt_volume( self, array, target_device_id, source_device_id, snap_name, - extra_specs, generation=0, loop=True): + extra_specs, snap_id, loop=True): """Unlink a snapshot from its target volume. :param array: the array serial number @@ -192,7 +192,7 @@ class PowerMaxProvision(object): :param target_device_id: target volume device id :param snap_name: the name for the snap shot :param extra_specs: extra specifications - :param generation: the generation number of the snapshot + :param snap_id: the unique snap id of the SnapVX :param loop: if looping call is required for handling retries """ @coordination.synchronized("emc-snapvx-{src_device_id}") @@ -202,15 +202,14 @@ class PowerMaxProvision(object): {'src': src_device_id, 'tgt': target_device_id}) self._unlink_volume(array, src_device_id, target_device_id, - snap_name, extra_specs, - list_volume_pairs=None, generation=generation, - loop=loop) + snap_name, extra_specs, snap_id=snap_id, + list_volume_pairs=None, 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, loop=True): + extra_specs, snap_id=None, list_volume_pairs=None, loop=True): """Unlink a target volume from its source volume. :param array: the array serial number @@ -218,8 +217,8 @@ class PowerMaxProvision(object): :param target_device_id: the target device id :param snap_name: the snap name :param extra_specs: extra specifications + :param snap_id: the unique snap id of the SnapVX :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 :returns: return code """ @@ -234,9 +233,8 @@ class PowerMaxProvision(object): if not kwargs['modify_vol_success']: 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) + extra_specs, snap_id=snap_id, unlink=True, + list_volume_pairs=list_volume_pairs) kwargs['modify_vol_success'] = True except exception.VolumeBackendAPIException: pass @@ -251,9 +249,8 @@ class PowerMaxProvision(object): 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) + extra_specs, snap_id=snap_id, unlink=True, + list_volume_pairs=list_volume_pairs) else: kwargs = {'retries': 0, 'modify_vol_success': False} @@ -262,31 +259,40 @@ class PowerMaxProvision(object): return rc def delete_volume_snap(self, array, snap_name, - source_device_id, restored=False, generation=0): + source_device_ids, snap_id=None, restored=False): """Delete a snapVx snapshot of a volume. :param array: the array serial number :param snap_name: the snapshot name - :param source_device_id: the source device id + :param source_device_ids: the source device ids + :param snap_id: the unique snap id of the SnapVX :param restored: Flag to indicate if restored session is being deleted - :param generation: the snapshot generation number """ @coordination.synchronized("emc-snapvx-{src_device_id}") def do_delete_volume_snap(src_device_id): - LOG.debug("Delete SnapVx: %(snap_name)s for volume %(vol)s.", - {'vol': src_device_id, 'snap_name': snap_name}) + LOG.debug("Delete SnapVx: %(snap_name)s for source %(src)s and " + "devices %(devs)s.", + {'snap_name': snap_name, 'src': src_device_id, + 'devs': source_device_ids}) self.rest.delete_volume_snap( - array, snap_name, src_device_id, restored, generation) + array, snap_name, source_device_ids, snap_id=snap_id, + restored=restored) - do_delete_volume_snap(source_device_id) + device_id = source_device_ids[0] if isinstance( + source_device_ids, list) else source_device_ids + + if snap_id is None: + snap_id = self.rest.get_snap_id(array, device_id, snap_name) + do_delete_volume_snap(device_id) def is_restore_complete(self, array, source_device_id, - snap_name, extra_specs): + snap_name, snap_id, extra_specs): """Check and wait for a restore to complete :param array: the array serial number :param source_device_id: source device id :param snap_name: snapshot name + :param snap_id: unique snap id :param extra_specs: extra specification :returns: bool """ @@ -302,7 +308,7 @@ class PowerMaxProvision(object): kwargs['retries'] = retries + 1 if not kwargs['wait_for_restore_called']: if self._is_restore_complete( - array, source_device_id, snap_name): + array, source_device_id, snap_name, snap_id): kwargs['wait_for_restore_called'] = True except Exception: exception_message = (_("Issue encountered waiting for " @@ -325,17 +331,19 @@ class PowerMaxProvision(object): rc = timer.start(interval=int(extra_specs[utils.INTERVAL])).wait() return rc - def _is_restore_complete(self, array, source_device_id, snap_name): + def _is_restore_complete( + self, array, source_device_id, snap_name, snap_id): """Helper function to check if restore is complete. :param array: the array serial number :param source_device_id: source device id :param snap_name: the snapshot name + :param snap_id: unique snap id :returns: restored -- bool """ restored = False snap_details = self.rest.get_volume_snap( - array, source_device_id, snap_name) + array, source_device_id, snap_name, snap_id) if snap_details: linked_devices = snap_details.get("linkedDevices", []) for linked_device in linked_devices: @@ -347,7 +355,7 @@ class PowerMaxProvision(object): return restored def delete_temp_volume_snap(self, array, snap_name, - source_device_id, generation=0): + source_device_id, snap_id): """Delete the temporary snapshot created for clone operations. There can be instances where the source and target both attempt to @@ -356,17 +364,17 @@ class PowerMaxProvision(object): :param array: the array serial number :param snap_name: the snapshot name :param source_device_id: the source device id - :param generation: the generation number for the snapshot + :param snap_id: the unique snap id of the SnapVX """ snapvx = self.rest.get_volume_snap( - array, source_device_id, snap_name, generation) + array, source_device_id, snap_name, snap_id) if snapvx: self.delete_volume_snap( - array, snap_name, source_device_id, - restored=False, generation=generation) + array, snap_name, source_device_id, snap_id=snap_id, + restored=False) def delete_volume_snap_check_for_links( - self, array, snap_name, source_devices, extra_specs, generation=0): + self, array, snap_name, source_devices, extra_specs, snap_id): """Check if a snap has any links before deletion. If a snapshot has any links, break the replication relationship @@ -375,7 +383,7 @@ class PowerMaxProvision(object): :param snap_name: the snapshot name :param source_devices: the source device ids :param extra_specs: the extra specifications - :param generation: the generation number for the snapshot + :param snap_id: the unique snap id of the SnapVX """ list_device_pairs = [] if not isinstance(source_devices, list): @@ -385,7 +393,7 @@ class PowerMaxProvision(object): "for volume %(vol)s.", {'vol': source_device, 'snap_name': snap_name}) linked_list = self.rest.get_snap_linked_device_list( - array, source_device, snap_name, generation) + array, source_device, snap_name, snap_id) if len(linked_list) == 1: target_device = linked_list[0]['targetDevice'] list_device_pairs.append((source_device, target_device)) @@ -394,15 +402,16 @@ class PowerMaxProvision(object): # If a single source volume has multiple targets, # we must unlink each target individually target_device = link['targetDevice'] - self._unlink_volume(array, source_device, target_device, - snap_name, extra_specs, generation) + self._unlink_volume( + array, source_device, target_device, snap_name, + extra_specs, snap_id=snap_id) if list_device_pairs: - self._unlink_volume(array, "", "", snap_name, extra_specs, - list_volume_pairs=list_device_pairs, - generation=generation) + self._unlink_volume( + array, "", "", snap_name, extra_specs, snap_id=snap_id, + list_volume_pairs=list_device_pairs) if source_devices: - self.delete_volume_snap(array, snap_name, source_devices, - restored=False, generation=generation) + self.delete_volume_snap( + array, snap_name, source_devices, snap_id, restored=False) def extend_volume(self, array, device_id, new_size, extra_specs, rdf_group=None): @@ -647,20 +656,22 @@ class PowerMaxProvision(object): LOG.debug("Deleting Snap Vx snapshot: source group: %(srcGroup)s " "snapshot: %(snap_name)s.", {'srcGroup': source_group_name, 'snap_name': snap_name}) - gen_list = self.rest.get_storagegroup_snap_generation_list( + snap_id_list = self.rest.get_storage_group_snap_id_list( array, source_group_name, snap_name) - if gen_list: - gen_list.sort(reverse=True) - for gen in gen_list: + if snap_id_list: + if not self.rest.is_snap_id: + snap_id_list.sort(reverse=True) + for snap_id in snap_id_list: self.rest.delete_storagegroup_snap( - array, source_group_name, snap_name, gen) + array, source_group_name, snap_name, snap_id) else: - LOG.debug("Unable to get generation number(s) for: %(srcGroup)s.", + LOG.debug("Unable to get snap ids for: %(srcGroup)s.", {'srcGroup': source_group_name}) def link_and_break_replica(self, array, source_group_name, target_group_name, snap_name, extra_specs, - list_volume_pairs, delete_snapshot=False): + list_volume_pairs, delete_snapshot=False, + snap_id=None): """Links a group snap and breaks the relationship. :param array: the array serial @@ -670,6 +681,7 @@ class PowerMaxProvision(object): :param extra_specs: extra specifications :param list_volume_pairs: the list of volume pairs :param delete_snapshot: delete snapshot flag + :param snap_id: the unique snapVx identifier """ LOG.debug("Linking Snap Vx snapshot: source group: %(srcGroup)s " "targetGroup: %(tgtGroup)s.", @@ -696,17 +708,36 @@ class PowerMaxProvision(object): self.delete_volume_snap(array, snap_name, source_devices) def revert_volume_snapshot(self, array, source_device_id, - snap_name, extra_specs): + snap_name, snap_id, extra_specs): """Revert a volume snapshot :param array: the array serial number :param source_device_id: device id of the source :param snap_name: snapvx snapshot name + :param snap_id: the unique snap identifier :param extra_specs: the extra specifications """ start_time = time.time() - self.rest.modify_volume_snap( - array, source_device_id, "", snap_name, extra_specs, restore=True) + try: + self.rest.modify_volume_snap( + array, source_device_id, "", snap_name, extra_specs, + snap_id=snap_id, restore=True) + except exception.VolumeBackendAPIException as ex: + if utils.REVERT_SS_EXC in ex: + exception_message = _( + "Link must be fully copied for this operation to proceed. " + "Please reset the volume state from error to available " + "and wait for awhile before attempting another " + "revert to snapshot operation. You may want to delete " + "the latest snapshot taken in this revert to snapshot " + "operation, as you will only be able to revert to the " + "last snapshot.") + else: + exception_message = (_( + "Revert to snapshot failed with exception " + "%(e)s.") % {'e': ex}) + raise exception.VolumeBackendAPIException( + message=exception_message) LOG.debug("Restore volume snapshot took: %(delta)s H:MM:SS.", {'delta': self.utils.get_time_delta(start_time, time.time())}) diff --git a/cinder/volume/drivers/dell_emc/powermax/rest.py b/cinder/volume/drivers/dell_emc/powermax/rest.py index 4ec223ba63b..986df42dbb4 100644 --- a/cinder/volume/drivers/dell_emc/powermax/rest.py +++ b/cinder/volume/drivers/dell_emc/powermax/rest.py @@ -81,6 +81,9 @@ class PowerMaxRest(object): self.u4p_failover_backoff_factor = 1 self.u4p_in_failover = False self.u4p_failover_lock = False + self.ucode_major_level = None + self.ucode_minor_level = None + self.is_snap_id = False def set_rest_credentials(self, array_info): """Given the array record set the rest server credentials. @@ -96,6 +99,9 @@ class PowerMaxRest(object): self.base_uri = ("https://%(ip_port)s/univmax/restapi" % { 'ip_port': ip_port}) self.session = self._establish_rest_session() + self.ucode_major_level, self.ucode_minor_level = ( + self.get_major_minor_ucode(array_info['SerialNumber'])) + self.is_snap_id = self._is_snapid_enabled() def set_u4p_failover_config(self, failover_info): """Set the environment failover Unisphere targets and configuration.. @@ -1522,18 +1528,9 @@ class PowerMaxRest(object): :param array: the array serial number :param device_id: volume device id """ - array_details = self.get_array_detail(array) - ucode_major_level = 0 - ucode_minor_level = 0 - if array_details: - split_ucode_level = array_details['ucode'].split('.') - ucode_level = [int(level) for level in split_ucode_level] - ucode_major_level = ucode_level[0] - ucode_minor_level = ucode_level[1] - - if ((ucode_major_level >= utils.UCODE_5978) - and (ucode_minor_level > utils.UCODE_5978_ELMSR)): + if ((self.ucode_major_level >= utils.UCODE_5978) + and (self.ucode_minor_level > utils.UCODE_5978_ELMSR)): # Use Rapid TDEV Deallocation to delete after ELMSR try: # Rename volume, removing the OS- @@ -2074,9 +2071,9 @@ class PowerMaxRest(object): job, extra_specs) def modify_volume_snap(self, array, source_id, target_id, snap_name, - extra_specs, link=False, unlink=False, + extra_specs, snap_id=None, link=False, unlink=False, rename=False, new_snap_name=None, restore=False, - list_volume_pairs=None, generation=0, copy=False): + list_volume_pairs=None, copy=False): """Modify a snapvx snapshot :param array: the array serial number @@ -2084,13 +2081,13 @@ class PowerMaxRest(object): :param target_id: the target device id :param snap_name: the snapshot name :param extra_specs: extra specifications + :param snap_id: the unique snap id of the snapVX :param link: Flag to indicate action = Link :param unlink: Flag to indicate action = Unlink :param rename: Flag to indicate action = Rename :param new_snap_name: Optional new snapshot name :param restore: Flag to indicate action = Restore :param list_volume_pairs: list of volume pairs to link, optional - :param generation: the generation number of the snapshot :param copy: If copy mode should be used for SnapVX target links """ action, operation, payload = '', '', {} @@ -2105,7 +2102,6 @@ class PowerMaxRest(object): elif restore: action = "Restore" - payload = {} if action == "Restore": operation = 'Restore snapVx snapshot' payload = {"deviceNameListSource": [{"name": source_id}], @@ -2127,14 +2123,18 @@ class PowerMaxRest(object): "copy": copy, "action": action, "star": 'false', "force": 'false', "exact": 'false', "remote": 'false', - "symforce": 'false', "generation": generation} - + "symforce": 'false'} elif action == "Rename": operation = 'Rename snapVx snapshot' payload = {"deviceNameListSource": [{"name": source_id}], "deviceNameListTarget": [{"name": source_id}], "action": action, "newsnapshotname": new_snap_name} - + if self.is_snap_id: + payload.update({"snap_id": snap_id}) if snap_id else ( + payload.update({"generation": "0"})) + else: + payload.update({"generation": snap_id}) if snap_id else ( + payload.update({"generation": "0"})) if action: status_code, job = self.modify_resource( array, REPLICATION, 'snapshot', payload, @@ -2142,22 +2142,28 @@ class PowerMaxRest(object): self.wait_for_job(operation, status_code, job, extra_specs) def delete_volume_snap(self, array, snap_name, - source_device_ids, restored=False, generation=0): + source_device_ids, snap_id=None, restored=False): """Delete the snapshot of a volume or volumes. :param array: the array serial number :param snap_name: the name of the snapshot :param source_device_ids: the source device ids + :param snap_id: the unique snap id of the snapVX :param restored: Flag to indicate terminate restore session - :param generation: the generation number of the snapshot """ device_list = [] if not isinstance(source_device_ids, list): source_device_ids = [source_device_ids] for dev in source_device_ids: device_list.append({"name": dev}) - payload = {"deviceNameListSource": device_list, - "generation": int(generation)} + payload = {"deviceNameListSource": device_list} + if self.is_snap_id: + payload.update({"snap_id": snap_id}) if snap_id else ( + payload.update({"generation": 0})) + else: + payload.update({"generation": snap_id}) if snap_id else ( + payload.update({"generation": 0})) + if restored: payload.update({"restore": True}) LOG.debug("The payload is %(payload)s.", @@ -2178,13 +2184,13 @@ class PowerMaxRest(object): return self.get_resource(array, REPLICATION, 'volume', resource_name, private='/private') - def get_volume_snap(self, array, device_id, snap_name, generation=0): + def get_volume_snap(self, array, device_id, snap_name, snap_id): """Given a volume snap info, retrieve the snapVx object. :param array: the array serial number :param device_id: the source volume device id :param snap_name: the name of the snapshot - :param generation: the generation number of the snapshot + :param snap_id: the unique snap id of the snapVX :returns: snapshot dict, or None """ snapshot = None @@ -2194,11 +2200,34 @@ class PowerMaxRest(object): bool(snap_info['snapshotSrcs'])): for snap in snap_info['snapshotSrcs']: if snap['snapshotName'] == snap_name: - if snap['generation'] == generation: - snapshot = snap - break + if self.is_snap_id: + if snap['snap_id'] == snap_id: + snapshot = snap + break + else: + if snap['generation'] == snap_id: + snapshot = snap + break return snapshot + def get_volume_snaps(self, array, device_id, snap_name): + """Given a volume snap info, retrieve the snapVx object. + + :param array: the array serial number + :param device_id: the source volume device id + :param snap_name: the name of the snapshot + :returns: snapshot dict, or None + """ + snapshots = list() + snap_info = self.get_volume_snap_info(array, device_id) + if snap_info: + if (snap_info.get('snapshotSrcs', None) and + bool(snap_info['snapshotSrcs'])): + for snap in snap_info['snapshotSrcs']: + if snap['snapshotName'] == snap_name: + snapshots.append(snap) + return snapshots + def get_volume_snapshot_list(self, array, source_device_id): """Get a list of snapshot details for a particular volume. @@ -2236,7 +2265,7 @@ class PowerMaxRest(object): return snapvx_tgt, snapvx_src, rdf_grp def is_sync_complete(self, array, source_device_id, - target_device_id, snap_name, extra_specs): + target_device_id, snap_name, extra_specs, snap_id): """Check if a sync session is complete. :param array: the array serial number @@ -2244,6 +2273,7 @@ class PowerMaxRest(object): :param target_device_id: target device id :param snap_name: snapshot name :param extra_specs: extra specifications + :param snap_id: the unique snap id of the SnapVX :returns: bool """ @@ -2259,7 +2289,7 @@ class PowerMaxRest(object): if not kwargs['wait_for_sync_called']: if self._is_sync_complete( array, source_device_id, snap_name, - target_device_id): + target_device_id, snap_id): kwargs['wait_for_sync_called'] = True except Exception: exception_message = (_("Issue encountered waiting for " @@ -2283,36 +2313,37 @@ class PowerMaxRest(object): return rc def _is_sync_complete(self, array, source_device_id, snap_name, - target_device_id): + target_device_id, snap_id): """Helper function to check if snapVx sync session is complete. :param array: the array serial number :param source_device_id: source device id :param snap_name: the snapshot name :param target_device_id: the target device id + :param snap_id: the unique snap id of the SnapVX :returns: defined -- bool """ defined = True session = self.get_sync_session( - array, source_device_id, snap_name, target_device_id) + array, source_device_id, snap_name, target_device_id, snap_id) if session: defined = session['defined'] return defined def get_sync_session(self, array, source_device_id, snap_name, - target_device_id, generation=0): + target_device_id, snap_id): """Get a particular sync session. :param array: the array serial number :param source_device_id: source device id :param snap_name: the snapshot name :param target_device_id: the target device id - :param generation: the generation number of the snapshot + :param snap_id: the unique snapid of the snapshot :returns: sync session -- dict, or None """ session = None linked_device_list = self.get_snap_linked_device_list( - array, source_device_id, snap_name, generation) + array, source_device_id, snap_name, snap_id) for target in linked_device_list: if target_device_id == target['targetDevice']: session = target @@ -2330,23 +2361,25 @@ class PowerMaxRest(object): snapshots = self.get_volume_snapshot_list(array, source_device_id) for snapshot in snapshots: try: + snap_id = snapshot['snap_id'] if self.is_snap_id else ( + snapshot['generation']) if bool(snapshot['linkedDevices']): link_info = {'linked_vols': snapshot['linkedDevices'], 'snap_name': snapshot['snapshotName'], - 'generation': snapshot['generation']} + 'snapid': snap_id} snap_dict_list.append(link_info) except KeyError: pass return snap_dict_list def get_snap_linked_device_list(self, array, source_device_id, - snap_name, generation=0, state=None): + snap_name, snap_id, state=None): """Get the list of linked devices for a particular snapVx snapshot. :param array: the array serial number :param source_device_id: source device id :param snap_name: the snapshot name - :param generation: the generation number of the snapshot + :param snap_id: the unique snapid of the snapshot :param state: filter for state of the link :returns: linked_device_list or empty list """ @@ -2355,25 +2388,24 @@ class PowerMaxRest(object): snap_dict_list = self._get_snap_linked_device_dict_list( array, source_device_id, snap_name, state=state) for snap_dict in snap_dict_list: - if generation == snap_dict['generation']: + if snap_id == snap_dict['snapid']: linked_device_list = snap_dict['linked_vols'] break return linked_device_list def _get_snap_linked_device_dict_list( self, array, source_device_id, snap_name, state=None): - """Get list of linked devices for all generations for a snapVx snapshot + """Get list of linked devices for all snap ids for a snapVx snapshot :param array: the array serial number :param source_device_id: source device id :param snap_name: the snapshot name :param state: filter for state of the link - :returns: list of dict of generations with linked devices + :returns: list of dict of snapids with linked devices """ snap_dict_list = [] snap_list = self._find_snap_vx_source_sessions( array, source_device_id) - snap_state = None for snap in snap_list: if snap['snap_name'] == snap_name: for linked_vol in snap['linked_vols']: @@ -2382,17 +2414,17 @@ class PowerMaxRest(object): # both snap_state and state are not None and are equal if not state or (snap_state and state and snap_state == state): - generation = snap['generation'] + snap_id = snap['snapid'] found = False for snap_dict in snap_dict_list: - if generation == snap_dict['generation']: + if snap_id == snap_dict['snapid']: snap_dict['linked_vols'].append( linked_vol) found = True break if not found: snap_dict_list.append( - {'generation': generation, + {'snapid': snap_id, 'linked_vols': [linked_vol]}) return snap_dict_list @@ -2406,27 +2438,27 @@ class PowerMaxRest(object): """ 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'] + snap_src_dict['snapid'] = session.get( + 'snap_id') if self.is_snap_id else session.get( + 'generation') + snap_src_dict['snap_name'] = session.get('snapshotName') + snap_src_dict['expired'] = session.get('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_link = session.get('linkedDevices')[0] + snap_src_dict['target_vol_id'] = snap_src_link.get( + 'targetDevice') + snap_src_dict['copy_mode'] = snap_src_link.get('copy') + snap_src_dict['state'] = snap_src_link.get('state') snap_src_dict_list.append(snap_src_dict) @@ -2449,8 +2481,9 @@ class PowerMaxRest(object): 'snapshotName') snap_tgt_dict['expired'] = snap_tgt_link.get( 'expired') - snap_tgt_dict['generation'] = snap_tgt_link.get( - 'generation') + snap_tgt_dict['snapid'] = snap_tgt_link.get( + 'snapid') if self.is_snap_id else ( + snap_tgt_link.get('generation')) return snap_src_dict_list, snap_tgt_dict @@ -3056,41 +3089,44 @@ class PowerMaxRest(object): job, extra_specs) def delete_storagegroup_snap(self, array, source_group, - snap_name, generation='0'): + snap_name, snap_id): """Delete a snapVx snapshot of a storage group. :param array: the array serial number :param source_group: the source group name :param snap_name: the name of the snapshot - :param generation: the generation number of the SnapVX + :param snap_id: the unique snap id of the SnapVX """ + postfix_uri = "/snapid/%s" % snap_id if self.is_snap_id else ( + "/generation/%s" % snap_id) + resource_name = ("%(sg_name)s/snapshot/%(snap_name)s" - "/generation/%(generation)s" + "%(postfix_uri)s" % {'sg_name': source_group, 'snap_name': snap_name, - 'generation': generation}) + 'postfix_uri': postfix_uri}) self.delete_resource( array, REPLICATION, 'storagegroup', resource_name=resource_name) - def get_storagegroup_snap_generation_list( + def get_storage_group_snap_id_list( self, array, source_group, snap_name): - """Get a snapshot and its generation count information for an sg. - - The most recent snapshot will have a gen number of 0. The oldest - snapshot will have a gen number = genCount - 1 (i.e. if there are 4 - generations of particular snapshot, the oldest will have a gen num of - 3). + """Get a snapshot and its snapid count information for an sg. :param array: name of the array -- str :param source_group: name of the storage group -- str :param snap_name: the name of the snapshot -- str - :returns: generation numbers -- list + :returns: snapids -- list """ - resource_name = ("%(sg_name)s/snapshot/%(snap_name)s/generation" - % {'sg_name': source_group, 'snap_name': snap_name}) + postfix_uri = "snapid" if self.is_snap_id else "generation" + resource_name = ("%(sg_name)s/snapshot/%(snap_name)s/%(postfix_uri)s" + % {'sg_name': source_group, 'snap_name': snap_name, + 'postfix_uri': postfix_uri}) response = self.get_resource(array, REPLICATION, 'storagegroup', resource_name=resource_name) - return response.get('generations', list()) if response else list() + if self.is_snap_id: + return response.get('snapids', list()) if response else list() + else: + return response.get('generations', list()) if response else list() def get_storagegroup_rdf_details(self, array, storagegroup_name, rdf_group_num): @@ -3277,3 +3313,59 @@ class PowerMaxRest(object): "requirements.") return unisphere_meets_min_req + + def get_snap_id(self, array, device_id, snap_name): + """Get the unique snap id for a particular snap name + + :param array: the array serial number + :param device_id: the source device ID + :param snap_name: the user supplied snapVX name + :raises: VolumeBackendAPIException + :returns: snap_id -- str + """ + snapshots = self.get_volume_snaps(array, device_id, snap_name) + if not snapshots: + exception_message = (_( + "Snapshot %(snap_name)s is not associated with " + "specified volume %(device_id)s.") % { + 'device_id': device_id, 'snap_name': snap_name}) + LOG.error(exception_message) + raise exception.VolumeBackendAPIException( + message=exception_message) + elif len(snapshots) > 1: + exception_message = (_( + "Snapshot %(snap_name)s is associated with more than " + "one snap id. No information available to choose " + "which one.") % { + 'device_id': device_id, 'snap_name': snap_name}) + LOG.error(exception_message) + raise exception.VolumeBackendAPIException( + message=exception_message) + else: + return snapshots[0].get('snap_id') if self.is_snap_id else ( + snapshots[0].get('generation')) + + def get_major_minor_ucode(self, array): + """Get the major and minor parts of the ucode + + :param array: the array serial number + :returns: ucode_major_level, ucode_minor_level -- str, str + """ + array_details = self.get_array_detail(array) + ucode_major_level = 0 + ucode_minor_level = 0 + + if array_details: + split_ucode_level = array_details['ucode'].split('.') + ucode_level = [int(level) for level in split_ucode_level] + ucode_major_level = ucode_level[0] + ucode_minor_level = ucode_level[1] + return ucode_major_level, ucode_minor_level + + def _is_snapid_enabled(self): + """Check if array is snap_id enabled + + :returns: boolean + """ + return (self.ucode_major_level >= utils.UCODE_5978 and + self.ucode_minor_level >= utils.UCODE_5978_HICKORY) diff --git a/cinder/volume/drivers/dell_emc/powermax/utils.py b/cinder/volume/drivers/dell_emc/powermax/utils.py index ababfd388a9..f74864f8b16 100644 --- a/cinder/volume/drivers/dell_emc/powermax/utils.py +++ b/cinder/volume/drivers/dell_emc/powermax/utils.py @@ -43,6 +43,7 @@ MAX_SRP_LENGTH = 16 TRUNCATE_5 = 5 TRUNCATE_27 = 27 UCODE_5978_ELMSR = 221 +UCODE_5978_HICKORY = 660 UCODE_5978 = 5978 UPPER_HOST_CHARS = 16 UPPER_PORT_GROUP_CHARS = 12 @@ -209,6 +210,9 @@ INST_ID = 'instanceId' DIR_ID = 'directorId' PORT_ID = 'portId' +# Revert snapshot exception +REVERT_SS_EXC = 'Link must be fully copied for this operation to proceed' + class PowerMaxUtils(object): """Utility class for Rest based PowerMax volume drivers.