From mboxrd@z Thu Jan  1 00:00:00 1970
From: Juan Quintela <quintela@redhat.com>
Subject: Re: [PATCH 3/4] kvm: Add kvm_has_pit_state2 helper
Date: Sun, 05 Feb 2012 21:03:49 +0100
Message-ID: <87d39tjbdm.fsf@elfo.elfo>
References: <cover.1328438750.git.jan.kiszka@web.de>
	<cover.1328438750.git.jan.kiszka@web.de>
	<6776cc5c00f4d06d23c11b68d18fd96e27f71718.1328438750.git.jan.kiszka@web.de>
Reply-To: quintela@redhat.com
Mime-Version: 1.0
Content-Type: text/plain
Cc: Avi Kivity <avi@redhat.com>, Marcelo Tosatti <mtosatti@redhat.com>,
	Anthony Liguori <aliguori@us.ibm.com>,
	qemu-devel <qemu-devel@nongnu.org>, kvm@vger.kernel.org
To: Jan Kiszka <jan.kiszka@web.de>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:62325 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1750763Ab2BEUDz (ORCPT <rfc822;kvm@vger.kernel.org>);
	Sun, 5 Feb 2012 15:03:55 -0500
In-Reply-To: <6776cc5c00f4d06d23c11b68d18fd96e27f71718.1328438750.git.jan.kiszka@web.de>
	(Jan Kiszka's message of "Sun, 5 Feb 2012 11:46:29 +0100")
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Jan Kiszka <jan.kiszka@web.de> wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> 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.

And then went to kvm-all.c

int kvm_check_extension(KVMState *s, unsigned int extension)
{
    int ret;

    ret = kvm_ioctl(s, KVM_CHECK_EXTENSION, extension);
    if (ret < 0) {
        ret = 0;
    }

    return ret;
}

What? it allways return zero?  Something should be wrong here.  I will
expect kvm_check_extension() to work by now.

Yes, kvm_ioctl() return 1 went the extension is there, just for the
people confused like me.

Later, Juan.