[v1,5/7] docs: mark intention to deprecate TCG tracing functionality

Message ID 20210505092259.8202-6-alex.bennee@linaro.org
State New
Headers show
Series
  • Untitled series #124624
Related show

Commit Message

Alex Bennée May 5, 2021, 9:22 a.m.
Currently attempts to add a new TCG trace events results in failures
to build. Previous discussions have suggested maybe it's time to mark
the feature as deprecated and push people towards using plugins.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

Cc: Luis Vilanova <vilanova@imperial.ac.uk>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
---
 docs/devel/tcg-plugins.rst |  2 ++
 docs/devel/tracing.rst     |  7 +++++++
 docs/system/deprecated.rst | 13 +++++++++++++
 3 files changed, 22 insertions(+)

-- 
2.20.1

Comments

Daniel P. Berrangé May 5, 2021, 9:33 a.m. | #1
On Wed, May 05, 2021 at 10:22:57AM +0100, Alex Bennée wrote:
> Currently attempts to add a new TCG trace events results in failures

> to build. Previous discussions have suggested maybe it's time to mark

> the feature as deprecated and push people towards using plugins.

> 

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

> Cc: Luis Vilanova <vilanova@imperial.ac.uk>

> Cc: Stefan Hajnoczi <stefanha@redhat.com>

> ---

>  docs/devel/tcg-plugins.rst |  2 ++

>  docs/devel/tracing.rst     |  7 +++++++

>  docs/system/deprecated.rst | 13 +++++++++++++

>  3 files changed, 22 insertions(+)

> 

> diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst

> index 18c6581d85..edf04e3091 100644

> --- a/docs/devel/tcg-plugins.rst

> +++ b/docs/devel/tcg-plugins.rst

> @@ -3,6 +3,8 @@

>     Copyright (c) 2019, Linaro Limited

>     Written by Emilio Cota and Alex Bennée

>  

> +.. _tcgplugin-ref:

> +

>  ================

>  QEMU TCG Plugins

>  ================

> diff --git a/docs/devel/tracing.rst b/docs/devel/tracing.rst

> index ba83954899..6b0f46cd54 100644

> --- a/docs/devel/tracing.rst

> +++ b/docs/devel/tracing.rst

> @@ -414,6 +414,13 @@ disabled, this check will have no performance impact.

>  "tcg"

>  -----

>  

> +.. warning::

> +   The ability to add new TCG trace points relies on a having a good

> +   understanding of the TCG internals. In the meantime TCG plugins

> +   have been introduced which solve many of the same problems with

> +   more of a focus on analysing guest code. See :ref:`tcgplugin-ref`

> +   for more details.

> +

>  Guest code generated by TCG can be traced by defining an event with the "tcg"

>  event property. Internally, this property generates two events:

>  "<eventname>_trans" to trace the event at translation time, and

> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst

> index 80cae86252..0c9d3c1e1e 100644

> --- a/docs/system/deprecated.rst

> +++ b/docs/system/deprecated.rst

> @@ -312,6 +312,19 @@ The ``I7200`` guest CPU relies on the nanoMIPS ISA, which is deprecated

>  (the ISA has never been upstreamed to a compiler toolchain). Therefore

>  this CPU is also deprecated.

>  

> +TCG introspection features

> +--------------------------

> +

> +TCG trace-events (since 6.1)

> +''''''''''''''''''''''''''''

> +

> +The ability to add new TCG trace points has bit rotted and as the


When you say this "has bit rotted", just how bad is the situation ?

Is the TCG tracing still usable at all, or is is fully broken
already ?


> +feature can be replicated with TCG plugins it will be deprecated. If

> +any user is currently using this feature and needs help with

> +converting to using TCG plugins they should contact the qemu-devel

> +mailing list.

> +


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 :|
Alex Bennée May 5, 2021, 10:35 a.m. | #2
Daniel P. Berrangé <berrange@redhat.com> writes:

> On Wed, May 05, 2021 at 10:22:57AM +0100, Alex Bennée wrote:

>> Currently attempts to add a new TCG trace events results in failures

>> to build. Previous discussions have suggested maybe it's time to mark

>> the feature as deprecated and push people towards using plugins.

>> 

>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

>> Cc: Luis Vilanova <vilanova@imperial.ac.uk>

>> Cc: Stefan Hajnoczi <stefanha@redhat.com>

>> ---

>>  docs/devel/tcg-plugins.rst |  2 ++

>>  docs/devel/tracing.rst     |  7 +++++++

>>  docs/system/deprecated.rst | 13 +++++++++++++

>>  3 files changed, 22 insertions(+)

>> 

>> diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst

>> index 18c6581d85..edf04e3091 100644

>> --- a/docs/devel/tcg-plugins.rst

>> +++ b/docs/devel/tcg-plugins.rst

>> @@ -3,6 +3,8 @@

>>     Copyright (c) 2019, Linaro Limited

>>     Written by Emilio Cota and Alex Bennée

>>  

>> +.. _tcgplugin-ref:

>> +

>>  ================

>>  QEMU TCG Plugins

>>  ================

>> diff --git a/docs/devel/tracing.rst b/docs/devel/tracing.rst

>> index ba83954899..6b0f46cd54 100644

>> --- a/docs/devel/tracing.rst

>> +++ b/docs/devel/tracing.rst

>> @@ -414,6 +414,13 @@ disabled, this check will have no performance impact.

>>  "tcg"

>>  -----

>>  

>> +.. warning::

>> +   The ability to add new TCG trace points relies on a having a good

>> +   understanding of the TCG internals. In the meantime TCG plugins

>> +   have been introduced which solve many of the same problems with

>> +   more of a focus on analysing guest code. See :ref:`tcgplugin-ref`

>> +   for more details.

>> +

>>  Guest code generated by TCG can be traced by defining an event with the "tcg"

>>  event property. Internally, this property generates two events:

>>  "<eventname>_trans" to trace the event at translation time, and

>> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst

>> index 80cae86252..0c9d3c1e1e 100644

>> --- a/docs/system/deprecated.rst

>> +++ b/docs/system/deprecated.rst

>> @@ -312,6 +312,19 @@ The ``I7200`` guest CPU relies on the nanoMIPS ISA, which is deprecated

>>  (the ISA has never been upstreamed to a compiler toolchain). Therefore

>>  this CPU is also deprecated.

>>  

>> +TCG introspection features

>> +--------------------------

>> +

>> +TCG trace-events (since 6.1)

>> +''''''''''''''''''''''''''''

>> +

>> +The ability to add new TCG trace points has bit rotted and as the

>

> When you say this "has bit rotted", just how bad is the situation ?

>

> Is the TCG tracing still usable at all, or is is fully broken

> already ?


Well patches 6/7 got it working for generic TCG things. I haven't been
able to get the architecture one working but I suspect that is some sort
of interaction between the per-arch trace header generation that I
haven't quite figured out yet.

It's currently broken without the included patches because it's not
really being exercised by anything.

>> +feature can be replicated with TCG plugins it will be deprecated. If

>> +any user is currently using this feature and needs help with

>> +converting to using TCG plugins they should contact the qemu-devel

>> +mailing list.

>> +

>

> Regards,

> Daniel



-- 
Alex Bennée
Alex Bennée May 5, 2021, 10:41 a.m. | #3
Alex Bennée <alex.bennee@linaro.org> writes:

> Daniel P. Berrangé <berrange@redhat.com> writes:

>

>> On Wed, May 05, 2021 at 10:22:57AM +0100, Alex Bennée wrote:

<snip>
>>> +TCG introspection features

>>> +--------------------------

>>> +

>>> +TCG trace-events (since 6.1)

