From: Serge Hallyn Date: Thu, 24 Jan 2013 18:04:54 +0000 (-0600) Subject: use a default per-container logfile X-Git-Tag: lxc-0.9.0.alpha3~1^2~17 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5e1e7aaf1e244405b813c0f9e20db2fc9e7f4d10;p=thirdparty%2Flxc.git 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 Acked-by: Stéphane Graber --- diff --git a/configure.ac b/configure.ac index d1f5ad950..e56724571 100644 --- 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 b55a20c54..2fb829137 100644 --- 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 67a989ed4..d350f01df 100644 --- 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 d2a90de7a..ff512e38b 100644 --- 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 a9260f22a..771995865 100644 --- 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 851a37aaa..437a40080 100644 --- 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 97769a5a2..970391ba2 100644 --- 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 76f6709f2..a15175015 100644 --- 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 d09c6882e..c263d0f81 100644 --- 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 1eb25a7da..9377f4f31 100644 --- 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 770d923ff..4da565484 100644 --- 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 1a1cc22df..48c1370de 100644 --- 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 2263cd16f..90e1ad661 100644 --- 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 3d996a5c7..f9bfe34b6 100644 --- 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 4bd6ab015..2b8b0a086 100644 --- 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 75486821c..1cf9462e9 100644 --- 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 a97dccaa8..b64acffe3 100644 --- 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 639b7500b..749d78a65 100644 --- 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 22be8395b..02e9a47c9 100644 --- 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 799225d97..b0643c739 100644 --- 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 5919d2c57..502a7a73a 100644 --- 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; }