On 03/14/2012 07:38 AM, Daniel P. Berrange wrote:
On Wed, Mar 14, 2012 at 06:54:40AM -0600, Eric Blake wrote:
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?
We should deprecate it :-)
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?
Missing :-)
+ 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.
We're returning const strings here, so there's no leak to deal with
Ah, I missed that. both getenv() and virConfGetValue properly hang on to their returned strings, so we are not leaking after all. I withdraw my objection on this part, but there's still the libvirt.conf and virsh interaction to document/fix. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org