Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

types: fix regression for vendor-specific field in nvme_id_ns #752

Merged
merged 1 commit into from
Dec 1, 2023

Conversation

cjp256
Copy link
Contributor

@cjp256 cjp256 commented Dec 1, 2023

Recent versions of nvme-cli have started reading vs from offset 392 instead of 384. Previous PRs coupled the use of nvme_id_ns for use in namespace management (create_ns). However, the NVMe spec has a different structure for namespace management, with only a subset of the fields allowed/shared and some additional fields.

To fix this, remove lbstm and restore the proper length for vs from nvme_id_ns.

I expect that create_ns() should fully switch over to the nvme_ns_mgmt_host_sw_specified struct which seems aligned with the spec (though has some newer fields than what is available in the latest NVMe Command Set Specification Revision 1.0c). This will have to be addressed separately in nvme-cli.

#307
linux-nvme/nvme-cli#1452

Recent versions of nvme-cli have started reading vs from offset
392 instead of 384. Previous PRs coupled the use of nvme_id_ns
for use in namespace management (create_ns).  However, the NVMe
spec has a different structure for namespace management, with
only a subset of the fields allowed/shared and some additional
fields.

To fix this, remove lbstm and restore the proper length for vs
from nvme_id_ns.

I expect that create_ns() should fully switch over to the
nvme_ns_mgmt_host_sw_specified struct which seems aligned with
the spec (though has some newer fields than what is available in
the latest NVMe Command Set Specification Revision 1.0c). This
will have to be addressed separately in nvme-cli.

linux-nvme#307
linux-nvme/nvme-cli#1452

Signed-off-by: Chris Patterson <cpatterson@microsoft.com>
@igaw
Copy link
Collaborator

igaw commented Dec 1, 2023

Ooops. The change da3373aae10e ("nvme: Add ns-mgmt host software specified fields") makes this revert possible without breaking nvme-cli.

@igaw igaw merged commit f92b2a3 into linux-nvme:master Dec 1, 2023
14 checks passed
@igaw
Copy link
Collaborator

igaw commented Dec 1, 2023

Thanks!

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