[Qemu-devel] [PATCH v2 0/2] buildsys: Fix and enable module build

[Qemu-devel] [PATCH v2 0/2] buildsys: Fix and enable module build

[Fam Zheng]

v2: Fix on error handling nit in configure. (Andreas)
    Skip rules.mak fix because Paolo applied it.

Fam


Fam Zheng (2):
  configure: Default to enable module build
  .travis.yml: Add "--disable-modules"

 .travis.yml |  3 ++
 configure   | 95 ++++++++++++++++++++++++++++++++++++++++++-------------------
 2 files changed, 69 insertions(+), 29 deletions(-)

-- 
2.1.0

[Qemu-devel] [PATCH v2 1/2] configure: Default to enable module build

[Fam Zheng]

We have module build support around for a while, but also had it bitrot
several times. It probably makes sense to enable it by default so that
people can notice and use it.

Counterpart to --enable-modules, which is turned as default,
--disable-modules is added to suppress it. If both are omitted, the
support is guesses as usual.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 configure | 95 ++++++++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 66 insertions(+), 29 deletions(-)

diff --git a/configure b/configure
index 7539645..8280e8a 100755
--- a/configure
+++ b/configure
@@ -271,7 +271,7 @@ gcov_tool="gcov"
 EXESUF=""
 DSOSUF=".so"
 LDFLAGS_SHARED="-shared"
-modules="no"
+modules=""
 prefix="/usr/local"
 mandir="\${prefix}/share/man"
 datadir="\${prefix}/share"
@@ -768,6 +768,9 @@ for opt do
   --enable-modules)
       modules="yes"
   ;;
+  --disable-modules)
+      modules="no"
+  ;;
   --cpu=*)
   ;;
   --target-list=*) target_list="$optarg"
@@ -1259,7 +1262,8 @@ Advanced options (experts only):
   --sysconfdir=PATH        install config in PATH$confsuffix
   --localstatedir=PATH     install local state in PATH (set at runtime on win32)
   --with-confsuffix=SUFFIX suffix for QEMU data inside datadir/libdir/sysconfdir [$confsuffix]
-  --enable-modules         enable modules support
+  --enable-modules         enable modules support (default)
+  --disable-modules        enable modules support
   --enable-debug-tcg       enable TCG debugging
   --disable-debug-tcg      disable TCG debugging (default)
   --enable-debug-info      enable debugging information (default)
@@ -2699,22 +2703,25 @@ if test "$mingw32" = yes; then
 else
     glib_req_ver=2.12
 fi
-glib_modules=gthread-2.0
-if test "$modules" = yes; then
-    glib_modules="$glib_modules gmodule-2.0"
-fi
 
-for i in $glib_modules; do
-    if $pkg_config --atleast-version=$glib_req_ver $i; then
-        glib_cflags=`$pkg_config --cflags $i`
-        glib_libs=`$pkg_config --libs $i`
-        CFLAGS="$glib_cflags $CFLAGS"
-        LIBS="$glib_libs $LIBS"
-        libs_qga="$glib_libs $libs_qga"
-    else
-        error_exit "glib-$glib_req_ver $i is required to compile QEMU"
-    fi
-done
+glib_module_try_config()
+{
+  if $pkg_config --atleast-version=$glib_req_ver $1; then
+    local probe_cflags=$($pkg_config --cflags $1)
+    local probe_libs=$($pkg_config --libs $1)
+    CFLAGS="$probe_cflags $CFLAGS"
+    LIBS="$probe_libs $LIBS"
+    libs_qga="$probe_libs $libs_qga"
+    glib_cflags="$probe_cflags $glib_cflags"
+    glib_libs="$probe_libs $glib_libs"
+    return 0
+  else
+    return 1
+  fi
+}
+
+glib_module_try_config gthread-2.0 || \
+  error_exit "glib-$glib_req_ver gthread-2.0 is required to compile QEMU"
 
 # g_test_trap_subprocess added in 2.38. Used by some tests.
 glib_subprocess=yes
@@ -2723,19 +2730,49 @@ if ! $pkg_config --atleast-version=2.38 glib-2.0; then
 fi
 
 ##########################################
