[Libvir] [Discussion] 0/4 Implement DHCP host mappings for virtual networks
This is a series of patches to implement DHCP IP & hostname mappings for libvirt virtual networks. ** NOTE: This patch is not to be applied. There is some memory corruption bug which I'm still tracking down. What this patch allows you to do is to create a table of IP address to host (defined by its hardware / MAC address) for each virtual network. Optionally you can also assign a hostname. When a host starts up and requests an IP address through DHCP, if it is in this table then it gets the fixed IP address from the table and, optionally, a fixed hostname. Four new API calls are added. Two are required to list the contents of the mapping table. Two more allow you to add and delete a mapping. The mapping table is semi-permanent. It is stored in /var/lib/libvirt/hosts-<networkname>.conf and survives across libvirtd restarts. The implementation simply uses dnsmasq's --dhcp-hostsfile flag. Refer to the dnsmasq(1) manual page. Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
This just adds the four new functions to the public API. Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
On Thu, Feb 21, 2008 at 08:56:17PM +0000, Richard W.M. Jones wrote:
This just adds the four new functions to the public API. [...] +/** + * virNetworkDHCPHostMapping: + * + * hostname mappings are returned by virNetworkListDHCPHostMapping. + */ +typedef struct _virNetworkDHCPHostMapping { + char *hwaddr; + char *ipaddr; + char *hostname; +} virNetworkDHCPHostMapping; + +typedef virNetworkDHCPHostMapping *virNetworkDHCPHostMappingPtr; + [...] +int virNetworkListDHCPHostMappings + (virNetworkPtr network, + virNetworkDHCPHostMappingPtr *const mappings, + int maxmappings); +int virNetworkFreeDHCPHostMappings + (virNetworkDHCPHostMappingPtr *const mappings, + int maxmappings);
Hum, do we really need to expose this structure at the API level, since we use it only for listing. Since everything is provided as strings why not use something like int virNetworkListDHCPHostMappings (virNetworkPtr network, int maxmappings, char **hwaddrtab, char **ipaddrtab, char **hostnametab); This also avoids the free function too, same functionality, quite a simpler interface. Also I don't understand the 'const' in that function and what virNetworkFreeDHCPHostMappings is supposed to be used for. If mappings is allocated by the user, it doesn't need a library deallocator, if it's allocated by the library I can't see how it's const, the API confuses me I must admit. Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
On Thu, Feb 21, 2008 at 05:13:51PM -0500, Daniel Veillard wrote:
On Thu, Feb 21, 2008 at 08:56:17PM +0000, Richard W.M. Jones wrote:
This just adds the four new functions to the public API. [...] +/** + * virNetworkDHCPHostMapping: + * + * hostname mappings are returned by virNetworkListDHCPHostMapping. + */ +typedef struct _virNetworkDHCPHostMapping { + char *hwaddr; + char *ipaddr; + char *hostname; +} virNetworkDHCPHostMapping; + +typedef virNetworkDHCPHostMapping *virNetworkDHCPHostMappingPtr; + [...] +int virNetworkListDHCPHostMappings + (virNetworkPtr network, + virNetworkDHCPHostMappingPtr *const mappings, + int maxmappings); +int virNetworkFreeDHCPHostMappings + (virNetworkDHCPHostMappingPtr *const mappings, + int maxmappings);
Hum, do we really need to expose this structure at the API level, since we use it only for listing. Since everything is provided as strings why not use something like
int virNetworkListDHCPHostMappings (virNetworkPtr network, int maxmappings, char **hwaddrtab, char **ipaddrtab, char **hostnametab);
This also avoids the free function too, same functionality, quite a simpler interface.
Simpler for who ? Apps using this data now have to pass around a whole bunch of pointers, instead of a single virNetworkDHCPHostMapping. When you map this API into higher level languages a struct is also going to be a much nicer thing to represent. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
On Thu, Feb 21, 2008 at 10:21:09PM +0000, Daniel P. Berrange wrote:
On Thu, Feb 21, 2008 at 05:13:51PM -0500, Daniel Veillard wrote:
On Thu, Feb 21, 2008 at 08:56:17PM +0000, Richard W.M. Jones wrote:
This just adds the four new functions to the public API. [...] +/** + * virNetworkDHCPHostMapping: + * + * hostname mappings are returned by virNetworkListDHCPHostMapping. + */ +typedef struct _virNetworkDHCPHostMapping { + char *hwaddr; + char *ipaddr; + char *hostname; +} virNetworkDHCPHostMapping; + +typedef virNetworkDHCPHostMapping *virNetworkDHCPHostMappingPtr; + [...] +int virNetworkListDHCPHostMappings + (virNetworkPtr network, + virNetworkDHCPHostMappingPtr *const mappings, + int maxmappings); +int virNetworkFreeDHCPHostMappings + (virNetworkDHCPHostMappingPtr *const mappings, + int maxmappings);
Hum, do we really need to expose this structure at the API level, since we use it only for listing. Since everything is provided as strings why not use something like
int virNetworkListDHCPHostMappings (virNetworkPtr network, int maxmappings, char **hwaddrtab, char **ipaddrtab, char **hostnametab);
This also avoids the free function too, same functionality, quite a simpler interface.
Simpler for who ? Apps using this data now have to pass around a whole bunch of pointers, instead of a single virNetworkDHCPHostMapping. When you map this API into higher level languages a struct is also going to be a much nicer thing to represent.
if the struct was a simpler representation it would be used for Add too, I don't think it's the case. Maybe the 3 arrays is not the nicest but defining a public structure holding 3 strings feels strange to me, especially when in the other place we use the 3 strings directly. Having a deallocator but no allocator function looks strange to me too. Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
[... Struct versus 3 arrays discussion snipped ...] I think both representations have their merits, and DV's "3 arrays" approach is much easier to marshal through the remote code, and simpler to produce OCaml bindings for (no idea about Python). The reason for using the struct was to provide extensibility. The dnsmasq "--dhcp-hostfiles" file format is much richer than merely (hwaddr, ipaddr [, hostname]) triplets. For example it can include lease times and client IDs[1]. The API calls I chose give us the opportunity to extend the structure with new elements in future. But to do this you have to provide a deallocator (because additional elements may need to be freed), and flags on the Add call. If we agree that we won't / can't / don't want to extend this structure, I'd be more than happy to use 3 arrays and lose the deallocator. It will simplify the remote code considerably. Rich. [1] See the dnsmasq manual page for the full details. I'm not convinced that the format is even well-specified by that manpage so probably at some point we'll need to look at the source code. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
On Fri, 2008-02-22 at 09:37 +0000, Richard W.M. Jones wrote:
The reason for using the struct was to provide extensibility.
Another reason for adding it via the network's XML definition rather than an ABI/API ... isn't extensibility the reason we don't have e.g. an API for listing, adding and removing domain network interfaces? Cheers, Mark.
On Fri, Feb 22, 2008 at 09:37:24AM +0000, Richard W.M. Jones wrote:
[... Struct versus 3 arrays discussion snipped ...]
First I should state it's a cool idea, even if I reacted to the proposed API. I also agree to some extend with Mark, the on disk storage should probably go in the network description. But that's orthogonal to the API side :)
I think both representations have their merits, and DV's "3 arrays" approach is much easier to marshal through the remote code, and simpler to produce OCaml bindings for (no idea about Python).
For python we will need manually generated bindings anyway to have something which looks nice from a python viewpoint, for example the list would be nicer exported as a dictionary based on the host names, and one or the other won't make it much different.
The reason for using the struct was to provide extensibility. The dnsmasq "--dhcp-hostfiles" file format is much richer than merely (hwaddr, ipaddr [, hostname]) triplets. For example it can include lease times and client IDs[1].
Okay, then we probably need to abstract the notion of dhcp host but I would not expose the structure then (which was my main beef in the intial patch) let's keep the structure private and provide accessors and lookups in the library, then we will be able to extend.
The API calls I chose give us the opportunity to extend the structure with new elements in future. But to do this you have to provide a deallocator (because additional elements may need to be freed), and flags on the Add call.
Agreed, but if you provide deallocator, provide allocators too, and acessors.
If we agree that we won't / can't / don't want to extend this structure, I'd be more than happy to use 3 arrays and lose the deallocator. It will simplify the remote code considerably.
Actually at the remote level we probably don't need to expose the informations in the exact same way as in the public API, do we ? The way things are exposed though the API opaque structure doesn't have to match exactly the way it's done at the driver level, and there we can shoot for the most convenient way (while trying to be portable if we extend the set of informations).
[1] See the dnsmasq manual page for the full details. I'm not convinced that the format is even well-specified by that manpage so probably at some point we'll need to look at the source code.
heh Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
On Fri, 2008-02-22 at 05:16 -0500, Daniel Veillard wrote:
I also agree to some extend with Mark, the on disk storage should probably go in the network description. But that's orthogonal to the API side :)
Heh, no it's not orthogonal ... XML *is* the API for this kind of stuff across libvirt :-) Cheers, Mark.
On Fri, Feb 22, 2008 at 10:47:44AM +0000, Mark McLoughlin wrote:
On Fri, 2008-02-22 at 05:16 -0500, Daniel Veillard wrote:
I also agree to some extend with Mark, the on disk storage should probably go in the network description. But that's orthogonal to the API side :)
Heh, no it's not orthogonal ... XML *is* the API for this kind of stuff across libvirt :-)
Sort of. XML is used as a way to avoid making any ABI guarantees. I had a look at the implementation of the network XML. There are three calls which matter: virNetworkDefineXML Define the network from the XML definition, but don't start it up. Saves the XML into a private directory so that it will be loaded when libvirtd restarts (and depending on the autostart setting may also be started up automatically). The current implementation always completely recreates the internal network definitions from scratch. virNetworkCreateXML Start up the network from the XML definition. As far as I can see, this doesn't persist the XML so the network will disappear after libvirtd is restarted. The current implementation always completely recreates the internal network definitions from scratch. virNetworkUndefine Stops the network. Deletes the config file and autostart setting from the private directory. The first two calls need to be changed to look for incremental changes in the XML, since its better to avoid fully restarting dnsmasq, and we certainly don't want to try taking down the bridge. Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
On Fri, Feb 22, 2008 at 11:07:15AM +0000, Richard W.M. Jones wrote:
On Fri, Feb 22, 2008 at 10:47:44AM +0000, Mark McLoughlin wrote:
On Fri, 2008-02-22 at 05:16 -0500, Daniel Veillard wrote:
I also agree to some extend with Mark, the on disk storage should probably go in the network description. But that's orthogonal to the API side :)
Heh, no it's not orthogonal ... XML *is* the API for this kind of stuff across libvirt :-)
Sort of. XML is used as a way to avoid making any ABI guarantees.
I had a look at the implementation of the network XML. There are three calls which matter:
virNetworkDefineXML
Define the network from the XML definition, but don't start it up.
Saves the XML into a private directory so that it will be loaded when libvirtd restarts (and depending on the autostart setting may also be started up automatically).
The current implementation always completely recreates the internal network definitions from scratch.
virNetworkCreateXML
Start up the network from the XML definition.
As far as I can see, this doesn't persist the XML so the network will disappear after libvirtd is restarted.
The current implementation always completely recreates the internal network definitions from scratch.
virNetworkUndefine
Stops the network.
Deletes the config file and autostart setting from the private directory.
The first two calls need to be changed to look for incremental changes in the XML, since its better to avoid fully restarting dnsmasq, and we certainly don't want to try taking down the bridge.
We can't change the semantics of these calls. virNetworkDefineXML() explicitly does not affect the current live config, saving the config to take effect next time the network is started. This is the same semantics as the virDomainDefineXML() call, and the virStoragePoolDefineXML() calls. To do live updates, we'd need API similar to the domain device hot attach and detach APIs Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
On Sun, Feb 24, 2008 at 10:20:24PM +0000, Daniel P. Berrange wrote:
We can't change the semantics of these calls. virNetworkDefineXML() explicitly does not affect the current live config, saving the config to take effect next time the network is started. This is the same semantics as the virDomainDefineXML() call, and the virStoragePoolDefineXML() calls.
Understood.
To do live updates, we'd need API similar to the domain device hot attach and detach APIs
Well given that it sounds like the original implementation is just as good. Any further comments on that? (http://www.redhat.com/archives/libvir-list/2008-February/thread.html#00281) Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top
On Mon, Feb 25, 2008 at 10:07:48AM +0000, Richard W.M. Jones wrote:
On Sun, Feb 24, 2008 at 10:20:24PM +0000, Daniel P. Berrange wrote:
We can't change the semantics of these calls. virNetworkDefineXML() explicitly does not affect the current live config, saving the config to take effect next time the network is started. This is the same semantics as the virDomainDefineXML() call, and the virStoragePoolDefineXML() calls.
Understood.
To do live updates, we'd need API similar to the domain device hot attach and detach APIs
Well given that it sounds like the original implementation is just as good. Any further comments on that? (http://www.redhat.com/archives/libvir-list/2008-February/thread.html#00281)
I'm fine with your original proposal, or DV's suggestion to use 3 separate arrays instead of the struct. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
This patch implements the QEMU driver part of the change. We run dnsmasq with the extra --dhcp-hostsfile=... parameter, and then include functions to read and write this file. When the file changes we send SIGHUP to dnsmasq. Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
This implements the changes in the client & server ends of the remote driver. Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
This adds four new subcommands to virsh allowing you to control the new functionality. Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
Hi Rich, On Thu, 2008-02-21 at 20:54 +0000, Richard W.M. Jones wrote:
What this patch allows you to do is to create a table of IP address to host (defined by its hardware / MAC address) for each virtual network. Optionally you can also assign a hostname. When a host starts up and requests an IP address through DHCP, if it is in this table then it gets the fixed IP address from the table and, optionally, a fixed hostname.
Four new API calls are added. Two are required to list the contents of the mapping table. Two more allow you to add and delete a mapping.
The mapping table is semi-permanent. It is stored in /var/lib/libvirt/hosts-<networkname>.conf and survives across libvirtd restarts.
Cool stuff, but why isn't this just an addition to the network's XML definition? Cheers, Mark.
On Fri, Feb 22, 2008 at 08:04:26AM +0000, Mark McLoughlin wrote:
On Thu, 2008-02-21 at 20:54 +0000, Richard W.M. Jones wrote:
What this patch allows you to do is to create a table of IP address to host (defined by its hardware / MAC address) for each virtual network. Optionally you can also assign a hostname. When a host starts up and requests an IP address through DHCP, if it is in this table then it gets the fixed IP address from the table and, optionally, a fixed hostname.
Four new API calls are added. Two are required to list the contents of the mapping table. Two more allow you to add and delete a mapping.
The mapping table is semi-permanent. It is stored in /var/lib/libvirt/hosts-<networkname>.conf and survives across libvirtd restarts.
Cool stuff, but why isn't this just an addition to the network's XML definition?
Well indeed. It's essentially because this implementation was thought to be easier. I haven't tried the other implementation so I'm not categorically sure that is true. Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
On Fri, Feb 22, 2008 at 08:04:26AM +0000, Mark McLoughlin wrote:
Hi Rich,
On Thu, 2008-02-21 at 20:54 +0000, Richard W.M. Jones wrote:
What this patch allows you to do is to create a table of IP address to host (defined by its hardware / MAC address) for each virtual network. Optionally you can also assign a hostname. When a host starts up and requests an IP address through DHCP, if it is in this table then it gets the fixed IP address from the table and, optionally, a fixed hostname.
Four new API calls are added. Two are required to list the contents of the mapping table. Two more allow you to add and delete a mapping.
The mapping table is semi-permanent. It is stored in /var/lib/libvirt/hosts-<networkname>.conf and survives across libvirtd restarts.
Cool stuff, but why isn't this just an addition to the network's XML definition?
It is possible, but tricky to get the on-the-fly updates when the semantics of the APIs for creating/definging networks. If you call 'virNetworkDefine' with a new XML doc, it loads the new config into memory, but does not apply the config until you destroy & restart the network in question. We can't destroy & restart the network since this would break networking for any live guests attached to the network. So if we wanted to represent this in the master XML for a network we'd have to follow the example of domain device hotplug, and have APIs to add/remove a snippet of XML describing the new entry. virNetworkAddHostMapping(virNetworkPtr net, char *xml); virNetworkRemoveHostMapping(virNetworkPtr net, char *xml); Where the XML was: '<mapping hostname="foo" mac="00:33:22:55:44:11"/>' Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
participants (4)
-
Daniel P. Berrange -
Daniel Veillard -
Mark McLoughlin -
Richard W.M. Jones