On Tue, 25 Oct 2022 11:21:19 +0200 Paolo Bonzini <pbonz...@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.ben...@linaro.org> > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> > --- Heh I currently have a very similar patch in my tree :-) Reviewed-by: Greg Kurz <gr...@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;