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

Re: [libvirt] udev node device backend



Dave Allan wrote:
Cole Robinson wrote:
On 11/04/2009 05:26 PM, Dave Allan wrote:
Dave Allan wrote:
Chris Lalancette wrote:
Daniel P. Berrange wrote:
On Wed, Oct 28, 2009 at 12:16:40PM +0100, Chris Lalancette wrote:
Dave Allan wrote:
Attached is a fully functional version of the node device udev based backend, incorporating all the feedback from earlier revisions. I broke the new capability fields out into a separate patch per Dan's suggestion, and I have also included a patch removing the DevKit backend.
3) I took a look at how the network is represented in the XML. In the HAL
backend, we get something that looks like:

<device>
  <name>net_00_13_20_f5_fa_e3</name>
  <parent>pci_8086_10bd</parent>
  <capability type='net'>
    <interface>eth0</interface>
    <address>00:13:20:f5:fa:e3</address>
    <capability type='80203'/>
  </capability>
</device>

That "<capability type='80203'/>" looks to be bogus (although I could be wrong; that same XML is encoded into the tests, so maybe there is something else going on). You are already in a <capability> block, so that should probably just be
"<type='80203'/>".  The same problem occurs in the udev backend.
Why do you think the '<capability type='80203'/>' bit is bogus ? That looks correct to me, showing that eth0 is a ethernet device (as opposed to a 80211
wireless, or neither)
Oh, I think the concept is useful, it's just that the way it is represented in
the XML looks weird:

<capability type='net'>
    ...
    <capability type='80203'/>
</capability>

Shouldn't this rather be:

<capability type='net'>
    ...
    <type>80203</type>
</capability>

Or:

<capability type='net' subtype='80203'>
    ...
</capability>

Or something like that?

Here's the latest version of the udev backend. I think I've addressed all of everybody's concerns, except for the translation of PCI ids to strings and the bogosity in the ethernet types. I've got working code for the PCI ids translation, but it's completely separate and involves modifying the build system again, so I'd like to get this set of patches in first. I'll sort out the ethernet types in a follow up patch as well. There's some additional follow up work to make the device tree look really nice, but that's also completely separate.

Dave
Attached are two additional patches. The first fixes a bug I found where I was reading the wrong sysfs attribute name, so the PCI device ID wasn't getting populated. The second uses libpciaccess to translate the PCI vendor and device IDs into their human readable names.

Dave


Just giving all these patches a spin now, and seeing a few issues.

Start daemon, do 'virsh nodedev-list', SIGINT the daemon and I
consistently see a segfault:

(gdb) bt
#0  0x0000003a91e75f52 in malloc_consolidate () from /lib64/libc.so.6
#1  0x0000003a91e79a72 in _int_malloc () from /lib64/libc.so.6
#2  0x0000003a91e7b058 in calloc () from /lib64/libc.so.6
#3 0x0000003a91a0b26f in _dl_new_object () from /lib64/ld-linux-x86-64.so.2
#4  0x0000003a91a06496 in _dl_map_object_from_fd ()
   from /lib64/ld-linux-x86-64.so.2
#5 0x0000003a91a0832a in _dl_map_object () from /lib64/ld-linux-x86-64.so.2 #6 0x0000003a91a13299 in dl_open_worker () from /lib64/ld-linux-x86-64.so.2 #7 0x0000003a91a0e7c6 in _dl_catch_error () from /lib64/ld-linux-x86-64.so.2
#8  0x0000003a91a12ca7 in _dl_open () from /lib64/ld-linux-x86-64.so.2
#9  0x0000003a91f1e4c0 in do_dlopen () from /lib64/libc.so.6
#10 0x0000003a91a0e7c6 in _dl_catch_error () from /lib64/ld-linux-x86-64.so.2
#11 0x0000003a91f1e617 in __libc_dlopen_mode () from /lib64/libc.so.6
#12 0x0000003a91ef6eb5 in init () from /lib64/libc.so.6
#13 0x0000003a92a0cab3 in pthread_once () from /lib64/libpthread.so.0
#14 0x0000003a91ef6fb4 in backtrace () from /lib64/libc.so.6
#15 0x0000003a91e703a7 in __libc_message () from /lib64/libc.so.6
#16 0x0000003a91e75dc6 in malloc_printerr () from /lib64/libc.so.6
#17 0x000000000042941c in virFree (ptrptr=0x72daa0) at util/memory.c:177
#18 0x00007ffff7acba22 in virNodeDevCapsDefFree (caps=0x72da70)
    at conf/node_device_conf.c:1413
#19 0x00007ffff7acbaa9 in virNodeDeviceDefFree (def=0x3a9217be80)
    at conf/node_device_conf.c:147
#20 0x00007ffff7acc5f5 in virNodeDeviceObjFree (dev=0x3a9217be80)
    at conf/node_device_conf.c:160
#21 0x00007ffff7acc8aa in virNodeDeviceObjListFree (devs=0x6cffe8)
    at conf/node_device_conf.c:173
#22 0x000000000046d02c in udevDeviceMonitorShutdown ()
    at node_device/node_device_udev.c:1154
#23 0x00007ffff7ad9f1e in virStateCleanup () at libvirt.c:851
#24 0x000000000041789d in qemudCleanup (server=0x6a8850) at libvirtd.c:2389
#25 main (server=0x6a8850) at libvirtd.c:318

Some minor compiler warnings I'm seeing on F12:

node_device/node_device_udev.c: In function 'udevGetUint64SysfsAttr':
node_device/node_device_udev.c:210: error: passing argument 4 of 'virStrToLong_ull' from incompatible pointer type ../src/util/util.h:157: note: expected 'long long unsigned int *' but argument is of type 'uint64_t *'
node_device/node_device_udev.c: In function 'udevProcessDisk':
node_device/node_device_udev.c:697: error: passing argument 3 of 'udevGetUint64SysfsAttr' from incompatible pointer type node_device/node_device_udev.c:200: note: expected 'uint64_t *' but argument is of type 'long long unsigned int *' node_device/node_device_udev.c:703: error: passing argument 3 of 'udevGetUint64SysfsAttr' from incompatible pointer type node_device/node_device_udev.c:200: note: expected 'uint64_t *' but argument is of type 'long long unsigned int *'
node_device/node_device_udev.c: In function 'udevProcessCDROM':
node_device/node_device_udev.c:736: error: passing argument 3 of 'udevGetUint64SysfsAttr' from incompatible pointer type node_device/node_device_udev.c:200: note: expected 'uint64_t *' but argument is of type 'long long unsigned int *' node_device/node_device_udev.c:742: error: passing argument 3 of 'udevGetUint64SysfsAttr' from incompatible pointer type node_device/node_device_udev.c:200: note: expected 'uint64_t *' but argument is of type 'long long unsigned int *'

In udevNodeRegister, you are using a VIR_ERROR0 where it should be a
VIR_DEBUG0.

I seem to get less PCI devices with the udev backend. The HAL backend
gives me the same amount of devices as lspci gives, udev gives me about
2/3 of that. If you can't reproduce I can provide more specifics.

USB device/product listing isn't the same as the previous HAL backend
and what is shown by lsusb (maybe the ls* tools use HAL? I haven't
checked). Compare these outputs:

udev =
<device>
  <name>usb_3-2</name>
  <udev_name>/sys/devices/pci0000:00/0000:00:1a.0/usb3/3-2</udev_name>
  <parent>usb_usb3</parent>

<parent_udev_name>/sys/devices/pci0000:00/0000:00:1a.0/usb3</parent_udev_name>
  <driver>
    <name>usb</name>
  </driver>
  <capability type='usb_device'>
    <bus>3</bus>
    <device>2</device>
    <product id='0x07e0'>Biometric_Coprocessor</product>
    <vendor id='0x0483'>STMicroelectronics</vendor>
  </capability>
</device>

hal =

<device>
  <name>usb_device_483_2016_noserial</name>
  <parent>usb_device_1d6b_1_0000_00_1a_0</parent>
  <driver>
    <name>usb</name>
  </driver>
  <capability type='usb_device'>
    <bus>3</bus>
    <device>2</device>
    <product id='0x2016'>Fingerprint Reader</product>
    <vendor id='0x0483'>SGS Thomson Microelectronics</vendor>
  </capability>
</device>

Also, either udev or libvirt is adding underscores in product names
where there aren't any listed in sysfs. Not sure if this is a problem or
not.

- Cole

Hi Cole,

Here's a revised patch set that fixes everything you found. Thanks for all the feedback. You should only have to apply the last one to your tree.

Dave

I noticed when starting to work on some other stuff that I ought to have named a struct member sysfs_path rather than udev_name. Here's an updated patch set that makes that change. I've just rolled it into the previous patch.

Dave


>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);
+            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 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.
---
 configure.in                                       |   47 +-
 daemon/libvirtd.c                                  |    3 +-
 po/POTFILES.in                                     |    3 +-
 src/Makefile.am                                    |   16 +-
 src/conf/node_device_conf.c                        |   34 +-
 src/conf/node_device_conf.h                        |    5 +
 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                 | 1351 ++++++++++++++++++++
 src/node_device/node_device_udev.h                 |   32 +
 src/util/util.c                                    |   28 +
 src/util/util.h                                    |    3 +
 tools/virsh.c                                      |   10 +-
 16 files changed, 1553 insertions(+), 33 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..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
+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])
+    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 2fcd9a9..7c7e68e 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -862,8 +862,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..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 */
 };
@@ -206,6 +208,9 @@ int virNodeDeviceHasCap(const virNodeDeviceObjPtr dev, const char *cap);

 virNodeDeviceObjPtr virNodeDeviceFindByName(const virNodeDeviceObjListPtr devs,
                                             const char *name);
+virNodeDeviceObjPtr
+virNodeDeviceFindByUdevName(const virNodeDeviceObjListPtr devs,
+                            const char *udev_name);

 virNodeDeviceObjPtr virNodeDeviceAssignDef(virConnectPtr conn,
                                            virNodeDeviceObjListPtr devs,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0506867..8583213 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -341,6 +341,7 @@ virNodeDeviceHasCap;
 virNodeDeviceObjRemove;
 virNodeDevCapTypeToString;
 virNodeDeviceFindByName;
+virNodeDeviceFindByUdevName;
 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..531dac1
--- /dev/null
+++ b/src/node_device/node_device_udev.c
@@ -0,0 +1,1351 @@
+/*
+ * 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 "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;
+
+/* 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 '%s' on device '%s'"),
+                  property_key, udev_device_get_sysname(udev_device));
+        virReportOOMError(NULL);
+        ret = PROPERTY_ERROR;
+    }
+
+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 (virStrToLong_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 (virStrToLong_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 attr value.
+ * That memory must be later freed by some other code.   */
+static int udevGetStringSysfsAttr(struct udev_device *udev_device,
+                                  const char *attr_name,
+                                  char **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) {
+        *value = strdup(udev_value);
+        if (*value == NULL) {
+            VIR_ERROR(_("Failed to allocate memory for "
+                        "attribute '%s' on device '%s'"),
+                      attr_name, udev_device_get_sysname(udev_device));
+            virReportOOMError(NULL);
+            ret = PROPERTY_ERROR;
+        }
+    } else {
+        ret = PROPERTY_MISSING;
+    }
+
+    return ret;
+}
+
+
+static int udevGetIntSysfsAttr(struct udev_device *udev_device,
+                               const char *attr_name,
+                               int *value,
+                               int base)
+{
+    const char *udev_value = NULL;
+    int ret = PROPERTY_FOUND;
+
+    udev_value = udev_device_get_sysattr_value(udev_device, attr_name);
+
+    if (udev_value != NULL) {
+        if (virStrToLong_i(udev_value, NULL, base, value) != 0) {
+            ret = PROPERTY_ERROR;
+        }
+    } else {
+        ret = PROPERTY_MISSING;
+    }
+
+    return ret;
+}
+
+
+static int udevGetUintSysfsAttr(struct udev_device *udev_device,
+                                const char *attr_name,
+                                unsigned int *value,
+                                int base)
+{
+    const char *udev_value = NULL;
+    int ret = PROPERTY_FOUND;
+
+    udev_value = udev_device_get_sysattr_value(udev_device, attr_name);
+
+    if (udev_value != NULL) {
+        if (virStrToLong_ui(udev_value, NULL, base, value) != 0) {
+            ret = PROPERTY_ERROR;
+        }
+    } else {
+        ret = PROPERTY_MISSING;
+    }
+
+    return ret;
+}
+
+
+static int udevGetUint64SysfsAttr(struct udev_device *udev_device,
+                                  const char *attr_name,
+                                  uint64_t *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) {
+        if (virStrToLong_ull(udev_value, NULL, 0, value) != 0) {
+            ret = PROPERTY_ERROR;
+        }
+    } else {
+        ret = PROPERTY_MISSING;
+    }
+
+    return ret;
+}
+
+
+static int udevGenerateDeviceName(struct udev_device *device,
+                                  virNodeDeviceDefPtr def,
+                                  const char *s)
+{
+    int ret = 0;
+    virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+    if (s == NULL) {
+        s = udev_device_get_sysname(device);
+    }
+
+    virBufferVSprintf(&buf, "%s_%s", udev_device_get_subsystem(device), s);
+
+    if (virBufferError(&buf)) {
+        ret = -1;
+    }
+
+    def->name = virBufferContentAndReset(&buf);
+    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,
+                            0) == PROPERTY_ERROR) {
+        goto out;
+    }
+
+    char *p = strrchr(devpath, '/');
+
+    if ((p == NULL) || (virStrToLong_ui(p+1,
+                                        &p,
+                                        16,
+                                        &data->pci_dev.domain) == -1)) {
+        goto out;
+    }
+
+    if ((p == NULL) || (virStrToLong_ui(p+1,
+                                        &p,
+                                        16,
+                                        &data->pci_dev.bus) == -1)) {
+        goto out;
+    }
+
+    if ((p == NULL) || (virStrToLong_ui(p+1,
+                                        &p,
+                                        16,
+                                        &data->pci_dev.slot) == -1)) {
+        goto out;
+    }
+
+    if ((p == NULL) || (virStrToLong_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 (udevGetStringProperty(device,
+                              "ID_VENDOR",
+                              &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 (udevGetStringProperty(device,
+                              "ID_MODEL",
+                              &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 (udevGetStringProperty(device,
+                              "INTERFACE",
+                              &data->usb_if.interface_name) == PROPERTY_ERROR) {
+        goto out;
+    }
+
+    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->udev_name);
+
+    if (!STRPREFIX(filename, "host")) {
+        goto out;
+    }
+
+    if (virStrToLong_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 && foundtype == 1) {
+        ret = -1;
+        virReportOOMError(NULL);
+    }
+
+    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->udev_name);
+
+    if (virStrToLong_ui(filename, &p, 10, &data->scsi.host) == -1) {
+        goto out;
+    }
+
+    if ((p == NULL) || (virStrToLong_ui(p+1, &p, 10, &data->scsi.bus) == -1)) {
+        goto out;
+    }
+
+    if ((p == NULL) || (virStrToLong_ui(p+1,
+                                        &p,
+                                        10,
+                                        &data->scsi.target) == -1)) {
+        goto out;
+    }
+
+    if ((p == NULL) || (virStrToLong_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:
+    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) {
+        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;
+
+    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;
+
+    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;
+        }
+    }
+
+    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;
+        }
+    }
+
+    /* 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);
+    } else if (STREQ(def->caps->data.storage.drive_type, "disk")) {
+        ret = udevProcessDisk(device, def);
+    } else {
+        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, 0) == 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;
+    }
+
+    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:
+        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 = virNodeDeviceFindByName(&driverState->devs, name);
+    if (dev != NULL) {
+        virNodeDeviceObjRemove(&driverState->devs, dev);
+    } else {
+        ret = -1;
+    }
+
+    return ret;
+}
+
+
+static int udevSetParent(struct udev_device *device,
+                         virNodeDeviceDefPtr def)
+{
+    struct udev_device *parent_device = NULL;
+    const char *parent_udev_name = NULL;
+    virNodeDeviceObjPtr dev = NULL;
+    int ret = -1;
+
+    parent_device = udev_device_get_parent(device);
+    if (parent_device == NULL) {
+        goto out;
+    }
+
+    parent_udev_name = udev_device_get_syspath(parent_device);
+    if (parent_udev_name == NULL) {
+        goto out;
+    }
+
+    def->parent_udev_name = strdup(parent_udev_name);
+    if (def->parent_udev_name == NULL) {
+        virReportOOMError(NULL);
+        goto out;
+    }
+
+    dev = virNodeDeviceFindByUdevName(&driverState->devs, parent_udev_name);
+    if (dev == NULL) {
+        def->parent = strdup("computer");
+    } else {
+        def->parent = strdup(dev->def->name);
+        virNodeDeviceObjUnlock(dev);
+    }
+
+    if (def->parent == 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) {
+        goto out;
+    }
+
+    def->udev_name = 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) {
+        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) {
+        udevAddOneDevice(device);
+        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;
+
+    if (fd != udev_monitor_get_fd(udev_monitor)) {
+        goto out;
+    }
+
+    device = udev_monitor_receive_device(udev_monitor);
+    if (device == NULL) {
+        goto out;
+    }
+
+    action = udev_device_get_action(device);
+
+    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) {
+        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;
+    }
+    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);
+    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) {
+        ret = -1;
+        goto out;
+    }
+
+    if (virMutexInit(&driverState->lock) < 0) {
+        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) {
+        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_ERROR0("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..d950106
--- /dev/null
+++ b/src/node_device/node_device_udev.h
@@ -0,0 +1,32 @@
+/*
+ * 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 SYSTEM_DESCRIPTION "fictional device to root the node device tree"
+#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..44f68f5 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__ */
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);
             } else {
                 parents[i] = NULL;
             }
