Re: [libvirt] udev node device backend

12 Nov 2009

Daniel P. Berrange wrote:
...
On Wed, Nov 11, 2009 at 05:06:18PM -0500, Dave Allan wrote:
...
...
From 94d99c19668d3c804c84ff878023b0f93560dc81 Mon Sep 17 00:00:00 2001
From: David Allan <dallan@redhat.com>
Date: Fri, 16 Oct 2009 16:52:40 -0400
Subject: [PATCH 1/6] Add several fields to node device capabilities
---
 src/conf/node_device_conf.c |   22 ++++++++++++++++++++++
 src/conf/node_device_conf.h |    7 +++++++
 2 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index c5083cc..ece339f 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -248,6 +248,12 @@ char *virNodeDeviceDefFormat(virConnectPtr conn,
             if (data->system.product_name)
                 virBufferEscapeString(&buf, "    <product>%s</product>\n",
                                       data->system.product_name);
+            if (data->system.dmi_devpath)
+                virBufferEscapeString(&buf, "    <dmi_devpath>%s</dmi_devpath>\n",
+                                      data->system.dmi_devpath);
I don't think we should be exposing this in the XML. It is a linux
specific concepts. We expose some relevant bits of DMI data in the
XML elsewhere, so would rather we added more data, than point clients
to sysfs.
Removed entirely.
...
...
+            if (data->system.description)
+                virBufferEscapeString(&buf, "    <description>%s</description>\n",
+                                      data->system.description);
I'm also not sure what this is useful for ? All it  seems to output is
<description>fictional device to root the node device tree</description>
which is really just documentation about the schema, not something
that needs to be included in actual document output.
I thought it might be used to describe the actual system at some point, 
but it's not important.  Removed.
...
...
virBufferAddLit(&buf, "    <hardware>\n");
             if (data->system.hardware.vendor_name)
                 virBufferEscapeString(&buf, "      <vendor>%s</vendor>\n",
@@ -325,6 +331,9 @@ char *virNodeDeviceDefFormat(virConnectPtr conn,
                               data->usb_if.subclass);
             virBufferVSprintf(&buf, "    <protocol>%d</protocol>\n",
                               data->usb_if.protocol);
+            if (data->usb_if.interface_name)
+                virBufferVSprintf(&buf, "    <interface_name>%s</interface_name>\n",
+                                  data->usb_if.interface_name);
What are the semantics of this element ?
On my system it comes out as
<interface_name>9/0/0</interface_name>
which is pretty much duplicating info already available in a structured
format
<class>9</class>
    <subclass>0</subclass>
    <protocol>0</protocol>
So do we actually need to add this new element ?
Removed.
...
...
...
From ecb4c2c2a0e42ed5f7578441b5290980663c549c Mon Sep 17 00:00:00 2001
From: David Allan <dallan@redhat.com>
Date: Tue, 3 Nov 2009 21:16:51 -0500
Subject: [PATCH 2/6] Implement a node device backend using libudev.
Monitoring for addition and removal of devices is implemented.
There is a lot of detail work in this code, so we should try to get people running it on a wide variety of hardware so we can shake out the differences in implementation between the HAL and libudev backends.
I have moved the new fields in the node device capabilities to a separate patch.
This version contains changes per all the feedback I've received on earlier versions.
---
...
diff --git a/configure.in b/configure.in
index 7ad1a90..4e5afef 100644
--- a/configure.in
+++ b/configure.in
@@ -1654,7 +1654,7 @@ test "$enable_shared" = no && lt_cv_objdir=.
 LV_LIBTOOL_OBJDIR=${lt_cv_objdir-.}
 AC_SUBST([LV_LIBTOOL_OBJDIR])
-dnl HAL or DeviceKit library for host device enumeration
+dnl HAL, DeviceKit, or libudev library for host device enumeration
 HAL_REQUIRED=0.0
 HAL_CFLAGS=
 HAL_LIBS=
@@ -1748,8 +1748,46 @@ AM_CONDITIONAL([HAVE_DEVKIT], [test "x$with_devkit" = "xyes"])
 AC_SUBST([DEVKIT_CFLAGS])
 AC_SUBST([DEVKIT_LIBS])
+UDEV_REQUIRED=143
Already agreed on IRC that we should increase this to 145
Done.
...
...
+UDEV_CFLAGS=
+UDEV_LIBS=
+AC_ARG_WITH([udev],
+  [  --with-udev        use libudev for host device enumeration],
+  [],
+  [with_udev=check])
+
+if test "$with_libvirtd" = "no" ; then
+  with_udev=no
+fi
+if test "x$with_udev" = "xyes" -o "x$with_udev" = "xcheck"; then
+  PKG_CHECK_MODULES(UDEV, libudev >= $UDEV_REQUIRED,
+    [with_udev=yes], [
+    if test "x$with_udev" = "xcheck" ; then
+       with_udev=no
+    else
+       AC_MSG_ERROR(
+         [You must install udev-devel >= $UDEV_REQUIRED to compile libvirt])
Typo here  -  libudev-devel
Fixed.
...
...
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index ece339f..4d1bfda 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -91,6 +91,26 @@ int virNodeDeviceHasCap(const virNodeDeviceObjPtr dev, const char *cap)
     return 0;
 }
+
+virNodeDeviceObjPtr
+virNodeDeviceFindByUdevName(const virNodeDeviceObjListPtr devs,
+                            const char *udev_name)
+{
+    unsigned int i;
+
+    for (i = 0; i < devs->count; i++) {
+        virNodeDeviceObjLock(devs->objs[i]);
+        if ((devs->objs[i]->def->udev_name != NULL) &&
+            (STREQ(devs->objs[i]->def->udev_name, udev_name))) {
+            return devs->objs[i];
+        }
+        virNodeDeviceObjUnlock(devs->objs[i]);
+    }
+
+    return NULL;
+}
+
+
 virNodeDeviceObjPtr virNodeDeviceFindByName(const virNodeDeviceObjListPtr devs,
                                             const char *name)
 {
@@ -117,6 +137,8 @@ void virNodeDeviceDefFree(virNodeDeviceDefPtr def)
     VIR_FREE(def->name);
     VIR_FREE(def->parent);
     VIR_FREE(def->driver);
+    VIR_FREE(def->udev_name);
+    VIR_FREE(def->parent_udev_name);
caps = def->caps;
     while (caps) {
@@ -228,9 +250,17 @@ char *virNodeDeviceDefFormat(virConnectPtr conn,
virBufferAddLit(&buf, "<device>\n");
     virBufferEscapeString(&buf, "  <name>%s</name>\n", def->name);
-
-    if (def->parent)
+    if (def->udev_name != NULL) {
+        virBufferEscapeString(&buf, "  <udev_name>%s</udev_name>\n",
+                              def->udev_name);
+    }
+    if (def->parent) {
         virBufferEscapeString(&buf, "  <parent>%s</parent>\n", def->parent);
+    }
+    if (def->parent_udev_name != NULL) {
+        virBufferEscapeString(&buf, "  <parent_udev_name>%s</parent_udev_name>\n",
+                              def->parent_udev_name);
+    }
     if (def->driver) {
         virBufferAddLit(&buf, "  <driver>\n");
         virBufferEscapeString(&buf, "    <name>%s</name>\n", def->driver);
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index f70184d..91ef94e 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -164,7 +164,9 @@ typedef struct _virNodeDeviceDef virNodeDeviceDef;
 typedef virNodeDeviceDef *virNodeDeviceDefPtr;
 struct _virNodeDeviceDef {
     char *name;                         /* device name (unique on node) */
+    char *udev_name;                    /* udev name/sysfs path */
     char *parent;			/* optional parent device name */
+    char *parent_udev_name;             /* udev parent name/sysfs path */
     char *driver;                       /* optional driver name */
     virNodeDevCapsDefPtr caps;		/* optional device capabilities */
 };
...
+
+static int udevProcessStorage(struct udev_device *device,
+                              virNodeDeviceDefPtr def)
+{
+    union _virNodeDevCapData *data = &def->caps->data;
+    int ret = -1;
+
+    data->storage.block = strdup(udev_device_get_devnode(device));
+    if (udevGetStringProperty(device,
+                              "DEVNAME",
+                              &data->storage.block) == PROPERTY_ERROR) {
+        goto out;
+    }
+    if (udevGetStringProperty(device,
+                              "ID_BUS",
+                              &data->storage.bus) == PROPERTY_ERROR) {
+        goto out;
+    }
+    if (udevGetStringProperty(device,
+                              "ID_SERIAL",
+                              &data->storage.serial) == PROPERTY_ERROR) {
+        goto out;
+    }
+    if (udevGetStringSysfsAttr(device,
+                               "device/vendor",
+                               &data->storage.vendor) == PROPERTY_ERROR) {
+        goto out;
+    }
+    udevStripSpaces(def->caps->data.storage.vendor);
+    if (udevGetStringSysfsAttr(device,
+                               "device/model",
+                               &data->storage.model) == PROPERTY_ERROR) {
+        goto out;
+    }
+    udevStripSpaces(def->caps->data.storage.model);
+    /* There is no equivalent of the hotpluggable property in libudev,
+     * but storage is going toward a world in which hotpluggable is
+     * expected, so I don't see a problem with not having a property
+     * for it. */
+
+    if (udevGetStringProperty(device,
+                              "ID_TYPE",
+                              &data->storage.drive_type) != PROPERTY_FOUND) {
+        /* If udev doesn't have it, perhaps we can guess it. */
+        if (udevKludgeStorageType(def) != 0) {
+            goto out;
+        }
+    }
+
+    /* NB: drive_type has changed from HAL; now it's "cd" instead of "cdrom" */
+    if (STREQ(def->caps->data.storage.drive_type, "cd")) {
+        ret = udevProcessCDROM(device, def);
Is this comment still accurate ?  It appears to show 'cdrom' for me
when using udev
Fixed in the later patch reflecting Cole's feedback; I've rolled those 
changes into the main patch.
...
<device>
  <name>block_sr0</name>
  <sysfs_path>/sys/devices/pci0000:00/0000:00:01.1/host1/target1:0:0/1:0:0:0/block/sr0</sysfs_path>
  <parent>scsi_1:0:0:0</parent>
  <parent_sysfs_path>/sys/devices/pci0000:00/0000:00:01.1/host1/target1:0:0/1:0:0:0</parent_sysfs_path>
  <capability type='storage'>
    <block>/dev/sr0</block>
    <bus>scsi</bus>
    <drive_type>cdrom</drive_type>
    <model>QEMU DVD-ROM</model>
    <vendor>QEMU</vendor>
    <capability type='removable'>
      <media_available>0</media_available>
      <media_size>0</media_size>
      <logical_block_size>0</logical_block_size>
      <num_blocks>0</num_blocks>
    </capability>
  </capability>
</device>
...
+
+static int udevSetupSystemDev(void)
+{
+    virNodeDeviceDefPtr def = NULL;
+    virNodeDeviceObjPtr dev = NULL;
+    struct udev *udev = NULL;
+    struct udev_device *device = NULL;
+    union _virNodeDevCapData *data = NULL;
+    char *tmp = NULL;
+    int ret = -1;
+
+    if (VIR_ALLOC(def) != 0) {
+        goto out;
+    }
+
+    def->name = strdup("computer");
+    if (def->name == NULL) {
+        goto out;
+    }
+
+    if (VIR_ALLOC(def->caps) != 0) {
+        goto out;
+    }
+
+    udev = udev_monitor_get_udev(DRV_STATE_UDEV_MONITOR(driverState));
+    device = udev_device_new_from_syspath(udev, DMI_DEVPATH);
+    if (device == NULL) {
+        goto out;
+    }
+
+    data = &def->caps->data;
+
+    data->system.dmi_devpath = strdup(DMI_DEVPATH);
+    data->system.description = strdup(SYSTEM_DESCRIPTION);
+
+    if (udevGetStringSysfsAttr(device,
+                               "product_name",
+                               &data->system.product_name) == PROPERTY_ERROR) {
+        goto out;
+    }
+    if (udevGetStringSysfsAttr(device,
+                               "sys_vendor",
+                               &data->system.hardware.vendor_name)
+        == PROPERTY_ERROR) {
+        goto out;
+    }
+    if (udevGetStringSysfsAttr(device,
+                               "product_version",
+                               &data->system.hardware.version)
+        == PROPERTY_ERROR) {
+        goto out;
+    }
+    if (udevGetStringSysfsAttr(device,
+                               "product_serial",
+                               &data->system.hardware.serial)
+        == PROPERTY_ERROR) {
+        goto out;
+    }
+
+    if (udevGetStringSysfsAttr(device,
+                               "product_uuid",
+                               &tmp) == PROPERTY_ERROR) {
+        goto out;
+    }
The udevGetStringSysfsAttr() method is returning empty string
for many of these, while HAL fills the fields with NULL. This
causes the udev generated XML to included many emptry elements.
<hardware>
      <vendor></vendor>
      <version></version>
      <serial></serial>
      <uuid>a6f7e16a-6e5e-a930-bca7-cc597167fab4</uuid>
    </hardware>
So I think udevGetStringSysfsAttr() should convert "" into NULL
Good point--done.
...
...
+#define virBuildPath(path, ...) virBuildPathInternal(path, __VA_ARGS__, NULL)
+int virBuildPathInternal(char **path, ...) __attribute__ ((sentinel));
This should use  ATTRIBUTE_SENTINAL, otherwise it won't compile with
non-gcc, or older gcc.
Fixed.
...
...
diff --git a/tools/virsh.c b/tools/virsh.c
index 0d0ebca..16b3f1c 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -5795,9 +5795,17 @@ cmdNodeListDevices (vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
         char **parents = vshMalloc(ctl, sizeof(char *) * num_devices);
         for (i = 0; i < num_devices; i++) {
             virNodeDevicePtr dev = virNodeDeviceLookupByName(ctl->conn, devices[i]);
+            virNodeDevicePtr parent_dev = NULL;
+
             if (dev && STRNEQ(devices[i], "computer")) {
                 const char *parent = virNodeDeviceGetParent(dev);
-                parents[i] = parent ? strdup(parent) : NULL;
+                parent_dev = virNodeDeviceLookupByName(ctl->conn, parent);
+                if (parent_dev) {
+                    parents[i] = strdup(parent);
+                } else {
+                    parents[i] = strdup("computer");
+                }
+                virNodeDeviceFree(parent_dev);
Why do we need lookup the actual device object here ? Is there 
really a time when we would report a parent which doesn't
actually exist ?
I'm not sure what's up with this--I'm fairly certain that I was losing 
devices from the tree view of virsh nodedev-list without it, but I'm 
currently seeing everything I should be, so I'm clearly losing my mind. 
  :)  In any case, I've reverted this change and everything seems to be 
working ok.
...
...
@@ -251,16 +251,16 @@ char *virNodeDeviceDefFormat(virConnectPtr conn,
virBufferAddLit(&buf, "<device>\n");
     virBufferEscapeString(&buf, "  <name>%s</name>\n", def->name);
-    if (def->udev_name != NULL) {
-        virBufferEscapeString(&buf, "  <udev_name>%s</udev_name>\n",
-                              def->udev_name);
+    if (def->sysfs_path != NULL) {
+        virBufferEscapeString(&buf, "  <sysfs_path>%s</sysfs_path>\n",
+                              def->sysfs_path);
     }
     if (def->parent) {
         virBufferEscapeString(&buf, "  <parent>%s</parent>\n", def->parent);
     }
-    if (def->parent_udev_name != NULL) {
-        virBufferEscapeString(&buf, "  <parent_udev_name>%s</parent_udev_name>\n",
-                              def->parent_udev_name);
+    if (def->parent_sysfs_path != NULL) {
+        virBufferEscapeString(&buf, "  <parent_sysfs_path>%s</parent_sysfs_path>\n",
+                              def->parent_sysfs_path);
     }
I'm not all that keen on us exposing an XML element named sysfs_path here,
since again that's Linux specific concept, and if an app needed more metadata
about a device then we ought to provide it directly, since most apps using
libvirt run remotely & so can't access /sysfs aanyway.
Removed from the XML.
...
One final thought which doesn't really fit elsehwere. In the device
names
# virsh nodedev-list 
block_QEMU_HARDDISK_QM00001
block_sr0
computer
net_54:52:00:39:ee:20
pci_0000:00:00.0
pci_0000:00:01.0
pci_0000:00:01.1
pci_0000:00:01.2
pci_0000:00:01.3
pci_0000:00:02.0
pci_0000:00:03.0
pci_0000:00:04.0
scsi_0:0:0:0
scsi_1:0:0:0
scsi_host0
scsi_host1
scsi_target0:0:0
scsi_target1:0:0
usb_1-0:1.0
usb_1-2
usb_1-2:1.0
usb_usb1
I think it would be worth getting rid of the punctuation characters, just
doing a straight search & replace with '_', for anything which isn't
in the set 0-9, a-Z, _
Good idea.  Done, and added the additional bit of disambiguation of the 
udev device name so devices have the form:

net_eth0_54_52_00_02_c4_46

Updated patch set attached.

Dave
...
From 4e5bbd37d51fb82f3be70ae248403d10c7b373cd Mon Sep 17 00:00:00 2001
From: David Allan <dallan@redhat.com>
Date: Fri, 16 Oct 2009 16:52:40 -0400
Subject: [PATCH 1/5] Add several fields to node device capabilities
---
 src/conf/node_device_conf.c |   22 ++++++++++++++++++++++
 src/conf/node_device_conf.h |    7 +++++++
 2 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index c5083cc..ece339f 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -248,6 +248,12 @@ char *virNodeDeviceDefFormat(virConnectPtr conn,
             if (data->system.product_name)
                 virBufferEscapeString(&buf, "    <product>%s</product>\n",
                                       data->system.product_name);
+            if (data->system.dmi_devpath)
+                virBufferEscapeString(&buf, "    <dmi_devpath>%s</dmi_devpath>\n",
+                                      data->system.dmi_devpath);
+            if (data->system.description)
+                virBufferEscapeString(&buf, "    <description>%s</description>\n",
+                                      data->system.description);
             virBufferAddLit(&buf, "    <hardware>\n");
             if (data->system.hardware.vendor_name)
                 virBufferEscapeString(&buf, "      <vendor>%s</vendor>\n",
@@ -325,6 +331,9 @@ char *virNodeDeviceDefFormat(virConnectPtr conn,
                               data->usb_if.subclass);
             virBufferVSprintf(&buf, "    <protocol>%d</protocol>\n",
                               data->usb_if.protocol);
+            if (data->usb_if.interface_name)
+                virBufferVSprintf(&buf, "    <interface_name>%s</interface_name>\n",
+                                  data->usb_if.interface_name);
             if (data->usb_if.description)
                 virBufferVSprintf(&buf, "    <description>%s</description>\n",
                                   data->usb_if.description);
@@ -394,10 +403,20 @@ char *virNodeDeviceDefFormat(virConnectPtr conn,
                                   "</media_available>\n", avl ? 1 : 0);
                 virBufferVSprintf(&buf, "      <media_size>%llu</media_size>\n",
                                   data->storage.removable_media_size);
+                virBufferVSprintf(&buf, "      <logical_block_size>%llu"
+                                  "</logical_block_size>\n",
+                                  data->storage.logical_block_size);
+                virBufferVSprintf(&buf, "      <num_blocks>%llu</num_blocks>\n",
+                                  data->storage.num_blocks);
                 virBufferAddLit(&buf, "    </capability>\n");
             } else {
                 virBufferVSprintf(&buf, "    <size>%llu</size>\n",
                                   data->storage.size);
+                virBufferVSprintf(&buf, "    <logical_block_size>%llu"
+                                  "</logical_block_size>\n",
+                                  data->storage.logical_block_size);
+                virBufferVSprintf(&buf, "    <num_blocks>%llu</num_blocks>\n",
+                                  data->storage.num_blocks);
             }
             if (data->storage.flags & VIR_NODE_DEV_CAP_STORAGE_HOTPLUGGABLE)
                 virBufferAddLit(&buf,
@@ -1315,6 +1334,8 @@ void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps)
     switch (caps->type) {
     case VIR_NODE_DEV_CAP_SYSTEM:
         VIR_FREE(data->system.product_name);
+        VIR_FREE(data->system.dmi_devpath);
+        VIR_FREE(data->system.description);
         VIR_FREE(data->system.hardware.vendor_name);
         VIR_FREE(data->system.hardware.version);
         VIR_FREE(data->system.hardware.serial);
@@ -1331,6 +1352,7 @@ void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps)
         VIR_FREE(data->usb_dev.vendor_name);
         break;
     case VIR_NODE_DEV_CAP_USB_INTERFACE:
+        VIR_FREE(data->usb_if.interface_name);
         VIR_FREE(data->usb_if.description);
         break;
     case VIR_NODE_DEV_CAP_NET:
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index a7bb6c6..f70184d 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -82,6 +82,8 @@ struct _virNodeDevCapsDef {
     union _virNodeDevCapData {
         struct {
             char *product_name;
+            char *description;
+            char *dmi_devpath;
             struct {
                 char *vendor_name;
                 char *version;
@@ -101,6 +103,7 @@ struct _virNodeDevCapsDef {
             unsigned function;
             unsigned product;
             unsigned vendor;
+            unsigned class;
             char *product_name;
             char *vendor_name;
         } pci_dev;
@@ -117,10 +120,12 @@ struct _virNodeDevCapsDef {
             unsigned _class;		/* "class" is reserved in C */
             unsigned subclass;
             unsigned protocol;
+            char *interface_name;
             char *description;
         } usb_if;
         struct {
             char *address;
+            unsigned address_len;
             char *ifname;
             enum virNodeDevNetCapType subtype;  /* LAST -> no subtype */
         } net;
@@ -139,6 +144,8 @@ struct _virNodeDevCapsDef {
         } scsi;
         struct {
             unsigned long long size;
+            unsigned long long num_blocks;
+            unsigned long long logical_block_size;
             unsigned long long removable_media_size;
             char *block;
             char *bus;
-- 
1.6.5.1
...
From 86231984a2d4001c9bed8e69cf1d300d8628d3ef Mon Sep 17 00:00:00 2001
From: David Allan <dallan@redhat.com>
Date: Thu, 12 Nov 2009 15:21:38 -0500
Subject: [PATCH 2/5] Implement a node device backend using libudev.
Monitoring for addition and removal of devices is implemented.

There is a lot of detail work in this code, so we should try to get people running it on a wide variety of hardware so we can shake out the differences in implementation between the HAL and libudev backends.

I have moved the new fields in the node device capabilities to a separate patch.

This version contains changes per all the feedback I've received on earlier versions.

Fixes per feedback from Cole

* Fixed crash on daemon shutdown due to incorrect free
* Fixed uint64_t warnings
* Fixed log level and added significant additional logging
* Fixed a bug that caused some PCI devices not to be created
* Changed USB human readable vendor & product strings to sysfs source
* Changed struct member from udev_name and parent_udev_name to sysfs_path and parent_sysfs_path to better interoperate with HAL

Fixes per feedback from danpb

* Removed dmi_devpath attribute from system device type
* Removed description attribute from system device type
* Removed interface_name attribute from usb interface device type
* Increased required libudev version to 145
* Fixed typo in error output to direct user to install libudev-devel >= 145
* Made empty strings from Sysfs convert to NULL
* Changed __attribute__ ((sentinel)) to ATTRIBUTE_SENTINEL
* Removed sysfs_path and parent_sysfs_path from XML
* Changed device naming to add device name for disambiguation and to convert non-alphanumeric characters to underscores
* Reverted changes to virsh.c
---
 configure.in                                       |   47 +-
 daemon/libvirtd.c                                  |    3 +-
 po/POTFILES.in                                     |    3 +-
 src/Makefile.am                                    |   16 +-
 src/conf/node_device_conf.c                        |   46 +-
 src/conf/node_device_conf.h                        |    8 +-
 src/libvirt_private.syms                           |    1 +
 src/node_device/node_device_driver.c               |   12 +-
 src/node_device/node_device_driver.h               |   22 +
 src/node_device/node_device_hal.h                  |   19 -
 ...evice_hal_linux.c => node_device_linux_sysfs.c} |    0
 src/node_device/node_device_udev.c                 | 1519 ++++++++++++++++++++
 src/node_device/node_device_udev.h                 |   31 +
 src/util/util.c                                    |   28 +
 src/util/util.h                                    |    3 +
 15 files changed, 1711 insertions(+), 47 deletions(-)
 rename src/node_device/{node_device_hal_linux.c => node_device_linux_sysfs.c} (100%)
 create mode 100644 src/node_device/node_device_udev.c
 create mode 100644 src/node_device/node_device_udev.h

diff --git a/configure.in b/configure.in
index 7ad1a90..dc1e43e 100644
--- a/configure.in
+++ b/configure.in
@@ -1654,7 +1654,7 @@ test "$enable_shared" = no && lt_cv_objdir=.
 LV_LIBTOOL_OBJDIR=${lt_cv_objdir-.}
 AC_SUBST([LV_LIBTOOL_OBJDIR])

-dnl HAL or DeviceKit library for host device enumeration
+dnl HAL, DeviceKit, or libudev library for host device enumeration
 HAL_REQUIRED=0.0
 HAL_CFLAGS=
 HAL_LIBS=
@@ -1748,8 +1748,46 @@ AM_CONDITIONAL([HAVE_DEVKIT], [test "x$with_devkit" = "xyes"])
 AC_SUBST([DEVKIT_CFLAGS])
 AC_SUBST([DEVKIT_LIBS])

+UDEV_REQUIRED=145
+UDEV_CFLAGS=
+UDEV_LIBS=
+AC_ARG_WITH([udev],
+  [  --with-udev        use libudev for host device enumeration],
+  [],
+  [with_udev=check])
+
+if test "$with_libvirtd" = "no" ; then
+  with_udev=no
+fi
+if test "x$with_udev" = "xyes" -o "x$with_udev" = "xcheck"; then
+  PKG_CHECK_MODULES(UDEV, libudev >= $UDEV_REQUIRED,
+    [with_udev=yes], [
+    if test "x$with_udev" = "xcheck" ; then
+       with_udev=no
+    else
+       AC_MSG_ERROR(
+         [You must install libudev-devel >= $UDEV_REQUIRED to compile libvirt])
+    fi
+  ])
+  if test "x$with_udev" = "xyes" ; then
+    AC_DEFINE_UNQUOTED([HAVE_UDEV], 1,
+      [use UDEV for host device enumeration])
+
+    old_CFLAGS=$CFLAGS
+    old_LDFLAGS=$LDFLAGS
+    CFLAGS="$CFLAGS $UDEV_CFLAGS"
+    LDFLAGS="$LDFLAGS $UDEV_LIBS"
+    AC_CHECK_FUNCS([udev_new],,[with_udev=no])
+    CFLAGS="$old_CFLAGS"
+    LDFLAGS="$old_LDFLAGS"
+  fi
+fi
+AM_CONDITIONAL([HAVE_UDEV], [test "x$with_udev" = "xyes"])
+AC_SUBST([UDEV_CFLAGS])
+AC_SUBST([UDEV_LIBS])
+
 with_nodedev=no;
-if test "$with_devkit" = "yes" -o "$with_hal" = "yes";
+if test "$with_devkit" = "yes" -o "$with_hal" = "yes" -o "$with_udev" = "yes";
 then
   with_nodedev=yes
   AC_DEFINE_UNQUOTED([WITH_NODE_DEVICES], 1, [with node device driver])
@@ -1914,6 +1952,11 @@ AC_MSG_NOTICE([  devkit: $DEVKIT_CFLAGS $DEVKIT_LIBS])
 else
 AC_MSG_NOTICE([  devkit: no])
 fi
+if test "$with_udev" = "yes" ; then
+AC_MSG_NOTICE([    udev: $UDEV_CFLAGS $UDEV_LIBS])
+else
+AC_MSG_NOTICE([    udev: no])
+fi
 if test "$with_netcf" = "yes" ; then
 AC_MSG_NOTICE([   netcf: $NETCF_CFLAGS $NETCF_LIBS])
 else
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 01c9bbc..ef07460 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -877,8 +877,7 @@ static struct qemud_server *qemudInitialize(void) {
 #ifdef WITH_STORAGE_DIR
     storageRegister();
 #endif
-#if defined(WITH_NODE_DEVICES) && \
-    (defined(HAVE_HAL) || defined(HAVE_DEVKIT))
+#if defined(WITH_NODE_DEVICES)
     nodedevRegister();
 #endif
     secretRegister();
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 000be09..266f70c 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -17,7 +17,8 @@ src/lxc/lxc_controller.c
 src/lxc/lxc_driver.c
 src/network/bridge_driver.c
 src/node_device/node_device_driver.c
-src/node_device/node_device_hal_linux.c
+src/node_device/node_device_linux_sysfs.c
+src/node_device/node_device_udev.c
 src/nodeinfo.c
 src/opennebula/one_conf.c
 src/opennebula/one_driver.c
diff --git a/src/Makefile.am b/src/Makefile.am
index 92dbae4..afe7eac 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -259,16 +259,20 @@ SECURITY_DRIVER_APPARMOR_SOURCES =				\


 NODE_DEVICE_DRIVER_SOURCES =					\
-		node_device/node_device_driver.c node_device/node_device_driver.h
+		node_device/node_device_driver.c \
+		node_device/node_device_driver.h \
+		node_device/node_device_linux_sysfs.c

 NODE_DEVICE_DRIVER_HAL_SOURCES =				\
 		node_device/node_device_hal.c			\
-		node_device/node_device_hal.h			\
-		node_device/node_device_hal_linux.c
+		node_device/node_device_hal.h

 NODE_DEVICE_DRIVER_DEVKIT_SOURCES =				\
 		node_device/node_device_devkit.c

+NODE_DEVICE_DRIVER_UDEV_SOURCES =				\
+		node_device/node_device_udev.c
+

 #########################
 #
@@ -647,6 +651,11 @@ libvirt_driver_nodedev_la_SOURCES += $(NODE_DEVICE_DRIVER_DEVKIT_SOURCES)
 libvirt_driver_nodedev_la_CFLAGS += $(DEVKIT_CFLAGS)
 libvirt_driver_nodedev_la_LDFLAGS += $(DEVKIT_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)
+endif

 if WITH_DRIVER_MODULES
 libvirt_driver_nodedev_la_LDFLAGS += -module -avoid-version
@@ -696,6 +705,7 @@ EXTRA_DIST +=							\
 		$(NODE_DEVICE_DRIVER_SOURCES)			\
 		$(NODE_DEVICE_DRIVER_HAL_SOURCES)		\
 		$(NODE_DEVICE_DRIVER_DEVKIT_SOURCES)		\
+		$(NODE_DEVICE_DRIVER_UDEV_SOURCES)		\
 		$(SECURITY_DRIVER_SELINUX_SOURCES)		\
 		$(SECURITY_DRIVER_APPARMOR_SOURCES)		\
 		$(SECRET_DRIVER_SOURCES)			\
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index ece339f..f3aca76 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -91,6 +91,26 @@ int virNodeDeviceHasCap(const virNodeDeviceObjPtr dev, const char *cap)
     return 0;
 }

+
+virNodeDeviceObjPtr
+virNodeDeviceFindBySysfsPath(const virNodeDeviceObjListPtr devs,
+                             const char *sysfs_path)
+{
+    unsigned int i;
+
+    for (i = 0; i < devs->count; i++) {
+        virNodeDeviceObjLock(devs->objs[i]);
+        if ((devs->objs[i]->def->sysfs_path != NULL) &&
+            (STREQ(devs->objs[i]->def->sysfs_path, sysfs_path))) {
+            return devs->objs[i];
+        }
+        virNodeDeviceObjUnlock(devs->objs[i]);
+    }
+
+    return NULL;
+}
+
+
 virNodeDeviceObjPtr virNodeDeviceFindByName(const virNodeDeviceObjListPtr devs,
                                             const char *name)
 {
@@ -117,6 +137,8 @@ void virNodeDeviceDefFree(virNodeDeviceDefPtr def)
     VIR_FREE(def->name);
     VIR_FREE(def->parent);
     VIR_FREE(def->driver);
+    VIR_FREE(def->sysfs_path);
+    VIR_FREE(def->parent_sysfs_path);

     caps = def->caps;
     while (caps) {
@@ -228,9 +250,17 @@ char *virNodeDeviceDefFormat(virConnectPtr conn,

     virBufferAddLit(&buf, "<device>\n");
     virBufferEscapeString(&buf, "  <name>%s</name>\n", def->name);
-
-    if (def->parent)
+    if (def->sysfs_path != NULL) {
+        virBufferEscapeString(&buf, "  <sysfs_path>%s</sysfs_path>\n",
+                              def->sysfs_path);
+    }
+    if (def->parent) {
         virBufferEscapeString(&buf, "  <parent>%s</parent>\n", def->parent);
+    }
+    if (def->parent_sysfs_path != NULL) {
+        virBufferEscapeString(&buf, "  <parent_sysfs_path>%s</parent_sysfs_path>\n",
+                              def->parent_sysfs_path);
+    }
     if (def->driver) {
         virBufferAddLit(&buf, "  <driver>\n");
         virBufferEscapeString(&buf, "    <name>%s</name>\n", def->driver);
@@ -248,12 +278,6 @@ char *virNodeDeviceDefFormat(virConnectPtr conn,
             if (data->system.product_name)
                 virBufferEscapeString(&buf, "    <product>%s</product>\n",
                                       data->system.product_name);
-            if (data->system.dmi_devpath)
-                virBufferEscapeString(&buf, "    <dmi_devpath>%s</dmi_devpath>\n",
-                                      data->system.dmi_devpath);
-            if (data->system.description)
-                virBufferEscapeString(&buf, "    <description>%s</description>\n",
-                                      data->system.description);
             virBufferAddLit(&buf, "    <hardware>\n");
             if (data->system.hardware.vendor_name)
                 virBufferEscapeString(&buf, "      <vendor>%s</vendor>\n",
@@ -331,9 +355,6 @@ char *virNodeDeviceDefFormat(virConnectPtr conn,
                               data->usb_if.subclass);
             virBufferVSprintf(&buf, "    <protocol>%d</protocol>\n",
                               data->usb_if.protocol);
-            if (data->usb_if.interface_name)
-                virBufferVSprintf(&buf, "    <interface_name>%s</interface_name>\n",
-                                  data->usb_if.interface_name);
             if (data->usb_if.description)
                 virBufferVSprintf(&buf, "    <description>%s</description>\n",
                                   data->usb_if.description);
@@ -1334,8 +1355,6 @@ void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps)
     switch (caps->type) {
     case VIR_NODE_DEV_CAP_SYSTEM:
         VIR_FREE(data->system.product_name);
-        VIR_FREE(data->system.dmi_devpath);
-        VIR_FREE(data->system.description);
         VIR_FREE(data->system.hardware.vendor_name);
         VIR_FREE(data->system.hardware.version);
         VIR_FREE(data->system.hardware.serial);
@@ -1352,7 +1371,6 @@ void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps)
         VIR_FREE(data->usb_dev.vendor_name);
         break;
     case VIR_NODE_DEV_CAP_USB_INTERFACE:
-        VIR_FREE(data->usb_if.interface_name);
         VIR_FREE(data->usb_if.description);
         break;
     case VIR_NODE_DEV_CAP_NET:
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index f70184d..e97242a 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -82,8 +82,6 @@ struct _virNodeDevCapsDef {
     union _virNodeDevCapData {
         struct {
             char *product_name;
-            char *description;
-            char *dmi_devpath;
             struct {
                 char *vendor_name;
                 char *version;
@@ -120,7 +118,6 @@ struct _virNodeDevCapsDef {
             unsigned _class;		/* "class" is reserved in C */
             unsigned subclass;
             unsigned protocol;
-            char *interface_name;
             char *description;
         } usb_if;
         struct {
@@ -164,7 +161,9 @@ typedef struct _virNodeDeviceDef virNodeDeviceDef;
 typedef virNodeDeviceDef *virNodeDeviceDefPtr;
 struct _virNodeDeviceDef {
     char *name;                         /* device name (unique on node) */
+    char *sysfs_path;                   /* udev name/sysfs path */
     char *parent;			/* optional parent device name */
+    char *parent_sysfs_path;            /* udev parent name/sysfs path */
     char *driver;                       /* optional driver name */
     virNodeDevCapsDefPtr caps;		/* optional device capabilities */
 };
@@ -206,6 +205,9 @@ int virNodeDeviceHasCap(const virNodeDeviceObjPtr dev, const char *cap);

 virNodeDeviceObjPtr virNodeDeviceFindByName(const virNodeDeviceObjListPtr devs,
                                             const char *name);
+virNodeDeviceObjPtr
+virNodeDeviceFindBySysfsPath(const virNodeDeviceObjListPtr devs,
+                             const char *sysfs_path);

 virNodeDeviceObjPtr virNodeDeviceAssignDef(virConnectPtr conn,
                                            virNodeDeviceObjListPtr devs,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0506867..c473d49 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -341,6 +341,7 @@ virNodeDeviceHasCap;
 virNodeDeviceObjRemove;
 virNodeDevCapTypeToString;
 virNodeDeviceFindByName;
+virNodeDeviceFindBySysfsPath;
 virNodeDeviceObjListFree;
 virNodeDeviceDefFree;
 virNodeDevCapsDefFree;
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index f33ff48..c139907 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -70,7 +70,10 @@ static int update_caps(virNodeDeviceObjPtr dev)
 }


-#ifdef __linux__
+#if defined (__linux__) && defined (HAVE_HAL)
+/* Under libudev changes to the driver name should be picked up as
+ * "change" events, so we don't call update driver name unless we're
+ * using the HAL backend. */
 static int update_driver_name(virConnectPtr conn,
                               virNodeDeviceObjPtr dev)
 {
@@ -658,10 +661,10 @@ void registerCommonNodeFuncs(virDeviceMonitorPtr driver)


 int nodedevRegister(void) {
-#if defined(HAVE_HAL) && defined(HAVE_DEVKIT)
+#if defined(HAVE_HAL) && defined(HAVE_UDEV)
     /* Register only one of these two - they conflict */
     if (halNodeRegister() == -1)
-        return devkitNodeRegister();
+        return udevNodeRegister();
     return 0;
 #else
 #ifdef HAVE_HAL
@@ -670,5 +673,8 @@ int nodedevRegister(void) {
 #ifdef HAVE_DEVKIT
     return devkitNodeRegister();
 #endif
+#ifdef HAVE_UDEV
+    return udevNodeRegister();
+#endif
 #endif
 }
diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h
index db01624..5be0781 100644
--- a/src/node_device/node_device_driver.h
+++ b/src/node_device/node_device_driver.h
@@ -45,6 +45,9 @@ int halNodeRegister(void);
 #ifdef HAVE_DEVKIT
 int devkitNodeRegister(void);
 #endif
+#ifdef HAVE_UDEV
+int udevNodeRegister(void);
+#endif

 void nodeDeviceLock(virDeviceMonitorStatePtr driver);
 void nodeDeviceUnlock(virDeviceMonitorStatePtr driver);
@@ -53,4 +56,23 @@ void registerCommonNodeFuncs(virDeviceMonitorPtr mon);

 int nodedevRegister(void);

+#ifdef __linux__
+
+#define check_fc_host(d) check_fc_host_linux(d)
+int check_fc_host_linux(union _virNodeDevCapData *d);
+
+#define check_vport_capable(d) check_vport_capable_linux(d)
+int check_vport_capable_linux(union _virNodeDevCapData *d);
+
+#define read_wwn(host, file, wwn) read_wwn_linux(host, file, wwn)
+int read_wwn_linux(int host, const char *file, char **wwn);
+
+#else  /* __linux__ */
+
+#define check_fc_host(d)
+#define check_vport_capable(d)
+#define read_wwn(host, file, wwn)
+
+#endif /* __linux__ */
+
 #endif /* __VIR_NODE_DEVICE_H__ */
diff --git a/src/node_device/node_device_hal.h b/src/node_device/node_device_hal.h
index c859fe3..8ac8a35 100644
--- a/src/node_device/node_device_hal.h
+++ b/src/node_device/node_device_hal.h
@@ -22,23 +22,4 @@
 #ifndef __VIR_NODE_DEVICE_HAL_H__
 #define __VIR_NODE_DEVICE_HAL_H__

-#ifdef __linux__
-
-#define check_fc_host(d) check_fc_host_linux(d)
-int check_fc_host_linux(union _virNodeDevCapData *d);
-
-#define check_vport_capable(d) check_vport_capable_linux(d)
-int check_vport_capable_linux(union _virNodeDevCapData *d);
-
-#define read_wwn(host, file, wwn) read_wwn_linux(host, file, wwn)
-int read_wwn_linux(int host, const char *file, char **wwn);
-
-#else  /* __linux__ */
-
-#define check_fc_host(d)
-#define check_vport_capable(d)
-#define read_wwn(host, file, wwn)
-
-#endif /* __linux__ */
-
 #endif /* __VIR_NODE_DEVICE_HAL_H__ */
diff --git a/src/node_device/node_device_hal_linux.c b/src/node_device/node_device_linux_sysfs.c
similarity index 100%
rename from src/node_device/node_device_hal_linux.c
rename to src/node_device/node_device_linux_sysfs.c
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
new file mode 100644
index 0000000..d608b76
--- /dev/null
+++ b/src/node_device/node_device_udev.c
@@ -0,0 +1,1519 @@
+/*
+ * node_device_udev.c: node device enumeration - libudev implementation
+ *
+ * Copyright (C) 2009 Red Hat
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
+ *
+ * Author: Dave Allan <dallan@redhat.com>
+ */
+
+#include <config.h>
+#include <libudev.h>
+#include <scsi/scsi.h>
+#include <c-ctype.h>
+
+#include "node_device_udev.h"
+#include "virterror_internal.h"
+#include "node_device_conf.h"
+#include "node_device_driver.h"
+#include "driver.h"
+#include "datatypes.h"
+#include "logging.h"
+#include "memory.h"
+#include "uuid.h"
+#include "util.h"
+#include "buf.h"
+#include "daemon/event.h"
+
+#define VIR_FROM_THIS VIR_FROM_NODEDEV
+
+static virDeviceMonitorStatePtr driverState = NULL;
+
+static int udevStrToLong_ull(char const *s,
+                             char **end_ptr,
+                             int base,
+                             unsigned long long *result)
+{
+    int ret = 0;
+
+    ret = virStrToLong_ull(s, end_ptr, base, result);
+    if (ret != 0) {
+        VIR_ERROR("Failed to convert '%s' to unsigned long long\n", s);
+    } else {
+        VIR_DEBUG("Converted '%s' to unsigned long %llu\n", s, *result);
+    }
+
+    return ret;
+}
+
+
+static int udevStrToLong_ui(char const *s,
+                            char **end_ptr,
+                            int base,
+                            unsigned int *result)
+{
+    int ret = 0;
+
+    ret = virStrToLong_ui(s, end_ptr, base, result);
+    if (ret != 0) {
+        VIR_ERROR("Failed to convert '%s' to unsigned int\n", s);
+    } else {
+        VIR_DEBUG("Converted '%s' to unsigned int %u\n", s, *result);
+    }
+
+    return ret;
+}
+
+static int udevStrToLong_i(char const *s,
+                           char **end_ptr,
+                           int base,
+                           int *result)
+{
+    int ret = 0;
+
+    ret = virStrToLong_i(s, end_ptr, base, result);
+    if (ret != 0) {
+        VIR_ERROR("Failed to convert '%s' to int\n", s);
+    } else {
+        VIR_DEBUG("Converted '%s' to int %u\n", s, *result);
+    }
+
+    return ret;
+}
+
+
+/* This function allocates memory from the heap for the property
+ * value.  That memory must be later freed by some other code. */
+static int udevGetDeviceProperty(struct udev_device *udev_device,
+                                 const char *property_key,
+                                 char **property_value)
+{
+    const char *udev_value = NULL;
+    int ret = PROPERTY_FOUND;
+
+    udev_value = udev_device_get_property_value(udev_device, property_key);
+    if (udev_value == NULL) {
+        VIR_INFO(_("udev reports device '%s' does not have property '%s'"),
+                 udev_device_get_sysname(udev_device), property_key);
+        ret = PROPERTY_MISSING;
+        goto out;
+    }
+
+    /* If this allocation is changed, the comment at the beginning
+     * of the function must also be changed. */
+    *property_value = strdup(udev_value);
+    if (*property_value == NULL) {
+        VIR_ERROR("Failed to allocate memory for property value for "
+                  "property key '%s' on device with sysname '%s'",
+                  property_key, udev_device_get_sysname(udev_device));
+        virReportOOMError(NULL);
+        ret = PROPERTY_ERROR;
+        goto out;
+    }
+
+    VIR_DEBUG("Found property key '%s' value '%s' "
+              "for device with sysname '%s'\n",
+              property_key, *property_value,
+              udev_device_get_sysname(udev_device));
+
+out:
+    return ret;
+}
+
+
+static int udevGetStringProperty(struct udev_device *udev_device,
+                                 const char *property_key,
+                                 char **value)
+{
+    return udevGetDeviceProperty(udev_device, property_key, value);
+}
+
+
+static int udevGetIntProperty(struct udev_device *udev_device,
+                              const char *property_key,
+                              int *value,
+                              int base)
+{
+    char *udev_value = NULL;
+    int ret = PROPERTY_FOUND;
+
+    ret = udevGetDeviceProperty(udev_device, property_key, &udev_value);
+
+    if (ret == PROPERTY_FOUND) {
+        if (udevStrToLong_i(udev_value, NULL, base, value) != 0) {
+            ret = PROPERTY_ERROR;
+        }
+    }
+
+    VIR_FREE(udev_value);
+    return ret;
+}
+
+
+static int udevGetUintProperty(struct udev_device *udev_device,
+                               const char *property_key,
+                               unsigned int *value,
+                               int base)
+{
+    char *udev_value = NULL;
+    int ret = PROPERTY_FOUND;
+
+    ret = udevGetDeviceProperty(udev_device, property_key, &udev_value);
+
+    if (ret == PROPERTY_FOUND) {
+        if (udevStrToLong_ui(udev_value, NULL, base, value) != 0) {
+            ret = PROPERTY_ERROR;
+        }
+    }
+
+    VIR_FREE(udev_value);
+    return ret;
+}
+
+
+/* This function allocates memory from the heap for the property
+ * value.  That memory must be later freed by some other code. */
+static int udevGetDeviceSysfsAttr(struct udev_device *udev_device,
+                                  const char *attr_name,
+                                  char **attr_value)
+{
+    const char *udev_value = NULL;
+    int ret = PROPERTY_FOUND;
+
+    udev_value = udev_device_get_sysattr_value(udev_device, attr_name);
+    if (udev_value == NULL) {
+        VIR_INFO(_("udev reports device '%s' does not have sysfs attr '%s'"),
+                 udev_device_get_sysname(udev_device), attr_name);
+        ret = PROPERTY_MISSING;
+        goto out;
+    }
+
+    /* If this allocation is changed, the comment at the beginning
+     * of the function must also be changed. */
+    *attr_value = strdup(udev_value);
+    if (*attr_value == NULL) {
+        VIR_ERROR("Failed to allocate memory for sysfs attribute value for "
+                  "sysfs attribute '%s' on device with sysname '%s'",
+                  attr_name, udev_device_get_sysname(udev_device));
+        virReportOOMError(NULL);
+        ret = PROPERTY_ERROR;
+        goto out;
+    }
+
+    VIR_DEBUG("Found sysfs attribute '%s' value '%s' "
+              "for device with sysname '%s'\n",
+              attr_name, *attr_value,
+              udev_device_get_sysname(udev_device));
+
+out:
+    return ret;
+}
+
+
+static int udevGetStringSysfsAttr(struct udev_device *udev_device,
+                                  const char *attr_name,
+                                  char **value)
+{
+    char *tmp = NULL;
+    int ret = PROPERTY_MISSING;
+
+    ret = udevGetDeviceSysfsAttr(udev_device, attr_name, &tmp);
+
+    if (tmp != NULL && (STREQ(tmp, ""))) {
+        VIR_FREE(tmp);
+        tmp = NULL;
+        ret = PROPERTY_MISSING;
+    }
+
+    *value = tmp;
+
+    return ret;
+}
+
+
+static int udevGetIntSysfsAttr(struct udev_device *udev_device,
+                               const char *attr_name,
+                               int *value,
+                               int base)
+{
+    char *udev_value = NULL;
+    int ret = PROPERTY_FOUND;
+
+    ret = udevGetDeviceSysfsAttr(udev_device, attr_name, &udev_value);
+
+    if (ret == PROPERTY_FOUND) {
+        if (udevStrToLong_i(udev_value, NULL, base, value) != 0) {
+            ret = PROPERTY_ERROR;
+        }
+    }
+
+    VIR_FREE(udev_value);
+    return ret;
+}
+
+
+static int udevGetUintSysfsAttr(struct udev_device *udev_device,
+                                const char *attr_name,
+                                unsigned int *value,
+                                int base)
+{
+    char *udev_value = NULL;
+    int ret = PROPERTY_FOUND;
+
+    ret = udevGetDeviceSysfsAttr(udev_device, attr_name, &udev_value);
+
+    if (ret == PROPERTY_FOUND) {
+        if (udevStrToLong_ui(udev_value, NULL, base, value) != 0) {
+            ret = PROPERTY_ERROR;
+        }
+    }
+
+    VIR_FREE(udev_value);
+    return ret;
+}
+
+
+static int udevGetUint64SysfsAttr(struct udev_device *udev_device,
+                                  const char *attr_name,
+                                  unsigned long long *value)
+{
+    char *udev_value = NULL;
+    int ret = PROPERTY_FOUND;
+
+    ret = udevGetDeviceSysfsAttr(udev_device, attr_name, &udev_value);
+
+    if (ret == PROPERTY_FOUND) {
+        if (udevStrToLong_ull(udev_value, NULL, 0, value) != 0) {
+            ret = PROPERTY_ERROR;
+        }
+    }
+
+    VIR_FREE(udev_value);
+    return ret;
+}
+
+
+static int udevGenerateDeviceName(struct udev_device *device,
+                                  virNodeDeviceDefPtr def,
+                                  const char *s)
+{
+    int ret = 0, i = 0;
+    virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+    virBufferVSprintf(&buf, "%s_%s",
+                      udev_device_get_subsystem(device),
+                      udev_device_get_sysname(device));
+
+    if (s != NULL) {
+        virBufferVSprintf(&buf, "_%s", s);
+    }
+
+    if (virBufferError(&buf)) {
+        VIR_ERROR("Buffer error when generating device name for device "
+                  "with sysname '%s'\n", udev_device_get_sysname(device));
+        ret = -1;
+    }
+
+    def->name = virBufferContentAndReset(&buf);
+
+    for (i = 0; i < strlen(def->name) ; i++) {
+        if (!(c_isalnum(*(def->name + i)))) {
+            *(def->name + i) = '_';
+        }
+    }
+
+    return ret;
+}
+
+
+static void udevLogFunction(struct udev *udev ATTRIBUTE_UNUSED,
+                            int priority ATTRIBUTE_UNUSED,
+                            const char *file,
+                            int line,
+                            const char *fn,
+                            const char *fmt,
+                            va_list args)
+{
+    VIR_ERROR_INT(file, fn, line, fmt, args);
+}
+
+
+static int udevProcessPCI(struct udev_device *device,
+                          virNodeDeviceDefPtr def)
+{
+    const char *devpath = NULL;
+    union _virNodeDevCapData *data = &def->caps->data;
+    int ret = -1;
+
+    devpath = udev_device_get_devpath(device);
+
+    if (udevGetUintProperty(device,
+                            "PCI_CLASS",
+                            &data->pci_dev.class,
+                            16) == PROPERTY_ERROR) {
+        goto out;
+    }
+
+    char *p = strrchr(devpath, '/');
+
+    if ((p == NULL) || (udevStrToLong_ui(p+1,
+                                         &p,
+                                         16,
+                                         &data->pci_dev.domain) == -1)) {
+        goto out;
+    }
+
+    if ((p == NULL) || (udevStrToLong_ui(p+1,
+                                         &p,
+                                         16,
+                                         &data->pci_dev.bus) == -1)) {
+        goto out;
+    }
+
+    if ((p == NULL) || (udevStrToLong_ui(p+1,
+                                         &p,
+                                         16,
+                                         &data->pci_dev.slot) == -1)) {
+        goto out;
+    }
+
+    if ((p == NULL) || (udevStrToLong_ui(p+1,
+                                         &p,
+                                         16,
+                                         &data->pci_dev.function) == -1)) {
+        goto out;
+    }
+
+    if (udevGetUintSysfsAttr(device,
+                             "vendor",
+                             &data->pci_dev.vendor,
+                             0) == PROPERTY_ERROR) {
+        goto out;
+    }
+
+    if (udevGetUintSysfsAttr(device,
+                             "device",
+                             &data->pci_dev.product,
+                             0) == PROPERTY_ERROR) {
+        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 (udevGenerateDeviceName(device, def, NULL) != 0) {
+        goto out;
+    }
+
+    ret = 0;
+
+out:
+    return ret;
+}
+
+
+static int udevProcessUSBDevice(struct udev_device *device,
+                                virNodeDeviceDefPtr def)
+{
+    union _virNodeDevCapData *data = &def->caps->data;
+    int ret = -1;
+
+    if (udevGetUintProperty(device,
+                            "BUSNUM",
+                            &data->usb_dev.bus,
+                            0) == PROPERTY_ERROR) {
+        goto out;
+    }
+
+    if (udevGetUintProperty(device,
+                            "DEVNUM",
+                            &data->usb_dev.device,
+                            0) == PROPERTY_ERROR) {
+        goto out;
+    }
+
+    if (udevGetUintProperty(device,
+                            "ID_VENDOR_ID",
+                            &data->usb_dev.vendor,
+                            16) == PROPERTY_ERROR) {
+        goto out;
+    }
+
+    if (udevGetStringSysfsAttr(device,
+                              "manufacturer",
+                              &data->usb_dev.vendor_name) == PROPERTY_ERROR) {
+        goto out;
+    }
+
+    if (udevGetUintProperty(device,
+                            "ID_MODEL_ID",
+                            &data->usb_dev.product,
+                            0) == PROPERTY_ERROR) {
+        goto out;
+    }
+
+    if (udevGetStringSysfsAttr(device,
+                              "product",
+                              &data->usb_dev.product_name) == PROPERTY_ERROR) {
+        goto out;
+    }
+
+    if (udevGenerateDeviceName(device, def, NULL) != 0) {
+        goto out;
+    }
+
+    ret = 0;
+
+out:
+    return ret;
+}
+
+
+/* XXX Is 10 the correct base for the Number/Class/SubClass/Protocol
+ * conversions?  */
+static int udevProcessUSBInterface(struct udev_device *device,
+                                   virNodeDeviceDefPtr def)
+{
+    int ret = -1;
+    union _virNodeDevCapData *data = &def->caps->data;
+
+    if (udevGetUintSysfsAttr(device,
+                             "bInterfaceNumber",
+                             &data->usb_if.number,
+                             10) == PROPERTY_ERROR) {
+        goto out;
+    }
+
+    if (udevGetUintSysfsAttr(device,
+                             "bInterfaceClass",
+                             &data->usb_if._class,
+                             10) == PROPERTY_ERROR) {
+        goto out;
+    }
+
+    if (udevGetUintSysfsAttr(device,
+                             "bInterfaceSubClass",
+                             &data->usb_if.subclass,
+                             10) == PROPERTY_ERROR) {
+        goto out;
+    }
+
+    if (udevGetUintSysfsAttr(device,
+                             "bInterfaceProtocol",
+                             &data->usb_if.protocol,
+                             10) == PROPERTY_ERROR) {
+        goto out;
+    }
+
+    if (udevGenerateDeviceName(device, def, NULL) != 0) {
+        goto out;
+    }
+
+    ret = 0;
+
+out:
+    return ret;
+}
+
+
+static int udevProcessNetworkInterface(struct udev_device *device,
+                                       virNodeDeviceDefPtr def)
+{
+    int ret = -1;
+    union _virNodeDevCapData *data = &def->caps->data;
+
+    if (udevGetStringProperty(device,
+                              "INTERFACE",
+                              &data->net.ifname) == PROPERTY_ERROR) {
+        goto out;
+    }
+
+    if (udevGetStringSysfsAttr(device,
+                               "address",
+                               &data->net.address) == PROPERTY_ERROR) {
+        goto out;
+    }
+
+    if (udevGetUintSysfsAttr(device,
+                             "addr_len",
+                             &data->net.address_len,
+                             0) == PROPERTY_ERROR) {
+        goto out;
+    }
+
+    if (udevGenerateDeviceName(device, def, data->net.address) != 0) {
+        goto out;
+    }
+
+    ret = 0;
+
+out:
+    return ret;
+}
+
+
+static int udevProcessSCSIHost(struct udev_device *device ATTRIBUTE_UNUSED,
+                               virNodeDeviceDefPtr def)
+{
+    int ret = -1;
+    union _virNodeDevCapData *data = &def->caps->data;
+    char *filename = NULL;
+
+    filename = basename(def->sysfs_path);
+
+    if (!STRPREFIX(filename, "host")) {
+        VIR_ERROR("SCSI host found, but its udev name '%s' does "
+                  "not begin with 'host'\n", filename);
+        goto out;
+    }
+
+    if (udevStrToLong_ui(filename + strlen("host"),
+                         NULL,
+                         0,
+                         &data->scsi_host.host) == -1) {
+        goto out;
+    }
+
+    check_fc_host(&def->caps->data);
+    check_vport_capable(&def->caps->data);
+
+    if (udevGenerateDeviceName(device, def, NULL) != 0) {
+        goto out;
+    }
+
+    ret = 0;
+
+out:
+    return ret;
+}
+
+
+static int udevGetSCSIType(unsigned int type, char **typestring)
+{
+    int ret = 0;
+    int foundtype = 1;
+
+    *typestring = NULL;
+
+    switch (type) {
+    case TYPE_DISK:
+        *typestring = strdup("disk");
+        break;
+    case TYPE_TAPE:
+        *typestring = strdup("tape");
+        break;
+    case TYPE_PROCESSOR:
+        *typestring = strdup("processor");
+        break;
+    case TYPE_WORM:
+        *typestring = strdup("worm");
+        break;
+    case TYPE_ROM:
+        *typestring = strdup("cdrom");
+        break;
+    case TYPE_SCANNER:
+        *typestring = strdup("scanner");
+        break;
+    case TYPE_MOD:
+        *typestring = strdup("mod");
+        break;
+    case TYPE_MEDIUM_CHANGER:
+        *typestring = strdup("changer");
+        break;
+    case TYPE_ENCLOSURE:
+        *typestring = strdup("enclosure");
+        break;
+    case TYPE_NO_LUN:
+    default:
+        foundtype = 0;
+        break;
+    }
+
+    if (*typestring == NULL) {
+        if (foundtype == 1) {
+            ret = -1;
+            virReportOOMError(NULL);
+        } else {
+            VIR_ERROR("Failed to find SCSI device type %d\n", type);
+        }
+    }
+
+    return ret;
+}
+
+
+static int udevProcessSCSIDevice(struct udev_device *device ATTRIBUTE_UNUSED,
+                                 virNodeDeviceDefPtr def)
+{
+    int ret = -1;
+    unsigned int tmp = 0;
+    union _virNodeDevCapData *data = &def->caps->data;
+    char *filename = NULL, *p = NULL;
+
+    filename = basename(def->sysfs_path);
+
+    if (udevStrToLong_ui(filename, &p, 10, &data->scsi.host) == -1) {
+        goto out;
+    }
+
+    if ((p == NULL) || (udevStrToLong_ui(p+1,
+                                         &p,
+                                         10,
+                                         &data->scsi.bus) == -1)) {
+        goto out;
+    }
+
+    if ((p == NULL) || (udevStrToLong_ui(p+1,
+                                         &p,
+                                         10,
+                                         &data->scsi.target) == -1)) {
+        goto out;
+    }
+
+    if ((p == NULL) || (udevStrToLong_ui(p+1,
+                                         &p,
+                                         10,
+                                         &data->scsi.lun) == -1)) {
+        goto out;
+    }
+
+    switch (udevGetUintSysfsAttr(device, "type", &tmp, 0)) {
+    case PROPERTY_FOUND:
+        if (udevGetSCSIType(tmp, &data->scsi.type) == -1) {
+            goto out;
+        }
+        break;
+    case PROPERTY_MISSING:
+        break; /* No type is not an error */
+    case PROPERTY_ERROR:
+    default:
+        goto out;
+        break;
+    }
+
+    if (udevGenerateDeviceName(device, def, NULL) != 0) {
+        goto out;
+    }
+
+    ret = 0;
+
+out:
+    if (ret != 0) {
+        VIR_ERROR("Failed to process SCSI device with sysfs path '%s'\n",
+                  def->sysfs_path);
+    }
+    return ret;
+}
+
+
+static int udevProcessDisk(struct udev_device *device,
+                           virNodeDeviceDefPtr def)
+{
+    union _virNodeDevCapData *data = &def->caps->data;
+    int ret = 0;
+
+    data->storage.drive_type = strdup("disk");
+    if (data->storage.drive_type == NULL) {
+        virReportOOMError(NULL);
+        ret = -1;
+        goto out;
+    }
+
+    if (udevGetUint64SysfsAttr(device,
+                               "size",
+                               &data->storage.num_blocks) == PROPERTY_ERROR) {
+        goto out;
+    }
+
+    if (udevGetUint64SysfsAttr(device,
+                               "queue/logical_block_size",
+                               &data->storage.logical_block_size)
+        == PROPERTY_ERROR) {
+        goto out;
+    }
+
+    data->storage.size = data->storage.num_blocks *
+        data->storage.logical_block_size;
+
+out:
+    return ret;
+}
+
+
+static int udevProcessCDROM(struct udev_device *device,
+                            virNodeDeviceDefPtr def)
+{
+    union _virNodeDevCapData *data = &def->caps->data;
+    int tmp_int = 0, ret = 0;
+
+    /* NB: the drive_type string provided by udev is different from
+     * that provided by HAL; now it's "cd" instead of "cdrom" We
+     * change it to cdrom to preserve compatibility with earlier
+     * versions of libvirt.  */
+    VIR_FREE(def->caps->data.storage.drive_type);
+    def->caps->data.storage.drive_type = strdup("cdrom");
+    if (def->caps->data.storage.drive_type == NULL) {
+        virReportOOMError(NULL);
+        goto out;
+    }
+
+    if ((udevGetIntSysfsAttr(device, "removable", &tmp_int, 0) == PROPERTY_FOUND) &&
+        (tmp_int == 1)) {
+        def->caps->data.storage.flags |= VIR_NODE_DEV_CAP_STORAGE_REMOVABLE;
+    }
+
+    if ((udevGetIntProperty(device, "ID_CDROM_MEDIA", &tmp_int, 0)
+         == PROPERTY_FOUND) && (tmp_int == 1)) {
+
+        def->caps->data.storage.flags |=
+            VIR_NODE_DEV_CAP_STORAGE_REMOVABLE_MEDIA_AVAILABLE;
+
+        if (udevGetUint64SysfsAttr(device,
+                                   "size",
+                                   &data->storage.num_blocks) == PROPERTY_ERROR) {
+            goto out;
+        }
+
+        if (udevGetUint64SysfsAttr(device,
+                                   "queue/logical_block_size",
+                                   &data->storage.logical_block_size) == PROPERTY_ERROR) {
+            goto out;
+        }
+
+        /* XXX This calculation is wrong for the qemu virtual cdrom
+         * which reports the size in 512 byte blocks, but the logical
+         * block size as 2048.  I don't have a physical cdrom on a
+         * devel system to see how they behave. */
+        def->caps->data.storage.removable_media_size =
+            def->caps->data.storage.num_blocks *
+            def->caps->data.storage.logical_block_size;
+    }
+
+out:
+    return ret;
+}
+
+
+/* This function exists to deal with the case in which a driver does
+ * not provide a device type in the usual place, but udev told us it's
+ * a storage device, and we can make a good guess at what kind of
+ * storage device it is from other information that is provided. */
+static int udevKludgeStorageType(virNodeDeviceDefPtr def)
+{
+    int ret = -1;
+
+    VIR_INFO("Could not find definitive storage type for device "
+             "with sysfs path '%s', trying to guess it\n",
+             def->sysfs_path);
+
+    if (STRPREFIX(def->caps->data.storage.block, "/dev/vd")) {
+        /* virtio disk */
+        def->caps->data.storage.drive_type = strdup("disk");
+        if (def->caps->data.storage.drive_type != NULL) {
+            ret = 0;
+        }
+    }
+
+    if (ret != 0) {
+        VIR_INFO("Could not determine storage type for device "
+                 "with sysfs path '%s'\n", def->sysfs_path);
+    } else {
+        VIR_DEBUG("Found storage type '%s' for device "
+                  "with sysfs path '%s'\n",
+                  def->caps->data.storage.drive_type,
+                  def->sysfs_path);
+    }
+
+    return ret;
+}
+
+
+static void udevStripSpaces(char *s)
+{
+    if (s == NULL) {
+        return;
+    }
+
+    while (virFileStripSuffix(s, " ")) {
+        /* do nothing */
+        ;
+    }
+
+    return;
+}
+
+
+static int udevProcessStorage(struct udev_device *device,
+                              virNodeDeviceDefPtr def)
+{
+    union _virNodeDevCapData *data = &def->caps->data;
+    int ret = -1;
+
+    data->storage.block = strdup(udev_device_get_devnode(device));
+    if (udevGetStringProperty(device,
+                              "DEVNAME",
+                              &data->storage.block) == PROPERTY_ERROR) {
+        goto out;
+    }
+    if (udevGetStringProperty(device,
+                              "ID_BUS",
+                              &data->storage.bus) == PROPERTY_ERROR) {
+        goto out;
+    }
+    if (udevGetStringProperty(device,
+                              "ID_SERIAL",
+                              &data->storage.serial) == PROPERTY_ERROR) {
+        goto out;
+    }
+    if (udevGetStringSysfsAttr(device,
+                               "device/vendor",
+                               &data->storage.vendor) == PROPERTY_ERROR) {
+        goto out;
+    }
+    udevStripSpaces(def->caps->data.storage.vendor);
+    if (udevGetStringSysfsAttr(device,
+                               "device/model",
+                               &data->storage.model) == PROPERTY_ERROR) {
+        goto out;
+    }
+    udevStripSpaces(def->caps->data.storage.model);
+    /* There is no equivalent of the hotpluggable property in libudev,
+     * but storage is going toward a world in which hotpluggable is
+     * expected, so I don't see a problem with not having a property
+     * for it. */
+
+    if (udevGetStringProperty(device,
+                              "ID_TYPE",
+                              &data->storage.drive_type) != PROPERTY_FOUND) {
+        /* If udev doesn't have it, perhaps we can guess it. */
+        if (udevKludgeStorageType(def) != 0) {
+            goto out;
+        }
+    }
+
+    if (STREQ(def->caps->data.storage.drive_type, "cd")) {
+        ret = udevProcessCDROM(device, def);
+    } else if (STREQ(def->caps->data.storage.drive_type, "disk")) {
+        ret = udevProcessDisk(device, def);
+    } else {
+        VIR_INFO("Unsupported storage type '%s'\n",
+                 def->caps->data.storage.drive_type);
+        goto out;
+    }
+
+    if (udevGenerateDeviceName(device, def, data->storage.serial) != 0) {
+        goto out;
+    }
+
+out:
+    return ret;
+}
+
+
+static int udevGetDeviceType(struct udev_device *device,
+                             enum virNodeDevCapType *type)
+{
+    const char *devtype = NULL;
+    char *tmp_string = NULL;
+    unsigned int tmp = 0;
+    int ret = 0;
+
+    devtype = udev_device_get_devtype(device);
+
+    if (devtype != NULL && STREQ(devtype, "usb_device")) {
+        *type = VIR_NODE_DEV_CAP_USB_DEV;
+        goto out;
+    }
+
+    if (devtype != NULL && STREQ(devtype, "usb_interface")) {
+        *type = VIR_NODE_DEV_CAP_USB_INTERFACE;
+        goto out;
+    }
+
+    if (devtype != NULL && STREQ(devtype, "scsi_host")) {
+        *type = VIR_NODE_DEV_CAP_SCSI_HOST;
+        goto out;
+    }
+
+    if (devtype != NULL && STREQ(devtype, "scsi_device")) {
+        *type = VIR_NODE_DEV_CAP_SCSI;
+        goto out;
+    }
+
+    if (devtype != NULL && STREQ(devtype, "disk")) {
+        *type = VIR_NODE_DEV_CAP_STORAGE;
+        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 &&
+        udevGetStringProperty(device,
+                              "INTERFACE",
+                              &tmp_string) == PROPERTY_FOUND) {
+        VIR_FREE(tmp_string);
+        *type = VIR_NODE_DEV_CAP_NET;
+        goto out;
+    }
+
+    VIR_INFO("Could not determine device type for device "
+             "with sysfs path '%s'\n",
+             udev_device_get_sysname(device));
+    ret = -1;
+
+out:
+    return ret;
+}
+
+
+static int udevGetDeviceDetails(struct udev_device *device,
+                                virNodeDeviceDefPtr def)
+{
+    int ret = 0;
+
+    switch (def->caps->type) {
+    case VIR_NODE_DEV_CAP_SYSTEM:
+        /* There's no libudev equivalent of system, so ignore it. */
+        break;
+    case VIR_NODE_DEV_CAP_PCI_DEV:
+        ret = udevProcessPCI(device, def);
+        break;
+    case VIR_NODE_DEV_CAP_USB_DEV:
+        ret = udevProcessUSBDevice(device, def);
+        break;
+    case VIR_NODE_DEV_CAP_USB_INTERFACE:
+        ret = udevProcessUSBInterface(device, def);
+        break;
+    case VIR_NODE_DEV_CAP_NET:
+        ret = udevProcessNetworkInterface(device, def);
+        break;
+    case VIR_NODE_DEV_CAP_SCSI_HOST:
+        ret = udevProcessSCSIHost(device, def);
+        break;
+    case VIR_NODE_DEV_CAP_SCSI:
+        ret = udevProcessSCSIDevice(device, def);
+        break;
+    case VIR_NODE_DEV_CAP_STORAGE:
+        ret = udevProcessStorage(device, def);
+        break;
+    default:
+        VIR_ERROR("Unknown device type %d\n", def->caps->type);
+        ret = -1;
+        break;
+    }
+
+    return ret;
+}
+
+
+static int udevRemoveOneDevice(struct udev_device *device)
+{
+    virNodeDeviceObjPtr dev = NULL;
+    const char *name = NULL;
+    int ret = 0;
+
+    name = udev_device_get_syspath(device);
+    dev = virNodeDeviceFindBySysfsPath(&driverState->devs, name);
+    if (dev != NULL) {
+        VIR_DEBUG("Removing device '%s' with sysfs path '%s'\n",
+                  dev->def->name, name);
+        virNodeDeviceObjRemove(&driverState->devs, dev);
+    } else {
+        VIR_INFO("Failed to find device to remove that has udev name '%s'\n",
+                 name);
+        ret = -1;
+    }
+
+    return ret;
+}
+
+
+static int udevSetParent(struct udev_device *device,
+                         virNodeDeviceDefPtr def)
+{
+    struct udev_device *parent_device = NULL;
+    const char *parent_sysfs_path = NULL;
+    virNodeDeviceObjPtr dev = NULL;
+    int ret = -1;
+
+    parent_device = udev_device_get_parent(device);
+    if (parent_device == NULL) {
+        VIR_INFO("Could not find udev parent for device with sysfs path '%s'\n",
+                 udev_device_get_syspath(device));
+        goto out;
+    }
+
+    parent_sysfs_path = udev_device_get_syspath(parent_device);
+    if (parent_sysfs_path == NULL) {
+        VIR_INFO("Could not get syspath for parent of '%s'\n",
+                 udev_device_get_syspath(device));
+        goto out;
+    }
+
+    def->parent_sysfs_path = strdup(parent_sysfs_path);
+    if (def->parent_sysfs_path == NULL) {
+        virReportOOMError(NULL);
+        goto out;
+    }
+
+    dev = virNodeDeviceFindBySysfsPath(&driverState->devs, parent_sysfs_path);
+    if (dev == NULL) {
+        def->parent = strdup("computer");
+    } else {
+        def->parent = strdup(dev->def->name);
+        virNodeDeviceObjUnlock(dev);
+    }
+
+    if (def->parent == NULL) {
+        virReportOOMError(NULL);
+        goto out;
+    }
+
+    ret = 0;
+
+out:
+    return ret;
+}
+
+
+static int udevAddOneDevice(struct udev_device *device)
+{
+    virNodeDeviceDefPtr def = NULL;
+    virNodeDeviceObjPtr dev = NULL;
+    int ret = -1;
+
+    if (VIR_ALLOC(def) != 0) {
+        virReportOOMError(NULL);
+        goto out;
+    }
+
+    def->sysfs_path = strdup(udev_device_get_syspath(device));
+    if (udevGetStringProperty(device,
+                              "DRIVER",
+                              &def->driver) == PROPERTY_ERROR) {
+        goto out;
+    }
+
+    if (VIR_ALLOC(def->caps) != 0) {
+        virReportOOMError(NULL);
+        goto out;
+    }
+
+    if (udevGetDeviceType(device, &def->caps->type) != 0) {
+        goto out;
+    }
+
+    if (udevGetDeviceDetails(device, def) != 0) {
+        goto out;
+    }
+
+    if (udevSetParent(device, def) != 0) {
+        goto out;
+    }
+
+    dev = virNodeDeviceAssignDef(NULL, &driverState->devs, def);
+    if (dev == NULL) {
+        VIR_ERROR("Failed to create device for '%s'\n", def->name);
+        virNodeDeviceDefFree(def);
+        goto out;
+    }
+
+    dev->devicePath = strdup(udev_device_get_devpath(device));
+    if (dev->devicePath == NULL) {
+        virReportOOMError(NULL);
+        virNodeDeviceObjRemove(&driverState->devs, dev);
+        goto out;
+    }
+
+    virNodeDeviceObjUnlock(dev);
+
+    ret = 0;
+
+out:
+    return ret;
+}
+
+
+static int udevProcessDeviceListEntry(struct udev *udev,
+                                      struct udev_list_entry *list_entry)
+{
+    struct udev_device *device;
+    const char *name = NULL;
+    int ret = -1;
+
+    name = udev_list_entry_get_name(list_entry);
+
+    device = udev_device_new_from_syspath(udev, name);
+    if (device != NULL) {
+        if (udevAddOneDevice(device) != 0) {
+            VIR_INFO("Failed to create node device for udev device '%s'\n",
+                     name);
+        }
+        udev_device_unref(device);
+        ret = 0;
+    }
+
+    return ret;
+}
+
+
+static int udevEnumerateDevices(struct udev *udev)
+{
+    struct udev_enumerate *udev_enumerate = NULL;
+    struct udev_list_entry *list_entry = NULL;
+    const char *name = NULL;
+    int ret = 0;
+
+    udev_enumerate = udev_enumerate_new(udev);
+
+    ret = udev_enumerate_scan_devices(udev_enumerate);
+    if (0 != ret) {
+        VIR_ERROR("udev scan devices returned %d\n", ret);
+        goto out;
+    }
+
+    udev_list_entry_foreach(list_entry,
+                            udev_enumerate_get_list_entry(udev_enumerate)) {
+
+        udevProcessDeviceListEntry(udev, list_entry);
+        name = udev_list_entry_get_name(list_entry);
+    }
+
+out:
+    udev_enumerate_unref(udev_enumerate);
+    return ret;
+}
+
+
+static int udevDeviceMonitorShutdown(void)
+{
+    int ret = 0;
+
+    struct udev_monitor *udev_monitor = NULL;
+    struct udev *udev = NULL;
+
+    if (driverState) {
+
+        nodeDeviceLock(driverState);
+        udev_monitor = DRV_STATE_UDEV_MONITOR(driverState);
+
+        if (udev_monitor != NULL) {
+            udev = udev_monitor_get_udev(udev_monitor);
+            udev_monitor_unref(udev_monitor);
+        }
+
+        if (udev != NULL) {
+            udev_unref(udev);
+        }
+
+        virNodeDeviceObjListFree(&driverState->devs);
+        nodeDeviceUnlock(driverState);
+        virMutexDestroy(&driverState->lock);
+        VIR_FREE(driverState);
+
+    } else {
+        ret = -1;
+    }
+
+    return ret;
+}
+
+
+static void udevEventHandleCallback(int watch ATTRIBUTE_UNUSED,
+                                    int fd,
+                                    int events ATTRIBUTE_UNUSED,
+                                    void *data ATTRIBUTE_UNUSED)
+{
+    struct udev_device *device = NULL;
+    struct udev_monitor *udev_monitor = DRV_STATE_UDEV_MONITOR(driverState);
+    const char *action = NULL;
+    int udev_fd = -1;
+
+    udev_fd = udev_monitor_get_fd(udev_monitor);
+    if (fd != udev_fd) {
+        VIR_ERROR("File descriptor returned by udev %d does not "
+                  "match node device file descriptor %d", fd, udev_fd);
+        goto out;
+    }
+
+    device = udev_monitor_receive_device(udev_monitor);
+    if (device == NULL) {
+        VIR_ERROR0("udev_monitor_receive_device returned NULL\n");
+        goto out;
+    }
+
+    action = udev_device_get_action(device);
+    VIR_DEBUG("udev action: '%s'\n", action);
+
+    if (STREQ(action, "add") || STREQ(action, "change")) {
+        udevAddOneDevice(device);
+        goto out;
+    }
+
+    if (STREQ(action, "remove")) {
+        udevRemoveOneDevice(device);
+        goto out;
+    }
+
+out:
+    return;
+}
+
+
+static int udevSetupSystemDev(void)
+{
+    virNodeDeviceDefPtr def = NULL;
+    virNodeDeviceObjPtr dev = NULL;
+    struct udev *udev = NULL;
+    struct udev_device *device = NULL;
+    union _virNodeDevCapData *data = NULL;
+    char *tmp = NULL;
+    int ret = -1;
+
+    if (VIR_ALLOC(def) != 0) {
+        virReportOOMError(NULL);
+        goto out;
+    }
+
+    def->name = strdup("computer");
+    if (def->name == NULL) {
+        virReportOOMError(NULL);
+        goto out;
+    }
+
+    if (VIR_ALLOC(def->caps) != 0) {
+        virReportOOMError(NULL);
+        goto out;
+    }
+
+    udev = udev_monitor_get_udev(DRV_STATE_UDEV_MONITOR(driverState));
+    device = udev_device_new_from_syspath(udev, DMI_DEVPATH);
+    if (device == NULL) {
+        VIR_ERROR("Failed to get udev device for syspath '%s'\n", DMI_DEVPATH);
+        goto out;
+    }
+
+    data = &def->caps->data;
+
+    if (udevGetStringSysfsAttr(device,
+                               "product_name",
+                               &data->system.product_name) == PROPERTY_ERROR) {
+        goto out;
+    }
+    if (udevGetStringSysfsAttr(device,
+                               "sys_vendor",
+                               &data->system.hardware.vendor_name)
+        == PROPERTY_ERROR) {
+        goto out;
+    }
+    if (udevGetStringSysfsAttr(device,
+                               "product_version",
+                               &data->system.hardware.version)
+        == PROPERTY_ERROR) {
+        goto out;
+    }
+    if (udevGetStringSysfsAttr(device,
+                               "product_serial",
+                               &data->system.hardware.serial)
+        == PROPERTY_ERROR) {
+        goto out;
+    }
+
+    if (udevGetStringSysfsAttr(device,
+                               "product_uuid",
+                               &tmp) == PROPERTY_ERROR) {
+        goto out;
+    }
+    virUUIDParse(tmp, def->caps->data.system.hardware.uuid);
+    VIR_FREE(tmp);
+
+    if (udevGetStringSysfsAttr(device,
+                               "bios_vendor",
+                               &data->system.firmware.vendor_name)
+        == PROPERTY_ERROR) {
+        goto out;
+    }
+    if (udevGetStringSysfsAttr(device,
+                               "bios_version",
+                               &data->system.firmware.version)
+        == PROPERTY_ERROR) {
+        goto out;
+    }
+    if (udevGetStringSysfsAttr(device,
+                               "bios_date",
+                               &data->system.firmware.release_date)
+        == PROPERTY_ERROR) {
+        goto out;
+    }
+
+    udev_device_unref(device);
+
+    dev = virNodeDeviceAssignDef(NULL, &driverState->devs, def);
+    if (dev == NULL) {
+        VIR_ERROR("Failed to create device for '%s'\n", def->name);
+        virNodeDeviceDefFree(def);
+        goto out;
+    }
+
+    virNodeDeviceObjUnlock(dev);
+
+    ret = 0;
+
+out:
+    return ret;
+}
+
+static int udevDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED)
+{
+    struct udev *udev = NULL;
+    struct udev_monitor *udev_monitor = NULL;
+    int ret = 0;
+
+    if (VIR_ALLOC(driverState) < 0) {
+        virReportOOMError(NULL);
+        ret = -1;
+        goto out;
+    }
+
+    if (virMutexInit(&driverState->lock) < 0) {
+        VIR_ERROR0("Failed to initialize mutex for driverState\n");
+        VIR_FREE(driverState);
+        ret = -1;
+        goto out;
+    }
+
+    nodeDeviceLock(driverState);
+
+    /*
+     * http://www.kernel.org/pub/linux/utils/kernel/hotplug/libudev/libudev-udev.ht...
+     *
+     * indicates no return value other than success, so we don't check
+     * its return value.
+     */
+    udev = udev_new();
+    udev_set_log_fn(udev, udevLogFunction);
+
+    udev_monitor = udev_monitor_new_from_netlink(udev, "udev");
+    if (udev_monitor == NULL) {
+        VIR_ERROR0("udev_monitor_new_from_netlink returned NULL\n");
+        ret = -1;
+        goto out;
+    }
+
+    udev_monitor_enable_receiving(udev_monitor);
+
+    /* udev can be retrieved from udev_monitor */
+    driverState->privateData = udev_monitor;
+    nodeDeviceUnlock(driverState);
+
+    /* We register the monitor with the event callback so we are
+     * notified by udev of device changes before we enumerate existing
+     * devices because libvirt will simply recreate the device if we
+     * try to register it twice, i.e., if the device appears between
+     * the time we register the callback and the time we begin
+     * enumeration.  The alternative is to register the callback after
+     * we enumerate, in which case we will fail to create any devices
+     * that appear while the enumeration is taking place.  */
+    if (virEventAddHandleImpl(udev_monitor_get_fd(udev_monitor),
+                              VIR_EVENT_HANDLE_READABLE,
+                              udevEventHandleCallback,
+                              NULL, NULL) == -1) {
+        ret = -1;
+        goto out;
+    }
+
+    /* Create a fictional 'computer' device to root the device tree. */
+    if (udevSetupSystemDev() != 0) {
+        ret = -1;
+        goto out;
+    }
+
+    /* Populate with known devices */
+
+    if (udevEnumerateDevices(udev) != 0) {
+        ret = -1;
+        goto out;
+    }
+
+out:
+    if (ret == -1) {
+        udevDeviceMonitorShutdown();
+    }
+    return ret;
+}
+
+
+static int udevDeviceMonitorReload(void)
+{
+    return 0;
+}
+
+
+static int udevDeviceMonitorActive(void)
+{
+    /* Always ready to deal with a shutdown */
+    return 0;
+}
+
+
+static virDrvOpenStatus udevNodeDrvOpen(virConnectPtr conn,
+                                        virConnectAuthPtr auth ATTRIBUTE_UNUSED,
+                                        int flags ATTRIBUTE_UNUSED)
+{
+    if (driverState == NULL) {
+        return VIR_DRV_OPEN_DECLINED;
+    }
+
+    conn->devMonPrivateData = driverState;
+
+    return VIR_DRV_OPEN_SUCCESS;
+}
+
+static int udevNodeDrvClose(virConnectPtr conn)
+{
+    conn->devMonPrivateData = NULL;
+    return 0;
+}
+
+static virDeviceMonitor udevDeviceMonitor = {
+    .name = "udevDeviceMonitor",
+    .open = udevNodeDrvOpen,
+    .close = udevNodeDrvClose,
+};
+
+static virStateDriver udevStateDriver = {
+    .initialize = udevDeviceMonitorStartup,
+    .cleanup = udevDeviceMonitorShutdown,
+    .reload = udevDeviceMonitorReload,
+    .active = udevDeviceMonitorActive,
+};
+
+int udevNodeRegister(void)
+{
+    VIR_DEBUG0("Registering udev node device backend\n");
+
+    registerCommonNodeFuncs(&udevDeviceMonitor);
+    if (virRegisterDeviceMonitor(&udevDeviceMonitor) < 0) {
+        return -1;
+    }
+
+    return virRegisterStateDriver(&udevStateDriver);
+}
diff --git a/src/node_device/node_device_udev.h b/src/node_device/node_device_udev.h
new file mode 100644
index 0000000..0fd39ae
--- /dev/null
+++ b/src/node_device/node_device_udev.h
@@ -0,0 +1,31 @@
+/*
+ * node_device_udev.h: node device enumeration - libudev implementation
+ *
+ * Copyright (C) 2009 Red Hat
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
+ *
+ * Author: Dave Allan <dallan@redhat.com>
+ */
+
+#include <libudev.h>
+#include <stdint.h>
+
+#define SYSFS_DATA_SIZE 4096
+#define DRV_STATE_UDEV_MONITOR(ds) ((struct udev_monitor *)((ds)->privateData))
+#define DMI_DEVPATH "/sys/devices/virtual/dmi/id"
+#define PROPERTY_FOUND 0
+#define PROPERTY_MISSING 1
+#define PROPERTY_ERROR -1
diff --git a/src/util/util.c b/src/util/util.c
index 853d3a0..e472e0c 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -2128,3 +2128,31 @@ void virFileWaitForDevices(virConnectPtr conn)
 void virFileWaitForDevices(virConnectPtr conn ATTRIBUTE_UNUSED) {}
 #endif
 #endif
+
+int virBuildPathInternal(char **path, ...)
+{
+    char *path_component = NULL;
+    virBuffer buf = VIR_BUFFER_INITIALIZER;
+    va_list ap;
+    int ret = 0;
+
+    va_start(ap, *path);
+
+    path_component = va_arg(ap, char *);
+    virBufferAdd(&buf, path_component, -1);
+
+    while ((path_component = va_arg(ap, char *)) != NULL)
+    {
+        virBufferAddChar(&buf, '/');
+        virBufferAdd(&buf, path_component, -1);
+    }
+
+    va_end(ap);
+
+    *path = virBufferContentAndReset(&buf);
+    if (*path == NULL) {
+        ret = -1;
+    }
+
+    return ret;
+}
diff --git a/src/util/util.h b/src/util/util.h
index f4e395e..8c9d401 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -248,4 +248,7 @@ char *virFileFindMountPoint(const char *type);

 void virFileWaitForDevices(virConnectPtr conn);

+#define virBuildPath(path, ...) virBuildPathInternal(path, __VA_ARGS__, NULL)
+int virBuildPathInternal(char **path, ...) ATTRIBUTE_SENTINEL;
+
 #endif /* __VIR_UTIL_H__ */
-- 
1.6.5.1
...
From fb2686fa7f9c3e553bdd5c3d08757dae3fef9c21 Mon Sep 17 00:00:00 2001
From: David Allan <dallan@redhat.com>
Date: Thu, 12 Nov 2009 12:12:09 -0500
Subject: [PATCH 3/5] Add scsi_target device type
Having libvirt understand scsi targets makes the node device tree look a bit better; there isn't a lot of use for them otherwise.  The device tree is still not perfect, but one step at a time.
---
 src/conf/node_device_conf.c        |   43 ++++++++++++++++++++++++++++++++++++
 src/conf/node_device_conf.h        |    4 +++
 src/node_device/node_device_udev.c |   34 ++++++++++++++++++++++++++++
 3 files changed, 81 insertions(+), 0 deletions(-)

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index f3aca76..f0bdff0 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -46,6 +46,7 @@ VIR_ENUM_IMPL(virNodeDevCap, VIR_NODE_DEV_CAP_LAST,
               "usb",
               "net",
               "scsi_host",
+              "scsi_target",
               "scsi",
               "storage")

@@ -387,6 +388,12 @@ char *virNodeDeviceDefFormat(virConnectPtr conn,
             }

             break;
+
+        case VIR_NODE_DEV_CAP_SCSI_TARGET:
+            virBufferVSprintf(&buf, "    <target>%s</target>\n",
+                              data->scsi_target.name);
+            break;
+
         case VIR_NODE_DEV_CAP_SCSI:
             virBufferVSprintf(&buf, "    <host>%d</host>\n", data->scsi.host);
             virBufferVSprintf(&buf, "    <bus>%d</bus>\n", data->scsi.bus);
@@ -654,6 +661,36 @@ out:
     return ret;
 }

+
+static int
+virNodeDevCapScsiTargetParseXML(virConnectPtr conn,
+                                xmlXPathContextPtr ctxt,
+                                virNodeDeviceDefPtr def,
+                                xmlNodePtr node,
+                                union _virNodeDevCapData *data)
+{
+    xmlNodePtr orignode;
+    int ret = -1;
+
+    orignode = ctxt->node;
+    ctxt->node = node;
+
+    data->scsi_target.name = virXPathString(conn, "string(./name[1])", ctxt);
+    if (!data->scsi_target.name) {
+        virNodeDeviceReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                                 _("no target name supplied for '%s'"),
+                                 def->name);
+        goto out;
+    }
+
+    ret = 0;
+
+out:
+    ctxt->node = orignode;
+    return ret;
+}
+
+
 static int
 virNodeDevCapScsiHostParseXML(virConnectPtr conn,
                               xmlXPathContextPtr ctxt,
@@ -1058,6 +1095,9 @@ virNodeDevCapsDefParseXML(virConnectPtr conn,
     case VIR_NODE_DEV_CAP_SCSI_HOST:
         ret = virNodeDevCapScsiHostParseXML(conn, ctxt, def, node, &caps->data, create);
         break;
+    case VIR_NODE_DEV_CAP_SCSI_TARGET:
+        ret = virNodeDevCapScsiTargetParseXML(conn, ctxt, def, node, &caps->data);
+        break;
     case VIR_NODE_DEV_CAP_SCSI:
         ret = virNodeDevCapScsiParseXML(conn, ctxt, def, node, &caps->data);
         break;
@@ -1381,6 +1421,9 @@ void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps)
         VIR_FREE(data->scsi_host.wwnn);
         VIR_FREE(data->scsi_host.wwpn);
         break;
+    case VIR_NODE_DEV_CAP_SCSI_TARGET:
+        VIR_FREE(data->scsi_target.name);
+        break;
     case VIR_NODE_DEV_CAP_SCSI:
         VIR_FREE(data->scsi.type);
         break;
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index e97242a..639a7e7 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -41,6 +41,7 @@ enum virNodeDevCapType {
     VIR_NODE_DEV_CAP_USB_INTERFACE,	/* USB interface */
     VIR_NODE_DEV_CAP_NET,		/* Network device */
     VIR_NODE_DEV_CAP_SCSI_HOST,		/* SCSI Host Bus Adapter */
+    VIR_NODE_DEV_CAP_SCSI_TARGET,	/* SCSI Target */
     VIR_NODE_DEV_CAP_SCSI,		/* SCSI device */
     VIR_NODE_DEV_CAP_STORAGE,		/* Storage device */
     VIR_NODE_DEV_CAP_LAST
@@ -133,6 +134,9 @@ struct _virNodeDevCapsDef {
             unsigned flags;
         } scsi_host;
         struct {
+            char *name;
+        } scsi_target;
+        struct {
             unsigned host;
             unsigned bus;
             unsigned target;
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index d608b76..010ff75 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -601,6 +601,32 @@ out:
 }


+static int udevProcessSCSITarget(struct udev_device *device ATTRIBUTE_UNUSED,
+                                 virNodeDeviceDefPtr def)
+{
+    int ret = -1;
+    const char *sysname = NULL;
+    union _virNodeDevCapData *data = &def->caps->data;
+
+    sysname = udev_device_get_sysname(device);
+
+    data->scsi_target.name = strdup(sysname);
+    if (data->scsi_target.name == NULL) {
+        virReportOOMError(NULL);
+        goto out;
+    }
+
+    if (udevGenerateDeviceName(device, def, NULL) != 0) {
+        goto out;
+    }
+
+    ret = 0;
+
+out:
+    return ret;
+}
+
+
 static int udevGetSCSIType(unsigned int type, char **typestring)
 {
     int ret = 0;
@@ -948,6 +974,11 @@ static int udevGetDeviceType(struct udev_device *device,
         goto out;
     }

+    if (devtype != NULL && STREQ(devtype, "scsi_target")) {
+        *type = VIR_NODE_DEV_CAP_SCSI_TARGET;
+        goto out;
+    }
+
     if (devtype != NULL && STREQ(devtype, "scsi_device")) {
         *type = VIR_NODE_DEV_CAP_SCSI;
         goto out;
@@ -1008,6 +1039,9 @@ static int udevGetDeviceDetails(struct udev_device *device,
     case VIR_NODE_DEV_CAP_SCSI_HOST:
         ret = udevProcessSCSIHost(device, def);
         break;
+    case VIR_NODE_DEV_CAP_SCSI_TARGET:
+        ret = udevProcessSCSITarget(device, def);
+        break;
     case VIR_NODE_DEV_CAP_SCSI:
         ret = udevProcessSCSIDevice(device, def);
         break;
-- 
1.6.5.1
...
From 39c6c695eec6a9e99a70e51824da3e86fafbfa23 Mon Sep 17 00:00:00 2001
From: David Allan <dallan@redhat.com>
Date: Thu, 12 Nov 2009 15:47:59 -0500
Subject: [PATCH 4/5] Remove DevKit node device backend
---
 configure.in                         |    7 +-
 src/Makefile.am                      |    9 -
 src/node_device/node_device_devkit.c |  447 ----------------------------------
 src/node_device/node_device_driver.c |    3 -
 src/node_device/node_device_driver.h |    3 -
 5 files changed, 1 insertions(+), 468 deletions(-)
 delete mode 100644 src/node_device/node_device_devkit.c

diff --git a/configure.in b/configure.in
index dc1e43e..c167381 100644
--- a/configure.in
+++ b/configure.in
@@ -1787,7 +1787,7 @@ AC_SUBST([UDEV_CFLAGS])
 AC_SUBST([UDEV_LIBS])

 with_nodedev=no;
-if test "$with_devkit" = "yes" -o "$with_hal" = "yes" -o "$with_udev" = "yes";
+if test "$with_hal" = "yes" -o "$with_udev" = "yes";
 then
   with_nodedev=yes
   AC_DEFINE_UNQUOTED([WITH_NODE_DEVICES], 1, [with node device driver])
@@ -1947,11 +1947,6 @@ AC_MSG_NOTICE([     hal: $HAL_CFLAGS $HAL_LIBS])
 else
 AC_MSG_NOTICE([     hal: no])
 fi
-if test "$with_devkit" = "yes" ; then
-AC_MSG_NOTICE([  devkit: $DEVKIT_CFLAGS $DEVKIT_LIBS])
-else
-AC_MSG_NOTICE([  devkit: no])
-fi
 if test "$with_udev" = "yes" ; then
 AC_MSG_NOTICE([    udev: $UDEV_CFLAGS $UDEV_LIBS])
 else
diff --git a/src/Makefile.am b/src/Makefile.am
index afe7eac..4aaad6b 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -267,9 +267,6 @@ NODE_DEVICE_DRIVER_HAL_SOURCES =				\
 		node_device/node_device_hal.c			\
 		node_device/node_device_hal.h

-NODE_DEVICE_DRIVER_DEVKIT_SOURCES =				\
-		node_device/node_device_devkit.c
-
 NODE_DEVICE_DRIVER_UDEV_SOURCES =				\
 		node_device/node_device_udev.c

@@ -646,11 +643,6 @@ libvirt_driver_nodedev_la_SOURCES += $(NODE_DEVICE_DRIVER_HAL_SOURCES)
 libvirt_driver_nodedev_la_CFLAGS += $(HAL_CFLAGS)
 libvirt_driver_nodedev_la_LDFLAGS += $(HAL_LIBS)
 endif
-if HAVE_DEVKIT
-libvirt_driver_nodedev_la_SOURCES += $(NODE_DEVICE_DRIVER_DEVKIT_SOURCES)
-libvirt_driver_nodedev_la_CFLAGS += $(DEVKIT_CFLAGS)
-libvirt_driver_nodedev_la_LDFLAGS += $(DEVKIT_LIBS)
-endif
 if HAVE_UDEV
 libvirt_driver_nodedev_la_SOURCES += $(NODE_DEVICE_DRIVER_UDEV_SOURCES)
 libvirt_driver_nodedev_la_CFLAGS += $(UDEV_CFLAGS)
@@ -704,7 +696,6 @@ EXTRA_DIST +=							\
 		$(STORAGE_DRIVER_DISK_SOURCES)			\
 		$(NODE_DEVICE_DRIVER_SOURCES)			\
 		$(NODE_DEVICE_DRIVER_HAL_SOURCES)		\
-		$(NODE_DEVICE_DRIVER_DEVKIT_SOURCES)		\
 		$(NODE_DEVICE_DRIVER_UDEV_SOURCES)		\
 		$(SECURITY_DRIVER_SELINUX_SOURCES)		\
 		$(SECURITY_DRIVER_APPARMOR_SOURCES)		\
diff --git a/src/node_device/node_device_devkit.c b/src/node_device/node_device_devkit.c
deleted file mode 100644
index d2ffa1d..0000000
--- a/src/node_device/node_device_devkit.c
+++ /dev/null
@@ -1,447 +0,0 @@
-/*
- * node_device_devkit.c: node device enumeration - DeviceKit-based implementation
- *
- * Copyright (C) 2008 Virtual Iron Software, Inc.
- * Copyright (C) 2008 David F. Lively
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the License, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
- *
- * Author: David F. Lively <dlively@virtualiron.com>
- */
-
-#include <config.h>
-#include <devkit-gobject.h>
-
-#include "node_device_conf.h"
-#include "virterror_internal.h"
-#include "driver.h"
-#include "datatypes.h"
-#include "event.h"
-#include "memory.h"
-#include "uuid.h"
-#include "logging.h"
-
-#include "node_device.h"
-
-/*
- * Host device enumeration (DeviceKit implementation)
- */
-
-static virDeviceMonitorStatePtr driverState;
-
-#define CONN_DRV_STATE(conn) \
-        ((virDeviceMonitorStatePtr)((conn)->devMonPrivateData))
-#define DRV_STATE_DKCLIENT(ds) ((DevkitClient *)((ds)->privateData))
-#define CONN_DKCLIENT(conn) DRV_STATE_DKCLIENT(CONN_DRV_STATE(conn))
-
-#define NODE_DEV_DKDEV(obj) ((DevkitDevice *)((obj)->privateData)
-
-static int get_str_prop(DevkitDevice *dkdev, const char *prop, char **val_p)
-{
-    char *val = devkit_device_dup_property_as_str(dkdev, prop);
-
-    if (val) {
-        if (*val) {
-            *val_p = val;
-            return 0;
-        } else {
-            /* Treat empty strings as NULL values */
-            VIR_FREE(val);
-        }
-    }
-
-    return -1;
-}
-
-#if 0
-static int get_int_prop(DevkitDevice *dkdev, const char *prop, int *val_p)
-{
-    if (! devkit_device_has_property(dkdev, prop))
-        return -1;
-    *val_p = devkit_device_get_property_as_int(dkdev, prop);
-    return 0;
-}
-
-static int get_uint64_prop(DevkitDevice *dkdev, const char *prop,
-                           unsigned long long *val_p)
-{
-    if (! devkit_device_has_property(dkdev, prop))
-        return -1;
-    *val_p = devkit_device_get_property_as_uint64(dkdev, prop);
-    return 0;
-}
-#endif
-
-static int gather_pci_cap(DevkitDevice *dkdev,
-                          union _virNodeDevCapData *d)
-{
-    const char *sysfs_path = devkit_device_get_native_path(dkdev);
-
-    if (sysfs_path != NULL) {
-        char *p = strrchr(sysfs_path, '/');
-        if (p) {
-            (void)virStrToLong_ui(p+1, &p, 16, &d->pci_dev.domain);
-            (void)virStrToLong_ui(p+1, &p, 16, &d->pci_dev.bus);
-            (void)virStrToLong_ui(p+1, &p, 16, &d->pci_dev.slot);
-            (void)virStrToLong_ui(p+1, &p, 16, &d->pci_dev.function);
-        }
-    }
-    return 0;
-}
-
-
-static int gather_usb_cap(DevkitDevice *dkdev,
-                          union _virNodeDevCapData *d)
-{
-    (void)get_str_prop(dkdev, "ID_VENDOR", &d->usb_dev.vendor_name);
-    (void)get_str_prop(dkdev, "ID_MODEL", &d->usb_dev.product_name);
-    return 0;
-}
-
-
-static int gather_net_cap(DevkitDevice *dkdev,
-                          union _virNodeDevCapData *d)
-{
-    const char *sysfs_path = devkit_device_get_native_path(dkdev);
-    const char *ifname;
-
-    if (sysfs_path == NULL)
-        return -1;
-    ifname = strrchr(sysfs_path, '/');
-    if (!ifname || !*ifname || !*(++ifname))
-        return -1;
-    if ((d->net.ifname = strdup(ifname)) == NULL)
-        return -1;
-
-    d->net.subtype = VIR_NODE_DEV_CAP_NET_LAST;
-
-    return 0;
-}
-
-
-static int gather_storage_cap(DevkitDevice *dkdev,
-                              union _virNodeDevCapData *d)
-{
-    const char *device = devkit_device_get_device_file(dkdev);
-
-    if (device && ((d->storage.block = strdup(device)) == NULL))
-        return -1;
-
-    return 0;
-}
-
-
-struct _caps_tbl_entry {
-    const char *cap_name;
-    enum virNodeDevCapType type;
-    int (*gather_fn)(DevkitDevice *dkdev,
-                     union _virNodeDevCapData *data);
-};
-
-typedef struct _caps_tbl_entry caps_tbl_entry;
-
-static caps_tbl_entry caps_tbl[] = {
-    { "pci",        VIR_NODE_DEV_CAP_PCI_DEV,   gather_pci_cap },
-    { "usb",        VIR_NODE_DEV_CAP_USB_DEV,   gather_usb_cap },
-    { "net",        VIR_NODE_DEV_CAP_NET,       gather_net_cap },
-    { "block",      VIR_NODE_DEV_CAP_STORAGE,   gather_storage_cap },
-    // TODO: more caps!
-};
-
-
-/* qsort/bsearch string comparator */
-static int cmpstringp(const void *p1, const void *p2)
-{
-    /* from man 3 qsort */
-    return strcmp(* (char * const *) p1, * (char * const *) p2);
-}
-
-
-static int gather_capability(DevkitDevice *dkdev,
-                             const char *cap_name,
-                             virNodeDevCapsDefPtr *caps_p)
-{
-    size_t caps_tbl_len = sizeof(caps_tbl) / sizeof(caps_tbl[0]);
-    caps_tbl_entry *entry;
-
-    entry = bsearch(&cap_name, caps_tbl, caps_tbl_len,
-                    sizeof(caps_tbl[0]), cmpstringp);
-
-    if (entry) {
-        virNodeDevCapsDefPtr caps;
-        if (VIR_ALLOC(caps) < 0)
-            return ENOMEM;
-        caps->type = entry->type;
-        if (entry->gather_fn) {
-            int rv = (*entry->gather_fn)(dkdev, &caps->data);
-            if (rv != 0) {
-                virNodeDevCapsDefFree(caps);
-                return rv;
-            }
-        }
-        caps->next = *caps_p;
-        *caps_p = caps;
-    }
-
-    return 0;
-}
-
-
-static int gather_capabilities(DevkitDevice *dkdev,
-                               virNodeDevCapsDefPtr *caps_p)
-{
-    const char *subsys = devkit_device_get_subsystem(dkdev);
-    const char *bus_name = devkit_device_get_property(dkdev, "ID_BUS");
-    virNodeDevCapsDefPtr caps = NULL;
-    int rv;
-
-    if (subsys) {
-        rv = gather_capability(dkdev, subsys, &caps);
-        if (rv != 0) goto failure;
-    }
-
-    if (bus_name && (subsys == NULL || !STREQ(bus_name, subsys))) {
-        rv = gather_capability(dkdev, bus_name, &caps);
-        if (rv != 0) goto failure;
-    }
-
-    *caps_p = caps;
-    return 0;
-
- failure:
-    while (caps) {
-        virNodeDevCapsDefPtr next = caps->next;
-        virNodeDevCapsDefFree(caps);
-        caps = next;
-    }
-    return rv;
-}
-
-static void dev_create(void *_dkdev, void *_dkclient ATTRIBUTE_UNUSED)
-{
-    DevkitDevice *dkdev = _dkdev;
-    const char *sysfs_path = devkit_device_get_native_path(dkdev);
-    virNodeDeviceObjPtr dev = NULL;
-    virNodeDeviceDefPtr def = NULL;
-    const char *name;
-    int rv;
-
-    if (sysfs_path == NULL)
-        /* Currently using basename(sysfs_path) as device name (key) */
-        return;
-
-    name = strrchr(sysfs_path, '/');
-    if (name == NULL)
-        name = sysfs_path;
-    else
-        ++name;
-
-    if (VIR_ALLOC(def) < 0)
-        goto failure;
-
-    if ((def->name = strdup(name)) == NULL)
-        goto failure;
-
-    // TODO: Find device parent, if any
-
-    rv = gather_capabilities(dkdev, &def->caps);
-    if (rv != 0) goto failure;
-
-    nodeDeviceLock(driverState);
-    dev = virNodeDeviceAssignDef(NULL,
-                                 &driverState->devs,
-                                 def);
-
-    if (!dev) {
-        nodeDeviceUnlock(driverState);
-        goto failure;
-    }
-
-    dev->privateData = dkdev;
-    dev->privateFree = NULL; /* XXX some free func needed ? */
-    virNodeDeviceObjUnlock(dev);
-
-    nodeDeviceUnlock(driverState);
-
-    return;
-
- failure:
-    DEBUG("FAILED TO ADD dev %s", name);
-    if (def)
-        virNodeDeviceDefFree(def);
-}
-
-
-static int devkitDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED)
-{
-    size_t caps_tbl_len = sizeof(caps_tbl) / sizeof(caps_tbl[0]);
-    DevkitClient *devkit_client = NULL;
-    GError *err = NULL;
-    GList *devs;
-    int i;
-
-    /* Ensure caps_tbl is sorted by capability name */
-    qsort(caps_tbl, caps_tbl_len, sizeof(caps_tbl[0]), cmpstringp);
-
-    if (VIR_ALLOC(driverState) < 0)
-        return -1;
-
-    if (virMutexInit(&driverState->lock) < 0) {
-        VIR_FREE(driverState);
-        return -1;
-    }
-
-    g_type_init();
-
-    /* Get new devkit_client and connect to daemon */
-    devkit_client = devkit_client_new(NULL);
-    if (devkit_client == NULL) {
-        DEBUG0("devkit_client_new returned NULL");
-        goto failure;
-    }
-    if (!devkit_client_connect(devkit_client, &err)) {
-        DEBUG0("devkit_client_connect failed");
-        goto failure;
-    }
-
-    /* Populate with known devices.
-     *
-     * This really should be:
-        devs = devkit_client_enumerate_by_subsystem(devkit_client, NULL, &err);
-        if (err) {
-            DEBUG0("devkit_client_enumerate_by_subsystem failed");
-            devs = NULL;
-            goto failure;
-        }
-        g_list_foreach(devs, dev_create, devkit_client);
-    * but devkit_client_enumerate_by_subsystem currently fails when the second
-    * arg is null (contrary to the API documentation).  So the following code
-    * (from Dan B) works around this by listing devices per handled subsystem.
-    */
-
-    for (i = 0 ; i < ARRAY_CARDINALITY(caps_tbl) ; i++) {
-        const char *caps[] = { caps_tbl[i].cap_name, NULL };
-        devs = devkit_client_enumerate_by_subsystem(devkit_client,
-                                                    caps,
-                                                    &err);
-        if (err) {
-            DEBUG0("devkit_client_enumerate_by_subsystem failed");
-            devs = NULL;
-            goto failure;
-        }
-        g_list_foreach(devs, dev_create, devkit_client);
-    }
-
-    driverState->privateData = devkit_client;
-
-    // TODO: Register to get DeviceKit events on device changes and
-    //       coordinate updates with queries and other operations.
-
-    return 0;
-
- failure:
-    if (err) {
-        DEBUG("\terror[%d]: %s", err->code, err->message);
-        g_error_free(err);
-    }
-    if (devs) {
-        g_list_foreach(devs, (GFunc)g_object_unref, NULL);
-        g_list_free(devs);
-    }
-    if (devkit_client)
-        g_object_unref(devkit_client);
-    VIR_FREE(driverState);
-
-    return -1;
-}
-
-
-static int devkitDeviceMonitorShutdown(void)
-{
-    if (driverState) {
-        DevkitClient *devkit_client;
-
-        nodeDeviceLock(driverState);
-        devkit_client = DRV_STATE_DKCLIENT(driverState);
-        virNodeDeviceObjListFree(&driverState->devs);
-        if (devkit_client)
-            g_object_unref(devkit_client);
-        nodeDeviceUnlock(driverState);
-        virMutexDestroy(&driveState->lock);
-        VIR_FREE(driverState);
-        return 0;
-    }
-    return -1;
-}
-
-
-static int devkitDeviceMonitorReload(void)
-{
-    /* XXX This isn't thread safe because its free'ing the thing
-     * we're locking */
-    (void)devkitDeviceMonitorShutdown();
-    return devkitDeviceMonitorStartup();
-}
-
-
-static int devkitDeviceMonitorActive(void)
-{
-    /* Always ready to deal with a shutdown */
-    return 0;
-}
-
-
-static virDrvOpenStatus
-devkitNodeDrvOpen(virConnectPtr conn,
-                  virConnectAuthPtr auth ATTRIBUTE_UNUSED,
-                  int flags ATTRIBUTE_UNUSED)
-{
-    if (driverState == NULL)
-        return VIR_DRV_OPEN_DECLINED;
-
-    conn->devMonPrivateData = driverState;
-
-    return VIR_DRV_OPEN_SUCCESS;
-}
-
-static int devkitNodeDrvClose(virConnectPtr conn ATTRIBUTE_UNUSED)
-{
-    conn->devMonPrivateData = NULL;
-    return 0;
-}
-
-
-static virDeviceMonitor devkitDeviceMonitor = {
-    .name = "devkitDeviceMonitor",
-    .open = devkitNodeDrvOpen,
-    .close = devkitNodeDrvClose,
-};
-
-
-static virStateDriver devkitStateDriver = {
-    .name = "DeviceKit",
-    .initialize = devkitDeviceMonitorStartup,
-    .cleanup = devkitDeviceMonitorShutdown,
-    .reload = devkitDeviceMonitorReload,
-    .active = devkitDeviceMonitorActive,
-};
-
-int devkitNodeRegister(void)
-{
-    registerCommonNodeFuncs(&devkitDeviceMonitor);
-    if (virRegisterDeviceMonitor(&devkitDeviceMonitor) < 0)
-        return -1;
-    return virRegisterStateDriver(&devkitStateDriver);
-}
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index c139907..cddd994 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -670,9 +670,6 @@ int nodedevRegister(void) {
 #ifdef HAVE_HAL
     return halNodeRegister();
 #endif
-#ifdef HAVE_DEVKIT
-    return devkitNodeRegister();
-#endif
 #ifdef HAVE_UDEV
     return udevNodeRegister();
 #endif
diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h
index 5be0781..4f0822c 100644
--- a/src/node_device/node_device_driver.h
+++ b/src/node_device/node_device_driver.h
@@ -42,9 +42,6 @@
 #ifdef HAVE_HAL
 int halNodeRegister(void);
 #endif
-#ifdef HAVE_DEVKIT
-int devkitNodeRegister(void);
-#endif
 #ifdef HAVE_UDEV
 int udevNodeRegister(void);
 #endif
-- 
1.6.5.1
...
From ff9a4c442c24bde4c429afd17609932688abcce8 Mon Sep 17 00:00:00 2001
From: David Allan <dallan@redhat.com>
Date: Thu, 12 Nov 2009 12:15:28 -0500
Subject: [PATCH 5/5] 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 |   64 ++++++++++++++++++++++++++++++++++-
 3 files changed, 73 insertions(+), 4 deletions(-)

diff --git a/configure.in b/configure.in
index c167381..8e7c059 100644
--- a/configure.in
+++ b/configure.in
@@ -1781,10 +1781,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 4aaad6b..d22a103 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -645,8 +645,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 010ff75..4ddf360 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 <c-ctype.h>

@@ -351,6 +352,61 @@ 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 = -1;
+    struct pci_id_match m;
+    const char *vendor_name = NULL, *device_name = NULL;
+
+    if (pci_system_init() != 0) {
+        VIR_ERROR0("Failed to initialize libpciaccess\n");
+        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,
+                    &vendor_name,
+                    &device_name,
+                    NULL,
+                    NULL);
+
+    if (vendor_name != NULL) {
+        *vendor_string = strdup(vendor_name);
+        if (*vendor_string == NULL) {
+            virReportOOMError(NULL);
+            goto out;
+        }
+    }
+
+    if (device_name != NULL) {
+        *product_string = strdup(device_name);
+        if (*product_string == NULL) {
+            virReportOOMError(NULL);
+            goto out;
+        }
+    }
+
+    /* pci_system_cleanup returns void */
+    pci_system_cleanup();
+
+    ret = 0;
+
+out:
+    return ret;
+}
+
+
 static int udevProcessPCI(struct udev_device *device,
                           virNodeDeviceDefPtr def)
 {
@@ -411,8 +467,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

    