[Qemu-devel] [PATCH V6 1/4 resend] nmi: convert cpu_index to cpu-index

[Qemu-devel] [PATCH V6 1/4 resend] nmi: convert cpu_index to cpu-index

[Lai Jiangshan]

"cpu-index" which uses hyphen is better name.

Signed-off-by:  Lai Jiangshan <laijs@cn.fujitsu.com>
---
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 5d4cb9e..e43ac7c 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -721,7 +721,7 @@ ETEXI
 #if defined(TARGET_I386)
     {
         .name       = "nmi",
-        .args_type  = "cpu_index:i",
+        .args_type  = "cpu-index:i",
         .params     = "cpu",
         .help       = "inject an NMI on the given CPU",
         .mhandler.cmd = do_inject_nmi,
diff --git a/monitor.c b/monitor.c
index 27883f8..a916771 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2545,7 +2545,7 @@ static void do_wav_capture(Monitor *mon, const QDict *qdict)
 static void do_inject_nmi(Monitor *mon, const QDict *qdict)
 {
     CPUState *env;
-    int cpu_index = qdict_get_int(qdict, "cpu_index");
+    int cpu_index = qdict_get_int(qdict, "cpu-index");
 
     for (env = first_cpu; env != NULL; env = env->next_cpu)
         if (env->cpu_index == cpu_index) {

[Qemu-devel] Re: [PATCH V6 1/4 resend] nmi: convert cpu_index to cpu-index

[Lai Jiangshan]

Hi, Luiz Capitulino

Any problem?

Thanks,
Lai

On 02/14/2011 06:09 PM, Lai Jiangshan wrote:
> "cpu-index" which uses hyphen is better name.
> 
> Signed-off-by:  Lai Jiangshan <laijs@cn.fujitsu.com>
> ---
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 5d4cb9e..e43ac7c 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -721,7 +721,7 @@ ETEXI
>  #if defined(TARGET_I386)
>      {
>          .name       = "nmi",
> -        .args_type  = "cpu_index:i",
> +        .args_type  = "cpu-index:i",
>          .params     = "cpu",
>          .help       = "inject an NMI on the given CPU",
>          .mhandler.cmd = do_inject_nmi,
> diff --git a/monitor.c b/monitor.c
> index 27883f8..a916771 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2545,7 +2545,7 @@ static void do_wav_capture(Monitor *mon, const QDict *qdict)
>  static void do_inject_nmi(Monitor *mon, const QDict *qdict)
>  {
>      CPUState *env;
> -    int cpu_index = qdict_get_int(qdict, "cpu_index");
> +    int cpu_index = qdict_get_int(qdict, "cpu-index");
>  
>      for (env = first_cpu; env != NULL; env = env->next_cpu)
>          if (env->cpu_index == cpu_index) {
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[Qemu-devel] Re: [PATCH V6 1/4 resend] nmi: convert cpu_index to cpu-index

[Luiz Capitulino]

On Mon, 21 Feb 2011 09:37:57 +0800
Lai Jiangshan <laijs@cn.fujitsu.com> wrote:

> Hi, Luiz Capitulino
> 
> Any problem?

Sorry for the delay. Looks good in general to me know, there's only one
small problem and it's the error message:

  (qemu) nmi 100
  Parameter 'cpu-index' expects a CPU number
  (qemu) 

I would expect that kind of error message when no CPU number is
provided, but in the case above the CPU number is provided but it
happens to be invalid. Why?

By the way, please add an introductory email with proper changelog
when submitting series/patches, so that it's easier to review.

> 
> Thanks,
> Lai
> 
> On 02/14/2011 06:09 PM, Lai Jiangshan wrote:
> > "cpu-index" which uses hyphen is better name.
> > 
> > Signed-off-by:  Lai Jiangshan <laijs@cn.fujitsu.com>
> > ---
> > diff --git a/hmp-commands.hx b/hmp-commands.hx
> > index 5d4cb9e..e43ac7c 100644
> > --- a/hmp-commands.hx
> > +++ b/hmp-commands.hx
> > @@ -721,7 +721,7 @@ ETEXI
> >  #if defined(TARGET_I386)
> >      {
> >          .name       = "nmi",
> > -        .args_type  = "cpu_index:i",
> > +        .args_type  = "cpu-index:i",
> >          .params     = "cpu",
> >          .help       = "inject an NMI on the given CPU",
> >          .mhandler.cmd = do_inject_nmi,
> > diff --git a/monitor.c b/monitor.c
> > index 27883f8..a916771 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -2545,7 +2545,7 @@ static void do_wav_capture(Monitor *mon, const QDict *qdict)
> >  static void do_inject_nmi(Monitor *mon, const QDict *qdict)
> >  {
> >      CPUState *env;
> > -    int cpu_index = qdict_get_int(qdict, "cpu_index");
> > +    int cpu_index = qdict_get_int(qdict, "cpu-index");
> >  
> >      for (env = first_cpu; env != NULL; env = env->next_cpu)
> >          if (env->cpu_index == cpu_index) {
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 

Re: [Qemu-devel] Re: [PATCH V6 1/4 resend] nmi: convert cpu_index to cpu-index

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Mon, 21 Feb 2011 09:37:57 +0800
> Lai Jiangshan <laijs@cn.fujitsu.com> wrote:
>
>> Hi, Luiz Capitulino
>> 
>> Any problem?
>
> Sorry for the delay. Looks good in general to me know, there's only one
> small problem and it's the error message:
>
>   (qemu) nmi 100
>   Parameter 'cpu-index' expects a CPU number
>   (qemu) 
>
> I would expect that kind of error message when no CPU number is
> provided, but in the case above the CPU number is provided but it
> happens to be invalid. Why?

This is not Lai Jiangshan's fault.  It's what
QERR_INVALID_PARAMETER_VALUE reports.  The current design of QError
makes it hard to do better.

"expects a valid CPU number" could be done, if you think that's better.

> By the way, please add an introductory email with proper changelog
> when submitting series/patches, so that it's easier to review.

Also make sure the parts are threaded together properly with In-Reply-To
and References headers, because that helps e-mail readers to keep the
parts together.  Lack of threading is annoying, and annoying reviewers
intentionally would be rude :)

Suggested workflow:

git-format-patch --cover-letter -ns ...
look over patch files, edit the cover letter to taste
git-send-email --to qemu-devel@nongnu.org 0*.patch