[libvirt] [PATCH v2 1/2] Explicitly error on uri=qemu://system

It's a fairly common error that a user tries to connect to a URI like qemu://system or qemu://session (missing a slash). This errors like: $ virsh --connect qemu://session error: failed to connect to the hypervisor error: Unable to resolve address 'session' service '16514': No address associated with hostname If you already know that the standard qemu URI has 3 slashes, that error will make it obvious enough. But new user's may not get it. There's even a RHEL support page explicitly mentioning it!: https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/7/htm... Catch this error early in libvirt.c virConnectOpen for qemu (and vbox which has similar rules https://bugzilla.redhat.com/show_bug.cgi?id=1038304 --- v2: Use conventional function naming Improve a comment Handle 'vz' driver src/libvirt.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/libvirt.c b/src/libvirt.c index a21d00e..919c9cb 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -928,6 +928,35 @@ virConnectGetDefaultURI(virConfPtr conf, } +/* + * Check to see if an invalid URI like qemu://system (missing /) was passed, + * offer the suggested fix. + */ +static int +virConnectCheckURIMissingSlash(const char *uristr, virURIPtr uri) +{ + /* To avoid false positives, only check drivers that mandate + a path component in the URI, like /system or /session */ + if (STRNEQ(uri->scheme, "qemu") && + STRNEQ(uri->scheme, "vbox") && + STRNEQ(uri->scheme, "vz")) + return 0; + + if (uri->path != NULL) + return 0; + + if (STREQ(uri->server, "session") || + STREQ(uri->server, "system")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid URI %s (maybe you want %s:///%s)"), + uristr, uri->scheme, uri->server); + return -1; + } + + return 0; +} + + static virConnectPtr do_open(const char *name, virConnectAuthPtr auth, @@ -995,6 +1024,12 @@ do_open(const char *name, NULLSTR(ret->uri->user), ret->uri->port, NULLSTR(ret->uri->path)); + if (virConnectCheckURIMissingSlash(alias ? alias : name, + ret->uri) < 0) { + VIR_FREE(alias); + goto failed; + } + VIR_FREE(alias); } else { VIR_DEBUG("no name, allowing driver auto-select"); -- 2.7.3

do_open and winsock_init don't follow the naming pattern of other functions in this file. Rename them to match --- src/libvirt.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 919c9cb..749089b 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -257,7 +257,7 @@ virConnectAuthPtr virConnectAuthPtrDefault = &virConnectAuthDefault; #if HAVE_WINSOCK2_H static int -winsock_init(void) +virWinsockInit(void) { WORD winsock_version, err; WSADATA winsock_data; @@ -386,7 +386,7 @@ virGlobalInit(void) VIR_DEBUG("register drivers"); #if HAVE_WINSOCK2_H - if (winsock_init() == -1) + if (virWinsockInit() == -1) goto error; #endif @@ -958,9 +958,9 @@ virConnectCheckURIMissingSlash(const char *uristr, virURIPtr uri) static virConnectPtr -do_open(const char *name, - virConnectAuthPtr auth, - unsigned int flags) +virConnectOpenInternal(const char *name, + virConnectAuthPtr auth, + unsigned int flags) { size_t i; int res; @@ -1161,7 +1161,7 @@ virConnectOpen(const char *name) VIR_DEBUG("name=%s", NULLSTR(name)); virResetLastError(); - ret = do_open(name, NULL, 0); + ret = virConnectOpenInternal(name, NULL, 0); if (!ret) goto error; return ret; @@ -1197,7 +1197,7 @@ virConnectOpenReadOnly(const char *name) VIR_DEBUG("name=%s", NULLSTR(name)); virResetLastError(); - ret = do_open(name, NULL, VIR_CONNECT_RO); + ret = virConnectOpenInternal(name, NULL, VIR_CONNECT_RO); if (!ret) goto error; return ret; @@ -1237,7 +1237,7 @@ virConnectOpenAuth(const char *name, VIR_DEBUG("name=%s, auth=%p, flags=%x", NULLSTR(name), auth, flags); virResetLastError(); - ret = do_open(name, auth, flags); + ret = virConnectOpenInternal(name, auth, flags); if (!ret) goto error; return ret; -- 2.7.3

On Tue, Apr 19, 2016 at 12:22:07 -0400, Cole Robinson wrote:
do_open and winsock_init don't follow the naming pattern of other functions in this file. Rename them to match --- src/libvirt.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
ACK

19.04.2016 19:22, Cole Robinson пишет:
It's a fairly common error that a user tries to connect to a URI like qemu://system or qemu://session (missing a slash). This errors like:
$ virsh --connect qemu://session error: failed to connect to the hypervisor error: Unable to resolve address 'session' service '16514': No address associated with hostname
If you already know that the standard qemu URI has 3 slashes, that error will make it obvious enough. But new user's may not get it. There's even a RHEL support page explicitly mentioning it!:
https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/7/htm...
Catch this error early in libvirt.c virConnectOpen for qemu (and vbox which has similar rules
https://bugzilla.redhat.com/show_bug.cgi?id=1038304 --- v2: Use conventional function naming Improve a comment Handle 'vz' driver
src/libvirt.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)
ACK

On 04/19/2016 12:46 PM, Maxim Nestratov wrote:
19.04.2016 19:22, Cole Robinson пишет:
It's a fairly common error that a user tries to connect to a URI like qemu://system or qemu://session (missing a slash). This errors like:
$ virsh --connect qemu://session error: failed to connect to the hypervisor error: Unable to resolve address 'session' service '16514': No address associated with hostname
If you already know that the standard qemu URI has 3 slashes, that error will make it obvious enough. But new user's may not get it. There's even a RHEL support page explicitly mentioning it!:
https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/7/htm...
Catch this error early in libvirt.c virConnectOpen for qemu (and vbox which has similar rules
https://bugzilla.redhat.com/show_bug.cgi?id=1038304 --- v2: Use conventional function naming Improve a comment Handle 'vz' driver
src/libvirt.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)
ACK
Thanks, pushed patch this now - Cole
participants (3)
-
Cole Robinson
-
Maxim Nestratov
-
Peter Krempa