Re: [PATCH v3 1/3] scripts/qapi/gen.py: add .trace-events file for module

[Markus Armbruster <armbru@redhat.com>]

Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:

> We are going to generate trace events for qmp commands. We should
> generate both trace events and trace-events.

What do you mean to say with the second sentence?  "trace events" == the
calls of generated trace_FOO(), and "trace-events" == the input file for
tracetool?

> For now, add .trace-events file object, to be filled in further commit.

What is a file object?  See below.

> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  scripts/qapi/gen.py | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 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')

Aha.  You're adding an output module FOO.trace-events for schema module
FOO.

We commonly use a single trace-events per directory, but I believe your
choice of one per module is simpler for the QAPI generator, and just
fine for tracing.

Please add

   class QAPIGenTrace(QAPIGen):
   
       def _top(self, fname):
           return (QAPIGen._top(self, fname)
                   + '# AUTOMATICALLY GENERATED, DO NOT MODIFY\n\n')


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