On Fri, Sep 30, 2016 at 5:47 PM, Michal Privoznik <mprivozn@xxxxxxxxxx> 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.
--
Nehal J Wani
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list