lxd/device: Add support for discovering multiple unix-hotplug devices

[kadinsayani]

Fixes 1/2 related issues raised in #14266. This fix has been tested locally with a USB hard disk and by @jonathan-conder with USB camera devices.

Summary of changes:

• Adds support for discovering multiple unix-hotplug devices which share the same productid and/or vendorid;
• Improves productid and vendorid matching logic;
• Resolves inconsistent device matching bug.

[kadinsayani]

I was trying to reason about why the initial implementation of loadUnixDevice() only matched the first device with a matching product and vendor id since this logic is explicitly described in the go doc comment. I found the original PR and this comment: #6530 (comment).

I don't think the approach in this PR is quite right. Furthermore, wrt #14373, I found this comment which provides reason to not add support for specifying subsystem types: #6530 (comment).

We'll have to rethink our solution here.

[tomponline]

I was trying to reason about why the initial implementation of loadUnixDevice() only matched the first device with a matching product and vendor id since this logic is explicitly described in the go doc comment. I found the original PR and this comment: #6530 (comment).

I don't think the approach in this PR is quite right. Furthermore, wrt #14373, I found this comment which provides reason to not add support for specifying subsystem types: #6530 (comment).

We'll have to rethink our solution here.

Which part of the comment makes you think multiple devices shouldn't be allowed?

[kadinsayani]

Which part of the comment makes you think multiple devices shouldn't be allowed?

Here's the part of the comment that suggests why only the first device is matched:

If we don't specify a subsystem we end up enumerating multiple devices in loadUnixDevice(). For example, if we plug in a usb with a FS on it, we get a device for the usb itself and then a device for the sda device node entry as well since they share the same product/vendor IDs

[kadinsayani]

Looks like this was an issue raised in the original PR but was never actually addressed properly:

The partitions (sda1, sda2 and sda3) only showed up during USB hotplug, on container start or when adding the device, only sda shows up

Also, this commit added logic to not add a device with a major or minor number of 0, but for most usb devices, the first sub device has a minor number of 0.

[kadinsayani]

Thanks to @jonathan-conder for testing this locally with his camera devices. The camera devices showed up as expected, but a mouse with a matching vendor id also appeared. This is because when udev_enumerate_add_match_property is called more than once, each matching result is ORed - only at least one property needs to match.

It's now clear why this commit added logic to not add a device with a major or minor number of 0. However, this approach doesn't allow all devices with a matching product or vendor id to be captured. For example, if we ignore devices with a minor number of 0, we may see cases like this:

$ ls -l /dev/video*
crw-rw----+ 1 root video 81, 0 Oct 19 05:29 /dev/video0
crw-rw----+ 1 root video 81, 1 Oct 19 05:29 /dev/video1
crw-rw----+ 1 root video 81, 2 Oct 19 05:41 /dev/video2
crw-rw----+ 1 root video 81, 3 Oct 19 05:41 /dev/video3
$ lxc exec u1 -- sh -c 'ls -l /dev/video*'
crw-rw---- 1 root root  81, 1 Oct 18 16:39 /dev/video1
crw-rw---- 1 root video 81, 2 Oct 18 16:41 /dev/video2
crw-rw---- 1 root video 81, 3 Oct 18 16:41 /dev/video3

We should see /dev/video[0-3].

I think the best approach is to require specifying subsystem type along with either vendor or product id. Requiring subsystem type to be specified will lead to less "edge cases". @tomponline, what do you think?

[kadinsayani]

Or, we just need a way to filter on both vendor id and product id. This would also be helpful for #14373 so we can filter on subsystem && vendorid. I'll look into this further.

[kadinsayani]

I believe I've got a fix working. Just waiting on a test with camera devices and then this PR will be good to go.

[kadinsayani]

The fix is now confirmed to be working. We've noticed another inconsistency where /dev/bus/usb/... devices are excluded from what is added by unix-hotplug, however, the device(s) appear after replugging the device. This comment provides some insight into the original decision for excluding /dev/bus/usb/... devices. I don't think we want to change this behaviour, but rather fix the inconsistency where /dev/bus/usb/... devices are added after replugging a device.

[jonathan-conder]

As Kadin says, I'm happy with this version. For our use case it doesn't matter whether the USB device is added or not, but based on that comment I agree it makes sense to avoid adding the USB device when replugging

[kadinsayani]

I've added a commit which resolves the inconsistent device matching bug. /dev/bus/usb/... will be ignored when unix-hotplug devices are registered and started (replugged or after container restart).

@tomponline this PR now fully resolves the issue.