Re: [PATCHv2 2/4] nvme: extend show-topology command to add support for multipath

[Nilay Shroff <nilay@linux.ibm.com>]

On 8/20/25 2:00 PM, Hannes Reinecke wrote:
> On 8/20/25 10:17, Daniel Wagner wrote:
>> On Tue, Aug 19, 2025 at 08:15:09AM +0200, Hannes Reinecke wrote:
>>>> Okay makes sense, so we'd print <numa-node> and exclude <queue-depth> if iopolicy
>>>> is numa. For 'queue-depth' iopolicy, we'd print <queue-depth> and exclude <numa-node>.
>>>> And for 'round-robin' iopolicy, we'd neither print <numa-node> nor <queue-depth>.
>>>> I'll update this in the next patch.
>>>>
>>> Hmm. I'd rather have _some_ value for 'round-robin', too, as otherwise
>>> the number of fields will be different (and making parsing harder).
>>
>> What type of parser do you mean, a carbon based one or a computer? I
>> strongly recommend to use the JSON output for the later.
>>
>> I would really prefer that the default stdout is easy for a human to
>> read not screen scrappers.
>>
> That was my intention, too. And my prime objection was to have a
> sequence of raw numbers, and requiring the user to figure out what
> these numbers are for.
> 
Yes, you are right — those sequences of raw numbers did look confusing,
and that’s why Daniel suggested adding annotations. However, I found it
difficult to annotate the numbers in the tree-style output of show-topology.
To address this, we decided to also support printing topology in a tabular
format, which is much easier for humans to interpret.

As you can see in the last patch of this series (4/4), we now support showing
the topology in a tabular format (when the user selects it). For reference:

# ./nvme show-topology -o tabular 
nvme-subsys1 - NQN=nvmet_subsystem
               hostnqn=nqn.2014-08.org.nvmexpress:uuid:779c3f46-2bb7-4e82-8f46-2b3e795e54f6
               iopolicy=numa

NSHead  NSID NSPath    ANAState  Nodes Qdepth Controller TrType Address                                          State 
------- ---- --------- --------- ----- ------ ---------- ------ ------------------------------------------------ ----- 
nvme1n1  1   nvme1c1n1 optimized  0,1     0    nvme1      tcp    traddr=127.0.0.2,trsvcid=4460,src_addr=127.0.0.1 live  
  -->    1   nvme1c2n1 optimized  2,3     0    nvme2      tcp    traddr=127.0.0.3,trsvcid=4460,src_addr=127.0.0.1 live  

So as you could see above, this format is much clearer and human-friendly.
(Please note that above table is printed using new table APIs introduced
in patch 3/3).

> But really, I'm not sure if we should print out values from the various
> I/O policies. For NUMA it probably makes sense, but for round-robin and
> queue-depths the values are extremely volatile, so I wonder what benefit
> for the user is here.
> 

I think the qdepth output could still be useful. For example, if I/Os are
queuing up on one path (perhaps because that path is slower), then the Qdepth
value might help indicate something unusual or explain why one path is being
chosen over another.

That said, if we all agree that tools or scripts should ideally rely on JSON
output for parsing, then the tabular output could be simplified further:

- For numa iopolicy: print <Nodes> and exclude <Qdepth>.
- For queue-depth iopolicy: print <Qdepth> and exclude <Nodes>.
- For round-robin iopolicy: exclude both <Nodes> and <Qdepth>.

Does this sound reasonable? Or do we still want to avoid printing 
<Qdepth> even for queue-depth iopolicy?

Thanks,
--Nilay