From 2adf2164918e2dc74fef2cdd0257917aff488640 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 1 Jun 2021 15:24:02 +0200 Subject: [PATCH 1/6] qemu-trace-stap: changing SYSTEMTAP_TAPSET considered harmful. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Setting SYSTEMTAP_TAPSET to some value other than /usr/share/systemtap/tapsets results in systemtap not finding the standard tapset library any more, which in turn breaks tracing because pid() and other standard systemtap functions are not available any more. So using SYSTEMTAP_TAPSET to point systemtap to the qemu probes will only work for the prefix=/usr installs because both qemu and system tapsets in the same directory then. All other prefixes are broken. Fix that by using the "-I $tapsetdir" command line switch instead. Signed-off-by: Gerd Hoffmann Reviewed-by: Stefan Hajnoczi Reviewed-by: Daniel P. Berrangé Message-id: 20210601132414.432430-2-kraxel@redhat.com Signed-off-by: Stefan Hajnoczi --- scripts/qemu-trace-stap | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/scripts/qemu-trace-stap b/scripts/qemu-trace-stap index 90527eb974..eb6e951ff2 100755 --- a/scripts/qemu-trace-stap +++ b/scripts/qemu-trace-stap @@ -55,11 +55,6 @@ def tapset_dir(binary): 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) @@ -81,11 +76,11 @@ def cmd_run(args): # We request an 8MB buffer, since the stap default 1MB buffer # can be easily overflowed by frequently firing QEMU traces - stapargs = ["stap", "-s", "8"] + stapargs = ["stap", "-s", "8", "-I", tapsets ] if args.pid is not None: stapargs.extend(["-x", args.pid]) stapargs.extend(["-e", script]) - subprocess.call(stapargs, env=tapset_env(tapsets)) + subprocess.call(stapargs) def cmd_list(args): @@ -101,10 +96,9 @@ def cmd_list(args): if verbose: print("Listing probes with name '%s'" % script) - proc = subprocess.Popen(["stap", "-l", script], + proc = subprocess.Popen(["stap", "-I", tapsets, "-l", script], stdout=subprocess.PIPE, - universal_newlines=True, - env=tapset_env(tapsets)) + universal_newlines=True) out, err = proc.communicate() if proc.returncode != 0: print("No probes found, are the tapsets installed in %s" % tapset_dir(args.binary)) From 117856c3748dfda50351d1c0328486ede5f2646c Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 1 Jun 2021 15:24:03 +0200 Subject: [PATCH 2/6] trace: iter init tweaks Rename trace_event_iter_init() to trace_event_iter_init_pattern(), add trace_event_iter_init_all() for interating over all events. Signed-off-by: Gerd Hoffmann Reviewed-by: Stefan Hajnoczi Message-id: 20210601132414.432430-3-kraxel@redhat.com Signed-off-by: Stefan Hajnoczi --- monitor/misc.c | 4 ++-- trace/control-target.c | 2 +- trace/control.c | 16 +++++++++++----- trace/control.h | 17 +++++++++++++---- trace/qmp.c | 6 +++--- trace/simple.c | 2 +- 6 files changed, 31 insertions(+), 16 deletions(-) diff --git a/monitor/misc.c b/monitor/misc.c index b28874d6dc..ffe7966870 100644 --- a/monitor/misc.c +++ b/monitor/misc.c @@ -1804,7 +1804,7 @@ void info_trace_events_completion(ReadLineState *rs, int nb_args, const char *st TraceEventIter iter; TraceEvent *ev; char *pattern = g_strdup_printf("%s*", str); - trace_event_iter_init(&iter, pattern); + trace_event_iter_init_pattern(&iter, pattern); while ((ev = trace_event_iter_next(&iter)) != NULL) { readline_add_completion(rs, trace_event_get_name(ev)); } @@ -1822,7 +1822,7 @@ void trace_event_completion(ReadLineState *rs, int nb_args, const char *str) TraceEventIter iter; TraceEvent *ev; char *pattern = g_strdup_printf("%s*", str); - trace_event_iter_init(&iter, pattern); + trace_event_iter_init_pattern(&iter, pattern); while ((ev = trace_event_iter_next(&iter)) != NULL) { readline_add_completion(rs, trace_event_get_name(ev)); } diff --git a/trace/control-target.c b/trace/control-target.c index e293eeed7c..8418673c18 100644 --- a/trace/control-target.c +++ b/trace/control-target.c @@ -127,7 +127,7 @@ void trace_init_vcpu(CPUState *vcpu) { TraceEventIter iter; TraceEvent *ev; - trace_event_iter_init(&iter, NULL); + trace_event_iter_init_all(&iter); while ((ev = trace_event_iter_next(&iter)) != NULL) { if (trace_event_is_vcpu(ev) && trace_event_get_state_static(ev) && diff --git a/trace/control.c b/trace/control.c index 4be38e1af2..ed38e813b2 100644 --- a/trace/control.c +++ b/trace/control.c @@ -91,7 +91,7 @@ TraceEvent *trace_event_name(const char *name) TraceEventIter iter; TraceEvent *ev; - trace_event_iter_init(&iter, NULL); + trace_event_iter_init_all(&iter); while ((ev = trace_event_iter_next(&iter)) != NULL) { if (strcmp(trace_event_get_name(ev), name) == 0) { return ev; @@ -100,10 +100,16 @@ TraceEvent *trace_event_name(const char *name) return NULL; } -void trace_event_iter_init(TraceEventIter *iter, const char *pattern) +void trace_event_iter_init_all(TraceEventIter *iter) { iter->event = 0; iter->group = 0; + iter->pattern = NULL; +} + +void trace_event_iter_init_pattern(TraceEventIter *iter, const char *pattern) +{ + trace_event_iter_init_all(iter); iter->pattern = pattern; } @@ -130,7 +136,7 @@ void trace_list_events(FILE *f) { TraceEventIter iter; TraceEvent *ev; - trace_event_iter_init(&iter, NULL); + trace_event_iter_init_all(&iter); while ((ev = trace_event_iter_next(&iter)) != NULL) { fprintf(f, "%s\n", trace_event_get_name(ev)); } @@ -150,7 +156,7 @@ static void do_trace_enable_events(const char *line_buf) TraceEvent *ev; bool is_pattern = trace_event_is_pattern(line_ptr); - trace_event_iter_init(&iter, line_ptr); + trace_event_iter_init_pattern(&iter, line_ptr); while ((ev = trace_event_iter_next(&iter)) != NULL) { if (!trace_event_get_state_static(ev)) { if (!is_pattern) { @@ -256,7 +262,7 @@ void trace_fini_vcpu(CPUState *vcpu) trace_guest_cpu_exit(vcpu); - trace_event_iter_init(&iter, NULL); + trace_event_iter_init_all(&iter); while ((ev = trace_event_iter_next(&iter)) != NULL) { if (trace_event_is_vcpu(ev) && trace_event_get_state_static(ev) && diff --git a/trace/control.h b/trace/control.h index 9522a7b318..ce40bd0405 100644 --- a/trace/control.h +++ b/trace/control.h @@ -20,15 +20,24 @@ typedef struct TraceEventIter { /** - * trace_event_iter_init: + * trace_event_iter_init_all: * @iter: the event iterator struct - * @pattern: optional pattern to filter events on name * * Initialize the event iterator struct @iter, - * optionally using @pattern to filter out events + * for all events. + */ +void trace_event_iter_init_all(TraceEventIter *iter); + +/** + * trace_event_iter_init_pattern: + * @iter: the event iterator struct + * @pattern: pattern to filter events on name + * + * Initialize the event iterator struct @iter, + * using @pattern to filter out events * with non-matching names. */ -void trace_event_iter_init(TraceEventIter *iter, const char *pattern); +void trace_event_iter_init_pattern(TraceEventIter *iter, const char *pattern); /** * trace_event_iter_next: diff --git a/trace/qmp.c b/trace/qmp.c index 85f81e47cc..3b4f4702b4 100644 --- a/trace/qmp.c +++ b/trace/qmp.c @@ -55,7 +55,7 @@ static bool check_events(bool has_vcpu, bool ignore_unavailable, bool is_pattern /* error for unavailable events */ TraceEventIter iter; TraceEvent *ev; - trace_event_iter_init(&iter, name); + trace_event_iter_init_pattern(&iter, name); while ((ev = trace_event_iter_next(&iter)) != NULL) { if (!ignore_unavailable && !trace_event_get_state_static(ev)) { error_setg(errp, "event \"%s\" is disabled", trace_event_get_name(ev)); @@ -90,7 +90,7 @@ TraceEventInfoList *qmp_trace_event_get_state(const char *name, } /* Get states (all errors checked above) */ - trace_event_iter_init(&iter, name); + trace_event_iter_init_pattern(&iter, name); while ((ev = trace_event_iter_next(&iter)) != NULL) { TraceEventInfo *value; bool is_vcpu = trace_event_is_vcpu(ev); @@ -153,7 +153,7 @@ void qmp_trace_event_set_state(const char *name, bool enable, } /* Apply changes (all errors checked above) */ - trace_event_iter_init(&iter, name); + trace_event_iter_init_pattern(&iter, name); while ((ev = trace_event_iter_next(&iter)) != NULL) { if (!trace_event_get_state_static(ev) || (has_vcpu && !trace_event_is_vcpu(ev))) { diff --git a/trace/simple.c b/trace/simple.c index 9cd2ed1fb3..97b6f85168 100644 --- a/trace/simple.c +++ b/trace/simple.c @@ -286,7 +286,7 @@ static int st_write_event_mapping(void) TraceEventIter iter; TraceEvent *ev; - trace_event_iter_init(&iter, NULL); + trace_event_iter_init_all(&iter); while ((ev = trace_event_iter_next(&iter)) != NULL) { uint64_t id = trace_event_get_id(ev); const char *name = trace_event_get_name(ev); From c5cc58b176f23f6664d0e12e5956af4d904dcca4 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 1 Jun 2021 15:24:04 +0200 Subject: [PATCH 3/6] trace: add trace_event_iter_init_group This allows to interate over an event group. Signed-off-by: Gerd Hoffmann Reviewed-by: Stefan Hajnoczi Message-id: 20210601132414.432430-4-kraxel@redhat.com Signed-off-by: Stefan Hajnoczi --- trace/control.c | 19 ++++++++++++++++--- trace/control.h | 13 +++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/trace/control.c b/trace/control.c index ed38e813b2..2c904b7ee4 100644 --- a/trace/control.c +++ b/trace/control.c @@ -104,6 +104,7 @@ void trace_event_iter_init_all(TraceEventIter *iter) { iter->event = 0; iter->group = 0; + iter->group_id = -1; iter->pattern = NULL; } @@ -113,20 +114,32 @@ void trace_event_iter_init_pattern(TraceEventIter *iter, const char *pattern) iter->pattern = pattern; } +void trace_event_iter_init_group(TraceEventIter *iter, size_t group_id) +{ + trace_event_iter_init_all(iter); + iter->group_id = group_id; +} + TraceEvent *trace_event_iter_next(TraceEventIter *iter) { while (iter->group < nevent_groups && event_groups[iter->group].events[iter->event] != NULL) { TraceEvent *ev = event_groups[iter->group].events[iter->event]; + size_t group = iter->group; iter->event++; if (event_groups[iter->group].events[iter->event] == NULL) { iter->event = 0; iter->group++; } - if (!iter->pattern || - g_pattern_match_simple(iter->pattern, trace_event_get_name(ev))) { - return ev; + if (iter->pattern && + !g_pattern_match_simple(iter->pattern, trace_event_get_name(ev))) { + continue; } + if (iter->group_id != -1 && + iter->group_id != group) { + continue; + } + return ev; } return NULL; diff --git a/trace/control.h b/trace/control.h index ce40bd0405..23b8393b29 100644 --- a/trace/control.h +++ b/trace/control.h @@ -13,8 +13,11 @@ #include "event-internal.h" typedef struct TraceEventIter { + /* iter state */ size_t event; size_t group; + /* filter conditions */ + size_t group_id; const char *pattern; } TraceEventIter; @@ -39,6 +42,16 @@ void trace_event_iter_init_all(TraceEventIter *iter); */ void trace_event_iter_init_pattern(TraceEventIter *iter, const char *pattern); +/** + * trace_event_iter_init_group: + * @iter: the event iterator struct + * @group_id: group_id to filter events by group. + * + * Initialize the event iterator struct @iter, + * using @group_id to filter for events in the group. + */ +void trace_event_iter_init_group(TraceEventIter *iter, size_t group_id); + /** * trace_event_iter_next: * @iter: the event iterator struct From 3f2a09842f989af020b8355622d5f7fa9bdeb832 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 1 Jun 2021 15:24:05 +0200 Subject: [PATCH 4/6] trace/simple: pass iter to st_write_event_mapping Pass an iter to st_write_event_mapping, so the function can interate different things depending on how we initialize the iter. Signed-off-by: Gerd Hoffmann Reviewed-by: Stefan Hajnoczi Message-id: 20210601132414.432430-5-kraxel@redhat.com Signed-off-by: Stefan Hajnoczi --- trace/simple.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/trace/simple.c b/trace/simple.c index 97b6f85168..ec2156d135 100644 --- a/trace/simple.c +++ b/trace/simple.c @@ -280,14 +280,12 @@ void trace_record_finish(TraceBufferRecord *rec) } } -static int st_write_event_mapping(void) +static int st_write_event_mapping(TraceEventIter *iter) { uint64_t type = TRACE_RECORD_TYPE_MAPPING; - TraceEventIter iter; TraceEvent *ev; - trace_event_iter_init_all(&iter); - while ((ev = trace_event_iter_next(&iter)) != NULL) { + while ((ev = trace_event_iter_next(iter)) != NULL) { uint64_t id = trace_event_get_id(ev); const char *name = trace_event_get_name(ev); uint32_t len = strlen(name); @@ -309,6 +307,7 @@ static int st_write_event_mapping(void) */ bool st_set_trace_file_enabled(bool enable) { + TraceEventIter iter; bool was_enabled = trace_fp; if (enable == !!trace_fp) { @@ -333,8 +332,9 @@ bool st_set_trace_file_enabled(bool enable) return was_enabled; } + trace_event_iter_init_all(&iter); if (fwrite(&header, sizeof header, 1, trace_fp) != 1 || - st_write_event_mapping() < 0) { + st_write_event_mapping(&iter) < 0) { fclose(trace_fp); trace_fp = NULL; return was_enabled; From 263b6e96449d07808bc6eb21ab24f3a8b7a49bb6 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 1 Jun 2021 15:24:06 +0200 Subject: [PATCH 5/6] trace/simple: add st_init_group Add helper function and call it for each trace event group added. Makes sure that events added at module load time are initialized properly. Signed-off-by: Gerd Hoffmann Reviewed-by: Stefan Hajnoczi Message-id: 20210601132414.432430-6-kraxel@redhat.com Signed-off-by: Stefan Hajnoczi --- trace/control.c | 4 ++++ trace/simple.c | 12 ++++++++++++ trace/simple.h | 1 + 3 files changed, 17 insertions(+) diff --git a/trace/control.c b/trace/control.c index 2c904b7ee4..d5b68e846e 100644 --- a/trace/control.c +++ b/trace/control.c @@ -82,6 +82,10 @@ void trace_event_register_group(TraceEvent **events) event_groups = g_renew(TraceEventGroup, event_groups, nevent_groups + 1); event_groups[nevent_groups].events = events; nevent_groups++; + +#ifdef CONFIG_TRACE_SIMPLE + st_init_group(nevent_groups - 1); +#endif } diff --git a/trace/simple.c b/trace/simple.c index ec2156d135..ac499edee0 100644 --- a/trace/simple.c +++ b/trace/simple.c @@ -422,3 +422,15 @@ bool st_init(void) atexit(st_flush_trace_buffer); return true; } + +void st_init_group(size_t group) +{ + TraceEventIter iter; + + if (!trace_writeout_enabled) { + return; + } + + trace_event_iter_init_group(&iter, group); + st_write_event_mapping(&iter); +} diff --git a/trace/simple.h b/trace/simple.h index 26ccbc8b8a..ee1983ce56 100644 --- a/trace/simple.h +++ b/trace/simple.h @@ -15,6 +15,7 @@ void st_print_trace_file_status(void); bool st_set_trace_file_enabled(bool enable); void st_set_trace_file(const char *file); bool st_init(void); +void st_init_group(size_t group); void st_flush_trace_buffer(void); typedef struct { From bbe47ed2928542e7db58146b6108e3f2836f278f Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 12 Jul 2021 17:57:10 +0200 Subject: [PATCH 6/6] trace, lttng: require .pc files The next version of lttng-libs will not require liburcu at run time anymore. Therefore, it is expected that distros will not include the urcubp libraries anymore when installing lttng-ust-devel. To avoid future problems, just require pkg-config to detect lttng-ust. The .pc files for lttng-ust correctly include liburcubp.a for static builds, and have always done since pkg-config files were added in 2011. Signed-off-by: Paolo Bonzini Message-id: 20210712155710.520889-1-pbonzini@redhat.com Signed-off-by: Stefan Hajnoczi --- configure | 18 ++---------------- meson.build | 4 ---- trace/meson.build | 2 +- 3 files changed, 3 insertions(+), 21 deletions(-) diff --git a/configure b/configure index 85db248ac1..4d0a2bfdd8 100755 --- a/configure +++ b/configure @@ -3606,21 +3606,8 @@ fi ########################################## # For 'ust' backend, test if ust headers are present if have_backend "ust"; then - cat > $TMPC << EOF -#include -int main(void) { return 0; } -EOF - if compile_prog "" "-Wl,--no-as-needed -ldl" ; then - if $pkg_config lttng-ust --exists; then - lttng_ust_libs=$($pkg_config --libs lttng-ust) - else - lttng_ust_libs="-llttng-ust -ldl" - fi - if $pkg_config liburcu-bp --exists; then - urcu_bp_libs=$($pkg_config --libs liburcu-bp) - else - urcu_bp_libs="-lurcu-bp" - fi + if $pkg_config lttng-ust --exists; then + lttng_ust_libs=$($pkg_config --libs lttng-ust) else error_exit "Trace backend 'ust' missing lttng-ust header files" fi @@ -4773,7 +4760,6 @@ fi if have_backend "ust"; then echo "CONFIG_TRACE_UST=y" >> $config_host_mak echo "LTTNG_UST_LIBS=$lttng_ust_libs" >> $config_host_mak - echo "URCU_BP_LIBS=$urcu_bp_libs" >> $config_host_mak fi if have_backend "dtrace"; then echo "CONFIG_TRACE_DTRACE=y" >> $config_host_mak diff --git a/meson.build b/meson.build index dd7f9ed6a8..4dea2d9dd3 100644 --- a/meson.build +++ b/meson.build @@ -319,10 +319,6 @@ lttng = not_found if 'CONFIG_TRACE_UST' in config_host lttng = declare_dependency(link_args: config_host['LTTNG_UST_LIBS'].split()) endif -urcubp = not_found -if 'CONFIG_TRACE_UST' in config_host - urcubp = declare_dependency(link_args: config_host['URCU_BP_LIBS'].split()) -endif pixman = not_found if have_system or have_tools pixman = dependency('pixman-1', required: have_system, version:'>=0.21.8', diff --git a/trace/meson.build b/trace/meson.build index 08f83a15c3..ef18f11d64 100644 --- a/trace/meson.build +++ b/trace/meson.build @@ -26,7 +26,7 @@ foreach dir : [ '.' ] + trace_events_subdirs input: trace_events_file, command: [ tracetool, group, '--format=ust-events-h', '@INPUT@', '@OUTPUT@' ], depend_files: tracetool_depends) - trace_ss.add(trace_ust_h, lttng, urcubp) + trace_ss.add(trace_ust_h, lttng) genh += trace_ust_h endif trace_ss.add(trace_h, trace_c)