diff mbox

[v1,25/29] qmp: add QMP interface "query-cpu-model-comparison"

Message ID 1470139155-53900-26-git-send-email-dahi@linux.vnet.ibm.com
State New
Headers show

Commit Message

David Hildenbrand Aug. 2, 2016, 11:59 a.m. UTC
Let's provide a standardized interface to compare two CPU models.

query-cpu-model-compare takes two models and returns what it knows about
their compability under a certain QEMU machine QEMU has been started with.

If modelA is a subset of modelB or if both are identical, modelA will run
in the same configuration as modelB. If modelA is however a superset of
modelB or if both are incompatible, one can try to create a compatible one
by "baselining" both models (follow up patch).

Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
---
 include/sysemu/arch_init.h              |  3 ++
 qapi-schema.json                        | 65 +++++++++++++++++++++++++++++++++
 qmp-commands.hx                         |  6 +++
 qmp.c                                   |  7 ++++
 stubs/Makefile.objs                     |  1 +
 stubs/arch-query-cpu-model-comparison.c | 12 ++++++
 6 files changed, 94 insertions(+)
 create mode 100644 stubs/arch-query-cpu-model-comparison.c

Comments

Eduardo Habkost Aug. 2, 2016, 2:45 p.m. UTC | #1
On Tue, Aug 02, 2016 at 01:59:11PM +0200, David Hildenbrand wrote:
> Let's provide a standardized interface to compare two CPU models.
> 
> query-cpu-model-compare takes two models and returns what it knows about
> their compability under a certain QEMU machine QEMU has been started with.
> 
> If modelA is a subset of modelB or if both are identical, modelA will run
> in the same configuration as modelB. If modelA is however a superset of
> modelB or if both are incompatible, one can try to create a compatible one
> by "baselining" both models (follow up patch).
> 
> Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
> ---
>  include/sysemu/arch_init.h              |  3 ++
>  qapi-schema.json                        | 65 +++++++++++++++++++++++++++++++++
>  qmp-commands.hx                         |  6 +++
>  qmp.c                                   |  7 ++++
>  stubs/Makefile.objs                     |  1 +
>  stubs/arch-query-cpu-model-comparison.c | 12 ++++++
>  6 files changed, 94 insertions(+)
>  create mode 100644 stubs/arch-query-cpu-model-comparison.c
> 
> diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
> index 37b2e86..96d47c0 100644
> --- a/include/sysemu/arch_init.h
> +++ b/include/sysemu/arch_init.h
> @@ -38,5 +38,8 @@ 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);
>  
>  #endif
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 43f7969..1f8f8ec 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3149,6 +3149,71 @@
>              'model': 'CpuModelInfo' },
>    'returns': 'CpuModelExpansionInfo' }
>  
> +##
> +# @CpuModelCompareResult:
> +#
> +# An enumeration of CPU model comparation results.
> +#
> +# @incompatible: both model definition are incompatible
> +#
> +# @identical: model A == model B
> +#
> +# @superset: model A > model B
> +#
> +# @subset: model A < model B

We need to clarify what superset/subset, ">" and "<" really mean.

> +#
> +# Since: 2.8.0
> +##
> +{ 'enum': 'CpuModelCompareResult',
> +  'data': [ 'incompatible', 'identical', 'superset', 'subset' ] }

I assume implementations are free to return "incompatible" if
they still don't have any extra logic to expand CPU models and
check for supersets/subsets. If that's the case, see my
suggestion below for a generic comparison function.

