On Thu, Oct 20, 2022 at 11:49:37AM +0200, Greg Kurz wrote: > On Thu, 20 Oct 2022 12:21:27 +1000 > Richard Henderson <richard.hender...@linaro.org> wrote: > > > On 10/20/22 01:16, Greg Kurz wrote: > > > > I don't understand why daemonize changes -d tid at all. > > If there's a bug there, please separate it out. > > > > I don't understand the is_main_log_thread checks. > > Why is the main thread special? > > > > The current code base either opens a per-thread file in > qemu_log_trylock() when -d tid is enabled, or only a > single global file in qemu_log_set_internal() in the > opposite case. > > The goal of this patch is to go through the `In case we > are a daemon redirect stderr to logfile` logic, so that > other users of stderr, aka. error_report(), can benefit > from it as well. Since this is only done for the global > file, the logic was changed to : _main_ thread to always > use the global file and other threads to use the per-thread > file. > > I now realize how terrible a choice this is. It violates > the current logic too much and brings new problems like > "how to identify the main thread"...
snip > > I would have thought that this was the only change required -- ignoring > > qemu_loglevel when > > daemonized. > > > > I was thinking the same at first, but this ended up in the > global file being open with a filename containing a '%d'... > I chose the direction of doing the g_strdup_printf() trick > for the global file as well but then I had to make sure > that qemu_log_trylock() wouldn't try later to open the same > file, hence the _main_ thread check... > > The question is actually : where stderr should point to in > the '-daemonize -D foo%d.log -d tid` case ? I'm tending towards thinking the question is wrong, because it is imposing semantics on -D that it was never designed to address. The '-d' option enables logging in QEMU, primary for things related to TCG. By default that logging goes to stderr, but it can be sent to 1 or mnay files, using -D. IOW, -D is NOT about controlling where stderr/out is connected. It is about where TCG logging goes. Separately, IIUC, you found that when using -daemonize any error_report() messages end up in the void, because stderr is connected to /dev/null. This patch is thus attempting to repurpose -D as a way to say where error_report() messages end up with daemonized, and this creates the complexity because -D was never intended to be a mechanism to control stderr or error_report output. 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. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
