[PATCH libnvme] fabrics: don't pass sqflow_disable if the kernel does not support the option

[Daniel Wagner]

On Mon, Jul 24, 2023 at 06:35:55PM +0300, Sagi Grimberg wrote:
> > How do we know options is initialized here? It looks like that's done
> > in __nvmf_supported_options(), called from nvmf_add_ctrl(), which is
> > called a few lines later in this function.

I've paired __nvmf_supported_options() with build_options(). We can
move the call to __nvmf_supported_options() further up if it helps.

> This is for connecting discovery log entries, so the kernel cap was
> already read once for the discovery controller.
> 
> Although granted, this can be udev driven on an existing discovery
> controller. Probably need to do it regardless of adding a controller.
> 
> I remember when adding the capability to check supported opts I added
> an interface to do it per feature. Not sure why it was changed, it is
> much more sane to just read it on demand given that there is just a
> simple and short string copy in the kernel...

I was trying to avoid to do a open/read/parse/close cycle for each argument we
are going append in the build_options() function.

> Ideally this would become:
> 	if (e->treq & NVMF_TREQ_DISABLE_SQFLOW &&
> 	    nvmf_check_cap("disable_sqflow"))
> 		c->cfg.disable_sqflow = true;
> 	else
> 		c->cfg.disable_sqflow = false;
> 
> Daniel? Any reason why we are caching these opts in an executable
> with a short lifetime? And apparently we need to be very precise when
> we rely on this cache to be stable.

Ah I see, in this case let's bring nvmf_check_cap() (although I couldn't find it
anywhere... I can remember I saw those patches)

> If you prefer to cache it, at least do it in the start of every
> fabrics callback so we don't need to understand when this cache
> is valid...

I don't want to cache it, I just wanted to avoid the above mentioned
open/read/parse/close cycle for each build options.