On Fri, Jun 07, 2019 at 12:14:37PM +0200, Philippe Mathieu-Daudé wrote: > On 6/7/19 12:08 PM, Daniel P. Berrangé wrote: > > On Thu, Jun 06, 2019 at 07:51:15PM +0200, Pino Toscano wrote: > >> On Thursday, 6 June 2019 13:12:32 CEST Daniel P. Berrangé wrote: > >>> 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 <ptosc...@redhat.com> > >>>> --- > >>>> > >>>> 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. > >> > >> When I started to work on this, the libssh version available was > >> 0.6.x IIRC, which is very old. This v6 uses APIs added in 0.8 > >> conditionally, so it will still build with libssh < 0.8 -- of course, > >> using an older libssh results in a less performant ssh driver, although > >> I would think this can be considered somehow acceptable. > >> > >>> 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. > >> > >> For now I do not see the need to enforce a minimum version required; > >> it can be easily added in the future in case we need to use an API only > >> available starting from some version, and there is no fallback way for > >> older versions. > > > > In general we aim to set a clear minimum version for all our third > > party deps based on our platform support policy. We don't want to > > keep backcompat code around forever even if it is posisble to add > > fallback with #ifdefs. So even if we might still work with 0.6.x, > > we should declare 0.7.1 our min version IMHO. > > With our CI setup we use: > > Trusty (Ubuntu 14.04.5 LTS) > Source: libssh > Version: 0.6.1-0ubuntu3 > Replaces: libssh-2-dev > > Xenial > Source: libssh > Version: 0.6.3-4.3 > Replaces: libssh-2-dev > > The distrib packages do not allow dual use.
I missed that Ubuntu, unusually, had older versions than RHEL. We don't use Trusty though - our Travis config requests Xenial these days. So 0.6.3 is sufficient as a min version > > Incidentally that reminds me that it is desirable to modify the > > various native arch tests/docker/dockerfiles/*docker files to > > list libssh as a package to install so that we get compile testing > > coverage. > > I'm testing Pino patch and already did that :) 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 :|