Re: [libvirt] PATCH: 7/12: Public API for node devices
On Fri, Nov 14, 2008 at 01:57:26PM -0500, David Lively wrote: On Fri, 2008-11-14 at 13:28 +, Daniel P. Berrange wrote: On Fri, Nov 14, 2008 at 12:46:09PM +, Mark McLoughlin wrote: On Thu, 2008-11-13 at 17:30 +, 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. Okay, then let's do this ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ [EMAIL PROTECTED] | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 7/12: Public API for node devices
On Fri, 2008-11-14 at 13:28 +, Daniel P. Berrange wrote: On Fri, Nov 14, 2008 at 12:46:09PM +, Mark McLoughlin wrote: On Thu, 2008-11-13 at 17:30 +, Daniel P. Berrange wrote: + +virNodeDevicePtrvirNodeDeviceLookupByName (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. I don't have a good understanding of what this API will be used for, aside from device passthrough. I wouldn't be surprised if we decide that it makes sense to allow using higher level names for passthrough, rather than e.g. PCI IDs - i.e. passthrough device with mac address X rather than vendor:device or bus:dev.func. In that case, you need the names to be stable, but you don't necessarily need them to have a predictable enough structure that people can construct a name themselves. 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. I think the key thing in the short term is that we make it clear that device names are completely unstable identifiers with no structure or meaning that can be relied upon. I'd almost be tempted to append a few bytes of randomness to the names for now ... 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. Another way would be to have multiple names for each device, by aggregating the PCI device and network device capabilities into a single object. The only really advantage to that would be that given a less specialized name for a device, you could easily iterate over the more specialized names. But if we need that we could add virNodeDeviceListChildren() I guess. IOW, the current scheme will probably work out just fine ... Cheers, Mark. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 7/12: Public API for node devices
On Thu, Nov 13, 2008 at 05:30:29PM +, 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 okidoc, reviewed many times already, +1 Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ [EMAIL PROTECTED] | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 7/12: Public API for node devices
On Fri, Nov 14, 2008 at 12:46:09PM +, Mark McLoughlin wrote: On Thu, 2008-11-13 at 17:30 +, 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. + +virNodeDevicePtrvirNodeDeviceLookupByName (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 namenet_00_13_20_f7_4a_06/name parentpci_8086_10de/parent capability type='net' interfaceeth0/interface address00:13:20:f7:4a:06/address capability type='80203' mac_address001320f74a06/address /capability /capability /device # ./virsh node-device-dumpxml pci_8086_10de device namepci_8086_10de/name parentcomputer/parent capability type='pci' domain0/domain bus0/bus slot25/slot function0/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, +
Re: [libvirt] PATCH: 7/12: Public API for node devices
On Fri, 2008-11-14 at 13:28 +, Daniel P. Berrange wrote: On Fri, Nov 14, 2008 at 12:46:09PM +, Mark McLoughlin wrote: On Thu, 2008-11-13 at 17:30 +, 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. + +virNodeDevicePtrvirNodeDeviceLookupByName (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
Re: [libvirt] PATCH: 7/12: Public API for node devices
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 Daniel diff -r 78738f80cf4c include/libvirt/libvirt.h --- a/include/libvirt/libvirt.h Wed Nov 12 21:05:32 2008 + +++ b/include/libvirt/libvirt.h Wed Nov 12 21:11:46 2008 + @@ -995,6 +995,74 @@ unsigned int flags); /* + * Host device enumeration + */ + +/** + * virNodeDevice: + * + * A virNodeDevice contains a node (host) device details. + */ + +typedef struct _virNodeDevice virNodeDevice; + +/** + * virNodeDevicePtr: + * + * A virNodeDevicePtr is a pointer to a virNodeDevice structure. Get + * one via virNodeDeviceLookupByKey, virNodeDeviceLookupByName, or + * virNodeDeviceCreate. Be sure to Call virNodeDeviceFree when done + * using a virNodeDevicePtr obtained from any of the above functions to + * avoid leaking memory. + */ + +typedef virNodeDevice *virNodeDevicePtr; + + +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); + +virNodeDevicePtrvirNodeDeviceLookupByName (virConnectPtr conn, + const char *name); + +const char *virNodeDeviceGetName (virNodeDevicePtr dev); + +const char *virNodeDeviceGetParent (virNodeDevicePtr dev); + +int virNodeDeviceNumOfCaps (virNodeDevicePtr dev); + +int virNodeDeviceListCaps(virNodeDevicePtr dev, + char **const names, + int maxnames); + +char * virNodeDeviceGetXMLDesc (virNodeDevicePtr dev, + unsigned int flags); + +virNodeDevicePtrvirNodeDeviceCreate (virConnectPtr conn, + const char *xml, + unsigned int flags); + +int virNodeDeviceDestroy(virNodeDevicePtr dev, + unsigned int flags); + +int virNodeDeviceFree (virNodeDevicePtr dev); + +/* * Domain Event Notification */ diff -r 78738f80cf4c include/libvirt/libvirt.h.in --- a/include/libvirt/libvirt.h.in Wed Nov 12 21:05:32 2008 + +++ b/include/libvirt/libvirt.h.in Wed Nov 12 21:11:46 2008 + @@ -995,6 +995,74 @@ unsigned int flags); /* + * Host device enumeration + */ + +/** + * virNodeDevice: + * + * A virNodeDevice contains a node (host) device details. + */ + +typedef struct _virNodeDevice virNodeDevice; + +/** + * virNodeDevicePtr: + * + * A virNodeDevicePtr is a pointer to a virNodeDevice structure. Get + * one via virNodeDeviceLookupByKey, virNodeDeviceLookupByName, or + * virNodeDeviceCreate. Be sure to Call virNodeDeviceFree when done + * using a virNodeDevicePtr obtained from any of the above functions to + * avoid leaking memory. + */ + +typedef virNodeDevice *virNodeDevicePtr; + + +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, +
