[libvirt] udev node device backend
Chris Lalancette
clalance at redhat.com
Wed Oct 28 11:16:40 UTC 2009
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.
I haven't reviewed the code yet, but I did get this built and running on one of
my machines here. I ran a script which collected the output of "virsh
nodedev-list" and "virsh nodedev-dumpxml <device>" against both the HAL backend
and the udev backend (the output from both is attached). Then I did a
comparison. Overall, it looks like you did really good work. There are some
discrepancies, though, and a few random notes below.
1) I did this on an F-11 x86_64 box, which has libudev 141 installed. In order
for it to build, I had to change configure.in to allow 141, and I also had to
add #define LIBUDEV_I_KNOW_THE_API_IS_SUBJECT_TO_CHANGE. Neither of these is an
issue for F-12, but it might be something to consider adding to the ./configure
checks so we can build it on slightly older setups.
2) There are a few build-time warnings that you'll want to clean up:
CC libvirt_driver_nodedev_la-node_device_udev.lo
node_device/node_device_udev.c: In function 'udevGetUint64SysfsAttr':
node_device/node_device_udev.c:209: warning: 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:472: warning: passing argument 3 of
'udevGetUint64SysfsAttr' from incompatible pointer type
node_device/node_device_udev.c:199: note: expected 'uint64_t *' but argument is
of type 'long long unsigned int *'
node_device/node_device_udev.c:478: warning: passing argument 3 of
'udevGetUint64SysfsAttr' from incompatible pointer type
node_device/node_device_udev.c:199: 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:511: warning: passing argument 3 of
'udevGetUint64SysfsAttr' from incompatible pointer type
node_device/node_device_udev.c:199: note: expected 'uint64_t *' but argument is
of type 'long long unsigned int *'
node_device/node_device_udev.c:517: warning: passing argument 3 of
'udevGetUint64SysfsAttr' from incompatible pointer type
node_device/node_device_udev.c:199: note: expected 'uint64_t *' but argument is
of type 'long long unsigned int *'
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.
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.
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.
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.
There are probably a few others, please take a look through my hal-output and
udev-output. Like I said, very good work overall, this is just nit-picking and
cleanups that will make it more like the HAL backend.
--
Chris Lalancette
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: hal-output
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20091028/0c981714/attachment-0002.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: udev-output
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20091028/0c981714/attachment-0003.ksh>
More information about the libvir-list
mailing list