Re: [PATCH] libmultipath: unset detect_checker for clariion / Unity arrays

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jun 08, 2022 at 07:56:27AM +0000, Martin Wilck wrote:
> On Tue, 2022-06-07 at 17:45 -0500, Benjamin Marzinski wrote:
> > Dell EMC would like to always use the emc_clariion checker. Currently
> > detect_checker will switch the checker to TUR for Unity arrays.
> > This can cause problems on some setups with replicated Unity LUNs,
> > which are handled correctly the the emc_checker, but not the TUR
> > checker.
> > 
> > Cc: vincent.chen1@xxxxxxxx
> > Signed-off-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx>
> 
> This points us to a flaw in our logic.
> 
> It was wrong in the first place to have autodetection take precedence,
> even over "overrides". The effect for users is that whenever
> "path_checker" is set, "detect_checker no" must also be set, which
> is highly surprising and adds no benefit.
> 
> We should assume that if a device has an explicit checker configured
> either in the device configuration, overrides, or the hwtable, this
> checker must be used, and fall back to autodetection only if this is
> not the case.
> 
> So while this patch is alright, I'd prefer a patch that fixes the
> logic.

I'm not sure that this is a good idea. IIRC, the current method was an
intentional choice.  The idea was that if a checker was autodetected, we
would use that. If not, we would fall back to the user defined checker.
For the most part this is useful for arrays that could be run in ALUA or
non-alua mode.  Changing the priority of detect_checker will suddenly
change how these arrays are configured.  Users that configured a
failback checker for cases where their arrays weren't in ALUA mode would
suddenly find that their arrays were always using the fallback method. 

Now I'm not saying that the original logic was the best option. But I am
saying that it hasn't caused many issues over the years that it's been
in existance. There are a number of arrays in the builtin hardware table
that define checkers. I assume that they either don't support ALUA, or
they are happy with only using their configured checker when their
arrays aren't in ALUA mode. I would rather fix the remaining cases where
the existing priority gives the wrong answer than suddenly change how
things work, in a way that could suddenly break things for an unknown
(but quite possibly large) number of existing users.

> (The same could be said for detect_prio, but I don't want to make
> outrageous demands).

The above arguments apply here, only moreso.
 
-Ben

> Martin
> 
> 
> 
> 
> 
> > ---
> >  libmultipath/hwtable.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c
> > index 39daadc2..415bf683 100644
> > --- a/libmultipath/hwtable.c
> > +++ b/libmultipath/hwtable.c
> > @@ -350,6 +350,7 @@ static struct hwentry default_hw[] = {
> >                 .no_path_retry = (300 / DEFAULT_CHECKINT),
> >                 .checker_name  = EMC_CLARIION,
> >                 .prio_name     = PRIO_EMC,
> > +               .detect_checker = DETECT_CHECKER_OFF,
> >         },
> >         {
> >                 /* Invista / VPLEX */
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel





[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux