[dm-devel] [PATCH 4/5] libmultipath: only read 0xc9 vpd page for devices with rdac checker

[Benjamin Marzinski]

On Fri, Oct 30, 2020 at 09:12:46PM +0000, Martin Wilck wrote:
> On Fri, 2020-10-23 at 16:15 -0500, Benjamin Marzinski wrote:
> > Only rdac arrays support 0xC9 vpd page inquiries. All other arrays
> > will
> > return a failure. Since all rdac arrays in the the built-in device
> > configuration explicitly set the RDAC path checker, and almost all
> > other
> > scsi arrays do not set a path checker, it makes sense to only do the
> > rdac inquiry when detecting array capabilities if the array's path
> > checker is explicitly set to rdac.
> > 
> > Multipath was doing the check if either the path checker was set to
> > rdac, or no path checker was set.  This means that for almost all
> > non-rdac arrays, multipath was issuing a bad inquiry. This was
> > annoying
> > users.
> 
> This is sort of funny. We created the entire check_rdac() feature in
> order to autodetect RDAC arrays. If we limit the autodetection to those
> arrays that have "rdac" set already, why would we do this at all? We
> could simply go with the hwtable / config file settings, as we used to
> before check_rdac() was created. All we'd need to do is use "alua" prio
> for arrays with "rdac" checker. What am I missing here?
> 
> I thought that this autodetection was intended because there are many
> Netapp OEMs which we may not all have included in the hwtable (thus
> having no path_checker set). For those, we'd choose the wrong settings
> by default with this patch, only to avoid some error messages about
> unsupported VPDs. I'm not convinced that that's a good idea.
> 
> Suggestion: we could try to retrieve VPD 0 (supported VPDs) before
> checking VPD 0xc9. That would avoid the errors on non-Netapp hardware,
> while still allowing us to autodetect RDAC systems that are missing in
> the hwtable.

When this idea was brought up in discussions about how to solve this,
there was a worry that older rdac devices might not not support vpd page
0 correctly. I'm not sure how valid that worry is.

-Ben

> 
> Regards
> Martin
> 
> > 
> > Cc: Steve Schremmer <sschremm at netapp.com>
> > Cc: NetApp RDAC team <ng-eseries-upstream-maintainers at netapp.com>
> > Signed-off-by: Benjamin Marzinski <bmarzins at redhat.com>
> > ---
> >  libmultipath/propsel.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
> > index fa4ac5d9..90a77d77 100644
> > --- a/libmultipath/propsel.c
> > +++ b/libmultipath/propsel.c
> > @@ -501,7 +501,7 @@ check_rdac(struct path * pp)
> >  	if (pp->bus != SYSFS_BUS_SCSI)
> >  		return 0;
> >  	/* Avoid ioctl if this is likely not an RDAC array */
> > -	if (__do_set_from_hwe(checker_name, pp, checker_name) &&
> > +	if (!__do_set_from_hwe(checker_name, pp, checker_name) ||
> >  	    strcmp(checker_name, RDAC))
> >  		return 0;
> >  	len = get_vpd_sgio(pp->fd, 0xC9, 0, buff, 44);
> 
> -- 
> Dr. Martin Wilck <mwilck at suse.com>, Tel. +49 (0)911 74053 2107
> SUSE  Software Solutions Germany GmbH
> HRB 36809, AG Nürnberg GF: Felix
> Imendörffer
>