qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/3] configure: fix coroutine backend selection logic
@ 2013-03-14 15:25 Peter Maydell
  2013-03-14 15:25 ` [Qemu-devel] [PATCH 1/3] configure: Provide and use convenience error reporting function Peter Maydell
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Peter Maydell @ 2013-03-14 15:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, patches

The main aim of this patchset is patch 3, which changes the coroutine
backend selection logic so that it goes 'ucontext -> sigaltstack'
rather than 'ucontext -> gthread', since the gthread backend is
broken. To do this properly on all platforms we have to refactor
the code a bit (which it needed anyway).

The first two patches here are generic configure cleanups which
I wanted for the third patch.

Peter Maydell (3):
  configure: Provide and use convenience error reporting function
  configure: Move upper() up so it's available earlier
  configure: Don't fall back to gthread coroutine backend

 Makefile.objs |    6 +-
 configure     |  266 ++++++++++++++++++++++++++-------------------------------
 2 files changed, 125 insertions(+), 147 deletions(-)

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Qemu-devel] [PATCH 1/3] configure: Provide and use convenience error reporting function
  2013-03-14 15:25 [Qemu-devel] [PATCH 0/3] configure: fix coroutine backend selection logic Peter Maydell
@ 2013-03-14 15:25 ` Peter Maydell
  2013-03-14 15:25 ` [Qemu-devel] [PATCH 2/3] configure: Move upper() up so it's available earlier Peter Maydell
  2013-03-14 15:25 ` [Qemu-devel] [PATCH 3/3] configure: Don't fall back to gthread coroutine backend Peter Maydell
  2 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2013-03-14 15:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, patches

Provide a convenience function for reporting an error and exiting,
and update various places in the configure script to use it.
This allows us to be a little more consistent about how format
our error messages and makes the calling code shorter.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure |  193 ++++++++++++++++++++++++-------------------------------------
 1 file changed, 75 insertions(+), 118 deletions(-)

diff --git a/configure b/configure
index a382ff2..c674d39 100755
--- a/configure
+++ b/configure
@@ -27,6 +27,17 @@ printf " '%s'" "$0" "$@" >> config.log
 echo >> config.log
 echo "#" >> config.log
 
+error_exit() {
+    echo
+    echo "ERROR: $1"
+    while test -n "$2"; do
+        echo "       $2"
+        shift
+    done
+    echo
+    exit 1
+}
+
 do_cc() {
     # Run the compiler, capturing its output to the log.
     echo $cc "$@" >> config.log
@@ -46,11 +57,10 @@ do_cc() {
     esac
     echo $cc -Werror "$@" >> config.log
     $cc -Werror "$@" >> config.log 2>&1 && return $?
-    echo "ERROR: configure test passed without -Werror but failed with -Werror."
-    echo "This is probably a bug in the configure script. The failing command"
-    echo "will be at the bottom of config.log."
-    echo "You can run configure with --disable-werror to bypass this check."
-    exit 1
+    error_exit "configure test passed without -Werror but failed with -Werror." \
+        "This is probably a bug in the configure script. The failing command" \
+        "will be at the bottom of config.log." \
+        "You can run configure with --disable-werror to bypass this check."
 }
 
 compile_object() {
@@ -494,11 +504,10 @@ SunOS)
         LDFLAGS="-L/opt/SUNWspro/prod/lib -R/opt/SUNWspro/prod/lib $LDFLAGS"
         LIBS="-lsunmath $LIBS"
       else
-        echo "QEMU will not link correctly on Solaris 8/X86 or 9/x86 without"
-        echo "libsunmath from the Sun Studio compilers tools, due to a lack of"
-        echo "C99 math features in libm.so in Solaris 8/x86 and Solaris 9/x86"
-        echo "Studio 11 can be downloaded from www.sun.com."
-        exit 1
+        error_exit "QEMU will not link correctly on Solaris 8/X86 or 9/x86 without" \
+            "libsunmath from the Sun Studio compilers tools, due to a lack of" \
+            "C99 math features in libm.so in Solaris 8/x86 and Solaris 9/x86" \
+            "Studio 11 can be downloaded from www.sun.com."
       fi
     fi
   fi
@@ -1176,8 +1185,7 @@ if test "$ARCH" = "unknown"; then
         echo "Unsupported CPU = $cpu, will use TCG with TCI (experimental)"
         ARCH=tci
     else
-        echo "Unsupported CPU = $cpu, try --enable-tcg-interpreter"
-        exit 1
+        error_exit "Unsupported CPU = $cpu, try --enable-tcg-interpreter"
     fi
 fi
 
@@ -1189,8 +1197,7 @@ EOF
 if compile_object ; then
   : C compiler works ok
 else
-    echo "ERROR: \"$cc\" either does not exist or does not work"
-    exit 1
+    error_exit "\"$cc\" either does not exist or does not work"
 fi
 
 # Consult white-list to determine whether to enable werror
@@ -1245,8 +1252,7 @@ fi
 
 if test "$static" = "yes" ; then
   if test "$pie" = "yes" ; then
-    echo "static and pie are mutually incompatible"
-    exit 1
+    error_exit "static and pie are mutually incompatible"
   else
     pie="no"
   fi
@@ -1285,8 +1291,7 @@ EOF
     fi
   else
     if test "$pie" = "yes"; then
-      echo "PIE not available due to missing toolchain support"
-      exit 1
+      error_exit "PIE not available due to missing toolchain support"
     else
       echo "Disabling PIE due to missing toolchain support"
       pie="no"
@@ -1301,40 +1306,36 @@ if test "$solaris" = "yes" ; then
   if has $install; then
     :
   else
-    echo "Solaris install program not found. Use --install=/usr/ucb/install or"
-    echo "install fileutils from www.blastwave.org using pkg-get -i fileutils"
-    echo "to get ginstall which is used by default (which lives in /opt/csw/bin)"
-    exit 1
+    error_exit "Solaris install program not found. Use --install=/usr/ucb/install or" \
+        "install fileutils from www.blastwave.org using pkg-get -i fileutils" \
+        "to get ginstall which is used by default (which lives in /opt/csw/bin)"
   fi
   if test "`path_of $install`" = "/usr/sbin/install" ; then
-    echo "Error: Solaris /usr/sbin/install is not an appropriate install program."
-    echo "try ginstall from the GNU fileutils available from www.blastwave.org"
-    echo "using pkg-get -i fileutils, or use --install=/usr/ucb/install"
-    exit 1
+    error_exit "Solaris /usr/sbin/install is not an appropriate install program." \
+        "try ginstall from the GNU fileutils available from www.blastwave.org" \
+        "using pkg-get -i fileutils, or use --install=/usr/ucb/install"
   fi
   if has ar; then
     :
   else
-    echo "Error: No path includes ar"
     if test -f /usr/ccs/bin/ar ; then
-      echo "Add /usr/ccs/bin to your path and rerun configure"
+      error_exit "No path includes ar" \
+          "Add /usr/ccs/bin to your path and rerun configure"
     fi
-    exit 1
+    error_exit "No path includes ar"
   fi
 fi
 
 if ! has $python; then
-  echo "Python not found. Use --python=/path/to/python"
-  exit 1
+  error_exit "Python not found. Use --python=/path/to/python"
 fi
 
 # Note that if the Python conditional here evaluates True we will exit
 # with status 1 which is a shell 'false' value.
 if ! "$python" -c 'import sys; sys.exit(sys.version_info < (2,4) or sys.version_info >= (3,))'; then
-  echo "Cannot use '$python', Python 2.4 or later is required."
-  echo "Note that Python 3 or later is not yet supported."
-  echo "Use --python=/path/to/python to specify a supported Python."
-  exit 1
+  error_exit "Cannot use '$python', Python 2.4 or later is required." \
+      "Note that Python 3 or later is not yet supported." \
+      "Use --python=/path/to/python to specify a supported Python."
 fi
 
 if test -z "${target_list+xxx}" ; then
@@ -1353,11 +1354,8 @@ esac
 feature_not_found() {
   feature=$1
 
-  echo "ERROR"
-  echo "ERROR: User requested feature $feature"
-  echo "ERROR: configure was not able to find it"
-  echo "ERROR"
-  exit 1;
+  error_exit "User requested feature $feature" \
+      "configure was not able to find it"
 }
 
 if test -z "$cross_prefix" ; then
@@ -1399,8 +1397,7 @@ fi
 # pkg-config probe
 
 if ! has "$pkg_config_exe"; then
-  echo "Error: pkg-config binary '$pkg_config_exe' not found"
-  exit 1
+  error_exit "pkg-config binary '$pkg_config_exe' not found"
 fi
 
 ##########################################
@@ -1439,11 +1436,8 @@ EOF
     if compile_prog "" "-lz" ; then
         :
     else
-        echo
-        echo "Error: zlib check failed"
-        echo "Make sure to have the zlib libs and headers installed."
-        echo
-        exit 1
+        error_exit "zlib check failed" \
+            "Make sure to have the zlib libs and headers installed."
     fi
 fi
 
@@ -1620,14 +1614,12 @@ if test "$xen_pci_passthrough" != "no"; then
     xen_pci_passthrough=yes
   else
     if test "$xen_pci_passthrough" = "yes"; then
-      echo "ERROR"
-      echo "ERROR: User requested feature Xen PCI Passthrough"
-      echo "ERROR: but this feature require /sys from Linux"
       if test "$xen_ctrl_version" -lt 340; then
-        echo "ERROR: This feature does not work with Xen 3.3"
+        error_exit "User requested feature Xen PCI Passthrough" \
+            "This feature does not work with Xen 3.3"
       fi
-      echo "ERROR"
-      exit 1;
+      error_exit "User requested feature Xen PCI Passthrough" \
+          " but this feature requires /sys from Linux"
     fi
     xen_pci_passthrough=no
   fi
@@ -2001,11 +1993,8 @@ EOF
     if compile_prog "$cfl" "$lib" ; then
         :
     else
-        echo
-        echo "Error: $drv check failed"
-        echo "Make sure to have the $drv libs and headers installed."
-        echo
-        exit 1
+        error_exit "$drv check failed" \
+            "Make sure to have the $drv libs and headers installed."
     fi
 }
 
