Re: [Qemu-devel] [PATCH v3 43/49] qapi: make s390 commands depend on TARGET_S390X

[Thomas Huth <thuth@redhat.com>]

On 21.03.2018 12:52, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Acked-by: Cornelia Huck <cohuck@redhat.com>
> ---
>  qapi/misc.json                          | 101 ----------------------
>  qapi/target.json                        | 106 ++++++++++++++++++++++++
>  include/sysemu/arch_init.h              |   7 --
>  hw/s390x/s390-skeys.c                   |   2 +-
>  monitor.c                               |  14 ----
>  qmp.c                                   |  14 ----
>  stubs/arch-query-cpu-model-baseline.c   |  13 ---
>  stubs/arch-query-cpu-model-comparison.c |  13 ---
>  target/s390x/cpu_models.c               |   5 +-
>  stubs/Makefile.objs                     |   2 -
>  10 files changed, 110 insertions(+), 167 deletions(-)
>  delete mode 100644 stubs/arch-query-cpu-model-baseline.c
>  delete mode 100644 stubs/arch-query-cpu-model-comparison.c
> 
> diff --git a/qapi/misc.json b/qapi/misc.json
> index 690eeda41f..1753a81b1e 100644
> --- a/qapi/misc.json
> +++ b/qapi/misc.json
> @@ -1821,27 +1821,6 @@
>  { 'command': 'query-dump-guest-memory-capability',
>    'returns': 'DumpGuestMemoryCapability' }
>  
> -##
> -# @dump-skeys:
> -#
> -# Dump guest's storage keys
> -#
> -# @filename: the path to the file to dump to
> -#
> -# This command is only supported on s390 architecture.
> -#
> -# Since: 2.5
> -#
> -# Example:
> -#
> -# -> { "execute": "dump-skeys",
> -#      "arguments": { "filename": "/tmp/skeys" } }
> -# <- { "return": {} }
> -#
> -##
> -{ 'command': 'dump-skeys',
> -  'data': { 'filename': 'str' } }
> -
>  ##
>  # @object-add:
>  #
> @@ -2208,46 +2187,6 @@
>            }
>  }
>  
> -##
> -# @query-cpu-model-comparison:
> -#
> -# Compares two CPU models, 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.
> -#
> -# Returns: a CpuModelBaselineInfo. Returns an error 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.
> -#
> -# Since: 2.8.0
> -##
> -{ 'command': 'query-cpu-model-comparison',
> -  'data': { 'modela': 'CpuModelInfo', 'modelb': 'CpuModelInfo' },
> -  'returns': 'CpuModelCompareInfo' }
> -
>  ##
>  # @CpuModelBaselineInfo:
>  #
> @@ -2260,46 +2199,6 @@
>  { 'struct': 'CpuModelBaselineInfo',
>    'data': { 'model': 'CpuModelInfo' } }
>  
> -##
> -# @query-cpu-model-baseline:
> -#
> -# Baseline two CPU models, 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.
> -#
> -# Returns: a CpuModelBaselineInfo. Returns an error 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.
> -#
> -# Since: 2.8.0
> -##
> -{ 'command': 'query-cpu-model-baseline',
> -  'data': { 'modela': 'CpuModelInfo',
> -            'modelb': 'CpuModelInfo' },
> -  'returns': 'CpuModelBaselineInfo' }
> -
>  ##
>  # @AddfdInfo:
>  #
> diff --git a/qapi/target.json b/qapi/target.json
> index f277b69a2a..b07a8926d8 100644
> --- a/qapi/target.json
> +++ b/qapi/target.json
> @@ -7,6 +7,8 @@
>  
>  { 'pragma': { 'top-unit': 'target' } }
>  
> +{ 'include': 'misc.json' }
> +
>  ##
>  # @rtc-reset-reinjection:
>  #
> @@ -182,3 +184,107 @@
>  ##
>  { 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
>    'if': 'defined(TARGET_I386)' }
> +
> +##
> +# @dump-skeys:
> +#
> +# Dump guest's storage keys
> +#
> +# @filename: the path to the file to dump to
> +#
> +# This command is only supported on s390 architecture.
> +#
> +# Since: 2.5
> +#
> +# Example:
> +#
> +# -> { "execute": "dump-skeys",
> +#      "arguments": { "filename": "/tmp/skeys" } }
> +# <- { "return": {} }
> +#
> +##
> +{ 'command': 'dump-skeys',
> +  'data': { 'filename': 'str' },
> +  'if': 'defined(TARGET_S390X)' }
> +
> +##
> +# @query-cpu-model-comparison:
> +#
> +# Compares two CPU models, 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.
> +#
> +# Returns: a CpuModelBaselineInfo. Returns an error 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.
> +#
> +# Since: 2.8.0
> +##
> +{ 'command': 'query-cpu-model-comparison',
> +  'data': { 'modela': 'CpuModelInfo', 'modelb': 'CpuModelInfo' },
> +  'returns': 'CpuModelCompareInfo',
> +  'if': 'defined(TARGET_S390X)' }
> +
> +##
> +# @query-cpu-model-baseline:
> +#
> +# Baseline two CPU models, 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.
> +#
> +# Returns: a CpuModelBaselineInfo. Returns an error 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.
> +#
> +# Since: 2.8.0
> +##
> +{ 'command': 'query-cpu-model-baseline',
> +  'data': { 'modela': 'CpuModelInfo',
> +            'modelb': 'CpuModelInfo' },
> +  'returns': 'CpuModelBaselineInfo',
> +  'if': 'defined(TARGET_S390X)' }
> diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
> index 32abdfe6a1..f0ef652b2a 100644
> --- a/include/sysemu/arch_init.h
> +++ b/include/sysemu/arch_init.h
> @@ -36,11 +36,4 @@ CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp);
>  CpuModelExpansionInfo *arch_query_cpu_model_expansion(CpuModelExpansionType type,
>                                                        CpuModelInfo *mode,
>                                                        Error **errp);
> -CpuModelCompareInfo *arch_query_cpu_model_comparison(CpuModelInfo *modela,
> -                                                     CpuModelInfo *modelb,
> -                                                     Error **errp);
> -CpuModelBaselineInfo *arch_query_cpu_model_baseline(CpuModelInfo *modela,
> -                                                    CpuModelInfo *modelb,
> -                                                    Error **errp);
> -
>  #endif
> diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
> index 76241c240e..59d28c2358 100644
> --- a/hw/s390x/s390-skeys.c
> +++ b/hw/s390x/s390-skeys.c
> @@ -13,7 +13,7 @@
>  #include "hw/boards.h"
>  #include "hw/s390x/storage-keys.h"
>  #include "qapi/error.h"
> -#include "qapi/qapi-commands-misc.h"
> +#include "qapi/target-qapi-commands.h"
>  #include "qapi/qmp/qdict.h"
>  #include "qemu/error-report.h"
>  #include "sysemu/kvm.h"
> diff --git a/monitor.c b/monitor.c
> index 4ad9225425..bd9a6950cf 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1157,19 +1157,12 @@ static void qmp_query_qmp_schema(QDict *qdict, QObject **ret_data,
>   */
>  static void qmp_unregister_commands_hack(void)
>  {
> -#ifndef TARGET_S390X
> -    qmp_unregister_command(&qmp_commands, "dump-skeys");
> -#endif
>  #ifndef TARGET_ARM
>      qmp_unregister_command(&qmp_commands, "query-gic-capabilities");
>  #endif
>  #if !defined(TARGET_S390X) && !defined(TARGET_I386)
>      qmp_unregister_command(&qmp_commands, "query-cpu-model-expansion");
>  #endif
> -#if !defined(TARGET_S390X)
> -    qmp_unregister_command(&qmp_commands, "query-cpu-model-baseline");
> -    qmp_unregister_command(&qmp_commands, "query-cpu-model-comparison");
> -#endif
>  #if !defined(TARGET_PPC) && !defined(TARGET_ARM) && !defined(TARGET_I386) \
>      && !defined(TARGET_S390X)
>      qmp_unregister_command(&qmp_commands, "query-cpu-definitions");
> @@ -4703,13 +4696,6 @@ QemuOptsList qemu_mon_opts = {
>      },
>  };
>  
> -#ifndef TARGET_S390X
> -void qmp_dump_skeys(const char *filename, Error **errp)
> -{
> -    error_setg(errp, QERR_FEATURE_DISABLED, "dump-skeys");
> -}
> -#endif
> -
>  #ifndef TARGET_ARM
>  GICCapabilityList *qmp_query_gic_capabilities(Error **errp)
>  {
> diff --git a/qmp.c b/qmp.c
> index d8f80cb04e..14972b78df 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -601,20 +601,6 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
>      return arch_query_cpu_model_expansion(type, model, errp);
>  }
>  
> -CpuModelCompareInfo *qmp_query_cpu_model_comparison(CpuModelInfo *modela,
> -                                                    CpuModelInfo *modelb,
> -                                                    Error **errp)
> -{
> -    return arch_query_cpu_model_comparison(modela, modelb, errp);
> -}
> -
> -CpuModelBaselineInfo *qmp_query_cpu_model_baseline(CpuModelInfo *modela,
> -                                                   CpuModelInfo *modelb,
> -                                                   Error **errp)
> -{
> -    return arch_query_cpu_model_baseline(modela, modelb, errp);
> -}

Not sure, but couldn't these two commands be implemented on other
architectures in the long run, too? So removing them now here seems
somewhat counterproductive?

 Thomas