On Tue, Jun 20, 2017 at 19:00:43 +0100, aruiz@gnome.org wrote:
From: Alberto Ruiz <aruiz@gnome.org>
Missing commit message.
--- docs/schemas/basictypes.rng | 16 +++++ docs/schemas/network.rng | 8 +++ src/conf/network_conf.c | 78 ++++++++++++++++++++++- src/conf/network_conf.h | 3 +- src/network/bridge_driver.c | 49 +++++++++++++- tests/networkxml2confdata/leasetime-days.conf | 17 +++++ tests/networkxml2confdata/leasetime-days.xml | 18 ++++++ tests/networkxml2confdata/leasetime-hours.conf | 17 +++++ tests/networkxml2confdata/leasetime-hours.xml | 18 ++++++ tests/networkxml2confdata/leasetime-infinite.conf | 17 +++++ tests/networkxml2confdata/leasetime-infinite.xml | 18 ++++++ tests/networkxml2confdata/leasetime-minutes.conf | 17 +++++ tests/networkxml2confdata/leasetime-minutes.xml | 18 ++++++ tests/networkxml2confdata/leasetime-seconds.conf | 17 +++++ tests/networkxml2confdata/leasetime-seconds.xml | 18 ++++++ tests/networkxml2confdata/leasetime.conf | 17 +++++ tests/networkxml2confdata/leasetime.xml | 18 ++++++ tests/networkxml2conftest.c | 7 ++ 18 files changed, 368 insertions(+), 3 deletions(-) create mode 100644 tests/networkxml2confdata/leasetime-days.conf create mode 100644 tests/networkxml2confdata/leasetime-days.xml 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/networkxml2confdata/leasetime.conf create mode 100644 tests/networkxml2confdata/leasetime.xml
diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 1b4f980e7..8a76c235a 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -518,4 +518,20 @@ </element> </define>
+ <define name="leaseTimeUnit"> + <choice> + <value>seconds</value> + <value>minutes</value> + <value>hours</value> + <value>days</value> + </choice> + </define> + + <define name="leaseTime"> + <data type="long"> + <param name="minInclusive">-1</param> + <param name="maxInclusive">4294967295</param> + </data> + </define> + </grammar> diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index 1a18e64b2..4b8056ab6 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -340,6 +340,14 @@ <!-- Define the range(s) of IP addresses that the DHCP server should hand out --> <element name="dhcp"> + <optional> + <element name="leasetime"> + <optional> + <attribute name="unit"><ref name="leaseTimeUnit"/></attribute>
This does not follow the XML style used everywhere else.
+ </optional> + <ref name="leaseTime"/> + </element> + </optional> <zeroOrMore> <element name="range"> <attribute name="start"><ref name="ipAddr"/></attribute> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index aa397768c..6f051493f 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -30,6 +30,8 @@ #include <fcntl.h> #include <string.h> #include <dirent.h> +#include <stdlib.h> +#include <inttypes.h>
#include "virerror.h" #include "datatypes.h" @@ -1031,12 +1033,82 @@ virNetworkDHCPHostDefParseXML(const char *networkName, return ret; }
+static int64_t +virNetworkDHCPDefGetLeaseTime (xmlNodePtr node, + xmlXPathContextPtr ctxt, + bool *error)
We usually return error from the function and if necessary return the value through pointer in arguments (backwards as you did).
+{ + int64_t multiplier, result = -1; + char *leaseString, *leaseUnit; + xmlNodePtr save; + + *error = 0; + + save = ctxt->node; + ctxt->node = node; + + leaseString = virXPathString ("string(./leasetime/text())", ctxt); + leaseUnit = virXPathString ("string(./leasetime/@unit)", ctxt); + + /* If value is not present we set the value to -2 */ + if (leaseString == NULL) { + result = -2; + goto cleanup; + } + + if (leaseUnit == NULL || strcmp (leaseUnit, "seconds") == 0) + multiplier = 1; + else if (strcmp (leaseUnit, "minutes") == 0) + multiplier = 60; + else if (strcmp (leaseUnit, "hours") == 0) + multiplier = 60 * 60; + else if (strcmp (leaseUnit, "days") == 0) + multiplier = 60 * 60 * 24; + else { + virReportError(VIR_ERR_XML_ERROR, + _("invalid value for unit parameter in <leasetime> element found in <dhcp> network, " + "only 'seconds', 'minutes', 'hours' or 'days' are valid: %s"), + leaseUnit); + *error = 1; + goto cleanup; + }
Does not follow libvirt coding style.
+ + errno = 0; + result = (int64_t) strtoll((const char*)leaseString, NULL, 10); + + /* Report any errors parsing the string */ + if (errno != 0) { + virReportError(VIR_ERR_XML_ERROR, + _("<leasetime> value could not be converted to a signed integer: %s"), + leaseString); + *error = 1; + goto cleanup; + } + + result = result * multiplier; + + if (result > UINT32_MAX) { + virReportError(VIR_ERR_XML_ERROR, + _("<leasetime> value cannot be greater than the equivalent of %" PRIo32 " seconds : %" PRId64), + UINT32_MAX, result);
Lines are too long
+ *error = 1; + } + +cleanup: + VIR_FREE(leaseString); + VIR_FREE(leaseUnit); + ctxt->node = save; + return result; +} + static int virNetworkDHCPDefParseXML(const char *networkName, xmlNodePtr node, + xmlXPathContextPtr ctxt, virNetworkIPDefPtr def) { int ret = -1; + bool error; xmlNodePtr cur; virSocketAddrRange range; virNetworkDHCPHostDef host; @@ -1044,6 +1116,10 @@ virNetworkDHCPDefParseXML(const char *networkName, memset(&range, 0, sizeof(range)); memset(&host, 0, sizeof(host));
+ def->leasetime = virNetworkDHCPDefGetLeaseTime (node, ctxt, &error);
This won't pass syntax-check
+ if (error) + goto cleanup; + cur = node->children; while (cur != NULL) { if (cur->type == XML_ELEMENT_NODE && @@ -1607,7 +1683,7 @@ virNetworkIPDefParseXML(const char *networkName, while (cur != NULL) { if (cur->type == XML_ELEMENT_NODE && xmlStrEqual(cur->name, BAD_CAST "dhcp")) { - if (virNetworkDHCPDefParseXML(networkName, cur, def) < 0) + if (virNetworkDHCPDefParseXML(networkName, cur, ctxt, def) < 0) goto cleanup; } else if (cur->type == XML_ELEMENT_NODE && xmlStrEqual(cur->name, BAD_CAST "tftp")) { diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 3b227db6f..f7557f581 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -170,7 +170,8 @@ struct _virNetworkIPDef { char *tftproot; char *bootfile; virSocketAddr bootserver; - }; + int64_t leasetime; +};
typedef struct _virNetworkForwardIfDef virNetworkForwardIfDef; typedef virNetworkForwardIfDef *virNetworkForwardIfDefPtr; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 7b99acafa..e51e469c8 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -41,6 +41,8 @@ #include <sys/ioctl.h> #include <net/if.h> #include <dirent.h> +#include <inttypes.h> +#include <stdint.h> #if HAVE_SYS_SYSCTL_H # include <sys/sysctl.h> #endif @@ -903,6 +905,40 @@ networkBuildDnsmasqHostsList(dnsmasqContext *dctx, return 0; }
+/* translates the leasetime value into a dnsmasq configuration string for dhcp-range/host */ +static char * +networkDnsmasqConfLeaseValueToString (int64_t leasetime) +{ + char *result = NULL; + virBuffer leasebuf = VIR_BUFFER_INITIALIZER; + + /* Leasetime parameter set on the XML */ + /* Less than -1 means there was no value set */ + if (leasetime < -1) { + virBufferAsprintf(&leasebuf, "%s", ""); + } + /* -1 means no expiration */ + else if (leasetime == -1) + virBufferAsprintf(&leasebuf, ",infinite"); + /* Minimum expiry value is 120 */ + /* TODO: Discuss if we should up as we do here or just forward whatever value to dnsmasq */ + else if (leasetime < 120) + virBufferAsprintf(&leasebuf, ",120"); + /* DHCP value for lease time is a unsigned four octect integer */ + else if (leasetime <= UINT32_MAX) + virBufferAsprintf(&leasebuf, ",%" PRId64, leasetime); + /* TODO: Discuss the use of "deprecated" for ipv6*/ + /* TODO: Discuss what is the default value that we want as dnsmasq's is 1 hour */ + /* TODO: Discuss what to do if value exceeds maximum, use default value for now */ + else { + virBufferAsprintf(&leasebuf, "%s", ""); + } + + result = virBufferContentAndReset(&leasebuf); + virBufferFreeAndReset (&leasebuf); + + return result; +}
int networkDnsmasqConfContents(virNetworkObjPtr network, @@ -1213,6 +1249,7 @@ networkDnsmasqConfContents(virNetworkObjPtr network, } for (r = 0; r < ipdef->nranges; r++) { int thisRange; + char *leasestr;
if (!(saddr = virSocketAddrFormat(&ipdef->ranges[r].start)) || !(eaddr = virSocketAddrFormat(&ipdef->ranges[r].end))) @@ -1220,12 +1257,22 @@ networkDnsmasqConfContents(virNetworkObjPtr network,
virBufferAsprintf(&configbuf, "dhcp-range=%s,%s", saddr, eaddr); - if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET6)) + + /* Add ipv6 prefix length parameter if needed */ + if (ipdef == ipv6def) virBufferAsprintf(&configbuf, ",%d", prefix); + + leasestr = networkDnsmasqConfLeaseValueToString (ipdef->leasetime); + if (!leasestr) + goto cleanup; + virBufferAsprintf(&configbuf, "%s", leasestr); + + /* Add the newline */ virBufferAddLit(&configbuf, "\n");
VIR_FREE(saddr); VIR_FREE(eaddr); + VIR_FREE(leasestr); thisRange = virSocketAddrGetRange(&ipdef->ranges[r].start, &ipdef->ranges[r].end, &ipdef->address,
Also this patch does not apply to current master. Peter