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

[Benjamin Marzinski]

On Mon, Nov 02, 2020 at 07:11:06PM -0600, Benjamin Marzinski wrote:
> 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.

Of course, that doesn't mean it wouldn't be useful to always do the
check on vpd page c9 for devices that do indicate support in vpd 0. So,
yes I agree that makes sense so unless Netapp has any objections, I can
change the patch to also check c9 if vpd page 0 says it's supported.

-Ben

> 
> -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
> > 
> 
> --
> dm-devel mailing list
> dm-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel