[systemd-commits] 5 commits - src/core src/udev

Tom Gundersen tomegun at kemper.freedesktop.org
Tue May 12 08:35:42 PDT 2015


 src/core/manager.c |    4 
 src/core/service.c |   19 ---
 src/udev/udevd.c   |  259 ++++++++++++++++++++++++++++-------------------------
 3 files changed, 143 insertions(+), 139 deletions(-)

New commits:
commit 34959677900a45aa67f511e828c68167f8a7c816
Author: Tom Gundersen <teg at jklm.no>
Date:   Tue May 12 17:21:51 2015 +0200

    core: drop redundant logging about notification messages
    
    Before:
    May 12 17:11:22 tomegun-x2402 systemd[1]: systemd-udevd.service: Got notification message for unit.
    May 12 17:11:22 tomegun-x2402 systemd[1]: systemd-udevd.service: Got notification message from PID 195 (READY=1)
    May 12 17:11:22 tomegun-x2402 systemd[1]: systemd-udevd.service: Ggot READY=1
    
    After:
    May 12 17:11:22 tomegun-x2402 systemd[1]: systemd-udevd.service: Got notification message from PID 195 (READY=1)

diff --git a/src/core/manager.c b/src/core/manager.c
index 28b9427..e547f71 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1496,10 +1496,10 @@ static void manager_invoke_notify_message(Manager *m, Unit *u, pid_t pid, char *
                 return;
         }
 
-        log_unit_debug(u, "Got notification message for unit.");
-
         if (UNIT_VTABLE(u)->notify_message)
                 UNIT_VTABLE(u)->notify_message(u, pid, tags, fds);
+        else
+                log_unit_debug(u, "Got notification message for unit. Ignoring.");
 }
 
 static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t revents, void *userdata) {
diff --git a/src/core/service.c b/src/core/service.c
index 0842ef1..07347b9 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -2818,20 +2818,18 @@ static void service_notify_message(Unit *u, pid_t pid, char **tags, FDSet *fds)
         assert(u);
 
         cc = strv_join(tags, ", ");
-        log_unit_debug(u, "Got notification message from PID "PID_FMT" (%s)", pid, isempty(cc) ? "n/a" : cc);
 
         if (s->notify_access == NOTIFY_NONE) {
                 log_unit_warning(u, "Got notification message from PID "PID_FMT", but reception is disabled.", pid);
                 return;
-        }
-
-        if (s->notify_access == NOTIFY_MAIN && pid != s->main_pid) {
+        } else if (s->notify_access == NOTIFY_MAIN && pid != s->main_pid) {
                 if (s->main_pid != 0)
                         log_unit_warning(u, "Got notification message from PID "PID_FMT", but reception only permitted for main PID "PID_FMT, pid, s->main_pid);
                 else
                         log_unit_debug(u, "Got notification message from PID "PID_FMT", but reception only permitted for main PID which is currently not known", pid);
                 return;
-        }
+        } else
+                log_unit_debug(u, "Got notification message from PID "PID_FMT" (%s)", pid, isempty(cc) ? "n/a" : cc);
 
         /* Interpret MAINPID= */
         e = strv_find_startswith(tags, "MAINPID=");
