From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39847) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bxFWa-0000i1-VR for qemu-devel@nongnu.org; Thu, 20 Oct 2016 11:44:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bxFWa-0003YM-2B for qemu-devel@nongnu.org; Thu, 20 Oct 2016 11:44:57 -0400 From: Pino Toscano Date: Thu, 20 Oct 2016 17:44:41 +0200 Message-ID: <2129980.is88ABC7Ux@thyrus.usersys.redhat.com> In-Reply-To: <20161020153250.GE11243@redhat.com> References: <1476976524-6599-1-git-send-email-ptoscano@redhat.com> <20161020153250.GE11243@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="nextPart25916731.v37qQXqv36"; micalg="pgp-sha256"; protocol="application/pgp-signature" Subject: Re: [Qemu-devel] [PATCH] ssh: switch from libssh2 to libssh List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Richard W.M. Jones" Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, jcody@redhat.com, kwolf@redhat.com, mreitz@redhat.com --nextPart25916731.v37qQXqv36 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="UTF-8" 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 > >=20 > > Kerberos authentication can be enabled once the libssh bug for it [1] is > > 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 >=20 >=20 > When I applied this patch and compiled it with warnings enabled: >=20 > 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; > ^~~ 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 =2D-- a/block/ssh.c +++ b/block/ssh.c @@ -519,6 +519,7 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *optio= ns, /* Create SSH session. */ s->session =3D ssh_new(); if (!s->session) { + ret =3D -EINVAL; goto err; } =20 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: >=20 > ./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= =2Epath=3D/var/tmp/fedora-24.img,format=3Draw,if=3Dvirtio \ > -serial stdio >=20 > Unfortunately this failed with a large number of sftp errors: >=20 > read failed: (sftp error code: 0) >=20 > 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, =2D-=20 Pino Toscano --nextPart25916731.v37qQXqv36 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part. Content-Transfer-Encoding: 7Bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJYCOZpAAoJEMPRTC2YZDfNTrUQAIJYQ20W2I+iHoTyCVf9yNdd SU+wEKLITiGfaluXLRwIgnveyNA04SZ8wkwlZsJn8FIg8pKEoTSpdLGp00nWki54 ojQY96V1wXvO3DYsUGBsGWKGPJztqGNQpgITBEnm7xO2o8gYLFD8+aefhY6GDmgx xoUil2XbVVqM5BeHNVdlryacV0vNmTn3JRjm4xvLmudR9ignb46H443tWbbvOjv5 uLAVRn8TZfgaGSiF7aUayQbWXdFBNkw0JkgXh8P5E36D9bcbB4q+dz3A77OrLung YUhGGH81lfLWUt87QhLKCNlOr6Jz8MI1PplvX2+D7joh162LWzQdui1LtV9NfmyR blUUFCx1yilRvlSId0A5ZCIRoGy1QdTLzLhNeQGFDDTVEsJ8Uk+mkx5eTsWQiX8W IY2Hx+w0qaobVsrkO0lDW1C4pExIwpQkRS/0GD1YiJA967TOcPbbVu3vR8Xj0p+n bVfSdpe73Rns9zTQanXjqk/vQNVBKmhcjoD9WJhjoUXN3XC83o5Wt5pfkcie+jqa sNHDQmciTQCTIRixpluwY33Z+WbHG8bld7tdzlFPzjes3HIUfvZhIhmIjp2QJdUw 78G1Fl4bCmlbDXLmhCjDyAB3Z2KWE0bLMhtTZmty9CVsz/07xdUr0naq8jMmwx0+ ugHTOapKqEt73naO723T =TKfv -----END PGP SIGNATURE----- --nextPart25916731.v37qQXqv36--