Dave Allan wrote:
> On Wed, May 26, 2010 at 03:55:44PM -0400, Cole Robinson wrote:
>> Wireless NICs were being ignored because we weren't correctly handling
>> device type. Fix this, as well as wireless NIC net subtype.
>
> Thanks for finding and fixing this. I made a v2 patch with the change
> that since DEVTYPE == "wlan" is a definitive indication that the
> device is a network interface, that case should be with the other
> device types for which DEVTYPE is definitive. v2 patch attached.
>
> Dave
>
>
>>From ccf89c57a6335c1c7c7e0b29ae09f4916b33622d Mon Sep 17 00:00:00 2001
> From: David Allan <dallan@xxxxxxxxxx>
> Date: Thu, 27 May 2010 10:44:02 -0400
> Subject: [PATCH 1/1] v2 of Cole's wlan support
>
> * Incorporated Jim's feedback
>
> * Moved case of DEVTYPE == "wlan" up as it's definitive that we have a network interface.
>
> * Made comment more detailed about the wired case to explain better
> how it differentiates between wired network interfaces and USB
> devices.
...
> int ret = -1;
Thanks Dave.
> + const char *devtype = udev_device_get_devtype(device);
> union _virNodeDevCapData *data = &def->caps->data;
>
> + if (devtype && STREQ(devtype, "wlan")) {
> + data->net.subtype = VIR_NODE_DEV_CAP_NET_80211;
> + } else {
> + data->net.subtype = VIR_NODE_DEV_CAP_NET_80203;
> + }
> +
> if (udevGetStringProperty(device,
> "INTERFACE",
> &data->net.ifname) == PROPERTY_ERROR) {
> @@ -1074,6 +1081,8 @@ static int udevGetDeviceType(struct udev_device *device,
> int ret = 0;
>
> devtype = udev_device_get_devtype(device);
> + VIR_DEBUG("Found device type '%s' for device '%s'",
> + devtype, udev_device_get_sysname(device));
Sorry I missed it the first time, but since devtype can be
NULL here, we have to avoid dereferencing it:
(it wouldn't cause trouble with glibc, but would segfault on others)
VIR_DEBUG("Found device type '%s' for device '%s'",
NULLSTR(devtype), udev_device_get_sysname(device));
> if (devtype != NULL && STREQ(devtype, "usb_device")) {
> *type = VIR_NODE_DEV_CAP_USB_DEV;
> @@ -1105,17 +1114,25 @@ static int udevGetDeviceType(struct udev_device *device,
> goto out;
> }
>
> + if (devtype != NULL && STREQ(devtype, "wlan")) {
> + *type = VIR_NODE_DEV_CAP_NET;
> + goto out;
> + }
> +
> if (udevGetUintProperty(device, "PCI_CLASS", &tmp, 16) == PROPERTY_FOUND) {
> *type = VIR_NODE_DEV_CAP_PCI_DEV;
> goto out;
> }
>
> - /* It does not appear that network interfaces set the device type
> - * property. */
> - if (devtype == NULL &&
> + /* It does not appear that wired network interfaces set the
> + * DEVTYPE property. USB devices also have an INTERFACE property,
> + * but they do set DEVTYPE, so if devtype is NULL and the
> + * INTERFACE property exists, we have a network device. */
> + if ((devtype == NULL) &&
I wouldn't bother to add parens above.
> udevGetStringProperty(device,
> "INTERFACE",
> &tmp_string) == PROPERTY_FOUND) {
> +
IMHO, that added blank line detracts from readability.
> VIR_FREE(tmp_string);
> *type = VIR_NODE_DEV_CAP_NET;
> goto out;
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list