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

[Brad Smith]

On Sun, Apr 14, 2013 at 02:24:06AM -0400, Brad Smith wrote:
> This is a back port of 7c2acc7062fe863cb71ff5849bb121deafe8df4b to the
> 1.4 stable branch without needing the new error_exit() function.
> 
> configure: Don't fall back to gthread coroutine backend
> 
> 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 simplify
> the Makefile logic so it just links in the backend the configure
> script selects.
> 
> Signed-off-by: Peter Maydell <address@hidden>
> Message-id: address@hidden
> Signed-off-by: Anthony Liguori <address@hidden>

Signed-off-by: Brad Smith <address@hidden>

> diff --git a/Makefile.objs b/Makefile.objs
> index 21e9c91..3884790 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -16,16 +16,7 @@ block-obj-y += qapi-types.o qapi-visit.o
>  
>  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
> -ifeq ($(CONFIG_SIGALTSTACK_COROUTINE),y)
> -block-obj-$(CONFIG_POSIX) += coroutine-sigaltstack.o
> -else
> -block-obj-$(CONFIG_POSIX) += coroutine-gthread.o
> -endif
> -endif
> -block-obj-$(CONFIG_WIN32) += coroutine-win32.o
> +block-obj-y += coroutine-$(CONFIG_COROUTINE_BACKEND).o
>  
>  ifeq ($(CONFIG_VIRTIO)$(CONFIG_VIRTFS)$(CONFIG_PCI),yyy)
>  # Lots of the fsdev/9pcode is pulled in by vl.c via qemu_fsdev_add.
> diff --git a/configure b/configure
> index e7468a9..8271757 100755
> --- a/configure
> +++ b/configure
> @@ -3035,34 +3035,67 @@ 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=win32
> +  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
> -  echo
> -  echo "Error: unknown coroutine backend $coroutine"
> -  echo
> -  exit 1
> +  case $coroutine in
> +  windows)
> +    if test "$mingw32" != "yes"; then
> +      echo
> +      echo "Error: 'windows' coroutine backend only valid for Windows"
> +      echo
> +      exit 1
> +    fi
> +    # Unfortunately the user visible backend name doesn't match the
> +    # coroutine-*.c filename for this case, so we have to adjust it here.
> +    coroutine=win32
> +    ;;
> +  ucontext)
> +    if test "$ucontext_works" != "yes"; then
> +      feature_not_found "ucontext"
> +    fi
> +    ;;
> +  gthread|sigaltstack)
> +    if test "$mingw32" = "yes"; then
> +      echo
> +      echo "Error: only the 'windows' coroutine backend is valid for Windows"
> +      echo
> +      exit 1
> +    fi
> +    ;;
> +  *)
> +    echo
> +    echo "Error: unknown coroutine backend $coroutine"
> +    echo
> +    exit 1
> +    ;;
> +  esac
>  fi
>  
>  ##########################################
> @@ -3345,7 +3378,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"
> @@ -3668,11 +3701,7 @@ 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
> +echo "CONFIG_COROUTINE_BACKEND=$coroutine" >> $config_host_mak
>  
>  if test "$open_by_handle_at" = "yes" ; then
>    echo "CONFIG_OPEN_BY_HANDLE=y" >> $config_host_mak
> diff --git a/tests/Makefile b/tests/Makefile
> index a2d62b8..894446d 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -24,19 +24,7 @@ gcov-files-test-string-input-visitor-y = 
> qapi/string-input-visitor.c
>  check-unit-y += tests/test-string-output-visitor$(EXESUF)
>  gcov-files-test-string-output-visitor-y = qapi/string-output-visitor.c
>  check-unit-y += tests/test-coroutine$(EXESUF)
> -ifeq ($(CONFIG_WIN32),y)
> -gcov-files-test-coroutine-y = coroutine-win32.c
> -else
> -ifeq ($(CONFIG_UCONTEXT_COROUTINE),y)
> -gcov-files-test-coroutine-y = coroutine-ucontext.c
> -else
> -ifeq ($(CONFIG_SIGALTSTACK_COROUTINE),y)
> -gcov-files-test-coroutine-y = coroutine-sigaltstack.c
> -else
> -gcov-files-test-coroutine-y = coroutine-gthread.c
> -endif
> -endif
> -endif
> +gcov-files-test-coroutine-y = coroutine-$(CONFIG_COROUTINE_BACKEND).c
>  check-unit-y += tests/test-visitor-serialization$(EXESUF)
>  check-unit-y += tests/test-iov$(EXESUF)
>  gcov-files-test-iov-y = util/iov.c

-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.