Re: [Qemu-devel] [PATCH 3/3] configure: Don't fall back to gthread coroutine backend

[Paolo Bonzini]

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 <address@hidden>
> ---
>  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
>