| Message ID | 20250123221212.123833-1-mwilck@suse.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Benjamin Marzinski |
| Headers | show |
| Series | multipathd: trigger uevents for blacklisted paths in reconfigure | expand |
On Thu, Jan 23, 2025 at 11:12:12PM +0100, Martin Wilck wrote: > If multipathd has already configured maps, and the user changes the > blacklist or other parameters that cause currently multipathed > devices to be skipped, and then runs "multipathd reconfigure" > or restarts multipathd, multipathd flushes the maps in question, > but doesn't trigger uevents for the now-blacklisted paths. > > This is because the blacklisted paths are removed from the discovered > maps internally when update_pathvec_from_dm() is called through > map_discovery() and update_multipath_table(); when later > trigger_paths_udev_change() is called from coalesce_maps(), the > map contains no paths for which an uevent could be triggered. > > The map_discovery() code flow is special, because we will call > coalesce_paths() afterwards anyway and reconstruct the mpvec. Unlike the > regular code flow, we don't want the maps to be "corrected" in this > case, because the maps discovered here aren't going to be reloaded. > We just want update_pathvec_from_dm() to populate the pathvec. > > Therefore add a new flag DI_DISCOVERY, which is only set when > update_multipath_table() is called from map_discovery(), and if > this flag is set, keep PATHINFO_SKIPPED paths in the map's table in > update_pathvec_from_dm(). Later on, the paths will still be visible > in the old mpp (ompp) in coalesce_maps(), and uevents will be > triggered for them to release them to systemd. > > We can't always do this for PATHINFO_SKIPPED, because in some cases > paths may be accepted in a map first and SKIPPED later (for example if > the WWID wasn't yet available at startup). Therefore the special > case for DI_DISCOVERY is necessary. > > Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com> > --- > libmultipath/discovery.h | 2 ++ > libmultipath/structs_vec.c | 6 +++++- > multipathd/main.c | 2 +- > 3 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/libmultipath/discovery.h b/libmultipath/discovery.h > index c4a8dd9..2b39eb0 100644 > --- a/libmultipath/discovery.h > +++ b/libmultipath/discovery.h > @@ -74,6 +74,7 @@ enum discovery_mode { > DI_BLACKLIST__, > DI_NOIO__, > DI_NOFALLBACK__, > + DI_DISCOVERY__, > }; > > #define DI_SYSFS (1 << DI_SYSFS__) > @@ -84,6 +85,7 @@ enum discovery_mode { > #define DI_BLACKLIST (1 << DI_BLACKLIST__) > #define DI_NOIO (1 << DI_NOIO__) /* Avoid IO on the device */ > #define DI_NOFALLBACK (1 << DI_NOFALLBACK__) /* do not allow wwid fallback */ > +#define DI_DISCOVERY (1 << DI_DISCOVERY__) /* set only during map discovery */ > > #define DI_ALL (DI_SYSFS | DI_IOCTL | DI_CHECKER | DI_PRIO | DI_WWID) > > diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c > index b2bb47c..7d53aa3 100644 > --- a/libmultipath/structs_vec.c > +++ b/libmultipath/structs_vec.c > @@ -107,6 +107,9 @@ static void update_pathvec_from_dm(vector pathvec, struct multipath *mpp, > bool mpp_has_wwid; > bool must_reload = false; > bool pg_deleted = false; > + bool map_discovery = !!(pathinfo_flags & DI_DISCOVERY); > + > + pathinfo_flags &= ~DI_DISCOVERY; > > if (!mpp->pg) > return; > @@ -193,7 +196,8 @@ static void update_pathvec_from_dm(vector pathvec, struct multipath *mpp, > rc = pathinfo(pp, conf, > DI_SYSFS|DI_WWID|DI_BLACKLIST|DI_NOFALLBACK|pathinfo_flags); > pthread_cleanup_pop(1); > - if (rc != PATHINFO_OK) { > + if (rc == PATHINFO_FAILED || > + (rc == PATHINFO_SKIPPED && !map_discovery)) { > condlog(1, "%s: error %d in pathinfo, discarding path", > pp->dev, rc); > vector_del_slot(pgp->paths, j--); > diff --git a/multipathd/main.c b/multipathd/main.c > index b4a366e..43a0240 100644 > --- a/multipathd/main.c > +++ b/multipathd/main.c > @@ -1758,7 +1758,7 @@ map_discovery (struct vectors * vecs) > return 1; > > vector_foreach_slot (vecs->mpvec, mpp, i) > - if (update_multipath_table(mpp, vecs->pathvec, 0) != DMP_OK) { > + if (update_multipath_table(mpp, vecs->pathvec, DI_DISCOVERY) != DMP_OK) { > vector_del_slot(vecs->mpvec, i--); > remove_map(mpp, vecs->pathvec); > } > -- > 2.48.1
On Fri, 2025-01-24 at 16:05 -0500, Benjamin Marzinski wrote: > On Thu, Jan 23, 2025 at 11:12:12PM +0100, Martin Wilck wrote: > > If multipathd has already configured maps, and the user changes the > > blacklist or other parameters that cause currently multipathed > > devices to be skipped, and then runs "multipathd reconfigure" > > or restarts multipathd, multipathd flushes the maps in question, > > but doesn't trigger uevents for the now-blacklisted paths. > > > > This is because the blacklisted paths are removed from the > > discovered > > maps internally when update_pathvec_from_dm() is called through > > map_discovery() and update_multipath_table(); when later > > trigger_paths_udev_change() is called from coalesce_maps(), the > > map contains no paths for which an uevent could be triggered. > > > > The map_discovery() code flow is special, because we will call > > coalesce_paths() afterwards anyway and reconstruct the mpvec. > > Unlike the > > regular code flow, we don't want the maps to be "corrected" in this > > case, because the maps discovered here aren't going to be reloaded. > > We just want update_pathvec_from_dm() to populate the pathvec. > > > > Therefore add a new flag DI_DISCOVERY, which is only set when > > update_multipath_table() is called from map_discovery(), and if > > this flag is set, keep PATHINFO_SKIPPED paths in the map's table in > > update_pathvec_from_dm(). Later on, the paths will still be visible > > in the old mpp (ompp) in coalesce_maps(), and uevents will be > > triggered for them to release them to systemd. > > > > We can't always do this for PATHINFO_SKIPPED, because in some cases > > paths may be accepted in a map first and SKIPPED later (for example > > if > > the WWID wasn't yet available at startup). Therefore the special > > case for DI_DISCOVERY is necessary. > > > > Signed-off-by: Martin Wilck <mwilck@suse.com> > > Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com> Thanks ... I just realized that this patch doesn't apply on "queue" because I had written it on top of some other work I was doing on my "tip" branch. It can be easily rebased though. Martin
diff --git a/libmultipath/discovery.h b/libmultipath/discovery.h index c4a8dd9..2b39eb0 100644 --- a/libmultipath/discovery.h +++ b/libmultipath/discovery.h @@ -74,6 +74,7 @@ enum discovery_mode { DI_BLACKLIST__, DI_NOIO__, DI_NOFALLBACK__, + DI_DISCOVERY__, }; #define DI_SYSFS (1 << DI_SYSFS__) @@ -84,6 +85,7 @@ enum discovery_mode { #define DI_BLACKLIST (1 << DI_BLACKLIST__) #define DI_NOIO (1 << DI_NOIO__) /* Avoid IO on the device */ #define DI_NOFALLBACK (1 << DI_NOFALLBACK__) /* do not allow wwid fallback */ +#define DI_DISCOVERY (1 << DI_DISCOVERY__) /* set only during map discovery */ #define DI_ALL (DI_SYSFS | DI_IOCTL | DI_CHECKER | DI_PRIO | DI_WWID) diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c index b2bb47c..7d53aa3 100644 --- a/libmultipath/structs_vec.c +++ b/libmultipath/structs_vec.c @@ -107,6 +107,9 @@ static void update_pathvec_from_dm(vector pathvec, struct multipath *mpp, bool mpp_has_wwid; bool must_reload = false; bool pg_deleted = false; + bool map_discovery = !!(pathinfo_flags & DI_DISCOVERY); + + pathinfo_flags &= ~DI_DISCOVERY; if (!mpp->pg) return; @@ -193,7 +196,8 @@ static void update_pathvec_from_dm(vector pathvec, struct multipath *mpp, rc = pathinfo(pp, conf, DI_SYSFS|DI_WWID|DI_BLACKLIST|DI_NOFALLBACK|pathinfo_flags); pthread_cleanup_pop(1); - if (rc != PATHINFO_OK) { + if (rc == PATHINFO_FAILED || + (rc == PATHINFO_SKIPPED && !map_discovery)) { condlog(1, "%s: error %d in pathinfo, discarding path", pp->dev, rc); vector_del_slot(pgp->paths, j--); diff --git a/multipathd/main.c b/multipathd/main.c index b4a366e..43a0240 100644 --- a/multipathd/main.c +++ b/multipathd/main.c @@ -1758,7 +1758,7 @@ map_discovery (struct vectors * vecs) return 1; vector_foreach_slot (vecs->mpvec, mpp, i) - if (update_multipath_table(mpp, vecs->pathvec, 0) != DMP_OK) { + if (update_multipath_table(mpp, vecs->pathvec, DI_DISCOVERY) != DMP_OK) { vector_del_slot(vecs->mpvec, i--); remove_map(mpp, vecs->pathvec); }
If multipathd has already configured maps, and the user changes the blacklist or other parameters that cause currently multipathed devices to be skipped, and then runs "multipathd reconfigure" or restarts multipathd, multipathd flushes the maps in question, but doesn't trigger uevents for the now-blacklisted paths. This is because the blacklisted paths are removed from the discovered maps internally when update_pathvec_from_dm() is called through map_discovery() and update_multipath_table(); when later trigger_paths_udev_change() is called from coalesce_maps(), the map contains no paths for which an uevent could be triggered. The map_discovery() code flow is special, because we will call coalesce_paths() afterwards anyway and reconstruct the mpvec. Unlike the regular code flow, we don't want the maps to be "corrected" in this case, because the maps discovered here aren't going to be reloaded. We just want update_pathvec_from_dm() to populate the pathvec. Therefore add a new flag DI_DISCOVERY, which is only set when update_multipath_table() is called from map_discovery(), and if this flag is set, keep PATHINFO_SKIPPED paths in the map's table in update_pathvec_from_dm(). Later on, the paths will still be visible in the old mpp (ompp) in coalesce_maps(), and uevents will be triggered for them to release them to systemd. We can't always do this for PATHINFO_SKIPPED, because in some cases paths may be accepted in a map first and SKIPPED later (for example if the WWID wasn't yet available at startup). Therefore the special case for DI_DISCOVERY is necessary. Signed-off-by: Martin Wilck <mwilck@suse.com> --- libmultipath/discovery.h | 2 ++ libmultipath/structs_vec.c | 6 +++++- multipathd/main.c | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-)