Luiz Capitulino <lcapitulino@xxxxxxxxxx> writes: > On Wed, 15 Dec 2010 11:49:23 +0100 > Markus Armbruster <armbru@xxxxxxxxxx> wrote: > >> Lai Jiangshan <laijs@xxxxxxxxxxxxxx> 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 <laijs@xxxxxxxxxxxxxx> >> >> 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. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
