[libvirt] udev node device backend
Dave Allan
dallan at redhat.com
Wed Nov 11 22:06:18 UTC 2009
Dave Allan wrote:
> 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
I noticed when starting to work on some other stuff that I ought to have
named a struct member sysfs_path rather than udev_name. Here's an
updated patch set that makes that change. I've just rolled it into the
previous patch.
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/20091111/1ba21660/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/20091111/1ba21660/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/20091111/1ba21660/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/20091111/1ba21660/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/20091111/1ba21660/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/20091111/1ba21660/attachment-0011.ksh>
More information about the libvir-list
mailing list