From 95b1943f5e2878e141284f55019b5d6b8540da50 Mon Sep 17 00:00:00 2001 From: Xing Yang Date: Thu, 5 Feb 2015 12:09:34 -0500 Subject: [PATCH] Fix detach volume from host problem in VMAX driver The VMAX driver unmaps a volume from a host without checking the host info in the connector, resulting the wrong host to be detached. This patch looks up the host info before detach and fixes the problem. Closes-Bug: #1382641 Change-Id: I68f053b34cc68cdb4f3a9e622faa47a260abd53a --- cinder/tests/test_emc_vmax.py | 68 ++- cinder/volume/drivers/emc/emc_vmax_common.py | 215 +++++----- cinder/volume/drivers/emc/emc_vmax_fast.py | 20 +- cinder/volume/drivers/emc/emc_vmax_fc.py | 72 ++-- cinder/volume/drivers/emc/emc_vmax_iscsi.py | 2 +- cinder/volume/drivers/emc/emc_vmax_masking.py | 396 +++++++++++++----- .../volume/drivers/emc/emc_vmax_provision.py | 4 +- cinder/volume/drivers/emc/emc_vmax_utils.py | 40 +- 8 files changed, 521 insertions(+), 296 deletions(-) diff --git a/cinder/tests/test_emc_vmax.py b/cinder/tests/test_emc_vmax.py index f345f5452..96ad991f3 100644 --- a/cinder/tests/test_emc_vmax.py +++ b/cinder/tests/test_emc_vmax.py @@ -534,7 +534,8 @@ class FakeEcomConnection(): result = self._enum_fcscsiendpoint() elif ResultClass == 'CIM_TargetMaskingGroup': result = self._assocnames_portgroup() - + elif ResultClass == 'Symm_LunMaskingView': + result = self._enum_maskingView() else: result = self._default_assocnames(objectpath) return result @@ -1115,6 +1116,14 @@ class FakeEcomConnection(): portgroups.append(portgroup) return portgroups + def _enum_maskingView(self): + maskingViews = [] + maskingView = {} + maskingView['CreationClassName'] = 'Symm_LunMaskingView' + maskingView['ElementName'] = 'myMaskingView' + maskingViews.append(maskingView) + return maskingViews + def _default_enum(self): names = [] name = {} @@ -1752,10 +1761,6 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase): EMCVMAXMasking, '_wrap_get_storage_group_from_volume', return_value=None) - @mock.patch.object( - EMCVMAXCommon, - '_wrap_find_device_number', - return_value={'storagesystem': EMCVMAXCommonData.storage_system}) @mock.patch.object( EMCVMAXUtils, 'find_storage_masking_group', @@ -1767,7 +1772,6 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase): def test_map_new_masking_view_no_fast_success(self, mock_check, mock_storage_group, - mock_wrap_device, mock_wrap_group, mock_volume_type): self.driver.initialize_connection(self.data.test_volume, @@ -1783,7 +1787,7 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase): return_value=None) @mock.patch.object( EMCVMAXCommon, - '_wrap_find_device_number', + 'find_device_number', return_value={'hostlunid': 1, 'storagesystem': EMCVMAXCommonData.storage_system}) @mock.patch.object( @@ -1818,7 +1822,7 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase): return_value=None) @mock.patch.object( EMCVMAXCommon, - '_wrap_find_device_number', + 'find_device_number', return_value={'hostlunid': 1, 'storagesystem': EMCVMAXCommonData.storage_system}) @mock.patch.object( @@ -1839,7 +1843,7 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase): return_value=None) @mock.patch.object( EMCVMAXCommon, - '_wrap_find_device_number', + 'find_device_number', return_value={'storagesystem': EMCVMAXCommonData.storage_system}) def test_map_no_fast_failed(self, mock_wrap_group, mock_wrap_device): self.assertRaises(exception.VolumeBackendAPIException, @@ -2291,7 +2295,7 @@ class EMCVMAXISCSIDriverFastTestCase(test.TestCase): return_value=None) @mock.patch.object( EMCVMAXCommon, - '_wrap_find_device_number', + 'find_device_number', return_value={'hostlunid': 1, 'storagesystem': EMCVMAXCommonData.storage_system}) @mock.patch.object( @@ -2309,7 +2313,7 @@ class EMCVMAXISCSIDriverFastTestCase(test.TestCase): return_value=None) @mock.patch.object( EMCVMAXCommon, - '_wrap_find_device_number', + 'find_device_number', return_value={'storagesystem': EMCVMAXCommonData.storage_system}) def test_map_fast_failed(self, mock_wrap_group, mock_wrap_device): self.assertRaises(exception.VolumeBackendAPIException, @@ -2947,6 +2951,8 @@ class EMCVMAXFCDriverFastTestCase(test.TestCase): driver.common.conn = FakeEcomConnection() driver.zonemanager_lookup_service = None self.driver = driver + self.driver.utils = EMCVMAXUtils(object) + self.driver.masking = EMCVMAXMasking('FC') def create_fake_config_file_fast(self): @@ -3202,7 +3208,12 @@ class EMCVMAXFCDriverFastTestCase(test.TestCase): EMCVMAXMasking, 'get_masking_view_by_volume', return_value=EMCVMAXCommonData.lunmaskctrl_name) - def test_detach_fast_success(self, mock_volume_type, mock_maskingview): + @mock.patch.object( + EMCVMAXCommon, + 'get_masking_views_by_port_group', + return_value=[]) + def test_detach_fast_success(self, mock_volume_type, mock_maskingview, + mock_mvs): common = self.driver.common common.get_target_wwns = mock.Mock( return_value=EMCVMAXCommonData.target_wwns) @@ -3210,8 +3221,8 @@ class EMCVMAXFCDriverFastTestCase(test.TestCase): self.data.connector) common.get_target_wwns.assert_called_once_with( EMCVMAXCommonData.storage_system, EMCVMAXCommonData.connector) - - self.assertEqual(0, len(data['data'])) + numTargetWwns = len(EMCVMAXCommonData.target_wwns) + self.assertEqual(numTargetWwns, len(data['data'])) @mock.patch.object( volume_types, @@ -3379,6 +3390,35 @@ class EMCVMAXFCDriverFastTestCase(test.TestCase): self.data.test_ctxt, self.data.test_volume, self.data.new_type, self.data.diff, self.data.test_host) + # Bug 1382641 + def test_get_sg_associated_with_connector(self): + conn = self.fake_ecom_connection() + controllerConfigService = ( + self.driver.utils.find_controller_configuration_service( + conn, self.data.storage_system)) + + mockSgInstanceName1 = {} + mockSgInstanceName1['CreationClassName'] = 'CIM_DeviceMaskingGroup' + mockSgInstanceName1['ElementName'] = 'OS_1382641_SG' + mockSgInstanceName2 = {} + mockSgInstanceName2['CreationClassName'] = 'CIM_DeviceMaskingGroup' + mockSgInstanceName2['ElementName'] = 'OS_not_1382641_SG' + self.driver.masking.get_associated_masking_groups_from_device = ( + mock.Mock(return_value=[mockSgInstanceName1, mockSgInstanceName2])) + + volumeInstanceName = ( + conn.EnumerateInstanceNames("EMC_StorageVolume")[0]) + volumeName = "1382641-Vol" + sgInstanceName = self.driver.masking._get_sg_associated_with_connector( + conn, controllerConfigService, volumeInstanceName, + volumeName, self.data.connector) + + (self.driver.masking.get_associated_masking_groups_from_device. + assert_called_once_with(conn, volumeInstanceName)) + + self.assertEqual(mockSgInstanceName1['ElementName'], + sgInstanceName['ElementName']) + def _cleanup(self): bExists = os.path.exists(self.config_file_path) if bExists: diff --git a/cinder/volume/drivers/emc/emc_vmax_common.py b/cinder/volume/drivers/emc/emc_vmax_common.py index 003419658..c0a46461a 100644 --- a/cinder/volume/drivers/emc/emc_vmax_common.py +++ b/cinder/volume/drivers/emc/emc_vmax_common.py @@ -189,8 +189,8 @@ class EMCVMAXCommon(object): % {'snapshotname': snapshotName, 'rc': rc}) - def _remove_members( - self, controllerConfigService, volumeInstance, extraSpecs): + def _remove_members(self, controllerConfigService, volumeInstance, + extraSpecs, connector): """This method unmaps a volume from a host. Removes volume from the Device Masking Group that belongs to @@ -202,14 +202,16 @@ class EMCVMAXCommon(object): :param controllerConfigService: instance name of ControllerConfigurationService - :param volume: volume Object + :param volumeInstance: volume instanceObject + :param extraSpecs: the volume extra specs + :param connector: the connector object """ volumeName = volumeInstance['ElementName'] LOG.debug("Detaching volume %s" % volumeName) fastPolicyName = extraSpecs[FASTPOLICY] return self.masking.remove_and_reset_members( self.conn, controllerConfigService, volumeInstance, - fastPolicyName, volumeName) + fastPolicyName, volumeName, connector) def _unmap_lun(self, volume, connector): """Unmaps a volume from the host. @@ -223,7 +225,7 @@ class EMCVMAXCommon(object): LOG.info(_LI("Unmap volume: %(volume)s") % {'volume': volumename}) - device_info = self.find_device_number(volume, connector) + device_info = self.find_device_number(volume) device_number = device_info['hostlunid'] if device_number is None: LOG.info(_LI("Volume %s is not mapped. No volume to unmap.") @@ -242,7 +244,8 @@ class EMCVMAXCommon(object): % {'storage_system': storage_system}) raise exception.VolumeBackendAPIException(data=exception_message) - self._remove_members(configservice, vol_instance, extraSpecs) + self._remove_members(configservice, vol_instance, + extraSpecs, connector) def initialize_connection(self, volume, connector): """Initializes the connection and returns device and connection info. @@ -276,7 +279,7 @@ class EMCVMAXCommon(object): LOG.info(_LI("Initialize connection: %(volume)s") % {'volume': volumeName}) self.conn = self._get_ecom_connection() - deviceInfoDict = self._wrap_find_device_number(volume, connector) + deviceInfoDict = self.find_device_number(volume) if ('hostlunid' in deviceInfoDict and deviceInfoDict['hostlunid'] is not None): isSameHost = self._is_same_host(connector, deviceInfoDict) @@ -320,7 +323,7 @@ class EMCVMAXCommon(object): self.conn, maskingViewDict) # Find host lun id again after the volume is exported to the host. - deviceInfoDict = self.find_device_number(volume, connector) + deviceInfoDict = self.find_device_number(volume) if 'hostlunid' not in deviceInfoDict: # Did not successfully attach to host, # so a rollback for FAST is required. @@ -356,15 +359,6 @@ class EMCVMAXCommon(object): return True return False - def _wrap_find_device_number(self, volume, connector): - """Aid for unit testing - - :params volume: the volume Object - :params connector: the connector Object - :returns: deviceInfoDict - """ - return self.find_device_number(volume, connector) - def terminate_connection(self, volume, connector): """Disallow connection from connector. @@ -452,7 +446,7 @@ class EMCVMAXCommon(object): self.conn, storageSystemName)) # create a volume to the size of the - # newSize - oldSize = additionalVolumeSize + # newSize - oldSize = additionalVolumeSize. unboundVolumeInstance = self._create_and_get_unbound_volume( self.conn, storageConfigService, volumeInstance.path, additionalVolumeSize) @@ -462,7 +456,7 @@ class EMCVMAXCommon(object): LOG.error(exceptionMessage) raise exception.VolumeBackendAPIException(data=exceptionMessage) - # add the new unbound volume to the original composite volume + # Add the new unbound volume to the original composite volume. rc, modifiedVolumeDict = ( self._modify_and_get_composite_volume_instance( self.conn, elementCompositionService, volumeInstance, @@ -475,7 +469,7 @@ class EMCVMAXCommon(object): LOG.error(exceptionMessage) raise exception.VolumeBackendAPIException(data=exceptionMessage) - # check the occupied space of the new extended volume + # Check the occupied space of the new extended volume. extendedVolumeInstance = self.utils.find_volume_instance( self.conn, modifiedVolumeDict, volumeName) extendedVolumeSize = self.utils.get_volume_size( @@ -487,7 +481,7 @@ class EMCVMAXCommon(object): 'volumeSize': extendedVolumeSize}) # If the requested size and the actual size don't - # tally throw an exception + # tally throw an exception. newSizeBits = self.utils.convert_gb_to_bits(newSize) diffVolumeSize = self.utils.compare_size( newSizeBits, extendedVolumeSize) @@ -540,7 +534,7 @@ class EMCVMAXCommon(object): "%(emcConfigFileName)s ") % {'arrayName': arrayName, 'emcConfigFileName': emcConfigFileName}) - # This value can be None + # This value can be None. fastPolicyName = self.utils.parse_fast_policy_name_from_file( emcConfigFileName) if fastPolicyName is not None: @@ -670,7 +664,7 @@ class EMCVMAXCommon(object): return True def migrate_volume(self, ctxt, volume, host, new_type=None): - """Migrate volume to another host + """Migrate volume to another host. :param ctxt: context :param volume: the volume object including the volume_type_id @@ -691,7 +685,7 @@ class EMCVMAXCommon(object): def _migrate_volume( self, volume, volumeInstance, targetPoolName, targetFastPolicyName, sourceFastPolicyName, new_type=None): - """Migrate volume to another host + """Migrate volume to another host. :param volume: the volume object including the volume_type_id :param volumeInstance: the volume instance @@ -713,7 +707,7 @@ class EMCVMAXCommon(object): if moved is False and sourceFastPolicyName is not None: # Return the volume to the default source fast policy storage - # group because the migrate was unsuccessful + # group because the migrate was unsuccessful. LOG.warn(_LW("Failed to migrate: %(volumeName)s from " "default source storage group " "for FAST policy: %(sourceFastPolicyName)s " @@ -726,7 +720,7 @@ class EMCVMAXCommon(object): storageSystemName, sourceFastPolicyName, volumeName) else: - # migrate was successful but still issues + # migrate was successful but still issues. self._migrate_rollback( self.conn, volumeInstance, storageSystemName, sourceFastPolicyName, volumeName, sourcePoolInstanceName) @@ -756,7 +750,7 @@ class EMCVMAXCommon(object): def _migrate_rollback(self, conn, volumeInstance, storageSystemName, sourceFastPolicyName, volumeName, sourcePoolInstanceName): - """Full rollback + """Full rollback. Failed on final step on adding migrated volume to new target default storage group for the target FAST policy @@ -797,7 +791,7 @@ class EMCVMAXCommon(object): def _migrate_cleanup(self, conn, volumeInstance, storageSystemName, sourceFastPolicyName, volumeName): - """If the migrate fails, put volume back to source FAST SG + """If the migrate fails, put volume back to source FAST SG. :param conn: connection info to ECOM :param volumeInstance: the volume instance @@ -816,22 +810,22 @@ 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)) - # This is the SG it should be in + # This is the SG it should be in. defaultStorageGroupInstanceName = ( self.fast.get_policy_default_storage_group( conn, controllerConfigurationService, sourceFastPolicyName)) - # It is not in any storage group. Must add it to default source + # It is not in any storage group. Must add it to default source. if assocStorageGroupInstanceName is None: self.add_to_default_SG(conn, volumeInstance, storageSystemName, sourceFastPolicyName, volumeName) - # It is in the incorrect storage group + # It is in the incorrect storage group. if (assocStorageGroupInstanceName is not None and (assocStorageGroupInstanceName != defaultStorageGroupInstanceName)): @@ -896,7 +890,7 @@ class EMCVMAXCommon(object): def _migrate_volume_from(self, volume, volumeInstance, targetPoolName, sourceFastPolicyName): - """Check FAST policies and migrate from source pool + """Check FAST policies and migrate from source pool. :param volume: the volume object including the volume_type_id :param volumeInstance: the volume instance @@ -913,13 +907,13 @@ class EMCVMAXCommon(object): % {'sourceFastPolicyName': sourceFastPolicyName}) # If the source volume is is FAST enabled it must first be removed - # from the default storage group for that policy + # from the default storage group for that policy. if sourceFastPolicyName is not None: self.remove_from_default_SG( self.conn, volumeInstance, storageSystemName, sourceFastPolicyName, volumeName) - # migrate from one pool to another + # Migrate from one pool to another. storageRelocationService = self.utils.find_storage_relocation_service( self.conn, storageSystemName) @@ -937,8 +931,8 @@ class EMCVMAXCommon(object): self.conn, storageRelocationService, volumeInstance.path, targetPoolInstanceName) except Exception as e: - # rollback by deleting the volume if adding the volume to the - # default storage group were to fail + # Rollback by deleting the volume if adding the volume to the + # default storage group were to fail. LOG.error(_LE("Exception: %s") % six.text_type(e)) exceptionMessage = (_("Error migrating volume: %(volumename)s. " "to target pool %(targetPoolName)s. ") @@ -947,8 +941,8 @@ class EMCVMAXCommon(object): LOG.error(exceptionMessage) return falseRet - # check that the volume is now migrated to the correct storage pool, - # if it is terminate the migrate session + # Check that the volume is now migrated to the correct storage pool, + # if it is terminate the migrate session. foundPoolInstanceName = self.utils.get_assoc_pool_from_volume( self.conn, volumeInstance.path) @@ -977,7 +971,7 @@ class EMCVMAXCommon(object): def remove_from_default_SG( self, conn, volumeInstance, storageSystemName, sourceFastPolicyName, volumeName): - """For FAST, remove volume from default storage group + """For FAST, remove volume from default storage group. :param conn: connection info to ECOM :param volumeInstance: the volume instance @@ -1018,7 +1012,7 @@ class EMCVMAXCommon(object): def add_to_default_SG( self, conn, volumeInstance, storageSystemName, targetFastPolicyName, volumeName): - """For FAST, add volume to default storage group + """For FAST, add volume to default storage group. :param conn: connection info to ECOM :param volumeInstance: the volume instance @@ -1088,8 +1082,8 @@ class EMCVMAXCommon(object): LOG.error(errorMessage) return falseRet - # get the pool from the source array and check that is is different - # to the pool in the target array + # Get the pool from the source array and check that is is different + # to the pool in the target array. assocPoolInstanceName = self.utils.get_assoc_pool_from_volume( self.conn, volumeInstanceName) assocPoolInstance = self.conn.GetInstance( @@ -1127,7 +1121,7 @@ class EMCVMAXCommon(object): extraSpecs = self.utils.get_volumetype_extraspecs(volume) configGroup = None - # If there are no extra specs then the default case is assumed + # If there are no extra specs then the default case is assumed. if extraSpecs: configGroup = self.configuration.config_group LOG.info(_LI("configGroup of current host: %s"), configGroup) @@ -1138,7 +1132,7 @@ class EMCVMAXCommon(object): return extraSpecs, configurationFile def _get_ecom_connection(self): - """Get the ecom connection + """Get the ecom connection. :returns: conn,the ecom connection """ @@ -1231,7 +1225,7 @@ class EMCVMAXCommon(object): def _find_storage_sync_sv_sv(self, snapshot, volume, waitforsync=True): - """Find the storage synchronized name + """Find the storage synchronized name. :param snapshot: snapshot object :param volume: volume object @@ -1262,7 +1256,7 @@ class EMCVMAXCommon(object): "Storage Synchronized instance: %(sync)s." % {'storage_system': storage_system, 'sync': foundsyncname}) - # Wait for SE_StorageSynchronized_SV_SV to be fully synced + # Wait for SE_StorageSynchronized_SV_SV to be fully synced. if waitforsync: self.utils.wait_for_sync(self.conn, foundsyncname) @@ -1290,17 +1284,17 @@ class EMCVMAXCommon(object): 'initiator': foundinitiatornames}) return foundinitiatornames - def find_device_number(self, volume, connector): + def find_device_number(self, volume): """Given the volume dict find a device number. Find a device number that a host can see for a volume. :param volume: the volume dict - :param connector: the connector dict :returns: data, the data dict """ foundNumDeviceNumber = None + foundMaskingViewName = None volumeName = volume['name'] volumeInstance = self._find_lun(volume) storageSystemName = volumeInstance['SystemName'] @@ -1388,7 +1382,7 @@ class EMCVMAXCommon(object): % {'len': len(endpoints)}) for targetendpoint in endpoints: wwn = targetendpoint['Name'] - # Add target wwn to the list if it is not already there + # Add target wwn to the list if it is not already there. if not any(d == wwn for d in targetWwns): targetWwns.append(wwn) else: @@ -1426,7 +1420,7 @@ class EMCVMAXCommon(object): instance = self.utils.get_existing_instance( self.conn, hardwareIdInstance.path) if instance is None: - # hardwareId doesn't exist any more. Skip it. + # HardwareId doesn't exist any more. Skip it. break foundHardwareIdList.append(hardwareIdInstance.path) break @@ -1455,7 +1449,7 @@ class EMCVMAXCommon(object): CINDER_EMC_CONFIG_FILE_POSTFIX) # The file saved in self.configuration may not be the correct one, - # double check + # double check. if configGroupName not in configurationFile: configurationFile = ( CINDER_EMC_CONFIG_FILE_PREFIX + configGroupName + @@ -1542,7 +1536,7 @@ class EMCVMAXCommon(object): data=exceptionMessage) # Get the FAST policy from the file this value can be None if the - # user doesn't want to associate with any FAST policy + # user doesn't want to associate with any FAST policy. fastPolicyName = self.utils.parse_fast_policy_name_from_file( configurationFile) if fastPolicyName is not None: @@ -1627,7 +1621,7 @@ class EMCVMAXCommon(object): 'OS-' + shortHostName + '-' + poolName + '-' + protocol + '-SG') maskingViewDict['maskingViewName'] = ( 'OS-' + shortHostName + '-' + poolName + '-' + protocol + '-MV') - # The portGroup is gotten from emc xml config file + # The portGroup is gotten from emc xml config file. maskingViewDict['pgGroupName'] = ( self.utils.parse_file_to_get_port_group_name( self.configuration.cinder_emc_config_file)) @@ -1683,8 +1677,8 @@ class EMCVMAXCommon(object): raise exception.VolumeBackendAPIException( data=exceptionMessage) except Exception as e: - # rollback by deleting the volume if adding the volume to the - # default storage group were to fail + # Rollback by deleting the volume if adding the volume to the + # default storage group were to fail. LOG.error(_LE("Exception: %s") % six.text_type(e)) errorMessage = (_( "Rolling back %(volumeName)s by deleting it. ") @@ -1882,7 +1876,7 @@ class EMCVMAXCommon(object): :param fastPolicyName: the FAST policy name(if it exists) :returns: rc """ - # check if the source volume contains any meta devices + # Check if the source volume contains any meta devices. metaHeadInstanceName = self.utils.get_volume_meta_head( self.conn, sourceInstance.path) @@ -1890,8 +1884,8 @@ class EMCVMAXCommon(object): return self._create_replica_and_delete_clone_relationship( repServiceInstanceName, cloneVolume, sourceVolume, sourceInstance, None, extraSpecs) - else: # composite volume with meta device members - # check if the meta members' capacity + else: # Composite volume with meta device members + # Check if the meta members' capacity. metaMemberInstanceNames = ( self.utils.get_meta_members_of_composite_volume( self.conn, metaHeadInstanceName)) @@ -1924,7 +1918,7 @@ class EMCVMAXCommon(object): "Capacity in bits: %(capInBits)lu ") % {'capInBits': volumeSizeInbits, 'targetVol': baseTargetVolumeInstance.path}) - else: # create append volume + else: # Create append volume. targetVolumeName = "MetaVol" volume = {'size': int(self.utils.convert_bits_to_gbs( volumeSizeInbits))} @@ -1942,8 +1936,8 @@ class EMCVMAXCommon(object): raise exception.VolumeBackendAPIException( data=exceptionMessage) - # append the new unbound volume to the - # base target composite volume + # Append the new unbound volume to the + # base target composite volume. baseTargetVolumeInstance = self.utils.find_volume_instance( self.conn, baseVolumeDict, baseVolumeName) elementCompositionService = ( @@ -2001,14 +1995,14 @@ class EMCVMAXCommon(object): syncInstanceName, storageSystemName = ( self._find_storage_sync_sv_sv(cloneVolume, sourceVolume)) - # Remove the Clone relationship so it can be used as a regular lun - # 8 - Detach operation + # Remove the Clone relationship so it can be used as a regular lun. + # 8 - Detach operation. rc, job = self.provision.delete_clone_relationship( self.conn, repServiceInstanceName, syncInstanceName, cloneName, sourceName) - # if FAST enabled place clone volume or volume from snapshot to - # default storage group + # If FAST enabled place clone volume or volume from snapshot to + # default storage group. if extraSpecs[FASTPOLICY] is not None: LOG.debug("Adding volume: %(cloneName)s to default storage group " "for FAST policy: %(fastPolicyName)s " @@ -2032,7 +2026,8 @@ class EMCVMAXCommon(object): raise exception.VolumeBackendAPIException( data=exceptionMessage) - # check if the clone/snapshot volume already part of the default sg + # Check if the clone/snapshot volume already part of the default + # sg. cloneInstance = self.utils.find_volume_instance( self.conn, cloneDict, cloneName) inDefaultSG = self.fast.is_volume_in_default_SG( @@ -2096,14 +2091,16 @@ class EMCVMAXCommon(object): % {'volumename': volumeName, 'fastPolicyName': fastPolicyName}) LOG.warn(warnMessage) - # check if it is part of another storage group - self._pre_check_for_deletion(controllerConfigurationService, - volumeInstance.path, volumeName) + # Check if it is part of another storage group. + self._remove_device_from_storage_group( + controllerConfigurationService, volumeInstance.path, + volumeName) else: - # check if volume is part of a storage group - self._pre_check_for_deletion(controllerConfigurationService, - volumeInstance.path, volumeName) + # Check if volume is part of a storage group. + self._remove_device_from_storage_group( + controllerConfigurationService, + volumeInstance.path, volumeName) LOG.debug("Delete Volume: %(name)s Method: EMCReturnToStoragePool " "ConfigServic: %(service)s TheElement: %(vol_instance)s " @@ -2118,8 +2115,8 @@ class EMCVMAXCommon(object): volumeName) except Exception as e: - # if we cannot successfully delete the volume then we want to - # return the volume to the default storage group + # If we cannot successfully delete the volume then we want to + # return the volume to the default storage group. if (fastPolicyName is not None and defaultStorageGroupInstanceName is not None and storageSystemName is not None): @@ -2146,9 +2143,10 @@ class EMCVMAXCommon(object): return (rc, volumeName) - def _pre_check_for_deletion(self, controllerConfigurationService, - volumeInstanceName, volumeName): - """Check is volume is part of a storage group prior to delete + def _remove_device_from_storage_group( + self, controllerConfigurationService, volumeInstanceName, + volumeName): + """Check is volume is part of a storage group prior to delete. Log a warning if volume is part of storage group @@ -2157,20 +2155,21 @@ class EMCVMAXCommon(object): :param volumeName: volume name (string) """ - storageGroupInstanceName = ( - self.masking.get_associated_masking_group_from_device( + storageGroupInstanceNames = ( + self.masking.get_associated_masking_groups_from_device( self.conn, volumeInstanceName)) - if storageGroupInstanceName is not None: - LOG.warn(_LW("Pre check for deletion " - "Volume: %(volumeName)s is part of a storage group " - "Attempting removal " - "from %(storageGroupInstanceName)s ") - % {'volumeName': volumeName, - 'storageGroupInstanceName': storageGroupInstanceName}) - self.provision.remove_device_from_storage_group( - self.conn, controllerConfigurationService, - storageGroupInstanceName, - volumeInstanceName, volumeName) + if storageGroupInstanceNames: + LOG.warn(_LW( + "Pre check for deletion " + "Volume: %(volumeName)s is part of a storage group " + "Attempting removal from %(storageGroupInstanceNames)s"), + {'volumeName': volumeName, + 'storageGroupInstanceNames': storageGroupInstanceNames}) + for storageGroupInstanceName in storageGroupInstanceNames: + self.provision.remove_device_from_storage_group( + self.conn, controllerConfigurationService, + storageGroupInstanceName, + volumeInstanceName, volumeName) def _find_lunmasking_scsi_protocol_controller(self, storageSystemName, connector): @@ -2214,7 +2213,7 @@ class EMCVMAXCommon(object): self.conn, controllerInstanceName) if instance is None: # Skip this controller as it doesn't exist - # any more + # any more. pass else: foundControllerInstanceName = ( @@ -2273,7 +2272,7 @@ class EMCVMAXCommon(object): 'connector': connector, 'ctrl': ctrl}) - # return 0 if masking view does not exist + # Return 0 if masking view does not exist. if ctrl is None: return 0 @@ -2301,12 +2300,14 @@ class EMCVMAXCommon(object): :returns: targetWwns, the target WWN list """ targetWwns = [] - mvInstanceName = self.get_masking_view_by_volume(volume) - targetWwns = self.masking.get_target_wwns(self.conn, mvInstanceName) - LOG.info(_LI("Target wwns in masking view %(maskingView)s: " - "%(targetWwns)s"), - {'maskingView': mvInstanceName, - 'targetWwns': str(targetWwns)}) + mvInstanceName = self.get_masking_view_by_volume(volume, connector) + if mvInstanceName is not None: + targetWwns = self.masking.get_target_wwns( + self.conn, mvInstanceName) + LOG.info(_LI("Target wwns in masking view %(maskingView)s: " + "%(targetWwns)s"), + {'maskingView': mvInstanceName, + 'targetWwns': str(targetWwns)}) return targetWwns def get_port_group_from_masking_view(self, maskingViewInstanceName): @@ -2318,18 +2319,18 @@ class EMCVMAXCommon(object): return self.masking.get_port_group_from_masking_view( self.conn, maskingViewInstanceName) - def get_masking_view_by_volume(self, volume): - """Given volume, retrieve the masking view instance name. + def get_masking_view_by_volume(self, volume, connector): + """Given volume, retrieve the masking view instance name :param volume: the volume - :param mvInstanceName: masking view instance name + :param connector: the connector object :returns maskingviewInstanceName """ - LOG.debug("Finding Masking View for volume %(volume)s" - % {'volume': volume}) + LOG.debug("Finding Masking View for volume %(volume)s", + {'volume': volume}) volumeInstance = self._find_lun(volume) return self.masking.get_masking_view_by_volume( - self.conn, volumeInstance) + self.conn, volumeInstance, connector) def get_masking_views_by_port_group(self, portGroupInstanceName): """Given port group, retrieve the masking view instance name. @@ -2384,7 +2385,7 @@ class EMCVMAXCommon(object): self.conn, storageSystemName) # If FAST is intended to be used we must first check that the pool - # is associated with the correct storage tier + # is associated with the correct storage tier. if extraSpecs[FASTPOLICY] is not None: foundPoolInstanceName = self.fast.get_pool_associated_to_policy( self.conn, extraSpecs[FASTPOLICY], extraSpecs[ARRAY], @@ -2409,7 +2410,7 @@ class EMCVMAXCommon(object): # Now that we have already checked that the pool is associated with # the correct storage tier and the volume was successfully created # add the volume to the default storage group created for - # volumes in pools associated with this fast policy + # volumes in pools associated with this fast policy. if extraSpecs[FASTPOLICY]: LOG.info(_LI("Adding volume: %(volumeName)s to default storage " "group for FAST policy: %(fastPolicyName)s ") diff --git a/cinder/volume/drivers/emc/emc_vmax_fast.py b/cinder/volume/drivers/emc/emc_vmax_fast.py index 700d302ce..f6c839e1d 100644 --- a/cinder/volume/drivers/emc/emc_vmax_fast.py +++ b/cinder/volume/drivers/emc/emc_vmax_fast.py @@ -180,7 +180,7 @@ class EMCVMAXFast(object): self.provision.add_members_to_masking_group( conn, controllerConfigService, storageGroupInstanceName, volumeInstance.path, volumeName) - # check to see if the volume is in the storage group + # Check to see if the volume is in the storage group. assocStorageGroupInstanceName = ( self.utils.get_storage_group_from_volume(conn, volumeInstance.path)) @@ -231,7 +231,7 @@ class EMCVMAXFast(object): tierPolicyServiceInstanceName = self.utils.get_tier_policy_service( conn, storageSystemInstanceName) - # get the fast policy instance name + # Get the fast policy instance name. tierPolicyRuleInstanceName = self._get_service_level_tier_policy( conn, tierPolicyServiceInstanceName, fastPolicyName) if tierPolicyRuleInstanceName is None: @@ -242,7 +242,7 @@ class EMCVMAXFast(object): LOG.error(exceptionMessage) return failedRet - # now associate it with a FAST policy + # Now associate it with a FAST policy. self.add_storage_group_to_tier_policy_rule( conn, tierPolicyServiceInstanceName, defaultStorageGroupInstanceName, tierPolicyRuleInstanceName, @@ -300,7 +300,7 @@ class EMCVMAXFast(object): :param storageGroupName: the storage group name (String) :param fastPolicyName: the fast policy name (String) """ - # 5 is ("Add InElements to Policy") + # 5 is ("Add InElements to Policy"). modificationType = '5' rc, job = conn.InvokeMethod( @@ -493,7 +493,7 @@ class EMCVMAXFast(object): conn, controllerConfigService) tierPolicyServiceInstanceName = self.utils.get_tier_policy_service( conn, storageSystemInstanceName) - # get the fast policy instance name + # Get the fast policy instance name. tierPolicyRuleInstanceName = self._get_service_level_tier_policy( conn, tierPolicyServiceInstanceName, fastPolicyName) if tierPolicyRuleInstanceName is None: @@ -510,7 +510,7 @@ class EMCVMAXFast(object): % {'storageGroupInstanceName': storageGroupInstanceName, 'tierPolicyRuleInstanceName': tierPolicyRuleInstanceName}) - # Associate the new storage group with the existing fast policy + # Associate the new storage group with the existing fast policy. try: self.add_storage_group_to_tier_policy_rule( conn, tierPolicyServiceInstanceName, @@ -527,8 +527,8 @@ class EMCVMAXFast(object): LOG.error(errorMessage) return failedRet - # check that the storage group has been associated with with the - # tier policy rule + # Check that the storage group has been associated with with the + # tier policy rule. assocTierPolicyInstanceName = ( self.get_associated_tier_policy_from_storage_group( conn, storageGroupInstanceName)) @@ -624,11 +624,11 @@ class EMCVMAXFast(object): tierPolicyRuleInstanceName = self._get_service_level_tier_policy( conn, tierPolicyServiceInstanceName, fastPolicyName) - # Get the associated storage tiers from the tier policy rule + # Get the associated storage tiers from the tier policy rule. storageTierInstanceNames = self.get_associated_tier_from_tier_policy( conn, tierPolicyRuleInstanceName) - # For each gold storage tier get the associated pools + # For each gold storage tier get the associated pools. foundPoolInstanceName = None for storageTierInstanceName in storageTierInstanceNames: assocStoragePoolInstanceNames = ( diff --git a/cinder/volume/drivers/emc/emc_vmax_fc.py b/cinder/volume/drivers/emc/emc_vmax_fc.py index 53dc26b45..580dc8844 100644 --- a/cinder/volume/drivers/emc/emc_vmax_fc.py +++ b/cinder/volume/drivers/emc/emc_vmax_fc.py @@ -15,7 +15,7 @@ import six from cinder import context -from cinder.i18n import _LI +from cinder.i18n import _LI, _LW from cinder.openstack.common import log as logging from cinder.volume import driver from cinder.volume.drivers.emc import emc_vmax_common @@ -186,38 +186,42 @@ class EMCVMAXFCDriver(driver.FibreChannelDriver): LOG.info(_LI("Start FC detach process for volume: %(volume)s") % {'volume': volume['name']}) - target_wwns, init_targ_map = self._build_initiator_target_map( - storage_system, volume, connector) - - mvInstanceName = self.common.get_masking_view_by_volume(volume) - portGroupInstanceName = self.common.get_port_group_from_masking_view( - mvInstanceName) - - LOG.info(_LI("Found port group: %(portGroup)s " - "in masking view %(maskingView)s"), - {'portGroup': portGroupInstanceName, - 'maskingView': mvInstanceName}) - - self.common.terminate_connection(volume, connector) - - LOG.info(_LI("Looking for masking views still associated with" - "Port Group %s"), portGroupInstanceName) - mvInstances = self.common.get_masking_views_by_port_group( - portGroupInstanceName) - if len(mvInstances) > 0: - LOG.debug("Found %(numViews)lu maskingviews." - % {'numViews': len(mvInstances)}) - data = {'driver_volume_type': 'fibre_channel', - 'data': {}} - else: # no views found - LOG.debug("No Masking Views were found. Deleting zone.") - data = {'driver_volume_type': 'fibre_channel', - 'data': {'target_wwn': target_wwns, - 'initiator_target_map': init_targ_map}} - - LOG.debug("Return FC data for zone removal: %(data)s." - % {'data': data}) - + mvInstanceName = self.common.get_masking_view_by_volume( + volume, connector) + data = {'driver_volume_type': 'fibre_channel', + 'data': {}} + if mvInstanceName is not None: + portGroupInstanceName = ( + self.common.get_port_group_from_masking_view( + mvInstanceName)) + + LOG.info(_LI("Found port group: %(portGroup)s " + "in masking view %(maskingView)s"), + {'portGroup': portGroupInstanceName, + 'maskingView': mvInstanceName}) + + self.common.terminate_connection(volume, connector) + + LOG.debug("Looking for masking views still associated with " + "Port Group %s", portGroupInstanceName) + mvInstances = self.common.get_masking_views_by_port_group( + portGroupInstanceName) + if len(mvInstances) > 0: + LOG.debug("Found %(numViews)lu MaskingViews.", + {'numViews': len(mvInstances)}) + else: # No views found. + target_wwns, init_targ_map = self._build_initiator_target_map( + storage_system, volume, connector) + LOG.debug("No MaskingViews were found. Deleting zone.") + data = {'driver_volume_type': 'fibre_channel', + 'data': {'target_wwn': target_wwns, + 'initiator_target_map': init_targ_map}} + + LOG.debug("Return FC data for zone removal: %(data)s.", + {'data': data}) + else: + LOG.warn(_LW("Volume %(volume)s is not in any masking view."), + {'volume': volume['name']}) return data def _build_initiator_target_map(self, storage_system, volume, connector): @@ -238,7 +242,7 @@ class EMCVMAXFCDriver(driver.FibreChannelDriver): target_wwns.extend(map_d['target_port_wwn_list']) for initiator in map_d['initiator_port_wwn_list']: init_targ_map[initiator] = map_d['target_port_wwn_list'] - else: # no lookup service, pre-zoned case + else: # No lookup service, pre-zoned case. target_wwns = self.common.get_target_wwns(storage_system, connector) for initiator in initiator_wwns: diff --git a/cinder/volume/drivers/emc/emc_vmax_iscsi.py b/cinder/volume/drivers/emc/emc_vmax_iscsi.py index a2159dea9..58eed9014 100644 --- a/cinder/volume/drivers/emc/emc_vmax_iscsi.py +++ b/cinder/volume/drivers/emc/emc_vmax_iscsi.py @@ -196,7 +196,7 @@ class EMCVMAXISCSIDriver(driver.ISCSIDriver): LOG.debug("ISCSI Discovery: Found %s" % (location)) properties['target_discovered'] = True - device_info = self.common.find_device_number(volume, connector) + device_info = self.common.find_device_number(volume) if device_info is None or device_info['hostlunid'] is None: exception_message = (_("Cannot find device number for volume " diff --git a/cinder/volume/drivers/emc/emc_vmax_masking.py b/cinder/volume/drivers/emc/emc_vmax_masking.py index 4726c0caf..d569fe065 100644 --- a/cinder/volume/drivers/emc/emc_vmax_masking.py +++ b/cinder/volume/drivers/emc/emc_vmax_masking.py @@ -71,7 +71,7 @@ class EMCVMAXMasking(object): # 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 FAST is enabled remove the volume from the default SG. if fastPolicyName is not None: defaultStorageGroupInstanceName = ( self._get_and_remove_from_storage_group_v2( @@ -253,7 +253,7 @@ class EMCVMAXMasking(object): self._get_storage_group_instance_name( conn, maskingViewDict, storageGroupInstanceName)) if storageGroupInstanceName is None: - # This may be used in exception hence _ instead of _LE + # This may be used in exception hence _ instead of _LE. msg = (_( "Cannot get or create a storage group: %(sgGroupName)s" " for volume %(volumeName)s ") % @@ -281,7 +281,7 @@ class EMCVMAXMasking(object): conn, maskingViewInstanceName)) if sgFromMvInstanceName is None: - # This may be used in exception hence _ instead of _LE + # This may be used in exception hence _ instead of _LE. msg = (_( "Cannot get storage group: %(sgGroupName)s " "from masking view %(maskingViewInstanceName)s.") % @@ -320,7 +320,7 @@ class EMCVMAXMasking(object): portGroupInstanceName = self._get_port_group_instance_name( conn, controllerConfigService, pgGroupName) if portGroupInstanceName is None: - # This may be used in exception hence _ instead of _LE + # This may be used in exception hence _ instead of _LE. msg = (_( "Cannot get port group: %(pgGroupName)s.") % {'pgGroupName': pgGroupName}) @@ -346,7 +346,7 @@ class EMCVMAXMasking(object): conn, controllerConfigService, igGroupName, connector, storageSystemName)) if initiatorGroupInstanceName is None: - # This may be used in exception hence _ instead of _LE + # This may be used in exception hence _ instead of _LE. msg = (_( "Cannot get or create initiator group: " "%(igGroupName)s.") % @@ -374,7 +374,7 @@ class EMCVMAXMasking(object): 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 + # This may be used in exception hence _ instead of _LE. msg = (_( "Unable to verify initiator group: %(igGroupName)s " "in masking view %(maskingViewName)s.") % @@ -403,9 +403,9 @@ class EMCVMAXMasking(object): storageGroupInstanceName, portGroupInstanceName, initiatorGroupInstanceName)) if maskingViewInstanceName is None: - # This may be used in exception hence _ instead of _LE + # This may be used in exception hence _ instead of _LE. msg = (_( - "Cannot create masking view: %(maskingViewName)s. ") % + "Cannot create masking view: %(maskingViewName)s.") % {'maskingViewName': maskingViewName}) LOG.error(msg) @@ -441,7 +441,7 @@ class EMCVMAXMasking(object): if not self._is_volume_in_storage_group( conn, storageGroupInstanceName, volumeInstance): - # This may be used in exception hence _ instead of _LE + # This may be used in exception hence _ instead of _LE. msg = (_( "Volume: %(volumeName)s was not added " "to storage group %(sgGroupName)s.") % @@ -549,7 +549,7 @@ class EMCVMAXMasking(object): break if foundMaskingViewInstanceName is not None: - # now check that is has not been deleted + # Now check that is has not been deleted. instance = self.utils.get_existing_instance( conn, foundMaskingViewInstanceName) if instance is None: @@ -676,10 +676,10 @@ class EMCVMAXMasking(object): conn, controllerConfigService, initiatorNames) # If you cannot find an initiatorGroup that matches the connector - # info create a new initiatorGroup + # info create a new initiatorGroup. if foundInitiatorGroupInstanceName is None: - # check that our connector information matches the - # hardwareId(s) on the symm + # Check that our connector information matches the + # hardwareId(s) on the symm. storageHardwareIDInstanceNames = ( self._get_storage_hardware_id_instance_names( conn, initiatorNames, storageSystemName)) @@ -705,7 +705,7 @@ class EMCVMAXMasking(object): return foundInitiatorGroupInstanceName def _find_initiator_names(self, conn, connector): - """check the connector object for initiators(ISCSI) or wwpns(FC). + """Check the connector object for initiators(ISCSI) or wwpns(FC). :param conn: the connection to the ecom :param connector: the connector object @@ -823,7 +823,7 @@ class EMCVMAXMasking(object): return foundHardwardIDsInstanceNames def _get_initiator_group_from_job(self, conn, job): - """After creating an new initiator group find it and return it + """After creating an new initiator group find it and return it. :param conn: the connection to the ecom server :param job: the create initiator group job @@ -881,7 +881,7 @@ class EMCVMAXMasking(object): return rc, job def find_new_masking_view(self, conn, jobDict): - """Find the newly created volume + """Find the newly created volume. :param conn: the connection to the ecom server :param jobDict: the job tuple @@ -1018,7 +1018,7 @@ class EMCVMAXMasking(object): self, conn, controllerConfigService, maskingViewName, storageGroupInstanceName, portGroupInstanceName, initiatorGroupInstanceName): - """Gets the masking view instance name + """Gets the masking view instance name. :param conn: the connection to the ecom server :param controllerConfigService: the controller configuration server @@ -1058,8 +1058,8 @@ class EMCVMAXMasking(object): foundStorageGroupInstanceName = ( self.utils.get_storage_group_from_volume( conn, rollbackDict['volumeInstance'].path)) - # volume is not associated with any storage group so add it back - # to the default + # Volume is not associated with any storage group so add it back + # to the default. if len(foundStorageGroupInstanceName) == 0: LOG.warning(_LW( "No storage group found. " @@ -1092,11 +1092,12 @@ class EMCVMAXMasking(object): {'foundStorageGroupInstanceName': foundStorageGroupInstanceName}) - # check the name see is it the default storage group or another + # Check the name, see is it the default storage group + # or another. if (foundStorageGroupInstanceName != rollbackDict['defaultStorageGroupInstanceName']): - # remove it from its current masking view and return it - # to its default masking view if fast is enabled + # 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, rollbackDict['controllerConfigService'], @@ -1241,7 +1242,7 @@ class EMCVMAXMasking(object): def _create_initiator_Group( self, conn, controllerConfigService, igGroupName, hardwareIdinstanceNames): - """Create a new initiator group + """Create a new initiator group. Given a list of hardwareId Instance name create a new initiator group @@ -1297,7 +1298,7 @@ class EMCVMAXMasking(object): def _get_port_group_from_masking_view( self, conn, maskingViewName, storageSystemName): - """Given the masking view name get the port group from it + """Given the masking view name get the port group from it. :param conn: connection the the ecom server :param maskingViewName: the name of the masking view @@ -1324,7 +1325,7 @@ class EMCVMAXMasking(object): def _delete_masking_view( self, conn, controllerConfigService, maskingViewName, maskingViewInstanceName): - """Delete a masking view + """Delete a masking view. :param conn: connection the ecom server :param controllerConfigService: the controller configuration service @@ -1350,7 +1351,7 @@ class EMCVMAXMasking(object): def get_masking_view_from_storage_group( self, conn, storageGroupInstanceName): - """Get the associated maskingview instance name + """Get the associated maskingview instance name. Given storage group instance name, get the associated masking view instance name @@ -1372,7 +1373,7 @@ class EMCVMAXMasking(object): self, conn, controllerConfigService, storageGroupInstanceName, volumeInstance, volumeName, sgGroupName, fastPolicyName, storageSystemName=None): - """Add a volume to an existing storage group + """Add a volume to an existing storage group. :param conn: connection to ecom server :param controllerConfigService: the controller configuration service @@ -1447,7 +1448,7 @@ class EMCVMAXMasking(object): % {'length': len(assocVolumeInstanceNames), 'volumeName': volumeName}) - # required for unit tests + # Required for unit tests. emptyStorageGroupInstanceName = ( self._wrap_get_storage_group_from_volume(conn, volumeInstanceName)) @@ -1476,7 +1477,7 @@ class EMCVMAXMasking(object): def get_devices_from_storage_group( self, conn, storageGroupInstanceName): - """Get the associated volume Instance names + """Get the associated volume Instance names. Given the storage group instance name get the associated volume Instance names @@ -1491,20 +1492,31 @@ class EMCVMAXMasking(object): return volumeInstanceNames - def get_associated_masking_group_from_device( + def get_associated_masking_groups_from_device( self, conn, volumeInstanceName): + """Get the associated storage groups from the volume Instance name. + + Given the volume instance name get the associated storage group + instance names. + + :param conn: connection the the ecom server + :param volumeInstanceName: the volume instance name + :returns: list of storage group instance names + """ maskingGroupInstanceNames = conn.AssociatorNames( volumeInstanceName, ResultClass='CIM_DeviceMaskingGroup', AssocClass='CIM_OrderedMemberOfCollection') if len(maskingGroupInstanceNames) > 0: - return maskingGroupInstanceNames[0] + return maskingGroupInstanceNames else: + LOG.info(_LI("Volume %(volumeName)s not in any storage group."), + {'volumeName': volumeInstanceName}) return None def remove_and_reset_members( self, conn, controllerConfigService, volumeInstance, - fastPolicyName, volumeName): + fastPolicyName, volumeName, connector=None): """Part of unmap device or rollback. Removes volume from the Device Masking Group that belongs to a @@ -1518,108 +1530,96 @@ class EMCVMAXMasking(object): :param volumeInstance: the volume Instance :param fastPolicyName: the fast policy name (if it exists) :param volumeName: the volume name - :returns: list volumeInstanceNames list of volume instance names + :param connector: the connector object + :returns: storage group instance name """ - rc = -1 - maskingGroupInstanceName = ( - self.get_associated_masking_group_from_device( - conn, volumeInstance.path)) + storageGroupInstanceName = None + if connector is not None: + storageGroupInstanceName = self._get_sg_associated_with_connector( + conn, controllerConfigService, volumeInstance.path, + volumeName, connector) + if storageGroupInstanceName is None: + return None + else: + storageGroupInstanceNames = ( + self.get_associated_masking_groups_from_device( + conn, volumeInstance.path)) + if storageGroupInstanceNames: + storageGroupInstanceName = storageGroupInstanceNames[0] + else: + return None volumeInstanceNames = self.get_devices_from_storage_group( - conn, maskingGroupInstanceName) + conn, storageGroupInstanceName) storageSystemInstanceName = self.utils.find_storage_system( conn, controllerConfigService) - isTieringPolicySupported = False - if fastPolicyName is not None: - tierPolicyServiceInstanceName = self.utils.get_tier_policy_service( - conn, storageSystemInstanceName) - - isTieringPolicySupported = self.fast.is_tiering_policy_enabled( - conn, tierPolicyServiceInstanceName) - LOG.debug( - "FAST policy enabled on %(storageSystem)s: %(isSupported)s" - % {'storageSystem': storageSystemInstanceName, - 'isSupported': isTieringPolicySupported}) - numVolInMaskingView = len(volumeInstanceNames) LOG.debug( - "There are %(numVol)d volumes in the masking view %(maskingGroup)s" - % {'numVol': numVolInMaskingView, - 'maskingGroup': maskingGroupInstanceName}) - - if numVolInMaskingView == 1: # last volume in the storage group - # delete masking view - mvInstanceName = self.get_masking_view_from_storage_group( - conn, maskingGroupInstanceName) - LOG.debug( - "Last volume in the storage group, deleting masking view " - "%(mvInstanceName)s" - % {'mvInstanceName': mvInstanceName}) - conn.DeleteInstance(mvInstanceName) - - # disassociate storage group from FAST policy - if fastPolicyName is not None and isTieringPolicySupported is True: - tierPolicyInstanceName = self.fast.get_tier_policy_by_name( - conn, storageSystemInstanceName['Name'], fastPolicyName) - - LOG.info(_LI( - "policy:%(policy)s, policy service:%(service)s, " - "masking group=%(maskingGroup)s") - % {'policy': tierPolicyInstanceName, - 'service': tierPolicyServiceInstanceName, - 'maskingGroup': maskingGroupInstanceName}) - - self.fast.delete_storage_group_from_tier_policy_rule( - conn, tierPolicyServiceInstanceName, - maskingGroupInstanceName, tierPolicyInstanceName) - - rc = self.provision.remove_device_from_storage_group( - conn, controllerConfigService, maskingGroupInstanceName, + "There are %(numVol)d volumes in the storage group " + "%(maskingGroup)s", + {'numVol': numVolInMaskingView, + 'maskingGroup': storageGroupInstanceName}) + isTieringPolicySupported, tierPolicyServiceInstanceName = ( + self._get_tiering_info(conn, storageSystemInstanceName, + fastPolicyName)) + if numVolInMaskingView == 1: + # Last volume in the storage group. + self._last_volume_delete_masking_view( + conn, storageGroupInstanceName) + self._get_and_remove_rule_association( + conn, fastPolicyName, + isTieringPolicySupported, + tierPolicyServiceInstanceName, + storageSystemInstanceName['name'], + storageGroupInstanceName) + + self.provision.remove_device_from_storage_group( + conn, controllerConfigService, storageGroupInstanceName, volumeInstance.path, volumeName) LOG.debug( - "Remove the last volume %(volumeName)s completed successfully." - % {'volumeName': volumeName}) + "Remove the last volume %(volumeName)s completed " + "successfully.", + {'volumeName': volumeName}) - # Delete storage group - conn.DeleteInstance(maskingGroupInstanceName) + # Delete storage group. + conn.DeleteInstance(storageGroupInstanceName) if isTieringPolicySupported: self._cleanup_tiering( conn, controllerConfigService, fastPolicyName, volumeInstance, volumeName) else: - # not the last volume + # Not the last volume so remove it from storage group in + # the masking view. LOG.debug("start: number of volumes in masking storage group: " - "%(numVol)d" % {'numVol': len(volumeInstanceNames)}) - rc = self.provision.remove_device_from_storage_group( - conn, controllerConfigService, maskingGroupInstanceName, + "%(numVol)d", {'numVol': len(volumeInstanceNames)}) + self.provision.remove_device_from_storage_group( + conn, controllerConfigService, storageGroupInstanceName, volumeInstance.path, volumeName) LOG.debug( "RemoveMembers for volume %(volumeName)s completed " - "successfully." % {'volumeName': volumeName}) - - # if FAST POLICY enabled, move the volume to the default SG + "successfully.", {'volumeName': volumeName}) + # 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) - # validation volumeInstanceNames = self.get_devices_from_storage_group( - conn, maskingGroupInstanceName) + conn, storageGroupInstanceName) LOG.debug( - "end: number of volumes in masking storage group: %(numVol)d" - % {'numVol': len(volumeInstanceNames)}) + "end: number of volumes in masking storage group: %(numVol)d", + {'numVol': len(volumeInstanceNames)}) - return rc + return storageGroupInstanceName def _cleanup_tiering( self, conn, controllerConfigService, fastPolicyName, volumeInstance, volumeName): - """Cleanup tiering + """Clean up tiering. :param conn: the ecom connection :param controllerConfigService: the controller configuration service @@ -1639,7 +1639,7 @@ class EMCVMAXMasking(object): self.fast.add_volume_to_default_storage_group_for_fast_policy( conn, controllerConfigService, volumeInstance, volumeName, fastPolicyName)) - # check default storage group number of volumes + # Check default storage group number of volumes. volumeInstanceNames = self.get_devices_from_storage_group( conn, defaultStorageGroupInstanceName) LOG.debug( @@ -1666,17 +1666,23 @@ class EMCVMAXMasking(object): targetWwns.append(targetPortInstanceName['Name']) return targetWwns - def get_masking_view_by_volume(self, conn, volumeInstance): + def get_masking_view_by_volume(self, conn, volumeInstance, connector): """Given volume, retrieve the masking view instance name. - :param volume: the volume instance - :param mvInstanceName: masking view instance name + :param volumeInstance: the volume instance + :param connector: the connector object + :returns mvInstanceName: masking view instance name """ - sgInstanceName = self.get_associated_masking_group_from_device( - conn, volumeInstance.path) - mvInstanceName = self.get_masking_view_from_storage_group( - conn, sgInstanceName) - LOG.debug("Found Masking View %(mv)s: " % {'mv': mvInstanceName}) + + storageSystemName = volumeInstance['SystemName'] + controllerConfigService = ( + self.utils.find_controller_configuration_service( + conn, storageSystemName)) + volumeName = volumeInstance['ElementName'] + mvInstanceName = ( + self._get_sg_or_mv_associated_with_initiator( + conn, controllerConfigService, volumeInstance.path, + volumeName, connector, False)) return mvInstanceName def get_masking_views_by_port_group(self, conn, portGroupInstanceName): @@ -1706,3 +1712,177 @@ class EMCVMAXMasking(object): else: LOG.warning(_LW("No port group found in masking view %(mv)s"), {'mv': maskingViewInstanceName}) + + def _get_sg_associated_with_connector( + self, conn, controllerConfigService, volumeInstanceName, + volumeName, connector): + """Get storage group associated with connector. + + If the connector gets passed then extra logic required to + get storage group. + + :param conn: the ecom connection + :param controllerConfigService: storage system instance name + :param volumeInstanceName + :param volumeName + :param connector + :returns: storageGroupInstanceName(can be None) + """ + return self._get_sg_or_mv_associated_with_initiator( + conn, controllerConfigService, volumeInstanceName, + volumeName, connector, True) + + def _get_tiering_info( + self, conn, storageSystemInstanceName, fastPolicyName): + """Get tiering specifics. + + :param conn: the ecom connection + :param storageSystemInstanceName: storage system instance name + :param fastPolicyName + + :returns: isTieringPolicySupported, tierPolicyServiceInstanceName + + """ + isTieringPolicySupported = False + tierPolicyServiceInstanceName = None + if fastPolicyName is not None: + tierPolicyServiceInstanceName = self.utils.get_tier_policy_service( + conn, storageSystemInstanceName) + + isTieringPolicySupported = self.fast.is_tiering_policy_enabled( + conn, tierPolicyServiceInstanceName) + LOG.debug( + "FAST policy enabled on %(storageSystem)s: %(isSupported)s", + {'storageSystem': storageSystemInstanceName, + 'isSupported': isTieringPolicySupported}) + + return isTieringPolicySupported, tierPolicyServiceInstanceName + + def _last_volume_delete_masking_view( + self, conn, storageGroupInstanceName): + """Delete the masking view. + + delete the masking view if the volume is the last one in the + storage group + + :param conn: the ecom connection + :param storageGroupInstanceName: storage group instance name + """ + # Delete masking view. + mvInstanceName = self.get_masking_view_from_storage_group( + conn, storageGroupInstanceName) + if mvInstanceName is not None: + LOG.debug( + "Last volume in the storage group, deleting masking view " + "%(mvInstanceName)s", + {'mvInstanceName': mvInstanceName}) + conn.DeleteInstance(mvInstanceName) + + def _get_and_remove_rule_association( + self, conn, fastPolicyName, isTieringPolicySupported, + tierPolicyServiceInstanceName, storageSystemName, + storageGroupInstanceName): + """Remove the storage group from the policy rule. + + :param conn: the ecom connection + :param isTieringPolicySupported: boolean + :param tierPolicyServiceInstanceName: the tier policy instance name + :param storageSystemName: storage system name + :param storageGroupInstanceName: the storage group instance name + """ + # Disassociate storage group from FAST policy. + if fastPolicyName is not None and isTieringPolicySupported is True: + tierPolicyInstanceName = self.fast.get_tier_policy_by_name( + conn, storageSystemName, fastPolicyName) + + LOG.info(_LI( + "policy:%(policy)s, policy service:%(service)s, " + "masking group=%(maskingGroup)s"), + {'policy': tierPolicyInstanceName, + 'service': tierPolicyServiceInstanceName, + 'maskingGroup': storageGroupInstanceName}) + + self.fast.delete_storage_group_from_tier_policy_rule( + conn, tierPolicyServiceInstanceName, + storageGroupInstanceName, tierPolicyInstanceName) + + def get_initiator_group_from_masking_view( + self, conn, maskingViewInstanceName): + """Get initiator group in a masking view. + + :param maskingViewInstanceName: masking view instance name + :returns: initiatorGroupInstanceName or None if it is not found + """ + initiatorGroupInstanceNames = conn.AssociatorNames( + maskingViewInstanceName, ResultClass='SE_InitiatorMaskingGroup') + if len(initiatorGroupInstanceNames) > 0: + LOG.debug("Found initiator group %(pg)s in masking view %(mv)s.", + {'pg': initiatorGroupInstanceNames[0], + 'mv': maskingViewInstanceName}) + return initiatorGroupInstanceNames[0] + else: + LOG.warn(_LW("No port group found in masking view %(mv)s."), + {'mv': maskingViewInstanceName}) + + def _get_sg_or_mv_associated_with_initiator( + self, conn, controllerConfigService, volumeInstanceName, + volumeName, connector, getSG=True): + """Get storage group or masking view associated with connector. + + If the connector gets passed then extra logic required to + get storage group. + + :param conn: the ecom connection + :param controllerConfigService: storage system instance name + :param volumeInstanceName - volume instance name + :param volumeName - volume element name + :param connector - the connector object + :param getSG - True if to get storage group; otherwise get masking + :returns: foundInstanceName(can be None) + """ + foundInstanceName = None + initiatorNames = self._find_initiator_names(conn, connector) + igInstanceNameFromConnector = self._find_initiator_masking_group( + conn, controllerConfigService, initiatorNames) + # Device can be shared by multi-SGs in a multi-host attach case. + storageGroupInstanceNames = ( + self.get_associated_masking_groups_from_device( + conn, volumeInstanceName)) + LOG.debug("Found storage groups volume " + "%(volumeName)s is in: %(storageGroups)s", + {'volumeName': volumeName, + 'storageGroups': storageGroupInstanceNames}) + if storageGroupInstanceNames: # not empty + # Get the SG by IGs. + for sgInstanceName in storageGroupInstanceNames: + # Get maskingview from storage group. + mvInstanceName = self.get_masking_view_from_storage_group( + conn, sgInstanceName) + LOG.debug("Found masking view associated with SG " + "%(storageGroup)s: %(maskingview)s", + {'maskingview': mvInstanceName, + 'storageGroup': sgInstanceName}) + # Get initiator group from masking view. + igInstanceName = ( + self.get_initiator_group_from_masking_view( + conn, mvInstanceName)) + LOG.debug("Initiator Group in masking view %(ig)s: " + "IG associated with connector%(igFromConnector)s", + {'ig': igInstanceName, + 'igFromConnector': igInstanceNameFromConnector}) + if igInstanceName == igInstanceNameFromConnector: + if getSG is True: + foundInstanceName = sgInstanceName + LOG.debug("Found the storage group associated with " + "initiator %(initiator)s: %(storageGroup)s", + {'initiator': initiatorNames, + 'storageGroup': foundInstanceName}) + else: + foundInstanceName = mvInstanceName + LOG.debug("Found the masking view associated with " + "initiator %(initiator)s: %(maskingview)s.", + {'initiator': initiatorNames, + 'maskingview': foundInstanceName}) + + break + return foundInstanceName diff --git a/cinder/volume/drivers/emc/emc_vmax_provision.py b/cinder/volume/drivers/emc/emc_vmax_provision.py index 486abe98f..5261e2969 100644 --- a/cinder/volume/drivers/emc/emc_vmax_provision.py +++ b/cinder/volume/drivers/emc/emc_vmax_provision.py @@ -389,7 +389,7 @@ class EMCVMAXProvision(object): raise exception.VolumeBackendAPIException( data=exceptionMessage) - # Find the newly created volume + # Find the newly created volume. volumeDict = self.get_volume_dict_from_job(conn, job['Job']) return volumeDict, rc @@ -639,7 +639,7 @@ class EMCVMAXProvision(object): return rc, job def get_target_endpoints(self, conn, storageHardwareService, hardwareId): - """Given the hardwareId get the + """Given the hardwareId, get the target end points. :param conn: the connection to the ecom server :param storageHardwareService: the storage HardwareId Service diff --git a/cinder/volume/drivers/emc/emc_vmax_utils.py b/cinder/volume/drivers/emc/emc_vmax_utils.py index f7ddbda6a..a032e9d37 100644 --- a/cinder/volume/drivers/emc/emc_vmax_utils.py +++ b/cinder/volume/drivers/emc/emc_vmax_utils.py @@ -64,7 +64,7 @@ class EMCVMAXUtils(object): self.protocol = prtcl def find_storage_configuration_service(self, conn, storageSystemName): - """Given the storage system name, get the storage configuration service + """Given the storage system name, get the storage configuration service. :param conn: connection to the ecom server :param storageSystemName: the storage system name @@ -835,18 +835,18 @@ class EMCVMAXUtils(object): if compositeTypeStr.lower() == stripedStr.lower(): compositeType = 3 except KeyError: - # Default to concatenated if not defined + # Default to concatenated if not defined. pass return compositeType def is_volume_bound_to_pool(self, conn, volumeInstance): - '''Check if volume is bound to a pool. + """Check if volume is bound to a pool. :param conn: the connection information to the ecom server :param storageServiceInstanceName: the storageSystem instance Name :returns: foundIsSupportsTieringPolicies - true/false - ''' + """ propertiesList = volumeInstance.properties.items() for properties in propertiesList: if properties[0] == 'EMCIsBound': @@ -861,12 +861,12 @@ class EMCVMAXUtils(object): return 'Undetermined' def get_space_consumed(self, conn, volumeInstance): - '''Check the space consumed of a volume. + """Check the space consumed of a volume. :param conn: the connection information to the ecom server :param volumeInstance: the volume Instance :returns: spaceConsumed - ''' + """ foundSpaceConsumed = None unitnames = conn.References( volumeInstance, ResultClass='CIM_AllocatedFromStoragePool', @@ -885,14 +885,14 @@ class EMCVMAXUtils(object): return foundSpaceConsumed def get_volume_size(self, conn, volumeInstance): - '''Get the volume size. + """Get the volume size. ConsumableBlocks * BlockSize :param conn: the connection information to the ecom server :param volumeInstance: the volume Instance :returns: volumeSizeOut - ''' + """ volumeSizeOut = 'Undetermined' numBlocks = 0 blockSize = 0 @@ -913,7 +913,7 @@ class EMCVMAXUtils(object): return volumeSizeOut def determine_member_count(self, sizeStr, memberCount, compositeType): - '''Determines how many members a volume should contain. + """Determines how many members a volume should contain. Based on the size of the proposed volume, the compositeType and the memberCount, determine (or validate) how many meta members there @@ -924,7 +924,7 @@ class EMCVMAXUtils(object): :param compositeType: the composite type :returns: memberCount - string :returns: errorDesc - the error description - ''' + """ errorDesc = None if compositeType in 'concatenated' and int(sizeStr) > 240: newMemberCount = int(sizeStr) / 240 @@ -1091,23 +1091,23 @@ class EMCVMAXUtils(object): return volumeTypeName def get_volumes_from_pool(self, conn, poolInstanceName): - '''Check the space consumed of a volume. + """Check the space consumed of a volume. :param conn: the connection information to the ecom server :param volumeInstance: the volume Instance :returns: spaceConsumed - ''' + """ return conn.AssociatorNames( poolInstanceName, AssocClass='CIM_AllocatedFromStoragePool', ResultClass='CIM_StorageVolume') def check_is_volume_bound_to_pool(self, conn, volumeInstance): - '''Check the space consumed of a volume. + """Check the space consumed of a volume. :param conn: the connection information to the ecom server :param volumeInstance: the volume Instance :returns: spaceConsumed - ''' + """ foundSpaceConsumed = None unitnames = conn.References( volumeInstance, ResultClass='CIM_AllocatedFromStoragePool', @@ -1130,11 +1130,11 @@ class EMCVMAXUtils(object): return 'Undetermined' def get_short_protocol_type(self, protocol): - '''Given the protocol type, return I for iscsi and F for fc + """Given the protocol type, return I for iscsi and F for fc :param protocol: iscsi or fc :returns: 'I' or 'F' - ''' + """ if protocol.lower() == ISCSI.lower(): return 'I' elif protocol.lower() == FC.lower(): @@ -1157,12 +1157,12 @@ class EMCVMAXUtils(object): return hardwareIdInstances def find_ip_protocol_endpoint(self, conn, storageSystemName): - '''Find the IP protocol endpoint for ISCSI. + """Find the IP protocol endpoint for ISCSI. :param conn: the connection to the ecom server :param storageSystemName: the storage system name :returns: foundIpAddress - ''' + """ foundIpAddress = None ipProtocolEndpointInstances = conn.EnumerateInstances( 'CIM_IPProtocolEndpoint') @@ -1262,10 +1262,10 @@ class EMCVMAXUtils(object): instance = None code, desc = arg[0], arg[1] if code == CIM_ERR_NOT_FOUND: - # Object doesn't exist any more + # Object doesn't exist any more. instance = None else: - # Something else that we cannot recover from has happened + # Something else that we cannot recover from has happened. LOG.error(_LE("Exception: %s"), six.text_type(desc)) exceptionMessage = (_( "Cannot verify the existence of object:" -- 2.45.2