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

Re: [libvirt] udev node device backend



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


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