[ovs-dev] [PATCH v2 0/9] tests: Clean up syslog.

[Ben Pfaff]

On Wed, Aug 08, 2018 at 03:07:04PM +0300, Ilya Maximets wrote:
> On 07.08.2018 00:12, Ben Pfaff wrote:
> > On Mon, Aug 06, 2018 at 06:18:19PM +0300, Ilya Maximets wrote:
> >> On 04.08.2018 03:17, Ben Pfaff wrote:
> >>> On Wed, Aug 01, 2018 at 05:00:09PM +0300, Ilya Maximets wrote:
> >>>> Each run of the testsuite produces millions lines in a system
> >>>> log. This is completely unnecessary and makes it difficult to
> >>>> use system logs on test / build servers.
> >>>>
> >>>> This series is aimed to disable most of the syslog messages.
> >>>> There are still few logs that requires significant changes in
> >>>> tests or code to disable. They will be removed separately if
> >>>> needed.
> >>>
> >>> This series seems technically high-quality.  Thank you.
> >>>
> >>> I find myself wondering whether we should just introduce an environment
> >>> variable to allowing configuring logging defaults.  Then we would cover
> >>> literally everything with a single line in atlocal.in.
> >>>
> >>> Any opinions?
> >>
> >> I thought about this. And implementation will have few issues:
> >>
> >> 1. Many daemons already has logging related options in cmdlines.
> >>    This means that it should not be "OVS_DEFAULT_LOGGING_OPTIONS",
> >>    but "OVS_FORCE_LOGGING_OPTIONS". i.e. the value in the environment
> >>    should have highest priority.
> >> 2. Leads from the 1st. I'm not sure that we should expose variables
> >>    like this for the end users, because there will be no way to change
> >>    the logging level for the targets forced by the environment.
> >>    So, it should be test only variable like
> >>    "TEST_OVS_FORCE_DEFAULT_OPTIONS". But we still can't be sure that
> >>    no one will use it in production.
> >> 3. Some tests (like some from vlog.at) requires ability to change the
> >>    log level for syslog, for example. We'll need to unset the variable
> >>    for these tests or change the tests themselves.
> >> 4. Modifications for all the utils/daemons and/or the vlog subsystem
> >>    are required to implement this environment variable which is
> >>    intended for test purposes only
> >>
> >> I'm not sure which solution is better.
> >> What do you think?
> > 
> > Most of these points seem to follow from #1, but I don't understand #1.
> > Why shouldn't the environment variable be just a default that can be
> > overridden by command-line arguments?  That is usually what environment
> > settings do.
> 
> Yes, you're right that having environment variable which could be overridden
> by command-line arguments is a common and usual thing. But tests contains a
> lot of daemons'/control utils' invocations like this:
>     ovs-vswitchd --enable-dummy=system --disable-system --detach \
>                  --no-chdir --pidfile --log-file \
>                  -vvconn -vofproto_dpif
>     or
>     ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg
> 
> In above cases environment variable will be overridden and we will have
> debug logs in syslog. So, we'll have to apply at least a half of current
> patch set to disable syslog in these cases. See patches 4, 5, 6 and 8.

Mmm, you're right, thanks.