Re: [dm-devel] [PATCH 5/7] multipathd: fully initialize paths added by update_pathvec_from_dm

[Martin Wilck <martin.wilck@suse.com>]

On Mon, 2021-11-08 at 10:29 -0600, Benjamin Marzinski wrote:
> On Fri, Nov 05, 2021 at 11:20:01PM +0000, Martin Wilck wrote:
> > 
> > If I run "udevadm trigger -c change /dev/sde" in this situation,
> > I'll
> > get the full info, as if I had run "add" before (some rules may
> > differ
> > between "add" and "change", but that's quite unusual).
> > 
> 
> udev rules may not change much, but, for instance, multipathd reacts
> differently to add and change events.  So there may be other
> consumers
> of uevents that care about the difference between add and change
> events.

Let's send "add" then. It makes sense if the device didn't exist
before, after all. When we trigger, we don't know if an "add" event is
already under way, but that's just how it is.

>  
> > > 
> > > > 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.
> > > 
> > > IIRC, the initial reason why INIT_MISSING_UDEV was added was
> > > because
> > > sometimes udev workers timed out, causing them to not run all the
> > > rules.
> > > Do you know offhand if USEC_INITIALIZED is set in this case? If
> > > we
> > > could
> > > differentiate between the following states:
> > 
> > udevd sets this property very early, when it first receives an
> > uevent.
> > But libudev calls won't return it until the database file is
> > written,
> > which happens last, after all rules and RUN statements have been
> > processed, _in the worker_. Thus if the worker is killed, the file
> > won't be written.
> > 
> > 
> > 0. the device doesn't exist in sysfs 
> 
> We simply delete devices that don't exist in sysfs, right? If we get
> a
> non-remove uevent for a device, but it doesn't exist in sysfs, then I
> would assume that a remove uevent will be shortly incomming.

True, but still it's one possible "state" the device may be in. I just
wanted to mention that.

>  
> > > 1: udev hasn't gotten an event for a device
> > 
> > I don't think we can detect this without listening for kernel
> > uevents.
> > And even if we listen to them, we could have missed some. udevd
> > doesn't
> > have an API for it, either, AFAIK.
> 
> Isn't this the most common INIT_PARTIAL case, where we are waiting
> for
> the coldplug uevent? If there is no database file when we are
> processing
> the device, we are in this state. Correct? 

Not necessarily. udev may have got an event, but not have finished
processing it, or failed to process it entirely (e.g. because of a
timeout, your case 2.). When udevd sees an "add" or "change" event for
a device for the first time, creating the db entry is the last action
it takes. During coldplug, udevd will receive a lot of events almost
simultaneously, but it may take considerable time until it has
processed them all.

> > > 2: udev got an event but timed out or failed while processing it
> > 
> > This would be the USEC_INITIALIZED case, IMO
> 
> If udev has, in the past, successfully processed an event for a
> device,
> but fails while processing a later uevent, it doesn't keep th data
> from
> the previous event. So it could lose the uid_attribute. But the
> database file should still exist. Correct?

That's true. But we can't do anything about it. libudev will return
what the database currently contains, which is the content from the
last successfuly processed "add" or "change" uevent, whether or not
other uevents are in the queue or being processed. I don't think this
scenario matters in the current discussion about partially initialized
paths. This is the "changed wwid" scenario which I think we handle
quite nicely today already. Or am I misunderstanding?

Cheers,
Martin


--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel