| Message ID | 20220121162234.2707906-4-vsementsov@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | trace qmp commands | expand |
On Fri, Jan 21, 2022 at 11:23 AM Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote: > > 1. Add --no-trace-events to suppress trace events generation in some > cases, and make trace events be generated by default. > 2. Add corresponding .trace-events files as outputs in qapi_files > custom target > 3. Define global qapi_trace_events list of .trace-events file targets, > to fill in trace/qapi.build and to use in trace/meson.build > 4. In trace/meson.build use the new array as an additional source of > .trace_events files to be processed > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> patch 2 and 3 come out clean under the kinda-sorta-linting I've been doing: Acked-by: John Snow <jsnow@redhat.com> (No time to do a bigger review, I'm drowning in backlog ... you and Markus know where to find me if you need something urgently, though.)
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes: > 1. Add --no-trace-events to suppress trace events generation in some > cases, and make trace events be generated by default. > 2. Add corresponding .trace-events files as outputs in qapi_files > custom target > 3. Define global qapi_trace_events list of .trace-events file targets, > to fill in trace/qapi.build and to use in trace/meson.build > 4. In trace/meson.build use the new array as an additional source of > .trace_events files to be processed > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > docs/devel/qapi-code-gen.rst | 23 +++++++++++++++++++++-- The doc update isn't mentioned in the commit message. > meson.build | 3 +++ > qapi/meson.build | 7 +++++++ > qga/meson.build | 11 ++++++++++- > scripts/qapi/main.py | 10 +++++++--- > tests/meson.build | 11 ++++++++++- > trace/meson.build | 11 ++++++++--- > 7 files changed, 66 insertions(+), 10 deletions(-) This commit consists of a small QAPI code generator change, build system work to put it to use, and QAPI documentation update for the series' feature. I'd reshuffle as follows: * Squash the main.py change into the previous commit. * Split off the doc update into its own commit. This way, build system experts can provide an R-by in good conscience without reviewing the doc update, and vice versa.
25.01.2022 13:25, Markus Armbruster wrote: > Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes: > >> 1. Add --no-trace-events to suppress trace events generation in some >> cases, and make trace events be generated by default. >> 2. Add corresponding .trace-events files as outputs in qapi_files >> custom target >> 3. Define global qapi_trace_events list of .trace-events file targets, >> to fill in trace/qapi.build and to use in trace/meson.build >> 4. In trace/meson.build use the new array as an additional source of >> .trace_events files to be processed >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> --- >> docs/devel/qapi-code-gen.rst | 23 +++++++++++++++++++++-- > > The doc update isn't mentioned in the commit message. > >> meson.build | 3 +++ >> qapi/meson.build | 7 +++++++ >> qga/meson.build | 11 ++++++++++- >> scripts/qapi/main.py | 10 +++++++--- >> tests/meson.build | 11 ++++++++++- >> trace/meson.build | 11 ++++++++--- >> 7 files changed, 66 insertions(+), 10 deletions(-) > > This commit consists of a small QAPI code generator change, build system > work to put it to use, and QAPI documentation update for the series' > feature. > > I'd reshuffle as follows: > > * Squash the main.py change into the previous commit. > > * Split off the doc update into its own commit. > > This way, build system experts can provide an R-by in good conscience > without reviewing the doc update, and vice versa. > But I think this way build will fail on previous commit. Or we should still keep trace-generation disabled in previous commit, and enable it only together with meson changes.
25.01.2022 14:03, Vladimir Sementsov-Ogievskiy wrote: > 25.01.2022 13:25, Markus Armbruster wrote: >> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes: >> >>> 1. Add --no-trace-events to suppress trace events generation in some >>> cases, and make trace events be generated by default. >>> 2. Add corresponding .trace-events files as outputs in qapi_files >>> custom target >>> 3. Define global qapi_trace_events list of .trace-events file targets, >>> to fill in trace/qapi.build and to use in trace/meson.build >>> 4. In trace/meson.build use the new array as an additional source of >>> .trace_events files to be processed >>> >>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >>> --- >>> docs/devel/qapi-code-gen.rst | 23 +++++++++++++++++++++-- >> >> The doc update isn't mentioned in the commit message. >> >>> meson.build | 3 +++ >>> qapi/meson.build | 7 +++++++ >>> qga/meson.build | 11 ++++++++++- >>> scripts/qapi/main.py | 10 +++++++--- >>> tests/meson.build | 11 ++++++++++- >>> trace/meson.build | 11 ++++++++--- >>> 7 files changed, 66 insertions(+), 10 deletions(-) >> >> This commit consists of a small QAPI code generator change, build system >> work to put it to use, and QAPI documentation update for the series' >> feature. >> >> I'd reshuffle as follows: >> >> * Squash the main.py change into the previous commit. >> >> * Split off the doc update into its own commit. >> >> This way, build system experts can provide an R-by in good conscience >> without reviewing the doc update, and vice versa. >> > > But I think this way build will fail on previous commit. Or we should still keep trace-generation disabled in previous commit, and enable it only together with meson changes. > May be keep positive option --gen-trace-events in previous commit, like in my previous version? This way meson-changing commit becomes self-sufficient. And then in additional commit change the default and drop --gen-trace-events option and add --no-trace-events instead.
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes: > 25.01.2022 14:03, Vladimir Sementsov-Ogievskiy wrote: >> 25.01.2022 13:25, Markus Armbruster wrote: >>> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes: >>> >>>> 1. Add --no-trace-events to suppress trace events generation in some >>>> cases, and make trace events be generated by default. >>>> 2. Add corresponding .trace-events files as outputs in qapi_files >>>> custom target >>>> 3. Define global qapi_trace_events list of .trace-events file targets, >>>> to fill in trace/qapi.build and to use in trace/meson.build >>>> 4. In trace/meson.build use the new array as an additional source of >>>> .trace_events files to be processed >>>> >>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >>>> --- >>>> docs/devel/qapi-code-gen.rst | 23 +++++++++++++++++++++-- >>> >>> The doc update isn't mentioned in the commit message. >>> >>>> meson.build | 3 +++ >>>> qapi/meson.build | 7 +++++++ >>>> qga/meson.build | 11 ++++++++++- >>>> scripts/qapi/main.py | 10 +++++++--- >>>> tests/meson.build | 11 ++++++++++- >>>> trace/meson.build | 11 ++++++++--- >>>> 7 files changed, 66 insertions(+), 10 deletions(-) >>> >>> This commit consists of a small QAPI code generator change, build system >>> work to put it to use, and QAPI documentation update for the series' >>> feature. >>> >>> I'd reshuffle as follows: >>> >>> * Squash the main.py change into the previous commit. >>> >>> * Split off the doc update into its own commit. >>> >>> This way, build system experts can provide an R-by in good conscience >>> without reviewing the doc update, and vice versa. >>> >> But I think this way build will fail on previous commit. Or we >> should still keep trace-generation disabled in previous commit, and >> enable it only together with meson changes. >> > > May be keep positive option --gen-trace-events in previous commit, like in my previous version? This way meson-changing commit becomes self-sufficient. And then in additional commit change the default and drop --gen-trace-events option and add --no-trace-events instead. You choose. But I'd spell it --gen-trace.
diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst index a3b5473089..a3430740bd 100644 --- a/docs/devel/qapi-code-gen.rst +++ b/docs/devel/qapi-code-gen.rst @@ -1619,7 +1619,10 @@ Code generated for commands These are the marshaling/dispatch functions for the commands defined in the schema. The generated code provides qmp_marshal_COMMAND(), and -declares qmp_COMMAND() that the user must implement. +declares qmp_COMMAND() that the user must implement. The generated code +contains trace events code. Corresponding .trace-events file with list +of trace events is generated too, and should be parsed by trace generator +later to generate trace event code, see `tracing <tracing.html>`. The following files are generated: @@ -1630,6 +1633,9 @@ The following files are generated: ``$(prefix)qapi-commands.h`` Function prototypes for the QMP commands specified in the schema + ``$(prefix)qapi-commands.trace-events`` + Trace events file for trace generator, see `tracing <tracing.html>`. + ``$(prefix)qapi-init-commands.h`` Command initialization prototype @@ -1689,14 +1695,27 @@ Example:: goto out; } + if (trace_event_get_state_backends(TRACE_QMP_ENTER_MY_COMMAND)) { + g_autoptr(GString) req_json = qobject_to_json(QOBJECT(args)); + + trace_qmp_enter_my_command(req_json->str); + } + retval = qmp_my_command(arg.arg1, &err); - error_propagate(errp, err); if (err) { + trace_qmp_exit_my_command(error_get_pretty(err), false); + error_propagate(errp, err); goto out; } qmp_marshal_output_UserDefOne(retval, ret, errp); + if (trace_event_get_state_backends(TRACE_QMP_EXIT_MY_COMMAND)) { + g_autoptr(GString) ret_json = qobject_to_json(*ret); + + trace_qmp_exit_my_command(ret_json->str, true); + } + out: visit_free(v); v = qapi_dealloc_visitor_new(); diff --git a/meson.build b/meson.build index 833fd6bc4c..e0cfafe8d9 100644 --- a/meson.build +++ b/meson.build @@ -41,6 +41,7 @@ qemu_icondir = get_option('datadir') / 'icons' config_host_data = configuration_data() genh = [] +qapi_trace_events = [] target_dirs = config_host['TARGET_DIRS'].split() have_linux_user = false @@ -2557,6 +2558,8 @@ if 'CONFIG_VHOST_USER' in config_host vhost_user = libvhost_user.get_variable('vhost_user_dep') endif +# NOTE: the trace/ subdirectory needs the qapi_trace_events variable +# that is filled in by qapi/. subdir('qapi') subdir('qobject') subdir('stubs') diff --git a/qapi/meson.build b/qapi/meson.build index c0c49c15e4..656ef0e039 100644 --- a/qapi/meson.build +++ b/qapi/meson.build @@ -114,6 +114,7 @@ foreach module : qapi_all_modules 'qapi-events-@0@.h'.format(module), 'qapi-commands-@0@.c'.format(module), 'qapi-commands-@0@.h'.format(module), + 'qapi-commands-@0@.trace-events'.format(module), ] endif if module.endswith('-target') @@ -137,6 +138,9 @@ foreach output : qapi_util_outputs if output.endswith('.h') genh += qapi_files[i] endif + if output.endswith('.trace-events') + qapi_trace_events += qapi_files[i] + endif util_ss.add(qapi_files[i]) i = i + 1 endforeach @@ -145,6 +149,9 @@ foreach output : qapi_specific_outputs + qapi_nonmodule_outputs if output.endswith('.h') genh += qapi_files[i] endif + if output.endswith('.trace-events') + qapi_trace_events += qapi_files[i] + endif specific_ss.add(when: 'CONFIG_SOFTMMU', if_true: qapi_files[i]) i = i + 1 endforeach diff --git a/qga/meson.build b/qga/meson.build index cfb1fbc085..1f2818a1b9 100644 --- a/qga/meson.build +++ b/qga/meson.build @@ -15,10 +15,19 @@ qga_qapi_outputs = [ 'qga-qapi-visit.h', ] +# We don't generate trace-events, just because it's not simple. For do it, +# we also should add .trace-events file into qga_qapi_outputs, and than +# add corresponding element of qga_qapi_files into qapi_trace_events +# global list, which is processed than in trace/meson.build. +# This means, that we'll have to move subdir('qga') above subdir('trace') +# in root meson.build. But that in turn will break the dependency of +# qga on qemuutil (which depends on trace_ss). +# So, resolving these dependencies and drop --no-trace-events is a TODO. qga_qapi_files = custom_target('QGA QAPI files', output: qga_qapi_outputs, input: 'qapi-schema.json', - command: [ qapi_gen, '-o', 'qga', '-p', 'qga-', '@INPUT0@' ], + command: [ qapi_gen, '-o', 'qga', '-p', 'qga-', '@INPUT0@', + '--no-trace-events' ], depend_files: qapi_gen_depends) qga_ss = ss.source_set() diff --git a/scripts/qapi/main.py b/scripts/qapi/main.py index 2e61409f04..d53e5b800c 100644 --- a/scripts/qapi/main.py +++ b/scripts/qapi/main.py @@ -32,7 +32,8 @@ def generate(schema_file: str, output_dir: str, prefix: str, unmask: bool = False, - builtins: bool = False) -> None: + builtins: bool = False, + add_trace_events: bool = False) -> None: """ Generate C code for the given schema into the target directory. @@ -49,7 +50,7 @@ def generate(schema_file: str, schema = QAPISchema(schema_file) gen_types(schema, output_dir, prefix, builtins) gen_visit(schema, output_dir, prefix, builtins) - gen_commands(schema, output_dir, prefix, False) + gen_commands(schema, output_dir, prefix, add_trace_events) gen_events(schema, output_dir, prefix) gen_introspect(schema, output_dir, prefix, unmask) @@ -74,6 +75,8 @@ def main() -> int: parser.add_argument('-u', '--unmask-non-abi-names', action='store_true', dest='unmask', help="expose non-ABI names in introspection") + parser.add_argument('--no-trace-events', action='store_true', + help="suppress adding trace events to qmp marshals") parser.add_argument('schema', action='store') args = parser.parse_args() @@ -88,7 +91,8 @@ def main() -> int: output_dir=args.output_dir, prefix=args.prefix, unmask=args.unmask, - builtins=args.builtins) + builtins=args.builtins, + add_trace_events=not args.no_trace_events) except QAPIError as err: print(f"{sys.argv[0]}: {str(err)}", file=sys.stderr) return 1 diff --git a/tests/meson.build b/tests/meson.build index 3f3882748a..b4b95cc198 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -31,13 +31,22 @@ test_qapi_outputs = [ 'test-qapi-visit.h', ] +# We don't generate trace-events, just because it's not simple. For do it, +# we also should add .trace-events file into test_qapi_outputs, and than +# add corresponding element of test_qapi_files into qapi_trace_events +# global list, which is processed than in trace/meson.build. +# This means, that we'll have to move subdir('tests') above subdir('trace') +# in root meson.build. But that in turn will break the dependency of +# tests on qemuutil (which depends on trace_ss). +# So, resolving these dependencies and drop --no-trace-events is a TODO. test_qapi_files = custom_target('Test QAPI files', output: test_qapi_outputs, input: files('qapi-schema/qapi-schema-test.json', 'qapi-schema/include/sub-module.json', 'qapi-schema/sub-sub-module.json'), command: [ qapi_gen, '-o', meson.current_build_dir(), - '-b', '-p', 'test-', '@INPUT0@' ], + '-b', '-p', 'test-', '@INPUT0@', + '--no-trace-events' ], depend_files: qapi_gen_depends) # meson doesn't like generated output in other directories diff --git a/trace/meson.build b/trace/meson.build index 573dd699c6..c4794a1f2a 100644 --- a/trace/meson.build +++ b/trace/meson.build @@ -2,10 +2,15 @@ specific_ss.add(files('control-target.c')) trace_events_files = [] -foreach dir : [ '.' ] + trace_events_subdirs - trace_events_file = meson.project_source_root() / dir / 'trace-events' +foreach item : [ '.' ] + trace_events_subdirs + qapi_trace_events + if item in qapi_trace_events + trace_events_file = item + group_name = item.full_path().split('/')[-1].underscorify() + else + trace_events_file = meson.project_source_root() / item / 'trace-events' + group_name = item == '.' ? 'root' : item.underscorify() + endif trace_events_files += [ trace_events_file ] - group_name = dir == '.' ? 'root' : dir.underscorify() group = '--group=' + group_name fmt = '@0@-' + group_name + '.@1@'
1. Add --no-trace-events to suppress trace events generation in some cases, and make trace events be generated by default. 2. Add corresponding .trace-events files as outputs in qapi_files custom target 3. Define global qapi_trace_events list of .trace-events file targets, to fill in trace/qapi.build and to use in trace/meson.build 4. In trace/meson.build use the new array as an additional source of .trace_events files to be processed Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- docs/devel/qapi-code-gen.rst | 23 +++++++++++++++++++++-- meson.build | 3 +++ qapi/meson.build | 7 +++++++ qga/meson.build | 11 ++++++++++- scripts/qapi/main.py | 10 +++++++--- tests/meson.build | 11 ++++++++++- trace/meson.build | 11 ++++++++--- 7 files changed, 66 insertions(+), 10 deletions(-)