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

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

Hi Daniel and Hannes,

Just a gentle ping on this one...

Do you agree with the reasoning I suggested for filtering
columns based on iopolicy? If we all have agreement then
I'd send out the next patchset with appropriate change.

Thanks,
--Nilay

On 8/20/25 5:29 PM, Nilay Shroff wrote:
> 
> 
> 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