Browse Source

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
changes/52/520052/1
Gorka Eguileor 4 years ago
parent
commit
4ee404466d
  1. 3
      os_brick/initiator/connectors/fibre_channel.py
  2. 33
      os_brick/initiator/linuxfc.py
  3. 174
      os_brick/tests/initiator/test_linuxfc.py

3
os_brick/initiator/connectors/fibre_channel.py

@ -157,8 +157,7 @@ class FibreChannelConnector(base.BaseLinuxConnector):
"Will rescan & retry. Try number: %(tries)s.",
{'tries': self.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

33
os_brick/initiator/linuxfc.py

@ -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 = [('-', '-')]

174
os_brick/tests/initiator/test_linuxfc.py

@ -44,87 +44,141 @@ class LinuxFCTestCase(base.TestCase):
has_fc = self.lfc.has_fc_support()
self.assertTrue(has_fc)
@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',
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):
# 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'}]
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',
side_effect=execute_results) as execute_mock:
self.lfc.rescan_hosts(hbas, 1)
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:
self.lfc.rescan_hosts(hbas, 1)
expected_commands = [
mock.call('tee', '-a', '/sys/class/scsi_host/host10/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:
side_effect=None) as execute_mock:
self.lfc.rescan_hosts(hbas, 1)
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='- - 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)]

Loading…
Cancel
Save