Re: [PATCH] nvmet: implement unique discovery NQN

[Hannes Reinecke <hare@suse.de>]

On 12/12/23 06:20, Chaitanya Kulkarni wrote:
> On 12/11/23 00:10, hare@kernel.org wrote:
>> From: Hannes Reinecke <hare@suse.de>
>>
>> Add a configfs attribute 'discovery_nqn' in the 'nvmet' configfs
>> directory to specify the unique discovery NQN.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>>    drivers/nvme/target/configfs.c | 45 ++++++++++++++++++++++++++++++++++
>>    drivers/nvme/target/core.c     |  7 ++++++
>>    2 files changed, 52 insertions(+)
>>
>> diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
>> index d937fe05129e..88911d0cd057 100644
>> --- a/drivers/nvme/target/configfs.c
>> +++ b/drivers/nvme/target/configfs.c
>> @@ -1587,6 +1587,11 @@ static struct config_group *nvmet_subsys_make(struct config_group *group,
>>    		return ERR_PTR(-EINVAL);
>>    	}
>>    
>> +	if (sysfs_streq(name, nvmet_disc_subsys->subsysnqn)) {
>> +		pr_err("can't create subsystem using unique discovery NQN\n");
>> +		return ERR_PTR(-EINVAL);
>> +	}
>> +
>>    	subsys = nvmet_subsys_alloc(name, NVME_NQN_NVME);
>>    	if (IS_ERR(subsys))
>>    		return ERR_CAST(subsys);
>> @@ -2131,7 +2136,47 @@ static const struct config_item_type nvmet_hosts_type = {
>>    
>>    static struct config_group nvmet_hosts_group;
>>    
>> +static ssize_t nvmet_root_discovery_nqn_show(struct config_item *item,
>> +					     char *page)
>> +{
>> +	return sprintf(page, "%s\n", nvmet_disc_subsys->subsysnqn);
>> +}
>> +
>> +static ssize_t nvmet_root_discovery_nqn_store(struct config_item *item,
>> +		const char *page, size_t count)
>> +{
>> +	size_t len;
>> +	struct list_head *entry;
>> +
>> +	len = strcspn(page, "\n");
>> +	if (!len || len > NVMF_NQN_FIELD_LEN - 1)
>> +		return -EINVAL;
>> +
>> +	down_write(&nvmet_config_sem);
>> +	list_for_each(entry, &nvmet_subsystems_group.cg_children) {
>> +		struct config_item *item = container_of(entry, struct config_item, ci_entry);
>> +		if (!strncmp(config_item_name(item), page, len)) {
>> +			pr_err("duplicate NQN %s\n", config_item_name(item));
>> +			up_write(&nvmet_config_sem);
>> +			return -EINVAL;
>> +		}
>> +	}
>> +	memset(nvmet_disc_subsys->subsysnqn, 0, NVMF_NQN_FIELD_LEN);
>> +	memcpy(nvmet_disc_subsys->subsysnqn, page, len);
>> +	up_write(&nvmet_config_sem);
>> +
>> +	return len;
>> +}
>> +
>>
> 
> please consider something like following, it removes long line
> re-structures the declaration totally untested :-
> 
> diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
> index 88911d0cd057..05d67ddfe83e 100644
> --- a/drivers/nvme/target/configfs.c
> +++ b/drivers/nvme/target/configfs.c
> @@ -2145,8 +2145,8 @@ static ssize_t
> nvmet_root_discovery_nqn_show(struct config_item *item,
>    static ssize_t nvmet_root_discovery_nqn_store(struct config_item *item,
>                   const char *page, size_t count)
>    {
> -       size_t len;
>           struct list_head *entry;
> +       size_t len;
> 
>           len = strcspn(page, "\n");
>           if (!len || len > NVMF_NQN_FIELD_LEN - 1)
> @@ -2154,7 +2154,9 @@ static ssize_t
> nvmet_root_discovery_nqn_store(struct config_item *item,
> 
>           down_write(&nvmet_config_sem);
>           list_for_each(entry, &nvmet_subsystems_group.cg_children) {
> -               struct config_item *item = container_of(entry, struct
> config_item, ci_entry);
> +               struct config_item *item =
> +                       container_of(entry, struct config_item, ci_entry);
> +
>                   if (!strncmp(config_item_name(item), page, len)) {
>                           pr_err("duplicate NQN %s\n",
> config_item_name(item));
>                           up_write(&nvmet_config_sem);
> 
> -ck
> 
> 
Ok, will be resending.

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), GF: Ivo Totev, Andrew McDonald,
Werner Knoblich