tools/nut-usbinfo.pl: Fixes#913
Conversation
See https://lwn.net/Articles/837033/ for an in-depth explanation of how "bind" events should be handled in udev rules. In short: " The ACTION line causes the entire file to be skipped for anything other than ADD or CHANGE events; in particular, that is what will happen with BIND events. That will cause properties associated with those events to be lost — and the device in question to be set up improperly (if at all). "
Why "!= usb" when we're already checking for usb and other values above? Always go to the end label if we didn't already jump to the section containing the actual rules.
This piece of the code was really confusing because it looked like, at a glance (and at more than a glance), that the line which we'd have read in the generated rules file was commented out. Turns out that beginning of lines in the generated rules could be in the middle of the line in the code that generated it. Reindent everything so that each line in the output corresponds to one line in the input, except for empty lines. There are no differences in output between the versions before and after this change.
|
LGTM for most of the changes, just gotta trust your research and judgement about the udev rules :) Thanks! I will leave this PR open for a few days to give a chance for others to comment if something would concern them. |
You might want to test them, even summarily, as I don't have the hardware to test it. |
|
@hadess I probably need to reread the LWN article and the bugs linked from it, but assuming that the NUT rules are changing permissions on nodes created from ADD events, what is the harm in skipping that when a BIND event comes in? It sounds like users of 4.14+ kernels should be seeing regressions of some sort, but I'm not sure what that looks like for NUT. I have no problem testing the two later commits at the same time, but scheduling time to test on non-production hardware might be tough (I had a few batteries die on non-critical UPSes around the house, and I haven't replaced them yet), so I want to understand what I'm looking for before rewiring things. |
The properties you set when processing an event aren't kept across subsequent events, so your device would end up without any properties. That's why the original rules also handled change events. |
Newer kernels and older systemd would see no properties set on the NUT handled devices. This is worked-around in newer versions of systemd. |
clepple
added
need testing
|
@hadess did a quick sanity check, and everything seems to work fine with Ubuntu 20.04, plus the following NUT, systemd, and kernel versions. Thanks again for the fix and patient explanations. commit 38b5f96 |
3 participants
Misc fixes