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

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 5/21/25 10:12 PM, Markus Armbruster wrote:
> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
> 
>> On 5/18/25 11:29 PM, Markus Armbruster wrote:
>>> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
>>>
>>>> From: Daniel P. Berrangé <berrange@redhat.com>
>>>>
>>>> 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.
>>>>
>>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>
>>> [...]
>>>
>>>> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
>>>> deleted file mode 100644
>>>> index e153291a7fc..00000000000
>>>> --- a/qapi/machine-target.json
>>>> +++ /dev/null
>>>
>>> [...]
>>>
>>>> -##
>>>> -# @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
>>>> -#     - if a model cannot be used
>>>> -#     - if a model contains an unknown cpu definition name, unknown
>>>> -#       properties or properties with wrong types.
>>>> -#
>>>> -# .. note:: This command isn't specific to s390x, but is only
>>>> -#    implemented on this architecture currently.
>>>
>>> We lose this note.  Intentional?
>>>
>>> Perhaps because we also have the "Some architectures ..." paragraph
>>> above?
>>>
>>>> -#
>>>> -# Since: 2.8
>>>> -##
>>>> -{ 'command': 'query-cpu-model-comparison',
>>>> -  'data': { 'modela': 'CpuModelInfo', 'modelb': 'CpuModelInfo' },
>>>> -  'returns': 'CpuModelCompareInfo',
>>>> -  'if': 'TARGET_S390X' }
>>>> -
>>>> -##
>>>> -# @query-cpu-model-baseline:
>>>> -#
>>>> -# Baseline two CPU models, @modela and @modelb, creating a compatible
>>>> -# third model.  The created model will always be a static,
>>>> -# migration-safe CPU model (see "static" CPU model expansion for
>>>> -# details).
>>>> -#
>>>> -# This interface can be used by tooling to create a compatible CPU
>>>> -# model out two CPU models.  The created CPU model will be identical
>>>> -# to or a subset of both CPU models when comparing them.  Therefore,
>>>> -# the created CPU model is guaranteed to run where the given CPU
>>>> -# models run.
>>>> -#
>>>> -# 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 baselining CPU models.  s390x
>>>> -# supports baselining CPU models.
>>>> -#
>>>> -# @modela: description of the first CPU model to baseline
>>>> -#
>>>> -# @modelb: description of the second CPU model to baseline
>>>> -#
>>>> -# Returns: a CpuModelBaselineInfo describing the baselined CPU model
>>>> -#
>>>> -# Errors:
>>>> -#     - if baselining CPU models is not supported
>>>> -#     - if a model cannot be used
>>>> -#     - if a model contains an unknown cpu definition name, unknown
>>>> -#       properties or properties with wrong types.
>>>> -#
>>>> -# .. note:: This command isn't specific to s390x, but is only
>>>> -#    implemented on this architecture currently.
>>>
>>> We lose this note.  Intentional?
>>>
>>> Perhaps because we also have the "Some architectures ..." paragraph
>>> above?
>>>
>>>> -#
>>>> -# Since: 2.8
>>>> -##
>>>> -{ 'command': 'query-cpu-model-baseline',
>>>> -  'data': { 'modela': 'CpuModelInfo',
>>>> -            'modelb': 'CpuModelInfo' },
>>>> -  'returns': 'CpuModelBaselineInfo',
>>>> -  'if': 'TARGET_S390X' }
>>>
>>> [...]
>>>
>>
>> Yes, those notes don't have any reason to be present anymore, and the
>> new "Some architectures..." paragraph replaces them.
> 
> Okay.  A brief mention in the commit message like "Adjust the doc
> comments accordingly" might be helpful.  Up to you!
> 

I add it on my side.
In case you'd be ready to pull this series, feel free to add it directly.

Thanks,
Pierrick