Re: [PATCH 1/3] nvmet: handle admin default command set identifier

Christoph Hellwig
2022-04-11T06:19:03Z

On Sun, Apr 10, 2022 at 08:12:47PM -0700, Chaitanya Kulkarni wrote:
> -static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
> +static void nvmet_execute_identify_cns_cs_ctrl_nvm(struct nvmet_req *req)

No, this does not work.

nvmet_execute_identify_ctrl handles the
"Identify Controller data structure" from the base spec, CNS 01h, no CSI.

The "I/O Command Set Specific Identify Controller data structure" from
the NVM Command set spec (CNS 06h, CSI 00h) is an entirely different
data structure and needs separate handling.

I'd also drop the cns here and in the other names, every identify command
has a CNS value associated with it and these names are already getting
rather long.

> + if (IS_ENABLED(CONFIG_BLK_DEV_ZONED)) {
> + nvmet_execute_identify_cns_cs_ctrl_zns(req);
> + return;
> }
> + break;

Nit: I'd invert the IS_ENABLED check. This saves one line, and one
level of indentation and just reads easier as well.