diff --git a/cinder/tests/unit/volume/drivers/dell_emc/vmax/test_vmax.py b/cinder/tests/unit/volume/drivers/dell_emc/vmax/test_vmax.py index 653336d2ff9..5f1b40a8c2e 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/vmax/test_vmax.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/vmax/test_vmax.py @@ -199,10 +199,13 @@ class VMAXCommonData(object): replication_driver_data=six.text_type(legacy_provider_location2), host=fake_host, volume_type=test_volume_type) + snapshot_id = '390eeb4d-0f56-4a02-ba14-167167967014' + test_clone_volume = fake_volume.fake_volume_obj( context=ctx, name='vol1', size=2, provider_auth=None, provider_location=six.text_type(provider_location2), - host=fake_host) + host=fake_host, source_volid=test_volume.id, + snapshot_id=snapshot_id) test_volume_snap_manage = fake_volume.fake_volume_obj( context=ctx, name='vol1', size=2, provider_auth=None, @@ -212,7 +215,6 @@ class VMAXCommonData(object): replication_driver_data=six.text_type(provider_location4)) snapshot_display_id = 'my_snap' - snapshot_id = '390eeb4d-0f56-4a02-ba14-167167967014' managed_snap_id = 'OS-390eeb4d-0f56-4a02-ba14-167167967014' test_snapshot_snap_name = 'OS-' + snapshot_id[:6] + snapshot_id[-9:] @@ -482,10 +484,10 @@ class VMAXCommonData(object): sg_details_rep = [{"childNames": [], "numDevicesNonGk": 2, "isLinkTarget": False, - "rdf": False, + "rdf": True, "capacityGB": 2.0, "name": storagegroup_name_source, - "snapVXSnapshots": ['12345'], + "snapVXSnapshots": ['6560405d-752f5a139'], "symmetrixId": array, "numSnapVXSnapshots": 1}] @@ -1365,32 +1367,11 @@ class VMAXUtilsTest(test.TestCase): self.assertEqual(ref_group_name, vol_grp_name) def test_get_volume_group_utils(self): - group = self.data.test_group_1 - array, extraspecs_dict = self.utils.get_volume_group_utils( - group, interval=1, retries=1) + array, intervals_retries = self.utils.get_volume_group_utils( + self.data.test_group_1, interval=1, retries=1) ref_array = self.data.array self.assertEqual(ref_array, array) - def test_update_extra_specs_list(self): - extra_specs = self.data.extra_specs - volume_type_id = 'abc' - extraspecs_dict = self.utils._update_extra_specs_list( - extra_specs, volume_type_id, interval=1, retries=1) - self.assertEqual(extra_specs, extraspecs_dict['extra_specs']) - - def test_update_intervals_and_retries(self): - extra_specs = self.data.extra_specs - ref_interval = 1 - extraspecs = self.utils._update_intervals_and_retries( - extra_specs, interval=1, retries=1) - self.assertEqual(ref_interval, extraspecs['interval']) - - def test_get_intervals_retries_dict(self): - ref_value = {'interval': 1, 'retries': 1} - ret_dict = self.utils.get_intervals_retries_dict( - interval=1, retries=1) - self.assertEqual(ref_value, ret_dict) - def test_update_volume_model_updates(self): volume_model_updates = [{'id': '1', 'status': 'available'}] volumes = [self.data.test_volume] @@ -2452,6 +2433,16 @@ class VMAXRestTest(test.TestCase): self.rest.modify_resource.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, target_id)]) + self.rest.modify_resource.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( @@ -2976,7 +2967,8 @@ class VMAXProvisionTest(test.TestCase): (self.provision.rest.modify_volume_snap. assert_called_once_with( array, source_device_id, target_device_id, - snap_name, extra_specs, unlink=True)) + snap_name, extra_specs, + list_volume_pairs=None, unlink=True)) @mock.patch('oslo_service.loopingcall.FixedIntervalLoopingCall', new=test_utils.ZeroIntervalLoopingCall) @@ -2988,7 +2980,7 @@ class VMAXProvisionTest(test.TestCase): 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, - unlink=True) + list_volume_pairs=None, unlink=True) @mock.patch('oslo_service.loopingcall.FixedIntervalLoopingCall', new=test_utils.ZeroIntervalLoopingCall) @@ -3281,19 +3273,6 @@ class VMAXProvisionTest(test.TestCase): target_group_name, snap_name, extra_specs, deleteSnapshot) - def test_unlink_group(self): - with mock.patch.object(self.rest, - 'modify_storagegroup_snap') as mock_mod: - self.provision._unlink_group( - self.data.array, self.data.storagegroup_name_source, - self.data.target_group_name, - self.data.group_snapshot_name, self.data.extra_specs) - mock_mod.assert_called_once_with( - self.data.array, self.data.storagegroup_name_source, - self.data.target_group_name, - self.data.group_snapshot_name, self.data.extra_specs, - unlink=True) - @mock.patch.object(rest.VMAXRest, 'get_storage_group', side_effect=[None, VMAXCommonData.sg_details[1]]) @mock.patch.object(provision.VMAXProvision, 'create_volume_group') @@ -3452,7 +3431,7 @@ class VMAXCommonTest(test.TestCase): self.common.delete_snapshot(self.data.test_snapshot, self.data.test_volume) self.provision.delete_volume_snap.assert_called_once_with( - self.data.array, snap_name, sourcedevice_id) + self.data.array, snap_name, [sourcedevice_id]) def test_delete_snapshot_not_found(self): with mock.patch.object(self.common, '_parse_snap_info', @@ -4838,26 +4817,61 @@ class VMAXCommonTest(test.TestCase): group, volumes) self.assertEqual(ref_model_update, model_update) + @mock.patch.object( + common.VMAXCommon, '_get_clone_vol_info', + return_value=(VMAXCommonData.device_id, + VMAXCommonData.extra_specs, 1, 'tgt_vol')) @mock.patch.object(volume_utils, 'is_group_a_cg_snapshot_type', return_value=True) @mock.patch.object(volume_utils, 'is_group_a_type', return_value=False) - def test_create_group_from_src_success(self, mock_type, mock_cg_type): - context = None - group = self.data.test_group_1 - group_snapshot = self.data.test_group_snapshot_1 - snapshots = [] - volumes = [self.data.test_volume] - source_group = None - source_vols = [] + def test_create_group_from_src_success(self, mock_type, + mock_cg_type, mock_info): ref_model_update = {'status': fields.GroupStatus.AVAILABLE} model_update, volumes_model_update = ( self.common.create_group_from_src( - context, group, volumes, - group_snapshot, snapshots, - source_group, source_vols)) + None, self.data.test_group_1, [self.data.test_volume], + self.data.test_group_snapshot_1, [], None, [])) self.assertEqual(ref_model_update, model_update) + @mock.patch.object( + common.VMAXCommon, '_remove_vol_and_cleanup_replication') + @mock.patch.object( + masking.VMAXMasking, 'remove_volumes_from_storage_group') + def test_rollback_create_group_from_src( + self, mock_rm, mock_clean): + rollback_dict = { + 'target_group_name': self.data.target_group_name, + 'snap_name': 'snap1', 'source_group_name': 'src_grp', + 'volumes': (self.data.device_id, self.data.extra_specs, + self.data.test_volume), + 'device_ids': [self.data.device_id], + 'interval_retries_dict': self.data.extra_specs} + for x in range(0, 2): + self.common._rollback_create_group_from_src( + self.data.array, rollback_dict) + self.assertEqual(2, mock_rm.call_count) + + def test_get_snap_src_dev_list(self): + src_dev_ids = self.common._get_snap_src_dev_list( + self.data.array, [self.data.test_snapshot]) + ref_dev_ids = [self.data.device_id] + self.assertEqual(ref_dev_ids, src_dev_ids) + + def test_get_clone_vol_info(self): + ref_dev_id = self.data.device_id + source_vols = [self.data.test_volume, + self.data.test_attached_volume] + src_snapshots = [self.data.test_snapshot] + src_dev_id1, extra_specs1, vol_size1, tgt_vol_name1 = ( + self.common._get_clone_vol_info( + self.data.test_clone_volume, source_vols, [])) + src_dev_id2, extra_specs2, vol_size2, tgt_vol_name2 = ( + self.common._get_clone_vol_info( + self.data.test_clone_volume, [], src_snapshots)) + self.assertEqual(ref_dev_id, src_dev_id1) + self.assertEqual(ref_dev_id, src_dev_id2) + def test_get_attributes_from_cinder_config(self): kwargs_expected = ( {'RestServerIp': '1.1.1.1', @@ -7249,7 +7263,7 @@ class VMAXCommonReplicationTest(test.TestCase): model_update['replication_status']) @mock.patch.object(utils.VMAXUtils, 'get_volume_group_utils', - return_value=(VMAXCommonData.array, [])) + return_value=(VMAXCommonData.array, {})) @mock.patch.object(common.VMAXCommon, '_cleanup_group_replication') @mock.patch.object(volume_utils, 'is_group_a_type', return_value=True) def test_delete_replication_group(self, mock_check, diff --git a/cinder/volume/drivers/dell_emc/vmax/common.py b/cinder/volume/drivers/dell_emc/vmax/common.py index f263934df59..d2eb234c27b 100644 --- a/cinder/volume/drivers/dell_emc/vmax/common.py +++ b/cinder/volume/drivers/dell_emc/vmax/common.py @@ -274,6 +274,8 @@ class VMAXCommon(object): if volume.group_id is not None: if (volume_utils.is_group_a_cg_snapshot_type(volume.group) or volume.group.is_replicated): + LOG.debug("Adding volume %(vol_id)s to group %(grp_id)s", + {'vol_id': volume.id, 'grp_id': volume.group_id}) self._add_new_volume_to_volume_group( volume, volume_dict['device_id'], volume_name, extra_specs, rep_driver_data) @@ -1007,8 +1009,11 @@ class VMAXCommon(object): device_id = name['keybindings']['DeviceID'] else: device_id = None - founddevice_id = self.rest.check_volume_device_id( - array, device_id, volume_name) + try: + founddevice_id = self.rest.check_volume_device_id( + array, device_id, volume_name) + except exception.VolumeBackendAPIException: + pass if founddevice_id is None: LOG.debug("Volume %(volume_name)s not found on the array.", @@ -3237,10 +3242,8 @@ class VMAXCommon(object): vol_grp_name = self.utils.update_volume_group_name(group) try: - array, __ = self.utils.get_volume_group_utils( + array, interval_retries_dict = self.utils.get_volume_group_utils( group, self.interval, self.retries) - interval_retries_dict = self.utils.get_intervals_retries_dict( - self.interval, self.retries) self.provision.create_volume_group( array, vol_grp_name, interval_retries_dict) if group.is_replicated: @@ -3288,7 +3291,7 @@ class VMAXCommon(object): :returns: model_update, volumes_model_update """ volumes_model_update = [] - array, extraspecs_dict_list = self.utils.get_volume_group_utils( + array, interval_retries_dict = self.utils.get_volume_group_utils( group, self.interval, self.retries) vol_grp_name = None @@ -3308,39 +3311,33 @@ class VMAXCommon(object): vol_grp_name = volume_group['name'] volume_device_ids = self._get_members_of_volume_group( array, vol_grp_name) - intervals_retries_dict = self.utils.get_intervals_retries_dict( - self.interval, self.retries) deleted_volume_device_ids = [] # Remove replication for group, if applicable if group.is_replicated: self._cleanup_group_replication( array, vol_grp_name, volume_device_ids, - intervals_retries_dict) + interval_retries_dict) try: if volume_device_ids: # First remove all the volumes from the SG self.masking.remove_volumes_from_storage_group( array, volume_device_ids, vol_grp_name, - intervals_retries_dict) + interval_retries_dict) for vol in volumes: - for extraspecs_dict in extraspecs_dict_list: - if (vol.volume_type_id in - extraspecs_dict['volumeTypeId']): - extraspecs = extraspecs_dict.get( - utils.EXTRA_SPECS) - device_id = self._find_device_on_array( - vol, extraspecs) - if device_id in volume_device_ids: - self.masking.remove_and_reset_members( - array, vol, device_id, vol.name, - extraspecs, False) - self._delete_from_srp( - array, device_id, "group vol", extraspecs) - else: - LOG.debug("Volume not found on the array.") - # Add the device id to the deleted list - deleted_volume_device_ids.append(device_id) + extra_specs = self._initial_setup(vol) + device_id = self._find_device_on_array( + vol, extra_specs) + if device_id in volume_device_ids: + self.masking.remove_and_reset_members( + array, vol, device_id, vol.name, + extra_specs, False) + self._delete_from_srp( + array, device_id, "group vol", extra_specs) + else: + LOG.debug("Volume not found on the array.") + # Add the device id to the deleted list + deleted_volume_device_ids.append(device_id) # Once all volumes are deleted then delete the SG self.rest.delete_storage_group(array, vol_grp_name) model_update = {'status': fields.GroupStatus.DELETED} @@ -3351,27 +3348,28 @@ class VMAXCommon(object): "Error received: %(e)s", {'e': e}) model_update = {'status': fields.GroupStatus.ERROR_DELETING} # Update the volumes_model_update + if len(deleted_volume_device_ids) is not 0: + LOG.debug("Device ids: %(dev)s are deleted.", + {'dev': deleted_volume_device_ids}) volumes_not_deleted = [] for vol in volume_device_ids: if vol not in deleted_volume_device_ids: volumes_not_deleted.append(vol) if not deleted_volume_device_ids: volumes_model_update = self.utils.update_volume_model_updates( - volumes_model_update, - deleted_volume_device_ids, + volumes_model_update, deleted_volume_device_ids, group.id, status='deleted') if not volumes_not_deleted: volumes_model_update = self.utils.update_volume_model_updates( - volumes_model_update, - volumes_not_deleted, - group.id, status='deleted') + volumes_model_update, volumes_not_deleted, + group.id, status='error_deleting') # As a best effort try to add back the undeleted volumes to sg - # Dont throw any exception in case of failure + # Don't throw any exception in case of failure try: if not volumes_not_deleted: self.masking.add_volumes_to_storage_group( array, volumes_not_deleted, - vol_grp_name, intervals_retries_dict) + vol_grp_name, interval_retries_dict) except Exception as ex: LOG.error("Error in rollback - %(ex)s. " "Failed to add back volumes to sg %(sg_name)s", @@ -3434,8 +3432,7 @@ class VMAXCommon(object): try: snap_name = self.utils.truncate_string(group_snapshot.id, 19) - self._create_group_replica(source_group, - snap_name) + self._create_group_replica(source_group, snap_name) except Exception as e: exception_message = (_("Failed to create snapshot for group: " @@ -3446,13 +3443,26 @@ class VMAXCommon(object): raise exception.VolumeBackendAPIException(data=exception_message) for snapshot in snapshots: + src_dev_id = self._get_src_device_id_for_group_snap(snapshot) snapshots_model_update.append( {'id': snapshot.id, + 'provider_location': six.text_type( + {'source_id': src_dev_id, 'snap_name': snap_name}), 'status': fields.SnapshotStatus.AVAILABLE}) model_update = {'status': fields.GroupStatus.AVAILABLE} return model_update, snapshots_model_update + def _get_src_device_id_for_group_snap(self, snapshot): + """Get the source device id for the provider_location. + + :param snapshot: the snapshot object + :return: src_device_id + """ + volume = snapshot.volume + extra_specs = self._initial_setup(volume) + return self._find_device_on_array(volume, extra_specs) + def _create_group_replica( self, source_group, snap_name): """Create a group replica. @@ -3461,9 +3471,8 @@ class VMAXCommon(object): :param source_group: the group object :param snap_name: the name of the snapshot """ - array, __ = ( - self.utils.get_volume_group_utils( - source_group, self.interval, self.retries)) + array, interval_retries_dict = self.utils.get_volume_group_utils( + source_group, self.interval, self.retries) vol_grp_name = None volume_group = ( self._find_volume_group(array, source_group)) @@ -3476,8 +3485,6 @@ class VMAXCommon(object): {'group_id': source_group.id}) raise exception.VolumeBackendAPIException( data=exception_message) - interval_retries_dict = self.utils.get_intervals_retries_dict( - self.interval, self.retries) self.provision.create_group_replica( array, vol_grp_name, snap_name, interval_retries_dict) @@ -3503,7 +3510,6 @@ class VMAXCommon(object): :raises: VolumeBackendApiException, NotImplementedError """ snapshots_model_update = [] - model_update = {} source_group = group_snapshot.get('group') grp_id = group_snapshot.group_id if not volume_utils.is_group_a_cg_snapshot_type(source_group): @@ -3518,15 +3524,12 @@ class VMAXCommon(object): vol_grp_name = None try: # Get the array serial - array, __ = ( - self.utils.get_volume_group_utils( - source_group, self.interval, self.retries)) + array, extra_specs = self.utils.get_volume_group_utils( + source_group, self.interval, self.retries) # Get the volume group dict for getting the group name - volume_group = ( - self._find_volume_group(array, source_group)) - if volume_group: - if 'name' in volume_group: - vol_grp_name = volume_group['name'] + volume_group = (self._find_volume_group(array, source_group)) + if volume_group and volume_group.get('name'): + vol_grp_name = volume_group['name'] if vol_grp_name is None: exception_message = ( _("Cannot find generic volume group %(grp_id)s.") % @@ -3536,9 +3539,9 @@ class VMAXCommon(object): # Check if the snapshot exists if 'snapVXSnapshots' in volume_group: if snap_name in volume_group['snapVXSnapshots']: - self.provision.delete_group_replica(array, - snap_name, - vol_grp_name) + src_devs = self._get_snap_src_dev_list(array, snapshots) + self.provision.delete_group_replica( + array, snap_name, vol_grp_name, src_devs, extra_specs) else: # Snapshot has been already deleted, return successfully LOG.error("Cannot find group snapshot %(snapId)s.", @@ -3556,6 +3559,20 @@ class VMAXCommon(object): return model_update, snapshots_model_update + def _get_snap_src_dev_list(self, array, snapshots): + """Get the list of source devices for a list of snapshots. + + :param array: the array serial number + :param snapshots: the list of snapshot objects + :return: src_dev_ids + """ + src_dev_ids = [] + for snap in snapshots: + 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 + def _find_volume_group(self, array, group): """Finds a volume group given the group. @@ -3603,7 +3620,7 @@ class VMAXCommon(object): and not group.is_replicated): raise NotImplementedError() - array, __ = self.utils.get_volume_group_utils( + array, interval_retries_dict = self.utils.get_volume_group_utils( group, self.interval, self.retries) model_update = {'status': fields.GroupStatus.AVAILABLE} add_vols = [vol for vol in add_volumes] if add_volumes else [] @@ -3618,8 +3635,6 @@ class VMAXCommon(object): vol_grp_name = volume_group['name'] if vol_grp_name is None: raise exception.GroupNotFound(group_id=group.id) - interval_retries_dict = self.utils.get_intervals_retries_dict( - self.interval, self.retries) # Add volume(s) to the group if add_device_ids: self.utils.check_rep_status_enabled(group) @@ -3741,15 +3756,12 @@ class VMAXCommon(object): """ if not volume_utils.is_group_a_cg_snapshot_type(group): raise NotImplementedError() - # Check if we need to create a snapshot create_snapshot = False volumes_model_update = [] if group_snapshot: - source_vols_or_snapshots = snapshots source_id = group_snapshot.id - actual_source_grp = group_snapshot + actual_source_grp = group_snapshot.get('group') elif source_group: - source_vols_or_snapshots = source_vols source_id = source_group.id actual_source_grp = source_group create_snapshot = True @@ -3759,88 +3771,169 @@ class VMAXCommon(object): raise exception.VolumeBackendAPIException( data=exception_message) + tgt_name = self.utils.update_volume_group_name(group) + rollback_dict = {} + array, interval_retries_dict = self.utils.get_volume_group_utils( + group, self.interval, self.retries) + source_sg = self._find_volume_group(array, actual_source_grp) + if source_sg is not None: + src_grp_name = (source_sg['name'] + if 'name' in source_sg else None) + rollback_dict['source_group_name'] = src_grp_name + else: + error_msg = (_("Cannot retrieve source volume group %(grp_id)s " + "from the array.") + % {'grp_id': actual_source_grp.id}) + LOG.error(error_msg) + raise exception.VolumeBackendAPIException(data=error_msg) + LOG.debug("Enter VMAX create_volume group_from_src. Group to be " "created: %(grpId)s, Source : %(SourceGrpId)s.", - {'grpId': group.id, - 'SourceGrpId': source_id}) + {'grpId': group.id, 'SourceGrpId': source_id}) - tgt_name = self.utils.update_volume_group_name(group) - self.create_group(context, group) - model_update = {'status': fields.GroupStatus.AVAILABLE} try: - array, extraspecs_dict_list = ( - self.utils.get_volume_group_utils( - group, self.interval, self.retries)) - vol_grp_name = "" + self.provision.create_volume_group( + array, tgt_name, interval_retries_dict) + rollback_dict.update({ + 'target_group_name': tgt_name, 'volumes': [], + 'device_ids': [], 'list_volume_pairs': [], + 'interval_retries_dict': interval_retries_dict}) + model_update = {'status': fields.GroupStatus.AVAILABLE} # Create the target devices - dict_volume_dicts = {} - target_volume_names = {} - for volume, source_vol_or_snapshot in zip( - volumes, source_vols_or_snapshots): - if 'size' in source_vol_or_snapshot: - volume_size = source_vol_or_snapshot['size'] - else: - volume_size = source_vol_or_snapshot['volume_size'] - for extraspecs_dict in extraspecs_dict_list: - if volume.volume_type_id in ( - extraspecs_dict['volumeTypeId']): - extraspecs = extraspecs_dict.get(utils.EXTRA_SPECS) - target_volume_name = ( - self.utils.get_volume_element_name(volume.id)) - volume_dict = self.provision.create_volume_from_sg( - array, target_volume_name, - tgt_name, volume_size, extraspecs) - dict_volume_dicts[volume.id] = volume_dict - target_volume_names[volume.id] = target_volume_name + list_volume_pairs = [] + for volume in volumes: + src_dev_id, extra_specs, vol_size, tgt_vol_name = ( + self._get_clone_vol_info( + volume, source_vols, snapshots)) + volume_dict = self._create_volume( + tgt_vol_name, vol_size, extra_specs) + device_id = volume_dict['device_id'] + # Add the volume to the volume group SG + self.masking.add_volume_to_storage_group( + extra_specs[utils.ARRAY], device_id, tgt_name, + tgt_vol_name, extra_specs) + # Record relevant information + list_volume_pairs.append((src_dev_id, device_id)) + # Add details to rollback dict + rollback_dict['device_ids'].append(device_id) + rollback_dict['list_volume_pairs'].append( + (src_dev_id, device_id)) + rollback_dict['volumes'].append( + (device_id, extra_specs, volume)) + volumes_model_update.append( + self.utils.get_grp_volume_model_update( + volume, volume_dict, group.id)) if create_snapshot is True: # We have to create a snapshot of the source group snap_name = self.utils.truncate_string(group.id, 19) self._create_group_replica(actual_source_grp, snap_name) - vol_grp_name = self.utils.update_volume_group_name( - source_group) + rollback_dict['snap_name'] = snap_name else: # We need to check if the snapshot exists snap_name = self.utils.truncate_string(source_id, 19) - source_group = actual_source_grp.get('group') - volume_group = self._find_volume_group(array, source_group) - if volume_group is not None: - if 'snapVXSnapshots' in volume_group: - if snap_name in volume_group['snapVXSnapshots']: - LOG.info("Snapshot is present on the array") - if 'name' in volume_group: - vol_grp_name = volume_group['name'] + if ('snapVXSnapshots' in source_sg and + snap_name in source_sg['snapVXSnapshots']): + LOG.info("Snapshot is present on the array") + else: + error_msg = ( + _("Cannot retrieve source snapshot %(snap_id)s " + "from the array.") % {'snap_id': source_id}) + LOG.error(error_msg) + raise exception.VolumeBackendAPIException(data=error_msg) # Link and break the snapshot to the source group - interval_retries_dict = self.utils.get_intervals_retries_dict( - self.interval, self.retries) self.provision.link_and_break_replica( - array, vol_grp_name, tgt_name, snap_name, - interval_retries_dict, delete_snapshot=create_snapshot) - except Exception: - exception_message = (_("Failed to create vol grp %(volGrpName)s" - " from source %(grpSnapshot)s.") - % {'volGrpName': group.id, - 'grpSnapshot': source_id}) - LOG.exception(exception_message) - raise exception.VolumeBackendAPIException(data=exception_message) - volumes_model_update = self.utils.update_volume_model_updates( - volumes_model_update, volumes, group.id, model_update['status']) - - # Update the provider_location & replication status - for volume_model_update in volumes_model_update: - if volume_model_update['id'] in dict_volume_dicts: - volume_model_update.update( - {'provider_location': six.text_type( - dict_volume_dicts[volume_model_update['id']])}) + array, src_grp_name, tgt_name, snap_name, + interval_retries_dict, list_volume_pairs, + delete_snapshot=create_snapshot) + # Update the replication status if group.is_replicated: volumes_model_update = self._replicate_group( array, volumes_model_update, tgt_name, interval_retries_dict) model_update.update({ 'replication_status': fields.ReplicationStatus.ENABLED}) + except Exception: + exception_message = (_("Failed to create vol grp %(volGrpName)s" + " from source %(grpSnapshot)s.") + % {'volGrpName': group.id, + 'grpSnapshot': source_id}) + LOG.error(exception_message) + if array is not None: + LOG.info("Attempting rollback for the create group from src.") + self._rollback_create_group_from_src(array, rollback_dict) + raise exception.VolumeBackendAPIException(data=exception_message) return model_update, volumes_model_update + def _get_clone_vol_info(self, volume, source_vols, snapshots): + """Get the clone volume info. + + :param volume: the new volume object + :param source_vols: the source volume list + :param snapshots: the source snapshot list + :returns: src_dev_id, extra_specs, vol_size, tgt_vol_name + """ + src_dev_id, vol_size = None, None + extra_specs = self._initial_setup(volume) + if not source_vols: + for snap in snapshots: + if snap.id == volume.snapshot_id: + src_dev_id, __ = self._parse_snap_info( + extra_specs[utils.ARRAY], snap) + vol_size = snap.volume_size + else: + for src_vol in source_vols: + if src_vol.id == volume.source_volid: + src_extra_specs = self._initial_setup(src_vol) + src_dev_id = self._find_device_on_array( + src_vol, src_extra_specs) + vol_size = src_vol.size + tgt_vol_name = self.utils.get_volume_element_name(volume.id) + return src_dev_id, extra_specs, vol_size, tgt_vol_name + + def _rollback_create_group_from_src(self, array, rollback_dict): + """Performs rollback for create group from src in case of failure. + + :param array: the array serial number + :param rollback_dict: dict containing rollback details + """ + try: + # Delete the snapshot if required + if rollback_dict.get("snap_name"): + try: + src_dev_ids = [ + a for a, b in rollback_dict['list_volume_pairs']] + self.provision.delete_group_replica( + array, rollback_dict["snap_name"], + rollback_dict["source_group_name"], + src_dev_ids, rollback_dict['interval_retries_dict']) + except Exception as e: + LOG.debug("Failed to delete group snapshot. Attempting " + "further rollback. Exception received: %(e)s.", + {'e': e}) + if rollback_dict.get('volumes'): + # Remove any devices which were added to the target SG + if rollback_dict['device_ids']: + self.masking.remove_volumes_from_storage_group( + array, rollback_dict['device_ids'], + rollback_dict['target_group_name'], + rollback_dict['interval_retries_dict']) + # Delete all the volumes + for dev_id, extra_specs, volume in rollback_dict['volumes']: + self._remove_vol_and_cleanup_replication( + array, dev_id, "group vol", extra_specs, volume) + self._delete_from_srp( + array, dev_id, "group vol", extra_specs) + # Delete the target SG + if rollback_dict.get("target_group_name"): + self.rest.delete_storage_group( + array, rollback_dict['target_group_name']) + LOG.info("Rollback completed for create group from src.") + except Exception as e: + LOG.error("Rollback failed for the create group from src. " + "Exception received: %(e)s.", {'e': e}) + def _replicate_group(self, array, volumes_model_update, group_name, extra_specs): """Replicate a cloned volume group. @@ -3854,10 +3947,11 @@ class VMAXCommon(object): rdf_group_no, remote_array = self.get_rdf_details(array) self.rest.replicate_group( array, group_name, rdf_group_no, remote_array, extra_specs) - # Need to set SRP to None for generic volume group - Not set + # Need to set SRP to None for remote generic volume group - Not set # automatically, and a volume can only be in one storage group # managed by FAST - self.rest.set_storagegroup_srp(array, group_name, "None", extra_specs) + self.rest.set_storagegroup_srp( + remote_array, group_name, "None", extra_specs) for volume_model_update in volumes_model_update: vol_id = volume_model_update['id'] loc = ast.literal_eval(volume_model_update['provider_location']) diff --git a/cinder/volume/drivers/dell_emc/vmax/provision.py b/cinder/volume/drivers/dell_emc/vmax/provision.py index 739a9d70880..4ab26b44dd0 100644 --- a/cinder/volume/drivers/dell_emc/vmax/provision.py +++ b/cinder/volume/drivers/dell_emc/vmax/provision.py @@ -174,7 +174,7 @@ class VMAXProvision(object): def _unlink_volume( self, array, source_device_id, target_device_id, snap_name, - extra_specs): + extra_specs, list_volume_pairs=None): """Unlink a target volume from its source volume. :param array: the array serial number @@ -182,6 +182,7 @@ class VMAXProvision(object): :param target_device_id: the target device id :param snap_name: the snap name :param extra_specs: extra specifications + :param list_volume_pairs: list of volume pairs, optional :return: return code """ @@ -196,7 +197,8 @@ class VMAXProvision(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) + extra_specs, unlink=True, + list_volume_pairs=list_volume_pairs) kwargs['modify_vol_success'] = True except exception.VolumeBackendAPIException: pass @@ -315,26 +317,32 @@ class VMAXProvision(object): do_delete_temp_snap(snap_name) def delete_volume_snap_check_for_links(self, array, snap_name, - source_device, extra_specs): + source_devices, extra_specs): """Check if a snap has any links before deletion. If a snapshot has any links, break the replication relationship before deletion. :param array: the array serial number :param snap_name: the snapshot name - :param source_device: the source device id + :param source_devices: the source device ids :param extra_specs: the extra specifications """ - LOG.debug("Check for linked devices to SnapVx: %(snap_name)s " - "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) - for link in linked_list: - target_device = link['targetDevice'] - self.break_replication_relationship( - array, target_device, source_device, snap_name, extra_specs) - self.delete_volume_snap(array, snap_name, source_device) + list_device_pairs = [] + if not isinstance(source_devices, list): + source_devices = [source_devices] + for source_device in source_devices: + LOG.debug("Check for linked devices to SnapVx: %(snap_name)s " + "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) + for link in linked_list: + target_device = link['targetDevice'] + list_device_pairs.append((source_device, target_device)) + if list_device_pairs: + self._unlink_volume(array, "", "", snap_name, extra_specs, + list_volume_pairs=list_device_pairs) + self.delete_volume_snap(array, snap_name, source_devices) def extend_volume(self, array, device_id, new_size, extra_specs): """Extend a volume. @@ -649,26 +657,26 @@ class VMAXProvision(object): self.rest.create_storagegroup_snap( array, source_group, snap_name, extra_specs) - def delete_group_replica(self, array, snap_name, - source_group_name): + def delete_group_replica(self, array, snap_name, source_group_name, + src_dev_ids, extra_specs): """Delete the snapshot. :param array: the array serial number :param snap_name: the name for the snap shot :param source_group_name: the source group name + :param src_dev_ids: the list of source device ids + :param extra_specs: extra specifications """ # Delete snapvx snapshot LOG.debug("Deleting Snap Vx snapshot: source group: %(srcGroup)s " "snapshot: %(snap_name)s.", - {'srcGroup': source_group_name, - 'snap_name': snap_name}) - # The check for existence of snapshot has already happened - # So we just need to delete the snapshot - self.rest.delete_storagegroup_snap(array, snap_name, source_group_name) + {'srcGroup': source_group_name, 'snap_name': snap_name}) + self.delete_volume_snap_check_for_links( + array, snap_name, src_dev_ids, extra_specs) def link_and_break_replica(self, array, source_group_name, target_group_name, snap_name, extra_specs, - delete_snapshot=False): + list_volume_pairs, delete_snapshot=False): """Links a group snap and breaks the relationship. :param array: the array serial @@ -676,6 +684,7 @@ class VMAXProvision(object): :param target_group_name: the target group name :param snap_name: the snapshot name :param extra_specs: extra specifications + :param list_volume_pairs: the list of volume pairs :param delete_snapshot: delete snapshot flag """ LOG.debug("Linking Snap Vx snapshot: source group: %(srcGroup)s " @@ -683,66 +692,24 @@ class VMAXProvision(object): {'srcGroup': source_group_name, 'tgtGroup': target_group_name}) # Link the snapshot - self.rest.modify_storagegroup_snap( - array, source_group_name, target_group_name, snap_name, - extra_specs, link=True) + self.rest.modify_volume_snap( + array, None, None, snap_name, extra_specs, link=True, + list_volume_pairs=list_volume_pairs) # Unlink the snapshot LOG.debug("Unlinking Snap Vx snapshot: source group: %(srcGroup)s " "targetGroup: %(tgtGroup)s.", {'srcGroup': source_group_name, 'tgtGroup': target_group_name}) - self._unlink_group(array, source_group_name, - target_group_name, snap_name, extra_specs) + self._unlink_volume(array, None, None, snap_name, extra_specs, + list_volume_pairs=list_volume_pairs) # Delete the snapshot if necessary if delete_snapshot: LOG.debug("Deleting Snap Vx snapshot: source group: %(srcGroup)s " "snapshot: %(snap_name)s.", {'srcGroup': source_group_name, 'snap_name': snap_name}) - self.rest.delete_storagegroup_snap(array, snap_name, - source_group_name) - - def _unlink_group( - self, array, source_group_name, target_group_name, snap_name, - extra_specs): - """Unlink a target group from it's source group. - - :param array: the array serial number - :param source_group_name: the source group name - :param target_group_name: the target device name - :param snap_name: the snap name - :param extra_specs: extra specifications - :returns: return code - """ - - def _unlink_grp(): - """Called at an interval until the synchronization is finished. - - :raises: loopingcall.LoopingCallDone - """ - retries = kwargs['retries'] - try: - kwargs['retries'] = retries + 1 - if not kwargs['modify_grp_snap_success']: - self.rest.modify_storagegroup_snap( - array, source_group_name, target_group_name, - snap_name, extra_specs, unlink=True) - kwargs['modify_grp_snap_success'] = True - except exception.VolumeBackendAPIException: - pass - - if kwargs['retries'] > UNLINK_RETRIES: - LOG.error("_unlink_grp failed after %(retries)d " - "tries.", {'retries': retries}) - raise loopingcall.LoopingCallDone(retvalue=30) - if kwargs['modify_grp_snap_success']: - raise loopingcall.LoopingCallDone() - - kwargs = {'retries': 0, - 'modify_grp_snap_success': False} - timer = loopingcall.FixedIntervalLoopingCall(_unlink_grp) - rc = timer.start(interval=UNLINK_INTERVAL).wait() - return rc + source_devices = [a for a, b in list_volume_pairs] + self.delete_volume_snap(array, snap_name, source_devices) def enable_group_replication(self, array, storagegroup_name, rdf_group_num, extra_specs, establish=False): @@ -799,15 +766,19 @@ class VMAXProvision(object): :param rdf_group_num: the rdf group number :param extra_specs: the extra specifications """ - action = "Split" - LOG.debug("Splitting remote replication for group %(sg)s", - {'sg': storagegroup_name}) - self.rest.modify_storagegroup_rdf( - array, storagegroup_name, rdf_group_num, action, extra_specs) - LOG.debug("Deleting remote replication for group %(sg)s", - {'sg': storagegroup_name}) - self.rest.delete_storagegroup_rdf( - array, storagegroup_name, rdf_group_num) + group_details = self.rest.get_storage_group_rep( + array, storagegroup_name) + if (group_details and group_details.get('rdf') + and group_details['rdf'] is True): + action = "Split" + LOG.debug("Splitting remote replication for group %(sg)s", + {'sg': storagegroup_name}) + self.rest.modify_storagegroup_rdf( + array, storagegroup_name, rdf_group_num, action, extra_specs) + LOG.debug("Deleting remote replication for group %(sg)s", + {'sg': storagegroup_name}) + self.rest.delete_storagegroup_rdf( + array, storagegroup_name, rdf_group_num) def revert_volume_snapshot(self, array, source_device_id, snap_name, extra_specs): diff --git a/cinder/volume/drivers/dell_emc/vmax/rest.py b/cinder/volume/drivers/dell_emc/vmax/rest.py index 994f944f79d..965877acb0b 100644 --- a/cinder/volume/drivers/dell_emc/vmax/rest.py +++ b/cinder/volume/drivers/dell_emc/vmax/rest.py @@ -1541,7 +1541,8 @@ class VMAXRest(object): def modify_volume_snap(self, array, source_id, target_id, snap_name, extra_specs, link=False, unlink=False, - rename=False, new_snap_name=None, restore=False): + rename=False, new_snap_name=None, restore=False, + list_volume_pairs=None): """Modify a snapvx snapshot :param array: the array serial number @@ -1554,9 +1555,9 @@ class VMAXRest(object): :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 """ - action = None - operation = '' + action, operation, payload = '', '', {} if link: action = "Link" elif unlink: @@ -1575,8 +1576,16 @@ class VMAXRest(object): "star": 'false', "force": 'false'} elif action in ('Link', 'Unlink'): operation = 'Modify snapVx relationship to target' - payload = {"deviceNameListSource": [{"name": source_id}], - "deviceNameListTarget": [{"name": target_id}], + src_list, tgt_list = [], [] + if list_volume_pairs: + for a, b in list_volume_pairs: + src_list.append({'name': a}) + tgt_list.append({'name': b}) + else: + src_list.append({'name': source_id}) + tgt_list.append({'name': target_id}) + payload = {"deviceNameListSource": src_list, + "deviceNameListTarget": tgt_list, "copy": 'true', "action": action, "star": 'false', "force": 'false', "exact": 'false', "remote": 'false', @@ -1595,19 +1604,22 @@ class VMAXRest(object): self.wait_for_job(operation, status_code, job, extra_specs) def delete_volume_snap(self, array, snap_name, - source_device_id, restored=False): - """Delete the snapshot of a volume. + source_device_ids, 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_id: the source device id + :param source_device_ids: the source device ids :param restored: Flag to indicate terminate restore session """ + 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} if restored: - payload = {"deviceNameListSource": [{"name": source_device_id}], - "restore": True} - else: - payload = {"deviceNameListSource": [{"name": source_device_id}]} + payload.update({"restore": True}) return self.delete_resource( array, REPLICATION, 'snapshot', snap_name, payload=payload, private='/private') @@ -2108,7 +2120,6 @@ class VMAXRest(object): :param storagegroup_name: the storage group name :returns: volume_list """ - volume_list = None params = {"storageGroupId": storagegroup_name} volume_list = self.get_volume_list(array, params) @@ -2134,50 +2145,6 @@ class VMAXRest(object): self.wait_for_job('Create storage group snapVx', status_code, job, extra_specs) - def modify_storagegroup_snap( - self, array, source_sg_id, target_sg_id, snap_name, - extra_specs, link=False, unlink=False): - """Link or unlink a snapVx to or from a target storagegroup. - - :param array: the array serial number - :param source_sg_id: the source device id - :param target_sg_id: the target device id - :param snap_name: the snapshot name - :param extra_specs: extra specifications - :param link: Flag to indicate action = Link - :param unlink: Flag to indicate action = Unlink - """ - payload = '' - if link: - payload = {"link": {"linkStorageGroupName": target_sg_id, - "copy": "true"}, - "action": "Link"} - elif unlink: - payload = {"unlink": {"unlinkStorageGroupName": target_sg_id}, - "action": "Unlink"} - - resource_name = ('%(sg_name)s/snapshot/%(snap_id)s/generation/0' - % {'sg_name': source_sg_id, 'snap_id': snap_name}) - - status_code, job = self.modify_resource( - array, REPLICATION, 'storagegroup', payload, - resource_name=resource_name) - - self.wait_for_job('Modify storagegroup snapVx relationship to target', - status_code, job, extra_specs) - - def delete_storagegroup_snap(self, array, snap_name, source_sg_id): - """Delete the snapshot of a storagegroup. - - :param array: the array serial number - :param snap_name: the name of the snapshot - :param source_sg_id: the source device id - """ - resource_name = ('%(sg_name)s/snapshot/%(snap_id)s/generation/0' - % {'sg_name': source_sg_id, 'snap_id': snap_name}) - return self.delete_resource( - array, REPLICATION, 'storagegroup', resource_name) - def get_storagegroup_rdf_details(self, array, storagegroup_name, rdf_group_num): """Get the remote replication details of a storage group. diff --git a/cinder/volume/drivers/dell_emc/vmax/utils.py b/cinder/volume/drivers/dell_emc/vmax/utils.py index 83d9f14ab2e..43324bece70 100644 --- a/cinder/volume/drivers/dell_emc/vmax/utils.py +++ b/cinder/volume/drivers/dell_emc/vmax/utils.py @@ -583,18 +583,29 @@ class VMAXUtils(object): :param status: string value reflects the status of the member volume :returns: volume_model_updates - updated volumes """ - LOG.info( - "Updating status for group: %(id)s.", - {'id': group_id}) + LOG.info("Updating status for group: %(id)s.", {'id': group_id}) if volumes: for volume in volumes: volume_model_updates.append({'id': volume.id, 'status': status}) else: - LOG.info("No volume found for group: %(cg)s.", - {'cg': group_id}) + LOG.info("No volume found for group: %(cg)s.", {'cg': group_id}) return volume_model_updates + @staticmethod + def get_grp_volume_model_update(volume, volume_dict, group_id): + """Create and return the volume model update on creation. + + :param volume: volume object + :param volume_dict: the volume dict + :param group_id: consistency group id + :returns: model_update + """ + LOG.info("Updating status for group: %(id)s.", {'id': group_id}) + model_update = ({'id': volume.id, 'status': 'available', + 'provider_location': six.text_type(volume_dict)}) + return model_update + @staticmethod def update_extra_specs(extraspecs): """Update extra specs. @@ -619,39 +630,21 @@ class VMAXUtils(object): " the provided extra_specs.") return extraspecs - @staticmethod - def get_intervals_retries_dict(interval, retries): - """Get the default intervals and retries. - - :param interval: Interval in seconds between retries - :param retries: Retry count - :returns: default_dict - """ - default_dict = {} - default_dict[INTERVAL] = interval - default_dict[RETRIES] = retries - return default_dict - def get_volume_group_utils(self, group, interval, retries): """Standard utility for generic volume groups. :param group: the generic volume group object to be created :param interval: Interval in seconds between retries :param retries: Retry count - :returns: array, extra specs dict list + :returns: array, intervals_retries_dict :raises: VolumeBackendAPIException """ arrays = set() - extraspecs_dict_list = [] # Check if it is a generic volume group instance if isinstance(group, Group): for volume_type in group.volume_types: - extraspecs_dict = ( - self._update_extra_specs_list( - volume_type.extra_specs, - volume_type.id, interval, retries)) - extraspecs_dict_list.append(extraspecs_dict) - arrays.add(extraspecs_dict[EXTRA_SPECS][ARRAY]) + extra_specs = self.update_extra_specs(volume_type.extra_specs) + arrays.add(extra_specs[ARRAY]) else: msg = (_("Unable to get volume type ids.")) LOG.error(msg) @@ -669,25 +662,8 @@ class VMAXUtils(object): LOG.error(msg) raise exception.VolumeBackendAPIException(data=msg) array = arrays.pop() - return array, extraspecs_dict_list - - def _update_extra_specs_list(self, extraspecs, volumetype_id, - interval, retries): - """Update the extra specs list. - - :param extraspecs: extraspecs - :param volumetype_Id: volume type identifier - :param interval: Interval in seconds between retries - :param retries: Retry count - :returns: extraspecs_dict_list - """ - extraspecs_dict = {} - extraspecs = self.update_extra_specs(extraspecs) - extraspecs = self._update_intervals_and_retries( - extraspecs, interval, retries) - extraspecs_dict["volumeTypeId"] = volumetype_id - extraspecs_dict[EXTRA_SPECS] = extraspecs - return extraspecs_dict + intervals_retries_dict = {INTERVAL: interval, RETRIES: retries} + return array, intervals_retries_dict def update_volume_group_name(self, group): """Format id and name consistency group. @@ -704,23 +680,6 @@ class VMAXUtils(object): group_name += group.id return group_name - @staticmethod - def _update_intervals_and_retries(extra_specs, interval, retries): - """Updates the extraSpecs with intervals and retries values. - - :param extra_specs: - :param interval: Interval in seconds between retries - :param retries: Retry count - :returns: Updated extra_specs - """ - extra_specs[INTERVAL] = interval - LOG.debug("The interval is set at: %(intervalInSecs)s.", - {'intervalInSecs': interval}) - extra_specs[RETRIES] = retries - LOG.debug("Retries are set at: %(retries)s.", - {'retries': retries}) - return extra_specs - @staticmethod def add_legacy_pools(pools): """Add legacy pools to allow extending a volume after upgrade. @@ -781,6 +740,7 @@ class VMAXUtils(object): msg = (_('Replication status should be %s for ' 'replication-enabled group.') % fields.ReplicationStatus.ENABLED) + LOG.error(msg) raise exception.InvalidInput(reason=msg) else: LOG.debug('Replication is not enabled on group %s, '