From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35442) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aaktN-0000vX-Di for qemu-devel@nongnu.org; Tue, 01 Mar 2016 09:03:14 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aaktJ-00032g-E0 for qemu-devel@nongnu.org; Tue, 01 Mar 2016 09:03:13 -0500 Received: from mx0.arrikto.com ([212.71.252.59]:40008) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aaktJ-00032T-8m for qemu-devel@nongnu.org; Tue, 01 Mar 2016 09:03:09 -0500 Date: Tue, 1 Mar 2016 16:03:05 +0200 From: Dimitris Aragiorgis Message-ID: <20160301140305.GA10785@arr> References: <1456833053-8896-1-git-send-email-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="CE+1k2dSO48ffgeK" Content-Disposition: inline In-Reply-To: <1456833053-8896-1-git-send-email-pbonzini@redhat.com> Subject: Re: [Qemu-devel] [PATCH] log: do not log if QEMU is daemonized but without -D List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: jtomko@redhat.com, qemu-devel@nongnu.org, kraxel@redhat.com --CE+1k2dSO48ffgeK Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi all, * Paolo Bonzini [2016-03-01 12:50:53 +0100]: > Commit 96c33a4 ("log: Redirect stderr to logfile if deamonized", > 2016-02-22) wanted to move stderr of a daemonized QEMU to the file > specified with -D. >=20 > However, if -D was not passed, the patch had the side effect of not > redirecting stderr to /dev/null. This happened because qemu_logfile > was set to stderr rather than the expected value of NULL. The fix > is simply in the "if" condition of do_qemu_set_log; the "if" for > closing the file is also changed to match. >=20 Sorry, that was my bad. I tested the proposed fix as well and it works. I would suggest to add some sort of comments since the whole do_qemu_set_log() is hard to parse in one pass. For example: diff --git a/util/log.c b/util/log.c index 8b921de..1d73be0 100644 --- a/util/log.c +++ b/util/log.c @@ -56,6 +56,10 @@ void do_qemu_set_log(int log_flags, bool use_own_buffers) #ifdef CONFIG_TRACE_LOG qemu_loglevel |=3D LOG_TRACE; #endif + /* Setup the logfile when: + * - The loglevel is set via -d option or CONFIG_TRACE_LOG, or + * - Both -daemonize and -D options are given + */ if (!qemu_logfile && (is_daemonized() ? logfilename !=3D NULL : qemu_loglevel)) { if (logfilename) { @@ -91,6 +95,10 @@ void do_qemu_set_log(int log_flags, bool use_own_buffers) log_append =3D 1; } } + /* Close the already setup logfile when loglevel gets reset (e.g. + * via the log HMP command), unless both -daemonize and -D options + * are given. + */ if (qemu_logfile && (is_daemonized() ? logfilename =3D=3D NULL : !qemu_loglevel)) { qemu_log_close(); Thanks for this quick fix and sorry again for any trouble caused in the fir= st place. dimara > Reported-by: Jan Tomko > Signed-off-by: Paolo Bonzini > --- > util/log.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) >=20 > diff --git a/util/log.c b/util/log.c > index a7ddc7e..8b921de 100644 > --- a/util/log.c > +++ b/util/log.c > @@ -56,7 +56,8 @@ void do_qemu_set_log(int log_flags, bool use_own_buffer= s) > #ifdef CONFIG_TRACE_LOG > qemu_loglevel |=3D LOG_TRACE; > #endif > - if ((qemu_loglevel || is_daemonized()) && !qemu_logfile) { > + if (!qemu_logfile && > + (is_daemonized() ? logfilename !=3D NULL : qemu_loglevel)) { > if (logfilename) { > qemu_logfile =3D fopen(logfilename, log_append ? "a" : "w"); > if (!qemu_logfile) { > @@ -72,6 +73,7 @@ void do_qemu_set_log(int log_flags, bool use_own_buffer= s) > } > } else { > /* Default to stderr if no log file specified */ > + assert(!is_daemonized()); > qemu_logfile =3D stderr; > } > /* must avoid mmap() usage of glibc by setting a buffer "by hand= " */ > @@ -89,7 +91,8 @@ void do_qemu_set_log(int log_flags, bool use_own_buffer= s) > log_append =3D 1; > } > } > - if (!qemu_loglevel && !is_daemonized() && qemu_logfile) { > + if (qemu_logfile && > + (is_daemonized() ? logfilename =3D=3D NULL : !qemu_loglevel)) { > qemu_log_close(); > } > } > --=20 > 2.5.0 >=20 --CE+1k2dSO48ffgeK Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJW1aEZAAoJEHFDHex6CBG9NegH/REofNhSV54ZksMgNsZl4enk BZbhshRnnp3JctiH7wpDjtVByiITzIN70HOkrWH2vxFT66kskawKbhsWKz9ntSm2 DoOpRJWI52lvsCrbHeVG7D4q1L2Y7D51pQrhYtLqwQRsdcc7eljBCXKn+gyRv3UX C/ECGtyDOddEdyStGQKX0q22g91aqqKq7xrzGP7qTwdjSYLSOaum1b1eQEam16uF 3XsgQwFHqcVQyW2i0dEcuejeL1dQ2cTSAOfBZ8G6WjkJLUp2Ios/rZhQbyTbIhOQ 0GSmeqUnXUWGETjfzK6DUhTy1OwD5j6QTVpdhCHKoL8GhrAltqzWNOu82s2DLuI= =jgvN -----END PGP SIGNATURE----- --CE+1k2dSO48ffgeK--