Merge pull request #379 from martin-belanger/v2.3-rc3

trid: Take Host NQN into account

NEWS.md

@@ -13,13 +13,13 @@ Bug fixes:
 * For TCP transport: use `sysfs` controller  `src_addr` attribute when matching to a configured "candidate" controller. This is to determine when an existing controller (located under the `sysfs`) can be reused instead of creating a new one. This avoids creating unnecessary duplicate connections.
 * Udev event handling: use `systemctl restart` instead of `systemctl start`. There is a small chance that a `start` operation has not completed when a new `start` is required. Issuing a `start` while a `start` is being performed has no effect. However, a `restart` will be handled properly.
 * `stafd`: Do not delete and recreate DC objects on kernel events indicating that an nvme device associated to a discovery controller was removed by the kernel. This was done to kick start the reconnect process, but was also causing the DLPE (Discovery Log Page Entries) cache to be lost. This could potentially result in `stacd` disconnecting from I/O controllers. Instead, keep the existing DC object which contains a valid DLPE cache and simply restart the "retry to connect" timer. This way the DLPE cache is maintained throughout the reconnect to DC process.
+* While testing Boot from SAN (BFS) and using a Host NQN during boot that is different from the Host NQN used after boot (i.e. the Host NQN defined in `/etc/nvme/hostnqn`), we found that nvme-stas and libnvme are reusing existing connections even if the Host NQN doesn't match. nvme-stas will now take a connection's Host NQN into consideration before deciding if a connection can be reused. A similar fix will be provided in libnvme as well.
 
 ## Changes with release 2.2.3
 
 Bug fixes:
 
 * When processing kernel nvme events, only react to `rediscover` and not to `connected` events. The `connected` event happens too early (before the nvme device has been fully identified).
-* 
 
 ## Changes with release 2.2.2
 

meson.build

@@ -9,7 +9,7 @@
 project(
     'nvme-stas',
     meson_version: '>= 0.53.0',
-    version: '2.3-rc2',
+    version: '2.3-rc3',
     license: 'Apache-2.0',
     default_options: [
         'buildtype=release',

stacctl.py

@@ -39,9 +39,10 @@ def _extract_cid(ctrl):
         ctrl['transport'],
         ctrl['traddr'],
         ctrl['trsvcid'],
+        ctrl['subsysnqn'],
         ctrl['host-traddr'],
         ctrl['host-iface'],
-        ctrl['subsysnqn'],
+        ctrl['host-nqn'],
     )
 
 
@@ -52,9 +53,9 @@ def status(args):  # pylint: disable=unused-argument
     info = json.loads(iface.process_info())
     info['controllers'] = iface.list_controllers(True)
     for controller in info['controllers']:
-        transport, traddr, trsvcid, host_traddr, host_iface, subsysnqn = _extract_cid(controller)
+        transport, traddr, trsvcid, subsysnqn, host_traddr, host_iface, host_nqn = _extract_cid(controller)
         controller.update(
-            json.loads(iface.controller_info(transport, traddr, trsvcid, host_traddr, host_iface, subsysnqn))
+            json.loads(iface.controller_info(transport, traddr, trsvcid, subsysnqn, host_traddr, host_iface, host_nqn))
         )
 
     print(pprint.pformat(info, width=120))

stacd.py

@@ -90,10 +90,10 @@ def process_info(self) -> str:
             return json.dumps(info)
 
         def controller_info(  # pylint: disable=too-many-arguments,no-self-use
-            self, transport, traddr, trsvcid, host_traddr, host_iface, subsysnqn
+            self, transport, traddr, trsvcid, subsysnqn, host_traddr, host_iface, host_nqn
         ) -> str:
             '''@brief D-Bus method used to return information about a controller'''
-            controller = STAC.get_controller(transport, traddr, trsvcid, host_traddr, host_iface, subsysnqn)
+            controller = STAC.get_controller(transport, traddr, trsvcid, subsysnqn, host_traddr, host_iface, host_nqn)
             return json.dumps(controller.info()) if controller else '{}'
 
         def list_controllers(self, detailed) -> list:  # pylint: disable=no-self-use

stafctl.py

@@ -15,7 +15,7 @@
 from argparse import ArgumentParser
 import dasbus.error
 from dasbus.connection import SystemMessageBus
-from staslib import defs
+from staslib import conf, defs
 
 
 def tron(args):  # pylint: disable=unused-argument
@@ -39,9 +39,10 @@ def _extract_cid(ctrl):
         ctrl['transport'],
         ctrl['traddr'],
         ctrl['trsvcid'],
+        ctrl['subsysnqn'],
         ctrl['host-traddr'],
         ctrl['host-iface'],
-        ctrl['subsysnqn'],
+        ctrl['host-nqn'],
     )
 
 