> +
> +##
> +# @CpuModelCompareInfo
> +#
> +# The result of a CPU model comparison.
> +#
> +# @result: The result of the compare operation.
> +# @responsible-properties: List of properties that led to the comparison result
> +#                          not being identical.
> +#
> +# @responsible-properties is a list of QOM property names that led to
> +# both CPUs not being detected as identical. For identical models, this
> +# list is empty.
> +# If a QOM property is read-only, that means there's no known way to make the
> +# CPU models identical. If the special property name "type" is included, the
> +# models are by definition not identical and cannot be made identical.
> +#
> +# Since: 2.8.0
> +##
> +{ 'struct': 'CpuModelCompareInfo',
> +  'data': {'result': 'CpuModelCompareResult',
> +           'responsible-properties': ['str']
> +          }
> +}
> +
> +##
> +# @query-cpu-model-comparison:
> +#
> +# Compares two CPU models, returning how they compare under a specific QEMU
> +# machine.
> +#
> +# Note: This interface should not be used when global properties of CPU classes
> +#       are changed (e.g. via "-cpu ...").
> +#
> +# s390x supports comparing of all CPU models.

This statement is not true until patch 28/29 is applied.

> Other architectures are not
> +# supported yet.

What if we provide a generic comparison function that does like
the following pseudocode:

def basic_comparison(modela, modelb):
  if modela.name == modelb.name:
    if modela.props == modelb.props:
      return "identical", []
    else:
      #XXX: maybe add some extra logic to check if
      # modela.props is a subset or superset of modelb.props?
      return "incompatible", set(modela.props.keys(), modelb.props.keys())
  else:
    return "incompatible", ["type"]

def full_comparison(modela, modelb):
  r,p = basic_comparison(modela, modelb)
  if r == "incompatible":
    try:
      modela = expand_cpu_model(modela, "full")
      modelb = expand_cpu_model(modelb, "full")
    except:
      # in case "full" expansion mode is not supported
      return r,p
    return basic_comparison(modela, modelb)


> +#
> +# 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' }
> +
>  # @AddfdInfo:
>  #
>  # Information about a file descriptor that was added to an fd set.
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 7ed9528..0af2098 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -3948,6 +3948,12 @@ EQMP
>      },
>  
>      {
> +        .name       = "query-cpu-model-comparison",
> +        .args_type  = "modela:q,modelb:q",
> +        .mhandler.cmd_new = qmp_marshal_query_cpu_model_comparison,
> +    },
> +
> +    {
>          .name       = "query-target",
>          .args_type  = "",
>          .mhandler.cmd_new = qmp_marshal_query_target,
> diff --git a/qmp.c b/qmp.c
> index 29fbfb8..f551019 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -614,6 +614,13 @@ 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);
> +}
> +
>  void qmp_add_client(const char *protocol, const char *fdname,
>                      bool has_skipauth, bool skipauth, bool has_tls, bool tls,
>                      Error **errp)
> diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
> index 4929842..da768f0 100644
> --- a/stubs/Makefile.objs
> +++ b/stubs/Makefile.objs
> @@ -1,5 +1,6 @@
>  stub-obj-y += arch-query-cpu-def.o
>  stub-obj-y += arch-query-cpu-model-expansion.o
> +stub-obj-y += arch-query-cpu-model-comparison.o
>  stub-obj-y += bdrv-next-monitor-owned.o
>  stub-obj-y += blk-commit-all.o
>  stub-obj-y += blockdev-close-all-bdrv-states.o
> diff --git a/stubs/arch-query-cpu-model-comparison.c b/stubs/arch-query-cpu-model-comparison.c
> new file mode 100644
> index 0000000..d5486ae
> --- /dev/null
> +++ b/stubs/arch-query-cpu-model-comparison.c
> @@ -0,0 +1,12 @@
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "sysemu/arch_init.h"
> +#include "qapi/qmp/qerror.h"
> +
> +CpuModelCompareInfo *arch_query_cpu_model_comparison(CpuModelInfo *modela,
> +                                                     CpuModelInfo *modelb,
> +                                                     Error **errp)
> +{
> +    error_setg(errp, QERR_UNSUPPORTED);
> +    return NULL;
> +}
> -- 
> 2.6.6
>
David Hildenbrand Aug. 2, 2016, 3:15 p.m. UTC | #2
> > +# @CpuModelCompareResult:
> > +#
> > +# An enumeration of CPU model comparation results.
> > +#
> > +# @incompatible: both model definition are incompatible
> > +#
> > +# @identical: model A == model B
> > +#
> > +# @superset: model A > model B
> > +#
> > +# @subset: model A < model B  
> 
> We need to clarify what superset/subset, ">" and "<" really mean.
> 

I think this is "feature subset" on the one hand and "earlier generation"
on the other hand - at least for s390x. But it boils down to runnability I
think: (< and > are actually quite misleading)

# @incompatible: both model definition are incompatible. A host which
#                can run model A can't run model B and the other way around.
#
# @identical: model A == model B. A host which can run model A can run
#             model B and the other way around.
#
# @superset: A host which can run model A can run model B, but not the
#            other way around.
#
# @subset: A host which can run model B can run model A, but not the
#          other way around.

> > +#
> > +# Since: 2.8.0
> > +##
> > +{ 'enum': 'CpuModelCompareResult',
> > +  'data': [ 'incompatible', 'identical', 'superset', 'subset' ] }  
> 
> I assume implementations are free to return "incompatible" if
> they still don't have any extra logic to expand CPU models and
> check for supersets/subsets. If that's the case, see my
> suggestion below for a generic comparison function.

incompatible basically means, you have to baseline to create a runnable CPU
model. Could be done, but see my last comment.

> 
> > +
> > +##
> > +# @CpuModelCompareInfo
> > +#
> > +# The result of a CPU model comparison.
> > +#
> > +# @result: The result of the compare operation.
> > +# @responsible-properties: List of properties that led to the comparison result
> > +#                          not being identical.
> > +#
> > +# @responsible-properties is a list of QOM property names that led to
> > +# both CPUs not being detected as identical. For identical models, this
> > +# list is empty.
> > +# If a QOM property is read-only, that means there's no known way to make the
> > +# CPU models identical. If the special property name "type" is included, the
> > +# models are by definition not identical and cannot be made identical.
> > +#
> > +# Since: 2.8.0
> > +##
> > +{ 'struct': 'CpuModelCompareInfo',
> > +  'data': {'result': 'CpuModelCompareResult',
> > +           'responsible-properties': ['str']
> > +          }
> > +}
> > +
> > +##
> > +# @query-cpu-model-comparison:
> > +#
> > +# Compares two CPU models, returning how they compare under a specific QEMU
> > +# machine.
> > +#
> > +# Note: This interface should not be used when global properties of CPU classes
> > +#       are changed (e.g. via "-cpu ...").
> > +#
> > +# s390x supports comparing of all CPU models.  
> 
> This statement is not true until patch 28/29 is applied.

Yes, will move it (also for the baseline patch),

> 
> > Other architectures are not
> > +# supported yet.  
> 
> What if we provide a generic comparison function that does like
> the following pseudocode:
> 
> def basic_comparison(modela, modelb):
>   if modela.name == modelb.name:
>     if modela.props == modelb.props:
>       return "identical", []
>     else:
>       #XXX: maybe add some extra logic to check if
>       # modela.props is a subset or superset of modelb.props?
>       return "incompatible", set(modela.props.keys(), modelb.props.keys())
>   else:
>     return "incompatible", ["type"]
> 
> def full_comparison(modela, modelb):
>   r,p = basic_comparison(modela, modelb)
>   if r == "incompatible":
>     try:
>       modela = expand_cpu_model(modela, "full")
>       modelb = expand_cpu_model(modelb, "full")
>     except:
>       # in case "full" expansion mode is not supported
>       return r,p
>     return basic_comparison(modela, modelb)
> 

While I agree that that would be nice to have, it doesn't fit for s390x right
now: The result on s390x does not rely on features/name only, but internal data
we don't want to expose.

e.g. z13.2 and z13s are considered identically.

z13 is a subset of z13.2, although they have exactly the same
features/properties (right now). It basically has to do with internal data
(e.g. address bus sizes for hamax as an example)

