Re: [RFC PATCH 0/5] Deprecate/rename singlestep command line option

[Markus Armbruster <armbru@redhat.com>]

Peter Maydell <peter.maydell@linaro.org> writes:

> The command line option '-singlestep' and its HMP equivalent
> the 'singlestep' command are very confusingly named, because
> they have nothing to do with single-stepping the guest (either
> via the gdb stub or by emulation of guest CPU architectural
> debug facilities). What they actually do is put TCG into a
> mode where it puts only one guest instruction into each
> translation block. This is useful for some circumstances
> such as when you want the -d debug logging to be easier to
> interpret, or if you have a finicky guest binary that wants
> to see interrupts delivered at something other than the end
> of a basic block.
>
> The confusing name is made worse by the fact that our
> documentation for these is so minimal as to be useless
> for telling users what they really do.
>
> This series:
>  * renames the 'singlestep' global variable to 'one_insn_per_tb'
>  * Adds new '-one-insn-per-tb' command line options and a
>    HMP 'one-insn-per-tb' command
>  * Documents the '-singlestep' options and 'singlestep'
>    HMP command as deprecated synonyms for the new ones
>
> It does not do anything about the other place where we surface
> 'singlestep', which is in the QMP StatusInfo object returned by the
> 'query-status' command.  This is incorrectly documented as "true if
> VCPUs are in single-step mode" and "singlestep is enabled through
> the GDB stub", because what it's actually returning is the
> one-insn-per-tb state.
>
> Things I didn't bother with because this is only an RFC but
> will do if it turns into a non-RFC patchset:
>  * test the bsd-user changes :-)
>  * add text to deprecated.rst
>
> So, questions:
>
> (1) is this worth bothering with at all? We could just
>     name our global variable etc better, and document what
>     -singlestep actually does, and not bother with the new
>     names for the options/commands.

The feature is kind of esoteric.  Rather weak excuse for not fixing bad
UI, in my opinion.  Weaker still since you already did a good part of
the actual work.

> (2) if we do do it, do we retain the old name indefinitely,
>     or actively put it on the deprecate-and-drop list?

By "the old name", you mean the CLI option name, right?

I'd prefer deprecate and drop.

> (3) what should we do about the HMP StatusInfo object?
>     I'm not sure how we handle compatibility for HMP.

Uh, you mean *QMP*, don't you?

As you wrote above, StatusInfo is returned by query-status, which is a
stable interface.  Changes to members therefore require the usual
deprecation grace period.  We'd add a new member with a sane name, and
deprecate the old one.

The matching HMP command is "info status".  It shows member @singlestep
as " (single step mode)".  Changing that is fine; HMP is not a stable
interface.