[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v1 RFC 09/10] QEMU: s390: cpu model QMP query-cp
From: |
Michael Mueller |
Subject: |
Re: [Qemu-devel] [PATCH v1 RFC 09/10] QEMU: s390: cpu model QMP query-cpu-model |
Date: |
Wed, 14 May 2014 11:07:46 +0200 |
On Tue, 13 May 2014 09:29:37 -0600
Eric Blake <address@hidden> 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
signature.asc
Description: PGP signature
- [Qemu-devel] [PATCH v1 RFC 00/10] QEMU: s390: cpu model implementation, Michael Mueller, 2014/05/13
- [Qemu-devel] [PATCH v1 RFC 01/10] QEMU: introduce function cpudesc_avail, Michael Mueller, 2014/05/13
- [Qemu-devel] [PATCH v1 RFC 03/10] QEMU: s390: cpu model facilities support, Michael Mueller, 2014/05/13
- [Qemu-devel] [PATCH v1 RFC 02/10] QEMU: s390: cpu model cpu class definition, Michael Mueller, 2014/05/13
- [Qemu-devel] [PATCH v1 RFC 07/10] QEMU: s390: cpu model class initialization, Michael Mueller, 2014/05/13
- [Qemu-devel] [PATCH v1 RFC 04/10] QEMU: s390: cpu model alias support, Michael Mueller, 2014/05/13
- [Qemu-devel] [PATCH v1 RFC 05/10] s390: update linux-headers for kvm VM device attributes, Michael Mueller, 2014/05/13
- [Qemu-devel] [PATCH v1 RFC 06/10] QEMU: s390: cpu model kvm VM attr interface routines, Michael Mueller, 2014/05/13
- [Qemu-devel] [PATCH v1 RFC 09/10] QEMU: s390: cpu model QMP query-cpu-model, Michael Mueller, 2014/05/13
- [Qemu-devel] [PATCH v1 RFC 08/10] QEMU: s390: cpu model QMP query-cpu-definitions, Michael Mueller, 2014/05/13
- [Qemu-devel] [PATCH v1 RFC 10/10] QEMU: s390: cpu model enablement, Michael Mueller, 2014/05/13