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@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?
>
> 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@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