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

[Peter Krempa]

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.

> +{
> +    /* These drivers _only_ accepts URIs with a 'path' element */

Only these drivers accept ... ? I don't quite follow the message of this
comment.

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

Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160419/d3b26b46/attachment-0001.sig>