diff --git a/os_brick/initiator/linuxfc.py b/os_brick/initiator/linuxfc.py index 2c77cf6bd..9785092ac 100644 --- a/os_brick/initiator/linuxfc.py +++ b/os_brick/initiator/linuxfc.py @@ -33,7 +33,74 @@ class LinuxFibreChannel(linuxscsi.LinuxSCSI): # Only load the sysfs attributes we care about HBA_ATTRIBUTES = ('port_name', 'node_name', 'port_state') - def _get_hba_channel_scsi_target_lun(self, hba, conn_props): + def _get_target_fc_transport_path(self, path, wwpn, lun): + """Scan target in the fc_transport path + + Scan for target in the following path: + * /sys/class/fc_transport/target* + + :returns: List with [c, t, l] if the target path exists else + empty list + """ + cmd = 'grep -Gil "%(wwpns)s" %(path)s*/port_name' % {'wwpns': wwpn, + 'path': path} + # We need to run command in shell to expand the * glob + out, _err = self._execute(cmd, shell=True) # nosec: B604 + # The grep command will only return 1 path (if found) + # associated with the target wwpn used for the search + # in the current HBA host + out_path = out.split('\n')[0] + if out_path.startswith(path): + return out_path.split('/')[4].split(':')[1:] + [lun] + + return [] + + def _get_target_fc_remote_ports_path(self, path, wwpn, lun): + """Scan target in the fc_remote_ports path + + Scan for target in the following path: + * /sys/class/fc_remote_ports/rport-* + + If the path exist, we fetch the target value from the + scsi_target_id file. + Example: /sys/class/fc_remote_ports/rport-6:0-1/scsi_target_id + + :returns: List with [c, t, l] if the target path exists else + empty list + """ + cmd = 'grep -Gil "%(wwpns)s" %(path)s*/port_name' % {'wwpns': wwpn, + 'path': path} + # We need to run command in shell to expand the * glob + out, _err = self._execute(cmd, shell=True) # nosec: B604 + # The scsi_target_id file contains the target ID. + # Example path: + # /sys/class/fc_remote_ports/rport-2:0-0/scsi_target_id + target_path = os.path.dirname(out) + '/scsi_target_id' + # There could be a case where the out variable has empty string + # and we end up with a path '/scsi_target_id' so check if it + # starts with the correct path + if target_path.startswith(path): + try: + scsi_target = '-1' + with open(target_path) as scsi_target_file: + lines = scsi_target_file.read() + scsi_target = lines.split('\n')[0] + except OSError: + # We were not able to read from the scsi_target_id + # file but we can still discover other targets so + # continue + pass + # If the target value is -1, it is not a real target so + # skip it + if scsi_target != '-1': + channel = target_path.split(':')[1].split('-')[0] + return [channel, scsi_target, lun] + + return [] + + def _get_hba_channel_scsi_target_lun(self, + hba, + conn_props): """Get HBA channels, SCSI targets, LUNs to FC targets for given HBA. Given an HBA and the connection properties we look for the HBA channels @@ -48,6 +115,13 @@ class LinuxFibreChannel(linuxscsi.LinuxSCSI): based on the contents of the connection information data to know which target ports to look for. + We scan for targets in the following two paths: + * /sys/class/fc_transport/target* + * /sys/class/fc_remote_ports/rport-* + + We search for targets in the fc_transport path first and if not + found, we search in the fc_remote_ports path + :returns: 2-Tuple with the first entry being a list of [c, t, l] entries where the target port was found, and the second entry of the tuple being a set of luns for ports that were not found. @@ -66,22 +140,27 @@ class LinuxFibreChannel(linuxscsi.LinuxSCSI): host_device = host_device[4:] path = '/sys/class/fc_transport/target%s:' % host_device + rpath = '/sys/class/fc_remote_ports/rport-%s:' % host_device + ctls = [] luns_not_found = set() for wwpn, lun in targets: - cmd = 'grep -Gil "%(wwpns)s" %(path)s*/port_name' % {'wwpns': wwpn, - 'path': path} try: - # We need to run command in shell to expand the * glob - out, _err = self._execute(cmd, shell=True) # nosec: B604 - ctls += [line.split('/')[4].split(':')[1:] + [lun] - for line in out.split('\n') if line.startswith(path)] + # Search for target in the fc_transport path first and if we + # don't find ctl, search for target in the fc_remote_ports path + ctl = (self._get_target_fc_transport_path(path, wwpn, lun) or + self._get_target_fc_remote_ports_path(rpath, wwpn, lun)) + + if ctl: + ctls.append(ctl) + 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}) # If we didn't find any paths add it to the not found list luns_not_found.add(lun) + return ctls, luns_not_found def rescan_hosts(self, diff --git a/os_brick/tests/initiator/test_linuxfc.py b/os_brick/tests/initiator/test_linuxfc.py index 63ab9a4d2..97a7728c8 100644 --- a/os_brick/tests/initiator/test_linuxfc.py +++ b/os_brick/tests/initiator/test_linuxfc.py @@ -15,11 +15,13 @@ import os.path from unittest import mock +import ddt from os_brick.initiator import linuxfc from os_brick.tests import base +@ddt.ddt class LinuxFCTestCase(base.TestCase): def setUp(self): @@ -70,85 +72,124 @@ class LinuxFCTestCase(base.TestCase): del connection_properties['initiator_target_lun_map'] return hbas, connection_properties - def test__get_hba_channel_scsi_target_lun_single_wwpn(self): - execute_results = ('/sys/class/fc_transport/target6:0:1/port_name\n', - '') + @staticmethod + def _get_expected_info(wwpns=["514f0c50023f6c00", "514f0c50023f6c01"], + targets=1, remote_scan=False): + + execute_results = [] + expected_cmds = [] + for i in range(0, targets): + expected_cmds += [ + mock.call(f'grep -Gil "{wwpns[i]}" ' + '/sys/class/fc_transport/target6:*/port_name', + shell=True) + ] + if remote_scan: + execute_results += [ + # We can only perform remote ports scan if the + # fc_transport path returns empty output + ('', ''), + # This is returned from the fc_remote_ports path + (f'/sys/class/fc_remote_ports/rport-6:0-{i+1}' + '/port_name\n', ''), + ] + expected_cmds += [ + mock.call(f'grep -Gil "{wwpns[i]}" ' + '/sys/class/fc_remote_ports/rport-6:*/port_name', + shell=True), + ] + else: + execute_results += [ + (f'/sys/class/fc_transport/target6:0:{i+1}/port_name\n', + '') + ] + + return execute_results, expected_cmds + + @mock.patch('builtins.open') + @ddt.data(True, False) + def test__get_hba_channel_scsi_target_lun_single_wwpn( + self, remote_scan, mock_open): + execute_results, expected_cmds = self._get_expected_info( + remote_scan=remote_scan) + if remote_scan: + mock_open = mock_open.return_value.__enter__.return_value + mock_open.read.return_value = ('1\n') hbas, con_props = self.__get_rescan_info() con_props['target_wwn'] = con_props['target_wwn'][0] con_props['targets'] = con_props['targets'][0:1] with mock.patch.object(self.lfc, '_execute', - return_value=execute_results) as execute_mock: + side_effect=execute_results) as execute_mock: res = self.lfc._get_hba_channel_scsi_target_lun(hbas[0], con_props) - execute_mock.assert_called_once_with( - 'grep -Gil "514f0c50023f6c00" ' - '/sys/class/fc_transport/target6:*/port_name', - shell=True) + execute_mock.assert_has_calls(expected_cmds) expected = ([['0', '1', 1]], set()) self.assertEqual(expected, res) - def test__get_hba_channel_scsi_target_lun_with_initiator_target_map(self): - execute_results = ('/sys/class/fc_transport/target6:0:1/port_name\n', - '') + @mock.patch('builtins.open') + @ddt.data(True, False) + def test__get_hba_channel_scsi_target_lun_with_initiator_target_map( + self, remote_scan, mock_open): + execute_results, expected_cmds = self._get_expected_info( + wwpns=["514f0c50023f6c01"]) + if remote_scan: + mock_open = mock_open.return_value.__enter__.return_value + mock_open.read.return_value = ('1\n') hbas, con_props = self.__get_rescan_info(zone_manager=True) con_props['target_wwn'] = con_props['target_wwn'][0] con_props['targets'] = con_props['targets'][0:1] hbas[0]['port_name'] = '50014380186af83e' with mock.patch.object(self.lfc, '_execute', - return_value=execute_results) as execute_mock: + side_effect=execute_results) as execute_mock: res = self.lfc._get_hba_channel_scsi_target_lun(hbas[0], con_props) - execute_mock.assert_called_once_with( - 'grep -Gil "514f0c50023f6c01" ' - '/sys/class/fc_transport/target6:*/port_name', - shell=True) + execute_mock.assert_has_calls(expected_cmds) expected = ([['0', '1', 1]], set()) self.assertEqual(expected, res) + @mock.patch('builtins.open') + @ddt.data(True, False) def test__get_hba_channel_scsi_target_lun_with_initiator_target_map_none( - self): - execute_results = ('/sys/class/fc_transport/target6:0:1/port_name\n', - '') + self, remote_scan, mock_open): + execute_results, expected_cmds = self._get_expected_info() + if remote_scan: + mock_open = mock_open.return_value.__enter__.return_value + mock_open.read.return_value = ('1\n') hbas, con_props = self.__get_rescan_info() con_props['target_wwn'] = con_props['target_wwn'][0] con_props['targets'] = con_props['targets'][0:1] con_props['initiator_target_map'] = None hbas[0]['port_name'] = '50014380186af83e' with mock.patch.object(self.lfc, '_execute', - return_value=execute_results) as execute_mock: + side_effect=execute_results) as execute_mock: res = self.lfc._get_hba_channel_scsi_target_lun(hbas[0], con_props) - execute_mock.assert_called_once_with( - 'grep -Gil "514f0c50023f6c00" ' - '/sys/class/fc_transport/target6:*/port_name', - shell=True) + execute_mock.assert_has_calls(expected_cmds) expected = ([['0', '1', 1]], set()) self.assertEqual(expected, res) - def test__get_hba_channel_scsi_target_lun_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', ''], - ] + @mock.patch('builtins.open') + @ddt.data(True, False) + def test__get_hba_channel_scsi_target_lun_multiple_wwpn( + self, remote_scan, mock_open): + execute_results, expected_cmds = self._get_expected_info(targets=2) + if remote_scan: + mock_open = mock_open.return_value.__enter__.return_value + mock_open.read.return_value = ('1\n') hbas, con_props = self.__get_rescan_info() with mock.patch.object(self.lfc, '_execute', side_effect=execute_results) as execute_mock: res = self.lfc._get_hba_channel_scsi_target_lun(hbas[0], con_props) - expected_cmds = [ - mock.call('grep -Gil "514f0c50023f6c00" ' - '/sys/class/fc_transport/target6:*/port_name', - shell=True), - mock.call('grep -Gil "514f0c50023f6c01" ' - '/sys/class/fc_transport/target6:*/port_name', - shell=True), - ] execute_mock.assert_has_calls(expected_cmds) expected = ([['0', '1', 1], ['0', '2', 1]], set()) self.assertEqual(expected, res) - def test__get_hba_channel_scsi_target_lun_multiple_wwpn_and_luns(self): - execute_results = [ - ['/sys/class/fc_transport/target6:0:1/port_name\n', ''], - ['/sys/class/fc_transport/target6:0:2/port_name\n', ''], - ] + @mock.patch('builtins.open') + @ddt.data(True, False) + def test__get_hba_channel_scsi_target_lun_multiple_wwpn_and_luns( + self, remote_scan, mock_open): + execute_results, expected_cmds = self._get_expected_info(targets=2) + if remote_scan: + mock_open = mock_open.return_value.__enter__.return_value + mock_open.read.return_value = ('1\n') hbas, con_props = self.__get_rescan_info() con_props['target_lun'] = [1, 7] con_props['targets'] = [ @@ -158,68 +199,51 @@ class LinuxFCTestCase(base.TestCase): with mock.patch.object(self.lfc, '_execute', side_effect=execute_results) as execute_mock: res = self.lfc._get_hba_channel_scsi_target_lun(hbas[0], con_props) - expected_cmds = [ - mock.call('grep -Gil "514f0c50023f6c00" ' - '/sys/class/fc_transport/target6:*/port_name', - shell=True), - mock.call('grep -Gil "514f0c50023f6c01" ' - '/sys/class/fc_transport/target6:*/port_name', - shell=True), - ] execute_mock.assert_has_calls(expected_cmds) expected = ([['0', '1', 1], ['0', '2', 7]], set()) self.assertEqual(expected, res) - def test__get_hba_channel_scsi_target_lun_zone_manager(self): - execute_results = ('/sys/class/fc_transport/target6:0:1/port_name\n', - '') + @mock.patch('builtins.open') + @ddt.data(True, False) + def test__get_hba_channel_scsi_target_lun_zone_manager( + self, remote_scan, mock_open): + execute_results, expected_cmds = self._get_expected_info() + if remote_scan: + mock_open = mock_open.return_value.__enter__.return_value + mock_open.read.return_value = ('1\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: + side_effect=execute_results) as execute_mock: res = self.lfc._get_hba_channel_scsi_target_lun(hbas[0], con_props) - execute_mock.assert_called_once_with( - 'grep -Gil "514f0c50023f6c00" ' - '/sys/class/fc_transport/target6:*/port_name', - shell=True) + execute_mock.assert_has_calls(expected_cmds) expected = ([['0', '1', 1]], set()) self.assertEqual(expected, res) - def test__get_hba_channel_scsi_target_lun_not_found(self): + def test__get_hba_channel_scsi_target_lun_both_paths_not_found(self): + _, expected_cmds = self._get_expected_info() 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_lun(hbas[0], con_props) - execute_mock.assert_called_once_with( - 'grep -Gil "514f0c50023f6c00" ' - '/sys/class/fc_transport/target6:*/port_name', - shell=True) + execute_mock.assert_has_calls(expected_cmds) self.assertEqual(([], set()), res) def test__get_hba_channel_scsi_target_lun_exception(self): + _, expected_cmds = self._get_expected_info() 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_lun(hbas[0], con_props) - execute_mock.assert_called_once_with( - 'grep -Gil "514f0c50023f6c00" ' - '/sys/class/fc_transport/target6:*/port_name', - shell=True) + execute_mock.assert_has_calls(expected_cmds) self.assertEqual(([], {1}), res) - def test__get_hba_channel_scsi_target_lun_some_exception(self): + def test__get_hba_channel_scsi_target_lun_fc_transport_exception(self): execute_effects = [ ('/sys/class/fc_transport/target6:0:1/port_name\n', ''), Exception() ] - expected_cmds = [ - mock.call('grep -Gil "514f0c50023f6c00" ' - '/sys/class/fc_transport/target6:*/port_name', - shell=True), - mock.call('grep -Gil "514f0c50023f6c01" ' - '/sys/class/fc_transport/target6:*/port_name', - shell=True), - ] + _, expected_cmds = self._get_expected_info() hbas, con_props = self.__get_rescan_info() with mock.patch.object(self.lfc, '_execute', @@ -229,6 +253,86 @@ class LinuxFCTestCase(base.TestCase): expected = ([['0', '1', 1]], {1}) self.assertEqual(expected, res) + @mock.patch('builtins.open') + def test__get_hba_channel_scsi_target_lun_fc_remote_ports_exception( + self, mock_open): + execute_effects = [ + ('', ''), + ('/sys/class/fc_remote_ports/rport-6:0-1/port_name\n', ''), + ('', ''), + Exception() + ] + mock_open = mock_open.return_value.__enter__.return_value + mock_open.read.return_value = ('1\n') + _, expected_cmds = self._get_expected_info() + hbas, con_props = self.__get_rescan_info() + + with mock.patch.object(self.lfc, '_execute', + side_effect=execute_effects) as execute_mock: + res = self.lfc._get_hba_channel_scsi_target_lun(hbas[0], con_props) + execute_mock.assert_has_calls(expected_cmds) + expected = ([['0', '1', 1]], {1}) + self.assertEqual(expected, res) + + @mock.patch('builtins.open') + def test__get_hba_channel_scsi_target_open_oserror( + self, mock_open): + execute_effects, expected_cmds = self._get_expected_info( + targets=2, remote_scan=True) + mock_open = mock_open.return_value.__enter__.return_value + mock_open.read.side_effect = ['1\n', OSError()] + hbas, con_props = self.__get_rescan_info() + + with mock.patch.object(self.lfc, '_execute', + side_effect=execute_effects) as execute_mock: + res = self.lfc._get_hba_channel_scsi_target_lun(hbas[0], con_props) + execute_mock.assert_has_calls(expected_cmds) + expected = ([['0', '1', 1]], set()) + self.assertEqual(expected, res) + + def test__get_target_fc_transport_path(self): + path = '/sys/class/fc_transport/target6:' + execute_results = ('/sys/class/fc_transport/target6:0:1/port_name\n', + '') + + _, con_props = self.__get_rescan_info() + with mock.patch.object(self.lfc, '_execute', + return_value=execute_results) as execute_mock: + ctl = self.lfc._get_target_fc_transport_path( + path, con_props['target_wwn'][0], 1) + execute_mock.assert_called_once_with( + 'grep -Gil "514f0c50023f6c00" ' + '/sys/class/fc_transport/target6:*/port_name', + shell=True) + self.assertEqual(['0', '1', 1], ctl) + + @mock.patch('builtins.open') + def test__get_target_fc_remote_ports_path(self, mock_open): + path = '/sys/class/fc_remote_ports/rport-6:' + execute_results = [ + ('/sys/class/fc_remote_ports/rport-6:0-1/port_name\n', ''), + ('1\n', ''), + ] + scsi_target_path = ( + '/sys/class/fc_remote_ports/rport-6:0-1/scsi_target_id') + mock_open.return_value.__enter__.return_value.read.return_value = ( + '1\n') + hbas, con_props = self.__get_rescan_info() + with mock.patch.object(self.lfc, '_execute', + side_effect=execute_results) as execute_mock: + ctl = self.lfc._get_target_fc_remote_ports_path( + path, con_props['target_wwn'][0], 1) + expected_cmds = [ + mock.call( + 'grep -Gil "514f0c50023f6c00" ' + '/sys/class/fc_remote_ports/rport-6:*/port_name', + shell=True), + ] + execute_mock.assert_has_calls(expected_cmds) + mock_open.assert_called_once_with(scsi_target_path) + + self.assertEqual(['0', '1', 1], ctl) + def test_rescan_hosts_initiator_map(self): """Test FC rescan with initiator map and not every HBA connected.""" get_chan_results = [([['2', '3', 1], ['4', '5', 1]], set()), diff --git a/releasenotes/notes/fix-fc-scanning-9164da9eb42aaed0.yaml b/releasenotes/notes/fix-fc-scanning-9164da9eb42aaed0.yaml new file mode 100644 index 000000000..0426ce326 --- /dev/null +++ b/releasenotes/notes/fix-fc-scanning-9164da9eb42aaed0.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + FC connector `bug #2051237 + `_: Fix + issue with fibre channel connector scanning partial targets. + We search for target information in sysfs, first in fc_transport + and then in fc_report_ports.