[libvirt] RFC: 'old' event for leaseshelper.c when lease renews
In the current version of dnsmasq, the leases helper script/program specified by --dhcp-script to dnsmasq is invoked on three events, 'add', 'old', 'del'. In short, add: -> a new lease has to be added to db del: -> a lease has to be deleted from db old:-> if lease has changed Now, the lease can be considered to be changed in 2 ways. [standard-change] The change could be to the associated hostname or MAC address [auxiliary-change] The change associated with expiry time or clientid When only --dhcp-script is set, 'old' events are sent only for standard-lease changes. But when --dhcp-script is set along with --leasefile-ro, 'old' events are sent for any change in the lease. So, right now, if a lease is renewed, the reflected change doesn't appear in our JSON formatted lease database <interface-name>.status. We have the following options with this: (i) We can simply add the --leasefile-ro option. But in that case, the leases file database which is generated by dnsmasq by the name <network-name>.leases will cease to exist. It won't contain any lease information. All lease database handling will be done by our leaseshelper. Note: this option given a lot of information which is not stored in <network-name>.leases file. (ii) We ask the dnsmasq developer(s) to add an extra command line option to enable auxiliary changes in lease to be propagated to leaseshelper via 'old' events. I had a small conversation with Simon Kelley, and he said: "Yes. For that application (libvirt), you clearly don't want a third-party patch. At very least I'd be willing to add a boolean option to dnsmasq which enables "old" events when the lease expiry time changes, independent of leasefile-ro." If we do this, then can retain <network-name>.leases and have our helper too. (iii) We do nothing. IMO, we should go for (i), since <network-name>.leases becomes obsolete, once we have our own leases helper. Also, enabling --leasefile-ro enables a lot of other options, like vendor-class, user-class, circuit-id and much more, which may be asked for in future and get added to the virNetworkDHCPLeases struct. But we will have to remove the option --dhcp-leasefile. Although it won't affect any existing APIs, if people did rely on the dnsmasq's generated leases file before, they will find it missing. There is one more point. The man page of dnsmasq for the leasefile-ro option, states, "In addition to the invocations given in --dhcp-script the lease-change script is called once, at dnsmasq startup, with the single argument "init". When called like this the script should write the saved state of the lease database, in dnsmasq leasefile format, to stdout and exit with zero exit code." This is something extra that we will have to add to leaseshelper if we go with (i). The easier option is to go with (ii), since Simon is already willing to add that functionality. -- Nehal J Wani
On Sat, Jun 28, 2014 at 05:06:26AM +0530, Nehal J Wani wrote:
In the current version of dnsmasq, the leases helper script/program specified by --dhcp-script to dnsmasq is invoked on three events, 'add', 'old', 'del'. In short, add: -> a new lease has to be added to db del: -> a lease has to be deleted from db old:-> if lease has changed Now, the lease can be considered to be changed in 2 ways. [standard-change] The change could be to the associated hostname or MAC address [auxiliary-change] The change associated with expiry time or clientid
When only --dhcp-script is set, 'old' events are sent only for standard-lease changes. But when --dhcp-script is set along with --leasefile-ro, 'old' events are sent for any change in the lease.
So, right now, if a lease is renewed, the reflected change doesn't appear in our JSON formatted lease database <interface-name>.status. We have the following options with this:
(i) We can simply add the --leasefile-ro option. But in that case, the leases file database which is generated by dnsmasq by the name <network-name>.leases will cease to exist. It won't contain any lease information. All lease database handling will be done by our leaseshelper. Note: this option given a lot of information which is not stored in <network-name>.leases file.
What purpose does the <network-name>.leases file that dnsmasq creates serve ? If our own leases file is able to provide any funtionality that is missing due to the loss of <network-name>.leases, then this option seems like the best.
(ii) We ask the dnsmasq developer(s) to add an extra command line option to enable auxiliary changes in lease to be propagated to leaseshelper via 'old' events. I had a small conversation with Simon Kelley, and he said: "Yes. For that application (libvirt), you clearly don't want a third-party patch. At very least I'd be willing to add a boolean option to dnsmasq which enables "old" events when the lease expiry time changes, independent of leasefile-ro." If we do this, then can retain <network-name>.leases and have our helper too.
I'd prefer (i) since that lets libvirt work properly with existing dnsmasq versions which are deployed. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Mon, Jun 30, 2014 at 1:52 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
On Sat, Jun 28, 2014 at 05:06:26AM +0530, Nehal J Wani wrote:
In the current version of dnsmasq, the leases helper script/program specified by --dhcp-script to dnsmasq is invoked on three events, 'add', 'old', 'del'. In short, add: -> a new lease has to be added to db del: -> a lease has to be deleted from db old:-> if lease has changed Now, the lease can be considered to be changed in 2 ways. [standard-change] The change could be to the associated hostname or MAC address [auxiliary-change] The change associated with expiry time or clientid
When only --dhcp-script is set, 'old' events are sent only for standard-lease changes. But when --dhcp-script is set along with --leasefile-ro, 'old' events are sent for any change in the lease.
So, right now, if a lease is renewed, the reflected change doesn't appear in our JSON formatted lease database <interface-name>.status. We have the following options with this:
(i) We can simply add the --leasefile-ro option. But in that case, the leases file database which is generated by dnsmasq by the name <network-name>.leases will cease to exist. It won't contain any lease information. All lease database handling will be done by our leaseshelper. Note: this option given a lot of information which is not stored in <network-name>.leases file.
What purpose does the <network-name>.leases file that dnsmasq creates serve ? If our own leases file is able to provide any funtionality that is missing due to the loss of <network-name>.leases, then this option seems like the best.
(ii) We ask the dnsmasq developer(s) to add an extra command line option to enable auxiliary changes in lease to be propagated to leaseshelper via 'old' events. I had a small conversation with Simon Kelley, and he said: "Yes. For that application (libvirt), you clearly don't want a third-party patch. At very least I'd be willing to add a boolean option to dnsmasq which enables "old" events when the lease expiry time changes, independent of leasefile-ro." If we do this, then can retain <network-name>.leases and have our helper too.
I'd prefer (i) since that lets libvirt work properly with existing dnsmasq versions which are deployed.
Agreed. The leaseile-ro option was added to dnsmasq in version 2.33 (Current version in fedora is 2.68 and that in CentOS is 2.48 [Dunno about RHEL]). So I guess, using it shouldn't be a harm. -- Nehal J Wani
I'd prefer (i) since that lets libvirt work properly with existing dnsmasq versions which are deployed.
Regards, Daniel
I cleared some more queries regarding leasesfile-ro option. Once can read the conversation at http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2014q2/008656.html. TL;DR: (i) We have to change the leasehelper program to honor the event 'init' and print leases info in dnsmasq leases format (so that dnsmasq knows about previous leases if it is restarted for some reason). (ii) Since we support DHCPv6, we will need to modify the JSON format to store server DUID too. Since we have one custom leases file for each network, I think it will suffice to store the DUID is just once in the JSON. Something like this: [ "server-duid": "00:01:00:01:1b:40:8d:94:00:25: 64:8b:e4:2c" { "iaid": "1221229", "ip-address": "2001:db8:ca2:2:1::95", "mac-address": "52:54:00:12:a2:6d", "hostname": "Fedora20", "client-id": "00:04:1a:c1:d9:6b:5a:0a:e2: bc:f8:4b:1e:37:2e:38:22:55" , "expiry-time": 1393244216 }, { "ip-address": "192.168.150.208", "mac-address": "52:54:00:11:56:b3", "hostname": "Wani-PC", "client-id": "01:52:54:00:11:56:b3", "expiry-time": 1393244248 } ] Do we want this in 1.2.6? Regards, Nehal J Wani
On 06/30/14 22:49, Nehal J Wani wrote:
I'd prefer (i) since that lets libvirt work properly with existing dnsmasq versions which are deployed.
Regards, Daniel
I cleared some more queries regarding leasesfile-ro option. Once can read the conversation at http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2014q2/008656.html.
TL;DR: (i) We have to change the leasehelper program to honor the event 'init' and print leases info in dnsmasq leases format (so that dnsmasq knows about previous leases if it is restarted for some reason). (ii) Since we support DHCPv6, we will need to modify the JSON format to store server DUID too. Since we have one custom leases file for each network, I think it will suffice to store the DUID is just once in the JSON. Something like this: [ "server-duid": "00:01:00:01:1b:40:8d:94:00:25: 64:8b:e4:2c" { "iaid": "1221229", "ip-address": "2001:db8:ca2:2:1::95", "mac-address": "52:54:00:12:a2:6d", "hostname": "Fedora20", "client-id": "00:04:1a:c1:d9:6b:5a:0a:e2: bc:f8:4b:1e:37:2e:38:22:55" , "expiry-time": 1393244216 }, { "ip-address": "192.168.150.208", "mac-address": "52:54:00:11:56:b3", "hostname": "Wani-PC", "client-id": "01:52:54:00:11:56:b3", "expiry-time": 1393244248 } ]
Do we want this in 1.2.6?
It's too late for 1.2.6, the release should happen any time today. Peter
I cleared some more queries regarding leasesfile-ro option. Once can read the conversation at http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2014q2/008656.html.
TL;DR: (i) We have to change the leasehelper program to honor the event 'init' and print leases info in dnsmasq leases format (so that dnsmasq knows about previous leases if it is restarted for some reason).
Bummer. When 'init' is sent to the leases helper program, the interface name is not known :'( so the helper program doesn't know which *.status file it has to read and print to stdout. Simon came up with the following hack: "" The most obvious nasty hack to make this work would be to have a set of filesystem links to the real lease-change script, each with a different name, and configure each dnsmasq to call a unique link. The script then checks argv[0] to find the name it was called by and then transforms that into the name of the corresponding database file. So we have something like scripts/interface1 is a link to /lib/libvirt/lease-change- script scripts/interface2 is a link to /lib/libvirt/lease-change-script and start dnsmasq with dnsmasq --interface=interface1 --dhcp-script=scripts/interface1 and the script finds the basename of argv[0[: scripts/interface1 -> interface1 and prepends the directory where the lease files are interface1 ->leasefiles/interface1 That works in the absence on the DNSMASQ_INTERFACE variable. "" Is this hack acceptable? -- Nehal J Wani
Bummer. When 'init' is sent to the leases helper program, the interface name is not known :'( so the helper program doesn't know which *.status file it has to read and print to stdout.
Simon came up with the following hack: "" The most obvious nasty hack to make this work would be to have a set of filesystem links to the real lease-change script, each with a different name, and configure each dnsmasq to call a unique link. The script then checks argv[0] to find the name it was called by and then transforms that into the name of the corresponding database file.
So we have something like
scripts/interface1 is a link to /lib/libvirt/lease-change- script scripts/interface2 is a link to /lib/libvirt/lease-change-script
and start dnsmasq with
dnsmasq --interface=interface1 --dhcp-script=scripts/interface1
and the script finds the basename of argv[0[:
scripts/interface1 -> interface1
and prepends the directory where the lease files are
interface1 ->leasefiles/interface1
That works in the absence on the DNSMASQ_INTERFACE variable. ""
Is this hack acceptable?
Ping! -- Nehal J Wani
participants (3)
-
Daniel P. Berrange -
Nehal J Wani -
Peter Krempa