On Tue, 13 May 2014 09:29:37 -0600
Eric Blake <eblake@xxxxxxxxxx> 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
Attachment:
signature.asc
Description: PGP signature
