The patch looks good, but I have one simple question... isn't expensive
to call getpid() on each log_rec?
Cheers
Fabio
On 03/25/2013 04:13 PM, Jan Friesse wrote:
> When serivce plugin calls fork and child uses logsys, it may lead to
> corrupted fdata (idx is updated but it's not shared between child and
> parent, but fdata is mmaped file and this is shared).
>
> Solutions is to:
> - Store corosync pid
> - On log, check if current pid == corosync pid
> - If so -> store in fdata, if not -> don't store
>
> Signed-off-by: Jan Friesse <jfriesse@xxxxxxxxxx>
> ---
> exec/logsys.c | 28 ++++++++++++++++++----------
> 1 files changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/exec/logsys.c b/exec/logsys.c
> index c4611bc..cd6a311 100644
> --- a/exec/logsys.c
> +++ b/exec/logsys.c
> @@ -205,6 +205,8 @@ static int logsys_dropped_messages = 0;
>
> void *logsys_rec_end;
>
> +static pid_t startup_pid = 0;
> +
> static DECLARE_LIST_INIT(logsys_print_finished_records);
>
> #define FDMAX_ARGS 64
> @@ -1242,16 +1244,20 @@ void _logsys_log_vprintf (
> short_file_name++; /* move past the "/" */
> #endif /* BUILDING_IN_PLACE */
>
> - /*
> - * Create a log record
> - */
> - _logsys_log_rec (
> - rec_ident,
> - function_name,
> - short_file_name,
> - file_line,
> - logsys_print_buffer, len + 1,
> - LOGSYS_REC_END);
> + if (startup_pid == 0 || startup_pid == getpid()) {
> + /*
> + * Create a log record if we are really true corosync
> + * process (not fork of some service) or if we didn't finished
> + * initialization yet.
> + */
> + _logsys_log_rec (
> + rec_ident,
> + function_name,
> + short_file_name,
> + file_line,
> + logsys_print_buffer, len + 1,
> + LOGSYS_REC_END);
> + }
>
> /*
> * If logsys is not going to print a message to a log target don't
> @@ -1324,6 +1330,8 @@ int _logsys_config_subsys_get (const char *subsys)
> void logsys_fork_completed (void)
> {
> logsys_loggers[LOGSYS_MAX_SUBSYS_COUNT].mode &= ~LOGSYS_MODE_FORK;
> + startup_pid = getpid();
> +
> (void)_logsys_wthread_create ();
> }
>
>
_______________________________________________
discuss mailing list
discuss@xxxxxxxxxxxx
http://lists.corosync.org/mailman/listinfo/discuss
