Re: [PATCH 0/4] hw/s390x: Alias @dump-skeys -> @dump-s390-skey and deprecate

On 09/03/2025 19.55, Pierrick Bouvier wrote: > On 6/3/24 05:18, Daniel P. Berrangé 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. > > > > That was going to be my question too. Seems like its possible to simply > > stub out the existing command for other targets. > > > > The renaming is just window dressing. > > > > Working on single-binary topic means specificities from every qemu > binary/ architecture has to be merged together. Despite appearing has a > bad thing now, it's definitely a step forward for QEMU, and will allow > to enable new usages. > > The hard way is to trigger a deep refactoring, involving lengthy > conversations where compromises have to be found ("let's implement this > for all arch"). The pragmatic way is to eliminate obvious stuff. > > This command is specific to an arch, so renaming is a good and obvious > strategy. For the backward compatible anxious developer, another > strategy would be to simply declare this command if the running target > is s390x. But then, you create a precedent to do something that should > not have existed in the first place. > > +1 for the renaming, and hope that users of this command are able to > change a line in their script to adapt to the new command. Sorry, but no: We've got plenty of other target specific commands... rtc-reset-reinjection , query-sev, query-gic-capabilities, just to name some few. So unless you provide a patch series to rename *all* of them and deprecate the previous names, I don't see the point why changing just one single s390x command is necessary.

Thomas