On 10/14/22 08:08, Greg Kurz wrote:
+ need_to_open_file = log_flags && !per_thread;
Pre-existing, but I think this should check log_per_thread instead of
per_thread.
+ } else if (filename) {
+ /*
+ * If we are daemonized, we will only log if there is a filename.
+ */
+ need_to_open_file = true;
Slightly nicer:
} else {
/*
* If daemonized, always log to the -D file if present.
*/
need_to_open_file = filename != NULL;
}
@@ -271,10 +276,22 @@ static bool qemu_set_log_internal(const char *filename,
bool changed_name,
if (!logfile && need_to_open_file) {
if (filename) {
- logfile = fopen(filename, log_append ? "a" : "w");
+ g_autofree char *fname = NULL;
+
+ /*
+ * 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);
+ }
+
+ logfile = fopen(fname, log_append ? "a" : "w");
if (!logfile) {
error_setg_errno(errp, errno, "Error opening logfile %s",
- filename);
+ fname);
return false;
}
/* In case we are a daemon redirect stderr to logfile */
This could conflict with the file opened by qemu_log_trylock() when
per-thread logging is enabled *and* QEMU is daemonized. Perhaps
something like:
1) change qemu_log_trylock() to
- if (log_per_thread) {
+ if (log_per_thread && log_thread_id() != getpid()) {
i.e. use the global_file for the main thread
2) change qemu_log_unlock() to
- if (!log_per_thread) {
+ if (!thread_file) {
to match (1)
3) change log_thread_id() to something like
...
#else
static __thread int my_id = -1;
static int counter;
if (my_id == -1) {
my_id = getpid() + qatomic_fetch_inc(&counter);
}
return my_id;
#endif
and perhaps do a dummy trylock/unlock late in qemu_set_log_internal(),
to ensure that the main thread is the one with log_thread_id() == getpid()?
I think this can be a separate patch before this one.
Paolo
