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

Sagi Grimberg sagi at grimberg.me
Tue Mar 15 01:57:35 PDT 2022


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

I don't think we even want this to be on a per-subsystem at all.

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

I think we could, I doubt anyone uses this interface at this point...



More information about the Linux-nvme mailing list