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/...
>
> 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