On Mon, 2023-03-20 at 15:37 -0500, Benjamin Marzinski wrote: > > > > I have to say I don't quite understand why we read from the > > environment > > at all if the libudev call fails. This was coded before I joined > > the > > project, so perhaps you can clarify it. Why do we expect the > > environment to provide the correct value if libudev cannot? > > I'm pretty sure that the udev database for a device isn't set up yet > when we are in the middle of processing the ADD event for it. You are right, of course. I had a brain fart. > When udev > calls "multipath -u" on the add event, we can't look up the value in > the > database because the database entry for the device doesn't exist yet. > We > have to use the value it passed in via the enviroment. > > > If we need to keep this fallback, I wonder if we need another field > > in > > "struct path" for it. For example, we could read MAJOR and MINOR > > from > > the environment and use uid_attribute only if the result matches > > the > > path's devt. > > This is basically what my patch does. It sets the can_use_env_uid > flag > only on the device that the uevent is for, so the only device that > can > get its WWID from the environment is the device whose uevent we are > processing. The other paths we find in path_discovery() must be > intialized, which means that they will have a udev database entry, > and > we can find their WWID there. I wasn't saying your patch was not correct. I was just not so excited about adding this field, which only multipath uses, to "struct path". But I just checked this doesn't change the struct size, and given that the environment is more important than I realized yesterday, your approach is more efficient than what I suggested. Martin -- dm-devel mailing list dm-devel@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/dm-devel
