On Wed, 2020-07-15 at 14:25 +0100, Daniel P. Berrangé wrote:
On Wed, Jul 15, 2020 at 02:25:14PM +0200, Andrea Bolognani wrote:
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.
Okay.
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.
Why is that so? Is it because otherwise we can't guarantee that ssh on the remote end will find it, seeing as $(libexecdir) can be changed at configure time and is usually not part of $PATH anyway? If the binary will show up in $PATH, then I think it's even more important to ensure it's very apparent that it's for internal use and not to be invoked directly. Including a message explaining this in the help output as well as the usage message that is printed when a URI is not passed on the command line would be a great start. As for the name of the binary itself, longer and more descriptive is clearly better to avoid confusion. What about 'virt-proxy-helper'? -- Andrea Bolognani / Red Hat / Virtualization