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

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