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 :|
