Dave Allan wrote:
Chris Lalancette wrote:
Daniel P. Berrange wrote:
On Wed, Oct 28, 2009 at 12:16:40PM +0100, Chris Lalancette wrote:
Dave Allan wrote:
Attached is a fully functional version of the node device udev
based backend, incorporating all the feedback from earlier
revisions. I broke the new capability fields out into a separate
patch per Dan's suggestion, and I have also included a patch
removing the DevKit backend.
3) I took a look at how the network is represented in the XML. In
the HAL
backend, we get something that looks like:
<device>
<name>net_00_13_20_f5_fa_e3</name>
<parent>pci_8086_10bd</parent>
<capability type='net'>
<interface>eth0</interface>
<address>00:13:20:f5:fa:e3</address>
<capability type='80203'/>
</capability>
</device>
That "<capability type='80203'/>" looks to be bogus (although I
could be wrong;
that same XML is encoded into the tests, so maybe there is something
else going
on). You are already in a <capability> block, so that should
probably just be
"<type='80203'/>". The same problem occurs in the udev backend.
Why do you think the '<capability type='80203'/>' bit is bogus ?
That looks
correct to me, showing that eth0 is a ethernet device (as opposed to
a 80211
wireless, or neither)
Oh, I think the concept is useful, it's just that the way it is
represented in
the XML looks weird:
<capability type='net'>
...
<capability type='80203'/>
</capability>
Shouldn't this rather be:
<capability type='net'>
...
<type>80203</type>
</capability>
Or:
<capability type='net' subtype='80203'>
...
</capability>
Or something like that?
Here's the latest version of the udev backend. I think I've addressed
all of everybody's concerns, except for the translation of PCI ids to
strings and the bogosity in the ethernet types. I've got working code
for the PCI ids translation, but it's completely separate and involves
modifying the build system again, so I'd like to get this set of patches
in first. I'll sort out the ethernet types in a follow up patch as
well. There's some additional follow up work to make the device tree
look really nice, but that's also completely separate.
Dave
Attached are two additional patches. The first fixes a bug I found
where I was reading the wrong sysfs attribute name, so the PCI device ID
wasn't getting populated. The second uses libpciaccess to translate the
PCI vendor and device IDs into their human readable names.
Dave
>From 2d22b32055a25b03d25c0ab164fc654a503c2795 Mon Sep 17 00:00:00 2001
From: David Allan <dal...@redhat.com>
Date: Wed, 4 Nov 2009 03:06:05 -0500
Subject: [PATCH 1/2] Fix mis-named PCI device ID property
---
src/node_device/node_device_udev.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c
index 10d09fe..ed36da0 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -304,7 +304,7 @@ static int udevProcessPCI(struct udev_device *device,
goto out;
}
- if (udevGetUintSysfsAttr(device, "product", &data->pci_dev.product, 0)
+ if (udevGetUintSysfsAttr(device, "device", &data->pci_dev.product, 0)
== PROPERTY_ERROR) {
goto out;
}
--
1.6.5.1
>From c3bcc186cf76f7a3c717c4cb43e130b19bbd985f Mon Sep 17 00:00:00 2001
From: David Allan <dal...@redhat.com>
Date: Wed, 4 Nov 2009 03:05:30 -0500
Subject: [PATCH 2/2] Add translation of PCI vendor and product IDs
This patch uses libpciaccess to provide human readable names for PCI vendor and
device IDs.
---
configure.in | 9 +++++++
src/Makefile.am | 4 +-
src/node_device/node_device_udev.c | 45 ++++++++++++++++++++++++++++++++++-
3 files changed, 54 insertions(+), 4 deletions(-)
diff --git a/configure.in b/configure.in
index 8254e94..b70e628 100644
--- a/configure.in
+++ b/configure.in
@@ -1727,10 +1727,19 @@ if test "x$with_udev" = "xyes" -o "x$with_udev" =
"xcheck"; then
CFLAGS="$old_CFLAGS"
LDFLAGS="$old_LDFLAGS"
fi
+ PCIACCESS_REQUIRED=0.10.0
+ PCIACCESS_CFLAGS=
+ PCIACCESS_LIBS=
+ PKG_CHECK_MODULES([PCIACCESS], [pciaccess >= $PCIACCESS_REQUIRED], [],
[PCIACCESS_FOUND=no])
+ if test "$PCIACCESS_FOUND" = "no" ; then
+ AC_MSG_ERROR([You must install libpciaccess/libpciaccess-devel >=
$PCIACCESS_REQUIRED to compile libvirt])
+ fi
fi
AM_CONDITIONAL([HAVE_UDEV], [test "x$with_udev" = "xyes"])
AC_SUBST([UDEV_CFLAGS])
AC_SUBST([UDEV_LIBS])
+AC_SUBST([PCIACCESS_CFLAGS])
+AC_SUBST([PCIACCESS_LIBS])
with_nodedev=no;
if test "$with_hal" = "yes" -o "$with_udev" = "yes";
diff --git a/src/Makefile.am b/src/Makefile.am
index 4459ad8..12fb2a9 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -644,8 +644,8 @@ libvirt_driver_nodedev_la_LDFLAGS += $(HAL_LIBS)
endif
if HAVE_UDEV
libvirt_driver_nodedev_la_SOURCES += $(NODE_DEVICE_DRIVER_UDEV_SOURCES)
-libvirt_driver_nodedev_la_CFLAGS += $(UDEV_CFLAGS)
-libvirt_driver_nodedev_la_LDFLAGS += $(UDEV_LIBS)
+libvirt_driver_nodedev_la_CFLAGS += $(UDEV_CFLAGS) $(PCIACCESS_CFLAGS)
+libvirt_driver_nodedev_la_LDFLAGS += $(UDEV_LIBS) $(PCIACCESS_LIBS)
endif
if WITH_DRIVER_MODULES
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c
index ed36da0..c749187 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -22,6 +22,7 @@
#include <config.h>
#include <libudev.h>
+#include <pciaccess.h>
#include <scsi/scsi.h>
#include "node_device_udev.h"
@@ -251,6 +252,42 @@ static void udevLogFunction(struct udev *udev
ATTRIBUTE_UNUSED,
}
+static int udevTranslatePCIIds(unsigned int vendor,
+ unsigned int product,
+ char **vendor_string,
+ char **product_string)
+{
+ int ret = 0;
+ struct pci_id_match m;
+
+ ret = pci_system_init();
+ if (ret != 0) {
+ goto out;
+ }
+
+ m.vendor_id = vendor;
+ m.device_id = product;
+ m.subvendor_id = PCI_MATCH_ANY;
+ m.subdevice_id = PCI_MATCH_ANY;
+ m.device_class = 0;
+ m.device_class_mask = 0;
+ m.match_data = 0;
+
+ /* pci_get_strings returns void */
+ pci_get_strings(&m,
+ (const char **)product_string,
+ (const char **)vendor_string,
+ NULL,
+ NULL);
+
+ /* pci_system_cleanup returns void */
+ pci_system_cleanup();
+
+out:
+ return ret;
+}
+
+
static int udevProcessPCI(struct udev_device *device,
virNodeDeviceDefPtr def)
{
@@ -309,8 +346,12 @@ static int udevProcessPCI(struct udev_device *device,
goto out;
}
- /* XXX FIXME: to do the vendor name and product name, we have to
- * parse /usr/share/hwdata/pci.ids. Use libpciaccess perhaps? */
+ if (udevTranslatePCIIds(data->pci_dev.vendor,
+ data->pci_dev.product,
+ &data->pci_dev.vendor_name,
+ &data->pci_dev.product_name) != 0) {
+ goto out;
+ }
if (udevGenerateDeviceName(device, def, NULL) != 0) {
goto out;
--
1.6.5.1
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
