[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.
- [Qemu-devel] [PATCH v3] qemu, qmp: convert do_inject_nmi() to QObject, QError, Lai Jiangshan, 2010/12/15
- [Qemu-devel] Re: [PATCH v3] qemu, qmp: convert do_inject_nmi() to QObject, QError, Luiz Capitulino, 2010/12/15
- [Qemu-devel] Re: [PATCH v3] qemu, qmp: convert do_inject_nmi() to QObject, QError, Avi Kivity, 2010/12/15
- [Qemu-devel] Re: [PATCH v3] qemu, qmp: convert do_inject_nmi() to QObject, QError, Luiz Capitulino, 2010/12/15
- [Qemu-devel] Re: [PATCH v3] qemu, qmp: convert do_inject_nmi() to QObject, QError, Markus Armbruster, 2010/12/15
- [Qemu-devel] Re: [PATCH v3] qemu, qmp: convert do_inject_nmi() to QObject, QError, Luiz Capitulino, 2010/12/15
- [Qemu-devel] Re: [PATCH v3] qemu, qmp: convert do_inject_nmi() to QObject, QError, Avi Kivity, 2010/12/16
- [Qemu-devel] Re: [PATCH v3] qemu, qmp: convert do_inject_nmi() to QObject, QError, Luiz Capitulino, 2010/12/16
- [Qemu-devel] Re: [PATCH v3] qemu, qmp: convert do_inject_nmi() to QObject, QError, Avi Kivity, 2010/12/16
- [Qemu-devel] Re: [PATCH v3] qemu, qmp: convert do_inject_nmi() to QObject, QError, Luiz Capitulino, 2010/12/16