Merge pull request #2495 from 2xsec/bugfix

add default log priority & cleanups

src/lxc/log.c

@@ -127,13 +127,13 @@ static int log_append_syslog(const struct lxc_log_appender *appender,
 	}
 
 	syslog(lxc_log_priority_to_syslog(event->priority),
-		"%s%s %s - %s:%s:%d - %s" ,
+	       "%s%s %s - %s:%s:%d - %s" ,
-		log_container_name ? log_container_name : "",
+	       log_container_name ? log_container_name : "",
-		log_container_name ? ":" : "",
+	       log_container_name ? ":" : "",
-		event->category,
+	       event->category,
-		event->locinfo->file, event->locinfo->func,
+	       event->locinfo->file, event->locinfo->func,
-		event->locinfo->line,
+	       event->locinfo->line,
-		msg);
+	       msg);
 	free(msg);
 
 	return 0;
@@ -156,10 +156,10 @@ static int log_append_stderr(const struct lxc_log_appender *appender,
 #endif
 
 	fprintf(stderr, "%s: %s%s", log_prefix,
-		log_container_name ? log_container_name : "",
+	        log_container_name ? log_container_name : "",
-		log_container_name ? ": " : "");
+	        log_container_name ? ": " : "");
 	fprintf(stderr, "%s: %s: %d ", event->locinfo->file,
-		event->locinfo->func, event->locinfo->line);
+	        event->locinfo->func, event->locinfo->line);
 	vfprintf(stderr, event->fmt, *event->vap);
 	fprintf(stderr, "\n");
 
@@ -167,7 +167,7 @@ static int log_append_stderr(const struct lxc_log_appender *appender,
 }
 
 /*---------------------------------------------------------------------------*/
-int lxc_unix_epoch_to_utc(char *buf, size_t bufsize, const struct timespec *time)
+static int lxc_unix_epoch_to_utc(char *buf, size_t bufsize, const struct timespec *time)
 {
 	int64_t epoch_to_days, z, era, doe, yoe, year, doy, mp, day, month,
 	    d_in_s, hours, h_in_s, minutes, seconds;
@@ -315,16 +315,15 @@ static int log_append_logfile(const struct lxc_log_appender *appender,
 		return 0;
 
 	n = snprintf(buffer, sizeof(buffer),
-			"%s%s%s %s %-8s %s - %s:%s:%d - ",
+		     "%s%s%s %s %-8s %s - %s:%s:%d - ",
-			log_prefix,
+		     log_prefix,
-			log_container_name ? " " : "",
+		     log_container_name ? " " : "",
-			log_container_name ? log_container_name : "",
+		     log_container_name ? log_container_name : "",
-			date_time,
+		     date_time,
-			lxc_log_priority_to_string(event->priority),
+		     lxc_log_priority_to_string(event->priority),
-			event->category,
+		     event->category,
-			event->locinfo->file, event->locinfo->func,
+		     event->locinfo->file, event->locinfo->func,
-			event->locinfo->line);
+		     event->locinfo->line);
-
 	if (n < 0)
 		return n;
 
@@ -493,23 +492,6 @@ static char *build_log_path(const char *name, const char *lxcpath)
 	return p;
 }
 
-void lxc_log_close(void)
-{
-	closelog();
-
-	free(log_vmname);
-	log_vmname = NULL;
-
-	if (lxc_log_fd == -1)
-		return;
-
-	close(lxc_log_fd);
-	lxc_log_fd = -1;
-
-	free(log_fname);
-	log_fname = NULL;
-}
-
 /*
  * This can be called:
  *   1. when a program calls lxc_log_init with no logfile parameter (in which
@@ -529,7 +511,7 @@ static int __lxc_log_set_file(const char *fname, int create_dirs)
 
 	if (strlen(fname) == 0) {
 		log_fname = NULL;
-		return 0;
+		return -1;
 	}
 
 #if USE_CONFIGPATH_LOGS
@@ -567,39 +549,6 @@ static int _lxc_log_set_file(const char *name, const char *lxcpath, int create_d
 	return ret;
 }
 
-int lxc_log_syslog(int facility)
-{
-	struct lxc_log_appender *appender;
-
-	openlog(log_prefix, LOG_PID, facility);
-	if (!lxc_log_category_lxc.appender) {
-		lxc_log_category_lxc.appender = &log_appender_syslog;
-		return 0;
-	}
-
-	appender = lxc_log_category_lxc.appender;
-	/* Check if syslog was already added, to avoid creating a loop */
-	while (appender) {
-		if (appender == &log_appender_syslog) {
-			/* not an error: openlog re-opened the connection */
-			return 0;
-		}
-		appender = appender->next;
-	}
-
-	appender = lxc_log_category_lxc.appender;
-	while (appender->next != NULL)
-		appender = appender->next;
-	appender->next = &log_appender_syslog;
-
-	return 0;
-}
-
-inline void lxc_log_enable_syslog(void)
-{
-	syslog_enable = 1;
-}
-
 /*
  * lxc_log_init:
  * Called from lxc front-end programs (like lxc-create, lxc-start) to
@@ -610,6 +559,9 @@ int lxc_log_init(struct lxc_log *log)
 	int ret;
 	int lxc_priority = LXC_LOG_LEVEL_ERROR;
 
+	if (!log)
+		return -1;
+
 	if (lxc_log_fd != -1) {
 		WARN("Log already initialized");
 		return 0;
@@ -623,10 +575,9 @@ int lxc_log_init(struct lxc_log *log)
 		lxc_loglevel_specified = 1;
 	}
 
-	if (!lxc_quiet_specified) {
+	if (!lxc_quiet_specified)
 		if (!log->quiet)
 			lxc_log_category_lxc.appender->next = &log_appender_stderr;
-	}
 
 	if (log->prefix)
 		lxc_log_set_prefix(log->prefix);
@@ -639,6 +590,11 @@ int lxc_log_init(struct lxc_log *log)
 			return 0;
 
 		ret = __lxc_log_set_file(log->file, 1);
+		if (ret < 0) {
+			ERROR("Failed to enable logfile");
+			return -1;
+		}
+
 		lxc_log_use_global_fd = 1;
 	} else {
 		/* if no name was specified, there nothing to do */
