[dm-devel] [PATCH 5/5] libmultipath: fix ALUA autodetection when paths are down

[Benjamin Marzinski]

On Wed, Nov 20, 2019 at 12:33:15PM +0000, Martin Wilck wrote:
> On Tue, 2019-11-19 at 16:29 -0600, Benjamin Marzinski wrote:
> > On Fri, Nov 15, 2019 at 02:41:54PM +0000, Martin Wilck wrote:
> > > From: Martin Wilck <mwilck at suse.com>
> > > 
> > > If a single path was offline when detect_alua() was called,
> > > multipathd would assume ALUA was generally unsupported.
> > > 
> > > Fix that by assuming that if at least one path has ALUA support and
> > > no path explicitly does not have it, ALUA is supported.
> > > 
> > > Signed-off-by: Martin Wilck <mwilck at suse.com>
> > > ---
> > >  libmultipath/discovery.c | 22 +++++++++++++++++++++-
> > >  libmultipath/propsel.c   | 20 +++++++++++++++++---
> > >  2 files changed, 38 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> > > index 4288c9fd..5f41dcb7 100644
> > > --- a/libmultipath/discovery.c
> > > +++ b/libmultipath/discovery.c
> > > @@ -871,6 +871,10 @@ get_serial (char * str, int maxlen, int fd)
> > >  	return 1;
> > >  }
> > >  
> > > /*
> > > + * Side effect: sets pp->tpgs if it could be determined.
> > > + * If ALUA calls fail because paths are unreachable, pp->tpgs
> > > remains unchanged.
> > > + */
> > >  static void
> > >  detect_alua(struct path * pp)
> > >  {
> > > @@ -881,12 +885,28 @@ detect_alua(struct path * pp)
> > >  	if (sysfs_get_timeout(pp, &timeout) <= 0)
> > >  		timeout = DEF_TIMEOUT;
> > >  
> > > -	if ((tpgs = get_target_port_group_support(pp, timeout)) <= 0) {
> > > +	tpgs = get_target_port_group_support(pp, timeout);
> > > +	if (tpgs == -RTPG_INQUIRY_FAILED)
> > > +		return;
> > > +	else if (tpgs <= 0) {
> > >  		pp->tpgs = TPGS_NONE;
> > >  		return;
> > >  	}
> > > +
> > > +	if (pp->fd == -1 || pp->offline)
> > > +		return;
> > > +
> >  
> > This is just a nitpick, but won't tpgs already be
> > -RTPG_INQUIRY_FAILED
> > if pp->fd == -1. This check makes more sense before
> > get_target_port_group_support().
> 
> Not really, because get_target_port_group_support() normally obtains
> INQUIRY data from sysfs, which can return something reasonable even
> if fd == -1 (e.g. if the path is temporarily offline). In particular,
> it could indicate that the device has no TPGS support. That's why I
> call it first.

Oops. My bad.

Reviewed-by: Benjamin Marzinsk <bmarzins at redhat.com>

-Ben

> 
> Regards,
> Martin
>