On 03/14/2012 06:37 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> 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@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list