On Fri, 2008-11-14 at 13:28 +0000, Daniel P. Berrange wrote:
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.
I like this idea as well.
>
> > +
> > +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.
Yep. But I think HAL is already trying to specify "stable names"
wherever possible. E.g., PCI devs aren't named by position on bus, but
by vendor-id/product-id, so that a PCI impl supporting hotplug will give
the same name for the card if it's taken out and reinserted (well, more
or less ...).
Perhaps we could just identify where we think HAL gets it wrong, and
implement our own naming scheme for those devices (assuming we can come
up with one we like better), and using the HAL names for the rest??
> 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.
Agreed. I think this extra level of heirarchy is useful, though I
originally found it confusing.
> > +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 ...
I have no objection to this.
Dave