@@ -52,10 +53,12 @@ def status(args):  # pylint: disable=unused-argument
     info = json.loads(iface.process_info())
     info['controllers'] = iface.list_controllers(True)
     for controller in info['controllers']:
-        transport, traddr, trsvcid, host_traddr, host_iface, subsysnqn = _extract_cid(controller)
-        controller['log_pages'] = iface.get_log_pages(transport, traddr, trsvcid, host_traddr, host_iface, subsysnqn)
+        transport, traddr, trsvcid, subsysnqn, host_traddr, host_iface, host_nqn = _extract_cid(controller)
+        controller['log_pages'] = iface.get_log_pages(
+            transport, traddr, trsvcid, subsysnqn, host_traddr, host_iface, host_nqn
+        )
         controller.update(
-            json.loads(iface.controller_info(transport, traddr, trsvcid, host_traddr, host_iface, subsysnqn))
+            json.loads(iface.controller_info(transport, traddr, trsvcid, subsysnqn, host_traddr, host_iface, host_nqn))
         )
 
     print(pprint.pformat(info, width=120))
@@ -75,7 +78,15 @@ def dlp(args):
     '''@brief retrieve a controller's discovery log pages from stafd'''
     bus = SystemMessageBus()
     iface = bus.get_proxy(defs.STAFD_DBUS_NAME, defs.STAFD_DBUS_PATH)
-    info = iface.get_log_pages(args.transport, args.traddr, args.trsvcid, args.host_traddr, args.host_iface, args.nqn)
+    info = iface.get_log_pages(
+        args.transport,
+        args.traddr,
+        args.trsvcid,
+        args.nqn,
+        args.host_traddr,
+        args.host_iface,
+        args.host_nqn,
+    )
     print(pprint.pformat(info, width=120))
 
 
@@ -153,6 +164,14 @@ def adlp(args):
     help='This field specifies the network interface used on the host to connect to the Controller (default: "%(default)s")',
     default='',
 )
+PRSR.add_argument(
+    '-q',
+    '--host-nqn',
+    metavar='<nqn>',
+    action='store',
+    help='This field specifies the host NQN (default: "%(default)s")',
+    default=conf.SysConf().hostnqn,
+)
 PRSR.add_argument(
     '-n',
     '--nqn',

stafd.py

@@ -70,9 +70,10 @@ def log_pages_changed(  # pylint: disable=too-many-arguments
             transport: str,
             traddr: str,
             trsvcid: str,
+            subsysnqn: str,
             host_traddr: str,
             host_iface: str,
-            subsysnqn: str,
+            host_nqn: str,
             device: str,
         ):
             '''@brief Signal sent when log pages have changed.'''
@@ -108,17 +109,17 @@ def process_info(self) -> str:
             return json.dumps(info)
 
         def controller_info(  # pylint: disable=no-self-use,too-many-arguments
-            self, transport, traddr, trsvcid, host_traddr, host_iface, subsysnqn
+            self, transport, traddr, trsvcid, subsysnqn, host_traddr, host_iface, host_nqn
         ) -> str:
             '''@brief D-Bus method used to return information about a controller'''
-            controller = STAF.get_controller(transport, traddr, trsvcid, host_traddr, host_iface, subsysnqn)
+            controller = STAF.get_controller(transport, traddr, trsvcid, subsysnqn, host_traddr, host_iface, host_nqn)
             return json.dumps(controller.info()) if controller else '{}'
 
         def get_log_pages(  # pylint: disable=no-self-use,too-many-arguments
-            self, transport, traddr, trsvcid, host_traddr, host_iface, subsysnqn
+            self, transport, traddr, trsvcid, subsysnqn, host_traddr, host_iface, host_nqn
         ) -> list:
             '''@brief D-Bus method used to retrieve the discovery log pages from one controller'''
-            controller = STAF.get_controller(transport, traddr, trsvcid, host_traddr, host_iface, subsysnqn)
+            controller = STAF.get_controller(transport, traddr, trsvcid, subsysnqn, host_traddr, host_iface, host_nqn)
             return controller.log_pages() if controller else list()
 
         def get_all_log_pages(self, detailed) -> str:  # pylint: disable=no-self-use

staslib/avahi.py

@@ -333,15 +333,19 @@ def get_controllers(self) -> list:
                 'transport': tcp,
                 'traddr': str(),
                 'trsvcid': str(),
-                'host-iface': str(),
                 'subsysnqn': 'nqn.2014-08.org.nvmexpress.discovery',
+                'host-traddr': str(),
+                'host-iface': str(),
+                'host-nqn': str(),
             },
             {
                 'transport': tcp,
                 'traddr': str(),
                 'trsvcid': str(),
-                'host-iface': str(),
                 'subsysnqn': 'nqn.2014-08.org.nvmexpress.discovery',
+                'host-traddr': str(),
+                'host-iface': str(),
+                'host-nqn': str(),
             },
             [...]
         ]

staslib/conf.py

@@ -334,9 +334,10 @@ def get_controllers(self):
             'transport':          [TRANSPORT],
             'traddr':             [TRADDR],
             'trsvcid':            [TRSVCID],
+            'subsysnqn':          [NQN],
             'host-traddr':        [TRADDR],
             'host-iface':         [IFACE],
-            'subsysnqn':          [NQN],
+            'host-nqn':           [NQN],
             'dhchap-ctrl-secret': [KEY],
             'hdr-digest':         [BOOL]
             'data-digest':        [BOOL]
@@ -721,9 +722,11 @@ def __init__(self, root_dir=defs.NBFT_SYSFS_PATH):
             hfis = data.get('hfi', [])
             discovery = data.get('discovery', [])
             subsystem = data.get('subsystem', [])
+            host = data.get('host', {})
+            hostnqn = host.get('nqn', None) if host.get('host_nqn_configured', False) else None
 
-            self._disc_ctrls.extend(NbftConf.__nbft_disc_to_cids(discovery, hfis))
-            self._subs_ctrls.extend(NbftConf.__nbft_subs_to_cids(subsystem, hfis))
+            self._disc_ctrls.extend(NbftConf.__nbft_disc_to_cids(hostnqn, discovery, hfis))
+            self._subs_ctrls.extend(NbftConf.__nbft_subs_to_cids(hostnqn, subsystem, hfis))
 
     dcs = property(lambda self: self._disc_ctrls)
     iocs = property(lambda self: self._subs_ctrls)
@@ -738,12 +741,14 @@ def get_controllers(self):
         return self.dcs if defs.PROG_NAME == 'stafd' else []
 
     @staticmethod
-    def __nbft_disc_to_cids(discovery, hfis):
+    def __nbft_disc_to_cids(hostnqn, discovery, hfis):
         cids = []
 
         for ctrl in discovery:
             cid = NbftConf.__uri2cid(ctrl['uri'])
             cid['subsysnqn'] = ctrl['nqn']
+            if hostnqn:
+                cid['host-nqn'] = hostnqn
 
             host_iface = NbftConf.__get_host_iface(ctrl.get('hfi_index'), hfis)
             if host_iface:
@@ -754,7 +759,7 @@ def __nbft_disc_to_cids(discovery, hfis):
         return cids
 
     @staticmethod
-    def __nbft_subs_to_cids(subsystem, hfis):
+    def __nbft_subs_to_cids(hostnqn, subsystem, hfis):
         cids = []
 
         for ctrl in subsystem:
@@ -766,6 +771,8 @@ def __nbft_subs_to_cids(subsystem, hfis):
                 'hdr-digest': ctrl['pdu_header_digest_required'],
                 'data-digest': ctrl['data_digest_required'],
             }
+            if hostnqn:
+                cid['host-nqn'] = hostnqn
 
             indexes = ctrl.get('hfi_indexes')
             if isinstance(indexes, list) and len(indexes) > 0:

staslib/service.py

@@ -393,9 +393,10 @@ def _config_ctrls_finish(self, configured_ctrl_list: list):  # pylint: disable=t
         for staf_data in self._get_log_pages_from_stafd():
             host_traddr = staf_data['discovery-controller']['host-traddr']
             host_iface = staf_data['discovery-controller']['host-iface']
+            host_nqn = staf_data['discovery-controller']['host-nqn']
             for dlpe in staf_data['log-pages']:
                 if dlpe.get('subtype') == 'nvme':  # eliminate discovery controllers
-                    tid = stas.tid_from_dlpe(dlpe, host_traddr, host_iface)
+                    tid = stas.tid_from_dlpe(dlpe, host_traddr, host_iface, host_nqn)
                     discovered_ctrls[tid] = dlpe
 
         discovered_ctrl_list = list(discovered_ctrls.keys())