@@ -2020,11 +2009,8 @@ for drv in $audio_drv_list; do
 
     fmod)
     if test -z $fmod_lib || test -z $fmod_inc; then
-        echo
-        echo "Error: You must specify path to FMOD library and headers"
-        echo "Example: --fmod-inc=/path/include/fmod --fmod-lib=/path/lib/libfmod-3.74.so"
-        echo
-        exit 1
+        error_exit "You must specify path to FMOD library and headers" \
+            "Example: --fmod-inc=/path/include/fmod --fmod-lib=/path/lib/libfmod-3.74.so"
     fi
     audio_drv_probe $drv fmod.h $fmod_lib "return FSOUND_GetVersion();" "-I $fmod_inc"
     libs_softmmu="$fmod_lib $libs_softmmu"
@@ -2067,11 +2053,8 @@ for drv in $audio_drv_list; do
 
     *)
     echo "$audio_possible_drivers" | grep -q "\<$drv\>" || {
-        echo
-        echo "Error: Unknown driver '$drv' selected"
-        echo "Possible drivers are: $audio_possible_drivers"
-        echo
-        exit 1
+        error_exit "Unknown driver '$drv' selected" \
+            "Possible drivers are: $audio_possible_drivers"
     }
     ;;
     esac
@@ -2200,8 +2183,7 @@ then
     LIBS="$glib_libs $LIBS"
     libs_qga="$glib_libs $libs_qga"
 else
-    echo "glib-$glib_req_ver required to compile QEMU"
-    exit 1
+    error_exit "glib-$glib_req_ver required to compile QEMU"
 fi
 
 ##########################################
@@ -2218,11 +2200,10 @@ if test "$pixman" = ""; then
 fi
 if test "$pixman" = "none"; then
   if test "$want_tools" != "no" -o "$softmmu" != "no"; then
-    echo "ERROR: pixman disabled but system emulation or tools build"
-    echo "       enabled.  You can turn off pixman only if you also"
-    echo "       disable all system emulation targets and the tools"
-    echo "       build with '--disable-tools --disable-system'."
-    exit 1
+    error_exit "pixman disabled but system emulation or tools build" \
+        "enabled.  You can turn off pixman only if you also" \
+        "disable all system emulation targets and the tools" \
+        "build with '--disable-tools --disable-system'."
   fi
   pixman_cflags=
   pixman_libs=
@@ -2231,12 +2212,11 @@ elif test "$pixman" = "system"; then
   pixman_libs=`$pkg_config --libs pixman-1 2>/dev/null`
 else
   if test ! -d ${source_path}/pixman/pixman; then
