Message ID | 4D58FF49.5090503@cn.fujitsu.com |
---|---|
State | New |
Headers | show |
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 >
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 > > >
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
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) {
"cpu-index" which uses hyphen is better name. Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com> ---