| Message ID | 1363274720-22535-4-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
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 >
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
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
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(-)