| Message ID | 4c80593d9666307ec7bf27ca7d12311435aedbc7.1461020644.git.crobinso@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
On 04/19/2016 04:30 AM, Peter Krempa wrote: > On Mon, Apr 18, 2016 at 19:04:04 -0400, Cole Robinson wrote: >> 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/html/Virtualization_Deployment_and_Administration_Guide/sect-Troubleshooting-Common_libvirt_errors_and_troubleshooting.html >> >> 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 >> --- >> src/libvirt.c | 32 ++++++++++++++++++++++++++++++++ >> 1 file changed, 32 insertions(+) >> >> diff --git a/src/libvirt.c b/src/libvirt.c >> index a21d00e..7607ae3 100644 >> --- a/src/libvirt.c >> +++ b/src/libvirt.c >> @@ -928,6 +928,33 @@ virConnectGetDefaultURI(virConfPtr conf, >> } >> >> >> +/* >> + * Check to see if an invalid URI like qemu://system (missing /) was passed, >> + * offer the suggested fix. >> + */ >> +static int >> +check_uri_missing_slash(const char *uristr, virURIPtr uri) > > Please use a name with "vir" prefix and camel case. This is totaly > against our naming convention. > Yes I know, but I was just following the pattern of the do_open function below. I'll change it to virConnectCheckURIMissingSlash >> +{ >> + /* These drivers _only_ accepts URIs with a 'path' element */ > > Only these drivers accept ... ? I don't quite follow the message of this > comment. > I'll change it to /* To avoid false positives, only check drivers that mandate a path component in the URI, like /system or /session */ >> + if (STRNEQ(uri->sceme, "qemu") && >> + STRNEQ(uri->scheme, "vbox")) >> + 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; >> +} > > ACK with the rename and fix of the comment. > Thanks, I'll send a v2 though with the vz change - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 04/19/2016 03:59 AM, Maxim Nestratov wrote: > 19.04.2016 2:04, 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/html/Virtualization_Deployment_and_Administration_Guide/sect-Troubleshooting-Common_libvirt_errors_and_troubleshooting.html >> >> >> Catch this error early in libvirt.c virConnectOpen for qemu (and vbox >> which has similar rules > > Please, add 'vz' also as it suffers from the problem too. Though it doesn't > support vz:///session it clearly says about it in the error message. Sounds good, I'll send a v2 - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
diff --git a/src/libvirt.c b/src/libvirt.c index a21d00e..7607ae3 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -928,6 +928,33 @@ virConnectGetDefaultURI(virConfPtr conf, } +/* + * Check to see if an invalid URI like qemu://system (missing /) was passed, + * offer the suggested fix. + */ +static int +check_uri_missing_slash(const char *uristr, virURIPtr uri) +{ + /* These drivers _only_ accepts URIs with a 'path' element */ + if (STRNEQ(uri->scheme, "qemu") && + STRNEQ(uri->scheme, "vbox")) + 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 +1022,11 @@ do_open(const char *name, NULLSTR(ret->uri->user), ret->uri->port, NULLSTR(ret->uri->path)); + if (check_uri_missing_slash(alias ? alias : name, ret->uri) < 0) { + VIR_FREE(alias); + goto failed; + } + VIR_FREE(alias); } else { VIR_DEBUG("no name, allowing driver auto-select");