Skip to content

Conversation

@Cav4ever
Copy link

@Cav4ever Cav4ever commented Jul 7, 2025

On nvme-cli 1.x, nvme list is no-ops when the nvme_core module isn't loaded:

# nvme list
Node                  SN                   Model                                    Namespace Usage                      Format           FW Rev
--------------------- -------------------- ---------------------------------------- --------- -------------------------- ---------------- --------

But on nvme-cli 2.x, it fails when trying to open the nonexistent /sys/class/nvme. It would be nice for the command to be no-ops again.

 # nvme list
Failed to scan topology: No such file or directory

commit 283585a fixes the issue, but commit f17afe1 reverts the change.

@Cav4ever
Copy link
Author

Cav4ever commented Jul 8, 2025

@igaw Hi Daniel,please help me review this patch.

are not loaded

On nvme-cli 1.x, "nvme list" is no-ops when the nvme_core module
isn't loaded.
But on nvme-cli 2.x, it fails when trying to open the nonexistent
/sys/class/nvme.
It would be better if 2.x behaved the same way.

Signed-off-by: Kou Wenqi <[email protected]>
@igaw
Copy link
Collaborator

igaw commented Jul 8, 2025

The commit message should contain the reason why this change is necessary. And given the amount of time this exact code has been touched by either someone complaining it should print a warning or not I have the suspicion this is going to cause another discussion.

@Cav4ever Cav4ever changed the title nvme: do not report an error when the modules are not loaded nvme: make "nvme list" do not report an error when the modules are not loaded Jul 9, 2025
@Cav4ever
Copy link
Author

Cav4ever commented Jul 9, 2025

@igaw

The commit message should contain the reason why this change is necessary.

Thank you for your review. I have revised the commit message; kindly take another look.

To ensure consistency, it would be better if the behavior of "nvme list" in version 2.x matched that of version 1.x when the NVMe modules are not loaded.

@igaw
Copy link
Collaborator

igaw commented Jul 9, 2025

I have to dig into it again. But one thing is, that I am extremely hesitant to change the behavior 3 years after we have released 2.x.

@igaw
Copy link
Collaborator

igaw commented Jul 25, 2025

I'll look into it after the v2.15 release. Just not enough time to dive into this topic right now.

@tbzatek
Copy link
Contributor

tbzatek commented Aug 1, 2025

<OT> Yeah, it's the kind of annoyance when user needs to modprobe manually. Similar situation with nvme connect missing the nvme-fabrics module or with nvmetcli complaining about a missing nvmet module. In all these cases user gets rather cryptic message, not immediately pointing to a module to be loaded.

@igaw
Copy link
Collaborator

igaw commented Nov 3, 2025

Please discuss this out with @martin-gpy who added this feature f17afe1 ("fabrics: print an error for ENOENT too")

Obviously, you can't have both.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants