Skip to content

Conversation

@sc108-lee
Copy link
Contributor

Below commands that use mmap_registers failed since a31081a (v2.9)
nvme show-regs /dev/nvme0n1
nvme effects-log /dev/nvme0n1

@igaw
Copy link
Collaborator

igaw commented Mar 14, 2025

If I read it correctly the change also implements the fallback for ns. Do we really need it:

We also don't need to do the fallback lookup via the namespaces because
Linux kernel v4.0 has introduced the link in /sys/class/nvme/nvme%d. And
nvme-cli 2.x depends on Linux kernel >= 4.15, so it's safe to drop the
fallback.

@sc108-lee
Copy link
Contributor Author

sc108-lee commented Mar 14, 2025

If I understand right, The commit comment assume that /sys/class/nvme/nvme0n1 also exist?
Currently, this modification do readlink to get nvme0 from nvme0n1. (since there is no nvme0n1 in /sys/class/nvme)

in kernel 6.1.4
[root@localhost csk]# ls /sys/class/nvme/
nvme0

@sc108-lee
Copy link
Contributor Author

if we do not want to lookup via the namespaces
then should we just cutting the device name nvme0n1 -> nvme0 ?
so we can support same functionality for block device with older version of nvme-cli

@igaw
Copy link
Collaborator

igaw commented Mar 14, 2025

The documentation says:

The <device> parameter is mandatory and must be the nvme admin character device (ex: /dev/nvme0)

Thus it's correct that these commands fail. I think it would be better to issue an info instead having this magic.

To indicate that backward compatibility is not supported
since we drop the block device fallback

Signed-off-by: Steven Seungcheol Lee <[email protected]>
@sc108-lee
Copy link
Contributor Author

Good point, I just concerned about backward compatibility.

After attempting to access the register via the fabric command and failing, I initially tried to check the block device and print an error message. However, I modified the implementation to allow only character devices, as intended by the documentation you mentioned.

@igaw igaw merged commit aec85d9 into linux-nvme:master Mar 17, 2025
18 checks passed
@igaw
Copy link
Collaborator

igaw commented Mar 17, 2025

There will be some people which will be unhappy that it doesn't work with block devices anymore, but with the message in place that shouldn't be too hard to figure out what is happening.

Thanks!

sc108-lee added a commit to sc108-lee/ocp-diag-autoval-ssd that referenced this pull request Apr 4, 2025
When using the "effects-log" command in nvme-cli without specifying the "csi" argument,
a default action is added to check the ctrl register,
allowing only character devices like nvme0.

In older versions of nvme-cli, the default value of csi was set to 0 and issued without any problems
But it has been fixed now.
It can be used regardless of the version when used as a character device.

Reference:
linux-nvme/nvme-cli#2742

Signed-off-by: Steven Seungcheol Lee <[email protected]>
sc108-lee added a commit to sc108-lee/ocp-diag-autoval-ssd that referenced this pull request Apr 4, 2025
The documentation says:
The <device> parameter is mandatory and must be the nvme admin character device (ex: /dev/nvme0)

Reference:
linux-nvme/nvme-cli#2742

Signed-off-by: Steven Seungcheol Lee <[email protected]>
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.

2 participants