@@ -2839,8 +2837,6 @@ static void service_notify_message(Unit *u, pid_t pid, char **tags, FDSet *fds)
                 if (parse_pid(e, &pid) < 0)
                         log_unit_warning(u, "Failed to parse MAINPID= field in notification message: %s", e);
                 else {
-                        log_unit_debug(u, "Got MAINPID=%s", e);
-
                         service_set_main_pid(s, pid);
                         unit_watch_pid(UNIT(s), pid);
                         notify_dbus = true;
@@ -2850,7 +2846,6 @@ static void service_notify_message(Unit *u, pid_t pid, char **tags, FDSet *fds)
         /* Interpret RELOADING= */
         if (strv_find(tags, "RELOADING=1")) {
 
-                log_unit_debug(u, "Got RELOADING=1");
                 s->notify_state = NOTIFY_RELOADING;
 
                 if (s->state == SERVICE_RUNNING)
@@ -2862,7 +2857,6 @@ static void service_notify_message(Unit *u, pid_t pid, char **tags, FDSet *fds)
         /* Interpret READY= */
         if (strv_find(tags, "READY=1")) {
 
-                log_unit_debug(u, "Ggot READY=1");
                 s->notify_state = NOTIFY_READY;
 
                 /* Type=notify services inform us about completed
@@ -2881,7 +2875,6 @@ static void service_notify_message(Unit *u, pid_t pid, char **tags, FDSet *fds)
         /* Interpret STOPPING= */
         if (strv_find(tags, "STOPPING=1")) {
 
-                log_unit_debug(u, "Got STOPPING=1");
                 s->notify_state = NOTIFY_STOPPING;
 
                 if (s->state == SERVICE_RUNNING)
@@ -2899,8 +2892,6 @@ static void service_notify_message(Unit *u, pid_t pid, char **tags, FDSet *fds)
                         if (!utf8_is_valid(e))
                                 log_unit_warning(u, "Status message in notification message is not UTF-8 clean.");
                         else {
-                                log_unit_debug(u, "Got STATUS=%s", e);
-
                                 t = strdup(e);
                                 if (!t)
                                         log_oom();
@@ -2925,8 +2916,6 @@ static void service_notify_message(Unit *u, pid_t pid, char **tags, FDSet *fds)
                 if (safe_atoi(e, &status_errno) < 0 || status_errno < 0)
                         log_unit_warning(u, "Failed to parse ERRNO= field in notification message: %s", e);
                 else {
-                        log_unit_debug(u, "Got ERRNO=%s", e);
-
                         if (s->status_errno != status_errno) {
                                 s->status_errno = status_errno;
                                 notify_dbus = true;
@@ -2936,13 +2925,11 @@ static void service_notify_message(Unit *u, pid_t pid, char **tags, FDSet *fds)
 
         /* Interpret WATCHDOG= */
         if (strv_find(tags, "WATCHDOG=1")) {
-                log_unit_debug(u, "Got WATCHDOG=1");
                 service_reset_watchdog(s);
         }
 
         /* Add the passed fds to the fd store */
         if (strv_find(tags, "FDSTORE=1")) {
-                log_unit_debug(u, "Got FDSTORE=1");
                 service_add_fd_store_set(s, fds);
         }
 

commit 005e945cc454c07fac381d59a6e0fe810eb1107b
Author: Tom Gundersen <teg at jklm.no>
Date:   Tue May 12 16:57:01 2015 +0200

    udevd: remove stale comment

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index e558098..1443a0d 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -1584,10 +1584,6 @@ int main(int argc, char *argv[]) {
                  * This needs to be after the inotify handling, to make sure,
                  * that the ping is send back after the possibly generated
                  * "change" events by the inotify device node watch.
-                 *
-                 * A single time we may receive a client connection which we need to
-                 * keep open to block the client. It will be closed right before we
-                 * exit.
                  */
                 if (is_ctrl)
                         on_ctrl_msg(NULL, fd_ctrl, 0, udev_ctrl);

commit 799a108c074bb3d6b4b44a9f2b69342cd60bb137
Author: Tom Gundersen <teg at jklm.no>
Date:   Tue May 12 16:55:29 2015 +0200

    udevd: explicitly update queue file before answering to ping
    
    This avoids updating the flag files twice for every loop, and also removes another dependency
    in the main-loop, so we are freer to reshufle it as we want.

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index 8528f71..e558098 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -696,6 +696,20 @@ static int on_uevent(sd_event_source *s, int fd, uint32_t revents, void *userdat
         return 1;
 }
 
+static void event_queue_update(void) {
+        int r;
+
+        if (!udev_list_node_is_empty(&event_list)) {
+                r = touch("/run/udev/queue");
+                if (r < 0)
+                        log_warning_errno(r, "could not touch /run/udev/queue: %m");
+        } else {
+                r = unlink("/run/udev/queue");
+                if (r < 0 && errno != ENOENT)
+                        log_warning("could not unlink /run/udev/queue: %m");
+        }
+}
+
 /* receive the udevd message from userspace */
 static int on_ctrl_msg(sd_event_source *s, int fd, uint32_t revents, void *userdata) {
         struct udev_ctrl *uctrl = userdata;
@@ -769,8 +783,13 @@ static int on_ctrl_msg(sd_event_source *s, int fd, uint32_t revents, void *userd
                 arg_children_max = i;
         }
 
-        if (udev_ctrl_get_ping(ctrl_msg) > 0)
+        if (udev_ctrl_get_ping(ctrl_msg) > 0) {
                 log_debug("udevd message (SYNC) received");
+                /* tell settle that we are busy or idle, this needs to be before the
+                 * PING handling
+                 */
+                event_queue_update();
+        }
 
         if (udev_ctrl_get_exit(ctrl_msg) > 0) {
                 log_debug("udevd message (EXIT) received");
@@ -987,20 +1006,6 @@ static int on_sigchld(sd_event_source *s, const struct signalfd_siginfo *si, voi
         return 1;
 }
 
-static void event_queue_update(void) {
-        int r;
-
-        if (!udev_list_node_is_empty(&event_list)) {
-                r = touch("/run/udev/queue");
-                if (r < 0)
-                        log_warning_errno(r, "could not touch /run/udev/queue: %m");
-        } else {
-                r = unlink("/run/udev/queue");
-                if (r < 0 && errno != ENOENT)
-                        log_warning("could not unlink /run/udev/queue: %m");
-        }
-}
-
 static int systemd_fds(struct udev *udev, int *rctrl, int *rnetlink) {
         int ctrl = -1, netlink = -1;
         int fd, n;
@@ -1575,11 +1580,6 @@ int main(int argc, char *argv[]) {
                 if (is_inotify)
                         on_inotify(NULL, fd_inotify, 0, udev);
 
-                /* tell settle that we are busy or idle, this needs to be before the
-                 * PING handling
-                 */
-                event_queue_update();
-
                 /*
                  * This needs to be after the inotify handling, to make sure,
                  * that the ping is send back after the possibly generated

commit a8389097c0cd91720424b25ee5c6825b4f69cb6a
Author: Tom Gundersen <teg at jklm.no>
Date:   Tue May 12 16:51:31 2015 +0200

    udevd: explicitly read out uevents we create ourselves
    
    Rather than skippling ctrl handling whenever we have handlede inotify events
    (and hence may have synthesized a 'change' event), just call the uevent
    handling explicitly from on_inotify() so that the event queue is up-to-date.

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index 65f4fe6..8528f71 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -910,9 +910,16 @@ static int on_inotify(sd_event_source *s, int fd, uint32_t revents, void *userda
                         continue;
 
                 log_debug("inotify event: %x for %s", e->mask, udev_device_get_devnode(dev));
-                if (e->mask & IN_CLOSE_WRITE)
+                if (e->mask & IN_CLOSE_WRITE) {
                         synthesize_change(dev);
-                else if (e->mask & IN_IGNORED)
+
+                        /* settle might be waiting on us to determine the queue
+                         * state. If we just handled an inotify event, we might have
+                         * generated a "change" event, but we won't have queued up
+                         * the resultant uevent yet. Do that.
+                         */
+                        on_uevent(NULL, -1, 0, monitor);
+                } else if (e->mask & IN_IGNORED)
                         udev_watch_end(udev, dev);
         }
 
@@ -1565,22 +1572,9 @@ int main(int argc, char *argv[]) {
                         continue;
 
                 /* device node watch */
-                if (is_inotify) {
+                if (is_inotify)
                         on_inotify(NULL, fd_inotify, 0, udev);
 
-                        /*
-                         * settle might be waiting on us to determine the queue
-                         * state. If we just handled an inotify event, we might have
-                         * generated a "change" event, but we won't have queued up
-                         * the resultant uevent yet.
-                         *
-                         * Before we go ahead and potentially tell settle that the
-                         * queue is empty, lets loop one more time to update the
-                         * queue state again before deciding.
-                         */
-                        continue;
-                }
-
                 /* tell settle that we are busy or idle, this needs to be before the
                  * PING handling
                  */

commit e82e8fa5b19243177e178ed78c2480505eda5e03
Author: Tom Gundersen <teg at jklm.no>
Date:   Tue May 12 14:54:52 2015 +0200

    udevd: move to sd-event-style event handlers

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index 27d8b12..65f4fe6 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -616,7 +616,7 @@ static void event_queue_cleanup(struct udev *udev, enum event_state match_type)
         }
 }
 
-static void worker_returned(int fd_worker) {
+static int on_worker(sd_event_source *s, int fd, uint32_t revents, void *userdata) {
         for (;;) {
                 struct worker_message msg;
                 struct iovec iovec = {
@@ -638,16 +638,15 @@ static void worker_returned(int fd_worker) {
                 struct ucred *ucred = NULL;
                 struct worker *worker;
 
-                size = recvmsg(fd_worker, &msghdr, MSG_DONTWAIT);
+                size = recvmsg(fd, &msghdr, MSG_DONTWAIT);
                 if (size < 0) {
                         if (errno == EAGAIN || errno == EINTR)
-                                return;
+                                return 1;
 
-                        log_error_errno(errno, "failed to receive message: %m");
-                        return;
+                        return log_error_errno(errno, "failed to receive message: %m");
                 } else if (size != sizeof(struct worker_message)) {
                         log_warning_errno(EIO, "ignoring worker message with invalid size %zi bytes", size);
-                        return;
+                        continue;
                 }
 
                 for (cmsg = CMSG_FIRSTHDR(&msghdr); cmsg; cmsg = CMSG_NXTHDR(&msghdr, cmsg)) {
@@ -675,10 +674,31 @@ static void worker_returned(int fd_worker) {
                 /* worker returned */
                 event_free(worker->event);
         }
+
+        return 1;
+}
+
+static int on_uevent(sd_event_source *s, int fd, uint32_t revents, void *userdata) {
+        struct udev_monitor *m = userdata;
+        struct udev_device *dev;
+        int r;
+
+        assert(m);
+
+        dev = udev_monitor_receive_device(m);
+        if (dev) {
+                udev_device_ensure_usec_initialized(dev, NULL);
+                r = event_queue_insert(dev);
+                if (r < 0)
+                        udev_device_unref(dev);
+        }
+
+        return 1;
 }
 
 /* receive the udevd message from userspace */
-static void handle_ctrl_msg(struct udev_ctrl *uctrl) {
+static int on_ctrl_msg(sd_event_source *s, int fd, uint32_t revents, void *userdata) {
+        struct udev_ctrl *uctrl = userdata;
         _cleanup_udev_ctrl_connection_unref_ struct udev_ctrl_connection *ctrl_conn = NULL;
         _cleanup_udev_ctrl_msg_unref_ struct udev_ctrl_msg *ctrl_msg = NULL;
         const char *str;
@@ -688,11 +708,11 @@ static void handle_ctrl_msg(struct udev_ctrl *uctrl) {
 
         ctrl_conn = udev_ctrl_get_connection(uctrl);
         if (!ctrl_conn)
-                return;
+                return 1;
 
         ctrl_msg = udev_ctrl_receive_msg(ctrl_conn);
         if (!ctrl_msg)
-                return;
+                return 1;
 
         i = udev_ctrl_get_set_log_level(ctrl_msg);
         if (i >= 0) {
@@ -759,7 +779,7 @@ static void handle_ctrl_msg(struct udev_ctrl *uctrl) {
                 udev_ctrl_conn = udev_ctrl_connection_ref(ctrl_conn);
         }
 
-        return;
+        return 1;
 }
 
 static int synthesize_change(struct udev_device *dev) {
@@ -866,21 +886,24 @@ static int synthesize_change(struct udev_device *dev) {
         return 0;
 }
 
-static int handle_inotify(struct udev *udev) {
+static int on_inotify(sd_event_source *s, int fd, uint32_t revents, void *userdata) {
+        struct udev *udev = userdata;
         union inotify_event_buffer buffer;
         struct inotify_event *e;
         ssize_t l;
 
-        l = read(fd_inotify, &buffer, sizeof(buffer));
+        assert(udev);
+
+        l = read(fd, &buffer, sizeof(buffer));
         if (l < 0) {
                 if (errno == EAGAIN || errno == EINTR)
-                        return 0;
+                        return 1;
 
                 return log_error_errno(errno, "Failed to read inotify fd: %m");
         }
 
         FOREACH_INOTIFY_EVENT(e, buffer, l) {
-                struct udev_device *dev;
+                _cleanup_udev_device_unref_ struct udev_device *dev = NULL;
 
                 dev = udev_watch_lookup(udev, e->wd);
                 if (!dev)
@@ -891,71 +914,70 @@ static int handle_inotify(struct udev *udev) {
                         synthesize_change(dev);
                 else if (e->mask & IN_IGNORED)
                         udev_watch_end(udev, dev);
-
-                udev_device_unref(dev);
         }
 
-        return 0;
+        return 1;
 }
 
-static void handle_signal(struct udev *udev, int signo) {
-        switch (signo) {
-        case SIGINT:
-        case SIGTERM:
-                udev_exit = true;
-                break;
-        case SIGCHLD:
-                for (;;) {
-                        pid_t pid;
-                        int status;
-                        struct worker *worker;
+static int on_request_exit(sd_event_source *s, const struct signalfd_siginfo *si, void *userdata) {
+        udev_exit = true;
 
-                        pid = waitpid(-1, &status, WNOHANG);
-                        if (pid <= 0)
-                                break;
+        return 1;
+}
 
-                        worker = hashmap_get(workers, UINT_TO_PTR(pid));
-                        if (!worker) {
-                                log_warning("worker ["PID_FMT"] is unknown, ignoring", pid);
-                                continue;
-                        }
+static int on_request_reload(sd_event_source *s, const struct signalfd_siginfo *si, void *userdata) {
+        reload = true;
 
-                        if (WIFEXITED(status)) {
-                                if (WEXITSTATUS(status) == 0)
-                                        log_debug("worker ["PID_FMT"] exited", pid);
-                                else
-                                        log_warning("worker ["PID_FMT"] exited with return code %i", pid, WEXITSTATUS(status));
-                        } else if (WIFSIGNALED(status)) {
-                                log_warning("worker ["PID_FMT"] terminated by signal %i (%s)",
-                                            pid, WTERMSIG(status), strsignal(WTERMSIG(status)));
-                        } else if (WIFSTOPPED(status)) {
-                                log_info("worker ["PID_FMT"] stopped", pid);
-                                continue;
-                        } else if (WIFCONTINUED(status)) {
-                                log_info("worker ["PID_FMT"] continued", pid);
-                                continue;
-                        } else {
-                                log_warning("worker ["PID_FMT"] exit with status 0x%04x", pid, status);
-                        }
+        return 1;
+}
 
-                        if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
-                                if (worker->event) {
-                                        log_error("worker ["PID_FMT"] failed while handling '%s'", pid, worker->event->devpath);
-                                        /* delete state from disk */
-                                        udev_device_delete_db(worker->event->dev);
-                                        udev_device_tag_index(worker->event->dev, NULL, false);
-                                        /* forward kernel event without amending it */
-                                        udev_monitor_send_device(monitor, NULL, worker->event->dev_kernel);
-                                }
-                        }
+static int on_sigchld(sd_event_source *s, const struct signalfd_siginfo *si, void *userdata) {
+        for (;;) {
+                pid_t pid;
+                int status;
+                struct worker *worker;
 
-                        worker_free(worker);
+                pid = waitpid(-1, &status, WNOHANG);
+                if (pid <= 0)
+                        return 1;
+
+                worker = hashmap_get(workers, UINT_TO_PTR(pid));
+                if (!worker) {
+                        log_warning("worker ["PID_FMT"] is unknown, ignoring", pid);
+                        return 1;
                 }
-                break;
-        case SIGHUP:
-                reload = true;
-                break;
+
+                if (WIFEXITED(status)) {
+                        if (WEXITSTATUS(status) == 0)
+                                log_debug("worker ["PID_FMT"] exited", pid);
+                        else
+                                log_warning("worker ["PID_FMT"] exited with return code %i", pid, WEXITSTATUS(status));
+                } else if (WIFSIGNALED(status)) {
+                        log_warning("worker ["PID_FMT"] terminated by signal %i (%s)", pid, WTERMSIG(status), strsignal(WTERMSIG(status)));
+                } else if (WIFSTOPPED(status)) {
+                        log_info("worker ["PID_FMT"] stopped", pid);
+                        return 1;
+                } else if (WIFCONTINUED(status)) {
+                        log_info("worker ["PID_FMT"] continued", pid);
+                        return 1;
+                } else
+                        log_warning("worker ["PID_FMT"] exit with status 0x%04x", pid, status);
+
+                if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
+                        if (worker->event) {
+                                log_error("worker ["PID_FMT"] failed while handling '%s'", pid, worker->event->devpath);
+                                /* delete state from disk */
+                                udev_device_delete_db(worker->event->dev);
+                                udev_device_tag_index(worker->event->dev, NULL, false);
+                                /* forward kernel event without amending it */
+                                udev_monitor_send_device(monitor, NULL, worker->event->dev_kernel);
+                        }
+                }
+
+                worker_free(worker);
         }
+
+        return 1;
 }
 
 static void event_queue_update(void) {
@@ -1502,18 +1524,11 @@ int main(int argc, char *argv[]) {
 
                 /* event has finished */
                 if (is_worker)
-                        worker_returned(fd_worker);
+                        on_worker(NULL, fd_worker, 0, NULL);
 
-                if (is_netlink) {
-                        struct udev_device *dev;
-
-                        dev = udev_monitor_receive_device(monitor);
-                        if (dev) {
-                                udev_device_ensure_usec_initialized(dev, NULL);
-                                if (event_queue_insert(dev) < 0)
-                                        udev_device_unref(dev);
-                        }
-                }
+                /* uevent from kernel */
+                if (is_netlink)
+                        on_uevent(NULL, fd_netlink, 0, monitor);
 
                 /* start new events */
                 if (!udev_list_node_is_empty(&event_list) && !udev_exit && !stop_exec_queue) {
@@ -1529,8 +1544,20 @@ int main(int argc, char *argv[]) {
                         ssize_t size;
 
                         size = read(fd_signal, &fdsi, sizeof(struct signalfd_siginfo));
-                        if (size == sizeof(struct signalfd_siginfo))
-                                handle_signal(udev, fdsi.ssi_signo);
+                        if (size == sizeof(struct signalfd_siginfo)) {
+                                switch (fdsi.ssi_signo) {
+                                case SIGINT:
+                                case SIGTERM:
+                                        on_request_exit(NULL, &fdsi, NULL);
+                                        break;
+                                case SIGHUP:
+                                        on_request_reload(NULL, &fdsi, NULL);
+                                        break;
+                                case SIGCHLD:
+                                        on_sigchld(NULL, &fdsi, NULL);
+                                        break;
+                                }
+                        }
                 }
 
                 /* we are shutting down, the events below are not handled anymore */
@@ -1539,7 +1566,7 @@ int main(int argc, char *argv[]) {
 
                 /* device node watch */
                 if (is_inotify) {
-                        handle_inotify(udev);
+                        on_inotify(NULL, fd_inotify, 0, udev);
 
                         /*
                          * settle might be waiting on us to determine the queue
@@ -1569,7 +1596,7 @@ int main(int argc, char *argv[]) {
                  * exit.
                  */
                 if (is_ctrl)
-                        handle_ctrl_msg(udev_ctrl);
+                        on_ctrl_msg(NULL, fd_ctrl, 0, udev_ctrl);
         }
 
 exit:



More information about the systemd-commits mailing list