Re: [libvirt] PATCH: 4/12: Refactoring URI probing

This patch changes the way hypervisor URIs are probed when NULL is passed to the virConnectOpen() method. Previously we had an explicit probe method called in each driver. This does not work when we move some drivers out of libvirt.so and into libvirtd daemon. So I've refactored the way this works so that we simply pass a NULL uri into the individual driver open methods. If they see a NULL uri, they will make an attempt to open, and return a VIR_DRV_OPEN_DECLINED code if its not suitable. This also works for the remote driver, probing supported URI inside the daemon. Quite alot of code churn but the end result should be functionally the same

+++ b/src/datatypes.c Wed Nov 12 21:59:20 2008 +0000 @@ -174,7 +174,7 @@ */ static void virReleaseConnect(virConnectPtr conn) { - DEBUG("release connection %p %s", conn, conn->name); + DEBUG("release connection %p", conn);

if (conn->domains != NULL) virHashFree(conn->domains, (virHashDeallocator) virDomainFreeName); if (conn->networks != NULL) @@ -188,7 +188,7 @@ if (virLastErr.conn == conn) virLastErr.conn = NULL; - VIR_FREE(conn->name); + xmlFreeURI(conn->uri); pthread_mutex_unlock(&conn->lock); pthread_mutex_destroy(&conn->lock); @@ -213,7 +213,7 @@ return(-1); } pthread_mutex_lock(&conn->lock); - DEBUG("unref connection %p %s %d", conn, conn->name, conn->refs); + DEBUG("unref connection %p %d", conn, conn->refs); conn->refs--; refs = conn->refs; if (refs == 0) { @@ -322,7 +322,7 @@ VIR_FREE(domain->name); VIR_FREE(domain); - DEBUG("unref connection %p %s %d", conn, conn->name, conn->refs); + DEBUG("unref connection %p %d", conn, conn->refs); conn->refs--; if (conn->refs == 0) { virReleaseConnect(conn); @@ -458,7 +458,7 @@ VIR_FREE(network->name); VIR_FREE(network); - DEBUG("unref connection %p %s %d", conn, conn->name, conn->refs); + DEBUG("unref connection %p %d", conn, conn->refs); conn->refs--; if (conn->refs == 0) { virReleaseConnect(conn); @@ -591,7 +591,7 @@ VIR_FREE(pool->name); VIR_FREE(pool); - DEBUG("unref connection %p %s %d", conn, conn->name, conn->refs); + DEBUG("unref connection %p %d", conn, conn->refs); conn->refs--; if (conn->refs == 0) { virReleaseConnect(conn); @@ -729,7 +729,7 @@ VIR_FREE(vol->pool); VIR_FREE(vol); - DEBUG("unref connection %p %s %d", conn, conn->name, conn->refs); + DEBUG("unref connection %p %d", conn, conn->refs); conn->refs--; if (conn->refs == 0) { virReleaseConnect(conn); diff -r d97fa69e255b src/datatypes.h --- a/src/datatypes.h Wed Nov 12 21:05:13 2008 +0000 +++ b/src/datatypes.h Wed Nov 12 21:59:20 2008 +0000 @@ -87,7 +87,7 @@ struct _virConnect { unsigned int magic; /* specific value to check */ int flags; /* a set of connection flags */ - char *name; /* connection URI */ + xmlURIPtr uri; /* connection URI */ /* The underlying hypervisor driver and network driver. */ virDriverPtr driver; diff -r d97fa69e255b src/driver.h --- a/src/driver.h Wed Nov 12 21:05:13 2008 +0000 +++ b/src/driver.h Wed Nov 12 21:59:20 2008 +0000 @@ -63,11 +63,8 @@ #define VIR_DRV_SUPPORTS_FEATURE(drv,conn,feature) \ ((drv)->supports_feature ? (drv)->supports_feature((conn),(feature)) : 0) -typedef const char * - (*virDrvProbe) (void); typedef virDrvOpenStatus (*virDrvOpen) (virConnectPtr conn, - xmlURIPtr uri, virConnectAuthPtr auth, int flags); typedef int @@ -302,7 +299,6 @@ int no; /* the number virDrvNo */ const char * name; /* the name of the driver */ unsigned long ver; /* the version of the backend */ - virDrvProbe probe; virDrvOpen open; virDrvClose close; virDrvDrvSupportsFeature supports_feature; diff -r d97fa69e255b src/libvirt.c --- a/src/libvirt.c Wed Nov 12 21:05:13 2008 +0000 +++ b/src/libvirt.c Wed Nov 12 21:59:20 2008 +0000 @@ -685,8 +685,11 @@ int flags) { int i, res; - virConnectPtr ret = NULL; - xmlURIPtr uri; + virConnectPtr ret; + + ret = virGetConnect(); + if (ret == NULL) + return NULL; /* * If no URI is passed, then check for an environment string if not @@ -699,74 +702,43 @@ DEBUG("Using LIBVIRT_DEFAULT_URI %s", defname); name = defname; } else { - const char *use = NULL; - const char *latest; - int probes = 0; - for (i = 0; i < virDriverTabCount; i++) { - if ((virDriverTab[i]->probe != NULL) && - ((latest = virDriverTab[i]->probe()) != NULL)) { - probes++; - - DEBUG("Probed %s", latest); - /* - * if running a xen kernel, give it priority over - * QEmu emulation - */ - if (STREQ(latest, "xen:///")) - use = latest; - else if (use == NULL) - use = latest; - } - } - if (use == NULL) { - name = "xen:///"; - DEBUG("Could not probe any hypervisor defaulting to %s", - name); - } else { - name = use; - DEBUG("Using %s as default URI, %d hypervisor found", - use, probes); - } - } - } - - /* Convert xen -> xen:/// for back compat */ - if (STRCASEEQ(name, "xen")) - name = "xen:///"; - - /* Convert xen:// -> xen:/// because xmlParseURI cannot parse the - * former. This allows URIs such as xen://localhost to work. - */ - if (STREQ (name, "xen://")) - name = "xen:///"; - - ret = virGetConnect(); - if (ret == NULL) - return NULL; - - uri = xmlParseURI (name); - if (!uri) { - virLibConnError (ret, VIR_ERR_INVALID_ARG, - _("could not parse connection URI")); - goto failed; - } - - DEBUG("name \"%s\" to URI components:\n" - " scheme %s\n" - " opaque %s\n" - " authority %s\n" - " server %s\n" - " user %s\n" - " port %d\n" - " path %s\n", - name, - uri->scheme, uri->opaque, uri->authority, uri->server, - uri->user, uri->port, uri->path); - - ret->name = strdup (name); - if (!ret->name) { - virLibConnError (ret, VIR_ERR_NO_MEMORY, _("allocating conn->name")); - goto failed; + name = NULL; + } + } + + if (name) { + /* Convert xen -> xen:/// for back compat */ + if (STRCASEEQ(name, "xen")) + name = "xen:///"; + + /* Convert xen:// -> xen:/// because xmlParseURI cannot parse the + * former. This allows URIs such as xen://localhost to work. + */ + if (STREQ (name, "xen://")) + name = "xen:///"; + + ret->uri = xmlParseURI (name); + if (!ret->uri) { + virLibConnError (ret, VIR_ERR_INVALID_ARG, + _("could not parse connection URI")); + goto failed; + } + + DEBUG("name \"%s\" to URI components:\n" + " scheme %s\n" + " opaque %s\n" + " authority %s\n" + " server %s\n" + " user %s\n" + " port %d\n" + " path %s\n", + name, + ret->uri->scheme, ret->uri->opaque, + ret->uri->authority, ret->uri->server, + ret->uri->user, ret->uri->port, + ret->uri->path);