Fixing FC scanning

Current FC tries to limit the scanning range by detecting the target and
channel, unfortunately this code has a good number of implementation
issues:

- Matching uses local WWNN instead of target's WWPN.
- Not using a shell to run the command, so the * glob won't expand.
- Not using -l on grep command to list file names instead of contents.
- Not making the search case insensitive.

This patch fixes all these issues by using the target's WWPNs instead
-taking into account FC Zone/Access control information if present- and
supporting both possible connection information formats for the WWPNs
(single value or list of values).

Rescan tests have been modified to adhere to unit tests best practices,
where each test case only tests the specific code in the method under
test and mocks everything else.

Closes-Bug: #1664653
Closes-Bug: #1684996
Closes-Bug: #1687607
Change-Id: Ib539f6a3652bab4399c30cd90f326829e839ec02
(cherry picked from commit 4ee404466d)
This commit is contained in:
Gorka Eguileor 2017-11-08 21:03:08 +01:00 committed by Keith Berger
parent 4bdd072de3
commit f821a87ef0
3 changed files with 141 additions and 71 deletions

View File

@ -158,8 +158,7 @@ class FibreChannelConnector(base.BaseLinuxConnector):
"Will rescan & retry. Try number: %(tries)s.",
{'tries': tries})
self._linuxfc.rescan_hosts(hbas,
connection_properties['target_lun'])
self._linuxfc.rescan_hosts(hbas, connection_properties)
self.tries = self.tries + 1
self.host_device = None

View File

@ -19,6 +19,7 @@ import os
from oslo_concurrency import processutils as putils
from oslo_log import log as logging
import six
from os_brick.initiator import linuxscsi
@ -34,7 +35,7 @@ class LinuxFibreChannel(linuxscsi.LinuxSCSI):
else:
return False
def _get_hba_channel_scsi_target(self, hba):
def _get_hba_channel_scsi_target(self, hba, conn_props):
"""Try to get the HBA channel and SCSI target for an HBA.
This method only works for Fibre Channel targets that implement a
@ -43,28 +44,44 @@ class LinuxFibreChannel(linuxscsi.LinuxSCSI):
:returns: List or None
"""
# We want the target's WWPNs, so we use the initiator_target_map if
# present for this hba or default to target_wwns if not present.
wwpns = conn_props['target_wwn']
if 'initiator_target_map' in conn_props:
wwpns = conn_props['initiator_target_map'].get(hba['port_name'],
wwpns)
# If it's not a string then it's an iterable (most likely a list),
# so we need to create a BRE for the grep query.
if not isinstance(wwpns, six.string_types):
wwpns = '\|'.join(wwpns)
# Leave only the number from the host_device field (ie: host6)
host_device = hba['host_device']
if host_device and len(host_device) > 4:
host_device = host_device[4:]
path = '/sys/class/fc_transport/target%s:' % host_device
cmd = 'grep %(wwnn)s %(path)s*/node_name' % {'wwnn': hba['node_name'],
'path': path}
# Since we'll run the command in a shell ensure BRE are being used
cmd = 'grep -Gil "%(wwpns)s" %(path)s*/port_name' % {'wwpns': wwpns,
'path': path}
try:
out, _err = self._execute(cmd)
# We need to run command in shell to expand the * glob
out, _err = self._execute(cmd, shell=True)
return [line.split('/')[4].split(':')[1:]
for line in out.split('\n') if line.startswith(path)]
except Exception as exc:
LOG.debug('Could not get HBA channel and SCSI target ID, path: '
'%(path)s, reason: %(reason)s', {'path': path,
'reason': exc})
'%(path)s*, reason: %(reason)s', {'path': path,
'reason': exc})
return None
def rescan_hosts(self, hbas, target_lun):
def rescan_hosts(self, hbas, connection_properties):
target_lun = connection_properties['target_lun']
for hba in hbas:
# Try to get HBA channel and SCSI target to use as filters
cts = self._get_hba_channel_scsi_target(hba)
cts = self._get_hba_channel_scsi_target(hba, connection_properties)
# If we couldn't get the channel and target use wildcards
if not cts:
cts = [('-', '-')]

View File

