Re: [PATCH 3/7] nvmet: add nvmet_is_disc_subsys() helper

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>,
	Chaitanya Kulkarni <chaitanyak@nvidia.com>,
	"linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>
Subject: Re: [PATCH 3/7] nvmet: add nvmet_is_disc_subsys() helper
Date: Wed, 22 Sep 2021 13:36:52 +0000	[thread overview]
Message-ID: <22124F64-CA07-4A71-933E-53FF980FA879@oracle.com> (raw)
In-Reply-To: <20210922063525.143045-4-hare@suse.de>

> On Sep 22, 2021, at 1:35 AM, Hannes Reinecke <hare@suse.de> wrote:
> 
> Add a helper function to determine if a given subsystem is a discovery
> subsystem.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
> drivers/nvme/target/admin-cmd.c | 2 +-
> drivers/nvme/target/core.c      | 6 +++---
> drivers/nvme/target/nvmet.h     | 5 +++++
> 3 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
> index aa6d84d8848e..b653ea4244fe 100644
> --- a/drivers/nvme/target/admin-cmd.c
> +++ b/drivers/nvme/target/admin-cmd.c
> @@ -1008,7 +1008,7 @@ u16 nvmet_parse_admin_cmd(struct nvmet_req *req)
> 
> 	if (nvme_is_fabrics(cmd))
> 		return nvmet_parse_fabrics_cmd(req);
> -	if (nvmet_req_subsys(req)->type == NVME_NQN_DISC)
> +	if (nvmet_is_disc_subsys(nvmet_req_subsys(req)))
> 		return nvmet_parse_discovery_cmd(req);
> 
> 	ret = nvmet_check_ctrl_status(req);
> diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
> index 88ed746c675f..61ceb42717f1 100644
> --- a/drivers/nvme/target/core.c
> +++ b/drivers/nvme/target/core.c
> @@ -1140,7 +1140,7 @@ static void nvmet_start_ctrl(struct nvmet_ctrl *ctrl)
> 	 * should verify iosqes,iocqes are zeroed, however that
> 	 * would break backwards compatibility, so don't enforce it.
> 	 */
> -	if (ctrl->subsys->type != NVME_NQN_DISC &&
> +	if (!nvmet_is_disc_subsys(ctrl->subsys) &&
> 	    (nvmet_cc_iosqes(ctrl->cc) != NVME_NVM_IOSQES ||
> 	     nvmet_cc_iocqes(ctrl->cc) != NVME_NVM_IOCQES)) {
> 		ctrl->csts = NVME_CSTS_CFS;
> @@ -1278,7 +1278,7 @@ bool nvmet_host_allowed(struct nvmet_subsys *subsys, const char *hostnqn)
> 	if (subsys->allow_any_host)
> 		return true;
> 
> -	if (subsys->type == NVME_NQN_DISC) /* allow all access to disc subsys */
> +	if (nvmet_is_disc_subsys(subsys)) /* allow all access to disc subsys */
> 		return true;
> 
> 	list_for_each_entry(p, &subsys->hosts, entry) {
> @@ -1411,7 +1411,7 @@ u16 nvmet_alloc_ctrl(const char *subsysnqn, const char *hostnqn,
> 	 * Discovery controllers may use some arbitrary high value
> 	 * in order to cleanup stale discovery sessions
> 	 */
> -	if ((ctrl->subsys->type == NVME_NQN_DISC) && !kato)
> +	if (nvmet_is_disc_subsys(ctrl->subsys) && !kato)
> 		kato = NVMET_DISC_KATO_MS;
> 
> 	/* keep-alive timeout in seconds */
> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
> index 7143c7fa7464..93a86d6053ef 100644
> --- a/drivers/nvme/target/nvmet.h
> +++ b/drivers/nvme/target/nvmet.h
> @@ -576,6 +576,11 @@ static inline struct nvmet_subsys *nvmet_req_subsys(struct nvmet_req *req)
> 	return req->sq->ctrl->subsys;
> }
> 
> +static inline bool nvmet_is_disc_subsys(struct nvmet_subsys *subsys)
> +{
> +    return subsys->type == NVME_NQN_DISC;
> +}
> +
> #ifdef CONFIG_NVME_TARGET_PASSTHRU
> void nvmet_passthru_subsys_free(struct nvmet_subsys *subsys);
> int nvmet_passthru_ctrl_enable(struct nvmet_subsys *subsys);
> -- 
> 2.29.2
> 
> 
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme

Looks Good. 

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