Read HBA information from sysfs

We were gathering HBA information using the systool command line tool,
but this tool is no longer going to be packaged in some Operating
Systems, for example in RHEL9 where the sysfsutils package is being
removed.

To prevent os-brick from breaking in those systems this patch removes
the usage of the command and reads the necessary information from sysfs.

Conveniently reading from sysfs should also be faster because we no
longer need to make a privsep call to run a command, instead we are
reading the minimum number of files and information possible.

Change-Id: Idb8b0c22a30e52c7f84a54dd9f410ff657f502a4

os_brick/initiator/linuxfc.py

@@ -14,7 +14,7 @@
 
 """Generic linux Fibre Channel utilities."""
 
-import errno
+import glob
 import os
 
 from oslo_concurrency import processutils as putils
@@ -26,13 +26,9 @@ LOG = logging.getLogger(__name__)
 
 
 class LinuxFibreChannel(linuxscsi.LinuxSCSI):
-
+    FC_HOST_SYSFS_PATH = '/sys/class/fc_host'
-    def has_fc_support(self):
+    # Only load the sysfs attributes we care about
-        FC_HOST_SYSFS_PATH = '/sys/class/fc_host'
+    HBA_ATTRIBUTES = ('port_name', 'node_name', 'port_state')
-        if os.path.isdir(FC_HOST_SYSFS_PATH):
-            return True
-        else:
-            return False
 
     def _get_hba_channel_scsi_target_lun(self, hba, conn_props):
         """Get HBA channels, SCSI targets, LUNs to FC targets for given HBA.
@@ -148,66 +144,24 @@ class LinuxFibreChannel(linuxscsi.LinuxSCSI):
                                            't': target_id,
                                            'l': target_lun})
 
-    def get_fc_hbas(self):
+    @classmethod
-        """Get the Fibre Channel HBA information."""
+    def get_fc_hbas(cls):
-
+        """Get the Fibre Channel HBA information from sysfs."""
-        if not self.has_fc_support():
-            # there is no FC support in the kernel loaded
-            # so there is no need to even try to run systool
-            LOG.debug("No Fibre Channel support detected on system.")
-            return []
-
-        out = None
-        try:
-            out, _err = self._execute('systool', '-c', 'fc_host', '-v',
-                                      run_as_root=True,
-                                      root_helper=self._root_helper)
-        except putils.ProcessExecutionError as exc:
-            # This handles the case where rootwrap is used
-            # and systool is not installed
-            # 96 = nova.cmd.rootwrap.RC_NOEXECFOUND:
-            if exc.exit_code == 96:
-                LOG.warning("systool is not installed")
-            return []
-        except OSError as exc:
-            # This handles the case where rootwrap is NOT used
-            # and systool is not installed
-            if exc.errno == errno.ENOENT:
-                LOG.warning("systool is not installed")
-            return []
-
-        # No FC HBAs were found
-        if out is None:
-            return []
-
-        lines = out.split('\n')
-        # ignore the first 2 lines
-        lines = lines[2:]
         hbas = []
-        hba = {}
+        for hostpath in glob.glob(f'{cls.FC_HOST_SYSFS_PATH}/*'):
-        lastline = None
+            try:
-        for line in lines:
+                hba = {'ClassDevice': os.path.basename(hostpath),
-            line = line.strip()
+                       'ClassDevicepath': os.path.realpath(hostpath)}
-            # 2 newlines denotes a new hba port
+                for attribute in cls.HBA_ATTRIBUTES:
-            if line == '' and lastline == '':
+                    with open(os.path.join(hostpath, attribute), 'rt') as f:
-                if len(hba) > 0:
+                        hba[attribute] = f.read().strip()
-                    hbas.append(hba)
+                hbas.append(hba)
-                    hba = {}
+            except Exception as exc:
-            else:
+                LOG.warning(f'Could not read attributes for {hostpath}: {exc}')
-                val = line.split('=')
-                if len(val) == 2:
-                    key = val[0].strip().replace(" ", "")
-                    value = val[1].strip()
-                    hba[key] = value.replace('"', '')
-            lastline = line
-
         return hbas
 
     def get_fc_hbas_info(self):
         """Get Fibre Channel WWNs and device paths from the system, if any."""
-
-        # Note(walter-boring) modern Linux kernels contain the FC HBA's in /sys
-        # and are obtainable via the systool app
         hbas = self.get_fc_hbas()
 
         hbas_info = []
@@ -224,9 +178,6 @@ class LinuxFibreChannel(linuxscsi.LinuxSCSI):
 
     def get_fc_wwpns(self):
         """Get Fibre Channel WWPNs from the system, if any."""
-
-        # Note(walter-boring) modern Linux kernels contain the FC HBA's in /sys
-        # and are obtainable via the systool app
         hbas = self.get_fc_hbas()
 
         wwpns = []
@@ -239,9 +190,6 @@ class LinuxFibreChannel(linuxscsi.LinuxSCSI):
 
     def get_fc_wwnns(self):
         """Get Fibre Channel WWNNs from the system, if any."""
-
-        # Note(walter-boring) modern Linux kernels contain the FC HBA's in /sys
-        # and are obtainable via the systool app
         hbas = self.get_fc_hbas()
 
         wwnns = []

os_brick/tests/initiator/test_linuxfc.py

@@ -34,16 +34,6 @@ class LinuxFCTestCase(base.TestCase):
         self.cmds.append(" ".join(cmd))
         return "", None
 
-    def test_has_fc_support(self):
-
-        self.mock_object(os.path, 'isdir', return_value=False)
-        has_fc = self.lfc.has_fc_support()
-        self.assertFalse(has_fc)
-
-        self.mock_object(os.path, 'isdir', return_value=True)
-        has_fc = self.lfc.has_fc_support()
-        self.assertTrue(has_fc)
-
     @staticmethod
     def __get_rescan_info(zone_manager=False):
 
@@ -384,35 +374,97 @@ class LinuxFCTestCase(base.TestCase):
             self.lfc.rescan_hosts(hbas, con_props)
             execute_mock.assert_not_called()
 
-    def test_get_fc_hbas_fail(self):
+    @mock.patch('glob.glob', return_value=[])
-        def fake_exec1(a, b, c, d, run_as_root=True, root_helper='sudo'):
+    def test_get_fc_hbas_no_hbas(self, mock_glob):
-            raise OSError
-
-        def fake_exec2(a, b, c, d, run_as_root=True, root_helper='sudo'):
-            return None, 'None found'
-
-        self.lfc._execute = fake_exec1
         hbas = self.lfc.get_fc_hbas()
-        self.assertEqual(0, len(hbas))
+        self.assertListEqual([], hbas)
-        self.lfc._execute = fake_exec2
+        mock_glob.assert_called_once_with('/sys/class/fc_host/*')
-        hbas = self.lfc.get_fc_hbas()
-        self.assertEqual(0, len(hbas))
 
-    def test_get_fc_hbas(self):
+    @mock.patch('os.path.realpath')
-        def fake_exec(a, b, c, d, run_as_root=True, root_helper='sudo'):
+    @mock.patch('glob.glob', return_value=['/sys/class/fc_host/host0',
-            return SYSTOOL_FC, None
+                                           '/sys/class/fc_host/host2'])
-        self.lfc._execute = fake_exec
+    @mock.patch('builtins.open', side_effect=IOError)
+    def test_get_fc_hbas_fail(self, mock_open, mock_glob, mock_path):
         hbas = self.lfc.get_fc_hbas()
-        self.assertEqual(2, len(hbas))
+        mock_glob.assert_called_once_with('/sys/class/fc_host/*')
-        hba1 = hbas[0]
+        self.assertListEqual([], hbas)
-        self.assertEqual("host0", hba1["ClassDevice"])
+        self.assertEqual(2, mock_open.call_count)
-        hba2 = hbas[1]
+        mock_open.assert_has_calls(
-        self.assertEqual("host2", hba2["ClassDevice"])
+            (mock.call('/sys/class/fc_host/host0/port_name', 'rt'),
+             mock.call('/sys/class/fc_host/host2/port_name', 'rt'))
+        )
+        self.assertEqual(2, mock_path.call_count)
+        mock_path.assert_has_calls(
+            (mock.call('/sys/class/fc_host/host0'),
+             mock.call('/sys/class/fc_host/host2'))
+        )
+
+    @mock.patch('os.path.realpath')
+    @mock.patch('glob.glob', return_value=['/sys/class/fc_host/host0',
+                                           '/sys/class/fc_host/host2'])
+    @mock.patch('builtins.open')
+    def test_get_fc_hbas(self, mock_open, mock_glob, mock_path):
+        mock_open.return_value.__enter__.return_value.read.side_effect = [
+            '0x50014380242b9750\n', '0x50014380242b9751\n', 'Online',
+            '0x50014380242b9752\n', '0x50014380242b9753\n', 'Online',
+        ]
+        pci_path = '/sys/devices/pci0000:20/0000:20:03.0/0000:21:00.'
+        host0_pci = f'{pci_path}0/host0/fc_host/host0'
+        host2_pci = f'{pci_path}1/host2/fc_host/host2'
+        mock_path.side_effect = [host0_pci, host2_pci]
+
+        hbas = self.lfc.get_fc_hbas()
+
+        expected = [
+            {'ClassDevice': 'host0',
+             'ClassDevicepath': host0_pci,
+             'port_name': '0x50014380242b9750',
+             'node_name': '0x50014380242b9751',
+             'port_state': 'Online'},
+            {'ClassDevice': 'host2',
+             'ClassDevicepath': host2_pci,
+             'port_name': '0x50014380242b9752',
+             'node_name': '0x50014380242b9753',
+             'port_state': 'Online'},
+        ]
+        self.assertListEqual(expected, hbas)
+        mock_glob.assert_called_once_with('/sys/class/fc_host/*')
+        self.assertEqual(6, mock_open.call_count)
+        mock_open.assert_has_calls(
+            (mock.call('/sys/class/fc_host/host0/port_name', 'rt'),
+             mock.call('/sys/class/fc_host/host0/node_name', 'rt'),
+             mock.call('/sys/class/fc_host/host0/port_state', 'rt'),
+             mock.call('/sys/class/fc_host/host2/port_name', 'rt'),
+             mock.call('/sys/class/fc_host/host2/node_name', 'rt'),
+             mock.call('/sys/class/fc_host/host2/port_state', 'rt')),
+            any_order=True,
+        )
+        self.assertEqual(2, mock_path.call_count)
+        mock_path.assert_has_calls(
+            (mock.call('/sys/class/fc_host/host0'),
+             mock.call('/sys/class/fc_host/host2'))
+        )
+
+    def _set_get_fc_hbas(self):
+        pci_path = '/sys/devices/pci0000:20/0000:20:03.0/0000:21:00.'
+        host0_pci = f'{pci_path}0/host0/fc_host/host0'
+        host2_pci = f'{pci_path}1/host2/fc_host/host2'
+        return_value = [{'ClassDevice': 'host0',
+                         'ClassDevicepath': host0_pci,
+                         'port_name': '0x50014380242b9750',
+                         'node_name': '0x50014380242b9751',
+                         'port_state': 'Online'},
+                        {'ClassDevice': 'host2',
+                         'ClassDevicepath': host2_pci,
+                         'port_name': '0x50014380242b9752',
+                         'node_name': '0x50014380242b9753',
+                         'port_state': 'Online'}]
+        mocked = self.mock_object(linuxfc.LinuxFibreChannel, 'get_fc_hbas',
+                                  return_value=return_value)
+        return mocked
 
     def test_get_fc_hbas_info(self):
-        def fake_exec(a, b, c, d, run_as_root=True, root_helper='sudo'):
+        mock_hbas = self._set_get_fc_hbas()
-            return SYSTOOL_FC, None
-        self.lfc._execute = fake_exec
         hbas_info = self.lfc.get_fc_hbas_info()
         expected_info = [{'device_path': '/sys/devices/pci0000:20/'
                                          '0000:20:03.0/0000:21:00.0/'
@@ -426,94 +478,22 @@ class LinuxFCTestCase(base.TestCase):
                           'host_device': 'host2',
                           'node_name': '50014380242b9753',
                           'port_name': '50014380242b9752'}, ]
-        self.assertEqual(expected_info, hbas_info)
+        self.assertListEqual(expected_info, hbas_info)
+        mock_hbas.assert_called_once_with()
 
     def test_get_fc_wwpns(self):
-        def fake_exec(a, b, c, d, run_as_root=True, root_helper='sudo'):
+        self._set_get_fc_hbas()
-            return SYSTOOL_FC, None
-
-        self.lfc._execute = fake_exec
         wwpns = self.lfc.get_fc_wwpns()
         expected_wwpns = ['50014380242b9750', '50014380242b9752']
         self.assertEqual(expected_wwpns, wwpns)
 
     def test_get_fc_wwnns(self):
-        def fake_exec(a, b, c, d, run_as_root=True, root_helper='sudo'):
+        self._set_get_fc_hbas()
-            return SYSTOOL_FC, None
+        wwnns = self.lfc.get_fc_wwnns()
-        self.lfc._execute = fake_exec
+        expected_wwnns = ['50014380242b9751', '50014380242b9753']
-        wwnns = self.lfc.get_fc_wwpns()
-        expected_wwnns = ['50014380242b9750', '50014380242b9752']
         self.assertEqual(expected_wwnns, wwnns)
 
 
-SYSTOOL_FC = """
-Class = "fc_host"
-
-  Class Device = "host0"
-  Class Device path = "/sys/devices/pci0000:20/0000:20:03.0/\
-0000:21:00.0/host0/fc_host/host0"
-    dev_loss_tmo        = "16"
-    fabric_name         = "0x100000051ea338b9"
-    issue_lip           = <store method only>
-    max_npiv_vports     = "0"
-    node_name           = "0x50014380242b9751"
-    npiv_vports_inuse   = "0"
-    port_id             = "0x960d0d"
-    port_name           = "0x50014380242b9750"
-    port_state          = "Online"
-    port_type           = "NPort (fabric via point-to-point)"
-    speed               = "8 Gbit"
-    supported_classes   = "Class 3"
-    supported_speeds    = "1 Gbit, 2 Gbit, 4 Gbit, 8 Gbit"
-    symbolic_name       = "QMH2572 FW:v4.04.04 DVR:v8.03.07.12-k"
-    system_hostname     = ""
-    tgtid_bind_type     = "wwpn (World Wide Port Name)"
-    uevent              =
-    vport_create        = <store method only>
-    vport_delete        = <store method only>
-
-    Device = "host0"
-    Device path = "/sys/devices/pci0000:20/0000:20:03.0/0000:21:00.0/host0"
-      edc                 = <store method only>
-      optrom_ctl          = <store method only>
-      reset               = <store method only>
-      uevent              = "DEVTYPE=scsi_host"
-
-
-  Class Device = "host2"
-  Class Device path = "/sys/devices/pci0000:20/0000:20:03.0/\
-0000:21:00.1/host2/fc_host/host2"
-    dev_loss_tmo        = "16"
-    fabric_name         = "0x100000051ea33b79"
-    issue_lip           = <store method only>
-    max_npiv_vports     = "0"
-    node_name           = "0x50014380242b9753"
-    npiv_vports_inuse   = "0"
-    port_id             = "0x970e09"
-    port_name           = "0x50014380242b9752"
-    port_state          = "Online"
-    port_type           = "NPort (fabric via point-to-point)"
-    speed               = "8 Gbit"
-    supported_classes   = "Class 3"
-    supported_speeds    = "1 Gbit, 2 Gbit, 4 Gbit, 8 Gbit"
-    symbolic_name       = "QMH2572 FW:v4.04.04 DVR:v8.03.07.12-k"
-    system_hostname     = ""
-    tgtid_bind_type     = "wwpn (World Wide Port Name)"
-    uevent              =
-    vport_create        = <store method only>
-    vport_delete        = <store method only>
-
-    Device = "host2"
-    Device path = "/sys/devices/pci0000:20/0000:20:03.0/0000:21:00.1/host2"
-      edc                 = <store method only>
-      optrom_ctl          = <store method only>
-      reset               = <store method only>
-      uevent              = "DEVTYPE=scsi_host"
-
-
-"""
-
-
 class LinuxFCS390XTestCase(LinuxFCTestCase):
 
     def setUp(self):
@@ -522,10 +502,14 @@ class LinuxFCS390XTestCase(LinuxFCTestCase):
         self.lfc = linuxfc.LinuxFibreChannelS390X(None,
                                                   execute=self.fake_execute)
 
-    def test_get_fc_hbas_info(self):
+    @mock.patch.object(linuxfc.LinuxFibreChannel, 'get_fc_hbas')
-        def fake_exec(a, b, c, d, run_as_root=True, root_helper='sudo'):
+    def test_get_fc_hbas_info(self, mock_hbas):
-            return SYSTOOL_FC_S390X, None
+        host_pci = '/sys/devices/css0/0.0.02ea/0.0.3080/host0/fc_host/host0'
-        self.lfc._execute = fake_exec
+        mock_hbas.return_value = [{'ClassDevice': 'host0',
+                                   'ClassDevicepath': host_pci,
+                                   'port_name': '0xc05076ffe680a960',
+                                   'node_name': '0x1234567898765432',
+                                   'port_state': 'Online'}]
         hbas_info = self.lfc.get_fc_hbas_info()
         expected = [{'device_path': '/sys/devices/css0/0.0.02ea/'
                                     '0.0.3080/host0/fc_host/host0',
@@ -554,38 +538,3 @@ class LinuxFCS390XTestCase(LinuxFCTestCase):
         expected_commands = [('tee -a /sys/bus/ccw/drivers/zfcp/'
                               '0.0.2319/0x50014380242b9751/unit_remove')]
         self.assertEqual(expected_commands, self.cmds)
-
-
-SYSTOOL_FC_S390X = """
-Class = "fc_host"
-
-  Class Device = "host0"
-  Class Device path = "/sys/devices/css0/0.0.02ea/0.0.3080/host0/fc_host/host0"
-    active_fc4s         = "0x00 0x00 0x01 0x00 0x00 0x00 0x00 0x00 0x00 0x00 \
-    0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 \
-    0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 "
-    dev_loss_tmo        = "60"
-    maxframe_size       = "2112 bytes"
-    node_name           = "0x1234567898765432"
-    permanent_port_name = "0xc05076ffe6803081"
-    port_id             = "0x010014"
-    port_name           = "0xc05076ffe680a960"
-    port_state          = "Online"
-    port_type           = "NPIV VPORT"
-    serial_number       = "IBM00000000000P30"
-    speed               = "8 Gbit"
-    supported_classes   = "Class 2, Class 3"
-    supported_fc4s      = "0x00 0x00 0x01 0x00 0x00 0x00 0x00 0x00 0x00 0x00 \
-    0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 \
-    0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 "
-    supported_speeds    = "2 Gbit, 4 Gbit"
-    symbolic_name       = "IBM     2827            00000000000P30  \
-    PCHID: 0308 NPIV UlpId: 01EA0A00   DEVNO: 0.0.1234 NAME: dummy"
-    tgtid_bind_type     = "wwpn (World Wide Port Name)"
-    uevent              =
-
-    Device = "host0"
-    Device path = "/sys/devices/css0/0.0.02ea/0.0.3080/host0"
-      uevent              = "DEVTYPE=scsi_host"
-
-"""

releasenotes/notes/no-systool-use-b7bc430de1033670.yaml

@@ -0,0 +1,5 @@
+---
+upgrade:
+  - |
+    No longer using ``systool`` to gather FC HBA information, so the
+    ``sysfsutils`` package is no longer needed.