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@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list