[PATCH] kvm-userspace: fix module build with --kerneldir

[PATCH] kvm-userspace: fix module build with --kerneldir

[Joerg Roedel]

From: Maik Hentsche <maik.hentsche@amd.com>

When kvm-userspace is build with a different kernel version than the
running kernel the depmod at the end will fail. This patch fixed the
problem.

Signed-off-by: Maik Hentsche <maik.hentsche@amd.com>
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 build/kernel_version.c |    9 +++++++++
 configure              |   14 ++++++++++++++
 kernel/Makefile        |    2 +-
 3 files changed, 24 insertions(+), 1 deletions(-)
 create mode 100644 build/kernel_version.c

diff --git a/build/kernel_version.c b/build/kernel_version.c
new file mode 100644
index 0000000..c2ff550
--- /dev/null
+++ b/build/kernel_version.c
@@ -0,0 +1,9 @@
+#include <stdio.h>
+#include <linux/utsrelease.h>
+
+int main()
+{
+        printf("%s",UTS_RELEASE);
+        return(0);
+}
+
diff --git a/configure b/configure
index 63f956c..49c732b 100755
--- a/configure
+++ b/configure
@@ -15,6 +15,12 @@ qemu_opts=()
 cross_prefix=
 arch=`uname -m`
 target_exec=
+# don't use uname if kerneldir is set
+no_uname=
+depmod_version=
+if [ -z "TMPDIR" ] ; then
+    TMPDIR=.
+fi
 
 usage() {
     cat <<-EOF
@@ -56,6 +62,7 @@ while [[ "$1" = -* ]]; do
 	    ;;
 	--kerneldir)
 	    kerneldir="$arg"
+            no_uname=1
 	    ;;
 	--with-patched-kernel)
 	    want_module=
@@ -112,6 +119,12 @@ if [ -d "$kerneldir/include2" ]; then
     kernelsourcedir=${kerneldir%/*}/source
 fi
 
+if [ -n "$no_uname" ]; then
+    $cc -I$kerneldir/include build/kernel_version.c -o $TMPDIR/kernel_version
+    depmod_version=`$TMPDIR/kernel_version`
+    rm $TMPDIR/kernel_version
+fi
+
 #configure user dir
 (cd user; ./configure --prefix="$prefix" --kerneldir="$libkvm_kerneldir" \
           --arch="$arch" --processor="$processor" \
@@ -143,6 +156,7 @@ CC=$cross_prefix$cc
 LD=$cross_prefix$ld
 OBJCOPY=$cross_prefix$objcopy
 AR=$cross_prefix$ar
+DEPMOD_VERSION=$depmod_version
 EOF
 
 cat <<EOF > kernel/config.kbuild
diff --git a/kernel/Makefile b/kernel/Makefile
index 33d6a84..9188802 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -107,7 +107,7 @@ install:
 		 $(ORIGMODDIR)/arch/$(ARCH_DIR)/kvm/*.ko; do \
 		if [ -f "$$i" ]; then mv "$$i" "$$i.orig"; fi; \
 	done
-	/sbin/depmod -a
+	/sbin/depmod -a $(DEPMOD_VERSION)
 
 tmpspec = .tmp.kvm-kmod.spec
 
-- 
1.5.6.4

Re: [PATCH] kvm-userspace: fix module build with --kerneldir

[Avi Kivity]

Joerg Roedel wrote:
> From: Maik Hentsche <maik.hentsche@amd.com>
>
> When kvm-userspace is build with a different kernel version than the
> running kernel the depmod at the end will fail. This patch fixed the
> problem.
>
> Signed-off-by: Maik Hentsche <maik.hentsche@amd.com>
> @@ -0,0 +1,9 @@
> +#include <stdio.h>
> +#include <linux/utsrelease.h>
> +
> +int main()
> +{
> +        printf("%s",UTS_RELEASE);
> +        return(0);
> +}
> +
>   

Can't the version be determined directly from kernedir itself?  e.g. 
kerneldir is /lib/modules/$version/build?


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

Re: [PATCH] kvm-userspace: fix module build with --kerneldir

[Maik Hentsche]

[-- Attachment #1: Type: text/plain, Size: 901 bytes --]

Avi Kivity <avi@redhat.com> wrote:

> Can't the version be determined directly from kernedir itself?  e.g. 
> kerneldir is /lib/modules/$version/build?

How do you get $version if you are crosscompiling? In this case the
usual way of asking uname doesn't work. Instead you somehow need to get
the version of the kernel you're building for from the kernel source
provided in --kerneldir. I don't know any better way than evaluating
UTSRELEASE.

so long
Maik

-- 
           \           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
  System   |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
  Center   |             AMD Saxony LLC (Wilmington, Delaware, US)
           / General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe,
Thomas McCoy

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] kvm-userspace: fix module build with --kerneldir

[Avi Kivity]

Maik Hentsche wrote:
> Avi Kivity <avi@redhat.com> wrote:
>
>   
>> Can't the version be determined directly from kernedir itself?  e.g. 
>> kerneldir is /lib/modules/$version/build?
>>     
>
> How do you get $version if you are crosscompiling? In this case the
> usual way of asking uname doesn't work. Instead you somehow need to get
> the version of the kernel you're building for from the kernel source
> provided in --kerneldir. I don't know any better way than evaluating
> UTSRELEASE.
>   

If the user specified $kerneldir, it should be in the form 
/lib/modules/$version/build, no?  If so, then you can extract $version 
by removing the prefix and suffix from the string.

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

Re: [PATCH] kvm-userspace: fix module build with --kerneldir

[Maik Hentsche]

[-- Attachment #1: Type: text/plain, Size: 779 bytes --]

(Sorry, I accidentally sent this mail without CC before. Resend with
correct CC list.)

Avi Kivity <avi@redhat.com> wrote:

> If the user specified $kerneldir, it should be in the form 
> /lib/modules/$version/build, no? 

Ah sorry, I did not know of this requirement. My kerneldir
is /tmp/linux (I build inside chroot).

so long
Maik

-- 
           \           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
  System   |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
  Center   |             AMD Saxony LLC (Wilmington, Delaware, US)
           / General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe,
Thomas McCoy

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] kvm-userspace: fix module build with --kerneldir

[Jan Kiszka]

Avi Kivity wrote:
> Maik Hentsche wrote:
>> Avi Kivity <avi@redhat.com> wrote:
>>
>>  
>>> Can't the version be determined directly from kernedir itself?  e.g.
>>> kerneldir is /lib/modules/$version/build?
>>>     
>>
>> How do you get $version if you are crosscompiling? In this case the
>> usual way of asking uname doesn't work. Instead you somehow need to get
>> the version of the kernel you're building for from the kernel source
>> provided in --kerneldir. I don't know any better way than evaluating
>> UTSRELEASE.
>>   
> 
> If the user specified $kerneldir, it should be in the form
> /lib/modules/$version/build, no?

This is not sufficiently generic. KVM should to accept arbitrary paths
to kernel directories, not just those special links (which could cause
problems anyway if the link is set up for the target and makes no sense
on the build host).

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2 ES-OS
Corporate Competence Center Embedded Linux

Re: [PATCH] kvm-userspace: fix module build with --kerneldir

[Avi Kivity]

Jan Kiszka wrote:
> Avi Kivity wrote:
>   
>> Maik Hentsche wrote:
>>     
>>> Avi Kivity <avi@redhat.com> wrote:
>>>
>>>  
>>>       
>>>> Can't the version be determined directly from kernedir itself?  e.g.
>>>> kerneldir is /lib/modules/$version/build?
>>>>     
>>>>         
>>> How do you get $version if you are crosscompiling? In this case the
>>> usual way of asking uname doesn't work. Instead you somehow need to get
>>> the version of the kernel you're building for from the kernel source
>>> provided in --kerneldir. I don't know any better way than evaluating
>>> UTSRELEASE.
>>>   
>>>       
>> If the user specified $kerneldir, it should be in the form
>> /lib/modules/$version/build, no?
>>     
>
> This is not sufficiently generic. KVM should to accept arbitrary paths
> to kernel directories, not just those special links (which could cause
> problems anyway if the link is set up for the target and makes no sense
> on the build host).
>
>   

Fair enough.  This seems to work:

    awk ' / = / { a[$1] = $3 } END { printf("%s.%s.%s%s\n", 
a["VERSION"], a["PATCHLEVEL"], a["SUBLEVEL"], a["EXTRAVERSION"]) }' 
"$kerneldir/Makefile"

Or alternatively

 awk '/Linux kernel version/ { print $NF }' "$kerneldir/.config"

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

Re: [PATCH] kvm-userspace: fix module build with --kerneldir

[Avi Kivity]

Maik Hentsche wrote:
>   
>> If the user specified $kerneldir, it should be in the form 
>> /lib/modules/$version/build, no? 
>>     
>
> Ah sorry, I did not know of this requirement. My kerneldir
> is /tmp/linux (I build inside chroot).
>   

I took it from the depmod manpage, but if you're building from 
/tmp/linux, clearly it isn't a strict requirement.

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

Re: [PATCH] kvm-userspace: fix module build with --kerneldir

[Eduardo Habkost]

Excerpts from Avi Kivity's message of Qua Nov 26 14:00:42 -0200 2008:
> Jan Kiszka wrote:
> > Avi Kivity wrote:
<snip>
> >> If the user specified $kerneldir, it should be in the form
> >> /lib/modules/$version/build, no?
> >>     
> >
> > This is not sufficiently generic. KVM should to accept arbitrary paths
> > to kernel directories, not just those special links (which could cause
> > problems anyway if the link is set up for the target and makes no sense
> > on the build host).
> >
> >   
> 
> Fair enough.  This seems to work:
> 
>     awk ' / = / { a[$1] = $3 } END { printf("%s.%s.%s%s\n", 
> a["VERSION"], a["PATCHLEVEL"], a["SUBLEVEL"], a["EXTRAVERSION"]) }' 
> "$kerneldir/Makefile"

What if CONFIG_LOCALVERSION is set?

> 
> Or alternatively
> 
>  awk '/Linux kernel version/ { print $NF }' "$kerneldir/.config"

What about `cat "$kerneldir/include/config/kernel.release"`?
-- 
Eduardo

Re: [PATCH] kvm-userspace: fix module build with --kerneldir

[Avi Kivity]

Eduardo Habkost wrote:
> What about `cat "$kerneldir/include/config/kernel.release"`?
>   

Obviously a winner.  I see it was moved in 2.6.17 (see f1d28fb04), so we 
have to support both the old and new locations.

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

Re: [PATCH] kvm-userspace: fix module build with --kerneldir

[Maik Hentsche]

[-- Attachment #1.1: Type: text/plain, Size: 1043 bytes --]

Please find my reworked patch attached. Support for pre-f1d28fb04
kernels was tested with 2.6.16.1. I CC-ed everyone who contributed to
this thread, thanks for your help. I hope the "bureaucracy" is correct.
I'm not a kernel developer and thus only know about the contribution
process what I found in the documentation.

so long
Maik

----

When kvm-userspace is build with a different kernel version than the
running kernel the depmod at the end will fail. This patch fixed the
problem.

Signed-off-by: Maik Hentsche <maik.hentsche@amd.com>
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>


-- 
           \           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
  System   |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
  Center   |             AMD Saxony LLC (Wilmington, Delaware, US)
           / General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe,
Thomas McCoy

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: kvm_patch.git.diff --]
[-- Type: text/x-patch; name=kvm_patch.git.diff, Size: 1809 bytes --]

diff --git a/configure b/configure
index 63f956c..97a7cb7 100755
--- a/configure
+++ b/configure
@@ -15,6 +15,12 @@ qemu_opts=()
 cross_prefix=
 arch=`uname -m`
 target_exec=
+# don't use uname if kerneldir is set
+no_uname=
+depmod_version=
+if [ -z "TMPDIR" ] ; then 
+    TMPDIR=.
+fi
 
 usage() {
     cat <<-EOF
@@ -56,6 +62,7 @@ while [[ "$1" = -* ]]; do
 	    ;;
 	--kerneldir)
 	    kerneldir="$arg"
+            no_uname=1
 	    ;;
 	--with-patched-kernel)
 	    want_module=
@@ -112,6 +119,21 @@ if [ -d "$kerneldir/include2" ]; then
     kernelsourcedir=${kerneldir%/*}/source
 fi
 
+if [ -n "$no_uname" ]; then
+    if [ -e "$kerneldir/.kernelrelease" ]; then
+        depmod_version=`cat "$kerneldir/.kernelrelease"`
+        
+    elif [ -e "$kerneldir/include/config/kernel.release" ]; then
+            depmod_version=`cat "$kerneldir/include/config/kernel.release"`
+    else
+        echo 
+        echo "Error: kernelversion not found"
+        echo "Please make sure your kernel is configured"
+        echo
+        exit 1
+    fi
+fi
+
 #configure user dir
 (cd user; ./configure --prefix="$prefix" --kerneldir="$libkvm_kerneldir" \
           --arch="$arch" --processor="$processor" \
@@ -143,6 +165,7 @@ CC=$cross_prefix$cc
 LD=$cross_prefix$ld
 OBJCOPY=$cross_prefix$objcopy
 AR=$cross_prefix$ar
+DEPMOD_VERSION=$depmod_version
 EOF
 
 cat <<EOF > kernel/config.kbuild
diff --git a/kernel/Makefile b/kernel/Makefile
index 41449d6..8315e3d 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -107,7 +107,7 @@ install:
 		 $(ORIGMODDIR)/arch/$(ARCH_DIR)/kvm/*.ko; do \
 		if [ -f "$$i" ]; then mv "$$i" "$$i.orig"; fi; \
 	done
-	/sbin/depmod -a
+	/sbin/depmod -a $(DEPMOD_VERSION)
 
 tmpspec = .tmp.kvm-kmod.spec
 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] kvm-userspace: fix module build with --kerneldir

[Joerg Roedel]

Hey Avi,

do you plan to apply this patch? Or, if not, can you give a comment why
not?

Thanks,

Joerg

On Mon, Dec 01, 2008 at 03:28:31PM +0100, Maik Hentsche wrote:
> Please find my reworked patch attached. Support for pre-f1d28fb04
> kernels was tested with 2.6.16.1. I CC-ed everyone who contributed to
> this thread, thanks for your help. I hope the "bureaucracy" is correct.
> I'm not a kernel developer and thus only know about the contribution
> process what I found in the documentation.
> 
> so long
> Maik
> 
> ----
> 
> When kvm-userspace is build with a different kernel version than the
> running kernel the depmod at the end will fail. This patch fixed the
> problem.
> 
> Signed-off-by: Maik Hentsche <maik.hentsche@amd.com>
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> 
> 
> -- 
>            \           AMD Saxony Limited Liability Company & Co. KG
>  Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
>   System   |                  Register Court Dresden: HRA 4896
>  Research  |              General Partner authorized to represent:
>   Center   |             AMD Saxony LLC (Wilmington, Delaware, US)
>            / General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe,
> Thomas McCoy

> diff --git a/configure b/configure
> index 63f956c..97a7cb7 100755
> --- a/configure
> +++ b/configure
> @@ -15,6 +15,12 @@ qemu_opts=()
>  cross_prefix=
>  arch=`uname -m`
>  target_exec=
> +# don't use uname if kerneldir is set
> +no_uname=
> +depmod_version=
> +if [ -z "TMPDIR" ] ; then 
> +    TMPDIR=.
> +fi
>  
>  usage() {
>      cat <<-EOF
> @@ -56,6 +62,7 @@ while [[ "$1" = -* ]]; do
>  	    ;;
>  	--kerneldir)
>  	    kerneldir="$arg"
> +            no_uname=1
>  	    ;;
>  	--with-patched-kernel)
>  	    want_module=
> @@ -112,6 +119,21 @@ if [ -d "$kerneldir/include2" ]; then
>      kernelsourcedir=${kerneldir%/*}/source
>  fi
>  
> +if [ -n "$no_uname" ]; then
> +    if [ -e "$kerneldir/.kernelrelease" ]; then
> +        depmod_version=`cat "$kerneldir/.kernelrelease"`
> +        
> +    elif [ -e "$kerneldir/include/config/kernel.release" ]; then
> +            depmod_version=`cat "$kerneldir/include/config/kernel.release"`
> +    else
> +        echo 
> +        echo "Error: kernelversion not found"
> +        echo "Please make sure your kernel is configured"
> +        echo
> +        exit 1
> +    fi
> +fi
> +
>  #configure user dir
>  (cd user; ./configure --prefix="$prefix" --kerneldir="$libkvm_kerneldir" \
>            --arch="$arch" --processor="$processor" \
> @@ -143,6 +165,7 @@ CC=$cross_prefix$cc
>  LD=$cross_prefix$ld
>  OBJCOPY=$cross_prefix$objcopy
>  AR=$cross_prefix$ar
> +DEPMOD_VERSION=$depmod_version
>  EOF
>  
>  cat <<EOF > kernel/config.kbuild
> diff --git a/kernel/Makefile b/kernel/Makefile
> index 41449d6..8315e3d 100644
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -107,7 +107,7 @@ install:
>  		 $(ORIGMODDIR)/arch/$(ARCH_DIR)/kvm/*.ko; do \
>  		if [ -f "$$i" ]; then mv "$$i" "$$i.orig"; fi; \
>  	done
> -	/sbin/depmod -a
> +	/sbin/depmod -a $(DEPMOD_VERSION)
>  
>  tmpspec = .tmp.kvm-kmod.spec
>  




-- 
           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

Re: [PATCH] kvm-userspace: fix module build with --kerneldir

[Avi Kivity]

Joerg Roedel wrote:
> Hey Avi,
>
> do you plan to apply this patch? Or, if not, can you give a comment why
> not?
>   

sorry, accidentally dropped, not applied.  Thanks for the remainder.

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