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

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

[Fam Zheng]

The first patch fixes the module build: it doesn't fail but it links all the
module objects into executables.

The second patch enables modules by default.

The last patch adds a non-module build in travis config.


Fam Zheng (3):
  rules.mak: Fix module build
  configure: Default to enable module build
  .travis.yml: Add "--disable-modules"

 .travis.yml |  3 ++
 configure   | 96 ++++++++++++++++++++++++++++++++++++++++++-------------------
 rules.mak   | 23 ++++++++-------
 3 files changed, 82 insertions(+), 40 deletions(-)

-- 
2.1.0

[Qemu-devel] [PATCH 1/3] rules.mak: Fix module build

[Fam Zheng]

Module build is broken since commit c261d774fb ( rules.mak: Fix DSO
build by pulling in archive symbols). That commit added .mo placeholders
of DSO to -y variables, in order to pull stub symbols to executable. But
the placeholders are unintentionally expanded in -y, rather than
filtered out while linking.

Fix it by moving the -objs expanding to before inserting .mo
placeholders.  Note that passing -cflags and -libs to member objects are
also moved to keep it happening before object expanding.

Reported-by: Bharata B Rao <bharata.rao@gmail.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 rules.mak | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/rules.mak b/rules.mak
index f500fef..3a05627 100644
--- a/rules.mak
+++ b/rules.mak
@@ -326,7 +326,17 @@ define unnest-vars
     $(if $1,$(call fix-paths,$1/,,$2))
 
     # Descend and include every subdir Makefile.objs
-    $(foreach v, $2, $(call unnest-var-recursive,$1,$2,$v))
+    $(foreach v, $2,
+        $(call unnest-var-recursive,$1,$2,$v)
+        # Pass the .mo-cflags and .mo-libs along to its member objects
+        $(foreach o, $(filter %.mo,$($v)),
+            $(foreach p,$($o-objs),
+                $(if $($o-cflags), $(eval $p-cflags += $($o-cflags)))
+                $(if $($o-libs), $(eval $p-libs += $($o-libs))))))
+
+    # For all %.mo objects that are directly added into -y, just expand them
+    $(foreach v,$(filter %-y,$2),
+        $(eval $v := $(foreach o,$($v),$(if $($o-objs),$($o-objs),$o))))
 
     $(foreach v,$(filter %-m,$2),
         # All .o found in *-m variables are single object modules, create .mo
@@ -353,18 +363,9 @@ define unnest-vars
             # according to .mo-objs. Report error if not set
             $(if $($o-objs),
                 $(eval $(o:%.mo=%$(DSOSUF)): module-common.o $($o-objs)),
-                $(error $o added in $v but $o-objs is not set))
-            # Pass the .mo-cflags and .mo-libs along to member objects
-            $(foreach p,$($o-objs),
-                $(if $($o-cflags), $(eval $p-cflags += $($o-cflags)))
-                $(if $($o-libs), $(eval $p-libs += $($o-libs)))))
+                $(error $o added in $v but $o-objs is not set)))
         $(shell mkdir -p ./ $(sort $(dir $($v))))
         # Include all the .d files
         $(eval -include $(addsuffix *.d, $(sort $(dir $($v)))))
         $(eval $v := $(filter-out %/,$($v))))
-
-    # For all %.mo objects that are directly added into -y, expand them to %.mo-objs
-    $(foreach v,$2,
-        $(eval $v := $(foreach o,$($v),$(if $($o-objs),$($o-objs),$o))))
-
 endef
-- 
2.1.0

[Qemu-devel] [PATCH 2/3] 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 | 96 ++++++++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 67 insertions(+), 29 deletions(-)

diff --git a/configure b/configure
index 7539645..2015179 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,26 @@ 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
+    error_exit "glib-$glib_req_ver $i is required to compile QEMU"
+  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 +2731,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 2/3] configure: Default to enable module build

[Andreas Färber]

Am 12.01.2015 um 05:43 schrieb 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>

General idea seems okay, however an error handling nit below.

> ---
>  configure | 96 ++++++++++++++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 67 insertions(+), 29 deletions(-)
> 
> diff --git a/configure b/configure
> index 7539645..2015179 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,26 @@ 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
> +    error_exit "glib-$glib_req_ver $i is required to compile QEMU"

Is this error_exit ever executed? I.e., shouldn't the two lines be
reordered?

> +  fi
> +}
> +
> +glib_module_try_config gthread-2.0 || \
> +  error_exit "glib-$glib_req_ver gthread-2.0 is required to compile QEMU"

Depending on the above, we might drop this error_exit duplication?

Regards,
Andreas

>  
>  # g_test_trap_subprocess added in 2.38. Used by some tests.
>  glib_subprocess=yes
> @@ -2723,19 +2731,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

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)

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

[Fam Zheng]

On Mon, 01/12 11:26, Andreas Färber wrote:
> Am 12.01.2015 um 05:43 schrieb 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>
> 
> General idea seems okay, however an error handling nit below.
> 
> > ---
> >  configure | 96 ++++++++++++++++++++++++++++++++++++++++++++-------------------
> >  1 file changed, 67 insertions(+), 29 deletions(-)
> > 
> > diff --git a/configure b/configure
> > index 7539645..2015179 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,26 @@ 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
> > +    error_exit "glib-$glib_req_ver $i is required to compile QEMU"
> 
> Is this error_exit ever executed? I.e., shouldn't the two lines be
> reordered?
> 
> > +  fi
> > +}
> > +
> > +glib_module_try_config gthread-2.0 || \
> > +  error_exit "glib-$glib_req_ver gthread-2.0 is required to compile QEMU"
> 
> Depending on the above, we might drop this error_exit duplication?
> 

Will remove the above error_exit and leave to caller to do flow control and
error reporting.

Thanks!

Fam

[Qemu-devel] [PATCH 3/3] .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

Re: [Qemu-devel] [PATCH 0/3] buildsys: Fix and enable module build

[Bharata B Rao]

On Mon, Jan 12, 2015 at 10:13 AM, Fam Zheng <famz@redhat.com> wrote:
> The first patch fixes the module build: it doesn't fail but it links all the
> module objects into executables.
>
> The second patch enables modules by default.
>
> The last patch adds a non-module build in travis config.

Tested and verified this patch series with gluster block driver.
Tested default --enable-modules and --disable-modules options.

Regards,
Bharata.

Re: [Qemu-devel] [PATCH 0/3] buildsys: Fix and enable module build

[Paolo Bonzini]

On 12/01/2015 05:43, Fam Zheng wrote:
> The first patch fixes the module build: it doesn't fail but it links all the
> module objects into executables.
> 
> The second patch enables modules by default.
> 
> The last patch adds a non-module build in travis config.
> 
> 
> Fam Zheng (3):
>   rules.mak: Fix module build
>   configure: Default to enable module build
>   .travis.yml: Add "--disable-modules"
> 
>  .travis.yml |  3 ++
>  configure   | 96 ++++++++++++++++++++++++++++++++++++++++++-------------------
>  rules.mak   | 23 ++++++++-------
>  3 files changed, 82 insertions(+), 40 deletions(-)
> 

Applied patch 1 for now.  The change to default to enable modules will
be in a separate pull request.

Paolo