[libvirt] [PATCH 1/2] virterror: Fix error message for VIR_ERR_INVALID_ARG
When a detail message is presented, nobody expects prefix 'invalid argument in' but something more general, like 'invalid argument:'. --- src/util/virterror.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/util/virterror.c b/src/util/virterror.c index c5babb1..f4541bd 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -794,7 +794,7 @@ virErrorMsg(virErrorNumber error, const char *info) if (info == NULL) errmsg = _("invalid argument in"); else - errmsg = _("invalid argument in %s"); + errmsg = _("invalid argument: %s"); break; case VIR_ERR_OPERATION_FAILED: if (info != NULL) -- 1.7.3.4
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. --- Although v2 was ACKed I send v3 so we have comprehensive picture. And yeah, I've changed the error message. 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..9ab897b 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", + _("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/26/2011 03:56 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. --- + if (conn->uri->server) { + remoteError(VIR_ERR_INVALID_ARG, "%s", + _("using unix socket and remote " + "server is not supported."));
You didn't take my suggestion at mentioning conn->uri->server in the error message. But that's a minor point, so I'm okay giving ACK, whether or not you make that tweak before pushing. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 08/26/2011 03:56 AM, Michal Privoznik wrote:
When a detail message is presented, nobody expects prefix 'invalid argument in' but something more general, like 'invalid argument:'. --- src/util/virterror.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/util/virterror.c b/src/util/virterror.c index c5babb1..f4541bd 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -794,7 +794,7 @@ virErrorMsg(virErrorNumber error, const char *info) if (info == NULL) errmsg = _("invalid argument in");
Do we need to fix this one too, to just "invalid argument"? Then again, I don't know if any callers pass NULL for info (this whole virterror.c code could use some major cleanups. I have a stale patch in my local git tree where I tried to simplify things once to not have an if/else inside every case: of that function, but it would take a while to dig it out and get it into shape with the current tree).
else - errmsg = _("invalid argument in %s"); + errmsg = _("invalid argument: %s");
ACK with the counterpart line fixed too. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Michal Privoznik