Tolerate new udev actions "bind" and "unbind" #6
Conversation
Recent kernel and udev versions have added new actions that are triggered on top of the usual add and remove actions. This causes the script to `exit 1` every time, which appears as a warning in udev logs. Why not a big deal, this is ugly, this patch fixes that and protects us against future new events as well.
|
It’s in addition, which means that your change in #7 will try to attach twice, which is probably harmless but still not great. |
|
@navaati I tried the navaati:patch-1 branch, and the USB device did not attach to the VM as expected. The virsh attach-device command failed with
When I use my branch Geekdude:master, it does indeed run the script twice; on the add action it fails with the same error, then on the bind action, it succeeds.
|
|
Oh ok that’s much curious, thanks for the testing. Let me see my current file… Ahah ! Interesting, I have an additionnal "sleep 1" at the end (in between line 100 and 101). So maybe the "add" action is too early and the "bind" action is the right one, and we can kind of combine both our patches to arrive at the right result… We need to check the udev doc regarding the lifecycle and timing of events. |
|
Most extensive documentation seems to be this LWN article (bless jcorbet): https://lwn.net/Articles/837033/. |
|
Ok so reading that, definitely "bind" is the later event and the one we want to use. I’ll rework my branch later today to use it (and ignore other events, in particular add) and remove the sleep, then I’ll test-drive that for a week. It makes the change more impactful, though, because it makes the script not work on systemd versions older than 247 (I think CentOS7 uses systemd 219), that’ll be a decision to be taken by the maintainers. |
|
I was thinking along the same lines as well. If bind is the event we want, is it better to break backward capability by only responding to bind events or to maintain backward capability, but to have the add action fail? The proper approach is probably to have conditionals that check the version systemd/kernel and respond according. Also, according to this line, "For drivers that support the new mechanism, a BIND event for a device will follow the ADD event once the device is ready to operate." We might still want to respond to both event types since, depending on the drivers, there might only be an ADD event. I don't think there is any issue with trying to attach a USB device twice with virsh, but that might be something we should test. So we really want bind events, but if there will not be a bind event, then the script should respond to the add event. |
As suggested in discussion under PR olavmrk#6 which is also absorbed, further changes were needed but they never materialised. This change is not compatible with systemd older than 247.
Recent kernel and udev versions have added new actions that are triggered on top of the usual add and remove actions. This causes the script to
exit 1every time, which appears as a warning in udev logs. While not a big deal, this is ugly. This patch fixes that and protects us against future new events as well.