From mboxrd@z Thu Jan  1 00:00:00 1970
From: Anthony Liguori <anthony@codemonkey.ws>
Subject: Re: [Qemu-devel] [PATCH V6 3/4] qmp, nmi: convert do_inject_nmi()
 to QObject
Date: Wed, 23 Feb 2011 13:25:38 -0600
Message-ID: <4D655F32.30307@codemonkey.ws>
References: <4D412AD2.1090509@cn.fujitsu.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Cc: Luiz Capitulino <lcapitulino@redhat.com>,
	Markus Armbruster <armbru@redhat.com>, qemu-devel@nongnu.org,
	aliguori@us.ibm.com, kvm@vger.kernel.org,
	Avi Kivity <avi@redhat.com>, Lai Jiangshan <eag0628@gmail.com>
To: Lai Jiangshan <laijs@cn.fujitsu.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-vx0-f174.google.com ([209.85.220.174]:39921 "EHLO
	mail-vx0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S932363Ab1BWTZn (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 23 Feb 2011 14:25:43 -0500
Received: by vxi39 with SMTP id 39so3231178vxi.19
        for <kvm@vger.kernel.org>; Wed, 23 Feb 2011 11:25:42 -0800 (PST)
In-Reply-To: <4D412AD2.1090509@cn.fujitsu.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On 01/27/2011 02:20 AM, Lai Jiangshan wrote:
> Make we can inject NMI via qemu-monitor-protocol.
> We use "inject-nmi" for the qmp command name, the meaning is clearer.
>
> Signed-off-by:  Lai Jiangshan<laijs@cn.fujitsu.com>
> ---
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index ec1a4db..e763bf9 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -725,7 +725,8 @@ ETEXI
>           .params     = "[cpu]",
>           .help       = "Inject an NMI on all CPUs if no argument is given, "
>                         "otherwise inject it on the specified 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 387b020..1b1c0ba 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2542,7 +2542,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;
> @@ -2550,7 +2550,7 @@ static void do_inject_nmi(Monitor *mon, const QDict *qdict)
>       if (!qdict_haskey(qdict, "cpu-index")) {
>           for (env = first_cpu; env != NULL; env = env->next_cpu)
>               cpu_interrupt(env, CPU_INTERRUPT_NMI);
> -        return;
> +        return 0;
>       }
>
>       cpu_index = qdict_get_int(qdict, "cpu-index");
> @@ -2560,8 +2560,10 @@ static void do_inject_nmi(Monitor *mon, const QDict *qdict)
>                   kvm_inject_interrupt(env, CPU_INTERRUPT_NMI);
>               else
>                   cpu_interrupt(env, CPU_INTERRUPT_NMI);
> -            break;
> +            return 0;
>           }
> +
> +    return -1;
>   }
>   #endif
>
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 56c4d8b..a887dd5 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -429,6 +429,34 @@ Example:
>
>   EQMP
>
> +#if defined(TARGET_I386)
> +    {
> +        .name       = "inject-nmi",
> +        .args_type  = "cpu-index:i?",
> +        .params     = "[cpu]",
> +        .help       = "Inject an NMI on all CPUs if no argument is given, "
> +                      "otherwise inject it on the specified CPU",
> +        .user_print = monitor_user_noop,
> +        .mhandler.cmd_new = do_inject_nmi,
> +    },
> +#endif
> +SQMP
> +inject-nmi
> +----------
> +
> +Inject an NMI on all CPUs or the given CPU (x86 only).
> +
> +Arguments:
> +
> +- "cpu-index": the index of the CPU to be injected NMI (json-int, optional)
> +
> +Example:
> +
> +->  { "execute": "inject-nmi", "arguments": { "cpu-index": 0 } }
> +<- { "return": {} }
>    

Please describe all expected errors.  Don't hide this command for 
!defined(TARGET_I386), instead have it throw an error in the implementation.

Don't have commands that multiple behavior based on the presence or 
absence of arguments.  Make it take a list of cpus if you want the 
ability to inject the NMI to more than one CPU.

Regards,

Anthony Liguori

> +
> +EQMP
> +
>       {
>           .name       = "migrate",
>           .args_type  = "detach:-d,blk:-b,inc:-i,uri:s",
>
>