[libvirt] udev node device backend
Dave Allan
dallan at redhat.com
Wed Nov 11 03:36:55 UTC 2009
Cole Robinson wrote:
> 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
Hi Cole,
Here's a revised patch set that fixes everything you found. Thanks for
all the feedback. You should only have to apply the last one to your tree.
Dave
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0001-Add-several-fields-to-node-device-capabilities.patch
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20091110/4d026617/attachment-0006.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0002-Implement-a-node-device-backend-using-libudev.patch
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20091110/4d026617/attachment-0007.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0003-Add-scsi_target-device-type.patch
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20091110/4d026617/attachment-0008.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0004-Remove-DevKit-node-device-backend.patch
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20091110/4d026617/attachment-0009.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0005-Add-translation-of-PCI-vendor-and-product-IDs.patch
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20091110/4d026617/attachment-0010.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0006-Fixes-per-feedback-from-Cole.patch
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20091110/4d026617/attachment-0011.ksh>
More information about the libvir-list
mailing list