Re: [PATCH] qemu: fix configuring kvm probe when using --kerneldir

From: Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>
To: Andre Przywara <andre.przywara@amd.com>
Cc: aliguori@us.ibm.com, Avi Kivity <avi@redhat.com>,
	qemu-devel@nongnu.org, kvm@vger.kernel.org
Subject: Re: [PATCH] qemu: fix configuring kvm probe when using --kerneldir
Date: Mon, 12 Jan 2009 07:42:34 +0100	[thread overview]
Message-ID: <496AE65A.1000701@linux.vnet.ibm.com> (raw)
In-Reply-To: <496760F3.7020902@amd.com>

Andre Przywara wrote:
> ehrhardt@linux.vnet.ibm.com wrote:
>> From: Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>
>>
>> There is already a variable kvm_cflags which gets the path of the kernel
>> includes when using --kerneldir. But eventually with newer kernels we 
>> all will
>> need arch/$arch/include too (my case was a incldue of asm/kvm.h which 
>> was not
>> found anymore). Headers in a full kernel source are not flattened to
>> one arch like they are if e.g. installed kernel headers are used.
> I also stumbled over this recently (in kvm-userspace.git), but I had 
> problems with the qemu part not including KVM support because in 
> qemu/configure the KVM build test failed due to the missing asm/kvm.h.
> I saw that --kerneldir gets not propagated to qemu, but 
> libkvm_kerneldir instead, which is hardcoded to point to `pwd`/kernel. 
> Shouldn't that be fixed, too?
> I use kvm-userspace.git and a not-installed kernel from kvm.git for 
> compiling, so I say "./configure --kerneldir=/src/kvm.git 
> --with-patched-kernel". I eventually hacked KVM's configure to 
> propagate --kerneldir to qemu and added arch/x86/include to the 
> include path in qemu/configure. This is of course a hack (that's why I 
> don't append it here), but it worked ;-)
> If someone proposes a clean and easy way to solve this, I'd be happy 
> to write a patch.

I know this issue and reported it ~a month ago. I also had issues 
compiling against a --kerneldir kernel because the libkvm_kerneldir was 
propagated. Eventually in the discussion it came up that we don't need 
to fix configure "technically", but maybe we should find a way to better 
inform users/developüers about this (I guess up to 99% that this works 
for you in kvm-userspace too):

(in a clean kvm-userspace)
  cd kernel
  make sync LINUX=path/to/your/kerneldir
  cd ..
  ./configure opt=whateveryouwant

This way your kerneldir is synced and flattened into kvm-userspace and 
propagating libkvm_kerneldir is fine since that are your kerneldir 
headers now.
Maybe a "fix" would be that if --kerneldir is provided to configure it 
has to ensure that THIS kerneldir is synced in before continuing.
You should be aware that the fix I sent on Friday was for plain qemu 
which doesn't have that kernel subdir indirection and therefore works a 
bit different.

>> To fix that, the includes added to cflags depending on --kerneldir 
>> should also
>> contian the arch includes. The patch adds a special check for x86 
>> because its
>> source layout recently changed, all others directly use 
>> arch/$cpu/include if
>> existent.
> This is one problem I also noticed. $cpu is not the same as the Linux' 
> arch name, is there a suitable variable or do we have to do a large 
> switch/case?

I looked around and there was no real 1:1 matching variable. But 
fortunately $cpu is similar enough to simplify that swicth/case a lot 
like I did in my patch here.

> Regards,
> Andre.
>
>
>>
>> Signed-off-by: Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>
>> ---
>>
>> [diffstat]
>>  configure |    6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> [diff]
>> diff --git a/configure b/configure
>> --- a/configure
>> +++ b/configure
>> @@ -963,6 +963,12 @@ EOF
>>  EOF
>>    if test "$kerneldir" != "" ; then
>>        kvm_cflags=-I"$kerneldir"/include
>> +      if test \( "$cpu" = "i386" -o "$cpu" = "x86_64" \) \
>> +         -a -d "$kerneldir/arch/x86/include" ; then
>> +            kvm_cflags="$kvm_cflags -I$kerneldir/arch/x86/include"
>> +        elif test -d "$kerneldir/arch/$cpu/include" ; then
>> +            kvm_cflags="$kvm_cflags -I$kerneldir/arch/$cpu/include"
>> +      fi
>>    else
>>        kvm_cflags=""
>>    fi

-- 

Grüsse / regards, 
Christian Ehrhardt
IBM Linux Technology Center, Open Virtualization