Re: [libvirt] [PATCH] Improve error message for disabled client-side drivers

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