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

[Paolo Bonzini <pbonzini@redhat.com>]

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
> 
>