-
Notifications
You must be signed in to change notification settings - Fork 697
nvme-print: update list-subsys output for iopolicy #3010
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: master
Are you sure you want to change the base?
Conversation
Before commit d0b4c6c, the iopolicy was displayed only in the verbose regular/json outputs of the list-subsys commands, similar to what is currently seen for the show-topology verbose outputs. But now, iopolicy is seen in the list-subsys regular output itself conflicting with its corresponding json output. So to maintain consistency across the board, move the iopolicy to the verbose output itself for the list-subsys command. Signed-off-by: Martin George <marting@netapp.com>
|
From the commit message I understand that @shroffni would like to see iopolicy all the time. I don't have a strong opinion but would like to avoid adding too much on the default output. I think it would be safe to add the iopolicy field to the default json output, this should not break any parser. I am fine with either solution: go back to the old behavior or add extend the json output. |
Exactly. We display so many other subsys attributes such as model, firmware, type, etc. under the verbose option. So why should iopolicy alone remain under the regular output - it only serves to crowd up the regular output more which is unnecessary and an eye sore. Ideally, the regular output should always be brief and concise. Detailed output should be under the verbose option itself as much as possible.
I'd prefer sticking to the old behavior itself of displaying all subsys attributes (including iopolicy) under the verbose option itself - no reason to make iopolicy alone an exception here. |
|
Printing the I/O policy in the non-verbose show-topology output was intentional. For example, when the output is presented in tabular form or in multipath format, it makes sense to also include the I/O policy name. In the multipath view, we display fields such as 'Nodes' or 'queue-depth', which are particularly relevant for the NUMA and queue-depth I/O policies, respectively. So including the I/O policy name helps users quickly associate these fields with the active policy and better understand the resulting I/O path behavior. It makes it easier to identify which attributes are relevant in a given context (e.g., Nodes for NUMA-based policies or queue-depth for queue-depth–based policies). Displaying the I/O policy name is also useful for the round-robin policy. In that case, the output more clearly conveys that when a namespace is shared and multiple paths are available, the I/O scheduler distributes I/O across those paths in a round-robin manner. Overall, IMO, including the I/O policy name in the non-verbose output improves clarity and makes the topology information easier to interpret at least for multipath/tabular view unless including I/O policy name is causing any regression. |
|
Well in that case, please restrict the Also if you have |
Yes my bad, as the function (stdout_subsys_config()) which prints the subsystem header is common across show-tpology and list-subsys output, it spilled over to the output of "nvme list-subsys". I will fix it.
Yes will add this in JSON output as well. Thanks! |
Before commit d0b4c6c, the iopolicy was displayed only in the verbose regular/json outputs of the list-subsys commands, similar to what is currently seen for the show-topology verbose outputs. But now, iopolicy is seen in the list-subsys regular output itself conflicting with its corresponding json output. So to maintain consistency across the board, move the iopolicy to the verbose output itself for the list-subsys command.