From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 6D34A5A004F for ; Wed, 19 Jun 2024 04:11:59 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1718763116; bh=8I2ctRozBRh2Di9syBIpit3Av7icvERRzJhZw1Yw1VQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Ic3tqJy943Txt8T3UXGVFRXMfaHG44XfxQpSt1ED2ak/M0ILDAf1LR8xBOmoZoyv4 Ps6jyb/nOYkcaCGqDagUruamNoBMHzcDlJptFWK9Nd0fgYgQLwAlSMN4odnhfoDAOE pnQ5zNCetYnahM9XZST/mcBoNu+FQzYiTfhNYzKhKFmcYX1zlXuUrntbmkM3uMjTq7 Fudey/u4KDDFd49mJi2kt4fOk4gk51zRt0aQt75WgeJEPNvjzcWPmc7YkMqOoIWKJz RQppb2Xd3RZFy/VHYjiJi7mvo0iuZbAA0ySMijWTQayuUR2brRLh6ddByoL2HS+h+6 BLF+FqHBzo9Zw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4W3nDr5HYMz4wyQ; Wed, 19 Jun 2024 12:11:56 +1000 (AEST) Date: Wed, 19 Jun 2024 12:06:01 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 1/6] conf, passt: Don't try to log to stderr after we close it Message-ID: References: <20240617120319.1206857-1-sbrivio@redhat.com> <20240617120319.1206857-2-sbrivio@redhat.com> <20240618080004.5fb4e355@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="335wnR7PdDqdcKhZ" Content-Disposition: inline In-Reply-To: <20240618080004.5fb4e355@elisabeth> Message-ID-Hash: TDBNFZ24CUS46XM4N6CMLVVJ47KZHQVF X-Message-ID-Hash: TDBNFZ24CUS46XM4N6CMLVVJ47KZHQVF X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top, Yalan Zhang X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --335wnR7PdDqdcKhZ Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Jun 18, 2024 at 08:00:14AM +0200, Stefano Brivio wrote: > On Tue, 18 Jun 2024 10:36:28 +1000 > David Gibson wrote: >=20 > > 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. > > >=20 > > > Make --force-stderr depend on --foreground. > > >=20 > > > Signed-off-by: Stefano Brivio =20 > >=20 > > Reviewed-by: David Gibson > >=20 > > > --- > > > conf.c | 3 +++ > > > passt.c | 2 +- > > > 2 files changed, 4 insertions(+), 1 deletion(-) > > >=20 > > > 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) > > > =20 > > > conf_ugid(runas, &uid, &gid); > > > =20 > > > + if (!c->foreground && c->force_stderr) > > > + die("Can't log to standard error if not running in foreground"); > > > + > > > if (logfile) { > > > logfile_init(c->mode =3D=3D 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"); > > > =20 > > > - if (!c.force_stderr && !isatty(fileno(stderr))) > > > + if (!c.foreground || (!c.force_stderr && !isatty(fileno(stderr)))) = =20 > >=20 > > What's the rationale for the isatty() check in any case? >=20 > To implement the behaviour from the man page: >=20 > -e, --stderr > Log to standard error too. Default is to log to the sys= =E2=80=90 > tem logger only, if started from an interactive terminal, > and to both system logger and standard error otherwise. Hmm.. maybe I'm getting confused reading either the man page or the code, but isn't that the opposite of what the code is doing? The code appears to be opening the log if we're *not* on an interactive terminal. > 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. >=20 > Given that KubeVirt doesn't have a system logger, it was otherwise > impossible to get messages out of passt. >=20 > 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. Right.. but what I'm after is the rationale for this depending on whether it's an interactive terminal at all. Seems to me the logical thing to do is to (by default) always log to stderr before daemonization (i.e. forever when in foreground mode), then to logfile and/or syslog after daemonization. Regardless of whether there's a tty or not. --=20 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 --335wnR7PdDqdcKhZ Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmZyPQgACgkQzQJF27ox 2GcWVA//XD6dxLqO70yEp8IKKyMAtBtj5e9j7AC/mr+aZBrdSAZDWkDGA4SC/ySg Xjc+6hPnWXJoQ3Ok04gjVTY8m3aBUDsWsJh20upeDvz9qRqryDXJfKu3lU68+4z1 TgXqvHcmblf2hF/pok4+arKrXoi9/p1vZFrpjbbioOMoPbL3dgZ0Gy84+v7saQr2 KQdwHXALyWrhynsT7DICF7L3jLFUroHz1/KeISbyHWdcIoUaLilHe5kp/sKoeYsk wxyBWPUuv6hN/5D0tv7F811re3IKvU1apyhCO/Maam9M4dxGU5gB2EvFI6E0DyDz oOhCP6Kxqivw0SDtO5bgkv/bbwb3qNFM582UcW1QY1OqEPNXPt81brDV8Q4y/Lm5 UQBt0X1Nu4WH6A/r+/Ma/+I8aEh6AuGPd85pt4B+QZjElnEZ2+dHl6AcXrbuPYvS uiqFJNG0ikpdtqiZoOBRexRYo0v01lwApvMZVio+OdC4GTAjhuIs0tNhIjXdwP+N 2kNG7cCgPBRNzRb78tgqEpl/lOzlgZbrGwF+gMeegio+92fKCk0NUGkaUEZfpgic 1e6to0rayD8YqWMx7mIvEsu9oNag04EVuu8t3xVY8OdqaucealEen08MU+kIR2Pl 3HNhH0kpPDYcAmo1STJXudhTCLUlzWN+h9NJUJWCbdfEFkEkC4Q= =BQKN -----END PGP SIGNATURE----- --335wnR7PdDqdcKhZ--