From 069dd5b80db1af365052e97a811b26a72d47a094 Mon Sep 17 00:00:00 2001 From: Helen Walsh Date: Mon, 27 Mar 2017 20:35:46 +0100 Subject: [PATCH] VMAX driver - Live Migration is dropping connection When Live migrating from one compute node to another the connection drops and requires the instance to be rebooted. To prevent this from happening we need to share the storage group and port group between masking views. Change-Id: I1483ca38362c5ff1724940c2abf1179e75e02c8e Closes-Bug: #1676459 --- .../unit/volume/drivers/dell_emc/test_vmax.py | 292 +++++++++++------- cinder/volume/drivers/dell_emc/vmax/common.py | 150 ++++++--- cinder/volume/drivers/dell_emc/vmax/iscsi.py | 2 +- .../volume/drivers/dell_emc/vmax/masking.py | 241 +++++++++------ cinder/volume/drivers/dell_emc/vmax/utils.py | 73 ----- 5 files changed, 429 insertions(+), 329 deletions(-) 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.