RE: [PATCH v2 1/4] configure: improve libzbc version check

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Damien Le Moal <Damien.LeMoal@xxxxxxx>
> Sent: Thursday, July 30, 2020 12:54 AM
> To: Dmitry Fomichev <Dmitry.Fomichev@xxxxxxx>; Jens Axboe
> <axboe@xxxxxxxxx>
> Cc: fio@xxxxxxxxxxxxxxx; Shinichiro Kawasaki
> <shinichiro.kawasaki@xxxxxxx>
> Subject: Re: [PATCH v2 1/4] configure: improve libzbc version check
> 
> On 2020/07/30 6:04, Dmitry Fomichev wrote:
> > Avoid parsing pkg-config output and just use --atleast-version to
> > check if libzbc is present and has an up to date version.
> >
> > Currently, support for libzbc ioengine is always included if libzbc is
> > found at the build system. Add the option to disable libzbc ioengine
> > support even if libzbc is found. This can be useful for
> > cross-compilation.
> >
> > Signed-off-by: Dmitry Fomichev <dmitry.fomichev@xxxxxxx>
> > ---
> >  configure | 28 +++++++++++++++-------------
> >  1 file changed, 15 insertions(+), 13 deletions(-)
> >
> > diff --git a/configure b/configure
> > index 5925e94f..9a5f35f2 100755
> > --- a/configure
> > +++ b/configure
> > @@ -213,6 +213,8 @@ for opt do
> >    ;;
> >    --enable-libnbd) libnbd="yes"
> >    ;;
> > +  --disable-libzbc) libzbc="no"
> > +  ;;
> >    --disable-tcmalloc) disable_tcmalloc="yes"
> >    ;;
> >    --enable-libaio-uring) libaio_uring="yes"
> > @@ -256,6 +258,7 @@ if test "$show_help" = "yes" ; then
> >    echo "--with-ime=             Install path for DDN's Infinite Memory Engine"
> >    echo "--enable-libiscsi       Enable iscsi support"
> >    echo "--enable-libnbd         Enable libnbd (NBD engine) support"
> > +  echo "--disable-libzbc        Disable libzbc even if found"
> 
> The message could simply be "Disable libzbc support".
> 
> >    echo "--disable-tcmalloc	Disable tcmalloc support"
> >    echo "--enable-libaio-uring   Enable libaio emulated over io_uring"
> >    echo "--dynamic-libengines	Lib-based ioengines as dynamic libraries"
> > @@ -2454,9 +2457,6 @@ fi
> >
> >  ##########################################
> >  # libzbc probe
> > -if test "$libzbc" != "yes" ; then
> > -  libzbc="no"
> > -fi
> >  cat > $TMPC << EOF
> >  #include <libzbc/zbc.h>
> >  int main(int argc, char **argv)
> > @@ -2466,19 +2466,21 @@ int main(int argc, char **argv)
> >    return zbc_open("foo=bar", O_RDONLY, &dev);
> >  }
> >  EOF
> > -if compile_prog "" "-lzbc" "libzbc"; then
> > -  libzbcvermaj=$(pkg-config --modversion libzbc | sed 's/\.[0-9]*\.[0-
> 9]*//')
> > -  if test "$libzbcvermaj" -ge "5" ; then
> > -    libzbc="yes"
> > +if test "$libzbc" != "no" ; then
> 
> If you make this:
> 
> if test "$libzbc" = "yes" ; then
> 
> > +  if compile_prog "" "-lzbc" "libzbc"; then
> > +    minimum_libzbc=5
> > +    if $(pkg-config --atleast-version=$minimum_libzbc libzbc); then
> > +      libzbc="yes"
> > +    else
> > +      print_config "libzbc engine" "libzbc version $minimum_libzbc or above
> required"
> > +      libzbc="no"
> > +    fi
> 
> then you can simplify the above by reversing the if condition,
> 
> >    else
> > -    print_config "libzbc engine" "Unsupported libzbc version (version 5 or
> above required)"
> > +    if test "$libzbc" = "yes" ; then
> 
> and you do not need this if. Note that I think you need to initialize the libzbc
> variable to "yes" by default for this to work.
> 

Right now, there is no default for libzbc ioengine support. If libzbc is found by
configure, then libzbc ioengine will always be added. The new flag, --disable-libzbc,
prevents adding support for libzbc ioengine in this case. If libzbc is not present,
fio configure is not failed with "feature not found".

Currently, no other feature in fio configure has the default option of "yes" and all
features that can be explicitly set to yes are set so by script command line options.
If a feature is set to yes and is not available, configure fails with "feature not found"
error. Do we really want configure to fail on any system that doesn't have libzbc
unless --disable-libzbc is added to the command line?

There is one feature option, "disable_lex",  with the default value, "". I guess we
could define libzbc option the same way to improve code clarity.

> > +      feature_not_found "libzbc" "libzbc or libzbc/zbc.h"
> > +    fi
> >      libzbc="no"
> >    fi
> > -else
> > -  if test "$libzbc" = "yes" ; then
> > -      feature_not_found "libzbc" "libzbc or libzbc/zbc.h"
> > -  fi
> > -  libzbc="no"
> >  fi
> >  print_config "libzbc engine" "$libzbc"
> >
> >
> 
> 
> --
> Damien Le Moal
> Western Digital Research




[Index of Archives]     [Linux Kernel]     [Linux SCSI]     [Linux IDE]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux