From c5d0fdd6ab5da48dd3e5d8da2e61ec0bf814d9cf Mon Sep 17 00:00:00 2001 From: Ales Novak Date: Thu, 9 Jul 2026 00:52:49 +0200 Subject: [PATCH 01/13] build: enable pylint Pylint reports were previously disabled. Enable them now. Signed-off-by: Ales Novak Signed-off-by: Daniel Wagner --- .github/workflows/python-sanity.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/python-sanity.yml b/.github/workflows/python-sanity.yml index dd5b953..3de7b03 100644 --- a/.github/workflows/python-sanity.yml +++ b/.github/workflows/python-sanity.yml @@ -36,4 +36,4 @@ jobs: run: python3 -m flake8 nvmet/ nvmetcli - name: pylint - run: python3 -m pylint --disable=all --enable=E nvmet/ nvmetcli + run: python3 -m pylint --enable=E nvmet/ nvmetcli From 1e4d54c882d360019b3eb1160af9f8e6c2d9c8fd Mon Sep 17 00:00:00 2001 From: Ales Novak Date: Wed, 8 Jul 2026 23:35:37 +0200 Subject: [PATCH 02/13] nvmet, nvmetcli: convert % formatting to f-strings f-strings are preferred for string formatting. Resolves pylin complaint C0209. Signed-off-by: Ales Novak Signed-off-by: Daniel Wagner --- nvmet/nvme.py | 168 ++++++++++++++++++++++---------------------- nvmet/test_nvmet.py | 8 +-- nvmetcli | 65 +++++++++-------- 3 files changed, 122 insertions(+), 119 deletions(-) diff --git a/nvmet/nvme.py b/nvmet/nvme.py index 2bc0cb8..4baec63 100644 --- a/nvmet/nvme.py +++ b/nvmet/nvme.py @@ -73,20 +73,20 @@ def _create_in_cfs(self, mode): create -> create the node which must not exist beforehand ''' if mode not in ['any', 'lookup', 'create']: - raise CFSError("Invalid mode: %s" % mode) + raise CFSError(f"Invalid mode: {mode}") if self.exists and mode == 'create': - raise CFSError("This %s already exists in configFS" % - self.__class__.__name__) - elif not self.exists and mode == 'lookup': - raise CFSNotFound("No such %s in configfs: %s" % - (self.__class__.__name__, self.path)) + raise CFSError(f"This {self.__class__.__name__} already " + f"exists in configFS") + if not self.exists and mode == 'lookup': + raise CFSNotFound(f"No such {self.__class__.__name__} in " + f"configfs: {self.path}") if not self.exists: try: os.mkdir(self.path) - except Exception: - raise CFSError("Could not create %s in configFS" % - self.__class__.__name__) + except Exception as exc: + raise CFSError(f"Could not create {self.__class__.__name__}" + f" in configFS") from exc self.get_enable() def _exists(self): @@ -94,8 +94,8 @@ def _exists(self): def _check_self(self): if not self.exists: - raise CFSNotFound("This %s does not exist in configFS" % - self.__class__.__name__) + raise CFSNotFound(f"This {self.__class__.__name__} does not " + f"exist in configFS") def list_attrs(self, group, writable=None): ''' @@ -109,7 +109,7 @@ def list_attrs(self, group, writable=None): self._check_self() names = [os.path.basename(name).split('_', 1)[1] - for name in glob("%s/%s_*" % (self._path, group)) + for name in glob(f"{self._path}/{group}_*") if os.path.isfile(name)] if writable is True: @@ -123,7 +123,10 @@ def list_attrs(self, group, writable=None): return names def _attr_is_writable(self, group, name): - s = os.stat("%s/%s_%s" % (self._path, group, name)) + ''' + Returns True if the attribute is writable, False otherwise. + ''' + s = os.stat(f"{self._path}/{group}_{name}") return s[stat.ST_MODE] & stat.S_IWUSR def set_attr(self, group, attribute, value): @@ -136,20 +139,20 @@ def set_attr(self, group, attribute, value): @type value: string ''' self._check_self() - path = "%s/%s_%s" % (self.path, str(group), str(attribute)) + path = f"{self.path}/{str(group)}_{str(attribute)}" if not os.path.isfile(path): - raise CFSError("Cannot find attribute: %s" % path) + raise CFSError(f"Cannot find attribute: {path}") if self._enable: - raise CFSError("Cannot set attribute while %s is enabled" % - self.__class__.__name__) + raise CFSError(f"Cannot set attribute while " + f"{self.__class__.__name__} is enabled") try: with open(path, 'w') as file_fd: file_fd.write(str(value)) except Exception as e: - raise CFSError("Cannot set attribute %s: %s" % (path, e)) + raise CFSError(f"Cannot set attribute {path}: {e}") from e def get_attr(self, group, attribute): ''' @@ -159,16 +162,16 @@ def get_attr(self, group, attribute): @return: The named attribute's value, as a string. ''' self._check_self() - path = "%s/%s_%s" % (self.path, str(group), str(attribute)) + path = f"{self.path}/{str(group)}_{str(attribute)}" if not os.path.isfile(path): - raise CFSError("Cannot find attribute: %s" % path) + raise CFSError(f"Cannot find attribute: {path}") with open(path, 'r') as file_fd: return file_fd.read().strip() def get_enable(self): self._check_self() - path = "%s/enable" % self.path + path = f"{self.path}/enable" if not os.path.isfile(path): return None @@ -178,17 +181,16 @@ def get_enable(self): def set_enable(self, value): self._check_self() - path = "%s/enable" % self.path + path = f"{self.path}/enable" if not os.path.isfile(path) or self._enable is None: - raise CFSError("Cannot enable %s" % self.path) + raise CFSError(f"Cannot enable {self.path}") try: with open(path, 'w') as file_fd: file_fd.write(str(value)) except Exception as e: - raise CFSError("Cannot enable %s: %s (%s)" % - (self.path, e, value)) + raise CFSError(f"Cannot enable {self.path}: {e} ({value})") from e self._enable = value def delete(self): @@ -240,8 +242,7 @@ def __init__(self): self._modprobe('nvmet') if not os.path.isdir(self.configfs_dir): - raise CFSError("%s does not exist. Giving up." % - self.configfs_dir) + raise CFSError(f"{self.configfs_dir} does not exist. Giving up.") self._path = self.configfs_dir self._create_in_cfs('lookup') @@ -278,7 +279,7 @@ def _modprobe(self, modname): def _list_subsystems(self): self._check_self() - for d in os.listdir("%s/subsystems/" % self._path): + for d in os.listdir(f"{self._path}/subsystems/"): yield Subsystem(d, 'lookup') subsystems = property(_list_subsystems, @@ -287,7 +288,7 @@ def _list_subsystems(self): def _list_ports(self): self._check_self() - for d in os.listdir("%s/ports/" % self._path): + for d in os.listdir(f"{self._path}/ports/"): yield Port(d, 'lookup') ports = property(_list_ports, @@ -296,7 +297,7 @@ def _list_ports(self): def _list_hosts(self): self._check_self() - for h in os.listdir("%s/hosts/" % self._path): + for h in os.listdir(f"{self._path}/hosts/"): yield Host(h, 'lookup') hosts = property(_list_hosts, @@ -371,21 +372,21 @@ def err_func(err_str): # Create the hosts first because the subsystems reference them for index, t in enumerate(config.get('hosts', [])): if 'nqn' not in t: - err_func("'nqn' not defined in host %d" % index) + err_func(f"'nqn' not defined in host {index}") continue Host.setup(t, err_func) for index, t in enumerate(config.get('subsystems', [])): if 'nqn' not in t: - err_func("'nqn' not defined in subsystem %d" % index) + err_func(f"'nqn' not defined in subsystem {index}") continue Subsystem.setup(t, err_func) for index, t in enumerate(config.get('ports', [])): if 'portid' not in t: - err_func("'portid' not defined in port %d" % index) + err_func(f"'portid' not defined in port {index}") continue Port.setup(self, t, err_func) @@ -424,7 +425,7 @@ class Subsystem(CFSNode): ''' def __repr__(self): - return "" % self.nqn + return f"" def __init__(self, nqn=None, mode='any'): ''' @@ -448,13 +449,13 @@ def __init__(self, nqn=None, mode='any'): self.nqn = nqn self.attr_groups = ['attr'] - self._path = "%s/subsystems/%s" % (self.configfs_dir, nqn) + self._path = f"{self.configfs_dir}/subsystems/{nqn}" self._create_in_cfs(mode) def _generate_nqn(self): prefix = "nqn.2014-08.org.nvmexpress:NVMf:uuid" name = str(uuid.uuid4()) - return "%s:%s" % (prefix, name) + return f"{prefix}:{name}" def delete(self): ''' @@ -471,7 +472,7 @@ def delete(self): def _list_namespaces(self): self._check_self() - for d in os.listdir("%s/namespaces/" % self._path): + for d in os.listdir(f"{self._path}/namespaces/"): yield Namespace(self, int(d), 'lookup') namespaces = property(_list_namespaces, @@ -486,7 +487,7 @@ def _get_passthru(self): def _list_allowed_hosts(self): return [os.path.basename(name) - for name in os.listdir("%s/allowed_hosts/" % self._path)] + for name in os.listdir(f"{self._path}/allowed_hosts/")] allowed_hosts = property(_list_allowed_hosts, doc="Get the list of Allowed Hosts for the " @@ -497,19 +498,19 @@ def add_allowed_host(self, nqn): Enable access for the host identified by I{nqn} to the Subsystem ''' try: - os.symlink("%s/hosts/%s" % (self.configfs_dir, nqn), - "%s/allowed_hosts/%s" % (self._path, nqn)) + os.symlink(f"{self.configfs_dir}/hosts/{nqn}", + f"{self._path}/allowed_hosts/{nqn}") except Exception as e: - raise CFSError("Could not symlink %s in configFS: %s" % (nqn, e)) + raise CFSError(f"Could not symlink {nqn} in configFS: {e}") from e def remove_allowed_host(self, nqn): ''' Disable access for the host identified by I{nqn} to the Subsystem ''' try: - os.unlink("%s/allowed_hosts/%s" % (self._path, nqn)) + os.unlink(f"{self._path}/allowed_hosts/{nqn}") except Exception as e: - raise CFSError("Could not unlink %s in configFS: %s" % (nqn, e)) + raise CFSError(f"Could not unlink {nqn} in configFS: {e}") from e def has_passthru(self): return os.path.isdir(os.path.join(self.path, "passthru")) @@ -529,7 +530,7 @@ def setup(cls, t, err_func): try: s = Subsystem(t['nqn']) except CFSError as e: - err_func("Could not create Subsystem object: %s" % e) + err_func(f"Could not create Subsystem object: {e}") return for ns in t.get('namespaces', []): @@ -560,7 +561,7 @@ class Namespace(CFSNode): MAX_NSID = 8192 def __repr__(self): - return "" % self.nsid + return f"" def __init__(self, subsystem, nsid=None, mode='any'): ''' @@ -591,16 +592,16 @@ def __init__(self, subsystem, nsid=None, mode='any'): nsid = index break if nsid is None: - raise CFSError("All NSIDs 1-%d in use" % self.MAX_NSID) + raise CFSError(f"All NSIDs 1-{self.MAX_NSID} in use") else: nsid = int(nsid) if nsid < 1 or nsid > self.MAX_NSID: - raise CFSError("NSID must be 1 to %d" % self.MAX_NSID) + raise CFSError(f"NSID must be 1 to {self.MAX_NSID}") self.attr_groups = ['device', 'ana', 'resv'] self._subsystem = subsystem self._nsid = nsid - self._path = "%s/namespaces/%d" % (self.subsystem.path, self.nsid) + self._path = f"{self.subsystem.path}/namespaces/{self.nsid}" self._create_in_cfs(mode) def _get_subsystem(self): @@ -612,7 +613,7 @@ def _get_nsid(self): def _get_grpid(self): self._check_self() _grpid = 0 - path = "%s/ana_grpid" % self.path + path = f"{self.path}/ana_grpid" if os.path.isfile(path): with open(path, 'r') as file_fd: _grpid = int(file_fd.read().strip()) @@ -620,7 +621,7 @@ def _get_grpid(self): def set_grpid(self, grpid): self._check_self() - path = "%s/ana_grpid" % self.path + path = f"{self.path}/ana_grpid" if os.path.isfile(path): with open(path, 'w') as file_fd: file_fd.write(str(grpid)) @@ -646,7 +647,7 @@ def setup(cls, subsys, n, err_func): try: ns = Namespace(subsys, n['nsid']) except CFSError as e: - err_func("Could not create Namespace object: %s" % e) + err_func(f"Could not create Namespace object: {e}") return ns._setup_attrs(n, err_func) @@ -671,12 +672,12 @@ def __init__(self, subsystem): @return: A Passthru object. ''' super(Passthru, self).__init__() - self._path = "%s/passthru" % (subsystem.path) + self._path = f"{subsystem.path}/passthru" self.attr_groups = ['device'] def _get_clear_ids(self): self._check_self() - path = "%s/clear_ids" % self.path + path = f"{self.path}/clear_ids" _ids = 0 if os.path.isfile(path): with open(path, 'r') as file_fd: @@ -688,14 +689,14 @@ def _get_clear_ids(self): def set_clear_ids(self, clear): self._check_self() - path = "%s/clear_ids" % self.path + path = f"{self.path}/clear_ids" if os.path.isfile(path): with open(path, 'w') as file_fd: file_fd.write(str(clear)) def _get_admin_timeout(self): self._check_self() - path = "%s/admin_timeout" % self.path + path = f"{self.path}/admin_timeout" _timeout = 0 if os.path.isfile(path): with open(path, 'r') as file_fd: @@ -707,14 +708,14 @@ def _get_admin_timeout(self): def set_admin_timeout(self, timeout): self._check_self() - path = "%s/admin_timeout" % self.path + path = f"{self.path}/admin_timeout" if os.path.isfile(path): with open(path, 'w') as file_fd: file_fd.write(str(timeout)) def _get_io_timeout(self): self._check_self() - path = "%s/io_timeout" % self.path + path = f"{self.path}/io_timeout" _timeout = 0 if os.path.isfile(path): with open(path, 'r') as file_fd: @@ -726,7 +727,7 @@ def _get_io_timeout(self): def set_io_timeout(self, timeout): self._check_self() - path = "%s/io_timeout" % self.path + path = f"{self.path}/io_timeout" if os.path.isfile(path): with open(path, 'w') as file_fd: file_fd.write(str(timeout)) @@ -736,7 +737,7 @@ def setup(cls, subsys, p, err_func): try: pt = Passthru(subsys) except CFSError as e: - err_func("Could not create Passthru object: %s" % e) + err_func(f"Could not create Passthru object: {e}") return pt._setup_attrs(p, err_func) if 'clear_ids' in p: @@ -762,14 +763,14 @@ class Port(CFSNode): MAX_PORTID = 8192 def __repr__(self): - return "" % self.portid + return f"" def __init__(self, portid, mode='any'): super(Port, self).__init__() self.attr_groups = ['addr', 'param'] self._portid = int(portid) - self._path = "%s/ports/%d" % (self.configfs_dir, self._portid) + self._path = f"{self.configfs_dir}/ports/{self._portid}" self._create_in_cfs(mode) def _get_portid(self): @@ -779,7 +780,7 @@ def _get_portid(self): def _list_subsystems(self): return [os.path.basename(name) - for name in os.listdir("%s/subsystems/" % self._path)] + for name in os.listdir(f"{self._path}/subsystems/")] subsystems = property(_list_subsystems, doc="Get the list of Subsystem for this Port.") @@ -789,19 +790,19 @@ def add_subsystem(self, nqn): Enable access to the Subsystem identified by I{nqn} through this Port. ''' try: - os.symlink("%s/subsystems/%s" % (self.configfs_dir, nqn), - "%s/subsystems/%s" % (self._path, nqn)) + os.symlink(f"{self.configfs_dir}/subsystems/{nqn}", + f"{self._path}/subsystems/{nqn}") except Exception as e: - raise CFSError("Could not symlink %s in configFS: %s" % (nqn, e)) + raise CFSError(f"Could not symlink {nqn} in configFS: {e}") from e def remove_subsystem(self, nqn): ''' Disable access to the Subsystem identified by I{nqn} through this Port. ''' try: - os.unlink("%s/subsystems/%s" % (self._path, nqn)) + os.unlink(f"{self._path}/subsystems/{nqn}") except Exception as e: - raise CFSError("Could not unlink %s in configFS: %s" % (nqn, e)) + raise CFSError(f"Could not unlink {nqn} in configFS: {e}") from e def delete(self): ''' @@ -818,7 +819,7 @@ def delete(self): def _list_referrals(self): self._check_self() - for d in os.listdir("%s/referrals/" % self._path): + for d in os.listdir(f"{self._path}/referrals/"): yield Referral(self, d, 'lookup') referrals = property(_list_referrals, @@ -826,8 +827,8 @@ def _list_referrals(self): def _list_ana_groups(self): self._check_self() - if os.path.isdir("%s/ana_groups/" % self._path): - for d in os.listdir("%s/ana_groups/" % self._path): + if os.path.isdir(f"{self._path}/ana_groups/"): + for d in os.listdir(f"{self._path}/ana_groups/"): yield ANAGroup(self, int(d), 'lookup') ana_groups = property(_list_ana_groups, @@ -848,7 +849,7 @@ def setup(cls, root, n, err_func): try: port = Port(n['portid']) except CFSError as e: - err_func("Could not create Port object: %s" % e) + err_func(f"Could not create Port object: {e}") return port._setup_attrs(n, err_func) @@ -874,7 +875,7 @@ class Referral(CFSNode): ''' def __repr__(self): - return "" % self.name + return f"" def __init__(self, port, name, mode='any'): super(Referral, self).__init__() @@ -885,7 +886,7 @@ def __init__(self, port, name, mode='any'): self.attr_groups = ['addr'] self.port = port self._name = name - self._path = "%s/referrals/%s" % (self.port.path, self._name) + self._path = f"{self.port.path}/referrals/{self._name}" self._create_in_cfs(mode) def _get_name(self): @@ -908,7 +909,7 @@ def setup(cls, port, n, err_func): try: r = Referral(port, n['name']) except CFSError as e: - err_func("Could not create Referral object: %s" % e) + err_func(f"Could not create Referral object: {e}") return r._setup_attrs(n, err_func) @@ -927,12 +928,12 @@ class ANAGroup(CFSNode): MAX_GRPID = 1024 def __repr__(self): - return "" % self.grpid + return f"" def __init__(self, port, grpid, mode='any'): super(ANAGroup, self).__init__() - if not os.path.isdir("%s/ana_groups" % port.path): + if not os.path.isdir(f"{port.path}/ana_groups"): raise CFSError("ANA not supported") if grpid is None: @@ -945,17 +946,16 @@ def __init__(self, port, grpid, mode='any'): grpid = index break if grpid is None: - raise CFSError("All ANA Group IDs 1-%d in use" % self.MAX_GRPID) + raise CFSError(f"All ANA Group IDs 1-{self.MAX_GRPID} in use") else: grpid = int(grpid) if grpid < 1 or grpid > self.MAX_GRPID: - raise CFSError("GRPID %d must be 1 to %d" - % (grpid, self.MAX_GRPID)) + raise CFSError(f"GRPID {grpid} must be 1 to {self.MAX_GRPID}") self.attr_groups = ['ana'] self._port = port self._grpid = grpid - self._path = "%s/ana_groups/%d" % (self._port.path, self.grpid) + self._path = f"{self._port.path}/ana_groups/{self.grpid}" self._create_in_cfs(mode) def _get_grpid(self): @@ -978,7 +978,7 @@ def setup(cls, port, n, err_func): try: a = ANAGroup(port, n['grpid']) except CFSError as e: - err_func("Could not create ANA Group object: %s" % e) + err_func(f"Could not create ANA Group object: {e}") return a._setup_attrs(n, err_func) @@ -1001,7 +1001,7 @@ class Host(CFSNode): ''' def __repr__(self): - return "" % self.nqn + return f"" def __init__(self, nqn, mode='any'): ''' @@ -1018,7 +1018,7 @@ def __init__(self, nqn, mode='any'): super(Host, self).__init__() self.nqn = nqn - self._path = "%s/hosts/%s" % (self.configfs_dir, nqn) + self._path = f"{self.configfs_dir}/hosts/{nqn}" self._create_in_cfs(mode) @classmethod @@ -1036,7 +1036,7 @@ def setup(cls, t, err_func): try: Host(t['nqn']) except CFSError as e: - err_func("Could not create Host object: %s" % e) + err_func(f"Could not create Host object: {e}") return def dump(self): diff --git a/nvmet/test_nvmet.py b/nvmet/test_nvmet.py index ecbbf90..db20a25 100644 --- a/nvmet/test_nvmet.py +++ b/nvmet/test_nvmet.py @@ -115,8 +115,8 @@ def test_namespace(self): self.assertEqual(len(list(s.namespaces)), 0) @unittest.skipUnless(test_devices_present(), - "Devices %s not available or suitable" - % ','.join(NVMET_TEST_DEVICES)) + f"Devices {','.join(NVMET_TEST_DEVICES)} " + f"not available or suitable") def test_namespace_attrs(self): root = nvme.Root() root.clear_existing() @@ -420,8 +420,8 @@ def test_invalid_input(self): portid=1 << 17, mode='create') @unittest.skipUnless(test_devices_present(), - "Devices %s not available or suitable" % ','.join( - NVMET_TEST_DEVICES)) + f"Devices {','.join(NVMET_TEST_DEVICES)} not " + f"available or suitable") def test_save_restore(self): root = nvme.Root() root.clear_existing() diff --git a/nvmetcli b/nvmetcli index bec4897..9151d18 100755 --- a/nvmetcli +++ b/nvmetcli @@ -44,10 +44,10 @@ class UINode(configshell.ConfigNode): self.refresh() def _init_group(self, group): - setattr(self.__class__, "ui_getgroup_%s" % group, + setattr(self.__class__, f"ui_getgroup_{group}", lambda self, attr: self.cfnode.get_attr(group, attr)) - setattr(self.__class__, "ui_setgroup_%s" % group, + setattr(self.__class__, f"ui_setgroup_{group}", lambda self, attr, value: self.cfnode.set_attr(group, attr, value)) @@ -56,7 +56,7 @@ class UINode(configshell.ConfigNode): for attr in attrs: writable = attr not in attrs_ro - name = "ui_desc_%s" % group + name = f"ui_desc_{group}" t, d = getattr(self.__class__, name, {}).get(attr, ('string', '')) self.define_config_group_param(group, attr, t, d, writable) @@ -80,7 +80,7 @@ class UINode(configshell.ConfigNode): ======== B{ls} ''' - self.shell.log.info("Status for %s: %s" % (self.path, self.status())) + self.shell.log.info(f"Status for {self.path}: {self.status()}") def ui_command_saveconfig(self, savefile=None): ''' @@ -105,7 +105,7 @@ class UIRootNode(UINode): def summary(self): info = [] try: - info.append("discovery=" + self.cfnode.get_attr("discovery", "nqn")) + info.append(f"discovery={self.cfnode.get_attr('discovery', 'nqn')}") except nvme.nvme.CFSError: pass return (", ".join(info), True) @@ -125,8 +125,8 @@ class UIRootNode(UINode): if errors: raise configshell.ExecutionError( - "Configuration restored, %d errors:\n%s" % - (len(errors), "\n".join(errors))) + "Configuration restored, {} errors:\n{}".format( + len(errors), "\n".join(errors))) class UISubsystemsNode(UINode): @@ -183,10 +183,10 @@ class UISubsystemNode(UINode): def summary(self): info = [] - info.append("version=" + self.cfnode.get_attr("attr", "version")) - info.append("allow_any=" + - self.cfnode.get_attr("attr", "allow_any_host")) - info.append("serial=" + self.cfnode.get_attr("attr", "serial")) + info.append(f"version={self.cfnode.get_attr('attr', 'version')}") + info.append(f"allow_any=" + f"{self.cfnode.get_attr('attr', 'allow_any_host')}") + info.append(f"serial={self.cfnode.get_attr('attr', 'serial')}") return (", ".join(info), True) @@ -371,8 +371,8 @@ class UINamespaceNode(UINode): if uuid.UUID(ns_uuid).int != 0: info.append("uuid=" + str(ns_uuid)) ns_nguid = self.cfnode.get_attr("device", "nguid") - if ngiud_set(ns_nguid): - info.append("nguid=" + ns_nguid) + if nguid_set(ns_nguid): + info.append(f"nguid={ns_nguid}") if self.cfnode.grpid != 0: info.append("grpid=" + str(self.cfnode.grpid)) try: @@ -412,7 +412,7 @@ class UIAllowedHostsNode(UINode): completions.append(host.cfnode.nqn) if len(completions) == 1: - return [completions[0] + ' '] + return [f"{completions[0]} "] else: return completions @@ -435,7 +435,7 @@ class UIAllowedHostsNode(UINode): completions.append(nqn) if len(completions) == 1: - return [completions[0] + ' '] + return [f"{completions[0]} "] else: return completions @@ -505,11 +505,11 @@ class UIPortNode(UINode): def summary(self): info = [] - info.append("trtype=" + self.cfnode.get_attr("addr", "trtype")) - info.append("traddr=" + self.cfnode.get_attr("addr", "traddr")) + info.append(f"trtype={self.cfnode.get_attr('addr', 'trtype')}") + info.append(f"traddr={self.cfnode.get_attr('addr', 'traddr')}") trsvcid = self.cfnode.get_attr("addr", "trsvcid") if trsvcid != "none": - info.append("trsvcid=%s" % trsvcid) + info.append(f"trsvcid={trsvcid}") ''' Support older target driver w/o the inline_data_size parameter @@ -519,7 +519,7 @@ class UIPortNode(UINode): except Exception: inline_data_size = "n/a" if inline_data_size != "n/a": - info.append("inline_data_size=" + inline_data_size) + info.append(f"inline_data_size={inline_data_size}") enabled = self.cfnode.subsystems or list(self.cfnode.referrals) return (", ".join(info), True if enabled else 0) @@ -552,7 +552,7 @@ class UIPortSubsystemsNode(UINode): completions.append(subsys.cfnode.nqn) if len(completions) == 1: - return [completions[0] + ' '] + return [f"{completions[0]} "] else: return completions @@ -575,7 +575,7 @@ class UIPortSubsystemsNode(UINode): completions.append(nqn) if len(completions) == 1: - return [completions[0] + ' '] + return [f"{completions[0]} "] else: return completions @@ -717,7 +717,7 @@ class UIANAGroupNode(UINode): def summary(self): info = [] - info.append("state=" + self.cfnode.get_attr("ana", "state")) + info.append(f"state={self.cfnode.get_attr('ana', 'state')}") return (", ".join(info), True) @@ -761,11 +761,14 @@ class UIHostNode(UINode): def usage(): - print("syntax: %s save [file_to_save_to]" % sys.argv[0]) - print(" %s restore [file_to_restore_from]" % sys.argv[0]) - print(" %s clear" % sys.argv[0]) - print(" %s --help (Print this information)" % sys.argv[0]) - print(" %s (Run shell command and exit)" % sys.argv[0]) + ''' + Prints the usage message. + ''' + print(f"syntax: {sys.argv[0]} save [file_to_save_to]") + print(f" {sys.argv[0]} restore [file_to_restore_from]") + print(f" {sys.argv[0]} clear") + print(f" {sys.argv[0]} --help (Print this information)") + print(f" {sys.argv[0]} (Run shell command and exit)") sys.exit(-1) @@ -784,11 +787,11 @@ def restore(from_file): if e.errno == errno.ENOENT: # Not an error if the restore file is not present - print("No saved config file at %s, ok, exiting" % from_file) + print(f"No saved config file at {from_file}, ok, exiting") sys.exit(0) else: - print("Error processing config file at %s, error %s, exiting" % - (from_file, str(e))) + print(f"Error processing config file at {from_file}, error {e}" + f", exiting") sys.exit(1) # These errors are non-fatal @@ -814,7 +817,7 @@ funcs = dict(save=save, restore=restore, clear=clear) def main(): if os.geteuid() != 0: - print("%s: must run as root." % sys.argv[0], file=sys.stderr) + print(f"{sys.argv[0]}: must run as root.", file=sys.stderr) sys.exit(-1) if len(sys.argv) > 1: From 8b0c4b5bf75cc2583f0ca253a62ec090665c7e3c Mon Sep 17 00:00:00 2001 From: Ales Novak Date: Wed, 8 Jul 2026 23:44:54 +0200 Subject: [PATCH 03/13] nvmet, nvmetcli: add docstrings to modules, classes and methods Resolves pylint complaints C0114 C0115, C0116. Signed-off-by: Ales Novak Signed-off-by: Daniel Wagner --- nvmet/nvme.py | 124 ++++++++++++++++++++++++++++++++++- nvmet/test_nvmet.py | 42 ++++++++++++ nvmetcli | 156 ++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 317 insertions(+), 5 deletions(-) diff --git a/nvmet/nvme.py b/nvmet/nvme.py index 4baec63..684ad74 100644 --- a/nvmet/nvme.py +++ b/nvmet/nvme.py @@ -46,7 +46,11 @@ class CFSNotFound(CFSError): pass -class CFSNode(object): +class CFSNode: + ''' + A node in the configfs filesystem. + This is the base class for all other objects. + ''' configfs_dir = '/sys/kernel/config/nvmet' @@ -56,12 +60,21 @@ def __init__(self): self.attr_groups = [] def __eq__(self, other): + ''' + Checks if two CFSNode objects are equal. + ''' return self._path == other._path def __ne__(self, other): + ''' + Checks if two CFSNode objects are not equal. + ''' return self._path != other._path def _get_path(self): + ''' + Returns the path of the CFSNode. + ''' return self._path def _create_in_cfs(self, mode): @@ -90,9 +103,15 @@ def _create_in_cfs(self, mode): self.get_enable() def _exists(self): + ''' + Returns True if the CFSNode exists, False otherwise. + ''' return os.path.isdir(self.path) def _check_self(self): + ''' + Checks if the CFSNode exists. + ''' if not self.exists: raise CFSNotFound(f"This {self.__class__.__name__} does not " f"exist in configFS") @@ -170,6 +189,9 @@ def get_attr(self, group, attribute): return file_fd.read().strip() def get_enable(self): + ''' + Returns the value of the 'enable' attribute. + ''' self._check_self() path = f"{self.path}/enable" if not os.path.isfile(path): @@ -180,6 +202,9 @@ def get_enable(self): return self._enable def set_enable(self, value): + ''' + Sets the value of the 'enable' attribute. + ''' self._check_self() path = f"{self.path}/enable" @@ -211,6 +236,9 @@ def delete(self): + " any other means, it will be False.") def dump(self): + ''' + Returns a dict with the config of the object. + ''' d = {} for group in self.attr_groups: a = {} @@ -222,6 +250,9 @@ def dump(self): return d def _setup_attrs(self, attr_dict, err_func): + ''' + Set up attributes from a dict. + ''' for group in self.attr_groups: for name, value in attr_dict.get(group, {}).items(): try: @@ -234,6 +265,9 @@ def _setup_attrs(self, attr_dict, err_func): class Root(CFSNode): + ''' + The root of the NVMe target configfs hierarchy. + ''' def __init__(self): super(Root, self).__init__() @@ -453,6 +487,9 @@ def __init__(self, nqn=None, mode='any'): self._create_in_cfs(mode) def _generate_nqn(self): + ''' + Generates a new NQN. + ''' prefix = "nqn.2014-08.org.nvmexpress:NVMf:uuid" name = str(uuid.uuid4()) return f"{prefix}:{name}" @@ -471,6 +508,9 @@ def delete(self): super(Subsystem, self).delete() def _list_namespaces(self): + ''' + Lists the namespaces of the subsystem. + ''' self._check_self() for d in os.listdir(f"{self._path}/namespaces/"): yield Namespace(self, int(d), 'lookup') @@ -479,6 +519,9 @@ def _list_namespaces(self): doc="Get the list of Namespaces for the Subsystem.") def _get_passthru(self): + ''' + Returns the passthru object of the subsystem. + ''' self._check_self() return Passthru(self) @@ -486,6 +529,9 @@ def _get_passthru(self): doc="Get the passthru node for the subsystem") def _list_allowed_hosts(self): + ''' + Lists the allowed hosts of the subsystem. + ''' return [os.path.basename(name) for name in os.listdir(f"{self._path}/allowed_hosts/")] @@ -513,6 +559,9 @@ def remove_allowed_host(self, nqn): raise CFSError(f"Could not unlink {nqn} in configFS: {e}") from e def has_passthru(self): + ''' + Check if the subsystem has a passthru node. + ''' return os.path.isdir(os.path.join(self.path, "passthru")) @classmethod @@ -605,12 +654,21 @@ def __init__(self, subsystem, nsid=None, mode='any'): self._create_in_cfs(mode) def _get_subsystem(self): + ''' + Returns the parent subsystem. + ''' return self._subsystem def _get_nsid(self): + ''' + Returns the namespace ID. + ''' return self._nsid def _get_grpid(self): + ''' + Returns the ANA group ID. + ''' self._check_self() _grpid = 0 path = f"{self.path}/ana_grpid" @@ -620,6 +678,9 @@ def _get_grpid(self): return _grpid def set_grpid(self, grpid): + ''' + Sets the ANA group ID. + ''' self._check_self() path = f"{self.path}/ana_grpid" if os.path.isfile(path): @@ -655,6 +716,9 @@ def setup(cls, subsys, n, err_func): ns.set_grpid(int(n['ana_grpid'])) def dump(self): + ''' + Returns a dict with the config of the object. + ''' d = super(Namespace, self).dump() d['nsid'] = self.nsid d['ana_grpid'] = self.grpid @@ -664,6 +728,7 @@ def dump(self): class Passthru(CFSNode): ''' This is an interface to a NVMe passthru in ConfigFS. + A Passthru is identified by its parent Subsystem. ''' def __init__(self, subsystem): @@ -676,6 +741,9 @@ def __init__(self, subsystem): self.attr_groups = ['device'] def _get_clear_ids(self): + ''' + Get the passthru namespace clear_ids attribute. + ''' self._check_self() path = f"{self.path}/clear_ids" _ids = 0 @@ -688,6 +756,9 @@ def _get_clear_ids(self): doc="Get the passthru namespace clear_ids attribute.") def set_clear_ids(self, clear): + ''' + Set the passthru namespace clear_ids attribute. + ''' self._check_self() path = f"{self.path}/clear_ids" if os.path.isfile(path): @@ -695,6 +766,9 @@ def set_clear_ids(self, clear): file_fd.write(str(clear)) def _get_admin_timeout(self): + ''' + Get the passthru admin command timeout. + ''' self._check_self() path = f"{self.path}/admin_timeout" _timeout = 0 @@ -707,6 +781,9 @@ def _get_admin_timeout(self): doc="Get the passthru admin command timeout.") def set_admin_timeout(self, timeout): + ''' + Set the passthru admin command timeout. + ''' self._check_self() path = f"{self.path}/admin_timeout" if os.path.isfile(path): @@ -714,6 +791,9 @@ def set_admin_timeout(self, timeout): file_fd.write(str(timeout)) def _get_io_timeout(self): + ''' + Get the passthru IO command timeout. + ''' self._check_self() path = f"{self.path}/io_timeout" _timeout = 0 @@ -726,6 +806,9 @@ def _get_io_timeout(self): doc="Get the passthru IO command timeout.") def set_io_timeout(self, timeout): + ''' + Set the passthru IO command timeout. + ''' self._check_self() path = f"{self.path}/io_timeout" if os.path.isfile(path): @@ -734,6 +817,9 @@ def set_io_timeout(self, timeout): @classmethod def setup(cls, subsys, p, err_func): + ''' + Set up a Passthru object based upon p dict, from saved config. + ''' try: pt = Passthru(subsys) except CFSError as e: @@ -748,6 +834,9 @@ def setup(cls, subsys, p, err_func): pt.set_io_timeout(int(p['io_timeout'])) def dump(self): + ''' + Returns a dict with the config of the object. + ''' d = super(Passthru, self).dump() d['clear_ids'] = self.ids d['admin_timeout'] = self.admin_timeout @@ -774,11 +863,17 @@ def __init__(self, portid, mode='any'): self._create_in_cfs(mode) def _get_portid(self): + ''' + Returns the port ID. + ''' return self._portid portid = property(_get_portid, doc="Get the Port ID as an int.") def _list_subsystems(self): + ''' + Lists the subsystems of the port. + ''' return [os.path.basename(name) for name in os.listdir(f"{self._path}/subsystems/")] @@ -818,6 +913,9 @@ def delete(self): super(Port, self).delete() def _list_referrals(self): + ''' + Lists the referrals of the port. + ''' self._check_self() for d in os.listdir(f"{self._path}/referrals/"): yield Referral(self, d, 'lookup') @@ -826,6 +924,9 @@ def _list_referrals(self): doc="Get the list of Referrals for this Port.") def _list_ana_groups(self): + ''' + Lists the ANA groups of the port. + ''' self._check_self() if os.path.isdir(f"{self._path}/ana_groups/"): for d in os.listdir(f"{self._path}/ana_groups/"): @@ -861,6 +962,9 @@ def setup(cls, root, n, err_func): Referral.setup(port, r, err_func) def dump(self): + ''' + Returns a dict with the config of the object. + ''' d = super(Port, self).dump() d['portid'] = self.portid d['subsystems'] = self.subsystems @@ -890,6 +994,9 @@ def __init__(self, port, name, mode='any'): self._create_in_cfs(mode) def _get_name(self): + ''' + Returns the name of the referral. + ''' return self._name name = property(_get_name, doc="Get the Referral name.") @@ -915,6 +1022,9 @@ def setup(cls, port, n, err_func): r._setup_attrs(n, err_func) def dump(self): + ''' + Returns a dict with the config of the object. + ''' d = super(Referral, self).dump() d['name'] = self.name return d @@ -959,6 +1069,9 @@ def __init__(self, port, grpid, mode='any'): self._create_in_cfs(mode) def _get_grpid(self): + ''' + Returns the ANA group ID. + ''' return self._grpid grpid = property(_get_grpid, doc="Get the ANA Group ID.") @@ -984,11 +1097,17 @@ def setup(cls, port, n, err_func): a._setup_attrs(n, err_func) def delete(self): + ''' + Deletes the ANA group. + ''' # ANA Group 1 is automatically created/deleted if self.grpid != 1: super(ANAGroup, self).delete() def dump(self): + ''' + Returns a dict with the config of the object. + ''' d = super(ANAGroup, self).dump() d['grpid'] = self.grpid return d @@ -1040,6 +1159,9 @@ def setup(cls, t, err_func): return def dump(self): + ''' + Returns a dict with the config of the object. + ''' d = super(Host, self).dump() d['nqn'] = self.nqn return d diff --git a/nvmet/test_nvmet.py b/nvmet/test_nvmet.py index db20a25..1f14b39 100644 --- a/nvmet/test_nvmet.py +++ b/nvmet/test_nvmet.py @@ -1,3 +1,6 @@ +""" +Tests for the nvmet API. +""" import os import random @@ -13,13 +16,22 @@ def test_devices_present(): + ''' + Check if the test devices are present. + ''' return len([x for x in NVMET_TEST_DEVICES if os.path.exists(x) and (stat.S_ISBLK(os.stat(x).st_mode) or os.path.isfile(x))]) >= 2 class TestNvmet(unittest.TestCase): + ''' + Tests for the nvmet API. + ''' def test_subsystem(self): + ''' + Test Subsystem creation and deletion. + ''' root = nvme.Root() root.clear_existing() for s in root.subsystems: @@ -64,6 +76,9 @@ def test_subsystem(self): self.assertEqual(len(list(root.subsystems)), 0) def test_namespace(self): + ''' + Test Namespace creation and deletion. + ''' root = nvme.Root() root.clear_existing() @@ -118,6 +133,9 @@ def test_namespace(self): f"Devices {','.join(NVMET_TEST_DEVICES)} " f"not available or suitable") def test_namespace_attrs(self): + ''' + Test Namespace attributes. + ''' root = nvme.Root() root.clear_existing() @@ -154,6 +172,9 @@ def test_namespace_attrs(self): n.delete() def test_recursive_delete(self): + ''' + Test recursive deletion of a Subsystem. + ''' root = nvme.Root() root.clear_existing() @@ -165,6 +186,9 @@ def test_recursive_delete(self): self.assertEqual(len(list(root.subsystems)), 0) def test_port(self): + ''' + Test Port creation and deletion. + ''' root = nvme.Root() root.clear_existing() for p in root.ports: @@ -201,6 +225,9 @@ def test_port(self): self.assertEqual(len(list(root.ports)), 0) def test_loop_port(self): + ''' + Test loop port functionality. + ''' root = nvme.Root() root.clear_existing() @@ -254,6 +281,9 @@ def test_loop_port(self): p.delete() def test_host(self): + ''' + Test Host creation and deletion. + ''' root = nvme.Root() root.clear_existing() for p in root.hosts: @@ -290,6 +320,9 @@ def test_host(self): self.assertEqual(len(list(root.hosts)), 0) def test_referral(self): + ''' + Test Referral creation and deletion. + ''' root = nvme.Root() root.clear_existing() @@ -373,6 +406,9 @@ def test_referral(self): self.assertEqual(len(list(p.referrals)), 0) def test_allowed_hosts(self): + ''' + Test allowed hosts functionality. + ''' nvme.Root() nvme.Host(nqn='hostnqn', mode='create') @@ -398,6 +434,9 @@ def test_allowed_hosts(self): self.assertRaises(nvme.CFSError, s.remove_allowed_host, 'foobar') def test_invalid_input(self): + ''' + Test invalid input to the API. + ''' root = nvme.Root() root.clear_existing() @@ -423,6 +462,9 @@ def test_invalid_input(self): f"Devices {','.join(NVMET_TEST_DEVICES)} not " f"available or suitable") def test_save_restore(self): + ''' + Test save and restore functionality. + ''' root = nvme.Root() root.clear_existing() diff --git a/nvmetcli b/nvmetcli index 9151d18..023c08a 100755 --- a/nvmetcli +++ b/nvmetcli @@ -29,11 +29,17 @@ from string import hexdigits import uuid -def ngiud_set(nguid): +def nguid_set(nguid): + ''' + Check if the nguid is set. + ''' return any(c in hexdigits and c != '0' for c in nguid) class UINode(configshell.ConfigNode): + ''' + A node in the UI. + ''' def __init__(self, name, parent=None, cfnode=None, shell=None): configshell.ConfigNode.__init__(self, name, parent, shell) self.cfnode = cfnode @@ -61,9 +67,15 @@ class UINode(configshell.ConfigNode): self.define_config_group_param(group, attr, t, d, writable) def refresh(self): + ''' + Refreshes and updates the objects tree from the current path. + ''' self._children = set([]) def status(self): + ''' + Displays the current node's status summary. + ''' return "None" def ui_command_refresh(self): @@ -94,6 +106,9 @@ class UINode(configshell.ConfigNode): class UIRootNode(UINode): + ''' + The root node of the UI. + ''' ui_desc_discovery = { 'nqn': ('string', 'Discovery NQN'), } @@ -103,6 +118,9 @@ class UIRootNode(UINode): shell=shell) def summary(self): + ''' + Returns a summary of the root node. + ''' info = [] try: info.append(f"discovery={self.cfnode.get_attr('discovery', 'nqn')}") @@ -111,6 +129,9 @@ class UIRootNode(UINode): return (", ".join(info), True) def refresh(self): + ''' + Refreshes and updates the objects tree from the current path. + ''' self._children = set([]) UISubsystemsNode(self) UIPortsNode(self) @@ -130,10 +151,16 @@ class UIRootNode(UINode): class UISubsystemsNode(UINode): + ''' + A node in the UI representing the subsystems. + ''' def __init__(self, parent): UINode.__init__(self, 'subsystems', parent) def refresh(self): + ''' + Refreshes and updates the objects tree from the current path. + ''' self._children = set([]) for subsys in self.parent.cfnode.subsystems: UISubsystemNode(self, subsys) @@ -165,6 +192,9 @@ class UISubsystemsNode(UINode): class UISubsystemNode(UINode): + ''' + A node in the UI representing a subsystem. + ''' ui_desc_attr = { 'allow_any_host': ('string', 'Allow access by any host if set to 1'), 'serial': ('string', 'Export serial number to hosts'), @@ -175,6 +205,9 @@ class UISubsystemNode(UINode): UINode.__init__(self, cfnode.nqn, parent, cfnode) def refresh(self): + ''' + Refreshes and updates the objects tree from the current path. + ''' self._children = set([]) UINamespacesNode(self) UIAllowedHostsNode(self) @@ -182,6 +215,9 @@ class UISubsystemNode(UINode): UIPassthruNode(self) def summary(self): + ''' + Returns a summary of the subsystem. + ''' info = [] info.append(f"version={self.cfnode.get_attr('attr', 'version')}") info.append(f"allow_any=" @@ -191,6 +227,9 @@ class UISubsystemNode(UINode): class UIPassthruNode(UINode): + ''' + A node in the UI representing a passthru. + ''' ui_desc_device = { 'path': ('string', 'Passthru device path') } @@ -200,9 +239,15 @@ class UIPassthruNode(UINode): UINode.__init__(self, 'passthru', parent, passthru) def refresh(self): + ''' + Refreshes and updates the objects tree from the current path. + ''' self._children = set([]) def ui_command_enable(self): + ''' + Enables the passthru. + ''' if self.cfnode.get_enable(): self.shell.log.info("The passthru is already enabled.") else: @@ -214,6 +259,9 @@ class UIPassthruNode(UINode): "The passthru could not be enabled.") def ui_command_disable(self): + ''' + Disables the passthru. + ''' if not self.cfnode.get_enable(): self.shell.log.info("The passthru is already disabled.") else: @@ -269,10 +317,16 @@ class UIPassthruNode(UINode): class UINamespacesNode(UINode): + ''' + A node in the UI representing the namespaces. + ''' def __init__(self, parent): UINode.__init__(self, 'namespaces', parent) def refresh(self): + ''' + Refreshes and updates the objects tree from the current path. + ''' self._children = set([]) for ns in self.parent.cfnode.namespaces: UINamespaceNode(self, ns) @@ -304,6 +358,9 @@ class UINamespacesNode(UINode): class UINamespaceNode(UINode): + ''' + A node in the UI representing a namespace. + ''' ui_desc_device = { 'path': ('string', 'Backing device path.'), 'nguid': ('string', 'Namspace Global Unique Identifier.'), @@ -314,6 +371,9 @@ class UINamespaceNode(UINode): UINode.__init__(self, str(cfnode.nsid), parent, cfnode) def status(self): + ''' + Returns the status of the namespace. + ''' if self.cfnode.get_enable(): return "enabled" return "disabled" @@ -386,10 +446,16 @@ class UINamespaceNode(UINode): class UIAllowedHostsNode(UINode): + ''' + A node in the UI representing the allowed hosts. + ''' def __init__(self, parent): UINode.__init__(self, 'allowed_hosts', parent) def refresh(self): + ''' + Refreshes and updates the objects tree from the current path. + ''' self._children = set([]) for host in self.parent.cfnode.allowed_hosts: UIAllowedHostNode(self, host) @@ -406,6 +472,9 @@ class UIAllowedHostsNode(UINode): UIAllowedHostNode(self, nqn) def ui_complete_create(self, parameters, text, current_param): + ''' + Completes the create command. + ''' completions = [] if current_param == 'nqn': for host in self.get_node('/hosts').children: @@ -429,6 +498,9 @@ class UIAllowedHostsNode(UINode): self.refresh() def ui_complete_delete(self, parameters, text, current_param): + ''' + Completes the delete command. + ''' completions = [] if current_param == 'nqn': for nqn in self.parent.cfnode.allowed_hosts: @@ -441,15 +513,24 @@ class UIAllowedHostsNode(UINode): class UIAllowedHostNode(UINode): + ''' + A node in the UI representing an allowed host. + ''' def __init__(self, parent, nqn): UINode.__init__(self, nqn, parent) class UIPortsNode(UINode): + ''' + A node in the UI representing the ports. + ''' def __init__(self, parent): UINode.__init__(self, 'ports', parent) def refresh(self): + ''' + Refreshes and updates the objects tree from the current path. + ''' self._children = set([]) for port in self.parent.cfnode.ports: UIPortNode(self, port) @@ -480,6 +561,9 @@ class UIPortsNode(UINode): class UIPortNode(UINode): + ''' + A node in the UI representing a port. + ''' ui_desc_addr = { 'adrfam': ('string', 'Address Family (e.g. ipv4 or fc)'), 'treq': ('string', 'Transport Security Requirements'), @@ -504,6 +588,9 @@ class UIPortNode(UINode): UIReferralsNode(self) def summary(self): + ''' + Returns a summary of the port. + ''' info = [] info.append(f"trtype={self.cfnode.get_attr('addr', 'trtype')}") info.append(f"traddr={self.cfnode.get_attr('addr', 'traddr')}") @@ -511,9 +598,7 @@ class UIPortNode(UINode): if trsvcid != "none": info.append(f"trsvcid={trsvcid}") - ''' - Support older target driver w/o the inline_data_size parameter - ''' + # Support older target driver w/o the inline_data_size parameter try: inline_data_size = self.cfnode.get_attr("param", "inline_data_size") except Exception: @@ -525,10 +610,16 @@ class UIPortNode(UINode): class UIPortSubsystemsNode(UINode): + ''' + A node in the UI representing the port subsystems. + ''' def __init__(self, parent): UINode.__init__(self, 'subsystems', parent) def refresh(self): + ''' + Refreshes and updates the objects tree from the current path. + ''' self._children = set([]) for host in self.parent.cfnode.subsystems: UIPortSubsystemNode(self, host) @@ -546,6 +637,9 @@ class UIPortSubsystemsNode(UINode): UIPortSubsystemNode(self, nqn) def ui_complete_create(self, parameters, text, current_param): + ''' + Completes the create command. + ''' completions = [] if current_param == 'nqn': for subsys in self.get_node('/subsystems').children: @@ -569,6 +663,9 @@ class UIPortSubsystemsNode(UINode): self.refresh() def ui_complete_delete(self, parameters, text, current_param): + ''' + Completes the delete command. + ''' completions = [] if current_param == 'nqn': for nqn in self.parent.cfnode.subsystems: @@ -581,15 +678,24 @@ class UIPortSubsystemsNode(UINode): class UIPortSubsystemNode(UINode): + ''' + A node in the UI representing a port subsystem. + ''' def __init__(self, parent, nqn): UINode.__init__(self, nqn, parent) class UIReferralsNode(UINode): + ''' + A node in the UI representing the referrals. + ''' def __init__(self, parent): UINode.__init__(self, 'referrals', parent) def refresh(self): + ''' + Refreshes and updates the objects tree from the current path. + ''' self._children = set([]) for r in self.parent.cfnode.referrals: UIReferralNode(self, r) @@ -619,6 +725,9 @@ class UIReferralsNode(UINode): class UIReferralNode(UINode): + ''' + A node in the UI representing a referral. + ''' ui_desc_addr = { 'adrfam': ('string', 'Address Family (e.g. ipv4 or fc)'), 'treq': ('string', 'Transport Security Requirements'), @@ -633,6 +742,9 @@ class UIReferralNode(UINode): UINode.__init__(self, cfnode.name, parent, cfnode) def status(self): + ''' + Returns the status of the referral. + ''' if self.cfnode.get_enable(): return "enabled" return "disabled" @@ -675,10 +787,16 @@ class UIReferralNode(UINode): class UIANAGroupsNode(UINode): + ''' + A node in the UI representing the ANA groups. + ''' def __init__(self, parent): UINode.__init__(self, 'ana_groups', parent) def refresh(self): + ''' + Refreshes and updates the objects tree from the current path. + ''' self._children = set([]) for a in self.parent.cfnode.ana_groups: UIANAGroupNode(self, a) @@ -708,6 +826,9 @@ class UIANAGroupsNode(UINode): class UIANAGroupNode(UINode): + ''' + A node in the UI representing an ANA group. + ''' ui_desc_ana = { 'state': ('string', 'ANA state'), } @@ -716,16 +837,25 @@ class UIANAGroupNode(UINode): UINode.__init__(self, str(cfnode.grpid), parent, cfnode) def summary(self): + ''' + Returns a summary of the ANA group. + ''' info = [] info.append(f"state={self.cfnode.get_attr('ana', 'state')}") return (", ".join(info), True) class UIHostsNode(UINode): + ''' + A node in the UI representing the hosts. + ''' def __init__(self, parent): UINode.__init__(self, 'hosts', parent) def refresh(self): + ''' + Refreshes and updates the objects tree from the current path. + ''' self._children = set([]) for host in self.parent.cfnode.hosts: UIHostNode(self, host) @@ -756,6 +886,9 @@ class UIHostsNode(UINode): class UIHostNode(UINode): + ''' + A node in the UI representing a host. + ''' def __init__(self, parent, cfnode): UINode.__init__(self, cfnode.nqn, parent, cfnode) @@ -773,10 +906,16 @@ def usage(): def save(to_file): + ''' + Saves the configuration to a file. + ''' nvme.Root().save_to_file(to_file) def restore(from_file): + ''' + Restores the configuration from a file. + ''' errors = None try: @@ -802,10 +941,16 @@ def restore(from_file): def clear(unused): + ''' + Clears the configuration. + ''' nvme.Root().clear_existing() def execute_cmd(cmd): + ''' + Executes a command in the shell. + ''' shell = configshell.ConfigShell('~/.nvmetcli') UIRootNode(shell) shell.run_cmdline(cmd) @@ -816,6 +961,9 @@ funcs = dict(save=save, restore=restore, clear=clear) def main(): + ''' + The main function. + ''' if os.geteuid() != 0: print(f"{sys.argv[0]}: must run as root.", file=sys.stderr) sys.exit(-1) From 1b77d99a7977fefc76b9b5650f96b5b5649e1805 Mon Sep 17 00:00:00 2001 From: Ales Novak Date: Wed, 8 Jul 2026 23:48:03 +0200 Subject: [PATCH 04/13] nvme.py: call open() with explicitly specified utf-8 We should not assume that the locale is UTF-8, set it explicitely. Resolves pylint complaint W1514. Signed-off-by: Ales Novak Signed-off-by: Daniel Wagner --- nvmet/nvme.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/nvmet/nvme.py b/nvmet/nvme.py index 684ad74..213dacf 100644 --- a/nvmet/nvme.py +++ b/nvmet/nvme.py @@ -168,7 +168,7 @@ def set_attr(self, group, attribute, value): f"{self.__class__.__name__} is enabled") try: - with open(path, 'w') as file_fd: + with open(path, 'w', encoding="utf-8") as file_fd: file_fd.write(str(value)) except Exception as e: raise CFSError(f"Cannot set attribute {path}: {e}") from e @@ -185,7 +185,7 @@ def get_attr(self, group, attribute): if not os.path.isfile(path): raise CFSError(f"Cannot find attribute: {path}") - with open(path, 'r') as file_fd: + with open(path, 'r', encoding="utf-8") as file_fd: return file_fd.read().strip() def get_enable(self): @@ -197,7 +197,7 @@ def get_enable(self): if not os.path.isfile(path): return None - with open(path, 'r') as file_fd: + with open(path, 'r', encoding="utf-8") as file_fd: self._enable = int(file_fd.read().strip()) return self._enable @@ -212,7 +212,7 @@ def set_enable(self, value): raise CFSError(f"Cannot enable {self.path}") try: - with open(path, 'w') as file_fd: + with open(path, 'w', encoding="utf-8") as file_fd: file_fd.write(str(value)) except Exception as e: raise CFSError(f"Cannot enable {self.path}: {e} ({value})") from e @@ -351,7 +351,7 @@ def save_to_file(self, savefile=None): if not os.path.exists(savefile_dir): os.makedirs(savefile_dir) - with open(savefile + ".temp", "w+") as f: + with open(savefile + ".temp", "w+", encoding="utf-8") as f: os.fchmod(f.fileno(), stat.S_IRUSR | stat.S_IWUSR) f.write(json.dumps(self.dump(), sort_keys=True, indent=2)) f.write("\n") @@ -439,7 +439,7 @@ def restore_from_file(self, savefile=None, clear_existing=True, else: savefile = DEFAULT_SAVE_FILE - with open(savefile, "r") as f: + with open(savefile, "r", encoding="utf-8") as f: config = json.loads(f.read()) return self.restore(config, clear_existing=clear_existing, abort_on_error=abort_on_error) @@ -673,7 +673,7 @@ def _get_grpid(self): _grpid = 0 path = f"{self.path}/ana_grpid" if os.path.isfile(path): - with open(path, 'r') as file_fd: + with open(path, 'r', encoding="utf-8") as file_fd: _grpid = int(file_fd.read().strip()) return _grpid @@ -684,7 +684,7 @@ def set_grpid(self, grpid): self._check_self() path = f"{self.path}/ana_grpid" if os.path.isfile(path): - with open(path, 'w') as file_fd: + with open(path, 'w', encoding="utf-8") as file_fd: file_fd.write(str(grpid)) grpid = property(_get_grpid, doc="Get the ANA Group ID.") @@ -748,7 +748,7 @@ def _get_clear_ids(self): path = f"{self.path}/clear_ids" _ids = 0 if os.path.isfile(path): - with open(path, 'r') as file_fd: + with open(path, 'r', encoding="utf-8") as file_fd: _ids = int(file_fd.read().strip()) return _ids @@ -762,7 +762,7 @@ def set_clear_ids(self, clear): self._check_self() path = f"{self.path}/clear_ids" if os.path.isfile(path): - with open(path, 'w') as file_fd: + with open(path, 'w', encoding="utf-8") as file_fd: file_fd.write(str(clear)) def _get_admin_timeout(self): @@ -773,7 +773,7 @@ def _get_admin_timeout(self): path = f"{self.path}/admin_timeout" _timeout = 0 if os.path.isfile(path): - with open(path, 'r') as file_fd: + with open(path, 'r', encoding="utf-8") as file_fd: _timeout = int(file_fd.read().strip()) return _timeout @@ -787,7 +787,7 @@ def set_admin_timeout(self, timeout): self._check_self() path = f"{self.path}/admin_timeout" if os.path.isfile(path): - with open(path, 'w') as file_fd: + with open(path, 'w', encoding="utf-8") as file_fd: file_fd.write(str(timeout)) def _get_io_timeout(self): @@ -798,7 +798,7 @@ def _get_io_timeout(self): path = f"{self.path}/io_timeout" _timeout = 0 if os.path.isfile(path): - with open(path, 'r') as file_fd: + with open(path, 'r', encoding="utf-8") as file_fd: _timeout = int(file_fd.read().strip()) return _timeout @@ -812,7 +812,7 @@ def set_io_timeout(self, timeout): self._check_self() path = f"{self.path}/io_timeout" if os.path.isfile(path): - with open(path, 'w') as file_fd: + with open(path, 'w', encoding="utf-8") as file_fd: file_fd.write(str(timeout)) @classmethod From 7478f670bb3028964b6ed839f6c704185bfd88d2 Mon Sep 17 00:00:00 2001 From: Ales Novak Date: Wed, 8 Jul 2026 23:50:59 +0200 Subject: [PATCH 05/13] nvme.py: use python3 super() without arguments Python3 changed the default arguments of super(), switch to that. Resolves python complaint R1725. Signed-off-by: Ales Novak Signed-off-by: Daniel Wagner --- nvmet/nvme.py | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/nvmet/nvme.py b/nvmet/nvme.py index 213dacf..14ef16b 100644 --- a/nvmet/nvme.py +++ b/nvmet/nvme.py @@ -269,7 +269,7 @@ class Root(CFSNode): The root of the NVMe target configfs hierarchy. ''' def __init__(self): - super(Root, self).__init__() + super().__init__() self.attr_groups = ['discovery'] if not os.path.isdir(self.configfs_dir): @@ -445,7 +445,7 @@ def restore_from_file(self, savefile=None, clear_existing=True, abort_on_error=abort_on_error) def dump(self): - d = super(Root, self).dump() + d = super().dump() d['subsystems'] = [s.dump() for s in self.subsystems] d['ports'] = [p.dump() for p in self.ports] d['hosts'] = [h.dump() for h in self.hosts] @@ -474,7 +474,7 @@ def __init__(self, nqn=None, mode='any'): @type mode:string @return: A Subsystem object. ''' - super(Subsystem, self).__init__() + super().__init__() if nqn is None: if mode == 'lookup': @@ -505,7 +505,7 @@ def delete(self): ns.delete() for h in self.allowed_hosts: self.remove_allowed_host(h) - super(Subsystem, self).delete() + super().delete() def _list_namespaces(self): ''' @@ -592,7 +592,7 @@ def setup(cls, t, err_func): s._setup_attrs(t, err_func) def dump(self): - d = super(Subsystem, self).dump() + d = super().dump() d['nqn'] = self.nqn d['namespaces'] = [ns.dump() for ns in self.namespaces] d['allowed_hosts'] = self.allowed_hosts @@ -626,7 +626,7 @@ def __init__(self, subsystem, nsid=None, mode='any'): @type mode:string @return: A Namespace object. ''' - super(Namespace, self).__init__() + super().__init__() if not isinstance(subsystem, Subsystem): raise CFSError("Invalid parent class") @@ -719,7 +719,7 @@ def dump(self): ''' Returns a dict with the config of the object. ''' - d = super(Namespace, self).dump() + d = super().dump() d['nsid'] = self.nsid d['ana_grpid'] = self.grpid return d @@ -736,7 +736,7 @@ def __init__(self, subsystem): @param subsystem: The parent Subsystem object. @return: A Passthru object. ''' - super(Passthru, self).__init__() + super().__init__() self._path = f"{subsystem.path}/passthru" self.attr_groups = ['device'] @@ -837,7 +837,7 @@ def dump(self): ''' Returns a dict with the config of the object. ''' - d = super(Passthru, self).dump() + d = super().dump() d['clear_ids'] = self.ids d['admin_timeout'] = self.admin_timeout d['io_timeout'] = self.io_timeout @@ -855,7 +855,7 @@ def __repr__(self): return f"" def __init__(self, portid, mode='any'): - super(Port, self).__init__() + super().__init__() self.attr_groups = ['addr', 'param'] self._portid = int(portid) @@ -910,7 +910,7 @@ def delete(self): a.delete() for r in self.referrals: r.delete() - super(Port, self).delete() + super().delete() def _list_referrals(self): ''' @@ -965,7 +965,7 @@ def dump(self): ''' Returns a dict with the config of the object. ''' - d = super(Port, self).dump() + d = super().dump() d['portid'] = self.portid d['subsystems'] = self.subsystems d['ana_groups'] = [a.dump() for a in self.ana_groups] @@ -982,7 +982,7 @@ def __repr__(self): return f"" def __init__(self, port, name, mode='any'): - super(Referral, self).__init__() + super().__init__() if not isinstance(port, Port): raise CFSError("Invalid parent class") @@ -1025,7 +1025,7 @@ def dump(self): ''' Returns a dict with the config of the object. ''' - d = super(Referral, self).dump() + d = super().dump() d['name'] = self.name return d @@ -1041,7 +1041,7 @@ def __repr__(self): return f"" def __init__(self, port, grpid, mode='any'): - super(ANAGroup, self).__init__() + super().__init__() if not os.path.isdir(f"{port.path}/ana_groups"): raise CFSError("ANA not supported") @@ -1102,13 +1102,13 @@ def delete(self): ''' # ANA Group 1 is automatically created/deleted if self.grpid != 1: - super(ANAGroup, self).delete() + super().delete() def dump(self): ''' Returns a dict with the config of the object. ''' - d = super(ANAGroup, self).dump() + d = super().dump() d['grpid'] = self.grpid return d @@ -1134,7 +1134,7 @@ def __init__(self, nqn, mode='any'): @type mode:string @return: A Host object. ''' - super(Host, self).__init__() + super().__init__() self.nqn = nqn self._path = f"{self.configfs_dir}/hosts/{nqn}" @@ -1162,7 +1162,7 @@ def dump(self): ''' Returns a dict with the config of the object. ''' - d = super(Host, self).dump() + d = super().dump() d['nqn'] = self.nqn return d From 92f9956a89efd3e460f99d6efe1a0ad129c51b7f Mon Sep 17 00:00:00 2001 From: Ales Novak Date: Wed, 8 Jul 2026 23:58:13 +0200 Subject: [PATCH 06/13] test_nvmet.py: redundant assertTrue -> fail assertTrue(False, ...) is redundant, replace it with fail. Resolves pylint complaint W1503. Signed-off-by: Ales Novak Signed-off-by: Daniel Wagner --- nvmet/test_nvmet.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/nvmet/test_nvmet.py b/nvmet/test_nvmet.py index 1f14b39..d732d17 100644 --- a/nvmet/test_nvmet.py +++ b/nvmet/test_nvmet.py @@ -34,8 +34,8 @@ def test_subsystem(self): ''' root = nvme.Root() root.clear_existing() - for s in root.subsystems: - self.assertTrue(False, 'Found Subsystem after clear') + for _ in root.subsystems: + self.fail('Found Subsystem after clear') # create mode s1 = nvme.Subsystem(nqn='testnqn1', mode='create') @@ -83,8 +83,8 @@ def test_namespace(self): root.clear_existing() s = nvme.Subsystem(nqn='testnqn', mode='create') - for n in s.namespaces: - self.assertTrue(False, 'Found Namespace in new Subsystem') + for _ in s.namespaces: + self.fail('Found Namespace in new Subsystem') # create mode n1 = nvme.Namespace(s, nsid=3, mode='create') @@ -191,8 +191,8 @@ def test_port(self): ''' root = nvme.Root() root.clear_existing() - for p in root.ports: - self.assertTrue(False, 'Found Port after clear') + for _ in root.ports: + self.fail('Found Port after clear') # create mode p1 = nvme.Port(portid=0, mode='create') @@ -286,8 +286,8 @@ def test_host(self): ''' root = nvme.Root() root.clear_existing() - for p in root.hosts: - self.assertTrue(False, 'Found Host after clear') + for _ in root.hosts: + self.fail('Found Host after clear') # create mode h1 = nvme.Host(nqn='foo', mode='create') From 97474fbadb19c3e746fc5496b6733ae34898c8b7 Mon Sep 17 00:00:00 2001 From: Ales Novak Date: Thu, 9 Jul 2026 00:01:38 +0200 Subject: [PATCH 07/13] nvmetcli: re-raise exceptions with "from" Re-raise exceptions with "from" so that the original exception is preserved. Resolves pylint complaint W0707. Signed-off-by: Ales Novak Signed-off-by: Daniel Wagner --- nvmetcli | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/nvmetcli b/nvmetcli index 023c08a..8fc9098 100755 --- a/nvmetcli +++ b/nvmetcli @@ -254,9 +254,9 @@ class UIPassthruNode(UINode): try: self.cfnode.set_enable(1) self.shell.log.info("The passthru has been enabled.") - except Exception: + except Exception as exc: raise configshell.ExecutionError( - "The passthru could not be enabled.") + "The passthru could not be enabled.") from exc def ui_command_disable(self): ''' @@ -268,9 +268,9 @@ class UIPassthruNode(UINode): try: self.cfnode.set_enable(0) self.shell.log.info("The passthru has been disabled.") - except Exception: + except Exception as exc: raise configshell.ExecutionError( - "The passthru could not be disabled.") + "The passthru could not be disabled.") from exc def ui_command_clear_ids(self, clear): ''' @@ -279,9 +279,9 @@ class UIPassthruNode(UINode): ''' try: self.cfnode.set_clear_ids(clear) - except Exception: + except Exception as exc: raise configshell.ExecutionError( - "Failed to set clear_ids for this passthru target.") + "Failed to set clear_ids for this passthru target.") from exc def ui_command_admin_timeout(self, timeout): ''' @@ -289,9 +289,9 @@ class UIPassthruNode(UINode): ''' try: self.cfnode.set_admin_timeout(timeout) - except Exception: + except Exception as exc: raise configshell.ExecutionError( - "Failed to set the admin passthru command timeout.") + "Failed to set the admin passthru command timeout.") from exc def ui_command_io_timeout(self, timeout): ''' @@ -299,9 +299,9 @@ class UIPassthruNode(UINode): ''' try: self.cfnode.set_io_timeout(timeout) - except Exception: + except Exception as exc: raise configshell.ExecutionError( - "Failed to set the IO passthru command timeout.") + "Failed to set the IO passthru command timeout.") from exc def summary(self): info = [] @@ -392,9 +392,9 @@ class UINamespaceNode(UINode): try: self.cfnode.set_enable(1) self.shell.log.info("The Namespace has been enabled.") - except Exception: + except Exception as exc: raise configshell.ExecutionError( - "The Namespace could not be enabled.") + "The Namespace could not be enabled.") from exc def ui_command_disable(self): ''' @@ -410,9 +410,9 @@ class UINamespaceNode(UINode): try: self.cfnode.set_enable(0) self.shell.log.info("The Namespace has been disabled.") - except Exception: + except Exception as exc: raise configshell.ExecutionError( - "The Namespace could not be disabled.") + "The Namespace could not be enabled.") from exc def ui_command_grpid(self, grpid): ''' @@ -420,9 +420,9 @@ class UINamespaceNode(UINode): ''' try: self.cfnode.set_grpid(grpid) - except Exception: + except Exception as exc: raise configshell.ExecutionError( - "Failed to set ANA Group ID for this Namespace.") + "Failed to set ANA Group ID for this Namespace.") from exc def summary(self): info = [] @@ -763,9 +763,9 @@ class UIReferralNode(UINode): try: self.cfnode.set_enable(1) self.shell.log.info("The Referral has been enabled.") - except Exception: + except Exception as exc: raise configshell.ExecutionError( - "The Referral could not be enabled.") + "The Referral could not be enabled.") from exc def ui_command_disable(self): ''' @@ -781,9 +781,9 @@ class UIReferralNode(UINode): try: self.cfnode.set_enable(0) self.shell.log.info("The Referral has been disabled.") - except Exception: + except Exception as exc: raise configshell.ExecutionError( - "The Referral could not be disabled.") + "The Referral could not be enabled.") from exc class UIANAGroupsNode(UINode): From 9d35c9b0e0ac727ec40026cc52f0f0f1d9f6ed0e Mon Sep 17 00:00:00 2001 From: Ales Novak Date: Thu, 9 Jul 2026 00:05:33 +0200 Subject: [PATCH 08/13] nvmetcli: mask unused arguments of functions with _ Unused function/method arguments should be masked with _. Resolves pylint complaint W0613. Signed-off-by: Ales Novak Signed-off-by: Daniel Wagner --- nvmetcli | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/nvmetcli b/nvmetcli index 8fc9098..bd90ddb 100755 --- a/nvmetcli +++ b/nvmetcli @@ -471,7 +471,7 @@ class UIAllowedHostsNode(UINode): self.parent.cfnode.add_allowed_host(nqn) UIAllowedHostNode(self, nqn) - def ui_complete_create(self, parameters, text, current_param): + def ui_complete_create(self, _parameters, _text, current_param): ''' Completes the create command. ''' @@ -497,7 +497,7 @@ class UIAllowedHostsNode(UINode): self.parent.cfnode.remove_allowed_host(nqn) self.refresh() - def ui_complete_delete(self, parameters, text, current_param): + def ui_complete_delete(self, _parameters, _text, current_param): ''' Completes the delete command. ''' @@ -636,7 +636,7 @@ class UIPortSubsystemsNode(UINode): self.parent.cfnode.add_subsystem(nqn) UIPortSubsystemNode(self, nqn) - def ui_complete_create(self, parameters, text, current_param): + def ui_complete_create(self, _parameters, _text, current_param): ''' Completes the create command. ''' @@ -662,7 +662,7 @@ class UIPortSubsystemsNode(UINode): self.parent.cfnode.remove_subsystem(nqn) self.refresh() - def ui_complete_delete(self, parameters, text, current_param): + def ui_complete_delete(self, _parameters, _text, current_param): ''' Completes the delete command. ''' @@ -940,7 +940,7 @@ def restore(from_file): sys.exit(0) -def clear(unused): +def clear(_unused): ''' Clears the configuration. ''' From 0a62ca91dfcdd15a05335defc594e68f344c40be Mon Sep 17 00:00:00 2001 From: Ales Novak Date: Thu, 9 Jul 2026 00:09:17 +0200 Subject: [PATCH 09/13] nvmet, nvmetcli: adjust/reorganize imports - standard imports should be placed first (resolves pylint C0411) - import on toplevel only (resolves pylint C0415) - import X.Y as Y should be rephrased to from X import Y (resolves pylint R0402) Signed-off-by: Ales Novak Signed-off-by: Daniel Wagner --- nvmet/__init__.py | 7 +++++-- nvmet/nvme.py | 2 +- nvmet/test_nvmet.py | 2 +- nvmetcli | 4 ++-- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/nvmet/__init__.py b/nvmet/__init__.py index facd019..7566dbd 100644 --- a/nvmet/__init__.py +++ b/nvmet/__init__.py @@ -1,2 +1,5 @@ -from .nvme import Root, Subsystem, Namespace, Port, Host, Referral, ANAGroup, \ - Passthru, DEFAULT_SAVE_FILE # noqa: F401 +""" +NVMe-oF Target configfs-based kernel driver API +""" +from .nvme import (ANAGroup, DEFAULT_SAVE_FILE, Host, Namespace, # noqa: F401 + Passthru, Port, Referral, Root, Subsystem, CFSError) diff --git a/nvmet/nvme.py b/nvmet/nvme.py index 14ef16b..61c1bf2 100644 --- a/nvmet/nvme.py +++ b/nvmet/nvme.py @@ -24,6 +24,7 @@ import json import subprocess import shlex +from doctest import testmod from glob import iglob as glob DEFAULT_SAVE_FILE = '/etc/nvmet/config.json' @@ -1168,7 +1169,6 @@ def dump(self): def _test(): - from doctest import testmod testmod() diff --git a/nvmet/test_nvmet.py b/nvmet/test_nvmet.py index d732d17..5c5e781 100644 --- a/nvmet/test_nvmet.py +++ b/nvmet/test_nvmet.py @@ -7,7 +7,7 @@ import stat import string import unittest -import nvmet.nvme as nvme +from nvmet import nvme # Default test devices are ram disks, but allow user to specify different # block devices or files. diff --git a/nvmetcli b/nvmetcli index bd90ddb..6609e7c 100755 --- a/nvmetcli +++ b/nvmetcli @@ -22,11 +22,11 @@ from __future__ import print_function import os import sys -import configshell -import nvmet as nvme import errno from string import hexdigits import uuid +import configshell +import nvmet as nvme def nguid_set(nguid): From 6bfd577b8a7125b378493c27d55205e7b2243ccd Mon Sep 17 00:00:00 2001 From: Ales Novak Date: Thu, 9 Jul 2026 00:12:30 +0200 Subject: [PATCH 10/13] nvme.py: reorganize kernel module loading - import on toplevel only (resolves pylint C0415) - linearize the processing Signed-off-by: Ales Novak Signed-off-by: Daniel Wagner --- nvmet/nvme.py | 50 +++++++++++++++++++++++++++++--------------------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/nvmet/nvme.py b/nvmet/nvme.py index 61c1bf2..38bd66d 100644 --- a/nvmet/nvme.py +++ b/nvmet/nvme.py @@ -26,6 +26,14 @@ import shlex from doctest import testmod from glob import iglob as glob +try: + from kmodpy import kmod +except ImportError: + kmod = None +try: + import kmod as kmod_ctypes +except ImportError: + kmod_ctypes = None DEFAULT_SAVE_FILE = '/etc/nvmet/config.json' @@ -283,33 +291,33 @@ def __init__(self): self._create_in_cfs('lookup') def _modprobe(self, modname): - try: - from kmodpy import kmod - + ''' + Load a kernel module. + ''' + if kmod: try: kmod.Kmod().modprobe(modname, quiet=True) + return except kmod.KmodError: pass - except ImportError: - # Try the ctypes library included with the libkmod itself. + + if kmod_ctypes: try: - import kmod + kmod_ctypes.Kmod().modprobe(modname) + return + except Exception: + pass - try: - kmod.Kmod().modprobe(modname) - except Exception: - pass - except ImportError: - # Try the binary specified in /proc - try: - modprobe_cmd = None - with open('/proc/sys/kernel/modprobe', 'r') as f: - modprobe_cmd = f.read() - if modprobe_cmd: - subprocess.run(shlex.split(modprobe_cmd) + [modname], - check=False) - except Exception: - pass + # Try the binary specified in /proc + try: + modprobe_cmd = None + with open('/proc/sys/kernel/modprobe', 'r', encoding="utf-8") as f: + modprobe_cmd = f.read() + if modprobe_cmd: + subprocess.run(shlex.split(modprobe_cmd) + [modname], + check=False) + except Exception: + pass def _list_subsystems(self): self._check_self() From e23db37a09fb11ec63cf7c370ee84f062459b5d6 Mon Sep 17 00:00:00 2001 From: Ales Novak Date: Thu, 9 Jul 2026 00:17:02 +0200 Subject: [PATCH 11/13] nvme.py, nvmetcli: various fixups - optimize if-else and returns (resolves pylint R1705) - removal of stray passes (resolves pylint W0107) - pythonic way of creating dict (resolves pylint R1735) - removal of unecessary argument Port.setup#root - fix shadowing by UIPassthruNode.ui_command_clear_ids#clear (resolves pylint W0621) Signed-off-by: Ales Novak Signed-off-by: Daniel Wagner --- nvmet/nvme.py | 7 +++---- nvmetcli | 36 ++++++++++++++---------------------- 2 files changed, 17 insertions(+), 26 deletions(-) diff --git a/nvmet/nvme.py b/nvmet/nvme.py index 38bd66d..9a1674a 100644 --- a/nvmet/nvme.py +++ b/nvmet/nvme.py @@ -42,7 +42,6 @@ class CFSError(Exception): ''' Generic slib error. ''' - pass class CFSNotFound(CFSError): @@ -50,9 +49,9 @@ class CFSNotFound(CFSError): The underlying configfs object does not exist. Happens when calling methods of an object that is instantiated but have been deleted from configfs, or when trying to lookup an + object that does not exist. ''' - pass class CFSNode: @@ -432,7 +431,7 @@ def err_func(err_str): err_func(f"'portid' not defined in port {index}") continue - Port.setup(self, t, err_func) + Port.setup(t, err_func) return errors @@ -945,7 +944,7 @@ def _list_ana_groups(self): doc="Get the list of ANA Groups for this Port.") @classmethod - def setup(cls, root, n, err_func): + def setup(cls, n, err_func): ''' Set up a Port object based upon n dict, from saved config. Guard against missing or bad dict items, but keep going. diff --git a/nvmetcli b/nvmetcli index 6609e7c..4ef8019 100755 --- a/nvmetcli +++ b/nvmetcli @@ -143,7 +143,6 @@ class UIRootNode(UINode): ''' errors = self.cfnode.restore_from_file(savefile, clear_existing) self.refresh() - if errors: raise configshell.ExecutionError( "Configuration restored, {} errors:\n{}".format( @@ -272,13 +271,13 @@ class UIPassthruNode(UINode): raise configshell.ExecutionError( "The passthru could not be disabled.") from exc - def ui_command_clear_ids(self, clear): + def ui_command_clear_ids(self, clear_id): ''' - If I{clear} is set to non-zero then clears the passthru namespace + If I{clear_id} is set to non-zero then clears the passthru namespace unique identifiers EUI/GUID/UUID. ''' try: - self.cfnode.set_clear_ids(clear) + self.cfnode.set_clear_ids(clear_id) except Exception as exc: raise configshell.ExecutionError( "Failed to set clear_ids for this passthru target.") from exc @@ -482,8 +481,7 @@ class UIAllowedHostsNode(UINode): if len(completions) == 1: return [f"{completions[0]} "] - else: - return completions + return completions def ui_command_delete(self, nqn): ''' @@ -508,8 +506,7 @@ class UIAllowedHostsNode(UINode): if len(completions) == 1: return [f"{completions[0]} "] - else: - return completions + return completions class UIAllowedHostNode(UINode): @@ -601,7 +598,7 @@ class UIPortNode(UINode): # Support older target driver w/o the inline_data_size parameter try: inline_data_size = self.cfnode.get_attr("param", "inline_data_size") - except Exception: + except nvme.CFSError: inline_data_size = "n/a" if inline_data_size != "n/a": info.append(f"inline_data_size={inline_data_size}") @@ -647,8 +644,7 @@ class UIPortSubsystemsNode(UINode): if len(completions) == 1: return [f"{completions[0]} "] - else: - return completions + return completions def ui_command_delete(self, nqn): ''' @@ -673,8 +669,7 @@ class UIPortSubsystemsNode(UINode): if len(completions) == 1: return [f"{completions[0]} "] - else: - return completions + return completions class UIPortSubsystemNode(UINode): @@ -957,7 +952,7 @@ def execute_cmd(cmd): sys.exit(0) -funcs = dict(save=save, restore=restore, clear=clear) +funcs = {'save': save, 'restore': restore, 'clear': clear} def main(): @@ -972,19 +967,16 @@ def main(): if sys.argv[1] == "--help": usage() - if sys.argv[1] in funcs.keys(): + if sys.argv[1] in funcs: if len(sys.argv) > 3: usage() - if len(sys.argv) == 3: - savefile = sys.argv[2] - else: - savefile = None + savefile = sys.argv[2] if len(sys.argv) == 3 else None funcs[sys.argv[1]](savefile) return - else: - concat_args = ' '.join(sys.argv[1:]) - execute_cmd(concat_args) + + concat_args = ' '.join(sys.argv[1:]) + execute_cmd(concat_args) try: shell = configshell.ConfigShell('~/.nvmetcli') From 8e2634a27574c66c14422f5fedf4b1fbe3c8d3b6 Mon Sep 17 00:00:00 2001 From: Ales Novak Date: Thu, 9 Jul 2026 11:46:59 +0200 Subject: [PATCH 12/13] nvme.py, nvmetcli: narrow down too general exception catches Resolves pylint complaint W0718. Signed-off-by: Ales Novak Signed-off-by: Daniel Wagner --- nvmet/nvme.py | 16 ++++++++-------- nvmetcli | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/nvmet/nvme.py b/nvmet/nvme.py index 9a1674a..878cee1 100644 --- a/nvmet/nvme.py +++ b/nvmet/nvme.py @@ -178,7 +178,7 @@ def set_attr(self, group, attribute, value): try: with open(path, 'w', encoding="utf-8") as file_fd: file_fd.write(str(value)) - except Exception as e: + except OSError as e: raise CFSError(f"Cannot set attribute {path}: {e}") from e def get_attr(self, group, attribute): @@ -222,7 +222,7 @@ def set_enable(self, value): try: with open(path, 'w', encoding="utf-8") as file_fd: file_fd.write(str(value)) - except Exception as e: + except OSError as e: raise CFSError(f"Cannot enable {self.path}: {e} ({value})") from e self._enable = value @@ -304,7 +304,7 @@ def _modprobe(self, modname): try: kmod_ctypes.Kmod().modprobe(modname) return - except Exception: + except OSError: pass # Try the binary specified in /proc @@ -315,7 +315,7 @@ def _modprobe(self, modname): if modprobe_cmd: subprocess.run(shlex.split(modprobe_cmd) + [modname], check=False) - except Exception: + except OSError: pass def _list_subsystems(self): @@ -554,7 +554,7 @@ def add_allowed_host(self, nqn): try: os.symlink(f"{self.configfs_dir}/hosts/{nqn}", f"{self._path}/allowed_hosts/{nqn}") - except Exception as e: + except OSError as e: raise CFSError(f"Could not symlink {nqn} in configFS: {e}") from e def remove_allowed_host(self, nqn): @@ -563,7 +563,7 @@ def remove_allowed_host(self, nqn): ''' try: os.unlink(f"{self._path}/allowed_hosts/{nqn}") - except Exception as e: + except OSError as e: raise CFSError(f"Could not unlink {nqn} in configFS: {e}") from e def has_passthru(self): @@ -895,7 +895,7 @@ def add_subsystem(self, nqn): try: os.symlink(f"{self.configfs_dir}/subsystems/{nqn}", f"{self._path}/subsystems/{nqn}") - except Exception as e: + except OSError as e: raise CFSError(f"Could not symlink {nqn} in configFS: {e}") from e def remove_subsystem(self, nqn): @@ -904,7 +904,7 @@ def remove_subsystem(self, nqn): ''' try: os.unlink(f"{self._path}/subsystems/{nqn}") - except Exception as e: + except OSError as e: raise CFSError(f"Could not unlink {nqn} in configFS: {e}") from e def delete(self): diff --git a/nvmetcli b/nvmetcli index 4ef8019..07fcab3 100755 --- a/nvmetcli +++ b/nvmetcli @@ -981,14 +981,14 @@ def main(): try: shell = configshell.ConfigShell('~/.nvmetcli') UIRootNode(shell) - except Exception as msg: + except (nvme.CFSError, configshell.ExecutionError) as msg: shell.log.error(str(msg)) return while not shell._exit: try: shell.run_interactive() - except Exception as msg: + except (nvme.CFSError, configshell.ExecutionError) as msg: shell.log.error(str(msg)) From 8d81493b14dae068d114ccb3ee908cbc579ebe48 Mon Sep 17 00:00:00 2001 From: Ales Novak Date: Thu, 9 Jul 2026 00:53:46 +0200 Subject: [PATCH 13/13] nvme.py, nvmetcli: mask some pylint errors - there's no way around getting configshell _exit value (masks pylint W0212) - we're not going to reorganize nvmet.nvme into several modules at this point (masks pylint C0302) Signed-off-by: Ales Novak Signed-off-by: Daniel Wagner --- nvmet/nvme.py | 2 ++ nvmetcli | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/nvmet/nvme.py b/nvmet/nvme.py index 878cee1..5bc1485 100644 --- a/nvmet/nvme.py +++ b/nvmet/nvme.py @@ -1181,3 +1181,5 @@ def _test(): if __name__ == "__main__": _test() + +# pylint: disable=too-many-lines diff --git a/nvmetcli b/nvmetcli index 07fcab3..f39e8af 100755 --- a/nvmetcli +++ b/nvmetcli @@ -985,7 +985,7 @@ def main(): shell.log.error(str(msg)) return - while not shell._exit: + while not shell._exit: # pylint: disable=protected-access try: shell.run_interactive() except (nvme.CFSError, configshell.ExecutionError) as msg: