Re: [PATCH 09/10] qapi: make most CPU commands unconditionally available

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, May 13, 2025 at 02:44:40PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > This removes the TARGET_* conditions from all the CPU commands
> > that are conceptually target independent. Top level stubs are
> > provided to cope with targets which do not currently implement
> > all of the commands.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  qapi/machine-target.json              | 397 --------------------------
> >  qapi/machine.json                     | 363 +++++++++++++++++++++++
> >  stubs/meson.build                     |   2 +
> >  stubs/monitor-cpu-s390x.c             |  23 ++
> >  stubs/monitor-cpu.c                   |  21 ++
> >  target/arm/arm-qmp-cmds.c             |   2 +-
> >  target/i386/cpu-system.c              |   2 +-
> >  target/i386/cpu.c                     |   2 +-
> >  target/loongarch/loongarch-qmp-cmds.c |   2 +-
> >  target/mips/system/mips-qmp-cmds.c    |  12 +-
> >  target/ppc/ppc-qmp-cmds.c             |  12 +-
> >  target/riscv/riscv-qmp-cmds.c         |   2 +-
> >  target/s390x/cpu_models_system.c      |   2 +-
> >  13 files changed, 437 insertions(+), 405 deletions(-)
> >  create mode 100644 stubs/monitor-cpu-s390x.c
> >  create mode 100644 stubs/monitor-cpu.c

snip

> > +##
> > +# @query-cpu-model-comparison:
> > +#
> > +# Compares two CPU models, @modela and @modelb, returning how they
> > +# compare in a specific configuration.  The results indicates how
> > +# both models compare regarding runnability.  This result can be
> > +# used by tooling to make decisions if a certain CPU model will
> > +# run in a certain configuration or if a compatible CPU model has
> > +# to be created by baselining.
> > +#
> > +# Usually, a CPU model is compared against the maximum possible CPU
> > +# model of a certain configuration (e.g. the "host" model for KVM).
> > +# If that CPU model is identical or a subset, it will run in that
> > +# configuration.
> > +#
> > +# The result returned by this command may be affected by:
> > +#
> > +# * QEMU version: CPU models may look different depending on the QEMU
> > +#   version.  (Except for CPU models reported as "static" in
> > +#   query-cpu-definitions.)
> > +# * machine-type: CPU model may look different depending on the
> > +#   machine-type.  (Except for CPU models reported as "static" in
> > +#   query-cpu-definitions.)
> > +# * machine options (including accelerator): in some architectures,
> > +#   CPU models may look different depending on machine and accelerator
> > +#   options.  (Except for CPU models reported as "static" in
> > +#   query-cpu-definitions.)
> > +# * "-cpu" arguments and global properties: arguments to the -cpu
> > +#   option and global properties may affect expansion of CPU models.
> > +#   Using query-cpu-model-expansion while using these is not advised.
> > +#
> > +# Some architectures may not support comparing CPU models.  s390x
> > +# supports comparing CPU models.
> > +#
> > +# @modela: description of the first CPU model to compare, referred to
> > +#     as "model A" in CpuModelCompareResult
> > +#
> > +# @modelb: description of the second CPU model to compare, referred to
> > +#     as "model B" in CpuModelCompareResult
> > +#
> > +# Returns: a CpuModelCompareInfo describing how both CPU models
> > +#     compare
> > +#
> > +# Errors:
> > +#     - if comparing CPU models is not supported by the target
> 
> You add "by the target", and ...
> 
> > +#     - if a model cannot be used
> > +#     - if a model contains an unknown cpu definition name, unknown
> > +#       properties or properties with wrong types.
> 
> delete this note:
> 
>    # .. note:: This command isn't specific to s390x, but is only
>    #    implemented on this architecture currently.
> 
> Lost: command currently works just for s390x targets.  Intentional?

Kind of.

Previously, the whole command would be tagged for TARGET_S390X
so the command was not available at all.

Now its always  available, but may return an error at runtime
for some targets.

Since we're not providing any mechanism for apps to detect
which targets will work and which won't, the implication
is that apps are expected to try it if they need it, and
cope with any error.

From that POV I thought it might be better not to call
out a point-in-time limitation of the impl. I'm not too
fussed either way though, so we could re-instate the
note.

> Same for query-cpu-model-baseline below.

Likewise


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|