Re: [Qemu-block] [PATCH v7] ssh: switch from libssh2 to libssh
Hi Pino, On 6/12/19 4:48 PM, 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). > > Adjust the various Docker/Travis scripts to use libssh when available > instead of libssh2. > > Signed-off-by: Pino Toscano > --- > > Changes from v6: > - fixed few checkpatch style issues > - detect libssh 0.8 via symbol detection > - adjust travis/docker test material > - remove dead "default" case in a switch > - use variables for storing MIN() results > - adapt a documentation bit > > 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 > > .travis.yml | 4 +- > block/Makefile.objs | 6 +- > block/ssh.c | 622 +- > block/trace-events| 14 +- > configure | 65 +- > docs/qemu-block-drivers.texi | 2 +- > .../dockerfiles/debian-win32-cross.docker | 1 - > .../dockerfiles/debian-win64-cross.docker | 1 - > tests/docker/dockerfiles/fedora.docker| 4 +- > tests/docker/dockerfiles/ubuntu.docker| 2 +- > tests/docker/dockerfiles/ubuntu1804.docker| 2 +- > tests/qemu-iotests/207.out| 2 +- > 12 files changed, 374 insertions(+), 351 deletions(-) > > diff --git a/.travis.yml b/.travis.yml > index b053a836a3..a2dac8b7c9 100644 > --- a/.travis.yml > +++ b/.travis.yml > @@ -31,7 +31,7 @@ addons: >- libseccomp-dev >- libspice-protocol-dev >- libspice-server-dev > - - libssh2-1-dev > + - libssh-dev >- liburcu-dev >- libusb-1.0-0-dev >- libvte-2.91-dev > @@ -261,7 +261,7 @@ matrix: > - libseccomp-dev > - libspice-protocol-dev > - libspice-server-dev > -- libssh2-1-dev > +- libssh-dev > - liburcu-dev > - libusb-1.0-0-dev > - libvte-2.91-dev > diff --git a/block/Makefile.objs b/block/Makefile.objs > index ae11605c9f..bf01429dd5 100644 > --- a/block/Makefile.objs > +++ b/block/Makefile.objs > @@ -31,7 +31,7 @@ block-obj-$(CONFIG_CURL) += curl.o > block-obj-$(CONFIG_RBD) += rbd.o > block-obj-$(CONFIG_GLUSTERFS) += gluster.o > block-obj-$(CONFIG_VXHS) += vxhs.o > -block-obj-$(CONFIG_LIBSSH2) += ssh.o > +block-obj-$(CONFIG_LIBSSH) += ssh.o > block-obj-y += accounting.o dirty-bitmap.o > block-obj-y += write-threshold.o > block-obj-y += backup.o > @@ -52,8 +52,8 @@ rbd.o-libs := $(RBD_LIBS) > gluster.o-cflags := $(GLUSTERFS_CFLAGS) > gluster.o-libs := $(GLUSTERFS_LIBS) > vxhs.o-libs:= $(VXHS_LIBS) > -ssh.o-cflags := $(LIBSSH2_CFLAGS) > -ssh.o-libs := $(LIBSSH2_LIBS) > +ssh.o-cflags := $(LIBSSH_CFLAGS) > +ssh.o-libs := $(LIBSSH_LIBS) > block-obj-dmg-bz2-$(CONFIG_BZIP2) += dmg-bz2.o > block-obj-$(if $(CONFIG_DMG),m,n) += $(block-obj-dmg-bz2-y) > dmg-bz2.o-libs := $(BZIP2_LIBS) > diff --git a/block/ssh.c b/block/ssh.c > index 12fd4f39e8..13768fad98 100644 > --- a/block/ssh.c > +++ b/block/ssh.c > @@ -24,8 +24,8 @@ > > #include "qemu/osdep.h" > > -#include > -#include > +#include > +#include > > #include "block/block_int.h" > #include "block/qdict.h" > @@ -44,13 +44,11 @@ > #include "trace.h" > > /* > - * TRACE_LIBSSH2= enables tracing in libssh2 itself. Note > - * that this requires that libssh2 was specially compiled with the > - * `./configure --enable-debug' option, so most likely you will have > - * to compile it yourself. The meaning of is described > - * here: http://www.libssh2.org/libssh2_trace.html > + * TRACE_LIBSSH= enables tracing in libssh itself. > + * The
[Qemu-block] [PATCH v7] ssh: switch from libssh2 to libssh
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).
Adjust the various Docker/Travis scripts to use libssh when available
instead of libssh2.
Signed-off-by: Pino Toscano
---
Changes from v6:
- fixed few checkpatch style issues
- detect libssh 0.8 via symbol detection
- adjust travis/docker test material
- remove dead "default" case in a switch
- use variables for storing MIN() results
- adapt a documentation bit
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
.travis.yml | 4 +-
block/Makefile.objs | 6 +-
block/ssh.c | 622 +-
block/trace-events| 14 +-
configure | 65 +-
docs/qemu-block-drivers.texi | 2 +-
.../dockerfiles/debian-win32-cross.docker | 1 -
.../dockerfiles/debian-win64-cross.docker | 1 -
tests/docker/dockerfiles/fedora.docker| 4 +-
tests/docker/dockerfiles/ubuntu.docker| 2 +-
tests/docker/dockerfiles/ubuntu1804.docker| 2 +-
tests/qemu-iotests/207.out| 2 +-
12 files changed, 374 insertions(+), 351 deletions(-)
diff --git a/.travis.yml b/.travis.yml
index b053a836a3..a2dac8b7c9 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -31,7 +31,7 @@ addons:
- libseccomp-dev
- libspice-protocol-dev
- libspice-server-dev
- - libssh2-1-dev
+ - libssh-dev
- liburcu-dev
- libusb-1.0-0-dev
- libvte-2.91-dev
@@ -261,7 +261,7 @@ matrix:
- libseccomp-dev
- libspice-protocol-dev
- libspice-server-dev
-- libssh2-1-dev
+- libssh-dev
- liburcu-dev
- libusb-1.0-0-dev
- libvte-2.91-dev
diff --git a/block/Makefile.objs b/block/Makefile.objs
index ae11605c9f..bf01429dd5 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -31,7 +31,7 @@ block-obj-$(CONFIG_CURL) += curl.o
block-obj-$(CONFIG_RBD) += rbd.o
block-obj-$(CONFIG_GLUSTERFS) += gluster.o
block-obj-$(CONFIG_VXHS) += vxhs.o
-block-obj-$(CONFIG_LIBSSH2) += ssh.o
+block-obj-$(CONFIG_LIBSSH) += ssh.o
block-obj-y += accounting.o dirty-bitmap.o
block-obj-y += write-threshold.o
block-obj-y += backup.o
@@ -52,8 +52,8 @@ rbd.o-libs := $(RBD_LIBS)
gluster.o-cflags := $(GLUSTERFS_CFLAGS)
gluster.o-libs := $(GLUSTERFS_LIBS)
vxhs.o-libs:= $(VXHS_LIBS)
-ssh.o-cflags := $(LIBSSH2_CFLAGS)
-ssh.o-libs := $(LIBSSH2_LIBS)
+ssh.o-cflags := $(LIBSSH_CFLAGS)
+ssh.o-libs := $(LIBSSH_LIBS)
block-obj-dmg-bz2-$(CONFIG_BZIP2) += dmg-bz2.o
block-obj-$(if $(CONFIG_DMG),m,n) += $(block-obj-dmg-bz2-y)
dmg-bz2.o-libs := $(BZIP2_LIBS)
diff --git a/block/ssh.c b/block/ssh.c
index 12fd4f39e8..13768fad98 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -24,8 +24,8 @@
#include "qemu/osdep.h"
-#include
-#include
+#include
+#include
#include "block/block_int.h"
#include "block/qdict.h"
@@ -44,13 +44,11 @@
#include "trace.h"
/*
- * TRACE_LIBSSH2= enables tracing in libssh2 itself. Note
- * that this requires that libssh2 was specially compiled with the
- * `./configure --enable-debug' option, so most likely you will have
- * to compile it yourself. The meaning of is described
- * here: http://www.libssh2.org/libssh2_trace.html
+ * TRACE_LIBSSH= enables tracing in libssh itself.
+ * The meaning of is described here:
+ * http://api.libssh.org/master/group__libssh__log.html
*/
-#define TRACE_LIBSSH2 0 /* or try: LIBSSH2_TRACE_SFTP */
+#define TRACE_LIBSSH 0 /* see: SSH_LOG_* */
typedef struct BDRVSSHState {
/* Coroutine. */
@@ -58,18 +56,15 @@ typedef struct BDRVSSHState {
/* SSH connectio
