[PATCH 3/3] nvmet: include all configured ports in the discovery log page

[Hannes Reinecke]

On 4/11/22 12:54, Sagi Grimberg wrote:
> 
>> When a per-port discovery subsystem is used we should include
>> all configured ports in the discovery log page, not just that one
>> through which the controller was connected.
>>
>> - /
>>    o- ports
>>    | o- 1 .. [trtype=tcp, traddr=127.0.0.1, trsvcid=8009]
>>    | | o- subsystems
>>    | |   o- nqn.discovery
>>    | o- 2 .. [trtype=tcp, traddr=127.0.0.1, trsvcid=4420]
>>    | | o- subsystems
>>    | |   o- nqn.discovery
>>    | |   o- nqn.io-1
>>    | o- 3 .. [trtype=tcp, traddr=127.0.0.1, trsvcid=4421]
>>    |   o- subsystems
>>    |     o- nqn.io-2
>>    o- subsystems
>>      o- nqn.discovery
>>      o- nqn.io-1
>>      | o- namespaces
>>      o- nqn.io-2
>>        o- namespaces
>>
>> A discovery on port 8009 will return information about
>> - nqn.discovery at port 8009
>> - nqn.discovery at port 4420
>> - nqn.io-1 at port 4420
>> A discovery on port 4420 will return the same information.
>> A discovery on port 4421 will return information about
>> - standard discovery subsystem on port 4421
>> - nqn.io-2 on port 4421
> 
> This is a different functionality than supporting unique discovery
> NQNs.
> 

Yes.

> If I want in your example to use a unique discovery subsystem but to
> report only on the port local I have to use two different unique
> discovery subsystems. Which is different than the standard discovery
> subsystem, why?
> 
Because this was a logical step from the way I've implemented unique 
discovery subsystems.

As discovery subsystems (in my implementation) are treated like 'normal' 
subsystems, they should be able to be linked to ports. If they are not 
linked we would have to implement some magic on which ports this 
subsystem will show up, and also making it inconsistent with all other 
subsystems.

> I'd submit to you that IMO unique discovery subsystems should not behave
> differently than the standard discovery subsystem. Please explain why
> do you think that unique discovery subsystems should behave differently
> with respect to the content of the log page.
> 
Well, arguably.
But I haven't been able to find a good way of keeping the original 
behaviour _and_ support unique discovery subsystems.

Sure, one could implement some magic variable in configfs like 
discovery_nqn, but that would have to be in configfs root directory, and 
really doesn't match up with current configfs layout.
One could go with your suggestion of having a discovery_subsystem entry 
per port, but we're back to the issue which killed my original patch:
How do we ensure uniqueness?
Each NQN here _need_ to be validated with the existing (and future) I/O 
subsystem NQNs, as they need to be unique. That requires a lookup over 
all ports and all referenced subsystems.
And we have to figure out if we allow for duplicated discovery NQNs; one 
can find arguments for either side.

That's why I came up with this approach.

And that's also why this is a separate patch, as it does expand existing 
functionality :-)

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare at suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer