[libvirt] [PATCH 0/1] Improve the nodedev tree relationships

The parent/child relationships in the udev node device backend are really kind of gross. If the parent of a device doesn't exist in the libvirt node device tree, the current code sets the parent to the root "computer" node. The result is that we have a lot of devices hanging directly off the root node and relationships like scsi target to scsi hba are broken. The attached patch simply changes the behavior to keep walking up the udev device tree until it finds an ancestor that exists in the libvirt node device tree, or until it actually reaches the root of the tree. The resulting tree is a lot cleaner and more informative, especially on systems with lots of devices. David Allan (1): Improve nodedev parent/child relationships src/node_device/node_device_udev.c | 54 ++++++++++++++++++++++-------------- 1 files changed, 33 insertions(+), 21 deletions(-)

* If a nodedev has a parent that we don't want to display, we should continue walking up the udev device tree to see if any of its earlier ancestors are devices that we display. It makes the tree much nicer looking than having a whole lot of devices hanging off the root node. --- src/node_device/node_device_udev.c | 54 ++++++++++++++++++++++-------------- 1 files changed, 33 insertions(+), 21 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index c437861..6e3ecd7 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1223,31 +1223,43 @@ static int udevSetParent(struct udev_device *device, 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'", - udev_device_get_syspath(device)); - } + parent_device = device; + do { - parent_sysfs_path = udev_device_get_syspath(parent_device); - if (parent_sysfs_path == NULL) { - VIR_INFO("Could not get syspath for parent of '%s'", - udev_device_get_syspath(device)); - parent_sysfs_path = ""; - } + parent_device = udev_device_get_parent(parent_device); + if (parent_device == NULL) { + break; + } - def->parent_sysfs_path = strdup(parent_sysfs_path); - if (def->parent_sysfs_path == NULL) { - virReportOOMError(); - goto out; - } + parent_sysfs_path = udev_device_get_syspath(parent_device); + if (parent_sysfs_path == NULL) { + VIR_INFO("Could not get syspath for parent of '%s'", + udev_device_get_syspath(parent_device)); + } - dev = virNodeDeviceFindBySysfsPath(&driverState->devs, parent_sysfs_path); - if (dev == NULL) { + dev = virNodeDeviceFindBySysfsPath(&driverState->devs, + parent_sysfs_path); + if (dev != NULL) { + def->parent = strdup(dev->def->name); + virNodeDeviceObjUnlock(dev); + + if (def->parent == NULL) { + virReportOOMError(); + goto out; + } + + def->parent_sysfs_path = strdup(parent_sysfs_path); + if (def->parent_sysfs_path == NULL) { + virReportOOMError(); + goto out; + } + + } + + } while (def->parent == NULL && parent_device != NULL); + + if (def->parent == NULL) { def->parent = strdup("computer"); - } else { - def->parent = strdup(dev->def->name); - virNodeDeviceObjUnlock(dev); } if (def->parent == NULL) { -- 1.7.0.1

On 05/27/2010 04:07 PM, David Allan wrote:
* If a nodedev has a parent that we don't want to display, we should continue walking up the udev device tree to see if any of its earlier ancestors are devices that we display. It makes the tree much nicer looking than having a whole lot of devices hanging off the root node.
ACK. (A diff with -b to ignore whitespace changes also aids in the evaluation of this patch). -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On Thu, May 27, 2010 at 06:07:54PM -0400, David Allan wrote:
* If a nodedev has a parent that we don't want to display, we should continue walking up the udev device tree to see if any of its earlier ancestors are devices that we display. It makes the tree much nicer looking than having a whole lot of devices hanging off the root node.
ACK this fixes the scsi host vs target relationship in the trees on my machine Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Fri, May 28, 2010 at 10:39:23AM +0100, Daniel P. Berrange wrote:
On Thu, May 27, 2010 at 06:07:54PM -0400, David Allan wrote:
* If a nodedev has a parent that we don't want to display, we should continue walking up the udev device tree to see if any of its earlier ancestors are devices that we display. It makes the tree much nicer looking than having a whole lot of devices hanging off the root node.
ACK this fixes the scsi host vs target relationship in the trees on my machine
Thanks, pushed. Dave
participants (4)
-
Daniel P. Berrange
-
Dave Allan
-
David Allan
-
Eric Blake