-    echo "ERROR: pixman not present. Your options:"
-    echo "  (1) Preferred: Install the pixman devel package (any recent"
-    echo "      distro should have packages as Xorg needs pixman too)."
-    echo "  (2) Fetch the pixman submodule, using:"
-    echo "      git submodule update --init pixman"
-    exit 1
+    error_exit "pixman not present. Your options:" \
+        "  (1) Preferred: Install the pixman devel package (any recent" \
+        "      distro should have packages as Xorg needs pixman too)." \
+        "  (2) Fetch the pixman submodule, using:" \
+        "      git submodule update --init pixman"
   fi
   mkdir -p pixman/pixman
   pixman_cflags="-I\$(SRC_PATH)/pixman/pixman -I\$(BUILD_DIR)/pixman/pixman"
@@ -2295,11 +2275,8 @@ else
 fi
 
 if test "$mingw32" != yes -a "$pthread" = no; then
-  echo
-  echo "Error: pthread check failed"
-  echo "Make sure to have the pthread libs and headers installed."
-  echo
-  exit 1
+  error_exit "pthread check failed" \
+      "Make sure to have the pthread libs and headers installed."
 fi
 
 ##########################################
@@ -2354,8 +2331,7 @@ fi
 
 if test "$virtio_blk_data_plane" = "yes" -a \
 	"$linux_aio" != "yes" ; then
-  echo "Error: virtio-blk-data-plane requires Linux AIO, please try --enable-linux-aio"
-  exit 1
+  error_exit "virtio-blk-data-plane requires Linux AIO, please try --enable-linux-aio"
 elif test -z "$virtio_blk_data_plane" ; then
   virtio_blk_data_plane=$linux_aio
 fi
@@ -2838,10 +2814,7 @@ elif compile_prog "" "-lm" ; then
   LIBS="-lm $LIBS"
   libs_qga="-lm $libs_qga"
 else
-  echo
-  echo "Error: libm check failed"
-  echo
-  exit 1
+  error_exit "libm check failed"
 fi
 
 ##########################################
@@ -3017,11 +2990,8 @@ fi
 
 $python "$source_path/scripts/tracetool.py" "--backend=$trace_backend" --check-backend  > /dev/null 2> /dev/null
 if test "$?" -ne 0 ; then
-  echo
-  echo "Error: invalid trace backend"
-  echo "Please choose a supported trace backend."
-  echo
-  exit 1
+  error_exit "invalid trace backend" \
+      "Please choose a supported trace backend."
 fi
 
 ##########################################
@@ -3036,10 +3006,7 @@ EOF
     LIBS="-lust -lurcu-bp $LIBS"
     libs_qga="-lust -lurcu-bp $libs_qga"
   else
-    echo
-    echo "Error: Trace backend 'ust' missing libust header files"
-    echo
-    exit 1
+    error_exit "Trace backend 'ust' missing libust header files"
   fi
 fi
 
@@ -3047,10 +3014,7 @@ fi
 # For 'dtrace' backend, test if 'dtrace' command is present
 if test "$trace_backend" = "dtrace"; then
   if ! has 'dtrace' ; then
-    echo
-    echo "Error: dtrace command is not found in PATH $PATH"
-    echo
-    exit 1
+    error_exit "dtrace command is not found in PATH $PATH"
   fi
   trace_backend_stap="no"
   if has 'stap' ; then
@@ -3108,10 +3072,7 @@ elif test "$coroutine" = "windows" ; then
 elif test "$coroutine" = "sigaltstack" ; then
   coroutine_backend=sigaltstack
 else
-  echo
-  echo "Error: unknown coroutine backend $coroutine"
-  echo
-  exit 1
+  error_exit "unknown coroutine backend $coroutine"
 fi
 
 ##########################################
@@ -3296,8 +3257,7 @@ if test "$softmmu" = yes ; then
       tools="$tools fsdev/virtfs-proxy-helper\$(EXESUF)"
     else
       if test "$virtfs" = yes; then
-        echo "VirtFS is supported only on Linux and requires libcap-devel and libattr-devel"
-        exit 1
+        error_exit "VirtFS is supported only on Linux and requires libcap-devel and libattr-devel"
       fi
       virtfs=no
     fi
@@ -3943,22 +3903,20 @@ case "$target" in
     ;;
   ${target_arch2}-linux-user)
     if test "$linux" != "yes" ; then
