[lxc-devel] [PATCH v2] Create log file in lxcpath for non-system containers

[Dwight Engen]

On Tue, 30 Apr 2013 08:37:44 -0500
Serge Hallyn <serge.hallyn at ubuntu.com> wrote:

> Quoting Dwight Engen (dwight.engen at oracle.com):
> > +/*
> > + * Build the path to the log file
> > + * @name     : the name of the container
> > + * @lxcpath  : the lxcpath to use as a basename or NULL to use
> > LOGPATH
> > + * Returns malloced path on sucess, or NULL on failure
> > + */
> > +static char *build_log_path(const char *name, const char *lxcpath)
> >  {
> >  	char *p;
> >  	int len, ret;
> >  
> >  	/*
> > -	 * '$logpath' + '/' + '$name' + '.log' + '\0'
> > -	 * or
> > +	 * If USE_CONFIGPATH_LOGS is true the resulting path will
> > be:
> >  	 * '$logpath' + '/' + '$name' + '/' + '$name' + '.log' +
> > '\0'
> > -	 * sizeof(LOGPATH) includes its \0
> > +	 *
> > +	 * If USE_CONFIGPATH_LOGS is false the resulting path will
> > be:
> > +	 * '$logpath' + '/' + '$name' + '.log' + '\0'
> >  	 */
> > -	len = sizeof(LOGPATH) + strlen(name) + 6;
> > +	len = strlen(name) + 6; /* 6 == '/' + '.log' + '\0' */
> > +	if (!lxcpath)
> > +		lxcpath = LOGPATH;
> >  #if USE_CONFIGPATH_LOGS
> > -	len += strlen(name) + 1;  /* add "/$container_name/" */
> > +	len += strlen(lxcpath) + 1 + strlen(name) + 1;  /* add
> > "/$container_name/" */ +#else
> > +	len += strlen(lxcpath) + 1;
> >  #endif
> >  	p = malloc(len);
> >  	if (!p)
> >  		return p;
> >  #if USE_CONFIGPATH_LOGS
> > -	ret = snprintf(p, len, "%s/%s/%s.log", LOGPATH, name,
> > name);
> > +	ret = snprintf(p, len, "%s/%s/%s.log", lxcpath, name,
> > name); #else
> > -	ret = snprintf(p, len, "%s/%s.log", LOGPATH, name);
> > +	ret = snprintf(p, len, "%s/%s.log", lxcpath, name);
> 
> Hi Dwight,
> 
> I've pushed this patch to staging, but I do notice an oddity when
> USE_CONFIGPATH_LOGS=n (as on ubuntu).  When I now start a container
> with an lxcpath, i.e. lxc-start -n c1 -P /home/serge/lxcbase,
> then the logfile is at /home/serge/lxcbase/dir1.log, not
> /home/serge/lxcbase/dir1/dir1.log.  I think it would be better to have
> something like
> 
>  #if USE_CONFIGPATH_LOGS
> 	ret = snprintf(p, len, "%s/%s/%s.log", lxcpath, name, name);
>  #else
> 	if (lxcpath == default_lxc_path())
> 		ret = snprintf(p, len, "%s/%s.log", lxcpath, name);
> 	else
> 		ret = snprintf(p, len, "%s/%s/%s.log", lxcpath, name,
> name); #endif
> 
> Well, that or we use $lxcpath/logs/$name.log, but I prefer to keep the
> rule: we either use $LOGPATH/$name.log, or else
> $lxcpath/$name/$name.log, always.
> 
> Thoughts?

Hi Serge, Hmm, I was just keeping the old behavior of
USE_CONFIGPATH_LOGS which meant you would always get the extra dir in
the middle (even on $LOGPATH), and otherwise the log would be in
$LOGPATH (or $lxcpath with this change).

I see your point though that having logs right in $lxcpath isn't so
nice, and I like what you've outlined above although I guess it
does slightly change the meaning of USE_CONFIGPATH_LOGS. I'll
submit a patch to do what you've suggested, and we can see if anyone
else cares to chime in. Thanks.

> thanks,
> -serge