Re: [PATCHv2 1/4] nvme: support <device> option in show-topology command

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

On 8/18/25 12:42 PM, Hannes Reinecke wrote:
> On 8/12/25 14:56, Nilay Shroff wrote:
>> Although the help text for the nvme show-topology command indicates
>> support for a <device> option, this option has no effect in practice
>> — specifying an NVMe device name does not filter the output.
>>
>> This commit adds proper support for the <device> option, enabling users
>> to filter the topology output based on the specified NVMe device.
>>
>> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
>> ---
>>   nvme-print-stdout.c |  9 +++++++++
>>   nvme.c              | 17 ++++++++++++++++-
>>   2 files changed, 25 insertions(+), 1 deletion(-)
>>
>> diff --git a/nvme-print-stdout.c b/nvme-print-stdout.c
>> index c597b608..eb56349a 100644
>> --- a/nvme-print-stdout.c
>> +++ b/nvme-print-stdout.c
>> @@ -5683,6 +5683,15 @@ static void stdout_simple_topology(nvme_root_t r,
>>         nvme_for_each_host(r, h) {
>>           nvme_for_each_subsystem(h, s) {
>> +            bool no_ctrl = true;
>> +            nvme_ctrl_t c;
>> +
>> +            nvme_subsystem_for_each_ctrl(s, c)
>> +                no_ctrl = false;
>> +
>> +            if (no_ctrl)
>> +                continue;
>> +
>>               if (!first)
>>                   printf("\n");
>>               first = false;
>> diff --git a/nvme.c b/nvme.c
>> index 27dac37b..c6779cf4 100644
>> --- a/nvme.c
>> +++ b/nvme.c
>> @@ -10155,6 +10155,8 @@ static int show_topology_cmd(int argc, char **argv, struct command *command, str
>>       const char *ranking = "Ranking order: namespace|ctrl";
>>       nvme_print_flags_t flags;
>>       _cleanup_nvme_root_ nvme_root_t r = NULL;
>> +    char *devname = NULL;
>> +    nvme_scan_filter_t filter = NULL;
>>       enum nvme_cli_topo_ranking rank;
>>       int err;
>>   @@ -10197,7 +10199,20 @@ static int show_topology_cmd(int argc, char **argv, struct command *command, str
>>           return -errno;
>>       }
>>   -    err = nvme_scan_topology(r, NULL, NULL);
>> +    if (optind < argc)
>> +        devname = basename(argv[optind++]);
>> +
>> +    if (devname) {
>> +        int subsys_id, nsid;
>> +
>> +        if (sscanf(devname, "nvme%dn%d", &subsys_id, &nsid) != 2) {
>> +            nvme_show_error("Invalid device name %s\n", devname);
>> +            return -EINVAL;
>> +        }
>> +        filter = nvme_match_device_filter;
>> +    }
>> +
>> +    err = nvme_scan_topology(r, filter, (void *)devname);
>>       if (err < 0) {
>>           nvme_show_error("Failed to scan topology: %s", nvme_strerror(errno));
>>           return err;
> 
> While I welcome this change, we've had discussions that
> 'nvme show-topology <device>' really should _not_ print out
> anything if the device does not exist.
> Have you checked this for your patch?
> 
Yes I checked that. If <device> doesn't exist then 
nvme show-topology would print nothing.

Thanks,
--Nilay