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
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