@@ -476,19 +477,20 @@ def _disconnect_from_staf(self, watcher):
         logging.debug('Stac._disconnect_from_staf()       - Disconnected from staf')
 
     def _log_pages_changed(  # pylint: disable=too-many-arguments
-        self, transport, traddr, trsvcid, host_traddr, host_iface, subsysnqn, device
+        self, transport, traddr, trsvcid, subsysnqn, host_traddr, host_iface, host_nqn, device
     ):
         if not self._alive():
             return
 
         logging.debug(
-            'Stac._log_pages_changed()          - transport=%s, traddr=%s, trsvcid=%s, host_traddr=%s, host_iface=%s, subsysnqn=%s, device=%s',
+            'Stac._log_pages_changed()          - transport=%s, traddr=%s, trsvcid=%s, subsysnqn=%s, host_traddr=%s, host_iface=%s, host_nqn=%s, device=%s',
             transport,
             traddr,
             trsvcid,
+            subsysnqn,
             host_traddr,
             host_iface,
-            subsysnqn,
+            host_nqn,
             device,
         )
         if self._cfg_soak_tmr:
@@ -693,9 +695,10 @@ def log_pages_changed(self, controller, device):
             controller.tid.transport,
             controller.tid.traddr,
             controller.tid.trsvcid,
+            controller.tid.subsysnqn,
             controller.tid.host_traddr,
             controller.tid.host_iface,
-            controller.tid.subsysnqn,
+            controller.tid.host_nqn,
             device,
         )
 
@@ -708,7 +711,12 @@ def dc_removed(self):
 
     def _referrals(self) -> list:
         return [
-            stas.tid_from_dlpe(dlpe, controller.tid.host_traddr, controller.tid.host_iface)
+            stas.tid_from_dlpe(
+                dlpe,
+                controller.tid.host_traddr,
+                controller.tid.host_iface,
+                controller.tid.host_nqn,
+            )
             for controller in self.get_controllers()
             for dlpe in controller.referrals()
         ]

staslib/stacd.idl

@@ -9,9 +9,10 @@
             <arg direction="in" type="s" name="transport"/>
             <arg direction="in" type="s" name="traddr"/>
             <arg direction="in" type="s" name="trsvcid"/>
+            <arg direction="in" type="s" name="subsysnqn"/>
             <arg direction="in" type="s" name="host_traddr"/>
             <arg direction="in" type="s" name="host_iface"/>
-            <arg direction="in" type="s" name="subsysnqn"/>
+            <arg direction="in" type="s" name="host_nqn"/>
             <arg direction="out" type="s" name="info_json"/>
         </method>
     </interface>

staslib/stafd.idl

@@ -9,9 +9,10 @@
             <arg direction="in" type="s" name="transport"/>
             <arg direction="in" type="s" name="traddr"/>
             <arg direction="in" type="s" name="trsvcid"/>
+            <arg direction="in" type="s" name="subsysnqn"/>
             <arg direction="in" type="s" name="host_traddr"/>
             <arg direction="in" type="s" name="host_iface"/>
-            <arg direction="in" type="s" name="subsysnqn"/>
+            <arg direction="in" type="s" name="host_nqn"/>
             <arg direction="out" type="s" name="info_json"/>
         </method>
     </interface>
@@ -25,9 +26,10 @@
             <arg direction="in" type="s" name="transport"/>
             <arg direction="in" type="s" name="traddr"/>
             <arg direction="in" type="s" name="trsvcid"/>
+            <arg direction="in" type="s" name="subsysnqn"/>
             <arg direction="in" type="s" name="host_traddr"/>
             <arg direction="in" type="s" name="host_iface"/>
-            <arg direction="in" type="s" name="subsysnqn"/>
+            <arg direction="in" type="s" name="host_nqn"/>
             <arg direction="out" type="aa{ss}" name="log_pages"/>
         </method>
         <method name="get_all_log_pages">
@@ -38,9 +40,10 @@
           <arg direction="out" type="s" name="transport"/>
           <arg direction="out" type="s" name="traddr"/>
           <arg direction="out" type="s" name="trsvcid"/>
+          <arg direction="out" type="s" name="subsysnqn"/>
           <arg direction="out" type="s" name="host_traddr"/>
           <arg direction="out" type="s" name="host_iface"/>
-          <arg direction="out" type="s" name="subsysnqn"/>
+          <arg direction="out" type="s" name="host_nqn"/>
           <arg direction="out" type="s" name="device"/>
         </signal>
         <signal name="dc_removed"></signal>