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 <bmarz...@redhat.com>

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

Reviewed-by: Martin Wilck <mwi...@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 <mwi...@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

Reply via email to