From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56594) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTq6x-0006lQ-Em for qemu-devel@nongnu.org; Thu, 11 Feb 2016 07:12:43 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aTq6u-0004Tu-3m for qemu-devel@nongnu.org; Thu, 11 Feb 2016 07:12:39 -0500 Received: from mx0.arrikto.com ([212.71.252.59]:57748) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTq6t-0004Tb-UB for qemu-devel@nongnu.org; Thu, 11 Feb 2016 07:12:36 -0500 Date: Thu, 11 Feb 2016 14:12:30 +0200 From: Dimitris Aragiorgis Message-ID: <20160211121230.GB7256@arr> References: <1450284981-10641-1-git-send-email-apyrgio@arrikto.com> <56B86013.1050504@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="ZoaI/ZTpAVc4A5k6" Content-Disposition: inline In-Reply-To: <56B86013.1050504@redhat.com> Subject: Re: [Qemu-devel] [PATCH] os-posix: Log to logfile in case of daemonize List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Alex Pyrgiotis , qemu-devel@nongnu.org --ZoaI/ZTpAVc4A5k6 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi, * Paolo Bonzini [2016-02-08 10:29:55 +0100]: >=20 >=20 > On 16/12/2015 17:56, Alex Pyrgiotis wrote: > > + > > + log =3D qemu_get_log_filename(); > > + if (log !=3D NULL) { > > + TFR(fd =3D qemu_open(log, O_RDWR | O_APPEND | O_CREAT, 064= 0)); >=20 > Here you are opening the same file twice, but the FILE* that is opened > in do_qemu_set_log does not necessarily have O_APPEND. >=20 This is partially true :) I am opening this file twice only if the -d option is passed along with -D. Still, point taken. > I like the idea of moving stderr to the logfile, but I'm not sure how to > do it. For now, can you prepare a simple patch that only does the "dup" > if "isatty" returns true for the file descriptor? This lets you use > redirection at the shell level to save the stdout and stderr. >=20 This could be a workaround. Still, I think it is a bit unorthodox to change the core behavior of logging depending on the type of output (tty or not). I understand that checking the type of output is sometimes used to enable/disable colors automatically, for example. Besides that, when one executes a daemon, shell redirection is hardly, if ever, used. More so if the daemon already has a logfile option. So, we decided to give it a go and find the least painful way to log the stderr of a QEMU process to a logfile. To our understanding, the logfile (-D option) is used only for messages generated by qemu_log()/qemu_log_mask(). The current situation however is that fprintf(stderr, ...) is used in various places throughout the codebase for logging/debug purposes. A simple solution would be to redirect the stderr to the logfile when -D is used, but this may confuse people who expect the current behavior for their logfiles. Therefore, our suggestion is to introduce another option, "-log-stderr". If this is given then we can 'dup2' stderr to the already opened logfile inside do_qemu_set_log(). And we should not close it even if -d is not given. Afterwards, os_setup_post() in case of daemonize, would always dup2 0, 1 to /dev/null and only if qemu_logfile is not NULL would dup2 2 to /dev/null as well. To sum up if one wants to log stderr to the logfile, one should pass -D along with -log-stderr. Eventually qemu_log(), qemu_log_mask(), and fprintf(stderr, ...) will end up writing to our logfile. The -daemonize option should respect the other options. What do you think? dimara > Paolo >=20 > > + } else { > > + TFR(fd =3D qemu_open("/dev/null", O_RDWR)); > > + } > > if (fd =3D=3D -1) { > > + fprintf(stderr, "Cannot open \"%s\" for logging\n", log); > > exit(1); > > } > > + g_free(log); > > } >=20 --ZoaI/ZTpAVc4A5k6 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) iQEcBAEBAgAGBQJWvHquAAoJEHFDHex6CBG9tqQH/3E7EfK1nKtA7b0Nvj13f9cX w5lfKu3cr6sctpF7E5aPz6mebRYilsoi+RbCW3KI6J4420rBfqpxgIK6fZ41jeKX U9vYKrZomA3ImsevyjHtoXB473dh0qKCLK/eIH9mt71O1oy48unultvgyCXdu8F9 Z3W2cM2jv4ZNPMOdqoBCOPAXnD3nh3lQahcO9WrXhy9vNvseaLQJ8jccHHoGO6Qa VTMpW/Fi819dL9mt3eYCzJgG5BO5gb4xbYyrH6QitF4mLHufdxmpkh/bb0JTmnY+ YbukqkD9+dR+JqmGzOZXjTbUvgX+wBecL25WW4mYZWnbZ4rLzZXvb87szlRjQuE= =jCe0 -----END PGP SIGNATURE----- --ZoaI/ZTpAVc4A5k6--