Merge "EMC VMAX - Recreating SG when it has been deleted"

cinder/tests/unit/test_emc_vmax.py

@@ -484,6 +484,10 @@ class EMCVMAXCommonData(object):
     test_host_v3 = {'capabilities': location_info_v3,
                     'host': fake_host_2_v3}
     initiatorNames = ["123456789012345", "123456789054321"]
+    storagegroups = [{'CreationClassName': storagegroup_creationclass,
+                      'ElementName': storagegroupname},
+                     {'CreationClassName': storagegroup_creationclass,
+                      'ElementName': 'OS-SRP_1-Bronze-DSS-SG'}]
     test_ctxt = {}
     new_type = {}
     diff = {}
@@ -2038,42 +2042,6 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase):
         self.assertRaises(exception.VolumeBackendAPIException,
                           self.driver.common.utils._get_random_portgroup, dom)
 
-    def test_cleanup_last_vol(self):
-        conn = FakeEcomConnection()
-        masking = self.driver.common.masking
-        extraSpecs = {'volume_backend_name': 'GOLD_BE',
-                      'isV3': True}
-        controllerConfigService = (
-            self.driver.utils.find_controller_configuration_service(
-                conn, self.data.storage_system))
-        storageGroupName = self.data.storagegroupname
-        storageGroupInstanceName = (
-            self.driver.utils.find_storage_masking_group(
-                conn, controllerConfigService, storageGroupName))
-        volumeInstance = EMC_StorageVolume()
-        volumeInstance.path = (
-            conn.EnumerateInstanceNames("EMC_StorageVolume")[0])
-
-        volumeName = self.data.test_volume['name']
-        masking._last_volume_delete_masking_view = mock.Mock()
-        storageSystemInstanceName = (
-            conn.EnumerateInstanceNames("EMC_StorageSystem")[0])
-        # Failure case, an exception is thrown in
-        # _remove_last_vol_and_delete_sg so the returning the vol to
-        # the default SG cannot continue
-        self.assertRaises(
-            exception.VolumeBackendAPIException,
-            masking._cleanup_last_vol, conn, controllerConfigService,
-            storageGroupInstanceName, storageGroupName, volumeInstance,
-            volumeName, storageSystemInstanceName, False, extraSpecs)
-
-        # Success case, the last vol is removed and the SG is deleted
-        masking._remove_last_vol_and_delete_sg = mock.Mock(return_value=True)
-        masking._cleanup_last_vol(
-            conn, controllerConfigService, storageGroupInstanceName,
-            storageGroupName, volumeInstance, volumeName,
-            storageSystemInstanceName, False, extraSpecs)
-
     def test_is_sync_complete(self):
         conn = self.fake_ecom_connection()
         syncname = SE_ConcreteJob()
@@ -6307,6 +6275,8 @@ class EMCV3DriverTestCase(test.TestCase):
         common = self.driver.common
         common.get_target_wwns = mock.Mock(
             return_value=EMCVMAXCommonData.target_wwns)
+        common.masking.utils.find_storage_masking_group = mock.Mock(
+            return_value=self.data.storagegroups[0])
         self.driver.common._initial_setup = mock.Mock(
             return_value=self.default_extraspec())
         data = self.driver.terminate_connection(self.data.test_volume_v3,
@@ -6361,15 +6331,11 @@ class EMCV3DriverTestCase(test.TestCase):
         extraSpecs = common._initial_setup(self.data.test_volume_v3)
         targetInstance = (
             conn.EnumerateInstanceNames("EMC_StorageVolume")[0])
-        storageGroupName = common.utils.get_v3_storage_group_name('SRP_1',
-                                                                  'Bronze',
-                                                                  'DSS')
         deviceID = targetInstance['DeviceID']
         common._delete_from_pool_v3.assert_called_with(storageConfigService,
                                                        targetInstance,
                                                        targetInstance['Name'],
                                                        deviceID,
-                                                       storageGroupName,
                                                        extraSpecs)
 
     def test_get_remaining_slo_capacity_wlp(self):
@@ -7514,6 +7480,99 @@ class EMCVMAXMaskingTest(test.TestCase):
                 controllerConfigService, maskingviewdict['volumeName']))
         self.assertIsNone(result)
 
