Re: [Qemu-devel] [PATCH V6 3/4] qmp, nmi: convert do_inject_nmi() to QObject

[Anthony Liguori <anthony@codemonkey.ws>]

On 02/25/2011 03:54 AM, Markus Armbruster wrote:
> Anthony Liguori<aliguori@linux.vnet.ibm.com>  writes:
>
>    
>> On 02/24/2011 10:20 AM, Markus Armbruster wrote:
>>      
>>> Anthony Liguori<aliguori@linux.vnet.ibm.com>   writes:
>>>
>>>
>>>        
>>>> On 02/24/2011 02:33 AM, Markus Armbruster wrote:
>>>>
>>>>          
>>>>> Anthony Liguori<anthony@codemonkey.ws>    writes:
>>>>>            
> [...]
>    
>>>>>> Please describe all expected errors.
>>>>>>
>>>>>>
>>>>>>              
>>>>> Quoting qmp-commands.hx:
>>>>>
>>>>>        3. Errors, in special, are not documented. Applications should NOT check
>>>>>           for specific errors classes or data (it's strongly recommended to only
>>>>>           check for the "error" key)
>>>>>
>>>>> Indeed, not a single error is documented there.  This is intentional.
>>>>>
>>>>>
>>>>>            
>>>> Yeah, but we're not 0.14 anymore and for 0.15, we need to document
>>>> errors.  If you are suggesting I send a patch to remove that section,
>>>> I'm more than happy to.
>>>>
>>>>          
>>> Two separate issues here: 1. Are we ready to commit to the current
>>> design of errors, and 2. Is it fair to reject Lai's patch now because he
>>> doesn't document his errors.
>>>
>>> I'm not commenting on 1. here.
>>>
>>> Regarding 2.: rejecting a patch because it doesn't document an aspect
>>> that current master intentionally leaves undocumented is not how you
>>> treat contributors.  At least not if you want any other than certified
>>> masochists who enjoy pain, and professionals who get adequately
>>> compensated for it.
>>>
>>> Lead by example, not by fiat.
>>>
>>>        
>> http://repo.or.cz/w/qemu/aliguori.git/blob/refs/heads/glib:/qmp-schema.json
>>
>> I am in the process of documenting the errors of every command.  It's
>> a royal pain but I'm going to document everything we have right now.
>> It's actually the last bit of work I need to finish before sending
>> QAPI out.
>>
>> So for new commands being added, it would be hugely helpful for the
>> authors to document the errors such that I don't have to reverse
>> engineer all of the possible error conditions.
>>      
> The moment this lands in master, you can begin to demand error
> descriptions from contributors.  Until then, I'll NAK error descriptions
> in qmp-commands.txt.  We left them undocumented there for good reasons:
>    

No, it was always a bad reason.  Good documentation is necessary to 
build good commands.  Errors are a huge part of the semantics of a 
command.  We cannot properly assess a command unless it's behavior in 
error conditions is well defined.

>>>>> Once we have an error design in place that has a reasonable hope to
>>>>> stand the test of time, and have errors documented for at least some of
>>>>> the commands here, we can start to require proper error documentation
>>>>> for new commands.  But not now.
>>>>>            
> I won't NAK non-normative error descriptions, say in commit messages, or
> in comments.  And I won't object to you asking for them.  But I feel you
> really shouldn't make it a condition for committing patches.  Especially
> not for simple patches that have been on list for months.
>    

I'm strongly committed to making QMP a first class interface in QEMU for 
0.15.  I feel documentation is a crucial part to making that happen.

I'm not asking for test cases even though that's something that we'll 
need for 0.15 because there's not enough infrastructure in master yet to 
do that in a reasonable way.  I realize I'm likely going to have to 
write that test case and I'm happy to do that.

But there's no reason that we shouldn't require thorough documentation 
for all new QMP commands moving forward including error semantics.  This 
is a critical part of having a first class API and no additional 
infrastructure is needed in master to do this.

Regards,

Anthony Liguori

> --
> 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
>