* [Qemu-devel] [PATCH] ssh: switch from libssh2 to libssh @ 2016-10-20 15:15 Pino Toscano 2016-10-20 15:32 ` Richard W.M. Jones 2016-10-20 16:04 ` Stefan Weil 0 siblings, 2 replies; 6+ messages in thread From: Pino Toscano @ 2016-10-20 15:15 UTC (permalink / raw) To: qemu-devel, qemu-block; +Cc: rjones, jcody, kwolf, mreitz, ptoscano Rewrite the implementation of the ssh block driver to use libssh instead 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 Kerberos authentication can be enabled once the libssh bug for it [1] is fixed. The development version of libssh (i.e. the future 0.8.x) supports fsync, so reuse the build time check for this. [1] https://red.libssh.org/issues/242 Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- block/Makefile.objs | 6 +- block/ssh.c | 543 +++++++++++++++++++++------------------------------- configure | 65 ++++--- 3 files changed, 249 insertions(+), 365 deletions(-) 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) += curl.o block-obj-$(CONFIG_RBD) += rbd.o block-obj-$(CONFIG_GLUSTERFS) += gluster.o block-obj-$(CONFIG_ARCHIPELAGO) += archipelago.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 @@ -38,8 +38,8 @@ rbd.o-cflags := $(RBD_CFLAGS) rbd.o-libs := $(RBD_LIBS) gluster.o-cflags := $(GLUSTERFS_CFLAGS) gluster.o-libs := $(GLUSTERFS_LIBS) -ssh.o-cflags := $(LIBSSH2_CFLAGS) -ssh.o-libs := $(LIBSSH2_LIBS) +ssh.o-cflags := $(LIBSSH_CFLAGS) +ssh.o-libs := $(LIBSSH_LIBS) archipelago.o-libs := $(ARCHIPELAGO_LIBS) block-obj-$(if $(CONFIG_BZIP2),m,n) += dmg-bz2.o dmg-bz2.o-libs := $(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 @@ #include "qemu/osdep.h" -#include <libssh2.h> -#include <libssh2_sftp.h> +#include <libssh/libssh.h> +#include <libssh/sftp.h> #include "block/block_int.h" #include "qapi/error.h" @@ -38,14 +38,12 @@ /* DEBUG_SSH=1 enables the DPRINTF (debugging printf) statements in * this block driver code. * - * TRACE_LIBSSH2=<bitmask> 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 <bitmask> is described - * here: http://www.libssh2.org/libssh2_trace.html + * TRACE_LIBSSH=<level> enables tracing in libssh itself. + * The meaning of <level> 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_* */ #define DPRINTF(fmt, ...) \ do { \ @@ -60,50 +58,39 @@ typedef struct BDRVSSHState { CoMutex lock; /* 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 */ - /* 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; /* Used to warn if 'flush' is not supported. */ - char *hostport; bool unsafe_flush_warning; } BDRVSSHState; static void ssh_state_init(BDRVSSHState *s) { memset(s, 0, sizeof *s); - s->sock = -1; - s->offset = -1; qemu_co_mutex_init(&s->lock); } 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 >= 0) { - close(s->sock); + ssh_disconnect(s->session); + ssh_free(s->session); } } @@ -118,13 +105,13 @@ session_error_setg(Error **errp, BDRVSSHState *s, const char *fs, ...) va_end(args); if (s->session) { - char *ssh_err; + const char *ssh_err; int ssh_err_code; - /* This is not an errno. See <libssh2.h>. */ - ssh_err_code = 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 <libssh/libssh.h>. */ + ssh_err = ssh_get_error(s->session); + ssh_err_code = 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, const char *fs, ...) va_end(args); if (s->sftp) { - char *ssh_err; - int ssh_err_code; - unsigned long sftp_err_code; + int sftp_err_code; - /* This is not an errno. See <libssh2.h>. */ - ssh_err_code = libssh2_session_last_error(s->session, - &ssh_err, NULL, 0); - /* See <libssh2_sftp.h>. */ - sftp_err_code = libssh2_sftp_last_error((s)->sftp); + /* This is not an errno. See <libssh/sftp.h>. */ + sftp_err_code = sftp_get_error(s->sftp); 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); if ((s)->sftp) { - char *ssh_err; - int ssh_err_code; - unsigned long sftp_err_code; + int sftp_err_code; - /* This is not an errno. See <libssh2.h>. */ - ssh_err_code = libssh2_session_last_error(s->session, - &ssh_err, NULL, 0); - /* See <libssh2_sftp.h>. */ - sftp_err_code = libssh2_sftp_last_error((s)->sftp); + /* This is not an errno. See <libssh/sftp.h>. */ + sftp_err_code = sftp_get_error(s->sftp); - 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); } va_end(args); @@ -272,68 +249,41 @@ static void ssh_parse_filename(const char *filename, QDict *options, static int check_host_key_knownhosts(BDRVSSHState *s, const char *host, int port, Error **errp) { - const char *home; - char *knh_file = NULL; - LIBSSH2_KNOWNHOSTS *knh = NULL; - struct libssh2_knownhost *found; - int ret, r; - const char *hostkey; - size_t len; - int type; + int ret; + int state; - hostkey = libssh2_session_hostkey(s->session, &len, &type); - if (!hostkey) { - ret = -EINVAL; - session_error_setg(errp, s, "failed to read remote host key"); - goto out; - } + state = ssh_is_server_known(s->session); - knh = libssh2_knownhost_init(s->session); - if (!knh) { - ret = -EINVAL; - session_error_setg(errp, s, - "failed to initialize known hosts support"); - goto out; - } - - home = getenv("HOME"); - if (home) { - knh_file = g_strdup_printf("%s/.ssh/known_hosts", home); - } else { - knh_file = 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_OPENSSH); - - r = 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 = -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 = -EINVAL; + session_error_setg(errp, s, + "host key for this server not found, another type " + "exists"); + goto out; + case SSH_SERVER_FILE_NOT_FOUND: + ret = -EINVAL; + session_error_setg(errp, s, "known_hosts file not found"); + goto out; + case SSH_SERVER_NOT_KNOWN: ret = -EINVAL; session_error_setg(errp, s, "no host key was found in known_hosts"); goto out; - case LIBSSH2_KNOWNHOST_CHECK_FAILURE: + case SSH_SERVER_ERROR: ret = -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 = -EINVAL; - session_error_setg(errp, s, "unknown error matching the host key" - " with known_hosts (%d)", r); + session_error_setg(errp, s, "error while checking for known server"); goto out; } @@ -341,10 +291,6 @@ static int check_host_key_knownhosts(BDRVSSHState *s, ret = 0; out: - if (knh != NULL) { - libssh2_knownhost_free(knh); - } - g_free(knh_file); return ret; } @@ -388,23 +334,37 @@ static int compare_fingerprint(const unsigned char *fingerprint, size_t len, static int check_host_key_hash(BDRVSSHState *s, const char *hash, - int hash_type, size_t fingerprint_len, Error **errp) + enum ssh_publickey_hash_type type, size_t fingerprint_len, + Error **errp) { - const char *fingerprint; + int r; + ssh_key pubkey; + unsigned char *server_hash; + size_t server_hash_len; - fingerprint = libssh2_hostkey_hash(s->session, hash_type); - if (!fingerprint) { + r = ssh_get_publickey(s->session, &pubkey); + if (r < 0) { session_error_setg(errp, s, "failed to read remote host key"); return -EINVAL; } - if(compare_fingerprint((unsigned char *) fingerprint, fingerprint_len, - hash) != 0) { + r = ssh_get_publickey_hash(pubkey, type, &server_hash, &server_hash_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) != 0) { + ssh_clean_pubkey_hash(&server_hash); error_setg(errp, "remote host key does not match host_key_check '%s'", hash); return -EPERM; } + ssh_clean_pubkey_hash(&server_hash); + return 0; } @@ -419,13 +379,13 @@ static int check_host_key(BDRVSSHState *s, const char *host, int port, /* host_key_check=md5:xx:yy:zz:... */ if (strncmp(host_key_check, "md5:", 4) == 0) { return check_host_key_hash(s, &host_key_check[4], - LIBSSH2_HOSTKEY_HASH_MD5, 16, errp); + SSH_PUBLICKEY_HASH_MD5, 16, errp); } /* host_key_check=sha1:xx:yy:zz:... */ if (strncmp(host_key_check, "sha1:", 5) == 0) { return check_host_key_hash(s, &host_key_check[5], - LIBSSH2_HOSTKEY_HASH_SHA1, 20, errp); + SSH_PUBLICKEY_HASH_SHA1, 20, errp); } /* host_key_check=yes */ @@ -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 **errp) { int r, ret; - const char *userauthlist; - LIBSSH2_AGENT *agent = NULL; - struct libssh2_agent_publickey *identity; - struct libssh2_agent_publickey *prev_identity = NULL; + int method; - userauthlist = libssh2_userauth_list(s->session, user, strlen(user)); - if (strstr(userauthlist, "publickey") == NULL) { + r = ssh_userauth_none(s->session, NULL); + if (r == SSH_AUTH_ERROR) { ret = -EPERM; - error_setg(errp, - "remote server does not support \"publickey\" authentication"); + session_error_setg(errp, s, "failed to call ssh_userauth_none"); goto out; } - /* Connect to ssh-agent and try each identity in turn. */ - agent = libssh2_agent_init(s->session); - if (!agent) { - ret = -EINVAL; - session_error_setg(errp, s, "failed to initialize ssh-agent support"); - goto out; - } - if (libssh2_agent_connect(agent)) { - ret = -ECONNREFUSED; - session_error_setg(errp, s, "failed to connect to ssh-agent"); - goto out; - } - if (libssh2_agent_list_identities(agent)) { - ret = -EINVAL; - session_error_setg(errp, s, - "failed requesting identities from ssh-agent"); - goto out; - } + method = ssh_userauth_list(s->session, NULL); - for(;;) { - r = libssh2_agent_get_identity(agent, &identity, prev_identity); - if (r == 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 = ssh_userauth_publickey_auto(s->session, NULL, NULL); + if (r == SSH_AUTH_ERROR) { ret = -EINVAL; - session_error_setg(errp, s, - "failed to obtain identity from ssh-agent"); + error_setg(errp, "failed to authenticate using publickey " + "authentication"); goto out; - } - r = libssh2_agent_userauth(agent, user, identity); - if (r == 0) { + } else if (r == SSH_AUTH_SUCCESS) { /* Authenticated! */ ret = 0; goto out; } - /* Failed to authenticate with this identity, try the next one. */ - prev_identity = identity; } ret = -EPERM; @@ -498,13 +433,6 @@ static int authenticate(BDRVSSHState *s, const char *user, Error **errp) "and the identities held by your ssh-agent"); out: - if (agent != NULL) { - /* Note: libssh2 implementation implicitly calls - * libssh2_agent_disconnect if necessary. - */ - libssh2_agent_free(agent); - } - return ret; } @@ -547,7 +475,7 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options, QemuOpts *opts = NULL; Error *local_err = NULL; const char *host, *user, *path, *host_key_check; - int port; + unsigned int port; opts = 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 = "yes"; } - /* Construct the host:port name for inet_connect. */ - g_free(s->hostport); - s->hostport = g_strdup_printf("%s:%d", host, port); - - /* Open the socket and connect. */ - s->sock = inet_connect(s->hostport, errp); - if (s->sock < 0) { - ret = -EIO; - goto err; - } - /* Create SSH session. */ - s->session = libssh2_session_init(); + s->session = 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 = ssh_options_set(s->session, SSH_OPTIONS_USER, user); + if (r < 0) { ret = -EINVAL; - session_error_setg(errp, s, "failed to initialize libssh2 session"); + session_error_setg(errp, s, + "failed to set the user in the libssh session"); goto err; } -#if TRACE_LIBSSH2 != 0 - libssh2_trace(s->session, TRACE_LIBSSH2); -#endif + r = ssh_options_set(s->session, SSH_OPTIONS_HOST, host); + if (r < 0) { + ret = -EINVAL; + session_error_setg(errp, s, + "failed to set the host in the libssh session"); + goto err; + } + + if (port > 0) { + r = ssh_options_set(s->session, SSH_OPTIONS_PORT, &port); + if (r < 0) { + ret = -EINVAL; + session_error_setg(errp, s, + "failed to set the port in the libssh session"); + goto err; + } + } + + /* Read ~/.ssh/config. */ + r = ssh_options_parse_config(s->session, NULL); + if (r < 0) { + ret = -EINVAL; + session_error_setg(errp, s, "failed to parse ~/.ssh/config"); + goto err; + } - r = libssh2_session_handshake(s->session, s->sock); - if (r != 0) { + /* Connect. */ + r = ssh_connect(s->session); + if (r < 0) { ret = -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, } /* Start SFTP. */ - s->sftp = libssh2_sftp_init(s->session); + s->sftp = sftp_new(s->session); if (!s->sftp) { + session_error_setg(errp, s, "failed to create sftp handle"); + ret = -EINVAL; + goto err; + } + + r = sftp_init(s->sftp); + if (r < 0) { session_error_setg(errp, s, "failed to initialize sftp handle"); ret = -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=0x%x creat_mode=0%o", path, ssh_flags, creat_mode); - s->sftp_handle = libssh2_sftp_open(s->sftp, path, ssh_flags, creat_mode); + s->sftp_handle = 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 = -EINVAL; goto err; } + /* Make sure the SFTP file is handled in blocking mode. */ + sftp_file_set_blocking(s->sftp_handle); + qemu_opts_del(opts); - r = libssh2_sftp_fstat(s->sftp_handle, &s->attrs); - if (r < 0) { + s->attrs = 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; err: + if (s->attrs) { + sftp_attributes_free(s->attrs); + } + s->attrs = NULL; if (s->sftp_handle) { - libssh2_sftp_close(s->sftp_handle); + sftp_close(s->sftp_handle); } s->sftp_handle = NULL; if (s->sftp) { - libssh2_sftp_shutdown(s->sftp); + sftp_free(s->sftp); } s->sftp = 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 = NULL; @@ -689,9 +652,11 @@ static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags, ssh_state_init(s); - ssh_flags = LIBSSH2_FXF_READ; + ssh_flags = 0; if (bdrv_flags & BDRV_O_RDWR) { - ssh_flags |= LIBSSH2_FXF_WRITE; + ssh_flags |= O_RDWR; + } else { + ssh_flags |= O_RDONLY; } /* Start up SSH. */ @@ -701,15 +666,11 @@ static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags, } /* Go non-blocking. */ - libssh2_session_set_blocking(s->session, 0); + ssh_set_blocking(s->session, 0); return 0; err: - if (s->sock >= 0) { - close(s->sock); - } - s->sock = -1; return ret; } @@ -751,8 +712,7 @@ static int ssh_create(const char *filename, QemuOpts *opts, Error **errp) } r = 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 = r; @@ -760,14 +720,14 @@ static int ssh_create(const char *filename, QemuOpts *opts, Error **errp) } if (total_size > 0) { - libssh2_sftp_seek64(s.sftp_handle, total_size-1); - r2 = libssh2_sftp_write(s.sftp_handle, c, 1); + sftp_seek64(s.sftp_handle, total_size - 1); + r2 = sftp_write(s.sftp_handle, c, 1); if (r2 < 0) { sftp_error_setg(errp, &s, "truncate failed"); ret = -EINVAL; goto out; } - s.attrs.filesize = total_size; + s.attrs->size = total_size; } ret = 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 = 0; - if (s->attrs.flags & LIBSSH2_SFTP_ATTR_PERMISSIONS) { - if (s->attrs.permissions & LIBSSH2_SFTP_S_IFREG) { - has_zero_init = 1; - } + if (s->attrs->type == SSH_FILEXFER_TYPE_REGULAR) { + has_zero_init = 1; } return has_zero_init; } -static void restart_coroutine(void *opaque) -{ - Coroutine *co = opaque; - - DPRINTF("co=%p", co); - - qemu_coroutine_enter(co); -} - -static coroutine_fn void set_fd_handler(BDRVSSHState *s, BlockDriverState *bs) -{ - int r; - IOHandler *rd_handler = NULL, *wr_handler = NULL; - Coroutine *co = qemu_coroutine_self(); - - r = libssh2_session_block_directions(s->session); - - if (r & LIBSSH2_SESSION_BLOCK_INBOUND) { - rd_handler = restart_coroutine; - } - if (r & LIBSSH2_SESSION_BLOCK_OUTBOUND) { - wr_handler = restart_coroutine; - } - - DPRINTF("s->sock=%d rd_handler=%p wr_handler=%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=%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 *bs) { - 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 function. - */ -#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 = (flags & SSH_SEEK_READ) != 0; - bool force = (flags & SSH_SEEK_FORCE) != 0; - - if (force || op_read != s->offset_op_read || offset != s->offset) { - DPRINTF("seeking to offset=%" PRIi64, offset); - libssh2_sftp_seek64(s->sftp_handle, offset); - s->offset = offset; - s->offset_op_read = op_read; - } } static coroutine_fn int ssh_read(BDRVSSHState *s, BlockDriverState *bs, @@ -890,7 +780,7 @@ static coroutine_fn int ssh_read(BDRVSSHState *s, BlockDriverState *bs, DPRINTF("offset=%" PRIi64 " size=%zu", offset, size); - ssh_seek(s, offset, SSH_SEEK_READ); + sftp_seek64(s->sftp_handle, offset); /* 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 = i->iov_base; end_of_vec = i->iov_base + i->iov_len; - /* 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 = 0; got < size; ) { again: - DPRINTF("sftp_read buf=%p size=%zu", buf, end_of_vec - buf); - r = libssh2_sftp_read(s->sftp_handle, buf, end_of_vec - buf); + DPRINTF("sftp_read buf=%p size=%zu (actual size=%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 = sftp_read(s->sftp_handle, buf, MIN(end_of_vec - buf, 16384)); DPRINTF("sftp_read returned %zd", r); - if (r == LIBSSH2_ERROR_EAGAIN || r == LIBSSH2_ERROR_TIMEOUT) { + if (r == SSH_AGAIN) { co_yield(s, bs); goto again; } - if (r < 0) { - sftp_error_report(s, "read failed"); - s->offset = -1; - return -EIO; - } - if (r == 0) { + if (r == SSH_EOF) { /* EOF: Short read so pad the buffer with zeroes and return it. */ qemu_iovec_memset(qiov, got, 0, size - got); return 0; } + if (r < 0) { + sftp_error_report(s, "read failed"); + return -EIO; + } got += r; buf += r; - s->offset += r; if (buf >= end_of_vec && got < size) { i++; buf = i->iov_base; @@ -965,7 +854,7 @@ static int ssh_write(BDRVSSHState *s, BlockDriverState *bs, DPRINTF("offset=%" PRIi64 " size=%zu", offset, size); - ssh_seek(s, offset, SSH_SEEK_WRITE); + sftp_seek64(s->sftp_handle, offset); /* 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, BlockDriverState *bs, for (written = 0; written < size; ) { again: DPRINTF("sftp_write buf=%p size=%zu", buf, end_of_vec - buf); - r = libssh2_sftp_write(s->sftp_handle, buf, end_of_vec - buf); + r = sftp_write(s->sftp_handle, buf, end_of_vec - buf); DPRINTF("sftp_write returned %zd", r); - if (r == LIBSSH2_ERROR_EAGAIN || r == LIBSSH2_ERROR_TIMEOUT) { + if (r == SSH_AGAIN) { co_yield(s, bs); goto again; } if (r < 0) { sftp_error_report(s, "write failed"); - s->offset = -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 == 0) { - ssh_seek(s, offset + written, SSH_SEEK_WRITE|SSH_SEEK_FORCE); - co_yield(s, bs); - goto again; - } written += r; buf += r; - s->offset += r; if (buf >= end_of_vec && written < size) { i++; buf = i->iov_base; end_of_vec = i->iov_base + i->iov_len; } - if (offset + written > s->attrs.filesize) - s->attrs.filesize = offset + written; + if (offset + written > s->attrs->size) { + s->attrs->size = offset + written; + } } return 0; @@ -1039,33 +913,40 @@ static coroutine_fn int ssh_co_writev(BlockDriverState *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 = true; } } -#ifdef HAS_LIBSSH2_SFTP_FSYNC +#ifdef HAS_LIBSSH_SFTP_FSYNC static coroutine_fn int ssh_flush(BDRVSSHState *s, BlockDriverState *bs) { int r; DPRINTF("fsync"); + + if (!sftp_extension_supported(s->sftp, "fsync@openssh.com", "1")) { + unsafe_flush_warning(s, "OpenSSH >= 6.3"); + return 0; + } again: - r = libssh2_sftp_fsync(s->sftp_handle); - if (r == LIBSSH2_ERROR_EAGAIN || r == LIBSSH2_ERROR_TIMEOUT) { + r = sftp_fsync(s->sftp_handle); + if (r == SSH_AGAIN) { co_yield(s, bs); goto again; } - if (r == LIBSSH2_ERROR_SFTP_PROTOCOL && - libssh2_sftp_last_error(s->sftp) == LIBSSH2_FX_OP_UNSUPPORTED) { - unsafe_flush_warning(s, "OpenSSH >= 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(BlockDriverState *bs) return ret; } -#else /* !HAS_LIBSSH2_SFTP_FSYNC */ +#else /* !HAS_LIBSSH_SFTP_FSYNC */ static coroutine_fn int ssh_co_flush(BlockDriverState *bs) { BDRVSSHState *s = bs->opaque; - unsafe_flush_warning(s, "libssh2 >= 1.4.4"); + unsafe_flush_warning(s, "libssh >= 0.8.0"); return 0; } -#endif /* !HAS_LIBSSH2_SFTP_FSYNC */ +#endif /* !HAS_LIBSSH_SFTP_FSYNC */ static int64_t ssh_getlength(BlockDriverState *bs) { BDRVSSHState *s = bs->opaque; int64_t length; - /* Note we cannot make a libssh2 call here. */ - length = (int64_t) s->attrs.filesize; + /* Note we cannot make a libssh call here. */ + length = (int64_t) s->attrs->size; DPRINTF("length=%" PRIi64, length); return length; @@ -1130,12 +1011,16 @@ static void bdrv_ssh_init(void) { int r; - r = libssh2_init(0); + r = ssh_init(); if (r != 0) { - fprintf(stderr, "libssh2 initialization failed, %d\n", r); + fprintf(stderr, "libssh initialization failed, %d\n", r); exit(EXIT_FAILURE); } +#if TRACE_LIBSSH != 0 + ssh_set_log_level(TRACE_LIBSSH); +#endif + bdrv_register(&bdrv_ssh); } diff --git a/configure b/configure index dd9e679..ff48c29 100755 --- a/configure +++ b/configure @@ -316,7 +316,7 @@ gcrypt_kdf="no" vte="" virglrenderer="" tpm="yes" -libssh2="" +libssh="" numa="" tcmalloc="no" jemalloc="no" @@ -1142,9 +1142,9 @@ for opt do ;; --enable-tpm) tpm="yes" ;; - --disable-libssh2) libssh2="no" + --disable-libssh) libssh="no" ;; - --enable-libssh2) libssh2="yes" + --enable-libssh) libssh="yes" ;; --disable-numa) numa="no" ;; @@ -1386,7 +1386,7 @@ disabled with --disable-FEATURE, default is enabled 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 ########################################## -# libssh2 probe -min_libssh2_version=1.2.8 -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 + libssh=no fi fi ########################################## -# libssh2_sftp_fsync probe +# libssh sftp_fsync probe -if test "$libssh2" = "yes"; then +if test "$libssh" = "yes"; then cat > $TMPC <<EOF #include <stdio.h> -#include <libssh2.h> -#include <libssh2_sftp.h> +#include <libssh/libssh.h> +#include <libssh/sftp.h> int main(void) { - LIBSSH2_SESSION *session; - LIBSSH2_SFTP *sftp; - LIBSSH2_SFTP_HANDLE *sftp_handle; - session = libssh2_session_init (); - sftp = libssh2_sftp_init (session); - sftp_handle = libssh2_sftp_open (sftp, "/", 0, 0); - libssh2_sftp_fsync (sftp_handle); + ssh_session session; + sftp_session sftp; + sftp_file sftp_handle; + session = ssh_new(); + sftp = sftp_new(session); + sftp_handle = 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="-DHAS_LIBSSH2_SFTP_FSYNC $QEMU_CFLAGS" + # libssh_cflags/libssh_libs defined in previous test. + if compile_prog "$libssh_cflags" "$libssh_libs" ; then + QEMU_CFLAGS="-DHAS_LIBSSH_SFTP_FSYNC $QEMU_CFLAGS" fi fi @@ -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" = "yes" ; then echo "ARCHIPELAGO_LIBS=$archipelago_libs" >> $config_host_mak fi -if test "$libssh2" = "yes" ; then - echo "CONFIG_LIBSSH2=m" >> $config_host_mak - echo "LIBSSH2_CFLAGS=$libssh2_cflags" >> $config_host_mak - echo "LIBSSH2_LIBS=$libssh2_libs" >> $config_host_mak +if test "$libssh" = "yes" ; then + echo "CONFIG_LIBSSH=m" >> $config_host_mak + echo "LIBSSH_CFLAGS=$libssh_cflags" >> $config_host_mak + echo "LIBSSH_LIBS=$libssh_libs" >> $config_host_mak fi # USB host support -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] ssh: switch from libssh2 to libssh 2016-10-20 15:15 [Qemu-devel] [PATCH] ssh: switch from libssh2 to libssh Pino Toscano @ 2016-10-20 15:32 ` Richard W.M. Jones 2016-10-20 15:44 ` Pino Toscano 2016-10-20 16:04 ` Stefan Weil 1 sibling, 1 reply; 6+ messages in thread From: Richard W.M. Jones @ 2016-10-20 15:32 UTC (permalink / raw) To: Pino Toscano; +Cc: qemu-devel, qemu-block, jcody, kwolf, mreitz On Thu, Oct 20, 2016 at 05:15:24PM +0200, Pino Toscano wrote: > Rewrite the implementation of the ssh block driver to use libssh instead > 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 > > Kerberos authentication can be enabled once the libssh bug for it [1] is > fixed. > > The development version of libssh (i.e. the future 0.8.x) supports > fsync, so reuse the build time check for this. > > [1] https://red.libssh.org/issues/242 > > Signed-off-by: Pino Toscano <ptoscano@redhat.com> When I applied this patch and compiled it with warnings enabled: block/ssh.c: In function ‘connect_to_ssh’: block/ssh.c:643:12: warning: ‘ret’ may be used uninitialized 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=kvm -cpu host -m 2048 \ -drive file.driver=ssh,file.user=[user],file.host=[host],file.path=/var/tmp/fedora-24.img,format=raw,if=virtio \ -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(-) > > 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) += curl.o > block-obj-$(CONFIG_RBD) += rbd.o > block-obj-$(CONFIG_GLUSTERFS) += gluster.o > block-obj-$(CONFIG_ARCHIPELAGO) += archipelago.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 > @@ -38,8 +38,8 @@ rbd.o-cflags := $(RBD_CFLAGS) > rbd.o-libs := $(RBD_LIBS) > gluster.o-cflags := $(GLUSTERFS_CFLAGS) > gluster.o-libs := $(GLUSTERFS_LIBS) > -ssh.o-cflags := $(LIBSSH2_CFLAGS) > -ssh.o-libs := $(LIBSSH2_LIBS) > +ssh.o-cflags := $(LIBSSH_CFLAGS) > +ssh.o-libs := $(LIBSSH_LIBS) > archipelago.o-libs := $(ARCHIPELAGO_LIBS) > block-obj-$(if $(CONFIG_BZIP2),m,n) += dmg-bz2.o > dmg-bz2.o-libs := $(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 @@ > > #include "qemu/osdep.h" > > -#include <libssh2.h> > -#include <libssh2_sftp.h> > +#include <libssh/libssh.h> > +#include <libssh/sftp.h> > > #include "block/block_int.h" > #include "qapi/error.h" > @@ -38,14 +38,12 @@ > /* DEBUG_SSH=1 enables the DPRINTF (debugging printf) statements in > * this block driver code. > * > - * TRACE_LIBSSH2=<bitmask> 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 <bitmask> is described > - * here: http://www.libssh2.org/libssh2_trace.html > + * TRACE_LIBSSH=<level> enables tracing in libssh itself. > + * The meaning of <level> 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_* */ > > #define DPRINTF(fmt, ...) \ > do { \ > @@ -60,50 +58,39 @@ typedef struct BDRVSSHState { > CoMutex lock; > > /* 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 */ > > - /* 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; > > /* Used to warn if 'flush' is not supported. */ > - char *hostport; > bool unsafe_flush_warning; > } BDRVSSHState; > > static void ssh_state_init(BDRVSSHState *s) > { > memset(s, 0, sizeof *s); > - s->sock = -1; > - s->offset = -1; > qemu_co_mutex_init(&s->lock); > } > > 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 >= 0) { > - close(s->sock); > + ssh_disconnect(s->session); > + ssh_free(s->session); > } > } > > @@ -118,13 +105,13 @@ session_error_setg(Error **errp, BDRVSSHState *s, const char *fs, ...) > va_end(args); > > if (s->session) { > - char *ssh_err; > + const char *ssh_err; > int ssh_err_code; > > - /* This is not an errno. See <libssh2.h>. */ > - ssh_err_code = 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 <libssh/libssh.h>. */ > + ssh_err = ssh_get_error(s->session); > + ssh_err_code = 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, const char *fs, ...) > va_end(args); > > if (s->sftp) { > - char *ssh_err; > - int ssh_err_code; > - unsigned long sftp_err_code; > + int sftp_err_code; > > - /* This is not an errno. See <libssh2.h>. */ > - ssh_err_code = libssh2_session_last_error(s->session, > - &ssh_err, NULL, 0); > - /* See <libssh2_sftp.h>. */ > - sftp_err_code = libssh2_sftp_last_error((s)->sftp); > + /* This is not an errno. See <libssh/sftp.h>. */ > + sftp_err_code = sftp_get_error(s->sftp); > > 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); > > if ((s)->sftp) { > - char *ssh_err; > - int ssh_err_code; > - unsigned long sftp_err_code; > + int sftp_err_code; > > - /* This is not an errno. See <libssh2.h>. */ > - ssh_err_code = libssh2_session_last_error(s->session, > - &ssh_err, NULL, 0); > - /* See <libssh2_sftp.h>. */ > - sftp_err_code = libssh2_sftp_last_error((s)->sftp); > + /* This is not an errno. See <libssh/sftp.h>. */ > + sftp_err_code = sftp_get_error(s->sftp); > > - 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); > } > > va_end(args); > @@ -272,68 +249,41 @@ static void ssh_parse_filename(const char *filename, QDict *options, > static int check_host_key_knownhosts(BDRVSSHState *s, > const char *host, int port, Error **errp) > { > - const char *home; > - char *knh_file = NULL; > - LIBSSH2_KNOWNHOSTS *knh = NULL; > - struct libssh2_knownhost *found; > - int ret, r; > - const char *hostkey; > - size_t len; > - int type; > + int ret; > + int state; > > - hostkey = libssh2_session_hostkey(s->session, &len, &type); > - if (!hostkey) { > - ret = -EINVAL; > - session_error_setg(errp, s, "failed to read remote host key"); > - goto out; > - } > + state = ssh_is_server_known(s->session); > > - knh = libssh2_knownhost_init(s->session); > - if (!knh) { > - ret = -EINVAL; > - session_error_setg(errp, s, > - "failed to initialize known hosts support"); > - goto out; > - } > - > - home = getenv("HOME"); > - if (home) { > - knh_file = g_strdup_printf("%s/.ssh/known_hosts", home); > - } else { > - knh_file = 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_OPENSSH); > - > - r = 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 = -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 = -EINVAL; > + session_error_setg(errp, s, > + "host key for this server not found, another type " > + "exists"); > + goto out; > + case SSH_SERVER_FILE_NOT_FOUND: > + ret = -EINVAL; > + session_error_setg(errp, s, "known_hosts file not found"); > + goto out; > + case SSH_SERVER_NOT_KNOWN: > ret = -EINVAL; > session_error_setg(errp, s, "no host key was found in known_hosts"); > goto out; > - case LIBSSH2_KNOWNHOST_CHECK_FAILURE: > + case SSH_SERVER_ERROR: > ret = -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 = -EINVAL; > - session_error_setg(errp, s, "unknown error matching the host key" > - " with known_hosts (%d)", r); > + session_error_setg(errp, s, "error while checking for known server"); > goto out; > } > > @@ -341,10 +291,6 @@ static int check_host_key_knownhosts(BDRVSSHState *s, > ret = 0; > > out: > - if (knh != NULL) { > - libssh2_knownhost_free(knh); > - } > - g_free(knh_file); > return ret; > } > > @@ -388,23 +334,37 @@ static int compare_fingerprint(const unsigned char *fingerprint, size_t len, > > static int > check_host_key_hash(BDRVSSHState *s, const char *hash, > - int hash_type, size_t fingerprint_len, Error **errp) > + enum ssh_publickey_hash_type type, size_t fingerprint_len, > + Error **errp) > { > - const char *fingerprint; > + int r; > + ssh_key pubkey; > + unsigned char *server_hash; > + size_t server_hash_len; > > - fingerprint = libssh2_hostkey_hash(s->session, hash_type); > - if (!fingerprint) { > + r = ssh_get_publickey(s->session, &pubkey); > + if (r < 0) { > session_error_setg(errp, s, "failed to read remote host key"); > return -EINVAL; > } > > - if(compare_fingerprint((unsigned char *) fingerprint, fingerprint_len, > - hash) != 0) { > + r = ssh_get_publickey_hash(pubkey, type, &server_hash, &server_hash_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) != 0) { > + ssh_clean_pubkey_hash(&server_hash); > error_setg(errp, "remote host key does not match host_key_check '%s'", > hash); > return -EPERM; > } > > + ssh_clean_pubkey_hash(&server_hash); > + > return 0; > } > > @@ -419,13 +379,13 @@ static int check_host_key(BDRVSSHState *s, const char *host, int port, > /* host_key_check=md5:xx:yy:zz:... */ > if (strncmp(host_key_check, "md5:", 4) == 0) { > return check_host_key_hash(s, &host_key_check[4], > - LIBSSH2_HOSTKEY_HASH_MD5, 16, errp); > + SSH_PUBLICKEY_HASH_MD5, 16, errp); > } > > /* host_key_check=sha1:xx:yy:zz:... */ > if (strncmp(host_key_check, "sha1:", 5) == 0) { > return check_host_key_hash(s, &host_key_check[5], > - LIBSSH2_HOSTKEY_HASH_SHA1, 20, errp); > + SSH_PUBLICKEY_HASH_SHA1, 20, errp); > } > > /* host_key_check=yes */ > @@ -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 **errp) > { > int r, ret; > - const char *userauthlist; > - LIBSSH2_AGENT *agent = NULL; > - struct libssh2_agent_publickey *identity; > - struct libssh2_agent_publickey *prev_identity = NULL; > + int method; > > - userauthlist = libssh2_userauth_list(s->session, user, strlen(user)); > - if (strstr(userauthlist, "publickey") == NULL) { > + r = ssh_userauth_none(s->session, NULL); > + if (r == SSH_AUTH_ERROR) { > ret = -EPERM; > - error_setg(errp, > - "remote server does not support \"publickey\" authentication"); > + session_error_setg(errp, s, "failed to call ssh_userauth_none"); > goto out; > } > > - /* Connect to ssh-agent and try each identity in turn. */ > - agent = libssh2_agent_init(s->session); > - if (!agent) { > - ret = -EINVAL; > - session_error_setg(errp, s, "failed to initialize ssh-agent support"); > - goto out; > - } > - if (libssh2_agent_connect(agent)) { > - ret = -ECONNREFUSED; > - session_error_setg(errp, s, "failed to connect to ssh-agent"); > - goto out; > - } > - if (libssh2_agent_list_identities(agent)) { > - ret = -EINVAL; > - session_error_setg(errp, s, > - "failed requesting identities from ssh-agent"); > - goto out; > - } > + method = ssh_userauth_list(s->session, NULL); > > - for(;;) { > - r = libssh2_agent_get_identity(agent, &identity, prev_identity); > - if (r == 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 = ssh_userauth_publickey_auto(s->session, NULL, NULL); > + if (r == SSH_AUTH_ERROR) { > ret = -EINVAL; > - session_error_setg(errp, s, > - "failed to obtain identity from ssh-agent"); > + error_setg(errp, "failed to authenticate using publickey " > + "authentication"); > goto out; > - } > - r = libssh2_agent_userauth(agent, user, identity); > - if (r == 0) { > + } else if (r == SSH_AUTH_SUCCESS) { > /* Authenticated! */ > ret = 0; > goto out; > } > - /* Failed to authenticate with this identity, try the next one. */ > - prev_identity = identity; > } > > ret = -EPERM; > @@ -498,13 +433,6 @@ static int authenticate(BDRVSSHState *s, const char *user, Error **errp) > "and the identities held by your ssh-agent"); > > out: > - if (agent != NULL) { > - /* Note: libssh2 implementation implicitly calls > - * libssh2_agent_disconnect if necessary. > - */ > - libssh2_agent_free(agent); > - } > - > return ret; > } > > @@ -547,7 +475,7 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options, > QemuOpts *opts = NULL; > Error *local_err = NULL; > const char *host, *user, *path, *host_key_check; > - int port; > + unsigned int port; > > opts = 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 = "yes"; > } > > - /* Construct the host:port name for inet_connect. */ > - g_free(s->hostport); > - s->hostport = g_strdup_printf("%s:%d", host, port); > - > - /* Open the socket and connect. */ > - s->sock = inet_connect(s->hostport, errp); > - if (s->sock < 0) { > - ret = -EIO; > - goto err; > - } > - > /* Create SSH session. */ > - s->session = libssh2_session_init(); > + s->session = 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 = ssh_options_set(s->session, SSH_OPTIONS_USER, user); > + if (r < 0) { > ret = -EINVAL; > - session_error_setg(errp, s, "failed to initialize libssh2 session"); > + session_error_setg(errp, s, > + "failed to set the user in the libssh session"); > goto err; > } > > -#if TRACE_LIBSSH2 != 0 > - libssh2_trace(s->session, TRACE_LIBSSH2); > -#endif > + r = ssh_options_set(s->session, SSH_OPTIONS_HOST, host); > + if (r < 0) { > + ret = -EINVAL; > + session_error_setg(errp, s, > + "failed to set the host in the libssh session"); > + goto err; > + } > + > + if (port > 0) { > + r = ssh_options_set(s->session, SSH_OPTIONS_PORT, &port); > + if (r < 0) { > + ret = -EINVAL; > + session_error_setg(errp, s, > + "failed to set the port in the libssh session"); > + goto err; > + } > + } > + > + /* Read ~/.ssh/config. */ > + r = ssh_options_parse_config(s->session, NULL); > + if (r < 0) { > + ret = -EINVAL; > + session_error_setg(errp, s, "failed to parse ~/.ssh/config"); > + goto err; > + } > > - r = libssh2_session_handshake(s->session, s->sock); > - if (r != 0) { > + /* Connect. */ > + r = ssh_connect(s->session); > + if (r < 0) { > ret = -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, > } > > /* Start SFTP. */ > - s->sftp = libssh2_sftp_init(s->session); > + s->sftp = sftp_new(s->session); > if (!s->sftp) { > + session_error_setg(errp, s, "failed to create sftp handle"); > + ret = -EINVAL; > + goto err; > + } > + > + r = sftp_init(s->sftp); > + if (r < 0) { > session_error_setg(errp, s, "failed to initialize sftp handle"); > ret = -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=0x%x creat_mode=0%o", > path, ssh_flags, creat_mode); > - s->sftp_handle = libssh2_sftp_open(s->sftp, path, ssh_flags, creat_mode); > + s->sftp_handle = 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 = -EINVAL; > goto err; > } > > + /* Make sure the SFTP file is handled in blocking mode. */ > + sftp_file_set_blocking(s->sftp_handle); > + > qemu_opts_del(opts); > > - r = libssh2_sftp_fstat(s->sftp_handle, &s->attrs); > - if (r < 0) { > + s->attrs = 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; > > err: > + if (s->attrs) { > + sftp_attributes_free(s->attrs); > + } > + s->attrs = NULL; > if (s->sftp_handle) { > - libssh2_sftp_close(s->sftp_handle); > + sftp_close(s->sftp_handle); > } > s->sftp_handle = NULL; > if (s->sftp) { > - libssh2_sftp_shutdown(s->sftp); > + sftp_free(s->sftp); > } > s->sftp = 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 = NULL; > > @@ -689,9 +652,11 @@ static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags, > > ssh_state_init(s); > > - ssh_flags = LIBSSH2_FXF_READ; > + ssh_flags = 0; > if (bdrv_flags & BDRV_O_RDWR) { > - ssh_flags |= LIBSSH2_FXF_WRITE; > + ssh_flags |= O_RDWR; > + } else { > + ssh_flags |= O_RDONLY; > } > > /* Start up SSH. */ > @@ -701,15 +666,11 @@ static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags, > } > > /* Go non-blocking. */ > - libssh2_session_set_blocking(s->session, 0); > + ssh_set_blocking(s->session, 0); > > return 0; > > err: > - if (s->sock >= 0) { > - close(s->sock); > - } > - s->sock = -1; > > return ret; > } > @@ -751,8 +712,7 @@ static int ssh_create(const char *filename, QemuOpts *opts, Error **errp) > } > > r = 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 = r; > @@ -760,14 +720,14 @@ static int ssh_create(const char *filename, QemuOpts *opts, Error **errp) > } > > if (total_size > 0) { > - libssh2_sftp_seek64(s.sftp_handle, total_size-1); > - r2 = libssh2_sftp_write(s.sftp_handle, c, 1); > + sftp_seek64(s.sftp_handle, total_size - 1); > + r2 = sftp_write(s.sftp_handle, c, 1); > if (r2 < 0) { > sftp_error_setg(errp, &s, "truncate failed"); > ret = -EINVAL; > goto out; > } > - s.attrs.filesize = total_size; > + s.attrs->size = total_size; > } > > ret = 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 = 0; > > - if (s->attrs.flags & LIBSSH2_SFTP_ATTR_PERMISSIONS) { > - if (s->attrs.permissions & LIBSSH2_SFTP_S_IFREG) { > - has_zero_init = 1; > - } > + if (s->attrs->type == SSH_FILEXFER_TYPE_REGULAR) { > + has_zero_init = 1; > } > > return has_zero_init; > } > > -static void restart_coroutine(void *opaque) > -{ > - Coroutine *co = opaque; > - > - DPRINTF("co=%p", co); > - > - qemu_coroutine_enter(co); > -} > - > -static coroutine_fn void set_fd_handler(BDRVSSHState *s, BlockDriverState *bs) > -{ > - int r; > - IOHandler *rd_handler = NULL, *wr_handler = NULL; > - Coroutine *co = qemu_coroutine_self(); > - > - r = libssh2_session_block_directions(s->session); > - > - if (r & LIBSSH2_SESSION_BLOCK_INBOUND) { > - rd_handler = restart_coroutine; > - } > - if (r & LIBSSH2_SESSION_BLOCK_OUTBOUND) { > - wr_handler = restart_coroutine; > - } > - > - DPRINTF("s->sock=%d rd_handler=%p wr_handler=%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=%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 *bs) > { > - 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 function. > - */ > -#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 = (flags & SSH_SEEK_READ) != 0; > - bool force = (flags & SSH_SEEK_FORCE) != 0; > - > - if (force || op_read != s->offset_op_read || offset != s->offset) { > - DPRINTF("seeking to offset=%" PRIi64, offset); > - libssh2_sftp_seek64(s->sftp_handle, offset); > - s->offset = offset; > - s->offset_op_read = op_read; > - } > } > > static coroutine_fn int ssh_read(BDRVSSHState *s, BlockDriverState *bs, > @@ -890,7 +780,7 @@ static coroutine_fn int ssh_read(BDRVSSHState *s, BlockDriverState *bs, > > DPRINTF("offset=%" PRIi64 " size=%zu", offset, size); > > - ssh_seek(s, offset, SSH_SEEK_READ); > + sftp_seek64(s->sftp_handle, offset); > > /* 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 = i->iov_base; > end_of_vec = i->iov_base + i->iov_len; > > - /* 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 = 0; got < size; ) { > again: > - DPRINTF("sftp_read buf=%p size=%zu", buf, end_of_vec - buf); > - r = libssh2_sftp_read(s->sftp_handle, buf, end_of_vec - buf); > + DPRINTF("sftp_read buf=%p size=%zu (actual size=%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 = sftp_read(s->sftp_handle, buf, MIN(end_of_vec - buf, 16384)); > DPRINTF("sftp_read returned %zd", r); > > - if (r == LIBSSH2_ERROR_EAGAIN || r == LIBSSH2_ERROR_TIMEOUT) { > + if (r == SSH_AGAIN) { > co_yield(s, bs); > goto again; > } > - if (r < 0) { > - sftp_error_report(s, "read failed"); > - s->offset = -1; > - return -EIO; > - } > - if (r == 0) { > + if (r == SSH_EOF) { > /* EOF: Short read so pad the buffer with zeroes and return it. */ > qemu_iovec_memset(qiov, got, 0, size - got); > return 0; > } > + if (r < 0) { > + sftp_error_report(s, "read failed"); > + return -EIO; > + } > > got += r; > buf += r; > - s->offset += r; > if (buf >= end_of_vec && got < size) { > i++; > buf = i->iov_base; > @@ -965,7 +854,7 @@ static int ssh_write(BDRVSSHState *s, BlockDriverState *bs, > > DPRINTF("offset=%" PRIi64 " size=%zu", offset, size); > > - ssh_seek(s, offset, SSH_SEEK_WRITE); > + sftp_seek64(s->sftp_handle, offset); > > /* 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, BlockDriverState *bs, > for (written = 0; written < size; ) { > again: > DPRINTF("sftp_write buf=%p size=%zu", buf, end_of_vec - buf); > - r = libssh2_sftp_write(s->sftp_handle, buf, end_of_vec - buf); > + r = sftp_write(s->sftp_handle, buf, end_of_vec - buf); > DPRINTF("sftp_write returned %zd", r); > > - if (r == LIBSSH2_ERROR_EAGAIN || r == LIBSSH2_ERROR_TIMEOUT) { > + if (r == SSH_AGAIN) { > co_yield(s, bs); > goto again; > } > if (r < 0) { > sftp_error_report(s, "write failed"); > - s->offset = -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 == 0) { > - ssh_seek(s, offset + written, SSH_SEEK_WRITE|SSH_SEEK_FORCE); > - co_yield(s, bs); > - goto again; > - } > > written += r; > buf += r; > - s->offset += r; > if (buf >= end_of_vec && written < size) { > i++; > buf = i->iov_base; > end_of_vec = i->iov_base + i->iov_len; > } > > - if (offset + written > s->attrs.filesize) > - s->attrs.filesize = offset + written; > + if (offset + written > s->attrs->size) { > + s->attrs->size = offset + written; > + } > } > > return 0; > @@ -1039,33 +913,40 @@ static coroutine_fn int ssh_co_writev(BlockDriverState *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 = true; > } > } > > -#ifdef HAS_LIBSSH2_SFTP_FSYNC > +#ifdef HAS_LIBSSH_SFTP_FSYNC > > static coroutine_fn int ssh_flush(BDRVSSHState *s, BlockDriverState *bs) > { > int r; > > DPRINTF("fsync"); > + > + if (!sftp_extension_supported(s->sftp, "fsync@openssh.com", "1")) { > + unsafe_flush_warning(s, "OpenSSH >= 6.3"); > + return 0; > + } > again: > - r = libssh2_sftp_fsync(s->sftp_handle); > - if (r == LIBSSH2_ERROR_EAGAIN || r == LIBSSH2_ERROR_TIMEOUT) { > + r = sftp_fsync(s->sftp_handle); > + if (r == SSH_AGAIN) { > co_yield(s, bs); > goto again; > } > - if (r == LIBSSH2_ERROR_SFTP_PROTOCOL && > - libssh2_sftp_last_error(s->sftp) == LIBSSH2_FX_OP_UNSUPPORTED) { > - unsafe_flush_warning(s, "OpenSSH >= 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(BlockDriverState *bs) > return ret; > } > > -#else /* !HAS_LIBSSH2_SFTP_FSYNC */ > +#else /* !HAS_LIBSSH_SFTP_FSYNC */ > > static coroutine_fn int ssh_co_flush(BlockDriverState *bs) > { > BDRVSSHState *s = bs->opaque; > > - unsafe_flush_warning(s, "libssh2 >= 1.4.4"); > + unsafe_flush_warning(s, "libssh >= 0.8.0"); > return 0; > } > > -#endif /* !HAS_LIBSSH2_SFTP_FSYNC */ > +#endif /* !HAS_LIBSSH_SFTP_FSYNC */ > > static int64_t ssh_getlength(BlockDriverState *bs) > { > BDRVSSHState *s = bs->opaque; > int64_t length; > > - /* Note we cannot make a libssh2 call here. */ > - length = (int64_t) s->attrs.filesize; > + /* Note we cannot make a libssh call here. */ > + length = (int64_t) s->attrs->size; > DPRINTF("length=%" PRIi64, length); > > return length; > @@ -1130,12 +1011,16 @@ static void bdrv_ssh_init(void) > { > int r; > > - r = libssh2_init(0); > + r = ssh_init(); > if (r != 0) { > - fprintf(stderr, "libssh2 initialization failed, %d\n", r); > + fprintf(stderr, "libssh initialization failed, %d\n", r); > exit(EXIT_FAILURE); > } > > +#if TRACE_LIBSSH != 0 > + ssh_set_log_level(TRACE_LIBSSH); > +#endif > + > bdrv_register(&bdrv_ssh); > } > > diff --git a/configure b/configure > index dd9e679..ff48c29 100755 > --- a/configure > +++ b/configure > @@ -316,7 +316,7 @@ gcrypt_kdf="no" > vte="" > virglrenderer="" > tpm="yes" > -libssh2="" > +libssh="" > numa="" > tcmalloc="no" > jemalloc="no" > @@ -1142,9 +1142,9 @@ for opt do > ;; > --enable-tpm) tpm="yes" > ;; > - --disable-libssh2) libssh2="no" > + --disable-libssh) libssh="no" > ;; > - --enable-libssh2) libssh2="yes" > + --enable-libssh) libssh="yes" > ;; > --disable-numa) numa="no" > ;; > @@ -1386,7 +1386,7 @@ disabled with --disable-FEATURE, default is enabled 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 > > ########################################## > -# libssh2 probe > -min_libssh2_version=1.2.8 > -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 > + libssh=no > fi > fi > > ########################################## > -# libssh2_sftp_fsync probe > +# libssh sftp_fsync probe > > -if test "$libssh2" = "yes"; then > +if test "$libssh" = "yes"; then > cat > $TMPC <<EOF > #include <stdio.h> > -#include <libssh2.h> > -#include <libssh2_sftp.h> > +#include <libssh/libssh.h> > +#include <libssh/sftp.h> > int main(void) { > - LIBSSH2_SESSION *session; > - LIBSSH2_SFTP *sftp; > - LIBSSH2_SFTP_HANDLE *sftp_handle; > - session = libssh2_session_init (); > - sftp = libssh2_sftp_init (session); > - sftp_handle = libssh2_sftp_open (sftp, "/", 0, 0); > - libssh2_sftp_fsync (sftp_handle); > + ssh_session session; > + sftp_session sftp; > + sftp_file sftp_handle; > + session = ssh_new(); > + sftp = sftp_new(session); > + sftp_handle = 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="-DHAS_LIBSSH2_SFTP_FSYNC $QEMU_CFLAGS" > + # libssh_cflags/libssh_libs defined in previous test. > + if compile_prog "$libssh_cflags" "$libssh_libs" ; then > + QEMU_CFLAGS="-DHAS_LIBSSH_SFTP_FSYNC $QEMU_CFLAGS" > fi > fi > > @@ -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" = "yes" ; then > echo "ARCHIPELAGO_LIBS=$archipelago_libs" >> $config_host_mak > fi > > -if test "$libssh2" = "yes" ; then > - echo "CONFIG_LIBSSH2=m" >> $config_host_mak > - echo "LIBSSH2_CFLAGS=$libssh2_cflags" >> $config_host_mak > - echo "LIBSSH2_LIBS=$libssh2_libs" >> $config_host_mak > +if test "$libssh" = "yes" ; then > + echo "CONFIG_LIBSSH=m" >> $config_host_mak > + echo "LIBSSH_CFLAGS=$libssh_cflags" >> $config_host_mak > + echo "LIBSSH_LIBS=$libssh_libs" >> $config_host_mak > fi > > # USB host support > -- > 2.7.4 -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] ssh: switch from libssh2 to libssh 2016-10-20 15:32 ` Richard W.M. Jones @ 2016-10-20 15:44 ` Pino Toscano 2016-10-20 16:11 ` Richard W.M. Jones 0 siblings, 1 reply; 6+ messages in thread From: Pino Toscano @ 2016-10-20 15:44 UTC (permalink / raw) To: Richard W.M. Jones; +Cc: qemu-devel, qemu-block, jcody, kwolf, mreitz [-- Attachment #1: Type: text/plain, Size: 2427 bytes --] On Thursday, 20 October 2016 16:32:50 CEST Richard W.M. Jones wrote: > On Thu, Oct 20, 2016 at 05:15:24PM +0200, Pino Toscano wrote: > > Rewrite the implementation of the ssh block driver to use libssh instead > > 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 > > > > Kerberos authentication can be enabled once the libssh bug for it [1] is > > fixed. > > > > The development version of libssh (i.e. the future 0.8.x) supports > > fsync, so reuse the build time check for this. > > > > [1] https://red.libssh.org/issues/242 > > > > Signed-off-by: Pino Toscano <ptoscano@redhat.com> > > > When I applied this patch and compiled it with warnings enabled: > > block/ssh.c: In function ‘connect_to_ssh’: > block/ssh.c:643:12: warning: ‘ret’ may be used uninitialized in this function [-Wmaybe-uninitialized] > return ret; > ^~~ Interesting, there was no warning for me. Anyway, I think this: diff --git a/block/ssh.c b/block/ssh.c index 7c316db..7ff376e 100644 --- a/block/ssh.c +++ b/block/ssh.c @@ -519,6 +519,7 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options, /* Create SSH session. */ s->session = ssh_new(); if (!s->session) { + ret = -EINVAL; goto err; } should fix it (added already locally). > 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=kvm -cpu host -m 2048 \ > -drive file.driver=ssh,file.user=[user],file.host=[host],file.path=/var/tmp/fedora-24.img,format=raw,if=virtio \ > -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 :-( Can you please enable the logging of the ssh driver, and libssh own logging too? Basically (see lines 45-46) set: #define DEBUG_SSH 1 #define TRACE_LIBSSH 4 > Also fsync was not supported for me, but I'm using 0.7.3 and the code > says I need 0.8.0. Yes, this is correct. Thanks, -- Pino Toscano [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] ssh: switch from libssh2 to libssh 2016-10-20 15:44 ` Pino Toscano @ 2016-10-20 16:11 ` Richard W.M. Jones 0 siblings, 0 replies; 6+ messages in thread From: Richard W.M. Jones @ 2016-10-20 16:11 UTC (permalink / raw) To: Pino Toscano; +Cc: qemu-devel, qemu-block, jcody, kwolf, mreitz On Thu, Oct 20, 2016 at 05:44:41PM +0200, Pino Toscano wrote: > On Thursday, 20 October 2016 16:32:50 CEST Richard W.M. Jones wrote: > > On Thu, Oct 20, 2016 at 05:15:24PM +0200, Pino Toscano wrote: > > > Rewrite the implementation of the ssh block driver to use libssh instead > > > 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 > > > > > > Kerberos authentication can be enabled once the libssh bug for it [1] is > > > fixed. > > > > > > The development version of libssh (i.e. the future 0.8.x) supports > > > fsync, so reuse the build time check for this. > > > > > > [1] https://red.libssh.org/issues/242 > > > > > > Signed-off-by: Pino Toscano <ptoscano@redhat.com> > > > > > > When I applied this patch and compiled it with warnings enabled: > > > > block/ssh.c: In function ‘connect_to_ssh’: > > block/ssh.c:643:12: warning: ‘ret’ may be used uninitialized in this function [-Wmaybe-uninitialized] > > return ret; > > ^~~ > > Interesting, there was no warning for me. Anyway, I think this: > > diff --git a/block/ssh.c b/block/ssh.c > index 7c316db..7ff376e 100644 > --- a/block/ssh.c > +++ b/block/ssh.c > @@ -519,6 +519,7 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options, > /* Create SSH session. */ > s->session = ssh_new(); > if (!s->session) { > + ret = -EINVAL; > goto err; > } > > should fix it (added already locally). Yes, this fixes the warning. > > 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=kvm -cpu host -m 2048 \ > > -drive file.driver=ssh,file.user=[user],file.host=[host],file.path=/var/tmp/fedora-24.img,format=raw,if=virtio \ > > -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 :-( > > Can you please enable the logging of the ssh driver, and libssh own > logging too? Basically (see lines 45-46) set: > > #define DEBUG_SSH 1 > #define TRACE_LIBSSH 4 I'll send you the full trace privately since it's enormous. Rich. > > Also fsync was not supported for me, but I'm using 0.7.3 and the code > > says I need 0.8.0. > > Yes, this is correct. > > Thanks, > -- > Pino Toscano -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] ssh: switch from libssh2 to libssh 2016-10-20 15:15 [Qemu-devel] [PATCH] ssh: switch from libssh2 to libssh Pino Toscano 2016-10-20 15:32 ` Richard W.M. Jones @ 2016-10-20 16:04 ` Stefan Weil 2016-10-20 16:52 ` Pino Toscano 1 sibling, 1 reply; 6+ messages in thread From: Stefan Weil @ 2016-10-20 16:04 UTC (permalink / raw) To: Pino Toscano, qemu-devel, qemu-block; +Cc: kwolf, jcody, rjones, mreitz Am 20.10.2016 um 17:15 schrieb Pino Toscano: > Rewrite the implementation of the ssh block driver to use libssh instead > of libssh. The libssh library has various advantages over libssh: libssh instead of libssh? In both sentences a "2" is missing. Cygwin does not provide libssh for Mingw-w64, see https://cygwin.com/cgi-bin2/package-grep.cgi?grep=libssh&arch=x86_64 Stefan ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] ssh: switch from libssh2 to libssh 2016-10-20 16:04 ` Stefan Weil @ 2016-10-20 16:52 ` Pino Toscano 0 siblings, 0 replies; 6+ messages in thread From: Pino Toscano @ 2016-10-20 16:52 UTC (permalink / raw) To: Stefan Weil; +Cc: qemu-devel, qemu-block, kwolf, jcody, rjones, mreitz [-- Attachment #1: Type: text/plain, Size: 669 bytes --] On Thursday, 20 October 2016 18:04:34 CEST Stefan Weil wrote: > Am 20.10.2016 um 17:15 schrieb Pino Toscano: > > Rewrite the implementation of the ssh block driver to use libssh instead > > of libssh. The libssh library has various advantages over libssh: > > libssh instead of libssh? In both sentences a "2" is missing. Right, they should be "... instead of libssh2" and "advantages over libssh2" -- fixed locally. > Cygwin does not provide libssh for Mingw-w64, see > https://cygwin.com/cgi-bin2/package-grep.cgi?grep=libssh&arch=x86_64 I guess I could ask them for these versions once this patch is approved (so there's an existing use case). -- Pino Toscano [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-10-20 16:52 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-10-20 15:15 [Qemu-devel] [PATCH] ssh: switch from libssh2 to libssh Pino Toscano 2016-10-20 15:32 ` Richard W.M. Jones 2016-10-20 15:44 ` Pino Toscano 2016-10-20 16:11 ` Richard W.M. Jones 2016-10-20 16:04 ` Stefan Weil 2016-10-20 16:52 ` Pino Toscano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).