[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PULL v2 23/25] trace: stop tracking static disabled state



On Fri, Nov 06, 2026 at 12:53:54PM +0100, Markus Armbruster wrote:
> I missed this, because I wasn't cc'ed.
> 
> Stefan Hajnoczi <stefanha@xxxxxxxxxx> writes:
> 
> > From: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> >
> > The TraceEvent struct contains a "bool sstate" field which track whether
> > a trace event was disabled statically. i.e. at build time via the
> > "disable" property annotation in trace-events.
> >
> > The only runtime use of this field is to let "trace-event-get-state"
> > report that an event is "unavailable" instead of reporting an error for
> > an unknown trace event.
> >
> > Trace events are not ABI so their names can change and the event can
> > come and go at any time. Thus there is little utility in reporting the
> > existence of a disabled event that cannot ever be used at runtime.
> >
> > Removing the "bool sstate" field from TraceEvent saves 8 bytes of
> > space in the struct, adding up to approx 64 KB across all current
> > events.
> >
> > Reviewed-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
> > Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> > Message-id: 20261102142844.3464712-24-berrange@xxxxxxxxxx
> > Signed-off-by: Stefan Hajnoczi <stefanha@xxxxxxxxxx>
> 
> [...]
> 
> > diff --git a/qapi/trace.json b/qapi/trace.json
> > index 29c76256055..8533e378977 100644
> > --- a/qapi/trace.json
> > +++ b/qapi/trace.json
> > @@ -16,8 +16,6 @@
> >  # @TraceEventState:
> >  #     State of a tracing event.
> >  #
> > -# @unavailable: The event is statically disabled.
> > -#
> >  # @disabled: The event is dynamically disabled.
> >  #
> >  # @enabled: The event is dynamically enabled.
> > @@ -25,7 +23,7 @@
> >  # Since: 2.2
> >  ##
> >  { 'enum': 'TraceEventState',
> > -  'data': ['unavailable', 'disabled', 'enabled'] }
> > +  'data': ['disabled', 'enabled'] }
> 
> No note in docs/about/removed-features.rst needed?

Yes it is required.


> > @@ -69,6 +67,7 @@
>    ##
>    # @trace-event-set-state:
>    #     Set the dynamic tracing state of events.
>    #
>    # @name: Event name pattern (case-sensitive glob).
>    #
> >  # @enable: Whether to enable tracing.
> >  #
> >  # @ignore-unavailable: Do not match unavailable events with @name.
> > +#      Deprecated: since 11.2
> >  #
> >  # Since: 2.2
> >  #
> 
> Are you trying to deprecate use of argument @ignore-unavailable?

Yes.


> If yes, this isn't how we do that.  Instead:

/face-palm

> 
>    ##
>    # @trace-event-set-state:
>    #     Set the dynamic tracing state of events.
>    #
>    # @name: Event name pattern (case-sensitive glob).
>    #
>    # @enable: Whether to enable tracing.
>    #
>    # @ignore-unavailable: Do not match unavailable events with @name.
>    #
>    # Features:
>    #
>    # @deprecated: Argument @ignore-unavailable is deperecated and does
>    #     nothing.
>    #
>    # Since: 2.2
>    #
>    # .. qmp-example::
>    #
>    #     -> { "execute": "trace-event-set-state",
>    #          "arguments": { "name": "qemu_memalign", "enable": true } }
>    #     <- { "return": {} }
>    ##
>    { 'command': 'trace-event-set-state',
>      'data': {'name': 'str', 'enable': 'bool',
>               '*ignore-unavailable': { 'type': 'bool',
>                                        'features': [ 'deprecated' ] } } }
> 
> with a matching update to docs/about/deprecated.rst.

Yes, will CC you on the fix.

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.