public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] kvm.h: __user requires compiler.h
@ 2008-03-12 17:10 Christian Borntraeger
  2008-03-18  6:50 ` Avi Kivity
  2008-03-21 20:30 ` Anthony Liguori
  0 siblings, 2 replies; 8+ messages in thread
From: Christian Borntraeger @ 2008-03-12 17:10 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm-devel

include/linux/kvm.h defines struct kvm_dirty_log to
	[...]
	union {
		void __user *dirty_bitmap; /* one bit per page */
		__u64 padding;
	};

__user requires compiler.h to compile. Currently, this works on x86
only coincidentally due to other include files. This patch makes 
kvm.h compile in all cases.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 include/linux/kvm.h |    1 +
 1 file changed, 1 insertion(+)

Index: kvm/include/linux/kvm.h
===================================================================
--- kvm.orig/include/linux/kvm.h
+++ kvm/include/linux/kvm.h
@@ -8,6 +8,7 @@
  */
 
 #include <asm/types.h>
+#include <linux/compiler.h>
 #include <linux/ioctl.h>
 #include <asm/kvm.h>
 



-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

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

* Re: [PATCH] kvm.h: __user requires compiler.h
  2008-03-12 17:10 [PATCH] kvm.h: __user requires compiler.h Christian Borntraeger
@ 2008-03-18  6:50 ` Avi Kivity
  2008-03-21 20:30 ` Anthony Liguori
  1 sibling, 0 replies; 8+ messages in thread
From: Avi Kivity @ 2008-03-18  6:50 UTC (permalink / raw)
  To: Christian Borntraeger; +Cc: kvm-devel

Christian Borntraeger wrote:
> include/linux/kvm.h defines struct kvm_dirty_log to
> 	[...]
> 	union {
> 		void __user *dirty_bitmap; /* one bit per page */
> 		__u64 padding;
> 	};
>
> __user requires compiler.h to compile. Currently, this works on x86
> only coincidentally due to other include files. This patch makes 
> kvm.h compile in all cases.
>   

Applied, thanks.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

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

* Re: [PATCH] kvm.h: __user requires compiler.h
  2008-03-12 17:10 [PATCH] kvm.h: __user requires compiler.h Christian Borntraeger
  2008-03-18  6:50 ` Avi Kivity
@ 2008-03-21 20:30 ` Anthony Liguori
  2008-03-23  8:42   ` Avi Kivity
  2008-03-24 19:46   ` Christian Borntraeger
  1 sibling, 2 replies; 8+ messages in thread
From: Anthony Liguori @ 2008-03-21 20:30 UTC (permalink / raw)
  To: Christian Borntraeger; +Cc: kvm-devel, Avi Kivity

This patch breaks QEMU build when doing a 'make sync'.  When you do a 
top-level ./configure, libkvm is built with kerneldir pointing to 
kvm-userspace/kernel/include.  While linux/kvm.h is present there, there 
isn't a linux/compiler.h.

The host kernelpath isn't normally part of the libkvm or QEMU build.  So 
we have a couple options.

1) make the host kernelpath (/lib/modules/$(uname -r)/build/include) 
part of the libkvm/QEMU build.

2) Do something else about __user

Suggestions?  #1 might be a pain since there may be include conflicts 
between the host kernel include and kernel/include.

Regards,

Anthony Liguori

Christian Borntraeger wrote:
> include/linux/kvm.h defines struct kvm_dirty_log to
> 	[...]
> 	union {
> 		void __user *dirty_bitmap; /* one bit per page */
> 		__u64 padding;
> 	};
> 
> __user requires compiler.h to compile. Currently, this works on x86
> only coincidentally due to other include files. This patch makes 
> kvm.h compile in all cases.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  include/linux/kvm.h |    1 +
>  1 file changed, 1 insertion(+)
> 
> Index: kvm/include/linux/kvm.h
> ===================================================================
> --- kvm.orig/include/linux/kvm.h
> +++ kvm/include/linux/kvm.h
> @@ -8,6 +8,7 @@
>   */
>  
>  #include <asm/types.h>
> +#include <linux/compiler.h>
>  #include <linux/ioctl.h>
>  #include <asm/kvm.h>
>  
> 
> 
> 
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Microsoft
> Defy all challenges. Microsoft(R) Visual Studio 2008.
> http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

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

