| Message ID | 1484200347-11188-5-git-send-email-tang.junhui@zte.com.cn (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | christophe varoqui |
| Headers | show |
On Thu, Jan 12, 2017 at 01:52:20PM +0800, tang.junhui@zte.com.cn wrote: > From: tang.junhui <tang.junhui@zte.com.cn> > > Usually calling domap() in ev_add_path() is needed, but only last path > need to call domap() in processing for merged uevents to reduce the > count of calling domap() and promote efficiency. So add input parameter > need_do_map to indicate whether need calling domap() in ev_add_path(). With the addition of checking if the merge_id equals the wwid, you are protected against accidentially merging paths that shouldn't be merged, which is great. But setting need_do_map for these paths doesn't completely make sure that if the wwid and merge_id differ, you will always call domap. A contrived situation where this fails would look like: add path1, path2, path3 where merge_id equals the wwid for path1 and path2, but there is a different wwid for path3. In this case, you would call domap on just the multipath device for path3, but since path1 and path2 matched the merge_id, they wouldn't force a call to domap. A less contrived example would be add path1, path2, path3, path4 Where these were devices that were actually pointing at two different LUNs, but all set ID_SERIAL the same. path1 and path2 pointed to one LUN, while path3 and path4 pointed to another LUN. In this case the wwid of path1 and path2 matched the merge_id, while the wwid of path3 and path4 was different. In this case you would call domap twice, on both path3 and path4, but nothing would call domap to create a multipath device for path1 and path2. In general, the code to set need_do_map if the wwid and merge_id don't match only works if either none of the device in a merged set have wwids that match the merge_id, or if the last device has a wwid that matches the merge_id. If there are devices with wwids that match the merge_id, but the last device in the set isn't one of them, then some devices will not get a multipath device created for them. Now, I don't know of any device that works like my above example, so your code will likely work fine for all real-world devices. Also, fixing this is a pain, as you don't find out until processing the last path in a set that things went wrong, and then you would have to go back and run the skipped functions on one of the paths you have already processed. The easy way to fix it is to use the other possibility that I mentioned before, which is to not have the merge_id, and just use the udev provided wwid, instead of fetching it from pathinfo. Like I mentioned, if you do this, you want to make sure that you only try to grab the wwid from the udev events for devices with the correct kernel name: ID_SERIAL only for "sd.*" devices, and ID_UID only for "dasd.*" devices. I also think that this should be configurable. Otherwise, you can either go through the messy work of calling domap correctly when the last device of a set has a wwid that doesn't match the merge_id, or we can decide that this won't acutally cause problems with any known device, and punt fixing it for now. And if it causes problems with some future devices, we can deal with it then. -Ben > > Change-Id: I7f33de49a1d89d91180dcb95cd94e4944ae7ff36 > Signed-off-by: tang.wenjun <tang.wenjun3@zte.com.cn> > --- > multipathd/cli_handlers.c | 3 ++- > multipathd/main.c | 47 ++++++++++++++++++++++++++++++++++++----------- > multipathd/main.h | 2 +- > 3 files changed, 39 insertions(+), 13 deletions(-) > > diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c > index b0eeca6..3a46c09 100644 > --- a/multipathd/cli_handlers.c > +++ b/multipathd/cli_handlers.c > @@ -670,7 +670,8 @@ cli_add_path (void * v, char ** reply, int * len, void * data) > pp->checkint = conf->checkint; > } > put_multipath_config(conf); > - return ev_add_path(pp, vecs); > + return ev_add_path(pp, vecs, 1); > + > blacklisted: > *reply = strdup("blacklisted\n"); > *len = strlen(*reply) + 1; > diff --git a/multipathd/main.c b/multipathd/main.c > index adc3258..ebd7de1 100644 > --- a/multipathd/main.c > +++ b/multipathd/main.c > @@ -608,7 +608,7 @@ ev_remove_map (char * devname, char * alias, int minor, struct vectors * vecs) > } > > static int > -uev_add_path (struct uevent *uev, struct vectors * vecs) > +uev_add_path (struct uevent *uev, struct vectors * vecs, int need_do_map) > { > struct path *pp; > int ret = 0, i; > @@ -640,9 +640,18 @@ uev_add_path (struct uevent *uev, struct vectors * vecs) > r = pathinfo(pp, conf, > DI_ALL | DI_BLACKLIST); > put_multipath_config(conf); > - if (r == PATHINFO_OK) > - ret = ev_add_path(pp, vecs); > - else if (r == PATHINFO_SKIPPED) { > + if (r == PATHINFO_OK) { > + /* > + * Make sure merging use the correct wwid > + * Othterwise calling domap() > + */ > + if (!need_do_map && > + uev->merge_id && > + strcmp(uev->merge_id, pp->wwid)) > + need_do_map = 1; > + > + ret = ev_add_path(pp, vecs, need_do_map); > + } else if (r == PATHINFO_SKIPPED) { > condlog(3, "%s: remove blacklisted path", > uev->kernel); > i = find_slot(vecs->pathvec, (void *)pp); > @@ -681,7 +690,16 @@ uev_add_path (struct uevent *uev, struct vectors * vecs) > conf = get_multipath_config(); > pp->checkint = conf->checkint; > put_multipath_config(conf); > - ret = ev_add_path(pp, vecs); > + /* > + * Make sure merging use the correct wwid > + * Othterwise calling domap() > + */ > + if (!need_do_map && > + uev->merge_id && > + strcmp(uev->merge_id, pp->wwid)) > + need_do_map = 1; > + > + ret = ev_add_path(pp, vecs, need_do_map); > } else { > condlog(0, "%s: failed to store path info, " > "dropping event", > @@ -699,7 +717,7 @@ uev_add_path (struct uevent *uev, struct vectors * vecs) > * 1: error > */ > int > -ev_add_path (struct path * pp, struct vectors * vecs) > +ev_add_path (struct path * pp, struct vectors * vecs, int need_do_map) > { > struct multipath * mpp; > char params[PARAMS_SIZE] = {0}; > @@ -767,6 +785,13 @@ rescan: > /* persistent reservation check*/ > mpath_pr_event_handle(pp); > > + if (!need_do_map) > + return 0; > + > + if (!dm_map_present(mpp->alias)) { > + mpp->action = ACT_CREATE; > + start_waiter = 1; > + } > /* > * push the map to the device-mapper > */ > @@ -995,7 +1020,7 @@ uev_update_path (struct uevent *uev, struct vectors * vecs) > } > > if (pp->initialized == INIT_REQUESTED_UDEV) > - retval = uev_add_path(uev, vecs); > + retval = uev_add_path(uev, vecs, 1); > else if (mpp && ro >= 0) { > condlog(2, "%s: update path write_protect to '%d' (uevent)", uev->kernel, ro); > > @@ -1150,7 +1175,7 @@ uev_trigger (struct uevent * uev, void * trigger_data) > put_multipath_config(conf); > > if (!strncmp(uev->action, "add", 3)) { > - r = uev_add_path(uev, vecs); > + r = uev_add_path(uev, vecs, 1); > goto out; > } > if (!strncmp(uev->action, "remove", 6)) { > @@ -1570,7 +1595,7 @@ check_path (struct vectors * vecs, struct path * pp, int ticks) > conf = get_multipath_config(); > ret = pathinfo(pp, conf, DI_ALL | DI_BLACKLIST); > if (ret == PATHINFO_OK) { > - ev_add_path(pp, vecs); > + ev_add_path(pp, vecs, 1); > pp->tick = 1; > } else if (ret == PATHINFO_SKIPPED) { > put_multipath_config(conf); > @@ -1686,7 +1711,7 @@ check_path (struct vectors * vecs, struct path * pp, int ticks) > } > if (!disable_reinstate && reinstate_path(pp, add_active)) { > condlog(3, "%s: reload map", pp->dev); > - ev_add_path(pp, vecs); > + ev_add_path(pp, vecs, 1); > pp->tick = 1; > return 0; > } > @@ -1709,7 +1734,7 @@ check_path (struct vectors * vecs, struct path * pp, int ticks) > /* Clear IO errors */ > if (reinstate_path(pp, 0)) { > condlog(3, "%s: reload map", pp->dev); > - ev_add_path(pp, vecs); > + ev_add_path(pp, vecs, 1); > pp->tick = 1; > return 0; > } > diff --git a/multipathd/main.h b/multipathd/main.h > index f72580d..f810d41 100644 > --- a/multipathd/main.h > +++ b/multipathd/main.h > @@ -22,7 +22,7 @@ void exit_daemon(void); > const char * daemon_status(void); > int need_to_delay_reconfig (struct vectors *); > int reconfigure (struct vectors *); > -int ev_add_path (struct path *, struct vectors *); > +int ev_add_path (struct path *, struct vectors *, int); > int ev_remove_path (struct path *, struct vectors *); > int ev_add_map (char *, char *, struct vectors *); > int ev_remove_map (char *, char *, int, struct vectors *); > -- > 2.8.1.windows.1 > -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Hello Ben Thank you for your patience again. I'll modify code according to your suggestion as this: 1) add configuration in the defaults section uid_attrs "sd:ID_SERIAL dasd:ID_UID" it would override any other configurations if this filed is configured and matched; 2) In uevent processing thread, we will assign merge_id according the label in uevents by this configuration; 3) this patch will take back: [PATCH 12/12] libmultipath: use existing wwid when wwid has already been existed in uevent 4) if this field is not configured, only do filtering and no merging works. Please confirm whether this modification is feasible. Regards, Tang Junhui 发件人: "Benjamin Marzinski" <bmarzins@redhat.com> 收件人: tang.junhui@zte.com.cn, 抄送: christophe.varoqui@opensvc.com, hare@suse.de, mwilck@suse.com, bart.vanassche@sandisk.com, dm-devel@redhat.com, zhang.kai16@zte.com.cn, tang.wenjun3@zte.com.cn 日期: 2017/01/17 05:38 主题: Re: [PATCH 04/11] multipathd: add need_do_map to indicate whether need calling domap() in ev_add_path() On Thu, Jan 12, 2017 at 01:52:20PM +0800, tang.junhui@zte.com.cn wrote: > From: tang.junhui <tang.junhui@zte.com.cn> > > Usually calling domap() in ev_add_path() is needed, but only last path > need to call domap() in processing for merged uevents to reduce the > count of calling domap() and promote efficiency. So add input parameter > need_do_map to indicate whether need calling domap() in ev_add_path(). With the addition of checking if the merge_id equals the wwid, you are protected against accidentially merging paths that shouldn't be merged, which is great. But setting need_do_map for these paths doesn't completely make sure that if the wwid and merge_id differ, you will always call domap. A contrived situation where this fails would look like: add path1, path2, path3 where merge_id equals the wwid for path1 and path2, but there is a different wwid for path3. In this case, you would call domap on just the multipath device for path3, but since path1 and path2 matched the merge_id, they wouldn't force a call to domap. A less contrived example would be add path1, path2, path3, path4 Where these were devices that were actually pointing at two different LUNs, but all set ID_SERIAL the same. path1 and path2 pointed to one LUN, while path3 and path4 pointed to another LUN. In this case the wwid of path1 and path2 matched the merge_id, while the wwid of path3 and path4 was different. In this case you would call domap twice, on both path3 and path4, but nothing would call domap to create a multipath device for path1 and path2. In general, the code to set need_do_map if the wwid and merge_id don't match only works if either none of the device in a merged set have wwids that match the merge_id, or if the last device has a wwid that matches the merge_id. If there are devices with wwids that match the merge_id, but the last device in the set isn't one of them, then some devices will not get a multipath device created for them. Now, I don't know of any device that works like my above example, so your code will likely work fine for all real-world devices. Also, fixing this is a pain, as you don't find out until processing the last path in a set that things went wrong, and then you would have to go back and run the skipped functions on one of the paths you have already processed. The easy way to fix it is to use the other possibility that I mentioned before, which is to not have the merge_id, and just use the udev provided wwid, instead of fetching it from pathinfo. Like I mentioned, if you do this, you want to make sure that you only try to grab the wwid from the udev events for devices with the correct kernel name: ID_SERIAL only for "sd.*" devices, and ID_UID only for "dasd.*" devices. I also think that this should be configurable. Otherwise, you can either go through the messy work of calling domap correctly when the last device of a set has a wwid that doesn't match the merge_id, or we can decide that this won't acutally cause problems with any known device, and punt fixing it for now. And if it causes problems with some future devices, we can deal with it then. -Ben > > Change-Id: I7f33de49a1d89d91180dcb95cd94e4944ae7ff36 > Signed-off-by: tang.wenjun <tang.wenjun3@zte.com.cn> > --- > multipathd/cli_handlers.c | 3 ++- > multipathd/main.c | 47 ++++++++++++++++++++++++++++++++++++----------- > multipathd/main.h | 2 +- > 3 files changed, 39 insertions(+), 13 deletions(-) > > diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c > index b0eeca6..3a46c09 100644 > --- a/multipathd/cli_handlers.c > +++ b/multipathd/cli_handlers.c > @@ -670,7 +670,8 @@ cli_add_path (void * v, char ** reply, int * len, void * data) > pp->checkint = conf->checkint; > } > put_multipath_config(conf); > - return ev_add_path(pp, vecs); > + return ev_add_path(pp, vecs, 1); > + > blacklisted: > *reply = strdup("blacklisted\n"); > *len = strlen(*reply) + 1; > diff --git a/multipathd/main.c b/multipathd/main.c > index adc3258..ebd7de1 100644 > --- a/multipathd/main.c > +++ b/multipathd/main.c > @@ -608,7 +608,7 @@ ev_remove_map (char * devname, char * alias, int minor, struct vectors * vecs) > } > > static int > -uev_add_path (struct uevent *uev, struct vectors * vecs) > +uev_add_path (struct uevent *uev, struct vectors * vecs, int need_do_map) > { > struct path *pp; > int ret = 0, i; > @@ -640,9 +640,18 @@ uev_add_path (struct uevent *uev, struct vectors * vecs) > r = pathinfo(pp, conf, > DI_ALL | DI_BLACKLIST); > put_multipath_config(conf); > - if (r == PATHINFO_OK) > - ret = ev_add_path(pp, vecs); > - else if (r == PATHINFO_SKIPPED) { > + if (r == PATHINFO_OK) { > + /* > + * Make sure merging use the correct wwid > + * Othterwise calling domap() > + */ > + if (!need_do_map && > + uev->merge_id && > + strcmp(uev->merge_id, pp->wwid)) > + need_do_map = 1; > + > + ret = ev_add_path(pp, vecs, need_do_map); > + } else if (r == PATHINFO_SKIPPED) { > condlog(3, "%s: remove blacklisted path", > uev->kernel); > i = find_slot(vecs->pathvec, (void *)pp); > @@ -681,7 +690,16 @@ uev_add_path (struct uevent *uev, struct vectors * vecs) > conf = get_multipath_config(); > pp->checkint = conf->checkint; > put_multipath_config(conf); > - ret = ev_add_path(pp, vecs); > + /* > + * Make sure merging use the correct wwid > + * Othterwise calling domap() > + */ > + if (!need_do_map && > + uev->merge_id && > + strcmp(uev->merge_id, pp->wwid)) > + need_do_map = 1; > + > + ret = ev_add_path(pp, vecs, need_do_map); > } else { > condlog(0, "%s: failed to store path info, " > "dropping event", > @@ -699,7 +717,7 @@ uev_add_path (struct uevent *uev, struct vectors * vecs) > * 1: error > */ > int > -ev_add_path (struct path * pp, struct vectors * vecs) > +ev_add_path (struct path * pp, struct vectors * vecs, int need_do_map) > { > struct multipath * mpp; > char params[PARAMS_SIZE] = {0}; > @@ -767,6 +785,13 @@ rescan: > /* persistent reservation check*/ > mpath_pr_event_handle(pp); > > + if (!need_do_map) > + return 0; > + > + if (!dm_map_present(mpp->alias)) { > + mpp->action = ACT_CREATE; > + start_waiter = 1; > + } > /* > * push the map to the device-mapper > */ > @@ -995,7 +1020,7 @@ uev_update_path (struct uevent *uev, struct vectors * vecs) > } > > if (pp->initialized == INIT_REQUESTED_UDEV) > - retval = uev_add_path(uev, vecs); > + retval = uev_add_path(uev, vecs, 1); > else if (mpp && ro >= 0) { > condlog(2, "%s: update path write_protect to '%d' (uevent)", uev->kernel, ro); > > @@ -1150,7 +1175,7 @@ uev_trigger (struct uevent * uev, void * trigger_data) > put_multipath_config(conf); > > if (!strncmp(uev->action, "add", 3)) { > - r = uev_add_path(uev, vecs); > + r = uev_add_path(uev, vecs, 1); > goto out; > } > if (!strncmp(uev->action, "remove", 6)) { > @@ -1570,7 +1595,7 @@ check_path (struct vectors * vecs, struct path * pp, int ticks) > conf = get_multipath_config(); > ret = pathinfo(pp, conf, DI_ALL | DI_BLACKLIST); > if (ret == PATHINFO_OK) { > - ev_add_path(pp, vecs); > + ev_add_path(pp, vecs, 1); > pp->tick = 1; > } else if (ret == PATHINFO_SKIPPED) { > put_multipath_config(conf); > @@ -1686,7 +1711,7 @@ check_path (struct vectors * vecs, struct path * pp, int ticks) > } > if (!disable_reinstate && reinstate_path(pp, add_active)) { > condlog(3, "%s: reload map", pp->dev); > - ev_add_path(pp, vecs); > + ev_add_path(pp, vecs, 1); > pp->tick = 1; > return 0; > } > @@ -1709,7 +1734,7 @@ check_path (struct vectors * vecs, struct path * pp, int ticks) > /* Clear IO errors */ > if (reinstate_path(pp, 0)) { > condlog(3, "%s: reload map", pp->dev); > - ev_add_path(pp, vecs); > + ev_add_path(pp, vecs, 1); > pp->tick = 1; > return 0; > } > diff --git a/multipathd/main.h b/multipathd/main.h > index f72580d..f810d41 100644 > --- a/multipathd/main.h > +++ b/multipathd/main.h > @@ -22,7 +22,7 @@ void exit_daemon(void); > const char * daemon_status(void); > int need_to_delay_reconfig (struct vectors *); > int reconfigure (struct vectors *); > -int ev_add_path (struct path *, struct vectors *); > +int ev_add_path (struct path *, struct vectors *, int); > int ev_remove_path (struct path *, struct vectors *); > int ev_add_map (char *, char *, struct vectors *); > int ev_remove_map (char *, char *, int, struct vectors *); > -- > 2.8.1.windows.1 > -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Tue, Jan 17, 2017 at 03:28:06PM +0800, tang.junhui@zte.com.cn wrote: > Hello Ben > > Thank you for your patience again. > > I'll modify code according to your suggestion as this: > 1) add configuration in the defaults section > uid_attrs "sd:ID_SERIAL dasd:ID_UID" > it would override any other configurations if this > filed is configured and matched; > > 2) In uevent processing thread, we will assign merge_id > according the label in uevents by this configuration; > > 3) this patch will take back: > [PATCH 12/12] libmultipath: use existing wwid when > wwid has already been existed in uevent > > 4) if this field is not configured, only do filtering and > no merging works. > > Please confirm whether this modification is feasible. Yes. This is perfectly reasonable solution. Thanks for doing all the work on this. -Ben > > Regards, > Tang Junhui > > ������: "Benjamin Marzinski" <bmarzins@redhat.com> > �ռ���: tang.junhui@zte.com.cn, > ����: christophe.varoqui@opensvc.com, hare@suse.de, > mwilck@suse.com, bart.vanassche@sandisk.com, dm-devel@redhat.com, > zhang.kai16@zte.com.cn, tang.wenjun3@zte.com.cn > ����: 2017/01/17 05:38 > ����: Re: [PATCH 04/11] multipathd: add need_do_map to indicate > whether need calling domap() in ev_add_path() > > -------------------------------------------------------------------------- > > On Thu, Jan 12, 2017 at 01:52:20PM +0800, tang.junhui@zte.com.cn wrote: > > From: tang.junhui <tang.junhui@zte.com.cn> > > > > Usually calling domap() in ev_add_path() is needed, but only last path > > need to call domap() in processing for merged uevents to reduce the > > count of calling domap() and promote efficiency. So add input parameter > > need_do_map to indicate whether need calling domap() in ev_add_path(). > > With the addition of checking if the merge_id equals the wwid, you are > protected against accidentially merging paths that shouldn't be merged, > which is great. But setting need_do_map for these paths doesn't > completely make sure that if the wwid and merge_id differ, you will > always call domap. > > A contrived situation where this fails would look like: > > add path1, path2, path3 > > where merge_id equals the wwid for path1 and path2, but there is a > different wwid for path3. In this case, you would call domap on just > the multipath device for path3, but since path1 and path2 matched the > merge_id, they wouldn't force a call to domap. > > A less contrived example would be > > add path1, path2, path3, path4 > > Where these were devices that were actually pointing at two different > LUNs, but all set ID_SERIAL the same. path1 and path2 pointed to one > LUN, while path3 and path4 pointed to another LUN. In this case the > wwid of path1 and path2 matched the merge_id, while the wwid of path3 > and path4 was different. In this case you would call domap twice, on > both path3 and path4, but nothing would call domap to create a multipath > device for path1 and path2. > > In general, the code to set need_do_map if the wwid and merge_id don't > match only works if either none of the device in a merged set have wwids > that match the merge_id, or if the last device has a wwid that matches > the merge_id. If there are devices with wwids that match the merge_id, > but the last device in the set isn't one of them, then some devices will > not get a multipath device created for them. > > Now, I don't know of any device that works like my above example, so > your code will likely work fine for all real-world devices. Also, > fixing this is a pain, as you don't find out until processing the last > path in a set that things went wrong, and then you would have to go back > and run the skipped functions on one of the paths you have already > processed. > > The easy way to fix it is to use the other possibility that I mentioned > before, which is to not have the merge_id, and just use the udev > provided wwid, instead of fetching it from pathinfo. Like I mentioned, > if you do this, you want to make sure that you only try to grab the wwid > from the udev events for devices with the correct kernel name: ID_SERIAL > only for "sd.*" devices, and ID_UID only for "dasd.*" devices. I also > think that this should be configurable. > > Otherwise, you can either go through the messy work of calling domap > correctly when the last device of a set has a wwid that doesn't match > the merge_id, or we can decide that this won't acutally cause problems > with any known device, and punt fixing it for now. And if it causes > problems with some future devices, we can deal with it then. > > -Ben > > > > > Change-Id: I7f33de49a1d89d91180dcb95cd94e4944ae7ff36 > > Signed-off-by: tang.wenjun <tang.wenjun3@zte.com.cn> > > --- > > multipathd/cli_handlers.c | 3 ++- > > multipathd/main.c | 47 > ++++++++++++++++++++++++++++++++++++----------- > > multipathd/main.h | 2 +- > > 3 files changed, 39 insertions(+), 13 deletions(-) > > > > diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c > > index b0eeca6..3a46c09 100644 > > --- a/multipathd/cli_handlers.c > > +++ b/multipathd/cli_handlers.c > > @@ -670,7 +670,8 @@ cli_add_path (void * v, char ** reply, int * len, > void * data) > > pp->checkint = conf->checkint; > > } > > put_multipath_config(conf); > > - return ev_add_path(pp, vecs); > > + return ev_add_path(pp, vecs, 1); > > + > > blacklisted: > > *reply = strdup("blacklisted\n"); > > *len = strlen(*reply) + 1; > > diff --git a/multipathd/main.c b/multipathd/main.c > > index adc3258..ebd7de1 100644 > > --- a/multipathd/main.c > > +++ b/multipathd/main.c > > @@ -608,7 +608,7 @@ ev_remove_map (char * devname, char * alias, int > minor, struct vectors * vecs) > > } > > > > static int > > -uev_add_path (struct uevent *uev, struct vectors * vecs) > > +uev_add_path (struct uevent *uev, struct vectors * vecs, int > need_do_map) > > { > > struct path *pp; > > int ret = 0, i; > > @@ -640,9 +640,18 @@ uev_add_path (struct uevent *uev, struct vectors * > vecs) > > r = pathinfo(pp, > conf, > > > DI_ALL | DI_BLACKLIST); > > > put_multipath_config(conf); > > - if (r == > PATHINFO_OK) > > - ret > = ev_add_path(pp, vecs); > > - else if (r == > PATHINFO_SKIPPED) { > > + if (r == > PATHINFO_OK) { > > + /* > > + * > Make sure merging use the correct wwid > > + * > Othterwise calling domap() > > + */ > > + if > (!need_do_map && > > + > uev->merge_id && > > + > strcmp(uev->merge_id, pp->wwid)) > > + > need_do_map = 1; > > + > > + ret > = ev_add_path(pp, vecs, need_do_map); > > + } else if (r == > PATHINFO_SKIPPED) { > > > condlog(3, "%s: remove blacklisted path", > > > uev->kernel); > > i = > find_slot(vecs->pathvec, (void *)pp); > > @@ -681,7 +690,16 @@ uev_add_path (struct uevent *uev, struct vectors * > vecs) > > conf = get_multipath_config(); > > pp->checkint = conf->checkint; > > put_multipath_config(conf); > > - ret = ev_add_path(pp, vecs); > > + /* > > + * Make sure merging use the correct > wwid > > + * Othterwise calling domap() > > + */ > > + if (!need_do_map && > > + uev->merge_id && > > + strcmp(uev->merge_id, pp->wwid)) > > + need_do_map = 1; > > + > > + ret = ev_add_path(pp, vecs, > need_do_map); > > } else { > > condlog(0, "%s: failed to store path > info, " > > "dropping event", > > @@ -699,7 +717,7 @@ uev_add_path (struct uevent *uev, struct vectors * > vecs) > > * 1: error > > */ > > int > > -ev_add_path (struct path * pp, struct vectors * vecs) > > +ev_add_path (struct path * pp, struct vectors * vecs, int need_do_map) > > { > > struct multipath * mpp; > > char params[PARAMS_SIZE] = {0}; > > @@ -767,6 +785,13 @@ rescan: > > /* persistent reservation check*/ > > mpath_pr_event_handle(pp); > > > > + if (!need_do_map) > > + return 0; > > + > > + if (!dm_map_present(mpp->alias)) { > > + mpp->action = ACT_CREATE; > > + start_waiter = 1; > > + } > > /* > > * push the map to the device-mapper > > */ > > @@ -995,7 +1020,7 @@ uev_update_path (struct uevent *uev, struct vectors > * vecs) > > } > > > > if (pp->initialized == > INIT_REQUESTED_UDEV) > > - retval = > uev_add_path(uev, vecs); > > + retval = > uev_add_path(uev, vecs, 1); > > else if (mpp && ro >= 0) { > > condlog(2, "%s: > update path write_protect to '%d' (uevent)", uev->kernel, ro); > > > > @@ -1150,7 +1175,7 @@ uev_trigger (struct uevent * uev, void * > trigger_data) > > put_multipath_config(conf); > > > > if (!strncmp(uev->action, "add", 3)) { > > - r = uev_add_path(uev, vecs); > > + r = uev_add_path(uev, vecs, 1); > > goto out; > > } > > if (!strncmp(uev->action, "remove", 6)) { > > @@ -1570,7 +1595,7 @@ check_path (struct vectors * vecs, struct path * > pp, int ticks) > > conf = > get_multipath_config(); > > ret = pathinfo(pp, > conf, DI_ALL | DI_BLACKLIST); > > if (ret == > PATHINFO_OK) { > > - > ev_add_path(pp, vecs); > > + > ev_add_path(pp, vecs, 1); > > > pp->tick = 1; > > } else if (ret == > PATHINFO_SKIPPED) { > > > put_multipath_config(conf); > > @@ -1686,7 +1711,7 @@ check_path (struct vectors * vecs, struct path * > pp, int ticks) > > } > > if (!disable_reinstate && > reinstate_path(pp, add_active)) { > > condlog(3, "%s: > reload map", pp->dev); > > - ev_add_path(pp, > vecs); > > + ev_add_path(pp, > vecs, 1); > > pp->tick = 1; > > return 0; > > } > > @@ -1709,7 +1734,7 @@ check_path (struct vectors * vecs, struct path * > pp, int ticks) > > /* Clear IO errors > */ > > if > (reinstate_path(pp, 0)) { > > > condlog(3, "%s: reload map", pp->dev); > > - > ev_add_path(pp, vecs); > > + > ev_add_path(pp, vecs, 1); > > > pp->tick = 1; > > > return 0; > > } > > diff --git a/multipathd/main.h b/multipathd/main.h > > index f72580d..f810d41 100644 > > --- a/multipathd/main.h > > +++ b/multipathd/main.h > > @@ -22,7 +22,7 @@ void exit_daemon(void); > > const char * daemon_status(void); > > int need_to_delay_reconfig (struct vectors *); > > int reconfigure (struct vectors *); > > -int ev_add_path (struct path *, struct vectors *); > > +int ev_add_path (struct path *, struct vectors *, int); > > int ev_remove_path (struct path *, struct vectors *); > > int ev_add_map (char *, char *, struct vectors *); > > int ev_remove_map (char *, char *, int, struct vectors *); > > -- > > 2.8.1.windows.1 > > -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Hi Ben & Tang, On Tue, 2017-01-17 at 10:14 -0600, Benjamin Marzinski wrote: > On Tue, Jan 17, 2017 at 03:28:06PM +0800, tang.junhui@zte.com.cn > wrote: > > Hello Ben > > > > Thank you for your patience again. > > > > I'll modify code according to your suggestion as this: > > 1) add configuration in the defaults section > > uid_attrs "sd:ID_SERIAL dasd:ID_UID" > > it would override any other configurations if this > > filed is configured and matched; > > > > 2) In uevent processing thread, we will assign merge_id > > according the label in uevents by this configuration; > > > > 3) this patch will take back: > > [PATCH 12/12] libmultipath: use existing wwid when > > wwid has already been existed in uevent > > > > 4) if this field is not configured, only do filtering and > > no merging works. > > > > Please confirm whether this modification is feasible. > > Yes. This is perfectly reasonable solution. Thanks for doing all the > work on this. I'm sorry to jump upon this old discussion again. I've just recently realized that with the introduction of "uid_attrs", the section on "WWID generation" in multipath.conf(5) needs to be corrected. Looking into that, I find it unfortunate that this new option was introduced. It makes an already complex configuration exercise even more confusing (try to rewrite that man page paragraph in an easily understandable way and I think you'll know what I mean). >From the end user point of view, "uid_attrs" seems to simply duplicate the functionality of "uid_attribute", just in more awkward syntax, and with higher priority. As a side effect, it enables uevent merging, which is documented in the man page but unexpected from the name of the option. I went through Ben's reasoning from Jan 16th again: > In general, the code to set need_do_map if the wwid and merge_id > don't > match only works if either none of the device in a merged set have > wwids > that match the merge_id, or if the last device has a wwid that > matches > the merge_id. If there are devices with wwids that match the > merge_id, > but the last device in the set isn't one of them, then some devices > will > not get a multipath device created for them. > [...] The easy way to fix it is to use the other possibility that I mentioned > before, which is to not have the merge_id, and just use the udev > provided wwid, instead of fetching it from pathinfo. Like I > mentioned, > if you do this, you want to make sure that you only try to grab the > wwid > from the udev events for devices with the correct kernel name: > ID_SERIAL > only for "sd.*" devices, and ID_UID only for "dasd.*" devices. I also > think that this should be configurable. > > Otherwise, you can either go through the messy work of calling domap > correctly when the last device of a set has a wwid that doesn't match > the merge_id, or we can decide that this won't acutally cause > problems > with any known device, and punt fixing it for now. And if it causes > problems with some future devices, we can deal with it then. It appears that Tang chose Ben's proposed "easy way" to fix the problem described. The key point appears to be to "use the udev provided wwid, instead of fetching it from pathinfo". But the end result may be confusing for users. I didn't react at the time, but in hindsight I'd find it much better if multipath's standard WWID generation procedure had been used for uevent merging. I admit I don't fully grok why that's harder to implement than the solution that got merged, probably because I didn't try yet :-) Anyway, I wonder if it would still be possible to change this at this point in time (i.e. after "uid_attrs" was merged). Cheers, Martin PS: And while I'm nitpicking anyway: The description of "uid_attrs" in the man page talks about "type of device", while what's really matched against is the kernel devnode name such as /dev/sdX or /dev/dasdY. The reader is left clueless what to do for devices other than sd or dasd (I suspect that that's actually unsupported, but the man page doesn't say anything about that). I can provide a patches for this (and for "WWID generation") but I'd like to discuss the main topic of this email first.
On Wed, Apr 05, 2017 at 03:26:06PM +0200, Martin Wilck wrote: > Hi Ben & Tang, > > On Tue, 2017-01-17 at 10:14 -0600, Benjamin Marzinski wrote: > > On Tue, Jan 17, 2017 at 03:28:06PM +0800, tang.junhui@zte.com.cn > > wrote: > > > Hello Ben > > > > > > Thank you for your patience again. > > > > > > I'll modify code according to your suggestion as this: > > > 1) add configuration in the defaults section > > > uid_attrs "sd:ID_SERIAL dasd:ID_UID" > > > it would override any other configurations if this > > > filed is configured and matched; > > > > > > 2) In uevent processing thread, we will assign merge_id > > > according the label in uevents by this configuration; > > > > > > 3) this patch will take back: > > > [PATCH 12/12] libmultipath: use existing wwid when > > > wwid has already been existed in uevent > > > > > > 4) if this field is not configured, only do filtering and > > > no merging works. > > > > > > Please confirm whether this modification is feasible. > > > > Yes. This is perfectly reasonable solution. Thanks for doing all the > > work on this. > > I'm sorry to jump upon this old discussion again. I've just recently > realized that with the introduction of "uid_attrs", the section on > "WWID generation" in multipath.conf(5) needs to be corrected. Looking > into that, I find it unfortunate that this new option was introduced. > It makes an already complex configuration exercise even more confusing > (try to rewrite that man page paragraph in an easily understandable way > and I think you'll know what I mean). > > >From the end user point of view, "uid_attrs" seems to simply duplicate > the functionality of "uid_attribute", just in more awkward syntax, and > with higher priority. As a side effect, it enables uevent merging, > which is documented in the man page but unexpected from the name of the > option. It should probably do a better job of masking "uid_attribute". The places in the code that currently use "uid_attribute" should all switch to using "uid_attrs" if it is set and applies to the device. If they don't, we could be in a situation where "uid_attrs" and "uid_attribute" are set to give different wwids, and a path's wwid could change based on whether or not ID_SERIAL was filled in when multipath got the uevent. Of course, this could only happen due to user error, so it's not that urgent of an issue. But I completely agree with you that it is totally non-obvious that the way to stop uevent merging is to unset uid_attrs. > I went through Ben's reasoning from Jan 16th again: > > > In general, the code to set need_do_map if the wwid and merge_id > > don't > > match only works if either none of the device in a merged set have > > wwids > > that match the merge_id, or if the last device has a wwid that > > matches > > the merge_id. If there are devices with wwids that match the > > merge_id, > > but the last device in the set isn't one of them, then some devices > > will > > not get a multipath device created for them. > > > [...] > The easy way to fix it is to use the other possibility that I > mentioned > > before, which is to not have the merge_id, and just use the udev > > provided wwid, instead of fetching it from pathinfo. Like I > > mentioned, > > if you do this, you want to make sure that you only try to grab the > > wwid > > from the udev events for devices with the correct kernel name: > > ID_SERIAL > > only for "sd.*" devices, and ID_UID only for "dasd.*" devices. I also > > think that this should be configurable. > > > > Otherwise, you can either go through the messy work of calling domap > > correctly when the last device of a set has a wwid that doesn't match > > the merge_id, or we can decide that this won't acutally cause > > problems > > with any known device, and punt fixing it for now. And if it causes > > problems with some future devices, we can deal with it then. > > It appears that Tang chose Ben's proposed "easy way" to fix the problem > described. The key point appears to be to "use the udev provided wwid, > instead of fetching it from pathinfo". But the end result may be > confusing for users. > > I didn't react at the time, but in hindsight I'd find it much better if > multipath's standard WWID generation procedure had been used for uevent > merging. I admit I don't fully grok why that's harder to implement than > the solution that got merged, probably because I didn't try yet :-) The issue is that you can't use the standard WWID generation code and choose to merge uevents as early as tang wanted to. So you are left with, noticing that your uevents shouldn't be merged after the fact, and trying to untable them. IIRC, I originally described a solution that involved a larger rewrite of the multipathd code, where we just processed path events individually, but never reloaded any devices until we had finished processing all the queued events, and then went back through the paths we changed, and made the appropriate map changes all at once. This way would allow you to process paths just like we always did. But I didn't feel strongly enough about it to actually write the code myself, when Tang was already working his idea. > Anyway, I wonder if it would still be possible to change this at this > point in time (i.e. after "uid_attrs" was merged). It's always possible to change things. As far as Red Hat goes, we haven't pulled in these changes yet, but I'm planning on rebasing Fedora to upstream shortly. So, it won't cause me any problems if uid_attrs goes away as long as it does so in the near future. I have no idea if other distributions have pulled this in. If so, then uid_attrs probably needs to remain accepted as a valid option by multipath. But since all correct configurations have it give the same answers as uid_attribute, we could probably just make the option do nothing. Of course, all this depends on your solution actually being enough better than Tang's to justify pulling his version out. -Ben > Cheers, > Martin > > PS: And while I'm nitpicking anyway: The description of "uid_attrs" in > the man page talks about "type of device", while what's really matched > against is the kernel devnode name such as /dev/sdX or /dev/dasdY. The > reader is left clueless what to do for devices other than sd or dasd (I > suspect that that's actually unsupported, but the man page doesn't say > anything about that). > > I can provide a patches for this (and for "WWID generation") but I'd > like to discuss the main topic of this email first. > > -- > 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
On Wed, 2017-04-05 at 13:45 -0500, Benjamin Marzinski wrote: > On Wed, Apr 05, 2017 at 03:26:06PM +0200, Martin Wilck wrote: > > >From the end user point of view, "uid_attrs" seems to simply > duplicate > > the functionality of "uid_attribute", just in more awkward syntax, > and > > with higher priority. As a side effect, it enables uevent merging, > > which is documented in the man page but unexpected from the name of > the > > option. > > It should probably do a better job of masking "uid_attribute". The > places in the code that currently use "uid_attribute" should all > switch > to using "uid_attrs" if it is set and applies to the device. > If they > don't, we could be in a situation where "uid_attrs" and > "uid_attribute" > are set to give different wwids, and a path's wwid could change based > on > whether or not ID_SERIAL was filled in when multipath got I'm not quite sure what you mean. The way it's currently implemented is in select_getuid(), pp->uid_attribute will be set from uid_attrs if it exists and matches the device name, and will henceforth be used everywhere where uid_attribute for this path is referenced. I don't see how this could lead to inconsistent settings. Even for RBD devices, uid_attrs would take precedence if users could uid_attrs="rbd:SOME_VARIABLE". (Btw, why do we hardcode the WWID generation for RBD rather than writing rbd udev rules and using the udev attributes created by them)? > > Anyway, I wonder if it would still be possible to change this at > > this > > point in time (i.e. after "uid_attrs" was merged). > > It's always possible to change things. As far as Red Hat goes, we > haven't pulled in these changes yet, but I'm planning on rebasing > Fedora > to upstream shortly. We're planning the same for openSUSE Factory. Currently we've left out Tang's patch, but the plan is to pull in upstream cleanly. > So, it won't cause me any problems if uid_attrs > goes away as long as it does so in the near future. I have no idea if > other distributions have pulled this in. If so, then uid_attrs > probably > needs to remain accepted as a valid option by multipath. But since > all > correct configurations have it give the same answers as > uid_attribute, > we could probably just make the option do nothing. > > Of course, all this depends on your solution actually being enough > better than Tang's to justify pulling his version out. Point taken :-) I don't have patches at this time. However, let me make a suggestion. Does "uid_attrs" really have to be user-configurable? IMO configuring this is asked too much from 99% of users. I'd suggest to hard-code the value of uid_attrs and use a boolean configuration option in the defaults section ("auto_uid_attribute", say) which would control whether WWID attributes should be configured "the uid_attrs way" or the "traditional way" (traditional being the fallback if uid_attrs didn't match, like now). Hard-coding this would have the advantage that we wouldn't be bound to the "sd:..." syntax. Rather, we could explore more intelligent ways to autodetect the uid_attribute depending on path properties which are known early on. Uevent merging could be controlled by a separate config option which would depend on the "auto_uid_attribute" option. This would mainly be done for clarity. Regards Martin
On Wed, Apr 05, 2017 at 10:16:33PM +0200, Martin Wilck wrote: > On Wed, 2017-04-05 at 13:45 -0500, Benjamin Marzinski wrote: > > On Wed, Apr 05, 2017 at 03:26:06PM +0200, Martin Wilck wrote: > > > > >From the end user point of view, "uid_attrs" seems to simply > > duplicate > > > the functionality of "uid_attribute", just in more awkward syntax, > > and > > > with higher priority. As a side effect, it enables uevent merging, > > > which is documented in the man page but unexpected from the name of > > the > > > option. > > > > It should probably do a better job of masking "uid_attribute". The > > places in the code that currently use "uid_attribute" should all > > switch > > to using "uid_attrs" if it is set and applies to the device. > > If they > > don't, we could be in a situation where "uid_attrs" and > > "uid_attribute" > > are set to give different wwids, and a path's wwid could change based > > on > > whether or not ID_SERIAL was filled in when multipath got > > I'm not quite sure what you mean. The way it's currently implemented is > in select_getuid(), pp->uid_attribute will be set from uid_attrs if it > exists and matches the device name, and will henceforth be used > everywhere where uid_attribute for this path is referenced. > I don't see how this could lead to inconsistent settings. > > Even for RBD devices, uid_attrs would take precedence if users could > uid_attrs="rbd:SOME_VARIABLE". (Btw, why do we hardcode the WWID > generation for RBD rather than writing rbd udev rules and using the > udev attributes created by them)? > That's what I get for responding without double-checking the code. I forgot that uid_attrs was wired into select_getuid and thought that if the path got set to INIT_MISSING_UDEV when it was initially discovered, it would use uid_attribute. My bad. > > > Anyway, I wonder if it would still be possible to change this at > > > this > > > point in time (i.e. after "uid_attrs" was merged). > > > > It's always possible to change things. As far as Red Hat goes, we > > haven't pulled in these changes yet, but I'm planning on rebasing > > Fedora > > to upstream shortly. > > We're planning the same for openSUSE Factory. Currently we've left out > Tang's patch, but the plan is to pull in upstream cleanly. > > > So, it won't cause me any problems if uid_attrs > > goes away as long as it does so in the near future. I have no idea if > > other distributions have pulled this in. If so, then uid_attrs > > probably > > needs to remain accepted as a valid option by multipath. But since > > all > > correct configurations have it give the same answers as > > uid_attribute, > > we could probably just make the option do nothing. > > > > Of course, all this depends on your solution actually being enough > > better than Tang's to justify pulling his version out. > > Point taken :-) I don't have patches at this time. However, let me make > a suggestion. Does "uid_attrs" really have to be user-configurable? IMO > configuring this is asked too much from 99% of users. I'd suggest to > hard-code the value of uid_attrs and use a boolean configuration option > in the defaults section ("auto_uid_attribute", say) which would control > whether WWID attributes should be configured "the uid_attrs way" or the > "traditional way" (traditional being the fallback if uid_attrs didn't > match, like now). Hard-coding this would have the advantage that we > wouldn't be bound to the "sd:..." syntax. Rather, we could explore more > intelligent ways to autodetect the uid_attribute depending on path > properties which are known early on. The idea behind making it configurable was to allow people to change this in the same way that they can change uid_attribute, under the assumption that if users need to be able to change uid_attribute, then they would equally need to be able to change uid_attrs. Of course, I don't know of any real use-case for this, and it's pretty hard to defend adding user-visible complexity to fix a problem that nobody has. So, sure I'm fine with your suggestion. -Ben > Uevent merging could be controlled by a separate config option which > would depend on the "auto_uid_attribute" option. This would mainly be > done for clarity. > > Regards > Martin > > -- > 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
diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c index b0eeca6..3a46c09 100644 --- a/multipathd/cli_handlers.c +++ b/multipathd/cli_handlers.c @@ -670,7 +670,8 @@ cli_add_path (void * v, char ** reply, int * len, void * data) pp->checkint = conf->checkint; } put_multipath_config(conf); - return ev_add_path(pp, vecs); + return ev_add_path(pp, vecs, 1); + blacklisted: *reply = strdup("blacklisted\n"); *len = strlen(*reply) + 1; diff --git a/multipathd/main.c b/multipathd/main.c index adc3258..ebd7de1 100644 --- a/multipathd/main.c +++ b/multipathd/main.c @@ -608,7 +608,7 @@ ev_remove_map (char * devname, char * alias, int minor, struct vectors * vecs) } static int -uev_add_path (struct uevent *uev, struct vectors * vecs) +uev_add_path (struct uevent *uev, struct vectors * vecs, int need_do_map) { struct path *pp; int ret = 0, i; @@ -640,9 +640,18 @@ uev_add_path (struct uevent *uev, struct vectors * vecs) r = pathinfo(pp, conf, DI_ALL | DI_BLACKLIST); put_multipath_config(conf); - if (r == PATHINFO_OK) - ret = ev_add_path(pp, vecs); - else if (r == PATHINFO_SKIPPED) { + if (r == PATHINFO_OK) { + /* + * Make sure merging use the correct wwid + * Othterwise calling domap() + */ + if (!need_do_map && + uev->merge_id && + strcmp(uev->merge_id, pp->wwid)) + need_do_map = 1; + + ret = ev_add_path(pp, vecs, need_do_map); + } else if (r == PATHINFO_SKIPPED) { condlog(3, "%s: remove blacklisted path", uev->kernel); i = find_slot(vecs->pathvec, (void *)pp); @@ -681,7 +690,16 @@ uev_add_path (struct uevent *uev, struct vectors * vecs) conf = get_multipath_config(); pp->checkint = conf->checkint; put_multipath_config(conf); - ret = ev_add_path(pp, vecs); + /* + * Make sure merging use the correct wwid + * Othterwise calling domap() + */ + if (!need_do_map && + uev->merge_id && + strcmp(uev->merge_id, pp->wwid)) + need_do_map = 1; + + ret = ev_add_path(pp, vecs, need_do_map); } else { condlog(0, "%s: failed to store path info, " "dropping event", @@ -699,7 +717,7 @@ uev_add_path (struct uevent *uev, struct vectors * vecs) * 1: error */ int -ev_add_path (struct path * pp, struct vectors * vecs) +ev_add_path (struct path * pp, struct vectors * vecs, int need_do_map) { struct multipath * mpp; char params[PARAMS_SIZE] = {0}; @@ -767,6 +785,13 @@ rescan: /* persistent reservation check*/ mpath_pr_event_handle(pp); + if (!need_do_map) + return 0; + + if (!dm_map_present(mpp->alias)) { + mpp->action = ACT_CREATE; + start_waiter = 1; + } /* * push the map to the device-mapper */ @@ -995,7 +1020,7 @@ uev_update_path (struct uevent *uev, struct vectors * vecs) } if (pp->initialized == INIT_REQUESTED_UDEV) - retval = uev_add_path(uev, vecs); + retval = uev_add_path(uev, vecs, 1); else if (mpp && ro >= 0) { condlog(2, "%s: update path write_protect to '%d' (uevent)", uev->kernel, ro); @@ -1150,7 +1175,7 @@ uev_trigger (struct uevent * uev, void * trigger_data) put_multipath_config(conf); if (!strncmp(uev->action, "add", 3)) { - r = uev_add_path(uev, vecs); + r = uev_add_path(uev, vecs, 1); goto out; } if (!strncmp(uev->action, "remove", 6)) { @@ -1570,7 +1595,7 @@ check_path (struct vectors * vecs, struct path * pp, int ticks) conf = get_multipath_config(); ret = pathinfo(pp, conf, DI_ALL | DI_BLACKLIST); if (ret == PATHINFO_OK) { - ev_add_path(pp, vecs); + ev_add_path(pp, vecs, 1); pp->tick = 1; } else if (ret == PATHINFO_SKIPPED) { put_multipath_config(conf); @@ -1686,7 +1711,7 @@ check_path (struct vectors * vecs, struct path * pp, int ticks) } if (!disable_reinstate && reinstate_path(pp, add_active)) { condlog(3, "%s: reload map", pp->dev); - ev_add_path(pp, vecs); + ev_add_path(pp, vecs, 1); pp->tick = 1; return 0; } @@ -1709,7 +1734,7 @@ check_path (struct vectors * vecs, struct path * pp, int ticks) /* Clear IO errors */ if (reinstate_path(pp, 0)) { condlog(3, "%s: reload map", pp->dev); - ev_add_path(pp, vecs); + ev_add_path(pp, vecs, 1); pp->tick = 1; return 0; } diff --git a/multipathd/main.h b/multipathd/main.h index f72580d..f810d41 100644 --- a/multipathd/main.h +++ b/multipathd/main.h @@ -22,7 +22,7 @@ void exit_daemon(void); const char * daemon_status(void); int need_to_delay_reconfig (struct vectors *); int reconfigure (struct vectors *); -int ev_add_path (struct path *, struct vectors *); +int ev_add_path (struct path *, struct vectors *, int); int ev_remove_path (struct path *, struct vectors *); int ev_add_map (char *, char *, struct vectors *); int ev_remove_map (char *, char *, int, struct vectors *);