Merge "VMAX driver - Fix error handling and checks for generic volume groups"

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, intervals_retries = self.utils.get_volume_group_utils(
-        array, extraspecs_dict = self.utils.get_volume_group_utils(
+            self.data.test_group_1, interval=1, retries=1)
-            group, 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):
+    def test_create_group_from_src_success(self, mock_type,
-        context = None
+                                           mock_cg_type, mock_info):
-        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 = []
         ref_model_update = {'status': fields.GroupStatus.AVAILABLE}
         model_update, volumes_model_update = (
             self.common.create_group_from_src(
-                context, group, volumes,
+                None, self.data.test_group_1, [self.data.test_volume],
-                group_snapshot, snapshots,
+                self.data.test_group_snapshot_1, [], None, []))
-                source_group, source_vols))
         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,

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(
+            try:
-                array, device_id, volume_name)
+                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:
+                    extra_specs = self._initial_setup(vol)
-                        if (vol.volume_type_id in
+                    device_id = self._find_device_on_array(
-                                extraspecs_dict['volumeTypeId']):
+                        vol, extra_specs)
-                            extraspecs = extraspecs_dict.get(
+                    if device_id in volume_device_ids:
-                                utils.EXTRA_SPECS)
+                        self.masking.remove_and_reset_members(
-                            device_id = self._find_device_on_array(
+                            array, vol, device_id, vol.name,
-                                vol, extraspecs)
+                            extra_specs, False)
-                            if device_id in volume_device_ids:
+                        self._delete_from_srp(
-                                self.masking.remove_and_reset_members(
+                            array, device_id, "group vol", extra_specs)
-                                    array, vol, device_id, vol.name,
+                    else:
-                                    extraspecs, False)
+                        LOG.debug("Volume not found on the array.")
-                                self._delete_from_srp(
+                    # Add the device id to the deleted list
-                                    array, device_id, "group vol", extraspecs)
+                    deleted_volume_device_ids.append(device_id)
-                            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,
+                    volumes_model_update, deleted_volume_device_ids,
-                    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_model_update, volumes_not_deleted,
-                    volumes_not_deleted,
+                    group.id, status='error_deleting')
-                    group.id, status='deleted')
             # 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,
