[libvirt PATCH 9/9] rpc: use new virt-nc binary for remote tunnelling

[Daniel P. Berrangé]

On Wed, Jul 15, 2020 at 02:25:14PM +0200, Andrea Bolognani wrote:
> On Wed, 2020-07-15 at 11:00 +0100, Daniel P. Berrangé wrote:
> > On Fri, Jul 10, 2020 at 07:21:47PM +0200, Andrea Bolognani wrote:
> > > Just a couple of comments about the UI: would it make sense to use
> > > something like
> > > 
> > >   qemu+ssh://host/system?tunnelcmd=virt-tunnel
> > > 
> > > instead? virt-nc could be seen as a bit of a misnomer, considering
> > > that (by design) it doesn't do nearly as much as the actual netcat
> > > does; as for the 'proxy' argument, I'm afraid it might lead people
> > > to believe it's used for HTTP proxying or some other form of
> > > proxying *between the client and the host*, whereas it's really
> > > something that only affects operations on the host itself - not to
> > > mention that we also have a virtproxyd now, further increasing the
> > > potential for confusion...
> > 
> > I chose proxy not tunnel, because SSH is providing the tunnel here.
> > virt-nc is a proxy linking the tunnel to the daemon. virtproxyd is
> > conceptually similar, again linking a libvirt client to the real
> > daemon.
> 
> Mh, that makes sense but I'm still wary of using "proxy" due to the
> potential for confusion, since in this case the proxy is on the
> opposite side of the connection than one would probably expect it
> to be. Something like "remoteproxy" or "serverproxy", perhaps?

I don't think there's really any problem of confusion here unless
someone doesn't read the docs at all, in which case they won't
even know about this parameter. So I don't think using a more
verbose term is any benefit.

> 
> > I probably shouldn't mention "virt-nc" by name in the URI and instead
> > have "proxy=netcat" vs "proxy=native", as users don't get to choose
> > the actual binary here - they are providing an enum string, which
> > gets mapped to the desired binary.
> 
> Yeah, that's much better.
> 
> Going back to the name of the command itself, since it's an internal
> implementation details, and as such it's not intended to be invoked
> by users and accordingly we're installing it under $(libexecdir)
> along with existing helpers, what about following the established
> naming convention and calling it 'libvirt_proxyhelper'?

Installing it to libexecdir is actually a mistake in this version. It
needs to be installed into bindir, as it must be present in $PATH.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|