Re: [Qemu-devel] [PATCH 0/3]: QMP: Introduce inject-nmi command

[Markus Armbruster]

Luiz Capitulino <address@hidden> writes:

> On Fri, 27 May 2011 09:55:05 -0500
> Anthony Liguori <address@hidden> wrote:
>
>> On 05/27/2011 09:04 AM, Luiz Capitulino wrote:
>> > On Thu, 26 May 2011 22:23:10 +0300
>> > Blue Swirl<address@hidden>  wrote:
>> >
>> >> I think I explained it many times, but let's try again.

Thanks!

>> >>
>> >> inject
>> >> ----------
>> >>
>> >> Inject a signal on guest machine.
>> >>
>> >> Arguments: signal name.
>> >>
>> >> Example:
>> >>
>> >> ->  { "execute": "inject",
>> >> "arguments": { "signal": "nmi" } }
>> >> <- { "return": {} }
>> >>
>> >> ->  { "execute": "inject",
>> >> "arguments": { "signal": "/address@hidden:l1int" } }
>> >> <- { "return": {} }
>> >
>> > Shouldn't this be broken into device and signal (or pin) arguments?
>> 
>> 
>> I dislike this approach strongly.
>> 
>> Overloading verbs to have multiple meanings is a bad thing for QMP.  It 
>> means less type safety.  Think of a C interface:
>> 
>> inject_nmi() <- good
>> inject_nim() <- compile error
>> 
>> inject("nmi") <- good
>> inject("nim") <- runtime error

Not sure how much mileage we'll get out of static typing in QMP, but
overloading commands for dissimilar tasks is bad taste.  One ugly
bastard like "change" is enough.  Thus, this "inject" command should
always be limited to irq-like operands.

>> Not to mention that "inject" doesn't mean "raise and then lower a pin". 
>>   Inject means insert or put in.

Yes.  Like "inject a fault".

>> I'm not opposed to being able to have a way to raise/lower a qemu_irq, 
>> but (a) that's orthogonal to this operation (b) we should design that 
>> interface properly.  b means that we should be able to enumerate pins, 
>> raise and lower pins, and pulse pins.

Once qdev models pins nicely, a command to manipulate them shouldn't be
hard.

> So, would you be in favor of merging the current series as it stands
> currently and design this new interface as a new command?

I recommend to commit the sucker already.  It's simple, and it solves a
problem that's relevant enough for Lai to pursue it for *months*, and
starting over now is just not worth it.

[...]