Re: [PATCH 1/6] conf, passt: Don't try to log to stderr after we close it

[Stefano Brivio <sbrivio@redhat.com>]

On Tue, 18 Jun 2024 10:36:28 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Mon, Jun 17, 2024 at 02:03:14PM +0200, Stefano Brivio wrote:
> > If we don't run in foreground, we close standard error as we
> > daemonise, so it makes no sense to check if the controlling terminal
> > is an interactive terminal or if --force-stderr was given, to decide
> > if we want to log to standard error.
> > 
> > Make --force-stderr depend on --foreground.
> > 
> > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>  
> 
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> 
> > ---
> >  conf.c  | 3 +++
> >  passt.c | 2 +-
> >  2 files changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/conf.c b/conf.c
> > index 94b3ed6..dbdbb62 100644
> > --- a/conf.c
> > +++ b/conf.c
> > @@ -1693,6 +1693,9 @@ void conf(struct ctx *c, int argc, char **argv)
> >  
> >  	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) {
> >  		logfile_init(c->mode == MODE_PASTA ? "pasta" : "passt",
> >  			     logfile, logsize);
> > diff --git a/passt.c b/passt.c
> > index a5e2c5a..aa9648a 100644
> > --- a/passt.c
> > +++ b/passt.c
> > @@ -302,7 +302,7 @@ int main(int argc, char **argv)
> >  	if (isolate_prefork(&c))
> >  		die("Failed to sandbox process, exiting");
> >  
> > -	if (!c.force_stderr && !isatty(fileno(stderr)))
> > +	if (!c.foreground || (!c.force_stderr && !isatty(fileno(stderr))))  
> 
> What's the rationale for the isatty() check in any case?

To implement the behaviour from the man page:

       -e, --stderr
              Log  to standard error too.  Default is to log to the sys‐
              tem logger only, if started from an interactive  terminal,
              and to both system logger and standard error otherwise.

which was needed, in turn, because of earlier versions of passt and of
the KubeVirt integration where passt was running in foreground, but (of
course) not attached to a terminal, and there was no option to force
printing to standard error.

Given that KubeVirt doesn't have a system logger, it was otherwise
impossible to get messages out of passt.

It's an abomination that just adds complexity now, and I don't think
anybody uses it that way anymore. I guess we should drop that, together
with qrap, in a few months from now.

-- 
Stefano