* Re: [PATCH] kvm.h: __user requires compiler.h
  2008-03-21 20:30 ` Anthony Liguori
@ 2008-03-23  8:42   ` Avi Kivity
  2008-03-24 19:46   ` Christian Borntraeger
  1 sibling, 0 replies; 8+ messages in thread
From: Avi Kivity @ 2008-03-23  8:42 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: kvm-devel, Christian Borntraeger

Anthony Liguori wrote:
> This patch breaks QEMU build when doing a 'make sync'.  When you do a 
> top-level ./configure, libkvm is built with kerneldir pointing to 
> kvm-userspace/kernel/include.  While linux/kvm.h is present there, 
> there isn't a linux/compiler.h.
>
> The host kernelpath isn't normally part of the libkvm or QEMU build.  
> So we have a couple options.
>
> 1) make the host kernelpath (/lib/modules/$(uname -r)/build/include) 
> part of the libkvm/QEMU build.
>
> 2) Do something else about __user
>
> Suggestions?  #1 might be a pain since there may be include conflicts 
> between the host kernel include and kernel/include.
>

We could hack 'make sync' to strip out __user (just like we run 
unifdef).  Of course the reasons for including linux/compiler.h are 
still valid, so it needs to remain.

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

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

* Re: [PATCH] kvm.h: __user requires compiler.h
  2008-03-21 20:30 ` Anthony Liguori
  2008-03-23  8:42   ` Avi Kivity
@ 2008-03-24 19:46   ` Christian Borntraeger
  2008-03-24 20:04     ` Avi Kivity
  1 sibling, 1 reply; 8+ messages in thread
From: Christian Borntraeger @ 2008-03-24 19:46 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: kvm-devel, Avi Kivity

Am Freitag, 21. März 2008 schrieb Anthony Liguori:
> This patch breaks QEMU build when doing a 'make sync'.  When you do a 
> top-level ./configure, libkvm is built with kerneldir pointing to 
> kvm-userspace/kernel/include.  While linux/kvm.h is present there, there 
> isn't a linux/compiler.h.

I checked with make headers_install, which strips out __user and compiler.h 
nicely. I never tried "make sync" since I was not aware of it.

Christian

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

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

* Re: [PATCH] kvm.h: __user requires compiler.h
  2008-03-24 19:46   ` Christian Borntraeger
@ 2008-03-24 20:04     ` Avi Kivity
  2008-03-25 15:42       ` Christian Borntraeger
  0 siblings, 1 reply; 8+ messages in thread
From: Avi Kivity @ 2008-03-24 20:04 UTC (permalink / raw)
  To: Christian Borntraeger; +Cc: kvm-devel

Christian Borntraeger wrote:
> Am Freitag, 21. März 2008 schrieb Anthony Liguori:
>   
>> This patch breaks QEMU build when doing a 'make sync'.  When you do a 
>> top-level ./configure, libkvm is built with kerneldir pointing to 
>> kvm-userspace/kernel/include.  While linux/kvm.h is present there, there 
>> isn't a linux/compiler.h.
>>     
>
> I checked with make headers_install, which strips out __user and compiler.h 
> nicely. I never tried "make sync" since I was not aware of it.
>   

Maybe we should generate the 'make sync' headers using 'make 
headers_install'.

-- 
Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

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

* Re: [PATCH] kvm.h: __user requires compiler.h
  2008-03-24 20:04     ` Avi Kivity
@ 2008-03-25 15:42       ` Christian Borntraeger
  2008-03-25 16:23         ` Avi Kivity
  0 siblings, 1 reply; 8+ messages in thread
From: Christian Borntraeger @ 2008-03-25 15:42 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm-devel

Am Montag, 24. März 2008 schrieb Avi Kivity:
> Christian Borntraeger wrote:
> > Am Freitag, 21. März 2008 schrieb Anthony Liguori:
> >   
> >> This patch breaks QEMU build when doing a 'make sync'.  When you do a 
> >> top-level ./configure, libkvm is built with kerneldir pointing to 
> >> kvm-userspace/kernel/include.  While linux/kvm.h is present there,
[...] 
> Maybe we should generate the 'make sync' headers using 'make 
> headers_install'.

headers_install works because there is 

# Eliminate the contents of (and inclusions of) compiler.h
HDRSED  := sed  -e "s/ inline / __inline__ /g" \
                -e "s/[[:space:]]__user[[:space:]]\{1,\}/ /g" \
                -e "s/(__user[[:space:]]\{1,\}/ (/g" \
                -e "s/[[:space:]]__force[[:space:]]\{1,\}/ /g" \
                -e "s/(__force[[:space:]]\{1,\}/ (/g" \
                -e "s/[[:space:]]__iomem[[:space:]]\{1,\}/ /g" \
                -e "s/(__iomem[[:space:]]\{1,\}/ (/g" \
                -e "s/[[:space:]]__attribute_const__[[:space:]]\{1,\}/\ /g" 
\
                -e "s/[[:space:]]__attribute_const__$$//" \
                -e "/^\#include <linux\/compiler.h>/d"

in scripts/Makefile.headersinst

If you dont want to do something like for make sync, what about providing a 
dummy compiler.h, which has
only this line?
#define __user 

Christian



-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

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

* Re: [PATCH] kvm.h: __user requires compiler.h
  2008-03-25 15:42       ` Christian Borntraeger
@ 2008-03-25 16:23         ` Avi Kivity
  0 siblings, 0 replies; 8+ messages in thread
From: Avi Kivity @ 2008-03-25 16:23 UTC (permalink / raw)
  To: Christian Borntraeger; +Cc: kvm-devel

Christian Borntraeger wrote:
> Am Montag, 24. März 2008 schrieb Avi Kivity:
>   
>> Christian Borntraeger wrote:
>>     
>>> Am Freitag, 21. März 2008 schrieb Anthony Liguori:
>>>   
>>>       
>>>> This patch breaks QEMU build when doing a 'make sync'.  When you do a 
>>>> top-level ./configure, libkvm is built with kerneldir pointing to 
>>>> kvm-userspace/kernel/include.  While linux/kvm.h is present there,
>>>>         
> [...] 
>   
>> Maybe we should generate the 'make sync' headers using 'make 
>> headers_install'.
>>     
>
> headers_install works because there is 
>
> # Eliminate the contents of (and inclusions of) compiler.h
> HDRSED  := sed  -e "s/ inline / __inline__ /g" \
>                 -e "s/[[:space:]]__user[[:space:]]\{1,\}/ /g" \
>                 -e "s/(__user[[:space:]]\{1,\}/ (/g" \
>                 -e "s/[[:space:]]__force[[:space:]]\{1,\}/ /g" \
>                 -e "s/(__force[[:space:]]\{1,\}/ (/g" \
>                 -e "s/[[:space:]]__iomem[[:space:]]\{1,\}/ /g" \
>                 -e "s/(__iomem[[:space:]]\{1,\}/ (/g" \
>                 -e "s/[[:space:]]__attribute_const__[[:space:]]\{1,\}/\ /g" 
> \
>                 -e "s/[[:space:]]__attribute_const__$$//" \
>                 -e "/^\#include <linux\/compiler.h>/d"
>
> in scripts/Makefile.headersinst
>
> If you dont want to do something like for make sync, what about providing a 
> dummy compiler.h, which has
> only this line?
> #define __user 
>   

I already added something similar, see 
bcb30c8bafc7cac75c38981a57bc1f94521e83f4.


-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

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

end of thread, other threads:[~2008-03-25 16:23 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-03-12 17:10 [PATCH] kvm.h: __user requires compiler.h Christian Borntraeger
2008-03-18  6:50 ` Avi Kivity
2008-03-21 20:30 ` Anthony Liguori
2008-03-23  8:42   ` Avi Kivity
2008-03-24 19:46   ` Christian Borntraeger
2008-03-24 20:04     ` Avi Kivity
2008-03-25 15:42       ` Christian Borntraeger
2008-03-25 16:23         ` Avi Kivity

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox