[dm-devel] multipathd: move 'filter_devnode' under vector lock
Christophe Varoqui
christophe.varoqui at opensvc.com
Wed May 11 17:36:39 UTC 2016
Merged
On Wed, May 11, 2016 at 7:18 PM, Benjamin Marzinski <bmarzins at redhat.com>
wrote:
> On Wed, May 11, 2016 at 12:35:41PM +0200, Hannes Reinecke wrote:
> > Ben Marzinski pointed out that filter_devnode() is used
> > without any lock or configuration settings in uev_trigger(),
> > and hence might be invalid when processing events during
> > reconfiguration.
> > So move it into the individual functions and handle it
> > with the vector lock held.
>
> ACK
>
> -Ben
>
> >
> > Signed-off-by: Hannes Reinecke <hare at suse.com>
> > ---
> > multipathd/main.c | 45 +++++++++++++++++++++++++++------------------
> > 1 file changed, 27 insertions(+), 18 deletions(-)
> >
> > diff --git a/multipathd/main.c b/multipathd/main.c
> > index 58e8854..2c7486d 100644
> > --- a/multipathd/main.c
> > +++ b/multipathd/main.c
> > @@ -580,6 +580,11 @@ uev_add_path (struct uevent *uev, struct vectors *
> vecs)
> > pthread_cleanup_push(cleanup_lock, &vecs->lock);
> > lock(vecs->lock);
> > pthread_testcancel();
> > + if (filter_devnode(conf->blist_devnode, conf->elist_devnode,
> > + uev->kernel) > 0) {
> > + ret = 0;
> > + goto out_unlock;
> > + }
> > pp = find_path_by_dev(vecs->pathvec, uev->kernel);
> > if (pp) {
> > int r;
> > @@ -637,6 +642,7 @@ uev_add_path (struct uevent *uev, struct vectors *
> vecs)
> > free_path(pp);
> > ret = 1;
> > }
> > +out_unlock:
> > lock_cleanup_pop(vecs->lock);
> > return ret;
> > }
> > @@ -780,22 +786,23 @@ fail:
> > static int
> > uev_remove_path (struct uevent *uev, struct vectors * vecs)
> > {
> > - struct path *pp;
> > - int ret;
> > + struct path *pp = NULL;
> > + int ret = 0;
> >
> > condlog(2, "%s: remove path (uevent)", uev->kernel);
> > pthread_cleanup_push(cleanup_lock, &vecs->lock);
> > lock(vecs->lock);
> > pthread_testcancel();
> > - pp = find_path_by_dev(vecs->pathvec, uev->kernel);
> > - if (pp)
> > - ret = ev_remove_path(pp, vecs);
> > - lock_cleanup_pop(vecs->lock);
> > - if (!pp) {
> > - /* Not an error; path might have been purged earlier */
> > - condlog(0, "%s: path already removed", uev->kernel);
> > - return 0;
> > + if (filter_devnode(conf->blist_devnode, conf->elist_devnode,
> > + uev->kernel) == 0) {
> > + pp = find_path_by_dev(vecs->pathvec, uev->kernel);
> > + if (pp)
> > + ret = ev_remove_path(pp, vecs);
> > + else
> > + /* Not an error; path might have been purged
> earlier */
> > + condlog(0, "%s: path already removed",
> uev->kernel);
> > }
> > + lock_cleanup_pop(vecs->lock);
> > return ret;
> > }
> >
> > @@ -905,7 +912,7 @@ uev_update_path (struct uevent *uev, struct vectors
> * vecs)
> > ro = uevent_get_disk_ro(uev);
> >
> > if (ro >= 0) {
> > - struct path * pp;
> > + struct path * pp = NULL;
> > struct multipath *mpp = NULL;
> >
> > condlog(2, "%s: update path write_protect to '%d'
> (uevent)",
> > @@ -918,6 +925,10 @@ uev_update_path (struct uevent *uev, struct vectors
> * vecs)
> > * need to be at the same indentation level, hence
> > * this slightly convoluted codepath.
> > */
> > + if (filter_devnode(conf->blist_devnode,
> conf->elist_devnode,
> > + uev->kernel) > 0) {
> > + goto out_unlock;
> > + }
> > pp = find_path_by_dev(vecs->pathvec, uev->kernel);
> > if (pp) {
> > if (pp->initialized == INIT_REQUESTED_UDEV) {
> > @@ -937,11 +948,13 @@ uev_update_path (struct uevent *uev, struct
> vectors * vecs)
> > uev->kernel, mpp->alias, retval);
> > }
> > }
> > + out_unlock:
> > lock_cleanup_pop(vecs->lock);
> > if (!pp) {
> > - condlog(0, "%s: spurious uevent, path not found",
> > - uev->kernel);
> > - return 1;
> > + if (retval)
> > + condlog(0, "%s: spurious uevent, path not
> found",
> > + uev->kernel);
> > + return retval;
> > }
> > if (retval == 2)
> > return uev_add_path(uev, vecs);
> > @@ -1059,10 +1072,6 @@ uev_trigger (struct uevent * uev, void *
> trigger_data)
> > /*
> > * path add/remove event
> > */
> > - if (filter_devnode(conf->blist_devnode, conf->elist_devnode,
> > - uev->kernel) > 0)
> > - goto out;
> > -
> > if (!strncmp(uev->action, "add", 3)) {
> > r = uev_add_path(uev, vecs);
> > goto out;
> > --
> > 2.6.6
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/dm-devel/attachments/20160511/e97e58d7/attachment.htm>
More information about the dm-devel
mailing list