On 03/14/2012 06:37 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Currently if the URI passed to virConnectOpen* is NULL, then we
- Look for LIBVIRT_DEFAULT_URI env var - Probe for drivers
This changes it so that
- Look for LIBVIRT_DEFAULT_URI env var - Look for 'uri_default' in $HOME/.libvirt/libvirt.conf - Probe for drivers
Nice. Do we need any followup patches for virsh? That is, where should VIRSH_DEFAULT_CONNECT_URI fit into the hierarchy?
docs/uri.html.in | 13 +++++++ src/libvirt.c | 107 +++++++++++++++++++++++++++++++++++++----------------- 2 files changed, 87 insertions(+), 33 deletions(-)
Where is the change to src/libvirt.conf to document this feature?
+++ b/docs/uri.html.in @@ -52,6 +52,19 @@ uri_aliases = [ set, no alias lookup will be attempted. </p>
+ <h2><a name="URI_default">Default URI choice</a></h2> + + <p> +If the URI passed to <code>virConnectOpen*</code> is NULL, then libvirt will use the following +logic to determine what URI to use. +</p> + + <ol> + <li>The environment variable <code>LIBVIRT_DEFAULT_URI</code></li> + <li>The client configuration file <code>uri_default</code> parameter</li> + <li>Probe each hypervisor in turn until one that works is found</li> + </ol> +
Looks good. The location of the client configuration file was mentioned earlier on the same page.
<h2> <a name="URI_virsh">Specifying URIs to virsh, virt-manager and virt-install</a>
Maybe this section should mention that without any -c option and without VIRSH_DEFAULT_CONNECT_URI, then virsh passes NULL to the virConnectOpen, which then falls back on the above hierarchy for libvirt in general.
+static int virConnectGetConfigFile(virConfPtr *conf)
Formatting nit - should this be two lines?
+ +static int virConnectGetDefaultURI(virConfPtr conf,
And this one?
+ const char **name) +{ + int ret = -1; + virConfValuePtr value = NULL; + char *defname = getenv("LIBVIRT_DEFAULT_URI"); + if (defname && *defname) { + VIR_DEBUG("Using LIBVIRT_DEFAULT_URI %s", defname); + *name = defname;
Needs a 'ret = 0; goto cleanup;' here, otherwise...
+ } + + if ((value = virConfGetValue(conf, "uri_default"))) { + if (value->type != VIR_CONF_STRING) { + virLibConnError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Expected a string for 'uri_default' config parameter")); + goto cleanup; + } + + *name = value->str;
this assignment is a memory leak if both the envvar and config file are present. Probably worth a v2 to fix the problems. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org