Re: [PATCH v2 2/3] trace: Add support for recorder back-end

[Daniel P . Berrangé]

On Fri, Jun 26, 2020 at 06:27:05PM +0200, Christophe de Dinechin wrote:
> The recorder library provides support for low-cost continuous
> recording of events, which can then be replayed. This makes it
> possible to collect data "after the fact",for example to show the
> events that led to a crash.
> 
> Recorder support in qemu is implemented using the existing tracing
> interface. In addition, it is possible to individually enable
> recorders that are not traces, although this is probably not
> recommended.
> 
> HMP COMMAND:
> The 'recorder' hmp command has been added, which supports two
> sub-commands:
> - recorder dump: Dump the current state of the recorder. You can
> - recorder trace: Set traces using the recorder_trace_set() syntax.
>   You can use "recorder trace help" to list all available recorders.
> 
> Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
> ---
>  configure                             |  5 +++
>  hmp-commands.hx                       | 19 ++++++++--
>  monitor/misc.c                        | 27 ++++++++++++++
>  scripts/tracetool/backend/recorder.py | 51 +++++++++++++++++++++++++++
>  trace/Makefile.objs                   |  2 ++
>  trace/control.c                       |  7 ++++
>  trace/recorder.c                      | 22 ++++++++++++
>  trace/recorder.h                      | 34 ++++++++++++++++++
>  util/module.c                         |  8 +++++
>  9 files changed, 173 insertions(+), 2 deletions(-)
>  create mode 100644 scripts/tracetool/backend/recorder.py
>  create mode 100644 trace/recorder.c
>  create mode 100644 trace/recorder.h

> +RECORDER_CONSTRUCTOR
> +void recorder_trace_init(void)
> +{
> +    recorder_trace_set(getenv("RECORDER_TRACES"));
> +
> +    // Allow a dump in case we receive some unhandled signal
> +    // For example, send USR2 to a hung process to get a dump
> +    if (getenv("RECORDER_TRACES"))
> +        recorder_dump_on_common_signals(0,0);
> +}

What is the syntax of this RECORDER_TRACES env variable, and perhaps
more importantly should we have this modelled as a command line arg
instead of an env variable. We've generally been aiming to get rid
of env variables and have QAPI modelled CLI. QAPI modelling would be
particularly important if we want to expose the ablity to change
settings on the fly via QMP.


> diff --git a/trace/recorder.h b/trace/recorder.h
> new file mode 100644
> index 0000000000..00b11a2d2f
> --- /dev/null
> +++ b/trace/recorder.h
> @@ -0,0 +1,34 @@
> +/*
> + * Recorder-based trace backend
> + *
> + * Copyright Red Hat 2020
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.

Why "version 2" (only), instead of "version 2 or later" ? QEMU generally
expects the latter for any new code, unless it is derived from existing
v2-only code in QEMU.

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 :|