On Wed, Nov 17, 2010 at 06:42:14PM +0100, Bernd Schubert wrote:
> Hello Lars,
> 
> thanks for your very helpful approach! So we won't go into an endless 
> discussion without a result :)
> 
> On Wednesday, November 17, 2010, Lars Ellenberg wrote:
> 
> > [PATCH 05 of 10] ha_logd: Use C99 initializers, also correct max entity
> > string length
> > 
> >     Applied with minimal changes.
> 
> Thanks for applying this and the others. Although, I will will be blamed now 
> that bisecting cluster-glue fails and it is not really my fault ;)
> Please see the attached patch. I run into the same trap myself at least a 
> hundred times, though.

Bah. Thanks.
Maybe I can rebase ... ah well, never mind, it's public already -_-

> > [PATCH 10 of 10] ha_logd: Add a SIGHUP signal handler to close/open log
> > files [PATCH 08 of 10] cl_log: Restore old logfile open/seek/write/close
> > behaviour
> > 
> >     Conceptually changed.
> >     Proposed patches attached.
> >     Note: not even compile tested :(
> > 
> >     Please try/comment.
> 
> I'm just making this work, but could you please tell me, what 
> "re_open_logfiles" is supposed to be for?

At the time I thought that I'd set this as a flag from the sighup
handler and other places, but then later changed all those places to
just do close_log_files() directly.
Should go. Hey, 'twas late ...

> +       if (re_open_logfiles) {
> +               close_log_files();
> +               re_open_logfiles = 0;
> +       } else if (now - last_stat_time < 61) {
> 
> re_open_logfiles was not defined, but obviously it has type 'int'. But what 
> is 
> the purpose of that?
> 
> 
> +static void maybe_close_log_file(const char *fname, struct log_file_context 
> *lfc)
> +{
> +       struct stat buf;
> +       if (!lcf->fp)
> +               return;
> +       if (stat(fname, &log_sb) || log_sb.st_ino != lfc->stat_buf.st_ino)
> +               close_log_file(lfc);
> +}
> 
> 
> That should work :) Unless logrotate does something very bad (compress it, 
> without renaming it, then delete it) *and* another process races and creates 
> the file *and* the inode is recycled. 

The inode cannot be recycled while we still have it open,
no matter if it was deleted.
But yes, that should work.
Though, I'm not sure, there are "things" with unstable inodes ;-)
...

This, btw, would be the place to implement auto-rotation of log files
based on size or time or both.

> Attached files:
> log_warning.patch: Fix commit 2474:4a748dfb780e
> reopenlogfiles.diff: Compilation fixed, not further tested. FIXME needs to be 
> done at least first.
> sighup.diff: Compilation fixed, not further tested

Thanks,
        will work on that tomorrow.

-- 
: Lars Ellenberg
: LINBIT | Your Way to High Availability
: DRBD/HA support and consulting http://www.linbit.com

DRBD® and LINBIT® are registered trademarks of LINBIT, Austria.
_______________________________________________________
Linux-HA-Dev: [email protected]
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/

Reply via email to