-      echo "ERROR: Target '$target' is only available on a Linux host"
-      exit 1
+      error_exit "Target '$target' is only available on a Linux host"
     fi
     target_user_only="yes"
     target_linux_user="yes"
     ;;
   ${target_arch2}-bsd-user)
     if test "$bsd" != "yes" ; then
-      echo "ERROR: Target '$target' is only available on a BSD host"
-      exit 1
+      error_exit "Target '$target' is only available on a BSD host"
     fi
     target_user_only="yes"
     target_bsd_user="yes"
     ;;
   *)
-    echo "ERROR: Target '$target' not recognised"
+    error_exit "Target '$target' not recognised"
     exit 1
     ;;
 esac
@@ -4087,8 +4045,7 @@ case "$target_arch2" in
     TARGET_ARCH=xtensa
   ;;
   *)
-    echo "Unsupported target CPU"
-    exit 1
+    error_exit "Unsupported target CPU"
   ;;
 esac
 # TARGET_BASE_ARCH needs to be defined after TARGET_ARCH
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [Qemu-devel] [PATCH 2/3] configure: Move upper() up so it's available earlier
  2013-03-14 15:25 [Qemu-devel] [PATCH 0/3] configure: fix coroutine backend selection logic Peter Maydell
  2013-03-14 15:25 ` [Qemu-devel] [PATCH 1/3] configure: Provide and use convenience error reporting function Peter Maydell
@ 2013-03-14 15:25 ` Peter Maydell
  2013-03-14 15:25 ` [Qemu-devel] [PATCH 3/3] configure: Don't fall back to gthread coroutine backend Peter Maydell
  2 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2013-03-14 15:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, patches

Move the definition of the upper() utility function up to
the top of the script, so that it's available to any code that
wants to use it.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index c674d39..d9dfde0 100755
--- a/configure
+++ b/configure
@@ -27,6 +27,10 @@ printf " '%s'" "$0" "$@" >> config.log
 echo >> config.log
 echo "#" >> config.log
 
+upper() {
+    echo "$@"| LC_ALL=C tr '[a-z]' '[A-Z]'
+}
+
 error_exit() {
     echo
     echo "ERROR: $1"
@@ -4055,10 +4059,6 @@ fi
 
 symlink "$source_path/Makefile.target" "$target_dir/Makefile"
 
-upper() {
-    echo "$@"| LC_ALL=C tr '[a-z]' '[A-Z]'
-}
-
 case "$cpu" in
   i386|x86_64|ppc)
     # The TCG interpreter currently does not support ld/st optimization.
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [Qemu-devel] [PATCH 3/3] configure: Don't fall back to gthread coroutine backend
  2013-03-14 15:25 [Qemu-devel] [PATCH 0/3] configure: fix coroutine backend selection logic Peter Maydell
  2013-03-14 15:25 ` [Qemu-devel] [PATCH 1/3] configure: Provide and use convenience error reporting function Peter Maydell
  2013-03-14 15:25 ` [Qemu-devel] [PATCH 2/3] configure: Move upper() up so it's available earlier Peter Maydell
@ 2013-03-14 15:25 ` Peter Maydell
  2013-03-14 15:54   ` Paolo Bonzini
  2 siblings, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2013-03-14 15:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, patches

The gthread coroutine backend is broken and does not produce a working
QEMU; it is only useful for some very limited debugging situations.
Clean up the backend selection logic in configure so that it now runs
"if on windows use windows; else prefer ucontext; else sigaltstack".

To do this we refactor the configure code to separate out "test
whether we have a working ucontext", "pick a default if user didn't
specify" and "validate that user didn't specify something invalid",
rather than having all three of these run together. We also have to
adjust the Makefile logic so it doesn't also encode an idea of the
default backend.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 Makefile.objs |    6 +++---
 configure     |   67 +++++++++++++++++++++++++++++++++++++--------------------
 2 files changed, 47 insertions(+), 26 deletions(-)

diff --git a/Makefile.objs b/Makefile.objs
index f99841c..7f541a4 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -18,12 +18,12 @@ block-obj-y += qemu-coroutine.o qemu-coroutine-lock.o qemu-coroutine-io.o
 block-obj-y += qemu-coroutine-sleep.o
 ifeq ($(CONFIG_UCONTEXT_COROUTINE),y)
 block-obj-$(CONFIG_POSIX) += coroutine-ucontext.o
-else
+endif
 ifeq ($(CONFIG_SIGALTSTACK_COROUTINE),y)
 block-obj-$(CONFIG_POSIX) += coroutine-sigaltstack.o
-else
-block-obj-$(CONFIG_POSIX) += coroutine-gthread.o
 endif
+ifeq ($(CONFIG_GTHREAD_COROUTINE),y)
+block-obj-$(CONFIG_POSIX) += coroutine-gthread.o
 endif
 block-obj-$(CONFIG_WIN32) += coroutine-win32.o
 
diff --git a/configure b/configure
index d9dfde0..9f7f5cd 100755
--- a/configure
+++ b/configure
@@ -3052,31 +3052,55 @@ fi
 ##########################################
 # check and set a backend for coroutine
 
-# default is ucontext, but always fallback to gthread
-# windows autodetected by make
-if test "$coroutine" = "" -o "$coroutine" = "ucontext"; then
-  if test "$darwin" != "yes"; then
-    cat > $TMPC << EOF
+# We prefer ucontext, but it's not always possible. The fallback
+# is sigcontext. gthread is not selectable except explicitly, because
+# it is not functional enough to run QEMU proper. (It is occasionally
+# useful for debugging purposes.)  On Windows the only valid backend
+# is the Windows-specific one.
+
+ucontext_works=no
+if test "$darwin" != "yes"; then
+  cat > $TMPC << EOF
 #include <ucontext.h>
 #ifdef __stub_makecontext
 #error Ignoring glibc stub makecontext which will always fail
 #endif
 int main(void) { makecontext(0, 0, 0); return 0; }
 EOF
-    if compile_prog "" "" ; then
-        coroutine_backend=ucontext
-    else
-	coroutine_backend=gthread
-    fi
+  if compile_prog "" "" ; then
+    ucontext_works=yes
+  fi
+fi
+
+if test "$coroutine" = ""; then
+  if test "$mingw32" = "yes"; then
+    coroutine=windows
+  elif test "$ucontext_works" = "yes"; then
+    coroutine=ucontext
+  else
+    coroutine=sigaltstack
   fi
-elif test "$coroutine" = "gthread" ; then
-  coroutine_backend=gthread
-elif test "$coroutine" = "windows" ; then
-  coroutine_backend=windows
-elif test "$coroutine" = "sigaltstack" ; then
-  coroutine_backend=sigaltstack
 else
-  error_exit "unknown coroutine backend $coroutine"
+  case $coroutine in
+  windows)
+    if test "$mingw32" != "yes"; then
+      error_exit "'windows' coroutine backend only valid for Windows"
+    fi
+    ;;
+  ucontext)
+    if test "$ucontext_works" != "yes"; then
+      feature_not_found "ucontext"
+    fi
+    ;;
+  gthread|sigaltstack)
+    if test "$mingw32" = "yes"; then
+      error_exit "only the 'windows' coroutine backend is valid for Windows"
+    fi
+    ;;
+  *)
+    error_exit "unknown coroutine backend $coroutine"
+    ;;
+  esac
 fi
 
 ##########################################
@@ -3383,7 +3407,7 @@ echo "OpenGL support    $opengl"
 echo "libiscsi support  $libiscsi"
 echo "build guest agent $guest_agent"
 echo "seccomp support   $seccomp"
-echo "coroutine backend $coroutine_backend"
+echo "coroutine backend $coroutine"
 echo "GlusterFS support $glusterfs"
 echo "virtio-blk-data-plane $virtio_blk_data_plane"
 echo "gcov              $gcov_tool"
@@ -3713,11 +3737,8 @@ if test "$rbd" = "yes" ; then
   echo "CONFIG_RBD=y" >> $config_host_mak
 fi
 
-if test "$coroutine_backend" = "ucontext" ; then
-  echo "CONFIG_UCONTEXT_COROUTINE=y" >> $config_host_mak
-elif test "$coroutine_backend" = "sigaltstack" ; then
-  echo "CONFIG_SIGALTSTACK_COROUTINE=y" >> $config_host_mak
-fi
+def="CONFIG_$(upper $coroutine)_COROUTINE"
+echo "$def=y" >> $config_host_mak
 
 if test "$open_by_handle_at" = "yes" ; then
   echo "CONFIG_OPEN_BY_HANDLE=y" >> $config_host_mak
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] configure: Don't fall back to gthread coroutine backend
  2013-03-14 15:25 ` [Qemu-devel] [PATCH 3/3] configure: Don't fall back to gthread coroutine backend Peter Maydell
