diff --git a/cinder/tests/unit/volume/drivers/dell_emc/test_vmax.py b/cinder/tests/unit/volume/drivers/dell_emc/test_vmax.py index 0a222d7de..94e8fede5 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/test_vmax.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/test_vmax.py @@ -16,7 +16,6 @@ import ast import os import shutil -import sys import tempfile import unittest import uuid @@ -287,6 +286,10 @@ class VMAXCommonData(object): 'CreationClassName': 'CIM_DeviceMaskingGroup', 'ElementName': 'OS_default_GOLD1_SG', 'SystemName': 'SYMMETRIX+000195900551'} + sg_instance_name = { + 'CreationClassName': 'CIM_DeviceMaskingGroup', + 'ElementName': 'OS-fakehost-SRP_1-Bronze-DSS-I-SG', + 'SystemName': 'SYMMETRIX+000195900551'} storage_system = 'SYMMETRIX+000195900551' storage_system_v3 = 'SYMMETRIX-+-000197200056' port_group = 'OS-portgroup-PG' @@ -2351,7 +2354,7 @@ class VMAXISCSIDriverNoFastTestCase(test.TestCase): def test_find_device_number(self): host = 'fakehost' - data = ( + data, __, __ = ( self.driver.common.find_device_number(self.data.test_volume, host)) self.assertEqual('OS-fakehost-MV', data['maskingview']) @@ -2362,7 +2365,7 @@ class VMAXISCSIDriverNoFastTestCase(test.TestCase): return_value=[]) def test_find_device_number_false(self, mock_ref_name): host = 'bogushost' - data = ( + data, __, __ = ( self.driver.common.find_device_number(self.data.test_volume, host)) self.assertFalse(data) @@ -2370,7 +2373,7 @@ class VMAXISCSIDriverNoFastTestCase(test.TestCase): def test_find_device_number_long_host(self): # Long host name host = 'myhost.mydomain.com' - data = ( + data, __, __ = ( self.driver.common.find_device_number(self.data.test_volume, host)) self.assertEqual('OS-myhost-MV', data['maskingview']) @@ -2383,7 +2386,7 @@ class VMAXISCSIDriverNoFastTestCase(test.TestCase): v2_host_over_38 = self.data.test_volume.copy() # Pool aware scheduler enabled v2_host_over_38['host'] = host - data = ( + data, __, __ = ( self.driver.common.find_device_number(v2_host_over_38, host)) self.assertEqual(amended, data['maskingview']) @@ -3411,8 +3414,9 @@ class VMAXISCSIDriverNoFastTestCase(test.TestCase): @mock.patch.object( common.VMAXCommon, 'find_device_number', - return_value={'hostlunid': 1, - 'storagesystem': VMAXCommonData.storage_system}) + return_value=({'hostlunid': 1, + 'storagesystem': VMAXCommonData.storage_system}, + False, {})) @mock.patch.object( masking.VMAXMasking, '_wrap_get_storage_group_from_volume', @@ -3449,10 +3453,19 @@ class VMAXISCSIDriverNoFastTestCase(test.TestCase): self, _mock_volume_type, mock_wrap_group, mock_storage_group, mock_add_volume): self.driver.common._wrap_find_device_number = mock.Mock( - return_value={}) + return_value=({}, False, {})) self.driver.initialize_connection(self.data.test_volume, self.data.connector) + @mock.patch.object( + common.VMAXCommon, + '_get_port_group_from_source', + return_value={'CreationClassName': 'CIM_TargetMaskingGroup', + 'ElementName': 'OS-portgroup-PG'}) + @mock.patch.object( + common.VMAXCommon, + '_get_storage_group_from_source', + return_value=VMAXCommonData.default_sg_instance_name) @mock.patch.object( common.VMAXCommon, '_is_same_host', @@ -3460,8 +3473,17 @@ class VMAXISCSIDriverNoFastTestCase(test.TestCase): @mock.patch.object( common.VMAXCommon, 'find_device_number', - return_value={'hostlunid': 1, - 'storagesystem': VMAXCommonData.storage_system}) + return_value=({'hostlunid': 1, + 'storagesystem': VMAXCommonData.storage_system}, + True, + {'hostlunid': 1, + 'storagesystem': VMAXCommonData.storage_system})) + @mock.patch.object( + common.VMAXCommon, + '_wrap_find_device_number', + return_value=({}, True, + {'hostlunid': 1, + 'storagesystem': VMAXCommonData.storage_system})) @mock.patch.object( volume_types, 'get_volume_type_extra_specs', @@ -3469,9 +3491,10 @@ class VMAXISCSIDriverNoFastTestCase(test.TestCase): def test_map_live_migration_no_fast_success(self, _mock_volume_type, mock_wrap_device, - mock_same_host): - utils.LIVE_MIGRATION_FILE = (self.tempdir + - '/livemigrationarray') + mock_device, + mock_same_host, + mock_sg_from_mv, + mock_pg_from_mv): extraSpecs = self.data.extra_specs rollback_dict = self.driver.common._populate_masking_dict( self.data.test_volume, self.data.connector, extraSpecs) @@ -3510,7 +3533,8 @@ class VMAXISCSIDriverNoFastTestCase(test.TestCase): @mock.patch.object( common.VMAXCommon, 'find_device_number', - return_value={'storagesystem': VMAXCommonData.storage_system}) + return_value=({'storagesystem': VMAXCommonData.storage_system}, + False, {})) @mock.patch.object( masking.VMAXMasking, '_wrap_get_storage_group_from_volume', @@ -4415,8 +4439,9 @@ class VMAXISCSIDriverFastTestCase(test.TestCase): @mock.patch.object( common.VMAXCommon, 'find_device_number', - return_value={'hostlunid': 1, - 'storagesystem': VMAXCommonData.storage_system}) + return_value=({'hostlunid': 1, + 'storagesystem': VMAXCommonData.storage_system}, + False, {})) @mock.patch.object( masking.VMAXMasking, '_wrap_get_storage_group_from_volume', @@ -4436,7 +4461,8 @@ class VMAXISCSIDriverFastTestCase(test.TestCase): @mock.patch.object( common.VMAXCommon, 'find_device_number', - return_value={'storagesystem': VMAXCommonData.storage_system}) + return_value=({'storagesystem': VMAXCommonData.storage_system}, + False, {})) @mock.patch.object( masking.VMAXMasking, '_wrap_get_storage_group_from_volume', @@ -5063,7 +5089,7 @@ class VMAXFCDriverNoFastTestCase(test.TestCase): @mock.patch.object( common.VMAXCommon, 'find_device_number', - return_value={'Name': "0001"}) + return_value=({'Name': "0001"}, False, {})) @mock.patch.object( volume_types, 'get_volume_type_extra_specs', @@ -5282,7 +5308,7 @@ class VMAXFCDriverNoFastTestCase(test.TestCase): return_value=volumeInstanceName) masking = self.driver.common.masking masking.get_masking_view_from_storage_group = mock.Mock( - return_value=None) + return_value={}) self.driver.manage_existing(volume, external_ref) utils.rename_volume.assert_called_once_with( common.conn, volumeInstanceName, volume['name']) @@ -5650,7 +5676,7 @@ class VMAXFCDriverFastTestCase(test.TestCase): @mock.patch.object( common.VMAXCommon, 'find_device_number', - return_value={'Name': "0001"}) + return_value=({'Name': "0001"}, False, {})) @mock.patch.object( volume_types, 'get_volume_type_extra_specs', @@ -6716,7 +6742,7 @@ class EMCV3DriverTestCase(test.TestCase): @mock.patch.object( common.VMAXCommon, 'find_device_number', - return_value={'Name': "0001"}) + return_value=({'Name': "0001"}, False, {})) @mock.patch.object( volume_types, 'get_volume_type_extra_specs', @@ -8193,7 +8219,7 @@ class VMAXMaskingTest(test.TestCase): @mock.patch.object( masking.VMAXMasking, "_validate_masking_view", - return_value=("mv_instance", "sg_instance", None)) + return_value=("mv_instance", VMAXCommonData.sg_instance_name, None)) @mock.patch.object( masking.VMAXMasking, "_get_and_remove_from_storage_group_v3") @@ -8251,6 +8277,37 @@ class VMAXMaskingTest(test.TestCase): masking.get_or_create_masking_view_and_map_lun, common.conn, maskingViewDict, extraSpecs) + @mock.patch.object( + masking.VMAXMasking, + '_get_storage_group_from_masking_view_instance', + return_value=VMAXCommonData.sg_instance_name) + def test_check_existing_storage_group(self, mock_sg_from_mv): + common = self.driver.common + conn = self.fake_ecom_connection() + mv_instance_name = {'CreationClassName': 'Symm_LunMaskingView', + 'ElementName': 'OS-fakehost-gold-I-MV'} + masking = common.masking + sgFromMvInstanceName, msg = ( + masking._check_existing_storage_group(conn, mv_instance_name)) + self.assertEqual(VMAXCommonData.sg_instance_name, + sgFromMvInstanceName) + self.assertIsNone(msg) + + @mock.patch.object( + masking.VMAXMasking, + '_get_storage_group_from_masking_view_instance', + return_value=None) + def test_check_existing_storage_group_none(self, mock_sg_from_mv): + common = self.driver.common + conn = self.fake_ecom_connection() + mv_instance_name = {'CreationClassName': 'Symm_LunMaskingView', + 'ElementName': 'OS-fakehost-gold-I-MV'} + masking = common.masking + sgFromMvInstanceName, msg = ( + masking._check_existing_storage_group(conn, mv_instance_name)) + self.assertIsNone(sgFromMvInstanceName) + self.assertIsNotNone(msg) + class VMAXFCTest(test.TestCase): def setUp(self): @@ -8653,87 +8710,6 @@ class VMAXUtilsTest(test.TestCase): self.assertEqual('CIM_DeviceMaskingGroup', modifiedInstance['CreationClassName']) - @mock.patch.object( - common.VMAXCommon, - '_find_lun', - return_value={'SystemName': VMAXCommonData.storage_system}) - @mock.patch('builtins.open' if sys.version_info >= (3,) - else '__builtin__.open') - def test_insert_live_migration_record(self, mock_open, mock_lun): - conn = FakeEcomConnection() - self.driver.common.conn = conn - extraSpecs = self.data.extra_specs - connector = {'initiator': self.data.iscsi_initiator, - 'ip': '10.0.0.2', - 'platform': u'x86_64', - 'host': 'fakehost', - 'os_type': 'linux2', - 'multipath': False} - maskingviewdict = self.driver.common._populate_masking_dict( - self.data.test_volume, self.data.connector, extraSpecs) - utils.LIVE_MIGRATION_FILE = ('/tempdir/livemigrationarray') - self.driver.utils.insert_live_migration_record( - self.data.test_volume, maskingviewdict, connector, extraSpecs) - mock_open.assert_called_once_with( - utils.LIVE_MIGRATION_FILE, "w") - - @mock.patch.object( - common.VMAXCommon, - '_find_lun', - return_value={'SystemName': VMAXCommonData.storage_system}) - def test_delete_live_migration_record(self, mock_lun): - conn = FakeEcomConnection() - self.driver.common.conn = conn - extraSpecs = self.data.extra_specs - connector = {'initiator': self.data.iscsi_initiator, - 'ip': '10.0.0.2', - 'platform': u'x86_64', - 'host': 'fakehost', - 'os_type': 'linux2', - 'multipath': False} - maskingviewdict = self.driver.common._populate_masking_dict( - self.data.test_volume, self.data.connector, extraSpecs) - tempdir = tempfile.mkdtemp() - utils.LIVE_MIGRATION_FILE = (tempdir + - '/livemigrationarray') - m = mock.mock_open() - with mock.patch('{}.open'.format(__name__), m, create=True): - with open(utils.LIVE_MIGRATION_FILE, "w") as f: - f.write('live migration details') - self.driver.utils.insert_live_migration_record( - self.data.test_volume, maskingviewdict, connector, extraSpecs) - self.driver.utils.delete_live_migration_record(self.data.test_volume) - m.assert_called_once_with(utils.LIVE_MIGRATION_FILE, "w") - shutil.rmtree(tempdir) - - @mock.patch.object( - common.VMAXCommon, - '_find_lun', - return_value={'SystemName': VMAXCommonData.storage_system}) - def test_get_live_migration_record(self, mock_lun): - conn = FakeEcomConnection() - self.driver.common.conn = conn - extraSpecs = self.data.extra_specs - connector = {'initiator': self.data.iscsi_initiator, - 'ip': '10.0.0.2', - 'platform': u'x86_64', - 'host': 'fakehost', - 'os_type': 'linux2', - 'multipath': False} - maskingviewdict = self.driver.common._populate_masking_dict( - self.data.test_volume, self.data.connector, extraSpecs) - tempdir = tempfile.mkdtemp() - utils.LIVE_MIGRATION_FILE = (tempdir + - '/livemigrationarray') - self.driver.utils.insert_live_migration_record( - self.data.test_volume, maskingviewdict, connector, extraSpecs) - record = self.driver.utils.get_live_migration_record( - self.data.test_volume, False) - self.assertEqual(maskingviewdict, record[0]) - self.assertEqual(connector, record[1]) - os.remove(utils.LIVE_MIGRATION_FILE) - shutil.rmtree(tempdir) - def test_get_iqn(self): conn = FakeEcomConnection() iqn = "iqn.1992-04.com.emc:600009700bca30c01b9c012000000003,t,0x0001" @@ -9503,6 +9479,102 @@ class VMAXCommonTest(test.TestCase): self.driver.create_snapshot(snapshot) self.driver.delete_snapshot(snapshot) + @mock.patch.object( + masking.VMAXMasking, + 'get_associated_masking_groups_from_device', + return_value=[VMAXCommonData.sg_instance_name]) + @mock.patch.object( + masking.VMAXMasking, + 'get_masking_view_from_storage_group', + return_value=[{'CreationClassName': 'Symm_LunMaskingView', + 'ElementName': 'OS-fakehost-gold-I-MV'}]) + def test_is_volume_multiple_masking_views_false(self, mock_mv_from_sg, + mock_sg_from_dev): + common = self.driver.common + common.conn = FakeEcomConnection() + volumeInstanceName = ( + common.conn.EnumerateInstanceNames("EMC_StorageVolume")[0]) + volumeInstance = common.conn.GetInstance(volumeInstanceName) + self.assertFalse( + common._is_volume_multiple_masking_views(volumeInstance)) + + @mock.patch.object( + masking.VMAXMasking, + 'get_associated_masking_groups_from_device', + return_value=[VMAXCommonData.sg_instance_name]) + @mock.patch.object( + masking.VMAXMasking, + 'get_masking_view_from_storage_group', + return_value=[{'CreationClassName': 'Symm_LunMaskingView', + 'ElementName': 'OS-fakehost-gold-I-MV'}, + {'CreationClassName': 'Symm_LunMaskingView', + 'ElementName': 'OS-fakehost-bronze-I-MV'}]) + def test_is_volume_multiple_masking_views_true(self, mock_mv_from_sg, + mock_sg_from_dev): + common = self.driver.common + common.conn = FakeEcomConnection() + volumeInstanceName = ( + common.conn.EnumerateInstanceNames("EMC_StorageVolume")[0]) + volumeInstance = common.conn.GetInstance(volumeInstanceName) + self.assertTrue( + common._is_volume_multiple_masking_views(volumeInstance)) + + @mock.patch.object( + masking.VMAXMasking, + '_get_storage_group_from_masking_view_instance', + return_value=VMAXCommonData.sg_instance_name) + def test_get_storage_group_from_source(self, mock_sg_from_mv): + common = self.driver.common + common.conn = FakeEcomConnection() + mv_instance_name = {'CreationClassName': 'Symm_LunMaskingView', + 'ElementName': 'OS-fakehost-gold-I-MV'} + deviceInfoDict = {'controller': mv_instance_name} + self.assertEqual(VMAXCommonData.sg_instance_name, + common._get_storage_group_from_source( + deviceInfoDict)) + + @mock.patch.object( + masking.VMAXMasking, + '_get_storage_group_from_masking_view_instance', + return_value=VMAXCommonData.sg_instance_name) + def test_get_storage_group_from_source_except(self, mock_sg_from_mv): + common = self.driver.common + common.conn = FakeEcomConnection() + deviceInfoDict = {} + self.assertRaises( + exception.VolumeBackendAPIException, + common._get_storage_group_from_source, deviceInfoDict) + + @mock.patch.object( + masking.VMAXMasking, + 'get_port_group_from_masking_view_instance', + return_value={'CreationClassName': 'CIM_TargetMaskingGroup', + 'ElementName': 'OS-portgroup-PG'}) + def test_get_port_group_from_source(self, mock_pg_from_mv): + common = self.driver.common + common.conn = FakeEcomConnection() + pg_instance_name = {'CreationClassName': 'CIM_TargetMaskingGroup', + 'ElementName': 'OS-portgroup-PG'} + mv_instance_name = {'CreationClassName': 'Symm_LunMaskingView', + 'ElementName': 'OS-fakehost-gold-I-MV'} + deviceInfoDict = {'controller': mv_instance_name} + self.assertEqual(pg_instance_name, + common._get_port_group_from_source( + deviceInfoDict)) + + @mock.patch.object( + masking.VMAXMasking, + 'get_port_group_from_masking_view_instance', + return_value={'CreationClassName': 'CIM_TargetMaskingGroup', + 'ElementName': 'OS-portgroup-PG'}) + def test_get_port_group_from_source_except(self, mock_pg_from_mv): + common = self.driver.common + common.conn = FakeEcomConnection() + deviceInfoDict = {} + self.assertRaises( + exception.VolumeBackendAPIException, + common._get_port_group_from_source, deviceInfoDict) + class VMAXProvisionTest(test.TestCase): def setUp(self): @@ -9606,10 +9678,11 @@ class VMAXISCSITest(test.TestCase): driver.db = FakeDB() self.driver = driver - def test_smis_get_iscsi_properties(self): - device_info = {'hostlunid': 1} - self.driver.common.find_device_number = ( - mock.Mock(return_value=device_info)) + @mock.patch.object( + common.VMAXCommon, + 'find_device_number', + return_value=({'hostlunid': 1}, False, {})) + def test_smis_get_iscsi_properties(self, mock_device): iqns_and_ips = ( [{'iqn': 'iqn.1992-04.com.emc:50000973f006dd80,t,0x0001', 'ip': '10.10.0.50'}, @@ -9627,8 +9700,9 @@ class VMAXISCSITest(test.TestCase): @mock.patch.object( common.VMAXCommon, 'find_device_number', - return_value={'hostlunid': 1, - 'storagesystem': VMAXCommonData.storage_system}) + return_value=({'hostlunid': 1, + 'storagesystem': VMAXCommonData.storage_system}, + False, {})) @mock.patch.object( common.VMAXCommon, 'initialize_connection', diff --git a/cinder/volume/drivers/dell_emc/vmax/common.py b/cinder/volume/drivers/dell_emc/vmax/common.py index 7fa3eacfb..f57709b85 100644 --- a/cinder/volume/drivers/dell_emc/vmax/common.py +++ b/cinder/volume/drivers/dell_emc/vmax/common.py @@ -518,7 +518,8 @@ class VMAXCommon(object): LOG.info("Unmap volume: %(volume)s.", {'volume': volumename}) - device_info = self.find_device_number(volume, connector['host']) + device_info, __, __ = self.find_device_number( + volume, connector['host']) if 'hostlunid' not in device_info: LOG.info("Volume %s is not mapped. No volume to unmap.", volumename) @@ -527,6 +528,9 @@ class VMAXCommon(object): vol_instance = self._find_lun(volume) storage_system = vol_instance['SystemName'] + if self._is_volume_multiple_masking_views(vol_instance): + return + configservice = self.utils.find_controller_configuration_service( self.conn, storage_system) if configservice is None: @@ -538,16 +542,23 @@ class VMAXCommon(object): self._remove_members(configservice, vol_instance, connector, extraSpecs) - livemigrationrecord = self.utils.get_live_migration_record(volume, - False) - if livemigrationrecord: - live_maskingviewdict = livemigrationrecord[0] - live_connector = livemigrationrecord[1] - live_extraSpecs = livemigrationrecord[2] - self._attach_volume( - volume, live_connector, live_extraSpecs, - live_maskingviewdict, True) - self.utils.delete_live_migration_record(volume) + + def _is_volume_multiple_masking_views(self, vol_instance): + """Check if volume is in more than one MV. + + :param vol_instance: the volume instance + :returns: boolean + """ + storageGroupInstanceNames = ( + self.masking.get_associated_masking_groups_from_device( + self.conn, vol_instance.path)) + + for storageGroupInstanceName in storageGroupInstanceNames: + mvInstanceNames = self.masking.get_masking_view_from_storage_group( + self.conn, storageGroupInstanceName) + if len(mvInstanceNames) > 1: + return True + return False def initialize_connection(self, volume, connector): """Initializes the connection and returns device and connection info. @@ -586,38 +597,39 @@ class VMAXCommon(object): LOG.info("Initialize connection: %(volume)s.", {'volume': volumeName}) self.conn = self._get_ecom_connection() - deviceInfoDict = self._wrap_find_device_number( - volume, connector['host']) + if self.utils.is_volume_failed_over(volume): extraSpecs = self._get_replication_extraSpecs( extraSpecs, self.rep_config) + deviceInfoDict, isLiveMigration, sourceInfoDict = ( + self._wrap_find_device_number( + volume, connector['host'])) maskingViewDict = self._populate_masking_dict( volume, connector, extraSpecs) if ('hostlunid' in deviceInfoDict and deviceInfoDict['hostlunid'] is not None): - isSameHost = self._is_same_host(connector, deviceInfoDict) - if isSameHost: - # Device is already mapped to same host so we will leave - # the state as is. - - deviceNumber = deviceInfoDict['hostlunid'] - LOG.info("Volume %(volume)s is already mapped. " - "The device number is %(deviceNumber)s.", - {'volume': volumeName, - 'deviceNumber': deviceNumber}) - # Special case, we still need to get the iscsi ip address. - portGroupName = ( - self._get_correct_port_group( - deviceInfoDict, maskingViewDict['storageSystemName'])) - - else: + deviceNumber = deviceInfoDict['hostlunid'] + LOG.info("Volume %(volume)s is already mapped. " + "The device number is %(deviceNumber)s.", + {'volume': volumeName, + 'deviceNumber': deviceNumber}) + # Special case, we still need to get the iscsi ip address. + portGroupName = ( + self._get_correct_port_group( + deviceInfoDict, maskingViewDict['storageSystemName'])) + else: + if isLiveMigration: + maskingViewDict['storageGroupInstanceName'] = ( + self._get_storage_group_from_source(sourceInfoDict)) + maskingViewDict['portGroupInstanceName'] = ( + self._get_port_group_from_source(sourceInfoDict)) deviceInfoDict, portGroupName = self._attach_volume( volume, connector, extraSpecs, maskingViewDict, True) - else: - deviceInfoDict, portGroupName = ( - self._attach_volume( - volume, connector, extraSpecs, maskingViewDict)) + else: + deviceInfoDict, portGroupName = ( + self._attach_volume( + volume, connector, extraSpecs, maskingViewDict)) if self.protocol.lower() == 'iscsi': deviceInfoDict['ip_and_iqn'] = ( @@ -645,12 +657,8 @@ class VMAXCommon(object): :raises: VolumeBackendAPIException """ volumeName = volume['name'] - maskingViewDict = self._populate_masking_dict( - volume, connector, extraSpecs) if isLiveMigration: maskingViewDict['isLiveMigration'] = True - self.utils.insert_live_migration_record(volume, maskingViewDict, - connector, extraSpecs) else: maskingViewDict['isLiveMigration'] = False @@ -658,7 +666,8 @@ class VMAXCommon(object): self.conn, maskingViewDict, extraSpecs) # Find host lun id again after the volume is exported to the host. - deviceInfoDict = self.find_device_number(volume, connector['host']) + deviceInfoDict, __, __ = self.find_device_number( + volume, connector['host']) if 'hostlunid' not in deviceInfoDict: # Did not successfully attach to host, # so a rollback for FAST is required. @@ -668,7 +677,6 @@ class VMAXCommon(object): (rollbackDict['isV3'] is not None)): (self.masking._check_if_rollback_action_for_masking_required( self.conn, rollbackDict)) - self.utils.delete_live_migration_record(volume) exception_message = (_("Error Attaching volume %(vol)s.") % {'vol': volumeName}) raise exception.VolumeBackendAPIException( @@ -737,6 +745,52 @@ class VMAXCommon(object): data=exception_message) return portGroupName + def _get_storage_group_from_source(self, deviceInfoDict): + """Get the storage group from the existing masking view. + + :params deviceInfoDict: the device info dictionary + :returns: storage group instance + """ + storageGroupInstanceName = None + if ('controller' in deviceInfoDict and + deviceInfoDict['controller'] is not None): + maskingViewInstanceName = deviceInfoDict['controller'] + + # Get the storage group from masking view + storageGroupInstanceName = ( + self.masking._get_storage_group_from_masking_view_instance( + self.conn, + maskingViewInstanceName)) + else: + exception_message = (_("Cannot get the storage group from " + "the masking view.")) + raise exception.VolumeBackendAPIException( + data=exception_message) + return storageGroupInstanceName + + def _get_port_group_from_source(self, deviceInfoDict): + """Get the port group from the existing masking view. + + :params deviceInfoDict: the device info dictionary + :returns: port group instance + """ + portGroupInstanceName = None + if ('controller' in deviceInfoDict and + deviceInfoDict['controller'] is not None): + maskingViewInstanceName = deviceInfoDict['controller'] + + # Get the port group from masking view + portGroupInstanceName = ( + self.masking.get_port_group_from_masking_view_instance( + self.conn, + maskingViewInstanceName)) + else: + exception_message = (_("Cannot get the port group from " + "the masking view.")) + raise exception.VolumeBackendAPIException( + data=exception_message) + return portGroupInstanceName + def check_ig_instance_name(self, initiatorGroupInstanceName): """Check if an initiator group instance is on the array. @@ -1898,6 +1952,8 @@ class VMAXCommon(object): volumeName = volume['name'] volumeInstance = self._find_lun(volume) storageSystemName = volumeInstance['SystemName'] + isLiveMigration = False + source_data = {} unitnames = self.conn.ReferenceNames( volumeInstance.path, @@ -1942,14 +1998,15 @@ class VMAXCommon(object): data = maskedvol if not data: if len(maskedvols) > 0: - data = maskedvols[0] + source_data = maskedvols[0] LOG.warning( "Volume is masked but not to host %(host)s as is " "expected. Assuming live migration.", {'host': hoststr}) + isLiveMigration = True LOG.debug("Device info: %(data)s.", {'data': data}) - return data + return data, isLiveMigration, source_data def get_target_wwns(self, storageSystem, connector): """Find target WWNs. @@ -2259,6 +2316,10 @@ class VMAXCommon(object): maskingViewDict['maskingViewName'] = ("%(prefix)s-MV" % {'prefix': prefix}) + + maskingViewDict['maskingViewNameLM'] = ("%(prefix)s-%(volid)s-MV" + % {'prefix': prefix, + 'volid': volume['id'][:8]}) volumeName = volume['name'] volumeInstance = self._find_lun(volume) storageSystemName = volumeInstance['SystemName'] @@ -4597,9 +4658,10 @@ class VMAXCommon(object): self.conn, volumeInstanceName)) for sgInstanceName in sgInstanceNames: - mvInstanceName = self.masking.get_masking_view_from_storage_group( - self.conn, sgInstanceName) - if mvInstanceName: + mvInstanceNames = ( + self.masking.get_masking_view_from_storage_group( + self.conn, sgInstanceName)) + for mvInstanceName in mvInstanceNames: exceptionMessage = (_( "Unable to import volume %(deviceId)s to cinder. " "Volume is in masking view %(mv)s.") diff --git a/cinder/volume/drivers/dell_emc/vmax/iscsi.py b/cinder/volume/drivers/dell_emc/vmax/iscsi.py index d389a6353..c20142f0b 100644 --- a/cinder/volume/drivers/dell_emc/vmax/iscsi.py +++ b/cinder/volume/drivers/dell_emc/vmax/iscsi.py @@ -233,7 +233,7 @@ class VMAXISCSIDriver(driver.ISCSIDriver): meaning use CHAP with the specified credentials. """ - device_info = self.common.find_device_number( + device_info, __, __ = self.common.find_device_number( volume, connector['host']) isError = False diff --git a/cinder/volume/drivers/dell_emc/vmax/masking.py b/cinder/volume/drivers/dell_emc/vmax/masking.py index fc60e2687..20df3cacd 100644 --- a/cinder/volume/drivers/dell_emc/vmax/masking.py +++ b/cinder/volume/drivers/dell_emc/vmax/masking.py @@ -89,7 +89,11 @@ class VMAXMasking(object): defaultStorageGroupInstanceName = None fastPolicyName = None storageGroupInstanceName = None - if isLiveMigration is False: + if isLiveMigration: + maskingViewDict['maskingViewName'] = ( + maskingViewDict['maskingViewNameLM']) + maskingViewName = maskingViewDict['maskingViewNameLM'] + else: if isV3: defaultStorageGroupInstanceName = ( self._get_v3_default_storagegroup_instancename( @@ -106,11 +110,6 @@ class VMAXMasking(object): volumeInstance.path, volumeName, fastPolicyName, extraSpecs)) - else: - # Live Migration - self.remove_and_reset_members( - conn, controllerConfigService, volumeInstance, volumeName, - extraSpecs, maskingViewDict['connector'], False) # If anything has gone wrong with the masking view we rollback try: @@ -118,6 +117,8 @@ class VMAXMasking(object): self._validate_masking_view(conn, maskingViewDict, defaultStorageGroupInstanceName, extraSpecs)) + instance = conn.GetInstance(storageGroupInstanceName) + maskingViewDict['sgGroupName'] = instance['ElementName'] LOG.debug( "The masking view in the attach operation is " "%(maskingViewInstanceName)s. The storage group " @@ -286,17 +287,38 @@ class VMAXMasking(object): LOG.info("Returning random Port Group: %(portGroupName)s.", {'portGroupName': pgGroupName}) - storageGroupInstanceName, errorMessage = ( - self._check_storage_group( - conn, maskingViewDict, defaultStorageGroupInstanceName)) - if errorMessage: - return None, storageGroupInstanceName, errorMessage + if maskingViewDict['isLiveMigration']: + try: + # We are sharing the storage group and port group + # between host and target + storageGroupInstanceName = ( + maskingViewDict['storageGroupInstanceName']) + storageGroupinstance = conn.GetInstance( + storageGroupInstanceName) + maskingViewDict['sgGroupName'] = ( + storageGroupinstance['ElementName']) + portGroupInstanceName = ( + maskingViewDict['portGroupInstanceName']) + portGroupInstance = conn.GetInstance( + portGroupInstanceName) + maskingViewDict['pgGroupName'] = ( + portGroupInstance['ElementName']) + except Exception: + errorMessage = (_( + "Unable to get storage group for live migration.")) + return None, None, errorMessage + else: + 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 + portGroupInstanceName, errorMessage = ( + self._check_port_group(conn, controllerConfigService, + pgGroupName)) + if errorMessage: + return None, storageGroupInstanceName, errorMessage initiatorGroupInstanceName, errorMessage = ( self._check_initiator_group(conn, controllerConfigService, @@ -337,7 +359,6 @@ class VMAXMasking(object): """ storageGroupInstanceName = None controllerConfigService = maskingViewDict['controllerConfigService'] - sgGroupName = maskingViewDict['sgGroupName'] igGroupName = maskingViewDict['igGroupName'] connector = maskingViewDict['connector'] storageSystemName = maskingViewDict['storageSystemName'] @@ -353,10 +374,9 @@ class VMAXMasking(object): if errorMessage: return storageGroupInstanceName, errorMessage + # Get the storage group from masking view storageGroupInstanceName, errorMessage = ( - self._check_existing_storage_group( - conn, controllerConfigService, sgGroupName, - maskingViewInstanceName)) + self._check_existing_storage_group(conn, maskingViewInstanceName)) return storageGroupInstanceName, errorMessage @@ -385,19 +405,15 @@ class VMAXMasking(object): return storageGroupInstanceName, msg def _check_existing_storage_group( - self, conn, controllerConfigService, - sgGroupName, maskingViewInstanceName): + self, conn, 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 :returns: string -- msg, the error message """ msg = None - sgFromMvInstanceName = ( self._get_storage_group_from_masking_view_instance( conn, maskingViewInstanceName)) @@ -405,10 +421,9 @@ class VMAXMasking(object): if sgFromMvInstanceName is None: # This may be used in exception hence the use of _. msg = (_( - "Cannot get storage group: %(sgGroupName)s " - "from masking view %(maskingViewInstanceName)s. ") % - {'sgGroupName': sgGroupName, - 'maskingViewInstanceName': maskingViewInstanceName}) + "Cannot get storage group from masking view " + "%(maskingViewInstanceName)s. ") % + {'maskingViewInstanceName': maskingViewInstanceName}) LOG.error(msg) return sgFromMvInstanceName, msg @@ -1619,19 +1634,36 @@ class VMAXMasking(object): foundView = self._find_masking_view( conn, maskingViewName, storageSystemName) if foundView: - groups = conn.AssociatorNames( - foundView, - ResultClass='CIM_TargetMaskingGroup') - if len(groups) > 0: - foundPortMaskingGroupInstanceName = groups[0] + foundPortMaskingGroupInstanceName = ( + self.get_port_group_from_masking_view_instance( + conn, foundView)) LOG.debug( - "Masking view: %(view)s InitiatorMaskingGroup: %(masking)s.", + "Masking view: %(view)s portMaskingGroup: %(masking)s.", {'view': maskingViewName, 'masking': foundPortMaskingGroupInstanceName}) return foundPortMaskingGroupInstanceName + def get_port_group_from_masking_view_instance( + self, conn, maskingViewInstanceName): + """Given the masking view name get the port group from it. + + :param conn: connection to the ecom server + :param maskingViewInstanceName: the masking view instance name + :returns: instance name foundPortMaskingGroupInstanceName + """ + + foundPortMaskingGroupInstanceName = None + + groups = conn.AssociatorNames( + maskingViewInstanceName, + ResultClass='CIM_TargetMaskingGroup') + if len(groups) > 0: + foundPortMaskingGroupInstanceName = groups[0] + + return foundPortMaskingGroupInstanceName + def _delete_masking_view( self, conn, controllerConfigService, maskingViewName, maskingViewInstanceName, extraSpecs): @@ -1673,14 +1705,11 @@ class VMAXMasking(object): :param storageGroupInstanceName: the storage group instance name :returns: instance name foundMaskingViewInstanceName """ - foundMaskingViewInstanceName = None maskingViews = conn.AssociatorNames( storageGroupInstanceName, ResultClass='Symm_LunMaskingView') - if len(maskingViews) > 0: - foundMaskingViewInstanceName = maskingViews[0] - return foundMaskingViewInstanceName + return maskingViews def add_volume_to_storage_group( self, conn, controllerConfigService, storageGroupInstanceName, @@ -1912,12 +1941,10 @@ class VMAXMasking(object): """ instance = conn.GetInstance(storageGroupInstanceName, LocalOnly=False) storageGroupName = instance['ElementName'] - mvInstanceName = self.get_masking_view_from_storage_group( + mvInstanceNames = self.get_masking_view_from_storage_group( conn, storageGroupInstanceName) - if mvInstanceName is None: - LOG.debug("Unable to get masking view %(maskingView)s " - "from storage group.", - {'maskingView': mvInstanceName}) + if not mvInstanceNames: + LOG.debug("Unable to get masking views from storage group.") @coordination.synchronized("emc-sg-{storageGroup}") def do_remove_volume_from_sg(storageGroup): @@ -1947,41 +1974,43 @@ class VMAXMasking(object): return do_remove_volume_from_sg(storageGroupName) else: - # need to lock masking view when we are locking the storage - # group to avoid possible deadlock situations from concurrent - # processes - maskingViewInstance = conn.GetInstance( - mvInstanceName, LocalOnly=False) - maskingViewName = maskingViewInstance['ElementName'] + for mvInstanceName in mvInstanceNames: + # need to lock masking view when we are locking the storage + # group to avoid possible deadlock situations from concurrent + # processes + maskingViewInstance = conn.GetInstance( + mvInstanceName, LocalOnly=False) + maskingViewName = maskingViewInstance['ElementName'] - @coordination.synchronized("emc-mv-{maskingView}") - @coordination.synchronized("emc-sg-{storageGroup}") - def do_remove_volume_from_sg(maskingView, storageGroup): - volumeInstanceNames = self.get_devices_from_storage_group( - conn, storageGroupInstanceName) - numVolInStorageGroup = len(volumeInstanceNames) - LOG.debug( - "There are %(numVol)d volumes in the storage group " - "%(maskingGroup)s associated with %(mvName)s", - {'numVol': numVolInStorageGroup, - 'maskingGroup': storageGroup, - 'mvName': maskingView}) + @coordination.synchronized("emc-mv-{maskingView}") + @coordination.synchronized("emc-sg-{storageGroup}") + def do_remove_volume_from_sg(maskingView, storageGroup): + volumeInstanceNames = self.get_devices_from_storage_group( + conn, storageGroupInstanceName) + numVolInStorageGroup = len(volumeInstanceNames) + LOG.debug( + "There are %(numVol)d volumes in the storage group " + "%(maskingGroup)s associated with %(mvName)s", + {'numVol': numVolInStorageGroup, + 'maskingGroup': storageGroup, + 'mvName': maskingView}) - if numVolInStorageGroup == 1: - # Last volume in the storage group. - self._last_vol_in_SG( - conn, controllerConfigService, - storageGroupInstanceName, - storageGroupName, volumeInstance, - volumeInstance['ElementName'], extraSpecs) - else: - # Not the last volume so remove it from storage group - self._multiple_vols_in_SG( - conn, controllerConfigService, - storageGroupInstanceName, - volumeInstance, volumeInstance['ElementName'], - numVolInStorageGroup, extraSpecs) - return do_remove_volume_from_sg(maskingViewName, storageGroupName) + if numVolInStorageGroup == 1: + # Last volume in the storage group. + self._last_vol_in_SG( + conn, controllerConfigService, + storageGroupInstanceName, + storageGroupName, volumeInstance, + volumeInstance['ElementName'], extraSpecs) + else: + # Not the last volume so remove it from storage group + self._multiple_vols_in_SG( + conn, controllerConfigService, + storageGroupInstanceName, + volumeInstance, volumeInstance['ElementName'], + numVolInStorageGroup, extraSpecs) + return do_remove_volume_from_sg(maskingViewName, + storageGroupName) def _last_vol_in_SG( self, conn, controllerConfigService, storageGroupInstanceName, @@ -2009,9 +2038,9 @@ class VMAXMasking(object): LOG.debug("Only one volume remains in storage group " "%(sgname)s. Driver will attempt cleanup.", {'sgname': storageGroupName}) - mvInstanceName = self.get_masking_view_from_storage_group( + mvInstanceNames = self.get_masking_view_from_storage_group( conn, storageGroupInstanceName) - if mvInstanceName is None: + if not mvInstanceNames: # Remove the volume from the storage group and delete the SG. self._remove_last_vol_and_delete_sg( conn, controllerConfigService, @@ -2020,18 +2049,21 @@ class VMAXMasking(object): volumeName, extraSpecs) status = True else: - maskingViewInstance = conn.GetInstance( - mvInstanceName, LocalOnly=False) - maskingViewName = maskingViewInstance['ElementName'] + mv_count = len(mvInstanceNames) + for mvInstanceName in mvInstanceNames: + maskingViewInstance = conn.GetInstance( + mvInstanceName, LocalOnly=False) + maskingViewName = maskingViewInstance['ElementName'] - def do_delete_mv_ig_and_sg(): - return self._delete_mv_ig_and_sg( - conn, controllerConfigService, mvInstanceName, - maskingViewName, storageGroupInstanceName, - storageGroupName, volumeInstance, volumeName, - extraSpecs) - do_delete_mv_ig_and_sg() - status = True + def do_delete_mv_ig_and_sg(): + return self._delete_mv_ig_and_sg( + conn, controllerConfigService, mvInstanceName, + maskingViewName, storageGroupInstanceName, + storageGroupName, volumeInstance, volumeName, + extraSpecs, mv_count) + do_delete_mv_ig_and_sg() + status = True + mv_count -= 1 return status def _multiple_vols_in_SG( @@ -2069,7 +2101,7 @@ class VMAXMasking(object): def _delete_mv_ig_and_sg( self, conn, controllerConfigService, mvInstanceName, maskingViewName, storageGroupInstanceName, storageGroupName, - volumeInstance, volumeName, extraSpecs): + volumeInstance, volumeName, extraSpecs, mv_count): """Delete the Masking view, the storage Group and the initiator group. :param conn: connection to the ecom server @@ -2081,6 +2113,7 @@ class VMAXMasking(object): :param volumeInstance: the volume Instance :param volumeName: the volume name :param extraSpecs: extra specs + :param mv_count: number of masking views """ isV3 = extraSpecs[ISV3] fastPolicyName = extraSpecs.get(FASTPOLICY, None) @@ -2108,16 +2141,20 @@ class VMAXMasking(object): storageSystemInstanceName['Name'], storageGroupInstanceName, extraSpecs) - self._remove_last_vol_and_delete_sg( - conn, controllerConfigService, storageGroupInstanceName, - storageGroupName, volumeInstance.path, volumeName, - extraSpecs) + if mv_count == 1: + if self._is_volume_in_storage_group( + conn, storageGroupInstanceName, + volumeInstance, storageGroupName): + self._remove_last_vol_and_delete_sg( + conn, controllerConfigService, storageGroupInstanceName, + storageGroupName, volumeInstance.path, volumeName, + extraSpecs) - LOG.debug( - "Volume %(volumeName)s successfully removed from SG and " - "Storage Group %(storageGroupName)s successfully deleted. ", - {'volumeName': volumeName, - 'storageGroupName': storageGroupName}) + LOG.debug( + "Volume %(volumeName)s successfully removed from SG and " + "Storage Group %(storageGroupName)s successfully deleted. ", + {'volumeName': volumeName, + 'storageGroupName': storageGroupName}) def _return_back_to_default_sg( self, conn, controllerConfigService, volumeInstance, volumeName, @@ -2472,10 +2509,10 @@ class VMAXMasking(object): # Get the SG by IGs. for sgInstanceName in storageGroupInstanceNames: # Get maskingview from storage group. - mvInstanceName = self.get_masking_view_from_storage_group( + mvInstanceNames = self.get_masking_view_from_storage_group( conn, sgInstanceName) # Get initiator group from masking view. - if mvInstanceName: + for mvInstanceName in mvInstanceNames: LOG.debug("Found masking view associated with SG " "%(storageGroup)s: %(maskingview)s", {'maskingview': mvInstanceName, diff --git a/cinder/volume/drivers/dell_emc/vmax/utils.py b/cinder/volume/drivers/dell_emc/vmax/utils.py index e88f07c21..85eed6c4b 100644 --- a/cinder/volume/drivers/dell_emc/vmax/utils.py +++ b/cinder/volume/drivers/dell_emc/vmax/utils.py @@ -16,14 +16,12 @@ import ast import datetime import hashlib -import os import random import re import time from xml.dom import minidom from oslo_log import log as logging -from oslo_serialization import jsonutils from oslo_service import loopingcall from oslo_utils import units import six @@ -2680,77 +2678,6 @@ class VMAXUtils(object): PropertyList=propertylist) return modifiedInstance - def insert_live_migration_record(self, volume, maskingviewdict, - connector, extraSpecs): - """Insert a record of live migration destination into a temporary file - - :param volume: the volume dictionary - :param maskingviewdict: the storage group instance name - :param connector: the connector Object - :param extraSpecs: the extraSpecs dict - """ - live_migration_details = self.get_live_migration_record(volume, True) - if live_migration_details: - if volume['id'] not in live_migration_details: - live_migration_details[volume['id']] = [maskingviewdict, - connector, extraSpecs] - else: - live_migration_details = {volume['id']: [maskingviewdict, - connector, extraSpecs]} - try: - with open(LIVE_MIGRATION_FILE, "w") as f: - jsonutils.dump(live_migration_details, f) - except Exception: - exceptionMessage = (_( - "Error in processing live migration file.")) - LOG.exception(exceptionMessage) - raise exception.VolumeBackendAPIException( - data=exceptionMessage) - - def delete_live_migration_record(self, volume): - """Delete record of live migration - - Delete record of live migration destination from file and if - after deletion of record, delete file if empty. - - :param volume: the volume dictionary - """ - live_migration_details = self.get_live_migration_record(volume, True) - if live_migration_details: - if volume['id'] in live_migration_details: - del live_migration_details[volume['id']] - with open(LIVE_MIGRATION_FILE, "w") as f: - jsonutils.dump(live_migration_details, f) - else: - LOG.debug("%(Volume)s doesn't exist in live migration " - "record.", - {'Volume': volume['id']}) - if not live_migration_details: - os.remove(LIVE_MIGRATION_FILE) - - def get_live_migration_record(self, volume, returnallrecords): - """get record of live migration destination from a temporary file - - :param volume: the volume dictionary - :param returnallrecords: if true, return all records in file - :returns: returns a single record or all records depending on - returnallrecords flag - """ - returned_record = None - if os.path.isfile(LIVE_MIGRATION_FILE): - with open(LIVE_MIGRATION_FILE, "rb") as f: - live_migration_details = jsonutils.load(f) - if returnallrecords: - returned_record = live_migration_details - else: - if volume['id'] in live_migration_details: - returned_record = live_migration_details[volume['id']] - else: - LOG.debug("%(Volume)s doesn't exist in live migration " - "record.", - {'Volume': volume['id']}) - return returned_record - def get_iqn(self, conn, ipendpointinstancename): """Get the IPv4Address from the ip endpoint instance name.