+            self._create_group_replica(source_group, snap_name)
-                                       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, __ = (
+        array, interval_retries_dict = self.utils.get_volume_group_utils(
-            self.utils.get_volume_group_utils(
+            source_group, self.interval, self.retries)
-                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, __ = (
+            array, extra_specs = self.utils.get_volume_group_utils(
-                self.utils.get_volume_group_utils(
+                source_group, self.interval, self.retries)
-                    source_group, self.interval, self.retries))
             # Get the volume group dict for getting the group name
-            volume_group = (
+            volume_group = (self._find_volume_group(array, source_group))
-                self._find_volume_group(array, source_group))
+            if volume_group and volume_group.get('name'):
-            if volume_group:
+                vol_grp_name = volume_group['name']
-                if 'name' in volume_group:
-                    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,
+                    src_devs = self._get_snap_src_dev_list(array, snapshots)
-                                                        snap_name,
+                    self.provision.delete_group_replica(
-                                                        vol_grp_name)
+                        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,
+                  {'grpId': group.id, 'SourceGrpId': source_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.provision.create_volume_group(
-                self.utils.get_volume_group_utils(
+                array, tgt_name, interval_retries_dict)
-                    group, self.interval, self.retries))
+            rollback_dict.update({
-            vol_grp_name = ""
+                '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 = {}
+            list_volume_pairs = []
-            target_volume_names = {}
+            for volume in volumes:
-            for volume, source_vol_or_snapshot in zip(
+                src_dev_id, extra_specs, vol_size, tgt_vol_name = (
-                    volumes, source_vols_or_snapshots):
+                    self._get_clone_vol_info(
-                if 'size' in source_vol_or_snapshot:
+                        volume, source_vols, snapshots))
-                    volume_size = source_vol_or_snapshot['size']
+                volume_dict = self._create_volume(
-                else:
+                    tgt_vol_name, vol_size, extra_specs)
-                    volume_size = source_vol_or_snapshot['volume_size']
+                device_id = volume_dict['device_id']
-                for extraspecs_dict in extraspecs_dict_list:
+                # Add the volume to the volume group SG
-                    if volume.volume_type_id in (
+                self.masking.add_volume_to_storage_group(
-                            extraspecs_dict['volumeTypeId']):
+                    extra_specs[utils.ARRAY], device_id, tgt_name,
-                        extraspecs = extraspecs_dict.get(utils.EXTRA_SPECS)
+                    tgt_vol_name, extra_specs)
-                        target_volume_name = (
+                # Record relevant information
-                            self.utils.get_volume_element_name(volume.id))
+                list_volume_pairs.append((src_dev_id, device_id))
-                        volume_dict = self.provision.create_volume_from_sg(
+                # Add details to rollback dict
-                            array, target_volume_name,
+                rollback_dict['device_ids'].append(device_id)
-                            tgt_name, volume_size, extraspecs)
+                rollback_dict['list_volume_pairs'].append(
-                        dict_volume_dicts[volume.id] = volume_dict
+                    (src_dev_id, device_id))
-                        target_volume_names[volume.id] = target_volume_name
+                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(
+                rollback_dict['snap_name'] = snap_name
-                    source_group)
             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')
+                if ('snapVXSnapshots' in source_sg and
-                volume_group = self._find_volume_group(array, source_group)
+                        snap_name in source_sg['snapVXSnapshots']):
-                if volume_group is not None:
+                    LOG.info("Snapshot is present on the array")
-                    if 'snapVXSnapshots' in volume_group:
+                else:
-                        if snap_name in volume_group['snapVXSnapshots']:
+                    error_msg = (
-                            LOG.info("Snapshot is present on the array")
+                        _("Cannot retrieve source snapshot %(snap_id)s "
-                    if 'name' in volume_group:
+                          "from the array.") % {'snap_id': source_id})
-                        vol_grp_name = volume_group['name']
+                    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,
+                array, src_grp_name, tgt_name, snap_name,
-                interval_retries_dict, delete_snapshot=create_snapshot)
+                interval_retries_dict, list_volume_pairs,
-        except Exception:
+                delete_snapshot=create_snapshot)
-            exception_message = (_("Failed to create vol grp %(volGrpName)s"
+            # Update the replication status
-                                   " 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']])})
             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'])

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 "
+        list_device_pairs = []
-                  "for volume %(vol)s.",
+        if not isinstance(source_devices, list):
-                  {'vol': source_device, 'snap_name': snap_name})
+            source_devices = [source_devices]
-        linked_list = self.rest.get_snap_linked_device_list(
+        for source_device in source_devices:
-            array, source_device, snap_name)
+            LOG.debug("Check for linked devices to SnapVx: %(snap_name)s "
-        for link in linked_list:
+                      "for volume %(vol)s.",
-            target_device = link['targetDevice']
+                      {'vol': source_device, 'snap_name': snap_name})
-            self.break_replication_relationship(
+            linked_list = self.rest.get_snap_linked_device_list(
-                array, target_device, source_device, snap_name, extra_specs)
+                array, source_device, snap_name)
-        self.delete_volume_snap(array, snap_name, source_device)
+            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,
+    def delete_group_replica(self, array, snap_name, source_group_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,
+                  {'srcGroup': source_group_name, 'snap_name': snap_name})
-                   'snap_name': snap_name})
+        self.delete_volume_snap_check_for_links(
-        # The check for existence of snapshot has already happened
+            array, snap_name, src_dev_ids, extra_specs)
-        # So we just need to delete the snapshot
-        self.rest.delete_storagegroup_snap(array, snap_name, source_group_name)
 
     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(
+        self.rest.modify_volume_snap(
-            array, source_group_name, target_group_name, snap_name,
+            array, None, None, snap_name, extra_specs, link=True,
-            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,
+        self._unlink_volume(array, None, None, snap_name, extra_specs,
-                           target_group_name, 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_devices = [a for a, b in list_volume_pairs]
-                                               source_group_name)
+            self.delete_volume_snap(array, snap_name, source_devices)
-
-    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
 
     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"
+        group_details = self.rest.get_storage_group_rep(
-        LOG.debug("Splitting remote replication for group %(sg)s",
+            array, storagegroup_name)
-                  {'sg': storagegroup_name})
+        if (group_details and group_details.get('rdf')
-        self.rest.modify_storagegroup_rdf(
+                and group_details['rdf'] is True):
-            array, storagegroup_name, rdf_group_num, action, extra_specs)
+            action = "Split"
-        LOG.debug("Deleting remote replication for group %(sg)s",
+            LOG.debug("Splitting remote replication for group %(sg)s",
-                  {'sg': storagegroup_name})
+                      {'sg': storagegroup_name})
-        self.rest.delete_storagegroup_rdf(
+            self.rest.modify_storagegroup_rdf(
-            array, storagegroup_name, rdf_group_num)
+                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):

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
+        action, operation, payload = '', '', {}
-        operation = ''
         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}],
+            src_list, tgt_list = [], []
-                       "deviceNameListTarget": [{"name": target_id}],
+            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):
+                           source_device_ids, restored=False):
-        """Delete the snapshot of a volume.
+        """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}],
+            payload.update({"restore": True})
-                       "restore": True}
-        else:
-            payload = {"deviceNameListSource": [{"name": source_device_id}]}
         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.

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(
+        LOG.info("Updating status for group: %(id)s.", {'id': group_id})
-            "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.",
+            LOG.info("No volume found for group: %(cg)s.", {'cg': group_id})
-                     {'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 = (
+                extra_specs = self.update_extra_specs(volume_type.extra_specs)
-                    self._update_extra_specs_list(
+                arrays.add(extra_specs[ARRAY])
-                        volume_type.extra_specs,
-                        volume_type.id, interval, retries))
-                extraspecs_dict_list.append(extraspecs_dict)
-                arrays.add(extraspecs_dict[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
+        intervals_retries_dict = {INTERVAL: interval, RETRIES: retries}
-
+        return array, intervals_retries_dict
-    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
 
     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, '