>>> +''''''''''''''''''''''''''''

>>> +

>>> +The ability to add new TCG trace points has bit rotted and as the

>>

>> When you say this "has bit rotted", just how bad is the situation ?

>>

>> Is the TCG tracing still usable at all, or is is fully broken

>> already ?

>

> Well patches 6/7 got it working for generic TCG things. I haven't been

> able to get the architecture one working but I suspect that is some sort

> of interaction between the per-arch trace header generation that I

> haven't quite figured out yet.


Ahh it's since 7609ffb919 (trace: fix tcg tracing build breakage) which
limited tcg/vcpu events to the root trace-events file.

-- 
Alex Bennée
Daniel P. Berrangé May 5, 2021, 10:52 a.m. | #4
On Wed, May 05, 2021 at 11:41:46AM +0100, Alex Bennée wrote:
> 

> Alex Bennée <alex.bennee@linaro.org> writes:

> 

> > Daniel P. Berrangé <berrange@redhat.com> writes:

> >

> >> On Wed, May 05, 2021 at 10:22:57AM +0100, Alex Bennée wrote:

> <snip>

> >>> +TCG introspection features

> >>> +--------------------------

> >>> +

> >>> +TCG trace-events (since 6.1)

> >>> +''''''''''''''''''''''''''''

> >>> +

> >>> +The ability to add new TCG trace points has bit rotted and as the

> >>

> >> When you say this "has bit rotted", just how bad is the situation ?

> >>

> >> Is the TCG tracing still usable at all, or is is fully broken

> >> already ?

> >

> > Well patches 6/7 got it working for generic TCG things. I haven't been

> > able to get the architecture one working but I suspect that is some sort

> > of interaction between the per-arch trace header generation that I

> > haven't quite figured out yet.

> 

> Ahh it's since 7609ffb919 (trace: fix tcg tracing build breakage) which

> limited tcg/vcpu events to the root trace-events file.


That commit is from release 2.10.0.

The other commit mentioned in patch 6 (73ff061032) is from 2.12.0.

So no one has been able to use this feature for 3+ years already.

Is it actually worth fixing and then deprecating for 2 releases before
deleting, as opposed to just deleting the broken code today on basis
that it can't have any current users ?

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 :|
Alex Bennée May 17, 2021, 10:47 a.m. | #5
Daniel P. Berrangé <berrange@redhat.com> writes:

> On Wed, May 05, 2021 at 11:41:46AM +0100, Alex Bennée wrote:

>> 

>> Alex Bennée <alex.bennee@linaro.org> writes:

>> 

>> > Daniel P. Berrangé <berrange@redhat.com> writes:

>> >

>> >> On Wed, May 05, 2021 at 10:22:57AM +0100, Alex Bennée wrote:

>> <snip>

>> >>> +TCG introspection features

>> >>> +--------------------------

>> >>> +

>> >>> +TCG trace-events (since 6.1)

>> >>> +''''''''''''''''''''''''''''

>> >>> +

>> >>> +The ability to add new TCG trace points has bit rotted and as the

>> >>

>> >> When you say this "has bit rotted", just how bad is the situation ?

>> >>

>> >> Is the TCG tracing still usable at all, or is is fully broken

>> >> already ?

>> >

>> > Well patches 6/7 got it working for generic TCG things. I haven't been

>> > able to get the architecture one working but I suspect that is some sort

>> > of interaction between the per-arch trace header generation that I

>> > haven't quite figured out yet.

>> 

>> Ahh it's since 7609ffb919 (trace: fix tcg tracing build breakage) which

>> limited tcg/vcpu events to the root trace-events file.

>

> That commit is from release 2.10.0.

>

> The other commit mentioned in patch 6 (73ff061032) is from 2.12.0.

>

> So no one has been able to use this feature for 3+ years already.

>

> Is it actually worth fixing and then deprecating for 2 releases before

> deleting, as opposed to just deleting the broken code today on basis

> that it can't have any current users ?


Well I can get it up and running with the aforementioned patches and it
seems reasonable to give some notice. I'm happy to defer to Stefan here
though as it's his sub-system.

>

> Regards,

> Daniel



-- 
Alex Bennée
Stefan Hajnoczi May 17, 2021, 1:44 p.m. | #6
On Mon, May 17, 2021 at 11:47:11AM +0100, Alex Bennée wrote:
> 

> Daniel P. Berrangé <berrange@redhat.com> writes:

> 

> > On Wed, May 05, 2021 at 11:41:46AM +0100, Alex Bennée wrote:

> >> 

> >> Alex Bennée <alex.bennee@linaro.org> writes:

> >> 

> >> > Daniel P. Berrangé <berrange@redhat.com> writes:

> >> >

> >> >> On Wed, May 05, 2021 at 10:22:57AM +0100, Alex Bennée wrote:

> >> <snip>

> >> >>> +TCG introspection features

> >> >>> +--------------------------

> >> >>> +

> >> >>> +TCG trace-events (since 6.1)

> >> >>> +''''''''''''''''''''''''''''

