From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:59127) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ULAMs-0007KU-Pr for qemu-devel@nongnu.org; Thu, 28 Mar 2013 06:47:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ULAMr-0004Wx-2h for qemu-devel@nongnu.org; Thu, 28 Mar 2013 06:47:38 -0400 Received: from mail-wg0-f43.google.com ([74.125.82.43]:56973) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ULAMq-0004Wo-PW for qemu-devel@nongnu.org; Thu, 28 Mar 2013 06:47:37 -0400 Received: by mail-wg0-f43.google.com with SMTP id f12so1405382wgh.22 for ; Thu, 28 Mar 2013 03:47:35 -0700 (PDT) Date: Thu, 28 Mar 2013 11:47:32 +0100 From: Stefan Hajnoczi Message-ID: <20130328104732.GA15114@stefanha-thinkpad.redhat.com> References: <1364399849-5518-1-git-send-email-rjones@redhat.com> <1364399849-5518-2-git-send-email-rjones@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1364399849-5518-2-git-send-email-rjones@redhat.com> Subject: Re: [Qemu-devel] [PATCH v4] block: Add support for Secure Shell (ssh) block device. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Richard W.M. Jones" Cc: qemu-devel@nongnu.org On Wed, Mar 27, 2013 at 03:57:29PM +0000, Richard W.M. Jones wrote: > From: "Richard W.M. Jones" > > qemu-system-x86_64 -drive file=ssh://hostname/some/image > > QEMU will ssh into 'hostname' and open '/some/image' which is made > available as a standard block device. > > You can specify a username (ssh://user@host/...) and/or a port number > (ssh://host:port/...). You can also use an alternate syntax using > properties (file.user, file.host, file.port, file.path). > > Current limitations: > > - Authentication must be done without passwords or passphrases, using > ssh-agent. Other authentication methods are not supported. > > - Uses a single connection, instead of concurrent AIO with multiple > SSH connections. > > This is implemented using libssh2 on the client side. The server just > requires a regular ssh daemon with sftp-server support. Most ssh > daemons on Unix/Linux systems will work out of the box. > > Signed-off-by: Richard W.M. Jones > Cc: Stefan Hajnoczi > Cc: Kevin Wolf > --- > block/Makefile.objs | 1 + > block/ssh.c | 878 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > configure | 47 +++ > qemu-doc.texi | 40 +++ > qemu-options.hx | 12 + > 5 files changed, 978 insertions(+) > create mode 100644 block/ssh.c Please run qemu-iotests, see tests/qemu-iotests/check. For example, with NBD: $ cd tests/qemu-iotests $ QEMU_PROG=$HOME/qemu/x86_64-softmmu/qemu-system-x86_64 PATH=$HOME/qemu:$PATH \ ./check -nbd A patch will be required to add -ssh support to ./check. > +/* Wrappers around error_report which make sure to dump as much > + * information from libssh2 as possible. > + */ > +static void > +session_error_report(BDRVSSHState *s, const char *fs, ...) > +{ > + va_list args; > + > + va_start(args, fs); > + > + if ((s)->session) { > + char *ssh_err; > + int ssh_err_len = sizeof ssh_err; This variable is unused and initialization to sizeof ssh_err seems wrong. Please drop it and pass NULL to libssh2_session_last_error(3). > + int ssh_err_code; > + > + libssh2_session_last_error((s)->session, > + &ssh_err, &ssh_err_len, 0); > + /* This is not an errno. See . */ > + ssh_err_code = libssh2_session_last_errno((s)->session); > + > + error_vprintf(fs, args); > + error_printf(": %s (libssh2 error code: %d)", ssh_err, ssh_err_code); > + } > + else { QEMU tends to use } else { on a single line. > + error_vprintf(fs, args); > + } In fact this else statement can be avoided: error_vprintf(fs, args); if (s->session) { ... error_printf(": %s (libssh2 error code: %d)", ssh_err, ssh_err_code); } > + > + va_end(args); > + error_printf("\n"); > +} > + > +static void > +sftp_error_report(BDRVSSHState *s, const char *fs, ...) > +{ > + va_list args; > + > + va_start(args, fs); > + > + if ((s)->sftp) { > + char *ssh_err; > + int ssh_err_len = sizeof ssh_err; > + int ssh_err_code; > + unsigned long sftp_err_code; > + > + libssh2_session_last_error((s)->session, > + &ssh_err, &ssh_err_len, 0); > + /* This is not an errno. See . */ > + ssh_err_code = libssh2_session_last_errno((s)->session); > + /* See . */ > + sftp_err_code = libssh2_sftp_last_error((s)->sftp); > + > + error_vprintf(fs, args); > + error_printf(": %s (libssh2 error code: %d, sftp error code: %lu)", > + ssh_err, ssh_err_code, sftp_err_code); > + } > + else { > + error_vprintf(fs, args); > + } Same comments apply to sftp_error_report(). > + > + va_end(args); > + error_printf("\n"); > +} > + > +#ifndef _WIN32 > +static const char *get_username(void) > +{ > + struct passwd *pwd; > + > + pwd = getpwuid(geteuid ()); > + if (!pwd) { > + error_report("failed to get current user name"); > + return NULL; > + } > + return pwd->pw_name; > +} > +#else > +static const char *get_username(void) > +{ > + error_report("on Windows, specify a remote user name in the URI"); > + return NULL; > +} > +#endif This is ugly since it's platform-specific and uses getpwuid(3) which is not reentrant. Does ssh(1) even use getpwuid(geteuid()) or does it check .ssh/config and then getenv("USER")? Perhaps we can just getenv("USER")? > + > +static int parse_uri(const char *filename, QDict *options) Use Error **errp argument instead of error_report()? > +{ > + URI *uri; > + char *host = NULL; Unused variable. > +static int check_host_key(BDRVSSHState *s, const char *host, int port) > +{ > + int ret; > + const char *home; > + char *knh_file = NULL; > + LIBSSH2_KNOWNHOSTS *knh = NULL; > + const char *fingerprint; > + size_t len; > + int type; > + > + knh = libssh2_knownhost_init(s->session); > + if (!knh) { > + ret = -EINVAL; > + session_error_report(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"); > + } Windows support? > + > + /* Read all known hosts from OpenSSH-style known_hosts file. */ > + libssh2_knownhost_readfile(knh, knh_file, LIBSSH2_KNOWNHOST_FILE_OPENSSH); > + > + fingerprint = libssh2_session_hostkey(s->session, &len, &type); > + if (fingerprint) { > + struct libssh2_knownhost *found; > + int r; > + > + r = libssh2_knownhost_checkp(knh, host, port, fingerprint, len, > + LIBSSH2_KNOWNHOST_TYPE_PLAIN| > + LIBSSH2_KNOWNHOST_KEYENC_RAW, > + &found); > + switch (r) { > + case LIBSSH2_KNOWNHOST_CHECK_MATCH: > + /* OK */ > + DPRINTF("host key OK: %s", found->key); > + break; > + case LIBSSH2_KNOWNHOST_CHECK_MISMATCH: > + ret = -EINVAL; > + session_error_report(s, "host key does not match the one in known_hosts (found key %s)", > + found->key); Does the user know the offending known_hosts line? ssh(1) normally says something like "Mismatch with line ~/.ssh/known_hosts:35" so you know which hostkey to drop if you wish to proceed. > + goto out; > + case LIBSSH2_KNOWNHOST_CHECK_NOTFOUND: > + ret = -EINVAL; > + session_error_report(s, "no host key was found in known_hosts"); > + goto out; ssh(1) prompts the user to accept the hostkey. When QEMU fails like this the user needs to run ssh(1) first to populate known_hosts? > @@ -1166,6 +1171,7 @@ echo " --disable-glusterfs disable GlusterFS backend" > echo " --enable-gcov enable test coverage analysis with gcov" > echo " --gcov=GCOV use specified gcov [$gcov_tool]" > echo " --enable-tpm enable TPM support" > +echo " --enable-libssh2 enable ssh block device support" Most options seem to also document --disable-FOO. Please do that.