Re: [libvirt] [PATCH v2 2/2] libssh_transport: add new libssh-based transport

2016-10-19 Thread Pino Toscano
On Tuesday, 18 October 2016 15:49:56 CEST Daniel P. Berrange wrote:
> On Tue, Oct 18, 2016 at 04:46:53PM +0200, Pino Toscano wrote:
> > On Tuesday, 18 October 2016 14:19:41 CEST Daniel P. Berrange wrote:
> > > On Mon, Oct 17, 2016 at 04:24:53PM +0200, Pino Toscano wrote:
> > > > Implement a new libssh transport, which uses libssh to communicate with
> > > > remote hosts, and use it in virNetSockets.
> > > > 
> > > > This new transport supports all the common ssh authentication methods,
> > > > making use of libvirt's auth callbacks for interaction with the user.
> > > > 
> > > > Most of the functionalities and implementation are based on the libssh2
> > > > transport.
> > > > ---
> > > >  config-post.h |2 +
> > > >  configure.ac  |3 +
> > > >  include/libvirt/virterror.h   |2 +
> > > >  m4/virt-libssh.m4 |   26 +
> > > >  src/Makefile.am   |   21 +-
> > > >  src/libvirt_libssh.syms   |   22 +
> > > >  src/remote/remote_driver.c|   41 ++
> > > >  src/rpc/virnetclient.c|  123 
> > > >  src/rpc/virnetclient.h|   13 +
> > > >  src/rpc/virnetlibsshsession.c | 1424 
> > > > +
> > > >  src/rpc/virnetlibsshsession.h |   80 +++
> > > >  src/rpc/virnetsocket.c|  179 ++
> > > >  src/rpc/virnetsocket.h|   13 +
> > > >  src/util/virerror.c   |9 +-
> > > >  14 files changed, 1955 insertions(+), 3 deletions(-)
> > > >  create mode 100644 m4/virt-libssh.m4
> > > >  create mode 100644 src/libvirt_libssh.syms
> > > >  create mode 100644 src/rpc/virnetlibsshsession.c
> > > >  create mode 100644 src/rpc/virnetlibsshsession.h
> > > 
> > > libvirt.spec.in and mingw-libvirt.spec.in need updating too, as well
> > > as docs/remote.html.in
> > 
> > OK for libvirt.spec.in and docs/remote.html.in, but libssh for mingw is
> > not available in Fedora.
> 
> Is it possible to build libssh for mingw ?  If so we'll need to look at
> getting that into Fedora.

I don't know -- I can request that to its maintainer once the libssh
transport is in (so there is an actual use case for adding mingw
support).

At least, libssh is supposed to build and work on Windows as well.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 2/2] libssh_transport: add new libssh-based transport

2016-10-19 Thread Pino Toscano
On Wednesday, 19 October 2016 08:33:27 CEST Peter Krempa wrote:
> > > > +memset(_passphrase, 0, sizeof(virConnectCredential));
> > > > +retr_passphrase.type = virCredTypeForPrompt(sess->cred, echo);
> > > > +retr_passphrase.prompt = virBufferCurrentContent();
> > > 
> > > This shouldn't really be used. Please get the content into a variable.
> > 
> > Not a problem to change this, but could you please explain a bit more
> > on the reasons? (So I can avoid doing the same again.)
> 
> Basically any call any function that uses the buffer invalidates the
> pointer returned by virBufferCurrentContent (or should be treated as
> such) which is prone to bugs when later modifying the code.
> 
> Most of the uses of the function are in special cases (or should be
> removed).

I see, thanks for the explanation.

> > > > +int
> > > > +virNetLibsshSessionConnect(virNetLibsshSessionPtr sess,
> > > > +   int sock)
> > > > +{
> > > > +int ret;
> > > > +const char *errmsg;
> > > > +
> > > > +VIR_DEBUG("sess=%p, sock=%d", sess, sock);
> > > > +
> > > > +if (!sess || sess->state != VIR_NET_LIBSSH_STATE_NEW) {
> > > > +virReportError(VIR_ERR_LIBSSH, "%s",
> > > > +   _("Invalid virNetLibsshSessionPtr"));
> > > > +return -1;
> > > > +}
> > > > +
> > > > +virObjectLock(sess);
> > > > +
> > > > +/* check if configuration is valid */
> > > > +if ((ret = virNetLibsshValidateConfig(sess)) < 0)
> > > > +goto error;
> > > > +
> > > > +/* read ~/.ssh/config */
> > > > +if ((ret = ssh_options_parse_config(sess->session, NULL)) < 0)
> > > > +goto error;
> > > > +
> > > > +/* set the socket FD for the libssh session */
> > > > +if ((ret = ssh_options_set(sess->session, SSH_OPTIONS_FD, )) 
> > > > < 0)
> > > 
> > > Is this guaranteed to copy the socket number at call time? Otherwise
> > > (similarly to the ones above will not work reliably).
> > 
> > I don't understand this sentence (it seems truncated), can you please
> > rephrase it?
> 
> Sorry I probably got sidetracked and did not finish my thought.
> 
> My question was whether ssh_options_set accesses the pointer to 'sock'
> right away and copies it. You are passing a pointer to a stack allocated
> variable which will get out of scope later, thus the pointer should not
> be accessed after the call to ssh_options_set returns.

libssh indeed copies the values passed to ssh_options_set, so the two
calls you mentioned (for SSH_OPTIONS_FD and SSH_OPTIONS_PORT) are safe.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 2/2] libssh_transport: add new libssh-based transport

2016-10-19 Thread Peter Krempa
On Tue, Oct 18, 2016 at 16:42:26 +0200, Pino Toscano wrote:
> On Tuesday, 18 October 2016 15:15:07 CEST Peter Krempa wrote:
> > On Mon, Oct 17, 2016 at 16:24:53 +0200, Pino Toscano wrote:
> > > Implement a new libssh transport, which uses libssh to communicate with
> > > remote hosts, and use it in virNetSockets.
> > > 
> > > This new transport supports all the common ssh authentication methods,
> > > making use of libvirt's auth callbacks for interaction with the user.
> > > 
> > > Most of the functionalities and implementation are based on the libssh2
> > > transport.
> > > ---
> > >  config-post.h |2 +
> > >  configure.ac  |3 +
> > >  include/libvirt/virterror.h   |2 +
> > >  m4/virt-libssh.m4 |   26 +
> > >  src/Makefile.am   |   21 +-
> > >  src/libvirt_libssh.syms   |   22 +
> > >  src/remote/remote_driver.c|   41 ++
> > >  src/rpc/virnetclient.c|  123 
> > >  src/rpc/virnetclient.h|   13 +
> > >  src/rpc/virnetlibsshsession.c | 1424 
> > > +
> > >  src/rpc/virnetlibsshsession.h |   80 +++
> > >  src/rpc/virnetsocket.c|  179 ++
> > >  src/rpc/virnetsocket.h|   13 +
> > >  src/util/virerror.c   |9 +-
> > >  14 files changed, 1955 insertions(+), 3 deletions(-)
> > >  create mode 100644 m4/virt-libssh.m4
> > >  create mode 100644 src/libvirt_libssh.syms
> > >  create mode 100644 src/rpc/virnetlibsshsession.c
> > >  create mode 100644 src/rpc/virnetlibsshsession.h
> > 
> > Is it possible to split out the src/rpc/virnetlibsshsession.(ch) changes
> > (plus the ones necessary to compile it) from adding all the bits that
> > actually make use of it? This patch is very big.
> 
> Yes, it is -- I was not sure how much should the changes be split,
> especially in cases like this when adding a new "thing" which is used
> only in a single place later on.
> 
> Just to be sure, a reasonable split for this patch would be:
> 1) add libssh bits in virerror
> 2) add virnetlibsshsession.(ch) + autofoo stuff for libssh
> 3) add glue code in virNetSocket + virNetClient
> or were you thinking about something else? (no problems on my side)

