Re: [Qemu-devel] [PATCH v3 4/4] trace: add ability to do simple printf logging via systemtap

[Eric Blake <eblake@redhat.com>]

[-- 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 --]