From: Stefano Brivio <sbrivio@redhat.com>
To: passt-dev@passt.top
Cc: Yalan Zhang <yalzhang@redhat.com>
Subject: [PATCH 3/6] log, passt: Always print to stderr before initialisation is complete
Date: Mon, 17 Jun 2024 14:03:16 +0200 [thread overview]
Message-ID: <20240617120319.1206857-4-sbrivio@redhat.com> (raw)
In-Reply-To: <20240617120319.1206857-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. Then, set LOG_PERROR only as we set this internal
flag, to make sure we don't duplicate messages.
Reported-by: Yalan Zhang <yalzhang@redhat.com>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
log.c | 4 +++-
log.h | 1 +
passt.1 | 3 ++-
passt.c | 17 ++++++++++-------
4 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/log.c b/log.c
index 3b5a1c6..939bb93 100644
--- a/log.c
+++ b/log.c
@@ -49,6 +49,7 @@ int log_trace; /* --trace mode enabled */
void vlogmsg(int pri, const char *format, va_list ap)
{
bool debug_print = (log_mask & LOG_MASK(LOG_DEBUG)) && log_file == -1;
+ bool before_daemon = !(log_flags & LOG_FLAG_DAEMON_READY);
bool early_print = !(log_flags & LOG_FLAG_CONF_PARSED);
struct timespec tp;
@@ -71,7 +72,8 @@ void vlogmsg(int pri, const char *format, va_list ap)
va_end(ap2);
}
- if (debug_print || (early_print && !(log_opt & LOG_PERROR))) {
+ if (debug_print || early_print ||
+ (before_daemon && (log_mask & LOG_MASK(LOG_PRI(pri))))) {
(void)vfprintf(stderr, format, ap);
if (format[strlen(format)] != '\n')
fprintf(stderr, "\n");
diff --git a/log.h b/log.h
index 6a3224a..680baab 100644
--- a/log.h
+++ b/log.h
@@ -10,6 +10,7 @@
#include <syslog.h>
#define LOG_FLAG_CONF_PARSED BIT(0) /* We already parsed logging options */
+#define LOG_FLAG_DAEMON_READY BIT(1) /* Daemonised, or ready in foreground */
#define LOGFILE_SIZE_DEFAULT (1024 * 1024UL)
#define LOGFILE_CUT_RATIO 30 /* When full, cut ~30% size */
diff --git a/passt.1 b/passt.1
index 3a23a43..31e528e 100644
--- a/passt.1
+++ b/passt.1
@@ -99,7 +99,8 @@ terminal, and to both system logger and standard error otherwise.
.TP
.BR \-l ", " \-\-log-file " " \fIPATH\fR
-Log to file \fIPATH\fR, not to standard error, and not to the system logger.
+Log to file \fIPATH\fR, not to standard error (once initialisation is complete),
+and not to the system logger.
.TP
.BR \-\-log-size " " \fISIZE\fR
diff --git a/passt.c b/passt.c
index aa9648a..fa86164 100644
--- a/passt.c
+++ b/passt.c
@@ -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(log_name = "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(log_name = "passt", 0, LOG_DAEMON);
c.mode = MODE_PASST;
} else {
@@ -302,13 +302,16 @@ int main(int argc, char **argv)
if (isolate_prefork(&c))
die("Failed to sandbox process, exiting");
- if (!c.foreground || (!c.force_stderr && !isatty(fileno(stderr))))
- __openlog(log_name, 0, LOG_DAEMON);
-
- if (!c.foreground)
+ if (!c.foreground) {
__daemon(c.pidfile_fd, devnull_fd);
- else
+ } else {
+ if (c.force_stderr || isatty(fileno(stderr)))
+ __openlog(log_name, LOG_PERROR, LOG_DAEMON);
+
pidfile_write(c.pidfile_fd, getpid());
+ }
+
+ log_set_flag(LOG_FLAG_DAEMON_READY);
if (pasta_child_pid)
kill(pasta_child_pid, SIGUSR1);
--
@@ -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(log_name = "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(log_name = "passt", 0, LOG_DAEMON);
c.mode = MODE_PASST;
} else {
@@ -302,13 +302,16 @@ int main(int argc, char **argv)
if (isolate_prefork(&c))
die("Failed to sandbox process, exiting");
- if (!c.foreground || (!c.force_stderr && !isatty(fileno(stderr))))
- __openlog(log_name, 0, LOG_DAEMON);
-
- if (!c.foreground)
+ if (!c.foreground) {
__daemon(c.pidfile_fd, devnull_fd);
- else
+ } else {
+ if (c.force_stderr || isatty(fileno(stderr)))
+ __openlog(log_name, LOG_PERROR, LOG_DAEMON);
+
pidfile_write(c.pidfile_fd, getpid());
+ }
+
+ log_set_flag(LOG_FLAG_DAEMON_READY);
if (pasta_child_pid)
kill(pasta_child_pid, SIGUSR1);
--
2.43.0
next prev parent reply other threads:[~2024-06-17 12:03 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-17 12:03 [PATCH 0/6] Fixes for early logging/prints and related cleanups Stefano Brivio
2024-06-17 12:03 ` [PATCH 1/6] conf, passt: Don't try to log to stderr after we close it Stefano Brivio
2024-06-18 0:36 ` David Gibson
2024-06-18 6:00 ` Stefano Brivio
2024-06-19 2:06 ` David Gibson
2024-06-19 8:13 ` Stefano Brivio
2024-06-20 0:12 ` David Gibson
2024-06-17 12:03 ` [PATCH 2/6] conf, log: Introduce internal log flags, instead of abusing log levels Stefano Brivio
2024-06-18 0:39 ` David Gibson
2024-06-18 6:01 ` Stefano Brivio
2024-06-17 12:03 ` Stefano Brivio [this message]
2024-06-18 0:44 ` [PATCH 3/6] log, passt: Always print to stderr before initialisation is complete David Gibson
2024-06-18 6:01 ` Stefano Brivio
2024-06-19 2:10 ` David Gibson
2024-06-19 8:17 ` Stefano Brivio
2024-06-20 0:12 ` David Gibson
2024-06-17 12:03 ` [PATCH 4/6] log: Add _perror() logging function variants Stefano Brivio
2024-06-18 0:46 ` David Gibson
2024-06-18 6:02 ` Stefano Brivio
2024-06-19 2:11 ` David Gibson
2024-06-19 8:25 ` Stefano Brivio
2024-06-20 0:13 ` David Gibson
2024-06-17 12:03 ` [PATCH 5/6] treewide: Replace perror() calls with calls to logging functions Stefano Brivio
2024-06-18 0:50 ` David Gibson
2024-06-17 12:03 ` [PATCH 6/6] treewide: Replace strerror() calls Stefano Brivio
2024-06-18 0:51 ` David Gibson
2024-06-18 6:02 ` 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=20240617120319.1206857-4-sbrivio@redhat.com \
--to=sbrivio@redhat.com \
--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).