[PATCH 0/2] Include lease time option into DHCP settings
This series is based on latest series from Alberto. It includes a new entry called <leasetime/> under <dhcp/> scope to add a default lease time for range and host options for dnsmasq. There is no point to configure both separately. If they are defined (range and/or host), they should have the same lease time value. This series includes some test cases to cover lease time XML syntax also. Julio Faracco (2): conf: Add <leasetime/> option for <dhcp/> settings tests: Add tests for <leasetime/> to cover dnsmasq settings docs/schemas/basictypes.rng | 9 +++ docs/schemas/network.rng | 11 ++++ src/conf/network_conf.c | 62 ++++++++++++++++++- src/conf/network_conf.h | 14 +++++ src/libvirt_private.syms | 2 + src/network/bridge_driver.c | 37 ++++++++++- src/util/virdnsmasq.c | 40 ++++++------ src/util/virdnsmasq.h | 1 + .../networkxml2confdata/leasetime-hours.conf | 16 +++++ tests/networkxml2confdata/leasetime-hours.xml | 12 ++++ .../leasetime-infinite.conf | 16 +++++ .../leasetime-infinite.xml | 12 ++++ .../leasetime-minutes.conf | 16 +++++ .../networkxml2confdata/leasetime-minutes.xml | 12 ++++ .../leasetime-seconds.conf | 16 +++++ .../networkxml2confdata/leasetime-seconds.xml | 12 ++++ tests/networkxml2conftest.c | 4 ++ tests/networkxml2xmlin/leasetime-hours.xml | 12 ++++ tests/networkxml2xmlin/leasetime-infinite.xml | 12 ++++ tests/networkxml2xmlin/leasetime-minutes.xml | 12 ++++ tests/networkxml2xmlin/leasetime-seconds.xml | 12 ++++ tests/networkxml2xmlout/leasetime-hours.xml | 14 +++++ .../networkxml2xmlout/leasetime-infinite.xml | 14 +++++ tests/networkxml2xmlout/leasetime-minutes.xml | 14 +++++ tests/networkxml2xmlout/leasetime-seconds.xml | 14 +++++ tests/networkxml2xmltest.c | 4 ++ 26 files changed, 376 insertions(+), 24 deletions(-) create mode 100644 tests/networkxml2confdata/leasetime-hours.conf create mode 100644 tests/networkxml2confdata/leasetime-hours.xml create mode 100644 tests/networkxml2confdata/leasetime-infinite.conf create mode 100644 tests/networkxml2confdata/leasetime-infinite.xml create mode 100644 tests/networkxml2confdata/leasetime-minutes.conf create mode 100644 tests/networkxml2confdata/leasetime-minutes.xml create mode 100644 tests/networkxml2confdata/leasetime-seconds.conf create mode 100644 tests/networkxml2confdata/leasetime-seconds.xml create mode 100644 tests/networkxml2xmlin/leasetime-hours.xml create mode 100644 tests/networkxml2xmlin/leasetime-infinite.xml create mode 100644 tests/networkxml2xmlin/leasetime-minutes.xml create mode 100644 tests/networkxml2xmlin/leasetime-seconds.xml create mode 100644 tests/networkxml2xmlout/leasetime-hours.xml create mode 100644 tests/networkxml2xmlout/leasetime-infinite.xml create mode 100644 tests/networkxml2xmlout/leasetime-minutes.xml create mode 100644 tests/networkxml2xmlout/leasetime-seconds.xml -- 2.24.1
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 default leasetime for both. If this XML entry is defined, it applies leasetime for each range or host defined under DHCP scope. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=913446 Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- docs/schemas/basictypes.rng | 9 ++++++ docs/schemas/network.rng | 11 +++++++ src/conf/network_conf.c | 62 ++++++++++++++++++++++++++++++++++++- src/conf/network_conf.h | 14 +++++++++ src/libvirt_private.syms | 2 ++ src/network/bridge_driver.c | 37 ++++++++++++++++++++-- src/util/virdnsmasq.c | 40 ++++++++++++------------ src/util/virdnsmasq.h | 1 + 8 files changed, 152 insertions(+), 24 deletions(-) diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 81465273c8..12f085c101 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -607,4 +607,13 @@ </element> </define> + <define name="leaseUnit"> + <choice> + <value>seconds</value> + <value>minutes</value> + <value>hours</value> + <value>infinite</value> + </choice> + </define> + </grammar> diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index 60453225d6..9a93529d52 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -398,6 +398,17 @@ </optional> </element> </optional> + <optional> + <element name="leasetime"> + <optional> + <attribute name="unit"><ref name="leaseUnit"/></attribute> + </optional> + <choice> + <data type="unsignedLong"/> + <empty/> + </choice> + </element> + </optional> </interleave> </element> </optional> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 819b645df7..e6e82ee9fc 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -70,6 +70,14 @@ VIR_ENUM_IMPL(virNetworkTaint, "hook-script", ); +VIR_ENUM_IMPL(virNetworkDHCPLeaseTimeUnit, + VIR_NETWORK_DHCP_LEASETIME_UNIT_LAST, + "seconds", + "minutes", + "hours", + "infinite", +); + static virClassPtr virNetworkXMLOptionClass; static void @@ -552,9 +560,50 @@ virNetworkDHCPHostDefParseXML(const char *networkName, } +static int +virNetworkDHCPLeaseTimeDefParseXML(virNetworkIPDefPtr def, + xmlNodePtr node, + xmlXPathContextPtr ctxt) +{ + g_autofree char *leasetime = NULL, *leaseunit = NULL; + + if (!(leaseunit = virXMLPropString(node, "unit"))) + def->leaseunit = VIR_NETWORK_DHCP_LEASETIME_UNIT_SECONDS; + else + def->leaseunit = virNetworkDHCPLeaseTimeUnitTypeFromString(leaseunit); + + if (def->leaseunit == VIR_NETWORK_DHCP_LEASETIME_UNIT_INFINITE) + return 0; + + if (!(leasetime = virXPathString("string(./dhcp/leasetime)", ctxt))) + return -1; + + if (virStrToLong_ul(leasetime, NULL, 10, &def->leasetime) < 0) + return -1; + + /* This boundary check is related to dnsmasq man page settings: + * "The minimum lease time is two minutes." */ + if ((def->leaseunit == VIR_NETWORK_DHCP_LEASETIME_UNIT_SECONDS && + def->leasetime < 120) || + (def->leaseunit == VIR_NETWORK_DHCP_LEASETIME_UNIT_MINUTES && + def->leasetime < 2)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("The minimum lease time should be greater " + "than 2 minutes")); + return -1; + } + + if (def->leasetime > 0) + return 0; + + return -1; +} + + static int virNetworkDHCPDefParseXML(const char *networkName, xmlNodePtr node, + xmlXPathContextPtr ctxt, virNetworkIPDefPtr def) { int ret = -1; @@ -583,7 +632,11 @@ virNetworkDHCPDefParseXML(const char *networkName, goto cleanup; if (VIR_APPEND_ELEMENT(def->hosts, def->nhosts, host) < 0) goto cleanup; + } else if (cur->type == XML_ELEMENT_NODE && + virXMLNodeNameEqual(cur, "leasetime")) { + if (virNetworkDHCPLeaseTimeDefParseXML(def, cur, ctxt) < 0) + goto cleanup; } else if (VIR_SOCKET_ADDR_IS_FAMILY(&def->address, AF_INET) && cur->type == XML_ELEMENT_NODE && virXMLNodeNameEqual(cur, "bootp")) { @@ -1143,7 +1196,7 @@ virNetworkIPDefParseXML(const char *networkName, } if ((dhcp = virXPathNode("./dhcp[1]", ctxt)) && - virNetworkDHCPDefParseXML(networkName, dhcp, def) < 0) + virNetworkDHCPDefParseXML(networkName, dhcp, ctxt, def) < 0) goto cleanup; if (virXPathNode("./tftp[1]", ctxt)) { @@ -2343,6 +2396,13 @@ virNetworkIPDefFormat(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); } + if (def->leaseunit == VIR_NETWORK_DHCP_LEASETIME_UNIT_INFINITE) { + virBufferAddLit(buf, "<leasetime unit='infinite'/>\n"); + } else if (def->leasetime) { + virBufferAsprintf(buf, "<leasetime unit='%s'>%lu</leasetime>\n", + virNetworkDHCPLeaseTimeUnitTypeToString(def->leaseunit), + def->leasetime); + } virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</dhcp>\n"); diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index db7243eef5..66a5e1ad4f 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -80,6 +80,17 @@ typedef enum { VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_LAST, } virNetworkForwardHostdevDeviceType; +typedef enum { + VIR_NETWORK_DHCP_LEASETIME_UNIT_SECONDS = 0, + VIR_NETWORK_DHCP_LEASETIME_UNIT_MINUTES, + VIR_NETWORK_DHCP_LEASETIME_UNIT_HOURS, + VIR_NETWORK_DHCP_LEASETIME_UNIT_INFINITE, + + VIR_NETWORK_DHCP_LEASETIME_UNIT_LAST, +} virNetworkDHCPLeaseTimeUnitType; + +VIR_ENUM_DECL(virNetworkDHCPLeaseTimeUnit); + /* The backend driver used for devices from the pool. Currently used * only for PCI devices (vfio vs. kvm), but could be used for other * device types in the future. @@ -176,6 +187,9 @@ struct _virNetworkIPDef { size_t nhosts; /* Zero or more dhcp hosts */ virNetworkDHCPHostDefPtr hosts; + unsigned long leasetime; + virNetworkDHCPLeaseTimeUnitType leaseunit; + char *tftproot; char *bootfile; virSocketAddr bootserver; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ec367653d5..79bb5cc160 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -770,6 +770,8 @@ virNetworkDefParseNode; virNetworkDefParseString; virNetworkDefParseXML; virNetworkDefUpdateSection; +virNetworkDHCPLeaseTimeUnitTypeFromString; +virNetworkDHCPLeaseTimeUnitTypeToString; virNetworkForwardTypeToString; virNetworkIPDefNetmask; virNetworkIPDefPrefix; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index f06099297a..d587e266de 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -966,6 +966,27 @@ static int networkConnectIsAlive(virConnectPtr conn G_GNUC_UNUSED) } +static char * +networkBuildDnsmasqLeaseTime(virNetworkIPDefPtr ipdef) +{ + char *leasetime = NULL; + const char *unit; + g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + + if (ipdef->leaseunit == VIR_NETWORK_DHCP_LEASETIME_UNIT_INFINITE) { + virBufferAddLit(&buf, "infinite"); + } else if (ipdef->leasetime) { + unit = virNetworkDHCPLeaseTimeUnitTypeToString(ipdef->leaseunit); + /* We get only first compatible char from string: 's', 'm' or 'h' */ + virBufferAsprintf(&buf, "%lu%c", ipdef->leasetime, unit[0]); + } + + leasetime = virBufferContentAndReset(&buf); + + return leasetime; +} + + /* the following does not build a file, it builds a list * which is later saved into a file */ @@ -975,6 +996,9 @@ networkBuildDnsmasqDhcpHostsList(dnsmasqContext *dctx, { size_t i; bool ipv6 = false; + g_autofree char *leasetime = NULL; + + leasetime = networkBuildDnsmasqLeaseTime(ipdef); if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET6)) ipv6 = true; @@ -982,7 +1006,8 @@ networkBuildDnsmasqDhcpHostsList(dnsmasqContext *dctx, 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) + host->name, host->id, leasetime, + ipv6) < 0) return -1; } @@ -1381,13 +1406,14 @@ networkDnsmasqConfContents(virNetworkObjPtr obj, } for (r = 0; r < ipdef->nranges; r++) { int thisRange; + g_autofree char *leasetime = NULL; if (!(saddr = virSocketAddrFormat(&ipdef->ranges[r].start)) || !(eaddr = virSocketAddrFormat(&ipdef->ranges[r].end))) goto cleanup; if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET6)) { - virBufferAsprintf(&configbuf, "dhcp-range=%s,%s,%d\n", + virBufferAsprintf(&configbuf, "dhcp-range=%s,%s,%d", saddr, eaddr, prefix); } else { /* IPv4 - dnsmasq requires a netmask rather than prefix */ @@ -1404,10 +1430,15 @@ networkDnsmasqConfContents(virNetworkObjPtr obj, if (!(netmaskStr = virSocketAddrFormat(&netmask))) goto cleanup; - virBufferAsprintf(&configbuf, "dhcp-range=%s,%s,%s\n", + virBufferAsprintf(&configbuf, "dhcp-range=%s,%s,%s", saddr, eaddr, netmaskStr); } + if ((leasetime = networkBuildDnsmasqLeaseTime(ipdef))) + virBufferAsprintf(&configbuf, ",%s", leasetime); + + virBufferAddLit(&configbuf, "\n"); + VIR_FREE(saddr); VIR_FREE(eaddr); thisRange = virSocketAddrGetRange(&ipdef->ranges[r].start, diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c index adc6f96bb6..fc977a47f5 100644 --- a/src/util/virdnsmasq.c +++ b/src/util/virdnsmasq.c @@ -296,11 +296,14 @@ hostsfileAdd(dnsmasqHostsfile *hostsfile, virSocketAddr *ip, const char *name, const char *id, + const char *leasetime, bool ipv6) { - char *ipstr = NULL; + g_autofree char *ipstr = NULL; + g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + if (VIR_REALLOC_N(hostsfile->hosts, hostsfile->nhosts + 1) < 0) - goto error; + return -1; if (!(ipstr = virSocketAddrFormat(ip))) return -1; @@ -308,34 +311,30 @@ hostsfileAdd(dnsmasqHostsfile *hostsfile, /* the first test determines if it is a dhcpv6 host */ if (ipv6) { if (name && id) { - hostsfile->hosts[hostsfile->nhosts].host = g_strdup_printf("id:%s,%s,[%s]", - id, name, ipstr); + virBufferAsprintf(&buf, "id:%s,%s", id, name); } else if (name && !id) { - hostsfile->hosts[hostsfile->nhosts].host = g_strdup_printf("%s,[%s]", - name, ipstr); + virBufferAsprintf(&buf, "%s", name); } else if (!name && id) { - hostsfile->hosts[hostsfile->nhosts].host = g_strdup_printf("id:%s,[%s]", - id, ipstr); + virBufferAsprintf(&buf, "id:%s", id); } + virBufferAsprintf(&buf, ",[%s]", ipstr); } else if (name && mac) { - hostsfile->hosts[hostsfile->nhosts].host = g_strdup_printf("%s,%s,%s", - mac, ipstr, name); + virBufferAsprintf(&buf, "%s,%s,%s", mac, ipstr, name); } else if (name && !mac) { - hostsfile->hosts[hostsfile->nhosts].host = g_strdup_printf("%s,%s", name, - ipstr); + virBufferAsprintf(&buf, "%s,%s", name, ipstr); } else { - hostsfile->hosts[hostsfile->nhosts].host = g_strdup_printf("%s,%s", mac, - ipstr); + virBufferAsprintf(&buf, "%s,%s", mac, ipstr); } - VIR_FREE(ipstr); + + if (leasetime) + virBufferAsprintf(&buf, ",%s", leasetime); + + if (!(hostsfile->hosts[hostsfile->nhosts].host = virBufferContentAndReset(&buf))) + return -1; hostsfile->nhosts++; return 0; - - error: - VIR_FREE(ipstr); - return -1; } static dnsmasqHostsfile * @@ -501,9 +500,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 ff0e56d635..6fbc101e64 100644 --- a/src/util/virdnsmasq.h +++ b/src/util/virdnsmasq.h @@ -87,6 +87,7 @@ int dnsmasqAddDhcpHost(dnsmasqContext *ctx, virSocketAddr *ip, const char *name, const char *id, + const char *leasetime, bool ipv6); int dnsmasqAddHost(dnsmasqContext *ctx, virSocketAddr *ip, -- 2.24.1
On Wed, Apr 15, 2020 at 01:18:49PM -0300, 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 default leasetime for both. If this XML entry is defined, it applies leasetime for each range or host defined under DHCP scope.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=913446
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- docs/schemas/basictypes.rng | 9 ++++++ docs/schemas/network.rng | 11 +++++++ src/conf/network_conf.c | 62 ++++++++++++++++++++++++++++++++++++- src/conf/network_conf.h | 14 +++++++++ src/libvirt_private.syms | 2 ++ src/network/bridge_driver.c | 37 ++++++++++++++++++++-- src/util/virdnsmasq.c | 40 ++++++++++++------------ src/util/virdnsmasq.h | 1 + 8 files changed, 152 insertions(+), 24 deletions(-)
diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 81465273c8..12f085c101 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -607,4 +607,13 @@ </element> </define>
+ <define name="leaseUnit"> + <choice> + <value>seconds</value> + <value>minutes</value> + <value>hours</value> + <value>infinite</value>
"infinite" is not really a unit - it is a value, so I don't think we should have this. At least one version of Alberto's patches used the value "0" to indicate no expiry, which I think is reasonable, as 0 is otherwise invalid. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
New tests are required to cover some new XML syntax entry for <leasetime/> option. This includes schema testing and other features like unit attribute and leasetime value. Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- tests/networkxml2confdata/leasetime-hours.conf | 16 ++++++++++++++++ tests/networkxml2confdata/leasetime-hours.xml | 12 ++++++++++++ .../networkxml2confdata/leasetime-infinite.conf | 16 ++++++++++++++++ tests/networkxml2confdata/leasetime-infinite.xml | 12 ++++++++++++ tests/networkxml2confdata/leasetime-minutes.conf | 16 ++++++++++++++++ tests/networkxml2confdata/leasetime-minutes.xml | 12 ++++++++++++ tests/networkxml2confdata/leasetime-seconds.conf | 16 ++++++++++++++++ tests/networkxml2confdata/leasetime-seconds.xml | 12 ++++++++++++ tests/networkxml2conftest.c | 4 ++++ tests/networkxml2xmlin/leasetime-hours.xml | 12 ++++++++++++ tests/networkxml2xmlin/leasetime-infinite.xml | 12 ++++++++++++ tests/networkxml2xmlin/leasetime-minutes.xml | 12 ++++++++++++ tests/networkxml2xmlin/leasetime-seconds.xml | 12 ++++++++++++ tests/networkxml2xmlout/leasetime-hours.xml | 14 ++++++++++++++ tests/networkxml2xmlout/leasetime-infinite.xml | 14 ++++++++++++++ tests/networkxml2xmlout/leasetime-minutes.xml | 14 ++++++++++++++ tests/networkxml2xmlout/leasetime-seconds.xml | 14 ++++++++++++++ tests/networkxml2xmltest.c | 4 ++++ 18 files changed, 224 insertions(+) create mode 100644 tests/networkxml2confdata/leasetime-hours.conf create mode 100644 tests/networkxml2confdata/leasetime-hours.xml create mode 100644 tests/networkxml2confdata/leasetime-infinite.conf create mode 100644 tests/networkxml2confdata/leasetime-infinite.xml create mode 100644 tests/networkxml2confdata/leasetime-minutes.conf create mode 100644 tests/networkxml2confdata/leasetime-minutes.xml create mode 100644 tests/networkxml2confdata/leasetime-seconds.conf create mode 100644 tests/networkxml2confdata/leasetime-seconds.xml create mode 100644 tests/networkxml2xmlin/leasetime-hours.xml create mode 100644 tests/networkxml2xmlin/leasetime-infinite.xml create mode 100644 tests/networkxml2xmlin/leasetime-minutes.xml create mode 100644 tests/networkxml2xmlin/leasetime-seconds.xml create mode 100644 tests/networkxml2xmlout/leasetime-hours.xml create mode 100644 tests/networkxml2xmlout/leasetime-infinite.xml create mode 100644 tests/networkxml2xmlout/leasetime-minutes.xml create mode 100644 tests/networkxml2xmlout/leasetime-seconds.xml diff --git a/tests/networkxml2confdata/leasetime-hours.conf b/tests/networkxml2confdata/leasetime-hours.conf new file mode 100644 index 0000000000..1599d4633e --- /dev/null +++ b/tests/networkxml2confdata/leasetime-hours.conf @@ -0,0 +1,16 @@ +##WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE +##OVERWRITTEN AND LOST. Changes to this configuration should be made using: +## virsh net-edit default +## or other application using the libvirt API. +## +## dnsmasq conf file created by libvirt +strict-order +except-interface=lo +bind-dynamic +interface=virbr0 +dhcp-range=192.168.122.2,192.168.122.254,255.255.255.0,1h +dhcp-no-override +dhcp-authoritative +dhcp-lease-max=253 +dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile +addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts diff --git a/tests/networkxml2confdata/leasetime-hours.xml b/tests/networkxml2confdata/leasetime-hours.xml new file mode 100644 index 0000000000..8df196e84e --- /dev/null +++ b/tests/networkxml2confdata/leasetime-hours.xml @@ -0,0 +1,12 @@ +<network xmlns:dnsmasq="http://libvirt.org/schemas/network/dnsmasq/1.0"> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <bridge name="virbr0"/> + <forward mode="nat" dev="eth1"/> + <ip address="192.168.122.1" netmask="255.255.255.0"> + <dhcp> + <range start="192.168.122.2" end="192.168.122.254"/> + <leasetime unit="hours">1</leasetime> + </dhcp> + </ip> +</network> diff --git a/tests/networkxml2confdata/leasetime-infinite.conf b/tests/networkxml2confdata/leasetime-infinite.conf new file mode 100644 index 0000000000..883ced6ade --- /dev/null +++ b/tests/networkxml2confdata/leasetime-infinite.conf @@ -0,0 +1,16 @@ +##WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE +##OVERWRITTEN AND LOST. Changes to this configuration should be made using: +## virsh net-edit default +## or other application using the libvirt API. +## +## dnsmasq conf file created by libvirt +strict-order +except-interface=lo +bind-dynamic +interface=virbr0 +dhcp-range=192.168.122.2,192.168.122.254,255.255.255.0,infinite +dhcp-no-override +dhcp-authoritative +dhcp-lease-max=253 +dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile +addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts diff --git a/tests/networkxml2confdata/leasetime-infinite.xml b/tests/networkxml2confdata/leasetime-infinite.xml new file mode 100644 index 0000000000..9ae3c4f7f7 --- /dev/null +++ b/tests/networkxml2confdata/leasetime-infinite.xml @@ -0,0 +1,12 @@ +<network xmlns:dnsmasq="http://libvirt.org/schemas/network/dnsmasq/1.0"> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <bridge name="virbr0"/> + <forward mode="nat" dev="eth1"/> + <ip address="192.168.122.1" netmask="255.255.255.0"> + <dhcp> + <range start="192.168.122.2" end="192.168.122.254"/> + <leasetime unit="infinite"/> + </dhcp> + </ip> +</network> diff --git a/tests/networkxml2confdata/leasetime-minutes.conf b/tests/networkxml2confdata/leasetime-minutes.conf new file mode 100644 index 0000000000..c093501a35 --- /dev/null +++ b/tests/networkxml2confdata/leasetime-minutes.conf @@ -0,0 +1,16 @@ +##WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE +##OVERWRITTEN AND LOST. Changes to this configuration should be made using: +## virsh net-edit default +## or other application using the libvirt API. +## +## dnsmasq conf file created by libvirt +strict-order +except-interface=lo +bind-dynamic +interface=virbr0 +dhcp-range=192.168.122.2,192.168.122.254,255.255.255.0,10m +dhcp-no-override +dhcp-authoritative +dhcp-lease-max=253 +dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile +addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts diff --git a/tests/networkxml2confdata/leasetime-minutes.xml b/tests/networkxml2confdata/leasetime-minutes.xml new file mode 100644 index 0000000000..e531db2848 --- /dev/null +++ b/tests/networkxml2confdata/leasetime-minutes.xml @@ -0,0 +1,12 @@ +<network xmlns:dnsmasq="http://libvirt.org/schemas/network/dnsmasq/1.0"> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <bridge name="virbr0"/> + <forward mode="nat" dev="eth1"/> + <ip address="192.168.122.1" netmask="255.255.255.0"> + <dhcp> + <range start="192.168.122.2" end="192.168.122.254"/> + <leasetime unit="minutes">10</leasetime> + </dhcp> + </ip> +</network> diff --git a/tests/networkxml2confdata/leasetime-seconds.conf b/tests/networkxml2confdata/leasetime-seconds.conf new file mode 100644 index 0000000000..4acc2839c5 --- /dev/null +++ b/tests/networkxml2confdata/leasetime-seconds.conf @@ -0,0 +1,16 @@ +##WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE +##OVERWRITTEN AND LOST. Changes to this configuration should be made using: +## virsh net-edit default +## or other application using the libvirt API. +## +## dnsmasq conf file created by libvirt +strict-order +except-interface=lo +bind-dynamic +interface=virbr0 +dhcp-range=192.168.122.2,192.168.122.254,255.255.255.0,1000s +dhcp-no-override +dhcp-authoritative +dhcp-lease-max=253 +dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile +addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts diff --git a/tests/networkxml2confdata/leasetime-seconds.xml b/tests/networkxml2confdata/leasetime-seconds.xml new file mode 100644 index 0000000000..4b4a57110e --- /dev/null +++ b/tests/networkxml2confdata/leasetime-seconds.xml @@ -0,0 +1,12 @@ +<network xmlns:dnsmasq="http://libvirt.org/schemas/network/dnsmasq/1.0"> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <bridge name="virbr0"/> + <forward mode="nat" dev="eth1"/> + <ip address="192.168.122.1" netmask="255.255.255.0"> + <dhcp> + <range start="192.168.122.2" end="192.168.122.254"/> + <leasetime>1000</leasetime> + </dhcp> + </ip> +</network> diff --git a/tests/networkxml2conftest.c b/tests/networkxml2conftest.c index a8355272b9..3cb6658f51 100644 --- a/tests/networkxml2conftest.c +++ b/tests/networkxml2conftest.c @@ -142,6 +142,10 @@ mymain(void) DO_TEST("dhcp6host-routed-network", dhcpv6); DO_TEST("ptr-domains-auto", dhcpv6); DO_TEST("dnsmasq-options", dhcpv6); + DO_TEST("leasetime-seconds", full); + DO_TEST("leasetime-minutes", full); + DO_TEST("leasetime-hours", full); + DO_TEST("leasetime-infinite", full); virObjectUnref(dhcpv6); virObjectUnref(full); diff --git a/tests/networkxml2xmlin/leasetime-hours.xml b/tests/networkxml2xmlin/leasetime-hours.xml new file mode 100644 index 0000000000..d8075111a4 --- /dev/null +++ b/tests/networkxml2xmlin/leasetime-hours.xml @@ -0,0 +1,12 @@ +<network> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <bridge name="virbr0"/> + <forward mode="nat" dev="eth1"/> + <ip address="192.168.122.1" netmask="255.255.255.0"> + <dhcp> + <range start="192.168.122.2" end="192.168.122.254"/> + <leasetime unit="hours">1</leasetime> + </dhcp> + </ip> +</network> diff --git a/tests/networkxml2xmlin/leasetime-infinite.xml b/tests/networkxml2xmlin/leasetime-infinite.xml new file mode 100644 index 0000000000..d0357620b0 --- /dev/null +++ b/tests/networkxml2xmlin/leasetime-infinite.xml @@ -0,0 +1,12 @@ +<network> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <bridge name="virbr0"/> + <forward mode="nat" dev="eth1"/> + <ip address="192.168.122.1" netmask="255.255.255.0"> + <dhcp> + <range start="192.168.122.2" end="192.168.122.254"/> + <leasetime unit="infinite"/> + </dhcp> + </ip> +</network> diff --git a/tests/networkxml2xmlin/leasetime-minutes.xml b/tests/networkxml2xmlin/leasetime-minutes.xml new file mode 100644 index 0000000000..21169766a4 --- /dev/null +++ b/tests/networkxml2xmlin/leasetime-minutes.xml @@ -0,0 +1,12 @@ +<network> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <bridge name="virbr0"/> + <forward mode="nat" dev="eth1"/> + <ip address="192.168.122.1" netmask="255.255.255.0"> + <dhcp> + <range start="192.168.122.2" end="192.168.122.254"/> + <leasetime unit="minutes">10</leasetime> + </dhcp> + </ip> +</network> diff --git a/tests/networkxml2xmlin/leasetime-seconds.xml b/tests/networkxml2xmlin/leasetime-seconds.xml new file mode 100644 index 0000000000..4cf64b851e --- /dev/null +++ b/tests/networkxml2xmlin/leasetime-seconds.xml @@ -0,0 +1,12 @@ +<network> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <bridge name="virbr0"/> + <forward mode="nat" dev="eth1"/> + <ip address="192.168.122.1" netmask="255.255.255.0"> + <dhcp> + <range start="192.168.122.2" end="192.168.122.254"/> + <leasetime unit="seconds">200</leasetime> + </dhcp> + </ip> +</network> diff --git a/tests/networkxml2xmlout/leasetime-hours.xml b/tests/networkxml2xmlout/leasetime-hours.xml new file mode 100644 index 0000000000..ed85f77569 --- /dev/null +++ b/tests/networkxml2xmlout/leasetime-hours.xml @@ -0,0 +1,14 @@ +<network> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <forward dev='eth1' mode='nat'> + <interface dev='eth1'/> + </forward> + <bridge name='virbr0' stp='on' delay='0'/> + <ip address='192.168.122.1' netmask='255.255.255.0'> + <dhcp> + <range start='192.168.122.2' end='192.168.122.254'/> + <leasetime unit='hours'>1</leasetime> + </dhcp> + </ip> +</network> diff --git a/tests/networkxml2xmlout/leasetime-infinite.xml b/tests/networkxml2xmlout/leasetime-infinite.xml new file mode 100644 index 0000000000..4373357588 --- /dev/null +++ b/tests/networkxml2xmlout/leasetime-infinite.xml @@ -0,0 +1,14 @@ +<network> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <forward dev='eth1' mode='nat'> + <interface dev='eth1'/> + </forward> + <bridge name='virbr0' stp='on' delay='0'/> + <ip address='192.168.122.1' netmask='255.255.255.0'> + <dhcp> + <range start='192.168.122.2' end='192.168.122.254'/> + <leasetime unit='infinite'/> + </dhcp> + </ip> +</network> diff --git a/tests/networkxml2xmlout/leasetime-minutes.xml b/tests/networkxml2xmlout/leasetime-minutes.xml new file mode 100644 index 0000000000..9e3d8591fa --- /dev/null +++ b/tests/networkxml2xmlout/leasetime-minutes.xml @@ -0,0 +1,14 @@ +<network> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <forward dev='eth1' mode='nat'> + <interface dev='eth1'/> + </forward> + <bridge name='virbr0' stp='on' delay='0'/> + <ip address='192.168.122.1' netmask='255.255.255.0'> + <dhcp> + <range start='192.168.122.2' end='192.168.122.254'/> + <leasetime unit='minutes'>10</leasetime> + </dhcp> + </ip> +</network> diff --git a/tests/networkxml2xmlout/leasetime-seconds.xml b/tests/networkxml2xmlout/leasetime-seconds.xml new file mode 100644 index 0000000000..a4e428f84b --- /dev/null +++ b/tests/networkxml2xmlout/leasetime-seconds.xml @@ -0,0 +1,14 @@ +<network> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <forward dev='eth1' mode='nat'> + <interface dev='eth1'/> + </forward> + <bridge name='virbr0' stp='on' delay='0'/> + <ip address='192.168.122.1' netmask='255.255.255.0'> + <dhcp> + <range start='192.168.122.2' end='192.168.122.254'/> + <leasetime unit='seconds'>200</leasetime> + </dhcp> + </ip> +</network> diff --git a/tests/networkxml2xmltest.c b/tests/networkxml2xmltest.c index ec679e72ee..700744785a 100644 --- a/tests/networkxml2xmltest.c +++ b/tests/networkxml2xmltest.c @@ -160,6 +160,10 @@ mymain(void) DO_TEST("metadata"); DO_TEST("set-mtu"); DO_TEST("dnsmasq-options"); + DO_TEST("leasetime-seconds"); + DO_TEST("leasetime-minutes"); + DO_TEST("leasetime-hours"); + DO_TEST("leasetime-infinite"); DO_TEST("isolated-ports"); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; -- 2.24.1
I resubmitted this series because our team needs to hack dnsmasq settings to change lease time. This feature would be so important to us to avoid workarounds. It is based on Alberto's patch from 2017. But personally I don't like this approach. IMHO, it would be nice to have specific attributes to configure lease time. For example: <range ... leasetime="10m"/> <host ... leasetime="20m"/> They can be different from each other. I still think that the idea should be better developed. I don't like that my example also (it is just an example). That's why I submitted... To listen opinions from others. -- Julio Cesar Faracco Em qua., 15 de abr. de 2020 às 13:19, Julio Faracco <jcfaracco@gmail.com> escreveu:
This series is based on latest series from Alberto. It includes a new entry called <leasetime/> under <dhcp/> scope to add a default lease time for range and host options for dnsmasq. There is no point to configure both separately. If they are defined (range and/or host), they should have the same lease time value.
This series includes some test cases to cover lease time XML syntax also.
Julio Faracco (2): conf: Add <leasetime/> option for <dhcp/> settings tests: Add tests for <leasetime/> to cover dnsmasq settings
docs/schemas/basictypes.rng | 9 +++ docs/schemas/network.rng | 11 ++++ src/conf/network_conf.c | 62 ++++++++++++++++++- src/conf/network_conf.h | 14 +++++ src/libvirt_private.syms | 2 + src/network/bridge_driver.c | 37 ++++++++++- src/util/virdnsmasq.c | 40 ++++++------ src/util/virdnsmasq.h | 1 + .../networkxml2confdata/leasetime-hours.conf | 16 +++++ tests/networkxml2confdata/leasetime-hours.xml | 12 ++++ .../leasetime-infinite.conf | 16 +++++ .../leasetime-infinite.xml | 12 ++++ .../leasetime-minutes.conf | 16 +++++ .../networkxml2confdata/leasetime-minutes.xml | 12 ++++ .../leasetime-seconds.conf | 16 +++++ .../networkxml2confdata/leasetime-seconds.xml | 12 ++++ tests/networkxml2conftest.c | 4 ++ tests/networkxml2xmlin/leasetime-hours.xml | 12 ++++ tests/networkxml2xmlin/leasetime-infinite.xml | 12 ++++ tests/networkxml2xmlin/leasetime-minutes.xml | 12 ++++ tests/networkxml2xmlin/leasetime-seconds.xml | 12 ++++ tests/networkxml2xmlout/leasetime-hours.xml | 14 +++++ .../networkxml2xmlout/leasetime-infinite.xml | 14 +++++ tests/networkxml2xmlout/leasetime-minutes.xml | 14 +++++ tests/networkxml2xmlout/leasetime-seconds.xml | 14 +++++ tests/networkxml2xmltest.c | 4 ++ 26 files changed, 376 insertions(+), 24 deletions(-) create mode 100644 tests/networkxml2confdata/leasetime-hours.conf create mode 100644 tests/networkxml2confdata/leasetime-hours.xml create mode 100644 tests/networkxml2confdata/leasetime-infinite.conf create mode 100644 tests/networkxml2confdata/leasetime-infinite.xml create mode 100644 tests/networkxml2confdata/leasetime-minutes.conf create mode 100644 tests/networkxml2confdata/leasetime-minutes.xml create mode 100644 tests/networkxml2confdata/leasetime-seconds.conf create mode 100644 tests/networkxml2confdata/leasetime-seconds.xml create mode 100644 tests/networkxml2xmlin/leasetime-hours.xml create mode 100644 tests/networkxml2xmlin/leasetime-infinite.xml create mode 100644 tests/networkxml2xmlin/leasetime-minutes.xml create mode 100644 tests/networkxml2xmlin/leasetime-seconds.xml create mode 100644 tests/networkxml2xmlout/leasetime-hours.xml create mode 100644 tests/networkxml2xmlout/leasetime-infinite.xml create mode 100644 tests/networkxml2xmlout/leasetime-minutes.xml create mode 100644 tests/networkxml2xmlout/leasetime-seconds.xml
-- 2.24.1
On Wed, Apr 15, 2020 at 01:25:37PM -0300, Julio Faracco wrote:
I resubmitted this series because our team needs to hack dnsmasq settings to change lease time. This feature would be so important to us to avoid workarounds.
It is based on Alberto's patch from 2017. But personally I don't like this approach. IMHO, it would be nice to have specific attributes to configure lease time. For example: <range ... leasetime="10m"/> <host ... leasetime="20m"/>
It is generally considered bad practice to have an XML attribute value which then has to be parsed again to extract information. For this reason, libvirt will use two attrbutes, one of the value and one for the units (with some sensible default units if not specified), even though this is admittedly more verbose. I agree it would be useful to have lease time per-host, as well as globally though, and IIRC one of the original versions of this patch did support that. We could do one of <ip address="192.168.122.1" netmask="255.255.255.0"> <dhcp> <range start="192.168.122.2" end="192.168.122.254" lease="12" units="hours"/> <host id="0:1:0:1:18:aa:62:fe:0:16:3e:44:55:66" ip="2001:db8:ca2:2:3::2" lease="30" units="mins"/> </dhcp> </ip> or <ip address="192.168.122.1" netmask="255.255.255.0"> <dhcp> <range start="192.168.122.2" end="192.168.122.254" leasetime="12" leaseunits="hours"/> <host id="0:1:0:1:18:aa:62:fe:0:16:3e:44:55:66" ip="2001:db8:ca2:2:3::2" leasetime="30" leaseunits="mins"/> </dhcp> </ip> or <ip address="192.168.122.1" netmask="255.255.255.0"> <dhcp> <range start="192.168.122.2" end="192.168.122.254"> <lease expiry="12" units="hours"/> </range> <host id="0:1:0:1:18:aa:62:fe:0:16:3e:44:55:66" ip="2001:db8:ca2:2:3::2"> <lease expiry="30" units="mins"/> </host> </dhcp> </ip> In all of these we can default to "minutes" if no units are given. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 4/17/20 12:00 PM, Daniel P. Berrangé wrote:
On Wed, Apr 15, 2020 at 01:25:37PM -0300, Julio Faracco wrote:
I resubmitted this series because our team needs to hack dnsmasq settings to change lease time. This feature would be so important to us to avoid workarounds.
It is based on Alberto's patch from 2017. But personally I don't like this approach. IMHO, it would be nice to have specific attributes to configure lease time. For example: <range ... leasetime="10m"/> <host ... leasetime="20m"/> It is generally considered bad practice to have an XML attribute value which then has to be parsed again to extract information.
For this reason, libvirt will use two attrbutes, one of the value and one for the units (with some sensible default units if not specified), even though this is admittedly more verbose.
I agree it would be useful to have lease time per-host, as well as globally though, and IIRC one of the original versions of this patch did support that.
The name of the original contributor that you (Julio) referenced in your cover letter sounded familiar, and I tried to find the original BZ for this when I saw your patches on the list, but I failed (bugzilla kept timing out on a *very* basic search term - this seems to happen to 80% of my queries these days...) But then it passed by in mail when Dan was cleaning up all the upstream libvirt BZes and moving them to gitlab :-), so just so everyone has all the background info: https://bugzilla.redhat.com/show_bug.cgi?id=913446 I also found the other similar patch from Nehal J Wani from around the same time: https://www.redhat.com/archives/libvir-list/2016-September/msg01063.html Without talking about the specifics of either patch, my recollection of the discussion from that time was that both contributors were trying to use a leasetime setting to solve a problem that it wouldn't have solved - their issue was that if a guest was turned off during the time when its lease expired, then when the guest was subsequently restarted it would end up with a different IP address. Of course setting a longer lease expiry would only delay the problem, not eliminate it. Further discussion revealed that if libvirt would just set the "dhcp-authoritative" option in the dnsmasq config, then dnsmasq would attempt to reissue the same IP to a guest even if its lease had already expired - Martin Wilck made this change to libvirt in commit 4ac20b3ae4, which seemed to satisfy the people who had thought they needed to modify the dhcp lease time, and so neither of the lease time patches was pushed upstream. The reason I bring up this old history is just as a cautionary tale that sometimes what you think you need isn't really what you actually need :-) (Recently Cole added the dnsmasq private namespace to the <network> XML, but that is only useful to add an entire option line, and can't do what you need, which is adding an extra option to every dhcp-host and dhcp-range line; there unfortunately is no standalone dnsmasq option to specify a global lease time)
We could do one of
<ip address="192.168.122.1" netmask="255.255.255.0"> <dhcp> <range start="192.168.122.2" end="192.168.122.254" lease="12" units="hours"/> <host id="0:1:0:1:18:aa:62:fe:0:16:3e:44:55:66" ip="2001:db8:ca2:2:3::2" lease="30" units="mins"/> </dhcp> </ip>
or
<ip address="192.168.122.1" netmask="255.255.255.0"> <dhcp> <range start="192.168.122.2" end="192.168.122.254" leasetime="12" leaseunits="hours"/> <host id="0:1:0:1:18:aa:62:fe:0:16:3e:44:55:66" ip="2001:db8:ca2:2:3::2" leasetime="30" leaseunits="mins"/> </dhcp> </ip>
or
<ip address="192.168.122.1" netmask="255.255.255.0"> <dhcp> <range start="192.168.122.2" end="192.168.122.254"> <lease expiry="12" units="hours"/> </range> <host id="0:1:0:1:18:aa:62:fe:0:16:3e:44:55:66" ip="2001:db8:ca2:2:3::2"> <lease expiry="30" units="mins"/> </host> </dhcp> </ip>
Nehal's patch had used this syntax: <leasetime units='hours'>12</leasetime> based on (I guess) the syntax of libvirt's <memory> element: <memory unit='KiB'>524288</memory> I don't have a preference for any of them, just thought I would point out existing usage in libvirt that has a value/units combination.
In all of these we can default to "minutes" if no units are given.
Regards, Daniel
On Sun, Apr 19, 2020 at 12:16:39AM -0400, Laine Stump wrote:
On 4/17/20 12:00 PM, Daniel P. Berrangé wrote:
On Wed, Apr 15, 2020 at 01:25:37PM -0300, Julio Faracco wrote:
I resubmitted this series because our team needs to hack dnsmasq settings to change lease time. This feature would be so important to us to avoid workarounds.
It is based on Alberto's patch from 2017. But personally I don't like this approach. IMHO, it would be nice to have specific attributes to configure lease time. For example: <range ... leasetime="10m"/> <host ... leasetime="20m"/> It is generally considered bad practice to have an XML attribute value which then has to be parsed again to extract information.
For this reason, libvirt will use two attrbutes, one of the value and one for the units (with some sensible default units if not specified), even though this is admittedly more verbose.
I agree it would be useful to have lease time per-host, as well as globally though, and IIRC one of the original versions of this patch did support that.
The name of the original contributor that you (Julio) referenced in your cover letter sounded familiar, and I tried to find the original BZ for this when I saw your patches on the list, but I failed (bugzilla kept timing out on a *very* basic search term - this seems to happen to 80% of my queries these days...) But then it passed by in mail when Dan was cleaning up all the upstream libvirt BZes and moving them to gitlab :-), so just so everyone has all the background info:
https://bugzilla.redhat.com/show_bug.cgi?id=913446
I also found the other similar patch from Nehal J Wani from around the same time:
https://www.redhat.com/archives/libvir-list/2016-September/msg01063.html
Without talking about the specifics of either patch, my recollection of the discussion from that time was that both contributors were trying to use a leasetime setting to solve a problem that it wouldn't have solved - their issue was that if a guest was turned off during the time when its lease expired, then when the guest was subsequently restarted it would end up with a different IP address. Of course setting a longer lease expiry would only delay the problem, not eliminate it. Further discussion revealed that if libvirt would just set the "dhcp-authoritative" option in the dnsmasq config, then dnsmasq would attempt to reissue the same IP to a guest even if its lease had already expired - Martin Wilck made this change to libvirt in commit 4ac20b3ae4, which seemed to satisfy the people who had thought they needed to modify the dhcp lease time, and so neither of the lease time patches was pushed upstream.
The reason I bring up this old history is just as a cautionary tale that sometimes what you think you need isn't really what you actually need :-)
(Recently Cole added the dnsmasq private namespace to the <network> XML, but that is only useful to add an entire option line, and can't do what you need, which is adding an extra option to every dhcp-host and dhcp-range line; there unfortunately is no standalone dnsmasq option to specify a global lease time)
We could do one of
<ip address="192.168.122.1" netmask="255.255.255.0"> <dhcp> <range start="192.168.122.2" end="192.168.122.254" lease="12" units="hours"/> <host id="0:1:0:1:18:aa:62:fe:0:16:3e:44:55:66" ip="2001:db8:ca2:2:3::2" lease="30" units="mins"/> </dhcp> </ip>
or
<ip address="192.168.122.1" netmask="255.255.255.0"> <dhcp> <range start="192.168.122.2" end="192.168.122.254" leasetime="12" leaseunits="hours"/> <host id="0:1:0:1:18:aa:62:fe:0:16:3e:44:55:66" ip="2001:db8:ca2:2:3::2" leasetime="30" leaseunits="mins"/> </dhcp> </ip>
or
<ip address="192.168.122.1" netmask="255.255.255.0"> <dhcp> <range start="192.168.122.2" end="192.168.122.254"> <lease expiry="12" units="hours"/> </range> <host id="0:1:0:1:18:aa:62:fe:0:16:3e:44:55:66" ip="2001:db8:ca2:2:3::2"> <lease expiry="30" units="mins"/> </host> </dhcp> </ip>
Nehal's patch had used this syntax:
<leasetime units='hours'>12</leasetime>
based on (I guess) the syntax of libvirt's <memory> element:
<memory unit='KiB'>524288</memory>
I don't have a preference for any of them, just thought I would point out existing usage in libvirt that has a value/units combination.
Annoyingly it seems we used "units" and "unit", though "unit" wins by a large margin over "units" I guess I have a slight preference for the last option, with the use of the child-element, as it is the cleanest XML design, even if it is slightly more verbose. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (3)
-
Daniel P. Berrangé -
Julio Faracco -
Laine Stump