Re: [libvirt] [PATCH] Don't drop expired lease while reading custom leases file

On Fri, Sep 30, 2016 at 5:47 PM, Michal Privoznik <mprivozn(a)redhat.com&gt; wrote: > On 28.09.2016 21:39, Nehal J Wani wrote: >> 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)) { >> > > Well, I like this patch. I really do. I always felt like read() function > should not do any validation and just present called the data. However, > this "breaks" the nsstest. We have two options here: either merge the > other patch of yours that fixes the NSS module, or temporarily just > "fix" the nss test (so that we can undo the change while merging the > other patch). > Which one of those two approaches do you like more? > > Michal I like the approach where the NSS module is fixed first, and then virLeaseReadCustomLeaseFile() is fixed. This way, we won't break anything in between and won't have to change anything temporarily.