[libvirt] [PATCH v2 4/5] Network: Add additional hosts internal infrastructure

Laine Stump laine at laine.org
Thu Apr 28 01:31:29 UTC 2011


Coming back to this now that I see how it's being used...

On 04/01/2011 06:45 AM, Michal Novotny wrote:
> Hi,
> this is the patch to introduce the internal infrastructure for
> additional hosts for network bridge driver using the addnhosts*
> API functions.
>
> This is necessary for next part of the patch to support DNS
> hosts definition in the network XML description.
>
> Michal
>
> Signed-off-by: Michal Novotny<minovotn at redhat.com>
> ---
>   src/libvirt_private.syms    |    1 +
>   src/network/bridge_driver.c |    3 +
>   src/util/dnsmasq.c          |  266 ++++++++++++++++++++++++++++++++++++++++++-
>   src/util/dnsmasq.h          |   22 ++++-
>   4 files changed, 287 insertions(+), 5 deletions(-)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 65a86d3..73c3f77 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -186,6 +186,7 @@ virUnrefStream;
>
>   # dnsmasq.h
>   dnsmasqAddDhcpHost;
> +dnsmasqAddHost;
>   dnsmasqContextFree;
>   dnsmasqContextNew;
>   dnsmasqDelete;
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 41b14f9..4ad3143 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -589,6 +589,9 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
>               if (dctx->hostsfile->nhosts)
>                   virCommandAddArgPair(cmd, "--dhcp-hostsfile",
>                                        dctx->hostsfile->path);
> +            if (dctx->addnhostsfile->nhosts)
> +                virCommandAddArgPair(cmd, "--addn-hosts",
> +                                     dctx->addnhostsfile->path);
>
>               dnsmasqContextFree(dctx);
>           }
> diff --git a/src/util/dnsmasq.c b/src/util/dnsmasq.c
> index be230e1..fee3b90 100644
> --- a/src/util/dnsmasq.c
> +++ b/src/util/dnsmasq.c
> @@ -48,6 +48,7 @@
>
>   #define VIR_FROM_THIS VIR_FROM_NETWORK
>   #define DNSMASQ_HOSTSFILE_SUFFIX "hostsfile"
> +#define DNSMASQ_ADDNHOSTSFILE_SUFFIX "addnhosts"
>
>   static void
>   dhcphostFree(dnsmasqDhcpHost *host)
> @@ -56,6 +57,231 @@ dhcphostFree(dnsmasqDhcpHost *host)
>   }
>
>   static void
> +addnhostFree(dnsmasqAddnHost *host)
> +{
> +    VIR_FREE(host->hostnames);

You haven't freed host->hostnames[0], hostnames[1], etc; you've only 
freed the array of pointers.


> +    VIR_FREE(host->ip);
> +}
> +
> +static void
> +addnhostsFree(dnsmasqAddnHostsfile *addnhostsfile)
> +{
> +    unsigned int i;
> +
> +    if (addnhostsfile->hosts) {
> +        for (i = 0; i<  addnhostsfile->nhosts; i++)
> +            addnhostFree(&addnhostsfile->hosts[i]);
> +
> +        VIR_FREE(addnhostsfile->hosts);
> +
> +        addnhostsfile->nhosts = 0;
> +    }
> +
> +    VIR_FREE(addnhostsfile->path);
> +
> +    VIR_FREE(addnhostsfile);
> +}
> +
> +static int
> +addnhostsAdd(dnsmasqAddnHostsfile *addnhostsfile,
> +             virSocketAddr *ip,

You could also mark ip as a const.

> +             const char *name)
> +{

It looks like this function could be simplified quite a lot if it was 
changed to take a char** instead of a char* (see my comment in 5/5 about 
saving it that way in the virNetworkDNSDefPtr).

> +    char *ipstr = NULL;
> +    int idx = -1;
> +    int i;
> +
> +    if (!(ipstr = virSocketFormatAddr(ip)))
> +        return -1;
> +
> +    for (i = 0; i<  addnhostsfile->nhosts; i++) {
> +        if (STREQ((const char *)addnhostsfile->hosts[i].ip, (const char *)ipstr)) {
> +            idx = i;
> +            break;
> +        }
> +    }
> +
> +    if (idx<  0) {
> +        if (VIR_REALLOC_N(addnhostsfile->hosts, addnhostsfile->nhosts + 1)<  0)
> +            goto alloc_error;
> +
> +        idx = addnhostsfile->nhosts;
> +        if (VIR_ALLOC(addnhostsfile->hosts[idx].hostnames)<  0)
> +            goto alloc_error;
> +
> +        if (virAsprintf(&addnhostsfile->hosts[idx].ip, "%s", ipstr)<  0)
> +            goto alloc_error;
> +
> +        addnhostsfile->hosts[idx].nhostnames = 0;
> +        addnhostsfile->nhosts++;
> +    }
> +
> +    if (VIR_REALLOC_N(addnhostsfile->hosts[idx].hostnames, addnhostsfile->hosts[idx].nhostnames + 1)<  0)
> +        goto alloc_error;
> +
> +    if (virAsprintf(&addnhostsfile->hosts[idx].hostnames[addnhostsfile->hosts[idx].nhostnames], "%s", name)<  0)
> +        goto alloc_error;
> +
> +    VIR_FREE(ipstr);
> +
> +    addnhostsfile->hosts[idx].nhostnames++;
> +
> +    return 0;
> +
> + alloc_error:
> +    virReportOOMError();
> +    VIR_FREE(ipstr);
> +    return -1;
> +}
> +
> +static dnsmasqAddnHostsfile *
> +addnhostsNew(const char *name,
> +             const char *config_dir)
> +{
> +    int err;
> +    dnsmasqAddnHostsfile *addnhostsfile;
> +
> +    if (VIR_ALLOC(addnhostsfile)<  0) {
> +        virReportOOMError();
> +        return NULL;
> +    }
> +
> +    addnhostsfile->hosts = NULL;
> +    addnhostsfile->nhosts = 0;
> +
> +    if (virAsprintf(&addnhostsfile->path, "%s/%s.%s", config_dir, name,
> +                    DNSMASQ_ADDNHOSTSFILE_SUFFIX)<  0) {
> +        virReportOOMError();
> +        goto error;
> +    }
> +
> +    if ((err = virFileMakePath(config_dir))) {
> +        virReportSystemError(err, _("cannot create config directory '%s'"),
> +                             config_dir);
> +        goto error;
> +    }
> +
> +    return addnhostsfile;
> +
> + error:
> +    addnhostsFree(addnhostsfile);
> +    return NULL;
> +}
> +
> +static int
> +addnhostsWrite(const char *path,
> +               dnsmasqAddnHost *hosts,
> +               unsigned int nhosts)
> +{
> +    char *tmp;
> +    FILE *f;
> +    bool istmp = true;
> +    unsigned int i, ii;
> +    int rc = 0;
> +
> +    if (nhosts == 0)
> +        return rc;
> +
> +    if (virAsprintf(&tmp, "%s.new", path)<  0)
> +        return ENOMEM;
> +
> +    if (!(f = fopen(tmp, "w"))) {
> +        istmp = false;
> +        if (!(f = fopen(path, "w"))) {
> +            rc = errno;
> +            goto cleanup;
> +        }
> +    }
> +
> +    for (i = 0; i<  nhosts; i++) {
> +        if (fputs(hosts[i].ip, f) == EOF || fputc('\t', f) == EOF) {
> +            rc = errno;
> +            VIR_FORCE_FCLOSE(f);
> +
> +            if (istmp)
> +                unlink(tmp);
> +
> +            goto cleanup;
> +        }
> +
> +        for (ii = 0; ii<  hosts[i].nhostnames; ii++) {
> +            if (fputs(hosts[i].hostnames[ii], f) == EOF || fputc('\t', f) == EOF) {
> +                rc = errno;
> +                VIR_FORCE_FCLOSE(f);
> +
> +                if (istmp)
> +                    unlink(tmp);
> +
> +                goto cleanup;
> +            }
> +        }
> +
> +        if (fputc('\n', f) == EOF) {
> +            rc = errno;
> +            VIR_FORCE_FCLOSE(f);
> +
> +            if (istmp)
> +                unlink(tmp);
> +
> +            goto cleanup;
> +        }
> +    }
> +
> +    if (VIR_FCLOSE(f) == EOF) {
> +        rc = errno;
> +        goto cleanup;
> +    }
> +
> +    if (istmp) {
> +        if (rename(tmp, path)<  0) {
> +            rc = errno;
> +            unlink(tmp);
> +            goto cleanup;
> +        }
> +
> +        if (unlink(tmp)<  0) {
> +            rc = errno;
> +            goto cleanup;
> +        }
> +    }
> +
> + cleanup:
> +    VIR_FREE(tmp);
> +
> +    return rc;
> +}
> +
> +static int
> +addnhostsSave(dnsmasqAddnHostsfile *addnhostsfile)
> +{
> +    int err = addnhostsWrite(addnhostsfile->path, addnhostsfile->hosts,
> +                             addnhostsfile->nhosts);
> +
> +    if (err<  0) {
> +        virReportSystemError(err, _("cannot write config file '%s'"),
> +                             addnhostsfile->path);
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +static int
> +addnhostsDelete(dnsmasqAddnHostsfile *addnhostsfile)
> +{
> +    if (!virFileExists(addnhostsfile->path))
> +        return 0;
> +
> +    if (unlink(addnhostsfile->path)<  0) {
> +        virReportSystemError(errno, _("cannot remove config file '%s'"),
> +                             addnhostsfile->path);
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +static void
>   hostsfileFree(dnsmasqHostsfile *hostsfile)
>   {
>       unsigned int i;
> @@ -255,6 +481,8 @@ dnsmasqContextNew(const char *network_name,
>
>       if (!(ctx->hostsfile = hostsfileNew(network_name, config_dir)))
>           goto error;
> +    if (!(ctx->addnhostsfile = addnhostsNew(network_name, config_dir)))
> +        goto error;
>
>       return ctx;
>
> @@ -277,6 +505,8 @@ dnsmasqContextFree(dnsmasqContext *ctx)
>
>       if (ctx->hostsfile)
>           hostsfileFree(ctx->hostsfile);
> +    if (ctx->addnhostsfile)
> +        addnhostsFree(ctx->addnhostsfile);
>
>       VIR_FREE(ctx);
>   }
> @@ -300,6 +530,24 @@ dnsmasqAddDhcpHost(dnsmasqContext *ctx,
>           hostsfileAdd(ctx->hostsfile, mac, ip, name);
>   }
>
> +/*
> + * dnsmasqAddHost:
> + * @ctx: pointer to the dnsmasq context for each network
> + * @ip: pointer to the socket address contains ip of the host
> + * @name: pointer to the string contains hostname of the host
> + *
> + * Add additional host entry.
> + */
> +
> +void
> +dnsmasqAddHost(dnsmasqContext *ctx,
> +               virSocketAddr *ip,
> +               const char *name)
> +{
> +    if (ctx->addnhostsfile)
> +        addnhostsAdd(ctx->addnhostsfile, ip, name);
> +}
> +
>   /**
>    * dnsmasqSave:
>    * @ctx: pointer to the dnsmasq context for each network
> @@ -309,10 +557,15 @@ dnsmasqAddDhcpHost(dnsmasqContext *ctx,
>   int
>   dnsmasqSave(const dnsmasqContext *ctx)
>   {
> +    int ret1 = 0;
> +    int ret2 = 0;
> +
>       if (ctx->hostsfile)
> -        return hostsfileSave(ctx->hostsfile);
> +        ret1 = hostsfileSave(ctx->hostsfile);
> +    if (ctx->addnhostsfile)
> +        ret2 = addnhostsSave(ctx->addnhostsfile);
>
> -    return 0;
> +    return ((ret1 == 0)&&  (ret2 == 0)) ? 0 : -1;
>   }
>
>
> @@ -325,10 +578,15 @@ dnsmasqSave(const dnsmasqContext *ctx)
>   int
>   dnsmasqDelete(const dnsmasqContext *ctx)
>   {
> +    int ret1 = 0;
> +    int ret2 = 0;
> +
>       if (ctx->hostsfile)
> -        return hostsfileDelete(ctx->hostsfile);
> +        ret1 = hostsfileDelete(ctx->hostsfile);
> +    if (ctx->addnhostsfile)
> +        ret2 = addnhostsDelete(ctx->addnhostsfile);
>
> -    return 0;
> +    return ((ret1 == 0)&&  (ret2 == 0)) ? 0 : -1;
>   }
>
>   /**
> diff --git a/src/util/dnsmasq.h b/src/util/dnsmasq.h
> index 02a961f..3f6320a 100644
> --- a/src/util/dnsmasq.h
> +++ b/src/util/dnsmasq.h
> @@ -44,7 +44,24 @@ typedef struct
>
>   typedef struct
>   {
> -    dnsmasqHostsfile *hostsfile;
> +    unsigned int    nhostnames;
> +    char            *ip;
> +    char            **hostnames;
> +
> +} dnsmasqAddnHost;
> +
> +typedef struct
> +{
> +    unsigned int     nhosts;
> +    dnsmasqAddnHost *hosts;
> +
> +    char            *path;  /* Absolute path of dnsmasq's hostsfile. */
> +} dnsmasqAddnHostsfile;
> +
> +typedef struct
> +{
> +    dnsmasqHostsfile     *hostsfile;
> +    dnsmasqAddnHostsfile *addnhostsfile;
>   } dnsmasqContext;
>
>   dnsmasqContext * dnsmasqContextNew(const char *network_name,
> @@ -54,6 +71,9 @@ void             dnsmasqAddDhcpHost(dnsmasqContext *ctx,
>                                       const char *mac,
>                                       virSocketAddr *ip,
>                                       const char *name);
> +void             dnsmasqAddHost(dnsmasqContext *ctx,
> +                                virSocketAddr *ip,
> +                                const char *name);
>   int              dnsmasqSave(const dnsmasqContext *ctx);
>   int              dnsmasqDelete(const dnsmasqContext *ctx);
>   int              dnsmasqReload(pid_t pid);




More information about the libvir-list mailing list