[collectd] Bug#535787: collectd: powerdns monitoring hangs on the pdns socket (pdns 2.9.22)

[Sebastian Harl]

Hi Luke,

On Mon, Jul 27, 2009 at 05:36:37PM -0700, Luke Heberling wrote:
> OK, two problems.
[...]
> Both problems are fixed in the attached patch, which I made against the squeeze 
> debian source. I've tested the resulting package successfully in a lenny 
> environment, both with and without a chrooted pdns_recursor.

Thanks a lot for your quick reply and the patch! See a minor comment
below.

> --- collectd-4.6.3.broken/src/powerdns.c	2009-06-02 02:17:47.000000000 -0700
> +++ collectd-4.6.3/src/powerdns.c	2009-07-27 16:44:32.000000000 -0700
> @@ -380,6 +380,16 @@ static int powerdns_get_data_dgram (list
>        break;
>      }
>  
> +    struct timeval timeout;
> +    timeout.tv_sec=5;
> +    timeout.tv_usec=0;

What do you think about using interval_g instead (or interval_g / 2 and
/ or setting some maximum value)? Basically, that should not make a big
difference, but, imho, it's a bit cleaner to let recv() fail before the
end of the read interval.

> +    status = setsockopt (sd, SOL_SOCKET, SO_RCVTIMEO, &timeout, sizeof (timeout));
> +    if (status != 0)
> +    {
> +      FUNC_ERROR ("setsockopt");
> +      break;
> +    }
> +
>      status = connect (sd, (struct sockaddr *) &item->sockaddr,
>          sizeof (item->sockaddr));
>      if (status != 0)
> @@ -911,11 +921,18 @@ static int powerdns_config (oconfig_item
>        powerdns_config_add_server (option);
>      else if (strcasecmp ("LocalSocket", option->key) == 0)
>      {
> -      char *temp = strdup (option->key);

Oops ... I wonder if that feature has ever been tested ;-)

> -      if (temp == NULL)
> -        return (1);
> -      sfree (local_sockpath);
> -      local_sockpath = temp;
> +      if ((option->values_num != 1) || (option->values[0].type != OCONFIG_TYPE_STRING))
> +      {
> +        WARNING ("powerdns plugin: `%s' needs exactly one string argument.", ci->key);
> +      }
> +      else
> +      {
> +        char *temp = strdup (option->values[0].value.string);
> +        if (temp == NULL)
> +          return (1);
> +        sfree (local_sockpath);
> +        local_sockpath = temp;
> +      }
>      }
>      else
>      {

Cheers,
Sebastian

-- 
Sebastian "tokkee" Harl +++ GnuPG-ID: 0x8501C7FC +++ http://tokkee.org/

Those who would give up Essential Liberty to purchase a little Temporary
Safety, deserve neither Liberty nor Safety.         -- Benjamin Franklin

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
Url : http://mailman.verplant.org/pipermail/collectd/attachments/20090728/77a31553/attachment.pgp