qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] qemu: fix configuring kvm probe when using --kerneldir
@ 2009-01-09 12:10 ehrhardt
  2009-01-09 14:36 ` [Qemu-devel] " Andre Przywara
  2009-01-09 20:05 ` Anthony Liguori
  0 siblings, 2 replies; 4+ messages in thread
From: ehrhardt @ 2009-01-09 12:10 UTC (permalink / raw)
  To: aliguori; +Cc: ehrhardt, qemu-devel, avi, kvm

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Qemu-devel] Re: [PATCH] qemu: fix configuring kvm probe when using --kerneldir
  2009-01-09 12:10 [Qemu-devel] [PATCH] qemu: fix configuring kvm probe when using --kerneldir ehrhardt
@ 2009-01-09 14:36 ` Andre Przywara
  2009-01-12  6:42   ` Christian Ehrhardt
  2009-01-09 20:05 ` Anthony Liguori
  1 sibling, 1 reply; 4+ messages in thread
From: Andre Przywara @ 2009-01-09 14:36 UTC (permalink / raw)
  To: ehrhardt; +Cc: aliguori, Avi Kivity, kvm, qemu-devel

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
-- 
Andre Przywara
AMD-OSRC (Dresden)
Tel: x84917

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Qemu-devel] Re: [PATCH] qemu: fix configuring kvm probe when using --kerneldir
  2009-01-09 12:10 [Qemu-devel] [PATCH] qemu: fix configuring kvm probe when using --kerneldir ehrhardt
  2009-01-09 14:36 ` [Qemu-devel] " Andre Przywara
@ 2009-01-09 20:05 ` Anthony Liguori
  1 sibling, 0 replies; 4+ messages in thread
From: Anthony Liguori @ 2009-01-09 20:05 UTC (permalink / raw)
  To: ehrhardt; +Cc: qemu-devel, avi, kvm

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
>   

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Qemu-devel] Re: [PATCH] qemu: fix configuring kvm probe when using --kerneldir
  2009-01-09 14:36 ` [Qemu-devel] " Andre Przywara
@ 2009-01-12  6:42   ` Christian Ehrhardt
  0 siblings, 0 replies; 4+ messages in thread
From: Christian Ehrhardt @ 2009-01-12  6:42 UTC (permalink / raw)
  To: Andre Przywara; +Cc: aliguori, Avi Kivity, kvm, qemu-devel

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2009-01-12  6:42 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-01-09 12:10 [Qemu-devel] [PATCH] qemu: fix configuring kvm probe when using --kerneldir ehrhardt
2009-01-09 14:36 ` [Qemu-devel] " Andre Przywara
2009-01-12  6:42   ` Christian Ehrhardt
2009-01-09 20:05 ` Anthony Liguori

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).