Re: [Qemu-devel] [PATCH] Import KVM headers including Makefile andimportscript

[Anthony Liguori <aliguori@us.ibm.com>]

Avi Kivity wrote:
> Anthony Liguori wrote:
>>> Can we put them under kvm/, as include/ looks like a generic include 
>>> directory, which this isn't.  Also, this generates a gratuitous 
>>> conflict with qemu-kvm.git, and we have enough of those already.
>>
>> I'd rather put them under linux/ because right now, we depend on a 
>> number of Linux headers (for USB pass through, for instance).
>>
>> The qemu-kvm.git layout is kvm/kernel/include.  That doesn't seem to 
>> make a lot of sense for QEMU.
>
> linux/ makes sense.  But let's coordinate the change.

BTW, the next logic step for qemu-kvm.git is to move /libkvm/libkvm.c to 
/libkvm-all.c, then move /libkvm/libkvm-<arch>.c to 
/target-<arch>/libkvm.c.  Then make each target that supports kvm depend 
on libkvm-all.o and libkvm.o.  kvmctl can be moved to the top-level too 
and treated like another qemu tool.  The trick is to build a 
kvmctl-libkvm.o or something like that but it should be too difficult.

> Hacks are not good.  I'd avoid them whenever possible.  Just as an 
> example, we'll need to hack the hack whenever we add an architecture 
> (ia64, s390, arm...).

It's purely aesthetic for me and I don't mind changing it.

>>
>> The fixup certainly can be made more readable.  For instance:
>>
>> # Expand asm/ includes to avoid having to do symlink trickery
>> s:^#include <asm/kvm\(.*\)>$:\
>> #if defined(__x86_64__) || defined(__i386__)            \
>> #include <asm-x86/kvm\1>                                \
>> #elif defined(__powerpc__)                              \
>> #include <asm-powerpc/kvm\1>                            \
>> #endif\
>> :g
>>
> Let's avoid it altogether (we can avoid the compiler.h hack by adding 
> a dummy <linux/compiler.h>).

I still think a fixup is needed because once concern I have is that it's 
far too brittle in its current state.  It's too easy to not pull in a 
header and end up using /usr/include/linux/foo.h.  This could lead to 
very subtle build errors that would be host OS dependent.

So I'm thinking that a common prefix to avoid confusion is warranted.  
So the fixup would become:

s:^#include <linux/\(.*\)>:#include "host/linux/\1":g
s:^#include <asm/\(.*\)>:#include "host/asm/\1":g

And we'd change all the existing #includes to use "host/foo".  I think 
that would make it much more robust.

-- 
Regards,

Anthony Liguori