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 <mwi...@suse.com>
> > Cc: Benjamin Marzinski <bmarz...@redhat.com>
> > Cc: Christophe Varoqui <christophe.varo...@opensvc.com>
> > Cc: DM-DEVEL ML <dm-devel@lists.linux.dev>
> > Signed-off-by: Xose Vazquez Perez <xose.vazq...@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.

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
> 


Reply via email to