[PATCH 1/3] nvme: add duplicate_connect option
James Smart
jsmart2021 at gmail.com
Thu Oct 19 08:58:13 PDT 2017
On 10/19/2017 8:20 AM, Christoph Hellwig wrote:
>> + { NVMF_OPT_DUP_CONNECT, "duplicate_connect=%d" },
>
> This is a boolean parameter, so please drop the "=%d here".
>
>> + case NVMF_OPT_DUP_CONNECT:
>> + if (match_int(args, &token)) {
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> + if (token == 0)
>> + opts->duplicate_connect = false;
>> + else if (token == 1)
>> + opts->duplicate_connect = true;
>> + else {
>> + pr_err("Invalid duplicate_connect %d\n", token);
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> + break;
>
> And replace all this code by:
>
> case NVMF_OPT_DUP_CONNECT:
> opts->duplicate_connect = true;
> break;
>
> That being said I'd be tempted to keep the existing behavior as
> the default, and make noduplicates the option.
>
ok, so the mere existence of the parameter means it's true. That's fine
and I can repost.
I disagree with allowing duplicates to be the default. Generically, I
think it should be the exception. Let the admin that will create
duplicates, thus is well aware of the ramifications and that it is a
duplication, use the explicit syntax. Note: currently patches only
allow the option for "nvme connect", not "connect-all".
Selfishly, making duplicates allowed the defaults also impacts the
scripts posted for FC, and widens the nvme cli syntax to connect-all.
-- james
More information about the Linux-nvme
mailing list