[libvirt] [Fwd: first cut public API for physical host interface configuration]

(Oops, I originally sent this with the wrong return address, which probably caused it to be silently swallowed...) To get started integrating libnetcf support into libvirt, the attached libvirt.h diff has a first attempt at the public API that will hook up to libnetcf on the libvirtd side. I started out with the virNetwork* API, and modified/removed as seemed appropriate. A few points worth mentioning: virNetwork has "defined" and "active" interfaces, but so far virInterface just has interfaces, as the active/inactive status is really controlled by 1) the "onboot" property in the XML, and 2) whether or not virInterfaceStart() has been called yet. The _virInterface struct that is referenced here is more or less identical to _virnetwork. libnetcf works with netcf structs (one per library instance) and netcf_if structs (one per interface, multiple per library instance. As far as I can see right now, the netcf struct will not need to be referenced directly from the client side, but the netcf_if for each interface is needed, and I guess a cookie representing it will be sent to the client side and stored in the _virInterface. Is that what the UUID in _virNetwork is used for? (I know, I know - "read the code!", it's just quicker to (and avoids misreading on my part) to ask) As before, any and all advice/corrections gratefully accepted! diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h index 779ea72..cac400e 100644 --- a/include/libvirt/libvirt.h +++ b/include/libvirt/libvirt.h @@ -854,7 +854,73 @@ int virNetworkGetAutostart (virNetworkPtr network, int virNetworkSetAutostart (virNetworkPtr network, int autostart); +/* + * Physical host interface configuration API + */ + +/** + * virInterface: + * + * a virInterface is a private structure representing a virtual interface. + */ +typedef struct _virInterface virInterface; + +/** + * virInterfacePtr: + * + * a virInterfacePtr is pointer to a virInterface private structure, this is the + * type used to reference a virtual interface in the API. + */ +typedef virInterface *virInterfacePtr; + +/* + * Get connection from interface. + */ +virConnectPtr virInterfaceGetConnect (virInterfacePtr interface); + +/* + * List defined interfaces + */ +int virConnectNumOfInterfaces (virConnectPtr conn); +int virConnectListInterfaces (virConnectPtr conn, + char **const names, + int maxnames); + +/* + * Lookup interface by name + */ +virInterfacePtr virInterfaceLookupByName (virConnectPtr conn, + const char *name); + const char *uuid); + +/* + * Define interface (or modify existing interface configuration) + */ +virInterfacePtr virInterfaceDefineXML (virConnectPtr conn, + const char *xmlDesc); + +/* + * Delete interface + */ +int virInterfaceUndefine (virInterfacePtr interface); + +/* + * Activate interface (ie call "ifup") + */ +int virInterfaceStart (virInterfacePtr interface); + +/* + * De-activate interface (call "ifdown") + */ +int virInterfaceStop (virInterfacePtr interface); + +/* + * Interface information + */ +const char* virInterfaceGetName (virInterfacePtr interface); +char * virInterfaceGetXMLDesc (virInterfacePtr interface, + int flags); /** * virStoragePool: *

On Wed, Mar 25, 2009 at 02:16:48PM -0400, Laine Stump wrote:
(Oops, I originally sent this with the wrong return address, which probably caused it to be silently swallowed...)
To get started integrating libnetcf support into libvirt, the attached libvirt.h diff has a first attempt at the public API that will hook up to libnetcf on the libvirtd side. I started out with the virNetwork* API, and modified/removed as seemed appropriate.
A few points worth mentioning:
virNetwork has "defined" and "active" interfaces, but so far virInterface just has interfaces, as the active/inactive status is really controlled by 1) the "onboot" property in the XML, and 2) whether or not virInterfaceStart() has been called yet.
When 'virNetwork' talks about "defined" vs "active" interfaces there are a few things to be aware of: - There is a concept of a persistent network. This is a network which has a config file associated with it. - There is a concept of a transient network. This is a network which does not have any config file associated wit hit. - A persistent network is first defined (virNetworkDefine). This creates the config file. - A persistent network can be started (virNetworkCreate). This is now an active + defined network - A transient network can be started (virNetworkCreateXML). This is just an active network - When 'virNetworkDestroy' is called on a active network it is stopped. If it is transient, all trace of it now goes away. If it was persistent, you can still query the defined config I think that does map reasonably well onto physical interfaces too. eg, you can define an interface by creating the ifcfg-eth0 file (or equivalent). A defined inteface can be made active by calling 'ifup'. You can also have transient interfaces for which there is no ifcfg-eth0 file. eg, if someone does 'brctl addbr foo' you now have a transient interface which is active, but has no config.
libnetcf works with netcf structs (one per library instance) and netcf_if structs (one per interface, multiple per library instance. As far as I can see right now, the netcf struct will not need to be referenced directly from the client side, but the netcf_if for each interface is needed, and I guess a cookie representing it will be sent to the client side and stored in the _virInterface. Is that what the UUID in _virNetwork is used for? (I know, I know - "read the code!", it's just quicker to (and avoids misreading on my part) to ask)
All use of netcf data structures would be internal to the specific libvirt driver implementation, eg the src/interface_netcf_driver.c virInterfacePtr struct should contain whatever unique identifiers are used in the public API. Typically name + UUID is sufficient for most of our APIs. So given a virInterffacePtr you should be able to lookup a corresponding netcf data structure based on either UUID or name - whichever works best.
+/* + * Lookup interface by name + */ +virInterfacePtr virInterfaceLookupByName (virConnectPtr conn, + const char *name); + const char *uuid);
There shouldn't be a uuid parameter here - instead have a 2nd method call +virInterfacePtr virInterfaceLookupByUUID (virConnectPtr conn, + const unsigned char *uuid); Also by convention, we always have an equivalent lookup which takes a UUID string, instead of the raw binary UUID format: +virInterfacePtr virInterfaceLookupByUUIDString (virConnectPtr conn, + const char *uuid);
+ +/* + * Define interface (or modify existing interface configuration) + */ +virInterfacePtr virInterfaceDefineXML (virConnectPtr conn, + const char *xmlDesc); + +/* + * Delete interface + */ +int virInterfaceUndefine (virInterfacePtr interface); + +/* + * Activate interface (ie call "ifup") + */ +int virInterfaceStart (virInterfacePtr interface); + +/* + * De-activate interface (call "ifdown") + */ +int virInterfaceStop (virInterfacePtr interface);
I'd prefer it if these were called virInterfaceCreate(virInterfacePtr) (for ifup) virInterfaceDestory(virInterafacePtr) (for ifdown) just so we follow the naming conventions used in the rest of the public APIs. It makes life easier on users / developers, so know that the same nouns are used for the same operations on each object. To support transient networks, there's also be a use case for an API taking an XML file & calling ifup on the definition immediately without writing out an ifcfg-XXX file. eg virInterfaceCreateXML(virConnectPtr con, const char *xmlDesc)
+/* + * Interface information + */ +const char* virInterfaceGetName (virInterfacePtr interface);
You'd also want equivalent for UUID query 5B int virInterfaceGetUUID (virInterfacePtr interface, unsigned char *uuid); int virInterfaceGetUUIDString (virInterfacePtr interface, char *buf);
+char * virInterfaceGetXMLDesc (virInterfacePtr interface, + int flags);
In summary, this all looks pretty sensible & as I'd imagined it would turn out... For extra bonus points, you'll wnat to consider adding a method to query ther network interfaces I/O stats, eg very similar for virDomainInterfaceStats int virInterfaceStats (virInterfacePtr interface, virInterfaceStatsPtr stats, size_t size); Regards, 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 :|

On Wed, 2009-03-25 at 19:59 +0000, Daniel P. Berrange wrote:
When 'virNetwork' talks about "defined" vs "active" interfaces there are a few things to be aware of:
- There is a concept of a persistent network. This is a network which has a config file associated with it. - There is a concept of a transient network. This is a network which does not have any config file associated wit hit.
For interfaces, I don't think it makes much sense to deal with transient interfaces, especially since implementing them would require that we reimplement the ifup functionality.
You can also have transient interfaces for which there is no ifcfg-eth0 file. eg, if someone does 'brctl addbr foo' you now have a transient interface which is active, but has no config.
But then you'd need to run dhclient or ifconfig agaonst foo etc. Transient interfaces get us to where we initially said we do not want to go: reimplementing a distro's network config scripts.
Typically name + UUID is sufficient for most of our APIs. So given a virInterffacePtr you should be able to lookup a corresponding netcf data structure based on either UUID or name - whichever works best.
Actually, UUID isn't so fun, since there's no place in the stock network config script to store it. For initscripts, we can just stick a NETCF_UUID or whatever variable into the interface config. On Debian, we would have to store that info in some lookaside file - and associate it by name with an interface, i.e. do something the application might as well do on its own. So I am not convinced that UUID is all that useful.
To support transient networks, there's also be a use case for an API taking an XML file & calling ifup on the definition immediately without writing out an ifcfg-XXX file. eg
Unless somebody feels like reimplementing ifup, no transient interfaces. David

On Thu, Mar 26, 2009 at 01:42:59PM -0700, David Lutterkort wrote:
Actually, UUID isn't so fun, since there's no place in the stock network config script to store it. For initscripts, we can just stick a NETCF_UUID or whatever variable into the interface config. On Debian, we would have to store that info in some lookaside file - and associate it by name with an interface, i.e. do something the application might as well do on its own. So I am not convinced that UUID is all that useful.
Well unique identifiers, more resilients than names are really needed in practice. And volativity of physical hardware will increase things like SR-IOV where you can dynamically create/remove an interface with its own PCI identification from a single hardware card means the old days where the number of interfaces was a nearly static config are over, even on servers. Maybe generating a unique ID based on a MAC address and possibly a PCI ID should be doable, and could avoid the lookaside file. But we already store per object files descriptions in the libvirt /etc/hierarchy, we do it for networks already, and I don't see the addition for network interfaces to be such a problem. Having a way from the UUID to find the MAC address sounds important to me but that may not be sufficient for proper unique identification anymore. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On Thu, Mar 26, 2009 at 01:42:59PM -0700, David Lutterkort wrote:
On Wed, 2009-03-25 at 19:59 +0000, Daniel P. Berrange wrote:
When 'virNetwork' talks about "defined" vs "active" interfaces there are a few things to be aware of:
- There is a concept of a persistent network. This is a network which has a config file associated with it. - There is a concept of a transient network. This is a network which does not have any config file associated wit hit.
For interfaces, I don't think it makes much sense to deal with transient interfaces, especially since implementing them would require that we reimplement the ifup functionality.
You can also have transient interfaces for which there is no ifcfg-eth0 file. eg, if someone does 'brctl addbr foo' you now have a transient interface which is active, but has no config.
But then you'd need to run dhclient or ifconfig agaonst foo etc. Transient interfaces get us to where we initially said we do not want to go: reimplementing a distro's network config scripts.
We probably don't want to do creation of transient interfaces, since that would entail reimplementing ifup & friends. I think it might stil be interesting to be able to report on the existance of transient interfaces created by external tools. For example, if someone is running a VPN client, they may have a transient tun0 device for which there is no ifcfg-XXX file, but we should still be able to report its live configuration to the users of the libvirt API, even if we don't let them ifup/down that interface.
Typically name + UUID is sufficient for most of our APIs. So given a virInterffacePtr you should be able to lookup a corresponding netcf data structure based on either UUID or name - whichever works best.
Actually, UUID isn't so fun, since there's no place in the stock network config script to store it. For initscripts, we can just stick a NETCF_UUID or whatever variable into the interface config. On Debian, we would have to store that info in some lookaside file - and associate it by name with an interface, i.e. do something the application might as well do on its own. So I am not convinced that UUID is all that useful.
I'm inclined to agree. Perhaps we should instead provide 'hardware address' as a unique identifier / property in its place, since that's an identifier that many apps will want to work with when looking at interfaces.
To support transient networks, there's also be a use case for an API taking an XML file & calling ifup on the definition immediately without writing out an ifcfg-XXX file. eg
Unless somebody feels like reimplementing ifup, no transient interfaces.
Regards, 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 :|

To summarize (correct anything I've gotten wrong): 1) Supporting transient interfaces would be nice, but would require too much re-implementation of ifup-type scripts in our code (and for every platorm), so it is out of scope. danpb thinks at least being able to query transient interfaces (those that exist, but have no ifcfg-* file) would be nice. (Since libnetcf currently does everything by reading the ifcfg-* files and converting them into XML, that would require some work). 2) Just querying by name isn't good enough, but using UUID creates extra complexity. The problem can also solved by querying with MAC address. I'll add in the appropriate functions. 3) For consistency, virInterfaceStart() should be renamed to virInterfaceCreate(), and virInterfaceStop() to either virInterfaceDestroy() or virInterfaceDelete() (I still like Startp/Stop, since htey imply that the objects can be re-used, but The idea of a less polluted namespace has its advantages). So which 4) virInterfaceDefineXML(), virInterfaceCreate(), and virInterfaceHumiliateAndEmasculate() (pending a concensus on name) should have a "flags" argument added, currently unused, but to allow for future expansion without needing to obsolete the API. 5) For bonus points, danpb suggests adding: int virInterfaceStats (virInterfacePtr interface, virInterfaceStatsPtr stats, size_t size); Useful idea, but I'll save the bonus points for later - right now I'm just trying to avoid getting kicked off the island! (seriously, though, that is outside the current scope of libnetcf. Depending on what stats you wanted, it could be implemented on linux by just sifting through /proc/net/dev. Are you just looking for the same stats that are delivered by virDomainInterfaceStats (except for the physical host)? Did I get everything? (I'm attaching another libvirt.h diff, based on the comments). diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h index 779ea72..b92a799 100644 --- a/include/libvirt/libvirt.h +++ b/include/libvirt/libvirt.h @@ -854,6 +854,79 @@ int virNetworkGetAutostart (virNetworkPtr network, int virNetworkSetAutostart (virNetworkPtr network, int autostart); +/* + * Physical host interface configuration API + */ + +/** + * virInterface: + * + * a virInterface is a private structure representing a virtual interface. + */ +typedef struct _virInterface virInterface; + +/** + * virInterfacePtr: + * + * a virInterfacePtr is pointer to a virInterface private structure, this is the + * type used to reference a virtual interface in the API. + */ +typedef virInterface *virInterfacePtr; + +/* + * Get connection from interface. + */ +virConnectPtr virInterfaceGetConnect (virInterfacePtr interface); + +/* + * List defined interfaces + */ +int virConnectNumOfInterfaces (virConnectPtr conn); +int virConnectListInterfaces (virConnectPtr conn, + char **const names, + int maxnames); + +/* + * Lookup interface by name or MAC address + */ +virInterfacePtr virInterfaceLookupByName (virConnectPtr conn, + const char *name); +virInterfacePtr virInterfaceLookupByMAC (virConnectPtr conn, + const unsigned char *mac); + +/* + * Define interface (or modify existing interface configuration) + */ +virInterfacePtr virInterfaceDefineXML (virConnectPtr conn, + const char *xmlDesc, + int flags); + +/* + * Delete interface + */ +int virInterfaceUndefine (virInterfacePtr interface); + +/* + * Activate interface (ie call "ifup") + */ +int virInterfaceCreate (virInterfacePtr interface, + int flags); + +/* + * De-activate interface (call "ifdown") + */ +int virInterfaceDestroy (virInterfacePtr interface, + int flags); + +/* + * Interface information + */ +const char* virInterfaceGetName (virInterfacePtr interface); +int virInterfaceGetMAC (virInterfacePtr interface, + unsigned char *mac); + +char * virInterfaceGetXMLDesc (virInterfacePtr interface, + int flags); /** * virStoragePool:

On Fri, Mar 27, 2009 at 12:58:44PM -0400, Laine Stump wrote:
To summarize (correct anything I've gotten wrong):
1) Supporting transient interfaces would be nice, but would require too much re-implementation of ifup-type scripts in our code (and for every platorm), so it is out of scope. danpb thinks at least being able to query transient interfaces (those that exist, but have no ifcfg-* file) would be nice. (Since libnetcf currently does everything by reading the ifcfg-* files and converting them into XML, that would require some work).
Well you'll need to be able to query live interfaces anyway, because you'll need to be able to report IPv4 addresses assigned by DHCP and report IPv6 addresses that auto-configure themselves. So given that you have to do this, the ability to present info on existing transient interfaces should more or less come for free.
2) Just querying by name isn't good enough, but using UUID creates extra complexity. The problem can also solved by querying with MAC address. I'll add in the appropriate functions.
3) For consistency, virInterfaceStart() should be renamed to virInterfaceCreate(), and virInterfaceStop() to either virInterfaceDestroy() or virInterfaceDelete() (I still like Startp/Stop, since htey imply that the objects can be re-used, but The idea of a less polluted namespace has its advantages). So which
To 'Destroy', since 'Delete' is really what 'Undefine' is providing.
4) virInterfaceDefineXML(), virInterfaceCreate(), and virInterfaceHumiliateAndEmasculate() (pending a concensus on name) should have a "flags" argument added, currently unused, but to allow for future expansion without needing to obsolete the API.
Yep,
5) For bonus points, danpb suggests adding:
int virInterfaceStats (virInterfacePtr interface, virInterfaceStatsPtr stats, size_t size);
Useful idea, but I'll save the bonus points for later - right now I'm just trying to avoid getting kicked off the island! (seriously, though, that is outside the current scope of libnetcf. Depending on what stats you wanted, it could be implemented on linux by just sifting through /proc/net/dev. Are you just looking for the same stats that are delivered by virDomainInterfaceStats (except for the physical host)?
Yes, you should be able to re-use the interface impl of virDomainInterfaceStats() that already exists, just passing in the appropriate ethXXX device name. All the hard logic thre is already done
+virInterfacePtr virInterfaceLookupByMAC (virConnectPtr conn, + const unsigned char *mac);
+int virInterfaceGetMAC (virInterfacePtr interface, + unsigned char *mac);
For these two, we should probably also have an equivalent String method virInterfacePtr virInterfaceLookupByMACString(virConnectPtr conn, const char *mac); int virInterfaceGetMACString (virInterfacePtr interface, char *mac); In the same was as we have UUID and UUIDString, because some apps may prefer to pass/fetch the raw unsigned bytes of the MAC addr, while others may have the MAC in human readable format Regards, 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 :|

On Mon, 2009-03-30 at 12:40 +0100, Daniel P. Berrange wrote:
Well you'll need to be able to query live interfaces anyway, because you'll need to be able to report IPv4 addresses assigned by DHCP and report IPv6 addresses that auto-configure themselves.
What's the use case behind that ? I would argue that any infrastructure that requires libvirt to get addresses of interfaces is fundamentally broken - I can see the case for getting the MAC address of interfaces on a host, but the MAC/IP address mapping really needs to be managed centrally. David

On Mon, Mar 30, 2009 at 02:45:51PM -0700, David Lutterkort wrote:
On Mon, 2009-03-30 at 12:40 +0100, Daniel P. Berrange wrote:
Well you'll need to be able to query live interfaces anyway, because you'll need to be able to report IPv4 addresses assigned by DHCP and report IPv6 addresses that auto-configure themselves.
What's the use case behind that ? I would argue that any infrastructure that requires libvirt to get addresses of interfaces is fundamentally broken - I can see the case for getting the MAC address of interfaces on a host, but the MAC/IP address mapping really needs to be managed centrally.
Simply that if the admin configures a new NIC using DHCP / IPV6 they will wish to know what address it got after it was brought online. libvirt does not have todo anything with these addresses, merely provide info about them in the XML for the interface. Applications using libvirt may do stuff with the addresses, for example they may wish todo a direct node<->node migration over an interface that's separate from the one used for talking to libvirtd. As such they'd need to know the IP addr of such an interface. NB, I don't think this neccessarily has to be part of netcf library. If netcf wants to restrict its scope to management of config files, and ability to bring an interface up & down that's no problem. libvirt can provide the functionality for getting 'live' IP address info, and getting I/O statistics, and other such stuff. 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 :|

On Tue, 2009-03-31 at 10:24 +0100, Daniel P. Berrange wrote:
Simply that if the admin configures a new NIC using DHCP / IPV6 they will wish to know what address it got after it was brought online. libvirt does not have todo anything with these addresses, merely provide info about them in the XML for the interface. Applications using libvirt may do stuff with the addresses, for example they may wish todo a direct node<->node migration over an interface that's separate from the one used for talking to libvirtd. As such they'd need to know the IP addr of such an interface.
This sounds very much like a job for DNS + DHCP with fixed MAC/IP mappings. libvirt might want to offer this IP lookup capability for adhoc setups, but it encourages very bad administration practices.
NB, I don't think this neccessarily has to be part of netcf library.
My objection isn't so much to where this should live, as that it's unnecessary at best and wallpapers over broken setups/apps at worst. David

On Wed, Mar 25, 2009 at 02:16:48PM -0400, Laine Stump wrote:
(Oops, I originally sent this with the wrong return address, which probably caused it to be silently swallowed...) [...]
Dan replied to most points, and raised the main problems
As before, any and all advice/corrections gratefully accepted! +/* + * Define interface (or modify existing interface configuration) + */ +virInterfacePtr virInterfaceDefineXML (virConnectPtr conn, + const char *xmlDesc);
I would add an extra "unsigned int flags" as the last parameter, unused for now
+/* + * Delete interface + */ +int virInterfaceUndefine (virInterfacePtr interface); + +/* + * Activate interface (ie call "ifup") + */ +int virInterfaceStart (virInterfacePtr interface);
Same for the Create() one per Dan's suggested renaming.
+ +/* + * De-activate interface (call "ifdown") + */ +int virInterfaceStop (virInterfacePtr interface);
For Stop/Destroy, my preference in naming would go to Delete, and also add an unsigned int flags argument. Thanks :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (4)
-
Daniel P. Berrange
-
Daniel Veillard
-
David Lutterkort
-
Laine Stump