Re: [PATCH v5 05/10] qapi: make the vcpu parameters deprecated for 8.1

[Alex Bennée]

Markus Armbruster <armbru@redhat.com> writes:

> Alex Bennée <alex.bennee@linaro.org> writes:
>
>> I don't think I can remove the parameters directly but certainly mark
>> them as deprecated.
>>
>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Message-Id: <20230523125000.3674739-6-alex.bennee@linaro.org>
>>
>> ---
>> v5
>>   - reword match description
>>   - fix reference to return for set operation
>> ---
>>  docs/about/deprecated.rst |  9 +++++++++
>>  qapi/trace.json           | 40 +++++++++++++++++----------------------
>>  2 files changed, 26 insertions(+), 23 deletions(-)
>>
>> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
>> index e934e0a13a..e44cde057f 100644
>> --- a/docs/about/deprecated.rst
>> +++ b/docs/about/deprecated.rst
>> @@ -254,6 +254,15 @@ it. Since all recent x86 hardware from the past >10 
>> years is capable of the
>>  QEMU API (QAPI) events
>>  ----------------------
>
> Not this patch's fault: the headline should be "QEMU Machine Protocol
> (QMP) events".  The section should directly follow section "QEMU Machine
> Protocol (QMP) commands".
>
> I'd go one step farther, and fuse the two sections under the heading
> "QEMU Machine Protocol (QMP)".
>
>>  
>> +``vcpu`` trace events (since 8.1)
>> +'''''''''''''''''''''''''''''''''
>> +
>> +The ability to instrument QEMU helper functions with vcpu aware trace
>
> Should this be "vCPU-aware"?
>
>> +points was removed in 7.0. However the QAPI still exposed the vcpu
>
> s/the QAPI/QMP/
>
>> +parameter. This argument has now been deprecated and the remaining
>> +used trace points converted to plain trace points selected just by
>
> "remaining trace points that used it"?
>
>> +name.
>> +
>>  ``MEM_UNPLUG_ERROR`` (since 6.2)
>>  ''''''''''''''''''''''''''''''''''''''''''''''''''''''''
>>  
>> diff --git a/qapi/trace.json b/qapi/trace.json
>> index 6bf0af0946..e561f3d3da 100644
>> --- a/qapi/trace.json
>> +++ b/qapi/trace.json
>> @@ -37,13 +37,14 @@
>>  #
>>  # @vcpu: Whether this is a per-vCPU event (since 2.7).
>>  #
>> -# An event is per-vCPU if it has the "vcpu" property in the
>> -# "trace-events" files.
>> +# Features:
>> +# @deprecated: Member @vcpu is deprecated, and always false.
>>  #
>>  # Since: 2.2
>>  ##
>>  { 'struct': 'TraceEventInfo',
>> -  'data': {'name': 'str', 'state': 'TraceEventState', 'vcpu': 'bool'} }
>> +  'data': {'name': 'str', 'state': 'TraceEventState',
>> +           'vcpu': { 'type': 'bool', 'features': ['deprecated'] } } }
>>  
>>  ##
>>  # @trace-event-get-state:
>> @@ -52,19 +53,15 @@
>>  #
>>  # @name: Event name pattern (case-sensitive glob).
>>  #
>> -# @vcpu: The vCPU to query (any by default; since 2.7).
>> +# @vcpu: The vCPU to query (since 2.7).
>>  #
>> -# Returns: a list of @TraceEventInfo for the matching events
>> -#
>> -# An event is returned if:
>> +# Features:
>> +# @deprecated: Member @vcpu is deprecated, and always false.
>
> This isn't quite right: parameter @vcpu cannot be false, it's int.
>
> I figure specifying the parameter makes no sense anymore, because if you
> do, the command will return an empty list.  Correct?

Well its not longer checked so I guess "and always ignored" would be
more correct.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro