Re: [PATCH 2/3] nvmet: restrict setting of discovery_nqn to discovery subsystem

[Christoph Hellwig <hch@lst.de>]

On Mon, Mar 14, 2022 at 11:53:32AM +0100, Hannes Reinecke wrote:
> Changing the discovery NQN only makes sense for the discovery subsystem,
> as any other subsystem can only have one (fixed) subsystem NQN.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/nvme/target/configfs.c | 32 +++++++++++++++++++++++++++-----
>  1 file changed, 27 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
> index 9b94af2444b2..58124abdbf62 100644
> --- a/drivers/nvme/target/configfs.c
> +++ b/drivers/nvme/target/configfs.c
> @@ -1310,6 +1310,12 @@ 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 = to_subsys(item);
> +
> +	/* Only the discovery subsystem can have a different subsystem NQN */
> +	if (subsys != nvmet_disc_subsys)
> +		return -EBUSY;
> +
>  	return snprintf(page, PAGE_SIZE, "%s\n",
>  			nvmet_disc_subsys->subsysnqn);

This completely breaks the existing ABI.  This attribute is shown on
the "normal subsystems" and in fact only exposed for those so far.

It is a fairly recent addition by you and without nvmetcli support, so
maybe we can revert this entire attribute to redo discovery controller
handling, but as-is this patch seems rather broken.