From a3f9709566543f17f1cc1e62b8435fd8358932b5 Mon Sep 17 00:00:00 2001 From: Kadin Sayani Date: Wed, 30 Oct 2024 05:54:17 -0600 Subject: [PATCH 1/2] lxd/device: Add support for discovering multiple unix hotplug devices Signed-off-by: Kadin Sayani --- lxd/device/unix_hotplug.go | 73 ++++++++++++++++++++++---------------- 1 file changed, 42 insertions(+), 31 deletions(-) diff --git a/lxd/device/unix_hotplug.go b/lxd/device/unix_hotplug.go index fdd259879028..b3e7e57d8e7d 100644 --- a/lxd/device/unix_hotplug.go +++ b/lxd/device/unix_hotplug.go @@ -89,8 +89,7 @@ func (d *unixHotplug) validateConfig(instConf instance.ConfigReader) error { // Register is run after the device is started or when LXD starts. func (d *unixHotplug) Register() error { - // Extract variables needed to run the event hook so that the reference to this device - // struct is not needed to be kept in memory. + // Extract variables needed to run the event hook so that the reference to this device struct is not required to be stored in memory. devicesPath := d.inst.DevicesPath() devConfig := d.config deviceName := d.name @@ -149,29 +148,36 @@ func (d *unixHotplug) Start() (*deviceConfig.RunConfig, error) { runConf := deviceConfig.RunConfig{} runConf.PostHooks = []func() error{d.Register} - device := d.loadUnixDevice() - if d.isRequired() && device == nil { + devices := d.loadUnixDevices() + if d.isRequired() && devices == nil { return nil, fmt.Errorf("Required Unix Hotplug device not found") } - if device == nil { + if devices == nil { return &runConf, nil } - devnum := device.Devnum() - major := uint32(devnum.Major()) - minor := uint32(devnum.Minor()) + for _, device := range devices { + devnum := device.Devnum() + major := uint32(devnum.Major()) + minor := uint32(devnum.Minor()) + + // Setup device. + var err error + if device.Subsystem() == "block" { + err = unixDeviceSetupBlockNum(d.state, d.inst.DevicesPath(), "unix", d.name, d.config, major, minor, device.Devnode(), false, &runConf) + } else { + err = unixDeviceSetupCharNum(d.state, d.inst.DevicesPath(), "unix", d.name, d.config, major, minor, device.Devnode(), false, &runConf) + } - // setup device - var err error - if device.Subsystem() == "block" { - err = unixDeviceSetupBlockNum(d.state, d.inst.DevicesPath(), "unix", d.name, d.config, major, minor, device.Devnode(), false, &runConf) - } else { - err = unixDeviceSetupCharNum(d.state, d.inst.DevicesPath(), "unix", d.name, d.config, major, minor, device.Devnode(), false, &runConf) - } + if err != nil { + err := unixDeviceRemove(d.inst.DevicesPath(), "unix", d.name, "", &runConf) + if err != nil { + return nil, err + } - if err != nil { - return nil, err + return nil, err + } } return &runConf, nil @@ -203,9 +209,9 @@ func (d *unixHotplug) postStop() error { return nil } -// loadUnixDevice scans the host machine for unix devices with matching product/vendor ids -// and returns the first matching device with the subsystem type char or block. -func (d *unixHotplug) loadUnixDevice() *udev.Device { +// loadUnixDevices scans the host machine for unix devices with matching product/vendor ids +// and returns the matching devices with subsystem types of char or block. +func (d *unixHotplug) loadUnixDevices() []*udev.Device { // Find device if exists u := udev.Udev{} e := u.NewEnumerate() @@ -230,27 +236,32 @@ func (d *unixHotplug) loadUnixDevice() *udev.Device { } devices, _ := e.Devices() - var device *udev.Device + var matchingDevices []*udev.Device for i := range devices { - device = devices[i] + device := devices[i] - if device == nil { + if device == nil || device.Devnum().Major() == 0 || device.Devnode() == "" || strings.HasPrefix(device.Subsystem(), "usb") { continue } - devnum := device.Devnum() - if devnum.Major() == 0 || devnum.Minor() == 0 { - continue + match := false + vendorIDMatch := device.PropertyValue("ID_VENDOR_ID") == d.config["vendorid"] + productIDMatch := device.PropertyValue("ID_MODEL_ID") == d.config["productid"] + + if d.config["vendorid"] != "" && d.config["productid"] != "" { + match = vendorIDMatch && productIDMatch + } else if d.config["vendorid"] != "" { + match = vendorIDMatch + } else if d.config["productid"] != "" { + match = productIDMatch } - if device.Devnode() == "" { + if !match { continue } - if !strings.HasPrefix(device.Subsystem(), "usb") { - return device - } + matchingDevices = append(matchingDevices, device) } - return nil + return matchingDevices } From ae8a3e5600d9f962c09719f7f84e0e8e877710a3 Mon Sep 17 00:00:00 2001 From: Kadin Sayani Date: Thu, 14 Nov 2024 09:15:58 -0700 Subject: [PATCH 2/2] lxd/device: Consolidate device matching logic to resolve inconsistent matching This commit consolidates `unix-hotplug` device matching logic into a new function, `deviceMatch`. Consolidating matching logic prevents inconsistent matching between `Register` and `Start`. Signed-off-by: Kadin Sayani --- lxd/device/unix_hotplug.go | 39 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/lxd/device/unix_hotplug.go b/lxd/device/unix_hotplug.go index b3e7e57d8e7d..4bda5376f2ae 100644 --- a/lxd/device/unix_hotplug.go +++ b/lxd/device/unix_hotplug.go @@ -16,16 +16,25 @@ import ( "github.com/canonical/lxd/shared/validate" ) -// unixHotplugIsOurDevice indicates whether the unixHotplug device event qualifies as part of our device. -// This function is not defined against the unixHotplug struct type so that it can be used in event -// callbacks without needing to keep a reference to the unixHotplug device struct. -func unixHotplugIsOurDevice(config deviceConfig.Device, unixHotplug *UnixHotplugEvent) bool { - // Check if event matches criteria for this device, if not return. - if (config["vendorid"] != "" && config["vendorid"] != unixHotplug.Vendor) || (config["productid"] != "" && config["productid"] != unixHotplug.Product) { +// unixHotplugDeviceMatch matches a unix-hotplug device based on vendorid and productid. USB bus and devices with a major number of 0 are ignored. This function is used to indicate whether a unix hotplug event qualifies as part of our registered devices, and to load matching devices. +func unixHotplugDeviceMatch(config deviceConfig.Device, vendorid string, productid string, subsystem string, major uint32) bool { + match := false + vendorIDMatch := vendorid == config["vendorid"] + productIDMatch := productid == config["productid"] + + if strings.HasPrefix(subsystem, "usb") || major == 0 { return false } - return true + if config["vendorid"] != "" && config["productid"] != "" { + match = vendorIDMatch && productIDMatch + } else if config["vendorid"] != "" { + match = vendorIDMatch + } else if config["productid"] != "" { + match = productIDMatch + } + + return match } type unixHotplug struct { @@ -100,7 +109,7 @@ func (d *unixHotplug) Register() error { runConf := deviceConfig.RunConfig{} if e.Action == "add" { - if !unixHotplugIsOurDevice(devConfig, &e) { + if !unixHotplugDeviceMatch(devConfig, e.Vendor, e.Product, e.Subsystem, e.Major) { return nil, nil } @@ -240,21 +249,11 @@ func (d *unixHotplug) loadUnixDevices() []*udev.Device { for i := range devices { device := devices[i] - if device == nil || device.Devnum().Major() == 0 || device.Devnode() == "" || strings.HasPrefix(device.Subsystem(), "usb") { + if device == nil || device.Devnode() == "" { continue } - match := false - vendorIDMatch := device.PropertyValue("ID_VENDOR_ID") == d.config["vendorid"] - productIDMatch := device.PropertyValue("ID_MODEL_ID") == d.config["productid"] - - if d.config["vendorid"] != "" && d.config["productid"] != "" { - match = vendorIDMatch && productIDMatch - } else if d.config["vendorid"] != "" { - match = vendorIDMatch - } else if d.config["productid"] != "" { - match = productIDMatch - } + match := unixHotplugDeviceMatch(d.config, device.PropertyValue("ID_VENDOR_ID"), device.PropertyValue("ID_MODEL_ID"), device.Subsystem(), uint32(device.Devnum().Major())) if !match { continue