[libvirt] [PATCH] Improve error message for disabled client-side drivers
Report that libvirt was built without that driver instead of trying to connect to a libvirtd, when we know that this is going to fail. --- I had this on my todo list for a while now, because multiple people on the mailing list and on IRC reported problems that can be summarized as: "I just built/installed libvirt and want to connect to an ESX server, but libvirt complains about missing certificates or wants to connect to a non-existing libvirtd on the ESX server. I don't get it..." The problem was always the same: libvirt built without the ESX driver. But people don't get that, because libvirt reported cryptic errors in that situation. I decided to fix this now because the problem was reported again on IRC today. src/libvirt.c | 28 ++++++++++++++++++++++++++++ 1 files changed, 28 insertions(+), 0 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 2754fd0..2487b82 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -1210,6 +1210,34 @@ do_open (const char *name, ret->flags = flags & VIR_CONNECT_RO; for (i = 0; i < virDriverTabCount; i++) { + /* We're going to probe the remote driver next. So we have already + * probed all other client-side-only driver before, but none of them + * accepted the URI. + * If the scheme corresponds to a known but disabled client-side-only + * driver then report a useful error, instead of a cryptic one about + * not being able to connect to libvirtd or not being able to find + * certificates. */ + if (virDriverTab[i]->no == VIR_DRV_REMOTE && + ret->uri != NULL && ret->uri->scheme != NULL && + ( +# ifndef WITH_PHYP + STRCASEEQ(ret->uri->scheme, "phyp") || +# endif +# ifndef WITH_ESX + STRCASEEQ(ret->uri->scheme, "esx") || + STRCASEEQ(ret->uri->scheme, "gsx") || +# endif +# ifndef WITH_XENAPI + STRCASEEQ(ret->uri->scheme, "xenapi") || +# endif + false)) { + virReportErrorHelper(NULL, VIR_FROM_NONE, VIR_ERR_INVALID_ARG, + __FILE__, __FUNCTION__, __LINE__, + _("libvirt was built without the '%s' driver"), + ret->uri->scheme); + goto failed; + } + DEBUG("trying driver %d (%s) ...", i, virDriverTab[i]->name); res = virDriverTab[i]->open (ret, auth, flags); -- 1.7.0.4
On 06/09/2010 05:28 PM, Matthias Bolte wrote:
Report that libvirt was built without that driver instead of trying to connect to a libvirtd, when we know that this is going to fail. ---
I had this on my todo list for a while now, because multiple people on the mailing list and on IRC reported problems that can be summarized as:
"I just built/installed libvirt and want to connect to an ESX server, but libvirt complains about missing certificates or wants to connect to a non-existing libvirtd on the ESX server. I don't get it..."
The problem was always the same: libvirt built without the ESX driver. But people don't get that, because libvirt reported cryptic errors in that situation.
Seems reasonable.
for (i = 0; i < virDriverTabCount; i++) { + /* We're going to probe the remote driver next. So we have already + * probed all other client-side-only driver before, but none of them + * accepted the URI. + * If the scheme corresponds to a known but disabled client-side-only + * driver then report a useful error, instead of a cryptic one about + * not being able to connect to libvirtd or not being able to find + * certificates. */
Thanks for the comment; it helps. My only worry is whether there is a maintenance burden on deciding which drivers are client-side-only, and whether we will remember to add future drivers here or to remove existing members of this list at a point when we add remote support for these drivers, but I don't think it is too bad. ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Thu, Jun 10, 2010 at 01:28:29AM +0200, Matthias Bolte wrote:
Report that libvirt was built without that driver instead of trying to connect to a libvirtd, when we know that this is going to fail. ---
I had this on my todo list for a while now, because multiple people on the mailing list and on IRC reported problems that can be summarized as:
"I just built/installed libvirt and want to connect to an ESX server, but libvirt complains about missing certificates or wants to connect to a non-existing libvirtd on the ESX server. I don't get it..."
The problem was always the same: libvirt built without the ESX driver. But people don't get that, because libvirt reported cryptic errors in that situation.
I decided to fix this now because the problem was reported again on IRC today.
src/libvirt.c | 28 ++++++++++++++++++++++++++++ 1 files changed, 28 insertions(+), 0 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c index 2754fd0..2487b82 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -1210,6 +1210,34 @@ do_open (const char *name, ret->flags = flags & VIR_CONNECT_RO;
for (i = 0; i < virDriverTabCount; i++) { + /* We're going to probe the remote driver next. So we have already + * probed all other client-side-only driver before, but none of them + * accepted the URI. + * If the scheme corresponds to a known but disabled client-side-only + * driver then report a useful error, instead of a cryptic one about + * not being able to connect to libvirtd or not being able to find + * certificates. */ + if (virDriverTab[i]->no == VIR_DRV_REMOTE && + ret->uri != NULL && ret->uri->scheme != NULL && + ( +# ifndef WITH_PHYP + STRCASEEQ(ret->uri->scheme, "phyp") || +# endif +# ifndef WITH_ESX + STRCASEEQ(ret->uri->scheme, "esx") || + STRCASEEQ(ret->uri->scheme, "gsx") || +# endif +# ifndef WITH_XENAPI + STRCASEEQ(ret->uri->scheme, "xenapi") || +# endif + false)) { + virReportErrorHelper(NULL, VIR_FROM_NONE, VIR_ERR_INVALID_ARG, + __FILE__, __FUNCTION__, __LINE__, + _("libvirt was built without the '%s' driver"), + ret->uri->scheme); + goto failed; + } + DEBUG("trying driver %d (%s) ...", i, virDriverTab[i]->name); res = virDriverTab[i]->open (ret, auth, flags);
ACK, this looks fine to me. One day I'd like to change the way we pick the drivers during the open method, but that's faar too invasive for now. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
2010/6/11 Daniel P. Berrange <berrange@redhat.com>:
On Thu, Jun 10, 2010 at 01:28:29AM +0200, Matthias Bolte wrote:
Report that libvirt was built without that driver instead of trying to connect to a libvirtd, when we know that this is going to fail. ---
I had this on my todo list for a while now, because multiple people on the mailing list and on IRC reported problems that can be summarized as:
"I just built/installed libvirt and want to connect to an ESX server, but libvirt complains about missing certificates or wants to connect to a non-existing libvirtd on the ESX server. I don't get it..."
The problem was always the same: libvirt built without the ESX driver. But people don't get that, because libvirt reported cryptic errors in that situation.
I decided to fix this now because the problem was reported again on IRC today.
src/libvirt.c | 28 ++++++++++++++++++++++++++++ 1 files changed, 28 insertions(+), 0 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c index 2754fd0..2487b82 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -1210,6 +1210,34 @@ do_open (const char *name, ret->flags = flags & VIR_CONNECT_RO;
for (i = 0; i < virDriverTabCount; i++) { + /* We're going to probe the remote driver next. So we have already + * probed all other client-side-only driver before, but none of them + * accepted the URI. + * If the scheme corresponds to a known but disabled client-side-only + * driver then report a useful error, instead of a cryptic one about + * not being able to connect to libvirtd or not being able to find + * certificates. */ + if (virDriverTab[i]->no == VIR_DRV_REMOTE && + ret->uri != NULL && ret->uri->scheme != NULL && + ( +# ifndef WITH_PHYP + STRCASEEQ(ret->uri->scheme, "phyp") || +# endif +# ifndef WITH_ESX + STRCASEEQ(ret->uri->scheme, "esx") || + STRCASEEQ(ret->uri->scheme, "gsx") || +# endif +# ifndef WITH_XENAPI + STRCASEEQ(ret->uri->scheme, "xenapi") || +# endif + false)) { + virReportErrorHelper(NULL, VIR_FROM_NONE, VIR_ERR_INVALID_ARG, + __FILE__, __FUNCTION__, __LINE__, + _("libvirt was built without the '%s' driver"), + ret->uri->scheme); + goto failed; + } + DEBUG("trying driver %d (%s) ...", i, virDriverTab[i]->name); res = virDriverTab[i]->open (ret, auth, flags);
ACK, this looks fine to me. One day I'd like to change the way we pick the drivers during the open method, but that's faar too invasive for now.
Regards, Daniel
Maybe something like having a scheme-to-driver mapping table would be nice, instead of asking each driver. Thanks, pushed. Matthias
On Fri, Jun 11, 2010 at 06:25:29PM +0200, Matthias Bolte wrote:
2010/6/11 Daniel P. Berrange <berrange@redhat.com>:
On Thu, Jun 10, 2010 at 01:28:29AM +0200, Matthias Bolte wrote:
Report that libvirt was built without that driver instead of trying to connect to a libvirtd, when we know that this is going to fail. ---
I had this on my todo list for a while now, because multiple people on the mailing list and on IRC reported problems that can be summarized as:
"I just built/installed libvirt and want to connect to an ESX server, but libvirt complains about missing certificates or wants to connect to a non-existing libvirtd on the ESX server. I don't get it..."
The problem was always the same: libvirt built without the ESX driver. But people don't get that, because libvirt reported cryptic errors in that situation.
I decided to fix this now because the problem was reported again on IRC today.
src/libvirt.c | 28 ++++++++++++++++++++++++++++ 1 files changed, 28 insertions(+), 0 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c index 2754fd0..2487b82 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -1210,6 +1210,34 @@ do_open (const char *name, ret->flags = flags & VIR_CONNECT_RO;
for (i = 0; i < virDriverTabCount; i++) { + /* We're going to probe the remote driver next. So we have already + * probed all other client-side-only driver before, but none of them + * accepted the URI. + * If the scheme corresponds to a known but disabled client-side-only + * driver then report a useful error, instead of a cryptic one about + * not being able to connect to libvirtd or not being able to find + * certificates. */ + if (virDriverTab[i]->no == VIR_DRV_REMOTE && + ret->uri != NULL && ret->uri->scheme != NULL && + ( +# ifndef WITH_PHYP + STRCASEEQ(ret->uri->scheme, "phyp") || +# endif +# ifndef WITH_ESX + STRCASEEQ(ret->uri->scheme, "esx") || + STRCASEEQ(ret->uri->scheme, "gsx") || +# endif +# ifndef WITH_XENAPI + STRCASEEQ(ret->uri->scheme, "xenapi") || +# endif + false)) { + virReportErrorHelper(NULL, VIR_FROM_NONE, VIR_ERR_INVALID_ARG, + __FILE__, __FUNCTION__, __LINE__, + _("libvirt was built without the '%s' driver"), + ret->uri->scheme); + goto failed; + } + DEBUG("trying driver %d (%s) ...", i, virDriverTab[i]->name); res = virDriverTab[i]->open (ret, auth, flags);
ACK, this looks fine to me. One day I'd like to change the way we pick the drivers during the open method, but that's faar too invasive for now.
Regards, Daniel
Maybe something like having a scheme-to-driver mapping table would be nice, instead of asking each driver.
Yes, that's exactly what I'd think. In addition when compiling out drivers instead of not adding anything to the driver table, we could add a no-op driver that simply reports that the driver isn't available. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
2010/6/11 Daniel P. Berrange <berrange@redhat.com>:
On Fri, Jun 11, 2010 at 06:25:29PM +0200, Matthias Bolte wrote:
2010/6/11 Daniel P. Berrange <berrange@redhat.com>:
On Thu, Jun 10, 2010 at 01:28:29AM +0200, Matthias Bolte wrote:
Report that libvirt was built without that driver instead of trying to connect to a libvirtd, when we know that this is going to fail. ---
ACK, this looks fine to me. One day I'd like to change the way we pick the drivers during the open method, but that's faar too invasive for now.
Regards, Daniel
Maybe something like having a scheme-to-driver mapping table would be nice, instead of asking each driver.
Yes, that's exactly what I'd think. In addition when compiling out drivers instead of not adding anything to the driver table, we could add a no-op driver that simply reports that the driver isn't available.
Daniel
I started this patch like that. I added a minimal driver that does exactly that and I edited virInitialize like this: # ifdef WITH_ESX if (esxRegister() == -1) return -1; # else if (disabledRegister("esx") == -1) return -1; if (disabledRegister("gsx") == -1) return -1; # endif But I dropped it after I thought that I could not make this work with driver modules. But as I think of it now again: I could just have called disabledRegister in virDriverLoadModule when the requested driver wasn't found. Damn, I should have give it a second thought before pushing this patch. :( My first approach wouldn't have touched the way do_open picks the drivers, so it's not that invasive. It would just have put those "disabled" entries in to the driver table. Maybe I should recreate that disabled driver, just to see if I can make it really working properly. Any interest in that approach? Matthias
On 06/11/2010 10:45 AM, Matthias Bolte wrote:
I started this patch like that. I added a minimal driver that does exactly that and I edited virInitialize like this:
# ifdef WITH_ESX if (esxRegister() == -1) return -1; # else if (disabledRegister("esx") == -1) return -1; if (disabledRegister("gsx") == -1) return -1; # endif
But I dropped it after I thought that I could not make this work with driver modules. But as I think of it now again: I could just have called disabledRegister in virDriverLoadModule when the requested driver wasn't found. Damn, I should have give it a second thought before pushing this patch. :(
My first approach wouldn't have touched the way do_open picks the drivers, so it's not that invasive. It would just have put those "disabled" entries in to the driver table.
Maybe I should recreate that disabled driver, just to see if I can make it really working properly.
Any interest in that approach?
Sure; we can always 'git revert' out one patch to replace it with a better approach, if we like how it turns out. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Matthias Bolte