Merge several fixes/features#2
Merged
Merged
Conversation
Contributor
|
just formality from a quick glance: patch 2 and 3 are missing SoB. |
There was a problem hiding this comment.
Pull request overview
This PR merges multiple upstream patches to modernize nvmetcli’s Python dependencies, switch to exported configshell symbols, and extend configfs support (discovery NQN, reservations, and NVMe passthru target configuration) across the CLI and backing nvmet library.
Changes:
- Drop Python
sixusage and update code to native Python 3 APIs (e.g.,dict.items()), updating packaging/docs accordingly. - Add support for the
discovery_nqnconfigfs attribute and surface it in the CLI. - Introduce passthru and reservation-related plumbing in
nvmet/nvme.pyand expose passthru controls in the interactive CLI.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| rpm/nvmetcli.spec.tmpl | Removes the runtime dependency on python-six. |
| README | Updates stated Python support (Python 3) after removing six. |
| nvmetcli | Switches to exported configshell symbols; adds discovery summary, passthru UI node, and reservation status display. |
| nvmet/nvme.py | Removes six dependency; adds discovery attr group, passthru support, reservation attr group; adds kmod fallback behavior. |
| nvmet/init.py | Exposes Passthru in the package exports. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+104
to
+107
| def summary(self): | ||
| info = [] | ||
| info.append("discovery=" + self.cfnode.get_attr("discovery", "nqn")) | ||
| return (", ".join(info), True) |
Contributor
|
May I suggest to consider this one too? https://lore.kernel.org/linux-nvme/20250728155504.403426-1-mlombard@redhat.com/ |
Contributor
Author
Yes, that was my candidate for 2nd run, will do. |
igaw
force-pushed
the
master
branch
2 times, most recently
from
603f4e7 to
1c1c5cd
Compare
alesax
force-pushed
the
pr
branch
2 times, most recently
from
fe66584 to
e9f04d2
Compare
Add support for reservation configuration. Signed-off-by: Guixin Liu <kanie@linux.alibaba.com> [alnovak: handle if resv_enable doesn't exist] Signed-off-by: Ales Novak <alnovak@suse.com> Signed-off-by: Daniel Wagner <wagi@kernel.org>
kmodpy is an unmaintained project. Python is not really good at
backwards compatibility. With the upstream project not keeping up with
python churn using the library is a maintenance burden.
nvmet does not use the python library in any substantial way, it only
loads a module once. This can be easily accomplished without any library
using the modprobe tool directly.
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
[alnovak: use subprocess.run() instead of system(), take into account if
there are extra arguments in /proc/sys/kernel/modprobe.]
Signed-off-by: Ales Novak <alnovak@suse.com>
Signed-off-by: Daniel Wagner <wagi@kernel.org>
This change adds the support for configuring NVMe passthru target. Signed-off-by: Nilay Shroff <nilay@linux.ibm.com> [alnovak: - handle lack of passthru support in the kernel - align refresh() method with others - fix typos (Namespace->Passthru, tomeout->timeout) - passhtru: fix when passhtru doesn't exist] Signed-off-by: Ales Novak <alnovak@suse.com> Signed-off-by: Daniel Wagner <wagi@kernel.org>
Python 2 has been truly and completely dead for over five years, I don't know about any Linux distribution which wouldn't have Python 3 as its default Python. Please, let it die. Signed-off-by: Matěj Cepl <mcepl@celp.eu> [alnovak: s/xrange/range/] Signed-off-by: Ales Novak <alnovak@suse.com> Signed-off-by: Daniel Wagner <wagi@kernel.org>
Implement support for the 'discovery_nqn' configfs attribute to allow the user to read or modify the persistent discovery NQN. Signed-off-by: Hannes Reinecke <hare@kernel.org> [alnovak: handle lack of support of discovery_nqn] Signed-off-by: Ales Novak <alnovak@suse.com> Signed-off-by: Daniel Wagner <wagi@kernel.org>
The configshell-fb module is now called configshell. From v2.0.0 onwards the preferred way is to import the module as configshell. Signed-off-by: Stephan Müller <stephan.mueller@phys.ethz.ch> Signed-off-by: Daniel Wagner <wagi@kernel.org>
Replace direct invocations of configshell internals with actually exported symbols. Signed-off-by: Stephan Müller <stephan.mueller@phys.ethz.ch> Signed-off-by: Daniel Wagner <wagi@kernel.org>
Contributor
|
Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This merges several patches submitted to the mailing list: