[libvirt] [PATCH] node_device_udev: Try harder to get human readable vendor:product
The manufacurer and product from USB device itself are usually not particularly useful -- they tend to be missing, or ugly (all-uppercase, padded with spaces, etc.). Prefer what's in the usb id database and fall back to descriptors only if the device is too now to be in database. --- Before: http://v3.sk/~lkundrak/virt/old.png After: http://v3.sk/~lkundrak/virt/libvirt.png src/node_device/node_device_udev.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 0fe474d..53792f0 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -580,6 +580,7 @@ static int udevProcessUSBDevice(struct udev_device *device, { union _virNodeDevCapData *data = &def->caps->data; int ret = -1; + int err; if (udevGetUintProperty(device, "BUSNUM", @@ -602,10 +603,17 @@ static int udevProcessUSBDevice(struct udev_device *device, goto out; } - if (udevGetStringSysfsAttr(device, - "manufacturer", - &data->usb_dev.vendor_name) == PROPERTY_ERROR) { + err = udevGetStringProperty(device, + "ID_VENDOR_FROM_DATABASE", + &data->usb_dev.vendor_name); + if (err == PROPERTY_ERROR) goto out; + if (err == PROPERTY_MISSING) { + if (udevGetStringSysfsAttr(device, + "manufacturer", + &data->usb_dev.vendor_name) == PROPERTY_ERROR) { + goto out; + } } if (udevGetUintProperty(device, @@ -615,10 +623,17 @@ static int udevProcessUSBDevice(struct udev_device *device, goto out; } - if (udevGetStringSysfsAttr(device, - "product", - &data->usb_dev.product_name) == PROPERTY_ERROR) { + err = udevGetStringProperty(device, + "ID_MODEL_FROM_DATABASE", + &data->usb_dev.product_name); + if (err == PROPERTY_ERROR) goto out; + if (err == PROPERTY_MISSING) { + if (udevGetStringSysfsAttr(device, + "product", + &data->usb_dev.product_name) == PROPERTY_ERROR) { + goto out; + } } if (udevGenerateDeviceName(device, def, NULL) != 0) { -- 1.9.3
On 09/09/2014 02:20 PM, Lubomir Rintel wrote:
The manufacurer and product from USB device itself are usually not particularly useful -- they tend to be missing, or ugly (all-uppercase, padded with spaces, etc.). Prefer what's in the usb id database and fall back to descriptors only if the device is too now to be in database.
s/now/new/
--- Before: http://v3.sk/~lkundrak/virt/old.png After: http://v3.sk/~lkundrak/virt/libvirt.png
src/node_device/node_device_udev.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-)
ACK I have included the bug link in the commit message: https://bugzilla.redhat.com/show_bug.cgi?id=1138887 and pushed it. Congratulations on your first libvirt patch! Jan
On Tue, Sep 09, 2014 at 02:20:43PM +0200, Lubomir Rintel wrote:
The manufacurer and product from USB device itself are usually not particularly useful -- they tend to be missing, or ugly (all-uppercase, padded with spaces, etc.). Prefer what's in the usb id database and fall back to descriptors only if the device is too now to be in database.
This last sentence doesn't make much sense.
--- Before: http://v3.sk/~lkundrak/virt/old.png After: http://v3.sk/~lkundrak/virt/libvirt.png
URLs aren't accessible for me, but I guess this is text-only anyway, so why couldn't it be part of the message?
src/node_device/node_device_udev.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 0fe474d..53792f0 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -580,6 +580,7 @@ static int udevProcessUSBDevice(struct udev_device *device, { union _virNodeDevCapData *data = &def->caps->data; int ret = -1; + int err;
if (udevGetUintProperty(device, "BUSNUM", @@ -602,10 +603,17 @@ static int udevProcessUSBDevice(struct udev_device *device, goto out; }
- if (udevGetStringSysfsAttr(device, - "manufacturer", - &data->usb_dev.vendor_name) == PROPERTY_ERROR) { + err = udevGetStringProperty(device, + "ID_VENDOR_FROM_DATABASE", + &data->usb_dev.vendor_name); + if (err == PROPERTY_ERROR) goto out; + if (err == PROPERTY_MISSING) { + if (udevGetStringSysfsAttr(device, + "manufacturer", + &data->usb_dev.vendor_name) == PROPERTY_ERROR) { + goto out; + } }
Bare with me as I'm not that familiar with udev API, but does this make sense for USB devices only? Other than that, the vendor name looks like a output-only, so it can be changed and apps shouldn't rely on it. Martin
participants (3)
-
Ján Tomko -
Lubomir Rintel -
Martin Kletzander