On Thu, Oct 29, 2026 at 05:27:39PM -0400, Benjamin Marzinski wrote:
> The code all looks good to me. I must admit, I'm a little paranoid about
> forcing perfectly normal ev_add_path() calls to double-check the wwid.
> Currently, that only happens if you enable "recheck_wwid", or if
> something is going wrong with initializing the path.

That's a fair concern. I made the check unconditional in ev_add_path()
because it is the one place every add passes through, but I didn't
weigh what it costs the normal case.

> The issue is that
> scsi devices can have multiple wwids, and the multipath code has to be
> in sync with the code that udev runs to the wwid, otherwise they can be
> different. I don't think these functions differ. I know I've checked
> them in the past. But unless there is a problem getting the wwid from
> udev or someone turns on recheck_wwid (I don't know how common that is)
> paths don't ever run this code.

can_recheck_wwid() already limits the check to SCSI paths using the
default uid_attribute, but I can't prove the two never pick different
designators on some device. If they did, every add on that device would
be refused and retriggered. That isn't a risk worth taking for a path
that has never had a problem.

> We could avoid this by hijacking the need_do_map argument to
> uev_add_path() and ev_add_path(). If, instead of just being 0 or 1, it
> could also be a third value for reinits (we should probably use an enum
> in this case), we could only double-check the wwid on reinits, which
> only happen when there is an issue getting the wwid from udev.

I like this, and v2 does it. need_do_map becomes enum path_add_mode
(PATH_ADD_NO_MAP, PATH_ADD_MAP, PATH_ADD_REINIT). PATH_ADD_REINIT
behaves like PATH_ADD_MAP and is passed only for the re-add at the end
of uev_update_path(). That covers both cases that set needs_reinit:
udev data not there yet, and udev reporting a changed WWID. The
changed-WWID re-add is the one that put sdq into the wrong map for us.
Ordinary adds, CLI adds and the checker's add_when_online path don't
run the check.

The uev_update_path() check stays as it was. It only runs when udev's
WWID differs from the cached one, and does nothing if the device
doesn't confirm the cached value.

> Thoughts? Am I just being paranoid here?

I don't think so. On how common the problem is: we hit it because a
test tool was opening and closing the sd devices read-write many times
a second. With udev's "watch" rule each close sends a change uevent
that runs scsi_id, so the host was producing far more udev traffic than
a normal one. I expect it to be rare elsewhere, but the result is data
going to the wrong LUN, so I'd rather not act on a changed WWID
without confirming it. Limiting the check to reinits keeps that and
leaves the normal path alone.

v2 follows in a separate thread.

Thanks,
Brian

Reply via email to