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

[Paolo Bonzini]

On 10/24/22 11:44, Alex Bennée 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?

log_append is used if you turn off the logging, which clears log_flags, 
and then turn it on again.  The usecase is that if you remove the file 
QEMU won't keep writing to a deleted file.  ¯\_(ツ)_/¯

However, it's messy.  In particular after changing the file name 
log_append will be 1 and that makes little sense.  The simplest thing to 
do here is just to not close the file, I sent a patch for that.

Paolo

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