From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id B003B5A004E for ; Tue, 18 Jun 2024 02:51:59 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1718671915; bh=SzPYIBbtDXsIEogLyKZKyHnHUPCKTBpOaELKm0HbPf8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=qpDrEFkeoopzREsc4vevASlS4SdluluqYMoOcgqdIkagPdssmR9JDY/PkfmfrAPXr g/LkBmTGuKaavNGvvlHjqjVZhxcFv4lkfCYI33L+7LrvGDgHD2FUSQJnbMCZg4p8jd I1nt1n4qWmi2pKIIh8BScVChgGgAsiMaRdsceI1GTP9QWPow7ncUhXvErJ8V624Vqi n8Nhs7NeK+g/BnC3QCB1aibto7WCOq/M9ZhGK0DmmFaFFE73sZXAJ3gwlw3Vp0Dcg2 5Ipygzb8Quxymlqd0NiAvTSu3QReadBjxB7dYPGMpR9ycK1A8eCqonnXKZDEWPOGgJ fND16G8y47GfA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4W37Vz205Qz4wyl; Tue, 18 Jun 2024 10:51:55 +1000 (AEST) Date: Tue, 18 Jun 2024 10:44:20 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 3/6] log, passt: Always print to stderr before initialisation is complete Message-ID: References: <20240617120319.1206857-1-sbrivio@redhat.com> <20240617120319.1206857-4-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="yRWweczhr2xUeyhW" Content-Disposition: inline In-Reply-To: <20240617120319.1206857-4-sbrivio@redhat.com> Message-ID-Hash: 7VYOOKQKT6MPNXAC5CWUD4I5H6SIHQFQ X-Message-ID-Hash: 7VYOOKQKT6MPNXAC5CWUD4I5H6SIHQFQ X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top, Yalan Zhang X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --yRWweczhr2xUeyhW Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Jun 17, 2024 at 02:03:16PM +0200, Stefano Brivio wrote: > 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. >=20 > 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. >=20 > 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. >=20 > 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. >=20 > 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. >=20 > Reported-by: Yalan Zhang > Signed-off-by: Stefano Brivio > --- > log.c | 4 +++- > log.h | 1 + > passt.1 | 3 ++- > passt.c | 17 ++++++++++------- > 4 files changed, 16 insertions(+), 9 deletions(-) >=20 > 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 =3D (log_mask & LOG_MASK(LOG_DEBUG)) && log_file =3D= =3D -1; > + bool before_daemon =3D !(log_flags & LOG_FLAG_DAEMON_READY); As in 2/6 would just a global bool be simpler than flags. > bool early_print =3D !(log_flags & LOG_FLAG_CONF_PARSED); > struct timespec tp; > =20 > @@ -71,7 +72,8 @@ void vlogmsg(int pri, const char *format, va_list ap) > va_end(ap2); > } > =20 > - 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)] !=3D '\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 > =20 > #define LOG_FLAG_CONF_PARSED BIT(0) /* We already parsed logging options= */ > +#define LOG_FLAG_DAEMON_READY BIT(1) /* Daemonised, or ready in foregrou= nd */ > =20 > #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. > =20 > .TP > .BR \-l ", " \-\-log-file " " \fIPATH\fR > -Log to file \fIPATH\fR, not to standard error, and not to the system log= ger. > +Log to file \fIPATH\fR, not to standard error (once initialisation is co= mplete), > +and not to the system logger. IIUC when -l is set we'll log to the logfile _as well as_ stderr before we daemonize. The description above doesn't exactly contradict that, but seems to imply something different. > .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 =3D basename(argv0); > if (strstr(name, "pasta")) { > - __openlog(log_name =3D "pasta", LOG_PERROR, LOG_DAEMON); > + __openlog(log_name =3D "pasta", 0, LOG_DAEMON); > =20 > sa.sa_handler =3D pasta_child_handler; > if (sigaction(SIGCHLD, &sa, NULL)) { > @@ -240,7 +240,7 @@ int main(int argc, char **argv) > =20 > c.mode =3D MODE_PASTA; > } else if (strstr(name, "passt")) { > - __openlog(log_name =3D "passt", LOG_PERROR, LOG_DAEMON); > + __openlog(log_name =3D "passt", 0, LOG_DAEMON); > =20 > c.mode =3D MODE_PASST; > } else { > @@ -302,13 +302,16 @@ int main(int argc, char **argv) > if (isolate_prefork(&c)) > die("Failed to sandbox process, exiting"); > =20 > - 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); > =20 > if (pasta_child_pid) > kill(pasta_child_pid, SIGUSR1); --=20 David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson --yRWweczhr2xUeyhW Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmZw2GMACgkQzQJF27ox 2GdNdA//dh1IbbodtugrtXWaxqNKJ45lOAaAqqjhmZSUw0jYFRMabkHGyZA67eDZ ZE+KSLeKQt+C2buGLU5c53MuwPYhw86W5J9mLrQfU46I5oCyfSFfxfyrHrN+ODdN Bb9dU3BQnqDlFWhz1XqaVnN5XV73el0d4SgMOBGWt1RU7cYf3tGi6YT7Vy4GDAbp GyQNs7QtqioyrIb5tI43ggQFS2tzeEBHvfgFA3dmgU7WHTZCO5J/squ4OrQyf9eO z8ijR1T9jIg9zWvAEIyah1eJQxzxziTQmeNQ1Axj6e3ZL2DzlYaUfL++2CO6qug3 tjbbj7zpsZqek2LZqyUAopsSdinfzDCSmHHLOk2UWtz6+APjiJumpY2GmvKOUZXf wXWR2INXIevfmUMpRvd7M9EuijiwKOzfAyNeC9RLJ57PCyiCIdmqMYH4eAliHG/U FZ77WPWhMJOVRJgBKHsFEpCuA8WNv+g6IB6/cHE6rUGP0twou7Wck1HNc6g5t7nF IrQTnrV+n3uXRxnkhQ7b5ygLV5fi0V6Paw1K2EGXyWyCrZnuqV6r/W6lZ2rNT1KC JHuj0vYE1wiXe27GpPaAc2B7QTqRqkOZgyLwwibfjOIT0g7/hzdA4FawPGUTpkTY 2LsvETYPtqWrggjc+bM+s3GgEkz5dDvjKzQC1Iz/+PKo75rWvmA= =EAJA -----END PGP SIGNATURE----- --yRWweczhr2xUeyhW--