[11/12] qmp: add cpu_set qmp command

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 include/sysemu/sysemu.h |    2 ++
 qapi-schema.json        |    9 +++++++++
 qmp-commands.hx         |   26 ++++++++++++++++++++++++++
 qmp.c                   |   12 ++++++++++++
 stubs/Makefile.objs     |    1 +
 stubs/do_cpu_hot_add.c  |    7 +++++++
 6 files changed, 57 insertions(+), 0 deletions(-)
 create mode 100644 stubs/do_cpu_hot_add.c

On 03/21/2013 08:28 AM, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> +++ b/qapi-schema.json
> @@ -1385,6 +1385,15 @@
>  { 'command': 'cpu', 'data': {'index': 'int'} }
>  
>  ##
> +# @cpu_set
> +#

I already mentioned naming this cpu-set on your cover letter.  Additionally:

> +# Sets specified cpu to online/ofline mode

s/ofline/offline/

> +#
> +# Notes: semantics is : cpu_set id=x status=online|offline
> +##
> +{ 'command': 'cpu_set', 'data': {'id': 'int', 'status': 'str'} }

Don't open-code a 'str'.  My preference would be a bool, with slightly
different naming.

Also, this is not the typical documentation style used in this file.
Rather, it would be more like:

# Sets specified cpu to online/offline mode
#
# @id: cpu id to be updated
#
# @online: true to put the cpu online, false to take it offline
#
##
{ 'command': 'cpu-set', 'data': {'id': 'int', 'online': 'bool'} }


> +void qmp_cpu_set(int64_t id, const char *status, Error **errp)
> +{
> +    if (!strcmp(status, "online")) {
> +        do_cpu_hot_add(id, errp);
> +    } else if (!strcmp(status, "offline")) {
> +        error_setg(errp, "Unplug is not implemented");
> +    } else {
> +        error_setg(errp, "Invalid parameter '%s'", status);
> +        return;

This return could be omitted, with no change in behavior.  But again, I
think a bool rather than an open-coded string parser would make this
simpler:

void qmp_cpu_set(int64_t id, bool online, Error **errp)
{
    if (online) {
        do_cpu_hot_add(id, errp);
    } else {
        error_setg(errp, "Unplug is not implemented");
    }
}

Il 21/03/2013 15:28, Igor Mammedov ha scritto:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  include/sysemu/sysemu.h |    2 ++
>  qapi-schema.json        |    9 +++++++++
>  qmp-commands.hx         |   26 ++++++++++++++++++++++++++
>  qmp.c                   |   12 ++++++++++++
>  stubs/Makefile.objs     |    1 +
>  stubs/do_cpu_hot_add.c  |    7 +++++++
>  6 files changed, 57 insertions(+), 0 deletions(-)
>  create mode 100644 stubs/do_cpu_hot_add.c
> 
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 4b8f721..8bcaf26 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -156,6 +156,8 @@ void drive_hot_add(Monitor *mon, const QDict *qdict);
>  void qemu_register_cpu_add_notifier(Notifier *notifier);
>  void qemu_system_cpu_hotplug_request(uint32_t id);
>  
> +void do_cpu_hot_add(const int64_t id, Error **errp);
> +
>  /* pcie aer error injection */
>  void pcie_aer_inject_error_print(Monitor *mon, const QObject *data);
>  int do_pcie_aer_inject_error(Monitor *mon,
> diff --git a/qapi-schema.json b/qapi-schema.json
> index fdaa9da..7acec6e 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1385,6 +1385,15 @@
>  { 'command': 'cpu', 'data': {'index': 'int'} }
>  
>  ##
> +# @cpu_set
> +#
> +# Sets specified cpu to online/ofline mode
> +#
> +# Notes: semantics is : cpu_set id=x status=online|offline
> +##
> +{ 'command': 'cpu_set', 'data': {'id': 'int', 'status': 'str'} }
> +
> +##
>  # @memsave:
>  #
>  # Save a portion of guest memory to a file.
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index b370060..283df76 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -385,6 +385,32 @@ Note: CPUs' indexes are obtained with the 'query-cpus' command.
>  EQMP
>  
>      {
> +        .name       = "cpu_set",
> +        .args_type  = "id:i,status:s",
> +        .mhandler.cmd_new = qmp_marshal_input_cpu_set,
> +    },
> +
> +SQMP
> +cpu_set
> +-------
> +
> +Sets virtual cpu to online/ofline state
> +
> +Arguments:
> +
> +- "id": virtual cpu id (json-int)
> +- "status": desired state of cpu, online/offline (json-string)
> +
> +Example:
> +
> +-> { "execute": "cpu_set",
> +             "arguments": { "id": 2,
> +                            "status": "online" } }
> +<- { "return": {} }
> +
> +EQMP
> +
> +    {
>          .name       = "memsave",
>          .args_type  = "val:l,size:i,filename:s,cpu:i?",
>          .mhandler.cmd_new = qmp_marshal_input_memsave,
> diff --git a/qmp.c b/qmp.c
> index 55b056b..5b84779 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -108,6 +108,18 @@ void qmp_cpu(int64_t index, Error **errp)
>      /* Just do nothing */
>  }
>  
> +void qmp_cpu_set(int64_t id, const char *status, Error **errp)
> +{
> +    if (!strcmp(status, "online")) {
> +        do_cpu_hot_add(id, errp);
> +    } else if (!strcmp(status, "offline")) {
> +        error_setg(errp, "Unplug is not implemented");
> +    } else {
> +        error_setg(errp, "Invalid parameter '%s'", status);
> +        return;
> +    }
> +}

Rather than adding stubs, what about the following:

1) put the QEMUMachineInitArgs in a global;

2) put a pointer to do_cpu_hot_add into the QEMUMachine;

3) call the callback from qmp_cpu_set if non-null, using the CPU model
from the QEMUMachineInitArgs

Paolo

>  #ifndef CONFIG_VNC
>  /* If VNC support is enabled, the "true" query-vnc command is
>     defined in the VNC subsystem */
> diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
> index 6a492f5..4154a2b 100644
> --- a/stubs/Makefile.objs
> +++ b/stubs/Makefile.objs
> @@ -26,3 +26,4 @@ stub-obj-$(CONFIG_WIN32) += fd-register.o
>  stub-obj-y += resume_vcpu.o
>  stub-obj-y += get_icc_bus.o
>  stub-obj-y += qemu_system_cpu_hotplug_request.o
> +stub-obj-y += do_cpu_hot_add.o
> diff --git a/stubs/do_cpu_hot_add.c b/stubs/do_cpu_hot_add.c
> new file mode 100644
> index 0000000..1f6d7a6
> --- /dev/null
> +++ b/stubs/do_cpu_hot_add.c
> @@ -0,0 +1,7 @@
> +#include "qapi/error.h"
> +#include "sysemu/sysemu.h"
> +
> +void do_cpu_hot_add(const int64_t id, Error **errp)
> +{
> +    error_setg(errp, "Not implemented");
> +}
>