diff mbox series

[v2] qemu-trace-stap: changing SYSTEMTAP_TAPSET considered harmful.

Message ID 20210409105810.374976-1-kraxel@redhat.com (mailing list archive)
State New
Headers show
Series [v2] qemu-trace-stap: changing SYSTEMTAP_TAPSET considered harmful. | expand

Commit Message

Gerd Hoffmann April 9, 2021, 10:58 a.m. UTC
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 <kraxel@redhat.com>
---
 scripts/qemu-trace-stap | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

Comments

Daniel P. Berrangé April 9, 2021, 11:17 a.m. UTC | #1
On Fri, Apr 09, 2021 at 12:58:10PM +0200, Gerd Hoffmann wrote:
> 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.

Do you know if "-I tapsetdir" appends or prepends to the search path ?

We need it to prepend to ensure that we override any QEMU stp files
that might be already present in /usr/share from an RPM provided
QEMU binary.


Regards,
Daniel
Gerd Hoffmann April 9, 2021, 11:43 a.m. UTC | #2
On Fri, Apr 09, 2021 at 12:17:28PM +0100, Daniel P. Berrangé wrote:
> On Fri, Apr 09, 2021 at 12:58:10PM +0200, Gerd Hoffmann wrote:
> > 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.
> 
> Do you know if "-I tapsetdir" appends or prepends to the search path ?
> 
> We need it to prepend to ensure that we override any QEMU stp files
> that might be already present in /usr/share from an RPM provided
> QEMU binary.

Dunno, "man stap" isn't clear on that one.  strace shows the files in
the SYSTEMTAP_TAPSET directory are read first.  That doesn't imply those
probes have a higher priority though.

take care,
  Gerd
diff mbox series

Patch

diff --git a/scripts/qemu-trace-stap b/scripts/qemu-trace-stap
index 90527eb974f4..eb6e951ff235 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))