From: Benjamin Marzinski <bmarzins@redhat.com>
To: Martin Wilck <mwilck@suse.com>
Cc: Brian Bunker <brian@purestorage.com>,
dm-devel@lists.linux.dev,
Christophe Varoqui <christophe.varoqui@opensvc.com>
Subject: Re: [PATCH v2 1/1] multipathd: verify udev WWIDs with the device before moving paths
Date: Mon, 9 Nov 2026 15:26:42 -0500 [thread overview]
Message-ID: <avIsgj9mjV1DU-M_@redhat.com> (raw)
In-Reply-To: <f9a399429d9c7ac2e4079e7cd89ae7c304ee3b24.camel@suse.com>
On Fri, Nov 06, 2026 at 11:19:25PM +0100, Martin Wilck wrote:
> On Fri, 2026-10-30 at 14:09 -0400, Benjamin Marzinski wrote:
> > On Thu, Oct 29, 2026 at 04:52:41PM -0700, Brian Bunker wrote:
> > > multipathd takes a path's WWID from udev. When a change uevent
> > > reports
> > > a different WWID, uev_update_path() removes the path from its map
> > > and
> > > re-adds it through uev_add_path(), which places it in whatever map
> > > owns
> > > the new WWID. Nothing checks the new value against the device
> > > itself;
> > > recheck_wwid only covers paths coming back up in the checker and
> > > "multipathd add path". If udev reports the wrong WWID, or the
> > > uevent is
> > > stale by the time it is processed, the path goes into the map of a
> > > different LUN and I/O for one LUN is silently sent to another.
> > >
> > > Use SG_IO to read VPD page 0x83 from the device in two places, for
> > > paths where can_recheck_wwid() is true:
> > >
> > > - In uev_update_path(), if the device still reports the cached
> > > WWID,
> > > ignore the udev value and leave the path where it is.
> > >
> > > - In ev_add_path(), when a path is being re-added because its udev
> > > data was missing or changed, refuse to add it if the device
> > > doesn't
> > > confirm its WWID. The path is orphaned, its WWID is cleared and
> > > it
> > > is marked INIT_MISSING_UDEV, so the checker retriggers udev
> > > (bounded
> > > by retrigger_tries) and the path is re-added from fresh data.
> > >
> > > To tell re-adds from ordinary adds, turn the need_do_map argument
> > > of
> > > uev_add_path() and ev_add_path() into enum path_add_mode, with a
> > > new
> > > PATH_ADD_REINIT value that behaves like PATH_ADD_MAP and also
> > > requests
> > > the WWID check. Ordinary adds are unchanged and never read the WWID
> > > from the device.
> > >
> > > If the SG_IO read fails, behavior is unchanged.
> > >
> > > Split the SG_IO read out of check_path_wwid_change() into
> > > get_sgio_wwid() so all the checks share it.
> > >
> > > Signed-off-by: Brian Bunker <brian@purestorage.com>
> > Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
>
> You will need convince me that this doesn't do more harm than good.
> We can't just use udev in one place and other methods elsewhere.
I get your concern, but we already do this when recheck_wwid is set.
Perhaps we could also gate this with that config option.
> I am open for a general redesign that uses sysfs for retrieving
> WWIDs, rather than udev. But using SG_IO seems wrong to me in general,
> because it can hang. If anything, we'd need to make this configurable,
> and we'd then need to use SG_IO consistently throughout the code (and
> SG_IO has significant disadvantages, as already mentioned). Also, SG_IO
> works for SCSI only. That may be a minor issue these days, because
> almost all disks handled by dm-multipath are SCSI disks, but at least
> formally we still support other block devices like NVMe and DASD.
>
> Unfortunately there's already a zoo of options for WWID detection which
> has grown over time.
>
> > About how likely this is: we hit it because a test tool was opening
> > and closing the sd devices read-write many times a second, and with
> > udev's "watch" rule every close generates a change uevent that runs
> > scsi_id against the device.
>
> I wouldn't call this a valid test. It's a deliberate abuse of udev
> functionality. If you do this, you need to disable udev's watch
> functionality, which is merely a convenience for users anyway. It's
> questionable if it's a useful feature on production servers in general.
>
> Thinking about it, I wonder if we should simply set "nowatch" for
> multipath members by default. AFAIU that would avoid the issue Brian
> encountered, at least in this test scenario.
I can't think of anything wrong with disabling that off the top of my
head. Nothing should be directly opening these devices read-write once
they are part of a multipath device.
-Ben
> Regards
> Martin
>
> --
> Dr. Martin Wilck <mwilck@suse.com>
> SUSE Software Solutions Germany GmbH, Frankenstr. 146, 90461 Nürnberg,
> Germany
> Geschäftsführer: Stefan Gaiser, Jochen Jaser, Abhinav Puri (HRB
> 36809,AG Nürnberg)
prev parent reply other threads:[~2026-11-09 20:26 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-29 23:52 [PATCH v2 0/1] multipathd: verify udev WWIDs with the device before moving paths Brian Bunker
2026-10-29 23:52 ` [PATCH v2 1/1] " Brian Bunker
2026-10-30 18:09 ` Benjamin Marzinski
2026-11-06 22:19 ` Martin Wilck
2026-11-09 18:57 ` Brian Bunker
2026-11-09 20:26 ` Benjamin Marzinski [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=avIsgj9mjV1DU-M_@redhat.com \
--to=bmarzins@redhat.com \
--cc=brian@purestorage.com \
--cc=christophe.varoqui@opensvc.com \
--cc=dm-devel@lists.linux.dev \
--cc=mwilck@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.