On 4/22/20 4: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> ---
[...]
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); + + /* 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;
Coverity pointed out there's a @new_lease leak here. In just a quick scan of the code it's not the only place and I wonder about the initialization of new_lease = *lease and then just VIR_ALLOC right over that... Probably should be initialized to NULL. Anyway - perhaps consider changing @expiry to @expirystr and @unit to @unitstr, then filling/using @expiry & @unit (instead of unitInt) for comparisons before the VIR_ALLOC(new_lease) and filling in the two fields once all the checks are done - probably allows those boundary checks to not span 4 lines... JMO... John
+ } + } + + *lease = new_lease; + + return 0; +} + +
[...]