@@ -672,9 +628,64 @@ int lxc_log_init(struct lxc_log *log)
 		ret = 0;
 	}
 
+	if (lxc_log_fd != -1) {
+		lxc_log_category_lxc.appender = &log_appender_logfile;
+		lxc_log_category_lxc.appender->next = &log_appender_stderr;
+	}
+
 	return ret;
 }
 
+void lxc_log_close(void)
+{
+	closelog();
+
+	free(log_vmname);
+	log_vmname = NULL;
+
+	if (lxc_log_fd == -1)
+		return;
+
+	close(lxc_log_fd);
+	lxc_log_fd = -1;
+
+	free(log_fname);
+	log_fname = NULL;
+}
+
+int lxc_log_syslog(int facility)
+{
+	struct lxc_log_appender *appender;
+
+	openlog(log_prefix, LOG_PID, facility);
+	if (!lxc_log_category_lxc.appender) {
+		lxc_log_category_lxc.appender = &log_appender_syslog;
+		return 0;
+	}
+
+	appender = lxc_log_category_lxc.appender;
+	/* Check if syslog was already added, to avoid creating a loop */
+	while (appender) {
+		if (appender == &log_appender_syslog) {
+			/* not an error: openlog re-opened the connection */
+			return 0;
+		}
+		appender = appender->next;
+	}
+
+	appender = lxc_log_category_lxc.appender;
+	while (appender->next != NULL)
+		appender = appender->next;
+	appender->next = &log_appender_syslog;
+
+	return 0;
+}
+
+inline void lxc_log_enable_syslog(void)
+{
+	syslog_enable = 1;
+}
+
 /*
  * This is called when we read a lxc.log.level entry in a lxc.conf file.  This
  * happens after processing command line arguments, which override the .conf

src/lxc/log.h

@@ -431,14 +431,14 @@ ATTR_UNUSED static inline void LXC_##LEVEL(struct lxc_log_locinfo* locinfo,	\
 
 extern int lxc_log_fd;
 
-extern int lxc_log_set_file(int *fd, const char *fname);
 extern int lxc_log_syslog(int facility);
 extern void lxc_log_enable_syslog(void);
 extern int lxc_log_set_level(int *dest, int level);
-extern void lxc_log_set_prefix(const char *prefix);
-extern const char *lxc_log_get_file(void);
 extern int lxc_log_get_level(void);
 extern bool lxc_log_has_valid_level(void);
+extern int lxc_log_set_file(int *fd, const char *fname);
+extern const char *lxc_log_get_file(void);
+extern void lxc_log_set_prefix(const char *prefix);
 extern const char *lxc_log_get_prefix(void);
 extern void lxc_log_options_no_override();
 #endif

src/lxc/tools/arguments.c

@@ -192,6 +192,7 @@ extern int lxc_arguments_parse(struct lxc_arguments *args, int argc,
 			       char *const argv[])
 {
 	int ret = 0;
+	bool logfile = false;
 	char shortopts[256];
 
 	ret = build_shortopts(args->options, shortopts, sizeof(shortopts));
@@ -215,9 +216,14 @@ extern int lxc_arguments_parse(struct lxc_arguments *args, int argc,
 			break;
 		case 'o':
 			args->log_file = optarg;
+			logfile = true;
 			break;
 		case 'l':
 			args->log_priority = optarg;
+			if (!logfile &&
+			    args->log_file &&
+			    strcmp(args->log_file, "none") == 0)
+			    args->log_file = NULL;
 			break;
 		case 'q':
 			args->quiet = 1;

src/lxc/tools/lxc_unshare.c

@@ -102,6 +102,8 @@ Options :\n\
 	.options      = my_longopts,
 	.parser       = my_parser,
 	.checker      = NULL,
+	.log_priority = "ERROR",
+	.log_file     = "none",
 	.daemonize    = 0,
 	.pidfile      = NULL,
 };
@@ -325,19 +327,16 @@ int main(int argc, char *argv[])
 	if (lxc_arguments_parse(&my_args, argc, argv))
 		exit(EXIT_FAILURE);
 
-	/* Only create log if explicitly instructed */
+	log.name = my_args.name;
-	if (my_args.log_file || my_args.log_priority) {
+	log.file = my_args.log_file;
-		log.name = my_args.name;
+	log.level = my_args.log_priority;
-		log.file = my_args.log_file;
+	log.prefix = my_args.progname;
-		log.level = my_args.log_priority;
+	log.quiet = my_args.quiet;
-		log.prefix = my_args.progname;
+	log.lxcpath = my_args.lxcpath[0];
-		log.quiet = my_args.quiet;
-		log.lxcpath = my_args.lxcpath[0];
 
-		if (lxc_log_init(&log)) {
+	if (lxc_log_init(&log)) {
-			free_ifname_list();
+		free_ifname_list();
-			exit(EXIT_FAILURE);
+		exit(EXIT_FAILURE);
-		}
 	}
 
 	if (!*my_args.argv) {