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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list