diff --git a/cinder/tests/unit/volume/drivers/test_lvm_driver.py b/cinder/tests/unit/volume/drivers/test_lvm_driver.py index 08cd428b79e..7da792d2686 100644 --- a/cinder/tests/unit/volume/drivers/test_lvm_driver.py +++ b/cinder/tests/unit/volume/drivers/test_lvm_driver.py @@ -25,6 +25,7 @@ from cinder.objects import fields from cinder.tests import fake_driver from cinder.tests.unit.brick import fake_lvm from cinder.tests.unit import fake_constants as fake +from cinder.tests.unit import fake_volume from cinder.tests.unit import utils as tests_utils from cinder.tests.unit.volume import test_driver from cinder.volume import configuration as conf @@ -986,3 +987,59 @@ class LVMISCSITestCase(test_driver.BaseDriverTestCase): connector = {'ip': '10.0.0.2', 'host': 'fakehost'} self.assertRaises(exception.InvalidConnectorException, iscsi_driver.validate_connector, connector) + + def test_multiattach_terminate_connection(self): + # Ensure that target_driver.terminate_connection is only called when a + # single active volume attachment remains per host for each volume. + + host1_connector = {'ip': '10.0.0.2', + 'host': 'fakehost1', + 'initiator': 'iqn.2012-07.org.fake:01'} + + host2_connector = {'ip': '10.0.0.3', + 'host': 'fakehost2', + 'initiator': 'iqn.2012-07.org.fake:02'} + + host1_attachment1 = fake_volume.fake_volume_attachment_obj( + self.context) + host1_attachment1.connector = host1_connector + + host1_attachment2 = fake_volume.fake_volume_attachment_obj( + self.context) + host1_attachment2.connector = host1_connector + + host2_attachment = fake_volume.fake_volume_attachment_obj(self.context) + host2_attachment.connector = host2_connector + + # Create a multiattach volume object with two active attachments on + # host1 and another single attachment on host2. + vol = fake_volume.fake_volume_obj(self.context) + vol.multiattach = True + vol.volume_attachment.objects.append(host1_attachment1) + vol.volume_attachment.objects.append(host1_attachment2) + vol.volume_attachment.objects.append(host2_attachment) + + self.configuration = conf.Configuration(None) + vg_obj = fake_lvm.FakeBrickLVM('cinder-volumes', False, None, + 'default') + lvm_driver = lvm.LVMVolumeDriver(configuration=self.configuration, + db=db, vg_obj=vg_obj) + + with mock.patch.object(lvm_driver.target_driver, + 'terminate_connection') as mock_term_conn: + + # Verify that terminate_connection is not called against host1 when + # there are multiple active attachments against that host. + self.assertTrue(lvm_driver.terminate_connection(vol, + host1_connector)) + mock_term_conn.assert_not_called() + + # Verify that terminate_connection is called against either host + # when only one active attachment per host is present. + vol.volume_attachment.objects.remove(host1_attachment1) + self.assertTrue(lvm_driver.terminate_connection(vol, + host1_connector)) + self.assertTrue(lvm_driver.terminate_connection(vol, + host2_connector)) + mock_term_conn.assert_has_calls([mock.call(vol, host1_connector), + mock.call(vol, host2_connector)]) diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py index 9e2988a402d..c40e7ba2f46 100644 --- a/cinder/volume/drivers/lvm.py +++ b/cinder/volume/drivers/lvm.py @@ -244,10 +244,6 @@ class LVMVolumeDriver(driver.VolumeDriver): # This includes volumes and snapshots. total_volumes = len(self.vg.get_volumes()) - supports_multiattach = True - if self.configuration.target_helper == 'lioadm': - supports_multiattach = False - # Skip enabled_pools setting, treat the whole backend as one pool # XXX FIXME if multipool support is added to LVM driver. single_pool = {} @@ -266,7 +262,7 @@ class LVMVolumeDriver(driver.VolumeDriver): total_volumes=total_volumes, filter_function=self.get_filter_function(), goodness_function=self.get_goodness_function(), - multiattach=supports_multiattach, + multiattach=True, backend_state='up' )) data["pools"].append(single_pool) @@ -845,13 +841,17 @@ class LVMVolumeDriver(driver.VolumeDriver): # we need to do here is check if there is more than one attachment for # the volume, if there is; let the caller know that they should NOT # remove the export. - has_shared_connections = False - if len(volume.volume_attachment) > 1: - has_shared_connections = True - # NOTE(jdg): For the TGT driver this is a noop, for LIO this removes # the initiator IQN from the targets access list, so we're good + # NOTE(lyarwood): Given the above note we should only call + # terminate_connection for the target lioadm driver when there is only + # one attachment left for the host specified by the connector to + # remove, otherwise the ACL will be removed prematurely while other + # attachments on the same host are still accessing the volume. + attachments = volume.volume_attachment + if volume.multiattach: + if sum(1 for a in attachments if a.connector == connector) > 1: + return True - self.target_driver.terminate_connection(volume, connector, - **kwargs) - return has_shared_connections + self.target_driver.terminate_connection(volume, connector, **kwargs) + return len(attachments) > 1