[libvirt] udev node device backend

[Dave Allan]

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)
> 
>> 4)  I also took a look at the output for one of the bridges.  In my HAL backend,
>> I see:
>>
>> <device>
>>   <name>net_00_13_20_f5_fa_e3_0</name>
>>   <parent>computer</parent>
>>   <capability type='net'>
>>     <interface>ovirtbr0</interface>
>>     <address>00:13:20:f5:fa:e3</address>
>>   </capability>
>> </device>
>>
>> However, in the udev backend I am missing the parent link (in point of fact, the
>> parent link is missing for quite a few elements), and I also have an additional
>> "<capability type='80203'/>" element:
>>
>> <device>
>>   <name>/sys/devices/virtual/net/ovirtbr0</name>
>>   <capability type='net'>
>>     <interface>ovirtbr0</interface>
>>     <address>00:13:20:f5:fa:e3</address>
>>     <capability type='80203'/>
>>   </capability>
>> </device>
>>
>> I'm not sure if either of those is a problem.
> 
> The missing parent link is a problem - if all else fails, it should at
> least point back to the top level 'computer' device.  In this case you
> are correct that  <capability type='80203'/> is bogus, since this is
> a bridge device, not an physical ethernet device.

Udev has an odd characteristic which is that it often sets the parent 
name to a string that is not the name of a device that it recognizes, 
i.e., you have a parent name, but looking it up returns nothing.  I have 
been setting the parent pointer to the string provided by udev, and then 
setting the parent string to computer when I do things like dump the 
tree if the parent device doesn't actually exist.  As I think about it, 
and the related device naming point that you raise below, the more I 
think parent should be a libvirt concept, set as you describe: if the 
parent device is NULL or is not a device known to libvirt, set the 
parent to computer.  I'll create a separate attribute for the udev 
parent string.

>> 5)  We are still missing the mapping of product/vendor id --> names.  This shows
>> up for instance in the parent of the eth0 device, where the HAL backend shows:
>>
>>  <product id='0x10bd'>82566DM-2 Gigabit Network Connection</product>
>>
>> and the udev backend shows nothing.  Probably not a show-stopper, but a
>> nice-to-have for human readers.
> 
> Yeah, we shouldn't hold this up just for missing human names - unique IDs
> are the important thing. Even HAL could miss the names if the PCI IDs database
> was not up2date.

Agreed.  I looked at the ids file and didn't relish the idea of parsing 
it myself, and the vendor names are not available through udev.  Is 
libpciaccess a viable option?  It has functions that will do what we 
want, but I haven't worked with it, so I don't know if it is suitable 
for use in libvirt.

>> 6)  SCSI device 1:0:0:0 (pci_8086_2920_scsi_host_0_scsi_device_lun0 in the HAL
>> backend, /sys/devices/pci0000:00/0000:00:1f.2/host1/target1:0:0/1:0:0:0 in the
>> udev backend) shows up with "<type>cdrom</type>" in HAL, but not in udev.
> 
> The type=cdrom thing is a nice to have but not a show stopper. If we
> commit without it, then just open a BZ so we remember to fix it some
> time.

Hmm.  I'll look into that.

