New attributed added in backends
CHECK_TRACE_EVENT_GET_STATE which when
present and is True wraps the code generated
by generate function in check_trace_event_get_state
check else it is outside the conditional block.
Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
---
scripts/tracetool/__init__.py | 1 -
scripts/tracetool/backend/__init__.py | 26 ++++++++++++++++-------
scripts/tracetool/format/h.py | 30 ++++++++++-----------------
3 files changed, 30 insertions(+), 27 deletions(-)
diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index 2ae2e562d6..d0a02c45d7 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -332,7 +332,6 @@ def formats(self):
return self._FMT.findall(self.fmt)
QEMU_TRACE = "trace_%(name)s"
- QEMU_TRACE_NOCHECK = "_nocheck__" + QEMU_TRACE
QEMU_TRACE_TCG = QEMU_TRACE + "_tcg"
QEMU_DSTATE = "_TRACE_%(NAME)s_DSTATE"
QEMU_BACKEND_DSTATE = "TRACE_%(NAME)s_BACKEND_DSTATE"
diff --git a/scripts/tracetool/backend/__init__.py b/scripts/tracetool/backend/__init__.py
index 7bfcc86cc5..4194719e52 100644
--- a/scripts/tracetool/backend/__init__.py
+++ b/scripts/tracetool/backend/__init__.py
@@ -23,6 +23,8 @@
Attribute Description
========= ====================================================================
PUBLIC If exists and is set to 'True', the backend is considered "public".
+CHECK_TRACE_EVENT_GET_STATE If exists and is set to 'True', generate function
+emits code inside check_trace_event_get_state check.
========= ====================================================================
@@ -101,22 +103,32 @@ class Wrapper:
def __init__(self, backends, format):
self._backends = [backend.replace("-", "_") for backend in backends]
self._format = format.replace("-", "_")
+ self.check_trace_event_get_state = False
for backend in self._backends:
assert exists(backend)
assert tracetool.format.exists(self._format)
+ for backend in self.backend_modules():
+ check_trace_event_get_state = getattr(backend, "CHECK_TRACE_EVENT_GET_STATE", False)
+ self.check_trace_event_get_state = self.check_trace_event_get_state or check_trace_event_get_state
- def _run_function(self, name, *args, **kwargs):
+ def backend_modules(self):
for backend in self._backends:
- func = tracetool.try_import("tracetool.backend." + backend,
- name % self._format, None)[1]
- if func is not None:
- func(*args, **kwargs)
+ module = tracetool.try_import("tracetool.backend." + backend)[1]
+ if module is not None:
+ yield module
+
+ def _run_function(self, name, *args, check_trace_event_get_state=None, **kwargs):
+ for backend in self.backend_modules():
+ func = getattr(backend,name%self._format,None)
+ if func is not None and (check_trace_event_get_state is None or
+ check_trace_event_get_state == getattr(backend, 'CHECK_TRACE_EVENT_GET_STATE', False)):
+ func(*args, **kwargs)
def generate_begin(self, events, group):
self._run_function("generate_%s_begin", events, group)
- def generate(self, event, group):
- self._run_function("generate_%s", event, group)
+ def generate(self, event, group, check_trace_event_get_state=None):
+ self._run_function("generate_%s", event, group, check_trace_event_get_state=check_trace_event_get_state)
def generate_backend_dstate(self, event, group):
self._run_function("generate_%s_backend_dstate", event, group)
diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py
index ea126b07ea..0ceb49eef5 100644
--- a/scripts/tracetool/format/h.py
+++ b/scripts/tracetool/format/h.py
@@ -59,33 +59,25 @@ def generate(events, backend, group):
out(' false)')
- # tracer without checks
out('',
'static inline void %(api)s(%(args)s)',
'{',
- api=e.api(e.QEMU_TRACE_NOCHECK),
+ api=e.api(),
args=e.args)
if "disable" not in e.properties:
- backend.generate(e, group)
-
+ backend.generate(e, group, check_trace_event_get_state=False)
+
+ if backend.check_trace_event_get_state:
+ if "disable" not in e.properties:
+ event_id = 'TRACE_' + e.name.upper()
+ cond = "trace_event_get_state(%s)" % event_id
+ out(' if (%(cond)s) {',
+ cond=cond)
+ backend.generate(e, group, check_trace_event_get_state=True)
+ out(' }')
out('}')
- cond = "true"
-
- out('',
- 'static inline void %(api)s(%(args)s)',
- '{',
- ' if (%(cond)s) {',
- ' %(api_nocheck)s(%(names)s);',
- ' }',
- '}',
- api=e.api(),
- api_nocheck=e.api(e.QEMU_TRACE_NOCHECK),
- args=e.args,
- names=", ".join(e.args.names()),
- cond=cond)
-
backend.generate_end(events, group)
out('#endif /* TRACE_%s_GENERATED_TRACERS_H */' % group.upper())
--
2.34.1On Wed, Aug 06, 2025 at 03:05:38PM +0000, Tanish Desai wrote:
> New attributed added in backends
> CHECK_TRACE_EVENT_GET_STATE which when
> present and is True wraps the code generated
> by generate function in check_trace_event_get_state
> check else it is outside the conditional block.
>
> Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
> ---
> scripts/tracetool/__init__.py | 1 -
> scripts/tracetool/backend/__init__.py | 26 ++++++++++++++++-------
> scripts/tracetool/format/h.py | 30 ++++++++++-----------------
> 3 files changed, 30 insertions(+), 27 deletions(-)
>
> diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
> index 2ae2e562d6..d0a02c45d7 100644
> --- a/scripts/tracetool/__init__.py
> +++ b/scripts/tracetool/__init__.py
> @@ -332,7 +332,6 @@ def formats(self):
> return self._FMT.findall(self.fmt)
>
> QEMU_TRACE = "trace_%(name)s"
> - QEMU_TRACE_NOCHECK = "_nocheck__" + QEMU_TRACE
> QEMU_TRACE_TCG = QEMU_TRACE + "_tcg"
> QEMU_DSTATE = "_TRACE_%(NAME)s_DSTATE"
> QEMU_BACKEND_DSTATE = "TRACE_%(NAME)s_BACKEND_DSTATE"
> diff --git a/scripts/tracetool/backend/__init__.py b/scripts/tracetool/backend/__init__.py
> index 7bfcc86cc5..4194719e52 100644
> --- a/scripts/tracetool/backend/__init__.py
> +++ b/scripts/tracetool/backend/__init__.py
> @@ -23,6 +23,8 @@
> Attribute Description
> ========= ====================================================================
> PUBLIC If exists and is set to 'True', the backend is considered "public".
> +CHECK_TRACE_EVENT_GET_STATE If exists and is set to 'True', generate function
> +emits code inside check_trace_event_get_state check.
> ========= ====================================================================
The '=' table formatting is broken. Further down in the file there is an
example of a wider first column and how the second column cells wrap:
=============================== ==============================================
Function Description
=============================== ==============================================
generate_<format>_begin(events) Generate backend- and format-specific file
header contents.
Please follow that style for consistency. You could also check reST
markup syntax alternatives like an unordered list if tables are too
clumsy.
>
>
> @@ -101,22 +103,32 @@ class Wrapper:
> def __init__(self, backends, format):
> self._backends = [backend.replace("-", "_") for backend in backends]
> self._format = format.replace("-", "_")
> + self.check_trace_event_get_state = False
> for backend in self._backends:
> assert exists(backend)
> assert tracetool.format.exists(self._format)
> + for backend in self.backend_modules():
> + check_trace_event_get_state = getattr(backend, "CHECK_TRACE_EVENT_GET_STATE", False)
> + self.check_trace_event_get_state = self.check_trace_event_get_state or check_trace_event_get_state
>
> - def _run_function(self, name, *args, **kwargs):
> + def backend_modules(self):
> for backend in self._backends:
> - func = tracetool.try_import("tracetool.backend." + backend,
> - name % self._format, None)[1]
> - if func is not None:
> - func(*args, **kwargs)
> + module = tracetool.try_import("tracetool.backend." + backend)[1]
> + if module is not None:
> + yield module
> +
> + def _run_function(self, name, *args, check_trace_event_get_state=None, **kwargs):
> + for backend in self.backend_modules():
> + func = getattr(backend,name%self._format,None)
Spaces are missing on this line: getattr(backend, name % self._format, None)
> + if func is not None and (check_trace_event_get_state is None or
> + check_trace_event_get_state == getattr(backend, 'CHECK_TRACE_EVENT_GET_STATE', False)):
> + func(*args, **kwargs)
>
> def generate_begin(self, events, group):
> self._run_function("generate_%s_begin", events, group)
>
> - def generate(self, event, group):
> - self._run_function("generate_%s", event, group)
> + def generate(self, event, group, check_trace_event_get_state=None):
> + self._run_function("generate_%s", event, group, check_trace_event_get_state=check_trace_event_get_state)
>
> def generate_backend_dstate(self, event, group):
> self._run_function("generate_%s_backend_dstate", event, group)
> diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py
> index ea126b07ea..0ceb49eef5 100644
> --- a/scripts/tracetool/format/h.py
> +++ b/scripts/tracetool/format/h.py
> @@ -59,33 +59,25 @@ def generate(events, backend, group):
>
> out(' false)')
>
> - # tracer without checks
> out('',
> 'static inline void %(api)s(%(args)s)',
> '{',
> - api=e.api(e.QEMU_TRACE_NOCHECK),
> + api=e.api(),
> args=e.args)
>
> if "disable" not in e.properties:
> - backend.generate(e, group)
> -
> + backend.generate(e, group, check_trace_event_get_state=False)
> +
> + if backend.check_trace_event_get_state:
This line can be indented...
> + if "disable" not in e.properties:
...and this duplicate "disable" check can be dropped.
> + event_id = 'TRACE_' + e.name.upper()
> + cond = "trace_event_get_state(%s)" % event_id
> + out(' if (%(cond)s) {',
> + cond=cond)
> + backend.generate(e, group, check_trace_event_get_state=True)
> + out(' }')
> out('}')
>
> - cond = "true"
> -
> - out('',
> - 'static inline void %(api)s(%(args)s)',
> - '{',
> - ' if (%(cond)s) {',
> - ' %(api_nocheck)s(%(names)s);',
> - ' }',
> - '}',
> - api=e.api(),
> - api_nocheck=e.api(e.QEMU_TRACE_NOCHECK),
> - args=e.args,
> - names=", ".join(e.args.names()),
> - cond=cond)
> -
> backend.generate_end(events, group)
>
> out('#endif /* TRACE_%s_GENERATED_TRACERS_H */' % group.upper())
> --
> 2.34.1
>
On Wed, Aug 06, 2025 at 03:05:38PM +0000, Tanish Desai wrote:
> New attributed added in backends
> CHECK_TRACE_EVENT_GET_STATE which when
> present and is True wraps the code generated
> by generate function in check_trace_event_get_state
> check else it is outside the conditional block.
>
> Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
> ---
> scripts/tracetool/__init__.py | 1 -
> scripts/tracetool/backend/__init__.py | 26 ++++++++++++++++-------
> scripts/tracetool/format/h.py | 30 ++++++++++-----------------
> 3 files changed, 30 insertions(+), 27 deletions(-)
>
> diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
> index 2ae2e562d6..d0a02c45d7 100644
> --- a/scripts/tracetool/__init__.py
> +++ b/scripts/tracetool/__init__.py
> @@ -332,7 +332,6 @@ def formats(self):
> return self._FMT.findall(self.fmt)
>
> QEMU_TRACE = "trace_%(name)s"
> - QEMU_TRACE_NOCHECK = "_nocheck__" + QEMU_TRACE
This is just removing obsolete code
> QEMU_TRACE_TCG = QEMU_TRACE + "_tcg"
> QEMU_DSTATE = "_TRACE_%(NAME)s_DSTATE"
> QEMU_BACKEND_DSTATE = "TRACE_%(NAME)s_BACKEND_DSTATE"
> diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py
> index ea126b07ea..0ceb49eef5 100644
> --- a/scripts/tracetool/format/h.py
> +++ b/scripts/tracetool/format/h.py
> @@ -59,33 +59,25 @@ def generate(events, backend, group):
>
> out(' false)')
>
> - # tracer without checks
> out('',
> 'static inline void %(api)s(%(args)s)',
> '{',
> - api=e.api(e.QEMU_TRACE_NOCHECK),
> + api=e.api(),
> args=e.args)
This bit is removing more obsolete code
>
> if "disable" not in e.properties:
> - backend.generate(e, group)
> -
> + backend.generate(e, group, check_trace_event_get_state=False)
> +
> + if backend.check_trace_event_get_state:
> + if "disable" not in e.properties:
> + event_id = 'TRACE_' + e.name.upper()
> + cond = "trace_event_get_state(%s)" % event_id
> + out(' if (%(cond)s) {',
> + cond=cond)
> + backend.generate(e, group, check_trace_event_get_state=True)
> + out(' }')
> out('}')
This is the actual new functionality
>
> - cond = "true"
> -
> - out('',
> - 'static inline void %(api)s(%(args)s)',
> - '{',
> - ' if (%(cond)s) {',
> - ' %(api_nocheck)s(%(names)s);',
> - ' }',
> - '}',
> - api=e.api(),
> - api_nocheck=e.api(e.QEMU_TRACE_NOCHECK),
> - args=e.args,
> - names=", ".join(e.args.names()),
> - cond=cond)
> -
This is further obsolete code.
It is best to have new functionality added in a separate commit
from the removal of obsolete code.
I've co-incidentally got removal of this obsolete code in the
tracing test suite series I posted, so one will need to be
rebased on top of the other, depending on what order Stefan
wants to take the patches.
> backend.generate_end(events, group)
>
> out('#endif /* TRACE_%s_GENERATED_TRACERS_H */' % group.upper())
> --
> 2.34.1
>
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 8/6/25 21:13, Daniel P. Berrangé wrote: > It is best to have new functionality added in a separate commit > from the removal of obsolete code. My mistake - when Tanish and I "un-rebased" these patches from on top of yours, we placed the removal of the "nocheck" functions in patch 1 instead of patch 2 where it made more sense. > I've co-incidentally got removal of this obsolete code in the > tracing test suite series I posted, so one will need to be > rebased on top of the other, depending on what order Stefan > wants to take the patches. If the main blocker (the /dev/stdout issue) is fixed quickly, your patches should go in first because they aid in reviewing this one. Tanish has started school again, so he'll probably concentrate on finishing what he has of the Rust work and I'll resubmit his patches later. Paolo
On Thu, Aug 07, 2025 at 12:35:39PM +0200, Paolo Bonzini wrote: > On 8/6/25 21:13, Daniel P. Berrangé wrote: > > It is best to have new functionality added in a separate commit > > from the removal of obsolete code. > > My mistake - when Tanish and I "un-rebased" these patches from on top of > yours, we placed the removal of the "nocheck" functions in patch 1 instead > of patch 2 where it made more sense. > > > I've co-incidentally got removal of this obsolete code in the > > tracing test suite series I posted, so one will need to be > > rebased on top of the other, depending on what order Stefan > > wants to take the patches. > > If the main blocker (the /dev/stdout issue) is fixed quickly, your patches > should go in first because they aid in reviewing this one. I posted a new version that drops the stdout patch https://lists.nongnu.org/archive/html/qemu-devel/2025-08/msg02716.html With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
© 2016 - 2025 Red Hat, Inc.