-# SHA command probe for modules
-if test "$modules" = yes; then
-    shacmd_probe="sha1sum sha1 shasum"
-    for c in $shacmd_probe; do
-        if has $c; then
-            shacmd="$c"
-            break
-        fi
-    done
-    if test "$shacmd" = ""; then
-        error_exit "one of the checksum commands is required to enable modules: $shacmd_probe"
+# SHA command and gmodule-2.0 probe for modules
+# return 0 if probe succeeds
+# $1: true - force mode, exit if probe fail
+#     false - optoinal mode, return 1 if probe fail
+module_try_enable()
+{
+  force=$1
+  shacmd_probe="sha1sum sha1 shasum"
+  for c in $shacmd_probe; do
+    if has $c; then
+      shacmd="$c"
+      break
     fi
-fi
+  done
+  if test "$shacmd" = ""; then
+    if $force; then
+      error_exit "one of the checksum commands is required to enable modules: $shacmd_probe"
+    else
+      modules="no"
+      return
+    fi
+  fi
+  if ! glib_module_try_config gmodule-2.0; then
+    if $force; then
+      error_exit "glib-$glib_req_ver gthread-2.0 is required to compile QEMU"
+    else
+      modules="no"
+      return
+    fi
+  fi
+  modules="yes"
+}
+
+case "$modules" in
+  yes)
+    module_try_enable true
+    ;;
+  "")
+    module_try_enable false
+    ;;
+  no)
+    ;;
+esac
 
 ##########################################
 # pixman support probe
-- 
2.1.0

Re: [Qemu-devel] [PATCH v2 1/2] configure: Default to enable module build

[Stefan Hajnoczi]

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

On Tue, Jan 13, 2015 at 04:53:58PM +0800, Fam Zheng wrote:
> We have module build support around for a while, but also had it bitrot
> several times. It probably makes sense to enable it by default so that
> people can notice and use it.
> 
> Counterpart to --enable-modules, which is turned as default,
> --disable-modules is added to suppress it. If both are omitted, the
> support is guesses as usual.

"Add --disable-modules as a counterpart to --enable-modules, which is
now turned on by default.  If both are omitted, support is guessed as
usual."

> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  configure | 95 ++++++++++++++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 66 insertions(+), 29 deletions(-)
> 
> diff --git a/configure b/configure
> index 7539645..8280e8a 100755
> --- a/configure
> +++ b/configure
> @@ -271,7 +271,7 @@ gcov_tool="gcov"
>  EXESUF=""
>  DSOSUF=".so"
>  LDFLAGS_SHARED="-shared"
> -modules="no"
> +modules=""
>  prefix="/usr/local"
>  mandir="\${prefix}/share/man"
>  datadir="\${prefix}/share"
> @@ -768,6 +768,9 @@ for opt do
>    --enable-modules)
>        modules="yes"
>    ;;
> +  --disable-modules)
> +      modules="no"
> +  ;;
>    --cpu=*)
>    ;;
>    --target-list=*) target_list="$optarg"
> @@ -1259,7 +1262,8 @@ Advanced options (experts only):
>    --sysconfdir=PATH        install config in PATH$confsuffix
>    --localstatedir=PATH     install local state in PATH (set at runtime on win32)
>    --with-confsuffix=SUFFIX suffix for QEMU data inside datadir/libdir/sysconfdir [$confsuffix]
> -  --enable-modules         enable modules support
> +  --enable-modules         enable modules support (default)
> +  --disable-modules        enable modules support
>    --enable-debug-tcg       enable TCG debugging
>    --disable-debug-tcg      disable TCG debugging (default)
>    --enable-debug-info      enable debugging information (default)
> @@ -2699,22 +2703,25 @@ if test "$mingw32" = yes; then
>  else
>      glib_req_ver=2.12
>  fi
> -glib_modules=gthread-2.0
> -if test "$modules" = yes; then
> -    glib_modules="$glib_modules gmodule-2.0"
> -fi
>  
> -for i in $glib_modules; do
> -    if $pkg_config --atleast-version=$glib_req_ver $i; then
> -        glib_cflags=`$pkg_config --cflags $i`
> -        glib_libs=`$pkg_config --libs $i`
> -        CFLAGS="$glib_cflags $CFLAGS"
> -        LIBS="$glib_libs $LIBS"
> -        libs_qga="$glib_libs $libs_qga"
> -    else
> -        error_exit "glib-$glib_req_ver $i is required to compile QEMU"
> -    fi
> -done
> +glib_module_try_config()

I guess "module" here means glib package or component?

It's a little confusing since gmodule-2.0 is needed for QEMU modular
build support :).

How about just glib_pkg_config()?

> +  if ! glib_module_try_config gmodule-2.0; then
> +    if $force; then
> +      error_exit "glib-$glib_req_ver gthread-2.0 is required to compile QEMU"

s/gthread-2.0/gmodule-2.0/

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH v2 1/2] configure: Default to enable module build

[Paolo Bonzini]

On 13/01/2015 09:53, Fam Zheng wrote:
> We have module build support around for a while, but also had it bitrot
> several times. It probably makes sense to enable it by default so that
> people can notice and use it.
> 
> Counterpart to --enable-modules, which is turned as default,
> --disable-modules is added to suppress it. If both are omitted, the
> support is guesses as usual.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>

I squashed this to unbreak static compilation, but Peter reported that
module linking fails on ARM host:
  LINK  block/curl.so