-- 
1.6.5.1

>From 1c18e7448a3ac9076183945fe2dae0536a88ba8b Mon Sep 17 00:00:00 2001
From: David Allan <dallan redhat com>
Date: Tue, 3 Nov 2009 14:52:43 -0500
Subject: [PATCH 3/6] 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 |   37 +++++++++++++++++++++++++++++++
 3 files changed, 84 insertions(+), 0 deletions(-)

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 4d1bfda..14b39fc 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")

@@ -396,6 +397,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);
@@ -663,6 +670,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,
@@ -1067,6 +1104,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;
@@ -1393,6 +1433,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 91ef94e..ba32b10 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
@@ -136,6 +137,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 531dac1..ed36da0 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -503,6 +503,35 @@ 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);
+    if (sysname == NULL) {
+        goto out;
+    }
+
+    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;
@@ -813,6 +842,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;
@@ -870,6 +904,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 78aa9e5d56bed4bfa9d784f0b618bc727c8af054 Mon Sep 17 00:00:00 2001
From: David Allan <dallan redhat com>
Date: Fri, 16 Oct 2009 14:36:07 -0400
Subject: [PATCH 4/6] Remove DevKit node device backend

---
 configure.in                         |   61 +-----
 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(+), 522 deletions(-)
 delete mode 100644 src/node_device/node_device_devkit.c

diff --git a/configure.in b/configure.in
index 4e5afef..5574053 100644
--- a/configure.in
+++ b/configure.in
@@ -1694,60 +1694,6 @@ AM_CONDITIONAL([HAVE_HAL], [test "x$with_hal" = "xyes"])
 AC_SUBST([HAL_CFLAGS])
 AC_SUBST([HAL_LIBS])

-DEVKIT_REQUIRED=0.0
-DEVKIT_CFLAGS=
-DEVKIT_LIBS=
-AC_ARG_WITH([devkit],
-  [  --with-devkit      use DeviceKit for host device enumeration],
-  [],
-  [with_devkit=no])
-
-if test "$with_libvirtd" = "no" ; then
-  with_devkit=no
-fi
-
-dnl Extra check needed while devkit pkg-config info missing glib2 dependency
-PKG_CHECK_MODULES(GLIB2, glib-2.0 >= 0.0,,[
-  if test "x$with_devkit" = "xcheck"; then
-    with_devkit=no
-  elif test "x$with_devkit" = "xyes"; then
-    AC_MSG_ERROR([required package DeviceKit requires package glib-2.0])
-  fi])
-
-if test "x$with_devkit" = "xyes" -o "x$with_devkit" = "xcheck"; then
-  PKG_CHECK_MODULES(DEVKIT, devkit-gobject >= $DEVKIT_REQUIRED,
-    [with_devkit=yes], [
-    if test "x$with_devkit" = "xcheck" ; then
-       with_devkit=no
-    else
-       AC_MSG_ERROR(
-         [You must install DeviceKit-devel >= $DEVKIT_REQUIRED to compile libvirt])
-    fi
-  ])
-  if test "x$with_devkit" = "xyes" ; then
-    AC_DEFINE_UNQUOTED([HAVE_DEVKIT], 1,
-      [use DeviceKit for host device enumeration])
-
-    dnl Add glib2 flags explicitly while devkit pkg-config info missing glib2 dependency
-    DEVKIT_CFLAGS="$GLIB2_CFLAGS $DEVKIT_CFLAGS"
-    DEVKIT_LIBS="$GLIB2_LIBS $DEVKIT_LIBS"
-
-    dnl Add more flags apparently required for devkit to work properly
-    DEVKIT_CFLAGS="$DEVKIT_CFLAGS -D_POSIX_PTHREAD_SEMANTICS -D_REENTRANT"
-
-    old_CFLAGS=$CFLAGS
-    old_LDFLAGS=$LDFLAGS
-    CFLAGS="$CFLAGS $DEVKIT_CFLAGS"
-    LDFLAGS="$LDFLAGS $DEVKIT_LIBS"
-    AC_CHECK_FUNCS([devkit_client_connect],,[with_devkit=no])
-    CFLAGS="$old_CFLAGS"
-    LDFLAGS="$old_LDFLAGS"
-  fi
-fi
-AM_CONDITIONAL([HAVE_DEVKIT], [test "x$with_devkit" = "xyes"])
-AC_SUBST([DEVKIT_CFLAGS])
-AC_SUBST([DEVKIT_LIBS])
-
 UDEV_REQUIRED=143
 UDEV_CFLAGS=
 UDEV_LIBS=
@@ -1787,7 +1733,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 +1893,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 5a985a5cdcd8446163b591c6186b2e2d932246e4 Mon Sep 17 00:00:00 2001
From: David Allan <dallan redhat com>
Date: Wed, 4 Nov 2009 03:05:30 -0500
Subject: [PATCH 5/6] Add translation of PCI vendor and product IDs

This patch uses libpciaccess to provide human readable names for PCI vendor and device IDs.
---
 configure.in                       |    9 +++++++
 src/Makefile.am                    |    4 +-
 src/node_device/node_device_udev.c |   45 ++++++++++++++++++++++++++++++++++-
 3 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/configure.in b/configure.in
