[libvirt] [PATCHv3 3/3] v7.9: put dnsmasq parameters into conf-file

Laine Stump laine at laine.org
Wed Dec 5 16:37:11 UTC 2012


On 12/03/2012 11:13 AM, Gene Czarcinski wrote:
> This patch changes how parameters are passed to dnsmasq.  Instead of
> being on the command line, the parameters are put into a file (one
> parameter per line) and a commandline --conf-file= specifies the
> location of the file.  The file is located in the same directory as
> the leases file.
>
> This also adds the dnsmasq parameter interface=<net-name>

Since my earlier patch for the CVE did that (when appropriate), this
comment is obsolete.


>
> Putting the dnsmasq parameters into a configuration file
> allows them to be examined and more easily understood than
> examining the command lines displayed by "ps ax".  This is
> especially true when a number of networks have been started.
>
> I suspect that when the use of dnsmasq was originally done,
> the command line was simple but has gotten more complicated
> over time and will likely become even more complicated in the
> future.
>
> I believe that if use of dnsmasq was done today with
> the current requirements for dnsmasq, a configuration file
> would have been used.  Many (most?) daemons use configuration
> files as oppose to large number of command line parameters.
>
> One potential addition to dnsmasq parameters is to specify
> the reverse-lookup queries which are not to be passed on up
> the chain.  For IPv4, such queries are rather simple and
> take the form:
>     <ipv4_address>.in-addr.arpa
> but ipv6 queries involve a lot longer specification.  The
> IPv6 query will be of the form:
>     <ipv6_address>.ip6.arpa
> where the above query expands to a 44 character string which is
> specified by --local=/<ipv6_address>.ip6.arpa/ which is
> certainly a lot of clutter to add to the command line.
> ---
>  src/network/bridge_driver.c                        | 190 +++++++++++++--------
>  src/network/bridge_driver.h                        |   7 +-
>  tests/networkxml2argvdata/dhcp6-nat-network.argv   |  29 ++--
>  tests/networkxml2argvdata/dhcp6-network.argv       |  29 ++--
>  .../dhcp6host-routed-network.argv                  |  25 ++-
>  tests/networkxml2argvdata/isolated-network.argv    |  31 ++--
>  .../networkxml2argvdata/nat-network-dns-hosts.argv |  19 +--
>  .../nat-network-dns-srv-record-minimal.argv        |  37 ++--
>  .../nat-network-dns-srv-record.argv                |  27 ++-
>  .../nat-network-dns-txt-record.argv                |  26 +--
>  tests/networkxml2argvdata/nat-network.argv         |  29 ++--
>  tests/networkxml2argvdata/netboot-network.argv     |  37 ++--
>  .../networkxml2argvdata/netboot-proxy-network.argv |  33 ++--
>  tests/networkxml2argvdata/routed-network.argv      |  15 +-
>  tests/networkxml2argvtest.c                        |  40 +----
>  15 files changed, 286 insertions(+), 288 deletions(-)
>
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index c07d61a..2ccc695 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -142,6 +142,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 */
> @@ -168,6 +178,7 @@ networkRemoveInactive(struct network_driver *driver,
>  {
>      char *leasefile = NULL;
>      char *radvdconfigfile = NULL;
> +    char *configfile = NULL;
>      char *radvdpidbase = NULL;
>      dnsmasqContext *dctx = NULL;
>      virNetworkDefPtr def = virNetworkObjGetPersistentDef(net);
> @@ -187,9 +198,13 @@ networkRemoveInactive(struct network_driver *driver,
>      if (!(radvdpidbase = networkRadvdPidfileBasename(def->name)))
>          goto no_memory;
>  
> +    if (!(configfile = networkDnsmasqConfigFileName(def->name)))
> +        goto no_memory;
> +
>      /* dnsmasq */
>      dnsmasqDelete(dctx);
>      unlink(leasefile);
> +    unlink(configfile);
>  
>      /* radvd */
>      unlink(radvdconfigfile);
> @@ -202,6 +217,7 @@ networkRemoveInactive(struct network_driver *driver,
>  
>  cleanup:
>      VIR_FREE(leasefile);
> +    VIR_FREE(configfile);
>      VIR_FREE(radvdconfigfile);
>      VIR_FREE(radvdpidbase);
>      dnsmasqContextFree(dctx);
> @@ -635,12 +651,13 @@ networkBuildDnsmasqHostsList(dnsmasqContext *dctx,
>  
>  
>  static int
> -networkBuildDnsmasqArgv(virNetworkObjPtr network,
> +networkDnsmasqConfContents(virNetworkObjPtr network,
>                          const char *pidfile,
> -                        virCommandPtr cmd,
> +                        char **configstr,
>                          dnsmasqContext *dctx,
>                          dnsmasqCapsPtr caps ATTRIBUTE_UNUSED)
>  {
> +    virBuffer configbuf = VIR_BUFFER_INITIALIZER;
>      int r, ret = -1;
>      int nbleases = 0;
>      int ii;
> @@ -651,46 +668,43 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
>      virNetworkIpDefPtr tmpipdef, ipdef, ipv4def, ipv6def;
>      bool dhcp4flag, dhcp6flag, ipv6SLAAC;
>  
> +    *configstr = NULL;
> +
>      /*
> -     * NB, be careful about syntax for dnsmasq options in long format.
> -     *
> -     * If the flag has a mandatory argument, it can be given using
> -     * either syntax:
> +     * All dnsmasq parameters are now put into a configuration
> +     * file except, of course, the command line --conf-file=
> +     * which specified the name and where the configuration
> +     * file is located.

"All dnsmasq parameters are put into a configuration file, except the
--conf-file= parameter, which specifies the location of the
configuration file."

>       *
> -     *     --foo bar
> -     *     --foo=bar
> +     * Therefore, all parameters must now be specified as:
>       *
> -     * If the flag has a optional argument, it *must* be given using
> -     * the syntax:
> -     *
> -     *     --foo=bar
> -     *
> -     * It is hard to determine whether a flag is optional or not,
> -     * without reading the dnsmasq source :-( The manpage is not
> -     * very explicit on this.
> +     *     foo=bar


"All conf file parameters must be specified as "foo=bar" (as opposed to
the "foo bar" form which is acceptable for dnsmasq commandline parameters".

>       */
>  
>      /*
>       * Needed to ensure dnsmasq uses same algorithm for processing
>       * multiple namedriver entries in /etc/resolv.conf as GLibC.
>       */
> -    virCommandAddArgList(cmd, "--strict-order",
> -                              "--domain-needed",
> -                              NULL);
>  
> -    if (network->def->domain) {
> -        virCommandAddArgPair(cmd, "--domain", network->def->domain);
> -        virCommandAddArg(cmd, "--expand-hosts");
> -    }
> -    /* need to specify local even if no domain specified */
> -    virCommandAddArgFormat(cmd, "--local=/%s/",
> -                           network->def->domain ? network->def->domain : "");
> +    /* create dnsmasq config file appropriate for this network */
> +    virBufferAddLit(&configbuf,
> +                              "# dnsmasq conf file created by libvirt\n"

We should be more verbose here, similar to what's done with the network
and domain xml files:

"WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE\n"
"OVERWRITTEN AND LOST. Changes to this configuration should be made
using:\n"
"  virsh net-edit %s\n"
"or other application using the libvirt API.\n",
network->def->name

> +                              "strict-order\n"
> +                              "domain-needed\n");
>  
> -    if (pidfile)
> -        virCommandAddArgPair(cmd, "--pid-file", pidfile);
> -
> -    /* *no* conf file */
> -    virCommandAddArg(cmd, "--conf-file=");
> +     if (network->def->domain) {
> +        virBufferAsprintf(&configbuf,
> +                 "domain=%s\n"
> +                 "expand-hosts\n",
> +                 network->def->domain);
> +     }
> +     /* need to specify local even if no domain specified */
> +    virBufferAsprintf(&configbuf,
> +                "local=/%s/\n",
> +                network->def->domain ? network->def->domain : "");
> +
> +     if (pidfile)
> +        virBufferAsprintf(&configbuf, "pid-file=%s\n", pidfile);
>  
>      if (dnsmasqCapsGet(caps, DNSMASQ_CAPS_BIND_DYNAMIC)) {
>          /* using --bind-dynamic with only --interface (no
> @@ -700,15 +714,14 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
>           * other than one of the virtual guests connected directly to
>           * this network). This was added in response to CVE 2012-3411.
>           */
> -        virCommandAddArgList(cmd,
> -                             "--bind-dynamic",
> -                             "--interface", network->def->bridge,
> -                             NULL);
> +        virBufferAsprintf(&configbuf,
> +                             "bind-dynamic\n"
> +                             "interface=%s\n",
> +                             network->def->bridge);
>      } else {
> -        virCommandAddArgList(cmd,
> -                             "--bind-interfaces",
> -                             "--except-interface=lo",
> -                             NULL);
> +        virBufferAddLit(&configbuf,
> +                             "bind-interfaces\n"
> +                             "except-interface=lo\n");
>          /*
>           * --interface does not actually work with dnsmasq < 2.47,
>           * due to DAD for ipv6 addresses on the interface.
> @@ -725,7 +738,7 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
>              if (!ipaddr)
>                  goto cleanup;
>              /* also part of CVE 2012-3411 - if the host's version of
> -             * dnsmasq doesn't have --bind-dynamic, only allow listening on
> +             * dnsmasq doesn't have bind-dynamic, only allow listening on
>               * private/local IP addresses (see RFC1918/RFC3484/RFC4193)
>               */
>              if (!virSocketAddrIsPrivate(&tmpipdef->address)) {
> @@ -734,7 +747,7 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
>                  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                                 _("Publicly routable address %s is prohibited. "
>                                   "The version of dnsmasq on this host (%d.%d) doesn't "
> -                                 "support the --bind-dynamic option, which is required "
> +                                 "support the bind-dynamic option, which is required "
>                                   "for safe operation on a publicly routable subnet "
>                                   "(see CVE-2012-3411). You must either upgrade dnsmasq, "
>                                   "or use a private/local subnet range for this network "
> @@ -742,21 +755,21 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
>                                 (int)version / 1000000, (int)(version % 1000000) / 1000);
>                  goto cleanup;
>              }
> -            virCommandAddArgList(cmd, "--listen-address", ipaddr, NULL);
> +            virBufferAsprintf(&configbuf, "listen-address=%s\n", ipaddr);
>              VIR_FREE(ipaddr);
>          }
>      }
>  
>      /* If this is an isolated network, set the default route option
>       * (3) to be empty to avoid setting a default route that's
> -     * guaranteed to not work, and set --no-resolv so that no dns
> +     * guaranteed to not work, and set no-resolv so that no dns
>       * requests are forwarded on to the dns server listed in the
>       * host's /etc/resolv.conf (since this could be used as a channel
>       * to build a connection to the outside).
>       */
>      if (network->def->forwardType == VIR_NETWORK_FORWARD_NONE) {
> -        virCommandAddArgList(cmd, "--dhcp-option=3",
> -                             "--no-resolv", NULL);
> +        virBufferAddLit(&configbuf, "dhcp-option=3\n"
> +                                      "no-resolv\n");
>      }
>  
>      if (network->def->dns != NULL) {
> @@ -764,7 +777,7 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
>          int i;
>  
>          for (i = 0; i < dns->ntxtrecords; i++) {
> -            virCommandAddArgFormat(cmd, "--txt-record=%s,%s",
> +            virBufferAsprintf(&configbuf, "txt-record=%s,%s\n",

Does dnsmasq's conf file require quotes around this when the value has
embedded spaces? (I'm assuming not, but that is one thing that
virCommandAddArgFormat does that is no longer happening - if there are
embedded spaces in the value, the *entire arg* is single-quoted).

>                                     dns->txtrecords[i].name,
>                                     dns->txtrecords[i].value);
>          }
> @@ -802,7 +815,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);
> @@ -882,8 +895,8 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
>                  virReportOOMError();
>                  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,
> @@ -901,8 +914,7 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
>                  virReportOOMError();
>                  goto cleanup;
>              }
> -            virCommandAddArg(cmd, "--dhcp-range");
> -            virCommandAddArgFormat(cmd, "%s,static", bridgeaddr);
> +            virBufferAsprintf(&configbuf, "dhcp-range=%s,static\n", bridgeaddr);
>              VIR_FREE(bridgeaddr);
>          }
>  
> @@ -912,16 +924,14 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
>          /* Note: the following is IPv4 only */
>          if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET)) {
>              if (ipdef->nranges || ipdef->nhosts)
> -                virCommandAddArg(cmd, "--dhcp-no-override");
> +                virBufferAddLit(&configbuf, "dhcp-no-override\n");
>  
>              if (ipdef->tftproot) {
> -                virCommandAddArgList(cmd, "--enable-tftp",
> -                                     "--tftp-root", ipdef->tftproot,
> -                                     NULL);
> +                virBufferAddLit(&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);
>  
> @@ -929,11 +939,11 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
>                          virReportOOMError();
>                          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);
>                  }
>              }
>          }
> @@ -949,9 +959,9 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
>              virReportOOMError();
>              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);
>      }
>  
>      /* this is done once per interface */
> @@ -963,19 +973,19 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
>       * file to allow for runtime additions.
>       */
>      if (dhcp4flag || dhcp6flag)
> -        virCommandAddArgPair(cmd, "--dhcp-hostsfile",
> -                             dctx->hostsfile->path);
> +            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",
> -                         dctx->addnhostsfile->path);
> +    virBufferAsprintf(&configbuf, "addn-hosts=%s\n",
> +                       dctx->addnhostsfile->path);
>  
>      /* Are we doing RA instead of radvd? */
>      if (CHECK_VERSION_RA(caps)) {
>          if (dhcp6flag)
> -            virCommandAddArg(cmd, "--enable-ra");
> +            virBufferAddLit(&configbuf, "enable-ra\n");
>          else {
>              for (ii = 0;
>                  (ipdef = virNetworkDefGetIpByIndex(network->def, AF_INET6, ii));
> @@ -983,8 +993,8 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
>                  if (!(ipdef->nranges || ipdef->nhosts)) {
>                      char *bridgeaddr = virSocketAddrFormat(&ipdef->address);
>                      if (bridgeaddr) {
> -                        virCommandAddArgFormat(cmd,
> -                            "--dhcp-range=%s,ra-only", bridgeaddr);
> +                        virBufferAsprintf(&configbuf,
> +                            "dhcp-range=%s,ra-only\n", bridgeaddr);
>                      } else {
>                          virReportOOMError();
>                          goto cleanup;
> @@ -995,9 +1005,15 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
>          }
>      }
>  
> +    if (!(*configstr = virBufferContentAndReset(&configbuf))) {
> +        virReportOOMError();

This is incorrect - virBufferContentAndReset() doesn't attempt to
allocate any memory; it only returns a pointer to memory that was
already allocated. It will return an error only if some earlier
virBuffer*() function had a problem, and in that case the error will
already be set/logged.

So, if it virBufferContentAndReset returns NULL, simply goto cleanup.

> +        goto cleanup;
> +    }
> +
>      ret = 0;
>  
>  cleanup:
> +    virBufferFreeAndReset(&configbuf);
>      VIR_FREE(record);
>      VIR_FREE(recordPort);
>      VIR_FREE(recordWeight);
> @@ -1005,13 +1021,17 @@ cleanup:
>      return ret;
>  }
>  
> +        /* build the dnsmasq command line */

strange indentation.
>  int
>  networkBuildDhcpDaemonCommandLine(virNetworkObjPtr network, virCommandPtr *cmdout,
>                                    char *pidfile, dnsmasqContext *dctx,
> -                                  dnsmasqCapsPtr caps)
> +                                  dnsmasqCapsPtr caps,
> +                                  int testOnly, char **testConfigStr)

Instead of tricking out networkBuildDhcpDaemonCommandLine() to return
after just calling networkDnsmasqConfContents() when you're calling from
the test code, you should change the test code to directly call
networkDnsmasqConfContents(). You'll need to add it to bridge_driver.h
and src/libvirt_private.syms.


>  {
>      virCommandPtr cmd = NULL;
> -    int ret = -1, ii;
> +    int ret = -1;
> +    char *configfile = NULL;
> +    char *configstr = NULL;
>  
>      network->dnsmasqPid = -1;
>  
> @@ -1024,11 +1044,33 @@ networkBuildDhcpDaemonCommandLine(virNetworkObjPtr network, virCommandPtr *cmdou
>      if (!virNetworkDefGetIpByIndex(network->def, AF_UNSPEC, 0))
>          return 0;
>  
> -    cmd = virCommandNew(dnsmasqCapsGetBinaryPath(caps));
> -    if (networkBuildDnsmasqArgv(network, pidfile, cmd, dctx, caps) < 0) {
> +    if (networkDnsmasqConfContents(network, pidfile, &configstr, dctx, caps) < 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;
> +    }
> +
> +    cmd = virCommandNew(dnsmasqCapsGetBinaryPath(caps));
> +    virCommandAddArgFormat(cmd, "--conf-file=%s", configfile);
> +
>      if (cmdout)
>          *cmdout = cmd;
>      ret = 0;
> @@ -1044,6 +1086,7 @@ networkStartDhcpDaemon(struct network_driver *driver,
>  {
>      virCommandPtr cmd = NULL;
>      char *pidfile = NULL;
> +    char *testconfigstr = NULL;
>      int ret = -1;
>      dnsmasqContext *dctx = NULL;
>  
> @@ -1084,8 +1127,9 @@ networkStartDhcpDaemon(struct network_driver *driver,
>  
>      dnsmasqCapsRefresh(&driver->dnsmasqCaps, false);
>  
> -    ret = networkBuildDhcpDaemonCommandLine(network, &cmd, pidfile,
> -                                            dctx, driver->dnsmasqCaps);
> +    ret = networkBuildDhcpDaemonCommandLine(network, &cmd, pidfile, dctx,
> +                                             driver->dnsmasqCaps,
> +                                             0, &testconfigstr);
>      if (ret < 0)
>          goto cleanup;
>  
> diff --git a/src/network/bridge_driver.h b/src/network/bridge_driver.h
> index 8c16bdd..f727f6a 100644
> --- a/src/network/bridge_driver.h
> +++ b/src/network/bridge_driver.h
> @@ -49,15 +49,16 @@ int networkGetNetworkAddress(const char *netname, char **netaddr)
>  int networkBuildDhcpDaemonCommandLine(virNetworkObjPtr network,
>                                        virCommandPtr *cmdout, char *pidfile,
>                                        dnsmasqContext *dctx,
> -                                      dnsmasqCapsPtr caps)
> -    ;
> +                                      dnsmasqCapsPtr caps,
> +                                      int testOnly, char **testConfigStr);
>  # else
>  /* Define no-op replacements that don't drag in any link dependencies.  */
>  #  define networkAllocateActualDevice(iface) 0
>  #  define networkNotifyActualDevice(iface) (iface=iface, 0)
>  #  define networkReleaseActualDevice(iface) (iface=iface, 0)
>  #  define networkGetNetworkAddress(netname, netaddr) (-2)
> -#  define networkBuildDhcpDaemonCommandLine(network, cmdout, pidfile, dctx, caps) 0
> +#  define networkBuildDhcpDaemonCommandLine(network, cmdout, pidfile, \
> +                    dctx, caps, testonly, testConfigStr) 0

Don't forget to change this back with you remove testonly and
testConfigStr (you won't notice it when you try to build, but other
platforms with no network driver will get a build error).


>  # endif

Again, I'm assuming that a successful pass of the test cases means that
the changes to the test data is correct.

You should rename all the .argv files to .conf though.

>  
>  typedef char *(*networkDnsmasqLeaseFileNameFunc)(const char *netname);
> diff --git a/tests/networkxml2argvdata/dhcp6-nat-network.argv b/tests/networkxml2argvdata/dhcp6-nat-network.argv
> index df8c507..bdb64d6 100644
> --- a/tests/networkxml2argvdata/dhcp6-nat-network.argv
> +++ b/tests/networkxml2argvdata/dhcp6-nat-network.argv
> @@ -1,15 +1,14 @@
> - at DNSMASQ@ \
> ---strict-order \
> ---domain-needed \
> ---local=// \
> ---conf-file= \
> ---bind-dynamic \
> ---interface virbr0 \
> ---dhcp-range 192.168.122.2,192.168.122.254 \
> ---dhcp-no-override \
> ---dhcp-range 2001:db8:ac10:fd01::1:10,2001:db8:ac10:fd01::1:ff \
> ---dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases \
> ---dhcp-lease-max=493 \
> ---dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile \
> ---addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts \
> ---enable-ra\
> +# dnsmasq conf file created by libvirt
> +strict-order
> +domain-needed
> +local=//
> +bind-dynamic
> +interface=virbr0
> +dhcp-range=192.168.122.2,192.168.122.254
> +dhcp-no-override
> +dhcp-range=2001:db8:ac10:fd01::1:10,2001:db8:ac10:fd01::1:ff
> +dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases
> +dhcp-lease-max=493
> +dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile
> +addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts
> +enable-ra
> diff --git a/tests/networkxml2argvdata/dhcp6-network.argv b/tests/networkxml2argvdata/dhcp6-network.argv
> index 059c418..42c7b35 100644
> --- a/tests/networkxml2argvdata/dhcp6-network.argv
> +++ b/tests/networkxml2argvdata/dhcp6-network.argv
> @@ -1,15 +1,14 @@
> - at DNSMASQ@ \
> ---strict-order \
> ---domain-needed \
> ---domain=mynet \
> ---expand-hosts \
> ---local=/mynet/ \
> ---conf-file= \
> ---bind-dynamic \
> ---interface virbr0 \
> ---dhcp-range 2001:db8:ac10:fd01::1:10,2001:db8:ac10:fd01::1:ff \
> ---dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases \
> ---dhcp-lease-max=240 \
> ---dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile \
> ---addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts \
> ---enable-ra\
> +# dnsmasq conf file created by libvirt
> +strict-order
> +domain-needed
> +domain=mynet
> +expand-hosts
> +local=/mynet/
> +bind-dynamic
> +interface=virbr0
> +dhcp-range=2001:db8:ac10:fd01::1:10,2001:db8:ac10:fd01::1:ff
> +dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases
> +dhcp-lease-max=240
> +dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile
> +addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts
> +enable-ra
> diff --git a/tests/networkxml2argvdata/dhcp6host-routed-network.argv b/tests/networkxml2argvdata/dhcp6host-routed-network.argv
> index 8f6d7d3..9ee3c28 100644
> --- a/tests/networkxml2argvdata/dhcp6host-routed-network.argv
> +++ b/tests/networkxml2argvdata/dhcp6host-routed-network.argv
> @@ -1,13 +1,12 @@
> - at DNSMASQ@ \
> ---strict-order \
> ---domain-needed \
> ---local=// \
> ---conf-file= \
> ---bind-dynamic \
> ---interface virbr1 \
> ---dhcp-range 192.168.122.1,static \
> ---dhcp-no-override \
> ---dhcp-range 2001:db8:ac10:fd01::1,static \
> ---dhcp-hostsfile=/var/lib/libvirt/dnsmasq/local.hostsfile \
> ---addn-hosts=/var/lib/libvirt/dnsmasq/local.addnhosts \
> ---enable-ra\
> +# dnsmasq conf file created by libvirt
> +strict-order
> +domain-needed
> +local=//
> +bind-dynamic
> +interface=virbr1
> +dhcp-range=192.168.122.1,static
> +dhcp-no-override
> +dhcp-range=2001:db8:ac10:fd01::1,static
> +dhcp-hostsfile=/var/lib/libvirt/dnsmasq/local.hostsfile
> +addn-hosts=/var/lib/libvirt/dnsmasq/local.addnhosts
> +enable-ra
> diff --git a/tests/networkxml2argvdata/isolated-network.argv b/tests/networkxml2argvdata/isolated-network.argv
> index b0783bd..740e423 100644
> --- a/tests/networkxml2argvdata/isolated-network.argv
> +++ b/tests/networkxml2argvdata/isolated-network.argv
> @@ -1,16 +1,15 @@
> - at DNSMASQ@ \
> ---strict-order \
> ---domain-needed \
> ---local=// \
> ---conf-file= \
> ---bind-interfaces \
> ---except-interface=lo \
> ---listen-address 192.168.152.1 \
> ---dhcp-option=3 \
> ---no-resolv \
> ---dhcp-range 192.168.152.2,192.168.152.254 \
> ---dhcp-no-override \
> ---dhcp-leasefile=/var/lib/libvirt/dnsmasq/private.leases \
> ---dhcp-lease-max=253 \
> ---dhcp-hostsfile=/var/lib/libvirt/dnsmasq/private.hostsfile \
> ---addn-hosts=/var/lib/libvirt/dnsmasq/private.addnhosts\
> +# dnsmasq conf file created by libvirt
> +strict-order
> +domain-needed
> +local=//
> +bind-interfaces
> +except-interface=lo
> +listen-address=192.168.152.1
> +dhcp-option=3
> +no-resolv
> +dhcp-range=192.168.152.2,192.168.152.254
> +dhcp-no-override
> +dhcp-leasefile=/var/lib/libvirt/dnsmasq/private.leases
> +dhcp-lease-max=253
> +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 fee137f..1a3df37 100644
> --- a/tests/networkxml2argvdata/nat-network-dns-hosts.argv
> +++ b/tests/networkxml2argvdata/nat-network-dns-hosts.argv
> @@ -1,10 +1,9 @@
> - at DNSMASQ@ \
> ---strict-order \
> ---domain-needed \
> ---domain=example.com \
> ---expand-hosts \
> ---local=/example.com/ \
> ---conf-file= \
> ---bind-dynamic \
> ---interface virbr0 \
> ---addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts\
> +# dnsmasq conf file created by libvirt
> +strict-order
> +domain-needed
> +domain=example.com
> +expand-hosts
> +local=/example.com/
> +bind-dynamic
> +interface=virbr0
> +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 1de6637..67600f4 100644
> --- a/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv
> +++ b/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv
> @@ -1,19 +1,18 @@
> - at DNSMASQ@ \
> ---strict-order \
> ---domain-needed \
> ---local=// \
> ---conf-file= \
> ---bind-interfaces \
> ---except-interface=lo \
> ---listen-address 192.168.122.1 \
> ---listen-address 192.168.123.1 \
> ---listen-address fc00:db8:ac10:fe01::1 \
> ---listen-address fc00:db8:ac10:fd01::1 \
> ---listen-address 10.24.10.1 \
> ---srv-host=name.tcp.,,,, \
> ---dhcp-range 192.168.122.2,192.168.122.254 \
> ---dhcp-no-override \
> ---dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases \
> ---dhcp-lease-max=253 \
> ---dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile \
> ---addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts\
> +# dnsmasq conf file created by libvirt
> +strict-order
> +domain-needed
> +local=//
> +bind-interfaces
> +except-interface=lo
> +listen-address=192.168.122.1
> +listen-address=192.168.123.1
> +listen-address=fc00:db8:ac10:fe01::1
> +listen-address=fc00:db8:ac10:fd01::1
> +listen-address=10.24.10.1
> +srv-host=name.tcp.,,,,
> +dhcp-range=192.168.122.2,192.168.122.254
> +dhcp-no-override
> +dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases
> +dhcp-lease-max=253
> +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 e7ecaa5..c03e6d9 100644
> --- a/tests/networkxml2argvdata/nat-network-dns-srv-record.argv
> +++ b/tests/networkxml2argvdata/nat-network-dns-srv-record.argv
> @@ -1,14 +1,13 @@
> - at DNSMASQ@ \
> ---strict-order \
> ---domain-needed \
> ---local=// \
> ---conf-file= \
> ---bind-dynamic \
> ---interface virbr0 \
> ---srv-host=name.tcp.test-domain-name,.,1024,10,10 \
> ---dhcp-range 192.168.122.2,192.168.122.254 \
> ---dhcp-no-override \
> ---dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases \
> ---dhcp-lease-max=253 \
> ---dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile \
> ---addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts\
> +# dnsmasq conf file created by libvirt
> +strict-order
> +domain-needed
> +local=//
> +bind-dynamic
> +interface=virbr0
> +srv-host=name.tcp.test-domain-name,.,1024,10,10
> +dhcp-range=192.168.122.2,192.168.122.254
> +dhcp-no-override
> +dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases
> +dhcp-lease-max=253
> +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 8ea0004..7dd9f3f 100644
> --- a/tests/networkxml2argvdata/nat-network-dns-txt-record.argv
> +++ b/tests/networkxml2argvdata/nat-network-dns-txt-record.argv
> @@ -1,13 +1,13 @@
> - at DNSMASQ@ \
> ---strict-order \
> ---domain-needed \
> ---local=// \
> ---conf-file= \
> ---bind-dynamic --interface virbr0 \
> -'--txt-record=example,example value' \
> ---dhcp-range 192.168.122.2,192.168.122.254 \
> ---dhcp-no-override \
> ---dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases \
> ---dhcp-lease-max=253 \
> ---dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile \
> ---addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts\
> +# dnsmasq conf file created by libvirt
> +strict-order
> +domain-needed
> +local=//
> +bind-dynamic
> +interface=virbr0
> +txt-record=example,example value
> +dhcp-range=192.168.122.2,192.168.122.254
> +dhcp-no-override
> +dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases
> +dhcp-lease-max=253
> +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 578a5ff..e71f54b 100644
> --- a/tests/networkxml2argvdata/nat-network.argv
> +++ b/tests/networkxml2argvdata/nat-network.argv
> @@ -1,15 +1,14 @@
> - at DNSMASQ@ \
> ---strict-order \
> ---domain-needed \
> ---local=// \
> ---conf-file= \
> ---bind-dynamic \
> ---interface virbr0 \
> ---dhcp-range 192.168.122.2,192.168.122.254 \
> ---dhcp-no-override \
> ---dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases \
> ---dhcp-lease-max=253 \
> ---dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile \
> ---addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts \
> ---dhcp-range=2001:db8:ac10:fe01::1,ra-only \
> ---dhcp-range=2001:db8:ac10:fd01::1,ra-only\
> +# dnsmasq conf file created by libvirt
> +strict-order
> +domain-needed
> +local=//
> +bind-dynamic
> +interface=virbr0
> +dhcp-range=192.168.122.2,192.168.122.254
> +dhcp-no-override
> +dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases
> +dhcp-lease-max=253
> +dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile
> +addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts
> +dhcp-range=2001:db8:ac10:fe01::1,ra-only
> +dhcp-range=2001:db8:ac10:fd01::1,ra-only
> diff --git a/tests/networkxml2argvdata/netboot-network.argv b/tests/networkxml2argvdata/netboot-network.argv
> index c5b75a3..df8df31 100644
> --- a/tests/networkxml2argvdata/netboot-network.argv
> +++ b/tests/networkxml2argvdata/netboot-network.argv
> @@ -1,19 +1,18 @@
> - at DNSMASQ@ \
> ---strict-order \
> ---domain-needed \
> ---domain=example.com \
> ---expand-hosts \
> ---local=/example.com/ \
> ---conf-file= \
> ---bind-interfaces \
> ---except-interface=lo \
> ---listen-address 192.168.122.1 \
> ---dhcp-range 192.168.122.2,192.168.122.254 \
> ---dhcp-no-override \
> ---enable-tftp \
> ---tftp-root /var/lib/tftproot \
> ---dhcp-boot pxeboot.img \
> ---dhcp-leasefile=/var/lib/libvirt/dnsmasq/netboot.leases \
> ---dhcp-lease-max=253 \
> ---dhcp-hostsfile=/var/lib/libvirt/dnsmasq/netboot.hostsfile \
> ---addn-hosts=/var/lib/libvirt/dnsmasq/netboot.addnhosts\
> +# dnsmasq conf file created by libvirt
> +strict-order
> +domain-needed
> +domain=example.com
> +expand-hosts
> +local=/example.com/
> +bind-interfaces
> +except-interface=lo
> +listen-address=192.168.122.1
> +dhcp-range=192.168.122.2,192.168.122.254
> +dhcp-no-override
> +enable-tftp
> +tftp-root=/var/lib/tftproot
> +dhcp-boot=pxeboot.img
> +dhcp-leasefile=/var/lib/libvirt/dnsmasq/netboot.leases
> +dhcp-lease-max=253
> +dhcp-hostsfile=/var/lib/libvirt/dnsmasq/netboot.hostsfile
> +addn-hosts=/var/lib/libvirt/dnsmasq/netboot.addnhosts
> diff --git a/tests/networkxml2argvdata/netboot-proxy-network.argv b/tests/networkxml2argvdata/netboot-proxy-network.argv
> index 16e1c85..a84d7a1 100644
> --- a/tests/networkxml2argvdata/netboot-proxy-network.argv
> +++ b/tests/networkxml2argvdata/netboot-proxy-network.argv
> @@ -1,17 +1,16 @@
> - at DNSMASQ@ \
> ---strict-order \
> ---domain-needed \
> ---domain=example.com \
> ---expand-hosts \
> ---local=/example.com/ \
> ---conf-file= \
> ---bind-interfaces \
> ---except-interface=lo \
> ---listen-address 192.168.122.1 \
> ---dhcp-range 192.168.122.2,192.168.122.254 \
> ---dhcp-no-override \
> ---dhcp-boot pxeboot.img,,10.20.30.40 \
> ---dhcp-leasefile=/var/lib/libvirt/dnsmasq/netboot.leases \
> ---dhcp-lease-max=253 \
> ---dhcp-hostsfile=/var/lib/libvirt/dnsmasq/netboot.hostsfile \
> ---addn-hosts=/var/lib/libvirt/dnsmasq/netboot.addnhosts\
> +# dnsmasq conf file created by libvirt
> +strict-order
> +domain-needed
> +domain=example.com
> +expand-hosts
> +local=/example.com/
> +bind-interfaces
> +except-interface=lo
> +listen-address=192.168.122.1
> +dhcp-range=192.168.122.2,192.168.122.254
> +dhcp-no-override
> +dhcp-boot=pxeboot.img,,10.20.30.40
> +dhcp-leasefile=/var/lib/libvirt/dnsmasq/netboot.leases
> +dhcp-lease-max=253
> +dhcp-hostsfile=/var/lib/libvirt/dnsmasq/netboot.hostsfile
> +addn-hosts=/var/lib/libvirt/dnsmasq/netboot.addnhosts
> diff --git a/tests/networkxml2argvdata/routed-network.argv b/tests/networkxml2argvdata/routed-network.argv
> index b3fbf49..eaf4bd3 100644
> --- a/tests/networkxml2argvdata/routed-network.argv
> +++ b/tests/networkxml2argvdata/routed-network.argv
> @@ -1,8 +1,7 @@
> - at DNSMASQ@ \
> ---strict-order \
> ---domain-needed \
> ---local=// \
> ---conf-file= \
> ---bind-dynamic \
> ---interface virbr1 \
> ---addn-hosts=/var/lib/libvirt/dnsmasq/local.addnhosts\
> +# dnsmasq conf file created by libvirt
> +strict-order
> +domain-needed
> +local=//
> +bind-dynamic
> +interface=virbr1
> +addn-hosts=/var/lib/libvirt/dnsmasq/local.addnhosts
> diff --git a/tests/networkxml2argvtest.c b/tests/networkxml2argvtest.c
> index a020db9..cc6be1c 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, dnsmasqCapsPtr caps)
>  {
> @@ -65,9 +34,6 @@ testCompareXMLToArgvFiles(const char *inxml, const char *outargv, dnsmasqCapsPtr
>      if (virtTestLoadFile(outargv, &outArgvData) < 0)
>          goto fail;
>  
> -    if (replaceTokens(&outArgvData, "@DNSMASQ@", DNSMASQ))
> -        goto fail;
> -
>      if (!(dev = virNetworkDefParseString(inXmlData)))
>          goto fail;
>  
> @@ -80,10 +46,8 @@ testCompareXMLToArgvFiles(const char *inxml, const char *outargv, dnsmasqCapsPtr
>      if (dctx == NULL)
>          goto fail;
>  
> -    if (networkBuildDhcpDaemonCommandLine(obj, &cmd, pidfile, dctx, caps) < 0)
> -        goto fail;
> -
> -    if (!(actual = virCommandToString(cmd)))
> +    if (networkBuildDhcpDaemonCommandLine(obj, &cmd, pidfile,
> +                        dctx, caps, 1, &actual) < 0)
>          goto fail;

Again - this should directly call networkDnsmasqConfContents instead of
networkBuildDhcpDaemonCommandLine.


>  
>      if (STRNEQ(outArgvData, actual)) {

I'll ACK this once you've made the changes I've pointed out above (and
rebased it on top of the updated versions of the other two patches).




More information about the libvir-list mailing list