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

Christoph Hellwig
2022-05-10T06:33:23Z

Any chance you could respin this to actually report the correct
Identify data structure?

On Mon, Apr 11, 2022 at 08:18:54AM +0200, Christoph Hellwig wrote:
> 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.
---end quoted text---