On Mon, 02.12.13 16:27, Werner Fink ([email protected]) wrote:
> that is the systemd-journald may ignore /dec/kmsg which are not a valid
> device but a lint to /dev/null as done in LinuX Containers (LXC). This
> change adds straight forward a check for /dev/kmsg.
>
> Signed-off-by: Werner Fink <[email protected]>
We don't use "S-o-b" in systemd...
> #include <systemd/sd-messages.h>
> #include <libudev.h>
> @@ -377,12 +379,32 @@ int server_flush_dev_kmsg(Server *s) {
>
> int server_open_dev_kmsg(Server *s) {
> struct epoll_event ev;
> + struct stat st;
>
> assert(s);
>
> s->dev_kmsg_fd = open("/dev/kmsg",
> O_RDWR|O_CLOEXEC|O_NONBLOCK|O_NOCTTY);
> if (s->dev_kmsg_fd < 0) {
> - log_warning("Failed to open /dev/kmsg, ignoring: %m");
> + /* Do not warn as it may not exists in LXC environments */
> + if (errno != ENOENT)
> + log_warning("Failed to open /dev/kmsg, ignoring:
> %m");
> + return 0;
> + }
This part makes sense though I'd prefer if we'd just downgrade the
message here on ENOENT, not completely get rid of.
log_full(errno == ENOENT ? LOG_DEBUG : LOG_WARNING, ...
or something like that...
> +
> + 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.
> return 0;
> }
>
> @@ -391,6 +413,9 @@ int server_open_dev_kmsg(Server *s) {
> ev.data.fd = s->dev_kmsg_fd;
> if (epoll_ctl(s->epoll_fd, EPOLL_CTL_ADD, s->dev_kmsg_fd, &ev) < 0) {
>
> + close_nointr_nofail(s->dev_kmsg_fd);
> + s->dev_kmsg_fd = -1;
This part looks OK.
> +
> /* This will fail with EPERM on older kernels where
> * /dev/kmsg is not readable. */
> if (errno == EPERM)
Could you rework your patch to just inclide the first bit (downgradingof
the log message to LOG_DEBUG if errno == ENOENT) and the last bit? I'd
merge it then.
Lennart
--
Lennart Poettering, Red Hat
_______________________________________________
systemd-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
