[libvirt] PATCH: 7/12: Public API for node devices

[David Lively]

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