Re: [dpdk-dev] [PATCH v2] log: respect rte_openlog_stream calls before rte_eal_init

[Thomas Monjalon <thomas.monjalon@6wind.com>]

2016-10-10 15:39, John Ousterhout:
> Before this patch, application-specific loggers could not be
> installed before rte_eal_init completed (the initialization process
> called rte_openlog_stream, overwriting any previously installed
> logger). This made it impossible for an application to capture the
> initial log messages generated during rte_eal_init. This patch changes
> initialization so that information from a previous call to
> rte_openlog_stream is not lost. Specifically:
> * The default log stream is now maintained separately from an
>   application-specific log stream installed with rte_openlog_stream.
> * rte_eal_common_log_init has been renamed to rte_eal_log_set_default,
>   since this is all it does. It no longer invokes rte_openlog_stream; it
>   just updates the default stream. Also, this method now returns void,
>   rather than int, since there are no errors.
> * The "early log" mechanism (e.g. rte_eal_log_early_init) has been
>   removed; all of the desired functionality can be achieved by calling
>   rte_eal_log_set_default.
> 
> Signed-off-by: John Ousterhout <ouster@cs.stanford.edu>
> ----
> v2:
> * Removed the early log mechanism, renamed rte_eal_common_log_init.
> 
> Note: I see from the code that Linux and BSD set different default streams:
> Linux uses stdout, while BSD uses stderr. This patch retains the distinction,
> though I'm not sure why it is there.

I don't know either.
What is best between stdout and stderr for logs?

[...]
> -int
> -rte_eal_log_early_init(void)
> -{
> -	rte_openlog_stream(stderr);
> -	return 0;
> +	rte_eal_set_default(stderr);

It should be rte_eal_log_set_default.

[...]
>  /*
> - * called by environment-specific log init function
> + * Called by environment-specific initialization functions.
>   */
> -int
> -rte_eal_common_log_init(FILE *default_log)
> +void
> +rte_eal_log_set_default(FILE *default_log)
>  {
>  	default_log_stream = default_log;
> -	rte_openlog_stream(default_log);
>  
>  #if RTE_LOG_LEVEL >= RTE_LOG_DEBUG
>  	RTE_LOG(NOTICE, EAL, "Debug logs available - lower performance\n");
>  #endif
> -
> -	return 0;
>  }

Do we really need a function for that?
Why not just initialize default_log_stream statically?

[...]
>  /**
> - * Common log initialization function (private to eal).
> + * Common log initialization function (private to eal).  Determines
> + * where log data is written when no call to eal_openlog_stream is
> + * in effect.

It should be rte_openlog_stream.