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

Tue Feb 4 23:01:27 UTC 2014

On Tue, Feb 04, 2014 at 04:52:16PM -0600, Serge Hallyn wrote:
> 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.

Right, when I set lxc.logfile and lxc.loglevel in a container's config,
I expect anything related to the container's life cycle to go to that
log file which isn't necessarily everything we do.

I guess I'd at least expect:
 - config parsing (load_config, clear_config, {get|set}_config_item, ...)
 - start
 - stop

Where config parsing will only work while using the default
loglevel/logfile or those passed on the command line, at least until we
parse the right line in the container's config :)

The rest can probably go to global logging for now (with default being
loglevel=ERROR and logfile=stderr), so identical to what we have now.

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

Thanks :)

> Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>
> 
> thanks,
> -serge
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel

-- 
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/a7842397/attachment.pgp>