[PATCH v3 0/9] error logging fixes
There are two topics covered here: 1) If a logFile is set, passt's behavior has been to send all error messages there, and *not* to stderr. This makes it difficult for another program that is exec'ing passt (and setting it to log to a file) to report useful error messages when passt fails - everything after the point that the logfile is opened is sent only to the logfile. The first patch makes a simple change to the logging functions that uses the value of the system logmask to decide if it should force writing messages to stderr even when a logfile has been specified. Change from "v2": I'm using Stefano's suggestion of "abusing" logmask, rather than adding a static bool to keep track. 2) All the rest of the patches eliminate use of the blanket usage() function when a commandline error is encountered (and add in specific/details error messages when previously usage() was all that was logged), and replace calls to err() followed by exit() with a single call to the new function die(). Change from "v2": I changed the name of the "log and exit" function from "errexit()" to "die()" at the suggestion of Dave Gibson (Stefano concurred). Although it seems a bit more violent, it does make moot many/most of Stefano's nits about needing to split lines to eliminate
80 characters (I did address all the rest of the things he pointed out, though)
NB: Yes, this says it is v3, and the previous version I sent was v2, and there *was no v1* - this is because I didn't realize that git-publish is automatically incrementing the version number every time I run it, and I had done a test-drive sending the patches to my personal address prior to sending them to the list - *that* was v1. Laine Stump (9): log to stderr until process is daemonized, even if a logfile is set add die() to log an error message and exit with a single call eliminate most calls to usage() in conf() make conf_ports() exit immediately after logging error make conf_pasta_ns() exit immediately after logging error make conf_ugid() exit immediately after logging error make conf_netns_opt() exit immediately after logging error log a detailed error (not usage()) when there are extra non-option arguments convert all remaining err() followed by exit() to die() conf.c | 468 ++++++++++++++++++++-------------------------------- isolation.c | 67 +++----- log.c | 24 +-- log.h | 1 + netlink.c | 3 +- passt.c | 29 ++-- pasta.c | 20 +-- tap.c | 30 ++-- 8 files changed, 244 insertions(+), 398 deletions(-) -- 2.39.1
Once a logfile (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 logmask 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 logmask to something else). This period
*almost* matches with "the time before the process is daemonized"; if
we just delay setting the logmask 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 logfile has been specified and opened.
This patch delays the setting of logmask until immediately before the
call to __daemon(). It also modifies logfn() slightly, so that it will
log to stderr any time logmask is LOG_EMERG, even if a logfile has
been opened.
Signed-off-by: Laine Stump
Almost all occurences of err() are either immediately followed by
exit(EXIT_FAILURE), usage(argv[0]) (which itself then calls
exit(EXIT_FAILURE), or that is what's done immediately after returning
from the function that calls err(). Modify the errfn macro so that its
instantiations can include exit(EXIT_FAILURE) at the end, and use that
to create a new function die() that will log an error and then
exit.
Signed-off-by: Laine Stump
Nearly all of the calls to usage() in conf() occur immediately after
logging a more detailed error message, and the fact that these errors
are occuring indicates that the user has already seen the passt usage
message (or read the manpage). Spamming the logfile with the complete
contents of the usage message serves only to obscure the more detailed
error message. The only time when the full usage message should be output
is if the user explicitly asks for it with -h (or its synonyms)
As a start to eliminating the excessive calls to usage(), this patch
replaces most calls to err() followed by usage() with a call to die()
instead. A few other usage() calls remain, but their removal involves
bit more nuance that should be properly explained in separate commit
messages.
Signed-off-by: Laine Stump
Rather than having conf_ports() (possibly) log an error, and then
letting the caller log the entire usage() message and exit, just log
the errors and exit immediately (using die()).
For some errors, conf_ports would previously not log any specific
message, leaving it up to the user to determine the problem by
guessing. We replace all of those silent returns with die()
(logging a specific error), thus permitting us to make conf_ports()
return void, which simplifies the caller.
While modifying the two callers to conf_ports() to not check for a
return value, we can further simplify the code by removing the check
for a non-null optarg, as that is guaranteed to never happen (due to
prior calls to getopt_long() with "argument required" for all relevant
options - getopt_long() would have already caught this error).
Signed-off-by: Laine Stump
As with conf_ports, this allows us to make the function return void.
Signed-off-by: Laine Stump
Again, it can then be made to return void, simplifying the caller.
Signed-off-by: Laine Stump
...and return void to simplify the caller.
Signed-off-by: Laine Stump
Signed-off-by: Laine Stump
This actually leaves us with 0 uses of err(), but someone could want
to use it in the future, so we may as well leave it around.
Signed-off-by: Laine Stump
participants (1)
-
Laine Stump