Re: [Qemu-devel] [Patch v1 25/29] qmp: add QMP interface "query-cpu-model-comparison"

[Eduardo Habkost]

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).

-- 
Eduardo