From mboxrd@z Thu Jan  1 00:00:00 1970
From: Juan Quintela <quintela@trasno.org>
Subject: Re: [PATCH] kvm: Add helpers for checking and requiring
	kvm extensions
Date: Mon, 04 May 2009 08:49:05 +0200
Message-ID: <m38wldmkgu.fsf@neno.mitica>
References: <1241350448-2340-1-git-send-email-avi@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org
To: Avi Kivity <avi@redhat.com>
Return-path: <qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org>
In-Reply-To: <1241350448-2340-1-git-send-email-avi@redhat.com> (Avi Kivity's
	message of "Sun\, 3 May 2009 14\:34\:08 +0300")
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>
Sender: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org
Errors-To: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org
List-Id: kvm.vger.kernel.org

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

You can remove the ifdef at this point.

Later, Juan.