> >> >>> +

> >> >>> +The ability to add new TCG trace points has bit rotted and as the

> >> >>

> >> >> When you say this "has bit rotted", just how bad is the situation ?

> >> >>

> >> >> Is the TCG tracing still usable at all, or is is fully broken

> >> >> already ?

> >> >

> >> > Well patches 6/7 got it working for generic TCG things. I haven't been

> >> > able to get the architecture one working but I suspect that is some sort

> >> > of interaction between the per-arch trace header generation that I

> >> > haven't quite figured out yet.

> >> 

> >> Ahh it's since 7609ffb919 (trace: fix tcg tracing build breakage) which

> >> limited tcg/vcpu events to the root trace-events file.

> >

> > That commit is from release 2.10.0.

> >

> > The other commit mentioned in patch 6 (73ff061032) is from 2.12.0.

> >

> > So no one has been able to use this feature for 3+ years already.

> >

> > Is it actually worth fixing and then deprecating for 2 releases before

> > deleting, as opposed to just deleting the broken code today on basis

> > that it can't have any current users ?

> 

> Well I can get it up and running with the aforementioned patches and it

> seems reasonable to give some notice. I'm happy to defer to Stefan here

> though as it's his sub-system.


Lluís Vilanova was the author and probably main user. He mentioned he's
been away from QEMU for a while.

If you want to drop the feature, I think that's fine since it has
already been broken for over 3 years. If someone wants it back then it
can be added via TCG plugins in the future.

Stefan

Patch

diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
index 18c6581d85..edf04e3091 100644
--- a/docs/devel/tcg-plugins.rst
+++ b/docs/devel/tcg-plugins.rst
@@ -3,6 +3,8 @@ 
    Copyright (c) 2019, Linaro Limited
    Written by Emilio Cota and Alex Bennée
 
+.. _tcgplugin-ref:
+
 ================
 QEMU TCG Plugins
 ================
diff --git a/docs/devel/tracing.rst b/docs/devel/tracing.rst
index ba83954899..6b0f46cd54 100644
--- a/docs/devel/tracing.rst
+++ b/docs/devel/tracing.rst
@@ -414,6 +414,13 @@  disabled, this check will have no performance impact.
 "tcg"
 -----
 
+.. warning::
+   The ability to add new TCG trace points relies on a having a good
+   understanding of the TCG internals. In the meantime TCG plugins
+   have been introduced which solve many of the same problems with
+   more of a focus on analysing guest code. See :ref:`tcgplugin-ref`
+   for more details.
+
 Guest code generated by TCG can be traced by defining an event with the "tcg"
 event property. Internally, this property generates two events:
 "<eventname>_trans" to trace the event at translation time, and
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 80cae86252..0c9d3c1e1e 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -312,6 +312,19 @@  The ``I7200`` guest CPU relies on the nanoMIPS ISA, which is deprecated
 (the ISA has never been upstreamed to a compiler toolchain). Therefore
 this CPU is also deprecated.
 
+TCG introspection features
+--------------------------
+
+TCG trace-events (since 6.1)
+''''''''''''''''''''''''''''
+
+The ability to add new TCG trace points has bit rotted and as the
+feature can be replicated with TCG plugins it will be deprecated. If
+any user is currently using this feature and needs help with
+converting to using TCG plugins they should contact the qemu-devel
+mailing list.
+
+
 Related binaries
 ----------------