public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: Stefano Brivio <sbrivio@redhat.com>
To: passt-dev@passt.top
Cc: Yalan Zhang <yalzhang@redhat.com>,
	David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH v4 4/8] log, passt: Always print to stderr before initialisation is complete
Date: Thu, 20 Jun 2024 06:07:56 +0200	[thread overview]
Message-ID: <20240620040800.3065684-5-sbrivio@redhat.com> (raw)
In-Reply-To: <20240620040800.3065684-1-sbrivio@redhat.com>

After commit 15001b39ef1d ("conf: set the log level much earlier"), we
had a phase during initialisation when messages wouldn't be printed to
standard error anymore.

Commit f67238aa864d ("passt, log: Call __openlog() earlier, log to
stderr until we detach") fixed that, but only for the case where no
log files are given.

If a log file is configured, vlogmsg() will not call passt_vsyslog(),
but during initialisation, LOG_PERROR is set, so to avoid duplicated
prints (which would result from passt_vsyslog() printing to stderr),
we don't call fprintf() from vlogmsg() either.

This is getting a bit too complicated. Instead of abusing LOG_PERROR,
define an internal logging flag that clearly represents that we're not
done with the initialisation phase yet.

If this flag is not set, make sure we always print to stderr, if the
log mask matches.

Reported-by: Yalan Zhang <yalzhang@redhat.com>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 log.c   | 17 ++++++++---------
 log.h   |  1 +
 passt.c | 15 +++++++--------
 3 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/log.c b/log.c
index 05b7f80..025129a 100644
--- a/log.c
+++ b/log.c
@@ -33,7 +33,6 @@
 static int	log_sock = -1;		/* Optional socket to system logger */
 static char	log_ident[BUFSIZ];	/* Identifier string for openlog() */
 static int	log_mask;		/* Current log priority mask */
-static int	log_opt;		/* Options for openlog() */
 
 static int	log_file = -1;		/* Optional log file descriptor */
 static size_t	log_size;		/* Maximum log file size in bytes */
@@ -45,6 +44,7 @@ static time_t	log_start;		/* Start timestamp */
 
 int		log_trace;		/* --trace mode enabled */
 bool		log_conf_parsed;	/* Logging options already parsed */
+bool		log_daemonised;		/* Daemonised, standard error is gone */
 
 void vlogmsg(int pri, const char *format, va_list ap)
 {
@@ -70,7 +70,8 @@ void vlogmsg(int pri, const char *format, va_list ap)
 		va_end(ap2);
 	}
 
-	if (debug_print || (!log_conf_parsed && !(log_opt & LOG_PERROR))) {
+	if (debug_print || !log_conf_parsed ||
+	    (!log_daemonised && (log_mask & LOG_MASK(LOG_PRI(pri))))) {
 		(void)vfprintf(stderr, format, ap);
 		if (format[strlen(format)] != '\n')
 			fprintf(stderr, "\n");
@@ -108,13 +109,15 @@ void trace_init(int enable)
 /**
  * __openlog() - Non-optional openlog() implementation, for custom vsyslog()
  * @ident:	openlog() identity (program name)
- * @option:	openlog() options
+ * @option:	openlog() options, unused
  * @facility:	openlog() facility (LOG_DAEMON)
  */
 void __openlog(const char *ident, int option, int facility)
 {
 	struct timespec tp;
 
+	(void)option;
+
 	clock_gettime(CLOCK_REALTIME, &tp);
 	log_start = tp.tv_sec;
 
@@ -135,7 +138,6 @@ void __openlog(const char *ident, int option, int facility)
 
 	log_mask |= facility;
 	strncpy(log_ident, ident, sizeof(log_ident) - 1);
-	log_opt = option;
 }
 
 /**
@@ -156,20 +158,17 @@ void __setlogmask(int mask)
  */
 void passt_vsyslog(int pri, const char *format, va_list ap)
 {
-	int prefix_len, n;
 	char buf[BUFSIZ];
+	int n;
 
 	/* Send without timestamp, the system logger should add it */
-	n = prefix_len = snprintf(buf, BUFSIZ, "<%i> %s: ", pri, log_ident);
+	n = snprintf(buf, BUFSIZ, "<%i> %s: ", pri, log_ident);
 
 	n += vsnprintf(buf + n, BUFSIZ - n, format, ap);
 
 	if (format[strlen(format)] != '\n')
 		n += snprintf(buf + n, BUFSIZ - n, "\n");
 
-	if (log_opt & LOG_PERROR)
-		fprintf(stderr, "%s", buf + prefix_len);
-
 	if (log_sock >= 0 && send(log_sock, buf, n, 0) != n)
 		fprintf(stderr, "Failed to send %i bytes to syslog\n", n);
 }
diff --git a/log.h b/log.h
index 3dab284..7bfd92d 100644
--- a/log.h
+++ b/log.h
@@ -30,6 +30,7 @@ void logmsg(int pri, const char *format, ...)
 
 extern int log_trace;
 extern bool log_conf_parsed;
+extern bool log_daemonised;
 
 void trace_init(int enable);
 #define trace(...)							\
diff --git a/passt.c b/passt.c
index 19ecd68..e3c1d50 100644
--- a/passt.c
+++ b/passt.c
@@ -200,8 +200,8 @@ void exit_handler(int signal)
 int main(int argc, char **argv)
 {
 	struct epoll_event events[EPOLL_EVENTS];
-	char *log_name, argv0[PATH_MAX], *name;
 	int nfds, i, devnull_fd = -1;
+	char argv0[PATH_MAX], *name;
 	struct ctx c = { 0 };
 	struct rlimit limit;
 	struct timespec now;
@@ -225,7 +225,7 @@ int main(int argc, char **argv)
 	strncpy(argv0, argv[0], PATH_MAX - 1);
 	name = basename(argv0);
 	if (strstr(name, "pasta")) {
-		__openlog(log_name = "pasta", LOG_PERROR, LOG_DAEMON);
+		__openlog("pasta", 0, LOG_DAEMON);
 
 		sa.sa_handler = pasta_child_handler;
 		if (sigaction(SIGCHLD, &sa, NULL)) {
@@ -240,7 +240,7 @@ int main(int argc, char **argv)
 
 		c.mode = MODE_PASTA;
 	} else if (strstr(name, "passt")) {
-		__openlog(log_name = "passt", LOG_PERROR, LOG_DAEMON);
+		__openlog("passt", 0, LOG_DAEMON);
 
 		c.mode = MODE_PASST;
 	} else {
@@ -302,13 +302,12 @@ int main(int argc, char **argv)
 	if (isolate_prefork(&c))
 		die("Failed to sandbox process, exiting");
 
-	if (!c.foreground)
-		__openlog(log_name, 0, LOG_DAEMON);
-
-	if (!c.foreground)
+	if (!c.foreground) {
 		__daemon(c.pidfile_fd, devnull_fd);
-	else
+		log_daemonised = true;
+	} else {
 		pidfile_write(c.pidfile_fd, getpid());
+	}
 
 	if (pasta_child_pid)
 		kill(pasta_child_pid, SIGUSR1);
-- 
@@ -200,8 +200,8 @@ void exit_handler(int signal)
 int main(int argc, char **argv)
 {
 	struct epoll_event events[EPOLL_EVENTS];
-	char *log_name, argv0[PATH_MAX], *name;
 	int nfds, i, devnull_fd = -1;
+	char argv0[PATH_MAX], *name;
 	struct ctx c = { 0 };
 	struct rlimit limit;
 	struct timespec now;
@@ -225,7 +225,7 @@ int main(int argc, char **argv)
 	strncpy(argv0, argv[0], PATH_MAX - 1);
 	name = basename(argv0);
 	if (strstr(name, "pasta")) {
-		__openlog(log_name = "pasta", LOG_PERROR, LOG_DAEMON);
+		__openlog("pasta", 0, LOG_DAEMON);
 
 		sa.sa_handler = pasta_child_handler;
 		if (sigaction(SIGCHLD, &sa, NULL)) {
@@ -240,7 +240,7 @@ int main(int argc, char **argv)
 
 		c.mode = MODE_PASTA;
 	} else if (strstr(name, "passt")) {
-		__openlog(log_name = "passt", LOG_PERROR, LOG_DAEMON);
+		__openlog("passt", 0, LOG_DAEMON);
 
 		c.mode = MODE_PASST;
 	} else {
@@ -302,13 +302,12 @@ int main(int argc, char **argv)
 	if (isolate_prefork(&c))
 		die("Failed to sandbox process, exiting");
 
-	if (!c.foreground)
-		__openlog(log_name, 0, LOG_DAEMON);
-
-	if (!c.foreground)
+	if (!c.foreground) {
 		__daemon(c.pidfile_fd, devnull_fd);
-	else
+		log_daemonised = true;
+	} else {
 		pidfile_write(c.pidfile_fd, getpid());
+	}
 
 	if (pasta_child_pid)
 		kill(pasta_child_pid, SIGUSR1);
-- 
2.43.0


  parent reply	other threads:[~2024-06-20  4:08 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-20  4:07 [PATCH v4 0/8] Fixes for early logging/prints and related cleanups Stefano Brivio
2024-06-20  4:07 ` [PATCH v4 1/8] conf, passt: Don't try to log to stderr after we close it Stefano Brivio
2024-06-20  4:07 ` [PATCH v4 2/8] conf, passt: Make --stderr do nothing, and deprecate it Stefano Brivio
2024-06-20  4:07 ` [PATCH v4 3/8] conf, log: Instead of abusing log levels, add log_conf_parsed flag Stefano Brivio
2024-06-20  4:07 ` Stefano Brivio [this message]
2024-06-20  4:34   ` [PATCH v4 4/8] log, passt: Always print to stderr before initialisation is complete David Gibson
2024-06-20  4:07 ` [PATCH v4 5/8] log: Add _perror() logging function variants Stefano Brivio
2024-06-20  4:07 ` [PATCH v4 6/8] treewide: Replace perror() calls with calls to logging functions Stefano Brivio
2024-06-20  4:07 ` [PATCH v4 7/8] treewide: Replace strerror() calls Stefano Brivio
2024-06-20  4:08 ` [PATCH v4 8/8] conf, passt: Don't call __openlog() if a log file is used Stefano Brivio

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240620040800.3065684-5-sbrivio@redhat.com \
    --to=sbrivio@redhat.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=passt-dev@passt.top \
    --cc=yalzhang@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).