Re: [Qemu-devel] [PATCH v6] ssh: switch from libssh2 to libssh

[Daniel P . Berrangé]

On Wed, Jun 05, 2019 at 11:36:54PM +0200, Pino Toscano wrote:
> Rewrite the implementation of the ssh block driver to use libssh instead
> of libssh2.  The libssh library has various advantages over libssh2:
> - easier API for authentication (for example for using ssh-agent)
> - easier API for known_hosts handling
> - supports newer types of keys in known_hosts
> 
> Use APIs/features available in libssh 0.8 conditionally, to support
> older versions (which are not recommended though).


> 
> Signed-off-by: Pino Toscano <address@hidden>
> ---
> 
> Changes from v5:
> - adapt to newer tracing APIs
> - disable ssh compression (mimic what libssh2 does by default)
> - use build time checks for libssh 0.8, and use newer APIs directly
> 
> Changes from v4:
> - fix wrong usages of error_setg/session_error_setg/sftp_error_setg
> - fix few return code checks
> - remove now-unused parameters in few internal functions
> - allow authentication with "none" method
> - switch to unsigned int for the port number
> - enable TCP_NODELAY on the socket
> - fix one reference error message in iotest 207
> 
> Changes from v3:
> - fix socket cleanup in connect_to_ssh()
> - add comments about the socket cleanup
> - improve the error reporting (closer to what was with libssh2)
> - improve EOF detection on sftp_read()
> 
> Changes from v2:
> - used again an own fd
> - fixed co_yield() implementation
> 
> Changes from v1:
> - fixed jumbo packets writing
> - fixed missing 'err' assignment
> - fixed commit message
> 
>  block/Makefile.objs        |   6 +-
>  block/ssh.c                | 610 +++++++++++++++++++------------------
>  block/trace-events         |  14 +-
>  configure                  |  62 ++--
>  tests/qemu-iotests/207.out |   2 +-
>  5 files changed, 351 insertions(+), 343 deletions(-)


> diff --git a/configure b/configure
> index b091b82cb3..bfdd70c40a 100755
> --- a/configure
> +++ b/configure

> @@ -3914,43 +3914,17 @@ EOF
>  fi
>  
>  ##########################################
> -# libssh2 probe
> -min_libssh2_version=1.2.8

The commit message says we're conditionally using APIs from 0.8.0,
but doesn't say what minimum version we actually need and there's
no check here.

In terms of our supported build platforms, the oldest libssh I
see is RHEL-7 with 0.7.1.

So assume it does actually compile on RHEL-7, then it is desirable
to have a min_libssh_Version=0.7.1 set here & checked below.

At some future date when support platforms changes, we'll then be
able to tweak it to 0.8.0 and drop the conditional compilation.

> -if test "$libssh2" != "no" ; then
> -  if $pkg_config --atleast-version=$min_libssh2_version libssh2; then
> -    libssh2_cflags=$($pkg_config libssh2 --cflags)
> -    libssh2_libs=$($pkg_config libssh2 --libs)
> -    libssh2=yes
> +# libssh probe
> +if test "$libssh" != "no" ; then
> +  if $pkg_config --exists libssh; then
> +    libssh_cflags=$($pkg_config libssh --cflags)
> +    libssh_libs=$($pkg_config libssh --libs)
> +    libssh=yes
>    else
> -    if test "$libssh2" = "yes" ; then
> -      error_exit "libssh2 >= $min_libssh2_version required for 
> --enable-libssh2"
> +    if test "$libssh" = "yes" ; then
> +      error_exit "libssh required for --enable-libssh"
>      fi
> -    libssh2=no
> -  fi
> -fi

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|