Hi

On 30.10.2013 20:04, Josip Rodin wrote:
On Wed, Oct 30, 2013 at 02:38:39PM +0100, Tom Jampen wrote:
+--- a/raddb/modules/detail     2013-10-30 11:55:01.600274413 +0100
++++ b/raddb/modules/detail     2013-10-30 12:07:34.703092048 +0100
+@@ -31,7 +31,7 @@ detail {
+       #  be ONE "listen" section reading detail files from a
+       #  particular directory.
+       #
+-      detailfile = 
${radacctdir}/%{%{Packet-Src-IP-Address}:-%{Packet-Src-IPv6-Address}}/detail-%Y%m%d
++      detailfile = 
${radacctdir}/%{%{Packet-Src-IP-Address}:-%{Packet-Src-IPv6-Address}}/detail

So you'd be moving the decision of rotation from FR to logrotate

Looking in my /var/log shows me mostly rotated logs. So yes, it is my personal understanding that for a sysadmin (at least rapidly growing) logs should be rotated automatically (so he has one place to take action for changing behavior). Why handle things differently for FR than what one is used to.

Even more confusing might be the fact that one FR log is rotated while others aren't. That doesn't really make things easier to understand...


- but I
wouldn't exactly be sure that FR is ready for it - does the detail module
necessarily close and reopen the files on every write, or will it keep
writing to the old file until you HUP it?

I agree, that should really be checked first. (Again, from a sysadmin point of view, why would one logfile be ready for rotating while others aren't... not easy to see for non-developers.)


There have been some race condition bugs in the HUP handling in the past, so
this should really be verified beforehand.

Absolutely.


Also, what about the people who are accustomed to the old defaults and have
their own scripts? It could easily break their setup, so we'd need a
NEWS.Debian warning at least.

Agreed. Maybe a good time for config file cleanup would be when migrating to freeradius 2.2 or even 3. Then it's obvious for everyone to check configs...


+@@ -43,7 +43,7 @@ server buffered-sql {
+               #  The location where the detail file is located.
+               #  This should be on local disk, and NOT on an NFS
+               #  mounted location!
+-              filename = "${radacctdir}/detail-*"
++              filename = "${radacctdir}/detail.example.com/detail"

+@@ -63,7 +63,7 @@ server copy-acct-to-home-server {
+               #  one large file.  File globbing also means that with
+               #  a common naming scheme for detail files, then you can
+               #  have many detail file writers, and only one reader.
+-              filename = ${radacctdir}/detail
++              filename = ${radacctdir}/detail.example.com/detail

+@@ -40,7 +40,7 @@ server read-detail.example.com {
+       #  the home server.
+       listen {
+               type = detail
+-              filename = "${radacctdir}/detail.example.com/detail-*:*"
++              filename = "${radacctdir}/detail.example.com/detail"

This is another beast altogether. What about race conditions here, that is,
what guarantees that the reader will be done reading all the data from the
detail file before it gets moved away by logrotate?

And why the new example hostname part in the first two, won't that also
break existing setups using the file?

Maybe it's a bad idea, but my intension was to make things more consistent. There's a lot of config to manage, so why use one path here and a different one there? Probably the best option would be to check with upstream first and prepare a config cleanup with the next major upgrade.

Regards,
Tom


--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Reply via email to