[lxc-devel] [PATCH] use a default per-container logfile (v2)
Serge Hallyn
serge.hallyn at canonical.com
Thu Jan 24 18:04:54 UTC 2013
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 at ubuntu.com>
---
configure.ac | 21 +++++++
src/lxc/Makefile.am | 7 ++-
src/lxc/confile.c | 5 --
src/lxc/log.c | 145 ++++++++++++++++++++++++++++++++++++++++++-----
src/lxc/log.h | 4 +-
src/lxc/lxc_attach.c | 2 +-
src/lxc/lxc_cgroup.c | 2 +-
src/lxc/lxc_checkpoint.c | 2 +-
src/lxc/lxc_console.c | 2 +-
src/lxc/lxc_execute.c | 2 +-
src/lxc/lxc_freeze.c | 2 +-
src/lxc/lxc_info.c | 2 +-
src/lxc/lxc_init.c | 2 +-
src/lxc/lxc_kill.c | 2 +-
src/lxc/lxc_monitor.c | 2 +-
src/lxc/lxc_restart.c | 2 +-
src/lxc/lxc_start.c | 2 +-
src/lxc/lxc_stop.c | 2 +-
src/lxc/lxc_unfreeze.c | 2 +-
src/lxc/lxc_wait.c | 2 +-
src/lxc/lxccontainer.c | 2 +-
21 files changed, 175 insertions(+), 39 deletions(-)
diff --git a/configure.ac b/configure.ac
index d1f5ad9..e567245 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 b55a20c..2fb8291 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 67a989e..d350f01 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 d2a90de..ff512e3 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 a9260f2..7719958 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 851a37a..437a400 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 97769a5..970391b 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 76f6709..a151750 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 d09c688..c263d0f 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 1eb25a7..9377f4f 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 770d923..4da5654 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 1a1cc22..48c1370 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 2263cd1..90e1ad6 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 3d996a5..f9bfe34 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 4bd6ab0..2b8b0a0 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 7548682..1cf9462 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 a97dcca..b64acff 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 639b750..749d78a 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 22be839..02e9a47 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 799225d..b0643c7 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 5919d2c..502a7a7 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;
}
--
1.8.0
More information about the lxc-devel
mailing list