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

Re: [libvirt] udev node device backend



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.

> 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.

> 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.


> 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.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


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