diff mbox series

[1/2] tracetool: remove redundant --target-type / --target-name args

Message ID 20240108171356.1037059-2-berrange@redhat.com
State New
Headers show
Series trace: fix ability to use systemtap with qemu tools | expand

Commit Message

Daniel P. Berrangé Jan. 8, 2024, 5:13 p.m. UTC
The --target-type and --target-name args are used to construct
the default probe prefix if '--probe-prefix' is not given. The
meson.build will always pass '--probe-prefix', so the other args
are effectively redundant.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 docs/devel/tracing.rst |  3 +--
 meson.build            |  2 --
 scripts/tracetool.py   | 24 +++++-------------------
 3 files changed, 6 insertions(+), 23 deletions(-)

Comments

John Snow Jan. 8, 2024, 9:50 p.m. UTC | #1
On Mon, Jan 8, 2024 at 12:14 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> The --target-type and --target-name args are used to construct
> the default probe prefix if '--probe-prefix' is not given. The
> meson.build will always pass '--probe-prefix', so the other args
> are effectively redundant.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Fine by me, provided there's no reason anyone is calling tracetool
manually for some reason I haven't thought about. I assume we'll hear
about it if so...

Python looks fine, of course.

Reviewed-by: John Snow <jsnow@redhat.com>

(Happy New Year!)

> ---
>  docs/devel/tracing.rst |  3 +--
>  meson.build            |  2 --
>  scripts/tracetool.py   | 24 +++++-------------------
>  3 files changed, 6 insertions(+), 23 deletions(-)
>
> diff --git a/docs/devel/tracing.rst b/docs/devel/tracing.rst
> index d288480db1..043bed7fd0 100644
> --- a/docs/devel/tracing.rst
> +++ b/docs/devel/tracing.rst
> @@ -357,8 +357,7 @@ probes::
>
>      scripts/tracetool.py --backends=dtrace --format=stap \
>                           --binary path/to/qemu-binary \
> -                         --target-type system \
> -                         --target-name x86_64 \
> +                         --probe-prefix qemu.system.x86_64 \
>                           --group=all \
>                           trace-events-all \
>                           qemu.stp
> diff --git a/meson.build b/meson.build
> index 6c77d9687d..535f15da69 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -3934,8 +3934,6 @@ foreach target : target_dirs
>                        command: [
>                          tracetool, '--group=all', '--format=' + stp['fmt'],
>                          '--binary=' + stp['bin'],
> -                        '--target-name=' + target_name,
> -                        '--target-type=' + target_type,
>                          '--probe-prefix=qemu.' + target_type + '.' + target_name,
>                          '@INPUT@', '@OUTPUT@'
>                        ],
> diff --git a/scripts/tracetool.py b/scripts/tracetool.py
> index ab7653a5ce..5de9ce96d3 100755
> --- a/scripts/tracetool.py
> +++ b/scripts/tracetool.py
> @@ -44,12 +44,9 @@ def error_opt(msg = None):
>      --help                   This help message.
>      --list-backends          Print list of available backends.
>      --check-backends         Check if the given backend is valid.
> -    --binary <path>          Full path to QEMU binary.
> -    --target-type <type>     QEMU emulator target type ('system' or 'user').
> -    --target-name <name>     QEMU emulator target name.
> -    --group <name>           Name of the event group
> -    --probe-prefix <prefix>  Prefix for dtrace probe names
> -                             (default: qemu-<target-type>-<target-name>).\
> +    --binary <path>          Full path to QEMU binary (required for 'stap' backend).
> +    --group <name>           Name of the event group.
> +    --probe-prefix <prefix>  Prefix for dtrace probe names (required for 'stap' backend).
>  """ % {
>              "script" : _SCRIPT,
>              "backends" : backend_descr,
> @@ -67,7 +64,7 @@ def main(args):
>
>      long_opts = ["backends=", "format=", "help", "list-backends",
>                   "check-backends", "group="]
> -    long_opts += ["binary=", "target-type=", "target-name=", "probe-prefix="]
> +    long_opts += ["binary=", "probe-prefix="]
>
>      try:
>          opts, args = getopt.getopt(args[1:], "", long_opts)
> @@ -79,8 +76,6 @@ def main(args):
>      arg_format = ""
>      arg_group = None
>      binary = None
> -    target_type = None
> -    target_name = None
>      probe_prefix = None
>      for opt, arg in opts:
>          if opt == "--help":
> @@ -102,10 +97,6 @@ def main(args):
>
>          elif opt == "--binary":
>              binary = arg
> -        elif opt == '--target-type':
> -            target_type = arg
> -        elif opt == '--target-name':
> -            target_name = arg
>          elif opt == '--probe-prefix':
>              probe_prefix = arg
>
> @@ -127,13 +118,8 @@ def main(args):
>      if arg_format == "stap":
>          if binary is None:
>              error_opt("--binary is required for SystemTAP tapset generator")
> -        if probe_prefix is None and target_type is None:
> -            error_opt("--target-type is required for SystemTAP tapset generator")
> -        if probe_prefix is None and target_name is None:
> -            error_opt("--target-name is required for SystemTAP tapset generator")
> -
>          if probe_prefix is None:
> -            probe_prefix = ".".join(["qemu", target_type, target_name])
> +            error_opt("--probe-prefix is required for SystemTAP tapset generator")
>
>      if len(args) < 2:
>          error_opt("missing trace-events and output filepaths")
> --
> 2.43.0
>
Stefan Hajnoczi March 12, 2024, 6:50 p.m. UTC | #2
On Mon, Jan 08, 2024 at 04:50:40PM -0500, John Snow wrote:
> On Mon, Jan 8, 2024 at 12:14 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > The --target-type and --target-name args are used to construct
> > the default probe prefix if '--probe-prefix' is not given. The
> > meson.build will always pass '--probe-prefix', so the other args
> > are effectively redundant.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> Fine by me, provided there's no reason anyone is calling tracetool
> manually for some reason I haven't thought about. I assume we'll hear
> about it if so...

That's okay. tracetool.py is internal to QEMU so we're free to change
the command-line options.

Stefan
diff mbox series

Patch

diff --git a/docs/devel/tracing.rst b/docs/devel/tracing.rst
index d288480db1..043bed7fd0 100644
--- a/docs/devel/tracing.rst
+++ b/docs/devel/tracing.rst
@@ -357,8 +357,7 @@  probes::
 
     scripts/tracetool.py --backends=dtrace --format=stap \
                          --binary path/to/qemu-binary \
-                         --target-type system \
-                         --target-name x86_64 \
+                         --probe-prefix qemu.system.x86_64 \
                          --group=all \
                          trace-events-all \
                          qemu.stp
diff --git a/meson.build b/meson.build
index 6c77d9687d..535f15da69 100644
--- a/meson.build
+++ b/meson.build
@@ -3934,8 +3934,6 @@  foreach target : target_dirs
                       command: [
                         tracetool, '--group=all', '--format=' + stp['fmt'],
                         '--binary=' + stp['bin'],
-                        '--target-name=' + target_name,
-                        '--target-type=' + target_type,
                         '--probe-prefix=qemu.' + target_type + '.' + target_name,
                         '@INPUT@', '@OUTPUT@'
                       ],
diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index ab7653a5ce..5de9ce96d3 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -44,12 +44,9 @@  def error_opt(msg = None):
     --help                   This help message.
     --list-backends          Print list of available backends.
     --check-backends         Check if the given backend is valid.
-    --binary <path>          Full path to QEMU binary.
-    --target-type <type>     QEMU emulator target type ('system' or 'user').
-    --target-name <name>     QEMU emulator target name.
-    --group <name>           Name of the event group
-    --probe-prefix <prefix>  Prefix for dtrace probe names
-                             (default: qemu-<target-type>-<target-name>).\
+    --binary <path>          Full path to QEMU binary (required for 'stap' backend).
+    --group <name>           Name of the event group.
+    --probe-prefix <prefix>  Prefix for dtrace probe names (required for 'stap' backend).
 """ % {
             "script" : _SCRIPT,
             "backends" : backend_descr,
@@ -67,7 +64,7 @@  def main(args):
 
     long_opts = ["backends=", "format=", "help", "list-backends",
                  "check-backends", "group="]
-    long_opts += ["binary=", "target-type=", "target-name=", "probe-prefix="]
+    long_opts += ["binary=", "probe-prefix="]
 
     try:
         opts, args = getopt.getopt(args[1:], "", long_opts)
@@ -79,8 +76,6 @@  def main(args):
     arg_format = ""
     arg_group = None
     binary = None
-    target_type = None
-    target_name = None
     probe_prefix = None
     for opt, arg in opts:
         if opt == "--help":
@@ -102,10 +97,6 @@  def main(args):
 
         elif opt == "--binary":
             binary = arg
-        elif opt == '--target-type':
-            target_type = arg
-        elif opt == '--target-name':
-            target_name = arg
         elif opt == '--probe-prefix':
             probe_prefix = arg
 
@@ -127,13 +118,8 @@  def main(args):
     if arg_format == "stap":
         if binary is None:
             error_opt("--binary is required for SystemTAP tapset generator")
-        if probe_prefix is None and target_type is None:
-            error_opt("--target-type is required for SystemTAP tapset generator")
-        if probe_prefix is None and target_name is None:
-            error_opt("--target-name is required for SystemTAP tapset generator")
-
         if probe_prefix is None:
-            probe_prefix = ".".join(["qemu", target_type, target_name])
+            error_opt("--probe-prefix is required for SystemTAP tapset generator")
 
     if len(args) < 2:
         error_opt("missing trace-events and output filepaths")