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

[Martin Wilck]

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.

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