@ 2013-03-14 15:54   ` Paolo Bonzini
  2013-03-14 16:43     ` Peter Maydell
  0 siblings, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2013-03-14 15:54 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-devel, patches

Il 14/03/2013 16:25, Peter Maydell ha scritto:
> The gthread coroutine backend is broken and does not produce a working
> QEMU; it is only useful for some very limited debugging situations.
> Clean up the backend selection logic in configure so that it now runs
> "if on windows use windows; else prefer ucontext; else sigaltstack".
> 
> To do this we refactor the configure code to separate out "test
> whether we have a working ucontext", "pick a default if user didn't
> specify" and "validate that user didn't specify something invalid",
> rather than having all three of these run together. We also have to
> adjust the Makefile logic so it doesn't also encode an idea of the
> default backend.

You need to fix also tests/Makefile (don't mention it, I missed the boat
for reviewing the patch on qemu-devel).

Perhaps it's simpler to export a single CONFIG_COROUTINE_BACKEND symbol
from configure to the Makefile?  (This way patch 2 also becomes
unnecessary).

Patch 1 looks good.

Thanks,

Paolo

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  Makefile.objs |    6 +++---
>  configure     |   67 +++++++++++++++++++++++++++++++++++++--------------------
>  2 files changed, 47 insertions(+), 26 deletions(-)
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index f99841c..7f541a4 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -18,12 +18,12 @@ block-obj-y += qemu-coroutine.o qemu-coroutine-lock.o qemu-coroutine-io.o
>  block-obj-y += qemu-coroutine-sleep.o
>  ifeq ($(CONFIG_UCONTEXT_COROUTINE),y)
>  block-obj-$(CONFIG_POSIX) += coroutine-ucontext.o
> -else
> +endif
>  ifeq ($(CONFIG_SIGALTSTACK_COROUTINE),y)
>  block-obj-$(CONFIG_POSIX) += coroutine-sigaltstack.o
> -else
> -block-obj-$(CONFIG_POSIX) += coroutine-gthread.o
>  endif
> +ifeq ($(CONFIG_GTHREAD_COROUTINE),y)
> +block-obj-$(CONFIG_POSIX) += coroutine-gthread.o
>  endif
>  block-obj-$(CONFIG_WIN32) += coroutine-win32.o
>  
> diff --git a/configure b/configure
> index d9dfde0..9f7f5cd 100755
> --- a/configure
> +++ b/configure
> @@ -3052,31 +3052,55 @@ fi
>  ##########################################
>  # check and set a backend for coroutine
>  
> -# default is ucontext, but always fallback to gthread
> -# windows autodetected by make
> -if test "$coroutine" = "" -o "$coroutine" = "ucontext"; then
> -  if test "$darwin" != "yes"; then
> -    cat > $TMPC << EOF
> +# We prefer ucontext, but it's not always possible. The fallback
> +# is sigcontext. gthread is not selectable except explicitly, because
> +# it is not functional enough to run QEMU proper. (It is occasionally
> +# useful for debugging purposes.)  On Windows the only valid backend
> +# is the Windows-specific one.
> +
> +ucontext_works=no
> +if test "$darwin" != "yes"; then
> +  cat > $TMPC << EOF
>  #include <ucontext.h>
>  #ifdef __stub_makecontext
>  #error Ignoring glibc stub makecontext which will always fail
>  #endif
>  int main(void) { makecontext(0, 0, 0); return 0; }
>  EOF
> -    if compile_prog "" "" ; then
> -        coroutine_backend=ucontext
> -    else
> -	coroutine_backend=gthread
> -    fi
> +  if compile_prog "" "" ; then
> +    ucontext_works=yes
> +  fi
> +fi
> +
> +if test "$coroutine" = ""; then
> +  if test "$mingw32" = "yes"; then
> +    coroutine=windows
> +  elif test "$ucontext_works" = "yes"; then
> +    coroutine=ucontext
> +  else
> +    coroutine=sigaltstack
>    fi
> -elif test "$coroutine" = "gthread" ; then
> -  coroutine_backend=gthread
> -elif test "$coroutine" = "windows" ; then
> -  coroutine_backend=windows
> -elif test "$coroutine" = "sigaltstack" ; then
> -  coroutine_backend=sigaltstack
>  else
> -  error_exit "unknown coroutine backend $coroutine"
> +  case $coroutine in
> +  windows)
> +    if test "$mingw32" != "yes"; then
> +      error_exit "'windows' coroutine backend only valid for Windows"
> +    fi
> +    ;;
> +  ucontext)
> +    if test "$ucontext_works" != "yes"; then
> +      feature_not_found "ucontext"
> +    fi
> +    ;;
> +  gthread|sigaltstack)
> +    if test "$mingw32" = "yes"; then
> +      error_exit "only the 'windows' coroutine backend is valid for Windows"
> +    fi
> +    ;;
> +  *)
> +    error_exit "unknown coroutine backend $coroutine"
> +    ;;
> +  esac
>  fi
>  
>  ##########################################
> @@ -3383,7 +3407,7 @@ echo "OpenGL support    $opengl"
>  echo "libiscsi support  $libiscsi"
>  echo "build guest agent $guest_agent"
>  echo "seccomp support   $seccomp"
> -echo "coroutine backend $coroutine_backend"
> +echo "coroutine backend $coroutine"
>  echo "GlusterFS support $glusterfs"
>  echo "virtio-blk-data-plane $virtio_blk_data_plane"
>  echo "gcov              $gcov_tool"
> @@ -3713,11 +3737,8 @@ if test "$rbd" = "yes" ; then
>    echo "CONFIG_RBD=y" >> $config_host_mak
>  fi
>  
> -if test "$coroutine_backend" = "ucontext" ; then
> -  echo "CONFIG_UCONTEXT_COROUTINE=y" >> $config_host_mak
> -elif test "$coroutine_backend" = "sigaltstack" ; then
> -  echo "CONFIG_SIGALTSTACK_COROUTINE=y" >> $config_host_mak
> -fi
> +def="CONFIG_$(upper $coroutine)_COROUTINE"
> +echo "$def=y" >> $config_host_mak
>  
>  if test "$open_by_handle_at" = "yes" ; then
>    echo "CONFIG_OPEN_BY_HANDLE=y" >> $config_host_mak
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] configure: Don't fall back to gthread coroutine backend
  2013-03-14 15:54   ` Paolo Bonzini
@ 2013-03-14 16:43     ` Peter Maydell
  0 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2013-03-14 16:43 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel, patches

