[v2,2/2] qemu, qmp: convert do_inject_nmi() to QObject, QError

Convert do_inject_nmi() to QObject, QError, we need to use it(via libvirt).

changed from v1
Add document.
Add error handling when the cpu index is invalid.

Signed-off-by:  Lai Jiangshan <laijs@cn.fujitsu.com>
---

Lai Jiangshan <laijs@cn.fujitsu.com> writes:

> Convert do_inject_nmi() to QObject, QError, we need to use it(via libvirt).
>
> changed from v1
> Add document.
> Add error handling when the cpu index is invalid.
>
> Signed-off-by:  Lai Jiangshan <laijs@cn.fujitsu.com>
> ---
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 23024ba..f86d9fe 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -724,7 +724,8 @@ ETEXI
>          .args_type  = "cpu_index:i",
>          .params     = "cpu",
>          .help       = "inject an NMI on the given CPU",
> -        .mhandler.cmd = do_inject_nmi,
> +        .user_print = monitor_user_noop,
> +        .mhandler.cmd_new = do_inject_nmi,
>      },
>  #endif
>  STEXI
> diff --git a/monitor.c b/monitor.c
> index ec31eac..f375eb3 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2119,7 +2119,7 @@ static void do_wav_capture(Monitor *mon, const QDict *qdict)
>  #endif
>  
>  #if defined(TARGET_I386)
> -static void do_inject_nmi(Monitor *mon, const QDict *qdict)
> +static int do_inject_nmi(Monitor *mon, const QDict *qdict, QObject **ret_data)
>  {
>      CPUState *env;
>      int cpu_index = qdict_get_int(qdict, "cpu_index");
> @@ -2127,8 +2127,11 @@ static void do_inject_nmi(Monitor *mon, const QDict *qdict)
>      for (env = first_cpu; env != NULL; env = env->next_cpu)
>          if (env->cpu_index == cpu_index) {
>              cpu_interrupt(env, CPU_INTERRUPT_NMI);
> -            break;
> +            return 0;
>          }
> +
> +    qerror_report(QERR_INVALID_CPU_INDEX, cpu_index);
> +    return -1;

do_cpu_set() reports invalud index like this:

        qerror_report(QERR_INVALID_PARAMETER_VALUE, "index",
                      "a CPU number");

What about sticking to that?

[...]

On Fri, 10 Dec 2010 14:36:08 +0800
Lai Jiangshan <laijs@cn.fujitsu.com> wrote:

> +SQMP
> +inject_nmi
> +----------
> +
> +Inject an NMI on the given CPU (x86 only).
> +
> +Arguments:
> +
> +- "cpu_index": the index of the CPU to be injected NMI (json-int)
> +
> +Example:
> +
> +-> { "execute": "inject_nmi", "arguments": { "cpu_index": 0 } }
> +<- { "return": {} }
> +
> +EQMP
> +

Avi, Anthony, can you please review this? Do we expect some kind of ack from
the guest? Do we expect it respond in some way?

Also note that the current series defines only one error condition: invalid
cpu index. Can this fail in other ways?

On Fri, Dec 10, 2010 at 09:20:26AM -0200, Luiz Capitulino wrote:
> On Fri, 10 Dec 2010 14:36:08 +0800
> Lai Jiangshan <laijs@cn.fujitsu.com> wrote:
> 
> > +SQMP
> > +inject_nmi
> > +----------
> > +
> > +Inject an NMI on the given CPU (x86 only).
> > +
> > +Arguments:
> > +
> > +- "cpu_index": the index of the CPU to be injected NMI (json-int)
> > +
> > +Example:
> > +
> > +-> { "execute": "inject_nmi", "arguments": { "cpu_index": 0 } }
> > +<- { "return": {} }
> > +
> > +EQMP
> > +
> 
> Avi, Anthony, can you please review this? Do we expect some kind of ack from
> the guest? Do we expect it respond in some way?

Looks good to me. Don't except any response from the guest.

> Also note that the current series defines only one error condition: invalid
> cpu index. Can this fail in other ways?
> --

Not really. An NMI can be pending already (which means the current
command has no effect), but i don't see the need to report that.

On Mon, 20 Dec 2010 08:47:46 -0200
Marcelo Tosatti <mtosatti@redhat.com> wrote:

> On Fri, Dec 10, 2010 at 09:20:26AM -0200, Luiz Capitulino wrote:
> > On Fri, 10 Dec 2010 14:36:08 +0800
> > Lai Jiangshan <laijs@cn.fujitsu.com> wrote:
> > 
> > > +SQMP
> > > +inject_nmi
> > > +----------
> > > +
> > > +Inject an NMI on the given CPU (x86 only).
> > > +
> > > +Arguments:
> > > +
> > > +- "cpu_index": the index of the CPU to be injected NMI (json-int)
> > > +
> > > +Example:
> > > +
> > > +-> { "execute": "inject_nmi", "arguments": { "cpu_index": 0 } }
> > > +<- { "return": {} }
> > > +
> > > +EQMP
> > > +
> > 
> > Avi, Anthony, can you please review this? Do we expect some kind of ack from
> > the guest? Do we expect it respond in some way?
> 
> Looks good to me. Don't except any response from the guest.
> 
> > Also note that the current series defines only one error condition: invalid
> > cpu index. Can this fail in other ways?
> > --
> 
> Not really. An NMI can be pending already (which means the current
> command has no effect), but i don't see the need to report that.

Ok, thanks for the feedback Marcelo.

>