Re: [PATCH v4 1/9] log to stderr until process is daemonized, even if a log file is set

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3353 bytes --]

On Wed, Feb 15, 2023 at 03:24:29AM -0500, Laine Stump wrote:
> Once a log file (specified on the commandline) is opened, the logging
> functions will stop sending error logs to stderr, which is annoying if
> passt has been started by another process that wants to collect error
> messages from stderr so it can report why passt failed to start. It
> would be much nicer if passt continued sending all log messages to
> stderr until it daemonizes itself (at which point the process that
> started passt can assume that it was started successfully).
> 
> The system log mask is set to LOG_EMERG when the process starts, and
> we're already using that to do "special" logging during the period
> from process start until the log level requested on the commandline is
> processed (setting the log mask to something else). This period
> *almost* matches with "the time before the process is daemonized"; if
> we just delay setting the log mask a tiny bit, then it will match
> exactly, and we can use it to determine if we need to send log
> messages to stderr even when a log file has been specified and opened.
> 
> This patch delays the setting of the log mask until immediately before
> the call to __daemon(). It also modifies logfn() slightly, so that it
> will log to stderr any time log mask is LOG_EMERG, even if a log file
> has been opened.
> 
> Signed-off-by: Laine Stump <laine@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  log.c   |  4 ++--
>  passt.c | 17 ++++++++++-------
>  2 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/log.c b/log.c
> index 468c730..6dc6673 100644
> --- a/log.c
> +++ b/log.c
> @@ -66,8 +66,8 @@ void name(const char *format, ...) {					\
>  		va_end(args);						\
>  	}								\
>  									\
> -	if ((setlogmask(0) & LOG_MASK(LOG_DEBUG) ||			\
> -	     setlogmask(0) == LOG_MASK(LOG_EMERG)) && log_file == -1) {	\
> +	if ((setlogmask(0) & LOG_MASK(LOG_DEBUG) && log_file == -1) ||	\
> +	     setlogmask(0) == LOG_MASK(LOG_EMERG)) {			\
>  		va_start(args, format);					\
>  		(void)vfprintf(stderr, format, args); 			\
>  		va_end(args);						\
> diff --git a/passt.c b/passt.c
> index d957e14..c48c2d5 100644
> --- a/passt.c
> +++ b/passt.c
> @@ -246,13 +246,6 @@ int main(int argc, char **argv)
>  	if (c.stderr || isatty(fileno(stdout)))
>  		__openlog(log_name, LOG_PERROR, LOG_DAEMON);
>  
> -	if (c.debug)
> -		__setlogmask(LOG_UPTO(LOG_DEBUG));
> -	else if (c.quiet)
> -		__setlogmask(LOG_UPTO(LOG_ERR));
> -	else
> -		__setlogmask(LOG_UPTO(LOG_INFO));
> -
>  	quit_fd = pasta_netns_quit_init(&c);
>  
>  	tap_sock_init(&c);
> @@ -296,6 +289,16 @@ int main(int argc, char **argv)
>  		exit(EXIT_FAILURE);
>  	}
>  
> +	/* Once the log mask is not LOG_EMERG, we will no longer
> +	 * log to stderr if there was a log file specified.
> +	 */
> +	if (c.debug)
> +		__setlogmask(LOG_UPTO(LOG_DEBUG));
> +	else if (c.quiet)
> +		__setlogmask(LOG_UPTO(LOG_ERR));
> +	else
> +		__setlogmask(LOG_UPTO(LOG_INFO));
> +
>  	if (!c.foreground)
>  		__daemon(pidfile_fd, devnull_fd);
>  	else

-- 
David Gibson			| 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]