[PATCH 35/35] monitor: add option --keep/-K

[Hannes Reinecke]

On 1/29/21 10:15 PM, Martin Wilck wrote:
> On Fri, 2021-01-29 at 13:11 -0800, Sagi Grimberg wrote:
>>
>>>>> If it does, it will recreate discovery controllers for every
>>>>> host_traddr/traddr/trsvcid tuple it finds. "--keep" semantics
>>>>> are
>>>>> only
>>>>> necessary for addresses on which no regular (non-discovery)
>>>>> connection
>>>>> exists.
>>>>
>>>> Wait, Maybe I'm missing something here, but are you saying that
>>>> for
>>>> every traddr/trsvcid it finds (both nvm and discovery) it will
>>>> attempt
>>>> to connect a discovery controller?
>>>>
>>>> If so, this is absolutely wrong.
>>>
>>> Currently, it tries to do that on startup, if (and only if) the
>>> --startup option is given. My expectation was that the connection
>>> attempts would simply fail if there was no discovery subsystem to
>>> connect to. Anyway, it's not the default behavior, and can be
>>> dropped
>>> completely if it's so bad that we shouldn't ever attempt to do it.
>>
>> IMO it needs to be dropped. I didn't understand this at first because
>> it never even occurred to me that such an assumption can be even
>> made.
>>
>>> If the service is started early on during boot, and event-based
>>> discovery works (i.e. we also have the mDNS part in place), this
>>> won't
>>> be necessary of course.
>>
>> This isn't necessary regardless. At best the discovery controller
>> endpoints should be obtained from discovery.conf or equivalent.
> 
> Understood.
> 
>> I don't even see how does this help in early boot anyways, how
>> do the existing controllers get connected?
> 
> For FC, it works today. You can start the monitor in the initrd, it
> will receive the fc_udev_device events, and autoconnect (just like
> traditional udev/systemd based discovery). For other transports, it'd
> be more work, as we'd need to do mDNS discovery from an initrd
> environment. I wouldn't say it's impossible though.
> 
mDNS isn't related to the initrd, mDNS is for discovery discovery :-)
For everything _but_ FC we have the problem that we don't know where to 
connect to for the initial discovery (ie we don't have a list of 
potential discovery controllers).
Currently we either have to specify the parameters manually, or add them 
to /etc/nvme/discovery.conf.
Which isn't _that_ dynamic.
mDNS will provide a way of promoting changes in any remote discovery 
controllers to the potential hosts, allowing the host to pick up 
information about potential discovery controllers from there.

So the monitor code would need to interact with mDNS to get the 
information, and should use that to establish discovery connections.

As this information is transient I would not tear down discovery 
controller connections on exit, but rather keep them alive.
That way a controller restart could pick up these existing connections 
and would not need to recreate them (which will be tricky as the monitor 
code doesn't have the information which connections to restart).

The alternative would be a callout to mDNS to get the current topology, 
but that doesn't exist yet.

Cheers,

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