On Tue, 2025-04-29 at 16:27 -0400, Benjamin Marzinski wrote:
> On Tue, Apr 29, 2025 at 09:59:40PM +0200, Martin Wilck wrote:
> > On Mon, 2025-04-28 at 17:45 -0400, Benjamin Marzinski wrote:
> > > If the multipath configuration is changed to blacklist existing
> > > devices,
> > > and multipathd is reloaded but the blacklisted multipaths device
> > > can't
> > > be removed, multipathd was marking the paths as INIT_PARTIAL,
> > > causing
> > > them to stay in the multipath device, at least until the
> > > partial_retrigger_delay timeout elapsed. Instead, mark them as
> > > INIT_REMOVED and set mpp->need_reload, so the device is reloaded
> > > and
> > > the
> > > paths are removed. To make sure the blacklisted paths are deleted
> > > when
> > > the multipath device is removed in coalesce_maps(), set their pp-
> > > >mpp
> > > to point to map before removing it.
> > >
> > > Fixes d9c61332 ("multipathd: trigger uevents for blacklisted
> > > paths in
> > > reconfigure")
> >
> > The patch looks good to me, but I only vaguely understand why the
> > bug
> > is introduced in d9c61332. Are you positive that before d9c61332,
> > the
> > hang didn't occur?
>
> Well, I'm pretty sure these blacklisted paths stopped getting deleted
> during reconfigure by d9c61332. Before d9c61332, blacklisted paths
> were
> immediately deleted in update_pathvec_from_dm().
Right, thanks.
I've taken the liberty to fix the typo ("libmutipath") and make some
minor coding style adjustments in my queue branch. You can inspect the
result there.
The coding style stuff is work in progress, I've added a clang-format
based workflow on my tip branch.
I'd like to avoid major reformatting of past code while enforcing
consistent formatting for present and future submissions which is more-
or-less in line with what we used to do "sub-conciously" during the
last years. clang-format has a lot of options [1]. I'm still struggling
to find style settings that match ours [2].
Let me know if you dislike this, or if you have suggestions for
improving the settings.
Regards
Martin
[1] https://clang.llvm.org/docs/ClangFormatStyleOptions.html
[2]
https://github.com/openSUSE/multipath-tools/blob/refs/heads/tip/.clang-format
> After this change
>
> @@ -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--);
>
> they started hanging around, so that uevents could be generated for
> them
> by trigger_paths_udev_change(). However, since coalesce_paths() will
> usually clear pp->mpp, they won't get removed when orphan_paths() is
> later called by remove_maps() to remove the old maps. I admit I
> didn't
> test if the paths got removed before that commit, but the commit
> message
> says that they did.
>
> -Ben
>
> >
> > >
> > > Signed-off-by: Benjamin Marzinski <bmarz...@redhat.com>
> >
> > Reviewed-by: Martin Wilck <mwi...@suse.com>