/usr/bin/ld: block/curl.o: relocation R_ARM_THM_MOVW_ABS_NC against
`__stack_chk_guard' can not be used when making a shared object;
recompile with -fPIC
block/curl.o: could not read symbols: Bad value
collect2: error: ld returned 1 exit status

diff --git a/configure b/configure
index a9ae57a..4fae00a 100755
--- a/configure
+++ b/configure
@@ -1536,9 +1536,6 @@ if compile_prog "-Werror -fno-gcse" "" ; then
 fi

 if test "$static" = "yes" ; then
-  if test "$modules" = "yes" ; then
-    error_exit "static and modules are mutually incompatible"
-  fi
   if test "$pie" = "yes" ; then
     error_exit "static and pie are mutually incompatible"
   else
Stage this hunk [y,n,q,a,d,/,j,J,g,e,?]? y
@@ -2763,6 +2760,14 @@ fi
 module_try_enable()
 {
   force=$1
+  if test "$static" = "yes"; then
+    if $force; then
+      error_exit "static and modules are mutually incompatible"
+    else
+      modules="no"
+      return
+    fi
+  fi
   shacmd_probe="sha1sum sha1 shasum"
   for c in $shacmd_probe; do
     if has $c; then


Is the above ok?

Paolo

Re: [Qemu-devel] [PATCH v2 1/2] configure: Default to enable module build

[Fam Zheng]

On Mon, 02/02 20:34, Paolo Bonzini wrote:
> 
> 
> On 13/01/2015 09:53, Fam Zheng wrote:
> > We have module build support around for a while, but also had it bitrot
> > several times. It probably makes sense to enable it by default so that
> > people can notice and use it.
> > 
> > Counterpart to --enable-modules, which is turned as default,
> > --disable-modules is added to suppress it. If both are omitted, the
> > support is guesses as usual.
> > 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> 
> I squashed this to unbreak static compilation, but Peter reported that
> module linking fails on ARM host:
>   LINK  block/curl.so
> /usr/bin/ld: block/curl.o: relocation R_ARM_THM_MOVW_ABS_NC against
> `__stack_chk_guard' can not be used when making a shared object;
> recompile with -fPIC
> block/curl.o: could not read symbols: Bad value
> collect2: error: ld returned 1 exit status

I don't see how -fPIC is missed in ARM host :( Does the below patch fix this?

> 
> diff --git a/configure b/configure
> index a9ae57a..4fae00a 100755
> --- a/configure
> +++ b/configure
> @@ -1536,9 +1536,6 @@ if compile_prog "-Werror -fno-gcse" "" ; then
>  fi
> 
>  if test "$static" = "yes" ; then
> -  if test "$modules" = "yes" ; then
> -    error_exit "static and modules are mutually incompatible"
> -  fi
>    if test "$pie" = "yes" ; then
>      error_exit "static and pie are mutually incompatible"
>    else
> Stage this hunk [y,n,q,a,d,/,j,J,g,e,?]? y
> @@ -2763,6 +2760,14 @@ fi
>  module_try_enable()
>  {
>    force=$1
> +  if test "$static" = "yes"; then
> +    if $force; then
> +      error_exit "static and modules are mutually incompatible"
> +    else
> +      modules="no"
> +      return
> +    fi
> +  fi
>    shacmd_probe="sha1sum sha1 shasum"
>    for c in $shacmd_probe; do
>      if has $c; then
> 
> 
> Is the above ok?

Yes, it looks correct.

Fam

Re: [Qemu-devel] [PATCH v2 1/2] configure: Default to enable module build

[Paolo Bonzini]

On 03/02/2015 02:29, Fam Zheng wrote:
> > Peter reported that module linking fails on ARM host:
> >   LINK  block/curl.so
> > /usr/bin/ld: block/curl.o: relocation R_ARM_THM_MOVW_ABS_NC against
> > `__stack_chk_guard' can not be used when making a shared object;
> > recompile with -fPIC
> > block/curl.o: could not read symbols: Bad value
> > collect2: error: ld returned 1 exit status
>
> I don't see how -fPIC is missed in ARM host :( Does the below patch fix this?

I haven't yet tested on ARM host, hope to do so some time this week.

Paolo

[Qemu-devel] [PATCH v2 2/2] .travis.yml: Add "--disable-modules"

[Fam Zheng]

Now we default to "--enable-modules", let's cover the old way in travis.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 .travis.yml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/.travis.yml b/.travis.yml
index ad66e5b..12bf1db 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -98,3 +98,6 @@ matrix:
           EXTRA_PKGS="liblttng-ust-dev liburcu-dev"
           EXTRA_CONFIG="--enable-trace-backends=ust"
       compiler: gcc
+    - env: TARGETS=i386-softmmu,x86_64-softmmu
+           EXTRA_CONFIG="--disable-modules"
+      compiler: gcc
-- 
2.1.0