From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56486) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bUbQM-000867-Vs for qemu-devel@nongnu.org; Tue, 02 Aug 2016 11:16:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bUbQJ-0004j8-IX for qemu-devel@nongnu.org; Tue, 02 Aug 2016 11:16:06 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:38207 helo=mx0a-001b2d01.pphosted.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bUbQJ-0004j0-D8 for qemu-devel@nongnu.org; Tue, 02 Aug 2016 11:16:03 -0400 Received: from pps.filterd (m0098420.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.11/8.16.0.11) with SMTP id u72FEwIK118773 for ; Tue, 2 Aug 2016 11:16:02 -0400 Received: from e06smtp13.uk.ibm.com (e06smtp13.uk.ibm.com [195.75.94.109]) by mx0b-001b2d01.pphosted.com with ESMTP id 24gre2q3gg-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Tue, 02 Aug 2016 11:16:01 -0400 Received: from localhost by e06smtp13.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 2 Aug 2016 16:16:00 +0100 Received: from b06cxnps3074.portsmouth.uk.ibm.com (d06relay09.portsmouth.uk.ibm.com [9.149.109.194]) by d06dlp01.portsmouth.uk.ibm.com (Postfix) with ESMTP id 3BDB817D8056 for ; Tue, 2 Aug 2016 16:17:33 +0100 (BST) Received: from d06av03.portsmouth.uk.ibm.com (d06av03.portsmouth.uk.ibm.com [9.149.37.213]) by b06cxnps3074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u72FFvj117432780 for ; Tue, 2 Aug 2016 15:15:57 GMT Received: from d06av03.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av03.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id u72FFuE1006682 for ; Tue, 2 Aug 2016 09:15:56 -0600 Date: Tue, 2 Aug 2016 17:15:54 +0200 From: David Hildenbrand In-Reply-To: <20160802144552.GH3337@thinpad.lan.raisama.net> References: <1470139155-53900-1-git-send-email-dahi@linux.vnet.ibm.com> <1470139155-53900-26-git-send-email-dahi@linux.vnet.ibm.com> <20160802144552.GH3337@thinpad.lan.raisama.net> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-Id: <20160802171554.6483ce9a@thinkpad-w530> Subject: Re: [Qemu-devel] [Patch v1 25/29] qmp: add QMP interface "query-cpu-model-comparison" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: qemu-devel@nongnu.org, jdenemar@redhat.com, imammedo@redhat.com, cornelia.huck@de.ibm.com, borntraeger@de.ibm.com, fiuczy@linux.vnet.ibm.com, mimu@linux.vnet.ibm.com > > +# @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