-
Notifications
You must be signed in to change notification settings - Fork 221
Return NVMe device nodes/paths (/dev/...) robustly across nvme-cli schemas
#4073
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
base: main
Are you sure you want to change the base?
Return NVMe device nodes/paths (/dev/...) robustly across nvme-cli schemas
#4073
Conversation
/dev/...) robustly across nvme-cli schemas
dfc5566 to
12ab6ee
Compare
lisa/tools/nvmecli.py
Outdated
| # ------------------------------- | ||
| # Pure Python (when jq is not available) | ||
| # ------------------------------- | ||
| nvme_devices = self.get_devices(force_run=force_run) # raw ["Devices"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Move the parser logic into
get_devices, so it can be shared, and managed in one place.get_devicesshould return a data structure, not a json object. - Place comments above the code line, not at the end.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Move the parser logic into
get_devices, so it can be shared, and managed in one place.get_devicesshould return a data structure, not a json object.- Place comments above the code line, not at the end.
-
get_devices has always been returning a json object, as its job is to get us the details of devices. And that is the output consumed by many other functions in other files too. It has always been the job of the get_disks to parse it and fetch the device paths. Hence I changed the logic in get_disks and left get_devices as is.
-
That was the order even before, I will update it, also will change the content of that json.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method isn’t widely used—based on my search, it only shows up twice in this class. Also, the comment # raw ["Devices"] looks like a recent addition.
12ab6ee to
c6a97d2
Compare
Upstream change context:
- nvme-cli reworked
nvme list -o jsonaround v2.11, removing thelegacy top-level
.Devices[].DevicePathand nesting device info under: Subsystems → Controllers → Namespaces.- Reference discussion and breakage report: linux-nvme/nvme-cli#2749 (thread points to commit 929f461 as the change introducing the new JSON)
- Some distro builds may still emit
DevicePath. This logic supports both.- jq option is implemented to simplify parsing, but a pure-Python fallback is also provided if
jqis not available on the target system. jq option is more efficient and robust, so it is preferred when possible.