Re: [PATCH 23/30] build: move coroutine backend selection to meson

[Marc-André Lureau]

On Fri, Dec 9, 2022 at 3:37 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> To simplify the code, rename coroutine-win32.c to match the option
> passed to configure.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  configure                                     | 62 -------------------
>  meson.build                                   | 32 +++++++++-
>  meson_options.txt                             |  3 +
>  scripts/meson-buildoptions.py                 |  1 +
>  scripts/meson-buildoptions.sh                 |  3 +
>  ...{coroutine-win32.c => coroutine-windows.c} |  0
>  util/meson.build                              |  2 +-
>  7 files changed, 38 insertions(+), 65 deletions(-)
>  rename util/{coroutine-win32.c => coroutine-windows.c} (100%)
>
> diff --git a/configure b/configure
> index fea9cbf3abd0..1f7c5bbba4b9 100755
> --- a/configure
> +++ b/configure
> @@ -275,7 +275,6 @@ softmmu="yes"
>  linux_user=""
>  bsd_user=""
>  pie=""
> -coroutine=""
>  plugins="$default_feature"
>  meson=""
>  ninja=""
> @@ -792,8 +791,6 @@ for opt do
>    ;;
>    --enable-fdt=*) fdt="$optarg"
>    ;;
> -  --with-coroutine=*) coroutine="$optarg"
> -  ;;
>    --with-git=*) git="$optarg"
>    ;;
>    --with-git-submodules=*)
> @@ -949,8 +946,6 @@ Advanced options (experts only):
>    --disable-werror         disable compilation abort on warning
>    --disable-stack-protector disable compiler-provided stack protection
>    --cpu=CPU                Build for host CPU [$cpu]
> -  --with-coroutine=BACKEND coroutine backend. Supported options:
> -                           ucontext, sigaltstack, windows
>    --enable-plugins
>                             enable plugins via shared library loading
>    --disable-containers     don't use containers for cross-building
> @@ -1373,61 +1368,6 @@ case "$fdt" in
>      ;;
>  esac
>
> -##########################################
> -# check and set a backend for coroutine
> -
> -# We prefer ucontext, but it's not always possible. The fallback
> -# is sigcontext. 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
> -    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
> -else
> -  case $coroutine in
> -  windows)
> -    if test "$mingw32" != "yes"; then
> -      error_exit "'windows' coroutine backend only valid for Windows"
> -    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
> -      error_exit "'ucontext' backend requested but makecontext not available"
> -    fi
> -    ;;
> -  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
> -
>  ########################################
>  # check if ccache is interfering with
>  # semantic analysis of macros
> @@ -2002,8 +1942,6 @@ if [ "$bsd" = "yes" ] ; then
>    echo "CONFIG_BSD=y" >> $config_host_mak
>  fi
>
> -echo "CONFIG_COROUTINE_BACKEND=$coroutine" >> $config_host_mak
> -
>  if test "$plugins" = "yes" ; then
>      echo "CONFIG_PLUGIN=y" >> $config_host_mak
>  fi
> diff --git a/meson.build b/meson.build
> index 7ee9f081d0a1..b9df49667a19 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -211,6 +211,34 @@ if get_option('prefer_static')
>    qemu_ldflags += get_option('b_pie') ? '-static-pie' : '-static'
>  endif
>
> +coroutine_backend = get_option('coroutine_backend')
> +ucontext_probe = '''
> +  #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; }'''
> +
> +# On Windows the only valid backend is the Windows specific one.
> +# For POSIX prefer ucontext, but it's not always possible. The fallback
> +# is sigcontext.
> +supported_backends = []
> +if targetos == 'windows'
> +  supported_backends += ['windows']
> +else
> +  if targetos != 'darwin' and cc.links(ucontext_probe)
> +    supported_backends += ['ucontext']
> +  endif
> +  supported_backends += ['sigaltstack']
> +endif
> +
> +if coroutine_backend == 'auto'
> +  coroutine_backend = supported_backends[0]
> +elif coroutine_backend not in supported_backends
> +  error('"@0@" backend requested but not available.  Available backends: 
> @1@' \
> +        .format(coroutine_backend, ', '.join(supported_backends)))
> +endif
> +
>  # Compiles if SafeStack *not* enabled
>  safe_stack_probe = '''
>    int main(void)
> @@ -232,7 +260,7 @@ if get_option('safe_stack') != not 
> cc.compiles(safe_stack_probe)
>    qemu_cflags += safe_stack_arg
>    qemu_ldflags += safe_stack_arg
>  endif
> -if get_option('safe_stack') and config_host['CONFIG_COROUTINE_BACKEND'] != 
> 'ucontext'
> +if get_option('safe_stack') and coroutine_backend != 'ucontext'
>    error('SafeStack is only supported with the ucontext coroutine backend')
>  endif
>
> @@ -3949,7 +3977,7 @@ summary(summary_info, bool_yn: true, section: 'Targets 
> and accelerators')
>
>  # Block layer
>  summary_info = {}
> -summary_info += {'coroutine backend': 
> config_host['CONFIG_COROUTINE_BACKEND']}
> +summary_info += {'coroutine backend': coroutine_backend}
>  summary_info += {'coroutine pool':    have_coroutine_pool}
>  if have_block
>    summary_info += {'Block whitelist (rw)': 
> get_option('block_drv_rw_whitelist')}
> diff --git a/meson_options.txt b/meson_options.txt
> index d39d2a060ae7..126f89517e9a 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -33,6 +33,9 @@ option('fuzzing_engine', type : 'string', value : '',
>         description: 'fuzzing engine library for OSS-Fuzz')
>  option('trace_file', type: 'string', value: 'trace',
>         description: 'Trace file prefix for simple backend')
> +option('coroutine_backend', type: 'combo',
> +       choices: ['ucontext', 'sigaltstack', 'windows', 'auto'],
> +       value: 'auto', description: 'coroutine backend to use')
>
>  # Everything else can be set via --enable/--disable-* option
>  # on the configure script command line.  After adding an option
> diff --git a/scripts/meson-buildoptions.py b/scripts/meson-buildoptions.py
> index 8e4e5c4e8bd6..66a79cf37ea8 100755
> --- a/scripts/meson-buildoptions.py
> +++ b/scripts/meson-buildoptions.py
> @@ -35,6 +35,7 @@
>  OPTION_NAMES = {
>      "b_coverage": "gcov",
>      "b_lto": "lto",
> +    "coroutine_backend": "with-coroutine",
>      "debug": "debug-info",
>      "malloc": "enable-malloc",
>      "pkgversion": "with-pkgversion",
> diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
> index 237220d9b308..29695ac88eea 100644
> --- a/scripts/meson-buildoptions.sh
> +++ b/scripts/meson-buildoptions.sh
> @@ -59,6 +59,8 @@ meson_options_help() {
>    printf "%s\n" '  --sysconfdir=VALUE       Sysconf data directory [etc]'
>    printf "%s\n" '  --tls-priority=VALUE     Default TLS protocol/cipher 
> priority string'
>    printf "%s\n" '                           [NORMAL]'
> +  printf "%s\n" '  --with-coroutine=CHOICE  coroutine backend to use 
> (choices:'
> +  printf "%s\n" '                           
> auto/sigaltstack/ucontext/windows)'
>    printf "%s\n" '  --with-pkgversion=VALUE  use specified string as 
> sub-version of the'
>    printf "%s\n" '                           package'
>    printf "%s\n" '  --with-trace-file=VALUE  Trace file prefix for simple 
> backend [trace]'
> @@ -236,6 +238,7 @@ _meson_option_parse() {
>      --disable-cocoa) printf "%s" -Dcocoa=disabled ;;
>      --enable-coreaudio) printf "%s" -Dcoreaudio=enabled ;;
>      --disable-coreaudio) printf "%s" -Dcoreaudio=disabled ;;
> +    --with-coroutine=*) quote_sh "-Dcoroutine_backend=$2" ;;
>      --enable-coroutine-pool) printf "%s" -Dcoroutine_pool=true ;;
>      --disable-coroutine-pool) printf "%s" -Dcoroutine_pool=false ;;
>      --enable-crypto-afalg) printf "%s" -Dcrypto_afalg=enabled ;;
> diff --git a/util/coroutine-win32.c b/util/coroutine-windows.c
> similarity index 100%
> rename from util/coroutine-win32.c
> rename to util/coroutine-windows.c
> diff --git a/util/meson.build b/util/meson.build
> index 25b9b61f9817..b2a0aea21beb 100644
> --- a/util/meson.build
> +++ b/util/meson.build
> @@ -74,7 +74,7 @@ if have_block or have_ga
>    util_ss.add(files('lockcnt.c'))
>    util_ss.add(files('main-loop.c'))
>    util_ss.add(files('qemu-coroutine.c', 'qemu-coroutine-lock.c', 
> 'qemu-coroutine-io.c'))
> -  
> util_ss.add(files('coroutine-@0@.c'.format(config_host['CONFIG_COROUTINE_BACKEND'])))
> +  util_ss.add(files(f'coroutine-@coroutine_backend@.c'))
>    util_ss.add(files('thread-pool.c', 'qemu-timer.c'))
>    util_ss.add(files('qemu-sockets.c'))
>  endif
> --
> 2.38.1
>
>


-- 
Marc-André Lureau