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

Reply via email to