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

[John Ousterhout <ouster@cs.stanford.edu>]

On Tue, Oct 11, 2016 at 1:08 AM, Thomas Monjalon <thomas.monjalon@6wind.com>
wrote:
>
> 2016-10-10 15:39, John Ousterhout:
> > ...
> >
> > 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?

I would guess that stdout makes more sense, since most log entries describe
normal operation, not errors. I'm happy to make these consistent, but this
would introduce a behavior change for BSD (which currently uses stderr);
would that be considered antisocial?

> [...]
> > -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.

Oops, right; will fix.

>
> [...]
> >  /*
> > - * 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?

Right now, different platforms have different defaults. BSD uses stderr
always. Linux starts out with stdout as the default, but later during
initialization it switches to a different default that logs messages both
to  standard output and also to syslog. I don't have enough experience with
DPDK to know whether a single approach is really right for all systems (or
which approach it should be), and since I'm a DPDK newbie I thought it best
to take a more conservative approach and avoid behavioral changes. My
personal preference would be to minimize mission creep with this patch and
leave that behavior in place for someone with more expertise to deal with
later (and this issue is orthogonal to the main goal of the patch). But, if
unifying the log defaults is considered essential to the patch (and is
noncontroversial), I'm willing to implement it.

> [...]
> >  /**
> > - * 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.

Oops; fixed.

Thanks for the comments.

-John-