From e62eaccf7a74a786ce92f22620f840eaeb145d1f Mon Sep 17 00:00:00 2001 From: Xing Yang Date: Mon, 19 Jan 2015 13:27:31 -0500 Subject: [PATCH] Roll back if VMAX masking view not created In emc_vmax_masking.py, an exception object is created if there is a failure in the masking view creation logic. However, the exception is not raised. As a result, initialize_connection will succeed in spite of a failure but the VM will not be able to access the disk. This patch adds rollback logic to handle failures in creating masking view. If the masking view cannot be created, the volume will be added back to the default storage group and an exception will be raised. Closes-Bug: #1393540 Change-Id: I1a00d8922cc876a943fb7822cd86c580d8e6beac --- cinder/tests/test_emc_vmax.py | 95 ++- cinder/volume/drivers/emc/emc_vmax_common.py | 15 +- cinder/volume/drivers/emc/emc_vmax_masking.py | 720 ++++++++++++------ 3 files changed, 552 insertions(+), 278 deletions(-) diff --git a/cinder/tests/test_emc_vmax.py b/cinder/tests/test_emc_vmax.py index b4c3cb08f..f345f5452 100644 --- a/cinder/tests/test_emc_vmax.py +++ b/cinder/tests/test_emc_vmax.py @@ -31,6 +31,7 @@ from cinder.volume.drivers.emc.emc_vmax_fast import EMCVMAXFast from cinder.volume.drivers.emc.emc_vmax_fc import EMCVMAXFCDriver from cinder.volume.drivers.emc.emc_vmax_iscsi import EMCVMAXISCSIDriver from cinder.volume.drivers.emc.emc_vmax_masking import EMCVMAXMasking +from cinder.volume.drivers.emc.emc_vmax_provision import EMCVMAXProvision from cinder.volume.drivers.emc.emc_vmax_utils import EMCVMAXUtils from cinder.volume import volume_types @@ -338,7 +339,9 @@ class FakeEcomConnection(): HardwareId=None, ElementSource=None, EMCInPools=None, CompositeType=None, EMCNumberOfMembers=None, EMCBindElements=None, - InElements=None, TargetPool=None, RequestedState=None): + InElements=None, TargetPool=None, RequestedState=None, + GroupName=None, Type=None, InitiatorMaskingGroup=None, + DeviceMaskingGroup=None, TargetMaskingGroup=None): rc = 0L myjob = SE_ConcreteJob() @@ -481,6 +484,8 @@ class FakeEcomConnection(): result = self._assoc_endpoint() elif ResultClass == 'EMC_StorageVolume': result = self._assoc_storagevolume(objectpath) + elif ResultClass == 'Symm_LunMaskingView': + result = self._assoc_maskingview() elif ResultClass == 'CIM_DeviceMaskingGroup': result = self._assoc_storagegroup() elif ResultClass == 'CIM_StorageExtent': @@ -674,6 +679,20 @@ class FakeEcomConnection(): assocs.append(assoc) return assocs + def _assoc_maskingview(self): + assocs = [] + assoc = SYMM_LunMasking() + assoc['Name'] = 'myMaskingView' + assoc['SystemName'] = self.data.storage_system + assoc['CreationClassName'] = 'Symm_LunMaskingView' + assoc['DeviceID'] = '1234' + assoc['SystemCreationClassName'] = '1234' + assoc['ElementName'] = 'OS-fakehost-gold-I-MV' + assoc.classname = assoc['CreationClassName'] + assoc.path = assoc + assocs.append(assoc) + return assocs + def _default_assoc(self, objectpath): return objectpath @@ -1168,7 +1187,7 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase): ecompassword.appendChild(ecompasswordtext) portgroup = doc.createElement("PortGroup") - portgrouptext = doc.createTextNode("myPortGroup") + portgrouptext = doc.createTextNode(self.data.port_group) portgroup.appendChild(portgrouptext) portgroups = doc.createElement("PortGroups") @@ -1741,10 +1760,16 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase): EMCVMAXUtils, 'find_storage_masking_group', return_value='value') - def test_map_new_masking_view_no_fast_success(self, _mock_volume_type, - mock_wrap_group, + @mock.patch.object( + EMCVMAXMasking, + '_check_adding_volume_to_storage_group', + return_value=None) + def test_map_new_masking_view_no_fast_success(self, + mock_check, + mock_storage_group, mock_wrap_device, - mock_storage_group): + mock_wrap_group, + mock_volume_type): self.driver.initialize_connection(self.data.test_volume, self.data.connector) @@ -1769,11 +1794,17 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase): EMCVMAXCommon, '_is_same_host', return_value=False) - def test_map_live_migration_no_fast_success(self, _mock_volume_type, - mock_wrap_group, - mock_wrap_device, + @mock.patch.object( + EMCVMAXMasking, + '_check_adding_volume_to_storage_group', + return_value=None) + def test_map_live_migration_no_fast_success(self, + mock_check, + mock_same_host, mock_storage_group, - mock_same_host): + mock_wrap_device, + mock_wrap_group, + mock_volume_type): self.driver.initialize_connection(self.data.test_volume, self.data.connector) @@ -1790,9 +1821,15 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase): '_wrap_find_device_number', return_value={'hostlunid': 1, 'storagesystem': EMCVMAXCommonData.storage_system}) - def test_already_mapped_no_fast_success(self, _mock_volume_type, + @mock.patch.object( + EMCVMAXCommon, + '_is_same_host', + return_value=True) + def test_already_mapped_no_fast_success(self, + mock_same_host, + mock_wrap_device, mock_wrap_group, - mock_wrap_device): + mock_volume_type): self.driver.initialize_connection(self.data.test_volume, self.data.connector) @@ -2077,7 +2114,7 @@ class EMCVMAXISCSIDriverFastTestCase(test.TestCase): timeout.appendChild(timeouttext) portgroup = doc.createElement("PortGroup") - portgrouptext = doc.createTextNode("myPortGroup") + portgrouptext = doc.createTextNode(self.data.port_group) portgroup.appendChild(portgrouptext) pool = doc.createElement("Pool") @@ -2257,8 +2294,12 @@ class EMCVMAXISCSIDriverFastTestCase(test.TestCase): '_wrap_find_device_number', return_value={'hostlunid': 1, 'storagesystem': EMCVMAXCommonData.storage_system}) - def test_map_fast_success(self, _mock_volume_type, mock_wrap_group, - mock_wrap_device): + @mock.patch.object( + EMCVMAXCommon, + '_is_same_host', + return_value=True) + def test_map_fast_success(self, mock_same_host, mock_wrap_device, + mock_wrap_group, mock_volume_type): self.driver.initialize_connection(self.data.test_volume, self.data.connector) @@ -2595,7 +2636,7 @@ class EMCVMAXFCDriverNoFastTestCase(test.TestCase): ecompassword.appendChild(ecompasswordtext) portgroup = doc.createElement("PortGroup") - portgrouptext = doc.createTextNode("myPortGroup") + portgrouptext = doc.createTextNode(self.data.port_group) portgroup.appendChild(portgrouptext) portgroups = doc.createElement("PortGroups") @@ -2746,8 +2787,17 @@ class EMCVMAXFCDriverNoFastTestCase(test.TestCase): EMCVMAXMasking, 'get_masking_view_from_storage_group', return_value=EMCVMAXCommonData.lunmaskctrl_name) + @mock.patch.object( + EMCVMAXProvision, + '_find_new_storage_group', + return_value='Any') + @mock.patch.object( + EMCVMAXMasking, + '_check_adding_volume_to_storage_group', + return_value=None) def test_map_lookup_service_no_fast_success( - self, _mock_volume_type, mock_maskingview): + self, mock_add_check, mock_new_sg, + mock_maskingview, mock_volume_type): self.data.test_volume['volume_name'] = "vmax-1234567" common = self.driver.common common.get_target_wwns_from_masking_view = mock.Mock( @@ -2930,7 +2980,7 @@ class EMCVMAXFCDriverFastTestCase(test.TestCase): ecompassword.appendChild(ecompasswordtext) portgroup = doc.createElement("PortGroup") - portgrouptext = doc.createTextNode("myPortGroup") + portgrouptext = doc.createTextNode(self.data.port_group) portgroup.appendChild(portgrouptext) pool = doc.createElement("Pool") @@ -3106,7 +3156,16 @@ class EMCVMAXFCDriverFastTestCase(test.TestCase): EMCVMAXMasking, 'get_masking_view_from_storage_group', return_value=EMCVMAXCommonData.lunmaskctrl_name) - def test_map_fast_success(self, _mock_volume_type, mock_maskingview): + @mock.patch.object( + EMCVMAXProvision, + '_find_new_storage_group', + return_value='Any') + @mock.patch.object( + EMCVMAXMasking, + '_check_adding_volume_to_storage_group', + return_value=None) + def test_map_fast_success(self, mock_add_check, mock_new_sg, + mock_maskingview, mock_volume_type): self.data.test_volume['volume_name'] = "vmax-1234567" common = self.driver.common common.get_target_wwns = mock.Mock( diff --git a/cinder/volume/drivers/emc/emc_vmax_common.py b/cinder/volume/drivers/emc/emc_vmax_common.py index 35ca54ec7..003419658 100644 --- a/cinder/volume/drivers/emc/emc_vmax_common.py +++ b/cinder/volume/drivers/emc/emc_vmax_common.py @@ -327,15 +327,9 @@ class EMCVMAXCommon(object): LOG.error(_LE("Error Attaching volume %(vol)s.") % {'vol': volumeName}) if rollbackDict['fastPolicyName'] is not None: - ( - self.masking - ._check_if_rollback_action_for_masking_required( - self.conn, - rollbackDict['controllerConfigService'], - rollbackDict['volumeInstance'], - rollbackDict['volumeName'], - rollbackDict['fastPolicyName'], - rollbackDict['defaultStorageGroupInstanceName'])) + (self.masking + .check_if_rollback_action_for_masking_required( + self.conn, rollbackDict)) exception_message = ("Error Attaching volume %(vol)s." % {'vol': volumeName}) raise exception.VolumeBackendAPIException( @@ -822,7 +816,7 @@ class EMCVMAXCommon(object): self.utils.find_controller_configuration_service( conn, storageSystemName)) - # check to see what SG it is in + # Check to see what SG it is in assocStorageGroupInstanceName = ( self.utils.get_storage_group_from_volume(conn, volumeInstance.path)) @@ -1525,7 +1519,6 @@ class EMCVMAXCommon(object): extraSpecs[MEMBERCOUNT] = memberCount extraSpecs[COMPOSITETYPE] = CONCATENATED LOG.debug("StripedMetaCount is not in the extra specs") - pass poolName = self.utils.parse_pool_name_from_file(configurationFile) if poolName is None: diff --git a/cinder/volume/drivers/emc/emc_vmax_masking.py b/cinder/volume/drivers/emc/emc_vmax_masking.py index 4dca2a324..4726c0caf 100644 --- a/cinder/volume/drivers/emc/emc_vmax_masking.py +++ b/cinder/volume/drivers/emc/emc_vmax_masking.py @@ -50,7 +50,7 @@ class EMCVMAXMasking(object): """Get or Create a masking view. Given a masking view tuple either get or create a masking view and add - the volume to the associated storage group + the volume to the associated storage group. :param conn: the connection to ecom :para maskingViewDict: the masking view tuple @@ -59,16 +59,10 @@ class EMCVMAXMasking(object): rollbackDict = {} controllerConfigService = maskingViewDict['controllerConfigService'] - sgGroupName = maskingViewDict['sgGroupName'] volumeInstance = maskingViewDict['volumeInstance'] - igGroupName = maskingViewDict['igGroupName'] - connector = maskingViewDict['connector'] - storageSystemName = maskingViewDict['storageSystemName'] maskingViewName = maskingViewDict['maskingViewName'] volumeName = maskingViewDict['volumeName'] - pgGroupName = maskingViewDict['pgGroupName'] isLiveMigration = maskingViewDict['isLiveMigration'] - fastPolicyName = maskingViewDict['fastPolicy'] defaultStorageGroupInstanceName = None @@ -76,143 +70,32 @@ class EMCVMAXMasking(object): # We need a rollback scenario for FAST. # We must make sure that volume is returned to default storage # group if anything goes wrong. + fastPolicyName = maskingViewDict['fastPolicy'] + # If FAST is enabled remove the volume from the default SG if fastPolicyName is not None: defaultStorageGroupInstanceName = ( - self.fast.get_and_verify_default_storage_group( - conn, controllerConfigService, volumeInstance.path, - volumeName, fastPolicyName)) - if defaultStorageGroupInstanceName is None: - exceptionMessage = (_( - "Cannot get the default storage group for FAST policy:" - " %(fastPolicyName)s.") - % {'fastPolicyName': fastPolicyName}) - LOG.error(exceptionMessage) - raise exception.VolumeBackendAPIException( - data=exceptionMessage) - - retStorageGroupInstanceName = ( - self.remove_device_from_default_storage_group( - conn, controllerConfigService, volumeInstance.path, + self._get_and_remove_from_storage_group_v2( + conn, controllerConfigService, + volumeInstance.path, volumeName, fastPolicyName)) - if retStorageGroupInstanceName is None: - exceptionMessage = (_( - "Failed to remove volume %(volumeName)s from default " - "SG: %(volumeName)s.") - % {'volumeName': volumeName}) - LOG.error(exceptionMessage) - raise exception.VolumeBackendAPIException( - data=exceptionMessage) - - try: - maskingViewInstanceName = self._find_masking_view( - conn, maskingViewName, storageSystemName) - if maskingViewInstanceName is None: - storageGroupInstanceName = ( - self._get_storage_group_instance_name( - conn, controllerConfigService, volumeInstance, - volumeName, sgGroupName, fastPolicyName, - storageSystemName, defaultStorageGroupInstanceName)) - if storageGroupInstanceName is None: - exceptionMessage = (_( - "Cannot get or create a storage group: %(sgGroupName)s" - " for volume %(volumeName)s ") - % {'sgGroupName': sgGroupName, - 'volumeName': volumeName}) - LOG.error(exceptionMessage) - raise - - portGroupInstanceName = self._get_port_group_instance_name( - conn, controllerConfigService, pgGroupName) - if portGroupInstanceName is None: - exceptionMessage = (_( - "Cannot get port group: %(pgGroupName)s. ") - % {'pgGroupName': pgGroupName}) - LOG.error(exceptionMessage) - raise - - initiatorGroupInstanceName = ( - self._get_initiator_group_instance_name( - conn, controllerConfigService, igGroupName, connector, - storageSystemName)) - if initiatorGroupInstanceName is None: - exceptionMessage = (_( - "Cannot get or create initiator group: " - "%(igGroupName)s. ") - % {'igGroupName': igGroupName}) - LOG.error(exceptionMessage) - raise - - maskingViewInstanceName = ( - self._get_masking_view_instance_name( - conn, controllerConfigService, maskingViewName, - storageGroupInstanceName, portGroupInstanceName, - initiatorGroupInstanceName)) - if maskingViewInstanceName is None: - exceptionMessage = (_( - "Cannot create masking view: %(maskingViewName)s. ") - % {'maskingViewName': maskingViewName}) - LOG.error(exceptionMessage) - raise - else: - # first verify that the initiator group matches the initiators - if not self._verify_initiator_group_from_masking_view( - conn, controllerConfigService, maskingViewName, - connector, storageSystemName, igGroupName): - exceptionMessage = (_( - "Unable to verify initiator group: %(igGroupName)s" - "in masking view %(maskingViewName)s ") - % {'igGroupName': igGroupName, - 'maskingViewName': maskingViewName}) - LOG.error(exceptionMessage) - raise - - # get the storage from the masking view and add the - # volume to it. - storageGroupInstanceName = ( - self._get_storage_group_from_masking_view( - conn, maskingViewName, storageSystemName)) - - if storageGroupInstanceName is None: - exceptionMessage = (_( - "Cannot get storage group from masking view: " - "%(maskingViewName)s. ") - % {'maskingViewName': maskingViewName}) - LOG.error(exceptionMessage) - raise - - if self._is_volume_in_storage_group( - conn, storageGroupInstanceName, - volumeInstance): - LOG.warn(_LW( - "Volume: %(volumeName)s is already part " - "of storage group %(sgGroupName)s ") - % {'volumeName': volumeName, - 'sgGroupName': sgGroupName}) - else: - self.add_volume_to_storage_group( - conn, controllerConfigService, - storageGroupInstanceName, volumeInstance, volumeName, - sgGroupName, fastPolicyName, storageSystemName) + # Validate new or existing masking view. + # Return the storage group so we can add the volume to it. + maskingViewInstanceName, storageGroupInstanceName, errorMessage = ( + self._validate_masking_view(conn, maskingViewDict, + defaultStorageGroupInstanceName)) - except Exception as e: - # rollback code if we cannot complete any of the steps above - # successfully then we must roll back by adding the volume back to - # the default storage group for that fast policy - if (fastPolicyName is not None and - defaultStorageGroupInstanceName is not None): - # if the exception happened before the volume was removed from - # the default storage group no action - self._check_if_rollback_action_for_masking_required( - conn, controllerConfigService, volumeInstance, volumeName, - fastPolicyName, defaultStorageGroupInstanceName) + LOG.debug( + "The masking view in the attach operation is " + "%(maskingViewInstanceName)s.", + {'maskingViewInstanceName': maskingViewInstanceName}) - LOG.error(_LE("Exception: %s") % six.text_type(e)) - errorMessage = (_( - "Failed to get or create masking view %(maskingViewName)s ") - % {'maskingViewName': maskingViewName}) - LOG.error(errorMessage) - exception.VolumeBackendAPIException(data=errorMessage) + if not errorMessage: + # Only after the masking view has been validated, add the volume + # to the storage group and recheck that it has been + # successfully added. + errorMessage = self._check_adding_volume_to_storage_group( + conn, maskingViewDict, storageGroupInstanceName) rollbackDict['controllerConfigService'] = controllerConfigService rollbackDict['defaultStorageGroupInstanceName'] = ( @@ -220,8 +103,390 @@ class EMCVMAXMasking(object): rollbackDict['volumeInstance'] = volumeInstance rollbackDict['volumeName'] = volumeName rollbackDict['fastPolicyName'] = fastPolicyName + + if errorMessage: + # Rollback code if we cannot complete any of the steps above + # successfully then we must roll back by adding the volume back to + # the default storage group for that fast policy. + if (fastPolicyName is not None): + # If the errorMessage was returned before the volume + # was removed from the default storage group no action. + self.check_if_rollback_action_for_masking_required( + conn, rollbackDict) + + exceptionMessage = (_( + "Failed to get, create or add volume %(volumeName)s " + "to masking view %(maskingViewName)s. " + "The error message received was %(errorMessage)s. ") + % {'maskingViewName': maskingViewName, + 'volumeName': volumeName, + 'errorMessage': errorMessage}) + LOG.error(exceptionMessage) + raise exception.VolumeBackendAPIException(data=exceptionMessage) + return rollbackDict + def _validate_masking_view(self, conn, maskingViewDict, + defaultStorageGroupInstanceName): + """Validate all the individual pieces of the masking view. + + :param conn - the ecom connection + :param maskingViewDict - the masking view dictionary + :param defaultStorageGroupInstanceName - the default SG + :returns: maskingViewInstanceName, storageGroupInstanceName, + errorMessage + """ + storageSystemName = maskingViewDict['storageSystemName'] + maskingViewName = maskingViewDict['maskingViewName'] + + maskingViewInstanceName = self._find_masking_view( + conn, maskingViewName, storageSystemName) + if maskingViewInstanceName is None: + maskingViewInstanceName, storageGroupInstanceName, errorMessage = ( + self._validate_new_masking_view( + conn, maskingViewDict, defaultStorageGroupInstanceName)) + + else: + storageGroupInstanceName, errorMessage = ( + self._validate_existing_masking_view( + conn, maskingViewDict, maskingViewInstanceName)) + + return maskingViewInstanceName, storageGroupInstanceName, errorMessage + + def _validate_new_masking_view(self, conn, maskingViewDict, + defaultStorageGroupInstanceName): + """Validate the creation of a new masking view. + + :param conn - the ecom connection + :param maskingViewDict - the masking view dictionary + :param defaultStorageGroupInstanceName - the default SG + :returns: maskingViewInstanceName, storageGroupInstanceName, + errorMessage + """ + controllerConfigService = maskingViewDict['controllerConfigService'] + igGroupName = maskingViewDict['igGroupName'] + connector = maskingViewDict['connector'] + storageSystemName = maskingViewDict['storageSystemName'] + maskingViewName = maskingViewDict['maskingViewName'] + pgGroupName = maskingViewDict['pgGroupName'] + + storageGroupInstanceName, errorMessage = ( + self._check_storage_group( + conn, maskingViewDict, defaultStorageGroupInstanceName)) + if errorMessage: + return None, storageGroupInstanceName, errorMessage + + portGroupInstanceName, errorMessage = ( + self._check_port_group(conn, controllerConfigService, + pgGroupName)) + if errorMessage: + return None, storageGroupInstanceName, errorMessage + + initiatorGroupInstanceName, errorMessage = ( + self._check_initiator_group(conn, controllerConfigService, + igGroupName, connector, + storageSystemName)) + if errorMessage: + return None, storageGroupInstanceName, errorMessage + + # Only after the components of the MV have been validated, + # add the volume to the storage group and re-check that it + # has been successfully added. This is necessary before + # creating a new masking view. + errorMessage = self._check_adding_volume_to_storage_group( + conn, maskingViewDict, storageGroupInstanceName) + if errorMessage: + return None, storageGroupInstanceName, errorMessage + + maskingViewInstanceName, errorMessage = ( + self._check_masking_view( + conn, controllerConfigService, + maskingViewName, storageGroupInstanceName, + portGroupInstanceName, initiatorGroupInstanceName)) + + return maskingViewInstanceName, storageGroupInstanceName, errorMessage + + def _validate_existing_masking_view(self, + conn, maskingViewDict, + maskingViewInstanceName): + """Validate the components of an existing masking view. + + :param conn - the ecom connection + :param maskingViewDict - the masking view dictionary + :param maskingViewInstanceName - the masking view instance name + :returns: storageGroupInstanceName, errorMessage + """ + storageGroupInstanceName = None + controllerConfigService = maskingViewDict['controllerConfigService'] + sgGroupName = maskingViewDict['sgGroupName'] + igGroupName = maskingViewDict['igGroupName'] + connector = maskingViewDict['connector'] + storageSystemName = maskingViewDict['storageSystemName'] + maskingViewName = maskingViewDict['maskingViewName'] + + # First verify that the initiator group matches the initiators. + errorMessage = self._check_existing_initiator_group( + conn, controllerConfigService, maskingViewName, + connector, storageSystemName, igGroupName) + + if errorMessage: + return storageGroupInstanceName, errorMessage + + storageGroupInstanceName, errorMessage = ( + self._check_existing_storage_group( + conn, controllerConfigService, sgGroupName, + maskingViewInstanceName)) + + return storageGroupInstanceName, errorMessage + + def _check_storage_group(self, conn, + maskingViewDict, storageGroupInstanceName): + """Get the storage group and return it. + + :param conn - the ecom connection + :param maskingViewDict - the masking view dictionary + :param defaultStorageGroupInstanceName - the default SG + :returns: storageGroupInstanceName, msg + """ + msg = None + storageGroupInstanceName = ( + self._get_storage_group_instance_name( + conn, maskingViewDict, storageGroupInstanceName)) + if storageGroupInstanceName is None: + # This may be used in exception hence _ instead of _LE + msg = (_( + "Cannot get or create a storage group: %(sgGroupName)s" + " for volume %(volumeName)s ") % + {'sgGroupName': maskingViewDict['sgGroupName'], + 'volumeName': maskingViewDict['volumeName']}) + LOG.error(msg) + return storageGroupInstanceName, msg + + def _check_existing_storage_group( + self, conn, controllerConfigService, + sgGroupName, maskingViewInstanceName): + """Check that we can get the existing storage group. + + :param conn - the ecom connection + :param controllerConfigService - controller configuration service + :param sgGroupName - the storage group name + :param maskingViewInstanceName - the masking view instance name + + :returns: storageGroupInstanceName, msg + """ + msg = None + + sgFromMvInstanceName = ( + self._get_storage_group_from_masking_view_instance( + conn, maskingViewInstanceName)) + + if sgFromMvInstanceName is None: + # This may be used in exception hence _ instead of _LE + msg = (_( + "Cannot get storage group: %(sgGroupName)s " + "from masking view %(maskingViewInstanceName)s.") % + {'sgGroupName': sgGroupName, + 'maskingViewInstanceName': maskingViewInstanceName}) + LOG.error(msg) + return sgFromMvInstanceName, msg + + def _get_storage_group_from_masking_view_instance( + self, conn, maskingViewInstance): + """Gets the Device Masking Group from masking view instance. + + :param conn: the connection to the ecom server + :param maskingViewInstance + :returns: instance name foundStorageGroupInstanceName + """ + foundStorageGroupInstanceName = None + groups = conn.AssociatorNames( + maskingViewInstance, + ResultClass='CIM_DeviceMaskingGroup') + if groups[0] > 0: + foundStorageGroupInstanceName = groups[0] + + return foundStorageGroupInstanceName + + def _check_port_group(self, conn, + controllerConfigService, pgGroupName): + """Check that you can either get or create a port group. + + :param conn - the ecom connection + :param controllerConfigService - controller configuration service + :param pgGroupName - the port group Name + :returns: portGroupInstanceName, msg + """ + msg = None + portGroupInstanceName = self._get_port_group_instance_name( + conn, controllerConfigService, pgGroupName) + if portGroupInstanceName is None: + # This may be used in exception hence _ instead of _LE + msg = (_( + "Cannot get port group: %(pgGroupName)s.") % + {'pgGroupName': pgGroupName}) + LOG.error(msg) + + return portGroupInstanceName, msg + + def _check_initiator_group( + self, conn, controllerConfigService, igGroupName, + connector, storageSystemName): + """Check that initiator group can be either be got or created. + + :param conn - the ecom connection + :param controllerConfigService - controller configuration service + :param igGroupName - the initiator group Name + :param connector + :param storageSystemName - the storage system name + :returns: initiatorGroupInstanceName, msg + """ + msg = None + initiatorGroupInstanceName = ( + self._get_initiator_group_instance_name( + conn, controllerConfigService, igGroupName, connector, + storageSystemName)) + if initiatorGroupInstanceName is None: + # This may be used in exception hence _ instead of _LE + msg = (_( + "Cannot get or create initiator group: " + "%(igGroupName)s.") % + {'igGroupName': igGroupName}) + LOG.error(msg) + + return initiatorGroupInstanceName, msg + + def _check_existing_initiator_group( + self, conn, controllerConfigService, maskingViewName, + connector, storageSystemName, igGroupName): + """Check that existing initiator group in the masking view. + + Check if the initiators in the initiator group match those in the + system. + + :param controllerConfigService - controller configuration service + :param maskingViewName - the masking view name + :param connector - the connector object + :param storageSystemName - the storage system name + :param igGroupName - the initiator group name + :returns: maskingViewInstanceName, msg + """ + msg = None + if not self._verify_initiator_group_from_masking_view( + conn, controllerConfigService, maskingViewName, + connector, storageSystemName, igGroupName): + # This may be used in exception hence _ instead of _LE + msg = (_( + "Unable to verify initiator group: %(igGroupName)s " + "in masking view %(maskingViewName)s.") % + {'igGroupName': igGroupName, + 'maskingViewName': maskingViewName}) + LOG.error(msg) + return msg + + def _check_masking_view( + self, conn, controllerConfigService, + maskingViewName, storageGroupInstanceName, + portGroupInstanceName, initiatorGroupInstanceName): + """Check that masking view can be either got or created. + + :param controllerConfigService - controller configuration service + :param maskingViewName - the masking view name + :param storageGroupInstanceName - storage group instance name + :param portGroupInstanceName - port group instance name + :param initiatorGroupInstanceName - the initiator group instance name + :returns: maskingViewInstanceName, msg + """ + msg = None + maskingViewInstanceName = ( + self._get_masking_view_instance_name( + conn, controllerConfigService, maskingViewName, + storageGroupInstanceName, portGroupInstanceName, + initiatorGroupInstanceName)) + if maskingViewInstanceName is None: + # This may be used in exception hence _ instead of _LE + msg = (_( + "Cannot create masking view: %(maskingViewName)s. ") % + {'maskingViewName': maskingViewName}) + LOG.error(msg) + + return maskingViewInstanceName, msg + + def _check_adding_volume_to_storage_group( + self, conn, maskingViewDict, storageGroupInstanceName): + """Add the volume to the storage group and double check it is there. + + :param conn - the ecom connection + :param maskingViewDict - the masking view dictionary + :returns: msg + """ + controllerConfigService = maskingViewDict['controllerConfigService'] + sgGroupName = maskingViewDict['sgGroupName'] + volumeInstance = maskingViewDict['volumeInstance'] + storageSystemName = maskingViewDict['storageSystemName'] + volumeName = maskingViewDict['volumeName'] + msg = None + if self._is_volume_in_storage_group( + conn, storageGroupInstanceName, + volumeInstance): + LOG.warn(_LW( + "Volume: %(volumeName)s is already part " + "of storage group %(sgGroupName)s "), + {'volumeName': volumeName, + 'sgGroupName': sgGroupName}) + else: + self.add_volume_to_storage_group( + conn, controllerConfigService, + storageGroupInstanceName, volumeInstance, volumeName, + sgGroupName, storageSystemName) + if not self._is_volume_in_storage_group( + conn, storageGroupInstanceName, + volumeInstance): + # 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) + + return msg + + def _get_and_remove_from_storage_group_v2( + self, conn, controllerConfigService, volumeInstanceName, + volumeName, fastPolicyName): + """Get the storage group and remove volume from it. + + :param controllerConfigService - controller configuration service + :param volumeInstanceName - volume instance name + :param volumeName - volume name + :param fastPolicyName - fast name + """ + defaultStorageGroupInstanceName = ( + self.fast.get_and_verify_default_storage_group( + conn, controllerConfigService, volumeInstanceName, + volumeName, fastPolicyName)) + if defaultStorageGroupInstanceName is None: + exceptionMessage = (_( + "Cannot get the default storage group for FAST policy: " + "%(fastPolicyName)s. ") + % {'fastPolicyName': fastPolicyName}) + LOG.error(exceptionMessage) + raise exception.VolumeBackendAPIException( + data=exceptionMessage) + + retStorageGroupInstanceName = ( + self.remove_device_from_default_storage_group( + conn, controllerConfigService, volumeInstanceName, + volumeName, fastPolicyName)) + if retStorageGroupInstanceName is None: + exceptionMessage = (_( + "Failed to remove volume %(volumeName)s from default SG: " + "%(volumeName)s. ") + % {'volumeName': volumeName}) + LOG.error(exceptionMessage) + raise exception.VolumeBackendAPIException( + data=exceptionMessage) + def _is_volume_in_storage_group( self, conn, storageGroupInstanceName, volumeInstance): """Check if the volume is already part of the storage group. @@ -272,7 +537,6 @@ class EMCVMAXMasking(object): :returns: foundMaskingViewInstanceName masking view instance name """ foundMaskingViewInstanceName = None - storageSystemInstanceName = self.utils.find_storageSystem( conn, storageSystemName) maskingViewInstances = conn.Associators( @@ -302,24 +566,16 @@ class EMCVMAXMasking(object): return foundMaskingViewInstanceName def _create_storage_group( - self, conn, controllerConfigService, storageGroupName, - volumeInstance, fastPolicyName, volumeName, storageSystemName, + self, conn, maskingViewDict, defaultStorageGroupInstanceName): """Create a new storage group that doesn't already exist. If fastPolicyName is not none we attempt to remove it from the default storage group of that policy and associate to the new storage group that will be part of the masking view. - Will not handle any exception in this method it will be handled - up the stack :param conn: connection the ecom server - :param controllerConfigService: the controller configuration service - :param storageGroupName: the proposed group name (String) - :param volumeInstance: useful information on the volume - :param fastPolicyName: the fast policy name (String) can be None - :param volumeName: the volume name (String) - :param storageSystemName: the storage system name (String) + :param maskingViewDict: masking view dictionary :param defaultStorageGroupInstanceName: the default storage group instance name (Can be None) :returns: foundStorageGroupInstanceName the instance Name of the @@ -328,32 +584,34 @@ class EMCVMAXMasking(object): failedRet = None foundStorageGroupInstanceName = ( self.provision.create_and_get_storage_group( - conn, controllerConfigService, storageGroupName, - volumeInstance.path)) + conn, maskingViewDict['controllerConfigService'], + maskingViewDict['sgGroupName'], + maskingViewDict['volumeInstance'].path)) if foundStorageGroupInstanceName is None: LOG.error(_LE( - "Cannot get storage Group from job : %(storageGroupName)s. ") - % {'storageGroupName': storageGroupName}) + "Cannot get storage Group from job : %(storageGroupName)s."), + {'storageGroupName': maskingViewDict['sgGroupName']}) return failedRet else: LOG.info(_LI( - "Created new storage group: %(storageGroupName)s ") - % {'storageGroupName': storageGroupName}) + "Created new storage group: %(storageGroupName)s "), + {'storageGroupName': maskingViewDict['sgGroupName']}) - if (fastPolicyName is not None and + if (maskingViewDict['fastPolicy'] is not None and defaultStorageGroupInstanceName is not None): assocTierPolicyInstanceName = ( self.fast.add_storage_group_and_verify_tier_policy_assoc( - conn, controllerConfigService, + conn, maskingViewDict['controllerConfigService'], foundStorageGroupInstanceName, - storageGroupName, fastPolicyName)) + maskingViewDict['sgGroupName'], + maskingViewDict['fastPolicy'])) if assocTierPolicyInstanceName is None: LOG.error(_LE( "Cannot add and verify tier policy association for storage" " group : %(storageGroupName)s to FAST policy : " - "%(fastPolicyName)s. ") - % {'storageGroupName': storageGroupName, - 'fastPolicyName': fastPolicyName}) + "%(fastPolicyName)s. "), + {'storageGroupName': maskingViewDict['sgGroupName'], + 'fastPolicyName': maskingViewDict['fastPolicy']}) return failedRet return foundStorageGroupInstanceName @@ -494,11 +752,12 @@ class EMCVMAXMasking(object): for initiatorMaskingGroupInstanceName in \ initiatorMaskingGroupInstanceNames: - # Check that it hasn't been deleted. If it has, continue - # to the next one in the loop. + # Check that it hasn't been deleted. If it has, break out + # of the for loop. instance = self.utils.get_existing_instance( conn, initiatorMaskingGroupInstanceName) if instance is None: + # MaskingGroup doesn't exist any more. continue storageHardwareIdInstances = ( @@ -540,6 +799,7 @@ class EMCVMAXMasking(object): hardwareIdInstances = ( self.utils.get_hardware_id_instances_from_array( conn, hardwareIdManagementService)) + for hardwareIdInstance in hardwareIdInstances: storageId = hardwareIdInstance['StorageID'] for initiatorName in initiatorNames: @@ -668,73 +928,37 @@ class EMCVMAXMasking(object): return foundStorageGroupInstanceName - def _get_storage_group_from_masking_view_instance( - self, conn, maskingViewInstance): - """Gets the Device Masking Group from masking view instance. - - :param conn: the connection to the ecom server - :param maskingViewInstance - :returns: instance name foundStorageGroupInstanceName - """ - foundStorageGroupInstanceName = None - groups = conn.AssociatorNames( - maskingViewInstance, - ResultClass='CIM_DeviceMaskingGroup') - if groups[0] > 0: - foundStorageGroupInstanceName = groups[0] - - return foundStorageGroupInstanceName - def _get_storage_group_instance_name( - self, conn, controllerConfigService, volumeInstance, volumeName, - sgGroupName, fastPolicyName, storageSystemName, + self, conn, maskingViewDict, defaultStorageGroupInstanceName): """Gets the storage group instance name. - If fastPolicy name is None - then NON FAST is assumed. If it is a valid fastPolicy name - then associate the new storage group with the fast policy. - If we are using an existing storage group then we must check that - it is associated with the correct fast policy + If fastPolicy name is None then NON FAST is assumed. If it is a + valid fastPolicy name then associate the new storage group with + the fast policy. If we are using an existing storage group then + we must check that it is associated with the correct fast policy. :param conn: the connection to the ecom server - :param controllerConfigService: the controller configuration server - :param volumeInstance: the volume instance - :param volumeName: the volume name (String) - :param sgGroupName: the storage group name (String) - :param fastPolicyName: the fast policy name (String): can be None - :param storageSystemName: the storage system name (String) + :param maskingViewDict: the masking view dictionary :param defaultStorageGroupInstanceName: default storage group instance name (can be None for Non FAST) :returns: instance name storageGroupInstanceName """ storageGroupInstanceName = self.utils.find_storage_masking_group( - conn, controllerConfigService, sgGroupName) + conn, maskingViewDict['controllerConfigService'], + maskingViewDict['sgGroupName']) if storageGroupInstanceName is None: storageGroupInstanceName = self._create_storage_group( - conn, controllerConfigService, sgGroupName, volumeInstance, - fastPolicyName, volumeName, storageSystemName, + conn, maskingViewDict, defaultStorageGroupInstanceName) if storageGroupInstanceName is None: errorMessage = (_( "Cannot create or find an storage group with name " "%(sgGroupName)s") - % {'sgGroupName': sgGroupName}) + % {'sgGroupName': maskingViewDict['sgGroupName']}) LOG.error(errorMessage) raise exception.VolumeBackendAPIException(data=errorMessage) - else: - if self._is_volume_in_storage_group( - conn, storageGroupInstanceName, volumeInstance): - LOG.warn(_LW("Volume: %(volumeName)s is already " - "part of storage group %(sgGroupName)s ") - % {'volumeName': volumeName, - 'sgGroupName': sgGroupName}) - else: - self.add_volume_to_storage_group( - conn, controllerConfigService, storageGroupInstanceName, - volumeInstance, volumeName, sgGroupName, fastPolicyName, - storageSystemName) return storageGroupInstanceName @@ -818,9 +1042,8 @@ class EMCVMAXMasking(object): return foundMaskingViewInstanceName - def _check_if_rollback_action_for_masking_required( - self, conn, controllerConfigService, volumeInstance, - volumeName, fastPolicyName, defaultStorageGroupInstanceName): + def check_if_rollback_action_for_masking_required( + self, conn, rollbackDict): """This is a rollback action for FAST. We need to be able to return the volume to the default storage group @@ -829,57 +1052,57 @@ class EMCVMAXMasking(object): the exception occurred. :param conn: the connection to the ecom server - :param controllerConfigService: the controller config service - :param volumeInstanceName: the volume instance name - :param volumeName: the volume name (String) - :param fastPolicyName: the fast policy name (String) - :param defaultStorageGroupInstanceName: the default storage group - instance name + :param rollbackDict: information for rolling back """ try: foundStorageGroupInstanceName = ( self.utils.get_storage_group_from_volume( - conn, volumeInstance.path)) + conn, rollbackDict['volumeInstance'].path)) # volume is not associated with any storage group so add it back # to the default if len(foundStorageGroupInstanceName) == 0: - infoMessage = (_( + LOG.warning(_LW( + "No storage group found. " "Performing rollback on Volume: %(volumeName)s " "To return it to the default storage group for FAST policy" - " %(fastPolicyName)s.") - % {'volumeName': volumeName, - 'fastPolicyName': fastPolicyName}) - LOG.warning(_LW("No storage group found. %s"), infoMessage) + " %(fastPolicyName)s."), + {'volumeName': rollbackDict['volumeName'], + 'fastPolicyName': rollbackDict['fastPolicyName']}) + assocDefaultStorageGroupName = ( self.fast .add_volume_to_default_storage_group_for_fast_policy( - conn, controllerConfigService, volumeInstance, - volumeName, fastPolicyName)) + conn, + rollbackDict['controllerConfigService'], + rollbackDict['volumeInstance'], + rollbackDict['volumeName'], + rollbackDict['fastPolicyName'])) if assocDefaultStorageGroupName is None: - errorMsg = (_( + LOG.error(_LE( "Failed to Roll back to re-add volume %(volumeName)s " "to default storage group for fast policy " "%(fastPolicyName)s: Please contact your sys admin to " - "get the volume re-added manually ") - % {'volumeName': volumeName, - 'fastPolicyName': fastPolicyName}) - LOG.error(errorMsg) + "get the volume re-added manually."), + {'volumeName': rollbackDict['volumeName'], + 'fastPolicyName': rollbackDict['fastPolicyName']}) if len(foundStorageGroupInstanceName) > 0: - errorMsg = (_( + LOG.info(_LI( "The storage group found is " - "%(foundStorageGroupInstanceName)s: ") - % {'foundStorageGroupInstanceName': - foundStorageGroupInstanceName}) - LOG.info(errorMsg) + "%(foundStorageGroupInstanceName)s: "), + {'foundStorageGroupInstanceName': + foundStorageGroupInstanceName}) # check the name see is it the default storage group or another if (foundStorageGroupInstanceName != - defaultStorageGroupInstanceName): + rollbackDict['defaultStorageGroupInstanceName']): # remove it from its current masking view and return it # to its default masking view if fast is enabled self.remove_and_reset_members( - conn, controllerConfigService, volumeInstance, - fastPolicyName, volumeName) + conn, + rollbackDict['controllerConfigService'], + rollbackDict['volumeInstance'], + rollbackDict['fastPolicyName'], + rollbackDict['volumeName']) except Exception as e: LOG.error(_LE("Exception: %s") % six.text_type(e)) errorMessage = (_( @@ -887,8 +1110,8 @@ class EMCVMAXMasking(object): "Please contact your system administrator to manually return " "your volume to the default storage group for fast policy " "%(fastPolicyName)s failed ") - % {'volumeName': volumeName, - 'fastPolicyName': fastPolicyName}) + % {'volumeName': rollbackDict['volumeName'], + 'fastPolicyName': rollbackDict['fastPolicyName']}) LOG.error(errorMessage) raise exception.VolumeBackendAPIException(data=errorMessage) @@ -919,7 +1142,7 @@ class EMCVMAXMasking(object): foundInitiatorMaskingGroupInstanceName = None foundView = self._find_masking_view( conn, maskingViewName, storageSystemName) - if foundView is not None: + if foundView: groups = conn.AssociatorNames( foundView, ResultClass='CIM_InitiatorMaskingGroup') @@ -965,7 +1188,7 @@ class EMCVMAXMasking(object): if (foundInitiatorGroupFromConnector != foundInitiatorGroupFromMaskingView): - if foundInitiatorGroupFromMaskingView is not None: + if foundInitiatorGroupFromMaskingView: maskingViewInstanceName = self._find_masking_view( conn, maskingViewName, storageSystemName) if foundInitiatorGroupFromConnector is None: @@ -989,9 +1212,9 @@ class EMCVMAXMasking(object): conn, maskingViewName, storageSystemName)) portGroupInstanceName = self._get_port_group_from_masking_view( conn, maskingViewName, storageSystemName) - if (foundInitiatorGroupFromConnector is not None and - storageGroupInstanceName is not None and - portGroupInstanceName is not None): + if (foundInitiatorGroupFromConnector and + storageGroupInstanceName and + portGroupInstanceName): self._delete_masking_view( conn, controllerConfigService, maskingViewName, maskingViewInstanceName) @@ -1000,7 +1223,7 @@ class EMCVMAXMasking(object): conn, controllerConfigService, maskingViewName, storageGroupInstanceName, portGroupInstanceName, foundInitiatorGroupFromConnector)) - if newMaskingViewInstanceName is not None: + if newMaskingViewInstanceName: LOG.debug( "The old masking view has been replaced: " "%(maskingViewName)s. " @@ -1084,8 +1307,7 @@ class EMCVMAXMasking(object): foundPortMaskingGroupInstanceName = None foundView = self._find_masking_view( conn, maskingViewName, storageSystemName) - - if foundView is not None: + if foundView: groups = conn.AssociatorNames( foundView, ResultClass='CIM_TargetMaskingGroup') -- 2.45.2