Re: [PATCH 4/4] nvmet: expose discovery subsystem

Hannes Reinecke
2022-05-11T06:27:25Z

On 5/10/22 20:59, Sagi Grimberg wrote:
>> Add a module option 'expose_discovery_subsys' to expose the
>> default discovery subsystem via configfs.
>> When this option is enabled the internal discovery subsystem
>> is disabled, and the admin has to create a discovery
>> subsystem manually.
>> The discovery subsystem then has to be linked into the ports
>> which should present the discovery subsystem.
>>
>> o- /
>>    o- ports
>>    | o- 2
>>    |   o- subsystems
>>    |     o- nqn.io-2
>>    | o- 3
>>    |   o- subsystems
>>    |     o- nqn.2014-08.org.nvmexpress.discovery
>>    |     o- nqn.io-1
>>    o- subsystems
>>      o- nqn.2014-08.org.nvmexpress.discovery
>>      | o- namespaces
>>      o- nqn.io-1
>>      | o- namespaces
>>      o- nqn.io-2
>>        o- namespaces
>>
>> So in this example the standard discovery service would be available on
>> port 3, presenting information about subsystem nqn.io-1.
>> Port 2 would not present a discovery service, but a controller can
>> connect
>> to subsystem nqn.io-2 on that port.
>
> So what is the behavior of this param?
>
> expose_discovery_subsys=1:
> both default and unique discovery subsystems function and behave
> in the same way, i.e. user created, linked to dedicated ports and
> answers log page info with subsystems on all ports they are linked to.
>
> expose_discovery_subsys=0:
> default discovery subsystem behaves like today but unique discovery
> subsystems behave differently (i.e. like first option?)
>
> This will be confusing to users I think.
>
> I would say that in case expose_discovery_subsys=0 unique discovery
> subsystems should still behave like the default discovery subsys,
> meaning return a log-page with only port local information.
>
> This way default and unique discovery subsystems are consistent with
> each other. Then the modparam could be something like:
>
> static int multiport_discovery_log_page;
> module_param(multiport_discovery_log_page, int, 0644);
> MODULE_PARM_DESC(multiport_discovery_log_page,
>          "Include subsystems from all linked ports in the "
>         "discovery log-page information. In this mode the "
>         "default discovery subsystem must also be created in "
>         "configfs like any other subsystem.");
>
> Thoughts?

My idea here was that each discovery subsystem will be presenting
informations about all ports it is linked in.
Consequently, to mimic the original behaviour you would create separate
discovery subsystems such that each port has its own discovery subsystems.

Plus your suggested module parameter would influence the default
discovery subsystem, too, right?
I tied it into exposed discovery subsystems on purpose, to allow the
user to control which ports are exposed to which discovery subsystem.

So, in the end, I'm not sure which way to go.
I'm fine with your suggestion, but I like the current patchset, too ...

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@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