+    def test_return_volume_to_default_storage_group_v3(self):
+        masking = self.driver.common.masking
+        conn = self.fake_ecom_connection()
+        volumeInstanceName = (
+            conn.EnumerateInstanceNames("EMC_StorageVolume")[0])
+        volumeInstance = conn.GetInstance(volumeInstanceName)
+        volumeName = "V3-Vol"
+        extraSpecs = {'volume_backend_name': 'V3_BE',
+                      'isV3': True,
+                      'storagetype:pool': 'SRP_1',
+                      'storagetype:workload': 'DSS',
+                      'storagetype:slo': 'Bronze'}
+        controllerConfigService = (
+            self.driver.utils.find_controller_configuration_service(
+                conn, self.data.storage_system))
+        masking.provisionv3.create_storage_group_v3 = mock.Mock(
+            return_value={'Value'})
+        masking._is_volume_in_storage_group = mock.Mock(
+            return_value=True)
+        masking.return_volume_to_default_storage_group_v3 = mock.Mock()
+        masking._return_back_to_default_sg(
+            conn, controllerConfigService, volumeInstance, volumeName,
+            extraSpecs)
+        masking.return_volume_to_default_storage_group_v3.assert_called_with(
+            conn, controllerConfigService,
+            volumeInstance, volumeName, extraSpecs)
+
+    def test_return_volume_to_default_storage_group_v3_exception(self):
+        masking = self.driver.common.masking
+        conn = self.fake_ecom_connection()
+        volumeInstanceName = (
+            conn.EnumerateInstanceNames("EMC_StorageVolume")[0])
+        volumeInstance = conn.GetInstance(volumeInstanceName)
+        volumeName = "V3-Vol"
+        extraSpecs = {'volume_backend_name': 'V3_BE',
+                      'isV3': True,
+                      'storagetype:pool': 'SRP_1',
+                      'storagetype:workload': 'DSS',
+                      'storagetype:slo': 'Bronze'}
+        controllerConfigService = (
+            self.driver.utils.find_controller_configuration_service(
+                conn, self.data.storage_system))
+
+        self.assertRaises(
+            exception.VolumeBackendAPIException,
+            masking.return_volume_to_default_storage_group_v3,
+            conn, controllerConfigService,
+            volumeInstance, volumeName, extraSpecs)
+
+    def test_add_volume_to_sg_and_verify(self):
+        masking = self.driver.common.masking
+        conn = self.fake_ecom_connection()
+        volumeInstanceName = (
+            conn.EnumerateInstanceNames("EMC_StorageVolume")[0])
+        volumeInstance = conn.GetInstance(volumeInstanceName)
+        volumeName = "V3-Vol"
+        storageGroupInstanceName = self.data.storagegroups[0]
+        sgGroupName = self.data.storagegroupname
+        extraSpecs = {'volume_backend_name': 'V3_BE',
+                      'isV3': True,
+                      'storagetype:pool': 'SRP_1',
+                      'storagetype:workload': 'DSS',
+                      'storagetype:slo': 'Bronze'}
+        controllerConfigService = (
+            self.driver.utils.find_controller_configuration_service(
+                conn, self.data.storage_system))
+        msg = masking._add_volume_to_sg_and_verify(
+            conn, controllerConfigService, storageGroupInstanceName,
+            volumeInstance, volumeName, sgGroupName, extraSpecs)
+        self.assertIsNone(msg)
+
+    def test_remove_volume_from_sg(self):
+        masking = self.driver.common.masking
+        conn = self.fake_ecom_connection()
+        volumeInstanceName = (
+            conn.EnumerateInstanceNames("EMC_StorageVolume")[0])
+        volumeInstance = conn.GetInstance(volumeInstanceName)
+        storageGroupInstanceName = self.data.storagegroups[1]
+        extraSpecs = {'volume_backend_name': 'V3_BE',
+                      'isV3': True,
+                      'storagetype:pool': 'SRP_1',
+                      'storagetype:workload': 'DSS',
+                      'storagetype:slo': 'Bronze'}
+        controllerConfigService = (
+            self.driver.utils.find_controller_configuration_service(
+                conn, self.data.storage_system))
+        masking._remove_volume_from_sg = mock.Mock()
+        masking._cleanup_deletion_v3(
+            conn, controllerConfigService, volumeInstance, extraSpecs)
+        masking._remove_volume_from_sg.assert_called_with(
+            conn, controllerConfigService, storageGroupInstanceName,
+            volumeInstance, extraSpecs)
+
 
 class EMCVMAXFCTest(test.TestCase):
     def setUp(self):

