Nvmet discovery implementation. Questions and a patch / proposal.

[Sagi Grimberg]

On 3/1/22 09:05, Hannes Reinecke wrote:
> On 2/28/22 17:39, Mark Ruijter wrote:
>>
>> The current discovery implementation only works after a subsystem has 
>> been created and has been linked to a port.
>> As a result, 'nvme discover' from an initiator will only show a 
>> discovery device after the first subsystem has
>> been exported.
>>
>> The patch that I propose adds these features:
>> A. Make discovery work immediately after the target port has been 
>> configured, without needing to configure
>>    subsystems first.
>> B. Move the discovery_nqn configuration option to the port instead of 
>> having the same configuration option
>>    duplicated in every subsystem.
>> C. Allow to enable and disable a target port without needing to unlink 
>> target subsystems.
>>
> The current nvmet discovery implementation was designed to the original 
> NVMe spec, where a discovery controller could only return informations 
> about the attached subsystems.
> But with the recent changes to the spec a discovery controller can now 
> return information about itself (ie which port it's running on), and so
> it does makes sense to have it enabled even with no subsystems attached.
> 
> But: the current implementation only has a _single_ discovery subsystem 
> (nvmet_disc_subsys), which is shared across all configured subsystems.
> And there's a filter when generating the discovery log page such that 
> only the information for the current subsystem is returned (cf 
> nvmet_execute_disc_get_log_page()).
> So in effect each subsystem has its own discovery subsystem.
> 
> Moving the discovery nqn to the port will only confuse matters, as then 
> it's quite unclear what will happen if you change the discovery 
> subsystem nqn on a per-port basis; with the current design you would
> change it for _all_ discovery subsystem ...
> 
> So if you want to do something like this, you'd need to change the 
> implementation to have a discovery subsystem per port, and also change 
> the logic in drivers/nvme/target/discovery.c such that the discovery
> subsystems will return informations for _all_ configured subsystems.
> 
> It's actually something I wanted to do, too, as I didn't like the 
> current implementation :-)

Why aren't we just creating the discovery subsystem as a static configfs
subsystems and just link it to different ports?

The port listener on first linkage is a different matter afaict, don't
see a strong argument to make the port listener without any subsystems
exported anyways... maybe if discovery log change event is being used...