On 14 March 2013 15:54, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 14/03/2013 16:25, Peter Maydell ha scritto:
>> The gthread coroutine backend is broken and does not produce a working
>> QEMU; it is only useful for some very limited debugging situations.
>> Clean up the backend selection logic in configure so that it now runs
>> "if on windows use windows; else prefer ucontext; else sigaltstack".
>>
>> To do this we refactor the configure code to separate out "test
>> whether we have a working ucontext", "pick a default if user didn't
>> specify" and "validate that user didn't specify something invalid",
>> rather than having all three of these run together. We also have to
>> adjust the Makefile logic so it doesn't also encode an idea of the
>> default backend.
>
> You need to fix also tests/Makefile (don't mention it, I missed the boat
> for reviewing the patch on qemu-devel).
>
> Perhaps it's simpler to export a single CONFIG_COROUTINE_BACKEND symbol
> from configure to the Makefile?  (This way patch 2 also becomes
> unnecessary).

Yes, that might be good. (Patch 2 we want anyway because there
are a few other places in configure that are currently using
hand-coded tr invocations, but it needn't be in this series
in that case).

-- PMM

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-03-14 16:43 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-14 15:25 [Qemu-devel] [PATCH 0/3] configure: fix coroutine backend selection logic Peter Maydell
2013-03-14 15:25 ` [Qemu-devel] [PATCH 1/3] configure: Provide and use convenience error reporting function Peter Maydell
2013-03-14 15:25 ` [Qemu-devel] [PATCH 2/3] configure: Move upper() up so it's available earlier Peter Maydell
2013-03-14 15:25 ` [Qemu-devel] [PATCH 3/3] configure: Don't fall back to gthread coroutine backend Peter Maydell
2013-03-14 15:54   ` Paolo Bonzini
2013-03-14 16:43     ` Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).