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

[Benjamin Marzinski]

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().

-Ben
 
>  	ret = get_target_port_group(pp, timeout);
>  	if (ret < 0 || get_asymmetric_access_state(pp, ret, timeout) < 0) {
> +		int state;
> +
> +		if (ret == -RTPG_INQUIRY_FAILED)
> +			return;
> +
> +		state = path_offline(pp);
> +		if (state == PATH_DOWN || state == PATH_PENDING)
> +			return;
> +
>  		pp->tpgs = TPGS_NONE;
>  		return;
>  	}
> diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
> index 27e8d68a..a5fc6ba0 100644
> --- a/libmultipath/propsel.c
> +++ b/libmultipath/propsel.c
> @@ -432,12 +432,26 @@ int select_hwhandler(struct config *conf, struct multipath *mp)
>  	static const char tpgs_origin[]= "(setting: autodetected from TPGS)";
>  	char *dh_state;
>  	int i;
> -	bool all_tpgs = true;
> +	bool all_tpgs = true, one_tpgs = false;
>  
>  	dh_state = &handler[2];
>  
> -	vector_foreach_slot(mp->paths, pp, i)
> -		all_tpgs = all_tpgs && (path_get_tpgs(pp) > 0);
> +	/*
> +	 * TPGS_UNDEF means that ALUA support couldn't determined either way
> +	 * yet, probably because the path was always down.
> +	 * If at least one path does have TPGS support, and no path has
> +	 * TPGS_NONE, assume that TPGS would be supported by all paths if
> +	 * all were up.
> +	 */
> +	vector_foreach_slot(mp->paths, pp, i) {
> +		int tpgs = path_get_tpgs(pp);
> +
> +		all_tpgs = all_tpgs && tpgs != TPGS_NONE;
> +		one_tpgs = one_tpgs ||
> +			(tpgs != TPGS_NONE && tpgs != TPGS_UNDEF);
> +	}
> +	all_tpgs = all_tpgs && one_tpgs;
> +
>  	if (mp->retain_hwhandler != RETAIN_HWHANDLER_OFF) {
>  		vector_foreach_slot(mp->paths, pp, i) {
>  			if (get_dh_state(pp, dh_state, sizeof(handler) - 2) > 0
> -- 
> 2.24.0