All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC 0/3] trace qmp commands
@ 2021-12-21 19:34 Vladimir Sementsov-Ogievskiy
  2021-12-21 19:35 ` [PATCH 1/3] scripts/qapi/commands: gen_commands(): add add_trace_points argument Vladimir Sementsov-Ogievskiy
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-12-21 19:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: stefanha, michael.roth, armbru, pbonzini, vsementsov

Hi all!

This series aims to add trace points for each qmp command with help of
qapi code generator.

That's a replacement for my
"[PATCH 0/5] trace: inroduce qmp: trace namespace"
Supersedes: <20210923195451.714796-1-vsementsov@virtuozzo.com>


There are some problems, so that's an RFC.

1. Main problem is that I failed to teach meson how to build trace
points for generated trace-events files. I can build it, but.. See
commit 03 for the description.

2. I added --add-trace-points option to qapi-gen, to generate trace
points optionally. Probably that would be better to do it without an
option, but I failed to fix build for qmp commands in qga and in tests

Vladimir Sementsov-Ogievskiy (3):
  scripts/qapi/commands: gen_commands(): add add_trace_points argument
  scripts/qapi-gen.py: add --add-trace-points option
  meson: generate trace points for qmp commands

 meson.build              |  1 +
 qapi/meson.build         |  4 +-
 scripts/qapi/commands.py | 84 ++++++++++++++++++++++++++++++++++------
 scripts/qapi/gen.py      | 13 +++++--
 scripts/qapi/main.py     | 10 +++--
 trace/meson.build        | 10 +++--
 6 files changed, 101 insertions(+), 21 deletions(-)

-- 
2.31.1



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/3] scripts/qapi/commands: gen_commands(): add add_trace_points argument
  2021-12-21 19:34 [PATCH RFC 0/3] trace qmp commands Vladimir Sementsov-Ogievskiy
@ 2021-12-21 19:35 ` Vladimir Sementsov-Ogievskiy
  2021-12-23 10:38   ` Vladimir Sementsov-Ogievskiy
  2021-12-21 19:35 ` [PATCH 2/3] scripts/qapi-gen.py: add --add-trace-points option Vladimir Sementsov-Ogievskiy
  2021-12-21 19:35 ` [PATCH 3/3] meson: generate trace points for qmp commands Vladimir Sementsov-Ogievskiy
  2 siblings, 1 reply; 9+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-12-21 19:35 UTC (permalink / raw)
  To: qemu-devel; +Cc: stefanha, michael.roth, armbru, pbonzini, vsementsov

Add possibility to generate trace points for each qmp command.

We should generate both trace points and trace-events file, for further
trace point code generation.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 scripts/qapi/commands.py | 84 ++++++++++++++++++++++++++++++++++------
 1 file changed, 73 insertions(+), 11 deletions(-)

diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index 21001bbd6b..e62f1a4125 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -53,7 +53,8 @@ def gen_command_decl(name: str,
 def gen_call(name: str,
              arg_type: Optional[QAPISchemaObjectType],
              boxed: bool,
-             ret_type: Optional[QAPISchemaType]) -> str:
+             ret_type: Optional[QAPISchemaType],
+             add_trace_points: bool) -> str:
     ret = ''
 
     argstr = ''
@@ -71,21 +72,65 @@ def gen_call(name: str,
     if ret_type:
         lhs = 'retval = '
 
-    ret = mcgen('''
+    qmp_name = f'qmpq_{c_name(name)}'
+    upper = qmp_name.upper()
+
+    if add_trace_points:
+        ret += mcgen('''
+
+    if (trace_event_get_state_backends(TRACE_%(upper)s)) {
+        g_autoptr(GString) req_json = qobject_to_json(QOBJECT(args));
+        trace_%(qmp_name)s("", req_json->str);
+    }
+    ''',
+                     upper=upper, qmp_name=qmp_name)
+
+    ret += mcgen('''
 
     %(lhs)sqmp_%(c_name)s(%(args)s&err);
-    error_propagate(errp, err);
 ''',
                 c_name=c_name(name), args=argstr, lhs=lhs)
-    if ret_type:
-        ret += mcgen('''
+
+    ret += mcgen('''
     if (err) {
+''')
+
+    if add_trace_points:
+        ret += mcgen('''
+        trace_%(qmp_name)s("FAIL: ", error_get_pretty(err));
+''',
+                     qmp_name=qmp_name)
+
+    ret += mcgen('''
+        error_propagate(errp, err);
         goto out;
     }
+''')
+
+    if ret_type:
+        ret += mcgen('''
 
     qmp_marshal_output_%(c_name)s(retval, ret, errp);
 ''',
                      c_name=ret_type.c_name())
+
+    if add_trace_points:
+        if ret_type:
+            ret += mcgen('''
+
+    if (trace_event_get_state_backends(TRACE_%(upper)s)) {
+        g_autoptr(GString) ret_json = qobject_to_json(*ret);
+        trace_%(qmp_name)s("RET:", ret_json->str);
+    }
+    ''',
+                     upper=upper, qmp_name=qmp_name)
+        else:
+            ret += mcgen('''
+
+    trace_%(qmp_name)s("SUCCESS", "");
+    ''',
+                         qmp_name=qmp_name)
+
     return ret
 
 
@@ -122,10 +167,14 @@ def gen_marshal_decl(name: str) -> str:
                  proto=build_marshal_proto(name))
 
 
+def gen_trace(name: str) -> str:
+    return f'qmpq_{c_name(name)}(const char *tag, const char *json) "%s%s"\n'
+
 def gen_marshal(name: str,
                 arg_type: Optional[QAPISchemaObjectType],
                 boxed: bool,
-                ret_type: Optional[QAPISchemaType]) -> str:
+                ret_type: Optional[QAPISchemaType],
+                add_trace_points: bool) -> str:
     have_args = boxed or (arg_type and not arg_type.is_empty())
     if have_args:
         assert arg_type is not None
@@ -180,7 +229,7 @@ def gen_marshal(name: str,
     }
 ''')
 
-    ret += gen_call(name, arg_type, boxed, ret_type)
+    ret += gen_call(name, arg_type, boxed, ret_type, add_trace_points)
 
     ret += mcgen('''
 
@@ -238,11 +287,12 @@ def gen_register_command(name: str,
 
 
 class QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor):
-    def __init__(self, prefix: str):
+    def __init__(self, prefix: str, add_trace_points: bool):
         super().__init__(
             prefix, 'qapi-commands',
             ' * Schema-defined QAPI/QMP commands', None, __doc__)
         self._visited_ret_types: Dict[QAPIGenC, Set[QAPISchemaType]] = {}
+        self.add_trace_points = add_trace_points
 
     def _begin_user_module(self, name: str) -> None:
         self._visited_ret_types[self._genc] = set()
@@ -261,6 +311,15 @@ def _begin_user_module(self, name: str) -> None:
 
 ''',
                              commands=commands, visit=visit))
+
+        if self.add_trace_points and c_name(commands) != 'qapi_commands':
+            self._genc.add(mcgen('''
+#include "trace/trace-qapi.h"
+#include "qapi/qmp/qjson.h"
+#include "trace/trace-%(nm)s_trace_events.h"
+''',
+                                 nm=c_name(commands)))
+
         self._genh.add(mcgen('''
 #include "%(types)s.h"
 
@@ -322,7 +381,9 @@ def visit_command(self,
         with ifcontext(ifcond, self._genh, self._genc):
             self._genh.add(gen_command_decl(name, arg_type, boxed, ret_type))
             self._genh.add(gen_marshal_decl(name))
-            self._genc.add(gen_marshal(name, arg_type, boxed, ret_type))
+            self._genc.add(gen_marshal(name, arg_type, boxed, ret_type,
+                                       self.add_trace_points))
+            self._gent.add(gen_trace(name))
         with self._temp_module('./init'):
             with ifcontext(ifcond, self._genh, self._genc):
                 self._genc.add(gen_register_command(
@@ -332,7 +393,8 @@ def visit_command(self,
 
 def gen_commands(schema: QAPISchema,
                  output_dir: str,
-                 prefix: str) -> None:
-    vis = QAPISchemaGenCommandVisitor(prefix)
+                 prefix: str,
+                 add_trace_points: bool) -> None:
+    vis = QAPISchemaGenCommandVisitor(prefix, add_trace_points)
     schema.visit(vis)
     vis.write(output_dir)
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/3] scripts/qapi-gen.py: add --add-trace-points option
  2021-12-21 19:34 [PATCH RFC 0/3] trace qmp commands Vladimir Sementsov-Ogievskiy
  2021-12-21 19:35 ` [PATCH 1/3] scripts/qapi/commands: gen_commands(): add add_trace_points argument Vladimir Sementsov-Ogievskiy
