use a default per-container logfile

Until now, if a lxc-* (i.e. lxc-start) command did not specify a logfile
(with -o logfile), the default was effectively 'none'.  With this patch,
the default becomes a per-container log file.

If a container config file specifies 'lxc.logfile', that will override
the default.  If a '-o logfile' argument is specifed at lxc-start,
then that will override both the default and the configuration file
entry.  Finally, '-o none' can be used to avoid having a logfile at
all (in other words, the previous default), and that will override
a lxc.logfile entry in the container configuration file.

If the user does not have rights to open the default, then 'none' will
be used.  However, in that case an error will show up on console.  (We
can work on removing that if it annoys people, but I think it is
helpful, at least while we're still ironing this set out)  If the user
or container configuration file specified a logfile, and the user does
not have rights to open the default, then the action will fail.

One slight "mis-behavior" which I have not fixed (and may not fix) is
that if a lxc.logfile is specified, the default logfile will still
get created before we read the configuration file to find out there
is a lxc.logfile entry.

changelog:  Jan 24:

 add --enable-configpath-log configure option

 When we log to /var/lib/lxc/$container/$container.log, several things
 need to be done differently than when we log into /var/log/lxc (for
 instance).  So give it a configure option so we know what to do

 When the user specifies a logfile, we bail if we can't open it.  But
 when opening the default logfile, the user may not have rights to
 open it, so in that case ignore it and continue as if using 'none'.

 When using /var/lib/lxc/$c/$c.log, we use $LOGPATH/$name/$name.log.
 Otherwise, we use $LOGPATH/$name.log.

 When using /var/lib/lxc/$c/$c.log, don't try to create the log path
 /var/lib/lxc/$c.  It can only not exist if the container doesn't
 exist.  We don't want to create the directory in that case.  When
 using /var/log/lxc, then we do want to create the path if it does
 not exist.

Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>
Acked-by: Stéphane Graber <stgraber@ubuntu.com>

diff --git a/configure.ac b/configure.ac
index d1f5ad950f9c27baeaf5d5d6006b5f0aef968237..e56724571e96b86135b8d6342cfab625aba827f1 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -157,6 +157,26 @@ AC_ARG_WITH([rootfs-path],
                [lxc rootfs mount point]
        )], [], [with_rootfs_path=['${libdir}/lxc/rootfs']])
 
+# Container log path.  By default, use $lxcpath.
+AC_MSG_CHECKING([Whether to place logfiles in container config path])
+AC_ARG_ENABLE([configpath-log],
+       [AC_HELP_STRING([--enable-configpath-log], [use logfiles in config path])],
+       [use_configpath_logs=yes], [use_configpath_logs=no])
+AC_MSG_RESULT([$use_configpath_logs])
+AM_CONDITIONAL([USE_CONFIGPATH_LOGS], [test "$use_configpath_logs" = "yes"])
+
+if test "$use_configpath_logs" = "yes"; then
+       default_log_path="${with_config_path}"
+else
+       default_log_path="/var/log/lxc"
+fi
+
+AC_ARG_WITH([log-path],
+       [AC_HELP_STRING(
+               [--with-log-path=dir],
+               [per container log path]
+       )], [], [with_log_path=['${default_log_path}']])
+
 # Expand some useful variables
 AS_AC_EXPAND(PREFIX, "$prefix")
 AS_AC_EXPAND(LIBDIR, "$libdir")
@@ -173,6 +193,7 @@ AS_AC_EXPAND(LXCPATH, "$with_config_path")
 AS_AC_EXPAND(LXCROOTFSMOUNT, "$with_rootfs_path")
 AS_AC_EXPAND(LXCTEMPLATEDIR, "$datadir/lxc/templates")
 AS_AC_EXPAND(LXCINITDIR, "$libexecdir")
