Re: [PATCH v5 0/9] Fixes for early logging/prints and related cleanups

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

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

On Thu, Jun 20, 2024 at 06:15:09PM +0200, Stefano Brivio wrote:
> The most apparent issue fixed by this series is the one from 4/6: with
> a log file configured, we wouldn't print to standard error anymore,
> during initialisation, which means that users such as libvirt lost
> the ability to report meaningful error messages that occurred during
> initialisation, in that case.
> 
> v5:
> - in 4/8, rename the new flag once more to 'log_runtime': we don't
>   want to log to standard error if we're running in foreground, a
>   log file is given, and initialisation is done, otherwise debugging
>   pasta when it spawns its own shell becomes rather impractical

Ah.. right.  See, I still think the semantics of always printing to
stderr when foreground make more sense, but I guess I do see the point
that having pasta messages appear in your pasta-spawned shell is ugly.

My preferred approach for that would to keep the basic semantics that
we always log to stderr when foreground, but when we're spawning a
pasta shell we default to 'quiet' log level.  That way if you really
do want messages to stderr along with your shell/command (which I
sometimes do), you can get that by using --debug or whatever.

Regardless of that quibble, though, the semantics are substantially
saner with this series than without so I'm happy to go ahead with it.

> - add 9/8, taking care of a direct stderr print left-over
> 
> v4:
> - in 4/8, name the new flag 'log_daemonised' instead of
>   'log_daemon_ready'
> 
> v3:
> - add 2/8: we don't really need --stderr anymore
> - in 5/8, save errno at the beginning of the _perror() helper
> - in 7/8, avoid assigning errno to whatever return code we have just
>   for the sake of using the new helpers: strerror() is actually less
>   convoluted than that
> - add 8/8: there's no need to call __openlog() with a log file
> 
> v2:
> - turn flag bitmap into simple, separate boolean flags
> - move errno description after message in _perror() functions
> - make some of the old perror() messages more descriptive
> *** BLURB HERE ***
> 
> Stefano Brivio (9):
>   conf, passt: Don't try to log to stderr after we close it
>   conf, passt: Make --stderr do nothing, and deprecate it
>   conf, log: Instead of abusing log levels, add log_conf_parsed flag
>   log, passt: Always print to stderr before initialisation is complete
>   log: Add _perror() logging function variants
>   treewide: Replace perror() calls with calls to logging functions
>   treewide: Replace strerror() calls
>   conf, passt: Don't call __openlog() if a log file is used
>   log: Don't report syslog failures to stderr after initialisation
> 
>  arch.c      | 10 ++++-----
>  conf.c      | 41 ++++++++++++---------------------
>  fwd.c       |  2 +-
>  isolation.c | 46 ++++++++++++++++---------------------
>  log.c       | 65 ++++++++++++++++++++++++++++++++---------------------
>  log.h       | 25 +++++++++++++++++----
>  netlink.c   |  4 ++--
>  passt.1     |  9 ++++----
>  passt.c     | 64 +++++++++++++++++++---------------------------------
>  passt.h     |  2 --
>  pasta.c     | 27 +++++++++-------------
>  pcap.c      |  8 +++----
>  tap.c       | 14 ++++++------
>  tcp.c       | 24 +++++++-------------
>  util.c      | 12 +++++-----
>  15 files changed, 164 insertions(+), 189 deletions(-)
> 

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

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