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

[Vladimir Sementsov-Ogievskiy]

18.01.2022 11:38, Markus Armbruster wrote:
> 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?

Right. Agree, sounds veird

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

OK thanks, I'll reword commit message with these terms)

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

OK

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


--
Best regards,
Vladimir