>> computer
>> net_00_13_20_f5_fa_e3
>> net_00_13_20_f5_fa_e3_0
>> net_1e_85_27_2d_a9_b0
>> net_computer_loopback
>> pci_8086_10bd
>> pci_8086_244e
>> pci_8086_2910
>> pci_8086_2920
>> pci_8086_2920_scsi_host
>> pci_8086_2920_scsi_host_0
>> pci_8086_2920_scsi_host_0_scsi_device_lun0
>> pci_8086_2920_scsi_host_0_scsi_host
>> pci_8086_2920_scsi_host_scsi_device_lun0
>> pci_8086_2920_scsi_host_scsi_host
>> pci_8086_2926
>> pci_8086_2926_scsi_host
>> pci_8086_2926_scsi_host_0
>> pci_8086_2930
>> pci_8086_2934
>> pci_8086_2935
>> pci_8086_2936
>> pci_8086_2937
>> pci_8086_2938
>> pci_8086_2939
>> pci_8086_293a
>> pci_8086_293c
>> pci_8086_293e
>> pci_8086_29c0
>> pci_8086_29c2
>> pci_8086_29c4
>> pci_8086_29c6
>> pci_8086_29c7
>> platform_floppy_0_storage_platform_floppy
>> storage_model_DVDRW_LH_20A1S
>> storage_serial_SATA_ST3320620AS_9QF5E3AP
>> usb_device_1d6b_1_0000_00_1a_0
>> usb_device_1d6b_1_0000_00_1a_0_if0
>> usb_device_1d6b_1_0000_00_1a_1
>> usb_device_1d6b_1_0000_00_1a_1_if0
>> usb_device_1d6b_1_0000_00_1a_2
>> usb_device_1d6b_1_0000_00_1a_2_if0
>> usb_device_1d6b_1_0000_00_1d_0
>> usb_device_1d6b_1_0000_00_1d_0_if0
>> usb_device_1d6b_1_0000_00_1d_1
>> usb_device_1d6b_1_0000_00_1d_1_if0
>> usb_device_1d6b_1_0000_00_1d_2
>> usb_device_1d6b_1_0000_00_1d_2_if0
>> usb_device_1d6b_2_0000_00_1a_7
>> usb_device_1d6b_2_0000_00_1a_7_if0
>> usb_device_1d6b_2_0000_00_1d_7
>> usb_device_1d6b_2_0000_00_1d_7_if0
> 
> 
> [snip]
> 
>> /sys/devices/pci0000:00/0000:00:00.0
>> /sys/devices/pci0000:00/0000:00:02.0
>> /sys/devices/pci0000:00/0000:00:03.0
>> /sys/devices/pci0000:00/0000:00:03.2
>> /sys/devices/pci0000:00/0000:00:03.3
>> /sys/devices/pci0000:00/0000:00:19.0
>> /sys/devices/pci0000:00/0000:00:19.0/net/eth0
>> /sys/devices/pci0000:00/0000:00:1a.0/usb3
>> /sys/devices/pci0000:00/0000:00:1a.0/usb3/3-0:1.0
>> /sys/devices/pci0000:00/0000:00:1a.1/usb4
>> /sys/devices/pci0000:00/0000:00:1a.1/usb4/4-0:1.0
>> /sys/devices/pci0000:00/0000:00:1a.2/usb5
>> /sys/devices/pci0000:00/0000:00:1a.2/usb5/5-0:1.0
>> /sys/devices/pci0000:00/0000:00:1a.7/usb1
>> /sys/devices/pci0000:00/0000:00:1a.7/usb1/1-0:1.0
>> /sys/devices/pci0000:00/0000:00:1b.0
>> /sys/devices/pci0000:00/0000:00:1d.0/usb6
>> /sys/devices/pci0000:00/0000:00:1d.0/usb6/6-0:1.0
>> /sys/devices/pci0000:00/0000:00:1d.1/usb7
>> /sys/devices/pci0000:00/0000:00:1d.1/usb7/7-0:1.0
>> /sys/devices/pci0000:00/0000:00:1d.2/usb8
>> /sys/devices/pci0000:00/0000:00:1d.2/usb8/8-0:1.0
>> /sys/devices/pci0000:00/0000:00:1d.7/usb2
>> /sys/devices/pci0000:00/0000:00:1d.7/usb2/2-0:1.0
>> /sys/devices/pci0000:00/0000:00:1e.0
>> /sys/devices/pci0000:00/0000:00:1f.0
>> /sys/devices/pci0000:00/0000:00:1f.2/host0
>> /sys/devices/pci0000:00/0000:00:1f.2/host0/target0:0:0/0:0:0:0
>> /sys/devices/pci0000:00/0000:00:1f.2/host0/target0:0:0/0:0:0:0/block/sda
>> /sys/devices/pci0000:00/0000:00:1f.2/host1
>> /sys/devices/pci0000:00/0000:00:1f.2/host1/target1:0:0/1:0:0:0
>> /sys/devices/pci0000:00/0000:00:1f.2/host1/target1:0:0/1:0:0:0/block/sr0
>> /sys/devices/pci0000:00/0000:00:1f.5
>> /sys/devices/pci0000:00/0000:00:1f.5/host2
>> /sys/devices/pci0000:00/0000:00:1f.5/host3
>> /sys/devices/virtual/net/lo
>> /sys/devices/virtual/net/ovirtbr0
>> /sys/devices/virtual/net/virbr0
>> computer
> 
> I don't like that we're using the sysfs paths for the name here.
> I'd like to see non-path based names for devices. Either by
> following the HAL naming, or defining our own naming scheme.
> 
> Mostly because if we use paths, then people will come to expect
> that this field is a path and if we then change it to not be a path
> apps will get confused/broken. If we did want to include a sysfs
> path or equivalent, then we ought to have an explicit named XML
> element for that.

That's a fair point.  I don't like the paths-as-names, either; it's the 
udev default, which is why I used it.  As much as HAL *should* be 
relatively static at this point, it's not guaranteed to be, so I think 
trying to follow its naming convention will be a maintenance problem. 
Defining our own naming scheme is a little work, definitely less work 
than trying to figure out HAL's convention, and not tricky.  I'll do so 
& add an element for the sysfs path.

Dave