[libvirt] [PATCH 1/2] v3: put dnsmasq parameters into a file instead of the command line

Laine Stump laine at laine.org
Tue Oct 23 21:22:55 UTC 2012


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.
>
> **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.

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)

>
> The new file and directory are put in the same directory as the
> leases file.
> ---

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.)


>  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, 

Extra trailing space on the line above. You should run "make
syntax-check" before sending patches, and fix anything it complains about.

>                          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);

The "virBufferAsprintf" line has a trailing blank. Also the if has a
multiline body, so it requires {} around it per the coding guildelines.

>  
>      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
> -     */

This comment shouldn't be removed until --interface is added in (and
even then, it should be left, somewhat modified, for
historical/informational purposes).


>      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 @@
> - at 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 @@
> - at 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 @@
> - at 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 @@
> - at 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 @@
> - at 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 @@
> - at 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 @@
> - at 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 @@
> - at 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 @@
> - at 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, 

You have an extra trailing space on the line above. Would have been
found with "make syntax-check".


> +                        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");





More information about the libvir-list mailing list