[systemd-commits] 3 commits - TODO src/core src/journal src/shared src/udev

Lennart Poettering lennart at kemper.freedesktop.org
Tue Dec 9 16:48:45 PST 2014


 TODO                     |    4 +--
 src/core/mount.c         |   58 +++++++++++++++++++++++++++++------------------
 src/core/path.c          |   32 ++++++-------------------
 src/journal/sd-journal.c |   14 +----------
 src/shared/util.c        |   19 +++------------
 src/shared/util.h        |    7 +++++
 src/udev/udevd.c         |   33 ++++++++++----------------
 7 files changed, 73 insertions(+), 94 deletions(-)

New commits:
commit b7307642391c8ebb9724c99e6b33239e2c0ff944
Author: Lennart Poettering <lennart at poettering.net>
Date:   Wed Dec 10 01:47:59 2014 +0100

    mount: clarify that we really need to replace the utab inotify code with the native API for this in libmount, as soon as that's stable

diff --git a/TODO b/TODO
index a09451d..23fb68c 100644
--- a/TODO
+++ b/TODO
@@ -1,7 +1,5 @@
 Preparations for 218:
 
-* port libmount hookup to use API's own inotify interface
-
 * cgroup delegation issues
 
 * should networkd's [BridgePort] section really be called like that?
@@ -43,6 +41,8 @@ External:
 
 Features:
 
+* port libmount hookup to use API's own inotify interface, as soon as that is table in libmount
+
 * bash completion for busctl, to make it truly useful
 
 * journald: broken file systems are real (btrfs), we need to handle
