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

[Dr. Werner Fink]

On Tue, Dec 10, 2013 at 09:53:22PM +0100, Lennart Poettering wrote:

Sorry for the late answer but I had been away sick last week ...

> On Mon, 02.12.13 16:27, Werner Fink (werner at suse.de) 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 <werner at suse.de>
> 
> We don't use "S-o-b" in systemd...

OK

[...]

> > +
> > +        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.


Werner

-- 
  "Having a smoking section in a restaurant is like having
          a peeing section in a swimming pool." -- Edward Burr
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/systemd-devel/attachments/20131217/46d788ef/attachment.pgp>