| Message ID | 1518239251-29392-4-git-send-email-bmarzins@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Mike Snitzer |
| Headers | show |
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; > }
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; }
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> --- multipathd/main.c | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-)