[libvirt] [PATCH v2] remote: Refuse connecting to remote socket
If users wants to connect to remote unix socket, e.g. 'qemu+unix://<remote>/system' currently the <remote> part is ignored, ending up connecting to localhost. Connecting to remote socket is not supported and user should have used TLS/TCP/SSH instead. --- src/remote/remote_driver.c | 13 ++++++++++--- 1 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 28f333b..563ccf4 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -325,9 +325,16 @@ doRemoteOpen (virConnectPtr conn, } else { if (STRCASEEQ (transport_str, "tls")) transport = trans_tls; - else if (STRCASEEQ (transport_str, "unix")) - transport = trans_unix; - else if (STRCASEEQ (transport_str, "ssh")) + else if (STRCASEEQ (transport_str, "unix")) { + if (conn->uri->server) { + remoteError(VIR_ERR_INVALID_ARG, "%s", + _("rempte_open: using unix socket and " + "remote server is not supported.")); + return VIR_DRV_OPEN_ERROR; + } else { + transport = trans_unix; + } + } else if (STRCASEEQ (transport_str, "ssh")) transport = trans_ssh; else if (STRCASEEQ (transport_str, "ext")) transport = trans_ext; -- 1.7.3.4
On 08/24/2011 03:41 AM, Michal Privoznik wrote:
If users wants to connect to remote unix socket, e.g. 'qemu+unix://<remote>/system' currently the<remote> part is ignored, ending up connecting to localhost. Connecting to remote socket is not supported and user should have used TLS/TCP/SSH instead. --- src/remote/remote_driver.c | 13 ++++++++++--- 1 files changed, 10 insertions(+), 3 deletions(-)
+ else if (STRCASEEQ (transport_str, "unix")) { + if (conn->uri->server) { + remoteError(VIR_ERR_INVALID_ARG, "%s", + _("rempte_open: using unix socket and " + "remote server is not supported."));
s/rempte_open/remote_open/ Then again, remoteError already automatically adds the function name (since it is a macro that includes __func__), so your repeat of the name is redundant. Also, why not list the parsed server, in case it helps the user: remoteError(VIR_ERR_INVALID_ARG, _("using unix socket and remote server '%s' is not supported"), conn->uri->server ACK with that fixed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 24.08.2011 18:01, Eric Blake wrote:
On 08/24/2011 03:41 AM, Michal Privoznik wrote:
If users wants to connect to remote unix socket, e.g. 'qemu+unix://<remote>/system' currently the<remote> part is ignored, ending up connecting to localhost. Connecting to remote socket is not supported and user should have used TLS/TCP/SSH instead. --- src/remote/remote_driver.c | 13 ++++++++++--- 1 files changed, 10 insertions(+), 3 deletions(-)
+ else if (STRCASEEQ (transport_str, "unix")) { + if (conn->uri->server) { + remoteError(VIR_ERR_INVALID_ARG, "%s", + _("rempte_open: using unix socket and " + "remote server is not supported."));
s/rempte_open/remote_open/
Then again, remoteError already automatically adds the function name (since it is a macro that includes __func__), so your repeat of the name is redundant.
Also, why not list the parsed server, in case it helps the user:
remoteError(VIR_ERR_INVALID_ARG, _("using unix socket and remote server '%s' is not supported"), conn->uri->server
ACK with that fixed.
remoteError(VIR_ERR_INVALID_ARG,..) actually prepend passed message with "error: invalid argument in". So I wanted the whole message to make sense :) So should I push mine, yours, or completely different version? Michal
On 08/25/2011 02:04 AM, Michal Privoznik wrote:
Then again, remoteError already automatically adds the function name (since it is a macro that includes __func__), so your repeat of the name is redundant.
Also, why not list the parsed server, in case it helps the user:
remoteError(VIR_ERR_INVALID_ARG, _("using unix socket and remote server '%s' is not supported"), conn->uri->server
ACK with that fixed.
remoteError(VIR_ERR_INVALID_ARG,..) actually prepend passed message with "error: invalid argument in". So I wanted the whole message to make sense :)
I think that's a bug in virterror.c then; most clients of VIR_ERR_INVALID_ARG are not expecting to fit into a pre-defined sentence like this. Better would be to fix virterror.c to be a sane message, then scrub the few clients that would no longer be worded sanely.
So should I push mine, yours, or completely different version?
Cleaning up existing VIR_ERR_INVALID_ARG oddities is a separate patch, and shouldn't interfere with you pushing this patch now. But I'm not sure which version to go with, whether it's better to read well now just to be changed later with a cleanup, or to read awkwardly now based on the assumption of a coming cleanup. Anyone else have thoughts on the matter? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Michal Privoznik