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

Martin Wilck
2021-01-29T21:15:49Z

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.

Martin