Re: [Qemu-devel] [PATCH v7 12/13] qmp: Add query-ppc-cpu-cores command

[Eric Blake <eblake@redhat.com>]

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

On 01/27/2016 10:49 PM, Bharata B Rao wrote:
> Show the details of PPC CPU cores via a new QMP command.
> 
> TODO: update qmp-commands.hx with example

Is this a stale comment? [1]

> 
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> ---

>  #include <sysemu/cpus.h>
> +#include <sysemu/kvm.h>
>  #include "qemu/error-report.h"
> +#include "qmp-commands.h"
> +
> +/*
> + * QMP: info ppc-cpu-cores
> + */
> +static int qmp_ppc_cpu_list(Object *obj, void *opaque)

Comment is a bit off - 'info ...' is an HMP command; this callback is
helping implement the QMP function query-ppc-cpu-cores.

> +++ b/qapi-schema.json
> @@ -4083,3 +4083,34 @@
>  ##
>  { 'enum': 'ReplayMode',
>    'data': [ 'none', 'record', 'play' ] }
> +
> +##
> +# @PPCCPUCore:
> +#
> +# Information about PPC CPU core devices
> +#
> +# @hotplugged: true if device was hotplugged
> +#
> +# @hotpluggable: true if device if could be added/removed while machine is running
> +#
> +# Since: 2.6

Missing docs on 'id' and 'threads'.

> +##
> +
> +{ 'struct': 'PPCCPUCore',
> +  'data': { '*id': 'str',
> +            'hotplugged': 'bool',
> +            'hotpluggable': 'bool',
> +            'threads' : ['CpuInfo']
> +          }
> +}
> +
> +##
> +# @query-ppc-cpu-core:
> +#
> +# Returns information for all PPC CPU core devices
> +#
> +# Returns: a list of @PPCCPUCore.
> +#
> +# Since: 2.6
> +##
> +{ 'command': 'query-ppc-cpu-cores', 'returns': ['PPCCPUCore'] }

Interface seems okay.

> +++ b/qmp-commands.hx
> @@ -4795,3 +4795,54 @@ Example:
>                   {"type": 0, "out-pport": 0, "pport": 0, "vlan-id": 3840,
>                    "pop-vlan": 1, "id": 251658240}
>     ]}
> +
> +EQMP
> +
> +#if defined TARGET_PPC64
> +    {
> +        .name       = "query-ppc-cpu-cores",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_marshal_query_ppc_cpu_cores,
> +    },
> +#endif

Hmm. Conditional compilation. Does the command show up in
'query-commands' and introspection output, even when the target is not
ppc64?  We may need to fix qapi introspection to support conditionals
better; maybe some of Marc-Andre's patches towards eliminating
qmp-commands.hx will come into play here.

> +
> +SQMP
> +@query-ppc-cpu-cores
> +--------------------
> +
> +Show PowerPC CPU core devices information.
> +
> +Example:
> +-> { "execute": "query-ppc-cpu-cores" }
> +<- {"return": [{"threads": [
> +                 {"arch": "ppc",

[1] looks like you provided an example after all.  Is it worth
documenting that this command is only conditionally available?


> +++ b/stubs/qmp_query_ppc_cpu_cores.c
> @@ -0,0 +1,10 @@
> +#include "qom/object.h"
> +#include "qapi/qmp/qerror.h"
> +#include "qemu/typedefs.h"
> +#include "qmp-commands.h"
> +
> +PPCCPUCoreList *qmp_query_ppc_cpu_cores(Error **errp)
> +{
> +    error_setg(errp, QERR_UNSUPPORTED);
> +    return 0;
> +}

Hmm - will the stub even be used, since you used an #ifdef in the .hx file?

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