7:28 a.m.
On Fri, Nov 14, 2008 at 12:46:09PM +0000, Mark McLoughlin wrote:
On Thu, 2008-11-13 at 17:30 +0000, Daniel P. Berrange wrote:
> This patch is the public API parts of the node device enumeration code.
> No changes since David's last submission of this, except for some
> Makefile tweaks
>
> +
> +int virNodeNumOfDevices (virConnectPtr conn,
> + unsigned int flags);
> +
> +int virNodeListDevices (virConnectPtr conn,
> + char **const names,
> + int maxnames,
> + unsigned int flags);
> +
> +int virNodeNumOfDevicesByCap (virConnectPtr conn,
> + const char *cap,
> + unsigned int flags);
> +
> +int virNodeListDevicesByCap (virConnectPtr conn,
> + const char *cap,
> + char **const names,
> + int maxnames,
> + unsigned int flags);
How about combining these two sets of functions and if the capability
type isn't supplied list all devices?
Yes, we could just remove the ByCap APIs, and add the 'const char *cap'
arg to the first two APIs, allowing NULL.
> +
> +virNodeDevicePtr virNodeDeviceLookupByName (virConnectPtr conn,
> + const char *name);
> +
> +const char * virNodeDeviceGetName (virNodeDevicePtr dev);
> +
How stable are these names? e.g. should we say that no-one should rely
on the format of the name and that the name of a given device could
change across node reboots? Even if HAL guarantees the name to be stable
(does it?), if you switch between HAL and DevKit it could change, right?
I don't think HAL explicitly guarentees it, it merely happens to have
been stable AFAICT. The naming is definitely completely different
between HAL and DevKit.
This is probably my biggest worry with the impl so far - some app
using it will need to have a stable identifier for a device and we
won't be providing it.
We could invent our own stable naming scheme for devices - the scheme
would vary per capability - eg for PCI devices we can use the bus,
function, slot identifiers. USB is hard to guarentee though - if a
device is plugged in & unpluged & plugged in again it won't get the
same address, and there's no real other identifier we can rely on
for this.
> +const char * virNodeDeviceGetParent (virNodeDevicePtr dev);
A GetParent() but no ListChildren() seems a little strange ...
Some of the hierarchy seems a little strange to me, e.g.
# ./virsh node-list-devices --cap net
...
net_00_13_20_f7_4a_06
# ./virsh node-device-dumpxml net_00_13_20_f7_4a_06
<device>
<name>net_00_13_20_f7_4a_06</name>
<parent>pci_8086_10de</parent>
<capability type='net'>
<interface>eth0</interface>
<address>00:13:20:f7:4a:06</address>
<capability type='80203'>
<mac_address>001320f74a06</address>
</capability>
</capability>
</device>
# ./virsh node-device-dumpxml pci_8086_10de
<device>
<name>pci_8086_10de</name>
<parent>computer</parent>
<capability type='pci'>
<domain>0</domain>
<bus>0</bus>
<slot>25</slot>
<function>0</function>
<product id='4318'>82567LM-3 Gigabit Network
Connection</product>
<vendor id='32902'>Intel Corporation</vendor>
</capability>
</device>
I see that all this naturally flows from the way hal does things, but
the pci device isn't a parent of the network device in any real
sense ... I guess I would expect to just see one device with both the
'net' and 'pci' capabilities.
So that's basically removing all explicit tracking of 'logical' devices
(NICs, disk, etc) and only ever representing 'physical' devices (PCI,
USB devices). The problem I can see is that one physical device can
result in many logical devices - even multiple instances of the same
capability - particularly multi-function USB devices can result in a
large number of sub-devices for audio, video, etc.
Or a SCSI host adapter can have a single PCI device, but result in
multiple host adapters in the OS view.
Separating the physical from logical devices gives us the opportunity
to define more stable names for devices with certain capabilities.
eg, for a USB network card, its hard to invent a stable name at the
level of the USB device, but for the logical NIC you can easily invent
a name based off the MAC address.
> +int virNodeDeviceNumOfCaps
(virNodeDevicePtr dev);
> +
> +int virNodeDeviceListCaps (virNodeDevicePtr dev,
> + char **const names,
> + int maxnames);
I think it would make more sense to me if there was a fixed set of
capability types and for them to be an enum in the API rather than
strings ...
> +char * virNodeDeviceGetXMLDesc (virNodeDevicePtr dev,
> + unsigned int flags);
> +
> +virNodeDevicePtr virNodeDeviceCreate (virConnectPtr conn,
> + const char *xml,
> + unsigned int flags);
Very strange, and we don't implement it? What's the idea with this?
Perhaps leave it out until we do implement it?
This is for SCSI FiberChannel adapters with NPIV support. They let
you create many virtual HBAs on the fly, which appear in /sys/class/scsi_host
on Linux alongside so called 'real' HBAs. You're right though, we can simply
leave this out until we actually implement NPIV support.
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 :|