[lxc-devel] [PATCH RFC] log: introduce container logging

[Serge Hallyn]

Quoting S.Çağlar Onur (caglar at 10ur.org):
> Hi Serge,
> 
> On Wed, Mar 12, 2014 at 11:02 AM, Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
> > Quoting S.Çağlar Onur (caglar at 10ur.org):
> >> Hi Serge,
> >>
> >> On Tue, Mar 11, 2014 at 8:13 PM, Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
> >> > This is not intended to be applied yet.
> >> >
> >> > This patch adds real per-container, thread-safe log preferences.  Currently
> >> > if a program loads two containers which both set a lxc.logfile, and start
> >> > then start them in sequence, the logfile for the second container will be
> >> > used.  This patch stores the log preferences in the lxc_container itself.
> >> > It leaves ERROR/INFO/etc as global log print helpers, while the new
> >> > CERROR/CINFO/etc become new per-container helpers.  To demonstrate how to
> >> > use these, some logging in lxcapi_start and lxcapi_create has been
> >> > converted.
> >> >
> >> > What this patch ignores, and why I say this patch is not ready to be
> >> > applied, is what to do about 'lxc-start -l info -o outfile'.  With this
> >> > patch, global logging (ERROR) will go to outfile, while per-container
> >> > logging will not.
> >>
> >> Wow, I didn't see that coming as I was thinking you were joking the
> >> other day :) One possible solution to this global v.s per-container
> >> logging problem could be using a well known global log file and
> >> introducing a new argument to override it.
> >>
> >> We could say that LXC_LOGPATH/lxc.log will have the global logs and
> >> logfile parameter will have the per-container logs. I guess the
> >> downside of this approach would be looking 2 different log files in 2
> >> different locations in case of a failure.
> >>
> >> Do you think we have lots of code that does something before we create
> >> an lxc_container struct? I could be wrong but I suspect we don't have
> >> much in as we almost have the lxc_container around all the time. So
> >> maybe we can make everything per-container level.
> >
> > My thought was that anything relating to the starting, stopping,
> > running, or lxc-monitor for a container would go to the per-container
> > log.  If things break while constructing the lxc_container, i.e. we
> > can't read the config file, that goes to the global log.  You might
> > be right that it is best to always use LXC_LOGPATH/lxc.log for the
> > global log, or maybe even just LXCPATH/lxc.log, to minimize the
> > amount of looking around we have to do when something fails.
> >
> > There is a practical problem.  We have lots of helper functions
> > which don't get a lxc_container passed to them.  Some don't even
> > get an lxc_conf.  Three obvious ways to fix that would be
> >
> > 1. always pass a lxc_container * everywhere.  Don't like that.
> 
> I've been thinking about this and I believe I reached a point where
> this option starts to make sense to me :)
> 
> Most of those helper functions are not generic, they are doing
> something on behalf of the container (or as a part of container setup
> process). I know this will be a very bad example but most of them
> could have been a method of a Container class if we were using C++ (as
> they might logically fit into that encapsulation). And based on this
> (flawed?) idea, I think I started to like passing container struct
> around way.

Ok, I appreciate your input.  Do you have time to start a trial
patch for this approach?  If not I'll give it a shot when I get
time.