linux-kernel - Re: [Qemu-devel] [PATCH v1 RFC 09/10] QEMU: s390: cpu model QMP query-cpu-model

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Wed, 14 May 2014 11:07:46 +0200
From:	Michael Mueller <mimu@...ux.vnet.ibm.com>
To:	Eric Blake <eblake@...hat.com>
Cc:	qemu-devel@...gnu.org, kvm@...r.kernel.org,
	linux-s390@...r.kernel.org, linux-kernel@...r.kernel.org,
	Gleb Natapov <gleb@...nel.org>, Alexander Graf <agraf@...e.de>,
	Christian Borntraeger <borntraeger@...ibm.com>,
	"Jason J. Herne" <jjherne@...ux.vnet.ibm.com>,
	Cornelia Huck <cornelia.huck@...ibm.com>,
	Paolo Bonzini <pbonzini@...hat.com>,
	Andreas Faerber <afaerber@...e.de>,
	Richard Henderson <rth@...ddle.net>
Subject: Re: [Qemu-devel] [PATCH v1 RFC 09/10] QEMU: s390: cpu model QMP
 query-cpu-model

On Tue, 13 May 2014 09:29:37 -0600
Eric Blake <eblake@...hat.com> 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 returns
> > the cpu model of cpu 0. eg:
> > 
> > request: '{"execute" : "query-cpu-model" }
> > answer:  '{"return" : {"name": "2827-ga2"}}
> > 
> > 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 name.
> > 
> 
> > +    }
> > +    cpu_model = g_try_malloc0(sizeof(*cpu_model));
> 
> 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...
> 
> > +    if (!cpu_model) {
> > +        goto out_no_mem;
> > +    }
> > +    cpu_model->name = 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);
> 
> ...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-definitions"
as well.

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

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

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)