| Message ID | 20220125215655.3111881-2-vsementsov@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | trace qmp commands | expand |
On Tue, Jan 25, 2022 at 10:56:49PM +0100, Vladimir Sementsov-Ogievskiy wrote: > @@ -264,6 +272,14 @@ def _genh(self) -> QAPIGenH: > assert self._current_module is not None > return self._module[self._current_module][1] > > + @property > + def _gent(self) -> QAPIGenTrace: If you respin maybe rename this to _gentrace() or even _gen_trace_events() so the name is clearer (although the latter collides with the self._gen_trace_events field and may need to be renamed to enable_trace_events or similar).
Let's tweak the title to more closely match existing commits:
qapi/gen: Add FOO.trace-events output module
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:
> We are going to generate trace events for QMP commands. We should
> generate both trace_*() function calls and trace-events files listing
> events for trace generator.
>
> So, add an output module FOO.trace-events for each FOO schema module.
>
> Since we're going to add trace events only to command marshallers,
> make the trace-events output optional, so we don't generate so many
> useless empty files.
>
> Currently nobody set add_trace_events to True, so new functionality is
> disabled. It will be enabled for QAPISchemaGenCommandVisitor
> in a further commit.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> scripts/qapi/gen.py | 33 +++++++++++++++++++++++++++++----
> 1 file changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
> index 995a97d2b8..a41a2c1d55 100644
> --- a/scripts/qapi/gen.py
> +++ b/scripts/qapi/gen.py
> @@ -192,6 +192,11 @@ def _bottom(self) -> str:
> return guardend(self.fname)
>
>
> +class QAPIGenTrace(QAPIGen):
> + def _top(self) -> str:
> + return super()._top() + '# AUTOMATICALLY GENERATED, DO NOT MODIFY\n\n'
> +
> +
> @contextmanager
> def ifcontext(ifcond: QAPISchemaIfCond, *args: QAPIGenCCode) -> Iterator[None]:
> """
> @@ -244,15 +249,18 @@ def __init__(self,
> what: str,
> user_blurb: str,
> builtin_blurb: Optional[str],
> - pydoc: str):
> + pydoc: str,
> + gen_trace_events: bool = False):
Let's rename to @gen_trace for consistency with PATCH 3's --gen-trace.
Hmm, PATCH 7 replaces it by --no-trace-events. I'm going to suggest
--suppress-tracing there. @gen_tracing?
> self._prefix = prefix
> self._what = what
> self._user_blurb = user_blurb
> 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,
> + Optional[QAPIGenTrace]]] = {}
> self._main_module: Optional[str] = None
> + self._gen_trace_events = gen_trace_events
>
> @property
> def _genc(self) -> QAPIGenC:
> @@ -264,6 +272,14 @@ def _genh(self) -> QAPIGenH:
> assert self._current_module is not None
> return self._module[self._current_module][1]
>
> + @property
> + def _gent(self) -> QAPIGenTrace:
> + assert self._gen_trace_events
> + assert self._current_module is not None
> + gent = self._module[self._current_module][2]
> + assert gent is not None
> + return gent
> +
> @staticmethod
> def _module_dirname(name: str) -> str:
> if QAPISchemaModule.is_user_module(name):
> @@ -293,7 +309,14 @@ 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: Optional[QAPIGenTrace]
> + if self._gen_trace_events:
> + gent = QAPIGenTrace(basename + '.trace-events')
> + else:
> + gent = None
A bit more compact:
gent: Optional[QAPIGenTrace] = None
if self._gen_trace_events:
gent = QAPIGenTrace(basename + '.trace-events')
> +
> + self._module[name] = (genc, genh, gent)
> self._current_module = name
>
> @contextmanager
> @@ -304,11 +327,13 @@ 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)
> + if gent is not None:
> + gent.write(output_dir)
>
> def _begin_builtin_module(self) -> None:
> pass
Stefan Hajnoczi <stefanha@redhat.com> writes: > On Tue, Jan 25, 2022 at 10:56:49PM +0100, Vladimir Sementsov-Ogievskiy wrote: >> @@ -264,6 +272,14 @@ def _genh(self) -> QAPIGenH: >> assert self._current_module is not None >> return self._module[self._current_module][1] >> >> + @property >> + def _gent(self) -> QAPIGenTrace: > > If you respin maybe rename this to _gentrace() or even > _gen_trace_events() so the name is clearer (although the latter collides > with the self._gen_trace_events field and may need to be renamed to > enable_trace_events or similar). We have ._genc() for .c, and ._genh() for .h. Applying the same to .trace-events results in ._gentrace_events, but that's ugly. I'm okay with ._gen_trace_events(). Regarding the name collision: I already suggested renaming attribute ._gen_trace_events to ._gen_tracing. We might want to rename ._genc() and .genh() to .gen_c() and .gen_h() for consistency. Don't worry about that now.
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py index 995a97d2b8..a41a2c1d55 100644 --- a/scripts/qapi/gen.py +++ b/scripts/qapi/gen.py @@ -192,6 +192,11 @@ def _bottom(self) -> str: return guardend(self.fname) +class QAPIGenTrace(QAPIGen): + def _top(self) -> str: + return super()._top() + '# AUTOMATICALLY GENERATED, DO NOT MODIFY\n\n' + + @contextmanager def ifcontext(ifcond: QAPISchemaIfCond, *args: QAPIGenCCode) -> Iterator[None]: """ @@ -244,15 +249,18 @@ def __init__(self, what: str, user_blurb: str, builtin_blurb: Optional[str], - pydoc: str): + pydoc: str, + gen_trace_events: bool = False): self._prefix = prefix self._what = what self._user_blurb = user_blurb 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, + Optional[QAPIGenTrace]]] = {} self._main_module: Optional[str] = None + self._gen_trace_events = gen_trace_events @property def _genc(self) -> QAPIGenC: @@ -264,6 +272,14 @@ def _genh(self) -> QAPIGenH: assert self._current_module is not None return self._module[self._current_module][1] + @property + def _gent(self) -> QAPIGenTrace: + assert self._gen_trace_events + assert self._current_module is not None + gent = self._module[self._current_module][2] + assert gent is not None + return gent + @staticmethod def _module_dirname(name: str) -> str: if QAPISchemaModule.is_user_module(name): @@ -293,7 +309,14 @@ 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: Optional[QAPIGenTrace] + if self._gen_trace_events: + gent = QAPIGenTrace(basename + '.trace-events') + else: + gent = None + + self._module[name] = (genc, genh, gent) self._current_module = name @contextmanager @@ -304,11 +327,13 @@ 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) + if gent is not None: + gent.write(output_dir) def _begin_builtin_module(self) -> None: pass
We are going to generate trace events for QMP commands. We should generate both trace_*() function calls and trace-events files listing events for trace generator. So, add an output module FOO.trace-events for each FOO schema module. Since we're going to add trace events only to command marshallers, make the trace-events output optional, so we don't generate so many useless empty files. Currently nobody set add_trace_events to True, so new functionality is disabled. It will be enabled for QAPISchemaGenCommandVisitor in a further commit. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- scripts/qapi/gen.py | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-)