Re: [libvirt] [PATCH v4] leaseshelper: improvements to support all events

On 11/18/14 18:16, Nehal J Wani wrote: > This patch enables the helper program to detect event(s) triggered when there > is a change in lease length or expiry and client-id. This transfers complete > control of leases database to libvirt and obsoletes use of the lease database > file (<network-name>.leases). That file will not be created, read, or written. > This is achieved by adding the option --leasefile-ro to dnsmasq and passing a > custom env var to leaseshelper, which helps us map events related to leases > with their corresponding network bridges, no matter what the event be. > > Also, this requires the addition of a new non-lease entry in our custom lease > database: "server-duid". It is required to identify a DHCPv6 server. > > Now that dnsmasq doesn't maintain its own leases database, it relies on our > helper program to tell it about previous leases and server duid. Thus, this > patch makes our leases program honor an extra action: "init", in which it sends > the known info in a particular format to dnsmasq by printing it to stdout. > > --- > > This is compatible with libvirt 1.2.6 as only additions have been > introduced, and the old leases file (*.status) will still be supported. > > v4: * Removed extra data structures for segregating ipv4 and ipv6 leases. > > v3: * Add server-duid as an entry in the lease object for every ipv6 lease. > * Remove unnecessary variables and double copies. > * Take value from DNSMASQ_OLD_HOSTNAME if hostname is not known. > http://www.redhat.com/archives/libvir-list/2014-August/msg00028.html > > v2: http://www.redhat.com/archives/libvir-list/2014-July/msg01109.html > > v1: https://www.redhat.com/archives/libvir-list/2014-July/msg00568.html > > > --- > src/network/bridge_driver.c | 3 + > src/network/leaseshelper.c | 142 +++++++++++++++++++++++++++++++++++--------- > 2 files changed, 117 insertions(+), 28 deletions(-) > > diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c > index 6cb421c..6ecbc37 100644 > --- a/src/network/bridge_driver.c > +++ b/src/network/bridge_driver.c > @@ -1280,7 +1280,10 @@ networkBuildDhcpDaemonCommandLine(virNetworkObjPtr network, > > cmd = virCommandNew(dnsmasqCapsGetBinaryPath(caps)); > virCommandAddArgFormat(cmd, "--conf-file=%s", configfile); > + /* Libvirt gains full control of leases database */ > + virCommandAddArgFormat(cmd, "--leasefile-ro"); One other thing that is worrying is that if you specify this option and the leaseshelper implements the init operation we'd no longer load a possibly pre-existing leases file that was present previous to the upgrade to this handling. This would cause the guests to be re-addressed from the moment of the upgrade. (exactly once). The question here is. Do we care about this slight breakage? I'm not entirely decided yet. I probably wouldn't care. A possible workaround will be to check whether the old leases file exists and refrain from starting with the --leasefile-ro option in that case.