[PATCH] nvme: print out valid arguments when reading from /dev/nvme-fabrics

[PATCH] nvme: print out valid arguments when reading from /dev/nvme-fabrics

[Hannes Reinecke]

Currently applications have a hard time figuring out which
nvme-over-fabrics arguments are supported for any given kernel;
the ioctl will return an error code on failure, and the application
has to guess whether this was due to an invalid argument or due
to a connection or controller error.
With this patch applications can read a list of supported
arguments by simply reading from /dev/nvme-fabrics, allowing
them to validate the connection string.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/nvme/host/fabrics.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index 0ac054f80a82..810bbc8c9b6b 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -1144,7 +1144,18 @@ static int nvmf_dev_show(struct seq_file *seq_file, void *private)
 	mutex_lock(&nvmf_dev_mutex);
 	ctrl = seq_file->private;
 	if (!ctrl) {
-		ret = -EINVAL;
+		const struct match_token *tok;
+		int idx;
+
+		for (idx = 0; idx < ARRAY_SIZE(opt_tokens); idx++) {
+			tok = &opt_tokens[idx];
+			if (tok->token == NVMF_OPT_ERR)
+				continue;
+			if (idx)
+				seq_puts(seq_file, ",");
+			seq_puts(seq_file, tok->pattern);
+		}
+		seq_puts(seq_file, "\n");
 		goto out_unlock;
 	}
 
-- 
2.29.2

Re: [PATCH] nvme: print out valid arguments when reading from /dev/nvme-fabrics

[Chaitanya Kulkarni]

On 11/19/21 00:39, Hannes Reinecke wrote:
> Currently applications have a hard time figuring out which
> nvme-over-fabrics arguments are supported for any given kernel;
> the ioctl will return an error code on failure, and the application
> has to guess whether this was due to an invalid argument or due
> to a connection or controller error.
> With this patch applications can read a list of supported
> arguments by simply reading from /dev/nvme-fabrics, allowing
> them to validate the connection string.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>

this seems useful and will avoid a trip to the source code.

Looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

Re: [PATCH] nvme: print out valid arguments when reading from /dev/nvme-fabrics

[Christoph Hellwig]

The subject should use a nvme-fabrics prefix.

> +		const struct match_token *tok;
> +		int idx;
> +
> +		for (idx = 0; idx < ARRAY_SIZE(opt_tokens); idx++) {
> +			tok = &opt_tokens[idx];
> +			if (tok->token == NVMF_OPT_ERR)
> +				continue;
> +			if (idx)
> +				seq_puts(seq_file, ",");
> +			seq_puts(seq_file, tok->pattern);
> +		}
> +		seq_puts(seq_file, "\n");

A helper for this logic would be nice.  Also shouldn't be there
some kind of token to distinguish from the case where a controller
has already been created?

Re: [PATCH] nvme: print out valid arguments when reading from /dev/nvme-fabrics

[Hannes Reinecke]

On 11/23/21 5:22 PM, Christoph Hellwig wrote:
> The subject should use a nvme-fabrics prefix.
> 
>> +		const struct match_token *tok;
>> +		int idx;
>> +
>> +		for (idx = 0; idx < ARRAY_SIZE(opt_tokens); idx++) {
>> +			tok = &opt_tokens[idx];
>> +			if (tok->token == NVMF_OPT_ERR)
>> +				continue;
>> +			if (idx)
>> +				seq_puts(seq_file, ",");
>> +			seq_puts(seq_file, tok->pattern);
>> +		}
>> +		seq_puts(seq_file, "\n");
> 
> A helper for this logic would be nice.  Also shouldn't be there
> some kind of token to distinguish from the case where a controller
> has already been created?
> 
Helper: sure.
But we don't need to distinguish the case where the controller has 
already been created as this is handled by the if condition further up.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

Re: [PATCH] nvme: print out valid arguments when reading from /dev/nvme-fabrics

[Christoph Hellwig]

On Tue, Nov 23, 2021 at 05:38:22PM +0100, Hannes Reinecke wrote:
> Helper: sure.
> But we don't need to distinguish the case where the controller has already 
> been created as this is handled by the if condition further up.

Well, a token that the reader clearly sees that there is no controller
yet

Re: [PATCH] nvme: print out valid arguments when reading from /dev/nvme-fabrics

[Hannes Reinecke]

On 11/23/21 5:42 PM, Christoph Hellwig wrote:
> On Tue, Nov 23, 2021 at 05:38:22PM +0100, Hannes Reinecke wrote:
>> Helper: sure.
>> But we don't need to distinguish the case where the controller has already
>> been created as this is handled by the if condition further up.
> 
> Well, a token that the reader clearly sees that there is no controller
> yet
> 
Ah. That. Sure, can do.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer