From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35697) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bxFL9-0004IM-1v for qemu-devel@nongnu.org; Thu, 20 Oct 2016 11:33:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bxFL5-0006Yh-2U for qemu-devel@nongnu.org; Thu, 20 Oct 2016 11:33:07 -0400 Date: Thu, 20 Oct 2016 16:32:50 +0100 From: "Richard W.M. Jones" Message-ID: <20161020153250.GE11243@redhat.com> References: <1476976524-6599-1-git-send-email-ptoscano@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1476976524-6599-1-git-send-email-ptoscano@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] ssh: switch from libssh2 to libssh List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Pino Toscano Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, jcody@redhat.com, kwolf@redhat.com, mreitz@redhat.com On Thu, Oct 20, 2016 at 05:15:24PM +0200, Pino Toscano wrote: > Rewrite the implementation of the ssh block driver to use libssh instea= d > of libssh. The libssh library has various advantages over libssh: > - easier API for authentication (for example for using ssh-agent) > - easier API for known_hosts handling > - supports newer types of keys in known_hosts >=20 > Kerberos authentication can be enabled once the libssh bug for it [1] i= s > fixed. >=20 > The development version of libssh (i.e. the future 0.8.x) supports > fsync, so reuse the build time check for this. >=20 > [1] https://red.libssh.org/issues/242 >=20 > Signed-off-by: Pino Toscano When I applied this patch and compiled it with warnings enabled: block/ssh.c: In function =E2=80=98connect_to_ssh=E2=80=99: block/ssh.c:643:12: warning: =E2=80=98ret=E2=80=99 may be used uninitiali= zed in this function [-Wmaybe-uninitialized] return ret; ^~~ To test the patch, I used virt-builder to create a virtual machine disk image on another machine (accessible over ssh). Then from my laptop I ran: ./x86_64-softmmu/qemu-system-x86_64 -nodefconfig \ -M accel=3Dkvm -cpu host -m 2048 \ -drive file.driver=3Dssh,file.user=3D[user],file.host=3D[host],file= .path=3D/var/tmp/fedora-24.img,format=3Draw,if=3Dvirtio \ -serial stdio Unfortunately this failed with a large number of sftp errors: read failed: (sftp error code: 0) and subsequently hung. So I'm afraid I couldn't test the patch at all :-= ( One slightly peculiar thing is that qemu ends up being linked to both libssh and libssh2. I believe the libssh2 dependency comes indirectly from libcurl. It's a slightly surprising situation but I suppose nothing to worry about. Also fsync was not supported for me, but I'm using 0.7.3 and the code says I need 0.8.0. I'll do a more detailed review when the above are fixed. Rich. > block/Makefile.objs | 6 +- > block/ssh.c | 543 +++++++++++++++++++++-----------------------= -------- > configure | 65 ++++--- > 3 files changed, 249 insertions(+), 365 deletions(-) >=20 > diff --git a/block/Makefile.objs b/block/Makefile.objs > index 67a036a..602a182 100644 > --- a/block/Makefile.objs > +++ b/block/Makefile.objs > @@ -19,7 +19,7 @@ block-obj-$(CONFIG_CURL) +=3D curl.o > block-obj-$(CONFIG_RBD) +=3D rbd.o > block-obj-$(CONFIG_GLUSTERFS) +=3D gluster.o > block-obj-$(CONFIG_ARCHIPELAGO) +=3D archipelago.o > -block-obj-$(CONFIG_LIBSSH2) +=3D ssh.o > +block-obj-$(CONFIG_LIBSSH) +=3D ssh.o > block-obj-y +=3D accounting.o dirty-bitmap.o > block-obj-y +=3D write-threshold.o > block-obj-y +=3D backup.o > @@ -38,8 +38,8 @@ rbd.o-cflags :=3D $(RBD_CFLAGS) > rbd.o-libs :=3D $(RBD_LIBS) > gluster.o-cflags :=3D $(GLUSTERFS_CFLAGS) > gluster.o-libs :=3D $(GLUSTERFS_LIBS) > -ssh.o-cflags :=3D $(LIBSSH2_CFLAGS) > -ssh.o-libs :=3D $(LIBSSH2_LIBS) > +ssh.o-cflags :=3D $(LIBSSH_CFLAGS) > +ssh.o-libs :=3D $(LIBSSH_LIBS) > archipelago.o-libs :=3D $(ARCHIPELAGO_LIBS) > block-obj-$(if $(CONFIG_BZIP2),m,n) +=3D dmg-bz2.o > dmg-bz2.o-libs :=3D $(BZIP2_LIBS) > diff --git a/block/ssh.c b/block/ssh.c > index 5ce12b6..7c316db 100644 > --- a/block/ssh.c > +++ b/block/ssh.c > @@ -24,8 +24,8 @@ > =20 > #include "qemu/osdep.h" > =20 > -#include > -#include > +#include > +#include > =20 > #include "block/block_int.h" > #include "qapi/error.h" > @@ -38,14 +38,12 @@ > /* DEBUG_SSH=3D1 enables the DPRINTF (debugging printf) statements in > * this block driver code. > * > - * TRACE_LIBSSH2=3D 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=3D enables tracing in libssh itself. > + * The meaning of is described here: > + * http://api.libssh.org/master/group__libssh__log.html > */ > #define DEBUG_SSH 0 > -#define TRACE_LIBSSH2 0 /* or try: LIBSSH2_TRACE_SFTP */ > +#define TRACE_LIBSSH 0 /* see: SSH_LOG_* */ > =20 > #define DPRINTF(fmt, ...) \ > do { \ > @@ -60,50 +58,39 @@ typedef struct BDRVSSHState { > CoMutex lock; > =20 > /* SSH connection. */ > - int sock; /* socket */ > - LIBSSH2_SESSION *session; /* ssh session */ > - LIBSSH2_SFTP *sftp; /* sftp session */ > - LIBSSH2_SFTP_HANDLE *sftp_handle; /* sftp remote file handle */ > + ssh_session session; /* ssh session */ > + sftp_session sftp; /* sftp session */ > + sftp_file sftp_handle; /* sftp remote file handle */ > =20 > - /* See ssh_seek() function below. */ > - int64_t offset; > - bool offset_op_read; > - > - /* File attributes at open. We try to keep the .filesize field > + /* File attributes at open. We try to keep the .size field > * updated if it changes (eg by writing at the end of the file). > */ > - LIBSSH2_SFTP_ATTRIBUTES attrs; > + sftp_attributes attrs; > =20 > /* Used to warn if 'flush' is not supported. */ > - char *hostport; > bool unsafe_flush_warning; > } BDRVSSHState; > =20 > static void ssh_state_init(BDRVSSHState *s) > { > memset(s, 0, sizeof *s); > - s->sock =3D -1; > - s->offset =3D -1; > qemu_co_mutex_init(&s->lock); > } > =20 > static void ssh_state_free(BDRVSSHState *s) > { > - g_free(s->hostport); > + if (s->attrs) { > + sftp_attributes_free(s->attrs); > + } > if (s->sftp_handle) { > - libssh2_sftp_close(s->sftp_handle); > + sftp_close(s->sftp_handle); > } > if (s->sftp) { > - libssh2_sftp_shutdown(s->sftp); > + sftp_free(s->sftp); > } > if (s->session) { > - libssh2_session_disconnect(s->session, > - "from qemu ssh client: " > - "user closed the connection"); > - libssh2_session_free(s->session); > - } > - if (s->sock >=3D 0) { > - close(s->sock); > + ssh_disconnect(s->session); > + ssh_free(s->session); > } > } > =20 > @@ -118,13 +105,13 @@ session_error_setg(Error **errp, BDRVSSHState *s,= const char *fs, ...) > va_end(args); > =20 > if (s->session) { > - char *ssh_err; > + const char *ssh_err; > int ssh_err_code; > =20 > - /* This is not an errno. See . */ > - ssh_err_code =3D libssh2_session_last_error(s->session, > - &ssh_err, NULL, 0); > - error_setg(errp, "%s: %s (libssh2 error code: %d)", > + /* This is not an errno. See . */ > + ssh_err =3D ssh_get_error(s->session); > + ssh_err_code =3D ssh_get_error_code(s->session); > + error_setg(errp, "%s: %s (libssh error code: %d)", > msg, ssh_err, ssh_err_code); > } else { > error_setg(errp, "%s", msg); > @@ -143,19 +130,14 @@ sftp_error_setg(Error **errp, BDRVSSHState *s, co= nst char *fs, ...) > va_end(args); > =20 > if (s->sftp) { > - char *ssh_err; > - int ssh_err_code; > - unsigned long sftp_err_code; > + int sftp_err_code; > =20 > - /* This is not an errno. See . */ > - ssh_err_code =3D libssh2_session_last_error(s->session, > - &ssh_err, NULL, 0); > - /* See . */ > - sftp_err_code =3D libssh2_sftp_last_error((s)->sftp); > + /* This is not an errno. See . */ > + sftp_err_code =3D sftp_get_error(s->sftp); > =20 > error_setg(errp, > - "%s: %s (libssh2 error code: %d, sftp error code: %= lu)", > - msg, ssh_err, ssh_err_code, sftp_err_code); > + "%s (sftp error code: %d)", > + msg, sftp_err_code); > } else { > error_setg(errp, "%s", msg); > } > @@ -171,18 +153,13 @@ sftp_error_report(BDRVSSHState *s, const char *fs= , ...) > error_vprintf(fs, args); > =20 > if ((s)->sftp) { > - char *ssh_err; > - int ssh_err_code; > - unsigned long sftp_err_code; > + int sftp_err_code; > =20 > - /* This is not an errno. See . */ > - ssh_err_code =3D libssh2_session_last_error(s->session, > - &ssh_err, NULL, 0); > - /* See . */ > - sftp_err_code =3D libssh2_sftp_last_error((s)->sftp); > + /* This is not an errno. See . */ > + sftp_err_code =3D sftp_get_error(s->sftp); > =20 > - error_printf(": %s (libssh2 error code: %d, sftp error code: %= lu)", > - ssh_err, ssh_err_code, sftp_err_code); > + error_printf(": (sftp error code: %d)", > + sftp_err_code); > } > =20 > va_end(args); > @@ -272,68 +249,41 @@ static void ssh_parse_filename(const char *filena= me, QDict *options, > static int check_host_key_knownhosts(BDRVSSHState *s, > const char *host, int port, Error= **errp) > { > - const char *home; > - char *knh_file =3D NULL; > - LIBSSH2_KNOWNHOSTS *knh =3D NULL; > - struct libssh2_knownhost *found; > - int ret, r; > - const char *hostkey; > - size_t len; > - int type; > + int ret; > + int state; > =20 > - hostkey =3D libssh2_session_hostkey(s->session, &len, &type); > - if (!hostkey) { > - ret =3D -EINVAL; > - session_error_setg(errp, s, "failed to read remote host key"); > - goto out; > - } > + state =3D ssh_is_server_known(s->session); > =20 > - knh =3D libssh2_knownhost_init(s->session); > - if (!knh) { > - ret =3D -EINVAL; > - session_error_setg(errp, s, > - "failed to initialize known hosts support")= ; > - goto out; > - } > - > - home =3D getenv("HOME"); > - if (home) { > - knh_file =3D g_strdup_printf("%s/.ssh/known_hosts", home); > - } else { > - knh_file =3D g_strdup_printf("/root/.ssh/known_hosts"); > - } > - > - /* Read all known hosts from OpenSSH-style known_hosts file. */ > - libssh2_knownhost_readfile(knh, knh_file, LIBSSH2_KNOWNHOST_FILE_O= PENSSH); > - > - r =3D libssh2_knownhost_checkp(knh, host, port, hostkey, len, > - LIBSSH2_KNOWNHOST_TYPE_PLAIN| > - LIBSSH2_KNOWNHOST_KEYENC_RAW, > - &found); > - switch (r) { > - case LIBSSH2_KNOWNHOST_CHECK_MATCH: > + switch (state) { > + case SSH_SERVER_KNOWN_OK: > /* OK */ > - DPRINTF("host key OK: %s", found->key); > break; > - case LIBSSH2_KNOWNHOST_CHECK_MISMATCH: > + case SSH_SERVER_KNOWN_CHANGED: > ret =3D -EINVAL; > session_error_setg(errp, s, > - "host key does not match the one in known_hosts" > - " (found key %s)", found->key); > + "host key does not match the one in known_hosts"= ); > goto out; > - case LIBSSH2_KNOWNHOST_CHECK_NOTFOUND: > + case SSH_SERVER_FOUND_OTHER: > + ret =3D -EINVAL; > + session_error_setg(errp, s, > + "host key for this server not found, another typ= e " > + "exists"); > + goto out; > + case SSH_SERVER_FILE_NOT_FOUND: > + ret =3D -EINVAL; > + session_error_setg(errp, s, "known_hosts file not found"); > + goto out; > + case SSH_SERVER_NOT_KNOWN: > ret =3D -EINVAL; > session_error_setg(errp, s, "no host key was found in known_ho= sts"); > goto out; > - case LIBSSH2_KNOWNHOST_CHECK_FAILURE: > + case SSH_SERVER_ERROR: > ret =3D -EINVAL; > - session_error_setg(errp, s, > - "failure matching the host key with known_hosts"= ); > + session_error_setg(errp, s, "server error"); > goto out; > default: > ret =3D -EINVAL; > - session_error_setg(errp, s, "unknown error matching the host k= ey" > - " with known_hosts (%d)", r); > + session_error_setg(errp, s, "error while checking for known se= rver"); > goto out; > } > =20 > @@ -341,10 +291,6 @@ static int check_host_key_knownhosts(BDRVSSHState = *s, > ret =3D 0; > =20 > out: > - if (knh !=3D NULL) { > - libssh2_knownhost_free(knh); > - } > - g_free(knh_file); > return ret; > } > =20 > @@ -388,23 +334,37 @@ static int compare_fingerprint(const unsigned cha= r *fingerprint, size_t len, > =20 > static int > check_host_key_hash(BDRVSSHState *s, const char *hash, > - int hash_type, size_t fingerprint_len, Error **err= p) > + enum ssh_publickey_hash_type type, size_t fingerpr= int_len, > + Error **errp) > { > - const char *fingerprint; > + int r; > + ssh_key pubkey; > + unsigned char *server_hash; > + size_t server_hash_len; > =20 > - fingerprint =3D libssh2_hostkey_hash(s->session, hash_type); > - if (!fingerprint) { > + r =3D ssh_get_publickey(s->session, &pubkey); > + if (r < 0) { > session_error_setg(errp, s, "failed to read remote host key"); > return -EINVAL; > } > =20 > - if(compare_fingerprint((unsigned char *) fingerprint, fingerprint_= len, > - hash) !=3D 0) { > + r =3D ssh_get_publickey_hash(pubkey, type, &server_hash, &server_h= ash_len); > + ssh_key_free(pubkey); > + if (r < 0) { > + session_error_setg(errp, s, > + "failed reading the hash of the server SSH = key"); > + return -EINVAL; > + } > + > + if (compare_fingerprint(server_hash, server_hash_len, hash) !=3D 0= ) { > + ssh_clean_pubkey_hash(&server_hash); > error_setg(errp, "remote host key does not match host_key_chec= k '%s'", > hash); > return -EPERM; > } > =20 > + ssh_clean_pubkey_hash(&server_hash); > + > return 0; > } > =20 > @@ -419,13 +379,13 @@ static int check_host_key(BDRVSSHState *s, const = char *host, int port, > /* host_key_check=3Dmd5:xx:yy:zz:... */ > if (strncmp(host_key_check, "md5:", 4) =3D=3D 0) { > return check_host_key_hash(s, &host_key_check[4], > - LIBSSH2_HOSTKEY_HASH_MD5, 16, errp)= ; > + SSH_PUBLICKEY_HASH_MD5, 16, errp); > } > =20 > /* host_key_check=3Dsha1:xx:yy:zz:... */ > if (strncmp(host_key_check, "sha1:", 5) =3D=3D 0) { > return check_host_key_hash(s, &host_key_check[5], > - LIBSSH2_HOSTKEY_HASH_SHA1, 20, errp= ); > + SSH_PUBLICKEY_HASH_SHA1, 20, errp); > } > =20 > /* host_key_check=3Dyes */ > @@ -440,57 +400,32 @@ static int check_host_key(BDRVSSHState *s, const = char *host, int port, > static int authenticate(BDRVSSHState *s, const char *user, Error **err= p) > { > int r, ret; > - const char *userauthlist; > - LIBSSH2_AGENT *agent =3D NULL; > - struct libssh2_agent_publickey *identity; > - struct libssh2_agent_publickey *prev_identity =3D NULL; > + int method; > =20 > - userauthlist =3D libssh2_userauth_list(s->session, user, strlen(us= er)); > - if (strstr(userauthlist, "publickey") =3D=3D NULL) { > + r =3D ssh_userauth_none(s->session, NULL); > + if (r =3D=3D SSH_AUTH_ERROR) { > ret =3D -EPERM; > - error_setg(errp, > - "remote server does not support \"publickey\" authenti= cation"); > + session_error_setg(errp, s, "failed to call ssh_userauth_none"= ); > goto out; > } > =20 > - /* Connect to ssh-agent and try each identity in turn. */ > - agent =3D libssh2_agent_init(s->session); > - if (!agent) { > - ret =3D -EINVAL; > - session_error_setg(errp, s, "failed to initialize ssh-agent su= pport"); > - goto out; > - } > - if (libssh2_agent_connect(agent)) { > - ret =3D -ECONNREFUSED; > - session_error_setg(errp, s, "failed to connect to ssh-agent"); > - goto out; > - } > - if (libssh2_agent_list_identities(agent)) { > - ret =3D -EINVAL; > - session_error_setg(errp, s, > - "failed requesting identities from ssh-agen= t"); > - goto out; > - } > + method =3D ssh_userauth_list(s->session, NULL); > =20 > - for(;;) { > - r =3D libssh2_agent_get_identity(agent, &identity, prev_identi= ty); > - if (r =3D=3D 1) { /* end of list */ > - break; > - } > - if (r < 0) { > + /* Try to authenticate with publickey, using the ssh-agent > + * if available. > + */ > + if (method & SSH_AUTH_METHOD_PUBLICKEY) { > + r =3D ssh_userauth_publickey_auto(s->session, NULL, NULL); > + if (r =3D=3D SSH_AUTH_ERROR) { > ret =3D -EINVAL; > - session_error_setg(errp, s, > - "failed to obtain identity from ssh-age= nt"); > + error_setg(errp, "failed to authenticate using publickey " > + "authentication"); > goto out; > - } > - r =3D libssh2_agent_userauth(agent, user, identity); > - if (r =3D=3D 0) { > + } else if (r =3D=3D SSH_AUTH_SUCCESS) { > /* Authenticated! */ > ret =3D 0; > goto out; > } > - /* Failed to authenticate with this identity, try the next one= . */ > - prev_identity =3D identity; > } > =20 > ret =3D -EPERM; > @@ -498,13 +433,6 @@ static int authenticate(BDRVSSHState *s, const cha= r *user, Error **errp) > "and the identities held by your ssh-agent"); > =20 > out: > - if (agent !=3D NULL) { > - /* Note: libssh2 implementation implicitly calls > - * libssh2_agent_disconnect if necessary. > - */ > - libssh2_agent_free(agent); > - } > - > return ret; > } > =20 > @@ -547,7 +475,7 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *o= ptions, > QemuOpts *opts =3D NULL; > Error *local_err =3D NULL; > const char *host, *user, *path, *host_key_check; > - int port; > + unsigned int port; > =20 > opts =3D qemu_opts_create(&ssh_runtime_opts, NULL, 0, &error_abort= ); > qemu_opts_absorb_qdict(opts, options, &local_err); > @@ -588,31 +516,54 @@ static int connect_to_ssh(BDRVSSHState *s, QDict = *options, > host_key_check =3D "yes"; > } > =20 > - /* Construct the host:port name for inet_connect. */ > - g_free(s->hostport); > - s->hostport =3D g_strdup_printf("%s:%d", host, port); > - > - /* Open the socket and connect. */ > - s->sock =3D inet_connect(s->hostport, errp); > - if (s->sock < 0) { > - ret =3D -EIO; > - goto err; > - } > - > /* Create SSH session. */ > - s->session =3D libssh2_session_init(); > + s->session =3D ssh_new(); > if (!s->session) { > + goto err; > + } > + > + /* Make sure we are in blocking mode during the connection and > + * authentication phases. > + */ > + ssh_set_blocking(s->session, 1); > + > + r =3D ssh_options_set(s->session, SSH_OPTIONS_USER, user); > + if (r < 0) { > ret =3D -EINVAL; > - session_error_setg(errp, s, "failed to initialize libssh2 sess= ion"); > + session_error_setg(errp, s, > + "failed to set the user in the libssh sessi= on"); > goto err; > } > =20 > -#if TRACE_LIBSSH2 !=3D 0 > - libssh2_trace(s->session, TRACE_LIBSSH2); > -#endif > + r =3D ssh_options_set(s->session, SSH_OPTIONS_HOST, host); > + if (r < 0) { > + ret =3D -EINVAL; > + session_error_setg(errp, s, > + "failed to set the host in the libssh sessi= on"); > + goto err; > + } > + > + if (port > 0) { > + r =3D ssh_options_set(s->session, SSH_OPTIONS_PORT, &port); > + if (r < 0) { > + ret =3D -EINVAL; > + session_error_setg(errp, s, > + "failed to set the port in the libssh s= ession"); > + goto err; > + } > + } > + > + /* Read ~/.ssh/config. */ > + r =3D ssh_options_parse_config(s->session, NULL); > + if (r < 0) { > + ret =3D -EINVAL; > + session_error_setg(errp, s, "failed to parse ~/.ssh/config"); > + goto err; > + } > =20 > - r =3D libssh2_session_handshake(s->session, s->sock); > - if (r !=3D 0) { > + /* Connect. */ > + r =3D ssh_connect(s->session); > + if (r < 0) { > ret =3D -EINVAL; > session_error_setg(errp, s, "failed to establish SSH session")= ; > goto err; > @@ -631,8 +582,15 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *= options, > } > =20 > /* Start SFTP. */ > - s->sftp =3D libssh2_sftp_init(s->session); > + s->sftp =3D sftp_new(s->session); > if (!s->sftp) { > + session_error_setg(errp, s, "failed to create sftp handle"); > + ret =3D -EINVAL; > + goto err; > + } > + > + r =3D sftp_init(s->sftp); > + if (r < 0) { > session_error_setg(errp, s, "failed to initialize sftp handle"= ); > ret =3D -EINVAL; > goto err; > @@ -641,17 +599,20 @@ static int connect_to_ssh(BDRVSSHState *s, QDict = *options, > /* Open the remote file. */ > DPRINTF("opening file %s flags=3D0x%x creat_mode=3D0%o", > path, ssh_flags, creat_mode); > - s->sftp_handle =3D libssh2_sftp_open(s->sftp, path, ssh_flags, cre= at_mode); > + s->sftp_handle =3D sftp_open(s->sftp, path, ssh_flags, creat_mode)= ; > if (!s->sftp_handle) { > session_error_setg(errp, s, "failed to open remote file '%s'",= path); > ret =3D -EINVAL; > goto err; > } > =20 > + /* Make sure the SFTP file is handled in blocking mode. */ > + sftp_file_set_blocking(s->sftp_handle); > + > qemu_opts_del(opts); > =20 > - r =3D libssh2_sftp_fstat(s->sftp_handle, &s->attrs); > - if (r < 0) { > + s->attrs =3D sftp_fstat(s->sftp_handle); > + if (!s->attrs) { > sftp_error_setg(errp, s, "failed to read file attributes"); > return -EINVAL; > } > @@ -659,19 +620,21 @@ static int connect_to_ssh(BDRVSSHState *s, QDict = *options, > return 0; > =20 > err: > + if (s->attrs) { > + sftp_attributes_free(s->attrs); > + } > + s->attrs =3D NULL; > if (s->sftp_handle) { > - libssh2_sftp_close(s->sftp_handle); > + sftp_close(s->sftp_handle); > } > s->sftp_handle =3D NULL; > if (s->sftp) { > - libssh2_sftp_shutdown(s->sftp); > + sftp_free(s->sftp); > } > s->sftp =3D NULL; > if (s->session) { > - libssh2_session_disconnect(s->session, > - "from qemu ssh client: " > - "error opening connection"); > - libssh2_session_free(s->session); > + ssh_disconnect(s->session); > + ssh_free(s->session); > } > s->session =3D NULL; > =20 > @@ -689,9 +652,11 @@ static int ssh_file_open(BlockDriverState *bs, QDi= ct *options, int bdrv_flags, > =20 > ssh_state_init(s); > =20 > - ssh_flags =3D LIBSSH2_FXF_READ; > + ssh_flags =3D 0; > if (bdrv_flags & BDRV_O_RDWR) { > - ssh_flags |=3D LIBSSH2_FXF_WRITE; > + ssh_flags |=3D O_RDWR; > + } else { > + ssh_flags |=3D O_RDONLY; > } > =20 > /* Start up SSH. */ > @@ -701,15 +666,11 @@ static int ssh_file_open(BlockDriverState *bs, QD= ict *options, int bdrv_flags, > } > =20 > /* Go non-blocking. */ > - libssh2_session_set_blocking(s->session, 0); > + ssh_set_blocking(s->session, 0); > =20 > return 0; > =20 > err: > - if (s->sock >=3D 0) { > - close(s->sock); > - } > - s->sock =3D -1; > =20 > return ret; > } > @@ -751,8 +712,7 @@ static int ssh_create(const char *filename, QemuOpt= s *opts, Error **errp) > } > =20 > r =3D connect_to_ssh(&s, uri_options, > - LIBSSH2_FXF_READ|LIBSSH2_FXF_WRITE| > - LIBSSH2_FXF_CREAT|LIBSSH2_FXF_TRUNC, > + O_RDWR | O_CREAT | O_TRUNC, > 0644, errp); > if (r < 0) { > ret =3D r; > @@ -760,14 +720,14 @@ static int ssh_create(const char *filename, QemuO= pts *opts, Error **errp) > } > =20 > if (total_size > 0) { > - libssh2_sftp_seek64(s.sftp_handle, total_size-1); > - r2 =3D libssh2_sftp_write(s.sftp_handle, c, 1); > + sftp_seek64(s.sftp_handle, total_size - 1); > + r2 =3D sftp_write(s.sftp_handle, c, 1); > if (r2 < 0) { > sftp_error_setg(errp, &s, "truncate failed"); > ret =3D -EINVAL; > goto out; > } > - s.attrs.filesize =3D total_size; > + s.attrs->size =3D total_size; > } > =20 > ret =3D 0; > @@ -793,90 +753,20 @@ static int ssh_has_zero_init(BlockDriverState *bs= ) > /* Assume false, unless we can positively prove it's true. */ > int has_zero_init =3D 0; > =20 > - if (s->attrs.flags & LIBSSH2_SFTP_ATTR_PERMISSIONS) { > - if (s->attrs.permissions & LIBSSH2_SFTP_S_IFREG) { > - has_zero_init =3D 1; > - } > + if (s->attrs->type =3D=3D SSH_FILEXFER_TYPE_REGULAR) { > + has_zero_init =3D 1; > } > =20 > return has_zero_init; > } > =20 > -static void restart_coroutine(void *opaque) > -{ > - Coroutine *co =3D opaque; > - > - DPRINTF("co=3D%p", co); > - > - qemu_coroutine_enter(co); > -} > - > -static coroutine_fn void set_fd_handler(BDRVSSHState *s, BlockDriverSt= ate *bs) > -{ > - int r; > - IOHandler *rd_handler =3D NULL, *wr_handler =3D NULL; > - Coroutine *co =3D qemu_coroutine_self(); > - > - r =3D libssh2_session_block_directions(s->session); > - > - if (r & LIBSSH2_SESSION_BLOCK_INBOUND) { > - rd_handler =3D restart_coroutine; > - } > - if (r & LIBSSH2_SESSION_BLOCK_OUTBOUND) { > - wr_handler =3D restart_coroutine; > - } > - > - DPRINTF("s->sock=3D%d rd_handler=3D%p wr_handler=3D%p", s->sock, > - rd_handler, wr_handler); > - > - aio_set_fd_handler(bdrv_get_aio_context(bs), s->sock, > - false, rd_handler, wr_handler, co); > -} > - > -static coroutine_fn void clear_fd_handler(BDRVSSHState *s, > - BlockDriverState *bs) > -{ > - DPRINTF("s->sock=3D%d", s->sock); > - aio_set_fd_handler(bdrv_get_aio_context(bs), s->sock, > - false, NULL, NULL, NULL); > -} > - > /* A non-blocking call returned EAGAIN, so yield, ensuring the > * handlers are set up so that we'll be rescheduled when there is an > * interesting event on the socket. > */ > static coroutine_fn void co_yield(BDRVSSHState *s, BlockDriverState *b= s) > { > - set_fd_handler(s, bs); > qemu_coroutine_yield(); > - clear_fd_handler(s, bs); > -} > - > -/* SFTP has a function `libssh2_sftp_seek64' which seeks to a position > - * in the remote file. Notice that it just updates a field in the > - * sftp_handle structure, so there is no network traffic and it cannot > - * fail. > - * > - * However, `libssh2_sftp_seek64' does have a catastrophic effect on > - * performance since it causes the handle to throw away all in-flight > - * reads and buffered readahead data. Therefore this function tries > - * to be intelligent about when to call the underlying libssh2 functio= n. > - */ > -#define SSH_SEEK_WRITE 0 > -#define SSH_SEEK_READ 1 > -#define SSH_SEEK_FORCE 2 > - > -static void ssh_seek(BDRVSSHState *s, int64_t offset, int flags) > -{ > - bool op_read =3D (flags & SSH_SEEK_READ) !=3D 0; > - bool force =3D (flags & SSH_SEEK_FORCE) !=3D 0; > - > - if (force || op_read !=3D s->offset_op_read || offset !=3D s->offs= et) { > - DPRINTF("seeking to offset=3D%" PRIi64, offset); > - libssh2_sftp_seek64(s->sftp_handle, offset); > - s->offset =3D offset; > - s->offset_op_read =3D op_read; > - } > } > =20 > static coroutine_fn int ssh_read(BDRVSSHState *s, BlockDriverState *bs= , > @@ -890,7 +780,7 @@ static coroutine_fn int ssh_read(BDRVSSHState *s, B= lockDriverState *bs, > =20 > DPRINTF("offset=3D%" PRIi64 " size=3D%zu", offset, size); > =20 > - ssh_seek(s, offset, SSH_SEEK_READ); > + sftp_seek64(s->sftp_handle, offset); > =20 > /* This keeps track of the current iovec element ('i'), where we > * will write to next ('buf'), and the end of the current iovec > @@ -900,35 +790,34 @@ static coroutine_fn int ssh_read(BDRVSSHState *s,= BlockDriverState *bs, > buf =3D i->iov_base; > end_of_vec =3D i->iov_base + i->iov_len; > =20 > - /* libssh2 has a hard-coded limit of 2000 bytes per request, > - * although it will also do readahead behind our backs. Therefore > - * we may have to do repeated reads here until we have read 'size' > - * bytes. > - */ > for (got =3D 0; got < size; ) { > again: > - DPRINTF("sftp_read buf=3D%p size=3D%zu", buf, end_of_vec - buf= ); > - r =3D libssh2_sftp_read(s->sftp_handle, buf, end_of_vec - buf)= ; > + DPRINTF("sftp_read buf=3D%p size=3D%zu (actual size=3D%zu)", > + buf, end_of_vec - buf, MIN(end_of_vec - buf, 16384)); > + /* The size of SFTP packets is limited to 32K bytes, so limit > + * the amount of data requested to 16K, as libssh currently > + * does not handle multiple requests on its own: > + * https://red.libssh.org/issues/58 > + */ > + r =3D sftp_read(s->sftp_handle, buf, MIN(end_of_vec - buf, 163= 84)); > DPRINTF("sftp_read returned %zd", r); > =20 > - if (r =3D=3D LIBSSH2_ERROR_EAGAIN || r =3D=3D LIBSSH2_ERROR_TI= MEOUT) { > + if (r =3D=3D SSH_AGAIN) { > co_yield(s, bs); > goto again; > } > - if (r < 0) { > - sftp_error_report(s, "read failed"); > - s->offset =3D -1; > - return -EIO; > - } > - if (r =3D=3D 0) { > + if (r =3D=3D SSH_EOF) { > /* EOF: Short read so pad the buffer with zeroes and retur= n it. */ > qemu_iovec_memset(qiov, got, 0, size - got); > return 0; > } > + if (r < 0) { > + sftp_error_report(s, "read failed"); > + return -EIO; > + } > =20 > got +=3D r; > buf +=3D r; > - s->offset +=3D r; > if (buf >=3D end_of_vec && got < size) { > i++; > buf =3D i->iov_base; > @@ -965,7 +854,7 @@ static int ssh_write(BDRVSSHState *s, BlockDriverSt= ate *bs, > =20 > DPRINTF("offset=3D%" PRIi64 " size=3D%zu", offset, size); > =20 > - ssh_seek(s, offset, SSH_SEEK_WRITE); > + sftp_seek64(s->sftp_handle, offset); > =20 > /* This keeps track of the current iovec element ('i'), where we > * will read from next ('buf'), and the end of the current iovec > @@ -978,44 +867,29 @@ static int ssh_write(BDRVSSHState *s, BlockDriver= State *bs, > for (written =3D 0; written < size; ) { > again: > DPRINTF("sftp_write buf=3D%p size=3D%zu", buf, end_of_vec - bu= f); > - r =3D libssh2_sftp_write(s->sftp_handle, buf, end_of_vec - buf= ); > + r =3D sftp_write(s->sftp_handle, buf, end_of_vec - buf); > DPRINTF("sftp_write returned %zd", r); > =20 > - if (r =3D=3D LIBSSH2_ERROR_EAGAIN || r =3D=3D LIBSSH2_ERROR_TI= MEOUT) { > + if (r =3D=3D SSH_AGAIN) { > co_yield(s, bs); > goto again; > } > if (r < 0) { > sftp_error_report(s, "write failed"); > - s->offset =3D -1; > return -EIO; > } > - /* The libssh2 API is very unclear about this. A comment in > - * the code says "nothing was acked, and no EAGAIN was > - * received!" which apparently means that no data got sent > - * out, and the underlying channel didn't return any EAGAIN > - * indication. I think this is a bug in either libssh2 or > - * OpenSSH (server-side). In any case, forcing a seek (to > - * discard libssh2 internal buffers), and then trying again > - * works for me. > - */ > - if (r =3D=3D 0) { > - ssh_seek(s, offset + written, SSH_SEEK_WRITE|SSH_SEEK_FORC= E); > - co_yield(s, bs); > - goto again; > - } > =20 > written +=3D r; > buf +=3D r; > - s->offset +=3D r; > if (buf >=3D end_of_vec && written < size) { > i++; > buf =3D i->iov_base; > end_of_vec =3D i->iov_base + i->iov_len; > } > =20 > - if (offset + written > s->attrs.filesize) > - s->attrs.filesize =3D offset + written; > + if (offset + written > s->attrs->size) { > + s->attrs->size =3D offset + written; > + } > } > =20 > return 0; > @@ -1039,33 +913,40 @@ static coroutine_fn int ssh_co_writev(BlockDrive= rState *bs, > static void unsafe_flush_warning(BDRVSSHState *s, const char *what) > { > if (!s->unsafe_flush_warning) { > - error_report("warning: ssh server %s does not support fsync", > - s->hostport); > + char *host; > + unsigned int port; > + > + ssh_options_get(s->session, SSH_OPTIONS_HOST, &host); > + ssh_options_get_port(s->session, &port); > + > + error_report("warning: ssh server %s:%u does not support fsync= ", > + host, port); > if (what) { > error_report("to support fsync, you need %s", what); > } > + ssh_string_free_char(host); > s->unsafe_flush_warning =3D true; > } > } > =20 > -#ifdef HAS_LIBSSH2_SFTP_FSYNC > +#ifdef HAS_LIBSSH_SFTP_FSYNC > =20 > static coroutine_fn int ssh_flush(BDRVSSHState *s, BlockDriverState *b= s) > { > int r; > =20 > DPRINTF("fsync"); > + > + if (!sftp_extension_supported(s->sftp, "fsync@openssh.com", "1")) = { > + unsafe_flush_warning(s, "OpenSSH >=3D 6.3"); > + return 0; > + } > again: > - r =3D libssh2_sftp_fsync(s->sftp_handle); > - if (r =3D=3D LIBSSH2_ERROR_EAGAIN || r =3D=3D LIBSSH2_ERROR_TIMEOU= T) { > + r =3D sftp_fsync(s->sftp_handle); > + if (r =3D=3D SSH_AGAIN) { > co_yield(s, bs); > goto again; > } > - if (r =3D=3D LIBSSH2_ERROR_SFTP_PROTOCOL && > - libssh2_sftp_last_error(s->sftp) =3D=3D LIBSSH2_FX_OP_UNSUPPOR= TED) { > - unsafe_flush_warning(s, "OpenSSH >=3D 6.3"); > - return 0; > - } > if (r < 0) { > sftp_error_report(s, "fsync failed"); > return -EIO; > @@ -1086,25 +967,25 @@ static coroutine_fn int ssh_co_flush(BlockDriver= State *bs) > return ret; > } > =20 > -#else /* !HAS_LIBSSH2_SFTP_FSYNC */ > +#else /* !HAS_LIBSSH_SFTP_FSYNC */ > =20 > static coroutine_fn int ssh_co_flush(BlockDriverState *bs) > { > BDRVSSHState *s =3D bs->opaque; > =20 > - unsafe_flush_warning(s, "libssh2 >=3D 1.4.4"); > + unsafe_flush_warning(s, "libssh >=3D 0.8.0"); > return 0; > } > =20 > -#endif /* !HAS_LIBSSH2_SFTP_FSYNC */ > +#endif /* !HAS_LIBSSH_SFTP_FSYNC */ > =20 > static int64_t ssh_getlength(BlockDriverState *bs) > { > BDRVSSHState *s =3D bs->opaque; > int64_t length; > =20 > - /* Note we cannot make a libssh2 call here. */ > - length =3D (int64_t) s->attrs.filesize; > + /* Note we cannot make a libssh call here. */ > + length =3D (int64_t) s->attrs->size; > DPRINTF("length=3D%" PRIi64, length); > =20 > return length; > @@ -1130,12 +1011,16 @@ static void bdrv_ssh_init(void) > { > int r; > =20 > - r =3D libssh2_init(0); > + r =3D ssh_init(); > if (r !=3D 0) { > - fprintf(stderr, "libssh2 initialization failed, %d\n", r); > + fprintf(stderr, "libssh initialization failed, %d\n", r); > exit(EXIT_FAILURE); > } > =20 > +#if TRACE_LIBSSH !=3D 0 > + ssh_set_log_level(TRACE_LIBSSH); > +#endif > + > bdrv_register(&bdrv_ssh); > } > =20 > diff --git a/configure b/configure > index dd9e679..ff48c29 100755 > --- a/configure > +++ b/configure > @@ -316,7 +316,7 @@ gcrypt_kdf=3D"no" > vte=3D"" > virglrenderer=3D"" > tpm=3D"yes" > -libssh2=3D"" > +libssh=3D"" > numa=3D"" > tcmalloc=3D"no" > jemalloc=3D"no" > @@ -1142,9 +1142,9 @@ for opt do > ;; > --enable-tpm) tpm=3D"yes" > ;; > - --disable-libssh2) libssh2=3D"no" > + --disable-libssh) libssh=3D"no" > ;; > - --enable-libssh2) libssh2=3D"yes" > + --enable-libssh) libssh=3D"yes" > ;; > --disable-numa) numa=3D"no" > ;; > @@ -1386,7 +1386,7 @@ disabled with --disable-FEATURE, default is enabl= ed if available: > glusterfs GlusterFS backend > archipelago Archipelago backend > tpm TPM support > - libssh2 ssh block device support > + libssh ssh block device support > numa libnuma support > tcmalloc tcmalloc support > jemalloc jemalloc support > @@ -3206,43 +3206,42 @@ EOF > fi > =20 > ########################################## > -# libssh2 probe > -min_libssh2_version=3D1.2.8 > -if test "$libssh2" !=3D "no" ; then > - if $pkg_config --atleast-version=3D$min_libssh2_version libssh2; the= n > - libssh2_cflags=3D$($pkg_config libssh2 --cflags) > - libssh2_libs=3D$($pkg_config libssh2 --libs) > - libssh2=3Dyes > +# libssh probe > +if test "$libssh" !=3D "no" ; then > + if $pkg_config --exists libssh; then > + libssh_cflags=3D$($pkg_config libssh --cflags) > + libssh_libs=3D$($pkg_config libssh --libs) > + libssh=3Dyes > else > - if test "$libssh2" =3D "yes" ; then > - error_exit "libssh2 >=3D $min_libssh2_version required for --ena= ble-libssh2" > + if test "$libssh" =3D "yes" ; then > + error_exit "libssh required for --enable-libssh" > fi > - libssh2=3Dno > + libssh=3Dno > fi > fi > =20 > ########################################## > -# libssh2_sftp_fsync probe > +# libssh sftp_fsync probe > =20 > -if test "$libssh2" =3D "yes"; then > +if test "$libssh" =3D "yes"; then > cat > $TMPC < #include > -#include > -#include > +#include > +#include > int main(void) { > - LIBSSH2_SESSION *session; > - LIBSSH2_SFTP *sftp; > - LIBSSH2_SFTP_HANDLE *sftp_handle; > - session =3D libssh2_session_init (); > - sftp =3D libssh2_sftp_init (session); > - sftp_handle =3D libssh2_sftp_open (sftp, "/", 0, 0); > - libssh2_sftp_fsync (sftp_handle); > + ssh_session session; > + sftp_session sftp; > + sftp_file sftp_handle; > + session =3D ssh_new(); > + sftp =3D sftp_new(session); > + sftp_handle =3D sftp_open(sftp, "/", 0, 0); > + sftp_fsync(sftp_handle); > return 0; > } > EOF > - # libssh2_cflags/libssh2_libs defined in previous test. > - if compile_prog "$libssh2_cflags" "$libssh2_libs" ; then > - QEMU_CFLAGS=3D"-DHAS_LIBSSH2_SFTP_FSYNC $QEMU_CFLAGS" > + # libssh_cflags/libssh_libs defined in previous test. > + if compile_prog "$libssh_cflags" "$libssh_libs" ; then > + QEMU_CFLAGS=3D"-DHAS_LIBSSH_SFTP_FSYNC $QEMU_CFLAGS" > fi > fi > =20 > @@ -4949,7 +4948,7 @@ echo "Archipelago support $archipelago" > echo "gcov $gcov_tool" > echo "gcov enabled $gcov" > echo "TPM support $tpm" > -echo "libssh2 support $libssh2" > +echo "libssh support $libssh" > echo "TPM passthrough $tpm_passthrough" > echo "QOM debugging $qom_cast_debug" > echo "lzo support $lzo" > @@ -5474,10 +5473,10 @@ if test "$archipelago" =3D "yes" ; then > echo "ARCHIPELAGO_LIBS=3D$archipelago_libs" >> $config_host_mak > fi > =20 > -if test "$libssh2" =3D "yes" ; then > - echo "CONFIG_LIBSSH2=3Dm" >> $config_host_mak > - echo "LIBSSH2_CFLAGS=3D$libssh2_cflags" >> $config_host_mak > - echo "LIBSSH2_LIBS=3D$libssh2_libs" >> $config_host_mak > +if test "$libssh" =3D "yes" ; then > + echo "CONFIG_LIBSSH=3Dm" >> $config_host_mak > + echo "LIBSSH_CFLAGS=3D$libssh_cflags" >> $config_host_mak > + echo "LIBSSH_LIBS=3D$libssh_libs" >> $config_host_mak > fi > =20 > # USB host support > --=20 > 2.7.4 --=20 Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rj= ones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW