Re: [PATCH 2/2] qemu-kvm: x86: Add support for VCPU event states

[Avi Kivity <avi@redhat.com>]

On 11/15/2009 04:31 PM, Jan Kiszka wrote:
>>>>
>>>> Is there a reason why you add 11 between 9 and 10?  We'll probably see
>>>> another 11 when someone else adds the next state.
>>>>
>>>>
>>>>          
>>> Logical grouping ("/* KVM-related states */").
>>>        
>> These aren't kvm-related, just not implemented in tcg yet.  Nothing
>> kvmish about them - it's all architectural state.
>>      
> Most of them are KVM-specific. TCG don't have to deal with event
> re-injection due to host page faults etc. on first try.
>    

Right, tcg can do some things atomically.  There's some non-kvm state in 
there.

>>> If anyone once tries to
>>> add non-KVM stuff here just because it's version 12, it should be
>>> rejected. I don't think you have to sort VMSTATE entries by their
>>> version number. Am I right, Juan?
>>>
>>>        
>> I'm worried about something else - someone looking at the end, seeing
>> version 10, and appending new state with version 11.
>>      
> Again, that's something proper review should catch (just like checking
> for the right place when adding a new IOCTL to kvm.h).
>    

And we know how well that works.  We should try to make things work by 
default and leave the review to catch the really difficult things (like 
trailing whitespace).

At least add a comment at the end warning people that simple append will 
fail.

-- 
error compiling committee.c: too many arguments to function