diff --git a/src/core/mount.c b/src/core/mount.c
index 66de85b..6b415b4 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -1665,6 +1665,10 @@ static int mount_dispatch_io(sd_event_source *source, int fd, uint32_t revents,
         if (fd == m->utab_inotify_fd) {
                 bool rescan = false;
 
+                /* FIXME: We *really* need to replace this with
+                 * libmount's own API for this, we should not hardcode
+                 * internal behaviour of libmount here. */
+
                 for (;;) {
                         uint8_t buffer[INOTIFY_EVENT_MAX] _alignas_(struct inotify_event);
                         struct inotify_event *e;

commit f7c1ad4fd4190bee32db0aa26c8e9fe7e19d8816
Author: Lennart Poettering <lennart at poettering.net>
Date:   Wed Dec 10 01:45:43 2014 +0100

    core: unify how we iterate over inotify events
    
    Let's add some syntactic sugar for iterating through inotify events, and
    use it everywhere.

diff --git a/src/core/mount.c b/src/core/mount.c
index 5f268c6..66de85b 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -1616,14 +1616,18 @@ static int mount_enumerate(Manager *m) {
 
         if (m->utab_inotify_fd < 0) {
                 m->utab_inotify_fd = inotify_init1(IN_NONBLOCK|IN_CLOEXEC);
-                if (m->utab_inotify_fd < 0)
-                        goto fail_with_errno;
+                if (m->utab_inotify_fd < 0) {
+                        r = -errno;
+                        goto fail;
+                }
 
                 (void) mkdir_p_label("/run/mount", 0755);
 
                 r = inotify_add_watch(m->utab_inotify_fd, "/run/mount", IN_MOVED_TO);
-                if (r < 0)
-                        goto fail_with_errno;
+                if (r < 0) {
+                        r = -errno;
+                        goto fail;
+                }
 
                 r = sd_event_add_io(m->event, &m->mount_utab_event_source, m->utab_inotify_fd, EPOLLIN, mount_dispatch_io, m);
                 if (r < 0)
@@ -1640,8 +1644,6 @@ static int mount_enumerate(Manager *m) {
 
         return 0;
 
-fail_with_errno:
-        r = -errno;
 fail:
         mount_shutdown(m);
         return r;
@@ -1661,22 +1663,32 @@ static int mount_dispatch_io(sd_event_source *source, int fd, uint32_t revents,
          * for mount options. */
 
         if (fd == m->utab_inotify_fd) {
-                char inotify_buffer[sizeof(struct inotify_event) + NAME_MAX + 1];
-                struct inotify_event *event;
-                char *p;
                 bool rescan = false;
 
-                while ((r = read(fd, inotify_buffer, sizeof(inotify_buffer))) > 0)
-                        for (p = inotify_buffer; p < inotify_buffer + r; ) {
-                                event = (struct inotify_event *) p;
-                                /* only care about changes to utab, but we have
-                                 * to monitor the directory to reliably get
-                                 * notifications about when utab is replaced
-                                 * using rename(2) */
-                                if ((event->mask & IN_Q_OVERFLOW) || streq(event->name, "utab"))
+                for (;;) {
+                        uint8_t buffer[INOTIFY_EVENT_MAX] _alignas_(struct inotify_event);
+                        struct inotify_event *e;
+                        ssize_t l;
+
+                        l = read(fd, buffer, sizeof(buffer));
+                        if (l < 0) {
+                                if (errno == EAGAIN || errno == EINTR)
+                                        break;
+
+                                log_error_errno(errno, "Failed to read utab inotify: %m");
+                                break;
+                        }
+
+                        FOREACH_INOTIFY_EVENT(e, buffer, l) {
+                                /* Only care about changes to utab,
+                                 * but we have to monitor the
+                                 * directory to reliably get
+                                 * notifications about when utab is
+                                 * replaced using rename(2) */
+                                if ((e->mask & IN_Q_OVERFLOW) || streq(e->name, "utab"))
                                         rescan = true;
-                                p += sizeof(struct inotify_event) + event->len;
                         }
+                }
 
                 if (!rescan)
                         return 0;
diff --git a/src/core/path.c b/src/core/path.c
index 3624bfc..656ed69 100644
--- a/src/core/path.c
+++ b/src/core/path.c
@@ -157,10 +157,9 @@ void path_spec_unwatch(PathSpec *s) {
 }
 
 int path_spec_fd_event(PathSpec *s, uint32_t revents) {
-        _cleanup_free_ uint8_t *buf = NULL;
+        uint8_t buffer[INOTIFY_EVENT_MAX] _alignas_(struct inotify_event);
         struct inotify_event *e;
-        ssize_t k;
-        int l;
+        ssize_t l;
         int r = 0;
 
         if (revents != EPOLLIN) {
@@ -168,33 +167,18 @@ int path_spec_fd_event(PathSpec *s, uint32_t revents) {
                 return -EINVAL;
         }
 
-        if (ioctl(s->inotify_fd, FIONREAD, &l) < 0)
-                return log_error_errno(errno, "FIONREAD failed: %m");
+        l = read(s->inotify_fd, buffer, sizeof(buffer));
+        if (l < 0) {
+                if (errno == EAGAIN || errno == EINTR)
+                        return 0;
 
-        assert(l > 0);
-
-        buf = malloc(l);
-        if (!buf)
-                return log_oom();
-
-        k = read(s->inotify_fd, buf, l);
-        if (k < 0)
                 return log_error_errno(errno, "Failed to read inotify event: %m");
+        }
 
-        e = (struct inotify_event*) buf;
-
-        while (k > 0) {
-                size_t step;
-
+        FOREACH_INOTIFY_EVENT(e, buffer, l) {
                 if ((s->type == PATH_CHANGED || s->type == PATH_MODIFIED) &&
                     s->primary_wd == e->wd)
                         r = 1;
-
-                step = sizeof(struct inotify_event) + e->len;
-                assert(step <= (size_t) k);
-
-                e = (struct inotify_event*) ((uint8_t*) e + step);
-                k -= step;
         }
 
         return r;
diff --git a/src/journal/sd-journal.c b/src/journal/sd-journal.c
index 5e2da99..23aad74 100644
--- a/src/journal/sd-journal.c
+++ b/src/journal/sd-journal.c
@@ -2320,7 +2320,6 @@ static int determine_change(sd_journal *j) {
 }
 
 _public_ int sd_journal_process(sd_journal *j) {
-        uint8_t buffer[sizeof(struct inotify_event) + FILENAME_MAX] _alignas_(struct inotify_event);
         bool got_something = false;
 
         assert_return(j, -EINVAL);
@@ -2329,6 +2328,7 @@ _public_ int sd_journal_process(sd_journal *j) {
         j->last_process_usec = now(CLOCK_MONOTONIC);
 
         for (;;) {
+                uint8_t buffer[INOTIFY_EVENT_MAX] _alignas_(struct inotify_event);
                 struct inotify_event *e;
                 ssize_t l;
 
@@ -2342,18 +2342,8 @@ _public_ int sd_journal_process(sd_journal *j) {
 
                 got_something = true;
 
-                e = (struct inotify_event*) buffer;
-                while (l > 0) {
-                        size_t step;
-
+                FOREACH_INOTIFY_EVENT(e, buffer, l)
                         process_inotify_event(j, e);
-
-                        step = sizeof(struct inotify_event) + e->len;
-                        assert(step <= (size_t) l);
-
-                        e = (struct inotify_event*) ((uint8_t*) e + step);
-                        l -= step;
-                }
         }
 }
 
diff --git a/src/shared/util.c b/src/shared/util.c
index 8c1cf52..ff8835b 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -2100,9 +2100,9 @@ int acquire_terminal(
                 assert(notify >= 0);
 
                 for (;;) {
-                        uint8_t inotify_buffer[sizeof(struct inotify_event) + FILENAME_MAX];
-                        ssize_t l;
+                        uint8_t buffer[INOTIFY_EVENT_MAX] _alignas_(struct inotify_event);
                         struct inotify_event *e;
+                        ssize_t l;
 
                         if (timeout != USEC_INFINITY) {
                                 usec_t n;
@@ -2123,9 +2123,8 @@ int acquire_terminal(
                                 }
                         }
 
-                        l = read(notify, inotify_buffer, sizeof(inotify_buffer));
+                        l = read(notify, buffer, sizeof(buffer));
                         if (l < 0) {
-
                                 if (errno == EINTR || errno == EAGAIN)
                                         continue;
 
@@ -2133,21 +2132,11 @@ int acquire_terminal(
                                 goto fail;
                         }
 
-                        e = (struct inotify_event*) inotify_buffer;
-
-                        while (l > 0) {
-                                size_t step;
-
+                        FOREACH_INOTIFY_EVENT(e, buffer, l) {
                                 if (e->wd != wd || !(e->mask & IN_CLOSE)) {
                                         r = -EIO;
                                         goto fail;
                                 }
-
-                                step = sizeof(struct inotify_event) + e->len;
-                                assert(step <= (size_t) l);
-
-                                e = (struct inotify_event*) ((uint8_t*) e + step);
-                                l -= step;
                         }
 
                         break;
diff --git a/src/shared/util.h b/src/shared/util.h
index b6fdf83..61094cc 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -1031,3 +1031,10 @@ int unquote_many_words(const char **p, ...) _sentinel_;
 int free_and_strdup(char **p, const char *s);
 
 int sethostname_idempotent(const char *s);
+
+#define INOTIFY_EVENT_MAX (sizeof(struct inotify_event) + NAME_MAX + 1)
+
+#define FOREACH_INOTIFY_EVENT(e, buffer, sz) \
+        for ((e) = (struct inotify_event*) (buffer);    \
+             (uint8_t*) (e) < (uint8_t*) (buffer) + (sz); \
+             (e) = (struct inotify_event*) ((uint8_t*) (e) + sizeof(struct inotify_event) + (e)->len))
diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index 6a8dda3..8bec03e 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -816,41 +816,34 @@ static int synthesize_change(struct udev_device *dev) {
 }
 
 static int handle_inotify(struct udev *udev) {
-        int nbytes, pos;
-        char *buf;
-        struct inotify_event *ev;
-        int r;
+        uint8_t buffer[INOTIFY_EVENT_MAX] _alignas_(struct inotify_event);
+        struct inotify_event *e;
+        ssize_t l;
 
-        r = ioctl(fd_inotify, FIONREAD, &nbytes);
-        if (r < 0 || nbytes <= 0)
-                return -errno;
+        l = read(fd_inotify, buffer, sizeof(buffer));
+        if (l < 0) {
+                if (errno == EAGAIN || errno == EINTR)
+                        return 0;
 
-        buf = malloc(nbytes);
-        if (!buf) {
-                log_error("error getting buffer for inotify");
-                return -ENOMEM;
+                return log_error_errno(errno, "Failed to read inotify fd: %m");
         }
 
-        nbytes = read(fd_inotify, buf, nbytes);
-
-        for (pos = 0; pos < nbytes; pos += sizeof(struct inotify_event) + ev->len) {
+        FOREACH_INOTIFY_EVENT(e, buffer, l) {
                 struct udev_device *dev;
 
-                ev = (struct inotify_event *)(buf + pos);
-                dev = udev_watch_lookup(udev, ev->wd);
+                dev = udev_watch_lookup(udev, e->wd);
                 if (!dev)
                         continue;
 
-                log_debug("inotify event: %x for %s", ev->mask, udev_device_get_devnode(dev));
-                if (ev->mask & IN_CLOSE_WRITE)
+                log_debug("inotify event: %x for %s", e->mask, udev_device_get_devnode(dev));
+                if (e->mask & IN_CLOSE_WRITE)
                         synthesize_change(dev);
-                else if (ev->mask & IN_IGNORED)
+                else if (e->mask & IN_IGNORED)
                         udev_watch_end(udev, dev);
 
                 udev_device_unref(dev);
         }
 
-        free(buf);
         return 0;
 }
 

commit df63dda6d4b4fc90f895cfd40d54e15928671624
Author: Lennart Poettering <lennart at poettering.net>
Date:   Wed Dec 10 00:52:56 2014 +0100

    mount: use bools where appropriate

diff --git a/src/core/mount.c b/src/core/mount.c
index a390768..5f268c6 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -1664,7 +1664,7 @@ static int mount_dispatch_io(sd_event_source *source, int fd, uint32_t revents,
                 char inotify_buffer[sizeof(struct inotify_event) + NAME_MAX + 1];
                 struct inotify_event *event;
                 char *p;
-                int rescan = 0;
+                bool rescan = false;
 
                 while ((r = read(fd, inotify_buffer, sizeof(inotify_buffer))) > 0)
                         for (p = inotify_buffer; p < inotify_buffer + r; ) {
@@ -1674,7 +1674,7 @@ static int mount_dispatch_io(sd_event_source *source, int fd, uint32_t revents,
                                  * notifications about when utab is replaced
                                  * using rename(2) */
                                 if ((event->mask & IN_Q_OVERFLOW) || streq(event->name, "utab"))
-                                        rescan = 1;
+                                        rescan = true;
                                 p += sizeof(struct inotify_event) + event->len;
                         }
 



More information about the systemd-commits mailing list