Re: patch for UNIX domain syslog

Hello Willy--

I've attached an updated patch against 1.3.13-75, which I believe addresses your concerns and contains the suggested bugfixes you so kindly took the time to point out.

For handling sendto() failures in send_log(), it appears that the existing code is fine (no need to close/recreate socket) for both UDP and UNIX-domain syslog server. So I left things alone (did not close/recreate socket). Closing/recreating socket after each failure would also work, but would lead to increased amount of unnecessary socket creation/destruction if syslog is temporarily unavailable for some reason (especially for verbose loggers).

Thank you.

On 12/4/07, Willy Tarreau <w#1wt.eu> wrote:
>
> Hello Robert,
>
> On Tue, Dec 04, 2007 at 06:00:11PM -0800, Robert Tsai wrote:
> > Hello--
> >
> > Thank you for HAProxy. The code in haproxy-1.3.13.1 only supports
> syslogging
> > to an internet address. The attached patch:
> >
> > - Adds support for syslogging to a UNIX domain socket (e.g.,
> > /dev/log). If the address field begins with '/' (absolute file path),
> then
> > AF_UNIX is used to construct the socket. Otherwise, AF_INET is used.
> > - Achieve clean single-source build on both Mac OS X and Linux
> > (sockaddr_in.sin_len and sockaddr_un.sun_len field aren't always
> present).
> >
> > Please consider this patch for inclusion into the upstream haproxy
> codebase.
>
> First, thanks for your contribution. I have a few comments before merging
> :
>
> - you are using an array of file descriptors in send_log() for
> possibly two log sockets. This can be limitating because if you have
> two unix sockets in the global section, you have no descriptors left
> for the log entries described in proxies (eg: udp log servers). I
> would suggest having one descriptor per protocol family instead, and/or
> storing the per-proxy fd in the proxies themselves if that helps.
>
> - have you tried to stop and/or restart the log server while haproxy is
> running to see if sendto() is enough to automatically reconnect to
> the socket ? (I doubt it is). It is possible that you will need to
> catch the error, and close() then recreate a new socket.
>
> - could you please update the documentation (doc/configuration.txt will
> be enough) to indicate how to use this new feature, and to remind
> people that they might have problems when using chroot and/or uid/gid ?
> Just giving a few hints, such as putting the log socket inside the
> chroot with proper permissions should help.
>
> I'm releasing 1.3.14 this morning, so your patch will not be in it, but
> if it remains small and non-intrusive, I will probably be able to merge
> it into 1.3.14.1. If it is not too much to ask, please rediff your patch
> against latest version (either 1.3.14 if it's out when you do it, or
> 1.3.13-75 which is on the site).
>
> Thanks!
> Willy
>
>

-- 
Robert Tsai | rob@xoopit.com | http://www.xoopit.com/




application/octet-stream attachment: patch-1.3.13-75-syslog-af_unix