[v2,0/3] trace: Add a trace backend for the recorder library

Message

Christophe de Dinechin June 26, 2020, 4:27 p.m. UTC

The recorder library implements low-cost always-on tracing, with three
usage models:

1. Flight recorder: Dump information on recent events in case of crash
2. Tracing: Individual traces can be enabled using environment variables
3. Real-time graphing / control, using the recorder_scope application

This short series introduces a new "recorder" back-end which connects
to the recorder. Traces using the recorder are intentionally "always on".
An example is given of how the recorder can also be used separately
from generated traces. This can be useful if you want to enable
multiple related traces for a particular topic.

This series requires a small makefile fix submitted earlier, included
here for convenience.

Christophe de Dinechin (3):
  Makefile: Compute libraries for libqemuutil.a and libvhost-user.a
  trace: Add support for recorder back-end
  trace: Example of "centralized" recorder tracing

 Makefile                              |  2 ++
 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 +++++
 util/qemu-thread-common.h             |  7 ++++
 11 files changed, 182 insertions(+), 2 deletions(-)
 create mode 100644 scripts/tracetool/backend/recorder.py
 create mode 100644 trace/recorder.c
 create mode 100644 trace/recorder.h

Comments

[e-mail] June 26, 2020, 4:34 p.m. UTC | #1

Patchew URL: https://patchew.org/QEMU/[e-mail]/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH v2 0/3] trace: Add a trace backend for the recorder library
Type: series
Message-id: [e-mail]

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
   10f7ffa..87fb952  master     -> master
 - [tag update]      patchew/[e-mail] -> patchew/[e-mail]
 * [new tag]         patchew/[e-mail] -> patchew/[e-mail]
Switched to a new branch 'test'
850bed5 trace: Example of "centralized" recorder tracing
1f9a8da trace: Add support for recorder back-end
76dd7c6 Makefile: Compute libraries for libqemuutil.a and libvhost-user.a

=== OUTPUT BEGIN ===
1/3 Checking commit 76dd7c67fe5e (Makefile: Compute libraries for libqemuutil.a and libvhost-user.a)
2/3 Checking commit 1f9a8daea6eb (trace: Add support for recorder back-end)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#130: 
new file mode 100644

WARNING: line over 80 characters
#182: FILE: scripts/tracetool/backend/recorder.py:48:
+        out('RECORDER_DEFINE(%(name)s, 8, "Tracetool recorder for %(api)s(%(args)s)");',

ERROR: do not use C99 // comments
#247: FILE: trace/recorder.c:18:
+    // Allow a dump in case we receive some unhandled signal

ERROR: do not use C99 // comments
#248: FILE: trace/recorder.c:19:
+    // For example, send USR2 to a hung process to get a dump

ERROR: braces {} are necessary for all arms of this statement
#249: FILE: trace/recorder.c:20:
+    if (getenv("RECORDER_TRACES"))
[...]

ERROR: space required after that ',' (ctx:VxV)
#250: FILE: trace/recorder.c:21:
+        recorder_dump_on_common_signals(0,0);
                                          ^

ERROR: do not use C99 // comments
#281: FILE: trace/recorder.h:24:
+// Disable recorder macros

ERROR: do not use C99 // comments
#289: FILE: trace/recorder.h:32:
+#endif // CONFIG_TRACE_RECORDER

ERROR: do not use C99 // comments
#291: FILE: trace/recorder.h:34:
+#endif // TRACE_RECORDER_H

ERROR: do not use C99 // comments
#312: FILE: util/module.c:158:
+        // New recorders may have been pulled in, activate them if necessary

total: 8 errors, 2 warnings, 237 lines checked

Patch 2/3 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/3 Checking commit 850bed523e63 (trace: Example of "centralized" recorder tracing)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/[e-mail]/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to [e-mail]

Stefan Hajnoczi June 30, 2020, 12:59 p.m. UTC | #2

On Fri, Jun 26, 2020 at 06:27:03PM +0200, Christophe de Dinechin wrote:
> The recorder library implements low-cost always-on tracing, with three
> usage models:
> 
> 1. Flight recorder: Dump information on recent events in case of crash
> 2. Tracing: Individual traces can be enabled using environment variables
> 3. Real-time graphing / control, using the recorder_scope application
> 
> This short series introduces a new "recorder" back-end which connects
> to the recorder. Traces using the recorder are intentionally "always on".
> An example is given of how the recorder can also be used separately
> from generated traces. This can be useful if you want to enable
> multiple related traces for a particular topic.
> 
> This series requires a small makefile fix submitted earlier, included
> here for convenience.
> 
> Christophe de Dinechin (3):
>   Makefile: Compute libraries for libqemuutil.a and libvhost-user.a
>   trace: Add support for recorder back-end
>   trace: Example of "centralized" recorder tracing

Please add a build to .travis.yml that enables recorder. That way we'll
catch build failures.

Thanks,
Stefan

Christophe de Dinechin July 3, 2020, 10:37 a.m. UTC | #3

On 2020-06-30 at 14:59 CEST, Stefan Hajnoczi wrote...
> On Fri, Jun 26, 2020 at 06:27:03PM +0200, Christophe de Dinechin wrote:
>> The recorder library implements low-cost always-on tracing, with three
>> usage models:
>>
>> 1. Flight recorder: Dump information on recent events in case of crash
>> 2. Tracing: Individual traces can be enabled using environment variables
>> 3. Real-time graphing / control, using the recorder_scope application
>>
>> This short series introduces a new "recorder" back-end which connects
>> to the recorder. Traces using the recorder are intentionally "always on".
>> An example is given of how the recorder can also be used separately
>> from generated traces. This can be useful if you want to enable
>> multiple related traces for a particular topic.
>>
>> This series requires a small makefile fix submitted earlier, included
>> here for convenience.
>>
>> Christophe de Dinechin (3):
>>   Makefile: Compute libraries for libqemuutil.a and libvhost-user.a
>>   trace: Add support for recorder back-end
>>   trace: Example of "centralized" recorder tracing
>
> Please add a build to .travis.yml that enables recorder. That way we'll
> catch build failures.

There is no recorder package in Xenial.

>
> Thanks,
> Stefan



--
Cheers,
Christophe de Dinechin (IRC c3d)

Daniel P. Berrangé July 3, 2020, 11:27 a.m. UTC | #4

On Fri, Jul 03, 2020 at 12:37:02PM +0200, Christophe de Dinechin wrote:
> 
> On 2020-06-30 at 14:59 CEST, Stefan Hajnoczi wrote...
> > On Fri, Jun 26, 2020 at 06:27:03PM +0200, Christophe de Dinechin wrote:
> >> The recorder library implements low-cost always-on tracing, with three
> >> usage models:
> >>
> >> 1. Flight recorder: Dump information on recent events in case of crash
> >> 2. Tracing: Individual traces can be enabled using environment variables
> >> 3. Real-time graphing / control, using the recorder_scope application
> >>
> >> This short series introduces a new "recorder" back-end which connects
> >> to the recorder. Traces using the recorder are intentionally "always on".
> >> An example is given of how the recorder can also be used separately
> >> from generated traces. This can be useful if you want to enable
> >> multiple related traces for a particular topic.
> >>
> >> This series requires a small makefile fix submitted earlier, included
> >> here for convenience.
> >>
> >> Christophe de Dinechin (3):
> >>   Makefile: Compute libraries for libqemuutil.a and libvhost-user.a
> >>   trace: Add support for recorder back-end
> >>   trace: Example of "centralized" recorder tracing
> >
> > Please add a build to .travis.yml that enables recorder. That way we'll
> > catch build failures.
> 
> There is no recorder package in Xenial.

Our .gitlab-ci.yml is going to start using the containers built from
tests/docker/dockerfiles. So if you add the package to the dockerfiles
that support it we can get coverage that way. I presume it'll need the
.gitlab-ci.yml modified to add the extra configure arg too.


Regards,
Daniel