diff --git a/cinder/tests/unit/volume/drivers/dell_emc/sc/test_fc.py b/cinder/tests/unit/volume/drivers/dell_emc/sc/test_fc.py index d0169a4f95d..bb2fcb1ce0d 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/sc/test_fc.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/sc/test_fc.py @@ -18,6 +18,7 @@ from cinder import context from cinder import exception from cinder import test from cinder.tests.unit import fake_constants as fake +from cinder.tests.unit import fake_volume from cinder.volume.drivers.dell_emc.sc import storagecenter_api from cinder.volume.drivers.dell_emc.sc import storagecenter_fc @@ -941,6 +942,106 @@ class DellSCSanFCDriverTestCase(test.TestCase): 'driver_volume_type': 'fibre_channel'} self.assertEqual(expected, res, 'Unexpected return data') + @mock.patch.object(storagecenter_api.SCApi, + 'find_server', + return_value=SCSERVER) + @mock.patch.object(storagecenter_api.SCApi, + 'find_volume', + return_value=VOLUME) + @mock.patch.object(storagecenter_api.SCApi, + 'unmap_volume', + return_value=True) + @mock.patch.object(storagecenter_api.SCApi, + 'find_wwns', + return_value=(1, + [u'5000D31000FCBE3D', + u'5000D31000FCBE35'], + {u'21000024FF30441C': + [u'5000D31000FCBE35'], + u'21000024FF30441D': + [u'5000D31000FCBE3D']})) + @mock.patch.object(storagecenter_api.SCApi, + 'get_volume_count', + return_value=1) + def test_terminate_connection_multiattached_host(self, + mock_get_volume_count, + mock_find_wwns, + mock_unmap_volume, + mock_find_volume, + mock_find_server, + mock_close_connection, + mock_open_connection, + mock_init): + connector = self.connector + + attachment1 = fake_volume.volume_attachment_ovo(self._context) + attachment1.connector = connector + attachment1.attached_host = connector['host'] + attachment1.attach_status = 'attached' + + attachment2 = fake_volume.volume_attachment_ovo(self._context) + attachment2.connector = connector + attachment2.attached_host = connector['host'] + attachment2.attach_status = 'attached' + + vol = fake_volume.fake_volume_obj(self._context) + vol.multiattach = True + vol.volume_attachment.objects.append(attachment1) + vol.volume_attachment.objects.append(attachment2) + + self.driver.terminate_connection(vol, connector) + mock_unmap_volume.assert_not_called() + + @mock.patch.object(storagecenter_api.SCApi, + 'find_server', + return_value=SCSERVER) + @mock.patch.object(storagecenter_api.SCApi, + 'find_volume', + return_value=VOLUME) + @mock.patch.object(storagecenter_api.SCApi, + 'unmap_volume', + return_value=True) + @mock.patch.object(storagecenter_api.SCApi, + 'find_wwns', + return_value=(1, + [u'5000D31000FCBE3D', + u'5000D31000FCBE35'], + {u'21000024FF30441C': + [u'5000D31000FCBE35'], + u'21000024FF30441D': + [u'5000D31000FCBE3D']})) + @mock.patch.object(storagecenter_api.SCApi, + 'get_volume_count', + return_value=1) + def test_terminate_connection_multiattached_diffhost(self, + mock_get_volume_count, + mock_find_wwns, + mock_unmap_volume, + mock_find_volume, + mock_find_server, + mock_close_connection, + mock_open_connection, + mock_init): + connector = self.connector + + attachment1 = fake_volume.volume_attachment_ovo(self._context) + attachment1.connector = connector + attachment1.attached_host = connector['host'] + attachment1.attach_status = 'attached' + + attachment2 = fake_volume.volume_attachment_ovo(self._context) + attachment2.connector = connector + attachment2.attached_host = 'host2' + attachment2.attach_status = 'attached' + + vol = fake_volume.fake_volume_obj(self._context) + vol.multiattach = True + vol.volume_attachment.objects.append(attachment1) + vol.volume_attachment.objects.append(attachment2) + + self.driver.terminate_connection(vol, connector) + mock_unmap_volume.assert_called_once_with(self.VOLUME, self.SCSERVER) + def test_terminate_secondary(self, mock_close_connection, mock_open_connection, diff --git a/cinder/tests/unit/volume/drivers/dell_emc/sc/test_sc.py b/cinder/tests/unit/volume/drivers/dell_emc/sc/test_sc.py index 82354fb9848..4f2bc9ae8e2 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/sc/test_sc.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/sc/test_sc.py @@ -1545,6 +1545,78 @@ class DellSCSanISCSIDriverTestCase(test.TestCase): volume, connector) + @mock.patch.object(storagecenter_api.SCApi, + 'find_server', + return_value=SCSERVER) + @mock.patch.object(storagecenter_api.SCApi, + 'find_volume', + return_value=VOLUME) + @mock.patch.object(storagecenter_api.SCApi, + 'unmap_volume', + return_value=True) + def test_terminate_connection_multiattached_host(self, + mock_unmap_volume, + mock_find_volume, + mock_find_server, + mock_close_connection, + mock_open_connection, + mock_init): + connector = self.connector + + attachment1 = fake_volume.volume_attachment_ovo(self._context) + attachment1.connector = connector + attachment1.attached_host = connector['host'] + attachment1.attach_status = 'attached' + + attachment2 = fake_volume.volume_attachment_ovo(self._context) + attachment2.connector = connector + attachment2.attached_host = connector['host'] + attachment2.attach_status = 'attached' + + vol = fake_volume.fake_volume_obj(self._context) + vol.multiattach = True + vol.volume_attachment.objects.append(attachment1) + vol.volume_attachment.objects.append(attachment2) + + self.driver.terminate_connection(vol, connector) + mock_unmap_volume.assert_not_called() + + @mock.patch.object(storagecenter_api.SCApi, + 'find_server', + return_value=SCSERVER) + @mock.patch.object(storagecenter_api.SCApi, + 'find_volume', + return_value=VOLUME) + @mock.patch.object(storagecenter_api.SCApi, + 'unmap_volume', + return_value=True) + def test_terminate_connection_multiattached_diffhost(self, + mock_unmap_volume, + mock_find_volume, + mock_find_server, + mock_close_connection, + mock_open_connection, + mock_init): + connector = self.connector + + attachment1 = fake_volume.volume_attachment_ovo(self._context) + attachment1.connector = connector + attachment1.attached_host = connector['host'] + attachment1.attach_status = 'attached' + + attachment2 = fake_volume.volume_attachment_ovo(self._context) + attachment2.connector = connector + attachment2.attached_host = 'host2' + attachment2.attach_status = 'attached' + + vol = fake_volume.fake_volume_obj(self._context) + vol.multiattach = True + vol.volume_attachment.objects.append(attachment1) + vol.volume_attachment.objects.append(attachment2) + + self.driver.terminate_connection(vol, connector) + mock_unmap_volume.assert_called_once_with(self.VOLUME, self.SCSERVER) + def _simple_volume(self, **kwargs): updates = {'display_name': fake.VOLUME_NAME, 'id': fake.VOLUME_ID, diff --git a/cinder/volume/drivers/dell_emc/sc/storagecenter_common.py b/cinder/volume/drivers/dell_emc/sc/storagecenter_common.py index 40dba788e22..572021c43c8 100644 --- a/cinder/volume/drivers/dell_emc/sc/storagecenter_common.py +++ b/cinder/volume/drivers/dell_emc/sc/storagecenter_common.py @@ -2034,3 +2034,26 @@ class SCCommonDriver(driver.ManageableVD, raise exception.Invalid(reason=msg) return True + + def is_multiattach_to_host(self, volume_attachment, host_name): + # When multiattach is enabled, a volume could be attached to two or + # more instances which are hosted on one nova host. + # Because the backend cannot recognize the volume is attached to two + # or more instances, we should keep the volume attached to the nova + # host until the volume is detached from the last instance. + LOG.info('is_multiattach_to_host: volume_attachment %s.', + volume_attachment) + LOG.info('is_multiattach_to_host: host_name %s.', host_name) + if not volume_attachment: + return False + + for a in volume_attachment: + LOG.debug('attachment %s.', a) + + attachment = [a for a in volume_attachment + if a['attach_status'] == + fields.VolumeAttachStatus.ATTACHED + and a['attached_host'] == host_name] + LOG.info('is_multiattach_to_host: attachment %s.', attachment) + + return len(attachment) > 1 diff --git a/cinder/volume/drivers/dell_emc/sc/storagecenter_fc.py b/cinder/volume/drivers/dell_emc/sc/storagecenter_fc.py index c75f2b03576..1e120ae105d 100644 --- a/cinder/volume/drivers/dell_emc/sc/storagecenter_fc.py +++ b/cinder/volume/drivers/dell_emc/sc/storagecenter_fc.py @@ -20,6 +20,7 @@ from oslo_utils import excutils from cinder import exception from cinder.i18n import _ from cinder import interface +from cinder import utils from cinder.volume import driver from cinder.volume.drivers.dell_emc.sc import storagecenter_common from cinder.zonemanager import utils as fczm_utils @@ -240,6 +241,7 @@ class SCFCDriver(storagecenter_common.SCCommonDriver, 'data': {}} return info + @utils.synchronized('{self.driver_prefix}-{volume.id}') def terminate_connection(self, volume, connector, force=False, **kwargs): # Special case if connector is None: @@ -249,6 +251,20 @@ class SCFCDriver(storagecenter_common.SCCommonDriver, volume_name = volume.get('id') provider_id = volume.get('provider_id') LOG.debug('Terminate connection: %s', volume_name) + LOG.debug('Volume details %s', volume) + + # None `connector` indicates force detach, then detach all even if the + # volume is multi-attached. + is_multiattached = (hasattr(volume, 'volume_attachment') and + self.is_multiattach_to_host( + volume.get('volume_attachment'), + connector['host'])) + if is_multiattached: + LOG.info('Cannot terminate connection: ' + '%(vol)s is multiattached.', + {'vol': volume_name}) + + return True with self._client.open_connection() as api: try: @@ -292,7 +308,6 @@ class SCFCDriver(storagecenter_common.SCCommonDriver, raise exception.VolumeBackendAPIException( data=_('Terminate connection failed')) - # basic return info... info = {'driver_volume_type': 'fibre_channel', 'data': {}} diff --git a/cinder/volume/drivers/dell_emc/sc/storagecenter_iscsi.py b/cinder/volume/drivers/dell_emc/sc/storagecenter_iscsi.py index fb01c51545f..a7d7c91e132 100644 --- a/cinder/volume/drivers/dell_emc/sc/storagecenter_iscsi.py +++ b/cinder/volume/drivers/dell_emc/sc/storagecenter_iscsi.py @@ -20,6 +20,7 @@ from oslo_utils import excutils from cinder import exception from cinder.i18n import _ from cinder import interface +from cinder import utils from cinder.volume import driver from cinder.volume.drivers.dell_emc.sc import storagecenter_common @@ -251,8 +252,11 @@ class SCISCSIDriver(storagecenter_common.SCCommonDriver, raise exception.VolumeBackendAPIException( _('Terminate connection failed')) + @utils.synchronized('{self.driver_prefix}-{volume.id}') def terminate_connection(self, volume, connector, force=False, **kwargs): # Special case + # None `connector` indicates force detach, then detach all even if the + # volume is multi-attached. if connector is None: return self.force_detach(volume) @@ -261,9 +265,18 @@ class SCISCSIDriver(storagecenter_common.SCCommonDriver, volume_name = volume.get('id') provider_id = volume.get('provider_id') initiator_name = None if not connector else connector.get('initiator') - LOG.debug('Terminate connection: %(vol)s:%(initiator)s', - {'vol': volume_name, - 'initiator': initiator_name}) + LOG.info('Volume in terminate connection: %(vol)s', + {'vol': volume}) + + is_multiattached = (hasattr(volume, 'volume_attachment') and + self.is_multiattach_to_host( + volume.get('volume_attachment'), + connector['host'])) + if is_multiattached: + LOG.info('Cannot terminate connection: ' + '%(vol)s is multiattached.', + {'vol': volume_name}) + return True with self._client.open_connection() as api: try: @@ -289,7 +302,7 @@ class SCISCSIDriver(storagecenter_common.SCCommonDriver, scserver = (None if not initiator_name else api.find_server(initiator_name, ssn)) - # If we have a server and a volume lets pull them apart. + # If we have a server and a volume lets pull them apart if ((scserver and api.unmap_volume(scvolume, scserver) is True) or (not scserver and api.unmap_all(scvolume))): @@ -309,6 +322,7 @@ class SCISCSIDriver(storagecenter_common.SCCommonDriver, rtn = True secondaryvol = api.get_volume( sclivevolume['secondaryVolume']['instanceId']) + if secondaryvol: if initiatorname: # Find our server. diff --git a/releasenotes/notes/sc-handle-multiattach-onterminate-6ab1f96f21bb284d.yaml b/releasenotes/notes/sc-handle-multiattach-onterminate-6ab1f96f21bb284d.yaml new file mode 100644 index 00000000000..973815970c3 --- /dev/null +++ b/releasenotes/notes/sc-handle-multiattach-onterminate-6ab1f96f21bb284d.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Dell EMC SC Driver: Fixes `bug 1822229 + `__ + to handle the volume mappings in the backend when a volume + is attached to multiple instances on the same host.