@ -44,87 +44,141 @@ class LinuxFCTestCase(base.TestCase):
has_fc = self.lfc.has_fc_support()
self.assertTrue(has_fc)
def test_rescan_hosts(self):
# We check that we try to get the HBA channel and SCSI target
execute_results = (
('/sys/class/fc_transport/target10:2:3/node_name:'
'0x5006016090203181\n/sys/class/fc_transport/target10:4:5/'
'node_name:0x5006016090203181', ''),
None,
None,
('/sys/class/fc_transport/target11:6:7/node_name:'
'0x5006016090203181\n/sys/class/fc_transport/target11:8:9/'
'node_name:0x5006016090203181', ''),
None,
None)
hbas = [{'host_device': 'host10', 'node_name': '5006016090203181'},
{'host_device': 'host11', 'node_name': '5006016090203181'}]
@staticmethod
def __get_rescan_info(zone_manager=False):
connection_properties = {
'initiator_target_map': {'50014380186af83c': ['514f0c50023f6c00'],
'50014380186af83e': ['514f0c50023f6c01']},
'target_discovered': False,
'target_lun': 1,
'target_wwn': ['514f0c50023f6c00', '514f0c50023f6c01']
}
hbas = [
{'device_path': ('/sys/devices/pci0000:00/0000:00:02.0/'
'0000:04:00.0/host6/fc_host/host6'),
'host_device': 'host6',
'node_name': '50014380186af83d',
'port_name': '50014380186af83c'},
{'device_path': ('/sys/devices/pci0000:00/0000:00:02.0/'
'0000:04:00.1/host7/fc_host/host7'),
'host_device': 'host7',
'node_name': '50014380186af83f',
'port_name': '50014380186af83e'},
]
if not zone_manager:
del connection_properties['initiator_target_map']
return hbas, connection_properties
def test__get_hba_channel_scsi_target_single_wwpn(self):
execute_results = ('/sys/class/fc_transport/target6:0:1/port_name\n',
'')
hbas, con_props = self.__get_rescan_info()
con_props['target_wwn'] = con_props['target_wwn'][0]
with mock.patch.object(self.lfc, '_execute',
side_effect=execute_results) as execute_mock:
self.lfc.rescan_hosts(hbas, 1)
return_value=execute_results) as execute_mock:
res = self.lfc._get_hba_channel_scsi_target(hbas[0], con_props)
execute_mock.assert_called_once_with(
'grep -Gil "514f0c50023f6c00" '
'/sys/class/fc_transport/target6:*/port_name',
shell=True)
expected = [['0', '1']]
self.assertListEqual(expected, res)
def test__get_hba_channel_scsi_target_multiple_wwpn(self):
execute_results = ('/sys/class/fc_transport/target6:0:1/port_name\n'
'/sys/class/fc_transport/target6:0:2/port_name\n',
'')
hbas, con_props = self.__get_rescan_info()
with mock.patch.object(self.lfc, '_execute',
return_value=execute_results) as execute_mock:
res = self.lfc._get_hba_channel_scsi_target(hbas[0], con_props)
execute_mock.assert_called_once_with(
'grep -Gil "514f0c50023f6c00\|514f0c50023f6c01" '
'/sys/class/fc_transport/target6:*/port_name',
shell=True)
expected = [['0', '1'], ['0', '2']]
self.assertListEqual(expected, res)
def test__get_hba_channel_scsi_target_zone_manager(self):
execute_results = ('/sys/class/fc_transport/target6:0:1/port_name\n',
'')
hbas, con_props = self.__get_rescan_info(zone_manager=True)
with mock.patch.object(self.lfc, '_execute',
return_value=execute_results) as execute_mock:
res = self.lfc._get_hba_channel_scsi_target(hbas[0], con_props)
execute_mock.assert_called_once_with(
'grep -Gil "514f0c50023f6c00" '
'/sys/class/fc_transport/target6:*/port_name',
shell=True)
expected = [['0', '1']]
self.assertListEqual(expected, res)
def test__get_hba_channel_scsi_target_not_found(self):
hbas, con_props = self.__get_rescan_info(zone_manager=True)
with mock.patch.object(self.lfc, '_execute',
return_value=('', '')) as execute_mock:
res = self.lfc._get_hba_channel_scsi_target(hbas[0], con_props)
execute_mock.assert_called_once_with(
'grep -Gil "514f0c50023f6c00" '
'/sys/class/fc_transport/target6:*/port_name',
shell=True)
self.assertListEqual([], res)
def test__get_hba_channel_scsi_target_exception(self):
hbas, con_props = self.__get_rescan_info(zone_manager=True)
with mock.patch.object(self.lfc, '_execute',
side_effect=Exception) as execute_mock:
res = self.lfc._get_hba_channel_scsi_target(hbas[0], con_props)
execute_mock.assert_called_once_with(
'grep -Gil "514f0c50023f6c00" '
'/sys/class/fc_transport/target6:*/port_name',
shell=True)
self.assertIsNone(res)
def test_rescan_hosts(self):
get_chan_results = [[['2', '3'], ['4', '5']], [['6', '7']]]
hbas, con_props = self.__get_rescan_info(zone_manager=True)
with mock.patch.object(self.lfc, '_execute',
return_value=None) as execute_mock, \
mock.patch.object(self.lfc, '_get_hba_channel_scsi_target',
side_effect=get_chan_results) as mock_get_chan:
self.lfc.rescan_hosts(hbas, con_props)
expected_commands = [
mock.call('grep 5006016090203181 /sys/class/fc_transport/'
'target10:*/node_name'),
mock.call('tee', '-a', '/sys/class/scsi_host/host10/scan',
mock.call('tee', '-a', '/sys/class/scsi_host/host6/scan',
process_input='2 3 1',
root_helper=None, run_as_root=True),
mock.call('tee', '-a', '/sys/class/scsi_host/host10/scan',
mock.call('tee', '-a', '/sys/class/scsi_host/host6/scan',
process_input='4 5 1',
root_helper=None, run_as_root=True),
mock.call('grep 5006016090203181 /sys/class/fc_transport/'
'target11:*/node_name'),
mock.call('tee', '-a', '/sys/class/scsi_host/host11/scan',
mock.call('tee', '-a', '/sys/class/scsi_host/host7/scan',
process_input='6 7 1',
root_helper=None, run_as_root=True),
mock.call('tee', '-a', '/sys/class/scsi_host/host11/scan',
process_input='8 9 1',
root_helper=None, run_as_root=True)]
execute_mock.assert_has_calls(expected_commands)
self.assertEqual(len(expected_commands), execute_mock.call_count)
expected_calls = [mock.call(hbas[0], con_props),
mock.call(hbas[1], con_props)]
mock_get_chan.assert_has_calls(expected_calls)
def test_rescan_hosts_wildcard(self):
hbas = [{'host_device': 'host10', 'node_name': '5006016090203181'},
{'host_device': 'host11', 'node_name': '5006016090203181'}]
hbas, con_props = self.__get_rescan_info(zone_manager=True)
with mock.patch.object(self.lfc, '_get_hba_channel_scsi_target',
return_value=None), \
side_effect=(None, [])), \
mock.patch.object(self.lfc, '_execute',
return_value=None) as execute_mock:
side_effect=None) as execute_mock:
self.lfc.rescan_hosts(hbas, 1)
self.lfc.rescan_hosts(hbas, con_props)
expected_commands = [
mock.call('tee', '-a', '/sys/class/scsi_host/host10/scan',
mock.call('tee', '-a', '/sys/class/scsi_host/host6/scan',
process_input='- - 1',
root_helper=None, run_as_root=True),
mock.call('tee', '-a', '/sys/class/scsi_host/host11/scan',
process_input='- - 1',
root_helper=None, run_as_root=True)]
execute_mock.assert_has_calls(expected_commands)
self.assertEqual(len(expected_commands), execute_mock.call_count)
def test_rescan_hosts_wildcard_exception(self):
def _execute(cmd, *args, **kwargs):
if cmd.startswith('grep'):
raise Exception
hbas = [{'host_device': 'host10', 'node_name': '5006016090203181'},
{'host_device': 'host11', 'node_name': '5006016090203181'}]
with mock.patch.object(self.lfc, '_execute',
side_effect=_execute) as execute_mock:
self.lfc.rescan_hosts(hbas, 1)
expected_commands = [
mock.call('grep 5006016090203181 /sys/class/fc_transport/'
'target10:*/node_name'),
mock.call('tee', '-a', '/sys/class/scsi_host/host10/scan',
process_input='- - 1',
root_helper=None, run_as_root=True),
mock.call('grep 5006016090203181 /sys/class/fc_transport/'
'target11:*/node_name'),
mock.call('tee', '-a', '/sys/class/scsi_host/host11/scan',
mock.call('tee', '-a', '/sys/class/scsi_host/host7/scan',
process_input='- - 1',
root_helper=None, run_as_root=True)]