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" <berra...@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 ebl...@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list