Hi all, * Paolo Bonzini <pbonz...@redhat.com> [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.
>
> 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.
>
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 |= 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 != NULL : qemu_loglevel)) {
if (logfilename) {
@@ -91,6 +95,10 @@ void do_qemu_set_log(int log_flags, bool use_own_buffers)
log_append = 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 == NULL : !qemu_loglevel)) {
qemu_log_close();
Thanks for this quick fix and sorry again for any trouble caused in the first
place.
dimara
> Reported-by: Jan Tomko <jto...@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> util/log.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> 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_buffers)
> #ifdef CONFIG_TRACE_LOG
> qemu_loglevel |= LOG_TRACE;
> #endif
> - if ((qemu_loglevel || is_daemonized()) && !qemu_logfile) {
> + if (!qemu_logfile &&
> + (is_daemonized() ? logfilename != NULL : qemu_loglevel)) {
> if (logfilename) {
> qemu_logfile = fopen(logfilename, log_append ? "a" : "w");
> if (!qemu_logfile) {
> @@ -72,6 +73,7 @@ void do_qemu_set_log(int log_flags, bool use_own_buffers)
> }
> } else {
> /* Default to stderr if no log file specified */
> + assert(!is_daemonized());
> qemu_logfile = 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_buffers)
> log_append = 1;
> }
> }
> - if (!qemu_loglevel && !is_daemonized() && qemu_logfile) {
> + if (qemu_logfile &&
> + (is_daemonized() ? logfilename == NULL : !qemu_loglevel)) {
> qemu_log_close();
> }
> }
> --
> 2.5.0
>
signature.asc
Description: Digital signature
