nvme: update device error handling for list-subsys#2827
Closed
martin-gpy wants to merge 1 commit intolinux-nvme:masterfrom
Closed
nvme: update device error handling for list-subsys#2827martin-gpy wants to merge 1 commit intolinux-nvme:masterfrom
martin-gpy wants to merge 1 commit intolinux-nvme:masterfrom
Conversation
igaw
reviewed
May 26, 2025
| if (sscanf(devname, "nvme%dn%d", &subsys_num, &nsid) != 2) { | ||
| nvme_show_error("Invalid device name %s", devname); | ||
| return -EINVAL; | ||
| } |
Collaborator
There was a problem hiding this comment.
IIRC, devname a filter name, why should we verify if it is valid?
Contributor
Author
There was a problem hiding this comment.
The problem comes when one passes an invalid device here and list-subsys seems to mess that up. For e.g. suppose one has a single subsystem with two namespaces in it:
# nvme list
Node Generic SN Model Namespace Usage Format FW Rev
--------------------- --------------------- -------------------- ---------------------------------------- ---------- -------------------------- ---------------- --------
/dev/nvme1n1 /dev/ng1n1 81CYrBSoKGRJAAAAAAAN NetApp ONTAP Controller 0x2 219.47 MB / 10.74 GB 512 B + 0 B 9.18.1
/dev/nvme1n2 /dev/ng1n2 81CYrBSoKGRJAAAAAAAN NetApp ONTAP Controller 0x1 330.56 MB / 10.74 GB 4 KiB + 0 B 9.18.1
# nvme list-subsys /dev/nvme1n1
nvme-subsys1 - NQN=nqn.1992-08.com.netapp:sn.48391d66c0a611ecaaa5d039ea165514:subsystem.subsys_CLIENT116_0
hostnqn=nqn.2014-08.org.nvmexpress:uuid:e6550026-173e-4959-ba74-be367844bd8a
\
+- nvme1 tcp traddr=192.168.2.116,trsvcid=4420,host_traddr=192.168.1.6,src_addr=192.168.1.6 live optimized
+- nvme2 tcp traddr=192.168.1.116,trsvcid=4420,host_traddr=192.168.1.6,src_addr=192.168.1.6 live optimized
+- nvme3 tcp traddr=192.168.2.117,trsvcid=4420,host_traddr=192.168.1.6,src_addr=192.168.1.6 live non-optimized
+- nvme4 tcp traddr=192.168.1.117,trsvcid=4420,host_traddr=192.168.1.6,src_addr=192.168.1.6 live non-optimized
But I also see a similar list-subsys output even for an invalid device like say nvme1n3, when ideally it should have errored out with an invalid device output:
# nvme list-subsys /dev/nvme1n3
nvme-subsys1 - NQN=nqn.1992-08.com.netapp:sn.48391d66c0a611ecaaa5d039ea165514:subsystem.subsys_CLIENT116_0
hostnqn=nqn.2014-08.org.nvmexpress:uuid:e6550026-173e-4959-ba74-be367844bd8a
\
+- nvme1 tcp traddr=192.168.2.116,trsvcid=4420,host_traddr=192.168.1.6,src_addr=192.168.1.6 live
+- nvme2 tcp traddr=192.168.1.116,trsvcid=4420,host_traddr=192.168.1.6,src_addr=192.168.1.6 live
+- nvme3 tcp traddr=192.168.2.117,trsvcid=4420,host_traddr=192.168.1.6,src_addr=192.168.1.6 live
+- nvme4 tcp traddr=192.168.1.117,trsvcid=4420,host_traddr=192.168.1.6,src_addr=192.168.1.6 live
And I see a blank output when I pass a different invalid device with a new subsys number. For e.g.
# nvme list-subsys /dev/nvme2n1
#
# echo $?
0
Contributor
Author
There was a problem hiding this comment.
But one problem I see with the patch above is that nsid does not get populated for a valid device. Need to fix that up.
Collaborator
There was a problem hiding this comment.
But then the filter is broken and this is just a workaround.
martin-gpy
force-pushed
the
subsys_err_handling
branch
from
c2e5484 to
e1e6b84
Compare
martin-gpy
force-pushed
the
subsys_err_handling
branch
2 times, most recently
from
6aa86af to
23e512a
Compare
The list-subsys command fails to error out if one passes an invalid or non-existent ns device as part of the <device> option. One would have ideally expected the nvme_match_device_filter() to catch this, but such cases would be better handled with a simple stat check for ns device validity before proceeding to the filter in the first place. That way, the stat check can take care of ns device validity whereas the filter can focus on filtering the subsystems and controllers, ignoring checking for invalid ns devices, as logically expected in a list subsystems command. Signed-off-by: Martin George <[email protected]>
martin-gpy
force-pushed
the
subsys_err_handling
branch
from
23e512a to
31b3f02
Compare
Collaborator
|
Hmm, still not explaining why the filter doesn't work. I'll look into it when I find some cycles to work on. |
Collaborator
|
Merged PR #2836 which addresses this issue. |
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
2 participants
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.
The list-subsys command does not properly check if the nvme device passed to the option is a valid device or not. Fix the same.