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

[Thomas Huth]

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