On Nov 21, 2012, at 8:55 PM, Laine Stump <la...@laine.org> wrote:
> In order to optionally take advantage of new features in dnsmasq when
> the host's version of dnsmasq supports them, but still be able to run
> on hosts that don't support the new features, we need to be able to
> detect the version of dnsmasq running on the host, and possibly
> determine from the help output what options are in this dnsmasq.
>
> This patch implements a greatly simplified version of the capabilities
> code we already have for qemu. A dnsmasqCaps device can be created and
> populated either from running a program on disk, reading a file with
> the concatenated output of "dnsmasq --version; dnsmasq --help", or
> examining a buffer in memory that contains the concatenated output of
> those two commands. Simple functions to retrieve capabilities flags,
> the version number, and the path of the binary are also included.
>
> bridge_driver.c creates a single dnsmasqCaps object at driver startup,
> and disposes of it at driver shutdown. Any time it must be used, the
> dnsmasqCapsRefresh method is called - it checks the mtime of the
> binary, and re-runs the checks if the binary has changed.
>
> networkxml2argvtest.c creates 2 "artificial" dnsmasqCaps objects at
> startup - one "restricted" (doesn't support --bind-dynamic) and one
> "full" (does support --bind-dynamic). Some of the test cases use one
> and some the other, to make sure both code pathes are tested.
> ---
> src/libvirt_private.syms | 8 +-
> src/network/bridge_driver.c | 59 ++++++----
> src/network/bridge_driver.h | 7 +-
> src/util/dnsmasq.c | 260 ++++++++++++++++++++++++++++++++++++++++++++
> src/util/dnsmasq.h | 20 +++-
> tests/networkxml2argvtest.c | 57 ++++++----
> 6 files changed, 366 insertions(+), 45 deletions(-)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 5a07139..24d2033 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -244,13 +244,19 @@ virDevicePCIAddressParseXML;
> # dnsmasq.h
> dnsmasqAddDhcpHost;
> dnsmasqAddHost;
> +dnsmasqCapsGet;
> +dnsmasqCapsGetBinaryPath;
> +dnsmasqCapsGetVersion;
> +dnsmasqCapsNewFromBuffer;
> +dnsmasqCapsNewFromFile;
> +dnsmasqCapsNewFromBinary;
> +dnsmasqCapsRefresh;
> dnsmasqContextFree;
> dnsmasqContextNew;
> dnsmasqDelete;
> dnsmasqReload;
> dnsmasqSave;
>
> -
> # domain_audit.h
> virDomainAuditCgroup;
> virDomainAuditCgroupMajor;
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index c153d36..34923ea 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -85,6 +85,7 @@ struct network_driver {
> char *networkConfigDir;
> char *networkAutostartDir;
> char *logDir;
> + dnsmasqCapsPtr dnsmasqCaps;
> };
>
>
> @@ -271,7 +272,8 @@ networkFindActiveConfigs(struct network_driver *driver) {
> char *radvdpidbase;
>
> ignore_value(virPidFileReadIfAlive(NETWORK_PID_DIR,
> obj->def->name,
> - &obj->dnsmasqPid,
> DNSMASQ));
> + &obj->dnsmasqPid,
> +
> dnsmasqCapsGetBinaryPath(driver->dnsmasqCaps)));
>
> if (!(radvdpidbase =
> networkRadvdPidfileBasename(obj->def->name))) {
> virReportOOMError();
> @@ -389,6 +391,9 @@ networkStartup(int privileged) {
> goto out_of_memory;
> }
>
> + if (!(driverState->dnsmasqCaps = dnsmasqCapsNewFromBinary(DNSMASQ))) {
> + /* attempt to continue anyway */
> + }
Maybe worth a VIR_DEBUG if we are checking for this case.
>
> if (virNetworkLoadAllConfigs(&driverState->networks,
> driverState->networkConfigDir,
> @@ -514,6 +519,8 @@ networkShutdown(void) {
> if (driverState->iptables)
> iptablesContextFree(driverState->iptables);
>
> + virObjectUnref(driverState->dnsmasqCaps);
> +
> networkDriverUnlock(driverState);
> virMutexDestroy(&driverState->lock);
>
> @@ -616,7 +623,8 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
> virNetworkIpDefPtr ipdef,
> const char *pidfile,
> virCommandPtr cmd,
> - dnsmasqContext *dctx)
> + dnsmasqContext *dctx,
> + dnsmasqCapsPtr caps ATTRIBUTE_UNUSED)
> {
> int r, ret = -1;
> int nbleases = 0;
> @@ -848,7 +856,8 @@ cleanup:
>
> int
> networkBuildDhcpDaemonCommandLine(virNetworkObjPtr network, virCommandPtr
> *cmdout,
> - char *pidfile, dnsmasqContext *dctx)
> + char *pidfile, dnsmasqContext *dctx,
> + dnsmasqCapsPtr caps)
> {
> virCommandPtr cmd = NULL;
> int ret = -1, ii;
> @@ -876,8 +885,8 @@ 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) {
> + cmd = virCommandNew(dnsmasqCapsGetBinaryPath(caps));
> + if (networkBuildDnsmasqArgv(network, ipdef, pidfile, cmd, dctx, caps) <
> 0) {
> goto cleanup;
> }
>
> @@ -891,7 +900,8 @@ cleanup:
> }
>
> static int
> -networkStartDhcpDaemon(virNetworkObjPtr network)
> +networkStartDhcpDaemon(struct network_driver *driver,
> + virNetworkObjPtr network)
> {
> virCommandPtr cmd = NULL;
> char *pidfile = NULL;
> @@ -935,7 +945,9 @@ networkStartDhcpDaemon(virNetworkObjPtr network)
> if (dctx == NULL)
> goto cleanup;
>
> - ret = networkBuildDhcpDaemonCommandLine(network, &cmd, pidfile, dctx);
> + dnsmasqCapsRefresh(driver->dnsmasqCaps, false);
> +
> + ret = networkBuildDhcpDaemonCommandLine(network, &cmd, pidfile, dctx,
> driver->dnsmasqCaps);
> if (ret < 0)
> goto cleanup;
>
> @@ -988,7 +1000,8 @@ cleanup:
> * Returns 0 on success, -1 on failure.
> */
> static int
> -networkRefreshDhcpDaemon(virNetworkObjPtr network)
> +networkRefreshDhcpDaemon(struct network_driver *driver,
> + virNetworkObjPtr network)
> {
> int ret = -1, ii;
> virNetworkIpDefPtr ipdef;
> @@ -996,7 +1009,7 @@ networkRefreshDhcpDaemon(virNetworkObjPtr network)
>
> /* if there's no running dnsmasq, just start it */
> if (network->dnsmasqPid <= 0 || (kill(network->dnsmasqPid, 0) < 0))
> - return networkStartDhcpDaemon(network);
> + return networkStartDhcpDaemon(driver, network);
>
> /* Look for first IPv4 address that has dhcp defined. */
> /* We support dhcp config on 1 IPv4 interface only. */
> @@ -1038,7 +1051,8 @@ cleanup:
> * Returns 0 on success, -1 on failure.
> */
> static int
> -networkRestartDhcpDaemon(virNetworkObjPtr network)
> +networkRestartDhcpDaemon(struct network_driver *driver,
> + virNetworkObjPtr network)
> {
> /* if there is a running dnsmasq, kill it */
> if (network->dnsmasqPid > 0) {
> @@ -1047,7 +1061,7 @@ networkRestartDhcpDaemon(virNetworkObjPtr network)
> network->dnsmasqPid = -1;
> }
> /* now start dnsmasq if it should be started */
> - return networkStartDhcpDaemon(network);
> + return networkStartDhcpDaemon(driver, network);
> }
>
> static int
> @@ -1245,7 +1259,8 @@ cleanup:
> }
>
> static int
> -networkRefreshRadvd(virNetworkObjPtr network)
> +networkRefreshRadvd(struct network_driver *driver ATTRIBUTE_UNUSED,
> + virNetworkObjPtr network)
> {
> /* if there's no running radvd, just start it */
> if (network->radvdPid <= 0 || (kill(network->radvdPid, 0) < 0))
> @@ -1265,7 +1280,8 @@ networkRefreshRadvd(virNetworkObjPtr network)
> #if 0
> /* currently unused, so it causes a build error unless we #if it out */
> static int
> -networkRestartRadvd(virNetworkObjPtr network)
> +networkRestartRadvd(struct network_driver *driver,
> + virNetworkObjPtr network)
> {
> char *radvdpidbase;
>
> @@ -1313,8 +1329,8 @@ networkRefreshDaemons(struct network_driver *driver)
> * dnsmasq and/or radvd, or restart them if they've
> * disappeared.
> */
> - networkRefreshDhcpDaemon(network);
> - networkRefreshRadvd(network);
> + networkRefreshDhcpDaemon(driver, network);
> + networkRefreshRadvd(driver, network);
> }
> virNetworkObjUnlock(network);
> }
> @@ -2224,7 +2240,8 @@ networkStartNetworkVirtual(struct network_driver
> *driver,
>
>
> /* start dnsmasq if there are any IP addresses (v4 or v6) */
> - if ((v4present || v6present) && networkStartDhcpDaemon(network) < 0)
> + if ((v4present || v6present) &&
> + networkStartDhcpDaemon(driver, network) < 0)
> goto err3;
>
> /* start radvd if there are any ipv6 addresses */
> @@ -2988,7 +3005,7 @@ networkUpdate(virNetworkPtr net,
> /* these sections all change things on the dnsmasq commandline,
> * so we need to kill and restart dnsmasq.
> */
> - if (networkRestartDhcpDaemon(network) < 0)
> + if (networkRestartDhcpDaemon(driver,network) < 0)
Small nit. Missing a space after the ,
> goto cleanup;
>
> } else if (section == VIR_NETWORK_SECTION_IP_DHCP_HOST) {
> @@ -3009,8 +3026,8 @@ networkUpdate(virNetworkPtr net,
> }
>
> if ((newDhcpActive != oldDhcpActive &&
> - networkRestartDhcpDaemon(network) < 0) ||
> - networkRefreshDhcpDaemon(network) < 0) {
> + networkRestartDhcpDaemon(driver, network) < 0) ||
> + networkRefreshDhcpDaemon(driver, network) < 0) {
> goto cleanup;
> }
>
> @@ -3021,7 +3038,7 @@ networkUpdate(virNetworkPtr net,
> * can just update the config files and send SIGHUP to
> * dnsmasq.
> */
> - if (networkRefreshDhcpDaemon(network) < 0)
> + if (networkRefreshDhcpDaemon(driver, network) < 0)
> goto cleanup;
>
> }
> @@ -3030,7 +3047,7 @@ networkUpdate(virNetworkPtr net,
> /* only a change in IP addresses will affect radvd, and all of
> radvd's
> * config is stored in the conf file which will be re-read with a
> SIGHUP.
> */
> - if (networkRefreshRadvd(network) < 0)
> + if (networkRefreshRadvd(driver, network) < 0)
> goto cleanup;
> }
>
> diff --git a/src/network/bridge_driver.h b/src/network/bridge_driver.h
> index 0fae275..3af74a1 100644
> --- a/src/network/bridge_driver.h
> +++ b/src/network/bridge_driver.h
> @@ -1,7 +1,7 @@
> /*
> * network_driver.h: core driver methods for managing networks
> *
> - * Copyright (C) 2006, 2007, 2011 Red Hat, Inc.
> + * Copyright (C) 2006-2012 Red Hat, Inc.
Liberal with the dates. Did Red Hat contribute in 2008 to 2010 to this file?
> * Copyright (C) 2006 Daniel P. Berrange
> *
> * This library is free software; you can redistribute it and/or
> @@ -48,7 +48,8 @@ int networkGetNetworkAddress(const char *netname, char
> **netaddr)
>
> int networkBuildDhcpDaemonCommandLine(virNetworkObjPtr network,
> virCommandPtr *cmdout, char *pidfile,
> - dnsmasqContext *dctx)
> + dnsmasqContext *dctx,
> + dnsmasqCapsPtr caps)
> ;
> # else
> /* Define no-op replacements that don't drag in any link dependencies. */
> @@ -56,7 +57,7 @@ int networkBuildDhcpDaemonCommandLine(virNetworkObjPtr
> network,
> # 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,
> caps) 0
> # endif
>
> typedef char *(*networkDnsmasqLeaseFileNameFunc)(const char *netname);
> diff --git a/src/util/dnsmasq.c b/src/util/dnsmasq.c
> index 9d1c07b..03b3792 100644
> --- a/src/util/dnsmasq.c
> +++ b/src/util/dnsmasq.c
> @@ -39,8 +39,10 @@
>
> #include "internal.h"
> #include "datatypes.h"
> +#include "bitmap.h"
> #include "dnsmasq.h"
> #include "util.h"
> +#include "command.h"
> #include "memory.h"
> #include "virterror_internal.h"
> #include "logging.h"
> @@ -583,3 +585,261 @@ dnsmasqReload(pid_t pid ATTRIBUTE_UNUSED)
>
> return 0;
> }
> +
> +/*
> + * dnsmasqCapabilities functions - provide useful information about the
> + * version of dnsmasq on this machine.
> + *
> + */
> +struct _dnsmasqCaps {
> + virObject object;
> + char *binaryPath;
> + bool noRefresh;
> + time_t mtime;
> + virBitmapPtr flags;
> + unsigned long version;
> +};
> +
> +static virClassPtr dnsmasqCapsClass;
> +
> +static void
> +dnsmasqCapsDispose(void *obj)
> +{
> + dnsmasqCapsPtr caps = obj;
> +
> + virBitmapFree(caps->flags);
> + VIR_FREE(caps->binaryPath);
> +}
> +
> +static int dnsmasqCapsOnceInit(void)
> +{
> + if (!(dnsmasqCapsClass = virClassNew("dnsmasqCaps",
> + sizeof(dnsmasqCaps),
> + dnsmasqCapsDispose))) {
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +VIR_ONCE_GLOBAL_INIT(dnsmasqCaps)
> +
> +static void
> +dnsmasqCapsSet(dnsmasqCapsPtr caps,
> + dnsmasqCapsFlags flag)
> +{
> + ignore_value(virBitmapSetBit(caps->flags, flag));
> +}
> +
> +
> +#define SKIP_BLANKS(p) do { while ((*(p) == ' ') || (*(p) == '\t')) (p)++; }
> while (0)
Do we need to make sure the command below is executed with LC_ALL=C or call
virCommandAddEnvPassCommon()?
> +#define DNSMASQ_VERSION_STR "Dnsmasq version "
> +
> +static int
> +dnsmasqCapsSetFromBuffer(dnsmasqCapsPtr caps, const char *buf)
> +{
> + const char *p;
> +
> + caps->noRefresh = true;
> +
> + p = buf;
> + if (!STRPREFIX(p, DNSMASQ_VERSION_STR))
> + goto fail;
> + p += strlen(DNSMASQ_VERSION_STR);
> + SKIP_BLANKS(p);
> + if (virParseVersionString(p, &caps->version, true) < 0)
> + goto fail;
> +
> + if (strstr(buf, "--bind-dynamic"))
> + dnsmasqCapsSet(caps, DNSMASQ_CAPS_BIND_DYNAMIC);
> +
> + VIR_INFO("dnsmasq version is %d.%d, --bind-dynamic is %s",
> + (int)caps->version / 1000000, (int)(caps->version % 1000000) /
> 1000,
> + dnsmasqCapsGet(caps, DNSMASQ_CAPS_BIND_DYNAMIC)
> + ? "present" : "NOT present");
> + return 0;
> +
> +fail:
> + p = strchr(buf, '\n');
> + if (!p)
> + p = strchr(buf, '\0');
> +
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("cannot parse %s version number in '%.*s'"),
> + caps->binaryPath, (int) (p - buf), buf);
> + return -1;
> +
> +}
> +
> +static int
> +dnsmasqCapsSetFromFile(dnsmasqCapsPtr caps, const char *path)
> +{
> + int ret = -1;
> + char *buf = NULL;
> +
> + if (virFileReadAll(path, 1024 * 1024, &buf) < 0)
> + goto cleanup;
> +
> + ret = dnsmasqCapsSetFromBuffer(caps, buf);
> +
> +cleanup:
> + VIR_FREE(buf);
> + return ret;
> +}
> +
> +int
> +dnsmasqCapsRefresh(dnsmasqCapsPtr caps, bool force)
> +{
> + int ret = -1;
> + struct stat sb;
> + virCommandPtr cmd = NULL;
> + char *help = NULL, *version = NULL, *complete = NULL;
> +
> + if (!caps || caps->noRefresh)
> + return 0;
> +
> + if (stat(caps->binaryPath, &sb) < 0) {
> + virReportSystemError(errno, _("Cannot check dnsmasq binary %s"),
> + caps->binaryPath);
> + return -1;
> + }
> + if (!force && caps->mtime == sb.st_mtime) {
> + return 0;
> + }
> + caps->mtime = sb.st_mtime;
> +
> + /* Make sure the binary we are about to try exec'ing exists.
> + * Technically we could catch the exec() failure, but that's
> + * in a sub-process so it's hard to feed back a useful error.
> + */
> + if (!virFileIsExecutable(caps->binaryPath)) {
Isn't this another stat()? We could just use the results of the previous stat().
> + virReportSystemError(errno, _("dnsmasq binary %s is not executable"),
> + caps->binaryPath);
> + goto cleanup;
> + }
> +
> + cmd = virCommandNewArgList(caps->binaryPath, "--version", NULL);
> + virCommandSetOutputBuffer(cmd, &version);
> + virCommandSetErrorBuffer(cmd, &version);
> + if (virCommandRun(cmd, NULL) < 0) {
> + virReportSystemError(errno, _("failed to run '%s --version': %s"),
> + caps->binaryPath, version);
> + goto cleanup;
> + }
> + virCommandFree(cmd);
> +
> + cmd = virCommandNewArgList(caps->binaryPath, "--help", NULL);
> + virCommandSetOutputBuffer(cmd, &help);
> + virCommandSetErrorBuffer(cmd, &help);
> + if (virCommandRun(cmd, NULL) < 0) {
> + virReportSystemError(errno, _("failed to run '%s --help': %s"),
> + caps->binaryPath, help);
> + goto cleanup;
> + }
> +
> + if (virAsprintf(&complete, "%s\n%s", version, help) < 0) {
> + virReportOOMError();
> + goto cleanup;
> + }
> +
> + ret = dnsmasqCapsSetFromBuffer(caps, complete);
> +
> +cleanup:
> + virCommandFree(cmd);
> + VIR_FREE(help);
> + VIR_FREE(version);
> + VIR_FREE(complete);
> + return ret;
> +}
> +
> +static dnsmasqCapsPtr
> +dnsmasqCapsNewEmpty(const char *binaryPath)
> +{
> + dnsmasqCapsPtr caps;
> +
> + if (dnsmasqCapsInitialize() < 0)
> + return NULL;
> + if (!(caps = virObjectNew(dnsmasqCapsClass)))
> + return NULL;
> + if (!(caps->flags = virBitmapNew(DNSMASQ_CAPS_LAST)))
> + goto error;
> + if (!(caps->binaryPath = strdup(binaryPath ? binaryPath : DNSMASQ)))
> + goto error;
> + return caps;
> +
> +error:
> + virReportOOMError();
> + virObjectUnref(caps);
> + return NULL;
> +}
> +
> +dnsmasqCapsPtr
> +dnsmasqCapsNewFromBuffer(const char *buf, const char *binaryPath)
> +{
> + dnsmasqCapsPtr caps = dnsmasqCapsNewEmpty(binaryPath);
> +
> + if (!caps)
> + return NULL;
> +
> + if (dnsmasqCapsSetFromBuffer(caps, buf) < 0) {
> + virObjectUnref(caps);
> + return NULL;
> + }
> + return caps;
> +}
> +
> +dnsmasqCapsPtr
> +dnsmasqCapsNewFromFile(const char *dataPath, const char *binaryPath)
> +{
> + dnsmasqCapsPtr caps = dnsmasqCapsNewEmpty(binaryPath);
> +
> + if (!caps)
> + return NULL;
> +
> + if (dnsmasqCapsSetFromFile(caps, dataPath) < 0) {
> + virObjectUnref(caps);
> + return NULL;
> + }
> + return caps;
> +}
> +
> +dnsmasqCapsPtr
> +dnsmasqCapsNewFromBinary(const char *binaryPath)
> +{
> + dnsmasqCapsPtr caps = dnsmasqCapsNewEmpty(binaryPath);
> +
> + if (!caps)
> + return NULL;
> +
> + if (dnsmasqCapsRefresh(caps, true) < 0) {
> + virObjectUnref(caps);
> + return NULL;
> + }
> + return caps;
> +}
> +
> +const char *
> +dnsmasqCapsGetBinaryPath(dnsmasqCapsPtr caps)
> +{
> + return caps ? caps->binaryPath : DNSMASQ;
> +}
> +
> +unsigned long
> +dnsmasqCapsGetVersion(dnsmasqCapsPtr caps)
> +{
> + if (caps)
> + return caps->version;
> + else
> + return 0;
> +}
> +
> +bool
> +dnsmasqCapsGet(dnsmasqCapsPtr caps, dnsmasqCapsFlags flag)
> +{
> + bool b;
> +
> + if (!caps || virBitmapGetBit(caps->flags, flag, &b) < 0)
> + return false;
> + else
> + return b;
> +}
> diff --git a/src/util/dnsmasq.h b/src/util/dnsmasq.h
> index ad612be..48f0bf9 100644
> --- a/src/util/dnsmasq.h
> +++ b/src/util/dnsmasq.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (C) 2007-2010 Red Hat, Inc.
> + * Copyright (C) 2007-2012 Red Hat, Inc.
Bit liberal on the date change. I assume Red Hat contributed in 2011 but no one
updated it.
> * Copyright (C) 2010 Satoru SATOH <satoru.sa...@gmail.com>
> *
> * This library is free software; you can redistribute it and/or
> @@ -22,6 +22,7 @@
> #ifndef __DNSMASQ_H__
> # define __DNSMASQ_H__
>
> +# include "virobject.h"
> # include "virsocketaddr.h"
>
> typedef struct
> @@ -65,6 +66,16 @@ typedef struct
> dnsmasqAddnHostsfile *addnhostsfile;
> } dnsmasqContext;
>
> +typedef enum {
> + DNSMASQ_CAPS_BIND_DYNAMIC = 0, /* support for --bind-dynamic */
> +
> + DNSMASQ_CAPS_LAST, /* this must always be the last item */
> +} dnsmasqCapsFlags;
> +
> +typedef struct _dnsmasqCaps dnsmasqCaps;
> +typedef dnsmasqCaps *dnsmasqCapsPtr;
> +
> +
> dnsmasqContext * dnsmasqContextNew(const char *network_name,
> const char *config_dir);
> void dnsmasqContextFree(dnsmasqContext *ctx);
> @@ -79,4 +90,11 @@ int dnsmasqSave(const dnsmasqContext *ctx);
> int dnsmasqDelete(const dnsmasqContext *ctx);
> int dnsmasqReload(pid_t pid);
>
> +dnsmasqCapsPtr dnsmasqCapsNewFromBuffer(const char *buf, const char
> *binaryPath);
> +dnsmasqCapsPtr dnsmasqCapsNewFromFile(const char *dataPath, const char
> *binaryPath);
> +dnsmasqCapsPtr dnsmasqCapsNewFromBinary(const char *binaryPath);
> +int dnsmasqCapsRefresh(dnsmasqCapsPtr caps, bool force);
> +bool dnsmasqCapsGet(dnsmasqCapsPtr caps, dnsmasqCapsFlags flag);
> +const char *dnsmasqCapsGetBinaryPath(dnsmasqCapsPtr caps);
> +unsigned long dnsmasqCapsGetVersion(dnsmasqCapsPtr caps);
> #endif /* __DNSMASQ_H__ */
> diff --git a/tests/networkxml2argvtest.c b/tests/networkxml2argvtest.c
> index 87519e4..69cbd1c 100644
> --- a/tests/networkxml2argvtest.c
> +++ b/tests/networkxml2argvtest.c
> @@ -46,7 +46,9 @@ static int replaceTokens(char **buf, const char *token,
> const char *replacement)
> return 0;
> }
>
> -static int testCompareXMLToArgvFiles(const char *inxml, const char *outargv)
> {
> +static int
> +testCompareXMLToArgvFiles(const char *inxml, const char *outargv,
> dnsmasqCapsPtr caps)
> +{
> char *inXmlData = NULL;
> char *outArgvData = NULL;
> char *actual = NULL;
> @@ -78,7 +80,7 @@ static int testCompareXMLToArgvFiles(const char *inxml,
> const char *outargv) {
> if (dctx == NULL)
> goto fail;
>
> - if (networkBuildDhcpDaemonCommandLine(obj, &cmd, pidfile, dctx) < 0)
> + if (networkBuildDhcpDaemonCommandLine(obj, &cmd, pidfile, dctx, caps) <
> 0)
> goto fail;
>
> if (!(actual = virCommandToString(cmd)))
> @@ -102,21 +104,27 @@ static int testCompareXMLToArgvFiles(const char *inxml,
> const char *outargv) {
> return ret;
> }
>
> +typedef struct {
> + const char *name;
> + dnsmasqCapsPtr caps;
> +} testInfo;
> +
> static int
> testCompareXMLToArgvHelper(const void *data)
> {
> int result = -1;
> + const testInfo *info = data;
> char *inxml = NULL;
> char *outxml = NULL;
>
> if (virAsprintf(&inxml, "%s/networkxml2argvdata/%s.xml",
> - abs_srcdir, (const char*)data) < 0 ||
> + abs_srcdir, info->name) < 0 ||
> virAsprintf(&outxml, "%s/networkxml2argvdata/%s.argv",
> - abs_srcdir, (const char*)data) < 0) {
> + abs_srcdir, info->name) < 0) {
> goto cleanup;
> }
>
> - result = testCompareXMLToArgvFiles(inxml, outxml);
> + result = testCompareXMLToArgvFiles(inxml, outxml, info->caps);
>
> cleanup:
> VIR_FREE(inxml);
> @@ -140,23 +148,34 @@ static int
> mymain(void)
> {
> int ret = 0;
> + dnsmasqCapsPtr restricted
> + = dnsmasqCapsNewFromBuffer("Dnsmasq version 2.48", DNSMASQ);
> + dnsmasqCapsPtr full
> + = dnsmasqCapsNewFromBuffer("Dnsmasq version 2.63\n--bind-dynamic",
> DNSMASQ);
>
> networkDnsmasqLeaseFileName = testDnsmasqLeaseFileName;
>
> -#define DO_TEST(name) \
> - 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");
> - DO_TEST("netboot-network");
> - DO_TEST("netboot-proxy-network");
> - DO_TEST("nat-network-dns-txt-record");
> - DO_TEST("nat-network-dns-srv-record");
> - DO_TEST("nat-network-dns-srv-record-minimal");
> - DO_TEST("nat-network-dns-hosts");
> +#define DO_TEST(xname, xcaps) \
> + do { \
> + static testInfo info; \
> + \
> + info.name = xname; \
> + info.caps = xcaps; \
> + if (virtTestRun("Network XML-2-Argv " xname, \
> + 1, testCompareXMLToArgvHelper, &info) < 0) { \
> + ret = -1; \
> + } \
> + } while (0)
> +
> + DO_TEST("isolated-network", restricted);
> + DO_TEST("netboot-network", restricted);
> + DO_TEST("netboot-proxy-network", restricted);
> + DO_TEST("nat-network-dns-srv-record-minimal", restricted);
> + DO_TEST("routed-network", full);
> + DO_TEST("nat-network", full);
> + DO_TEST("nat-network-dns-txt-record", full);
> + DO_TEST("nat-network-dns-srv-record", full);
> + DO_TEST("nat-network-dns-hosts", full);
>
> return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE;
> }
> --
> 1.7.11.7
Overall looks good. Just had a few questions. Unfortunately I'm on an iPad with
no source around so apologies if they are dumb.
--
Doug
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
