[libvirt] [PATCH 1/4] Make avahi startup more robust.

[Chris Lalancette]

On 04/21/2010 04:17 PM, Daniel Veillard wrote:
> On Wed, Apr 21, 2010 at 02:50:18PM -0400, Chris Lalancette wrote:
>> On 04/21/2010 12:25 PM, Eric Blake wrote:
>>> On 04/21/2010 10:03 AM, Chris Lalancette wrote:
>>>> If the hostname of the current virtualization machine
>>>> could not be resolved, then libvirtd would fail to
>>>> start.  However, for disconnected operation (on a laptop,
>>>> for instance) the hostname may very legitimately not
>>>> be resolvable.  This patch makes it so that if we can't
>>>> resolve the hostname, avahi doesn't fail, it just uses
>>>> a less useful MDNS string.
>>>
>>> ACK on the concept, but fix the corner-case memory leak before pushing.
>>>
>>>>
>>>>          if (!mdns_name) {
>>>> -            char groupname[64], *localhost, *tmp;
>>>> +            char *groupname, *localhost, *tmp;
>>>>              /* Extract the host part of the potentially FQDN */
>>>>              localhost = virGetHostname(NULL);
>>>
>>> Here, localhost can be allocated...
>>>
>>>>              if (localhost == NULL)
>>>> +                ret = virAsprintf(&groupname, "Virtualization Host");
>>>> +            else {
>>>> +                if ((tmp = strchr(localhost, '.')))
>>>> +                    *tmp = '\0';
>>>> +                ret = virAsprintf(&groupname, "Virtualization Host %s",
>>>> +                                  localhost);
>>>
>>> then groupname fails...
>>>
>>>> +            }
>>>> +            if (ret < 0) {
>>>> +                virReportOOMError();
>>>>                  goto cleanup;
>>>
>>> ...and we leak localhost.
>>>
>>>> -
>>>> -            if ((tmp = strchr(localhost, '.')))
>>>> -                *tmp = '\0';
>>>> -            snprintf(groupname, sizeof(groupname)-1, "Virtualization Host %s", localhost);
>>>> -            groupname[sizeof(groupname)-1] = '\0';
>>>> +            }
>>>>              group = libvirtd_mdns_add_group(server->mdns, groupname);
>>>>              VIR_FREE(localhost);
>>>> +            VIR_FREE(groupname);
>>>
>>> But on success, there is no leak.
>>>
>>
>> Oops, good call.  I'll respin the patch with that change.
> 
>   Okay, but this is an important patch, I tested this too, without it if
> one cannot resolve the hostname at boot time, libvirtd service fails to
> start, while it properly starts with that patch applied. I tested it
> myself on one of my boxes where I had the trouble.
> 
>   So ACK once the leak is fixed !

I fixed the leak and pushed it.  Thanks.

-- 
Chris Lalancette