<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Apr 18, 2016 at 11:52 PM, Cole Robinson <span dir="ltr"><<a href="mailto:crobinso@redhat.com" target="_blank">crobinso@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 04/15/2016 08:18 PM, Alberto Ruiz wrote:<br>
> From 112f61ec5cfdc39f7a157825c4209f7bae34c483 Mon Sep 17 00:00:00 2001<br>
> From: Alberto Ruiz <<a href="mailto:aruiz@gnome.org">aruiz@gnome.org</a>><br>
> Date: Wed, 13 Apr 2016 17:00:45 +0100<br>
> Subject: [PATCH] network: Add support for dhcp-range lease time in the network<br>
>  XML configuration format and dnsmasq<br>
><br>
<br>
Also mention the bug in the commit message, just link it like<br>
<br>
<a href="https://bugzilla.redhat.com/show_bug.cgi?id=913446" rel="noreferrer" target="_blank">https://bugzilla.redhat.com/show_bug.cgi?id=913446</a><br>
<br>
Needs documentation but that will be dependent on what the final patch looks<br>
like, so fine to skip for now.<br>
<br>
The main questions are:<br>
<br>
1) is the XML format fine? <range ... lease='XXX'/>. lease sounds kinda<br>
non-specific to me, maybe leasetime or leaseTime.<br></blockquote><div><br></div><div>Sounds good to me, though since pointed out in a later email, we might want to make this a child tag on its own within <dhcp> instead of a per range property.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
2) what to use for the input format? right now it's just string passthrough to<br>
dnsmasq, which takes a format like XX[s|m|h|d|w], or 'infinite'. Accepting<br>
that format kind of sticks us with that for all time, which probably isn't a<br>
good precedent. the easy way would probably be to just say the value needs to<br>
be in minutes, and maybe -1 == infinite. But that will take a bit more code to<br>
adapt that value to the dnsmasq format.<br></blockquote><div><br></div><div>Sticking to minutes seems like a loss of granularity, we can't really predict that a given setup might care about second level granularity for leases can we? I'm all for the argument of sanity checking the input and not doing an opaque passthrough, but I think in the end we might want to support some sort of seconds/minutes/hours/days notation (or stick to seconds and let people figure the amount out with a calculator).<br><br>Thanks a lot for the input.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
CC laining for his thoughts<br>
<br>
And one tiny comment below:<br>
<br>
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c<br>
> index 4fb2e2a..449c9ed 100644<br>
> --- a/src/conf/network_conf.c<br>
> +++ b/src/conf/network_conf.c<br>
> @@ -313,6 +313,10 @@ static void<br>
>  virNetworkIpDefClear(virNetworkIpDefPtr def)<br>
>  {<br>
>      VIR_FREE(def->family);<br>
> +<br>
> +    while (def->nranges)<br>
> +        VIR_FREE(def->ranges[--def->nranges].lease);<br>
> +<br>
>      VIR_FREE(def->ranges);<br>
><br>
>      while (def->nhosts)<br>
> @@ -855,7 +859,6 @@ int virNetworkIpDefNetmask(const virNetworkIpDef *def,<br>
>                                          VIR_SOCKET_ADDR_FAMILY(&def->address));<br>
>  }<br>
><br>
> -<br>
<br>
stray whitespace change here<br>
<span class="HOEnZb"><font color="#888888"><br>
- Cole<br>
</font></span></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div>Alberto Ruiz<br></div><span>Associate </span>Engineering Manager - Desktop Management Tools<br></div>Red Hat<br></div></div></div></div>
</div></div>