Re: [PATCH RFC] multipath-tools: add Generic SCSI in hwtable

[Benjamin Marzinski <bmarzins@redhat.com>]

On Tue, Mar 18, 2025 at 12:08:32PM +0100, Martin Wilck wrote:
> On Mon, 2025-03-17 at 16:00 -0400, Benjamin Marzinski wrote:
> > On Mon, Mar 17, 2025 at 06:33:51PM +0100, Xose Vazquez Perez wrote:
> > > Cc: Martin Wilck <mwilck@suse.com>
> > > Cc: Benjamin Marzinski <bmarzins@redhat.com>
> > > Cc: Christophe Varoqui <christophe.varoqui@opensvc.com>
> > > Cc: DM-DEVEL ML <dm-devel@lists.linux.dev>
> > > Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
> > > ---
> > >  libmultipath/hwtable.c | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > > 
> > > diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c
> > > index f8cf3fa9..34b1fd2f 100644
> > > --- a/libmultipath/hwtable.c
> > > +++ b/libmultipath/hwtable.c
> > > @@ -79,6 +79,17 @@
> > >  #endif
> > >  
> > 
> > If we wanted to make these changes, we could just change
> > DEFAULT_PGPOLICY, DEFAULT_FAILBACK, and DEFAULT_NO_PATH_RETRY. But I
> > still think that for completely unknown devices, we should stick with
> > our current defaults. They're safe and I haven't heard any complaints
> > about them.
> 
> I agree. The point of the hwtable is to change the defaults for
> specific devices. Using "vendor = .*" and "product = ".*" is against
> the spirit of the hwtable. 
> 
> Unless I am mistaken, this  change would override user settings in the
> "defaults" section of multipath.conf. This happens a lot already for
> those systems that do have a specific hwtable entry, and I am sure that
> that comes as a surprise for users (it has confused myself a
> significant couple of times). By adding these catch-all entries, we'd
> make the "defaults" section ineffective for any setting that's
> available in the "devices" section, too.
> 
> Thinking about it, that might actually be a good thing, as it would
> eliminate the uncertainty whether or not a given "defaults" setting
> would take effect for a given storage device. Maybe we should
> officially stop supporting settings in "defaults" that can be
> overridden by device-spefic settings, just to reduce confusion in this
> area, and recommend using a catch-all device setting (in
> multipath.conf, not in the built-in hwtable) instead.

The device configs from /etc/multipath.conf will override values in the
built-in hwtable. So a catch-all device config in /etc/multipath.conf is
the same as putting options in the overrides section, not the defaults
section.

Now that we changed how hwentry configs work, we could probably
deprecate the overrides section, but I don't think we can replace the
defaults section.

right?

-Ben
 
> Anyway, that would be a possible cause for major breakage, as user
> settings in the field would suddenly stop being effective. We can't
> possibly just let it slip in with a minor hwtable patch like this.
> 
> Regards
> Martin
> 
> > -Ben
> > 
> > >  static struct hwentry default_hw[] = {
> > > +	/*
> > > +	 * Generic SCSI devices
> > > +	 */
> > > +	{
> > > +		/* Generic SCSI */
> > > +		.vendor        = ".*",
> > > +		.product       = ".*",
> > > +		.pgpolicy      = GROUP_BY_PRIO,
> > > +		.pgfailback    = -FAILBACK_IMMEDIATE,
> > > +		.no_path_retry = 30,
> > > +	},
> > >  	/*
> > >  	 * Generic NVMe devices
> > >  	 *
> > > -- 
> > > 2.48.1
> >