On 4/22/20 10:05 PM, Julio Faracco wrote:
If an user is trying to configure a dhcp neetwork settings, it is not possible to change the leasetime of a range or a host entry. This is available using dnsmasq extra options, but they are associated with dhcp-range or dhcp-hosts fields. This patch implements a leasetime for range and hosts tags. They can be defined under that settings:
<dhcp> <range ...> <lease/> </range> <host ...> <lease/> </host> </dhcp>
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=913446
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- docs/schemas/basictypes.rng | 8 ++ docs/schemas/network.rng | 20 +++++ src/conf/network_conf.c | 159 +++++++++++++++++++++++++++++++----- src/conf/network_conf.h | 27 +++++- src/libvirt_private.syms | 3 + src/network/bridge_driver.c | 56 +++++++++++-- src/network/bridge_driver.h | 1 + src/test/test_driver.c | 2 +- src/util/virdnsmasq.c | 60 +++++++++----- src/util/virdnsmasq.h | 3 + src/vbox/vbox_network.c | 16 ++-- tests/networkxml2conftest.c | 15 ++-- 12 files changed, 306 insertions(+), 64 deletions(-)
diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 81465273c8..271ed18afb 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -607,4 +607,12 @@ </element> </define>
+ <define name="leaseUnit"> + <choice> + <value>seconds</value> + <value>mins</value> + <value>hours</value>
Since seconds and hours are specified fully, I think "mins" should be too. It's not any longer than "seconds" anyway :-)
+ </choice> + </define> + </grammar> diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index 60453225d6..88b6f4dfdd 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -371,6 +371,16 @@ <element name="range"> <attribute name="start"><ref name="ipAddr"/></attribute> <attribute name="end"><ref name="ipAddr"/></attribute> + <interleave> + <optional> + <element name="lease"> + <attribute name="expiry"><ref name="unsignedLong"/></attribute> + <optional> + <attribute name="unit"><ref name="leaseUnit"/></attribute> + </optional> + </element> + </optional> + </interleave> </element> </zeroOrMore> <zeroOrMore> @@ -388,6 +398,16 @@ <attribute name="name"><text/></attribute> </choice> <attribute name="ip"><ref name="ipAddr"/></attribute> + <interleave> + <optional> + <element name="lease"> + <attribute name="expiry"><ref name="unsignedLong"/></attribute> + <optional> + <attribute name="unit"><ref name="leaseUnit"/></attribute> + </optional> + </element> + </optional> + </interleave> </element> </zeroOrMore> <optional> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 819b645df7..286a0edb7c 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -70,6 +70,13 @@ VIR_ENUM_IMPL(virNetworkTaint, "hook-script", );
+VIR_ENUM_IMPL(virNetworkDHCPLeaseTimeUnit, + VIR_NETWORK_DHCP_LEASETIME_UNIT_LAST, + "seconds", + "mins", + "hours", +); + static virClassPtr virNetworkXMLOptionClass;
static void @@ -132,12 +139,20 @@ virNetworkForwardPfDefClear(virNetworkForwardPfDefPtr def) }
+static void +virNetworkDHCPLeaseTimeDefClear(virNetworkDHCPLeaseTimeDefPtr lease) +{ + VIR_FREE(lease); +} + + static void virNetworkDHCPHostDefClear(virNetworkDHCPHostDefPtr def) { VIR_FREE(def->mac); VIR_FREE(def->id); VIR_FREE(def->name); + VIR_FREE(def->lease); }
@@ -145,6 +160,9 @@ static void virNetworkIPDefClear(virNetworkIPDefPtr def) { VIR_FREE(def->family); + + while (def->nranges) + virNetworkDHCPLeaseTimeDefClear(def->ranges[--def->nranges].lease); VIR_FREE(def->ranges);
while (def->nhosts) @@ -391,11 +409,55 @@ int virNetworkIPDefNetmask(const virNetworkIPDef *def,
static int -virSocketAddrRangeParseXML(const char *networkName, - virNetworkIPDefPtr ipdef, - xmlNodePtr node, - virSocketAddrRangePtr range) +virNetworkDHCPLeaseTimeDefParseXML(virNetworkDHCPLeaseTimeDefPtr *lease, + xmlNodePtr node) +{ + virNetworkDHCPLeaseTimeDefPtr new_lease = *lease; + g_autofree char *expiry = NULL, *unit = NULL; + + if (!(expiry = virXMLPropString(node, "expiry"))) + return 0; + + if (VIR_ALLOC(new_lease) < 0) + return -1; + + if (virStrToLong_ul(expiry, NULL, 10, &new_lease->expiry) < 0) + return -1; + + if (!(unit = virXMLPropString(node, "unit"))) + new_lease->unit = VIR_NETWORK_DHCP_LEASETIME_UNIT_MINUTES; + else + new_lease->unit = virNetworkDHCPLeaseTimeUnitTypeFromString(unit);
Ouch. This is unsafe. Firstly, if an uses specifies say unit="microfornight" TypeFromString() will return -1 because the string is not from the enum. Then, assigning -1 to the virNetworkDHCPLeaseTimeUnitType enum which doesn't contain that value is dangerous too...
+ + /* infinite */ + if (new_lease->expiry > 0) { + /* This boundary check is related to dnsmasq man page settings: + * "The minimum lease time is two minutes." */ + if ((new_lease->unit == VIR_NETWORK_DHCP_LEASETIME_UNIT_SECONDS && + new_lease->expiry < 120) || + (new_lease->unit == VIR_NETWORK_DHCP_LEASETIME_UNIT_MINUTES && + new_lease->expiry < 2)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("The minimum lease time should be greater " + "than 2 minutes")); + return -1; + } + } + + *lease = new_lease; + + return 0; +} + +
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index f06099297a..87f0452611 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -966,6 +966,30 @@ static int networkConnectIsAlive(virConnectPtr conn G_GNUC_UNUSED) }
+static char * +networkBuildDnsmasqLeaseTime(virNetworkDHCPLeaseTimeDefPtr lease) +{ + char *leasetime = NULL; + const char *unit; + g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + + if (!lease) + return NULL; + + if (lease->expiry == 0) { + virBufferAddLit(&buf, "infinite"); + } else { + unit = virNetworkDHCPLeaseTimeUnitTypeToString(lease->unit); + /* We get only first compatible char from string: 's', 'm' or 'h' */ + virBufferAsprintf(&buf, "%lu%c", lease->expiry, unit[0]);
... because here unit will be NULL and dereferenced. Michal