[lxc-devel] [PATCH] logging: Add lxc_log_options_no_override function

[Serge Hallyn]

Quoting Stéphane Graber (stgraber at ubuntu.com):
> > Perhaps the right thing to do is
> > . make loglevel and logfile per-thread
> > . store the lxc.loglevel from config file into c->loglevel, and NOT set
> >   it globally (in per-thread)
> > . at lxcapi_create and lxcapi_start, set the global loglevel to the
> >   c->loglevel (if defined), and reset at end.
> 
> Is that you volunteering? :)

/me nonchalantly takes a step back

> More seriously, I've spent the last 30min or so looking at my screen
> trying to figure out a vaguely sane way out of our current logging
> nightmare and haven't found a magic way out yet.
> 
> So we clearly have two kind of things we want to log:
>  - Container related messages (99% of what we log)
>  - Non-container stuff (the odd 1% that we don't know where to log)
> 
> We also have two ways of setting where to log things:
>  - Command line argument to the binaries
>  - Container config options
> 
> So now for the easy cases. If a user passes a command line argument, we
> expect that to apply to both any log entry and override any
> per-container setting we may have.
> 
> For the case where the user doesn't, I think we should have a global
> lxc.logfile and lxc.loglevel which tells LXC what to do with those log
> entries that aren't container-related and for the container-related
> ones, send them to whatever's set in the container config (or in our
> hardcoded defaults).

Hm, that's a good idea, and actually if we let it be set through
either /etc/lxc/lxc.conf or through environment, so I could do

LXC_LOGLEVEL=INFO LXC_LOGFILE=/tmp/a lxc-ls

that would rock.

> Now I think the above is what we WANT, the question is how to do it,
> that's where it gets tricky...
> 
> So clearly we want to split the two kinds of logging functions
> (container, non-container), have the former look at the container's
> loglevel and logfile and the latter look at the global loglevel and
> logfile with all of those being overriddable (from the command line).
> 
> The obvious problem there is how to lookup the per-container loglevel
> and logfile without having to get struct container passed to every
> single function in our codebase. That's where I'm kind of stuck, so far
> the best I can come up with is to have LXC use global variables
> (possibly TLS) to store the current container being handled, making sure
> all relevant functions set/clear those variables appropriately and then
> have them used by the logging functions.

Right, I don't think we want to be carrying a piece of state around
to all functions just for logging info.  I think a per-thread instance
of those variables is the way to go.

I think if a user sets lxc.loglevel in a container config, the only
things they want logged there are things during lxcapi_start, and
*maybe* during create or destroy, those I'm not sure about those.
So the task of set/unseting the per-thread loglevel/loginfo should
actually not be daunting.

> Now, specifically for the change I sent, my intent was not to make
> things much worse than they are, albeit maybe a bit more confusing but
> solve a real bug that we got reported a few times (lxc.loglevel and
> lxc.logfile are entirely ignored when using the API as lxc_container_new
> sets those for us and we can't reset them).
> 
> For the standard case:
> p1 = Container("p1")
> p1.set_config_item("lxc.loglevel", "debug")
> p1.start()
> 
> p2 = Container("p2")
> p2.set_config_item("lxc.loglevel", "error")
> p2.start()
> 
> 
> The above should work fine, as start() will fork, so the values set
> after start() won't affect the running container, at least not until you
> call stop() and see things logged from the wrong level...

Ok.  While I wasn't yet ack'ing, I wasn't suggesting I wanted to nack
your patch - rather, I couldn't reason about it while my mind was
blocked with the above concerns :)  I'm a simpleton

Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>

thanks,
-serge