qemu: fix configuring kvm probe when using --kerneldir
diff mbox

Message ID 1231503054-10618-1-git-send-email-ehrhardt@linux.vnet.ibm.com
State Accepted, archived
Headers show

Commit Message

ehrhardt@linux.vnet.ibm.com Jan. 9, 2009, 12:10 p.m. UTC
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.

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.

Signed-off-by: Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>
---

[diffstat]
 configure |    6 ++++++
 1 file changed, 6 insertions(+)

[diff]
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Andre Przywara Jan. 9, 2009, 2:36 p.m. UTC | #1
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.

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

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
Anthony Liguori Jan. 9, 2009, 8:05 p.m. UTC | #2
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.
>
> 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.
>
> Signed-off-by: Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>
>   

Applied.  Thanks.

Regards,

Anthony Liguori

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

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
ehrhardt@linux.vnet.ibm.com Jan. 12, 2009, 6:42 a.m. UTC | #3
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

Patch
diff mbox

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