+AS_AC_EXPAND(LOGPATH, "$with_log_path")
 
 # Check for some standard kernel headers
 AC_CHECK_HEADERS([linux/unistd.h linux/netlink.h linux/genetlink.h],

diff --git a/src/lxc/Makefile.am b/src/lxc/Makefile.am
index b55a20c54cfebfd2fa3955fc56b3d90b41a9890a..2fb82913707d68e3b7516fc1e2faf1de0a5f6122 100644 (file)
--- a/src/lxc/Makefile.am
+++ b/src/lxc/Makefile.am
@@ -89,12 +89,17 @@ AM_CFLAGS=-I$(top_srcdir)/src \
        -DLXCROOTFSMOUNT=\"$(LXCROOTFSMOUNT)\" \
        -DLXCPATH=\"$(LXCPATH)\" \
        -DLXCINITDIR=\"$(LXCINITDIR)\" \
-       -DLXCTEMPLATEDIR=\"$(LXCTEMPLATEDIR)\"
+       -DLXCTEMPLATEDIR=\"$(LXCTEMPLATEDIR)\" \
+       -DLOGPATH=\"$(LOGPATH)\"
 
 if ENABLE_APPARMOR
 AM_CFLAGS += -DHAVE_APPARMOR
 endif
 
+if USE_CONFIGPATH_LOGS
+AM_CFLAGS += -DUSE_CONFIGPATH_LOGS
+endif
+
 if ENABLE_SECCOMP
 AM_CFLAGS += -DHAVE_SECCOMP
 liblxc_so_SOURCES += seccomp.c

diff --git a/src/lxc/confile.c b/src/lxc/confile.c
index 67a989ed43650060f4280ffa82a08e729178b184..d350f01df1c8c5c1548028506d112832607cc3c8 100644 (file)
--- a/src/lxc/confile.c
+++ b/src/lxc/confile.c
@@ -927,11 +927,6 @@ static int config_aa_profile(const char *key, const char *value,
 static int config_logfile(const char *key, const char *value,
                             struct lxc_conf *lxc_conf)
 {
-       if (lxc_log_get_file()) {
-               DEBUG("Log file already specified - ignoring new value");
-               return 0;
-       }
-
        return lxc_log_set_file(value);
 }
 

diff --git a/src/lxc/log.c b/src/lxc/log.c
index d2a90de7a6846045cd0564d7a24935031aa94107..ff512e38ba4b95ad830827e783eb29e590dbafb8 100644 (file)
--- a/src/lxc/log.c
+++ b/src/lxc/log.c
@@ -42,6 +42,8 @@
 int lxc_log_fd = -1;
 static char log_prefix[LXC_LOG_PREFIX_SIZE] = "lxc";
 static int lxc_loglevel_specified = 0;
+// if logfile was specifed on command line, it won't be overridden by lxc.logfile
+static int lxc_log_specified = 0;
 
 lxc_log_define(lxc_log, lxc);
 
@@ -123,6 +125,36 @@ extern void lxc_log_setprefix(const char *prefix)
        log_prefix[sizeof(log_prefix) - 1] = 0;
 }
 
+static int build_dir(const char *name)
+{
+       char *n = strdup(name);  // because we'll be modifying it
+       char *p, *e;
+       int ret;
+
+       if (!n) {
+               ERROR("Out of memory while creating directory '%s'.", name);
+               return -1;
+       }
+
+       e = &n[strlen(n)];
+       for (p = n+1; p < e; p++) {
+               if (*p != '/')
+                       continue;
+               *p = '\0';
+               if (access(n, F_OK)) {
+                       ret = lxc_unpriv(mkdir(n, 0755));
+                       if (ret && errno != -EEXIST) {
+                               SYSERROR("failed to create directory '%s'.", n);
+                               free(n);
+                               return -1;
+                       }
+               }
+               *p = '/';
+       }
+       free(n);
+       return 0;
+}
+
 /*---------------------------------------------------------------------------*/
 static int log_open(const char *name)
 {
@@ -148,11 +180,46 @@ static int log_open(const char *name)
        return newfd;
 }
 
+static char *build_log_path(const char *name)
+{
+       char *p;
+       int len, ret;
+
+       /*
+        * '$logpath' + '/' + '$name' + '.log' + '\0'
+        * or
+        * '$logpath' + '/' + '$name' + '/' + '$name' + '.log' + '\0'
+        * sizeof(LOGPATH) includes its \0
+        */
+       len = sizeof(LOGPATH) + strlen(name) + 6;
+#if USE_CONFIGPATH_LOGS
+       len += strlen(name) + 1;  /* add "/$container_name/" */
+#endif
+       p = malloc(len);
+       if (!p)
+               return p;
+#if USE_CONFIGPATH_LOGS
+       ret = snprintf(p, len, "%s/%s/%s.log", LOGPATH, name, name);
+#else
+       ret = snprintf(p, len, "%s/%s.log", LOGPATH, name);
+#endif
+       if (ret < 0 || ret >= len) {
+               free(p);
+               return NULL;
+       }
+       return p;
+}
+
+int do_lxc_log_set_file(const char *fname, int from_default);
+
 /*---------------------------------------------------------------------------*/
-extern int lxc_log_init(const char *file, const char *priority,
-                       const char *prefix, int quiet)
+extern int lxc_log_init(const char *name, const char *file,
+                       const char *priority, const char *prefix, int quiet)
 {
        int lxc_priority = LXC_LOG_PRIORITY_ERROR;
+       int ret;
+       char *tmpfile = NULL;
+       int want_lxc_log_specified = 0;
 
        if (lxc_log_fd != -1)
                return 0;
@@ -176,10 +243,38 @@ extern int lxc_log_init(const char *file, const char *priority,
        if (prefix)
                lxc_log_setprefix(prefix);
 
-       if (file)
-               return lxc_log_set_file(file);
+       if (file && strcmp(file, "none") == 0) {
+               want_lxc_log_specified = 1;
+               return 0;
+       }
 
-       return 0;
+       if (!file) {
+               tmpfile = build_log_path(name);
+               if (!tmpfile) {
+                       ERROR("could not build log path");
+                       return -1;
+               }
+       } else {
+               want_lxc_log_specified = 1;
+       }
+
+       ret = do_lxc_log_set_file(tmpfile ? tmpfile : file, !want_lxc_log_specified);
+
+       if (want_lxc_log_specified)
+               lxc_log_specified = 1;
+       /*
+        * If !want_lxc_log_specified, that is, if the user did not request
+        * this logpath, then ignore failures and continue logging to console
+        */
+       if (!want_lxc_log_specified && ret != 0) {
+               INFO("Ignoring failure to open default logfile.");
+               ret = 0;
+       }
+
+       if (tmpfile)
+               free(tmpfile);
+
+       return ret;
 }
 
 /*
@@ -200,30 +295,50 @@ extern int lxc_log_set_level(int level)
 }
 
 char *log_fname;  // default to NULL, set in lxc_log_set_file.
-
 /*
- * This is called when we read a lxc.logfile entry in a lxc.conf file.  This
- * happens after processing command line arguments, which override the .conf
- * settings.  So only set the logfile if previously unset.
+ * This can be called:
+ *   1. when a program calls lxc_log_init with no logfile parameter (in which
+ *      case the default is used).  In this case lxc.logfile can override this.
+ *   2. when a program calls lxc_log_init with a logfile parameter.  In this
+ *     case we don't want lxc.logfile to override this.
+ *   3. When a lxc.logfile entry is found in config file.
  */
-extern int lxc_log_set_file(const char *fname)
+int do_lxc_log_set_file(const char *fname, int from_default)
 {
+       if (lxc_log_specified) {
+               INFO("lxc.logfile overriden by command line");
+               return 0;
+       }
        if (lxc_log_fd != -1) {
-               // this should've been caught at config_logfile.
-               ERROR("Race in setting logfile?");
+               // we are overriding the default.
+               close(lxc_log_fd);
+               free(log_fname);
+       }
+
+#if USE_CONFIGPATH_LOGS
+       // we don't build_dir for the default if the default is
+       // i.e. /var/lib/lxc/$container/$container.log
+       if (!from_default)
+#endif
+       if (build_dir(fname)) {
+               ERROR("failed to create dir for log file \"%s\" : %s", fname,
+                     strerror(errno));
                return -1;
        }
 
        lxc_log_fd = log_open(fname);
-       if (lxc_log_fd == -1) {
-               ERROR("failed to open log file %s\n", fname);
+       if (lxc_log_fd == -1)
                return -1;
-       }
 
        log_fname = strdup(fname);
        return 0;
 }
 
+extern int lxc_log_set_file(const char *fname)
+{
+       return do_lxc_log_set_file(fname, 0);
+}
+
 extern int lxc_log_get_level(void)
 {
        if (!lxc_loglevel_specified)

diff --git a/src/lxc/log.h b/src/lxc/log.h
index a9260f22a48cf84ac989ca1a93bd5e4e6dfca2c1..771995865428643b6658eb6629d34718f8f07c0e 100644 (file)
--- a/src/lxc/log.h
+++ b/src/lxc/log.h
@@ -287,8 +287,8 @@ extern struct lxc_log_category lxc_log_category_lxc;
 
 extern int lxc_log_fd;
 
-extern int lxc_log_init(const char *file, const char *priority,
-                       const char *prefix, int quiet);
+extern int lxc_log_init(const char *name, const char *file,
+                       const char *priority, const char *prefix, int quiet);
 
 extern void lxc_log_setprefix(const char *a_prefix);
 extern int lxc_log_set_level(int level);

diff --git a/src/lxc/lxc_attach.c b/src/lxc/lxc_attach.c
index 851a37aaa11be101daad9255f652596ad57d4ca3..437a40080defa358ea8ab02c670e3709b4a94379 100644 (file)
--- a/src/lxc/lxc_attach.c
+++ b/src/lxc/lxc_attach.c
@@ -139,7 +139,7 @@ int main(int argc, char *argv[])
        if (ret)
                return ret;
 
-       ret = lxc_log_init(my_args.log_file, my_args.log_priority,
+       ret = lxc_log_init(my_args.name, my_args.log_file, my_args.log_priority,
                           my_args.progname, my_args.quiet);
        if (ret)
                return ret;

diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index 97769a5a266cce95559db69f018dbf5eba138822..970391ba2785e5bd212a329975dfeb0a555b8c81 100644 (file)
--- a/src/lxc/lxc_cgroup.c
+++ b/src/lxc/lxc_cgroup.c
@@ -68,7 +68,7 @@ int main(int argc, char *argv[])
        if (lxc_arguments_parse(&my_args, argc, argv))
                return -1;
 
-       if (lxc_log_init(my_args.log_file, my_args.log_priority,
+       if (lxc_log_init(my_args.name, my_args.log_file, my_args.log_priority,
                         my_args.progname, my_args.quiet))
                return -1;
 

diff --git a/src/lxc/lxc_checkpoint.c b/src/lxc/lxc_checkpoint.c
index 76f6709f299c9b0beb9f4cea9d2b3c60bd16b8ec..a15175015fd6cd99ee96a67792920f77a6d1e6a1 100644 (file)
--- a/src/lxc/lxc_checkpoint.c
+++ b/src/lxc/lxc_checkpoint.c
@@ -115,7 +115,7 @@ int main(int argc, char *argv[])
        if (ret)
                return ret;
 
-       ret = lxc_log_init(my_args.log_file, my_args.log_priority,
+       ret = lxc_log_init(my_args.name, my_args.log_file, my_args.log_priority,
                           my_args.progname, my_args.quiet);
        if (ret)
                return ret;

diff --git a/src/lxc/lxc_console.c b/src/lxc/lxc_console.c
index d09c6882ea650daabb91f950e44a1e71a32f18b3..c263d0f81bc7b4468cdd8495e1ecce21147c550e 100644 (file)
--- a/src/lxc/lxc_console.c
+++ b/src/lxc/lxc_console.c
@@ -187,7 +187,7 @@ int main(int argc, char *argv[])
        if (err)
                return -1;
 
-       err = lxc_log_init(my_args.log_file, my_args.log_priority,
+       err = lxc_log_init(my_args.name, my_args.log_file, my_args.log_priority,
                           my_args.progname, my_args.quiet);
        if (err)
                return -1;

diff --git a/src/lxc/lxc_execute.c b/src/lxc/lxc_execute.c
index 1eb25a7da879e53fee8d32f82949710e947a70e8..9377f4f31156f7032f41c7a2df19030c084d8934 100644 (file)
--- a/src/lxc/lxc_execute.c
+++ b/src/lxc/lxc_execute.c
@@ -99,7 +99,7 @@ int main(int argc, char *argv[])
        if (lxc_arguments_parse(&my_args, argc, argv))
                return -1;
 
-       if (lxc_log_init(my_args.log_file, my_args.log_priority,
+       if (lxc_log_init(my_args.name, my_args.log_file, my_args.log_priority,
                         my_args.progname, my_args.quiet))
                return -1;
 

diff --git a/src/lxc/lxc_freeze.c b/src/lxc/lxc_freeze.c
index 770d923ffaca979a98faa904f0c503f0b1e90cb0..4da56548435b68e750851c170b44968660bb9040 100644 (file)
--- a/src/lxc/lxc_freeze.c
+++ b/src/lxc/lxc_freeze.c
@@ -54,7 +54,7 @@ int main(int argc, char *argv[])
        if (lxc_arguments_parse(&my_args, argc, argv))
                return -1;
 
-       if (lxc_log_init(my_args.log_file, my_args.log_priority,
+       if (lxc_log_init(my_args.name, my_args.log_file, my_args.log_priority,
                         my_args.progname, my_args.quiet))
                return -1;
 

diff --git a/src/lxc/lxc_info.c b/src/lxc/lxc_info.c
index 1a1cc22df36550cf16dcb3661b1a76605ff89229..48c1370de32a7573dcd31d16b2f9803381608213 100644 (file)
--- a/src/lxc/lxc_info.c
+++ b/src/lxc/lxc_info.c
@@ -79,7 +79,7 @@ int main(int argc, char *argv[])
        if (ret)
                return 1;
 
-       if (lxc_log_init(my_args.log_file, my_args.log_priority,
+       if (lxc_log_init(my_args.name, my_args.log_file, my_args.log_priority,
                         my_args.progname, my_args.quiet))
                return 1;
 

diff --git a/src/lxc/lxc_init.c b/src/lxc/lxc_init.c
index 2263cd16f1c1a1d9b6d07c3e2245bf11c8e69264..90e1ad6618f6d5c025279b2a71c98bd36c3b15df 100644 (file)
--- a/src/lxc/lxc_init.c
+++ b/src/lxc/lxc_init.c
@@ -79,7 +79,7 @@ int main(int argc, char *argv[])
        if (lxc_caps_init())
                exit(err);
 
-       if (lxc_log_init(NULL, 0, basename(argv[0]), quiet))
+       if (lxc_log_init(NULL, "none", 0, basename(argv[0]), quiet))
                exit(err);
 
        if (!argv[optind]) {

diff --git a/src/lxc/lxc_kill.c b/src/lxc/lxc_kill.c
index 3d996a5c7fc292b8bb8868c8e473fde2bb6fc3d9..f9bfe34b60e812b454737a10b22962ee9b3aad4a 100644 (file)
--- a/src/lxc/lxc_kill.c
+++ b/src/lxc/lxc_kill.c
@@ -61,7 +61,7 @@ int main(int argc, char *argv[], char *envp[])
        if (ret)
                return ret;
 
-       ret = lxc_log_init(my_args.log_file, my_args.log_priority,
+       ret = lxc_log_init(my_args.name, my_args.log_file, my_args.log_priority,
                           my_args.progname, my_args.quiet);
        if (ret)
                return ret;

diff --git a/src/lxc/lxc_monitor.c b/src/lxc/lxc_monitor.c
index 4bd6ab0156340a27e04d435ff8b779fe75fd7450..2b8b0a0864e815a36dd116d47cb0d21b05d3f4ed 100644 (file)
--- a/src/lxc/lxc_monitor.c
+++ b/src/lxc/lxc_monitor.c
@@ -65,7 +65,7 @@ int main(int argc, char *argv[])
        if (lxc_arguments_parse(&my_args, argc, argv))
                return -1;
 
-       if (lxc_log_init(my_args.log_file, my_args.log_priority,
+       if (lxc_log_init(my_args.name, my_args.log_file, my_args.log_priority,
                         my_args.progname, my_args.quiet))
                return -1;
 

diff --git a/src/lxc/lxc_restart.c b/src/lxc/lxc_restart.c
index 75486821cb34161da3248c6790b348425cded036..1cf9462e9395aee690e9866d6bb241fb50a791b4 100644 (file)
--- a/src/lxc/lxc_restart.c
+++ b/src/lxc/lxc_restart.c
@@ -122,7 +122,7 @@ int main(int argc, char *argv[])
        if (lxc_arguments_parse(&my_args, argc, argv))
                return -1;
 
-       if (lxc_log_init(my_args.log_file, my_args.log_priority,
+       if (lxc_log_init(my_args.name, my_args.log_file, my_args.log_priority,
                         my_args.progname, my_args.quiet))
                return -1;
 

diff --git a/src/lxc/lxc_start.c b/src/lxc/lxc_start.c
index a97dccaa86cbb78245d472daa90bd12673781f06..b64acffe3a52af29a124940e4668bb7d314308db 100644 (file)
--- a/src/lxc/lxc_start.c
+++ b/src/lxc/lxc_start.c
@@ -164,7 +164,7 @@ int main(int argc, char *argv[])
        else
                args = my_args.argv;
 
-       if (lxc_log_init(my_args.log_file, my_args.log_priority,
+       if (lxc_log_init(my_args.name, my_args.log_file, my_args.log_priority,
                         my_args.progname, my_args.quiet))
                return err;
 

diff --git a/src/lxc/lxc_stop.c b/src/lxc/lxc_stop.c
index 639b7500be35a9e88c41fadce45a4198740f1d00..749d78a65b97629d74e93765ac4caed9943ef79c 100644 (file)
--- a/src/lxc/lxc_stop.c
+++ b/src/lxc/lxc_stop.c
@@ -53,7 +53,7 @@ int main(int argc, char *argv[])
        if (lxc_arguments_parse(&my_args, argc, argv))
                return -1;
 
-       if (lxc_log_init(my_args.log_file, my_args.log_priority,
+       if (lxc_log_init(my_args.name, my_args.log_file, my_args.log_priority,
                         my_args.progname, my_args.quiet))
                return -1;
 

diff --git a/src/lxc/lxc_unfreeze.c b/src/lxc/lxc_unfreeze.c
index 22be8395b0123a2c1c699f8183aee4e7f5ef42d3..02e9a47c938fc27db78d4683285d9aefc3c701f6 100644 (file)
--- a/src/lxc/lxc_unfreeze.c
+++ b/src/lxc/lxc_unfreeze.c
@@ -53,7 +53,7 @@ int main(int argc, char *argv[])
        if (lxc_arguments_parse(&my_args, argc, argv))
                return -1;
 
-       if (lxc_log_init(my_args.log_file, my_args.log_priority,
+       if (lxc_log_init(my_args.name, my_args.log_file, my_args.log_priority,
                         my_args.progname, my_args.quiet))
                return -1;
 

diff --git a/src/lxc/lxc_wait.c b/src/lxc/lxc_wait.c
index 799225d970df75998d01162e1354c6802bebd92e..b0643c73946ad7c1018687013e5398943631f773 100644 (file)
--- a/src/lxc/lxc_wait.c
+++ b/src/lxc/lxc_wait.c
@@ -83,7 +83,7 @@ int main(int argc, char *argv[])
        if (lxc_arguments_parse(&my_args, argc, argv))
                return -1;
 
-       if (lxc_log_init(my_args.log_file, my_args.log_priority,
+       if (lxc_log_init(my_args.name, my_args.log_file, my_args.log_priority,
                         my_args.progname, my_args.quiet))
                return -1;
 

diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
index 5919d2c574acca258cafa3e49c5dfe6679c0463f..502a7a73af0e80ab525eb84013db974002d9cd69 100644 (file)
--- a/src/lxc/lxccontainer.c
+++ b/src/lxc/lxccontainer.c
@@ -976,7 +976,7 @@ struct lxc_container *lxc_container_new(const char *name)
        c->set_cgroup_item = lxcapi_set_cgroup_item;
 
        /* we'll allow the caller to update these later */
-       if (lxc_log_init(NULL, NULL, "lxc_container", 0)) {
+       if (lxc_log_init(NULL, "none", NULL, "lxc_container", 0)) {
                fprintf(stderr, "failed to open log\n");
                goto err;
        }