index 5574053..0663600 100644
--- a/configure.in
+++ b/configure.in
@@ -1727,10 +1727,19 @@ if test "x$with_udev" = "xyes" -o "x$with_udev" = "xcheck"; then
     CFLAGS="$old_CFLAGS"
     LDFLAGS="$old_LDFLAGS"
   fi
+  PCIACCESS_REQUIRED=0.10.0
+  PCIACCESS_CFLAGS=
+  PCIACCESS_LIBS=
+  PKG_CHECK_MODULES([PCIACCESS], [pciaccess >= $PCIACCESS_REQUIRED], [], [PCIACCESS_FOUND=no])
+  if test "$PCIACCESS_FOUND" = "no" ; then
+     AC_MSG_ERROR([You must install libpciaccess/libpciaccess-devel >= $PCIACCESS_REQUIRED to compile libvirt])
+   fi
 fi
 AM_CONDITIONAL([HAVE_UDEV], [test "x$with_udev" = "xyes"])
 AC_SUBST([UDEV_CFLAGS])
 AC_SUBST([UDEV_LIBS])
+AC_SUBST([PCIACCESS_CFLAGS])
+AC_SUBST([PCIACCESS_LIBS])

 with_nodedev=no;
 if test "$with_hal" = "yes" -o "$with_udev" = "yes";
diff --git a/src/Makefile.am b/src/Makefile.am
index 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 ed36da0..c749187 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -22,6 +22,7 @@

 #include <config.h>
 #include <libudev.h>
+#include <pciaccess.h>
 #include <scsi/scsi.h>

 #include "node_device_udev.h"
@@ -251,6 +252,42 @@ static void udevLogFunction(struct udev *udev ATTRIBUTE_UNUSED,
 }


+static int udevTranslatePCIIds(unsigned int vendor,
+                               unsigned int product,
+                               char **vendor_string,
+                               char **product_string)
+{
+    int ret = 0;
+    struct pci_id_match m;
+
+    ret = pci_system_init();
+    if (ret != 0) {
+        goto out;
+    }
+
+    m.vendor_id = vendor;
+    m.device_id = product;
+    m.subvendor_id = PCI_MATCH_ANY;
+    m.subdevice_id = PCI_MATCH_ANY;
+    m.device_class = 0;
+    m.device_class_mask = 0;
+    m.match_data = 0;
+
+    /* pci_get_strings returns void */
+    pci_get_strings(&m,
+                    (const char **)product_string,
+                    (const char **)vendor_string,
+                    NULL,
+                    NULL);
+
+    /* pci_system_cleanup returns void */
+    pci_system_cleanup();
+
+out:
+    return ret;
+}
+
+
 static int udevProcessPCI(struct udev_device *device,
                           virNodeDeviceDefPtr def)
 {
@@ -309,8 +346,12 @@ static int udevProcessPCI(struct udev_device *device,
         goto out;
     }

-    /* XXX FIXME: to do the vendor name and product name, we have to
-     * parse /usr/share/hwdata/pci.ids.  Use libpciaccess perhaps? */
+    if (udevTranslatePCIIds(data->pci_dev.vendor,
+                            data->pci_dev.product,
+                            &data->pci_dev.vendor_name,
+                            &data->pci_dev.product_name) != 0) {
+        goto out;
+    }

     if (udevGenerateDeviceName(device, def, NULL) != 0) {
         goto out;
-- 
1.6.5.1

>From 284409336cb28245963596aeeb28cc6e57d2fede Mon Sep 17 00:00:00 2001
From: David Allan <dallan redhat com>
Date: Fri, 6 Nov 2009 16:21:57 -0500
Subject: [PATCH 6/6] 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
---
 src/conf/node_device_conf.c        |   24 ++--
 src/conf/node_device_conf.h        |    8 +-
 src/libvirt_private.syms           |    2 +-
 src/node_device/node_device_udev.c |  376 ++++++++++++++++++++++++++----------
 4 files changed, 290 insertions(+), 120 deletions(-)

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 14b39fc..36c5443 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -94,15 +94,15 @@ int virNodeDeviceHasCap(const virNodeDeviceObjPtr dev, const char *cap)


 virNodeDeviceObjPtr
-virNodeDeviceFindByUdevName(const virNodeDeviceObjListPtr devs,
-                            const char *udev_name)
+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->udev_name != NULL) &&
-            (STREQ(devs->objs[i]->def->udev_name, udev_name))) {
+        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]);
@@ -138,8 +138,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);
+    VIR_FREE(def->sysfs_path);
+    VIR_FREE(def->parent_sysfs_path);

     caps = def->caps;
     while (caps) {
@@ -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);
     }
     if (def->driver) {
         virBufferAddLit(&buf, "  <driver>\n");
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index ba32b10..5bee393 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -168,9 +168,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 *sysfs_path;                   /* udev name/sysfs path */
     char *parent;			/* optional parent device name */
-    char *parent_udev_name;             /* udev parent name/sysfs path */
+    char *parent_sysfs_path;            /* udev parent name/sysfs path */
     char *driver;                       /* optional driver name */
     virNodeDevCapsDefPtr caps;		/* optional device capabilities */
 };
@@ -213,8 +213,8 @@ int virNodeDeviceHasCap(const virNodeDeviceObjPtr dev, const char *cap);
 virNodeDeviceObjPtr virNodeDeviceFindByName(const virNodeDeviceObjListPtr devs,
                                             const char *name);
 virNodeDeviceObjPtr
-virNodeDeviceFindByUdevName(const virNodeDeviceObjListPtr devs,
-                            const char *udev_name);
+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 8583213..c473d49 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -341,7 +341,7 @@ virNodeDeviceHasCap;
 virNodeDeviceObjRemove;
 virNodeDevCapTypeToString;
 virNodeDeviceFindByName;
