Re: [PATCH 1/6] nvmet: make discovery NQN configurable

From: Himanshu Madhani <himanshu.madhani@oracle.com>
To: Hannes Reinecke <hare@suse.de>
Cc: Christoph Hellwig <hch@lst.de>, Sagi Grimberg <sagi@grimberg.me>,
	Keith Busch <keith.busch@wdc.com>,
	"linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>
Subject: Re: [PATCH 1/6] nvmet: make discovery NQN configurable
Date: Wed, 22 Sep 2021 13:32:53 +0000	[thread overview]
Message-ID: <13F8510F-E2CB-41F7-B620-E5944493C62F@oracle.com> (raw)
In-Reply-To: <20210921151529.29419-2-hare@suse.de>

> On Sep 21, 2021, at 10:15 AM, Hannes Reinecke <hare@suse.de> wrote:
> 
> TPAR8013 allows for unique discovery NQNs, so make the discovery
> controller NQN configurable by exposing a subsys attribute
> 'discovery_nqn'.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
> drivers/nvme/target/configfs.c | 42 ++++++++++++++++++++++++++++++++++
> drivers/nvme/target/core.c     |  3 ++-
> 2 files changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
> index be5d82421e3a..d80717b9779d 100644
> --- a/drivers/nvme/target/configfs.c
> +++ b/drivers/nvme/target/configfs.c
> @@ -1233,6 +1233,47 @@ static ssize_t nvmet_subsys_attr_model_store(struct config_item *item,
> }
> CONFIGFS_ATTR(nvmet_subsys_, attr_model);
> 
> +static ssize_t nvmet_subsys_attr_discovery_nqn_show(struct config_item *item,
> +			char *page)
> +{
> +	struct nvmet_subsys *subsys = nvmet_disc_subsys;
> +
> +	return snprintf(page, PAGE_SIZE, "%s\n",
> +			subsys->subsysnqn);
> +}
> +
> +static ssize_t nvmet_subsys_attr_discovery_nqn_store(struct config_item *item,
> +			const char *page, size_t count)
> +{
> +	struct nvmet_subsys *subsys = to_subsys(item);
> +	char *subsysnqn;
> +	int len;
> +
> +	len = strcspn(page, "\n");
> +	if (!len)
> +		return -EINVAL;
> +
> +	subsysnqn = kmemdup_nul(page, len, GFP_KERNEL);
> +	if (!subsysnqn)
> +		return -ENOMEM;
> +
> +	/*
> +	 * The discovery NQN must be different from
> +	 * subsystem NQN.
> +	 */
> +	if (!strcmp(subsysnqn, subsys->subsysnqn)) {
> +		kfree(subsysnqn);
> +		return -EBUSY;
> +	}
> +	down_write(&nvmet_config_sem);
> +	kfree(nvmet_disc_subsys->subsysnqn);
> +	nvmet_disc_subsys->subsysnqn = subsysnqn;
> +	up_write(&nvmet_config_sem);
> +
> +	return count;
> +}
> +CONFIGFS_ATTR(nvmet_subsys_, attr_discovery_nqn);
> +
> #ifdef CONFIG_BLK_DEV_INTEGRITY
> static ssize_t nvmet_subsys_attr_pi_enable_show(struct config_item *item,
> 						char *page)
> @@ -1262,6 +1303,7 @@ static struct configfs_attribute *nvmet_subsys_attrs[] = {
> 	&nvmet_subsys_attr_attr_cntlid_min,
> 	&nvmet_subsys_attr_attr_cntlid_max,
> 	&nvmet_subsys_attr_attr_model,
> +	&nvmet_subsys_attr_attr_discovery_nqn,
> #ifdef CONFIG_BLK_DEV_INTEGRITY
> 	&nvmet_subsys_attr_attr_pi_enable,
> #endif
> diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
> index b8425fa34300..88ed746c675f 100644
> --- a/drivers/nvme/target/core.c
> +++ b/drivers/nvme/target/core.c
> @@ -1491,7 +1491,8 @@ static struct nvmet_subsys *nvmet_find_get_subsys(struct nvmet_port *port,
> 	if (!port)
> 		return NULL;
> 
> -	if (!strcmp(NVME_DISC_SUBSYS_NAME, subsysnqn)) {
> +	if (!strcmp(NVME_DISC_SUBSYS_NAME, subsysnqn) ||
> +	    !strcmp(nvmet_disc_subsys->subsysnqn, subsysnqn)) {
> 		if (!kref_get_unless_zero(&nvmet_disc_subsys->ref))
> 			return NULL;
> 		return nvmet_disc_subsys;
> -- 
> 2.29.2
> 
> 
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme

Agree with Comments from Chaitanya. 

With those changes you can add  

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>

--
Himanshu Madhani	 Oracle Linux Engineering

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme