Broken external module build on 2.6.23

Hello,

     The merge with upstream changeset: af5ca3f4ec5cc4432a42a73b050dd8898ce8fd00

broke building an external module against 2.6.23:

CC [M] /root/kvm-userspace/kernel/kvm_main.o
/root/kvm-userspace/kernel/kvm_main.c:1271: error: unknown field ‘name’ specified in initializer

Which makes sense, since the .name parameter doesn't exist pre-2.6.24. I've been trying to think of a way to work around it in external-module-compat.h, but have so far been unsuccessful. Does anyone have any good ideas?

Chris Lalancette

-------------------------------------------------------------------------
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 will solve it:

@@ -1267,7 +1279,11 @@ static int kvm_resume(struct sys_device *dev)
 }
 
 static struct sysdev_class kvm_sysdev_class = {
+#ifdef set_kset_name
+	set_kset_name("kvm"),
+#else
 	.name = "kvm",
+#endif
 	.suspend = kvm_suspend,
 	.resume = kvm_resume,
 };

The major breakage with the external module isn't the above, but the fact when kvm_host.h and friends were added to mainline, the external module build miscompiles and crashes if you're not in full sync with the headers of kvm.git and the kernel that you compile against. I posted a patch already but it was only a band aid and it didn't make it more robust.

I'm perfectly fine with a compile error, but I don't want to risk miscompiles if somebody changes the order of the #include or adds some more #include, again.

The below patch should be much more robust. The empty files that I removed can be placed later in a include-compat directory and an -Ipwd/include-compat can be added after -Ipwd/include, if they're still needed. This way it's always safe to build against any new kernel source. So I recommend you use this too even if it shouldn't be needed with 2.6.23 because 2.6.23 is still old enough and it misses kvm_host.h etc... (my miscompile troubles stared with 2.6.24)

index 7a435b5..faec72b 100644
@@ -25,7 +25,9 @@ hack = $(call _hack,tmp/$(strip $1))
 unifdef = $(call _unifdef,tmp/$(strip $1))
 
 all::
-	$(MAKE) -C $(KERNELDIR) M=`pwd` "$$@"
+	$(MAKE) -C $(KERNELDIR) M=`pwd` \
+		LINUXINCLUDE="-I`pwd`/include -Iinclude -include include/linux/autoconf.h" \
+		"$$@"
 
 sync:
 	rm -rf tmp

deleted file mode 100644
index 4258b93..0000000
@@ -1,5 +0,0 @@
-/*
- * Empty file to satisfy #include <linux/cmpxchg.h> for older kernels.
- */
-
-

deleted file mode 100644
index b2e1ba3..0000000
@@ -1,16 +0,0 @@
-/*
- *  include/linux/anon_inodes.h
- *
- *  Copyright (C) 2007  Davide Libenzi <[email protected]>
- *
- */
-
-#ifndef _LINUX_ANON_INODES_H
-#define _LINUX_ANON_INODES_H
-
-int anon_inode_getfd(int *pfd, struct inode **pinode, struct file **pfile,
-		     const char *name, const struct file_operations *fops,
-		     void *priv);
-
-#endif /* _LINUX_ANON_INODES_H */
-

deleted file mode 100644
index 71b2ae1..0000000
@@ -1,5 +0,0 @@
-/*
- * Empty file to satisfy #include <linux/mutex.h> for older kernels.
- */
-
-

-------------------------------------------------------------------------
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/
_______________________________________________
kvm-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Yes, the patch wasn't complete, it was primarily to show the idea to remove the fragile dependency on exact #include order in the headers. If the only trouble is the lack of build for older kernels I guess I can proceed to add that last bit. Those compat files for old kernels should be added to a brand new include-compat directory instead of being mixed with the kvm headers and -Iinclude-compat can be added at the end of the LINUXINCLUDES definition.

$LINUX kvm headers should be included before the $KERNELDIR headers, and include-compat should be included after the $KERNELDIR headers.

I created a more complete patch below, this will solve all my silent time-wasting miscompilations (currnently when building kvm.git on a newer mainline kernels, certain kvm*.h $KERNELDIR headers are included instead of my modified ones in $LINUX, so the build can crash if $LINUX and $KERNELDIR don't have the kvm*.h headers in sync) and this below patch will retain compatibility with older kernels too. If patch doesn't wipe kernel/include you can follow it with a rm -r kernel/include. I verified libkvm already gives more priority to whatever is in kernel/include and only if kernel/include are missing it uses the kvm.h in /usr/include/linux/kvm.h (I filled all my /usr/src and /usr/include headers with #error "x" to be sure... along with running gcc -E and checking the cpp work).

diff -urN 1/kernel/include/asm-x86/cmpxchg.h 2/kernel/include/asm-x86/cmpxchg.h
@@ -1,5 +0,0 @@
-/*
- * Empty file to satisfy #include <linux/cmpxchg.h> for older kernels.
- */
-
-

diff -urN 1/kernel/include/linux/anon_inodes.h 2/kernel/include/linux/anon_inodes.h
@@ -1,16 +0,0 @@
-/*
- *  include/linux/anon_inodes.h
- *
- *  Copyright (C) 2007  Davide Libenzi <[email protected]>
- *
- */
-
-#ifndef _LINUX_ANON_INODES_H
-#define _LINUX_ANON_INODES_H
-
-int anon_inode_getfd(int *pfd, struct inode **pinode, struct file **pfile,
-		     const char *name, const struct file_operations *fops,
-		     void *priv);
-
-#endif /* _LINUX_ANON_INODES_H */
-

diff -urN 1/kernel/include/linux/mutex.h 2/kernel/include/linux/mutex.h
@@ -1,5 +0,0 @@
-/*
- * Empty file to satisfy #include <linux/mutex.h> for older kernels.
- */
-
-

diff -urN 1/kernel/include-compat/asm-x86/cmpxchg.h 2/kernel/include-compat/asm-x86/cmpxchg.h
@@ -0,0 +1,5 @@
+/*
+ * Empty file to satisfy #include <linux/cmpxchg.h> for older kernels.
+ */
+
+

diff -urN 1/kernel/include-compat/linux/anon_inodes.h 2/kernel/include-compat/linux/anon_inodes.h
@@ -0,0 +1,16 @@
+/*
+ *  include/linux/anon_inodes.h
+ *
+ *  Copyright (C) 2007  Davide Libenzi <[email protected]>
+ *
+ */
+
+#ifndef _LINUX_ANON_INODES_H
+#define _LINUX_ANON_INODES_H
+
+int anon_inode_getfd(int *pfd, struct inode **pinode, struct file **pfile,
+		     const char *name, const struct file_operations *fops,
+		     void *priv);
+
+#endif /* _LINUX_ANON_INODES_H */
+

diff -urN 1/kernel/include-compat/linux/mutex.h 2/kernel/include-compat/linux/mutex.h
@@ -0,0 +1,5 @@
+/*
+ * Empty file to satisfy #include <linux/mutex.h> for older kernels.
+ */
+
+

index 052d561..ae69799 100644
@@ -10,8 +10,8 @@
 #include <linux/compiler.h>
 #include <linux/version.h>
 #include <linux/string.h>
-#include "include/linux/kvm.h"
-#include "include/linux/kvm_para.h"
+#include <include/linux/kvm.h>
+#include <include/linux/kvm_para.h>
 #include <linux/cpu.h>
 #include <asm/processor.h>
 #include <linux/hrtimer.h>

index 7a435b5..0ee55a0 100644
@@ -25,17 +25,21 @@ hack = $(call _hack,tmp/$(strip $1))
 unifdef = $(call _unifdef,tmp/$(strip $1))
 
 all::
-	$(MAKE) -C $(KERNELDIR) M=`pwd` "$$@"
+	# include header priority 1) $LINUX 2) $KERNELDIR 3) include-compat
+	$(MAKE) -C $(KERNELDIR) M=`pwd` \
+		LINUXINCLUDE="-I`pwd`/include -Iinclude -Iinclude-compat \
+		-include include/linux/autoconf.h" \
+		"$$@"
 
 sync:
-	rm -rf tmp
+	rm -rf tmp include
 	rsync --exclude='*.mod.c' -R \
              "$(LINUX)"/arch/x86/kvm/./*.[ch] \
              "$(LINUX)"/virt/kvm/./*.[ch] \
 	     "$(LINUX)"/./include/linux/kvm*.h \
 	     "$(LINUX)"/./include/asm-x86/kvm*.h \
              tmp/
-	rm -rf include/asm
+	mkdir -p include/linux include/asm-x86
 	ln -s asm-x86 include/asm
 
 	$(call unifdef, include/linux/kvm.h)

@@ -78,6 +82,7 @@ rpm:	all
 
 clean:
 	$(MAKE) -C $(KERNELDIR) M=`pwd` $@
+	rm -rf include
 
 svnclean:
 	svn st | grep '^\?' | awk '{print $2}' | xargs rm -rf

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

Thanks, applied that. I added an include-compat/asm symlink so that it builds.

Please turn on git rename detection for better readability with such patches.

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/

Actually I applied the second version you sent.

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/