Libvirt, on its own, shouldn't decide whether an expired lease should stay in the custom leases database or not. It should rather rely on the 'DEL' event from dnsmasq. --- src/util/virlease.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/util/virlease.c b/src/util/virlease.c index 920ebaf..b49105d 100644 --- a/src/util/virlease.c +++ b/src/util/virlease.c @@ -57,7 +57,6 @@ virLeaseReadCustomLeaseFile(virJSONValuePtr leases_array_new, { char *lease_entries = NULL; virJSONValuePtr leases_array = NULL; - long long currtime = 0; long long expirytime; int custom_lease_file_len = 0; virJSONValuePtr lease_tmp = NULL; @@ -66,8 +65,6 @@ virLeaseReadCustomLeaseFile(virJSONValuePtr leases_array_new, size_t i; int ret = -1; - currtime = (long long) time(NULL); - /* Read entire contents */ if ((custom_lease_file_len = virFileReadAll(custom_lease_file, VIR_NETWORK_DHCP_LEASE_FILE_SIZE_MAX, @@ -109,11 +106,6 @@ virLeaseReadCustomLeaseFile(virJSONValuePtr leases_array_new, _("failed to parse json")); goto cleanup; } - /* Check whether lease has expired or not */ - if (expirytime < currtime) { - i++; - continue; - } /* Check whether lease has to be included or not */ if (ip_to_delete && STREQ(ip_tmp, ip_to_delete)) {

The NSS module shouldn't rely on custom leases database to not have entries for leases which have expired. Change-Id: Ic3e043003d33ded0da74696a1d27ed4967ddbfb8

--- tools/nss/libvirt_nss.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tools/nss/libvirt_nss.c b/tools/nss/libvirt_nss.c index 54c4a2a..cb3edf5 100644 --- a/tools/nss/libvirt_nss.c +++ b/tools/nss/libvirt_nss.c @@ -42,6 +42,7 @@ #include "virlease.h" #include "viralloc.h" #include "virfile.h" +#include "virtime.h" #include "virerror.h" #include "virstring.h" #include "virsocketaddr.h" @@ -114,6 +115,8 @@ findLease(const char *name, ssize_t i, nleases; leaseAddress *tmpAddress = NULL; size_t ntmpAddress = 0; + time_t currtime; + long long expirytime; *address = NULL; *naddress = 0; @@ -161,6 +164,11 @@ findLease(const char *name, nleases = virJSONValueArraySize(leases_array); DEBUG("Read %zd leases", nleases); + if ((currtime = time(NULL)) == (time_t) - 1) { + ERROR("Failed to get current system time"); + goto cleanup; + } + for (i = 0; i < nleases; i++) { virJSONValuePtr lease; const char *lease_name; @@ -181,6 +189,16 @@ findLease(const char *name, if (STRNEQ_NULLABLE(name, lease_name)) continue; + if (virJSONValueObjectGetNumberLong(lease, "expiry-time", &expirytime) < 0) { + /* A lease cannot be present without expiry-time */ + ERROR("expiry-time field missing for %s", name); + goto cleanup; + } + + /* Do not report expired lease */ + if (expirytime < (long long) currtime)

+ continue; + DEBUG("Found record for %s", lease_name); *found = true;

On Mon, Oct 3, 2016 at 1:19 PM, Michal Privoznik <mprivozn(a)redhat.com&gt; wrote: > On 30.09.2016 17:11, Nehal J Wani wrote: >> The NSS module shouldn't rely on custom leases database to not have >> entries for leases which have expired. >> >> Change-Id: Ic3e043003d33ded0da74696a1d27ed4967ddbfb8 > > Oh, is this a result of some git hook script? Yes, indeed. I forgot to remove it. My bad. #GerritChronicles > >> --- >> tools/nss/libvirt_nss.c | 18 ++++++++++++++++++ >> 1 file changed, 18 insertions(+) >> >> diff --git a/tools/nss/libvirt_nss.c b/tools/nss/libvirt_nss.c >> index 54c4a2a..cb3edf5 100644 >> --- a/tools/nss/libvirt_nss.c >> +++ b/tools/nss/libvirt_nss.c >> @@ -42,6 +42,7 @@ >> #include "virlease.h" >> #include "viralloc.h" >> #include "virfile.h" >> +#include "virtime.h" >> #include "virerror.h" >> #include "virstring.h" >> #include "virsocketaddr.h" >> @@ -114,6 +115,8 @@ findLease(const char *name, >> ssize_t i, nleases; >> leaseAddress *tmpAddress = NULL; >> size_t ntmpAddress = 0; >> + time_t currtime; >> + long long expirytime; >> >> *address = NULL; >> *naddress = 0; >> @@ -161,6 +164,11 @@ findLease(const char *name, >> nleases = virJSONValueArraySize(leases_array); >> DEBUG("Read %zd leases", nleases); >> >> + if ((currtime = time(NULL)) == (time_t) - 1) { >> + ERROR("Failed to get current system time"); >> + goto cleanup; >> + } >> + >> for (i = 0; i < nleases; i++) { >> virJSONValuePtr lease; >> const char *lease_name; >> @@ -181,6 +189,16 @@ findLease(const char *name, >> if (STRNEQ_NULLABLE(name, lease_name)) >> continue; >> >> + if (virJSONValueObjectGetNumberLong(lease, "expiry-time", &expirytime) < 0) { >> + /* A lease cannot be present without expiry-time */ >> + ERROR("expiry-time field missing for %s", name); >> + goto cleanup; >> + } >> + >> + /* Do not report expired lease */ >> + if (expirytime < (long long) currtime) > > I am putting here a debug message (which is no-op unless enabled) as it > might help me in debugging in the future. Maybe we should at that debug message at other places too, where we do this, for example, in networkGetDHCPLeases()