[systemd-devel] [PATCH] journal: Introduce journal-syslogd

[Susant Sahani]

On 04/10/2015 04:15 PM, Lennart Poettering wrote:
> On Thu, 09.04.15 23:43, Susant Sahani (susant at redhat.com) wrote:
>
>>     This tiny daemon enables to pull journal entries and push to a UDP
>> multicast address in syslog RFC 5424 format. systemd-journal-syslogd
>> runs with own user systemd-journal-syslog. It starts running after
>> the network is up.
>
> Hmm, before we merge this, one more thing: I think we need a more
> explanatory name for this. If we call it "systemd-journal-syslogd",
> then people might assume this might be invovled with or necessary for
> systemd *reading* syslog traffic. However, that's not what it does, it
> only *sends* syslog traffic. Hence maybe call it
> "systemd-journal-syslog-emitd" or so?

Sorry for the late reply.  Yes this would be a confusion.
systemd-journal-syslog-emitd would be bit long :). As zbignew suggested
journal-netlogd sounds OK .
>
> Also, how can we tap into the multicast traffic for this? Is there any
> better tool for this around, that is commonly available and used?
>
> How did you test this?

We need to test whether the formatting of message is correct i.e. can be 
parsed by any RFC 5424 parser such as rsyslog. I tested it using rsyslog 
imudp . If we capture  the traffic of these UDP messages via wireshark , 
these messages are getting parsed .
>
>> +        if (rename(temp_path, m->state_file) < 0) {
>> +                r = -errno;
>> +                goto finish;
>> +        }
>> +
>> +        free(temp_path);
>> +        temp_path = NULL;
>> +
>> + finish:
>> +        if (r < 0)
>> +                log_error_errno(r, "Failed to save state %s: %m", m->state_file);
>> +
>
> The failure path should remove the file temp_path here. Add something like:
>
> if (temp_path)
>          (void) unlink(temp_path);
>
>
>> +static int network_send(Manager *m, struct iovec *iovec, unsigned n_iovec) {
>> +        struct msghdr mh = {
>> +                .msg_iov = iovec,
>> +                .msg_iovlen = n_iovec,
>> +        };
>> +
>> +        assert(m);
>> +        assert(iovec);
>> +        assert(n_iovec > 0);
>> +
>> +        if (m->address.sockaddr.sa.sa_family == AF_INET) {
>> +                mh.msg_name = &m->address.sockaddr.sa;
>> +                mh.msg_namelen = sizeof(m->address.sockaddr.sa);
>
> This actually looks wrong? This should be
> sizeof(m->address.sockaddr.in)?
>
>> +        } else if (m->address.sockaddr.sa.sa_family == AF_INET6) {
>> +                mh.msg_name = &m->address.sockaddr.in6;
>
> And this doesn't look right either, this should be
> &m->address.sockaddr.sa, otherwise this should generate a type error,
> no?
>

Will fix these in the new version . thanks !
> Lennart
>

Susant