1. Patchew
  2. QEMU
  3. [PATCH v2 0/4] trace qmp commands
  4. View patch

[PATCH v2 3/4] scripts/qapi-gen.py: add --add-trace-points option

Vladimir Sementsov-Ogievskiy posted 4 patches 4 years, 1 month ago
Maintainers: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>, Stefan Hajnoczi <stefanha@redhat.com>, Michael Roth <michael.roth@amd.com>, John Snow <jsnow@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Markus Armbruster <armbru@redhat.com>
There is a newer version of this series
[PATCH v2 3/4] scripts/qapi-gen.py: add --add-trace-points option
Posted by Vladimir Sementsov-Ogievskiy 4 years, 1 month ago 
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>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.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
Re: [PATCH v2 3/4] scripts/qapi-gen.py: add --add-trace-points option
Posted by Stefan Hajnoczi 4 years ago 
On Thu, Dec 23, 2021 at 12:07:55PM +0100, Vladimir Sementsov-Ogievskiy wrote:
> @@ -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")

Please call it --add-trace-events for consistency with QEMU tracing
terminology.

Thanks,
Stefan
Re: [PATCH v2 3/4] scripts/qapi-gen.py: add --add-trace-points option
Posted by John Snow 4 years ago 
On Thu, Dec 23, 2021 at 6:08 AM Vladimir Sementsov-Ogievskiy
<vsementsov@virtuozzo.com> 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>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.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")

"Add trace events to generated marshaling functions." maybe?

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

I suppose the flag is so that non-QEMU invocations of the QAPI
generator (for tests, etc) will compile correctly without tracepoint
definitions, yeah?
Re: [PATCH v2 3/4] scripts/qapi-gen.py: add --add-trace-points option
Posted by Vladimir Sementsov-Ogievskiy 4 years ago 
12.01.2022 03:38, John Snow wrote:
> On Thu, Dec 23, 2021 at 6:08 AM Vladimir Sementsov-Ogievskiy
> <vsementsov@virtuozzo.com> 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>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.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")
> 
> "Add trace events to generated marshaling functions." maybe?
> 
>>       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
>>
> 
> I suppose the flag is so that non-QEMU invocations of the QAPI
> generator (for tests, etc) will compile correctly without tracepoint
> definitions, yeah?
> 

Yes, I had troubles with tests and some other code units, so I decided to do less to not fix things I'm not interested in) If needed it may be done in separate.

-- 
Best regards,
Vladimir

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.