From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michael Tremer <michael.tremer@ipfire.org>
To: development@lists.ipfire.org
Subject: Re: [PATCH 2/3] unbound-dhcp-leases-bridge: Only reload if leases
 have actually changed
Date: Sat, 27 Apr 2024 10:38:01 +0200
Message-ID: <8519A600-B316-4587-876A-321C7BAF983B@ipfire.org>
In-Reply-To: <21bef3c1-22c3-45fa-beed-e36cd133d2df@howitts.co.uk>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============2529036883256318632=="
List-Id: <development.lists.ipfire.org>

--===============2529036883256318632==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Hello,

> On 26 Apr 2024, at 17:54, Nick Howitt <nick(a)howitts.co.uk> wrote:
>=20
> Can I just ask here as I don't know python file i/o at all.

There is pretty much no difference to the regular C API, except that Python h=
as a little bit more aggressive buffering.

> The reason I moved filecmp outside the "with tempfile....." was because the=
 file was being held open without an EOF so the filecmp always failed, making=
 it useless. When you do the f.flush(), foes the EOF get added to the temp fi=
le, or does the same problem that I had exist here?

Normally, if you call f.write(=E2=80=A6) the data might not always be written=
 to disk immediately. There is a small buffer to avoid tiny write operations.=
 flush() manually writes everything to disk. That way, we can keep the file h=
andle open (so that we can later delete the file) and it can be opened again =
to call compare.

Alternatively, there could have been a compare function that works on the alr=
eady opened file descriptor.

> On 26/04/2024 16:09, Michael Tremer wrote:
>> This patches changes that leases will always be written in
>> alphanumerical order so that we can later compare the newly generated
>> file with the previous version. If it has not changed, we skip reload
>> Unbound.
>>=20
>> Suggested-by: Nick Howitt <nick(a)howitts.co.uk>
>> Signed-off-by: Michael Tremer <michael.tremer(a)ipfire.org>
>> ---
>> config/unbound/unbound-dhcp-leases-bridge | 20 ++++++++++++++------
>> 1 file changed, 14 insertions(+), 6 deletions(-)
>>=20
>> diff --git a/config/unbound/unbound-dhcp-leases-bridge b/config/unbound/un=
bound-dhcp-leases-bridge
>> index 5d5696af0..80c8267e8 100644
>> --- a/config/unbound/unbound-dhcp-leases-bridge
>> +++ b/config/unbound/unbound-dhcp-leases-bridge
>> @@ -22,6 +22,7 @@
>> import argparse
>> import datetime
>> import daemon
>> +import filecmp
>> import functools
>> import ipaddress
>> import logging
>> @@ -516,24 +517,29 @@ class UnboundConfigWriter(object):
>>=20
>> def update_dhcp_leases(self, leases):
>> # Write out all leases
>> - self.write_dhcp_leases(leases)
>> + if self.write_dhcp_leases(leases):
>> + log.debug("Reloading Unbound...")
>>=20
>> - log.debug("Reloading Unbound...")
>> -
>> - # Reload the configuration without dropping the cache
>> - self._control("reload_keep_cache")
>> + # Reload the configuration without dropping the cache
>> + self._control("reload_keep_cache")
>>=20
>> def write_dhcp_leases(self, leases):
>> log.debug("Writing DHCP leases...")
>>=20
>> with tempfile.NamedTemporaryFile(mode=3D"w") as f:
>> - for l in leases:
>> + for l in sorted(leases, key=3Dlambda x: x.ipaddr):
>> for rr in l.rrset:
>> f.write("local-data: \"%s\"\n" % " ".join(rr))
>>=20
>> # Flush the file
>> f.flush()
>>=20
>> + # Compare if the new leases file has changed from the previous version
>> + if filecmp.cmp(f.name, self.path, shallow=3DFalse):
>> + log.debug("The generated leases file has not changed")
>> +
>> + return False
>> +
>> # Make file readable for everyone
>> os.fchmod(f.fileno(), stat.S_IRUSR|stat.S_IWUSR|stat.S_IRGRP|stat.S_IROTH)
>>=20
>> @@ -543,6 +549,8 @@ class UnboundConfigWriter(object):
>> # Move the file to its destination
>> os.link(f.name, self.path)
>>=20
>> + return True
>> +
>> def _control(self, *args):
>> command =3D ["unbound-control"]
>> command.extend(args)
>>=20
>=20


--===============2529036883256318632==--