On Fri, Apr 10, 2015 at 12:45:34PM +0200, Lennart Poettering wrote:
> On Thu, 09.04.15 23:43, Susant Sahani (sus...@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?
Yeah, I have to agree very much here: "journal-syslogd" name is too similar
to "syslogd", the original syslog daemon, and is bound to cause endless
confusion.
Maybe we should go for journal-netlogd, reminiscent of netconsole,
which is actually a similar thing. s-j-s-emitd is a bit too long.
I see some typos in the patch... I'll reply separately.
Zbyszek
>
> 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?
>
> > + 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?
>
> Lennart
>
> --
> Lennart Poettering, Red Hat
>
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
