Re: [Qemu-devel] [PATCH 3/4] kvm: Add kvm_has_pit_state2 helper

[Jan Kiszka]

On 2012-02-05 21:03, Juan Quintela wrote:
> Jan Kiszka <address@hidden> wrote:
>> From: Jan Kiszka <address@hidden>
>>
>> To be used for in-kernel PIT emulation.
> 
> ....
> 
>> +    int pit_state2;
> 
> This is used as a bool.
> 
>>      int xsave, xcrs;
>>      int many_ioeventfds;
>>      int irqchip_inject_ioctl;
>> @@ -954,6 +955,10 @@ int kvm_init(void)
>>      s->xcrs = kvm_check_extension(s, KVM_CAP_XCRS);
>>  #endif
>>  
>> +#ifdef KVM_CAP_PIT_STATE2
>> +    s->pit_state2 = kvm_check_extension(s, KVM_CAP_PIT_STATE2);
>> +#endif
>> +
> 
> [ this happened to me when I was reviewing this patch, but culprit is
> not this patch]
> 
> really kvm_check_extension() should also return a bool, but that is a
> bigger change that this patch series tend to introduce.
> 
> So, I went to "man ioctl"
> 
>> RETURN VALUE
>>        Usually, on success zero is returned.  A few ioctl() requests  use  
>> the
>>        return  value  as an output parameter and return a nonnegative value 
>> on
>>       success.  On error, -1 is returned, and errno is set appropriately.
> 
> Usually is the important word there.

Right, because the driver decides in the end what is returned.

In case of KVM_CHECK_EXTENSION, it is > 0 for supported. Sometimes it
returns an integer value that encode "how much" is supported, e.g.
KVM_CAP_IRQ_ROUTING provides the number of supported GSI routes.

That said, all those boolean caps could indeed be encoded as such. Some
smarter way to initialize and retrieve them would also be nice. Too much
boilerplate code.

Thanks for bending your brain around this :)

Jan

signature.asc
Description: OpenPGP digital signature