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