cinder/volume/drivers/emc/emc_vmax_common.py

@@ -414,7 +414,7 @@ class EMCVMAXCommon(object):
             return deviceInfoDict
 
     def _attach_volume(self, volume, connector, extraSpecs,
-                       maskingViewDict, isLiveMigration=None):
+                       maskingViewDict, isLiveMigration=False):
         """Attach a volume to a host.
 
         If live migration is being undertaken then the volume
@@ -2198,12 +2198,9 @@ class EMCVMAXCommon(object):
         deviceId = volumeInstance['DeviceID']
 
         if extraSpecs[ISV3]:
-            storageGroupName = self.utils.get_v3_storage_group_name(
-                extraSpecs[POOL], extraSpecs[SLO],
-                extraSpecs[WORKLOAD])
             rc = self._delete_from_pool_v3(
                 storageConfigService, volumeInstance, volumeName,
-                deviceId, storageGroupName, extraSpecs)
+                deviceId, extraSpecs)
         else:
             rc = self._delete_from_pool(storageConfigService, volumeInstance,
                                         volumeName, deviceId,
@@ -3432,15 +3429,13 @@ class EMCVMAXCommon(object):
         return rc
 
     def _delete_from_pool_v3(self, storageConfigService, volumeInstance,
-                             volumeName, deviceId, storageGroupName,
+                             volumeName, deviceId, extraSpecs):
-                             extraSpecs):
         """Delete from pool (v3).
 
         :param storageConfigService: the storage config service
         :param volumeInstance: the volume instance
         :param volumeName: the volume Name
         :param deviceId: the device ID of the volume
-        :param storageGroupName: the name of the default SG
         :param extraSpecs: extra specifications
         :returns: int -- return code
         :raises: VolumeBackendAPIException
@@ -3452,9 +3447,9 @@ class EMCVMAXCommon(object):
 
         # Check if it is part of a storage group and delete it
         # extra logic for case when volume is the last member.
-        sgFromVolInstanceName = self.masking.remove_and_reset_members(
+        self.masking.remove_and_reset_members(
             self.conn, controllerConfigurationService, volumeInstance,
-            volumeName, extraSpecs, None, 'noReset')
+            volumeName, extraSpecs, None, False)
 
         LOG.debug("Delete Volume: %(name)s  Method: EMCReturnToStoragePool "
                   "ConfigServic: %(service)s  TheElement: %(vol_instance)s "
@@ -3472,23 +3467,9 @@ class EMCVMAXCommon(object):
             # If we cannot successfully delete the volume, then we want to
             # return the volume to the default storage group,
             # which should be the SG it previously belonged to.
-            storageGroupInstanceName = self.utils.find_storage_masking_group(
+            self.masking.return_volume_to_default_storage_group_v3(
-                self.conn, controllerConfigurationService, storageGroupName)
+                self.conn, controllerConfigurationService,
-
+                volumeInstance, volumeName, extraSpecs)
-            if sgFromVolInstanceName is not storageGroupInstanceName:
-                LOG.debug(
-                    "Volume: %(volumeName)s was not previously part of "
-                    " %(storageGroupInstanceName)s. "
-                    "Returning to %(storageGroupName)s.",
-                    {'volumeName': volumeName,
-                     'storageGroupInstanceName': storageGroupInstanceName,
-                     'storageGroupName': storageGroupName})
-
-            if storageGroupInstanceName is not None:
-                self.masking.add_volume_to_storage_group(
-                    self.conn, controllerConfigurationService,
-                    storageGroupInstanceName, volumeInstance, volumeName,
-                    storageGroupName, extraSpecs)
 
             errorMessage = (_("Failed to delete volume %(volumeName)s.") %
                             {'volumeName': volumeName})
@@ -3854,12 +3835,9 @@ class EMCVMAXCommon(object):
                     self.conn, storageSystem))
             deviceId = targetInstance['DeviceID']
             volumeName = targetInstance['Name']
-            storageGroupName = self.utils.get_v3_storage_group_name(
-                extraSpecs[POOL], extraSpecs[SLO],
-                extraSpecs[WORKLOAD])
             rc = self._delete_from_pool_v3(
                 storageConfigService, targetInstance, volumeName,
-                deviceId, storageGroupName, extraSpecs)
+                deviceId, extraSpecs)
             # Re-throw the exception.
             raise
 

cinder/volume/drivers/emc/emc_vmax_masking.py

@@ -519,34 +519,53 @@ class EMCVMAXMasking(object):
         if self._is_volume_in_storage_group(
                 conn, storageGroupInstanceName,
                 volumeInstance, sgGroupName):
-            LOG.debug(
+            LOG.warning(_LW(
                 "Volume: %(volumeName)s is already part "
-                "of storage group %(sgGroupName)s.",
+                "of storage group %(sgGroupName)s."),
                 {'volumeName': volumeName,
                  'sgGroupName': sgGroupName})
         else:
-            self.add_volume_to_storage_group(
+            msg = self._add_volume_to_sg_and_verify(
-                conn, controllerConfigService,
+                conn, controllerConfigService, storageGroupInstanceName,
-                storageGroupInstanceName, volumeInstance, volumeName,
+                volumeInstance, volumeName, sgGroupName,
-                sgGroupName, maskingViewDict['extraSpecs'])
+                maskingViewDict['extraSpecs'])
-            if not self._is_volume_in_storage_group(
-                    conn, storageGroupInstanceName,
-                    volumeInstance, sgGroupName):
-                # This may be used in exception hence _ instead of _LE.
-                msg = (_(
-                    "Volume: %(volumeName)s was not added "
-                    "to storage group %(sgGroupName)s. ") %
-                    {'volumeName': volumeName,
-                     'sgGroupName': sgGroupName})
-                LOG.error(msg)
-            else:
-                LOG.info(_LI(
-                    "Successfully added %(volumeName)s to %(sgGroupName)s."),
-                    {'volumeName': volumeName,
-                     'sgGroupName': sgGroupName})
 
         return msg
 
+    def _add_volume_to_sg_and_verify(
+            self, conn, controllerConfigService, storageGroupInstanceName,
+            volumeInstance, volumeName, sgGroupName, extraSpecs):
+        """Add the volume to the storage group and double check it is there.
+
+        :param conn: the ecom connection
+        :param controllerConfigService: controller service
+        :param storageGroupInstanceName: storage group instance name
+        :param volumeInstance: the volume instance
+        :param volumeName: the volume name
+        :param sgGroupName: the storage group name
+        :param extraSpecs: the extra specifications
+        :returns: string -- the error message
+        """
+        msg = None
+        self.add_volume_to_storage_group(
+            conn, controllerConfigService, storageGroupInstanceName,
+            volumeInstance, volumeName, sgGroupName, extraSpecs)
+        if not self._is_volume_in_storage_group(
+                conn, storageGroupInstanceName, volumeInstance, sgGroupName):
+            # This may be used in exception hence _ instead of _LE.
+            msg = (_(
+                "Volume: %(volumeName)s was not added "
+                "to storage group %(sgGroupName)s.") %
+                {'volumeName': volumeName,
+                 'sgGroupName': sgGroupName})
+            LOG.error(msg)
+        else:
+            LOG.info(_LI("Successfully added %(volumeName)s to "
+                         "%(sgGroupName)s."),
+                     {'volumeName': volumeName,
+                      'sgGroupName': sgGroupName})
+        return msg
+
     def _get_and_remove_from_storage_group_v2(
             self, conn, controllerConfigService, volumeInstanceName,
             volumeName, fastPolicyName, extraSpecs):
@@ -1747,14 +1766,11 @@ class EMCVMAXMasking(object):
 
     def remove_and_reset_members(
             self, conn, controllerConfigService, volumeInstance,
-            volumeName, extraSpecs, connector=None, noReset=None):
+            volumeName, extraSpecs, connector=None, reset=True):
-        """Part of unmap device or rollback.
+        """This is called on a delete, unmap device or rollback.
 
