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
