Re: [Qemu-devel] [PATCH for-2.7 v6 1/2] QMP: add query-hotpluggable-cpus

[David Gibson <dgibson@redhat.com>]

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

On Mon, 11 Apr 2016 11:35:12 +0200
Igor Mammedov <imammedo@redhat.com> wrote:

> On Mon, 11 Apr 2016 14:20:27 +1000
> David Gibson <dgibson@redhat.com> wrote:
> 
> > On Fri,  8 Apr 2016 13:29:55 +0200
> > Igor Mammedov <imammedo@redhat.com> wrote:
> >   
> > > it will allow mgmt to query present and hotpluggable
> > > CPU objects, it is required from a target platform that
> > > wish to support command to implement and set
> > >  MachineClass.query_hotpluggable_cpus
> > > callback, which will return a list of possible CPU objects
> > > with options that would be needed for hotplugging possible
> > > CPU objects.
> > > 
> > > There are:
> > > 'type': 'str' - QOM CPU object type for usage with device_add
> > > 'vcpus-count': 'int' - number of logical VCPU threads per
> > >                         CPU object (mgmt needs to know)
> > > 
> > > and a set of optional fields that are to used for hotplugging
> > > a CPU objects and would allows mgmt tools to know what/where
> > > it could be hotplugged;
> > > [node],[socket],[core],[thread]
> > > 
> > > For present CPUs there is a 'qom-path' field which
> > > would allow mgmt to inspect whatever object/abstraction
> > > the target platform considers as CPU object.
> > > 
> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > ---
> > > v6:
> > >  - fix style issues in qapi-schema and qmp-commands,
> > >    Eric Blake <eblake@redhat.com>
> > >  - rebase on top current master (query-gic-capabilities conflict)
> > > v5:
> > >  - fix s390 build failure:
> > >     undefined reference to `qmp_query_hotpluggable_cpus'
> > > v4:
> > >  - add MachineClass method to get CPU object list
> > > v3:
> > >  - add 'vcpus-count' field, pkrempa@redhat.com
> > >  - s/CpuInstanceProps/CpuInstanceProperties/
> > >  - use '#optional' marker
> > >  - make "props" as always present even if it's empty
> > >  - fix JSON examples
> > >  - fix minor typos
> > > 
> > > query_fixup
> > > ---
> > >  include/hw/boards.h |  5 +++++
> > >  monitor.c           | 13 +++++++++++++
> > >  qapi-schema.json    | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> > >  qmp-commands.hx     | 41 +++++++++++++++++++++++++++++++++++++++++
> > >  4 files changed, 105 insertions(+)
> > > 
> > > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > > index aad5f2a..c122a70 100644
> > > --- a/include/hw/boards.h
> > > +++ b/include/hw/boards.h
> > > @@ -81,6 +81,10 @@ typedef struct {
> > >   *    Returns an array of @CPUArchId architecture-dependent CPU IDs
> > >   *    which includes CPU IDs for present and possible to hotplug CPUs.
> > >   *    Caller is responsible for freeing returned list.
> > > + * @query_hotpluggable_cpus:
> > > + *    Returns a @HotpluggableCPUList, which describes CPUs objects which
> > > + *    could be added with -device/device_add.
> > > + *    Caller is responsible for freeing returned list.
> > >   */
> > >  struct MachineClass {
> > >      /*< private >*/
> > > @@ -123,6 +127,7 @@ struct MachineClass {
> > >                                             DeviceState *dev);
> > >      unsigned (*cpu_index_to_socket_id)(unsigned cpu_index);
> > >      CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
> > > +    HotpluggableCPUList *(*query_hotpluggable_cpus)(MachineState *machine);
> > >  };
> > >  
> > >  /**
> > > diff --git a/monitor.c b/monitor.c
> > > index d1c1930..b469225 100644
> > > --- a/monitor.c
> > > +++ b/monitor.c
> > > @@ -4267,3 +4267,16 @@ GICCapabilityList *qmp_query_gic_capabilities(Error **errp)
> > >      return NULL;
> > >  }
> > >  #endif
> > > +
> > > +HotpluggableCPUList *qmp_query_hotpluggable_cpus(Error **errp)
> > > +{
> > > +    MachineState *ms = MACHINE(qdev_get_machine());
> > > +    MachineClass *mc = MACHINE_GET_CLASS(ms);
> > > +
> > > +    if (!mc->query_hotpluggable_cpus) {
> > > +        error_setg(errp, QERR_FEATURE_DISABLED, "query-hotpluggable-cpus");
> > > +        return NULL;
> > > +    }
> > > +
> > > +    return mc->query_hotpluggable_cpus(ms);
> > > +}
> > > diff --git a/qapi-schema.json b/qapi-schema.json
> > > index 54634c4..4d1d71d 100644
> > > --- a/qapi-schema.json
> > > +++ b/qapi-schema.json
> > > @@ -4178,3 +4178,49 @@
> > >  # Since: 2.6
> > >  ##
> > >  { 'command': 'query-gic-capabilities', 'returns': ['GICCapability'] }
> > > +
> > > +##
> > > +# CpuInstanceProperties
> > > +#
> > > +# @node: #optional NUMA node ID the CPU belongs to
> > > +# @socket: #optional socket number within node/board the CPU belongs to
> > > +# @core: #optional core number within socket the CPU belongs to
> > > +# @thread: #optional thread number within core the CPU belongs to
> > > +#
> > > +# Since: 2.7
> > > +##
> > > +{ 'struct': 'CpuInstanceProperties',
> > > +  'data': { '*node': 'int',
> > > +            '*socket': 'int',
> > > +            '*core': 'int',
> > > +            '*thread': 'int'
> > > +  }
> > > +}    
> > 
> > Is there somewhere we should document the fact that these particular
> > properties are common ones, but there could be more.  The point is that
> > management should not assume these are the only fields here, but should
> > be prepared to accept anything, and echo it back to the device_add.  
> 
> Something along these lines?

Something like that, yes.

> +##
> +# CpuInstanceProperties
> +#
> +# List of properties to be used for hotplugging a CPU instance,
> +# it should be passed by management with device_add command when
> +# a CPU is being hotplugged.
> +#
> +# Note: currently there are 4 properties that could be present
> +# but management should be prepared to pass through other
> +# properties with device_add command to allow for future
> +# interface extension.
>  
> > > +
> > > +##
> > > +# @HotpluggableCPU
> > > +#
> > > +# @type: CPU object type for usage with device_add command
> > > +# @props: list of properties to be used for hotplugging CPU
> > > +# @vcpus-count: number of logical VCPU threads @HotpluggableCPU provides
> > > +# @qom-path: #optional link to existing CPU object if CPU is present or
> > > +#            omitted if CPU is not present.
> > > +#
> > > +# Since: 2.7
> > > +##
> > > +{ 'struct': 'HotpluggableCPU',
> > > +  'data': { 'type': 'str',
> > > +            'vcpus-count': 'int',
> > > +            'props': 'CpuInstanceProperties',
> > > +            '*qom-path': 'str'
> > > +          }
> > > +}
> > > +
> > > +##
> > > +# @query-hotpluggable-cpus
> > > +#
> > > +# Returns: a list of HotpluggableCPU objects.
> > > +#
> > > +# Since: 2.7
> > > +##
> > > +{ 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] }
> > > diff --git a/qmp-commands.hx b/qmp-commands.hx
> > > index de896a5..96f4454 100644
> > > --- a/qmp-commands.hx
> > > +++ b/qmp-commands.hx
> > > @@ -4880,3 +4880,44 @@ Example:
> > >                  { "version": 3, "emulated": false, "kernel": true } ] }
> > >  
> > >  EQMP
> > > +
> > > +    {
> > > +        .name       = "query-hotpluggable-cpus",
> > > +        .args_type  = "",
> > > +        .mhandler.cmd_new = qmp_marshal_query_hotpluggable_cpus,
> > > +    },
> > > +
> > > +SQMP
> > > +Show existing/possible CPUs
> > > +---------------------------
> > > +
> > > +Arguments: None.
> > > +
> > > +Example for x86 target started with -smp 2,sockets=2,cores=1,threads=3,maxcpus=6:
> > > +
> > > +-> { "execute": "query-hotpluggable-cpus" }
> > > +<- {"return": [
> > > +     { "props": { "core": 0, "socket": 1, "thread": 2},
> > > +       "type": "qemu64-x86_64-cpu", "vcpus-count": 1 },
> > > +     { "props": { "core": 0, "socket": 1, "thread": 1},
> > > +       "type": "qemu64-x86_64-cpu", "vcpus-count": 1 },
> > > +     { "props": { "core": 0, "socket": 1, "thread": 0},
> > > +       "type": "qemu64-x86_64-cpu", "vcpus-count": 1 },
> > > +     { "props": { "core": 0, "socket": 0, "thread": 2},
> > > +       "type": "qemu64-x86_64-cpu", "vcpus-count": 1 },
> > > +     { "props": { "core": 0, "socket": 0, "thread": 1},
> > > +       "type": "qemu64-x86_64-cpu", "vcpus-count": 1,
> > > +       "qom-path": "/machine/unattached/device[3]"},
> > > +     { "props": { "core": 0, "socket": 0, "thread": 0},
> > > +       "type": "qemu64-x86_64-cpu", "vcpus-count": 1,
> > > +       "qom-path": "/machine/unattached/device[0]"}
> > > +   ]}'
> > > +
> > > +Example for SPAPR target started with -smp 2,cores=2,maxcpus=4:    
> > 
> > Might be best to say "pseries" here, since that's the exposed machine
> > type name, even though we use sPAPR a lot internally.  
> Di you mean something like this?
> 
> +Example for 'pseries' target started with -smp 2,cores=2,maxcpus=4: 

Perhaps "'pseries' machine type" explicitly, since "target" tends to
refer to ISA rather than board in qemu.

> 
> >   
> > > +
> > > +-> { "execute": "query-hotpluggable-cpus" }
> > > +<- {"return": [
> > > +     { "props": { "core": 1 }, "type": "spapr-cpu-core", "vcpus-count": 1 },
> > > +     { "props": { "core": 0 }, "type": "spapr-cpu-core", "vcpus-count": 1,
> > > +       "qom-path": "/machine/unattached/device[0]"}
> > > +   ]}'
> > > -- 
> > > 1.8.3.1
> > >     
> > 
> >   
> 


-- 
David Gibson <dgibson@redhat.com>
Senior Software Engineer, Virtualization, Red Hat

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