On Tue, 17.12.13 18:02, Dr. Werner Fink ([email protected]) 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 _______________________________________________ systemd-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/systemd-devel