@ 2021-12-21 19:35 ` Vladimir Sementsov-Ogievskiy
  2021-12-23  6:50   ` Philippe Mathieu-Daudé
  2021-12-21 19:35 ` [PATCH 3/3] meson: generate trace points for qmp commands Vladimir Sementsov-Ogievskiy
  2 siblings, 1 reply; 9+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-12-21 19:35 UTC (permalink / raw)
  To: qemu-devel; +Cc: stefanha, michael.roth, armbru, pbonzini, vsementsov

Add and option to generate trace points. We should generate both trace
points and trace-events files for further trace point code generation.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 scripts/qapi/gen.py  | 13 ++++++++++---
 scripts/qapi/main.py | 10 +++++++---
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index 995a97d2b8..605b3fe68a 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -251,7 +251,7 @@ def __init__(self,
         self._builtin_blurb = builtin_blurb
         self._pydoc = pydoc
         self._current_module: Optional[str] = None
-        self._module: Dict[str, Tuple[QAPIGenC, QAPIGenH]] = {}
+        self._module: Dict[str, Tuple[QAPIGenC, QAPIGenH, QAPIGen]] = {}
         self._main_module: Optional[str] = None
 
     @property
@@ -264,6 +264,11 @@ def _genh(self) -> QAPIGenH:
         assert self._current_module is not None
         return self._module[self._current_module][1]
 
+    @property
+    def _gent(self) -> QAPIGen:
+        assert self._current_module is not None
+        return self._module[self._current_module][2]
+
     @staticmethod
     def _module_dirname(name: str) -> str:
         if QAPISchemaModule.is_user_module(name):
@@ -293,7 +298,8 @@ def _add_module(self, name: str, blurb: str) -> None:
         basename = self._module_filename(self._what, name)
         genc = QAPIGenC(basename + '.c', blurb, self._pydoc)
         genh = QAPIGenH(basename + '.h', blurb, self._pydoc)
-        self._module[name] = (genc, genh)
+        gent = QAPIGen(basename + '.trace-events')
+        self._module[name] = (genc, genh, gent)
         self._current_module = name
 
     @contextmanager
@@ -304,11 +310,12 @@ def _temp_module(self, name: str) -> Iterator[None]:
         self._current_module = old_module
 
     def write(self, output_dir: str, opt_builtins: bool = False) -> None:
-        for name, (genc, genh) in self._module.items():
+        for name, (genc, genh, gent) in self._module.items():
             if QAPISchemaModule.is_builtin_module(name) and not opt_builtins:
                 continue
             genc.write(output_dir)
             genh.write(output_dir)
+            gent.write(output_dir)
 
     def _begin_builtin_module(self) -> None:
         pass
diff --git a/scripts/qapi/main.py b/scripts/qapi/main.py
index f2ea6e0ce4..3adf0319cf 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_points: 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)
+    gen_commands(schema, output_dir, prefix, add_trace_points)
     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('--add-trace-points', action='store_true',
+                        help="add trace points 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_points=args.add_trace_points)
     except QAPIError as err:
         print(f"{sys.argv[0]}: {str(err)}", file=sys.stderr)
         return 1
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/3] meson: generate trace points for qmp commands
  2021-12-21 19:34 [PATCH RFC 0/3] trace qmp commands Vladimir Sementsov-Ogievskiy
  2021-12-21 19:35 ` [PATCH 1/3] scripts/qapi/commands: gen_commands(): add add_trace_points argument Vladimir Sementsov-Ogievskiy
  2021-12-21 19:35 ` [PATCH 2/3] scripts/qapi-gen.py: add --add-trace-points option Vladimir Sementsov-Ogievskiy
@ 2021-12-21 19:35 ` Vladimir Sementsov-Ogievskiy
  2021-12-22 22:11   ` Paolo Bonzini
  2 siblings, 1 reply; 9+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-12-21 19:35 UTC (permalink / raw)
  To: qemu-devel; +Cc: stefanha, michael.roth, armbru, pbonzini, vsementsov

I need help with this thing. Now it works like this:

make -j9
ninja: error: '/work/src/qemu/up/up-trace-qmp-commands/build/qapi/qapi-commands-authz.trace-events', needed by 'trace/trace-events-all', missing and no known rule to make it
make[1]: *** [Makefile:162: run-ninja] Error 1
make[1]: Leaving directory '/work/src/qemu/up/up-trace-qmp-commands/build'
make: *** [GNUmakefile:11: all] Error 2

OK, let's try to make it by hand:

make qapi/qapi-commands-authz.trace-events
changing dir to build for make "qapi/qapi-commands-authz.trace-events"...
make[1]: Entering directory '/work/src/qemu/up/up-trace-qmp-commands/build'
  GIT     ui/keycodemapdb meson tests/fp/berkeley-testfloat-3 tests/fp/berkeley-softfloat-3 dtc capstone slirp
[1/1] Generating shared QAPI source files with a custom command
make[1]: Leaving directory '/work/src/qemu/up/up-trace-qmp-commands/build'

It works! So meson doesn't understand that absolute path is the same
as relative.. But I failed to make it the correct way :(

And after it, just run "make" again and it build the whole project.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 meson.build       |  1 +
 qapi/meson.build  |  4 +++-
 trace/meson.build | 10 +++++++---
 3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/meson.build b/meson.build
index f45ecf31bd..20d32fd20d 100644
--- a/meson.build
+++ b/meson.build
@@ -38,6 +38,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
diff --git a/qapi/meson.build b/qapi/meson.build
index c0c49c15e4..333ca60583 100644
--- a/qapi/meson.build
+++ b/qapi/meson.build
@@ -114,7 +114,9 @@ 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),
     ]
+    qapi_trace_events += ['qapi-commands-@0@.trace-events'.format(module)]
   endif
   if module.endswith('-target')
     qapi_specific_outputs += qapi_module_outputs
@@ -126,7 +128,7 @@ endforeach
 qapi_files = custom_target('shared QAPI source files',
   output: qapi_util_outputs + qapi_specific_outputs + qapi_nonmodule_outputs,
   input: [ files('qapi-schema.json') ],
-  command: [ qapi_gen, '-o', 'qapi', '-b', '@INPUT0@' ],
+  command: [ qapi_gen, '-o', 'qapi', '-b', '@INPUT0@', '--add-trace-points' ],
   depend_files: [ qapi_inputs, qapi_gen_depends ])
 
 # Now go through all the outputs and add them to the right sourceset.
diff --git a/trace/meson.build b/trace/meson.build
index 573dd699c6..77e44fa68d 100644
--- a/trace/meson.build
+++ b/trace/meson.build
@@ -2,10 +2,14 @@
 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 path : [ '.' ] + trace_events_subdirs + qapi_trace_events
+  if path.contains('trace-events')
+    trace_events_file = meson.project_build_root() / 'qapi' / path
+  else
+    trace_events_file = meson.project_source_root() / path / 'trace-events'
+  endif
   trace_events_files += [ trace_events_file ]
-  group_name = dir == '.' ? 'root' : dir.underscorify()
+  group_name = path == '.' ? 'root' : path.underscorify()
   group = '--group=' + group_name
   fmt = '@0@-' + group_name + '.@1@'
 
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] meson: generate trace points for qmp commands
  2021-12-21 19:35 ` [PATCH 3/3] meson: generate trace points for qmp commands Vladimir Sementsov-Ogievskiy
@ 2021-12-22 22:11   ` Paolo Bonzini
  2021-12-23  9:33     ` Vladimir Sementsov-Ogievskiy
  0 siblings, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2021-12-22 22:11 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: Michael Roth, qemu-devel, Hajnoczi, Stefan, Armbruster, Markus

[-- Attachment #1: Type: text/plain, Size: 805 bytes --]

Il mar 21 dic 2021, 20:35 Vladimir Sementsov-Ogievskiy <
vsementsov@virtuozzo.com> ha scritto:

--- a/trace/meson.build
+++ b/trace/meson.build
@@ -2,10 +2,14 @@
 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 path : [ '.' ] + trace_events_subdirs + qapi_trace_events
+  if path.contains('trace-events')
+    trace_events_file = meson.project_build_root() / 'qapi' / path



Just using "trace_events_file = 'qapi' / path" might work, since the build
is nonrecursive.

If it doesn't, use the custom target object, possibly indexing it as
ct[index]. You can use a dictionary to store the custom targets and find
them from the "path" variable.

Paolo


>
>

[-- Attachment #2: Type: text/html, Size: 1549 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] scripts/qapi-gen.py: add --add-trace-points option
  2021-12-21 19:35 ` [PATCH 2/3] scripts/qapi-gen.py: add --add-trace-points option Vladimir Sementsov-Ogievskiy
@ 2021-12-23  6:50   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-12-23  6:50 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-devel
  Cc: michael.roth, armbru, stefanha, pbonzini

On 12/21/21 20:35, Vladimir Sementsov-Ogievskiy wrote:
> Add and option to generate trace points. We should generate both trace
> points and trace-events files for further trace point code generation.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  scripts/qapi/gen.py  | 13 ++++++++++---
>  scripts/qapi/main.py | 10 +++++++---
>  2 files changed, 17 insertions(+), 6 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] meson: generate trace points for qmp commands
  2021-12-22 22:11   ` Paolo Bonzini
@ 2021-12-23  9:33     ` Vladimir Sementsov-Ogievskiy
  2021-12-23 10:41       ` Vladimir Sementsov-Ogievskiy
  0 siblings, 1 reply; 9+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-12-23  9:33 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: qemu-devel, Hajnoczi, Stefan, Michael Roth, Armbruster, Markus

23.12.2021 01:11, Paolo Bonzini wrote:
> Il mar 21 dic 2021, 20:35 Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com <mailto:vsementsov@virtuozzo.com>> ha scritto:
> 
>     --- a/trace/meson.build
>     +++ b/trace/meson.build
>     @@ -2,10 +2,14 @@
>       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 path : [ '.' ] + trace_events_subdirs + qapi_trace_events
>     +  if path.contains('trace-events')
>     +    trace_events_file = meson.project_build_root() / 'qapi' / path
> 
> 
> 
> Just using "trace_events_file = 'qapi' / path" might work, since the build is nonrecursive.

This say:

ninja: error: '../trace/qapi/qapi-commands-authz.trace-events', needed by 'trace/trace-events-all', missing and no known rule to make it
make[1]: *** [Makefile:162: run-ninja] Error 1
make[1]: Leaving directory '/work/src/qemu/up/up-trace-qmp-commands/build'
make: *** [GNUmakefile:11: all] Error 2


so, it consider the path relative to current "trace" directory.

> 
> If it doesn't, use the custom target object, possibly indexing it as ct[index]. You can use a dictionary to store the custom targets and find them from the "path" variable.
> 

O! Great thanks! Magic. The following hack works:

diff --git a/meson.build b/meson.build
index 20d32fd20d..c42a76a14c 100644
--- a/meson.build
+++ b/meson.build
@@ -39,6 +39,7 @@ qemu_icondir = get_option('datadir') / 'icons'
  config_host_data = configuration_data()
  genh = []
  qapi_trace_events = []
+qapi_trace_events_targets = {}
  
  target_dirs = config_host['TARGET_DIRS'].split()
  have_linux_user = false
diff --git a/qapi/meson.build b/qapi/meson.build
index 333ca60583..d4de04459d 100644
--- a/qapi/meson.build
+++ b/qapi/meson.build
@@ -139,6 +139,9 @@ foreach output : qapi_util_outputs
    if output.endswith('.h')
      genh += qapi_files[i]
    endif
+  if output.endswith('.trace-events')
+    qapi_trace_events_targets += {output: qapi_files[i]}
+  endif
    util_ss.add(qapi_files[i])
    i = i + 1
  endforeach
@@ -147,6 +150,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_targets += {output: qapi_files[i]}
+  endif
    specific_ss.add(when: 'CONFIG_SOFTMMU', if_true: qapi_files[i])
    i = i + 1
  endforeach
diff --git a/trace/meson.build b/trace/meson.build
index 77e44fa68d..daa24c3a2d 100644
--- a/trace/meson.build
+++ b/trace/meson.build
@@ -4,7 +4,7 @@ specific_ss.add(files('control-target.c'))
  trace_events_files = []
  foreach path : [ '.' ] + trace_events_subdirs + qapi_trace_events
    if path.contains('trace-events')
-    trace_events_file = meson.project_build_root() / 'qapi' / path
+    trace_events_file = qapi_trace_events_targets[path]
    else
      trace_events_file = meson.project_source_root() / path / 'trace-events'
    endif



-- 
Best regards,
Vladimir


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] scripts/qapi/commands: gen_commands(): add add_trace_points argument
  2021-12-21 19:35 ` [PATCH 1/3] scripts/qapi/commands: gen_commands(): add add_trace_points argument Vladimir Sementsov-Ogievskiy
@ 2021-12-23 10:38   ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 9+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-12-23 10:38 UTC (permalink / raw)
  To: qemu-devel; +Cc: stefanha, michael.roth, armbru, pbonzini

21.12.2021 22:35, Vladimir Sementsov-Ogievskiy wrote:
> Add possibility to generate trace points for each qmp command.
> 
> We should generate both trace points and trace-events file, for further
> trace point code generation.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy<vsementsov@virtuozzo.com>
> ---
>   scripts/qapi/commands.py | 84 ++++++++++++++++++++++++++++++++++------
>   1 file changed, 73 insertions(+), 11 deletions(-)
> 
> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
> index 21001bbd6b..e62f1a4125 100644
> --- a/scripts/qapi/commands.py
> +++ b/scripts/qapi/commands.py
> @@ -53,7 +53,8 @@ def gen_command_decl(name: str,
>   def gen_call(name: str,
>                arg_type: Optional[QAPISchemaObjectType],
>                boxed: bool,
> -             ret_type: Optional[QAPISchemaType]) -> str:
> +             ret_type: Optional[QAPISchemaType],
> +             add_trace_points: bool) -> str:
>       ret = ''
>   
>       argstr = ''
> @@ -71,21 +72,65 @@ def gen_call(name: str,
>       if ret_type:
>           lhs = 'retval = '
>   
> -    ret = mcgen('''
> +    qmp_name = f'qmpq_{c_name(name)}'

That was called qmpq_ because qmp_ conflicts with existing qmp_ trace points for jobs. But looking at them, they don't add much information to new qmpq_ trace events, so, in v2 I'll remove old qmp_ trace points (not many of them) and new generated trace points will be named simply qmp_*


-- 
Best regards,
Vladimir


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] meson: generate trace points for qmp commands
  2021-12-23  9:33     ` Vladimir Sementsov-Ogievskiy
@ 2021-12-23 10:41       ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 9+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-12-23 10:41 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: qemu-devel, Hajnoczi, Stefan, Michael Roth, Armbruster, Markus

23.12.2021 12:33, Vladimir Sementsov-Ogievskiy wrote:
> 23.12.2021 01:11, Paolo Bonzini wrote:
>> Il mar 21 dic 2021, 20:35 Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com <mailto:vsementsov@virtuozzo.com>> ha scritto:
>>
>>     --- a/trace/meson.build
>>     +++ b/trace/meson.build
>>     @@ -2,10 +2,14 @@
>>       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 path : [ '.' ] + trace_events_subdirs + qapi_trace_events
>>     +  if path.contains('trace-events')
>>     +    trace_events_file = meson.project_build_root() / 'qapi' / path
>>
>>
>>
>> Just using "trace_events_file = 'qapi' / path" might work, since the build is nonrecursive.
> 
> This say:
> 
> ninja: error: '../trace/qapi/qapi-commands-authz.trace-events', needed by 'trace/trace-events-all', missing and no known rule to make it
> make[1]: *** [Makefile:162: run-ninja] Error 1
> make[1]: Leaving directory '/work/src/qemu/up/up-trace-qmp-commands/build'
> make: *** [GNUmakefile:11: all] Error 2
> 
> 
> so, it consider the path relative to current "trace" directory.
> 
>>
>> If it doesn't, use the custom target object, possibly indexing it as ct[index]. You can use a dictionary to store the custom targets and find them from the "path" variable.
>>
> 
> O! Great thanks! Magic. The following hack works:
> 
> diff --git a/meson.build b/meson.build
> index 20d32fd20d..c42a76a14c 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -39,6 +39,7 @@ qemu_icondir = get_option('datadir') / 'icons'
>   config_host_data = configuration_data()
>   genh = []
>   qapi_trace_events = []
> +qapi_trace_events_targets = {}
> 
>   target_dirs = config_host['TARGET_DIRS'].split()
>   have_linux_user = false
> diff --git a/qapi/meson.build b/qapi/meson.build
> index 333ca60583..d4de04459d 100644
> --- a/qapi/meson.build
> +++ b/qapi/meson.build
> @@ -139,6 +139,9 @@ foreach output : qapi_util_outputs
>     if output.endswith('.h')
>       genh += qapi_files[i]
>     endif
> +  if output.endswith('.trace-events')
> +    qapi_trace_events_targets += {output: qapi_files[i]}
> +  endif
>     util_ss.add(qapi_files[i])
>     i = i + 1
>   endforeach
> @@ -147,6 +150,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_targets += {output: qapi_files[i]}
> +  endif
>     specific_ss.add(when: 'CONFIG_SOFTMMU', if_true: qapi_files[i])
>     i = i + 1
>   endforeach
> diff --git a/trace/meson.build b/trace/meson.build
> index 77e44fa68d..daa24c3a2d 100644
> --- a/trace/meson.build
> +++ b/trace/meson.build
> @@ -4,7 +4,7 @@ specific_ss.add(files('control-target.c'))
>   trace_events_files = []
>   foreach path : [ '.' ] + trace_events_subdirs + qapi_trace_events
>     if path.contains('trace-events')
> -    trace_events_file = meson.project_build_root() / 'qapi' / path
> +    trace_events_file = qapi_trace_events_targets[path]
>     else
>       trace_events_file = meson.project_source_root() / path / 'trace-events'
>     endif
> 
> 
> 

Or even simpler, I can use a list combined from needed qapi_files[] elements. So, the solution is to use custom target objects or their indexed subobjects instead of raw paths. This way Meson resolves dependencies better.

-- 
Best regards,
Vladimir


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-12-23 10:42 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-21 19:34 [PATCH RFC 0/3] trace qmp commands Vladimir Sementsov-Ogievskiy
2021-12-21 19:35 ` [PATCH 1/3] scripts/qapi/commands: gen_commands(): add add_trace_points argument Vladimir Sementsov-Ogievskiy
2021-12-23 10:38   ` Vladimir Sementsov-Ogievskiy
2021-12-21 19:35 ` [PATCH 2/3] scripts/qapi-gen.py: add --add-trace-points option Vladimir Sementsov-Ogievskiy
2021-12-23  6:50   ` Philippe Mathieu-Daudé
2021-12-21 19:35 ` [PATCH 3/3] meson: generate trace points for qmp commands Vladimir Sementsov-Ogievskiy
2021-12-22 22:11   ` Paolo Bonzini
2021-12-23  9:33     ` Vladimir Sementsov-Ogievskiy
2021-12-23 10:41       ` Vladimir Sementsov-Ogievskiy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.