Re: [RFC/PATCH 1/2] network: add dnsmasq option 'dhcp-authoritative'

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 09/21/2016 04:49 AM, Martin Wilck wrote:
The dnsmasq man page recommends that dhcp-authoritative "should be
set when dnsmasq is definitely the only DHCP server on a network".
This is the case for libvirt-managed virtual networks.

The effect of this is that VMs that fail to renew their DHCP lease
in time (e.g. if the VM or host is suspended) will be able to
re-acquire the lease even if it's expired, unless the IP address has
been taken by some other host. This avoids various annoyances caused
by changing VM IP addresses.

We had earlier (finally!) agreed in principle on pushing this patch, but were in freeze at the time, and I later forgot to do it.

I checked to be sure that the dnsmasq on one of the oldest Linux distros that we still support does have the dhcp-authoritative option. It does (it's dnsmasq-2.48-something) so we don't need any extra code to verify the option is there.

I do have a couple comments below, so don't hit the "next" button yet! :-)

---
  src/network/bridge_driver.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 7b99aca..cb4fb1c 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1258,8 +1258,10 @@ networkDnsmasqConfContents(virNetworkObjPtr network,
/* Note: the following is IPv4 only */
          if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET)) {
-            if (ipdef->nranges || ipdef->nhosts)
+            if (ipdef->nranges || ipdef->nhosts) {
                  virBufferAddLit(&configbuf, "dhcp-no-override\n");
+                virBufferAddLit(&configbuf, "dhcp-authoritative\n");
+	    }

You accidentally got a tab character in the line below, so I replaced it with spaces so that make syntax-check passes.

if (ipdef->tftproot) {
                  virBufferAddLit(&configbuf, "enable-tftp\n");


Although your cover letter included diff statistics for the networkxml2conftest test cases that needed the extra line for dhcp-authoritative, those diffs weren't included in the patch itself. Fortunately it's simple to regenerate the test case outputs (VIR_TEST_REGENERATE_OUTPUT=1 tests/networkxml2conftest), so I regenerated and verified them.


ACK, and pushed. Thanks for the submission, and sorry for the delay (and the seemingly protracted debate about a single line - we've been burned in the past by unforeseen consequences of adding a seemingly innocuous dnsmasq option to our dnsmasq conf files, and I was nervous about that happening again, so I wanted to make sure we explored all possibilities.)

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]