[Qemu-devel] vhost_net.c broken by --kerneldir

[Qemu-devel] vhost_net.c broken by --kerneldir

[Hollis Blanchard]

I'm scratching my head on this one, but here's my problem:
% ./configure --target-list=x86_64-softmmu 
--kerneldir=/home/hollisb/work/linux-2.6.hg
% make V=1
[...]
gcc -I/home/hollisb/work/qemu.git/slirp -Werror -m32 
-fstack-protector-all -Wold-style-definition -Wold-style-declaration -I. 
-I/home/hollisb/work/qemu.git -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE 
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes 
-Wredundant-decls -Wall -Wundef -Wendif-labels -Wwrite-strings 
-Wmissing-prototypes -fno-strict-aliasing  -DHAS_AUDIO 
-DHAS_AUDIO_CHOICE -I/home/hollisb/work/qemu.git/fpu 
-I/home/hollisb/work/qemu.git/tcg 
-I/home/hollisb/work/qemu.git/tcg/i386  -DTARGET_PHYS_ADDR_BITS=64 -I.. 
-I/home/hollisb/work/qemu.git/target-i386 -DNEED_CPU_H     
-I/home/hollisb/work/linux-2.6.hg/include 
-I/home/hollisb/work/linux-2.6.hg/arch/x86/include -MMD -MP -MT 
vhost_net.o -MF ./vhost_net.d -O2 -g  -c -o vhost_net.o 
/home/hollisb/work/qemu.git/hw/vhost_net.c
In file included from 
/home/hollisb/work/linux-2.6.hg/arch/x86/include/asm/sigcontext.h:5,
                  from /usr/include/bits/sigcontext.h:28,
                  from /usr/include/signal.h:339,
                  from /home/hollisb/work/qemu.git/cpu-defs.h:29,
                  from /home/hollisb/work/qemu.git/target-i386/cpu.h:46,
                  from /home/hollisb/work/qemu.git/qemu-common.h:100,
                  from /home/hollisb/work/qemu.git/net.h:5,
                  from /home/hollisb/work/qemu.git/hw/vhost_net.c:13:
/home/hollisb/work/linux-2.6.hg/include/linux/types.h:13:2: error: 
#warning "Attempt to use kernel headers from user space, see 
http://kernelnewbies.org/KernelHeaders"

The problem seems to be that jump from /usr/include/bits/sigcontext.h to 
asm/sigcontext.h inside <kerneldir> rather than inside /usr/include. It 
seems like adding -Ikerneldir/arch/foo/include will always be a problem, 
since it will always be used to satisfy "#include <asm/bar.h>". Only 
files built with KVM_CFLAGS would be affected, and I guess vhost_net.c 
accidentally gets into a broken include path where the other KVM_CFLAGS 
files doesn't.

I'm wondering why this isn't causing more problems for more people. My 
host is Fedora 12, FWIW, but this doesn't seem like it could at all be 
related to toolchain version, for example.

-- 
Hollis Blanchard
Mentor Graphics, Embedded Systems Division

Re: [Qemu-devel] vhost_net.c broken by --kerneldir

[Arnd Bergmann]

On Wednesday 25 August 2010, Hollis Blanchard wrote:

> The problem seems to be that jump from /usr/include/bits/sigcontext.h to 
> asm/sigcontext.h inside <kerneldir> rather than inside /usr/include. It 
> seems like adding -Ikerneldir/arch/foo/include will always be a problem, 
> since it will always be used to satisfy "#include <asm/bar.h>". Only 
> files built with KVM_CFLAGS would be affected, and I guess vhost_net.c 
> accidentally gets into a broken include path where the other KVM_CFLAGS 
> files doesn't.
> 
> I'm wondering why this isn't causing more problems for more people. My 
> host is Fedora 12, FWIW, but this doesn't seem like it could at all be 
> related to toolchain version, for example.

We only recently fixed the kernel to have this warning in types.h, which
triggers more often than kernel.h, where it used to be before. In 2.6.35
and before, you consequently would not have noticed the problem.

	Arnd

[Qemu-devel] Re: vhost_net.c broken by --kerneldir

[Hollis Blanchard]

On 08/24/2010 05:55 PM, Hollis Blanchard wrote:
>
> The problem seems to be that jump from /usr/include/bits/sigcontext.h 
> to asm/sigcontext.h inside <kerneldir> rather than inside 
> /usr/include. It seems like adding -Ikerneldir/arch/foo/include will 
> always be a problem, since it will always be used to satisfy "#include 
> <asm/bar.h>". Only files built with KVM_CFLAGS would be affected, and 
> I guess vhost_net.c accidentally gets into a broken include path where 
> the other KVM_CFLAGS files doesn't.

Small correction: kvm.c doesn't build either (when I configure 
--disable-vhost-net).

Hollis Blanchard
Mentor Graphics, Embedded Systems Division

Re: [Qemu-devel] vhost_net.c broken by --kerneldir

[Hollis Blanchard]

On 08/25/2010 06:37 AM, Arnd Bergmann wrote:
> On Wednesday 25 August 2010, Hollis Blanchard wrote:
>    
>> The problem seems to be that jump from /usr/include/bits/sigcontext.h to
>> asm/sigcontext.h inside<kerneldir>  rather than inside /usr/include. It
>> seems like adding -Ikerneldir/arch/foo/include will always be a problem,
>> since it will always be used to satisfy "#include<asm/bar.h>". Only
>> files built with KVM_CFLAGS would be affected, and I guess vhost_net.c
>> accidentally gets into a broken include path where the other KVM_CFLAGS
>> files doesn't.
>>
>> I'm wondering why this isn't causing more problems for more people. My
>> host is Fedora 12, FWIW, but this doesn't seem like it could at all be
>> related to toolchain version, for example.
>>      
> We only recently fixed the kernel to have this warning in types.h, which
> triggers more often than kernel.h, where it used to be before. In 2.6.35
> and before, you consequently would not have noticed the problem.
>    

Thanks Arnd, that explains it.

It looks like the --kerneldir option needs to be re-thought.

Hollis Blanchard
Mentor Graphics, Embedded Systems Division

Re: [Qemu-devel] Re: vhost_net.c broken by --kerneldir

[Gleb Natapov]

On Wed, Aug 25, 2010 at 10:21:18AM -0700, Hollis Blanchard wrote:
> On 08/24/2010 05:55 PM, Hollis Blanchard wrote:
> >
> >The problem seems to be that jump from
> >/usr/include/bits/sigcontext.h to asm/sigcontext.h inside
> ><kerneldir> rather than inside /usr/include. It seems like adding
> >-Ikerneldir/arch/foo/include will always be a problem, since it
> >will always be used to satisfy "#include <asm/bar.h>". Only files
> >built with KVM_CFLAGS would be affected, and I guess vhost_net.c
> >accidentally gets into a broken include path where the other
> >KVM_CFLAGS files doesn't.
> 
> Small correction: kvm.c doesn't build either (when I configure
> --disable-vhost-net).
> 
I just add --extra-cflags=-D__EXPORTED_HEADERS__ to ./configure

--
			Gleb.

Re: [Qemu-devel] vhost_net.c broken by --kerneldir

[Arnd Bergmann]

On Wednesday 25 August 2010, Hollis Blanchard wrote:
> > We only recently fixed the kernel to have this warning in types.h, which
> > triggers more often than kernel.h, where it used to be before. In 2.6.35
> > and before, you consequently would not have noticed the problem.
> >    
> 
> Thanks Arnd, that explains it.
> 
> It looks like the --kerneldir option needs to be re-thought.

Yes. I believe that we should just kill that option, since there is
no reason for building with set of headers from a different kernel
than the one your glibc normally uses.

You obviously need copies of some headers to match qemu code, e.g.
the kvm headers, but for those we have run-time compatibility code
in qemu. Building with a newer kernel header is pointless because
qemu would not use any of the features that were added after the
release of the qemu tree you are trying to build. Building with
an older kernel header is equally pointless because all it would
do is to require #ifdef magic in qemu that ends up preventing you
from using all features of new kernels.

	Arnd

Re: [Qemu-devel] vhost_net.c broken by --kerneldir

[Gleb Natapov]

On Thu, Aug 26, 2010 at 05:15:35PM +0200, Arnd Bergmann wrote:
> On Wednesday 25 August 2010, Hollis Blanchard wrote:
> > > We only recently fixed the kernel to have this warning in types.h, which
> > > triggers more often than kernel.h, where it used to be before. In 2.6.35
> > > and before, you consequently would not have noticed the problem.
> > >    
> > 
> > Thanks Arnd, that explains it.
> > 
> > It looks like the --kerneldir option needs to be re-thought.
> 
> Yes. I believe that we should just kill that option, since there is
> no reason for building with set of headers from a different kernel
> than the one your glibc normally uses.
> 
> You obviously need copies of some headers to match qemu code, e.g.
> the kvm headers, but for those we have run-time compatibility code
> in qemu. Building with a newer kernel header is pointless because
> qemu would not use any of the features that were added after the
> release of the qemu tree you are trying to build. Building with
> an older kernel header is equally pointless because all it would
> do is to require #ifdef magic in qemu that ends up preventing you
> from using all features of new kernels.
> 
You forgot about developers. Developer may want to use latest kvm kernel
headers to compile code that he added to qemu to use new kernel feature.

--
			Gleb.

Re: [Qemu-devel] vhost_net.c broken by --kerneldir

[Anthony Liguori]

On 08/26/2010 10:21 AM, Gleb Natapov wrote:
> You forgot about developers. Developer may want to use latest kvm kernel
> headers to compile code that he added to qemu to use new kernel feature.
>    

The Right Way to do this is a make headers_install to a temporary 
directory and then -I include that directory.  Building directly from 
the kernel source tree was never the right thing to do.

Regards,

Anthony Liguori

> --
> 			Gleb.
>
>    

Re: [Qemu-devel] vhost_net.c broken by --kerneldir

[Arnd Bergmann]

On Thursday 26 August 2010, Gleb Natapov wrote:
> You forgot about developers. Developer may want to use latest kvm kernel
> headers to compile code that he added to qemu to use new kernel feature.

In that case, you already need to install the kernel in order to test
it, so you might as well install the headers along with it.

Also, you're not normally touching multiple headers at a time, but rather
add a feature in one file, so it's not hard to add it to both copies of
that file. This may seem like an unnecessary step, but using the kernel
headers without installing them first can cause any number of problems
that we don't want anyone to have to deal with.

	Arnd

Re: [Qemu-devel] vhost_net.c broken by --kerneldir

[Hollis Blanchard]

On 08/26/2010 08:48 AM, Anthony Liguori wrote:
> On 08/26/2010 10:21 AM, Gleb Natapov wrote:
>> You forgot about developers. Developer may want to use latest kvm kernel
>> headers to compile code that he added to qemu to use new kernel feature.
>
> The Right Way to do this is a make headers_install to a temporary 
> directory and then -I include that directory.  Building directly from 
> the kernel source tree was never the right thing to do.

Thanks, this worked well.

What are your thoughts on renaming the configure option to --kernel-headers?

   --kerneldir=PATH         look for kernel includes in PATH

Hollis Blanchard
Mentor Graphics, Embedded Systems Division

Re: [Qemu-devel] vhost_net.c broken by --kerneldir

[Anthony Liguori]

On 08/26/2010 11:41 AM, Hollis Blanchard wrote:
> On 08/26/2010 08:48 AM, Anthony Liguori wrote:
>> On 08/26/2010 10:21 AM, Gleb Natapov wrote:
>>> You forgot about developers. Developer may want to use latest kvm 
>>> kernel
>>> headers to compile code that he added to qemu to use new kernel 
>>> feature.
>>
>> The Right Way to do this is a make headers_install to a temporary 
>> directory and then -I include that directory.  Building directly from 
>> the kernel source tree was never the right thing to do.
>
> Thanks, this worked well.
>
> What are your thoughts on renaming the configure option to 
> --kernel-headers?
>
>   --kerneldir=PATH         look for kernel includes in PATH

That's fine but FWIW, --kerneldir was intended to work with a kernel 
source tree.  It's just that that's become impossible in recent kernels.

Regards,

Anthon Liguori

> Hollis Blanchard
> Mentor Graphics, Embedded Systems Division
>

Re: [Qemu-devel] vhost_net.c broken by --kerneldir

[Michael S. Tsirkin]

On Thu, Aug 26, 2010 at 05:15:35PM +0200, Arnd Bergmann wrote:
> On Wednesday 25 August 2010, Hollis Blanchard wrote:
> > > We only recently fixed the kernel to have this warning in types.h, which
> > > triggers more often than kernel.h, where it used to be before. In 2.6.35
> > > and before, you consequently would not have noticed the problem.
> > >    
> > 
> > Thanks Arnd, that explains it.
> > 
> > It looks like the --kerneldir option needs to be re-thought.
> 
> Yes. I believe that we should just kill that option, since there is
> no reason for building with set of headers from a different kernel
> than the one your glibc normally uses.

As others pointed out, it's very useful for development.
I would suggest we do what qemu-kvm.git does instead, and
keep a copy of some headers in qemu tree.
This way developer just copies the header into qemu tree,
and we can get rid of a bunch of ifdefs in code.

> You obviously need copies of some headers to match qemu code, e.g.
> the kvm headers, but for those we have run-time compatibility code
> in qemu. Building with a newer kernel header is pointless because
> qemu would not use any of the features that were added after the
> release of the qemu tree you are trying to build. Building with
> an older kernel header is equally pointless because all it would
> do is to require #ifdef magic in qemu that ends up preventing you
> from using all features of new kernels.
> 
> 	Arnd