[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v3] qemu, qmp: convert do_inject_nmi() to QObjec

From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH v3] qemu, qmp: convert do_inject_nmi() to QObject, QError
Date: Wed, 15 Dec 2010 18:39:07 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux)

Luiz Capitulino <address@hidden> writes:

> On Wed, 15 Dec 2010 11:49:23 +0100
> Markus Armbruster <address@hidden> wrote:
>> Lai Jiangshan <address@hidden> 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.
>> >
>> > changed from v2
>> > use QERR_INVALID_PARAMETER_VALUE as Markus suggest.
>> >
>> > Signed-off-by:  Lai Jiangshan <address@hidden>
>> A note on commit messages:
>> The commit message should describe the current version of the patch.
>> Don't repeat the subject line in the body.
>> Patch history is very useful for review, but usually uninteresting once
>> the patch is committed.  Thus, it's best to put it after the "---"
>> separator.
>> Subsystem tags in the subject line are helpful.  But "qemu" doesn't
>> provide any information there :)
>> Regarding the patch:
>> The conversion looks good.
>> The new QMP command is called "inject_nmi", while the existing human
>> monitor command is called "nmi".  Luiz asked for this name change.  I
>> don't mind.  But should we rename the human monitor command for
>> consistency?
> I don't think so, we don't need (and maybe don't even want) naming parity
> between QMP and HMP. Remember that one of our mistakes was to couple the two.

Human "nmi" and QMP "inject_nmi" are identical commands, aren't they?
Giving the same things the same name isn't coupling :)

The mistake that matters here was adopting existing human commands for
QMP uncritically.

> Also, Avi asked for more descriptive names in QMP and I agree with him, I
> would even be in favor of calling it inject-non-maskable-interrupt.

I like inject_nmi better than nmi.  inject-non-maskable-interrupt is too
long even for QMP.

>> Regardless, the differing command name is worth mentioning in the commit
>> message.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]