conf, passt: Make --stderr do nothing, and deprecate it
The original behaviour of printing messages to standard error by default when running from a non-interactive terminal was introduced because the first KubeVirt integration draft used to start passt in foreground and get messages via standard error. For development purposes, the system logger was more convenient at that point, and passt was running from interactive terminals only if not started by the KubeVirt integration. This behaviour was introduced by84a62b79a2
("passt: Also log to stderr, don't fork to background if not interactive"). Later, I added command-line options in1e49d194d0
("passt, pasta: Introduce command-line options and port re-mapping") and accidentally reversed this condition, which wasn't a problem as --stderr could force printing to standard error anyway (and it was used by KubeVirt). Nowadays, the KubeVirt integration uses a log file (requested via libvirt configuration), and the same applies for Podman if one actually needs to look at runtime logs. There are no use cases left, as far as I know, where passt runs in foreground in non-interactive terminals. Seize the chance to reintroduce some sanity here. If we fork to background, standard error is closed, so --stderr is useless in that case. If we run in foreground, there's no harm in printing messages to standard error, and that accidentally became the default behaviour anyway, so --stderr is not needed in that case. It would be needed for non-interactive terminals, but there are no use cases, and if there were, let's log to standard error anyway: the user can always redirect standard error to /dev/null if needed. Before we're up and running, we need to print to standard error anyway if something happens, otherwise we can't report failure to start in any kind of usage, stand-alone or in integrations. So, make --stderr do nothing, and deprecate it. While at it, drop a left-over comment about --foreground being the default only for interactive terminals, because it's not the case anymore. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
This commit is contained in:
parent
b74801645c
commit
bca0fefa32
4 changed files with 7 additions and 29 deletions
17
conf.c
17
conf.c
|
@ -727,9 +727,7 @@ static void usage(const char *name, FILE *f, int status)
|
||||||
" --trace Be extra verbose, implies --debug\n"
|
" --trace Be extra verbose, implies --debug\n"
|
||||||
" -q, --quiet Don't print informational messages\n"
|
" -q, --quiet Don't print informational messages\n"
|
||||||
" -f, --foreground Don't run in background\n"
|
" -f, --foreground Don't run in background\n"
|
||||||
" default: run in background if started from a TTY\n"
|
" default: run in background\n"
|
||||||
" -e, --stderr Log to stderr too\n"
|
|
||||||
" default: log to system logger only if started from a TTY\n"
|
|
||||||
" -l, --log-file PATH Log (only) to given file\n"
|
" -l, --log-file PATH Log (only) to given file\n"
|
||||||
" --log-size BYTES Maximum size of log file\n"
|
" --log-size BYTES Maximum size of log file\n"
|
||||||
" default: 1 MiB\n"
|
" default: 1 MiB\n"
|
||||||
|
@ -1387,18 +1385,10 @@ void conf(struct ctx *c, int argc, char **argv)
|
||||||
c->quiet = 0;
|
c->quiet = 0;
|
||||||
break;
|
break;
|
||||||
case 'e':
|
case 'e':
|
||||||
if (logfile)
|
warn("--stderr will be dropped soon");
|
||||||
die("Can't log to both file and stderr");
|
|
||||||
|
|
||||||
c->force_stderr = 1;
|
|
||||||
logfile = NULL;
|
|
||||||
break;
|
break;
|
||||||
case 'l':
|
case 'l':
|
||||||
if (c->force_stderr)
|
|
||||||
die("Can't log to both stderr and file");
|
|
||||||
|
|
||||||
logfile = optarg;
|
logfile = optarg;
|
||||||
c->force_stderr = 0;
|
|
||||||
break;
|
break;
|
||||||
case 'q':
|
case 'q':
|
||||||
c->quiet = 1;
|
c->quiet = 1;
|
||||||
|
@ -1631,9 +1621,6 @@ void conf(struct ctx *c, int argc, char **argv)
|
||||||
|
|
||||||
conf_ugid(runas, &uid, &gid);
|
conf_ugid(runas, &uid, &gid);
|
||||||
|
|
||||||
if (!c->foreground && c->force_stderr)
|
|
||||||
die("Can't log to standard error if not running in foreground");
|
|
||||||
|
|
||||||
if (logfile) {
|
if (logfile) {
|
||||||
logfile_init(c->mode == MODE_PASTA ? "pasta" : "passt",
|
logfile_init(c->mode == MODE_PASTA ? "pasta" : "passt",
|
||||||
logfile, logsize);
|
logfile, logsize);
|
||||||
|
|
15
passt.1
15
passt.1
|
@ -96,21 +96,14 @@ Default is to fork into background.
|
||||||
|
|
||||||
.TP
|
.TP
|
||||||
.BR \-e ", " \-\-stderr
|
.BR \-e ", " \-\-stderr
|
||||||
Log to standard error too.
|
This option has no effect, and is maintained for compatibility purposes only.
|
||||||
Default is to log to the system logger only, if started from an interactive
|
|
||||||
terminal, and to both system logger and standard error otherwise.
|
|
||||||
|
|
||||||
This option cannot be specified together with \fB--log-file\fR, because there
|
Note that this configuration option is \fBdeprecated\fR and will be removed in a
|
||||||
might be a reasonable expectation that messages are logged to standard error as
|
future version.
|
||||||
well as to a file, which is however not supported.
|
|
||||||
|
|
||||||
.TP
|
.TP
|
||||||
.BR \-l ", " \-\-log-file " " \fIPATH\fR
|
.BR \-l ", " \-\-log-file " " \fIPATH\fR
|
||||||
Log to file \fIPATH\fR, not to standard error, and not to the system logger.
|
Log to file \fIPATH\fR, and not to the system logger.
|
||||||
|
|
||||||
This option cannot be specified together with \fB--stderr\fR, because there
|
|
||||||
might be a reasonable expectation that messages are logged to a file as well as
|
|
||||||
to standard error, which is however not supported.
|
|
||||||
|
|
||||||
Specifying this option multiple times does \fInot\fR lead to multiple log files:
|
Specifying this option multiple times does \fInot\fR lead to multiple log files:
|
||||||
the last given option takes effect.
|
the last given option takes effect.
|
||||||
|
|
2
passt.c
2
passt.c
|
@ -302,7 +302,7 @@ int main(int argc, char **argv)
|
||||||
if (isolate_prefork(&c))
|
if (isolate_prefork(&c))
|
||||||
die("Failed to sandbox process, exiting");
|
die("Failed to sandbox process, exiting");
|
||||||
|
|
||||||
if (!c.foreground || (!c.force_stderr && !isatty(fileno(stderr))))
|
if (!c.foreground)
|
||||||
__openlog(log_name, 0, LOG_DAEMON);
|
__openlog(log_name, 0, LOG_DAEMON);
|
||||||
|
|
||||||
if (!c.foreground)
|
if (!c.foreground)
|
||||||
|
|
2
passt.h
2
passt.h
|
@ -180,7 +180,6 @@ struct ip6_ctx {
|
||||||
* @trace: Enable tracing (extra debug) mode
|
* @trace: Enable tracing (extra debug) mode
|
||||||
* @quiet: Don't print informational messages
|
* @quiet: Don't print informational messages
|
||||||
* @foreground: Run in foreground, don't log to stderr by default
|
* @foreground: Run in foreground, don't log to stderr by default
|
||||||
* @force_stderr: Force logging to stderr
|
|
||||||
* @nofile: Maximum number of open files (ulimit -n)
|
* @nofile: Maximum number of open files (ulimit -n)
|
||||||
* @sock_path: Path for UNIX domain socket
|
* @sock_path: Path for UNIX domain socket
|
||||||
* @pcap: Path for packet capture file
|
* @pcap: Path for packet capture file
|
||||||
|
@ -231,7 +230,6 @@ struct ctx {
|
||||||
int trace;
|
int trace;
|
||||||
int quiet;
|
int quiet;
|
||||||
int foreground;
|
int foreground;
|
||||||
int force_stderr;
|
|
||||||
int nofile;
|
int nofile;
|
||||||
char sock_path[UNIX_PATH_MAX];
|
char sock_path[UNIX_PATH_MAX];
|
||||||
char pcap[PATH_MAX];
|
char pcap[PATH_MAX];
|
||||||
|
|
Loading…
Reference in a new issue