No, this is exactly the split I was thinking about.

> > > diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
> > > index 361dc1a..b296aac 100644
> > > --- a/src/rpc/virnetclient.c
> > > +++ b/src/rpc/virnetclient.c
> > > @@ -505,6 +505,129 @@ virNetClientPtr virNetClientNewLibSSH2(const char 
> > > *host,
> > >  }
> > >  #undef DEFAULT_VALUE
> > >  
> > > +#define DEFAULT_VALUE(VAR, VAL) \
> > > +if (!VAR)   \
> > > +VAR = VAL;
> > > +virNetClientPtr virNetClientNewLibssh(const char *host,
> > > +  const char *port,
> > > +  int family,
> > > +  const char *username,
> > > +  const char *privkeyPath,
> > > +  const char *knownHostsPath,
> > > +  const char *knownHostsVerify,
> > > +  const char *authMethods,
> > > +  const char *netcatPath,
> > > +  const char *socketPath,
> > > +  virConnectAuthPtr authPtr,
> > > +  virURIPtr uri)
> > > +{
> > > +virNetSocketPtr sock = NULL;
> > > +virNetClientPtr ret = NULL;
> > > +
> > > +virBuffer buf = VIR_BUFFER_INITIALIZER;
> > > +char *nc = NULL;
> > > +char *command = NULL;
> > > +
> > > +char *homedir = virGetUserDirectory();
> > > +char *confdir = virGetUserConfigDirectory();
> > > +char *knownhosts = NULL;
> > > +char *privkey = NULL;
> > > +
> > > +/* Use default paths for known hosts an public keys if not provided 
> > > */
> > 
> > So is libssh able to handle e.g. ECDSA keys in known hosts? Libssh2 was
> > not and truncated the known hosts file which was not acceptable.
> 
> Yes, it is. For example in my tests I'm passing
>   known_hosts=$HOME/.ssh/known_hosts
> as additional query item to the qemu+libssh URLs, and it works well.

Cool!

> > > +if (confdir) {
> > > +if (!knownHostsPath) {
> > > +if (virFileExists(confdir)) {
> > > +virBufferAsprintf(, "%s/known_hosts", confdir);
> > > +if (!(knownhosts = virBufferContentAndReset()))
> > 
> > Use virAsprintf instead of the two lines above.
> 
> OK.
> 
> Small side note: all the glue code in virNetSocket, virNetClient and
> remote_driver.c was basically a copy from the libssh2
> implementation, so all these fixes should be done there too.

Hmm, I'm wondering whether that code was refactored or was wrong from
the beginning :).

Anyways I'll probably visit it and clean it up.

> > > + 

Re: [libvirt] [PATCH v2 2/2] libssh_transport: add new libssh-based transport

2016-10-18 Thread Daniel P. Berrange
On Tue, Oct 18, 2016 at 04:46:53PM +0200, Pino Toscano wrote:
> On Tuesday, 18 October 2016 14:19:41 CEST Daniel P. Berrange wrote:
> > On Mon, Oct 17, 2016 at 04:24:53PM +0200, Pino Toscano wrote:
> > > Implement a new libssh transport, which uses libssh to communicate with
> > > remote hosts, and use it in virNetSockets.
> > > 
> > > This new transport supports all the common ssh authentication methods,
> > > making use of libvirt's auth callbacks for interaction with the user.
> > > 
> > > Most of the functionalities and implementation are based on the libssh2
> > > transport.
> > > ---
> > >  config-post.h |2 +
> > >  configure.ac  |3 +
> > >  include/libvirt/virterror.h   |2 +
> > >  m4/virt-libssh.m4 |   26 +
> > >  src/Makefile.am   |   21 +-
> > >  src/libvirt_libssh.syms   |   22 +
> > >  src/remote/remote_driver.c|   41 ++
> > >  src/rpc/virnetclient.c|  123 
> > >  src/rpc/virnetclient.h|   13 +
> > >  src/rpc/virnetlibsshsession.c | 1424 
> > > +
> > >  src/rpc/virnetlibsshsession.h |   80 +++
> > >  src/rpc/virnetsocket.c|  179 ++
> > >  src/rpc/virnetsocket.h|   13 +
> > >  src/util/virerror.c   |9 +-
> > >  14 files changed, 1955 insertions(+), 3 deletions(-)
> > >  create mode 100644 m4/virt-libssh.m4
> > >  create mode 100644 src/libvirt_libssh.syms
> > >  create mode 100644 src/rpc/virnetlibsshsession.c
> > >  create mode 100644 src/rpc/virnetlibsshsession.h
> > 
> > libvirt.spec.in and mingw-libvirt.spec.in need updating too, as well
> > as docs/remote.html.in
> 
> OK for libvirt.spec.in and docs/remote.html.in, but libssh for mingw is
> not available in Fedora.

Is it possible to build libssh for mingw ?  If so we'll need to look at
getting that into Fedora.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v2 2/2] libssh_transport: add new libssh-based transport

2016-10-18 Thread Pino Toscano
On Tuesday, 18 October 2016 14:19:41 CEST Daniel P. Berrange wrote:
> On Mon, Oct 17, 2016 at 04:24:53PM +0200, Pino Toscano wrote:
> > Implement a new libssh transport, which uses libssh to communicate with
> > remote hosts, and use it in virNetSockets.
> > 
> > This new transport supports all the common ssh authentication methods,
> > making use of libvirt's auth callbacks for interaction with the user.
> > 
> > Most of the functionalities and implementation are based on the libssh2
> > transport.
> > ---
> >  config-post.h |2 +
> >  configure.ac  |3 +
> >  include/libvirt/virterror.h   |2 +
> >  m4/virt-libssh.m4 |   26 +
> >  src/Makefile.am   |   21 +-
> >  src/libvirt_libssh.syms   |   22 +
> >  src/remote/remote_driver.c|   41 ++
> >  src/rpc/virnetclient.c|  123 
> >  src/rpc/virnetclient.h|   13 +
> >  src/rpc/virnetlibsshsession.c | 1424 
> > +
> >  src/rpc/virnetlibsshsession.h |   80 +++
> >  src/rpc/virnetsocket.c|  179 ++
> >  src/rpc/virnetsocket.h|   13 +
> >  src/util/virerror.c   |9 +-
> >  14 files changed, 1955 insertions(+), 3 deletions(-)
> >  create mode 100644 m4/virt-libssh.m4
> >  create mode 100644 src/libvirt_libssh.syms
> >  create mode 100644 src/rpc/virnetlibsshsession.c
> >  create mode 100644 src/rpc/virnetlibsshsession.h
> 
> libvirt.spec.in and mingw-libvirt.spec.in need updating too, as well
> as docs/remote.html.in

OK for libvirt.spec.in and docs/remote.html.in, but libssh for mingw is
not available in Fedora.

Thanks,
-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 2/2] libssh_transport: add new libssh-based transport

2016-10-18 Thread Pino Toscano
On Tuesday, 18 October 2016 15:15:07 CEST Peter Krempa wrote:
> On Mon, Oct 17, 2016 at 16:24:53 +0200, Pino Toscano wrote:
> > Implement a new libssh transport, which uses libssh to communicate with
> > remote hosts, and use it in virNetSockets.
> > 
> > This new transport supports all the common ssh authentication methods,
> > making use of libvirt's auth callbacks for interaction with the user.
> > 
> > Most of the functionalities and implementation are based on the libssh2
> > transport.
> > ---
> >  config-post.h |2 +
> >  configure.ac  |3 +
> >  include/libvirt/virterror.h   |2 +
> >  m4/virt-libssh.m4 |   26 +
> >  src/Makefile.am   |   21 +-
> >  src/libvirt_libssh.syms   |   22 +
> >  src/remote/remote_driver.c|   41 ++
> >  src/rpc/virnetclient.c|  123 
> >  src/rpc/virnetclient.h|   13 +
> >  src/rpc/virnetlibsshsession.c | 1424 
> > +
> >  src/rpc/virnetlibsshsession.h |   80 +++
> >  src/rpc/virnetsocket.c|  179 ++
> >  src/rpc/virnetsocket.h|   13 +
> >  src/util/virerror.c   |9 +-
> >  14 files changed, 1955 insertions(+), 3 deletions(-)
> >  create mode 100644 m4/virt-libssh.m4
> >  create mode 100644 src/libvirt_libssh.syms
> >  create mode 100644 src/rpc/virnetlibsshsession.c
> >  create mode 100644 src/rpc/virnetlibsshsession.h
> 
> Is it possible to split out the src/rpc/virnetlibsshsession.(ch) changes
> (plus the ones necessary to compile it) from adding all the bits that
> actually make use of it? This patch is very big.

Yes, it is -- I was not sure how much should the changes be split,
especially in cases like this when adding a new "thing" which is used
only in a single place later on.

Just to be sure, a reasonable split for this patch would be:
1) add libssh bits in virerror
2) add virnetlibsshsession.(ch) + autofoo stuff for libssh
3) add glue code in virNetSocket + virNetClient
or were you thinking about something else? (no problems on my side)

> > diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
> > index 361dc1a..b296aac 100644
> > --- a/src/rpc/virnetclient.c
> > +++ b/src/rpc/virnetclient.c
> > @@ -505,6 +505,129 @@ virNetClientPtr virNetClientNewLibSSH2(const char 
> > *host,
> >  }
> >  #undef DEFAULT_VALUE
> >  
> > +#define DEFAULT_VALUE(VAR, VAL) \
> > +if (!VAR)   \
> > +VAR = VAL;
> > +virNetClientPtr virNetClientNewLibssh(const char *host,
> > +  const char *port,
> > +  int family,
> > +  const char *username,
> > +  const char *privkeyPath,
> > +  const char *knownHostsPath,
> > +  const char *knownHostsVerify,
> > +  const char *authMethods,
> > +  const char *netcatPath,
> > +  const char *socketPath,
> > +  virConnectAuthPtr authPtr,
> > +  virURIPtr uri)
> > +{
> > +virNetSocketPtr sock = NULL;
> > +virNetClientPtr ret = NULL;
> > +
> > +virBuffer buf = VIR_BUFFER_INITIALIZER;
> > +char *nc = NULL;
> > +char *command = NULL;
> > +
> > +char *homedir = virGetUserDirectory();
> > +char *confdir = virGetUserConfigDirectory();
> > +char *knownhosts = NULL;
> > +char *privkey = NULL;
> > +
> > +/* Use default paths for known hosts an public keys if not provided */
> 
> So is libssh able to handle e.g. ECDSA keys in known hosts? Libssh2 was
> not and truncated the known hosts file which was not acceptable.

Yes, it is. For example in my tests I'm passing
  known_hosts=$HOME/.ssh/known_hosts
as additional query item to the qemu+libssh URLs, and it works well.

> 
> > +if (confdir) {
> > +if (!knownHostsPath) {
> > +if (virFileExists(confdir)) {
> > +virBufferAsprintf(, "%s/known_hosts", confdir);
> > +if (!(knownhosts = virBufferContentAndReset()))
> 
> Use virAsprintf instead of the two lines above.

OK.

Small side note: all the glue code in virNetSocket, virNetClient and
remote_driver.c was basically a copy from the libssh2
implementation, so all these fixes should be done there too.

> > +memset(_passphrase, 0, sizeof(virConnectCredential));
> > +retr_passphrase.type = virCredTypeForPrompt(sess->cred, echo);
> > +retr_passphrase.prompt = virBufferCurrentContent();
> 
> This shouldn't really be used. Please get the content into a variable.

Not a problem to change this, but could you please explain a bit more
on the reasons? (So I can avoid doing the same again.)

> > +p = virStrncpy(buf, retr_passphrase.result,
> > +   

Re: [libvirt] [PATCH v2 2/2] libssh_transport: add new libssh-based transport

2016-10-18 Thread Daniel P. Berrange
On Mon, Oct 17, 2016 at 04:24:53PM +0200, Pino Toscano wrote:
> Implement a new libssh transport, which uses libssh to communicate with
> remote hosts, and use it in virNetSockets.
> 
> This new transport supports all the common ssh authentication methods,
> making use of libvirt's auth callbacks for interaction with the user.
> 
> Most of the functionalities and implementation are based on the libssh2
> transport.
> ---
>  config-post.h |2 +
>  configure.ac  |3 +
>  include/libvirt/virterror.h   |2 +
>  m4/virt-libssh.m4 |   26 +
>  src/Makefile.am   |   21 +-
>  src/libvirt_libssh.syms   |   22 +
>  src/remote/remote_driver.c|   41 ++
>  src/rpc/virnetclient.c|  123 
>  src/rpc/virnetclient.h|   13 +
>  src/rpc/virnetlibsshsession.c | 1424 
> +
>  src/rpc/virnetlibsshsession.h |   80 +++
>  src/rpc/virnetsocket.c|  179 ++
>  src/rpc/virnetsocket.h|   13 +
>  src/util/virerror.c   |9 +-
>  14 files changed, 1955 insertions(+), 3 deletions(-)
>  create mode 100644 m4/virt-libssh.m4
>  create mode 100644 src/libvirt_libssh.syms
>  create mode 100644 src/rpc/virnetlibsshsession.c
>  create mode 100644 src/rpc/virnetlibsshsession.h

libvirt.spec.in and mingw-libvirt.spec.in need updating too, as well
as docs/remote.html.in


> +static virClassPtr virNetLibsshSessionClass;
> +static int
> +virNetLibsshSessionOnceInit(void)
> +{
> +const char *dbgLevelStr;
> +
> +if (!(virNetLibsshSessionClass = virClassNew(virClassForObjectLockable(),
> + "virNetLibsshSession",
> + sizeof(virNetLibsshSession),
> + 
> virNetLibsshSessionDispose)))
> +return -1;
> +
> +if (ssh_init() < 0)
> +return -1;

You need to report a libvirt error here.


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v2 2/2] libssh_transport: add new libssh-based transport

2016-10-18 Thread Peter Krempa
On Mon, Oct 17, 2016 at 16:24:53 +0200, Pino Toscano wrote:
> Implement a new libssh transport, which uses libssh to communicate with
> remote hosts, and use it in virNetSockets.
> 
> This new transport supports all the common ssh authentication methods,
> making use of libvirt's auth callbacks for interaction with the user.
> 
> Most of the functionalities and implementation are based on the libssh2
> transport.
> ---
>  config-post.h |2 +
>  configure.ac  |3 +
>  include/libvirt/virterror.h   |2 +
>  m4/virt-libssh.m4 |   26 +
>  src/Makefile.am   |   21 +-
>  src/libvirt_libssh.syms   |   22 +
>  src/remote/remote_driver.c|   41 ++
>  src/rpc/virnetclient.c|  123 
>  src/rpc/virnetclient.h|   13 +
>  src/rpc/virnetlibsshsession.c | 1424 
> +
>  src/rpc/virnetlibsshsession.h |   80 +++
>  src/rpc/virnetsocket.c|  179 ++
>  src/rpc/virnetsocket.h|   13 +
>  src/util/virerror.c   |9 +-
>  14 files changed, 1955 insertions(+), 3 deletions(-)
>  create mode 100644 m4/virt-libssh.m4
>  create mode 100644 src/libvirt_libssh.syms
>  create mode 100644 src/rpc/virnetlibsshsession.c
>  create mode 100644 src/rpc/virnetlibsshsession.h

Is it possible to split out the src/rpc/virnetlibsshsession.(ch) changes
(plus the ones necessary to compile it) from adding all the bits that
actually make use of it? This patch is very big.

> diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
> index efe83aa..2efee8f 100644
> --- a/include/libvirt/virterror.h
> +++ b/include/libvirt/virterror.h
> @@ -131,6 +131,7 @@ typedef enum {
>  VIR_FROM_XENXL = 64,/* Error from Xen xl config code */
>  
>  VIR_FROM_PERF = 65, /* Error from perf */
> +VIR_FROM_LIBSSH = 66,   /* Error from libssh connection transport */
>  
>  # ifdef VIR_ENUM_SENTINELS
>  VIR_ERR_DOMAIN_LAST
> @@ -317,6 +318,7 @@ typedef enum {
>  VIR_ERR_NO_CLIENT = 96, /* Client was not found */
>  VIR_ERR_AGENT_UNSYNCED = 97,/* guest agent replies with wrong id
> to guest-sync command */
> +VIR_ERR_LIBSSH = 98,/* error in libssh transport driver 
> */

These error handling changes can be split to a separate patch as well.

>  } virErrorNumber;
>  
>  /**

[...]

> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
> index a3cd7cd..db2bdd4 100644
> --- a/src/remote/remote_driver.c
> +++ b/src/remote/remote_driver.c
> @@ -673,6 +673,7 @@ remoteConnectSupportsFeatureUnlocked(virConnectPtr conn,
>   *   - xxx:///-> UNIX domain socket
>   *   - xxx+ssh:///-> SSH connection (legacy)
>   *   - xxx+libssh2:///-> SSH connection (using libssh2)
> + *   - xxx+libssh:/// -> SSH connection (using libssh)
>   */
>  static int
>  doRemoteOpen(virConnectPtr conn,
> @@ -689,6 +690,7 @@ doRemoteOpen(virConnectPtr conn,
>  trans_libssh2,
>  trans_ext,
>  trans_tcp,
> +trans_libssh,
>  } transport;
>  #ifndef WIN32
>  char *daemonPath = NULL;
> @@ -736,6 +738,8 @@ doRemoteOpen(virConnectPtr conn,
>  transport = trans_ext;
>  } else if (STRCASEEQ(transport_str, "tcp")) {
>  transport = trans_tcp;
> +} else if (STRCASEEQ(transport_str, "libssh")) {
> +transport = trans_libssh;
>  } else {
>  virReportError(VIR_ERR_INVALID_ARG, "%s",
> _("remote_open: transport in URL not 
> recognised "
> @@ -959,6 +963,43 @@ doRemoteOpen(virConnectPtr conn,
>  priv->is_secure = 1;
>  break;
>  
> +case trans_libssh:
> +if (!sockname) {
> +/* Right now we don't support default session connections */
> +if (STREQ_NULLABLE(conn->uri->path, "/session")) {
> +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> +   _("Connecting to session instance without "
> + "socket path is not supported by the libssh 
> "
> + "connection driver"));
> +goto failed;
> +}
> +
> +if (VIR_STRDUP(sockname,
> +   flags & VIR_DRV_OPEN_REMOTE_RO ?
> +   LIBVIRTD_PRIV_UNIX_SOCKET_RO : 
> LIBVIRTD_PRIV_UNIX_SOCKET) < 0)
> +goto failed;
> +}
> +
> +VIR_DEBUG("Starting libssh session");
> +
> +priv->client = virNetClientNewLibssh(priv->hostname,
> + port,
> + AF_UNSPEC,
> + username,
> + keyfile,
> +