Pull request

-----BEGIN PGP SIGNATURE-----
 
 iQEzBAABCAAdFiEEhpWov9P5fNqsNXdanKSrs4Grc8gFAl4zTL4ACgkQnKSrs4Gr
 c8j38wf9EMliB2OwzQGqmKLa+yE5lTzZLe4ou7yOR5jAwt6VfpT10PxtE2WSF0no
 KSiT8fhlY+AKx0Y0v57Qis0B9iLolPLOW7iTu07dBn5eQ7+mQiKeZi5OA+FtSq6i
 cnOug0/PRRKNtCz+U0AwEOMjDmBUkWsGuQRUe4mO+Trkb/GKdBoF7NsSqNoHJpJZ
 sv2se9WImJMV1OLmtwQ94l5hzsEhz5wWZ3n4DPa7J1qYBWjzaPydjNkIyO8kgshb
 i2FcNsL9As3jhIzuCUX8zRKzTaTJVLF8e07iInWNswKBUbt7LxbqtOZWMoTavgpu
 gsbBG3Rk6l08IM0cL5/r1PDYgroIuQ==
 =/0ID
 -----END PGP SIGNATURE-----

Merge remote-tracking branch 'remotes/stefanha/tags/tracing-pull-request' into staging

Pull request

# gpg: Signature made Thu 30 Jan 2020 21:38:06 GMT
# gpg:                using RSA key 8695A8BFD3F97CDAAC35775A9CA4ABB381AB73C8
# gpg: Good signature from "Stefan Hajnoczi <stefanha@redhat.com>" [full]
# gpg:                 aka "Stefan Hajnoczi <stefanha@gmail.com>" [full]
# Primary key fingerprint: 8695 A8BF D3F9 7CDA AC35  775A 9CA4 ABB3 81AB 73C8

* remotes/stefanha/tags/tracing-pull-request:
  qemu_set_log_filename: filename argument may be NULL
  hw/display/qxl.c: Use trace_event_get_state_backends()
  memory.c: Use trace_event_get_state_backends()
  docs/devel/tracing.txt: Recommend only trace_event_get_state_backends()
  Makefile: Keep trace-events-subdirs ordered

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Makefile.objs

@@ -132,8 +132,8 @@ trace-events-subdirs += nbd
 trace-events-subdirs += scsi
 endif
 ifeq ($(CONFIG_SOFTMMU),y)
-trace-events-subdirs += chardev
 trace-events-subdirs += audio
+trace-events-subdirs += chardev
 trace-events-subdirs += hw/9pfs
 trace-events-subdirs += hw/acpi
 trace-events-subdirs += hw/alpha
@@ -181,6 +181,7 @@ trace-events-subdirs += migration
 trace-events-subdirs += net
 trace-events-subdirs += ui
 endif
+trace-events-subdirs += hw/core
 trace-events-subdirs += hw/display
 trace-events-subdirs += qapi
 trace-events-subdirs += qom
@@ -193,7 +194,6 @@ trace-events-subdirs += target/riscv
 trace-events-subdirs += target/s390x
 trace-events-subdirs += target/sparc
 trace-events-subdirs += util
-trace-events-subdirs += hw/core
 
 trace-events-files = $(SRC_PATH)/trace-events $(trace-events-subdirs:%=$(SRC_PATH)/%/trace-events)
 

docs/devel/tracing.txt

@@ -342,8 +342,10 @@ edit the "trace-events-all" file).
 
 In addition, there might be cases where relatively complex computations must be
 performed to generate values that are only used as arguments for a trace
-function. In these cases you can use the macro 'TRACE_${EVENT_NAME}_ENABLED' to
+function. In these cases you can use 'trace_event_get_state_backends()' to
-guard such computations and avoid its compilation when the event is disabled:
+guard such computations, so they are skipped if the event has been either
+compile-time disabled or run-time disabled. If the event is compile-time
+disabled, this check will have no performance impact.
 
     #include "trace.h"  /* needed for trace event prototype */
     
@@ -356,7 +358,7 @@ guard such computations and avoid its compilation when the event is disabled:
             align = getpagesize();
         }
         ptr = qemu_memalign(align, size);
