From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1M0s8P-00009Z-8J
	for qemu-devel@nongnu.org; Mon, 04 May 2009 02:58:41 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1M0s8K-00007U-BE
	for qemu-devel@nongnu.org; Mon, 04 May 2009 02:58:40 -0400
Received: from [199.232.76.173] (port=59247 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1M0s8J-00007R-8v
	for qemu-devel@nongnu.org; Mon, 04 May 2009 02:58:35 -0400
Received: from mx20.gnu.org ([199.232.41.8]:10991)
	by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32)
	(Exim 4.60) (envelope-from <avi@redhat.com>) id 1M0s8I-0003jq-Ok
	for qemu-devel@nongnu.org; Mon, 04 May 2009 02:58:34 -0400
Received: from mx2.redhat.com ([66.187.237.31])
	by mx20.gnu.org with esmtp (Exim 4.60)
	(envelope-from <avi@redhat.com>) id 1M0s8H-0003pl-M5
	for qemu-devel@nongnu.org; Mon, 04 May 2009 02:58:33 -0400
Message-ID: <49FE91F3.8070806@redhat.com>
Date: Mon, 04 May 2009 09:57:55 +0300
From: Avi Kivity <avi@redhat.com>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [PATCH] kvm: Add helpers for checking and requiring
	kvm extensions
References: <1241350448-2340-1-git-send-email-avi@redhat.com>
	<m38wldmkgu.fsf@neno.mitica>
In-Reply-To: <m38wldmkgu.fsf@neno.mitica>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Juan Quintela <quintela@trasno.org>
Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org

Juan Quintela wrote:
> Avi Kivity <avi@redhat.com> wrote:
>
> Hi
>
>   
>> diff --git a/kvm-all.c b/kvm-all.c
>> index 36659a9..1642a2a 100644
>> --- a/kvm-all.c
>> +++ b/kvm-all.c
>> @@ -64,6 +64,30 @@ struct KVMState
>>  
>>  static KVMState *kvm_state;
>>  
>> +int kvm_check_extension(int extension)
>> +{
>> +    int ret;
>> +
>> +    ret = kvm_ioctl(kvm_state, KVM_CHECK_EXTENSION, extension);
>> +    if (ret < 0) {
>> +        fprintf(stderr, "KVM_CHECK_EXTENSION failed: %s\n", strerror(errno));
>> +        exit(1);
>> +    }
>> +    return ret;
>> +}
>>
>>     
> Are you sure you want the exit(1) in this case?
>   

A negative result of kvm_ioctl() means the ioctl itself failed.  This 
shouldn't happen, so we exit.

If the extension is not present, it returns 0.

> With the exit() call, you are unable to check if one extension is
> present at all.  And you check the return of the following code.
>   
>>      s->coalesced_mmio = 0;
>>  #ifdef KVM_CAP_COALESCED_MMIO
>> -    ret = kvm_ioctl(s, KVM_CHECK_EXTENSION, KVM_CAP_COALESCED_MMIO);
>> -    if (ret > 0)
>> -        s->coalesced_mmio = ret;
>> +    s->coalesced_mmio = kvm_check_extension(KVM_CAP_COALESCED_MMIO);
>>  #endif
>>     
>
>   

Here we use the return value (0 or 1).

> You can remove the ifdef at this point.
>   

It won't compile if KVM_CAP_COLAESCED_MMIO is not defined.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.