[systemd-devel] [PATCH] Avoid a busy systemd-journald in LXC environments

[Lennart Poettering]

On Tue, 17.12.13 18:02, Dr. Werner Fink (werner at suse.de) wrote:

> > > +
> > > +        if (fstat(s->dev_kmsg_fd, &st) < 0) {
> > > +                log_error("Failed to stat /dev/kmsg fd, ignoring: %m");
> > > +                close_nointr_nofail(s->dev_kmsg_fd);
> > > +                s->dev_kmsg_fd = -1;
> > > +                return 0;
> > > +        }
> > > +
> > > +        if (!S_ISCHR(st.st_mode) || major(st.st_rdev) != 1 || minor(st.st_rdev) != 11) {
> > > +                int old_errno = errno;
> > > +                errno = ENODEV;
> > > +                log_warning("Irregular device /dev/kmsg, ignoring: %m");
> > > +                errno = old_errno;
> > > +                close_nointr_nofail(s->dev_kmsg_fd);
> > > +                s->dev_kmsg_fd = -1;
> > 
> > I am really not convinced by this. LXC should either not set up
> > /dev/kmsg, or should do it in a way that doesn't create problems with
> > what userspace expects. I am pretty sure the onus here is on LXC to
> > provide something that is compatible or nothing at all...
> > 
> > Also, we do not hardcode major/minor pairs. Ever.
> 
> Would be something like
> 
> +        if (fstat(s->dev_kmsg_fd, &st) < 0) {
> +                log_error("Failed to stat /dev/kmsg fd, ignoring: %m");
> +                close_nointr_nofail(s->dev_kmsg_fd);
> +                s->dev_kmsg_fd = -1;
> +                return 0;
> +        }
> +
> +        if (!S_ISCHR(st.st_mode) || !(ud = udev_device_new_from_devnum(s->udev, 'c', st.st_rdev))) {
> +                int old_errno = errno;
> +                errno = ENODEV;
> +                log_warning("Irregular device /dev/kmsg, ignoring: %m");
> +                errno = old_errno;
> +                close_nointr_nofail(s->dev_kmsg_fd);
> +                s->dev_kmsg_fd = -1;
> +                return 0;
> +        }
> +
> +        if (strcmp("/dev/kmsg", udev_device_get_devnode(ud)) != 0) {
> +                int old_errno = errno;
> +                errno = EPFNOSUPPORT;
> +                log_warning("Irregular device /dev/kmsg, ignoring: %m");
> +                errno = old_errno;
> +                close_nointr_nofail(s->dev_kmsg_fd);
> +                s->dev_kmsg_fd = -1;
> +                return 0;
> +        }
> +
> +        udev_device_unref(ud);
> 
> OK for you? I've that tested yet but before doing this I'd like to know if this
> would be accepted.

But why? LXC should either provide a working /dev/kmsg or none at
all. Providing something that doesn't work where we then have to check
with a lot of code if they are playing games with us or not is not an
option really, sorry.

Anyway, I suggested in my original reply that I'd be happy to merge a
patch that downgrades the warning message to debug on ENOENT. I have now
made such a change in git, and also added another change that closes
/dev/kmsg if we cannot make use of it anyway.

(Also, log_warning() and friends save/restore errno internally. And
instead of repeating destruction paths in all if branches is something
we don't do. Use "goto" to some unified destruction path at the end of
the call. And running non-trivial functions from if checks is also not
that a good idea).

Anyway, with the code now in git, is there still something left to do to
make journald work nicely with a properly set up LXC for you?

Lennart

-- 
Lennart Poettering, Red Hat