(that's where we indictate "type" under "responsible-properties" - they can
never be made identically, you have to baseline).

Thanks a lot!!!

David
Eduardo Habkost Aug. 2, 2016, 3:47 p.m. UTC | #3
On Tue, Aug 02, 2016 at 05:15:54PM +0200, David Hildenbrand wrote:
> > > +# @CpuModelCompareResult:
> > > +#
> > > +# An enumeration of CPU model comparation results.
> > > +#
> > > +# @incompatible: both model definition are incompatible
> > > +#
> > > +# @identical: model A == model B
> > > +#
> > > +# @superset: model A > model B
> > > +#
> > > +# @subset: model A < model B  
> > 
> > We need to clarify what superset/subset, ">" and "<" really mean.
> > 
> 
> I think this is "feature subset" on the one hand and "earlier generation"
> on the other hand - at least for s390x. But it boils down to runnability I
> think: (< and > are actually quite misleading)

It sounds like we need to clarify what are the use cases and
requirements, to find out how to document the exact meaning of
"superset" and "subset".

I assume this is closely related to the semantics of
query-cpu-model-baseline (which I didn't review yet). In other
words: "superset" and "subset" means you can save a
query-cpu-model-baseline call because you already know modela or
modelb can be used as baseline, already. Is that correct?

In this case, I will get back to this while reviewing and
discussing query-cpu-model-baseline.

> 
> # @incompatible: both model definition are incompatible. A host which
> #                can run model A can't run model B and the other way around.
> #
> # @identical: model A == model B. A host which can run model A can run
> #             model B and the other way around.
> #
> # @superset: A host which can run model A can run model B, but not the
> #            other way around.
> #
> # @subset: A host which can run model B can run model A, but not the
> #          other way around.
> 
> > > +#
> > > +# Since: 2.8.0
> > > +##
> > > +{ 'enum': 'CpuModelCompareResult',
> > > +  'data': [ 'incompatible', 'identical', 'superset', 'subset' ] }  
> > 
> > I assume implementations are free to return "incompatible" if
> > they still don't have any extra logic to expand CPU models and
> > check for supersets/subsets. If that's the case, see my
> > suggestion below for a generic comparison function.
> 
> incompatible basically means, you have to baseline to create a runnable CPU
> model. Could be done, but see my last comment.
> 
> > 
> > > +
> > > +##
> > > +# @CpuModelCompareInfo
> > > +#
> > > +# The result of a CPU model comparison.
> > > +#
> > > +# @result: The result of the compare operation.
> > > +# @responsible-properties: List of properties that led to the comparison result
> > > +#                          not being identical.
> > > +#
> > > +# @responsible-properties is a list of QOM property names that led to
> > > +# both CPUs not being detected as identical. For identical models, this
> > > +# list is empty.
> > > +# If a QOM property is read-only, that means there's no known way to make the
> > > +# CPU models identical. If the special property name "type" is included, the
> > > +# models are by definition not identical and cannot be made identical.
> > > +#
> > > +# Since: 2.8.0
> > > +##
> > > +{ 'struct': 'CpuModelCompareInfo',
> > > +  'data': {'result': 'CpuModelCompareResult',
> > > +           'responsible-properties': ['str']
> > > +          }
> > > +}
> > > +
> > > +##
> > > +# @query-cpu-model-comparison:
> > > +#
> > > +# Compares two CPU models, returning how they compare under a specific QEMU
> > > +# machine.
> > > +#
> > > +# Note: This interface should not be used when global properties of CPU classes
> > > +#       are changed (e.g. via "-cpu ...").
> > > +#
> > > +# s390x supports comparing of all CPU models.  
> > 
> > This statement is not true until patch 28/29 is applied.
> 
> Yes, will move it (also for the baseline patch),
> 
> > 
> > > Other architectures are not
> > > +# supported yet.  
> > 
> > What if we provide a generic comparison function that does like
> > the following pseudocode:
> > 
> > def basic_comparison(modela, modelb):
> >   if modela.name == modelb.name:
> >     if modela.props == modelb.props:
> >       return "identical", []
> >     else:
> >       #XXX: maybe add some extra logic to check if
> >       # modela.props is a subset or superset of modelb.props?
> >       return "incompatible", set(modela.props.keys(), modelb.props.keys())
> >   else:
> >     return "incompatible", ["type"]
> > 
> > def full_comparison(modela, modelb):
> >   r,p = basic_comparison(modela, modelb)
> >   if r == "incompatible":
> >     try:
> >       modela = expand_cpu_model(modela, "full")
> >       modelb = expand_cpu_model(modelb, "full")
> >     except:
> >       # in case "full" expansion mode is not supported
> >       return r,p
> >     return basic_comparison(modela, modelb)
> > 
> 
> While I agree that that would be nice to have, it doesn't fit for s390x right
> now: The result on s390x does not rely on features/name only, but internal data
> we don't want to expose.
> 
> e.g. z13.2 and z13s are considered identically.
> 
> z13 is a subset of z13.2, although they have exactly the same
> features/properties (right now). It basically has to do with internal data
> (e.g. address bus sizes for hamax as an example)
> 
> (that's where we indictate "type" under "responsible-properties" - they can
> never be made identically, you have to baseline).

Right, I don't mean it to be enough for all architectures, but as
a basic implementation that is minimally useful when there's no
arch-specific comparison function.
David Hildenbrand Aug. 3, 2016, 7:09 a.m. UTC | #4
> > >   
> > 
> > I think this is "feature subset" on the one hand and "earlier generation"
> > on the other hand - at least for s390x. But it boils down to runnability I
> > think: (< and > are actually quite misleading)  
> 
> It sounds like we need to clarify what are the use cases and
> requirements, to find out how to document the exact meaning of
> "superset" and "subset".
> 
> I assume this is closely related to the semantics of
> query-cpu-model-baseline (which I didn't review yet). In other
> words: "superset" and "subset" means you can save a
> query-cpu-model-baseline call because you already know modela or
> modelb can be used as baseline, already. Is that correct?

Depends on the order of parameters. But I think this was clarified to my reply
in the cover letter. Will improve the overall explanation of
query-cpu-model-comparison

> 
> In this case, I will get back to this while reviewing and
> discussing query-cpu-model-baseline.

That sure makes sense, they go hand-in-hand, especially for the two commands
"virsh cpu-baseline" and "virsh cpu-compare". It's all about testing/detecting
runnable CPU models for different environments.

[...]
> > > > Other architectures are not
> > > > +# supported yet.    
> > > 
> > > What if we provide a generic comparison function that does like
> > > the following pseudocode:
> > > 
> > > def basic_comparison(modela, modelb):
> > >   if modela.name == modelb.name:
> > >     if modela.props == modelb.props:
> > >       return "identical", []
> > >     else:
> > >       #XXX: maybe add some extra logic to check if
> > >       # modela.props is a subset or superset of modelb.props?
> > >       return "incompatible", set(modela.props.keys(), modelb.props.keys())
> > >   else:
> > >     return "incompatible", ["type"]
> > > 
> > > def full_comparison(modela, modelb):
> > >   r,p = basic_comparison(modela, modelb)
> > >   if r == "incompatible":
> > >     try:
> > >       modela = expand_cpu_model(modela, "full")
> > >       modelb = expand_cpu_model(modelb, "full")
> > >     except:
> > >       # in case "full" expansion mode is not supported
> > >       return r,p
> > >     return basic_comparison(modela, modelb)
> > >   
> > 
> > While I agree that that would be nice to have, it doesn't fit for s390x right
> > now: The result on s390x does not rely on features/name only, but internal data
> > we don't want to expose.
> > 
> > e.g. z13.2 and z13s are considered identically.
> > 
> > z13 is a subset of z13.2, although they have exactly the same
> > features/properties (right now). It basically has to do with internal data
> > (e.g. address bus sizes for hamax as an example)
> > 
> > (that's where we indictate "type" under "responsible-properties" - they can
> > never be made identically, you have to baseline).  
> 
> Right, I don't mean it to be enough for all architectures, but as
> a basic implementation that is minimally useful when there's no
> arch-specific comparison function.

Then the question would be: Is it better to have a wrong result than any result?
As it doesn't work on s390x, there could also be a problem with other
architectures.

Especially when comparing against "host", the name after expansion would give
no identication about runnability. And only living with
"incompatible" and "identical" might also not really be enough.

While this could make sense, I'd suggest postponing such a basic comapare
function until other architectures support the basics (expanding to full) and
then see if this basic function would work for them (maybe x86 is a good
candidate once the expanded name would not be "host" anymore).

Thanks Eduardo!

David
Eduardo Habkost Aug. 3, 2016, 5:39 p.m. UTC | #5
On Wed, Aug 03, 2016 at 09:09:25AM +0200, David Hildenbrand wrote:
[...]
> > > > > Other architectures are not
> > > > > +# supported yet.    
> > > > 
> > > > What if we provide a generic comparison function that does like
> > > > the following pseudocode:
> > > > 
> > > > def basic_comparison(modela, modelb):
> > > >   if modela.name == modelb.name:
> > > >     if modela.props == modelb.props:
> > > >       return "identical", []
> > > >     else:
> > > >       #XXX: maybe add some extra logic to check if
> > > >       # modela.props is a subset or superset of modelb.props?
> > > >       return "incompatible", set(modela.props.keys(), modelb.props.keys())
> > > >   else:
> > > >     return "incompatible", ["type"]
> > > > 
> > > > def full_comparison(modela, modelb):
> > > >   r,p = basic_comparison(modela, modelb)
> > > >   if r == "incompatible":
> > > >     try:
> > > >       modela = expand_cpu_model(modela, "full")
> > > >       modelb = expand_cpu_model(modelb, "full")
> > > >     except:
> > > >       # in case "full" expansion mode is not supported
> > > >       return r,p
> > > >     return basic_comparison(modela, modelb)
> > > >   
> > > 
> > > While I agree that that would be nice to have, it doesn't fit for s390x right
> > > now: The result on s390x does not rely on features/name only, but internal data
> > > we don't want to expose.
> > > 
> > > e.g. z13.2 and z13s are considered identically.
> > > 
> > > z13 is a subset of z13.2, although they have exactly the same
> > > features/properties (right now). It basically has to do with internal data
> > > (e.g. address bus sizes for hamax as an example)
> > > 
> > > (that's where we indictate "type" under "responsible-properties" - they can
> > > never be made identically, you have to baseline).  
> > 
> > Right, I don't mean it to be enough for all architectures, but as
> > a basic implementation that is minimally useful when there's no
> > arch-specific comparison function.
> 
> Then the question would be: Is it better to have a wrong result than any result?
> As it doesn't work on s390x, there could also be a problem with other
> architectures.
> 
> Especially when comparing against "host", the name after expansion would give
> no identication about runnability. And only living with
> "incompatible" and "identical" might also not really be enough.
> 
> While this could make sense, I'd suggest postponing such a basic comapare
> function until other architectures support the basics (expanding to full) and
> then see if this basic function would work for them (maybe x86 is a good
> candidate once the expanded name would not be "host" anymore).

Agreed. After we have a x86 implementation, we can look into
implementing a generic comparison function (but make it opt-in,
instead of enabling it for all architectures at once).
David Hildenbrand Aug. 4, 2016, 7:34 a.m. UTC | #6
> +##
> +# @CpuModelCompareResult:
> +#
> +# An enumeration of CPU model comparation results.
> +#
> +# @incompatible: both model definition are incompatible
> +#
> +# @identical: model A == model B
> +#
> +# @superset: model A > model B
> +#
> +# @subset: model A < model B
> +#
> +# Since: 2.8.0
> +##

This comment is now:

##                                                                              
# @CpuModelCompareResult:                                                       
#                                                                               
# An enumeration of CPU model comparation results. The result is usually        
# calcualted using e.g. at CPU features or CPU generations.                     
#                                                                               
# @incompatible: If model A is incompatible to model B, model A is not          
#                guaranteed to run where model B runs and the other way around. 
#                                                                               
# @identical: If model A is identical to model B, model A is guaranteed to run  
#             where model B runs and the other way around.                      
#                                                                               
# @superset: If model A is a superset of model B, model B is guaranteed to run  
#            where model A runs. There are no guarantees about the other way.   
#                                                                               
# @subset: If model A is a subset of model B, model A is guaranteed to run      
#          where model B runs. There are no guarantees about the other way.     
#                                                                               
# Since: 2.8.0                                                                  
##

Think it's all about guarantees.

> +{ 'enum': 'CpuModelCompareResult',
> +  'data': [ 'incompatible', 'identical', 'superset', 'subset' ] }
> +
> +##
> +# @CpuModelCompareInfo
> +#
> +# The result of a CPU model comparison.
> +#
> +# @result: The result of the compare operation.
> +# @responsible-properties: List of properties that led to the comparison result
> +#                          not being identical.
> +#
> +# @responsible-properties is a list of QOM property names that led to
> +# both CPUs not being detected as identical. For identical models, this
> +# list is empty.
> +# If a QOM property is read-only, that means there's no known way to make the
> +# CPU models identical. If the special property name "type" is included, the
> +# models are by definition not identical and cannot be made identical.
> +#
> +# Since: 2.8.0
> +##
> +{ 'struct': 'CpuModelCompareInfo',
> +  'data': {'result': 'CpuModelCompareResult',
> +           'responsible-properties': ['str']
> +          }
> +}
> +
> +##
> +# @query-cpu-model-comparison:
> +#
> +# Compares two CPU models, returning how they compare under a specific QEMU
> +# machine.
> +#
> +# Note: This interface should not be used when global properties of CPU classes
> +#       are changed (e.g. via "-cpu ...").
> +#
> +# s390x supports comparing of all CPU models. Other architectures are not
> +# supported yet.
> +#
> +# 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
> +##

This comment is now:

##                                                                              
# @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 ceratin 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                                                                  
##  

(excluding the remark about s390x in this patch)

David
Eduardo Habkost Aug. 4, 2016, 2:36 p.m. UTC | #7
On Thu, Aug 04, 2016 at 09:34:16AM +0200, David Hildenbrand wrote:
> ##                                                                              
> # @CpuModelCompareResult:                                                       
> #                                                                               
> # An enumeration of CPU model comparation results. The result is usually        
> # calcualted using e.g. at CPU features or CPU generations.                     
> #                                                                               
> # @incompatible: If model A is incompatible to model B, model A is not          
> #                guaranteed to run where model B runs and the other way around. 
> #                                                                               
> # @identical: If model A is identical to model B, model A is guaranteed to run  
> #             where model B runs and the other way around.                      
> #                                                                               
> # @superset: If model A is a superset of model B, model B is guaranteed to run  
> #            where model A runs. There are no guarantees about the other way.   
> #                                                                               
> # @subset: If model A is a subset of model B, model A is guaranteed to run      
> #          where model B runs. There are no guarantees about the other way.     
> #                                                                               
> # Since: 2.8.0                                                                  
> ##
> 
> Think it's all about guarantees.
> 
[...]
> This comment is now:
> 
> ##                                                                              
> # @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 ceratin 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                                                                  
> ##  
> 
> (excluding the remark about s390x in this patch)

Both look very clear to me. Thanks!
diff mbox

Patch

diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
index 37b2e86..96d47c0 100644
--- a/include/sysemu/arch_init.h
+++ b/include/sysemu/arch_init.h
@@ -38,5 +38,8 @@  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);
 
 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index 43f7969..1f8f8ec 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3149,6 +3149,71 @@ 
             'model': 'CpuModelInfo' },
   'returns': 'CpuModelExpansionInfo' }
 
