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

[Stefano Brivio <sbrivio@redhat.com>]

On Wed, 19 Jun 2024 12:14:53 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Tue, Jun 18, 2024 at 09:14:22AM +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>
> > ---
> >  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))))
> >  		__openlog(log_name, 0, LOG_DAEMON);  
> 
> Hm.. kind of preexisting, but shouldn't we still skip the __openlog()
> if we have a logfile?

Ah, true. I would add this as a separate patch.

> Or make __openlog() open either the syslog or
> the logfile as appropriate (but in that case we should rename it not
> to look like openlog(3)).

I would rather keep __openlog() as openlog() implementation, because
the semantics are well specified like this.

We just need another function, or even a direct setting, for LOG_PERROR
(or get rid of that flag, internally?).

-- 
Stefano