[PATCH]: support setting a default suppress() globally
Hi! Attached is a patch that makes it possible to set a default value for suppress() in the global options block, which will then be inherited by all drivers that support this option. Of course, it can still be overridden on a per-destination basis. What this means is, that if one has a larger number of destinations that all need the same suppress settings, we'd write that like this before this patch: destination d_1 { file("/var/log/messages-1" suppress(10)); }; destination d_2 { file("/var/log/messages-2" suppress(10)); }; ... destination d_N { file("/var/log/messages-N" suppress(10)); }; destination d_XX { file("/var/log/messages-XX" suppress(5)); }; After the patch, this can be shortened to: options { suppress(10); }; destination d_1 { file("/var/log/messages-1"); }; ... destination d_N { file("/var/log/messages-N"); }; destination d_XX { file("/var/log/messages-XX" suppress(5)); }; Both result in exactly the same behaviour, but the second is a lot shorter, and easier to maintain, as long as the majority of destinations need the same suppress() setting. Hopefully people will find this useful. The patch is also available from the work/global-suppress branch of my tree at git://git.balabit.hu/algernon/syslog-ng-3.3.git -- |8]
Hi Gergely, Sorry for not reviewing your patch any sooner, but you know the drill. :) Could you please post your patches in-line next time? It is much easier to read and react on things that way. Thanks. But here comes the review:
diff --git a/modules/affile/affile-grammar.ym b/modules/affile/affile-grammar.ym index 2a7c134..5379b81 100644 --- a/modules/affile/affile-grammar.ym +++ b/modules/affile/affile-grammar.ym @@ -132,6 +132,7 @@ dest_affile_params (*last_driver) = affile_dd_new($1, 0); free($1); last_writer_options = &((AFFileDestDriver *) (*last_driver))->writer_options; + last_writer_options->suppress = configuration->suppress; } dest_affile_options { $$ = (*last_driver); } ;
A better way to do this would be to do the inheritance in the log_writer_options_init() function, just like all the rest of the options. E.g. please initialize the per-writer suppress to default to -1 (in log_writer_options_defaults), and in log_writer_options_init() inherit the global value if the setting stays at -1. I guess all the rest should be implemented this way. I really like the patch, so please post an updated patch for inclusion. Thanks.
@@ -163,6 +164,7 @@ dest_afpipe_params free($1); last_writer_options = &((AFFileDestDriver *) (*last_driver))->writer_options; last_writer_options->flush_lines = 0; + last_writer_options->suppress = configuration->suppress; } dest_afpipe_options { $$ = (*last_driver); } ;
hmm.. that flush_lines initialization seems spiffy, so I've removed that with this patch: commit ac51af8f501ed5b723c055cbb30c0cb1e09f0117 Author: Balazs Scheidler <bazsi@balabit.hu> Date: Tue Mar 1 21:47:15 2011 +0100 pipe(): use the default flush_lines() value if one is not specified For some historic reason the pipe() destination overrode the global flush_lines() settings by default. This is inconsistent behaviour, so fix it. Signed-off-by: Balazs Scheidler <bazsi@balabit.hu> -- Bazsi
'Evening! On Tue, 2011-03-01 at 21:49 +0100, Balazs Scheidler wrote:
Could you please post your patches in-line next time? It is much easier to read and react on things that way. Thanks.
Will do. About time I set up my Gnus anyway.
diff --git a/modules/affile/affile-grammar.ym b/modules/affile/affile-grammar.ym index 2a7c134..5379b81 100644 --- a/modules/affile/affile-grammar.ym +++ b/modules/affile/affile-grammar.ym @@ -132,6 +132,7 @@ dest_affile_params (*last_driver) = affile_dd_new($1, 0); free($1); last_writer_options = &((AFFileDestDriver *) (*last_driver))->writer_options; + last_writer_options->suppress = configuration->suppress; } dest_affile_options { $$ = (*last_driver); } ;
A better way to do this would be to do the inheritance in the log_writer_options_init() function, just like all the rest of the options.
E.g. please initialize the per-writer suppress to default to -1 (in log_writer_options_defaults), and in log_writer_options_init() inherit the global value if the setting stays at -1.
Understood, I'll update the patch as soon as I get around to it (probably this weekend, if all goes well). -- |8]
Balazs Scheidler <bazsi@balabit.hu> writes:
diff --git a/modules/affile/affile-grammar.ym b/modules/affile/affile-grammar.ym index 2a7c134..5379b81 100644 --- a/modules/affile/affile-grammar.ym +++ b/modules/affile/affile-grammar.ym @@ -132,6 +132,7 @@ dest_affile_params (*last_driver) = affile_dd_new($1, 0); free($1); last_writer_options = &((AFFileDestDriver *) (*last_driver))->writer_options; + last_writer_options->suppress = configuration->suppress; } dest_affile_options { $$ = (*last_driver); } ;
A better way to do this would be to do the inheritance in the log_writer_options_init() function, just like all the rest of the options.
E.g. please initialize the per-writer suppress to default to -1 (in log_writer_options_defaults), and in log_writer_options_init() inherit the global value if the setting stays at -1.
I guess all the rest should be implemented this way.
I really like the patch, so please post an updated patch for inclusion.
I updated the patch, and its available from the integration/global-suppress branch (rebased onto the tip of your master branch as of an hour or so ago) of my syslog-ng-3.3 repo at: git://git.balabit.hu/algernon/syslog-ng-3.3.git The updated patch is included below: --8<---------------cut here---------------start------------->8---
From ee380184fb6147db9ca410c92e466bad1bf00721 Mon Sep 17 00:00:00 2001 From: Gergely Nagy <algernon@balabit.hu> Date: Fri, 4 Mar 2011 10:56:55 +0100 Subject: [PATCH] config: Support setting a default suppress() globally.
While setting suppress() on a per-destination basis is flexible, when one wants to have the same setting for all destinations by default, it's more convenient to do that globally in the options block. That is implemented herein: one can set a global suppress(), and it will be the default for all destinations that support it, yet, can still be overridden on a per-destination basis. Signed-off-by: Gergely Nagy <algernon@balabit.hu> --- lib/cfg-grammar.y | 1 + lib/cfg.h | 1 + lib/logwriter.c | 4 +++- 3 files changed, 5 insertions(+), 1 deletions(-) diff --git a/lib/cfg-grammar.y b/lib/cfg-grammar.y index 8d45af3..0a78b68 100644 --- a/lib/cfg-grammar.y +++ b/lib/cfg-grammar.y @@ -600,6 +600,7 @@ options_item | KW_TIME_REOPEN '(' LL_NUMBER ')' { configuration->time_reopen = $3; } | KW_TIME_REAP '(' LL_NUMBER ')' { configuration->time_reap = $3; } | KW_TIME_SLEEP '(' LL_NUMBER ')' {} + | KW_SUPPRESS '(' LL_NUMBER ')' { configuration->suppress = $3; } | KW_THREADED '(' yesno ')' { configuration->threaded = $3; } | KW_LOG_FIFO_SIZE '(' LL_NUMBER ')' { configuration->log_fifo_size = $3; } | KW_LOG_IW_SIZE '(' LL_NUMBER ')' { msg_error("Using a global log-iw-size() option was removed, please use a per-source log-iw-size()", NULL); } diff --git a/lib/cfg.h b/lib/cfg.h index 455b36d..82a30c5 100644 --- a/lib/cfg.h +++ b/lib/cfg.h @@ -76,6 +76,7 @@ struct _GlobalConfig gchar *dns_cache_hosts; gint time_reopen; gint time_reap; + gint suppress; gint log_fifo_size; gint log_msg_size; diff --git a/lib/logwriter.c b/lib/logwriter.c index 0a21876..5586230 100644 --- a/lib/logwriter.c +++ b/lib/logwriter.c @@ -1079,7 +1079,7 @@ log_writer_options_defaults(LogWriterOptions *options) options->flush_timeout = -1; log_template_options_defaults(&options->template_options); options->time_reopen = -1; - options->suppress = 0; + options->suppress = -1; } void @@ -1155,6 +1155,8 @@ log_writer_options_init(LogWriterOptions *options, GlobalConfig *cfg, guint32 op options->flush_lines = cfg->flush_lines; if (options->flush_timeout == -1) options->flush_timeout = cfg->flush_timeout; + if (options->suppress == -1) + options->suppress = cfg->suppress; if (options->mem_fifo_size < options->flush_lines) { -- 1.7.0.4 --8<---------------cut here---------------end--------------->8---
On Fri, 2011-03-04 at 11:41 +0100, Gergely Nagy wrote:
Balazs Scheidler <bazsi@balabit.hu> writes:
diff --git a/modules/affile/affile-grammar.ym b/modules/affile/affile-grammar.ym index 2a7c134..5379b81 100644 --- a/modules/affile/affile-grammar.ym +++ b/modules/affile/affile-grammar.ym @@ -132,6 +132,7 @@ dest_affile_params (*last_driver) = affile_dd_new($1, 0); free($1); last_writer_options = &((AFFileDestDriver *) (*last_driver))->writer_options; + last_writer_options->suppress = configuration->suppress; } dest_affile_options { $$ = (*last_driver); } ;
A better way to do this would be to do the inheritance in the log_writer_options_init() function, just like all the rest of the options.
E.g. please initialize the per-writer suppress to default to -1 (in log_writer_options_defaults), and in log_writer_options_init() inherit the global value if the setting stays at -1.
I guess all the rest should be implemented this way.
I really like the patch, so please post an updated patch for inclusion.
I updated the patch, and its available from the integration/global-suppress branch (rebased onto the tip of your master branch as of an hour or so ago) of my syslog-ng-3.3 repo at:
git://git.balabit.hu/algernon/syslog-ng-3.3.git
The updated patch is included below:
--8<---------------cut here---------------start------------->8---
Applied, thanks. -- Bazsi
participants (2)
-
Balazs Scheidler
-
Gergely Nagy