-        Removes volume from the Device Masking Group that belongs to a
+        If the connector is not None get the associated SG and remove volume
-        Masking View. Check if fast policy is in the extra specs, if it isn't
+        from the storage group, otherwise it is a VMAX3 deletion.
-        we do not need to do any thing for FAST. Assume that
-        isTieringPolicySupported is False unless the FAST policy is in
-        the extra specs and tiering is enabled on the array.
 
         :param conn: connection the the ecom server
         :param controllerConfigService: the controller configuration service
@@ -1762,33 +1778,72 @@ class EMCVMAXMasking(object):
         :param volumeName: the volume name
         :param extraSpecs: additional info
         :param connector: optional
-        :param noReset: optional, if none, then reset
+        :param reset: reset, return to original SG (optional)
         :returns: storageGroupInstanceName
         """
-        fastPolicyName = extraSpecs.get(FASTPOLICY, None)
-        isV3 = extraSpecs[ISV3]
         storageGroupInstanceName = None
         if connector is not None:
             storageGroupInstanceName = self._get_sg_associated_with_connector(
                 conn, controllerConfigService, volumeInstance.path,
                 volumeName, connector)
-            if storageGroupInstanceName is None:
+            if storageGroupInstanceName:
-                return None
+                self._remove_volume_from_sg(
+                    conn, controllerConfigService, storageGroupInstanceName,
+                    volumeInstance, extraSpecs)
         else:  # Connector is None in V3 volume deletion case.
-            storageGroupInstanceNames = (
+            self._cleanup_deletion_v3(
-                self.get_associated_masking_groups_from_device(
+                conn, controllerConfigService, volumeInstance, extraSpecs)
-                    conn, volumeInstance.path))
+        if reset:
-            if storageGroupInstanceNames:
+            self._return_back_to_default_sg(
-                storageGroupInstanceName = storageGroupInstanceNames[0]
+                conn, controllerConfigService, volumeInstance, volumeName,
-            else:
+                extraSpecs)
-                return None
+
+        return storageGroupInstanceName
+
+    def _cleanup_deletion_v3(
+            self, conn, controllerConfigService, volumeInstance, extraSpecs):
+        """Pre cleanup before VMAX3 deletion operation
+
+        :param conn: the ecom connection
+        :param controllerConfigService: storage system instance name
+        :param volumeInstance: the volume instance
+        :param extraSpecs: the extra specifications
+        """
+        storageGroupInstanceNames = (
+            self.get_associated_masking_groups_from_device(
+                conn, volumeInstance.path))
+
+        if storageGroupInstanceNames:
+            sgNum = len(storageGroupInstanceNames)
+            if len(storageGroupInstanceNames) > 1:
+                LOG.warning(_LW("Volume %(volumeName)s is belong to "
+                                "%(sgNum)s storage groups."),
+                            {'volumeName': volumeInstance['ElementName'],
+                             'sgNum': sgNum})
+            for storageGroupInstanceName in storageGroupInstanceNames:
+                self._remove_volume_from_sg(
+                    conn, controllerConfigService,
+                    storageGroupInstanceName,
+                    volumeInstance,
+                    extraSpecs)
+
+    def _remove_volume_from_sg(
+            self, conn, controllerConfigService, storageGroupInstanceName,
+            volumeInstance, extraSpecs):
+
+        """Remove volume from storage group
+
+        :param conn: the ecom connection
+        :param controllerConfigService: storage system instance name
+        :param storageGroupInstanceName: the SG instance name
+        :param volumeInstance: the volume instance
+        :param extraSpecs: the extra specifications
+        """
         instance = conn.GetInstance(storageGroupInstanceName, LocalOnly=False)
         storageGroupName = instance['ElementName']
 
         volumeInstanceNames = self.get_devices_from_storage_group(
             conn, storageGroupInstanceName)
-        storageSystemInstanceName = self.utils.find_storage_system(
-            conn, controllerConfigService)
 
         numVolInStorageGroup = len(volumeInstanceNames)
         LOG.debug(
@@ -1797,25 +1852,18 @@ class EMCVMAXMasking(object):
             {'numVol': numVolInStorageGroup,
              'maskingGroup': storageGroupInstanceName})
 
-        if not isV3:
-            isTieringPolicySupported, __ = (
-                self._get_tiering_info(conn, storageSystemInstanceName,
-                                       fastPolicyName))
-
         if numVolInStorageGroup == 1:
             # Last volume in the storage group.
             self._last_vol_in_SG(
                 conn, controllerConfigService, storageGroupInstanceName,
-                storageGroupName, volumeInstance, volumeName, extraSpecs)
+                storageGroupName, volumeInstance,
+                volumeInstance['ElementName'], extraSpecs)
         else:
-            # Not the last volume so remove it from storage group in
+            # Not the last volume so remove it from storage group
-            # the masking view.
             self._multiple_vols_in_SG(
                 conn, controllerConfigService, storageGroupInstanceName,
-                storageGroupName, volumeInstance, volumeName,
+                volumeInstance, volumeInstance['ElementName'],
-                numVolInStorageGroup, fastPolicyName, extraSpecs)
+                numVolInStorageGroup, extraSpecs)
-
-        return storageGroupInstanceName
 
     def _last_vol_in_SG(
             self, conn, controllerConfigService, storageGroupInstanceName,
@@ -1881,29 +1929,20 @@ class EMCVMAXMasking(object):
 
     def _multiple_vols_in_SG(
             self, conn, controllerConfigService, storageGroupInstanceName,
-            storageGroupName, volumeInstance, volumeName, numVolsInSG,
+            volumeInstance, volumeName, numVolsInSG, extraSpecs):
-            fastPolicyName, extraSpecs):
+        """If the volume is not the last in the storage group
-        """Necessary steps if the volume is not the last in the SG.
 
-        1. Remove the volume from the SG.
+        Remove the volume from the SG.
-        2. Return the volume to default SG if necessary.
 
         :param conn: the ecom connection
         :param controllerConfigService: storage system instance name
         :param storageGroupInstanceName: the SG instance name
-        :param storageGroupName: the Storage group name (String)
         :param volumeInstance: the volume instance
         :param volumeName: the volume name
         :param numVolsInSG: the number of volumes in the SG
-        :param fastPolicyName: the FAST policy name
         :param extraSpecs: the extra specifications
         """
