Re: [PATCH 5/5] qemu-options: Remove the deprecated -singlestep option

[Thomas Huth <thuth@redhat.com>]

On 16/01/2024 10.46, Philippe Mathieu-Daudé wrote:
> On 16/1/24 07:27, Markus Armbruster wrote:
>> Daniel P. Berrangé <berrange@redhat.com> writes:
>>
>>> On Mon, Jan 15, 2024 at 05:39:19PM +0000, Peter Maydell wrote:
>>>> On Mon, 15 Jan 2024 at 13:54, Thomas Huth <thuth@redhat.com> wrote:
>>>>>
>>>>> On 12/01/2024 16.39, Philippe Mathieu-Daudé wrote:
>>>>>> Hi Thomas
>>>>>>
>>>>>> +Laurent & Peter
>>>>>>
>>>>>> On 12/1/24 11:00, Thomas Huth wrote:
>>>>>>> It's been marked as deprecated since QEMU 8.1, so it should be fine
>>>>>>> to remove this now.
>>>>>>>
>>>>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>>
>>>>>> StatusInfo::singlestep was deprecated at the same time,
>>>>>> can we remove it?
>>>>>>
>>>>>> IOW could we complete your patch with this?
>>>>
>>>>>> diff --git a/qapi/run-state.json b/qapi/run-state.json
>>>>>> index ca05502e0a..08bc99cb85 100644
>>>>>> --- a/qapi/run-state.json
>>>>>> +++ b/qapi/run-state.json
>>>>>> @@ -106,25 +106,15 @@
>>>>>>   #
>>>>>>   # @running: true if all VCPUs are runnable, false if not runnable
>>>>>>   #
>>>>>> -# @singlestep: true if using TCG with one guest instruction per
>>>>>> -#     translation block
>>>>>> -#
>>>>>>   # @status: the virtual machine @RunState
>>>>>>   #
>>>>>>   # Features:
>>>>>>   #
>>>>>> -# @deprecated: Member 'singlestep' is deprecated (with no
>>>>>> -#     replacement).
>>>>>> -#
>>>>>>   # Since: 0.14
>>>>>>   #
>>>>>> -# Notes: @singlestep is enabled on the command line with '-accel
>>>>>> -#     tcg,one-insn-per-tb=on', or with the HMP 'one-insn-per-tb'
>>>>>> -#     command.
>>>>>>   ##
>>>>>>   { 'struct': 'StatusInfo',
>>>>>>     'data': {'running': 'bool',
>>>>>> -           'singlestep': { 'type': 'bool', 'features': [ 'deprecated' 
>>>>>> ]},
>>>>>>              'status': 'RunState'} }
>>>>>
>>>>> Uh, oh, that's a bigger change already ... can we safely remove the field
>>>>> here without upsetting 3rd party apps that rely on this interface?
>>>>
>>>> That was the whole point of marking it 'deprecated' in the JSON,
>>>> I thought? We don't think anybody's using it, we've given fair
>>>> warning, isn't the next step "remove it"? Markus, you're the
>>>> expert on QAPI deprecations...
>>>
>>> Yes, it is fine to delete it without thinking further about possible usage,
>>> unless someone steps forward quickly with new information that wasn't known
>>> when the deprecation was added....
>>
>> Concur.
> 
> Thanks all for the feedback.
> 
> Thomas, are you OK to post a v2 with the changes I suggested
> or do you want me to do it?

Since your changes were bigger than mine, I think it's just fair if you take 
credit for the patch. So yes, please go ahead and assemble it as a v2! Thanks!

  Thomas