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

[Christoph Hellwig <hch@lst.de>]

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---