Re: [PATCH v1 03/15] meson: move libmpathpersist test

[Paolo Bonzini]

On 25/09/20 17:40, Alex Bennée wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
> 
> This is the first compiler/linker test that has been moved to Meson.
> Add more section headings to keep things clearer.
> 
> [thuth: Add check for mpathpersist.found() before showing 
> mpathpersist_new_api]
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> Message-Id: <20200918103430.297167-3-thuth@redhat.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  configure         | 77 +++-----------------------------------------
>  meson.build       | 82 ++++++++++++++++++++++++++++++++++++++++++-----
>  meson_options.txt |  2 ++
>  3 files changed, 80 insertions(+), 81 deletions(-)
> 
> diff --git a/configure b/configure
> index 48bf437021f6..b81868cf0231 100755
> --- a/configure
> +++ b/configure
> @@ -403,7 +403,7 @@ netmap="no"
>  sdl="auto"
>  sdl_image="auto"
>  virtfs=""
> -mpath=""
> +mpath="auto"
>  vnc="enabled"
>  sparse="no"
>  vde=""
> @@ -1116,9 +1116,9 @@ for opt do
>    ;;
>    --enable-virtfs) virtfs="yes"
>    ;;
> -  --disable-mpath) mpath="no"
> +  --disable-mpath) mpath="disabled"
>    ;;
> -  --enable-mpath) mpath="yes"
> +  --enable-mpath) mpath="enabled"
>    ;;
>    --disable-vnc) vnc="disabled"
>    ;;
> @@ -3848,57 +3848,6 @@ if test "$modules" = yes; then
>      fi
>  fi
>  
> -##########################################
> -# libmpathpersist probe
> -
> -if test "$mpath" != "no" ; then
> -  # probe for the new API
> -  cat > $TMPC <<EOF
> -#include <libudev.h>
> -#include <mpath_persist.h>
> -unsigned mpath_mx_alloc_len = 1024;
> -int logsink;
> -static struct config *multipath_conf;
> -extern struct udev *udev;
> -extern struct config *get_multipath_config(void);
> -extern void put_multipath_config(struct config *conf);
> -struct udev *udev;
> -struct config *get_multipath_config(void) { return multipath_conf; }
> -void put_multipath_config(struct config *conf) { }
> -
> -int main(void) {
> -    udev = udev_new();
> -    multipath_conf = mpath_lib_init();
> -    return 0;
> -}
> -EOF
> -  if compile_prog "" "-ludev -lmultipath -lmpathpersist" ; then
> -    mpathpersist=yes
> -    mpathpersist_new_api=yes
> -  else
> -    # probe for the old API
> -    cat > $TMPC <<EOF
> -#include <libudev.h>
> -#include <mpath_persist.h>
> -unsigned mpath_mx_alloc_len = 1024;
> -int logsink;
> -int main(void) {
> -    struct udev *udev = udev_new();
> -    mpath_lib_init(udev);
> -    return 0;
> -}
> -EOF
> -    if compile_prog "" "-ludev -lmultipath -lmpathpersist" ; then
> -      mpathpersist=yes
> -      mpathpersist_new_api=no
> -    else
> -      mpathpersist=no
> -    fi
> -  fi
> -else
> -  mpathpersist=no
> -fi
> -
>  ##########################################
>  # pthread probe
>  PTHREADLIBS_LIST="-pthread -lpthread -lpthreadGC2"
> @@ -6340,23 +6289,11 @@ if test "$softmmu" = yes ; then
>        fi
>        virtfs=no
>      fi
> -    if test "$mpath" != no && test "$mpathpersist" = yes ; then
> -      mpath=yes
> -    else
> -      if test "$mpath" = yes; then
> -        error_exit "Multipath requires libmpathpersist devel"
> -      fi
> -      mpath=no
> -    fi
>    else
>      if test "$virtfs" = yes; then
>        error_exit "VirtFS is supported only on Linux"
>      fi
>      virtfs=no
> -    if test "$mpath" = yes; then
> -      error_exit "Multipath is supported only on Linux"
> -    fi
> -    mpath=no
>    fi
>  fi
>  
> @@ -6897,12 +6834,6 @@ fi
>  if test "$virtfs" = "yes" ; then
>    echo "CONFIG_VIRTFS=y" >> $config_host_mak
>  fi
> -if test "$mpath" = "yes" ; then
> -  echo "CONFIG_MPATH=y" >> $config_host_mak
> -  if test "$mpathpersist_new_api" = "yes"; then
> -    echo "CONFIG_MPATH_NEW_API=y" >> $config_host_mak
> -  fi
> -fi
>  if test "$vhost_scsi" = "yes" ; then
>    echo "CONFIG_VHOST_SCSI=y" >> $config_host_mak
>  fi
> @@ -7992,7 +7923,7 @@ NINJA=${ninja:-$PWD/ninjatool} $meson setup \
>          -Dstrip=$(if test "$strip_opt" = yes; then echo true; else echo 
> false; fi) \
>          -Db_pie=$(if test "$pie" = yes; then echo true; else echo false; fi) 
> \
>          -Db_coverage=$(if test "$gcov" = yes; then echo true; else echo 
> false; fi) \
> -     -Dsdl=$sdl -Dsdl_image=$sdl_image \
> +     -Dmpath=$mpath -Dsdl=$sdl -Dsdl_image=$sdl_image \
>       -Dvnc=$vnc -Dvnc_sasl=$vnc_sasl -Dvnc_jpeg=$vnc_jpeg -Dvnc_png=$vnc_png 
> \
>       -Dgettext=$gettext -Dxkbcommon=$xkbcommon -Du2f=$u2f\
>          $cross_arg \
> diff --git a/meson.build b/meson.build
> index 48724988972d..c09d869f9b02 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -81,6 +81,14 @@ if 'SPARSE_CFLAGS' in config_host
>                         'compile_commands.json'])
>  endif
>  
> +###########################################
> +# Target-specific checks and dependencies #
> +###########################################
> +
> +if targetos != 'linux' and get_option('mpath').enabled()
> +  error('Multipath is supported only on Linux')
> +endif
> +
>  m = cc.find_library('m', required: false)
>  util = cc.find_library('util', required: false)
>  winmm = []
> @@ -112,6 +120,11 @@ elif targetos == 'haiku'
>              cc.find_library('network'),
>              cc.find_library('bsd')]
>  endif
> +
> +################
> +# Dependencies #
> +################
> +
>  # The path to glib.h is added to all compilation commands.  This was
>  # grandfathered in from the QEMU Makefiles.
>  add_project_arguments(config_host['GLIB_CFLAGS'].split(),
> @@ -218,10 +231,6 @@ if 'CONFIG_SPICE' in config_host
>                               link_args: config_host['SPICE_LIBS'].split())
>  endif
>  rt = cc.find_library('rt', required: false)
> -libmpathpersist = not_found
> -if config_host.has_key('CONFIG_MPATH')
> -  libmpathpersist = cc.find_library('mpathpersist')
> -endif
>  libdl = not_found
>  if 'CONFIG_PLUGIN' in config_host
>    libdl = cc.find_library('dl', required: true)
> @@ -252,9 +261,62 @@ if 'CONFIG_CURL' in config_host
>                              link_args: config_host['CURL_LIBS'].split())
>  endif
>  libudev = not_found
> -if target_os == 'linux'
> -  libudev = dependency('libudev', static: enable_static)
> +if targetos == 'linux'
> +  libudev = dependency('libudev',
> +                       required: get_option('mpath'),
> +                       static: enable_static)
> +endif
> +
> +mpathpersist = not_found
> +if targetos == 'linux' and not get_option('mpath').disabled()
> +  libmultipath = cc.find_library('multipath',
> +                                 required: get_option('mpath'),
> +                                 static: enable_static)
> +  libmpathpersist = cc.find_library('mpathpersist',
> +                                    required: get_option('mpath'),
> +                                    static: enable_static)
> +  if libmultipath.found() and libmpathpersist.found() and libudev.found()
> +    if cc.links('''
> +      #include <libudev.h>
> +      #include <mpath_persist.h>
> +      unsigned mpath_mx_alloc_len = 1024;
> +      int logsink;
> +      static struct config *multipath_conf;
> +      extern struct udev *udev;
> +      extern struct config *get_multipath_config(void);
> +      extern void put_multipath_config(struct config *conf);
> +      struct udev *udev;
> +      struct config *get_multipath_config(void) { return multipath_conf; }
> +      void put_multipath_config(struct config *conf) { }
> +      int main(void) {
> +          udev = udev_new();
> +          multipath_conf = mpath_lib_init();
> +          return 0;
> +      }''', dependencies: [libmultipath, libmpathpersist, libudev])
> +      mpathpersist = declare_dependency(dependencies: [libmultipath, 
> libmpathpersist, libudev])
> +      mpathpersist_new_api = true
> +    elif cc.links('''
> +      #include <libudev.h>
> +      #include <mpath_persist.h>
> +      unsigned mpath_mx_alloc_len = 1024;
> +      int logsink;
> +      int main(void) {
> +          struct udev *udev = udev_new();
> +          mpath_lib_init(udev);
> +          return 0;
> +      }''', dependencies: [libmultipath, libmpathpersist, libudev])
> +      mpathpersist = declare_dependency(dependencies: [libmultipath, 
> libmpathpersist, libudev])
> +      mpathpersist_new_api = false
> +    else
> +      if get_option('mpath').enabled()
> +        error('Cannot detect libmpathpersist API')
> +      else
> +        warning('Cannot detect libmpathpersist API, disabling')
> +      endif
> +    endif
> +  endif
>  endif
> +
>  brlapi = not_found
>  if 'CONFIG_BRLAPI' in config_host
>    brlapi = declare_dependency(link_args: config_host['BRLAPI_LIBS'].split())
> @@ -436,6 +498,10 @@ has_gettid = cc.has_function('gettid')
>  # Create config-host.h
>  
>  config_host_data.set('CONFIG_LIBUDEV', libudev.found())
> +config_host_data.set('CONFIG_MPATH', mpathpersist.found())
> +if mpathpersist.found()
> +  config_host_data.set('CONFIG_MPATH_NEW_API', mpathpersist_new_api)
> +endif
>  config_host_data.set('CONFIG_SDL', sdl.found())
>  config_host_data.set('CONFIG_SDL_IMAGE', sdl_image.found())
>  config_host_data.set('CONFIG_VNC', vnc.found())
> @@ -1185,7 +1251,7 @@ if have_tools
>  
>      executable('qemu-pr-helper', files('scsi/qemu-pr-helper.c', 
> 'scsi/utils.c'),
>                 dependencies: [authz, crypto, io, qom, qemuutil,
> -                              libcap_ng, libudev, libmpathpersist],
> +                              libcap_ng, mpathpersist],
>                 install: true)
>    endif
>  
> @@ -1407,7 +1473,7 @@ summary_info += {'Audio drivers':     
> config_host['CONFIG_AUDIO_DRIVERS']}
>  summary_info += {'Block whitelist (rw)': 
> config_host['CONFIG_BDRV_RW_WHITELIST']}
>  summary_info += {'Block whitelist (ro)': 
> config_host['CONFIG_BDRV_RO_WHITELIST']}
>  summary_info += {'VirtFS support':    config_host.has_key('CONFIG_VIRTFS')}
> -summary_info += {'Multipath support': config_host.has_key('CONFIG_MPATH')}
> +summary_info += {'Multipath support': mpathpersist.found()}
>  summary_info += {'VNC support':       vnc.found()}
>  if vnc.found()
>    summary_info += {'VNC SASL support':  sasl.found()}
> diff --git a/meson_options.txt b/meson_options.txt
> index 543cf70043a2..b38a6ae92a92 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -6,6 +6,8 @@ option('docdir', type : 'string', value : 'doc',
>  option('gettext', type : 'boolean', value : true,
>         description: 'Localization of the GTK+ user interface')
>  
> +option('mpath', type : 'feature', value : 'auto',
> +       description: 'Multipath persistent reservation passthrough')
>  option('sdl', type : 'feature', value : 'auto',
>         description: 'SDL user interface')
>  option('sdl_image', type : 'feature', value : 'auto',
> 

This is not the latest version of the series, can you drop patches 2 and 3?

Paolo