qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Richard Henderson <richard.henderson@linaro.org>
To: qemu-devel@nongnu.org
Cc: "Alex Bennée" <alex.bennee@linaro.org>
Subject: [PULL 39/39] util/log: Support per-thread log files
Date: Wed, 20 Apr 2022 11:06:18 -0700	[thread overview]
Message-ID: <20220420180618.1183855-40-richard.henderson@linaro.org> (raw)
In-Reply-To: <20220420180618.1183855-1-richard.henderson@linaro.org>

Add a new log flag, tid, to turn this feature on.
Require the log filename to be set, and to contain %d.

Do not allow tid to be turned off once it is on, nor let
the filename be change thereafter.  This avoids the need
for signalling each thread to re-open on a name change.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20220417183019.755276-40-richard.henderson@linaro.org>
---
 include/qemu/log.h |   1 +
 util/log.c         | 163 +++++++++++++++++++++++++++++++++++----------
 2 files changed, 128 insertions(+), 36 deletions(-)

diff --git a/include/qemu/log.h b/include/qemu/log.h
index a325bca661..c5643d8dd5 100644
--- a/include/qemu/log.h
+++ b/include/qemu/log.h
@@ -34,6 +34,7 @@ bool qemu_log_separate(void);
 #define CPU_LOG_PLUGIN     (1 << 18)
 /* LOG_STRACE is used for user-mode strace logging. */
 #define LOG_STRACE         (1 << 19)
+#define LOG_PER_THREAD     (1 << 20)
 
 /* Lock/unlock output. */
 
diff --git a/util/log.c b/util/log.c
index df0710720f..d6eb0378c3 100644
--- a/util/log.c
+++ b/util/log.c
@@ -27,6 +27,9 @@
 #include "qemu/thread.h"
 #include "qemu/lockable.h"
 #include "qemu/rcu.h"
