Re: [Qemu-devel] [PATCH v7 13/13] hmp: Add "info ppc-cpu-cores" command

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2153 bytes --]

On 01/27/2016 10:49 PM, Bharata B Rao wrote:
> This is the hmp equivalent of "query ppc-cpu-cores"

The QMP command is spelled "query-ppc-cpu-cores".

Most HMP commands prefer '_' over '-'; so this should be 'info
ppc_cpu_cores'.

> 
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> ---
>  hmp-commands-info.hx | 16 ++++++++++++++++
>  hmp.c                | 31 +++++++++++++++++++++++++++++++
>  hmp.h                |  1 +
>  3 files changed, 48 insertions(+)
> 

> +++ b/hmp.c
> @@ -2375,3 +2375,34 @@ void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict *qdict)
>  
>      qapi_free_RockerOfDpaGroupList(list);
>  }
> +
> +void hmp_info_ppc_cpu_cores(Monitor *mon, const QDict *qdict)
> +{
> +    Error *err = NULL;
> +    PPCCPUCoreList *ppc_cpu_core_list = qmp_query_ppc_cpu_cores(&err);
> +    PPCCPUCoreList *s = ppc_cpu_core_list;
> +    CpuInfoList *thread;
> +
> +    while (s) {
> +        monitor_printf(mon, "PowerPC CPU device: \"%s\"\n",
> +                       s->value->id ? s->value->id : "");

This should probably be checking s->value->has_id rather than assuming
that s->value->id will be NULL when not present (well, I'd like to clean
up qapi to avoid the need for has_FOO when FOO  is a pointer, but we're
not there yet).

> +        monitor_printf(mon, "  hotplugged: %s\n",
> +                           s->value->hotplugged ? "true" : "false");
> +        monitor_printf(mon, "  hotpluggable: %s\n",
> +                           s->value->hotpluggable ? "true" : "false");
> +        monitor_printf(mon, "  Threads:\n");
> +        for (thread = s->value->threads; thread; thread = thread->next) {
> +            monitor_printf(mon, "    CPU #%" PRId64 ":", thread->value->CPU);
> +            monitor_printf(mon, " nip=0x%016" PRIx64,
> +                           thread->value->u.ppc->nip);

This uses value->u.ppc without first checking that the discriminator
value->arch is set to CPU_INFO_ARCH_PPC; could that be a problem down
the road?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]