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

[Stéphane Graber]

On Tue, Feb 04, 2014 at 03:53:48PM -0600, Serge Hallyn wrote:
> Quoting Stéphane Graber (stgraber at ubuntu.com):
> > In current LXC, loglevel and logfile are write-once functions.
> > That behaviour was appropriate when those two were first introduced
> > (pre-API) but with current API, one would expect to be able to
> > set_config_item those multiple times.
> > 
> > So instead, introduce lxc_log_options_no_override which when called
> > turns those two config keys read-only and have all existing binaries
> > which use log_init call that function once they're done setting the
> > value requested by the user.
> 
> Maybe I'm a bit to frazzled for this right now, but let's consider
> the ways loglevel and logfile can be set:
> 
> . they can come from command-line to lxc-start etc
> 
> . they can come from a container configuration file
> 
> (anything else?)
> 
> So let's say we have have an api program which does:
> 
> 	c1 = lxc_container_new("a1", NULL);
> 	c2 = lxc_container_new("a2", NULL);
> 	c1.start()
> 
> Let's say that c1 says lxc.loglevel=3 and c2 says lxc.loglevel=5.
> In the above pathological example c1 will end up running with
> lxc.loglevel=5 because we store the loglevel globally.
> 
> We don't always have a container available INFO() etc, so we can't
> just move loglevel+loginfo into the container.
> 
> 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? :)


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).


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.



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

> 
> -serge

-- 
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20140204/f9257f04/attachment.pgp>