+##
+# @CpuModelCompareResult:
+#
+# An enumeration of CPU model comparation results.
+#
+# @incompatible: both model definition are incompatible
+#
+# @identical: model A == model B
+#
+# @superset: model A > model B
+#
+# @subset: model A < model B
+#
+# Since: 2.8.0
+##
+{ 'enum': 'CpuModelCompareResult',
+  'data': [ 'incompatible', 'identical', 'superset', 'subset' ] }
+
+##
+# @CpuModelCompareInfo
+#
+# The result of a CPU model comparison.
+#
+# @result: The result of the compare operation.
+# @responsible-properties: List of properties that led to the comparison result
+#                          not being identical.
+#
+# @responsible-properties is a list of QOM property names that led to
+# both CPUs not being detected as identical. For identical models, this
+# list is empty.
+# If a QOM property is read-only, that means there's no known way to make the
+# CPU models identical. If the special property name "type" is included, the
+# models are by definition not identical and cannot be made identical.
+#
+# Since: 2.8.0
+##
+{ 'struct': 'CpuModelCompareInfo',
+  'data': {'result': 'CpuModelCompareResult',
+           'responsible-properties': ['str']
+          }
+}
+
+##
+# @query-cpu-model-comparison:
+#
+# Compares two CPU models, returning how they compare under a specific QEMU
+# machine.
+#
+# Note: This interface should not be used when global properties of CPU classes
+#       are changed (e.g. via "-cpu ...").
+#
+# s390x supports comparing of all CPU models. Other architectures are not
+# supported yet.
+#
+# 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' }
+
 # @AddfdInfo:
 #
 # Information about a file descriptor that was added to an fd set.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 7ed9528..0af2098 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3948,6 +3948,12 @@  EQMP
     },
 
     {
+        .name       = "query-cpu-model-comparison",
+        .args_type  = "modela:q,modelb:q",
+        .mhandler.cmd_new = qmp_marshal_query_cpu_model_comparison,
+    },
+
+    {
         .name       = "query-target",
         .args_type  = "",
         .mhandler.cmd_new = qmp_marshal_query_target,
diff --git a/qmp.c b/qmp.c
index 29fbfb8..f551019 100644
--- a/qmp.c
+++ b/qmp.c
@@ -614,6 +614,13 @@  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);
+}
+
 void qmp_add_client(const char *protocol, const char *fdname,
                     bool has_skipauth, bool skipauth, bool has_tls, bool tls,
                     Error **errp)
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 4929842..da768f0 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -1,5 +1,6 @@ 
 stub-obj-y += arch-query-cpu-def.o
 stub-obj-y += arch-query-cpu-model-expansion.o
+stub-obj-y += arch-query-cpu-model-comparison.o
 stub-obj-y += bdrv-next-monitor-owned.o
 stub-obj-y += blk-commit-all.o
 stub-obj-y += blockdev-close-all-bdrv-states.o
diff --git a/stubs/arch-query-cpu-model-comparison.c b/stubs/arch-query-cpu-model-comparison.c
new file mode 100644
index 0000000..d5486ae
--- /dev/null
+++ b/stubs/arch-query-cpu-model-comparison.c
@@ -0,0 +1,12 @@ 
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "sysemu/arch_init.h"
+#include "qapi/qmp/qerror.h"
+
+CpuModelCompareInfo *arch_query_cpu_model_comparison(CpuModelInfo *modela,
+                                                     CpuModelInfo *modelb,
+                                                     Error **errp)
+{
+    error_setg(errp, QERR_UNSUPPORTED);
+    return NULL;
+}