Re: [PATCH v2 2/2] util/log: Always send errors to logfile when daemonized

[Greg Kurz]

On Mon, 24 Oct 2022 10:44:11 +0100
Alex Bennée <alex.bennee@linaro.org> wrote:

> 
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
> <snip>
> >> If we want to connect stdout/err to something when daemonized
> >> then lets either have a dedicated option for that, or simply
> >> tell apps not to use -daemonize and to take care of daemonzing
> >> themselves, thus having full control over stdout/err. The latter
> >> is what libvirt uses, because we actually want stderr/out on a
> >> pipe, not a file, in order to enforce rollover.
> >
> > I would gladly get rid of -daemonize, unfortunately it has many users.
> > Adding further complication to it is not beautiful, but overall I
> > think Greg's patch does make sense.  In particular I would continue
> > the refactoring by moving
> >
> >
> >             /*
> >              * If per-thread, filename contains a single %d that should be
> >              * converted.
> >              */
> >             if (per_thread) {
> >                 fname = g_strdup_printf(filename, getpid());
> >             } else {
> >                 fname = g_strdup(filename);
> >             }
> >
> >             return fopen(fname, log_append ? "a" : "w");
> >
> > to a new function that can be used in both qemu_log_trylock() and
> > qemu_set_log_internal().  (In fact this refactoring is a bugfix
> > because per-thread log files do not currently obey log_append).
> 
> What is the use case for log_append. AFAICT it only ever applied if you
> did a dynamic set_log. Was it ever really used or should it be dropped
> as an excessive complication?
> 

The use case seems to be able to temporarily disable logging,
which closes the log file, without loosing already logged stuff
when logging is re-enabled. QEMU not overwriting previous logs
from the same run is certainly a legitimate expectation from the
user.

Complexity mostly stems from the fact that the log file gets closed
when doing `log none` from the monitor. The logic is also a bit
inconsistent : initial open ensures that we go with a pristine log
file, but renaming the file from the monitor will gladly append
messages to a pre-existing unrelated file...

> From my point of view appending to an existing per-thread log is just
> going to cause confusion.
> 

... and cause confusion all the same.

I'd rather leave the log file always open, except on renames,
and always open in truncating mode.

> >
> > Paolo
> 
>