[dm-devel] [PATCH 0/9] Add protocol specific config subsection

[Martin Wilck]

On Tue, 2022-04-12 at 17:01 -0500, Benjamin Marzinski wrote:
> On Tue, Apr 12, 2022 at 08:47:38PM +0000, Martin Wilck wrote:
> > 
> > To clarify once more: this is what I meant, built-in configs would
> > be
> > overridden, user configs wouldn't be. This is different from
> > "defaults", as "defaults" don't override hardware-specific built-
> > ins.
> 
> But what do you call a device config that is the result of merging
> (via
> merge_hwe()) a built-in and a non-built-in config.  Do we really want
> to
> track that some of the values of this merged config need to check the
> protocol section, and some don't? We could remove merging identical
> configs, but that simply makes it harder for users to figure out how
> their device will be configured from the configuration output.

Yes, I figured this might be tricky. My vague idea was to track the
origin with sort of a bit field inside the hwe's. Never mind now.

> I understand your idea. I'd just rather that it worked on all the
> device
> configs, instead of only the built-in ones. I think overriding only
> the
> built-in configs is needlessly complicated, both from a coding and
> from
> an explaining point of view.

I think that users are able to distinguish between built-in defaults
and settings they made explicitly. Being able to differentiate between
these in the "origin" log messages would also have a certain value.

But I said to you not to over-complicate matters, so I suppose I
shouldn't do that, either. If you think just using "overrides" is
sufficient, I'm fine with that.

> > > The biggest reason is that some of the builtin device configs do
> > > things
> > > like set no_path_retry to "queue". 
> > 
> > You don't need to use "overrides" for that:
> > 
> > devices {
> >         device {
> >                 vendor .*
> >                 product .*
> >                 no_path_retry 75
> >         }
> > }
> > You can follow up with more device entries that define exceptions
> > for
> > the general rule above. Am I missing something?
> > 
> > AFAICT the only thing you can do with "overrides" but not with the
> > trick above is override actual hardware-specific user configs, and
> > I
> > have a hard time figuring out why someone would work out detailed
> > device-specific configs just to override them again with a big
> > hammer.
> 
> Fair enough. I added the overrides section before you made paths have
> a
> vector of device configs. Back then, there was no way to have a
> device
> config that would work like your above example.  My original idea was
> to
> be able to have a special device section like this:
> 

Right. Didn't think about that.

> device {
>         all_devs yes
>         no_path_retry 75
> }
> 
> that would get merged with all the device sections. The overrides
> section was the compromise after my original idea was NAKed. We
> probably
> could deprecate the overrides section now that we have a vector of
> device configs. But then we shouldn't go putting the protocol stuff
> there. 

Well I guess as we have had the section for decades, we might as well
just leave it in. Sorry for distracting you with my rant.

Still to be determined whether "protocol" should simply go into
"overrides", or into a separate section (with the precedence semantics
you consider appropriate). I fine with either, so I guess it'll be
"overrides".

Regards
Martin