On 04/01/2011 06:45 AM, Michal Novotny wrote:
Make: passed
Make check: passed
Make syntax-check: passed
Hi,
this is the patch to move the dnsmasqContext creation to the
networkSaveDnsmasqHostsfile() function and also it passes the
hosts-file and addn-hosts to the file only if applicable, i.e.
if it's already set.
Originally I wanted to call the DhcpHostsfile and AddnHostsfile
creation on the first call to dnsmasqAddDhcpHost/dnsmasqAddHost
however that way I would have kept track of the path name to
be generated which would require storing network name and config
directory somewhere in the structure and that's why I changed
it to simple approach used in this patch.
Michal
This all looks straightforward. The only comment I have (other than to
sanitize the commit comment) would be that some day in the future I
would like to be able to specify static dhcp hosts under multiple IP
addresses within a network (dnsmasq can handle this, it just needs a bit
of extra trickery in the commandline), so anything you do to make that
easier would be good. (but as long as you don't make it any more
difficult than it already is, I'll ACK this when you resubmit the series).
Signed-off-by: Michal Novotny<minovotn@xxxxxxxxxx>
---
src/network/bridge_driver.c | 38 +++++++++++++++++++++-----------------
1 files changed, 21 insertions(+), 17 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index b6ce39d..41b14f9 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -420,13 +420,20 @@ networkShutdown(void) {
}
-static int
+static dnsmasqContext*
networkSaveDnsmasqHostsfile(virNetworkIpDefPtr ipdef,
- dnsmasqContext *dctx,
+ char *name,
bool force)
{
unsigned int i;
+ dnsmasqContext *dctx = dnsmasqContextNew(name,
+ DNSMASQ_STATE_DIR);
+ if (dctx == NULL) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
if (! force&& virFileExists(dctx->hostsfile->path))
return 0;
@@ -437,9 +444,14 @@ networkSaveDnsmasqHostsfile(virNetworkIpDefPtr ipdef,
}
if (dnsmasqSave(dctx)< 0)
- return -1;
+ goto cleanup;
- return 0;
+ return dctx;
+
+cleanup:
+ dnsmasqContextFree(dctx);
+
+ return NULL;
}
static int
@@ -451,6 +463,7 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
int nbleases = 0;
int ii;
virNetworkIpDefPtr tmpipdef;
+ dnsmasqContext *dctx = NULL;
/*
* NB, be careful about syntax for dnsmasq options in long format.
@@ -572,18 +585,11 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
if (ipdef->nranges || ipdef->nhosts)
virCommandAddArg(cmd, "--dhcp-no-override");
- if (ipdef->nhosts> 0) {
- dnsmasqContext *dctx = dnsmasqContextNew(network->def->name,
- DNSMASQ_STATE_DIR);
- if (dctx == NULL) {
- virReportOOMError();
- goto cleanup;
- }
-
- if (networkSaveDnsmasqHostsfile(ipdef, dctx, false) == 0) {
+ if ((dctx = networkSaveDnsmasqHostsfile(ipdef, network->def->name, false))) {
+ if (dctx->hostsfile->nhosts)
virCommandAddArgPair(cmd, "--dhcp-hostsfile",
dctx->hostsfile->path);
- }
+
dnsmasqContextFree(dctx);
}
@@ -2203,11 +2209,9 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) {
}
}
if (ipv4def) {
- dnsmasqContext *dctx = dnsmasqContextNew(network->def->name, DNSMASQ_STATE_DIR);
+ dnsmasqContext* dctx = networkSaveDnsmasqHostsfile(ipv4def, network->def->name, true);
if (dctx == NULL)
goto cleanup;
-
- networkSaveDnsmasqHostsfile(ipv4def, dctx, true);
dnsmasqContextFree(dctx);
}
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list