[libvirt] [PATCH] Allow overriding default URI in config file

[Eric Blake]

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 at 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 at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120314/e6fcb383/attachment-0001.sig>