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

Re: [libvirt] [PATCH v2 3/5] Network: Move dnsmasqContext creation to networkSaveDnsmasqHostsfile() and pass to dnsmasq only if applicable



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 redhat com>
---
  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);
      }



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