Re: [PATCH 0/4] hw/s390x: Alias @dump-skeys -> @dump-s390-skey and deprecate
Hi Daniel, Dave, Markus & Thomas.
On 4/6/24 06:58, Markus Armbruster wrote:
"Dr. David Alan Gilbert" <dave(a)treblig.org> writes:
> * Daniel P. Berrangé (berrange(a)redhat.com) wrote:
>> On Fri, May 31, 2024 at 06:47:45AM +0200, Thomas Huth wrote:
>>> On 30/05/2024 09.45, Philippe Mathieu-Daudé wrote:
>>>> We are trying to unify all qemu-system-FOO to a single binary.
>>>> In order to do that we need to remove QAPI target specific code.
>>>>
>>>> @dump-skeys is only available on qemu-system-s390x. This series
>>>> rename it as @dump-s390-skey, making it available on other
>>>> binaries. We take care of backward compatibility via deprecation.
>>>>
>>>> Philippe Mathieu-Daudé (4):
>>>> hw/s390x: Introduce the @dump-s390-skeys QMP command
>>>> hw/s390x: Introduce the 'dump_s390_skeys' HMP command
>>>> hw/s390x: Deprecate the HMP 'dump_skeys' command
>>>> hw/s390x: Deprecate the QMP @dump-skeys command
>>>
>>> Why do we have to rename the command? Just for the sake of it? I think
>>> renaming HMP commands is maybe ok, but breaking the API in QMP is something
>>> you should consider twice.
I'm looking at how to include this command in the new "single binary".
Markus explained in an earlier series, just expanding this command as
stub to targets that don't implement it is not backward compatible and
breaks QMP introspection. Currently on s390x we get a result, on other
targets the command doesn't exist. If we add a stubs, then other targets
return something (even if it is an empty list), confusing management
interface.
So this approach use to deprecate process to include a new command
which behaves differently on non-s390x targets.
If we don't care for this particular case, better. However I'd still
like to discuss this approach for other target-specific commands.
PRO rename: the command's tie to S390 is them immediately
obvious, which
may be useful when the command becomes available in qemu-systems capable
of running other targets.
CON rename: users need to adapt.
What are the users? Not libvirt, as far as I can tell.
Years ago we said, "all HMP must be based on QMP". Now we realize HMP
became stable because QMP-exposed, although not consumed externally...
Does the concept of "internal QMP commands" makes sense for HMP debug
ones? (Looking at a way to not expose them). We could use the "x-"
prefix to not care about stable / backward compat, but what is the point
of exposing to QMP commands that will never be accessed there?
>> That was going to be my question too. Seems like its possible
to simply
>> stub out the existing command for other targets.
That's going to happen whether we rename the commands or not.
> Are these commands really supposed to be stable, or are they just debug
> commands? If they are debug, then add the x- and don't worry too much.
OK.
docs/devel/qapi-code-gen.rst:
Names beginning with ``x-`` used to signify "experimental". This
convention has been replaced by special feature "unstable".
Feature "unstable" is what makes something unstable, and is what
machines should check.
What I mentioned earlier could be 'Feature "internal" or
"debug"'.
An "x-" prefix may still be useful for humans. Machines
should *not*
key on the prefix. It's unreliable anyway: InputBarrierProperties
member @x-origin is stable despite it's name. Renames to gain or lose
the prefix may or may not be worth the bother.
Could follow the rules and be renamed as "origin-coordinate-x".
Making an existing part of the interface unstable should be treated
similar to deprecating it: we keep it stable for at least the
deprecation period. Not written policy, because we didn't consider such
changes when we documented our deprecation policy in
docs/about/deprecated.rst.
Alternative path to a renamed command (*if* we want that):
1. Make it unstable.
2. But keep it stable for two releases.
3. Rename.
[...]