[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] util/log: do not close and reopen log files when flags are t
From: |
Greg Kurz |
Subject: |
Re: [PATCH] util/log: do not close and reopen log files when flags are turned off |
Date: |
Tue, 25 Oct 2022 14:33:15 +0200 |
On Tue, 25 Oct 2022 11:21:19 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:
> log_append makes sure that if you turn off the logging (which clears
> log_flags and makes need_to_open_file false) the old log is not
> overwritten. The usecase is that if you remove or move the file
> QEMU will not keep writing to the old file. However, this is
> not always the desited behavior, in particular having log_append==1
> after changing the file name makes little sense.
>
> When qemu_set_log_internal is called from the logfile monitor
> command, filename must be non-NULL and therefore changed_name must
> be true. Therefore, the only case where the file is closed and
> need_to_open_file == false is indeed when log_flags becomes
> zero. In this case, just flush the file and do not bother
> closing it, thus faking the same append behavior as previously.
>
> The behavioral change is that changing the logfile twice, for
> example log1 -> log2 -> log1, will cause log1 to be overwritten.
> This can simply be documented, since it is not a particularly
> surprising behavior.
>
> Suggested-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
Heh I currently have a very similar patch in my tree :-)
Reviewed-by: Greg Kurz <groug@kaod.org>
I'll include this and other bug fixes as prerequisites for my
on-going work on logging when daemonized.
> util/log.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/util/log.c b/util/log.c
> index d6eb0378c3a3..06d0173788dc 100644
> --- a/util/log.c
> +++ b/util/log.c
> @@ -44,7 +44,6 @@ static FILE *global_file;
> static __thread FILE *thread_file;
>
> int qemu_loglevel;
> -static bool log_append;
> static bool log_per_thread;
> static GArray *debug_regions;
>
> @@ -259,19 +258,19 @@ static bool qemu_set_log_internal(const char *filename,
> bool changed_name,
> daemonized = is_daemonized();
> need_to_open_file = log_flags && !per_thread && (!daemonized ||
> filename);
>
> - if (logfile && (!need_to_open_file || changed_name)) {
> - qatomic_rcu_set(&global_file, NULL);
> - if (logfile != stderr) {
> + if (logfile) {
> + fflush(logfile);
> + if (changed_name && logfile != stderr) {
> RCUCloseFILE *r = g_new0(RCUCloseFILE, 1);
> r->fd = logfile;
> call_rcu(r, rcu_close_file, rcu);
> + logfile = NULL;
> }
> - logfile = NULL;
> }
>
> if (!logfile && need_to_open_file) {
> if (filename) {
> - logfile = fopen(filename, log_append ? "a" : "w");
> + logfile = fopen(filename, "w");
> if (!logfile) {
> error_setg_errno(errp, errno, "Error opening logfile %s",
> filename);
> @@ -290,8 +289,6 @@ static bool qemu_set_log_internal(const char *filename,
> bool changed_name,
> logfile = stderr;
> }
>
> - log_append = 1;
> -
> qatomic_rcu_set(&global_file, logfile);
> }
> return true;