-        storageSystemInstanceName = self.utils.find_storage_system(
+
-            conn, controllerConfigService)
-        if not extraSpecs[ISV3]:
-            isTieringPolicySupported, __ = (
-                self._get_tiering_info(conn, storageSystemInstanceName,
-                                       fastPolicyName))
         LOG.debug("Start: number of volumes in masking storage group: "
                   "%(numVol)d", {'numVol': numVolsInSG})
         self.provision.remove_device_from_storage_group(
@@ -1914,19 +1953,6 @@ class EMCVMAXMasking(object):
             "RemoveMembers for volume %(volumeName)s completed "
             "successfully.", {'volumeName': volumeName})
 
-        # Add it back to the default storage group.
-        if extraSpecs[ISV3]:
-            self._return_volume_to_default_storage_group_v3(
-                conn, controllerConfigService, storageGroupName,
-                volumeInstance, volumeName, storageSystemInstanceName,
-                extraSpecs)
-        else:
-            # V2 if FAST POLICY enabled, move the volume to the default SG.
-            if fastPolicyName is not None and isTieringPolicySupported:
-                self._cleanup_tiering(
-                    conn, controllerConfigService, fastPolicyName,
-                    volumeInstance, volumeName, extraSpecs)
-
         volumeInstanceNames = self.get_devices_from_storage_group(
             conn, storageGroupInstanceName)
         LOG.debug(
@@ -1939,10 +1965,6 @@ class EMCVMAXMasking(object):
             volumeInstance, volumeName, extraSpecs):
         """Delete the Masking view, the storage Group and  the initiator group.
 
-        Also does necessary cleanup like removing the policy from the
-        storage group for V2 and returning the volume to the default
-        storage group.
-
         :param conn: connection the the ecom server
         :param controllerConfigService: the controller configuration service
         :param mvInstanceName: masking view instance name
@@ -1978,56 +2000,49 @@ class EMCVMAXMasking(object):
                 storageSystemInstanceName['Name'],
                 storageGroupInstanceName, extraSpecs)
 
-        self._cleanup_last_vol(
-            conn, controllerConfigService, storageGroupInstanceName,
-            storageGroupName, volumeInstance, volumeName,
-            storageSystemInstanceName, isTieringPolicySupported, extraSpecs)
-
-    def _cleanup_last_vol(
-            self, conn, controllerConfigService, storageGroupInstanceName,
-            storageGroupName, volumeInstance, volumeName,
-            storageSystemInstanceName, isTieringPolicySupported, extraSpecs):
-        """Do necessary cleanup when the volume is the last in the SG.
-
-        This includes removing the last volume from the SG and deleting the
-        SG.  It also means moving the volume to the default SG for VMAX3 and
-        FAST for VMAX2.
-
-        :param conn: connection the the ecom server
-        :param controllerConfigService: the controller configuration service
-        :param storageGroupInstanceName: storage group instance name
-        :param storageGroupName: the storage group name
-        :param volumeInstance: the volume Instance
-        :param volumeName: the volume name
-        :param storageSystemInstanceName: the storage system instance name
-        :param isTieringPolicySupported: tiering policy supported flag
-        :param extraSpecs: extra specs
-        """
-        # Remove the last volume and delete the storage group.
         self._remove_last_vol_and_delete_sg(
             conn, controllerConfigService, storageGroupInstanceName,
             storageGroupName, volumeInstance.path, volumeName,
             extraSpecs)
+
+        LOG.debug(
+            "Volume %(volumeName)s successfully removed from SG and "
+            "Storage Group %(storageGroupName)s successfully deleted. ",
+            {'volumeName': volumeName,
+             'storageGroupName': storageGroupName})
+
+    def _return_back_to_default_sg(
+            self, conn, controllerConfigService, volumeInstance, volumeName,
+            extraSpecs):
+        """Return volume to default storage group
+
+        Moving the volume to the default SG for VMAX3 and
+        FAST for VMAX2.
+
+        :param conn: connection the the ecom server
+        :param controllerConfigService: the controller configuration service
+        :param volumeInstance: the volume Instance
+        :param volumeName: the volume name
+        :param extraSpecs: extra specs
+        """
         # Add it back to the default storage group.
         if extraSpecs[ISV3]:
-            self._return_volume_to_default_storage_group_v3(
+            self.return_volume_to_default_storage_group_v3(
-                conn, controllerConfigService, storageGroupName,
+                conn, controllerConfigService,
-                volumeInstance, volumeName, storageSystemInstanceName,
+                volumeInstance, volumeName, extraSpecs)
-                extraSpecs)
         else:
             # V2 if FAST POLICY enabled, move the volume to the default
             # SG.
             fastPolicyName = extraSpecs.get(FASTPOLICY, None)
+            storageSystemInstanceName = self.utils.find_storage_system(
+                conn, controllerConfigService)
+            isTieringPolicySupported, __ = (
+                self._get_tiering_info(conn, storageSystemInstanceName,
+                                       fastPolicyName))
             if fastPolicyName is not None and isTieringPolicySupported:
                 self._cleanup_tiering(
                     conn, controllerConfigService, fastPolicyName,
                     volumeInstance, volumeName, extraSpecs)
-        LOG.debug(
-            "Volume %(volumeName)s successfully removed from SG and "
-            "returned to default storage group where applicable. "
-            "Storage Group %(storageGroupName)s successfully deleted. ",
-            {'volumeName': volumeName,
-             'storageGroupName': storageGroupName})
 
     def _get_sg_associated_with_connector(
             self, conn, controllerConfigService, volumeInstanceName,
@@ -2141,51 +2156,41 @@ class EMCVMAXMasking(object):
                 conn, tierPolicyServiceInstanceName,
                 storageGroupInstanceName, tierPolicyInstanceName, extraSpecs)
 
-    def _return_volume_to_default_storage_group_v3(
+    def return_volume_to_default_storage_group_v3(
-            self, conn, controllerConfigService, storageGroupName,
+            self, conn, controllerConfigurationService,
-            volumeInstance, volumeName, storageSystemInstanceName,
+            volumeInstance, volumeName, extraSpecs):
-            extraSpecs):
         """Return volume to the default storage group in v3.
 
         :param conn: the ecom connection
         :param controllerConfigService: controller config service
