[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] udev node device backend



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.html#udev-new
+     *
+     * 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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]