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@xxxxxxxxx>
---
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