-        :param storageGroupName: storage group name
         :param volumeInstance: volumeInstance
         :param volumeName: the volume name
-        :param storageSystemInstanceName: the storage system instance name
         :param extraSpecs: additional info
         :raises: VolumeBackendAPIException
         """
-        # First strip the shortHostname from the storage group name.
+        storageGroupName = self.utils.get_v3_storage_group_name(
-        defaultStorageGroupName, shorthostName = (
+            extraSpecs[self.utils.POOL], extraSpecs[self.utils.SLO],
-            self.utils.strip_short_host_name(storageGroupName))
+            extraSpecs[self.utils.WORKLOAD])
+        storageGroupInstanceName = self.utils.find_storage_masking_group(
+            conn, controllerConfigurationService, storageGroupName)
 
-        # Check if host name exists which signifies detach operation.
+        if not storageGroupInstanceName:
-        if shorthostName is not None:
+            storageGroupInstanceName = (
-            # Populate maskingViewDict and storageGroupInstanceName.
+                self.provisionv3.create_storage_group_v3(
-            maskingViewDict = {}
+                    conn, controllerConfigurationService, storageGroupName,
-            maskingViewDict['sgGroupName'] = defaultStorageGroupName
+                    extraSpecs[self.utils.POOL], extraSpecs[self.utils.SLO],
-            maskingViewDict['volumeInstance'] = volumeInstance
+                    extraSpecs[self.utils.WORKLOAD], extraSpecs))
-            maskingViewDict['volumeName'] = volumeName
+            if not storageGroupInstanceName:
-            maskingViewDict['controllerConfigService'] = (
+                errorMessage = (_("Failed to create storage group "
-                controllerConfigService)
+                                  "%(storageGroupName)s.") %
-            maskingViewDict['storageSystemName'] = (
+                                {'storageGroupName': storageGroupName})
-                storageSystemInstanceName)
-            sgInstanceName = self.utils.find_storage_masking_group(
-                conn, controllerConfigService, defaultStorageGroupName)
-            if sgInstanceName is not None:
-                errorMessage = (
-                    self._check_adding_volume_to_storage_group(
-                        conn, maskingViewDict,
-                        sgInstanceName))
-            else:
-                errorMessage = (_(
-                    "Storage group %(sgGroupName)s "
-                    "does not exist.")
-                    % {'sgGroupName': defaultStorageGroupName})
                 LOG.error(errorMessage)
-                raise exception.VolumeBackendAPIException(
+                raise exception.VolumeBackendAPIException(data=errorMessage)
-                    data=errorMessage)
+
+        self._add_volume_to_sg_and_verify(
+            conn, controllerConfigurationService,
+            storageGroupInstanceName, volumeInstance, volumeName,
+            storageGroupName, extraSpecs)
 
     def _cleanup_tiering(
             self, conn, controllerConfigService, fastPolicyName,

cinder/volume/drivers/emc/emc_vmax_utils.py

@@ -64,6 +64,9 @@ class EMCVMAXUtils(object):
     This Utility class is for EMC volume drivers based on SMI-S.
     It supports VMAX arrays.
     """
+    SLO = 'storagetype:slo'
+    WORKLOAD = 'storagetype:workload'
+    POOL = 'storagetype:pool'
 
     def __init__(self, prtcl):
         if not pywbemAvailable:
@@ -1528,16 +1531,6 @@ class EMCVMAXUtils(object):
                                'workload': workload})
         return storageGroupName
 
-    def strip_short_host_name(self, storageGroupName):
-        tempList = storageGroupName.split("-")
-        if len(tempList) == 6:
-            shorthostName = tempList.pop(1)
-            updatedStorageGroup = "-".join(tempList)
-            return updatedStorageGroup, shorthostName
-        else:
-            shorthostName = None
-            return storageGroupName, shorthostName
-
     def _get_fast_settings_from_storage_group(self, storageGroupInstance):
         """Get the emc FAST setting from the storage group.