On 06/20/2017 02:00 PM, aruiz@xxxxxxxxx wrote:
> From: Alberto Ruiz <aruiz@xxxxxxxxx>
>
> ---
> src/network/bridge_driver.c | 56 ++++++++++++++++++++++++++-------------------
> src/util/virdnsmasq.c | 52 +++++++++++++++++++----------------------
> src/util/virdnsmasq.h | 1 +
> 3 files changed, 57 insertions(+), 52 deletions(-)
As far as I can see, this doesn't set a different lease time for each
static host entry (which is what the title implies), but just sets the
single specified leasetime for *all* static host entries.
Aside from that, I'm not sure what is the value of setting a leastime on
a static host entry. An explanation of that in the commit log would be
helpful in determining whether or not there's even a point to doing this.
Also, I forgot to say it wrt to the previous patch, but you need to add
something to docs/formatnetwork.html.in to document the new config knob.
>
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index e51e469c8..1cffd4dcf 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -861,30 +861,6 @@ networkKillDaemon(pid_t pid, const char *daemonName, const char *networkName)
> return ret;
> }
>
> -/* the following does not build a file, it builds a list
> - * which is later saved into a file
> - */
> -
> -static int
> -networkBuildDnsmasqDhcpHostsList(dnsmasqContext *dctx,
> - virNetworkIPDefPtr ipdef)
> -{
> - size_t i;
> - bool ipv6 = false;
> -
> - if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET6))
> - ipv6 = true;
> - for (i = 0; i < ipdef->nhosts; i++) {
> - virNetworkDHCPHostDefPtr host = &(ipdef->hosts[i]);
> - if (VIR_SOCKET_ADDR_VALID(&host->ip))
> - if (dnsmasqAddDhcpHost(dctx, host->mac, &host->ip,
> - host->name, host->id, ipv6) < 0)
> - return -1;
> - }
> -
> - return 0;
> -}
> -
> static int
> networkBuildDnsmasqHostsList(dnsmasqContext *dctx,
> virNetworkDNSDefPtr dnsdef)
> @@ -940,6 +916,38 @@ networkDnsmasqConfLeaseValueToString (int64_t leasetime)
> return result;
> }
>
> +/* the following does not build a file, it builds a list
> + * which is later saved into a file
> + */
> +
> +static int
> +networkBuildDnsmasqDhcpHostsList(dnsmasqContext *dctx,
> + virNetworkIPDefPtr ipdef)
> +{
> + int ret = -1;
> + size_t i;
> + bool ipv6 = false;
> + char *leasetime = networkDnsmasqConfLeaseValueToString(ipdef->leasetime);
> +
> + if (!leasetime)
> + goto cleanup;
> +
> + if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET6))
> + ipv6 = true;
> + for (i = 0; i < ipdef->nhosts; i++) {
> + virNetworkDHCPHostDefPtr host = &(ipdef->hosts[i]);
> + if (VIR_SOCKET_ADDR_VALID(&host->ip))
> + if (dnsmasqAddDhcpHost(dctx, host->mac, &host->ip,
> + host->name, host->id, leasetime, ipv6) < 0)
> + goto cleanup;
> + }
> +
> + ret = 0;
> +cleanup:
> + VIR_FREE(leasetime);
> + return ret;
> +}
> +
> int
> networkDnsmasqConfContents(virNetworkObjPtr network,
> const char *pidfile,
> diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c
> index 1b78c1fad..92f834fe7 100644
> --- a/src/util/virdnsmasq.c
> +++ b/src/util/virdnsmasq.c
> @@ -308,52 +308,47 @@ hostsfileAdd(dnsmasqHostsfile *hostsfile,
> virSocketAddr *ip,
> const char *name,
> const char *id,
> + const char *leasetime,
> bool ipv6)
> {
> + int ret = -1;
> char *ipstr = NULL;
> + virBuffer hostbuf = VIR_BUFFER_INITIALIZER;
> +
> if (VIR_REALLOC_N(hostsfile->hosts, hostsfile->nhosts + 1) < 0)
> goto error;
>
> if (!(ipstr = virSocketAddrFormat(ip)))
> - return -1;
> + goto error;
>
> /* the first test determines if it is a dhcpv6 host */
> if (ipv6) {
> - if (name && id) {
> - if (virAsprintf(&hostsfile->hosts[hostsfile->nhosts].host,
> - "id:%s,%s,[%s]", id, name, ipstr) < 0)
> - goto error;
> - } else if (name && !id) {
> - if (virAsprintf(&hostsfile->hosts[hostsfile->nhosts].host,
> - "%s,[%s]", name, ipstr) < 0)
> - goto error;
> - } else if (!name && id) {
> - if (virAsprintf(&hostsfile->hosts[hostsfile->nhosts].host,
> - "id:%s,[%s]", id, ipstr) < 0)
> - goto error;
> - }
> + if (name && id)
> + virBufferAsprintf(&hostbuf, "id:%s,%s,[%s]", id, name, ipstr);
> + else if (name && !id)
> + virBufferAsprintf(&hostbuf, "%s,[%s]", name, ipstr);
> + else if (!name && id)
> + virBufferAsprintf(&hostbuf, "id:%s,[%s]", id, ipstr);
> } else if (name && mac) {
> - if (virAsprintf(&hostsfile->hosts[hostsfile->nhosts].host, "%s,%s,%s",
> - mac, ipstr, name) < 0)
> - goto error;
> + virBufferAsprintf(&hostbuf, "%s,%s,%s", mac, ipstr, name);
> } else if (name && !mac) {
> - if (virAsprintf(&hostsfile->hosts[hostsfile->nhosts].host, "%s,%s",
> - name, ipstr) < 0)
> - goto error;
> + virBufferAsprintf(&hostbuf, "%s,%s", name, ipstr);
> } else {
> - if (virAsprintf(&hostsfile->hosts[hostsfile->nhosts].host, "%s,%s",
> - mac, ipstr) < 0)
> - goto error;
> + virBufferAsprintf(&hostbuf, "%s,%s", mac, ipstr);
> }
> - VIR_FREE(ipstr);
>
> - hostsfile->nhosts++;
> + /* The leasetime string already includes comma if there's any value at all */
> + virBufferAsprintf(&hostbuf, "%s", leasetime);
>
> - return 0;
> + if (!(hostsfile->hosts[hostsfile->nhosts].host = virBufferContentAndReset (&hostbuf)))
> + goto error;
>
> + hostsfile->nhosts++;
> + ret = 0;
> error:
> + virBufferFreeAndReset(&hostbuf);
> VIR_FREE(ipstr);
> - return -1;
> + return ret;
> }
>
> static dnsmasqHostsfile *
> @@ -524,9 +519,10 @@ dnsmasqAddDhcpHost(dnsmasqContext *ctx,
> virSocketAddr *ip,
> const char *name,
> const char *id,
> + const char *leasetime,
> bool ipv6)
> {
> - return hostsfileAdd(ctx->hostsfile, mac, ip, name, id, ipv6);
> + return hostsfileAdd(ctx->hostsfile, mac, ip, name, id, leasetime, ipv6);
> }
>
> /*
> diff --git a/src/util/virdnsmasq.h b/src/util/virdnsmasq.h
> index f47bea3ab..c3ea271d4 100644
> --- a/src/util/virdnsmasq.h
> +++ b/src/util/virdnsmasq.h
> @@ -88,6 +88,7 @@ int dnsmasqAddDhcpHost(dnsmasqContext *ctx,
> virSocketAddr *ip,
> const char *name,
> const char *id,
> + const char *leastime,
> bool ipv6);
> int dnsmasqAddHost(dnsmasqContext *ctx,
> virSocketAddr *ip,
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
