[syslog-ng] merging stuff from merge-queue/3.4

[Gergely Nagy]

On Sat, Nov 17, 2012 at 9:00 AM, Balazs Scheidler <bazsi at balabit.hu> wrote:
> system-source: Use follow-freq(1) on FreeBSD prior to 9.1
>
>    Are you sure time based polling would work here? I mean time based
>    polling also checks if the size of the file has changed, as it was
>    meant to be used only with regular files.

Yes, I am sure it works, due to this earlier patch:

commit 169bc655620bf641eb0429258533037ff9ae52bd
Author: Gergely Nagy <algernon at balabit.hu>
Date:   Fri Oct 5 12:16:57 2012 +0200

    logreader: When following a file, treat non-regular files specially

>    I'm afraid that the inability to use kqueue on /dev/klog would
>    prevent using kqueue completely, as the syslog-ng file source code
>    requires either regular files or pollable devices.

With the above patch, there is a workaround that will do until we have
a proper device() source. In the long run, I want a better solution
than this workaround, but until then, this is in my opinion, an
acceptable workaround. As the other option which does not require
extensive changes is to disable kqueue completely (and fall back to
poll) - which is what has been the case 'till 3.3.6, and it's just as
bad as this workaround, to be honest. I'd rather not to that, since in
the long run, we want device() anyway.

> dbparser: support inheriting properties in a corellation action
>
>    I merged this one, although I have some doubts. It uses
>    log_msg_clone_cow(), which does have some tricky semantics, as it
>    assumes that the cloned message will not change, once cloned.
>
>    I was thinking about removing clone_cow() completely because of this
>    tricky stuff, and whenever we need a clone we're about to change the
>    LogMessage anyway, and delaying the actual change means that we have
>    to do two malloc() calls, instead of one.
>
>    Can you test whether changing the message that triggered the action
>    with a rewrite rule, keeps the cloned one intact? This would need a
>    parallel path to the db-parser().

Mhm, I'll do that and report back (most likely this won't happen 'till
next thursday, though).

> dbparser: Make the context length available to action conditions
>
>    I didn't pick this one. I was thinking about using a template
>    function to return the number of entries in the current context.

Indeed, using a template function crossed my mind too, but to do that,
I would have to do a dance I deemed too expensive, both by the amount
of code that would need to be written and the number of hoops the
template function would have to jump through: it would need to be
called first, then look up the context, then append the string to the
result.. That seemed overly complicated to me, compared to the few
lines the patch adds.

>    This patch changes the log message while it could keep it read-only,
>    and even if I wanted to do this macro-like, I'd have created a real
>    macro. Both template formatting and template functions receive the
>    context as arguments anyway, so the required information is readily
>    available.

On the other hand, yeah... keeping the message read-only would be a
nice bonus. I'll rework this patch.

-- 
|8]