-virNodeDeviceFindByUdevName;
+virNodeDeviceFindBySysfsPath;
 virNodeDeviceObjListFree;
 virNodeDeviceDefFree;
 virNodeDevCapsDefFree;
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index c749187..7b78912 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -42,6 +42,59 @@

 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,
@@ -63,13 +116,19 @@ static int udevGetDeviceProperty(struct udev_device *udev_device,
      * of the function must also be changed. */
     *property_value = strdup(udev_value);
     if (*property_value == NULL) {
-        VIR_ERROR(_("Failed to allocate memory for "
-                    "property '%s' on device '%s'"),
+        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;
 }
@@ -94,7 +153,7 @@ static int udevGetIntProperty(struct udev_device *udev_device,
     ret = udevGetDeviceProperty(udev_device, property_key, &udev_value);

     if (ret == PROPERTY_FOUND) {
-        if (virStrToLong_i(udev_value, NULL, base, value) != 0) {
+        if (udevStrToLong_i(udev_value, NULL, base, value) != 0) {
             ret = PROPERTY_ERROR;
         }
     }
@@ -115,7 +174,7 @@ static int udevGetUintProperty(struct udev_device *udev_device,
     ret = udevGetDeviceProperty(udev_device, property_key, &udev_value);

     if (ret == PROPERTY_FOUND) {
-        if (virStrToLong_ui(udev_value, NULL, base, value) != 0) {
+        if (udevStrToLong_ui(udev_value, NULL, base, value) != 0) {
             ret = PROPERTY_ERROR;
         }
     }
@@ -125,52 +184,70 @@ static int udevGetUintProperty(struct udev_device *udev_device,
 }


-/* This function allocates memory from the heap for the attr value.
- * That memory must be later freed by some other code.   */
-static int udevGetStringSysfsAttr(struct udev_device *udev_device,
+/* 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 **value)
+                                  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) {
-        *value = strdup(udev_value);
-        if (*value == NULL) {
-            VIR_ERROR(_("Failed to allocate memory for "
-                        "attribute '%s' on device '%s'"),
-                      attr_name, udev_device_get_sysname(udev_device));
-            virReportOOMError(NULL);
-            ret = PROPERTY_ERROR;
-        }
-    } else {
+    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)
+{
+    return udevGetDeviceSysfsAttr(udev_device, attr_name, value);
+}
+
+
 static int udevGetIntSysfsAttr(struct udev_device *udev_device,
                                const char *attr_name,
                                int *value,
                                int base)
 {
-    const char *udev_value = NULL;
+    char *udev_value = NULL;
     int ret = PROPERTY_FOUND;

-    udev_value = udev_device_get_sysattr_value(udev_device, attr_name);
+    ret = udevGetDeviceSysfsAttr(udev_device, attr_name, &udev_value);

-    if (udev_value != NULL) {
-        if (virStrToLong_i(udev_value, NULL, base, value) != 0) {
+    if (ret == PROPERTY_FOUND) {
+        if (udevStrToLong_i(udev_value, NULL, base, value) != 0) {
             ret = PROPERTY_ERROR;
         }
-    } else {
-        ret = PROPERTY_MISSING;
     }

+    VIR_FREE(udev_value);
     return ret;
 }

@@ -180,40 +257,38 @@ static int udevGetUintSysfsAttr(struct udev_device *udev_device,
                                 unsigned int *value,
                                 int base)
 {
-    const char *udev_value = NULL;
+    char *udev_value = NULL;
     int ret = PROPERTY_FOUND;

-    udev_value = udev_device_get_sysattr_value(udev_device, attr_name);
+    ret = udevGetDeviceSysfsAttr(udev_device, attr_name, &udev_value);

-    if (udev_value != NULL) {
-        if (virStrToLong_ui(udev_value, NULL, base, value) != 0) {
+    if (ret == PROPERTY_FOUND) {
+        if (udevStrToLong_ui(udev_value, NULL, base, value) != 0) {
             ret = PROPERTY_ERROR;
         }
-    } else {
-        ret = PROPERTY_MISSING;
     }

+    VIR_FREE(udev_value);
     return ret;
 }


 static int udevGetUint64SysfsAttr(struct udev_device *udev_device,
                                   const char *attr_name,
-                                  uint64_t *value)
+                                  unsigned long long *value)
 {
-    const char *udev_value = NULL;
+    char *udev_value = NULL;
     int ret = PROPERTY_FOUND;

-    udev_value = udev_device_get_sysattr_value(udev_device, attr_name);
+    ret = udevGetDeviceSysfsAttr(udev_device, attr_name, &udev_value);

-    if (udev_value != NULL) {
-        if (virStrToLong_ull(udev_value, NULL, 0, value) != 0) {
+    if (ret == PROPERTY_FOUND) {
+        if (udevStrToLong_ull(udev_value, NULL, 0, value) != 0) {
             ret = PROPERTY_ERROR;
         }
-    } else {
-        ret = PROPERTY_MISSING;
     }

+    VIR_FREE(udev_value);
     return ret;
 }

@@ -232,6 +307,8 @@ static int udevGenerateDeviceName(struct udev_device *device,
     virBufferVSprintf(&buf, "%s_%s", udev_device_get_subsystem(device), 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;
     }

@@ -257,11 +334,12 @@ static int udevTranslatePCIIds(unsigned int vendor,
                                char **vendor_string,
                                char **product_string)
 {
-    int ret = 0;
+    int ret = -1;
     struct pci_id_match m;
+    const char *vendor_name = NULL, *device_name = NULL;

-    ret = pci_system_init();
-    if (ret != 0) {
+    if (pci_system_init() != 0) {
+        VIR_ERROR0("Failed to initialize libpciaccess\n");
         goto out;
     }

@@ -275,14 +353,32 @@ static int udevTranslatePCIIds(unsigned int vendor,

     /* pci_get_strings returns void */
     pci_get_strings(&m,
-                    (const char **)product_string,
-                    (const char **)vendor_string,
+                    &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;
 }
@@ -300,37 +396,37 @@ static int udevProcessPCI(struct udev_device *device,
     if (udevGetUintProperty(device,
                             "PCI_CLASS",
                             &data->pci_dev.class,
-                            0) == PROPERTY_ERROR) {
+                            16) == PROPERTY_ERROR) {
         goto out;
     }

     char *p = strrchr(devpath, '/');

-    if ((p == NULL) || (virStrToLong_ui(p+1,
-                                        &p,
-                                        16,
-                                        &data->pci_dev.domain) == -1)) {
+    if ((p == NULL) || (udevStrToLong_ui(p+1,
+                                         &p,
+                                         16,
+                                         &data->pci_dev.domain) == -1)) {
         goto out;
     }

-    if ((p == NULL) || (virStrToLong_ui(p+1,
-                                        &p,
-                                        16,
-                                        &data->pci_dev.bus) == -1)) {
+    if ((p == NULL) || (udevStrToLong_ui(p+1,
+                                         &p,
+                                         16,
+                                         &data->pci_dev.bus) == -1)) {
         goto out;
     }

-    if ((p == NULL) || (virStrToLong_ui(p+1,
-                                        &p,
-                                        16,
-                                        &data->pci_dev.slot) == -1)) {
+    if ((p == NULL) || (udevStrToLong_ui(p+1,
+                                         &p,
+                                         16,
+                                         &data->pci_dev.slot) == -1)) {
         goto out;
     }

-    if ((p == NULL) || (virStrToLong_ui(p+1,
-                                        &p,
-                                        16,
-                                        &data->pci_dev.function) == -1)) {
+    if ((p == NULL) || (udevStrToLong_ui(p+1,
+                                         &p,
+                                         16,
+                                         &data->pci_dev.function) == -1)) {
         goto out;
     }

@@ -341,8 +437,10 @@ static int udevProcessPCI(struct udev_device *device,
         goto out;
     }

-    if (udevGetUintSysfsAttr(device, "device", &data->pci_dev.product, 0)
-        == PROPERTY_ERROR) {
+    if (udevGetUintSysfsAttr(device,
+                             "device",
+                             &data->pci_dev.product,
+                             0) == PROPERTY_ERROR) {
         goto out;
     }

@@ -391,8 +489,8 @@ static int udevProcessUSBDevice(struct udev_device *device,
         goto out;
     }

-    if (udevGetStringProperty(device,
-                              "ID_VENDOR",
+    if (udevGetStringSysfsAttr(device,
+                              "manufacturer",
                               &data->usb_dev.vendor_name) == PROPERTY_ERROR) {
         goto out;
     }
@@ -404,8 +502,8 @@ static int udevProcessUSBDevice(struct udev_device *device,
         goto out;
     }

-    if (udevGetStringProperty(device,
-                              "ID_MODEL",
+    if (udevGetStringSysfsAttr(device,
+                              "product",
                               &data->usb_dev.product_name) == PROPERTY_ERROR) {
         goto out;
     }
@@ -517,16 +615,18 @@ static int udevProcessSCSIHost(struct udev_device *device ATTRIBUTE_UNUSED,
     union _virNodeDevCapData *data = &def->caps->data;
     char *filename = NULL;

-    filename = basename(def->udev_name);
+    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 (virStrToLong_ui(filename + strlen("host"),
-                        NULL,
-                        0,
-                        &data->scsi_host.host) == -1) {
+    if (udevStrToLong_ui(filename + strlen("host"),
+                         NULL,
+                         0,
+                         &data->scsi_host.host) == -1) {
         goto out;
     }

@@ -552,9 +652,6 @@ static int udevProcessSCSITarget(struct udev_device *device ATTRIBUTE_UNUSED,
     union _virNodeDevCapData *data = &def->caps->data;

     sysname = udev_device_get_sysname(device);
-    if (sysname == NULL) {
-        goto out;
-    }

     data->scsi_target.name = strdup(sysname);
     if (data->scsi_target.name == NULL) {
@@ -614,9 +711,13 @@ static int udevGetSCSIType(unsigned int type, char **typestring)
         break;
     }

-    if (*typestring == NULL && foundtype == 1) {
-        ret = -1;
-        virReportOOMError(NULL);
+    if (*typestring == NULL) {
+        if (foundtype == 1) {
+            ret = -1;
+            virReportOOMError(NULL);
+        } else {
+            VIR_ERROR("Failed to find SCSI device type %d\n", type);
+        }
     }

     return ret;
@@ -631,27 +732,30 @@ static int udevProcessSCSIDevice(struct udev_device *device ATTRIBUTE_UNUSED,
     union _virNodeDevCapData *data = &def->caps->data;
     char *filename = NULL, *p = NULL;

-    filename = basename(def->udev_name);
+    filename = basename(def->sysfs_path);

-    if (virStrToLong_ui(filename, &p, 10, &data->scsi.host) == -1) {
+    if (udevStrToLong_ui(filename, &p, 10, &data->scsi.host) == -1) {
         goto out;
     }

-    if ((p == NULL) || (virStrToLong_ui(p+1, &p, 10, &data->scsi.bus) == -1)) {
+    if ((p == NULL) || (udevStrToLong_ui(p+1,
+                                         &p,
+                                         10,
+                                         &data->scsi.bus) == -1)) {
         goto out;
     }

-    if ((p == NULL) || (virStrToLong_ui(p+1,
-                                        &p,
-                                        10,
-                                        &data->scsi.target) == -1)) {
+    if ((p == NULL) || (udevStrToLong_ui(p+1,
+                                         &p,
+                                         10,
+                                         &data->scsi.target) == -1)) {
         goto out;
     }

-    if ((p == NULL) || (virStrToLong_ui(p+1,
-                                        &p,
-                                        10,
-                                        &data->scsi.lun) == -1)) {
+    if ((p == NULL) || (udevStrToLong_ui(p+1,
+                                         &p,
+                                         10,
+                                         &data->scsi.lun) == -1)) {
         goto out;
     }

@@ -676,6 +780,10 @@ static int udevProcessSCSIDevice(struct udev_device *device ATTRIBUTE_UNUSED,
     ret = 0;

 out:
+    if (ret != 0) {
+        VIR_ERROR("Failed to process SCSI device with sysfs path '%s'\n",
+                  def->sysfs_path);
+    }
     return ret;
 }

@@ -688,6 +796,7 @@ static int udevProcessDisk(struct udev_device *device,

     data->storage.drive_type = strdup("disk");
     if (data->storage.drive_type == NULL) {
+        virReportOOMError(NULL);
         ret = -1;
         goto out;
     }
@@ -705,7 +814,6 @@ static int udevProcessDisk(struct udev_device *device,
         goto out;
     }

-
     data->storage.size = data->storage.num_blocks *
         data->storage.logical_block_size;

@@ -720,6 +828,17 @@ static int udevProcessCDROM(struct udev_device *device,
     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;
@@ -765,6 +884,10 @@ 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");
@@ -773,6 +896,16 @@ static int udevKludgeStorageType(virNodeDeviceDefPtr def)
         }
     }

+    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;
 }

@@ -840,12 +973,13 @@ static int udevProcessStorage(struct udev_device *device,
         }
     }

-    /* 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);
     } 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;
     }

@@ -898,7 +1032,7 @@ static int udevGetDeviceType(struct udev_device *device,
         goto out;
     }

-    if (udevGetUintProperty(device, "PCI_CLASS", &tmp, 0) == PROPERTY_FOUND) {
+    if (udevGetUintProperty(device, "PCI_CLASS", &tmp, 16) == PROPERTY_FOUND) {
         *type = VIR_NODE_DEV_CAP_PCI_DEV;
         goto out;
     }
@@ -914,6 +1048,9 @@ static int udevGetDeviceType(struct udev_device *device,
         goto out;
     }

+    VIR_INFO("Could not determine device type for device "
+             "with sysfs path '%s'\n",
+             udev_device_get_sysname(device));
     ret = -1;

 out:
@@ -955,6 +1092,7 @@ static int udevGetDeviceDetails(struct udev_device *device,
         ret = udevProcessStorage(device, def);
         break;
     default:
+        VIR_ERROR("Unknown device type %d\n", def->caps->type);
         ret = -1;
         break;
     }
@@ -970,11 +1108,14 @@ static int udevRemoveOneDevice(struct udev_device *device)
     int ret = 0;

     name = udev_device_get_syspath(device);
-
-    dev = virNodeDeviceFindByName(&driverState->devs, name);
+    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;
     }

@@ -986,27 +1127,31 @@ static int udevSetParent(struct udev_device *device,
                          virNodeDeviceDefPtr def)
 {
     struct udev_device *parent_device = NULL;
-    const char *parent_udev_name = 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_udev_name = udev_device_get_syspath(parent_device);
-    if (parent_udev_name == NULL) {
+    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_udev_name = strdup(parent_udev_name);
-    if (def->parent_udev_name == NULL) {
+    def->parent_sysfs_path = strdup(parent_sysfs_path);
+    if (def->parent_sysfs_path == NULL) {
         virReportOOMError(NULL);
         goto out;
     }

-    dev = virNodeDeviceFindByUdevName(&driverState->devs, parent_udev_name);
+    dev = virNodeDeviceFindBySysfsPath(&driverState->devs, parent_sysfs_path);
     if (dev == NULL) {
         def->parent = strdup("computer");
     } else {
@@ -1015,6 +1160,7 @@ static int udevSetParent(struct udev_device *device,
     }

     if (def->parent == NULL) {
+        virReportOOMError(NULL);
         goto out;
     }

@@ -1032,10 +1178,11 @@ static int udevAddOneDevice(struct udev_device *device)
     int ret = -1;

     if (VIR_ALLOC(def) != 0) {
+        virReportOOMError(NULL);
         goto out;
     }

-    def->udev_name = strdup(udev_device_get_syspath(device));
+    def->sysfs_path = strdup(udev_device_get_syspath(device));
     if (udevGetStringProperty(device,
                               "DRIVER",
                               &def->driver) == PROPERTY_ERROR) {
@@ -1061,6 +1208,7 @@ static int udevAddOneDevice(struct udev_device *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;
     }
@@ -1092,7 +1240,10 @@ static int udevProcessDeviceListEntry(struct udev *udev,

     device = udev_device_new_from_syspath(udev, name);
     if (device != NULL) {
-        udevAddOneDevice(device);
+        if (udevAddOneDevice(device) != 0) {
+            VIR_INFO("Failed to create node device for udev device '%s'\n",
+                     name);
+        }
         udev_device_unref(device);
         ret = 0;
     }
@@ -1171,17 +1322,23 @@ static void udevEventHandleCallback(int watch 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;

-    if (fd != udev_monitor_get_fd(udev_monitor)) {
+    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);
@@ -1209,21 +1366,25 @@ static int udevSetupSystemDev(void)
     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;
     }

@@ -1286,6 +1447,12 @@ static int udevSetupSystemDev(void)
     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;
@@ -1301,11 +1468,13 @@ static int udevDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED)
     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;
@@ -1324,6 +1493,7 @@ static int udevDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED)

     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;
     }
@@ -1418,7 +1588,7 @@ static virStateDriver udevStateDriver = {

 int udevNodeRegister(void)
 {
-    VIR_ERROR0("Registering udev node device backend\n");
+    VIR_DEBUG0("Registering udev node device backend\n");

     registerCommonNodeFuncs(&udevDeviceMonitor);
     if (virRegisterDeviceMonitor(&udevDeviceMonitor) < 0) {
-- 
1.6.5.1


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