-        if (TRACE_QEMU_VMALLOC_ENABLED) { /* preprocessor macro */
+        if (trace_event_get_state_backends(TRACE_QEMU_VMALLOC)) {
             void *complex;
             /* some complex computations to produce the 'complex' value */
             trace_qemu_vmalloc(size, ptr, complex);
@@ -364,10 +366,6 @@ guard such computations and avoid its compilation when the event is disabled:
         return ptr;
     }
 
-You can check both if the event has been disabled and is dynamically enabled at
-the same time using the 'trace_event_get_state_backends' routine (see header
-"trace/control.h" for more information).
-
 === "tcg" ===
 
 Guest code generated by TCG can be traced by defining an event with the "tcg"

hw/display/qxl.c

@@ -1764,7 +1764,7 @@ async_common:
         qxl_set_mode(d, val, 0);
         break;
     case QXL_IO_LOG:
-        if (TRACE_QXL_IO_LOG_ENABLED || d->guestdebug) {
+        if (trace_event_get_state_backends(TRACE_QXL_IO_LOG) || d->guestdebug) {
             /* We cannot trust the guest to NUL terminate d->ram->log_buf */
             char *log_buf = g_strndup((const char *)d->ram->log_buf,
                                       sizeof(d->ram->log_buf));

memory.c

@@ -434,7 +434,7 @@ static MemTxResult  memory_region_read_accessor(MemoryRegion *mr,
     tmp = mr->ops->read(mr->opaque, addr, size);
     if (mr->subpage) {
         trace_memory_region_subpage_read(get_cpu_index(), mr, addr, tmp, size);
-    } else if (TRACE_MEMORY_REGION_OPS_READ_ENABLED) {
+    } else if (trace_event_get_state_backends(TRACE_MEMORY_REGION_OPS_READ)) {
         hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
         trace_memory_region_ops_read(get_cpu_index(), mr, abs_addr, tmp, size);
     }
@@ -456,7 +456,7 @@ static MemTxResult memory_region_read_with_attrs_accessor(MemoryRegion *mr,
     r = mr->ops->read_with_attrs(mr->opaque, addr, &tmp, size, attrs);
     if (mr->subpage) {
         trace_memory_region_subpage_read(get_cpu_index(), mr, addr, tmp, size);
-    } else if (TRACE_MEMORY_REGION_OPS_READ_ENABLED) {
+    } else if (trace_event_get_state_backends(TRACE_MEMORY_REGION_OPS_READ)) {
         hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
         trace_memory_region_ops_read(get_cpu_index(), mr, abs_addr, tmp, size);
     }
@@ -476,7 +476,7 @@ static MemTxResult memory_region_write_accessor(MemoryRegion *mr,
 
     if (mr->subpage) {
         trace_memory_region_subpage_write(get_cpu_index(), mr, addr, tmp, size);
-    } else if (TRACE_MEMORY_REGION_OPS_WRITE_ENABLED) {
+    } else if (trace_event_get_state_backends(TRACE_MEMORY_REGION_OPS_WRITE)) {
         hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
         trace_memory_region_ops_write(get_cpu_index(), mr, abs_addr, tmp, size);
     }
@@ -496,7 +496,7 @@ static MemTxResult memory_region_write_with_attrs_accessor(MemoryRegion *mr,
 
     if (mr->subpage) {
         trace_memory_region_subpage_write(get_cpu_index(), mr, addr, tmp, size);
-    } else if (TRACE_MEMORY_REGION_OPS_WRITE_ENABLED) {
+    } else if (trace_event_get_state_backends(TRACE_MEMORY_REGION_OPS_WRITE)) {
         hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
         trace_memory_region_ops_write(get_cpu_index(), mr, abs_addr, tmp, size);
     }

trace/control.c

@@ -229,9 +229,7 @@ void trace_init_file(const char *file)
     /* If both the simple and the log backends are enabled, "--trace file"
      * only applies to the simple backend; use "-D" for the log backend.
      */
-    if (file) {
+    qemu_set_log_filename(file, &error_fatal);
-        qemu_set_log_filename(file, &error_fatal);
-    }
 #else
     if (file) {
         fprintf(stderr, "error: --trace file=...: "

util/log.c

@@ -148,25 +148,29 @@ void qemu_log_needs_buffers(void)
  * 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
  */
 void qemu_set_log_filename(const char *filename, Error **errp)
 {
-    char *pidstr;
     g_free(logfilename);
     logfilename = NULL;
 
-    pidstr = strstr(filename, "%");
+    if (filename) {
-    if (pidstr) {
+            char *pidstr = strstr(filename, "%");
-        /* We only accept one %d, no other format strings */
+            if (pidstr) {
-        if (pidstr[1] != 'd' || strchr(pidstr + 2, '%')) {
+                /* We only accept one %d, no other format strings */
-            error_setg(errp, "Bad logfile format: %s", filename);
+                if (pidstr[1] != 'd' || strchr(pidstr + 2, '%')) {
-            return;
+                    error_setg(errp, "Bad logfile format: %s", filename);
-        } else {
+                    return;
-            logfilename = g_strdup_printf(filename, getpid());
+                } else {
-        }
+                    logfilename = g_strdup_printf(filename, getpid());
-    } else {
+                }
-        logfilename = g_strdup(filename);
+            } else {
+                logfilename = g_strdup(filename);
+            }
     }
+
     qemu_log_close();
     qemu_set_log(qemu_loglevel);
 }

vl.c

@@ -3903,10 +3903,7 @@ int main(int argc, char **argv, char **envp)
 
     /* Open the logfile at this point and set the log mask if necessary.
      */
-    if (log_file) {
+    qemu_set_log_filename(log_file, &error_fatal);
-        qemu_set_log_filename(log_file, &error_fatal);
-    }
-
     if (log_mask) {
         int mask;
         mask = qemu_str_to_log_mask(log_mask);