From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53050) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WkVAb-0005UV-Ax for qemu-devel@nongnu.org; Wed, 14 May 2014 05:08:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WkVAS-0002Qs-8g for qemu-devel@nongnu.org; Wed, 14 May 2014 05:08:13 -0400 Received: from e06smtp18.uk.ibm.com ([195.75.94.114]:38946) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WkVAS-0002Qi-0o for qemu-devel@nongnu.org; Wed, 14 May 2014 05:08:04 -0400 Received: from /spool/local by e06smtp18.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 14 May 2014 10:08:02 +0100 Received: from b06cxnps4075.portsmouth.uk.ibm.com (d06relay12.portsmouth.uk.ibm.com [9.149.109.197]) by d06dlp03.portsmouth.uk.ibm.com (Postfix) with ESMTP id 9153B1B08041 for ; Wed, 14 May 2014 10:08:13 +0100 (BST) Received: from d06av02.portsmouth.uk.ibm.com (d06av02.portsmouth.uk.ibm.com [9.149.37.228]) by b06cxnps4075.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id s4E97xu166650350 for ; Wed, 14 May 2014 09:07:59 GMT Received: from d06av02.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av02.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s4E97veA005722 for ; Wed, 14 May 2014 03:07:58 -0600 Date: Wed, 14 May 2014 11:07:46 +0200 From: Michael Mueller Message-ID: <20140514110746.42b6b814@bee> In-Reply-To: <53723A61.4020601@redhat.com> References: <1399993222-16339-1-git-send-email-mimu@linux.vnet.ibm.com> <1399993222-16339-10-git-send-email-mimu@linux.vnet.ibm.com> <53723A61.4020601@redhat.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=PGP-SHA1; boundary="Sig_/C/Ftq.Q4daj=rjN5fcS=J4="; protocol="application/pgp-signature" Subject: Re: [Qemu-devel] [PATCH v1 RFC 09/10] QEMU: s390: cpu model QMP query-cpu-model List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: linux-s390@vger.kernel.org, kvm@vger.kernel.org, Gleb Natapov , linux-kernel@vger.kernel.org, Alexander Graf , qemu-devel@nongnu.org, Christian Borntraeger , "Jason J. Herne" , Cornelia Huck , Paolo Bonzini , Andreas Faerber , Richard Henderson --Sig_/C/Ftq.Q4daj=rjN5fcS=J4= Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable On Tue, 13 May 2014 09:29:37 -0600 Eric Blake wrote: Hi Eric, > On 05/13/2014 09:00 AM, Michael Mueller wrote: > > This patch implements a new QMP request named "query-cpu-model". It ret= urns > > the cpu model of cpu 0. eg: > >=20 > > request: '{"execute" : "query-cpu-model" } > > answer: '{"return" : {"name": "2827-ga2"}} > >=20 > > Alias names are resolved to their respective machine type and GA names > > already during cpu instantiation. Thus, also a cpu name like "host", > > which is implemented as alias, will return its normalized cpu model nam= e. > >=20 >=20 > > + } > > + cpu_model =3D g_try_malloc0(sizeof(*cpu_model)); >=20 > It's simpler to just use g_malloc0 and rely on glibc's exit-on-OOM > behavior than to try and deal with NULL - this isn't user input (so > unlikely to be so huge as to cause OOM), and would be more in line with > what most other QMP code does. But that said... >=20 > > + if (!cpu_model) { > > + goto out_no_mem; > > + } > > + cpu_model->name =3D g_try_malloc0(CPU_MODEL_NAME_LEN); > > + if (!cpu_model->name) { > > + goto out_no_mem; > > + } > > + snprintf(cpu_model->name, CPU_MODEL_NAME_LEN - 1, "%04x-ga%u", > > + cc->proc->type, cc->mach->ga); >=20 > ...why not just use g_strdup_printf() instead of trying to size a buffer > yourself? In other words, skip the g_try_malloc0 to begin with. I will use that function and I think I can use it with "query-cpu-definitio= ns" as well. >=20 > The fact that you are packing two pieces of information into one string > is a bit worrisome - that means that the client of the QMP command has > to parse the string back into two pieces of information if they ever > need either item in isolation. If the user never has a need to split > the name down into parts, you are okay; I don't know S390 well enough to I see your point, but I consider it as a name only, which needs to be unique to identify different configurations but has no meaning on the management interface level. > know whether anyone will care about type separate from ga. But if > someone DOES care, then the QMP command should return the parts already > split, as in { "type": 2827, "ga": 2 }, or even as convenience provide > both split and combined forms: { "name": "2827-ga2", "type": 2827, "ga": = 2 } >=20 Offering both options seems to be desirable but I have to talk with libvirt= if that could be done in a transparent form for them. Thanks a lot for your comments Michael --Sig_/C/Ftq.Q4daj=rjN5fcS=J4= Content-Type: application/pgp-signature; name=signature.asc Content-Disposition: attachment; filename=signature.asc -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) iQIcBAEBAgAGBQJTczJiAAoJELPcPLQSJsKQI5kP/39fScXk99oIs042U7nhoDsQ T6MEgzPGy0RgO+PsFPABtrhBxIvMtVf6DxUMTHOBnJ6NKnUZt1q5HAmgm2n9KIBC +Eb5JTphsRxh7jWzmr3Q9RQW40Atbh3aMzVZNCvSmoQ5AyxHGUnCu/ZqQNbOa7SI iK6dNok9ZsLkPcnVgjvvDnmuDKWBH4vpI5G4sMY1NQUB783RrRp2itiYQTnZDaL7 IrIw68RzFMe2RVXyHQCKFj/YivzXpMSbvyIosX0Cv2cAEViOeQJPqqqY20upMr+m CCh8KZOy7martSg1p/8tDZV/hFfFftTqPwSDC2XPLgHGxIl57X0ezfdZAN+1gep8 3CbRFJdw0P3QgBAL2NSN2oNSD3BRlUQTarzUEe2ETqdmjYWw0ygkPxjgrFRvfeZm HpFayYK3JQYzGo5E1Sf8F68d0S+YVLwiYM1xBZMbqmqUtIKJsnFFHR7v4rAcWBnt Eo10FLz2mFjn/P0Yq3HwDyJ/Xxrfwbs8Rgs/NG6bLmCtGJqrE4HrnUs1myuizXg3 cRbOnm2BlhLcjg/o9aNdZDmVeLbSc3RYpea6yPkM1obAVY3wbH1YBKsRIlGRfs34 FlDFPK4SopESIKQODeNT35Kbm0S+wpzeR0Y3ygd28gwy3BvlUlQ5hg/zT+nJs9IN WWpNv0YgURWHHqPD31iX =uWfH -----END PGP SIGNATURE----- --Sig_/C/Ftq.Q4daj=rjN5fcS=J4=--