Re: [RFC PATCH 3/5] call start_waiter_thread() before setup_multipath()

[Martin Wilck <mwilck@suse.com>]

On Fri, 2018-02-09 at 23:07 -0600, Benjamin Marzinski wrote:
> If setup_multipath() is called before the waiter thread has started,
> there is a window where a dm event can occur between when
> setup_multipath() updates the device state and when the waiter thread
> starts waiting for new events, causing the new event to be missed and
> the multipath device to not get updated.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

The window will be still there, but smaller than before.

Reviewed-by: Martin Wilck <mwilck@suse.com>





> ---
>  multipathd/main.c | 37 ++++++++++++++++++++-----------------
>  1 file changed, 20 insertions(+), 17 deletions(-)
> 
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 94b2406..efc39d7 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -321,7 +321,7 @@ set_multipath_wwid (struct multipath * mpp)
>  }
>  
>  static int
> -update_map (struct multipath *mpp, struct vectors *vecs)
> +update_map (struct multipath *mpp, struct vectors *vecs, int
> new_map)
>  {
>  	int retries = 3;
>  	char params[PARAMS_SIZE] = {0};
> @@ -351,6 +351,12 @@ retry:
>  	dm_lib_release();
>  
>  fail:
> +	if (new_map && (retries < 0 || start_waiter_thread(mpp,
> vecs))) {
> +		condlog(0, "%s: failed to create new map", mpp-
> >alias);
> +		remove_map(mpp, vecs, 1);
> +		return 1;
> +	}
> +
>  	if (setup_multipath(vecs, mpp))
>  		return 1;
>  
> @@ -395,12 +401,9 @@ add_map_without_path (struct vectors *vecs, char
> *alias)
>  
>  	vector_set_slot(vecs->mpvec, mpp);
>  
> -	if (update_map(mpp, vecs) != 0) /* map removed */
> +	if (update_map(mpp, vecs, 1) != 0) /* map removed */
>  		return NULL;
>  
> -	if (start_waiter_thread(mpp, vecs))
> -		goto out;
> -
>  	return mpp;
>  out:
>  	remove_map(mpp, vecs, PURGE_VEC);
> @@ -554,7 +557,7 @@ ev_add_map (char * dev, char * alias, struct
> vectors * vecs)
>  		if (mpp->wait_for_udev > 1) {
>  			condlog(2, "%s: performing delayed actions",
>  				mpp->alias);
> -			if (update_map(mpp, vecs))
> +			if (update_map(mpp, vecs, 0))
>  				/* setup multipathd removed the map
> */
>  				return 1;
>  		}
> @@ -865,6 +868,11 @@ retry:
>  	}
>  	dm_lib_release();
>  
> +	if ((mpp->action == ACT_CREATE ||
> +	     (mpp->action == ACT_NOTHING && start_waiter && !mpp-
> >waiter)) &&
> +	    start_waiter_thread(mpp, vecs))
> +			goto fail_map;
> +
>  	/*
>  	 * update our state from kernel regardless of create or
> reload
>  	 */
> @@ -873,11 +881,6 @@ retry:
>  
>  	sync_map_state(mpp);
>  
> -	if ((mpp->action == ACT_CREATE ||
> -	     (mpp->action == ACT_NOTHING && start_waiter && !mpp-
> >waiter)) &&
> -	    start_waiter_thread(mpp, vecs))
> -			goto fail_map;
> -
>  	if (retries >= 0) {
>  		condlog(2, "%s [%s]: path added to devmap %s",
>  			pp->dev, pp->dev_t, mpp->alias);
> @@ -1479,7 +1482,8 @@ missing_uev_wait_tick(struct vectors *vecs)
>  		if (mpp->wait_for_udev && --mpp->uev_wait_tick <= 0)
> {
>  			timed_out = 1;
>  			condlog(0, "%s: timeout waiting on creation
> uevent. enabling reloads", mpp->alias);
> -			if (mpp->wait_for_udev > 1 &&
> update_map(mpp, vecs)) {
> +			if (mpp->wait_for_udev > 1 &&
> +			    update_map(mpp, vecs, 0)) {
>  				/* update_map removed map */
>  				i--;
>  				continue;
> @@ -1511,7 +1515,7 @@ ghost_delay_tick(struct vectors *vecs)
>  			condlog(0, "%s: timed out waiting for active
> path",
>  				mpp->alias);
>  			mpp->force_udev_reload = 1;
> -			if (update_map(mpp, vecs) != 0) {
> +			if (update_map(mpp, vecs, 0) != 0) {
>  				/* update_map removed map */
>  				i--;
>  				continue;
> @@ -2169,14 +2173,13 @@ configure (struct vectors * vecs)
>  	 * start dm event waiter threads for these new maps
>  	 */
>  	vector_foreach_slot(vecs->mpvec, mpp, i) {
> -		if (setup_multipath(vecs, mpp)) {
> -			i--;
> -			continue;
> -		}
>  		if (start_waiter_thread(mpp, vecs)) {
>  			remove_map(mpp, vecs, 1);
>  			i--;
> +			continue;
>  		}
> +		if (setup_multipath(vecs, mpp))
> +			i--;
>  	}
>  	return 0;
>  }

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel