Re: [PATCH 0/2] Include lease time option into DHCP settings

On 4/17/20 12:00 PM, Daniel P. Berrangé wrote: The name of the original contributor that you (Julio) referenced in your cover letter sounded familiar, and I tried to find the original BZ for this when I saw your patches on the list, but I failed (bugzilla kept timing out on a *very* basic search term - this seems to happen to 80% of my queries these days...) But then it passed by in mail when Dan was cleaning up all the upstream libvirt BZes and moving them to gitlab :-), so just so everyone has all the background info:   https://bugzilla.redhat.com/show_bug.cgi?id=913446 I also found the other similar patch from Nehal J Wani from around the same time: https://www.redhat.com/archives/libvir-list/2016-September/msg01063.html Without talking about the specifics of either patch, my recollection of the discussion from that time was that both contributors were trying to use a leasetime setting to solve a problem that it wouldn't have solved - their issue was that if a guest was turned off during the time when its lease expired, then when the guest was subsequently restarted it would end up with a different IP address. Of course setting a longer lease expiry would only delay the problem, not eliminate it. Further discussion revealed that if libvirt would just set the "dhcp-authoritative" option in the dnsmasq config, then dnsmasq would attempt to reissue the same IP to a guest even if its lease had already expired - Martin Wilck made this change to libvirt in commit 4ac20b3ae4, which seemed to satisfy the people who had thought they needed to modify the dhcp lease time, and so neither of the lease time patches was pushed upstream. The reason I bring up this old history is just as a cautionary tale that sometimes what you think you need isn't really what you actually need :-) (Recently Cole added the dnsmasq private namespace to the <network> XML, but that is only useful to add an entire option line, and can't do what you need, which is adding an extra option to every dhcp-host and dhcp-range line; there unfortunately is no standalone dnsmasq option to specify a global lease time) Nehal's patch had used this syntax: <leasetime units='hours'>12</leasetime> based on (I guess) the syntax of libvirt's <memory> element:  <memory unit='KiB'>524288</memory> I don't have a preference for any of them, just thought I would point out existing usage in libvirt that has a value/units combination.