[PATCH 1/2] passt: make --quiet set the log level to warning

[PATCH 1/2] passt: make --quiet set the log level to warning

[Paul Holzinger]

Based on the man page and help output --quiet hides informational
messages. This means that warnings should still be logged. This was
discussed in[1].

[1] https://archives.passt.top/passt-dev/20240216114304.7234a83f@elisabeth/T/#m42652824644973674e84baf9e0bf1d0e88104450

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
---
 passt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/passt.c b/passt.c
index a90568f..5d7e7c4 100644
--- a/passt.c
+++ b/passt.c
@@ -326,7 +326,7 @@ int main(int argc, char **argv)
 	if (c.debug)
 		__setlogmask(LOG_UPTO(LOG_DEBUG));
 	else if (c.quiet)
-		__setlogmask(LOG_UPTO(LOG_ERR));
+		__setlogmask(LOG_UPTO(LOG_WARNING));
 	else
 		__setlogmask(LOG_UPTO(LOG_INFO));
 
-- 
@@ -326,7 +326,7 @@ int main(int argc, char **argv)
 	if (c.debug)
 		__setlogmask(LOG_UPTO(LOG_DEBUG));
 	else if (c.quiet)
-		__setlogmask(LOG_UPTO(LOG_ERR));
+		__setlogmask(LOG_UPTO(LOG_WARNING));
 	else
 		__setlogmask(LOG_UPTO(LOG_INFO));
 
-- 
2.43.2

[PATCH 2/2] conf: set the log level much earlier

[Paul Holzinger]

--quiet is supposed to silence the "No routable interface" message but
it does not work because the log level was set long after conf_ip4/6()
was called which means it uses the default level which logs everything.

To address this move the log level logic directly after the option
parsing in conf().

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
---
 conf.c  | 10 ++++++++++
 passt.c | 10 ----------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/conf.c b/conf.c
index 9c99531..17cf279 100644
--- a/conf.c
+++ b/conf.c
@@ -1646,6 +1646,16 @@ void conf(struct ctx *c, int argc, char **argv)
 			     logfile, logsize);
 	}
 
+	/* Once the log mask is not LOG_EARLY, 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_WARNING));
+	else
+		__setlogmask(LOG_UPTO(LOG_INFO));
+
 	nl_sock_init(c, false);
 	if (!v6_only)
 		c->ifi4 = conf_ip4(ifi4, &c->ip4, c->mac);
diff --git a/passt.c b/passt.c
index 5d7e7c4..a061f2b 100644
--- a/passt.c
+++ b/passt.c
@@ -320,16 +320,6 @@ int main(int argc, char **argv)
 	if (isolate_prefork(&c))
 		die("Failed to sandbox process, exiting");
 
-	/* Once the log mask is not LOG_EARLY, 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_WARNING));
-	else
-		__setlogmask(LOG_UPTO(LOG_INFO));
-
 	if (!c.foreground)
 		__daemon(pidfile_fd, devnull_fd);
 	else
-- 
@@ -320,16 +320,6 @@ int main(int argc, char **argv)
 	if (isolate_prefork(&c))
 		die("Failed to sandbox process, exiting");
 
-	/* Once the log mask is not LOG_EARLY, 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_WARNING));
-	else
-		__setlogmask(LOG_UPTO(LOG_INFO));
-
 	if (!c.foreground)
 		__daemon(pidfile_fd, devnull_fd);
 	else
-- 
2.43.2

Re: [PATCH 2/2] conf: set the log level much earlier

[David Gibson]

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

On Thu, Feb 22, 2024 at 06:17:41PM +0100, Paul Holzinger wrote:
> --quiet is supposed to silence the "No routable interface" message but
> it does not work because the log level was set long after conf_ip4/6()
> was called which means it uses the default level which logs everything.
> 
> To address this move the log level logic directly after the option
> parsing in conf().
> 
> Signed-off-by: Paul Holzinger <pholzing@redhat.com>

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

> ---
>  conf.c  | 10 ++++++++++
>  passt.c | 10 ----------
>  2 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index 9c99531..17cf279 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -1646,6 +1646,16 @@ void conf(struct ctx *c, int argc, char **argv)
>  			     logfile, logsize);
>  	}
>  
> +	/* Once the log mask is not LOG_EARLY, 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_WARNING));
> +	else
> +		__setlogmask(LOG_UPTO(LOG_INFO));
> +
>  	nl_sock_init(c, false);
>  	if (!v6_only)
>  		c->ifi4 = conf_ip4(ifi4, &c->ip4, c->mac);
> diff --git a/passt.c b/passt.c
> index 5d7e7c4..a061f2b 100644
> --- a/passt.c
> +++ b/passt.c
> @@ -320,16 +320,6 @@ int main(int argc, char **argv)
>  	if (isolate_prefork(&c))
>  		die("Failed to sandbox process, exiting");
>  
> -	/* Once the log mask is not LOG_EARLY, 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_WARNING));
> -	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 --]

Re: [PATCH 1/2] passt: make --quiet set the log level to warning

[David Gibson]

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

On Thu, Feb 22, 2024 at 06:17:39PM +0100, Paul Holzinger wrote:
> Based on the man page and help output --quiet hides informational
> messages. This means that warnings should still be logged. This was
> discussed in[1].
> 
> [1] https://archives.passt.top/passt-dev/20240216114304.7234a83f@elisabeth/T/#m42652824644973674e84baf9e0bf1d0e88104450
> 
> Signed-off-by: Paul Holzinger <pholzing@redhat.com>

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

in terms of implementing what it says it does.  Stefano, your call on
whether this is something we want to do, or whether updating the docs
would be better.

> ---
>  passt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/passt.c b/passt.c
> index a90568f..5d7e7c4 100644
> --- a/passt.c
> +++ b/passt.c
> @@ -326,7 +326,7 @@ int main(int argc, char **argv)
>  	if (c.debug)
>  		__setlogmask(LOG_UPTO(LOG_DEBUG));
>  	else if (c.quiet)
> -		__setlogmask(LOG_UPTO(LOG_ERR));
> +		__setlogmask(LOG_UPTO(LOG_WARNING));
>  	else
>  		__setlogmask(LOG_UPTO(LOG_INFO));
>  

-- 
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 --]

Re: [PATCH 1/2] passt: make --quiet set the log level to warning

[Stefano Brivio]

On Fri, 23 Feb 2024 19:12:00 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Thu, Feb 22, 2024 at 06:17:39PM +0100, Paul Holzinger wrote:
> > Based on the man page and help output --quiet hides informational
> > messages. This means that warnings should still be logged. This was
> > discussed in[1].
> > 
> > [1] https://archives.passt.top/passt-dev/20240216114304.7234a83f@elisabeth/T/#m42652824644973674e84baf9e0bf1d0e88104450
> > 
> > Signed-off-by: Paul Holzinger <pholzing@redhat.com>  
> 
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> 
> in terms of implementing what it says it does.  Stefano, your call on
> whether this is something we want to do, or whether updating the docs
> would be better.

No, no, I was actually suggesting this change: "quiet" seems to better
suited to describe the fact that we cut down on messages that are not
fundamental, but warnings arguably are -- even though the distinction
between error and warning is still useful for a system logger, I guess.

-- 
Stefano

Re: [PATCH 1/2] passt: make --quiet set the log level to warning

[Stefano Brivio]

On Thu, 22 Feb 2024 18:17:39 +0100
Paul Holzinger <pholzing@redhat.com> wrote:

> Based on the man page and help output --quiet hides informational
> messages. This means that warnings should still be logged. This was
> discussed in[1].
> 
> [1] https://archives.passt.top/passt-dev/20240216114304.7234a83f@elisabeth/T/#m42652824644973674e84baf9e0bf1d0e88104450
> 
> Signed-off-by: Paul Holzinger <pholzing@redhat.com>

Applied.

-- 
Stefano

Re: [PATCH 2/2] conf: set the log level much earlier

[Stefano Brivio]

On Thu, 22 Feb 2024 18:17:41 +0100
Paul Holzinger <pholzing@redhat.com> wrote:

> --quiet is supposed to silence the "No routable interface" message but
> it does not work because the log level was set long after conf_ip4/6()
> was called which means it uses the default level which logs everything.
> 
> To address this move the log level logic directly after the option
> parsing in conf().
> 
> Signed-off-by: Paul Holzinger <pholzing@redhat.com>

Applied.

-- 
Stefano