10:07 a.m.
I am sorry about the clutter with my multiple patch submissions.
I am just trynig to get it right so that, the next time, I will not make so many stupid
mistakes.
Gene Czarcinski (2):
v3: put dnsmasq parameters into a file instead of the command line
v3: add dnsmasq interface= parameter so bind-interfaces works
src/network/bridge_driver.c | 183 ++++++++++++++-------
src/network/bridge_driver.h | 8 +-
tests/networkxml2argvdata/isolated-network.argv | 25 ++-
.../networkxml2argvdata/nat-network-dns-hosts.argv | 15 +-
.../nat-network-dns-srv-record-minimal.argv | 36 ++--
.../nat-network-dns-srv-record.argv | 36 ++--
.../nat-network-dns-txt-record.argv | 30 ++--
tests/networkxml2argvdata/nat-network.argv | 28 ++--
tests/networkxml2argvdata/netboot-network.argv | 29 ++--
.../networkxml2argvdata/netboot-proxy-network.argv | 26 ++-
tests/networkxml2argvdata/routed-network.argv | 13 +-
tests/networkxml2argvtest.c | 44 +----
12 files changed, 278 insertions(+), 195 deletions(-)
--
1.7.11.7
10:07 a.m.
New subject: [libvirt] [PATCH 1/2] v3: put dnsmasq parameters into a file instead of the command line
This patch changes the way parameters are passed to dnsmasq. They are
put into a conf-file instead of being on the dnsmasq command line.
**NOTE ** This has updated the related tests for the new
data format, etc.
**NOTE** This patch does NOT include specifying interface=
The command line now contains --conf-file=<filename> and a new
parameter --conf-dir=<directoryname> has been added.
The new file and directory are put in the same directory as the
leases file.
---
src/network/bridge_driver.c | 179 ++++++++++++++-------
src/network/bridge_driver.h | 8 +-
tests/networkxml2argvdata/isolated-network.argv | 24 +--
.../networkxml2argvdata/nat-network-dns-hosts.argv | 14 +-
.../nat-network-dns-srv-record-minimal.argv | 35 ++--
.../nat-network-dns-srv-record.argv | 35 ++--
.../nat-network-dns-txt-record.argv | 29 ++--
tests/networkxml2argvdata/nat-network.argv | 27 ++--
tests/networkxml2argvdata/netboot-network.argv | 28 ++--
.../networkxml2argvdata/netboot-proxy-network.argv | 25 +--
tests/networkxml2argvdata/routed-network.argv | 12 +-
tests/networkxml2argvtest.c | 44 +----
12 files changed, 264 insertions(+), 196 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 8837843..ab13df5 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -136,6 +136,16 @@ networkDnsmasqLeaseFileNameFunc networkDnsmasqLeaseFileName =
networkDnsmasqLeaseFileNameDefault;
static char *
+networkDnsmasqConfigFileName(const char *netname)
+{
+ char *conffile;
+
+ ignore_value(virAsprintf(&conffile, DNSMASQ_STATE_DIR "/%s.conf",
+ netname));
+ return conffile;
+}
+
+static char *
networkRadvdPidfileBasename(const char *netname)
{
/* this is simple but we want to be sure it's consistently done */
@@ -559,23 +569,25 @@ networkBuildDnsmasqHostsfile(dnsmasqContext *dctx,
return 0;
}
-
+ /* build the dnsmasq conf file contents */
static int
-networkBuildDnsmasqArgv(virNetworkObjPtr network,
+networkDnsmasqConfContents(virNetworkObjPtr network,
virNetworkIpDefPtr ipdef,
const char *pidfile,
- virCommandPtr cmd,
+ char **configstr,
dnsmasqContext *dctx)
{
- int r, ret = -1;
+ virBuffer configbuf = VIR_BUFFER_INITIALIZER;;
+ int r, ret = -1, ii;
int nbleases = 0;
- int ii;
char *record = NULL;
char *recordPort = NULL;
char *recordWeight = NULL;
char *recordPriority = NULL;
virNetworkIpDefPtr tmpipdef;
+ *configstr = NULL;
+
/*
* NB, be careful about syntax for dnsmasq options in long format.
*
@@ -595,28 +607,22 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
* very explicit on this.
*/
- /*
- * Needed to ensure dnsmasq uses same algorithm for processing
- * multiple namedriver entries in /etc/resolv.conf as GLibC.
- */
- virCommandAddArgList(cmd, "--strict-order", "--bind-interfaces",
NULL);
-
+ /* create dnsmasq config file appropriate for this network */
+ virBufferAsprintf(&configbuf, "# dnsmasq conf file created by
libvirt\n"
+ "strict-order\n"
+ "bind-interfaces\n"
+ "except-interface=lo\n"
+ "domain-needed\n"
+ "local=/%s/\n",
+ network->def->domain ? network->def->domain :
"");
if (network->def->domain)
- virCommandAddArgPair(cmd, "--domain", network->def->domain);
- /* need to specify local even if no domain specified */
- virCommandAddArgFormat(cmd, "--local=/%s/",
- network->def->domain ? network->def->domain :
"");
- virCommandAddArg(cmd, "--domain-needed");
+ virBufferAsprintf(&configbuf,
+ "domain=%s\n"
+ "expand-hosts\n",
+ network->def->domain);
if (pidfile)
- virCommandAddArgPair(cmd, "--pid-file", pidfile);
-
- /* *no* conf file */
- virCommandAddArg(cmd, "--conf-file=");
-
- virCommandAddArgList(cmd,
- "--except-interface", "lo",
- NULL);
+ virBufferAsprintf(&configbuf, "pid-file=%s\n", pidfile);
/* If this is an isolated network, set the default route option
* (3) to be empty to avoid setting a default route that's
@@ -626,16 +632,21 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
* to build a connection to the outside).
*/
if (network->def->forwardType == VIR_NETWORK_FORWARD_NONE) {
- virCommandAddArgList(cmd, "--dhcp-option=3",
- "--no-resolv", NULL);
+ virBufferAsprintf(&configbuf, "dhcp-option=3\n"
+ "no-resolv\n");
}
+ /*
+ * Needed to ensure dnsmasq uses same algorithm for processing
+ * multiple namedriver entries in /etc/resolv.conf as GLibC.
+ */
+
if (network->def->dns != NULL) {
virNetworkDNSDefPtr dns = network->def->dns;
int i;
for (i = 0; i < dns->ntxtrecords; i++) {
- virCommandAddArgFormat(cmd, "--txt-record=%s,%s",
+ virBufferAsprintf(&configbuf, "txt-record=%s,%s\n",
dns->txtrecords[i].name,
dns->txtrecords[i].value);
}
@@ -673,7 +684,7 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
goto cleanup;
}
- virCommandAddArgPair(cmd, "--srv-host", record);
+ virBufferAsprintf(&configbuf, "srv-host=%s\n", record);
VIR_FREE(record);
VIR_FREE(recordPort);
VIR_FREE(recordWeight);
@@ -682,21 +693,14 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
}
}
- /*
- * --interface does not actually work with dnsmasq < 2.47,
- * due to DAD for ipv6 addresses on the interface.
- *
- * virCommandAddArgList(cmd, "--interface", ipdef->bridge, NULL);
- *
- * So listen on all defined IPv[46] addresses
- */
for (ii = 0;
(tmpipdef = virNetworkDefGetIpByIndex(network->def, AF_UNSPEC, ii));
- ii++) {
+ ii++)
+ {
char *ipaddr = virSocketAddrFormat(&tmpipdef->address);
if (!ipaddr)
goto cleanup;
- virCommandAddArgList(cmd, "--listen-address", ipaddr, NULL);
+ virBufferAsprintf(&configbuf, "listen-address=%s\n", ipaddr);
VIR_FREE(ipaddr);
}
@@ -710,8 +714,8 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
VIR_FREE(saddr);
goto cleanup;
}
- virCommandAddArg(cmd, "--dhcp-range");
- virCommandAddArgFormat(cmd, "%s,%s", saddr, eaddr);
+ virBufferAsprintf(&configbuf, "dhcp-range=%s,%s\n",
+ saddr, eaddr);
VIR_FREE(saddr);
VIR_FREE(eaddr);
nbleases += virSocketAddrGetRange(&ipdef->ranges[r].start,
@@ -727,8 +731,7 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
char *bridgeaddr = virSocketAddrFormat(&ipdef->address);
if (!bridgeaddr)
goto cleanup;
- virCommandAddArg(cmd, "--dhcp-range");
- virCommandAddArgFormat(cmd, "%s,static", bridgeaddr);
+ virBufferAsprintf(&configbuf, "dhcp-range=%s,static\n",
bridgeaddr);
VIR_FREE(bridgeaddr);
}
@@ -736,17 +739,13 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
char *leasefile = networkDnsmasqLeaseFileName(network->def->name);
if (!leasefile)
goto cleanup;
- virCommandAddArgFormat(cmd, "--dhcp-leasefile=%s", leasefile);
+ virBufferAsprintf(&configbuf, "dhcp-leasefile=%s\n",
leasefile);
VIR_FREE(leasefile);
- virCommandAddArgFormat(cmd, "--dhcp-lease-max=%d", nbleases);
+ virBufferAsprintf(&configbuf, "dhcp-lease-max=%d\n",
nbleases);
}
if (ipdef->nranges || ipdef->nhosts)
- virCommandAddArg(cmd, "--dhcp-no-override");
-
- /* add domain to any non-qualified hostnames in /etc/hosts or addn-hosts */
- if (network->def->domain)
- virCommandAddArg(cmd, "--expand-hosts");
+ virBufferAsprintf(&configbuf, "dhcp-no-override\n");
if (networkBuildDnsmasqHostsfile(dctx, ipdef, network->def->dns) < 0)
goto cleanup;
@@ -756,38 +755,42 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
* file to allow for runtime additions.
*/
if (ipdef->nranges || ipdef->nhosts)
- virCommandAddArgPair(cmd, "--dhcp-hostsfile",
+ virBufferAsprintf(&configbuf, "dhcp-hostsfile=%s\n",
dctx->hostsfile->path);
/* Likewise, always create this file and put it on the commandline, to allow for
* for runtime additions.
*/
- virCommandAddArgPair(cmd, "--addn-hosts",
+ virBufferAsprintf(&configbuf, "addn-hosts=%s\n",
dctx->addnhostsfile->path);
if (ipdef->tftproot) {
- virCommandAddArgList(cmd, "--enable-tftp",
- "--tftp-root", ipdef->tftproot,
- NULL);
+ virBufferAsprintf(&configbuf, "enable-tftp\n");
+ virBufferAsprintf(&configbuf, "tftp-root=%s\n",
ipdef->tftproot);
}
if (ipdef->bootfile) {
- virCommandAddArg(cmd, "--dhcp-boot");
if (VIR_SOCKET_ADDR_VALID(&ipdef->bootserver)) {
char *bootserver = virSocketAddrFormat(&ipdef->bootserver);
if (!bootserver)
goto cleanup;
- virCommandAddArgFormat(cmd, "%s%s%s",
+ virBufferAsprintf(&configbuf, "dhcp-boot=%s%s%s\n",
ipdef->bootfile, ",,", bootserver);
VIR_FREE(bootserver);
} else {
- virCommandAddArg(cmd, ipdef->bootfile);
+ virBufferAsprintf(&configbuf, "dhcp-boot=%s\n",
ipdef->bootfile);
}
}
}
+ if (!(*configstr = virBufferContentAndReset(&configbuf))) {
+ virReportOOMError();
+ goto cleanup;
+ }
ret = 0;
+
cleanup:
+ virBufferFreeAndReset(&configbuf);
VIR_FREE(record);
VIR_FREE(recordPort);
VIR_FREE(recordWeight);
@@ -795,13 +798,18 @@ cleanup:
return ret;
}
+ /* build the dnsmasq command line */
int
networkBuildDhcpDaemonCommandLine(virNetworkObjPtr network, virCommandPtr *cmdout,
- char *pidfile, dnsmasqContext *dctx)
+ char *pidfile, dnsmasqContext *dctx,
+ char *configdir,
+ int testOnly, char **testConfigStr)
{
virCommandPtr cmd = NULL;
int ret = -1, ii;
virNetworkIpDefPtr ipdef;
+ char *configfile = NULL;
+ char *configstr = NULL;
network->dnsmasqPid = -1;
@@ -825,15 +833,41 @@ networkBuildDhcpDaemonCommandLine(virNetworkObjPtr network,
virCommandPtr *cmdou
if (!virNetworkDefGetIpByIndex(network->def, AF_UNSPEC, 0))
return 0;
- cmd = virCommandNew(DNSMASQ);
- if (networkBuildDnsmasqArgv(network, ipdef, pidfile, cmd, dctx) < 0) {
+ if (networkDnsmasqConfContents(network, ipdef, pidfile, &configstr, dctx) <
0)
+ goto cleanup;
+ if (!configstr)
+ goto cleanup;
+
+ if (testOnly) {
+ *testConfigStr = configstr;
+ return 0;
+ }
+
+ /* construct the filename */
+ if (!(configfile = networkDnsmasqConfigFileName(network->def->name))) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ /* Write the file */
+ if (virFileWriteStr(configfile, configstr, 0600) < 0) {
+ virReportSystemError(errno,
+ _("couldn't write dnsmasq config file
'%s'"),
+ configfile);
goto cleanup;
}
+ VIR_INFO("dnsmasq conf file %s written", configfile);
+
+ cmd = virCommandNew(DNSMASQ);
+ virCommandAddArgFormat(cmd, "--conf-file=%s", configfile);
+ virCommandAddArgFormat(cmd, "--conf-dir=%s", configdir);
if (cmdout)
*cmdout = cmd;
ret = 0;
cleanup:
+ VIR_FREE(configstr);
+ VIR_FREE(configfile);
if (ret < 0)
virCommandFree(cmd);
return ret;
@@ -844,9 +878,12 @@ networkStartDhcpDaemon(virNetworkObjPtr network)
{
virCommandPtr cmd = NULL;
char *pidfile = NULL;
+ char *testconfigstr = NULL;
+ char *configdir = NULL;
int ret = -1;
dnsmasqContext *dctx = NULL;
+ VIR_INFO("starting dhcp daemon (dnsmasq)");
if (!virNetworkDefGetIpByIndex(network->def, AF_UNSPEC, 0)) {
/* no IPv6 addresses, so we don't need to run radvd */
ret = 0;
@@ -882,7 +919,18 @@ networkStartDhcpDaemon(virNetworkObjPtr network)
if (dctx == NULL)
goto cleanup;
- ret = networkBuildDhcpDaemonCommandLine(network, &cmd, pidfile, dctx);
+ ignore_value(virAsprintf(&configdir, DNSMASQ_STATE_DIR "/%s.d",
+ network->def->name));
+ if (virFileMakePath(configdir) < 0) {
+ virReportSystemError(errno,
+ _("cannot create directory %s"),
+ configdir);
+ goto cleanup;
+ }
+
+
+ ret = networkBuildDhcpDaemonCommandLine(network, &cmd, pidfile, dctx,
+ configdir, 0, &testconfigstr);
if (ret < 0)
goto cleanup;
@@ -911,6 +959,7 @@ networkStartDhcpDaemon(virNetworkObjPtr network)
ret = 0;
cleanup:
VIR_FREE(pidfile);
+ VIR_FREE(configdir);
virCommandFree(cmd);
dnsmasqContextFree(dctx);
return ret;
@@ -2841,6 +2890,14 @@ static int networkUndefine(virNetworkPtr net) {
}
}
+ {
+ char *configfile = networkDnsmasqConfigFileName(network->def->name);
+ if (!configfile)
+ goto cleanup;
+ unlink(configfile);
+ VIR_FREE(configfile);
+ }
+
if (dhcp_present) {
char *leasefile;
dnsmasqContext *dctx = dnsmasqContextNew(network->def->name,
DNSMASQ_STATE_DIR);
diff --git a/src/network/bridge_driver.h b/src/network/bridge_driver.h
index 0fae275..00675c4 100644
--- a/src/network/bridge_driver.h
+++ b/src/network/bridge_driver.h
@@ -48,15 +48,17 @@ int networkGetNetworkAddress(const char *netname, char **netaddr)
int networkBuildDhcpDaemonCommandLine(virNetworkObjPtr network,
virCommandPtr *cmdout, char *pidfile,
- dnsmasqContext *dctx)
- ;
+ dnsmasqContext *dctx,
+ char *configdir,
+ int testOnly, char **testConfigStr);
# else
/* Define no-op replacements that don't drag in any link dependencies. */
# define networkAllocateActualDevice(iface) 0
# define networkNotifyActualDevice(iface) 0
# define networkReleaseActualDevice(iface) 0
# define networkGetNetworkAddress(netname, netaddr) (-2)
-# define networkBuildDhcpDaemonCommandLine(network, cmdout, pidfile, dctx) 0
+# define networkBuildDhcpDaemonCommandLine(network, cmdout, pidfile, \
+ dctx, configdir, testOnly, testConfigStr) 0
# endif
typedef char *(*networkDnsmasqLeaseFileNameFunc)(const char *netname);
diff --git a/tests/networkxml2argvdata/isolated-network.argv
b/tests/networkxml2argvdata/isolated-network.argv
index 13e77b2..042158b 100644
--- a/tests/networkxml2argvdata/isolated-network.argv
+++ b/tests/networkxml2argvdata/isolated-network.argv
@@ -1,9 +1,15 @@
-@DNSMASQ@ --strict-order --bind-interfaces \
---local=// --domain-needed --conf-file= \
---except-interface lo --dhcp-option=3 --no-resolv \
---listen-address 192.168.152.1 \
---dhcp-range 192.168.152.2,192.168.152.254 \
---dhcp-leasefile=/var/lib/libvirt/dnsmasq/private.leases --dhcp-lease-max=253 \
---dhcp-no-override \
---dhcp-hostsfile=/var/lib/libvirt/dnsmasq/private.hostsfile \
---addn-hosts=/var/lib/libvirt/dnsmasq/private.addnhosts\
+# dnsmasq conf file created by libvirt
+strict-order
+bind-interfaces
+except-interface=lo
+domain-needed
+local=//
+dhcp-option=3
+no-resolv
+listen-address=192.168.152.1
+dhcp-range=192.168.152.2,192.168.152.254
+dhcp-leasefile=/var/lib/libvirt/dnsmasq/private.leases
+dhcp-lease-max=253
+dhcp-no-override
+dhcp-hostsfile=/var/lib/libvirt/dnsmasq/private.hostsfile
+addn-hosts=/var/lib/libvirt/dnsmasq/private.addnhosts
diff --git a/tests/networkxml2argvdata/nat-network-dns-hosts.argv
b/tests/networkxml2argvdata/nat-network-dns-hosts.argv
index 03a0676..91eb682 100644
--- a/tests/networkxml2argvdata/nat-network-dns-hosts.argv
+++ b/tests/networkxml2argvdata/nat-network-dns-hosts.argv
@@ -1,4 +1,10 @@
-@DNSMASQ@ --strict-order --bind-interfaces --domain=example.com \
---local=/example.com/ --domain-needed \
---conf-file= --except-interface lo --listen-address 192.168.122.1 \
---expand-hosts --addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts\
+# dnsmasq conf file created by libvirt
+strict-order
+bind-interfaces
+except-interface=lo
+domain-needed
+local=/example.com/
+domain=example.com
+expand-hosts
+listen-address=192.168.122.1
+addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts
diff --git a/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv
b/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv
index 210a60c..d92497b 100644
--- a/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv
+++ b/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv
@@ -1,17 +1,18 @@
-@DNSMASQ@ \
---strict-order \
---bind-interfaces \
---local=// --domain-needed --conf-file= \
---except-interface lo \
---srv-host=name.tcp.,,,, \
---listen-address 192.168.122.1 \
---listen-address 192.168.123.1 \
---listen-address 2001:db8:ac10:fe01::1 \
---listen-address 2001:db8:ac10:fd01::1 \
---listen-address 10.24.10.1 \
---dhcp-range 192.168.122.2,192.168.122.254 \
---dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases \
---dhcp-lease-max=253 \
---dhcp-no-override \
---dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile \
---addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts\
+# dnsmasq conf file created by libvirt
+strict-order
+bind-interfaces
+except-interface=lo
+domain-needed
+local=//
+srv-host=name.tcp.,,,,
+listen-address=192.168.122.1
+listen-address=192.168.123.1
+listen-address=2001:db8:ac10:fe01::1
+listen-address=2001:db8:ac10:fd01::1
+listen-address=10.24.10.1
+dhcp-range=192.168.122.2,192.168.122.254
+dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases
+dhcp-lease-max=253
+dhcp-no-override
+dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile
+addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts
diff --git a/tests/networkxml2argvdata/nat-network-dns-srv-record.argv
b/tests/networkxml2argvdata/nat-network-dns-srv-record.argv
index 833d3cd..d8846c2 100644
--- a/tests/networkxml2argvdata/nat-network-dns-srv-record.argv
+++ b/tests/networkxml2argvdata/nat-network-dns-srv-record.argv
@@ -1,17 +1,18 @@
-@DNSMASQ@ \
---strict-order \
---bind-interfaces \
---local=// --domain-needed --conf-file= \
---except-interface lo \
---srv-host=name.tcp.test-domain-name,.,1024,10,10 \
---listen-address 192.168.122.1 \
---listen-address 192.168.123.1 \
---listen-address 2001:db8:ac10:fe01::1 \
---listen-address 2001:db8:ac10:fd01::1 \
---listen-address 10.24.10.1 \
---dhcp-range 192.168.122.2,192.168.122.254 \
---dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases \
---dhcp-lease-max=253 \
---dhcp-no-override \
---dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile \
---addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts\
+# dnsmasq conf file created by libvirt
+strict-order
+bind-interfaces
+except-interface=lo
+domain-needed
+local=//
+srv-host=name.tcp.test-domain-name,.,1024,10,10
+listen-address=192.168.122.1
+listen-address=192.168.123.1
+listen-address=2001:db8:ac10:fe01::1
+listen-address=2001:db8:ac10:fd01::1
+listen-address=10.24.10.1
+dhcp-range=192.168.122.2,192.168.122.254
+dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases
+dhcp-lease-max=253
+dhcp-no-override
+dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile
+addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts
diff --git a/tests/networkxml2argvdata/nat-network-dns-txt-record.argv
b/tests/networkxml2argvdata/nat-network-dns-txt-record.argv
index 3481507..bf00513 100644
--- a/tests/networkxml2argvdata/nat-network-dns-txt-record.argv
+++ b/tests/networkxml2argvdata/nat-network-dns-txt-record.argv
@@ -1,11 +1,18 @@
-@DNSMASQ@ --strict-order --bind-interfaces \
---local=// --domain-needed --conf-file= \
---except-interface lo '--txt-record=example,example value' \
---listen-address 192.168.122.1 --listen-address 192.168.123.1 \
---listen-address 2001:db8:ac10:fe01::1 \
---listen-address 2001:db8:ac10:fd01::1 --listen-address 10.24.10.1 \
---dhcp-range 192.168.122.2,192.168.122.254 \
---dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases \
---dhcp-lease-max=253 --dhcp-no-override \
---dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile \
---addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts\
+# dnsmasq conf file created by libvirt
+strict-order
+bind-interfaces
+except-interface=lo
+domain-needed
+local=//
+txt-record=example,example value
+listen-address=192.168.122.1
+listen-address=192.168.123.1
+listen-address=2001:db8:ac10:fe01::1
+listen-address=2001:db8:ac10:fd01::1
+listen-address=10.24.10.1
+dhcp-range=192.168.122.2,192.168.122.254
+dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases
+dhcp-lease-max=253
+dhcp-no-override
+dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile
+addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts
diff --git a/tests/networkxml2argvdata/nat-network.argv
b/tests/networkxml2argvdata/nat-network.argv
index 37fd2fc..d542bbc 100644
--- a/tests/networkxml2argvdata/nat-network.argv
+++ b/tests/networkxml2argvdata/nat-network.argv
@@ -1,10 +1,17 @@
-@DNSMASQ@ --strict-order --bind-interfaces \
---local=// --domain-needed --conf-file= \
---except-interface lo --listen-address 192.168.122.1 \
---listen-address 192.168.123.1 --listen-address 2001:db8:ac10:fe01::1 \
---listen-address 2001:db8:ac10:fd01::1 --listen-address 10.24.10.1 \
---dhcp-range 192.168.122.2,192.168.122.254 \
---dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases \
---dhcp-lease-max=253 --dhcp-no-override \
---dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile \
---addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts\
+# dnsmasq conf file created by libvirt
+strict-order
+bind-interfaces
+except-interface=lo
+domain-needed
+local=//
+listen-address=192.168.122.1
+listen-address=192.168.123.1
+listen-address=2001:db8:ac10:fe01::1
+listen-address=2001:db8:ac10:fd01::1
+listen-address=10.24.10.1
+dhcp-range=192.168.122.2,192.168.122.254
+dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases
+dhcp-lease-max=253
+dhcp-no-override
+dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile
+addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts
diff --git a/tests/networkxml2argvdata/netboot-network.argv
b/tests/networkxml2argvdata/netboot-network.argv
index 5408eb7..4f5fedd 100644
--- a/tests/networkxml2argvdata/netboot-network.argv
+++ b/tests/networkxml2argvdata/netboot-network.argv
@@ -1,10 +1,18 @@
-@DNSMASQ@ --strict-order --bind-interfaces --domain=example.com \
---local=/example.com/ --domain-needed --conf-file= \
---except-interface lo --listen-address 192.168.122.1 \
---dhcp-range 192.168.122.2,192.168.122.254 \
---dhcp-leasefile=/var/lib/libvirt/dnsmasq/netboot.leases \
---dhcp-lease-max=253 --dhcp-no-override --expand-hosts \
---dhcp-hostsfile=/var/lib/libvirt/dnsmasq/netboot.hostsfile \
---addn-hosts=/var/lib/libvirt/dnsmasq/netboot.addnhosts \
---enable-tftp \
---tftp-root /var/lib/tftproot --dhcp-boot pxeboot.img\
+# dnsmasq conf file created by libvirt
+strict-order
+bind-interfaces
+except-interface=lo
+domain-needed
+local=/example.com/
+domain=example.com
+expand-hosts
+listen-address=192.168.122.1
+dhcp-range=192.168.122.2,192.168.122.254
+dhcp-leasefile=/var/lib/libvirt/dnsmasq/netboot.leases
+dhcp-lease-max=253
+dhcp-no-override
+dhcp-hostsfile=/var/lib/libvirt/dnsmasq/netboot.hostsfile
+addn-hosts=/var/lib/libvirt/dnsmasq/netboot.addnhosts
+enable-tftp
+tftp-root=/var/lib/tftproot
+dhcp-boot=pxeboot.img
diff --git a/tests/networkxml2argvdata/netboot-proxy-network.argv
b/tests/networkxml2argvdata/netboot-proxy-network.argv
index 21e01e3..8b9c03a 100644
--- a/tests/networkxml2argvdata/netboot-proxy-network.argv
+++ b/tests/networkxml2argvdata/netboot-proxy-network.argv
@@ -1,9 +1,16 @@
-@DNSMASQ@ --strict-order --bind-interfaces --domain=example.com \
---local=/example.com/ --domain-needed --conf-file= \
---except-interface lo --listen-address 192.168.122.1 \
---dhcp-range 192.168.122.2,192.168.122.254 \
---dhcp-leasefile=/var/lib/libvirt/dnsmasq/netboot.leases \
---dhcp-lease-max=253 --dhcp-no-override --expand-hosts \
---dhcp-hostsfile=/var/lib/libvirt/dnsmasq/netboot.hostsfile \
---addn-hosts=/var/lib/libvirt/dnsmasq/netboot.addnhosts \
---dhcp-boot pxeboot.img,,10.20.30.40\
+# dnsmasq conf file created by libvirt
+strict-order
+bind-interfaces
+except-interface=lo
+domain-needed
+local=/example.com/
+domain=example.com
+expand-hosts
+listen-address=192.168.122.1
+dhcp-range=192.168.122.2,192.168.122.254
+dhcp-leasefile=/var/lib/libvirt/dnsmasq/netboot.leases
+dhcp-lease-max=253
+dhcp-no-override
+dhcp-hostsfile=/var/lib/libvirt/dnsmasq/netboot.hostsfile
+addn-hosts=/var/lib/libvirt/dnsmasq/netboot.addnhosts
+dhcp-boot=pxeboot.img,,10.20.30.40
diff --git a/tests/networkxml2argvdata/routed-network.argv
b/tests/networkxml2argvdata/routed-network.argv
index 9fedb2b..ad9e121 100644
--- a/tests/networkxml2argvdata/routed-network.argv
+++ b/tests/networkxml2argvdata/routed-network.argv
@@ -1,4 +1,8 @@
-@DNSMASQ@ --strict-order --bind-interfaces \
---local=// --domain-needed --conf-file= \
---except-interface lo --listen-address 192.168.122.1 \
---addn-hosts=/var/lib/libvirt/dnsmasq/local.addnhosts\
+# dnsmasq conf file created by libvirt
+strict-order
+bind-interfaces
+except-interface=lo
+domain-needed
+local=//
+listen-address=192.168.122.1
+addn-hosts=/var/lib/libvirt/dnsmasq/local.addnhosts
diff --git a/tests/networkxml2argvtest.c b/tests/networkxml2argvtest.c
index 87519e4..78ac8cf 100644
--- a/tests/networkxml2argvtest.c
+++ b/tests/networkxml2argvtest.c
@@ -15,37 +15,6 @@
#include "memory.h"
#include "network/bridge_driver.h"
-/* Replace all occurrences of @token in @buf by @replacement and adjust size of
- * @buf accordingly. Returns 0 on success and -1 on out-of-memory errors. */
-static int replaceTokens(char **buf, const char *token, const char *replacement) {
- size_t token_start, token_end;
- size_t buf_len, rest_len;
- const size_t token_len = strlen(token);
- const size_t replacement_len = strlen(replacement);
- const int diff = replacement_len - token_len;
-
- buf_len = rest_len = strlen(*buf) + 1;
- token_end = 0;
- for (;;) {
- char *match = strstr(*buf + token_end, token);
- if (match == NULL)
- break;
- token_start = match - *buf;
- rest_len -= token_start + token_len - token_end;
- token_end = token_start + token_len;
- buf_len += diff;
- if (diff > 0)
- if (VIR_REALLOC_N(*buf, buf_len) < 0)
- return -1;
- if (diff != 0)
- memmove(*buf + token_end + diff, *buf + token_end, rest_len);
- memcpy(*buf + token_start, replacement, replacement_len);
- token_end += diff;
- }
- /* if diff < 0, we could shrink the buffer here... */
- return 0;
-}
-
static int testCompareXMLToArgvFiles(const char *inxml, const char *outargv) {
char *inXmlData = NULL;
char *outArgvData = NULL;
@@ -55,6 +24,7 @@ static int testCompareXMLToArgvFiles(const char *inxml, const char
*outargv) {
virNetworkObjPtr obj = NULL;
virCommandPtr cmd = NULL;
char *pidfile = NULL;
+ char *configdir = NULL;
dnsmasqContext *dctx = NULL;
if (virtTestLoadFile(inxml, &inXmlData) < 0)
@@ -62,10 +32,6 @@ static int testCompareXMLToArgvFiles(const char *inxml, const char
*outargv) {
if (virtTestLoadFile(outargv, &outArgvData) < 0)
goto fail;
-
- if (replaceTokens(&outArgvData, "@DNSMASQ@", DNSMASQ))
- goto fail;
-
if (!(dev = virNetworkDefParseString(inXmlData)))
goto fail;
@@ -78,12 +44,9 @@ static int testCompareXMLToArgvFiles(const char *inxml, const char
*outargv) {
if (dctx == NULL)
goto fail;
- if (networkBuildDhcpDaemonCommandLine(obj, &cmd, pidfile, dctx) < 0)
- goto fail;
-
- if (!(actual = virCommandToString(cmd)))
+ if (networkBuildDhcpDaemonCommandLine(obj, &cmd, pidfile,
+ dctx, configdir, 1, &actual) < 0)
goto fail;
-
if (STRNEQ(outArgvData, actual)) {
virtTestDifference(stderr, outArgvData, actual);
goto fail;
@@ -147,7 +110,6 @@ mymain(void)
if (virtTestRun("Network XML-2-Argv " name, \
1, testCompareXMLToArgvHelper, (name)) < 0) \
ret = -1
-
DO_TEST("isolated-network");
DO_TEST("routed-network");
DO_TEST("nat-network");
--
1.7.11.7
11:01 a.m.
New subject: [libvirt] [PATCH 1/2] v3: put dnsmasq parameters into a file instead of the command line
On 10/23/2012 11:07 AM, Gene Czarcinski wrote:
This patch changes the way parameters are passed to dnsmasq. They
are
put into a conf-file instead of being on the dnsmasq command line.
I was thinking about this last night after I learned from you that the
conf file *isn't* reread when dnsmasq received SIGHUP. That being the
case, what are the other reasons for switching from commandline to conf
file? I suppose one is that it eliminates clutter in the ps output (and
gets it away from prying eyes (since anyone can see the commandline by
looking in the output of ps -AlF). Another would be it avoids hitting
the commandline limit (which is rather large, but still exists) when
there are a lot of srv and txt entries. Anything else?
11:57 a.m.
New subject: [libvirt] [PATCH 1/2] v3: put dnsmasq parameters into a file instead of the command line
On 10/23/2012 12:01 PM, Laine Stump wrote:
Not really. Maybe there is also the fact that I believe this is a
"cleaner," but not necessarily better, approach.
There is also the possibility that Simon could be convinced to add the
capability of re-reading configuration files. I will ask to see what
his reaction is. But, his reaction may be: "Sure, go ahead and do that;
I look forward to your patches." I am not sure I want to wander in that
swamp right now.
The most important thing to me is adding the --conf-dir=<directory>.
This will allow debugging different parameters without having to
recompile/rebuild the entire libvirt set of packages. Now, when I first
did this, I thought that the configuration files in the directory would
be re-read. When it did not do that, and, upon doing a little research,
found that it was working as designed.
Oh, BTW, there is one other little thing about the command line
parameters. To support IPv6 completely, the should be
local=/<reverse-ip6-addr>.ip6.arpa/ parameters and these are very long.
Putting things into a conf-file just makes it (IMHO) a lot simpler to
understand.
I do realize that this is open source and I can always "do my own thing"
and continue using the patches myself. However, that can mean a lot of
work just keeping up.
One last observation/question: Why not?
Gene
<snip>
12:13 p.m.
New subject: [libvirt] [PATCH 1/2] v3: put dnsmasq parameters into a file instead of the command line
On 10/23/2012 12:57 PM, Gene Czarcinski wrote:
Sure. I think it's a net gain. It's just that my main reason for wanting
to do this switch was to avoid restarting dnsmasq for every little
change (mainly because that's one of the things I've been working on
lately - virNetworkUpdate()), and when I saw that carrot go away, I
needed a sanity check to rationalize the change.
One last observation/question: Why not?
I often find myself falling into the hole of making change for change's
sake, which leads to cleaner code, but makes backporting fixes to
earlier releases more difficult. I need to constantly watch myself for
too much of this behavior (some amount is good, as it does lean to
cleaner code, but there is such a thing as too much of anything).
I tried your earlier --interface patch on F17 last night, and built it
on F16 and RHEL6 but didn't get a chance to test it. So instead I'm
going to try a build with both of these patches on the three platforms
today and (hopefully) push them both prior to the 1.0.0 freeze late
tonight. That way they'll have a reasonable amount of testing, and will
get even more (and especially on other distros) during the freeze, with
time enough to tweak (or even back out if necessary) before 1.0.0.
2:24 p.m.
New subject: [libvirt] [PATCH 1/2] v3: put dnsmasq parameters into a file instead of the command line
On 10/23/2012 01:13 PM, Laine Stump wrote:
OK, there are
"branches" at v0.10.2-maint, v0.9.11-maint, and
v0.9.6-maint. Would it be useful to attecmp backporting any of this
stuff to those "levels"?
Here is what I sent to the dnsmasq discussion list:
Can you provide a bit more info as to what you would like to change
without restarting dnsmasq.
Gene
3:10 p.m.
New subject: [libvirt] [PATCH 1/2] v3: put dnsmasq parameters into a file instead of the command line
On 10/23/2012 03:24 PM, Gene Czarcinski wrote:
I wouldn't worry about that quite yet. Let's wait until it's pushed
upstream. At the point, we'll probably want the first two (for F17 and
F18, which have dnsmasq-2.63 which according to you causes problems).
Right now, the ranges of IP addresses, the IP addresses to listen on,
the domain. There may be other things in the future as
virNetworkUpdate() gets fleshed out.
3:55 p.m.
New subject: [libvirt] [PATCH 1/2] v3: put dnsmasq parameters into a file instead of the command line
On 10/23/2012 04:10 PM, Laine Stump wrote:
Not me, Simon
Kelley the dnsmasq developer/maintainer/etc.
Rather than just pasting his comment here, got to look at the message he
wrote:
http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2012q4/006415.html
There might be a way to make it work with just the gateway address (that
is what listen-address really is because dnsmasq does not really need an
address just the interface), but that is not how it was done.
Gene
6:23 a.m.
New subject: [libvirt] A Big OOPS!!
A big OOPS!!!
On 10/23/2012 04:55 PM, Gene Czarcinski wrote:
After I sent
the message, I just got something in from Simon Kelley
which has some new info:
http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2012q4/006445.html
The heart of it is this:
--------------------------------------------------
I disagree. I
believe that the problem still exists.
What Simon says implies that everything is OK and nothing needs to be
done but consider this:
1. What harm does it do to add the interface=<> specification in
addition to everything else?
2. Note that Simon states "Setting --bind-interfaces* and a single
--listen-address ". Well, I can define multiple IPv4 and/or IPv6
listen-addresses to be on a single virtual interface. From what Simon
says, that means all bets are off.
3. I suspect that many/most instances of dnsmasq only has a single
address and that is why the problem has not manifested itself.
4. I do not know if a v4 and v6 address counts as one or two.
Gene
10:49 a.m.
New subject: [libvirt] A Big OOPS!!
On 10/24/2012 07:23 AM, Gene Czarcinski wrote:
And one more round on this ...
http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2012q4/006447.html
--------------------------------------------
--------------------------------------------------------
So, interface= is NOT necessary and everything is working just fine
using only --listen-address=
I still believe that specifying interface= rather than the (possibly
multiple) --list-address= is just plain cleaner but ... what now exists
works.
The problem only occurs when you do something that libvirt's use of
dnsmasq does not do ... have multiple dnsmasq instances each with
multiple interfaces and only using --listen-address.
So ... put it in ... don't put it in ... your options.
Gene
6:46 a.m.
New subject: [libvirt] [PATCH 1/2] v3: put dnsmasq parameters into a file instead of the command line
On 10/23/2012 04:10 PM, Laine Stump wrote:
http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2012q4/006440.html
Gene
6:56 a.m.
New subject: [libvirt] [PATCH 1/2] v3: put dnsmasq parameters into a file instead of the command line
On 10/24/2012 07:46 AM, Gene Czarcinski wrote:
One other thought occurs to me.
As far as I know, the only way to get dnsmasq restarted is to do a
net-destroy and then a net-start. While you can do this while a virtual
guest is on that network, the virtual guest has to be rebooted for it to
work again.
Is there some way (some command) that will cause dnsmasq to be restarted
(possibly with new parameters as it re-does its configuration). If this
was done, then little, except some cached names, would be lost). If
there is not, maybe there should be. However, there are likely limits
such as not changing the gateway addresses on the interface.
Gene
Gene
12:31 p.m.
New subject: [libvirt] [PATCH 1/2] v3: put dnsmasq parameters into a file instead of the command line
On 10/24/2012 07:56 AM, Gene Czarcinski wrote:
Well, you could indirectly trigger it by adding (and then removing) a
dhcp <range> element with virsh net-update.
I recently made a patch to check for a running dnsmasq anytime libvirt
is restarted. If it's not running, it will be restarted. If it is
running, a SIGHUP will be sent.
Aside from that, the (newly created) accepted way to change a network's
config while the network is up is to use the virNetworkUpdate API
(available via virsh net-update). It permits you to add/delete/modify
certain parts of the network config and have those changes take effect
immediately if desired. the bridge_driver backend of virNetworkUpdate
decides when it is necessary to either SIGHUP or restart dnsmasq
according to what parts of the network definition have changed.
A "live" change of the entire network definition (i.e. with
virNetworkDefine() isn't supported, and won't be - anything that you
want to be able to change while the network is up should be added to the
"sections" modifiable by virNetworkUpdate. This will permit us to easily
figure out what re-initialization is needed for a particular change
without needing to just redo everything (for example, if a dhcp static
host is added, we just need to SIGHUP dnsmasq, but if a dynamic range is
added/deleted, we need to restart dnsmasq. If an IP address is
added/removed from the bridge, we should be able to write code to simply
redo the IP addresses on the existing bridge, rather than tear it down
and build a new bridge (although that one isn't implemented yet).
3:27 p.m.
New subject: [libvirt] [PATCH 1/2] v3: put dnsmasq parameters into a file instead of the command line
On 10/24/2012 01:31 PM, Laine Stump wrote:
Now this
sounds like pretty much what I wanted. It is OK to restart
dnsmasq but not OK to take the network down and then back up while a
virtual guest is using it. Changing dhcp-range, hosts, or dhcp-host
should be no problem because the network interface remains.
That said, there is something critical and that is the "gateway"
addresses. Other software (not dsnmasq) needs to know these addresses
because they are really the interface between reality and the virtual
network. That should not be able to be changed without taking down the
network.
Dnsmasq does not need the listen-address specifications if it is given
the drive name. It will then listen on port 53 for all networks defined
on the interface. For dhcp dnsmasq listens on 0.0.0.0:67/68 for v4 and
:::547 for v6. It then filers any incoming packet to make sure that a
dhcp-range has been specified for a subnet defined on that interface.
I know it is supported [or at least it works] but I am a little fuzzy
about the usefulness of multiple IP addresses on the same interface
(other than IPv4 and IPv6).
Gene
9:36 a.m.
New subject: [libvirt] [PATCH 1/2] v3: put dnsmasq parameters into a file instead of the command line
On 10/24/2012 04:27 PM, Gene Czarcinski wrote:
Depends. For the guests, all you need to do is set the linkstate down,
then back up; for most OSes that causes the dhcp client to request a new
lease. (really, we should probably do the same thing (at least
optionally) when changing the range and the static hosts list, if we
want to guarantee that all the guests are always obeying those settings.
Is that 100% true? Awhile back someone filed a bug against libvirt
saying that, because they had incorrectly specified the dynamic range to
include the broadcast address of the subnet, they were getting a client
that had been assigned the broadcast address as its IP.
Well, I don't think it has much effect for dhcp (maybe it does when a
client sends a unicast dhcp renew request (if I'm remembering dhcp
correctly - it's been awhile since I looked at the details :-), but for
DNS it will respond to DNS requests on any of those addresses.
Or are you talking about having multiple IPs in general? I guess it
makes it possible for multiple subnets to share the same infrastructure
(don't know how useful that is when the infrastructure is virtual :-).
Anyway, if something works, someone will figure out a practical use for it.
2:13 p.m.
New subject: [libvirt] [PATCH 1/2] v3: put dnsmasq parameters into a file instead of the command line
On 10/26/2012 10:36 AM, Laine Stump wrote:
The darn corner cases
always cause problems. This might even be a bug
with dnsmasq ... it should at least gripe about any dhcp-range with
includes the broadcast address.
If you specify the
interface, yes dnsmasq should (and does) look at all
of the subnetworks. However, I do not know is that is true if you use
listen-address. I will try to remember to test that.
It is
possible to run more than one subnet on the same fabric ... real
or virtual. There might even be some real-world application where you
want to separate a control network from the regular old (Internet
accessible) network. Of course, anyone who did not encrypt that second
network odes not have all of their oars in the water ;)
And very real world application for running multiple networks on the
same fabric (the same interface) is: dual stack IPv4 and IPv6.
Gene
1:24 p.m.
New subject: [libvirt] problem trying to use net-update
On 10/24/2012 01:31 PM, Laine Stump wrote:
I finally got
around to testing this .... how is this suppose to work?
Is this a "work in progress?"
I did not try this first but it is what seemed to work ...
I edited a network definition so that it space for a dhcp-range
addition. Then, with the network not started, i used net-update to add
a ip-dhcp-range and it worked (the file was updated). Edit the xml
again to remove the added range, and start the network. Do the same
thing again with net-update ... the file was updated and dnsmasq was
restarted.
What I started out to do and what did not work was to add a dns-host
definition to a network. This should not require a restart but result a
[Refresh] SIGHUP of the dnsmasq process.
1. Using net-edit to add the dns-host definition [resulted in
networkCreate() being executed] and then issuing SIGHUP to the process
and the addnhosts file was read and things worked as I wanted them to.
2. Trying to get the same thing accomplished using net-update
[dns-host] was a failure. The only thing that happened was to get the
following error message: "this function is not supported by the
connection driver: can't update 'dns host' section of network
'default'" where I tried a bunch of different networks. Some of them
already had a dns-host definition and some did not.
Should I expect this to work? Is this a bug?
Gene
4:22 p.m.
New subject: [libvirt] [PATCH 1/2] v3: put dnsmasq parameters into a file instead of the command line
On 10/23/2012 11:07 AM, Gene Czarcinski wrote:
And the idea here is that people can add stuff into that directory if
they want to frob with the dnsmasq config?
I can see this leading to support nightmares if we don't somehow mark
the system as "tainted" any time there is a file in there (similar to
what is done if qemu commandline extensions are used).
Aside from that, and the small bits I mention below, this looks fine,
and it's so far worked properly (with the --interface patch included) on
F17 (dnsmasq-2.63) and F16 (dnsmasq-2.59), and RHEL6 (dnsmasq-2.48). I
unfortunately don't have RHEL5 on real hardware to try out.
I would like to push this (and --interface) before the freeze, but am
uneasy about the conf-dir change. Can we split that into its own patch
too, and consider it separately? (some people may want to weigh in on
it, and we may want to require)
First some review of code that *isn't* there - it would be nice if
networkRefreshDhcpDaemon() checked for the existence of the conf file
and, if it wasn't there, called networkRestartDhcpDaemon() instead.
Thinking about it, though, I suppose it's not really necessary, since
the existing dnsmasq process should behave identically to the new one.
(that won't be the case when the --interface option is added though. I'm
not sure how to deal with that; I suppose we can just tell people
they'll have to restart their networks in order to get that fix.)
Extra trailing space on the line above. You should run "make
syntax-check" before sending patches, and fix anything it complains about.
The "virBufferAsprintf" line has a trailing blank. Also the if has a
multiline body, so it requires {} around it per the coding guildelines.
This comment shouldn't be removed until --interface is added in (and
even then, it should be left, somewhat modified, for
historical/informational purposes).
You have an extra trailing space on the line above. Would have been
found with "make syntax-check".
10:07 a.m.
New subject: [libvirt] [PATCH 2/2] v3: add dnsmasq interface= parameter so bind-interfaces works
The problem is that, without interface=, bind-interfaces does not
work. Bind-interfaces uses SO_BINDTODEVICE. Unless this in
in effect, there is no guarantee that the kernel will route
DHCP (v4 or v6) packets to the correct instance of dnsmasq,
when there is more than one.
**NOTE** This patch assumes that the patch to put dnsmasq parameters
into a file instead of the command line, has been applied.
---
src/network/bridge_driver.c | 6 ++++++
tests/networkxml2argvdata/isolated-network.argv | 1 +
tests/networkxml2argvdata/nat-network-dns-hosts.argv | 1 +
tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv | 1 +
tests/networkxml2argvdata/nat-network-dns-srv-record.argv | 1 +
tests/networkxml2argvdata/nat-network-dns-txt-record.argv | 1 +
tests/networkxml2argvdata/nat-network.argv | 1 +
tests/networkxml2argvdata/netboot-network.argv | 1 +
tests/networkxml2argvdata/netboot-proxy-network.argv | 1 +
tests/networkxml2argvdata/routed-network.argv | 1 +
10 files changed, 15 insertions(+)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index ab13df5..6e5d9da 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -693,6 +693,12 @@ networkDnsmasqConfContents(virNetworkObjPtr network,
}
}
+ /* This must be defined so that the kernel knows which dnsmasq to route
+ * packets to when more than one instance if running
+ */
+ if (network->def->bridge != NULL)
+ virBufferAsprintf(&configbuf, "interface=%s\n",
network->def->bridge);
+
for (ii = 0;
(tmpipdef = virNetworkDefGetIpByIndex(network->def, AF_UNSPEC, ii));
ii++)
diff --git a/tests/networkxml2argvdata/isolated-network.argv
b/tests/networkxml2argvdata/isolated-network.argv
index 042158b..abcde93 100644
--- a/tests/networkxml2argvdata/isolated-network.argv
+++ b/tests/networkxml2argvdata/isolated-network.argv
@@ -6,6 +6,7 @@ domain-needed
local=//
dhcp-option=3
no-resolv
+interface=virbr2
listen-address=192.168.152.1
dhcp-range=192.168.152.2,192.168.152.254
dhcp-leasefile=/var/lib/libvirt/dnsmasq/private.leases
diff --git a/tests/networkxml2argvdata/nat-network-dns-hosts.argv
b/tests/networkxml2argvdata/nat-network-dns-hosts.argv
index 91eb682..7dce6f9 100644
--- a/tests/networkxml2argvdata/nat-network-dns-hosts.argv
+++ b/tests/networkxml2argvdata/nat-network-dns-hosts.argv
@@ -6,5 +6,6 @@ domain-needed
local=/example.com/
domain=example.com
expand-hosts
+interface=virbr0
listen-address=192.168.122.1
addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts
diff --git a/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv
b/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv
index d92497b..d87d438 100644
--- a/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv
+++ b/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv
@@ -5,6 +5,7 @@ except-interface=lo
domain-needed
local=//
srv-host=name.tcp.,,,,
+interface=virbr0
listen-address=192.168.122.1
listen-address=192.168.123.1
listen-address=2001:db8:ac10:fe01::1
diff --git a/tests/networkxml2argvdata/nat-network-dns-srv-record.argv
b/tests/networkxml2argvdata/nat-network-dns-srv-record.argv
index d8846c2..53882fe 100644
--- a/tests/networkxml2argvdata/nat-network-dns-srv-record.argv
+++ b/tests/networkxml2argvdata/nat-network-dns-srv-record.argv
@@ -5,6 +5,7 @@ except-interface=lo
domain-needed
local=//
srv-host=name.tcp.test-domain-name,.,1024,10,10
+interface=virbr0
listen-address=192.168.122.1
listen-address=192.168.123.1
listen-address=2001:db8:ac10:fe01::1
diff --git a/tests/networkxml2argvdata/nat-network-dns-txt-record.argv
b/tests/networkxml2argvdata/nat-network-dns-txt-record.argv
index bf00513..cc3ed28 100644
--- a/tests/networkxml2argvdata/nat-network-dns-txt-record.argv
+++ b/tests/networkxml2argvdata/nat-network-dns-txt-record.argv
@@ -5,6 +5,7 @@ except-interface=lo
domain-needed
local=//
txt-record=example,example value
+interface=virbr0
listen-address=192.168.122.1
listen-address=192.168.123.1
listen-address=2001:db8:ac10:fe01::1
diff --git a/tests/networkxml2argvdata/nat-network.argv
b/tests/networkxml2argvdata/nat-network.argv
index d542bbc..431fffb 100644
--- a/tests/networkxml2argvdata/nat-network.argv
+++ b/tests/networkxml2argvdata/nat-network.argv
@@ -4,6 +4,7 @@ bind-interfaces
except-interface=lo
domain-needed
local=//
+interface=virbr0
listen-address=192.168.122.1
listen-address=192.168.123.1
listen-address=2001:db8:ac10:fe01::1
diff --git a/tests/networkxml2argvdata/netboot-network.argv
b/tests/networkxml2argvdata/netboot-network.argv
index 4f5fedd..8405095 100644
--- a/tests/networkxml2argvdata/netboot-network.argv
+++ b/tests/networkxml2argvdata/netboot-network.argv
@@ -6,6 +6,7 @@ domain-needed
local=/example.com/
domain=example.com
expand-hosts
+interface=virbr1
listen-address=192.168.122.1
dhcp-range=192.168.122.2,192.168.122.254
dhcp-leasefile=/var/lib/libvirt/dnsmasq/netboot.leases
diff --git a/tests/networkxml2argvdata/netboot-proxy-network.argv
b/tests/networkxml2argvdata/netboot-proxy-network.argv
index 8b9c03a..d7c8966 100644
--- a/tests/networkxml2argvdata/netboot-proxy-network.argv
+++ b/tests/networkxml2argvdata/netboot-proxy-network.argv
@@ -6,6 +6,7 @@ domain-needed
local=/example.com/
domain=example.com
expand-hosts
+interface=virbr1
listen-address=192.168.122.1
dhcp-range=192.168.122.2,192.168.122.254
dhcp-leasefile=/var/lib/libvirt/dnsmasq/netboot.leases
diff --git a/tests/networkxml2argvdata/routed-network.argv
b/tests/networkxml2argvdata/routed-network.argv
index ad9e121..771240f 100644
--- a/tests/networkxml2argvdata/routed-network.argv
+++ b/tests/networkxml2argvdata/routed-network.argv
@@ -4,5 +4,6 @@ bind-interfaces
except-interface=lo
domain-needed
local=//
+interface=virbr1
listen-address=192.168.122.1
addn-hosts=/var/lib/libvirt/dnsmasq/local.addnhosts
--
1.7.11.7
1:25 p.m.
New subject: [libvirt] [PATCH 2/2] v3: add dnsmasq interface= parameter so bind-interfaces works
On 10/23/2012 11:07 AM, Gene Czarcinski wrote:
Gene - did I understand the last several messages on this topic
correctly? I *think* it all said that we don't need --interface, and
dnsmasq will behave identically as long as all the ip addresses
specified on the commandline (or in the conf file) are on the same
interface, right?
Although I'm holding off on the "change from long commandline to using a
conf file" patch until after 1.0.0 is released (due to the potential for
a regression), I had been considering pushing youre earlier version of
the --interface patch *before* 1.0.0 (under the assumption that people
would experience breakage if we didn't have it). Now I *think* that I
don't need to do that, but watned to verify.
In addition to the problem that Simon had originally thought we would
have, are there any other easily identifiable bugs caused by a lack of
--interface=? If so, I still may consider pushing it. I've tested on
everything down to RHEL5 (which has dnsmasq-2.45); when discussing this
with Dan Berrange earlier in the week he said he recalled some sort of
race condition where the interface sometimes wasn't available early
enough (or something like that) when using --interface, which was why it
was taken out. I haven't seen that, but then I may not be exercising it
in the right way (or maybe it's not as problematic when both --interface
and --listen-address are specified).
At any rate, if there is still a specific problem that it is solving, we
can push that change and deal with any fallout found on old distros
later (since my initial testing doesn't find any, I'm hoping that's
fairly safe). If not, then I think we should leave it, at least until
after 1.0.0, and maybe push it then.
4499
days inactive
4505
days old
19 comments
2 participants
participants (2)
-
Gene Czarcinski -
Laine Stump