+#ifdef CONFIG_LINUX
+#include <sys/syscall.h>
+#endif
 
 
 typedef struct RCUCloseFILE {
@@ -38,22 +41,40 @@ typedef struct RCUCloseFILE {
 static QemuMutex global_mutex;
 static char *global_filename;
 static FILE *global_file;
+static __thread FILE *thread_file;
 
 int qemu_loglevel;
-static int log_append = 0;
+static bool log_append;
+static bool log_per_thread;
 static GArray *debug_regions;
 
 /* Returns true if qemu_log() will really write somewhere. */
 bool qemu_log_enabled(void)
 {
-    return qatomic_read(&global_file) != NULL;
+    return log_per_thread || qatomic_read(&global_file) != NULL;
 }
 
 /* Returns true if qemu_log() will write somewhere other than stderr. */
 bool qemu_log_separate(void)
 {
-    FILE *logfile = qatomic_read(&global_file);
-    return logfile && logfile != stderr;
+    if (log_per_thread) {
+        return true;
+    } else {
+        FILE *logfile = qatomic_read(&global_file);
+        return logfile && logfile != stderr;
+    }
+}
+
+static int log_thread_id(void)
+{
+#ifdef CONFIG_GETTID
+    return gettid();
+#elif defined(SYS_gettid)
+    return syscall(SYS_gettid);
+#else
+    static int counter;
+    return qatomic_fetch_inc(&counter);
+#endif
 }
 
 /* Lock/unlock output. */
@@ -62,20 +83,34 @@ FILE *qemu_log_trylock(void)
 {
     FILE *logfile;
 
-    rcu_read_lock();
-    /*
-     * FIXME: typeof_strip_qual, as used by qatomic_rcu_read,
-     * does not work with pointers to undefined structures,
-     * such as we have with struct _IO_FILE and musl libc.
-     * Since all we want is a read of a pointer, cast to void**,
-     * which does work with typeof_strip_qual.
-     */
-    logfile = qatomic_rcu_read((void **)&global_file);
-    if (logfile) {
-        qemu_flockfile(logfile);
-    } else {
-        rcu_read_unlock();
+    logfile = thread_file;
+    if (!logfile) {
+        if (log_per_thread) {
+            g_autofree char *filename
+                = g_strdup_printf(global_filename, log_thread_id());
+            logfile = fopen(filename, "w");
+            if (!logfile) {
+                return NULL;
+            }
+            thread_file = logfile;
+        } else {
+            rcu_read_lock();
+            /*
+             * FIXME: typeof_strip_qual, as used by qatomic_rcu_read,
+             * does not work with pointers to undefined structures,
+             * such as we have with struct _IO_FILE and musl libc.
+             * Since all we want is a read of a pointer, cast to void**,
+             * which does work with typeof_strip_qual.
+             */
+            logfile = qatomic_rcu_read((void **)&global_file);
+            if (!logfile) {
+                rcu_read_unlock();
+                return NULL;
+            }
+        }
     }
+
+    qemu_flockfile(logfile);
     return logfile;
 }
 
@@ -84,7 +119,9 @@ void qemu_log_unlock(FILE *logfile)
     if (logfile) {
         fflush(logfile);
         qemu_funlockfile(logfile);
-        rcu_read_unlock();
+        if (!log_per_thread) {
+            rcu_read_unlock();
+        }
     }
 }
 
@@ -112,40 +149,81 @@ static void rcu_close_file(RCUCloseFILE *r)
     g_free(r);
 }
 
+/**
+ * valid_filename_template:
+ *
+ * Validate the filename template.  Require %d if per_thread, allow it
+ * otherwise; require no other % within the template.
+ */
+
+typedef enum {
+    vft_error,
+    vft_stderr,
+    vft_strdup,
+    vft_pid_printf,
+} ValidFilenameTemplateResult;
+
+static ValidFilenameTemplateResult
+valid_filename_template(const char *filename, bool per_thread, Error **errp)
+{
+    if (filename) {
+        char *pidstr = strstr(filename, "%");
+
+        if (pidstr) {
+            /* We only accept one %d, no other format strings */
+            if (pidstr[1] != 'd' || strchr(pidstr + 2, '%')) {
+                error_setg(errp, "Bad logfile template: %s", filename);
+                return 0;
+            }
+            return per_thread ? vft_strdup : vft_pid_printf;
+        }
+    }
+    if (per_thread) {
+        error_setg(errp, "Filename template with '%%d' required for 'tid'");
+        return vft_error;
+    }
+    return filename ? vft_strdup : vft_stderr;
+}
+
 /* enable or disable low levels log */
 static bool qemu_set_log_internal(const char *filename, bool changed_name,
                                   int log_flags, Error **errp)
 {
     bool need_to_open_file;
     bool daemonized;
+    bool per_thread;
     FILE *logfile;
 
     QEMU_LOCK_GUARD(&global_mutex);
     logfile = global_file;
 
+    per_thread = log_flags & LOG_PER_THREAD;
+
     if (changed_name) {
         char *newname = NULL;
 
         /*
-         * Allow the user to include %d in their logfile which will be
-         * substituted with the current PID. This is useful for debugging many
-         * nested linux-user tasks but will result in lots of logs.
-         *
-         * filename may be NULL. In that case, log output is sent to stderr
+         * Once threads start opening their own log files, we have no
+         * easy mechanism to tell them all to close and re-open.
+         * There seems little cause to do so either -- this option
+         * will most often be used at user-only startup.
          */
-        if (filename) {
-            char *pidstr = strstr(filename, "%");
+        if (log_per_thread) {
+            error_setg(errp, "Cannot change log filename after setting 'tid'");
+            return false;
+        }
 
-            if (pidstr) {
-                /* We only accept one %d, no other format strings */
-                if (pidstr[1] != 'd' || strchr(pidstr + 2, '%')) {
-                    error_setg(errp, "Bad logfile format: %s", filename);
-                    return false;
-                }
-                newname = g_strdup_printf(filename, getpid());
-            } else {
-                newname = g_strdup(filename);
-            }
+        switch (valid_filename_template(filename, per_thread, errp)) {
+        case vft_error:
+            return false;
+        case vft_stderr:
+            break;
+        case vft_strdup:
+            newname = g_strdup(filename);
+            break;
+        case vft_pid_printf:
+            newname = g_strdup_printf(filename, getpid());
+            break;
         }
 
         g_free(global_filename);
@@ -153,8 +231,18 @@ static bool qemu_set_log_internal(const char *filename, bool changed_name,
         filename = newname;
     } else {
         filename = global_filename;
+        if (per_thread &&
+            valid_filename_template(filename, true, errp) == vft_error) {
+            return false;
+        }
     }
 
+    /* Once the per-thread flag is set, it cannot be unset. */
+    if (per_thread) {
+        log_per_thread = true;
+    }
+    /* The flag itself is not relevant for need_to_open_file. */
+    log_flags &= ~LOG_PER_THREAD;
 #ifdef CONFIG_TRACE_LOG
     log_flags |= LOG_TRACE;
 #endif
@@ -163,12 +251,13 @@ static bool qemu_set_log_internal(const char *filename, bool changed_name,
     /*
      * In all cases we only log if qemu_loglevel is set.
      * Also:
+     *   If per-thread, open the file for each thread in qemu_log_lock.
      *   If not daemonized we will always log either to stderr
      *     or to a file (if there is a filename).
      *   If we are daemonized, we will only log if there is a filename.
      */
     daemonized = is_daemonized();
-    need_to_open_file = log_flags && (!daemonized || filename);
+    need_to_open_file = log_flags && !per_thread && (!daemonized || filename);
 
     if (logfile && (!need_to_open_file || changed_name)) {
         qatomic_rcu_set(&global_file, NULL);
@@ -354,6 +443,8 @@ const QEMULogItem qemu_log_items[] = {
 #endif
     { LOG_STRACE, "strace",
       "log every user-mode syscall, its input, and its result" },
+    { LOG_PER_THREAD, "tid",
+      "open a separate log file per thread; filename must contain '%d'" },
     { 0, NULL, NULL },
 };
 
-- 
2.34.1



  parent reply	other threads:[~2022-04-20 18:52 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-20 18:05 [PULL 00/39] Logging cleanup and per-thread logfiles Richard Henderson
2022-04-20 18:05 ` [PULL 01/39] util/log: Drop manual log buffering Richard Henderson
2022-04-20 18:05 ` [PULL 02/39] target/hexagon: Remove qemu_set_log in hexagon_translate_init Richard Henderson
2022-04-20 18:05 ` [PULL 03/39] util/log: Return bool from qemu_set_log_filename Richard Henderson
2022-04-20 18:05 ` [PULL 04/39] util/log: Pass Error pointer to qemu_set_log Richard Henderson
2022-04-20 18:05 ` [PULL 05/39] os-posix: Use qemu_log_enabled Richard Henderson
2022-04-20 18:05 ` [PULL 06/39] util/log: Move qemu_log_lock, qemu_log_unlock out of line Richard Henderson
2022-04-20 18:05 ` [PULL 07/39] util/log: Rename qemu_log_lock to qemu_log_trylock Richard Henderson
2022-04-20 18:05 ` [PULL 08/39] hw/xen: Split out xen_pv_output_msg Richard Henderson
2022-04-20 18:05 ` [PULL 09/39] *: Use fprintf between qemu_log_trylock/unlock Richard Henderson
2022-04-20 18:05 ` [PULL 10/39] util/log: Remove qemu_log_vprintf Richard Henderson
2022-04-20 18:05 ` [PULL 11/39] tcg: Pass the locked filepointer to tcg_dump_ops Richard Henderson
2022-04-20 18:05 ` [PULL 12/39] exec/translator: Pass the locked filepointer to disas_log hook Richard Henderson
2022-04-20 18:05 ` [PULL 13/39] exec/log: Remove log_disas and log_target_disas Richard Henderson
2022-04-20 18:05 ` [PULL 14/39] accel/tcg: Use cpu_dump_state between qemu_log_trylock/unlock Richard Henderson
2022-04-20 18:05 ` [PULL 15/39] target/nios2: Remove log_cpu_state from reset Richard Henderson
2022-04-20 18:05 ` [PULL 16/39] util/log: Use qemu_log_trylock/unlock in qemu_log Richard Henderson
2022-04-20 18:05 ` [PULL 17/39] util/log: Drop return value from qemu_log Richard Henderson
2022-04-20 18:05 ` [PULL 18/39] util/log: Mark qemu_log_trylock as G_GNUC_WARN_UNUSED_RESULT Richard Henderson
2022-04-20 18:05 ` [PULL 19/39] util/log: Remove qemu_log_flush Richard Henderson
2022-04-20 18:05 ` [PULL 20/39] util/log: Drop call to setvbuf Richard Henderson
2022-04-20 18:06 ` [PULL 21/39] bsd-user: Expand log_page_dump inline Richard Henderson
2022-04-20 18:06 ` [PULL 22/39] linux-user: " Richard Henderson
2022-04-20 18:06 ` [PULL 23/39] tests/unit: Do not reference QemuLogFile directly Richard Henderson
2022-04-20 18:06 ` [PULL 24/39] include/exec/log: " Richard Henderson
2022-04-20 18:06 ` [PULL 25/39] include/qemu/log: Move entire implementation out-of-line Richard Henderson
2022-04-20 18:06 ` [PULL 26/39] sysemu/os-win32: Test for and use _lock_file/_unlock_file Richard Henderson
2022-04-20 18:06 ` [PULL 27/39] util/log: Introduce qemu_set_log_filename_flags Richard Henderson
2022-04-20 18:06 ` [PULL 28/39] bsd-user: Use qemu_set_log_filename_flags Richard Henderson
2022-04-20 18:06 ` [PULL 29/39] linux-user: " Richard Henderson
2022-04-20 18:06 ` [PULL 30/39] softmmu: " Richard Henderson
2022-04-20 18:06 ` [PULL 31/39] util/log: Remove qemu_log_close Richard Henderson
2022-04-20 18:06 ` [PULL 32/39] util/log: Rename logfilename to global_filename Richard Henderson
2022-04-20 18:06 ` [PULL 33/39] util/log: Rename qemu_logfile to global_file Richard Henderson
2022-04-20 18:06 ` [PULL 34/39] util/log: Rename qemu_logfile_mutex to global_mutex Richard Henderson
2022-04-20 18:06 ` [PULL 35/39] util/log: Hoist the eval of is_daemonized in qemu_set_log_internal Richard Henderson
2022-04-20 18:06 ` [PULL 36/39] util/log: Combine two logfile closes Richard Henderson
2022-04-20 18:06 ` [PULL 37/39] util/log: Rename QemuLogFile to RCUCloseFILE Richard Henderson
2022-04-20 18:06 ` [PULL 38/39] util/log: Limit RCUCloseFILE to file closing Richard Henderson
2022-04-20 18:06 ` Richard Henderson [this message]
2022-04-20 23:42 ` [PULL 00/39] Logging cleanup and per-thread logfiles Richard Henderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220420180618.1183855-40-richard.henderson@linaro.org \
    --to=richard.henderson@linaro.org \
    --cc=alex.bennee@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).