Re: [PATCH v2 10/10] hmp: Deprecate 'singlestep' member of StatusInfo

[Peter Maydell <peter.maydell@linaro.org>]

On Tue, 4 Apr 2023 at 09:25, Markus Armbruster <armbru@redhat.com> wrote:
>
> In the title: "qmp:"
>
> Peter Maydell <peter.maydell@linaro.org> writes:
> > diff --git a/qapi/run-state.json b/qapi/run-state.json
> > index 9d34afa39e0..1de8c5c55d0 100644
> > --- a/qapi/run-state.json
> > +++ b/qapi/run-state.json
> > @@ -104,16 +104,27 @@
> >  #
> >  # @running: true if all VCPUs are runnable, false if not runnable
> >  #
> > -# @singlestep: true if VCPUs are in single-step mode
> > +# @one-insn-per-tb: true if using TCG with one guest instruction
> > +#                   per translation block
> > +#
> > +# @singlestep: deprecated synonym for @one-insn-per-tb
> >  #
> >  # @status: the virtual machine @RunState
> >  #
> > +# Features:
> > +# @deprecated: Member 'singlestep' is deprecated. Use @one-insn-per-tb instead.
>
> Wrap this line, please.
>
> > +#
> >  # Since: 0.14
> >  #
> > -# Notes: @singlestep is enabled through the GDB stub
> > +# Notes: @one-insn-per-tb is enabled on the command line with
> > +#        '-accel tcg,one-insn-per-tb=on', or with the HMP
> > +#        'one-insn-per-tb' command.
> >  ##
>
> Hmm.  We report it in query-status, which means it's relevant to QMP
> clients.  We provide the command to control it only in HMP, which means
> it's not relevant to QMP clients.
>
> Why is reading it relevant to QMP clients, but not writing?

I suspect that neither is very relevant to QMP clients, but I
thought we had a requirement that HMP interfaces went
via QMP ones ? If not, we could just make the HMP query
interface directly look at the TCG property, the way the
write interface does.

I don't want to add a QMP interface for writing it unless
there's somebody who actually has a use case for it.

> Use cases for reading it via QMP query-status?
>
> Have you considered tacking feature 'unstable' to it?

Nope, because I don't know anything about what that does :-)

> >  { 'struct': 'StatusInfo',
> > -  'data': {'running': 'bool', 'singlestep': 'bool', 'status': 'RunState'} }
> > +  'data': {'running': 'bool',
> > +           'singlestep': { 'type': 'bool', 'features': [ 'deprecated' ]},
> > +           'one-insn-per-tb': 'bool',
> > +           'status': 'RunState'} }
> >
> >  ##
> >  # @query-status:
>
> I see a bunch of query-status results in
> tests/qemu-iotests/{183,234,262,280}.out.  Do they need an update?

"make check" passed, so I guess not, unless those don't run
in "make check"...

Do those .out files need exact matching output, or can they
be written to say "we don't care about what value this field
has or whether it exists" ?

thanks
-- PMM