* [Qemu-devel] [PATCH v3 0/4] trace: make systemtap easier to use for simple logging
@ 2019-01-22 16:49 Daniel P. Berrangé
2019-01-22 16:49 ` [Qemu-devel] [PATCH v3 1/4] display: ensure qxl log_buf is a nul terminated string Daniel P. Berrangé
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Daniel P. Berrangé @ 2019-01-22 16:49 UTC (permalink / raw)
To: qemu-devel
Cc: Gerd Hoffmann, Eric Blake, Alex Williamson, Stefan Hajnoczi,
Daniel P. Berrangé
This is a followup to
v1: https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg04173.html
v2: https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg04759.html
Changed in v3:
- g_strndup the QXL log_buf before printing to avoid a race
with guest removing the nul terminator (Stefan)
- Add filtering of PIDs with "-p PID" arg to qemu-trace-stap
- Add a man page for qemu-trace-stap (Eric)
- Remove examples in docs/devel/tracing.txt and just refer
people to the above man page.
- Use strerror(errno) instead of raw errno in vfio traces (Eric/Alex)
- Quote probe wildcards to be shell glob safe (Eric)
- Mention explicitly that it is launched separately from QEMU (Eric)
Changed in v2:
- Fix safety of QXL logging
- Handle format specifier macros is a more reliable manner
- Fix trace-events files missing newline
- Remove use of %m formats
Daniel P. Berrangé (4):
display: ensure qxl log_buf is a nul terminated string
trace: enforce that every trace-events file has a final newline
trace: forbid use of %m in trace event format strings
trace: add ability to do simple printf logging via systemtap
MAINTAINERS | 1 +
Makefile | 10 ++
Makefile.target | 11 +-
docs/devel/tracing.txt | 4 +
hw/display/qxl.c | 14 ++-
hw/display/trace-events | 2 +-
hw/gpio/trace-events | 2 +-
hw/vfio/pci.c | 2 +-
hw/vfio/trace-events | 2 +-
scripts/qemu-trace-stap | 174 +++++++++++++++++++++++++++
scripts/qemu-trace-stap.texi | 139 +++++++++++++++++++++
scripts/tracetool/__init__.py | 6 +
scripts/tracetool/format/log_stap.py | 127 +++++++++++++++++++
13 files changed, 485 insertions(+), 9 deletions(-)
create mode 100755 scripts/qemu-trace-stap
create mode 100644 scripts/qemu-trace-stap.texi
create mode 100644 scripts/tracetool/format/log_stap.py
--
2.20.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH v3 1/4] display: ensure qxl log_buf is a nul terminated string
2019-01-22 16:49 [Qemu-devel] [PATCH v3 0/4] trace: make systemtap easier to use for simple logging Daniel P. Berrangé
@ 2019-01-22 16:49 ` Daniel P. Berrangé
2019-01-22 17:23 ` Eric Blake
2019-01-22 16:49 ` [Qemu-devel] [PATCH v3 2/4] trace: enforce that every trace-events file has a final newline Daniel P. Berrangé
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Daniel P. Berrangé @ 2019-01-22 16:49 UTC (permalink / raw)
To: qemu-devel
Cc: Gerd Hoffmann, Eric Blake, Alex Williamson, Stefan Hajnoczi,
Daniel P. Berrangé
The QXL_IO_LOG command allows the guest to send log messages to the host
via a buffer in the QXLRam struct. QEMU prints these to the console if
the qxl 'guestdebug' option is set to non-zero. It will also feed them
to the trace subsystem if any backends are built-in.
In both cases the log_buf data will get treated as being as a nul
terminated string, by the printf '%s' format specifier and / or other
code reading the buffer.
QEMU does nothing to guarantee that the log_buf really is nul terminated,
so there is potential for out of bounds array access.
This would affect any QEMU which has the log, syslog or ftrace trace
backends built into QEMU. It can only be triggered if the 'qxl_io_log'
trace event is enabled, however, so they are not vulnerable without
specific administrative action to enable this.
It would also affect QEMU if the 'guestdebug' parameter is set to a
non-zero value, which again is not the default and requires explicit
admin opt-in.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
hw/display/qxl.c | 14 ++++++++++----
hw/display/trace-events | 2 +-
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 8e9a65e75b..da8fd5a40a 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1763,10 +1763,16 @@ async_common:
qxl_set_mode(d, val, 0);
break;
case QXL_IO_LOG:
- trace_qxl_io_log(d->id, d->ram->log_buf);
- if (d->guestdebug) {
- fprintf(stderr, "qxl/guest-%d: %" PRId64 ": %s", d->id,
- qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), d->ram->log_buf);
+ if (TRACE_QXL_IO_LOG_ENABLED || 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));
+ trace_qxl_io_log(d->id, log_buf);
+ if (d->guestdebug) {
+ fprintf(stderr, "qxl/guest-%d: %" PRId64 ": %s", d->id,
+ qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), log_buf);
+ }
+ g_free(log_buf);
}
break;
case QXL_IO_RESET:
diff --git a/hw/display/trace-events b/hw/display/trace-events
index 5a48c6cb6a..387c6b8931 100644
--- a/hw/display/trace-events
+++ b/hw/display/trace-events
@@ -72,7 +72,7 @@ qxl_interface_update_area_complete_rest(int qid, uint32_t num_updated_rects) "%d
qxl_interface_update_area_complete_overflow(int qid, int max) "%d max=%d"
qxl_interface_update_area_complete_schedule_bh(int qid, uint32_t num_dirty) "%d #dirty=%d"
qxl_io_destroy_primary_ignored(int qid, const char *mode) "%d %s"
-qxl_io_log(int qid, const uint8_t *log_buf) "%d %s"
+qxl_io_log(int qid, const char *log_buf) "%d %s"
qxl_io_read_unexpected(int qid) "%d"
qxl_io_unexpected_vga_mode(int qid, uint64_t addr, uint64_t val, const char *desc) "%d 0x%"PRIx64"=%"PRIu64" (%s)"
qxl_io_write(int qid, const char *mode, uint64_t addr, const char *aname, uint64_t val, unsigned size, int async) "%d %s addr=%"PRIu64 " (%s) val=%"PRIu64" size=%u async=%d"
--
2.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH v3 2/4] trace: enforce that every trace-events file has a final newline
2019-01-22 16:49 [Qemu-devel] [PATCH v3 0/4] trace: make systemtap easier to use for simple logging Daniel P. Berrangé
2019-01-22 16:49 ` [Qemu-devel] [PATCH v3 1/4] display: ensure qxl log_buf is a nul terminated string Daniel P. Berrangé
@ 2019-01-22 16:49 ` Daniel P. Berrangé
2019-01-22 16:49 ` [Qemu-devel] [PATCH v3 3/4] trace: forbid use of %m in trace event format strings Daniel P. Berrangé
2019-01-22 16:49 ` [Qemu-devel] [PATCH v3 4/4] trace: add ability to do simple printf logging via systemtap Daniel P. Berrangé
3 siblings, 0 replies; 9+ messages in thread
From: Daniel P. Berrangé @ 2019-01-22 16:49 UTC (permalink / raw)
To: qemu-devel
Cc: Gerd Hoffmann, Eric Blake, Alex Williamson, Stefan Hajnoczi,
Daniel P. Berrangé
When generating the trace-events-all file, the build system simply
concatenates all the individual trace-events files. If any one of those
files does not have a final newline, the printf format string will have
the contents of the first line of the next file appended to it, which is
usually a '#' comment.
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
hw/gpio/trace-events | 2 +-
scripts/tracetool/__init__.py | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/gpio/trace-events b/hw/gpio/trace-events
index cb41a89756..5d4dd200c2 100644
--- a/hw/gpio/trace-events
+++ b/hw/gpio/trace-events
@@ -4,4 +4,4 @@
nrf51_gpio_read(uint64_t offset, uint64_t r) "offset 0x%" PRIx64 " value 0x%" PRIx64
nrf51_gpio_write(uint64_t offset, uint64_t value) "offset 0x%" PRIx64 " value 0x%" PRIx64
nrf51_gpio_set(int64_t line, int64_t value) "line %" PRIi64 " value %" PRIi64
-nrf51_gpio_update_output_irq(int64_t line, int64_t value) "line %" PRIi64 " value %" PRIi64
\ No newline at end of file
+nrf51_gpio_update_output_irq(int64_t line, int64_t value) "line %" PRIi64 " value %" PRIi64
diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index 0e3c9e146c..3478ac93ab 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -350,6 +350,8 @@ def read_events(fobj, fname):
events = []
for lineno, line in enumerate(fobj, 1):
+ if line[-1] != '\n':
+ raise ValueError("%s does not end with a new line" % fname)
if not line.strip():
continue
if line.lstrip().startswith('#'):
--
2.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH v3 3/4] trace: forbid use of %m in trace event format strings
2019-01-22 16:49 [Qemu-devel] [PATCH v3 0/4] trace: make systemtap easier to use for simple logging Daniel P. Berrangé
2019-01-22 16:49 ` [Qemu-devel] [PATCH v3 1/4] display: ensure qxl log_buf is a nul terminated string Daniel P. Berrangé
2019-01-22 16:49 ` [Qemu-devel] [PATCH v3 2/4] trace: enforce that every trace-events file has a final newline Daniel P. Berrangé
@ 2019-01-22 16:49 ` Daniel P. Berrangé
2019-01-22 17:25 ` Eric Blake
2019-01-22 16:49 ` [Qemu-devel] [PATCH v3 4/4] trace: add ability to do simple printf logging via systemtap Daniel P. Berrangé
3 siblings, 1 reply; 9+ messages in thread
From: Daniel P. Berrangé @ 2019-01-22 16:49 UTC (permalink / raw)
To: qemu-devel
Cc: Gerd Hoffmann, Eric Blake, Alex Williamson, Stefan Hajnoczi,
Daniel P. Berrangé
The '%m' format specifier instructs glibc's printf() implementation to
insert the contents of strerror(errno). Since this is a glibc extension
it should generally be avoided in QEMU due to need for portability to a
variety of platforms.
Even though vfio is Linux-only code that could otherwise use "%m", it
must still be avoided in trace-events files because several of the
backends do not use the format string and so this error information is
invisible to them.
The errno string value should be given as an explicit trace argument
instead, making it accessible to all backends. This also allows it to
work correctly with future patches that use the format string with
systemtap's simple printf code.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
hw/vfio/pci.c | 2 +-
hw/vfio/trace-events | 2 +-
scripts/tracetool/__init__.py | 4 ++++
3 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index c0cb1ec289..dd12f36391 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2581,7 +2581,7 @@ static void vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_GET_IRQ_INFO, &irq_info);
if (ret) {
/* This can fail for an old kernel or legacy PCI dev */
- trace_vfio_populate_device_get_irq_info_failure();
+ trace_vfio_populate_device_get_irq_info_failure(strerror(errno));
} else if (irq_info.count == 1) {
vdev->pci_aer = true;
} else {
diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
index a85e8662ea..f41ca96160 100644
--- a/hw/vfio/trace-events
+++ b/hw/vfio/trace-events
@@ -37,7 +37,7 @@ vfio_pci_hot_reset_has_dep_devices(const char *name) "%s: hot reset dependent de
vfio_pci_hot_reset_dep_devices(int domain, int bus, int slot, int function, int group_id) "\t%04x:%02x:%02x.%x group %d"
vfio_pci_hot_reset_result(const char *name, const char *result) "%s hot reset: %s"
vfio_populate_device_config(const char *name, unsigned long size, unsigned long offset, unsigned long flags) "Device %s config:\n size: 0x%lx, offset: 0x%lx, flags: 0x%lx"
-vfio_populate_device_get_irq_info_failure(void) "VFIO_DEVICE_GET_IRQ_INFO failure: %m"
+vfio_populate_device_get_irq_info_failure(const char *errstr) "VFIO_DEVICE_GET_IRQ_INFO failure: %s"
vfio_realize(const char *name, int group_id) " (%s) group %d"
vfio_mdev(const char *name, bool is_mdev) " (%s) is_mdev %d"
vfio_add_ext_cap_dropped(const char *name, uint16_t cap, uint16_t offset) "%s 0x%x@0x%x"
diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index 3478ac93ab..6fca674936 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -274,6 +274,10 @@ class Event(object):
props = groups["props"].split()
fmt = groups["fmt"]
fmt_trans = groups["fmt_trans"]
+ if fmt.find("%m") != -1 or fmt_trans.find("%m") != -1:
+ raise ValueError("Event format '%m' is forbidden, pass the error "
+ "as an explicit trace argument")
+
if len(fmt_trans) > 0:
fmt = [fmt_trans, fmt]
args = Arguments.build(groups["args"])
--
2.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH v3 4/4] trace: add ability to do simple printf logging via systemtap
2019-01-22 16:49 [Qemu-devel] [PATCH v3 0/4] trace: make systemtap easier to use for simple logging Daniel P. Berrangé
` (2 preceding siblings ...)
2019-01-22 16:49 ` [Qemu-devel] [PATCH v3 3/4] trace: forbid use of %m in trace event format strings Daniel P. Berrangé
@ 2019-01-22 16:49 ` Daniel P. Berrangé
2019-01-22 17:42 ` Eric Blake
3 siblings, 1 reply; 9+ messages in thread
From: Daniel P. Berrangé @ 2019-01-22 16:49 UTC (permalink / raw)
To: qemu-devel
Cc: Gerd Hoffmann, Eric Blake, Alex Williamson, Stefan Hajnoczi,
Daniel P. Berrangé
The dtrace systemtap trace backend for QEMU is very powerful but it is
also somewhat unfriendly to users who aren't familiar with systemtap,
or who don't need its power right now.
stap -e "....some strange script...."
The 'log' backend for QEMU by comparison is very crude but incredibly
easy to use:
$ qemu -d trace:qio* ...some args...
23266@1547735759.137292:qio_channel_socket_new Socket new ioc=0x563a8a39d400
23266@1547735759.137305:qio_task_new Task new task=0x563a891d0570 source=0x563a8a39d400 func=0x563a86f1e6c0 opaque=0x563a89078000
23266@1547735759.137326:qio_task_thread_start Task thread start task=0x563a891d0570 worker=0x563a86f1ce50 opaque=0x563a891d9d90
23273@1547735759.137491:qio_task_thread_run Task thread run task=0x563a891d0570
23273@1547735759.137503:qio_channel_socket_connect_sync Socket connect sync ioc=0x563a8a39d400 addr=0x563a891d9d90
23273@1547735759.138108:qio_channel_socket_connect_fail Socket connect fail ioc=0x563a8a39d400
This commit introduces a way to do simple printf style logging of probe
points using systemtap. In particular it creates another set of tapsets,
one per emulator:
/usr/share/systemtap/tapset/qemu-*-log.stp
These pre-define probe functions which simply call printf() on their
arguments. The printf() format string is taken from the normal
trace-events files, with a little munging to the format specifiers
to cope with systemtap's more restrictive syntax.
With this you can now do
$ stap -e 'probe qemu.system.x86_64.log.qio*{}'
22806@1547735341399856820 qio_channel_socket_new Socket new ioc=0x56135d1d7c00
22806@1547735341399862570 qio_task_new Task new task=0x56135cd66eb0 source=0x56135d1d7c00 func=0x56135af746c0 opaque=0x56135bf06400
22806@1547735341399865943 qio_task_thread_start Task thread start task=0x56135cd66eb0 worker=0x56135af72e50 opaque=0x56135c071d70
22806@1547735341399976816 qio_task_thread_run Task thread run task=0x56135cd66eb0
We go one step further though and introduce a 'qemu-trace-stap' tool to
make this even easier
$ qemu-trace-stap run qemu-system-x86_64 'qio*'
22806@1547735341399856820 qio_channel_socket_new Socket new ioc=0x56135d1d7c00
22806@1547735341399862570 qio_task_new Task new task=0x56135cd66eb0 source=0x56135d1d7c00 func=0x56135af746c0 opaque=0x56135bf06400
22806@1547735341399865943 qio_task_thread_start Task thread start task=0x56135cd66eb0 worker=0x56135af72e50 opaque=0x56135c071d70
22806@1547735341399976816 qio_task_thread_run Task thread run task=0x56135cd66eb0
This tool is clever in that it will automatically change the
SYSTEMTAP_TAPSET env variable to point to the directory containing the
right set of probes for the QEMU binary path you give it. This is useful
if you have QEMU installed in /usr but are trying to test and trace a
binary in /home/berrange/usr/qemu-git. In that case you'd do
$ qemu-trace-stap run /home/berrange/usr/qemu-git/bin/qemu-system-x86_64 'qio*'
And it'll make sure /home/berrange/usr/qemu-git/share/systemtap/tapset
is used for the trace session
The 'qemu-trace-stap' script takes a verbose arg so you can understand
what it is running
$ qemu-trace-stap run /home/berrange/usr/qemu-git/bin/qemu-system-x86_64 'qio*'
Using tapset dir '/home/berrange/usr/qemu-git/share/systemtap/tapset' for binary '/home/berrange/usr/qemu-git/bin/qemu-system-x86_64'
Compiling script 'probe qemu.system.x86_64.log.qio* {}'
Running script, <Ctrl>-c to quit
...trace output...
It can enable multiple probes at once
$ qemu-trace-stap run qemu-system-x86_64 'qio*' 'qcrypto*' 'buffer*'
By default it monitors all existing running processes and all future
launched proceses. This can be restricted to a specific PID using the
--pid arg
$ qemu-trace-stap run --pid 2532 qemu-system-x86_64 'qio*'
Finally if you can't remember what probes are valid it can tell you
$ qemu-trace-stap list qemu-system-x86_64
ahci_check_irq
ahci_cmd_done
ahci_dma_prepare_buf
ahci_dma_prepare_buf_fail
ahci_dma_rw_buf
ahci_irq_lower
...snip...
Or list just those matching a prefix pattern
$ qemu-trace-stap list -v qemu-system-x86_64 'qio*'
Using tapset dir '/home/berrange/usr/qemu-git/share/systemtap/tapset' for binary '/home/berrange/usr/qemu-git/bin/qemu-system-x86_64'
Listing probes with name 'qemu.system.x86_64.log.qio*'
qio_channel_command_abort
qio_channel_command_new_pid
qio_channel_command_new_spawn
qio_channel_command_wait
qio_channel_file_new_fd
...snip...
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
MAINTAINERS | 1 +
Makefile | 10 ++
Makefile.target | 11 +-
docs/devel/tracing.txt | 4 +
scripts/qemu-trace-stap | 174 +++++++++++++++++++++++++++
scripts/qemu-trace-stap.texi | 139 +++++++++++++++++++++
scripts/tracetool/format/log_stap.py | 127 +++++++++++++++++++
7 files changed, 465 insertions(+), 1 deletion(-)
create mode 100755 scripts/qemu-trace-stap
create mode 100644 scripts/qemu-trace-stap.texi
create mode 100644 scripts/tracetool/format/log_stap.py
diff --git a/MAINTAINERS b/MAINTAINERS
index af339b86db..efa42c79fe 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2017,6 +2017,7 @@ F: trace-events
F: qemu-option-trace.texi
F: scripts/tracetool.py
F: scripts/tracetool/
+F: scripts/qemu-trace-stap*
F: docs/devel/tracing.txt
T: git https://github.com/stefanha/qemu.git tracing
diff --git a/Makefile b/Makefile
index c9bdb67274..1cb2f2fbcb 100644
--- a/Makefile
+++ b/Makefile
@@ -305,6 +305,9 @@ DOCS+=docs/qemu-cpu-models.7
ifdef CONFIG_VIRTFS
DOCS+=fsdev/virtfs-proxy-helper.1
endif
+ifdef CONFIG_TRACE_SYSTEMTAP
+DOCS+=scripts/qemu-trace-stap.1
+endif
else
DOCS=
endif
@@ -700,6 +703,9 @@ ifneq ($(TOOLS),)
$(INSTALL_DIR) "$(DESTDIR)$(mandir)/man8"
$(INSTALL_DATA) qemu-nbd.8 "$(DESTDIR)$(mandir)/man8"
endif
+ifdef CONFIG_TRACE_SYSTEMTAP
+ $(INSTALL_DATA) scripts/qemu-trace-stap.1 "$(DESTDIR)$(mandir)/man1"
+endif
ifneq (,$(findstring qemu-ga,$(TOOLS)))
$(INSTALL_DATA) qemu-ga.8 "$(DESTDIR)$(mandir)/man8"
$(INSTALL_DATA) docs/interop/qemu-ga-ref.html "$(DESTDIR)$(qemu_docdir)"
@@ -738,6 +744,9 @@ endif
ifneq ($(HELPERS-y),)
$(call install-prog,$(HELPERS-y),$(DESTDIR)$(libexecdir))
endif
+ifdef CONFIG_TRACE_SYSTEMTAP
+ $(INSTALL_PROG) "scripts/qemu-trace-stap" $(DESTDIR)$(bindir)
+endif
ifneq ($(BLOBS),)
set -e; for x in $(BLOBS); do \
$(INSTALL_DATA) $(SRC_PATH)/pc-bios/$$x "$(DESTDIR)$(qemu_datadir)"; \
@@ -838,6 +847,7 @@ qemu-nbd.8: qemu-nbd.texi qemu-option-trace.texi
qemu-ga.8: qemu-ga.texi
docs/qemu-block-drivers.7: docs/qemu-block-drivers.texi
docs/qemu-cpu-models.7: docs/qemu-cpu-models.texi
+scripts/qemu-trace-stap.1: scripts/qemu-trace-stap.texi
html: qemu-doc.html docs/interop/qemu-qmp-ref.html docs/interop/qemu-ga-ref.html
info: qemu-doc.info docs/interop/qemu-qmp-ref.info docs/interop/qemu-ga-ref.info
diff --git a/Makefile.target b/Makefile.target
index 39f72e81be..401dc1ea6f 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -45,7 +45,7 @@ config-target.h: config-target.h-timestamp
config-target.h-timestamp: config-target.mak
ifdef CONFIG_TRACE_SYSTEMTAP
-stap: $(QEMU_PROG).stp-installed $(QEMU_PROG).stp $(QEMU_PROG)-simpletrace.stp
+stap: $(QEMU_PROG).stp-installed $(QEMU_PROG).stp $(QEMU_PROG)-simpletrace.stp $(QEMU_PROG)-log.stp
ifdef CONFIG_USER_ONLY
TARGET_TYPE=user
@@ -84,6 +84,14 @@ $(QEMU_PROG)-simpletrace.stp: $(BUILD_DIR)/trace-events-all $(tracetool-y)
--probe-prefix=qemu.$(TARGET_TYPE).$(TARGET_NAME) \
$< > $@,"GEN","$(TARGET_DIR)$(QEMU_PROG)-simpletrace.stp")
+$(QEMU_PROG)-log.stp: $(BUILD_DIR)/trace-events-all $(tracetool-y)
+ $(call quiet-command,$(TRACETOOL) \
+ --group=all \
+ --format=log-stap \
+ --backends=$(TRACE_BACKENDS) \
+ --probe-prefix=qemu.$(TARGET_TYPE).$(TARGET_NAME) \
+ $< > $@,"GEN","$(TARGET_DIR)$(QEMU_PROG)-log.stp")
+
else
stap:
endif
@@ -227,6 +235,7 @@ ifdef CONFIG_TRACE_SYSTEMTAP
$(INSTALL_DIR) "$(DESTDIR)$(qemu_datadir)/../systemtap/tapset"
$(INSTALL_DATA) $(QEMU_PROG).stp-installed "$(DESTDIR)$(qemu_datadir)/../systemtap/tapset/$(QEMU_PROG).stp"
$(INSTALL_DATA) $(QEMU_PROG)-simpletrace.stp "$(DESTDIR)$(qemu_datadir)/../systemtap/tapset/$(QEMU_PROG)-simpletrace.stp"
+ $(INSTALL_DATA) $(QEMU_PROG)-log.stp "$(DESTDIR)$(qemu_datadir)/../systemtap/tapset/$(QEMU_PROG)-log.stp"
endif
GENERATED_FILES += config-target.h
diff --git a/docs/devel/tracing.txt b/docs/devel/tracing.txt
index bc52f12485..056aa56496 100644
--- a/docs/devel/tracing.txt
+++ b/docs/devel/tracing.txt
@@ -317,6 +317,10 @@ probes:
--target-name x86_64 \
<trace-events-all >qemu.stp
+To facilitate simple usage of systemtap where there merely needs to be printf
+logging of certain probes, a helper script "qemu-trace-stap" is provided.
+Consult its manual page for guidance on its usage.
+
== Trace event properties ==
Each event in the "trace-events-all" file can be prefixed with a space-separated
diff --git a/scripts/qemu-trace-stap b/scripts/qemu-trace-stap
new file mode 100755
index 0000000000..ee6e7ee8ba
--- /dev/null
+++ b/scripts/qemu-trace-stap
@@ -0,0 +1,174 @@
+#!/usr/bin/python
+# -*- python -*-
+#
+# Copyright (C) 2019 Red Hat, Inc
+#
+# QEMU SystemTap Trace Tool
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; under version 2 of the License.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, see <http://www.gnu.org/licenses/>.
+
+from __future__ import print_function
+
+import argparse
+import copy
+import os.path
+import re
+import subprocess
+import sys
+
+
+def probe_prefix(binary):
+ dirname, filename = os.path.split(binary)
+ return re.sub("-", ".", filename) + ".log"
+
+
+def which(binary):
+ for path in os.environ["PATH"].split(os.pathsep):
+ if os.path.exists(os.path.join(path, binary)):
+ return os.path.join(path, binary)
+
+ print("Unable to find '%s' in $PATH" % binary)
+ sys.exit(1)
+
+
+def tapset_dir(binary):
+ dirname, filename = os.path.split(binary)
+ if dirname == '':
+ thisfile = which(binary)
+ else:
+ thisfile = os.path.realpath(binary)
+ if not os.path.exists(thisfile):
+ print("Unable to find '%s'" % thisfile)
+ sys.exit(1)
+
+ basedir = os.path.split(thisfile)[0]
+ tapset = os.path.join(basedir, "..", "share", "systemtap", "tapset")
+ return os.path.realpath(tapset)
+
+
+def tapset_env(tapset_dir):
+ tenv = copy.copy(os.environ)
+ tenv["SYSTEMTAP_TAPSET"] = tapset_dir
+ return tenv
+
+def cmd_run(args):
+ prefix = probe_prefix(args.binary)
+ tapsets = tapset_dir(args.binary)
+
+ if args.verbose:
+ print("Using tapset dir '%s' for binary '%s'" % (tapsets, args.binary))
+
+ probes = []
+ for probe in args.probes:
+ probes.append("probe %s.%s {}" % (prefix, probe))
+ if len(probes) == 0:
+ print("At least one probe pattern must be specified")
+ sys.exit(1)
+
+ script = " ".join(probes)
+ if args.verbose:
+ print("Compiling script '%s'" % script)
+ script = """probe begin { print("Running script, <Ctrl>-c to quit\\n") } """ + script
+
+ # We request an 8MB buffer, since the stap default 1MB buffer
+ # can be easily overflowed by frequently firing QEMU traces
+ stapargs = ["stap", "-s", "8"]
+ if args.pid is not None:
+ stapargs.extend(["-x", args.pid])
+ stapargs.extend(["-e", script])
+ subprocess.call(stapargs, env=tapset_env(tapsets))
+
+
+def cmd_list(args):
+ tapsets = tapset_dir(args.binary)
+
+ if args.verbose:
+ print("Using tapset dir '%s' for binary '%s'" % (tapsets, args.binary))
+
+ def print_probes(verbose, name):
+ prefix = probe_prefix(args.binary)
+ offset = len(prefix) + 1
+ script = prefix + "." + name
+
+ if verbose:
+ print("Listing probes with name '%s'" % script)
+ proc = subprocess.Popen(["stap", "-l", script],
+ stdout=subprocess.PIPE, env=tapset_env(tapsets))
+ out, err = proc.communicate()
+ if proc.returncode != 0:
+ print("No probes found, are the tapsets installed in %s" % tapset_dir(args.binary))
+ sys.exit(1)
+
+ for line in out.splitlines():
+ if line.startswith(prefix):
+ print("%s" % line[offset:])
+
+ if len(args.probes) == 0:
+ print_probes(args.verbose, "*")
+ else:
+ for probe in args.probes:
+ print_probes(args.verbose, probe)
+
+
+def main():
+ parser = argparse.ArgumentParser(description="QEMU SystemTap trace tool")
+ parser.add_argument("-v", "--verbose", help="Print verbose progress info",
+ action='store_true')
+
+ subparser = parser.add_subparsers(help="commands")
+ subparser.required = True
+ subparser.dest = "command"
+
+ runparser = subparser.add_parser("run", help="Run a trace session",
+ formatter_class=argparse.RawDescriptionHelpFormatter,
+ epilog="""
+
+To watch all trace points on the qemu-system-x86_64 binary:
+
+ %(argv0)s run qemu-system-x86_64
+
+To only watch the trace points matching the qio* and qcrypto* patterns
+
+ %(argv0)s run qemu-system-x86_64 'qio*' 'qcrypto*'
+""" % {"argv0": sys.argv[0]})
+ runparser.set_defaults(func=cmd_run)
+ runparser.add_argument("--pid", "-p", dest="pid",
+ help="Restrict tracing to a specific process ID")
+ runparser.add_argument("binary", help="QEMU system or user emulator binary")
+ runparser.add_argument("probes", help="Probe names or wildcards",
+ nargs=argparse.REMAINDER)
+
+ listparser = subparser.add_parser("list", help="List probe points",
+ formatter_class=argparse.RawDescriptionHelpFormatter,
+ epilog="""
+
+To list all trace points on the qemu-system-x86_64 binary:
+
+ %(argv0)s list qemu-system-x86_64
+
+To only list the trace points matching the qio* and qcrypto* patterns
+
+ %(argv0)s list qemu-system-x86_64 'qio*' 'qcrypto*'
+""" % {"argv0": sys.argv[0]})
+ listparser.set_defaults(func=cmd_list)
+ listparser.add_argument("binary", help="QEMU system or user emulator binary")
+ listparser.add_argument("probes", help="Probe names or wildcards",
+ nargs=argparse.REMAINDER)
+
+ args = parser.parse_args()
+
+ args.func(args)
+ sys.exit(0)
+
+if __name__ == '__main__':
+ main()
diff --git a/scripts/qemu-trace-stap.texi b/scripts/qemu-trace-stap.texi
new file mode 100644
index 0000000000..235c596ab9
--- /dev/null
+++ b/scripts/qemu-trace-stap.texi
@@ -0,0 +1,139 @@
+@example
+@c man begin SYNOPSIS
+@command{qemu-trace-stap} @var{GLOBAL-OPTIONS} @var{COMMAND} @var{COMMAND-OPTIONS} @var{ARGS...}
+@c man end
+@end example
+
+@c man begin DESCRIPTION
+
+The @command{qemu-trace-stap} program facilitates tracing of the execution
+of QEMU emulators using SystemTap.
+
+It is required to have the SystemTap runtime environment installed to use
+this program, since it is a wrapper around execution of the @command{stap}
+program.
+
+@c man end
+
+@c man begin OPTIONS
+
+The following global options may be used regardless of which command
+is executed
+
+@table @option
+@item @var{--verbose}, @var{-v}
+
+Display verbose information about command execution
+
+@end table
+
+The following commands are valid:
+
+@table @option
+
+@item @var{list} @var{BINARY} @var{PATTERN...}
+
+List all the probe names provided by @var{BINARY} that match
+@var{PATTERN}.
+
+If @var{BINARY} is not an absolute path, it will be located by searching
+the directories listed in the @code{$PATH} environment variable.
+
+@var{PATTERN} is a plain string that is used to filter the results of
+this command. It may optionally contain a @code{*} wildcard to facilitate
+matching multiple probes without listing each one explicitly. Multiple
+@var{PATTERN} arguments may be given, causing listing of probes that match
+any of the listed names. If no @var{PATTERN} is given, the all possible
+probes will be listed.
+
+For example, to list all probes available in the @command{qemu-system-x86_64}
+binary:
+
+@example
+$ qemu-trace-stap list qemu-system-x86_64
+@end example
+
+To filter the list to only cover probes related to QEMU's cryptographic
+subsystem, in a binary outside @code{$PATH}
+
+@example
+$ qemu-trace-stap list /opt/qemu/4.0.0/bin/qemu-system-x86_64 'qcrypto*'
+@end example
+
+
+@item @var{run} @var{OPTIONS} @var{BINARY} @var{PATTERN...}
+
+Run a trace session, printing formatted output any time a process that is
+executing @var{BINARY} triggers a probe matching @var{PATTERN}.
+
+If @var{BINARY} is not an absolute path, it will be located by searching
+the directories listed in the @code{$PATH} environment variable.
+
+@var{PATTERN} is a plain string that matches a probe name shown by the
+@var{list} command. It may optionally contain a @code{*} wildcard to
+facilitate matching multiple probes without listing each one explicitly.
+Multiple @var{PATTERN} arguments may be given, causing all matching probes
+to be monitored. At least one @var{PATTERN} is required, since stap is not
+capable of tracing all known QEMU probes concurrently without overflowing
+its trace buffer.
+
+Invokation of this command does not need to be synchronized with
+invokation of the QEMU process(es). It will match probes on all
+existing running processes and all future launched processes,
+unless told to only monitor a specific procss.
+
+Valid command specific options are:
+
+@table @option
+@item @var{--pid=PID}, @var{-p PID}
+
+Restrict the tracing session so that it only triggers for the process
+identified by @code{PID}.
+
+@end table
+
+For example, to monitor all processes executing @command{qemu-system-x86_64},
+displaying all I/O related probes:
+
+@example
+$ qemu-trace-stap run qemu-system-x86_64 'qio*'
+@end example
+
+To monitor only the QEMU process with PID 1732
+
+@example
+$ qemu-trace-stap run --pid=1732 qemu-system-x86_64 'qio*'
+@end example
+
+To monitor QEMU processes running an alternative binary outside of
+@code{$PATH}, displaying verbose information about setup of the
+tracing environment:
+
+@example
+$ qemu-trace-stap -v run /opt/qemu/4.0.0/qemu-system-x86_64 'qio*'
+@end example
+
+@end table
+
+@c man end
+
+@ignore
+
+@setfilename qemu-trace-stap
+@settitle QEMU SystemTap trace tool
+
+@c man begin LICENSE
+
+Copyright (C) 2018 Red Hat, Inc.
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; under version 2 of the License.
+
+@c man end
+
+@c man begin SEEALSO
+qemu(1), stap(1)
+@c man end
+
+@end ignore
diff --git a/scripts/tracetool/format/log_stap.py b/scripts/tracetool/format/log_stap.py
new file mode 100644
index 0000000000..3ccbc09d61
--- /dev/null
+++ b/scripts/tracetool/format/log_stap.py
@@ -0,0 +1,127 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+"""
+Generate .stp file that printfs log messages (DTrace with SystemTAP only).
+"""
+
+__author__ = "Daniel P. Berrange <berrange@redhat.com>"
+__copyright__ = "Copyright (C) 2014-2019, Red Hat, Inc."
+__license__ = "GPL version 2 or (at your option) any later version"
+
+__maintainer__ = "Daniel Berrange"
+__email__ = "berrange@redhat.com"
+
+import re
+
+from tracetool import out
+from tracetool.backend.dtrace import binary, probeprefix
+from tracetool.backend.simple import is_string
+from tracetool.format.stap import stap_escape
+
+def global_var_name(name):
+ return probeprefix().replace(".", "_") + "_" + name
+
+STATE_SKIP = 0
+STATE_LITERAL = 1
+STATE_MACRO = 2
+
+def c_macro_to_format(macro):
+ if macro.startswith("PRI"):
+ return macro[3]
+
+ if macro == "TARGET_FMT_plx":
+ return "%016x"
+
+ raise Exception("Unhandled macro '%s'" % macro)
+
+def c_fmt_to_stap(fmt):
+ state = 0
+ bits = []
+ literal = ""
+ macro = ""
+ escape = 0;
+ for i in range(len(fmt)):
+ if fmt[i] == '\\':
+ if escape:
+ escape = 0
+ else:
+ escape = 1
+ if state != STATE_LITERAL:
+ raise Exception("Unexpected escape outside string literal")
+ literal = literal + fmt[i]
+ elif fmt[i] == '"' and not escape:
+ if state == STATE_LITERAL:
+ state = STATE_SKIP
+ bits.append(literal)
+ literal = ""
+ else:
+ if state == STATE_MACRO:
+ bits.append(c_macro_to_format(macro))
+ state = STATE_LITERAL
+ elif fmt[i] == ' ' or fmt[i] == '\t':
+ if state == STATE_MACRO:
+ bits.append(c_macro_to_format(macro))
+ macro = ""
+ state = STATE_SKIP
+ elif state == STATE_LITERAL:
+ literal = literal + fmt[i]
+ else:
+ escape = 0
+ if state == STATE_SKIP:
+ state = STATE_MACRO
+
+ if state == STATE_LITERAL:
+ literal = literal + fmt[i]
+ else:
+ macro = macro + fmt[i]
+
+ if state == STATE_MACRO:
+ bits.append(c_macro_to_format(macro))
+ elif state == STATE_LITERAL:
+ bits.append(literal)
+
+ fmt = re.sub("%(\d*)z(x|u|d)", "%\\1\\2", "".join(bits))
+ return fmt
+
+def generate(events, backend, group):
+ out('/* This file is autogenerated by tracetool, do not edit. */',
+ '')
+
+ for event_id, e in enumerate(events):
+ if 'disable' in e.properties:
+ continue
+
+ out('probe %(probeprefix)s.log.%(name)s = %(probeprefix)s.%(name)s ?',
+ '{',
+ probeprefix=probeprefix(),
+ name=e.name)
+
+ # Get references to userspace strings
+ for type_, name in e.args:
+ name = stap_escape(name)
+ if is_string(type_):
+ out(' try {',
+ ' arg%(name)s_str = %(name)s ? ' +
+ 'user_string_n(%(name)s, 512) : "<null>"',
+ ' } catch {}',
+ name=name)
+
+ # Determine systemtap's view of variable names
+ fields = ["pid()", "gettimeofday_ns()"]
+ for type_, name in e.args:
+ name = stap_escape(name)
+ if is_string(type_):
+ fields.append("arg" + name + "_str")
+ else:
+ fields.append(name)
+
+ # Emit the entire record in a single SystemTap printf()
+ arg_str = ', '.join(arg for arg in fields)
+ fmt_str = "%d@%d " + e.name + " " + c_fmt_to_stap(e.fmt) + "\\n"
+ out(' printf("%(fmt_str)s", %(arg_str)s)',
+ fmt_str=fmt_str, arg_str=arg_str)
+
+ out('}')
+
+ out()
--
2.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/4] display: ensure qxl log_buf is a nul terminated string
2019-01-22 16:49 ` [Qemu-devel] [PATCH v3 1/4] display: ensure qxl log_buf is a nul terminated string Daniel P. Berrangé
@ 2019-01-22 17:23 ` Eric Blake
0 siblings, 0 replies; 9+ messages in thread
From: Eric Blake @ 2019-01-22 17:23 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel
Cc: Gerd Hoffmann, Alex Williamson, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 1464 bytes --]
On 1/22/19 10:49 AM, Daniel P. Berrangé wrote:
> The QXL_IO_LOG command allows the guest to send log messages to the host
> via a buffer in the QXLRam struct. QEMU prints these to the console if
> the qxl 'guestdebug' option is set to non-zero. It will also feed them
> to the trace subsystem if any backends are built-in.
>
> In both cases the log_buf data will get treated as being as a nul
> terminated string, by the printf '%s' format specifier and / or other
> code reading the buffer.
>
> QEMU does nothing to guarantee that the log_buf really is nul terminated,
> so there is potential for out of bounds array access.
>
> This would affect any QEMU which has the log, syslog or ftrace trace
> backends built into QEMU. It can only be triggered if the 'qxl_io_log'
> trace event is enabled, however, so they are not vulnerable without
> specific administrative action to enable this.
>
> It would also affect QEMU if the 'guestdebug' parameter is set to a
> non-zero value, which again is not the default and requires explicit
> admin opt-in.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> hw/display/qxl.c | 14 ++++++++++----
> hw/display/trace-events | 2 +-
> 2 files changed, 11 insertions(+), 5 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v3 3/4] trace: forbid use of %m in trace event format strings
2019-01-22 16:49 ` [Qemu-devel] [PATCH v3 3/4] trace: forbid use of %m in trace event format strings Daniel P. Berrangé
@ 2019-01-22 17:25 ` Eric Blake
0 siblings, 0 replies; 9+ messages in thread
From: Eric Blake @ 2019-01-22 17:25 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel
Cc: Gerd Hoffmann, Alex Williamson, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 1343 bytes --]
On 1/22/19 10:49 AM, Daniel P. Berrangé wrote:
> The '%m' format specifier instructs glibc's printf() implementation to
> insert the contents of strerror(errno). Since this is a glibc extension
> it should generally be avoided in QEMU due to need for portability to a
> variety of platforms.
Up to you if you want to mention that syslog() portably supports %m, but
not all trace backends use syslog().
>
> Even though vfio is Linux-only code that could otherwise use "%m", it
> must still be avoided in trace-events files because several of the
> backends do not use the format string and so this error information is
> invisible to them.
>
> The errno string value should be given as an explicit trace argument
> instead, making it accessible to all backends. This also allows it to
> work correctly with future patches that use the format string with
> systemtap's simple printf code.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> hw/vfio/pci.c | 2 +-
> hw/vfio/trace-events | 2 +-
> scripts/tracetool/__init__.py | 4 ++++
> 3 files changed, 6 insertions(+), 2 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v3 4/4] trace: add ability to do simple printf logging via systemtap
2019-01-22 16:49 ` [Qemu-devel] [PATCH v3 4/4] trace: add ability to do simple printf logging via systemtap Daniel P. Berrangé
@ 2019-01-22 17:42 ` Eric Blake
2019-01-23 11:59 ` Daniel P. Berrangé
0 siblings, 1 reply; 9+ messages in thread
From: Eric Blake @ 2019-01-22 17:42 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel
Cc: Gerd Hoffmann, Alex Williamson, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 6616 bytes --]
On 1/22/19 10:49 AM, Daniel P. Berrangé wrote:
> The dtrace systemtap trace backend for QEMU is very powerful but it is
> also somewhat unfriendly to users who aren't familiar with systemtap,
> or who don't need its power right now.
>
> stap -e "....some strange script...."
>
> By default it monitors all existing running processes and all future
> launched proceses. This can be restricted to a specific PID using the
> --pid arg
>
> $ qemu-trace-stap run --pid 2532 qemu-system-x86_64 'qio*'
>
Can --pid mode be smart enough to check /proc/NNN/exe without the user
having to specify the explicit qemu-system-x86_64 argument? (Could be
followup patch)
> +++ b/scripts/qemu-trace-stap
> @@ -0,0 +1,174 @@
> +# QEMU SystemTap Trace Tool
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; under version 2 of the License.
> +#
Why GPLv2-only?
> +def main():
> + parser = argparse.ArgumentParser(description="QEMU SystemTap trace tool")
> + parser.add_argument("-v", "--verbose", help="Print verbose progress info",
> + action='store_true')
> +
> + subparser = parser.add_subparsers(help="commands")
> + subparser.required = True
> + subparser.dest = "command"
> +
> + runparser = subparser.add_parser("run", help="Run a trace session",
> + formatter_class=argparse.RawDescriptionHelpFormatter,
> + epilog="""
> +
> +To watch all trace points on the qemu-system-x86_64 binary:
> +
> + %(argv0)s run qemu-system-x86_64
> +
> +To only watch the trace points matching the qio* and qcrypto* patterns
> +
> + %(argv0)s run qemu-system-x86_64 'qio*' 'qcrypto*'
> +""" % {"argv0": sys.argv[0]})
> + runparser.set_defaults(func=cmd_run)
> + runparser.add_argument("--pid", "-p", dest="pid",
> + help="Restrict tracing to a specific process ID")
> + runparser.add_argument("binary", help="QEMU system or user emulator binary")
> + runparser.add_argument("probes", help="Probe names or wildcards",
> + nargs=argparse.REMAINDER)
> +
> + listparser = subparser.add_parser("list", help="List probe points",
> + formatter_class=argparse.RawDescriptionHelpFormatter,
> + epilog="""
> +
> +To list all trace points on the qemu-system-x86_64 binary:
> +
> + %(argv0)s list qemu-system-x86_64
No mention of --pid mode in the --help output?
> +++ b/scripts/qemu-trace-stap.texi
> @@ -0,0 +1,139 @@
> +@example
> +@c man begin SYNOPSIS
> +@command{qemu-trace-stap} @var{GLOBAL-OPTIONS} @var{COMMAND} @var{COMMAND-OPTIONS} @var{ARGS...}
> +@c man end
> +@end example
> +
> +@c man begin DESCRIPTION
> +
> +The @command{qemu-trace-stap} program facilitates tracing of the execution
> +of QEMU emulators using SystemTap.
> +
> +It is required to have the SystemTap runtime environment installed to use
> +this program, since it is a wrapper around execution of the @command{stap}
> +program.
> +
> +@c man end
> +
> +@c man begin OPTIONS
> +
> +The following global options may be used regardless of which command
> +is executed
> +
Could use trailing ':'
> +@table @option
> +@item @var{--verbose}, @var{-v}
> +
> +Display verbose information about command execution
Trailing '.'
> +@item @var{run} @var{OPTIONS} @var{BINARY} @var{PATTERN...}
> +
> +Run a trace session, printing formatted output any time a process that is
> +executing @var{BINARY} triggers a probe matching @var{PATTERN}.
> +
> +If @var{BINARY} is not an absolute path, it will be located by searching
> +the directories listed in the @code{$PATH} environment variable.
> +
> +@var{PATTERN} is a plain string that matches a probe name shown by the
> +@var{list} command. It may optionally contain a @code{*} wildcard to
> +facilitate matching multiple probes without listing each one explicitly.
> +Multiple @var{PATTERN} arguments may be given, causing all matching probes
> +to be monitored. At least one @var{PATTERN} is required, since stap is not
> +capable of tracing all known QEMU probes concurrently without overflowing
> +its trace buffer.
> +
> +Invokation of this command does not need to be synchronized with
s/Invokation/Invocation/
> +invokation of the QEMU process(es). It will match probes on all
and again
> +existing running processes and all future launched processes,
> +unless told to only monitor a specific procss.
s/procss/process/
> +
> +Valid command specific options are:
> +
> +@table @option
> +@item @var{--pid=PID}, @var{-p PID}
> +
> +Restrict the tracing session so that it only triggers for the process
> +identified by @code{PID}.
> +
> +@end table
> +
> +For example, to monitor all processes executing @command{qemu-system-x86_64},
Maybe "executing @command{qemu-system-x86_64} as found on $PATH,"
> +@setfilename qemu-trace-stap
> +@settitle QEMU SystemTap trace tool
> +
> +@c man begin LICENSE
> +
> +Copyright (C) 2018 Red Hat, Inc.
Welcome to 2019
> +
> +This program is free software; you can redistribute it and/or modify
> +it under the terms of the GNU General Public License as published by
> +the Free Software Foundation; under version 2 of the License.
Why GPLv2-only?
> +
> +@c man end
> +
> +@c man begin SEEALSO
> +qemu(1), stap(1)
> +@c man end
> +
> +@end ignore
> diff --git a/scripts/tracetool/format/log_stap.py b/scripts/tracetool/format/log_stap.py
> new file mode 100644
> index 0000000000..3ccbc09d61
> --- /dev/null
> +++ b/scripts/tracetool/format/log_stap.py
> @@ -0,0 +1,127 @@
> +#!/usr/bin/env python
> +# -*- coding: utf-8 -*-
> +
> +"""
> +Generate .stp file that printfs log messages (DTrace with SystemTAP only).
> +"""
> +
> +__author__ = "Daniel P. Berrange <berrange@redhat.com>"
> +__copyright__ = "Copyright (C) 2014-2019, Red Hat, Inc."
> +__license__ = "GPL version 2 or (at your option) any later version"
At least this file is GPLv2+.
Typos are trivial, licensing less so, but I trust you can fix licensing
without me having to see a respin (as it won't change code). So:
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v3 4/4] trace: add ability to do simple printf logging via systemtap
2019-01-22 17:42 ` Eric Blake
@ 2019-01-23 11:59 ` Daniel P. Berrangé
0 siblings, 0 replies; 9+ messages in thread
From: Daniel P. Berrangé @ 2019-01-23 11:59 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Gerd Hoffmann, Alex Williamson, Stefan Hajnoczi
On Tue, Jan 22, 2019 at 11:42:31AM -0600, Eric Blake wrote:
> On 1/22/19 10:49 AM, Daniel P. Berrangé wrote:
> > The dtrace systemtap trace backend for QEMU is very powerful but it is
> > also somewhat unfriendly to users who aren't familiar with systemtap,
> > or who don't need its power right now.
> >
> > stap -e "....some strange script...."
> >
>
> > By default it monitors all existing running processes and all future
> > launched proceses. This can be restricted to a specific PID using the
> > --pid arg
> >
> > $ qemu-trace-stap run --pid 2532 qemu-system-x86_64 'qio*'
> >
>
> Can --pid mode be smart enough to check /proc/NNN/exe without the user
> having to specify the explicit qemu-system-x86_64 argument? (Could be
> followup patch)
It could do that, but this complicates the parsing of command line
arguments, as the binary is a positional arg.
> > +++ b/scripts/qemu-trace-stap
> > @@ -0,0 +1,174 @@
>
> > +# QEMU SystemTap Trace Tool
> > +#
> > +# This program is free software; you can redistribute it and/or modify
> > +# it under the terms of the GNU General Public License as published by
> > +# the Free Software Foundation; under version 2 of the License.
> > +#
>
> Why GPLv2-only?
A mistake, it should be v2-or-later
>
> > +def main():
> > + parser = argparse.ArgumentParser(description="QEMU SystemTap trace tool")
> > + parser.add_argument("-v", "--verbose", help="Print verbose progress info",
> > + action='store_true')
> > +
> > + subparser = parser.add_subparsers(help="commands")
> > + subparser.required = True
> > + subparser.dest = "command"
> > +
> > + runparser = subparser.add_parser("run", help="Run a trace session",
> > + formatter_class=argparse.RawDescriptionHelpFormatter,
> > + epilog="""
> > +
> > +To watch all trace points on the qemu-system-x86_64 binary:
> > +
> > + %(argv0)s run qemu-system-x86_64
> > +
> > +To only watch the trace points matching the qio* and qcrypto* patterns
> > +
> > + %(argv0)s run qemu-system-x86_64 'qio*' 'qcrypto*'
> > +""" % {"argv0": sys.argv[0]})
> > + runparser.set_defaults(func=cmd_run)
> > + runparser.add_argument("--pid", "-p", dest="pid",
> > + help="Restrict tracing to a specific process ID")
> > + runparser.add_argument("binary", help="QEMU system or user emulator binary")
> > + runparser.add_argument("probes", help="Probe names or wildcards",
> > + nargs=argparse.REMAINDER)
> > +
> > + listparser = subparser.add_parser("list", help="List probe points",
> > + formatter_class=argparse.RawDescriptionHelpFormatter,
> > + epilog="""
> > +
> > +To list all trace points on the qemu-system-x86_64 binary:
> > +
> > + %(argv0)s list qemu-system-x86_64
>
> No mention of --pid mode in the --help output?
getopt generates --help output listing all arguments. The epilog is
just a bit of text to augment what it already prints
> Typos are trivial, licensing less so, but I trust you can fix licensing
> without me having to see a respin (as it won't change code). So:
> Reviewed-by: Eric Blake <eblake@redhat.com>
Will incorporate these changes
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 :|
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-01-23 12:13 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-22 16:49 [Qemu-devel] [PATCH v3 0/4] trace: make systemtap easier to use for simple logging Daniel P. Berrangé
2019-01-22 16:49 ` [Qemu-devel] [PATCH v3 1/4] display: ensure qxl log_buf is a nul terminated string Daniel P. Berrangé
2019-01-22 17:23 ` Eric Blake
2019-01-22 16:49 ` [Qemu-devel] [PATCH v3 2/4] trace: enforce that every trace-events file has a final newline Daniel P. Berrangé
2019-01-22 16:49 ` [Qemu-devel] [PATCH v3 3/4] trace: forbid use of %m in trace event format strings Daniel P. Berrangé
2019-01-22 17:25 ` Eric Blake
2019-01-22 16:49 ` [Qemu-devel] [PATCH v3 4/4] trace: add ability to do simple printf logging via systemtap Daniel P. Berrangé
2019-01-22 17:42 ` Eric Blake
2019-01-23 11:59 ` Daniel P. Berrangé
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.