Support force disconnect for FC
This patch adds support for the force and ignore_errors on the disconnect_volume of the FC connector like we have in the iSCSI connector. Related-Bug: #2004555 Change-Id: Ia74ecfba03ba23de9d30eb33706245a7f85e1d66 (cherry picked from commit 570df49db9de3030e658619138588b836c007f8c)
This commit is contained in:
parent
a7dfaf4243
commit
e00d3ca753
@ -352,7 +352,7 @@ class FibreChannelConnector(base.BaseLinuxConnector):
|
||||
target_wwn - World Wide Name
|
||||
target_lun - LUN id of the volume
|
||||
"""
|
||||
|
||||
exc = exception.ExceptionChainer()
|
||||
devices = []
|
||||
wwn = None
|
||||
|
||||
@ -368,18 +368,35 @@ class FibreChannelConnector(base.BaseLinuxConnector):
|
||||
wwn = self._linuxscsi.get_scsi_wwn(path)
|
||||
mpath_path = self._linuxscsi.find_multipath_device_path(wwn)
|
||||
if mpath_path:
|
||||
self._linuxscsi.flush_multipath_device(mpath_path)
|
||||
with exc.context(force, 'Flushing %s failed', mpath_path):
|
||||
self._linuxscsi.flush_multipath_device(mpath_path)
|
||||
real_path = typing.cast(str, real_path)
|
||||
dev_info = self._linuxscsi.get_device_info(real_path)
|
||||
devices.append(dev_info)
|
||||
|
||||
# If flush failed, then remove it forcefully since force=True
|
||||
if mpath_path and exc:
|
||||
with exc.context(force, 'Removing multipath %s failed',
|
||||
mpath_path):
|
||||
mpath_name = os.path.basename(os.path.realpath(mpath_path))
|
||||
self._linuxscsi.multipath_del_map(mpath_name)
|
||||
|
||||
LOG.debug("devices to remove = %s", devices)
|
||||
self._remove_devices(connection_properties, devices, device_info)
|
||||
self._remove_devices(connection_properties, devices, device_info,
|
||||
force, exc)
|
||||
|
||||
if exc: # type: ignore
|
||||
LOG.warning('There were errors removing %s, leftovers may remain '
|
||||
'in the system', volume_paths)
|
||||
if not ignore_errors:
|
||||
raise exc # type: ignore
|
||||
|
||||
def _remove_devices(self,
|
||||
connection_properties: dict,
|
||||
devices: list,
|
||||
device_info: dict) -> None:
|
||||
device_info: dict,
|
||||
force: bool,
|
||||
exc) -> None:
|
||||
# There may have been more than 1 device mounted
|
||||
# by the kernel for this volume. We have to remove
|
||||
# all of them
|
||||
@ -394,11 +411,12 @@ class FibreChannelConnector(base.BaseLinuxConnector):
|
||||
# paths, whereas for multipaths we have multiple link formats.
|
||||
was_multipath = '/pci-' not in path_used and was_symlink
|
||||
for device in devices:
|
||||
device_path = device['device']
|
||||
flush = self._linuxscsi.requires_flush(device_path,
|
||||
path_used,
|
||||
was_multipath)
|
||||
self._linuxscsi.remove_scsi_device(device_path, flush=flush)
|
||||
with exc.context(force, 'Removing device %s failed', device):
|
||||
device_path = device['device']
|
||||
flush = self._linuxscsi.requires_flush(device_path,
|
||||
path_used,
|
||||
was_multipath)
|
||||
self._linuxscsi.remove_scsi_device(device_path, flush=flush)
|
||||
|
||||
def _get_pci_num(self, hba: Optional[dict]) -> tuple:
|
||||
# NOTE(walter-boring)
|
||||
|
@ -86,12 +86,15 @@ class FibreChannelConnectorS390X(fibre_channel.FibreChannelConnector):
|
||||
]
|
||||
return host_device
|
||||
|
||||
def _remove_devices(self, connection_properties, devices, device_info):
|
||||
def _remove_devices(self, connection_properties, devices, device_info,
|
||||
force, exc):
|
||||
hbas = self._linuxfc.get_fc_hbas_info()
|
||||
targets = connection_properties['targets']
|
||||
possible_devs = self._get_possible_devices(hbas, targets)
|
||||
for platform, pci_num, target_wwn, lun in possible_devs:
|
||||
target_lun = self._get_lun_string(lun)
|
||||
self._linuxfc.deconfigure_scsi_device(pci_num,
|
||||
target_wwn,
|
||||
target_lun)
|
||||
with exc.context(force, 'Removing device %s:%s:%s failed',
|
||||
pci_num, target_wwn, target_lun):
|
||||
self._linuxfc.deconfigure_scsi_device(pci_num,
|
||||
target_wwn,
|
||||
target_lun)
|
||||
|
@ -759,3 +759,21 @@ class LinuxSCSI(executor.Executor):
|
||||
check_exit_code=False,
|
||||
root_helper=self._root_helper)
|
||||
return stdout.strip() == 'ok'
|
||||
|
||||
@utils.retry((putils.ProcessExecutionError, exception.BrickException),
|
||||
retries=3)
|
||||
def multipath_del_map(self, mpath):
|
||||
"""Stop monitoring a multipath given its device name (eg: dm-7).
|
||||
|
||||
Method ensures that the multipath device mapper actually dissapears
|
||||
from sysfs.
|
||||
"""
|
||||
map_name = self.get_dm_name(mpath)
|
||||
if map_name:
|
||||
self._execute('multipathd', 'del', 'map', map_name,
|
||||
run_as_root=True, timeout=5,
|
||||
root_helper=self._root_helper)
|
||||
|
||||
if map_name and self.get_dm_name(mpath):
|
||||
raise exception.BrickException("Multipath doesn't go away")
|
||||
LOG.debug('Multipath %s no longer present', mpath)
|
||||
|
@ -743,10 +743,13 @@ class FibreChannelConnectorTestCase(test_connector.ConnectorTestCase):
|
||||
@mock.patch('os_brick.utils.get_dev_path')
|
||||
def test__remove_devices(self, path_used, was_multipath, get_dev_path_mock,
|
||||
flush_mock, remove_mock):
|
||||
exc = exception.ExceptionChainer()
|
||||
get_dev_path_mock.return_value = path_used
|
||||
self.connector._remove_devices(mock.sentinel.con_props,
|
||||
[{'device': '/dev/sda'}],
|
||||
mock.sentinel.device_info)
|
||||
mock.sentinel.device_info,
|
||||
force=False, exc=exc)
|
||||
self.assertFalse(bool(exc))
|
||||
get_dev_path_mock.assert_called_once_with(mock.sentinel.con_props,
|
||||
mock.sentinel.device_info)
|
||||
flush_mock.assert_called_once_with('/dev/sda', path_used,
|
||||
@ -754,6 +757,39 @@ class FibreChannelConnectorTestCase(test_connector.ConnectorTestCase):
|
||||
remove_mock.assert_called_once_with('/dev/sda',
|
||||
flush=flush_mock.return_value)
|
||||
|
||||
@ddt.data(('/dev/mapper/<WWN>', True),
|
||||
('/dev/mapper/mpath0', True),
|
||||
# Check real devices are properly detected as non multipaths
|
||||
('/dev/sda', False),
|
||||
('/dev/disk/by-path/pci-1-fc-1-lun-1', False))
|
||||
@ddt.unpack
|
||||
@mock.patch('os_brick.initiator.linuxscsi.LinuxSCSI.remove_scsi_device')
|
||||
@mock.patch('os_brick.initiator.linuxscsi.LinuxSCSI.requires_flush')
|
||||
@mock.patch('os_brick.utils.get_dev_path')
|
||||
def test__remove_devices_fails(self, path_used, was_multipath,
|
||||
get_dev_path_mock, flush_mock, remove_mock):
|
||||
exc = exception.ExceptionChainer()
|
||||
get_dev_path_mock.return_value = path_used
|
||||
remove_mock.side_effect = Exception
|
||||
self.connector._remove_devices(mock.sentinel.con_props,
|
||||
[{'device': '/dev/sda'},
|
||||
{'device': '/dev/sdb'}],
|
||||
mock.sentinel.device_info,
|
||||
force=True, exc=exc)
|
||||
self.assertTrue(bool(exc))
|
||||
get_dev_path_mock.assert_called_once_with(mock.sentinel.con_props,
|
||||
mock.sentinel.device_info)
|
||||
|
||||
expect_flush = [mock.call('/dev/sda', path_used, was_multipath),
|
||||
mock.call('/dev/sdb', path_used, was_multipath)]
|
||||
self.assertEqual(len(expect_flush), flush_mock.call_count)
|
||||
flush_mock.assert_has_calls(expect_flush)
|
||||
|
||||
expect_remove = [mock.call('/dev/sda', flush=flush_mock.return_value),
|
||||
mock.call('/dev/sdb', flush=flush_mock.return_value)]
|
||||
self.assertEqual(len(expect_remove), remove_mock.call_count)
|
||||
remove_mock.assert_has_calls(expect_remove)
|
||||
|
||||
@mock.patch.object(linuxscsi.LinuxSCSI, 'find_multipath_device')
|
||||
@mock.patch.object(linuxscsi.LinuxSCSI, 'wait_for_rw')
|
||||
@mock.patch.object(os.path, 'exists', return_value=True)
|
||||
@ -815,3 +851,94 @@ class FibreChannelConnectorTestCase(test_connector.ConnectorTestCase):
|
||||
'tee -a /sys/block/sdc/device/delete',
|
||||
]
|
||||
self.assertEqual(expected_commands, self.cmds)
|
||||
|
||||
@ddt.data((False, Exception), (True, Exception), (False, None))
|
||||
@ddt.unpack
|
||||
@mock.patch.object(fibre_channel.FibreChannelConnector, '_remove_devices')
|
||||
@mock.patch.object(linuxscsi.LinuxSCSI, 'multipath_del_map')
|
||||
@mock.patch.object(linuxscsi.LinuxSCSI, 'flush_multipath_device')
|
||||
@mock.patch.object(linuxscsi.LinuxSCSI, 'wait_for_rw')
|
||||
@mock.patch.object(os.path, 'exists', return_value=True)
|
||||
@mock.patch.object(os.path, 'realpath')
|
||||
@mock.patch.object(linuxfc.LinuxFibreChannel, 'get_fc_hbas')
|
||||
@mock.patch.object(linuxfc.LinuxFibreChannel, 'get_fc_hbas_info')
|
||||
@mock.patch.object(linuxscsi.LinuxSCSI, 'get_scsi_wwn')
|
||||
@mock.patch.object(linuxscsi.LinuxSCSI, 'get_device_info')
|
||||
@mock.patch.object(linuxscsi.LinuxSCSI, 'find_multipath_device_path')
|
||||
@mock.patch.object(base.BaseLinuxConnector, 'check_valid_device')
|
||||
def test_disconnect_volume_fails(self, ignore_exc, side_effect,
|
||||
check_valid_device_mock,
|
||||
find_mp_device_path_mock,
|
||||
get_device_info_mock,
|
||||
get_scsi_wwn_mock,
|
||||
get_fc_hbas_info_mock,
|
||||
get_fc_hbas_mock,
|
||||
realpath_mock,
|
||||
exists_mock,
|
||||
wait_for_rw_mock,
|
||||
flush_mock,
|
||||
del_map_mock,
|
||||
remove_mock):
|
||||
|
||||
flush_mock.side_effect = side_effect
|
||||
del_map_mock.side_effect = side_effect
|
||||
|
||||
check_valid_device_mock.return_value = True
|
||||
self.connector.use_multipath = True
|
||||
get_fc_hbas_mock.side_effect = self.fake_get_fc_hbas
|
||||
get_fc_hbas_info_mock.side_effect = self.fake_get_fc_hbas_info
|
||||
|
||||
wwn = '360002ac00000000000000b860000741c'
|
||||
multipath_devname = f'/dev/disk/by-id/dm-uuid-mpath-{wwn}'
|
||||
realpath_mock.return_value = '/dev/dm-1'
|
||||
devices = {"device": multipath_devname,
|
||||
"id": wwn,
|
||||
"devices": [{'device': '/dev/sdb',
|
||||
'address': '1:0:0:1',
|
||||
'host': 1, 'channel': 0,
|
||||
'id': 0, 'lun': 1},
|
||||
{'device': '/dev/sdc',
|
||||
'address': '1:0:0:2',
|
||||
'host': 1, 'channel': 0,
|
||||
'id': 0, 'lun': 1}]}
|
||||
get_device_info_mock.side_effect = devices['devices']
|
||||
get_scsi_wwn_mock.return_value = wwn
|
||||
|
||||
location = '10.0.2.15:3260'
|
||||
name = 'volume-00000001'
|
||||
vol = {'id': 1, 'name': name}
|
||||
initiator_wwn = ['1234567890123456', '1234567890123457']
|
||||
|
||||
find_mp_device_path_mock.return_value = '/dev/mapper/mpatha'
|
||||
|
||||
connection_info = self.fibrechan_connection(vol, location,
|
||||
initiator_wwn)
|
||||
if side_effect and not ignore_exc:
|
||||
self.assertRaises(exception.ExceptionChainer,
|
||||
self.connector.disconnect_volume,
|
||||
connection_info['data'], devices['devices'][0],
|
||||
force=True, ignore_errors=ignore_exc)
|
||||
else:
|
||||
self.connector.disconnect_volume(connection_info['data'],
|
||||
devices['devices'][0],
|
||||
force=True,
|
||||
ignore_errors=ignore_exc)
|
||||
|
||||
flush_mock.assert_called_once_with(
|
||||
find_mp_device_path_mock.return_value)
|
||||
|
||||
expected = [
|
||||
mock.call(f'/dev/disk/by-path/pci-0000:05:00.2-fc-0x{wwn}-lun-1')
|
||||
for wwn in initiator_wwn]
|
||||
if side_effect:
|
||||
del_map_mock.assert_called_once_with('dm-1')
|
||||
expected.append(mock.call(find_mp_device_path_mock.return_value))
|
||||
else:
|
||||
del_map_mock.assert_not_called()
|
||||
|
||||
remove_mock.assert_called_once_with(connection_info['data'],
|
||||
devices['devices'],
|
||||
devices['devices'][0],
|
||||
True, mock.ANY)
|
||||
self.assertEqual(len(expected), realpath_mock.call_count)
|
||||
realpath_mock.assert_has_calls(expected)
|
||||
|
@ -13,6 +13,7 @@
|
||||
# under the License.
|
||||
from unittest import mock
|
||||
|
||||
from os_brick import exception
|
||||
from os_brick.initiator.connectors import fibre_channel_s390x
|
||||
from os_brick.initiator import linuxfc
|
||||
from os_brick.tests.initiator import test_connector
|
||||
@ -66,10 +67,32 @@ class FibreChannelConnectorS390XTestCase(test_connector.ConnectorTestCase):
|
||||
'deconfigure_scsi_device')
|
||||
def test_remove_devices(self, mock_deconfigure_scsi_device,
|
||||
mock_get_fc_hbas_info, mock_get_possible_devices):
|
||||
exc = exception.ExceptionChainer()
|
||||
connection_properties = {'targets': [5, 2]}
|
||||
self.connector._remove_devices(connection_properties, devices=None,
|
||||
device_info=None)
|
||||
device_info=None, force=False, exc=exc)
|
||||
mock_deconfigure_scsi_device.assert_called_with(3, 5,
|
||||
"0x0002000000000000")
|
||||
mock_get_fc_hbas_info.assert_called_once_with()
|
||||
mock_get_possible_devices.assert_called_once_with([], [5, 2])
|
||||
self.assertFalse(bool(exc))
|
||||
|
||||
@mock.patch.object(fibre_channel_s390x.FibreChannelConnectorS390X,
|
||||
'_get_possible_devices', return_value=[('', 3, 5, 2), ])
|
||||
@mock.patch.object(linuxfc.LinuxFibreChannelS390X, 'get_fc_hbas_info',
|
||||
return_value=[])
|
||||
@mock.patch.object(linuxfc.LinuxFibreChannelS390X,
|
||||
'deconfigure_scsi_device')
|
||||
def test_remove_devices_force(self, mock_deconfigure_scsi_device,
|
||||
mock_get_fc_hbas_info,
|
||||
mock_get_possible_devices):
|
||||
exc = exception.ExceptionChainer()
|
||||
mock_deconfigure_scsi_device.side_effect = Exception
|
||||
connection_properties = {'targets': [5, 2]}
|
||||
self.connector._remove_devices(connection_properties, devices=None,
|
||||
device_info=None, force=True, exc=exc)
|
||||
mock_deconfigure_scsi_device.assert_called_with(3, 5,
|
||||
"0x0002000000000000")
|
||||
mock_get_fc_hbas_info.assert_called_once_with()
|
||||
mock_get_possible_devices.assert_called_once_with([], [5, 2])
|
||||
self.assertTrue(bool(exc))
|
||||
|
@ -1299,6 +1299,64 @@ loop0 0"""
|
||||
self.linuxscsi.multipath_del_path('/dev/sda')
|
||||
self.assertEqual(['multipathd del path /dev/sda'], self.cmds)
|
||||
|
||||
@mock.patch.object(linuxscsi.LinuxSCSI, 'get_dm_name', return_value=None)
|
||||
def test_multipath_del_map_not_present(self, name_mock):
|
||||
self.linuxscsi.multipath_del_map('dm-7')
|
||||
self.assertEqual([], self.cmds)
|
||||
name_mock.assert_called_once_with('dm-7')
|
||||
|
||||
@mock.patch.object(linuxscsi.LinuxSCSI, '_execute')
|
||||
@mock.patch.object(linuxscsi.LinuxSCSI, 'get_dm_name', return_value=None)
|
||||
def test_multipath_del_map(self, name_mock, exec_mock):
|
||||
exec_mock.side_effect = [putils.ProcessExecutionError, None]
|
||||
|
||||
mpath_name = '3600d0230000000000e13955cc3757800'
|
||||
name_mock.side_effect = [mpath_name, mpath_name, None]
|
||||
self.linuxscsi.multipath_del_map('dm-7')
|
||||
|
||||
self.assertEqual(2, exec_mock.call_count)
|
||||
exec_mock.assert_has_calls(
|
||||
[mock.call('multipathd', 'del', 'map', mpath_name,
|
||||
run_as_root=True, timeout=5,
|
||||
root_helper=self.linuxscsi._root_helper)] * 2)
|
||||
self.assertEqual(3, name_mock.call_count)
|
||||
name_mock.assert_has_calls([mock.call('dm-7')] * 3)
|
||||
|
||||
@mock.patch.object(linuxscsi.LinuxSCSI, '_execute')
|
||||
@mock.patch.object(linuxscsi.LinuxSCSI, 'get_dm_name')
|
||||
def test_multipath_del_map_retries_cmd_fails(self, name_mock, exec_mock):
|
||||
exec_mock.side_effect = putils.ProcessExecutionError
|
||||
mpath_name = '3600d0230000000000e13955cc3757800'
|
||||
name_mock.return_value = mpath_name
|
||||
self.assertRaises(putils.ProcessExecutionError,
|
||||
self.linuxscsi.multipath_del_map, 'dm-7')
|
||||
|
||||
self.assertEqual(3, exec_mock.call_count)
|
||||
exec_mock.assert_has_calls(
|
||||
[mock.call('multipathd', 'del', 'map', mpath_name,
|
||||
run_as_root=True, timeout=5,
|
||||
root_helper=self.linuxscsi._root_helper)] * 3)
|
||||
|
||||
self.assertEqual(3, name_mock.call_count)
|
||||
name_mock.assert_has_calls([mock.call('dm-7')] * 3)
|
||||
|
||||
@mock.patch.object(linuxscsi.LinuxSCSI, '_execute')
|
||||
@mock.patch.object(linuxscsi.LinuxSCSI, 'get_dm_name')
|
||||
def test_multipath_del_map_retries_remains(self, name_mock, exec_mock):
|
||||
mpath_name = '3600d0230000000000e13955cc3757800'
|
||||
name_mock.return_value = mpath_name
|
||||
self.assertRaises(exception.BrickException,
|
||||
self.linuxscsi.multipath_del_map, 'dm-7')
|
||||
|
||||
self.assertEqual(3, exec_mock.call_count)
|
||||
exec_mock.assert_has_calls(
|
||||
[mock.call('multipathd', 'del', 'map', mpath_name,
|
||||
run_as_root=True, timeout=5,
|
||||
root_helper=self.linuxscsi._root_helper)] * 3)
|
||||
|
||||
self.assertEqual(6, name_mock.call_count)
|
||||
name_mock.assert_has_calls([mock.call('dm-7')] * 6)
|
||||
|
||||
@ddt.data(('/dev/sda', '/dev/sda', False, True, None),
|
||||
# This checks that we ignore the was_multipath parameter if it
|
||||
# doesn't make sense (because the used path is the one we are
|
||||
|
@ -0,0 +1,5 @@
|
||||
---
|
||||
features:
|
||||
- |
|
||||
FC connector: Added support to ``force`` and ``ignore_errors`` parameters
|
||||
on ``disconnect_volume`` method.
|
Loading…
x
Reference in New Issue
Block a user