Hi Ben, On Thu, 2021-11-04 at 23:10 +0100, Martin Wilck wrote: > On Wed, 2021-10-20 at 14:15 -0500, Benjamin Marzinski wrote: > > ... > > > > > Multipath now has a new state to deal with these devices, > > INIT_PARTIAL. > > Devices in this state are treated mostly like INIT_OK devices, but > > when "multipathd add path" is called or an add/change uevent > > happens > > on these devices, multipathd will finish initializing them, and > > remove > > them if necessary. > > > > Signed-off-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx> > > Nice. Somehow in my mind the issue always look much more complex. > I like this, but I want to give it some testing before I finally ack > it. I noted that running "multipathd add path $path" for a path in INIT_PARTIAL state changes this paths's init state to INIT_OK, even if the udev still has no information about it (*). The reason is that pp->wwid is set, and thus pathinfo() won't even try to retrieve the WWID, although for INIT_PARTIAL paths the origin of the WWID is not 100% trustworthy (being just copied from pp->mpp- >wwid). pathinfo() will return PATHINFO_OK without having retrieved the WWID. I suppose we could apply a similar logic as in uev_update_path() here, clearing pp->wwid before calling pathinfo(). If udev was still unaware of the path, this would mean a transition from INIT_PARTIAL to INIT_MISSING_UDEV. OTOH, we'd now need to be prepared to have to remove the path from the map if the WWID doesn't match after the call to pathinfo(). This means we'd basically need to reimplement the "changed WWID" logic from uev_update_path(), and thus we'd need to unify both code paths. In general, the difference between INIT_MISSING_UDEV and INIT_PARTIAL is subtle. Correct me if I'm wrong, but INIT_PARTIAL means "udev has no information about this device" and INIT_MISSING_UDEV currently means "we weren't able to figure out a WWID for the path" (meaning that INIT_MISSING_UDEV is a misnomer - when fallback are allowed, INIT_MISSING_UDEV is actually independent of the device's state in the udev db. We should rename this to INIT_MISSING_WWID, perhaps). The semantics of the two states are very different though: INIT_MISSING_UDEV will trigger an attempt to regenerate an uevent, whereas INIT_PARTIAL will just stick passively. IMO it'd make sense to trigger an uevent in the INIT_PARTIAL case, too, albeit perhaps not immediately (after the default uevent timeout - 180s?). Also, we currently don't track the udev state of devices cleanly. We set INIT_MISSING_UDEV if we can't obtain uid_attribute, which doesn't necessarily mean that udev is unaware of the device. I believe we should e.g. check the USEC_INITIALIZED property - it is non-NULL if and only if the device is present in the udev db. If uid_attribute isn't set regardless, we could conclude that the udev rules just don't set it. It might make sense to retrigger *one* uevent in that case, but no more. IMO we need a clear definition of the INIT_xyz states and their transitions. We won't get along with intuitive logic any more. Cheers, Martin (*) my test procedure is simply to delete the paths' files from /run/udev/data and to restart multipathd. -- dm-devel mailing list dm-devel@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/dm-devel
