[systemd-commits] 4 commits - man/systemd.path.xml src/path.c src/path.h src/service.c src/service.h

Michal Schmidt michich at kemper.freedesktop.org
Mon Dec 5 02:50:58 PST 2011


 man/systemd.path.xml |   17 +-
 src/path.c           |  358 ++++++++++++++++++++++++++++-----------------------
 src/path.h           |    9 +
 src/service.c        |  181 ++++++++++++++++++++++---
 src/service.h        |    3 
 5 files changed, 377 insertions(+), 191 deletions(-)

New commits:
commit 2096e009a790073a934f5cd07d17024d3b199d0b
Author: Michal Schmidt <mschmidt at redhat.com>
Date:   Sat Dec 3 21:34:34 2011 +0100

    service: stop the service if ExecStartPost ends with a failure
    
    The handling of failures in ExecStartPost is inconsistent. If the
    command times out, the service is stopped. But if the command exits
    with a failure, the service keeps running.
    
    It makes more sense to stop the service when ExecStartPost fails.
    If this behaviour is not desired, the ExecStartPost command can be
    prefixed with "-".

diff --git a/src/service.c b/src/service.c
index 07137d2..5243e69 100644
--- a/src/service.c
+++ b/src/service.c
@@ -2870,20 +2870,22 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) {
                                 break;
 
                         case SERVICE_START_POST:
-                                if (success) {
-                                        if (s->pid_file) {
-                                                int r = service_load_pid_file(s, true);
-                                                if (r < 0) {
-                                                        r = service_demand_pid_file(s);
-                                                        if (r < 0 || !cgroup_good(s))
-                                                                service_enter_stop(s, false);
-                                                        break;
-                                                }
-                                        } else
-                                                service_search_main_pid(s);
+                                if (!success) {
+                                        service_enter_stop(s, false);
+                                        break;
                                 }
 
-                                s->reload_failure = !success;
+                                if (s->pid_file) {
+                                        int r = service_load_pid_file(s, true);
+                                        if (r < 0) {
+                                                r = service_demand_pid_file(s);
+                                                if (r < 0 || !cgroup_good(s))
+                                                        service_enter_stop(s, false);
+                                                break;
+                                        }
+                                } else
+                                        service_search_main_pid(s);
+
                                 service_enter_running(s, true);
                                 break;
 

commit 3a11183858af30bc9b4e9dac430dd7541deec19b
Author: Michal Schmidt <mschmidt at redhat.com>
Date:   Sat Dec 3 02:13:30 2011 +0100

    service: handle services with racy daemonization gracefully
    
    There are a lot of forking daemons that do not exactly follow the
    initialization steps as described in daemon(7). It is common that they
    do not bother waiting in the parent process for the child to write the
    PID file before exiting. The daemons' developers often do not perceive
    this as a bug and they're unwilling to change.
    
    Currently systemd warns about the missing PID file and falls back to
    guessing the main PID. Being not quite deterministic, the guess can be
    wrong with bad consequences. If the guessing is disabled, determinism is
    achieved at the cost of losing the ability of noticing when the main
    process of the service dies.
    
    As long as it does not negatively affect properly written services,
    systemd should strive for compatibility even with services with racy
    daemonization. It is possible to provide determinism _and_ main process
    supervision to them.
    
    If the PID file is not there, rather than guessing and considering the
    service running immediately after getting the SIGCHLD from the ExecStart
    (or ExecStartPost) process, we can keep the service in the activating
    state for a bit longer. We can use inotify to wait for the PID file to
    appear. Only when it finally does appear and we read a valid PID from
    it, we'll move the service to the running state. If the PID file never
    appears, the usual timeout kicks in and the service fails.

diff --git a/src/service.c b/src/service.c
index 6fc2484..07137d2 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1302,8 +1302,8 @@ static int service_load_pid_file(Service *s, bool may_warn) {
 
         if ((r = read_one_line_file(s->pid_file, &k)) < 0) {
                 if (may_warn)
-                        log_warning("Failed to read PID file %s after %s. The service might be broken.",
-                                    s->pid_file, service_state_to_string(s->state));
+                        log_info("PID file %s not readable (yet?) after %s.",
+                                 s->pid_file, service_state_to_string(s->state));
                 return r;
         }
 
@@ -1315,8 +1315,8 @@ static int service_load_pid_file(Service *s, bool may_warn) {
 
         if (kill(pid, 0) < 0 && errno != EPERM) {
                 if (may_warn)
-                        log_warning("PID %lu read from file %s does not exist. Your service or init script might be broken.",
-                                    (unsigned long) pid, s->pid_file);
+                        log_info("PID %lu read from file %s does not exist.",
+                                 (unsigned long) pid, s->pid_file);
                 return -ESRCH;
         }
 
@@ -1328,7 +1328,8 @@ static int service_load_pid_file(Service *s, bool may_warn) {
                           (unsigned long) s->main_pid, (unsigned long) pid);
                 service_unwatch_main_pid(s);
                 s->main_pid_known = false;
-        }
+        } else
+                log_debug("Main PID loaded: %lu", (unsigned long) pid);
 
         if ((r = service_set_main_pid(s, pid)) < 0)
                 return r;
@@ -1359,6 +1360,7 @@ static int service_search_main_pid(Service *s) {
         if ((pid = cgroup_bonding_search_main_pid_list(s->meta.cgroup_bondings)) <= 0)
                 return -ENOENT;
 
+        log_debug("Main PID guessed: %lu", (unsigned long) pid);
         if ((r = service_set_main_pid(s, pid)) < 0)
                 return r;
 
@@ -1451,6 +1453,17 @@ static int service_notify_sockets_dead(Service *s) {
         return 0;
 }
 
+static void service_unwatch_pid_file(Service *s) {
+        if (!s->pid_file_pathspec)
+                return;
+
+        log_debug("Stopping watch for %s's PID file %s", s->meta.id, s->pid_file_pathspec->path);
+        pathspec_unwatch(s->pid_file_pathspec, UNIT(s));
+        pathspec_done(s->pid_file_pathspec);
+        free(s->pid_file_pathspec);
+        s->pid_file_pathspec = NULL;
+}
+
 static void service_set_state(Service *s, ServiceState state) {
         ServiceState old_state;
         assert(s);
@@ -1458,6 +1471,8 @@ static void service_set_state(Service *s, ServiceState state) {
         old_state = s->state;
         s->state = state;
 
+        service_unwatch_pid_file(s);
+
         if (state != SERVICE_START_PRE &&
             state != SERVICE_START &&
             state != SERVICE_START_POST &&
@@ -2602,6 +2617,95 @@ static bool service_check_snapshot(Unit *u) {
         return !s->got_socket_fd;
 }
 
+static int service_retry_pid_file(Service *s) {
+        int r;
+
+        assert(s->pid_file);
+        assert(s->state == SERVICE_START || s->state == SERVICE_START_POST);
+
+        r = service_load_pid_file(s, false);
+        if (r < 0)
+                return r;
+
+        service_unwatch_pid_file(s);
+
+        service_enter_running(s, true);
+        return 0;
+}
+
+static int service_watch_pid_file(Service *s) {
+        int r;
+
+        log_debug("Setting watch for %s's PID file %s", s->meta.id, s->pid_file_pathspec->path);
+        r = pathspec_watch(s->pid_file_pathspec, UNIT(s));
+        if (r < 0)
+                goto fail;
+
+        /* the pidfile might have appeared just before we set the watch */
+        service_retry_pid_file(s);
+
+        return 0;
+fail:
+        log_error("Failed to set a watch for %s's PID file %s: %s",
+                  s->meta.id, s->pid_file_pathspec->path, strerror(-r));
+        service_unwatch_pid_file(s);
+        return r;
+}
+
+static int service_demand_pid_file(Service *s) {
+        PathSpec *ps;
+
+        assert(s->pid_file);
+        assert(!s->pid_file_pathspec);
+
+        ps = new0(PathSpec, 1);
+        if (!ps)
+                return -ENOMEM;
+
+        ps->path = strdup(s->pid_file);
+        if (!ps->path) {
+                free(ps);
+                return -ENOMEM;
+        }
+
+        path_kill_slashes(ps->path);
+
+        /* PATH_CHANGED would not be enough. There are daemons (sendmail) that
+         * keep their PID file open all the time. */
+        ps->type = PATH_MODIFIED;
+        ps->inotify_fd = -1;
+
+        s->pid_file_pathspec = ps;
+
+        return service_watch_pid_file(s);
+}
+
+static void service_fd_event(Unit *u, int fd, uint32_t events, Watch *w) {
+        Service *s = SERVICE(u);
+
+        assert(s);
+        assert(fd >= 0);
+        assert(s->state == SERVICE_START || s->state == SERVICE_START_POST);
+        assert(s->pid_file_pathspec);
+        assert(pathspec_owns_inotify_fd(s->pid_file_pathspec, fd));
+
+        log_debug("inotify event for %s", u->meta.id);
+
+        if (pathspec_fd_event(s->pid_file_pathspec, events) < 0)
+                goto fail;
+
+        if (service_retry_pid_file(s) == 0)
+                return;
+
+        if (service_watch_pid_file(s) < 0)
+                goto fail;
+
+        return;
+fail:
+        service_unwatch_pid_file(s);
+        service_enter_signal(s, SERVICE_STOP_SIGTERM, false);
+}
+
 static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) {
         Service *s = SERVICE(u);
         bool success;
@@ -2707,7 +2811,7 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) {
                                 success = true;
                 }
 
-               log_full(success ? LOG_DEBUG : LOG_NOTICE,
+                log_full(success ? LOG_DEBUG : LOG_NOTICE,
                          "%s: control process exited, code=%s status=%i", u->meta.id, sigchld_code_to_string(code), status);
                 s->failure = s->failure || !success;
 
@@ -2742,27 +2846,41 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) {
                         case SERVICE_START:
                                 assert(s->type == SERVICE_FORKING);
 
-                                /* Let's try to load the pid
-                                 * file here if we can. We
-                                 * ignore the return value,
-                                 * since the PID file might
-                                 * actually be created by a
-                                 * START_POST script */
-
-                                if (success) {
-                                        service_load_pid_file(s, !s->exec_command[SERVICE_EXEC_START_POST]);
-                                        service_search_main_pid(s);
+                                if (!success) {
+                                        service_enter_signal(s, SERVICE_FINAL_SIGTERM, false);
+                                        break;
+                                }
 
-                                        service_enter_start_post(s);
+                                if (s->pid_file) {
+                                        /* Let's try to load the pid file here if we can.
+                                         * The PID file might actually be created by a START_POST
+                                         * script. In that case don't worry if the loading fails. */
+                                        bool has_start_post = !!s->exec_command[SERVICE_EXEC_START_POST];
+                                        int r = service_load_pid_file(s, !has_start_post);
+                                        if (!has_start_post && r < 0) {
+                                                r = service_demand_pid_file(s);
+                                                if (r < 0 || !cgroup_good(s))
+                                                        service_enter_signal(s, SERVICE_FINAL_SIGTERM, false);
+                                                break;
+                                        }
                                 } else
-                                        service_enter_signal(s, SERVICE_FINAL_SIGTERM, false);
+                                        service_search_main_pid(s);
 
+                                service_enter_start_post(s);
                                 break;
 
                         case SERVICE_START_POST:
                                 if (success) {
-                                        service_load_pid_file(s, true);
-                                        service_search_main_pid(s);
+                                        if (s->pid_file) {
+                                                int r = service_load_pid_file(s, true);
+                                                if (r < 0) {
+                                                        r = service_demand_pid_file(s);
+                                                        if (r < 0 || !cgroup_good(s))
+                                                                service_enter_stop(s, false);
+                                                        break;
+                                                }
+                                        } else
+                                                service_search_main_pid(s);
                                 }
 
                                 s->reload_failure = !success;
@@ -2907,6 +3025,20 @@ static void service_cgroup_notify_event(Unit *u) {
                  * except when we don't know pid which to expect the
                  * SIGCHLD for. */
 
+        case SERVICE_START:
+        case SERVICE_START_POST:
+                /* If we were hoping for the daemon to write its PID file,
+                 * we can give up now. */
+                if (s->pid_file_pathspec) {
+                        log_warning("%s never wrote its PID file. Failing.", s->meta.id);
+                        service_unwatch_pid_file(s);
+                        if (s->state == SERVICE_START)
+                                service_enter_signal(s, SERVICE_FINAL_SIGTERM, false);
+                        else
+                                service_enter_stop(s, false);
+                }
+                break;
+
         case SERVICE_RUNNING:
                 service_enter_running(s, true);
                 break;
@@ -3216,7 +3348,7 @@ static int service_enumerate(Manager *m) {
         r = 0;
 
 #ifdef TARGET_SUSE
-	sysv_facility_in_insserv_conf (m);
+        sysv_facility_in_insserv_conf (m);
 #endif
 
 finish:
@@ -3511,6 +3643,7 @@ const UnitVTable service_vtable = {
 
         .sigchld_event = service_sigchld_event,
         .timer_event = service_timer_event,
+        .fd_event = service_fd_event,
 
         .reset_failed = service_reset_failed,
 
diff --git a/src/service.h b/src/service.h
index e28f74b..15d58cc 100644
--- a/src/service.h
+++ b/src/service.h
@@ -86,6 +86,8 @@ typedef enum NotifyAccess {
         _NOTIFY_ACCESS_INVALID = -1
 } NotifyAccess;
 
+typedef struct PathSpec PathSpec;
+
 struct Service {
         Meta meta;
 
@@ -157,6 +159,7 @@ struct Service {
         Set *configured_sockets;
 
         Watch timer_watch;
+        PathSpec *pid_file_pathspec;
 
         NotifyAccess notify_access;
 };

commit e92238567b9fc83ef77e359588d7b005ecae3d70
Author: Michal Schmidt <mschmidt at redhat.com>
Date:   Sat Dec 3 10:22:26 2011 +0100

    path: add PathModified (= PathChanged + IN_MODIFY)

diff --git a/man/systemd.path.xml b/man/systemd.path.xml
index 10d8f73..5b1ff75 100644
--- a/man/systemd.path.xml
+++ b/man/systemd.path.xml
@@ -113,6 +113,7 @@
                                 <term><varname>PathExists=</varname></term>
                                 <term><varname>PathExistsGlob=</varname></term>
                                 <term><varname>PathChanged=</varname></term>
+                                <term><varname>PathModified=</varname></term>
                                 <term><varname>DirectoryNotEmpty=</varname></term>
 
                                 <listitem><para>Defines paths to
@@ -129,8 +130,14 @@
                                 specified. <varname>PathChanged=</varname>
                                 may be used to watch a file or
                                 directory and activate the configured
-                                unit whenever it changes or is
-                                modified. <varname>DirectoryNotEmpty=</varname>
+                                unit whenever it changes. It is not activated
+                                on every write to the watched file but it is
+                                activated if the file which was open for writing
+                                gets closed. <varname>PathModified=</varname>
+                                is similar, but additionally it is activated
+                                also on simple writes to the watched file.
+
+                                <varname>DirectoryNotEmpty=</varname>
                                 may be used to watch a directory and
                                 activate the configured unit whenever
                                 it contains at least one file.</para>
@@ -154,11 +161,7 @@
                                 activated, then the configured unit is
                                 immediately activated as
                                 well. Something similar does not apply
-                                to
-                                <varname>PathChanged=</varname>. The
-                                latter is not activated on simple
-                                writes but only if files with were
-                                opened for writing are closed.
+                                to <varname>PathChanged=</varname>.
                                 </para></listitem>
                         </varlistentry>
                         <varlistentry>
diff --git a/src/path.c b/src/path.c
index db6f873..1e5d825 100644
--- a/src/path.c
+++ b/src/path.c
@@ -44,6 +44,7 @@ int pathspec_watch(PathSpec *s, Unit *u) {
                 [PATH_EXISTS] = IN_DELETE_SELF|IN_MOVE_SELF|IN_ATTRIB,
                 [PATH_EXISTS_GLOB] = IN_DELETE_SELF|IN_MOVE_SELF|IN_ATTRIB,
                 [PATH_CHANGED] = IN_DELETE_SELF|IN_MOVE_SELF|IN_ATTRIB|IN_CLOSE_WRITE|IN_CREATE|IN_DELETE|IN_MOVED_FROM|IN_MOVED_TO,
+                [PATH_MODIFIED] = IN_DELETE_SELF|IN_MOVE_SELF|IN_ATTRIB|IN_CLOSE_WRITE|IN_CREATE|IN_DELETE|IN_MOVED_FROM|IN_MOVED_TO|IN_MODIFY,
                 [PATH_DIRECTORY_NOT_EMPTY] = IN_DELETE_SELF|IN_MOVE_SELF|IN_ATTRIB|IN_CREATE|IN_MOVED_TO
         };
 
@@ -713,6 +714,7 @@ static const char* const path_type_table[_PATH_TYPE_MAX] = {
         [PATH_EXISTS] = "PathExists",
         [PATH_EXISTS_GLOB] = "PathExistsGlob",
         [PATH_CHANGED] = "PathChanged",
+        [PATH_MODIFIED] = "PathModified",
         [PATH_DIRECTORY_NOT_EMPTY] = "DirectoryNotEmpty"
 };
 
diff --git a/src/path.h b/src/path.h
index 4e6ccf5..1d78fe4 100644
--- a/src/path.h
+++ b/src/path.h
@@ -41,6 +41,7 @@ typedef enum PathType {
         PATH_EXISTS_GLOB,
         PATH_DIRECTORY_NOT_EMPTY,
         PATH_CHANGED,
+        PATH_MODIFIED,
         _PATH_TYPE_MAX,
         _PATH_TYPE_INVALID = -1
 } PathType;

commit 4b562198c79e4ebfc3d84b69a1dae374bc6cf9f5
Author: Michal Schmidt <mschmidt at redhat.com>
Date:   Sat Dec 3 01:38:30 2011 +0100

    path: refactor PathSpec usage
    
    path_*() functions operate on "Path *p" and they do not touch PathSpec
    internals directly.
    
    pathspec_*() functions operate on "PathSpec *s". The PathSpec class will
    be useful outside of path.c.

diff --git a/src/path.c b/src/path.c
index 142fd2d..db6f873 100644
--- a/src/path.c
+++ b/src/path.c
@@ -39,26 +39,202 @@ static const UnitActiveState state_translation_table[_PATH_STATE_MAX] = {
         [PATH_FAILED] = UNIT_FAILED
 };
 
-static void path_init(Unit *u) {
-        Path *p = PATH(u);
+int pathspec_watch(PathSpec *s, Unit *u) {
+        static const int flags_table[_PATH_TYPE_MAX] = {
+                [PATH_EXISTS] = IN_DELETE_SELF|IN_MOVE_SELF|IN_ATTRIB,
+                [PATH_EXISTS_GLOB] = IN_DELETE_SELF|IN_MOVE_SELF|IN_ATTRIB,
+                [PATH_CHANGED] = IN_DELETE_SELF|IN_MOVE_SELF|IN_ATTRIB|IN_CLOSE_WRITE|IN_CREATE|IN_DELETE|IN_MOVED_FROM|IN_MOVED_TO,
+                [PATH_DIRECTORY_NOT_EMPTY] = IN_DELETE_SELF|IN_MOVE_SELF|IN_ATTRIB|IN_CREATE|IN_MOVED_TO
+        };
+
+        bool exists = false;
+        char *k, *slash;
+        int r;
 
         assert(u);
-        assert(u->meta.load_state == UNIT_STUB);
+        assert(s);
 
-        p->directory_mode = 0755;
+        pathspec_unwatch(s, u);
+
+        if (!(k = strdup(s->path)))
+                return -ENOMEM;
+
+        if ((s->inotify_fd = inotify_init1(IN_NONBLOCK|IN_CLOEXEC)) < 0) {
+                r = -errno;
+                goto fail;
+        }
+
+        if (unit_watch_fd(u, s->inotify_fd, EPOLLIN, &s->watch) < 0) {
+                r = -errno;
+                goto fail;
+        }
+
+        if ((s->primary_wd = inotify_add_watch(s->inotify_fd, k, flags_table[s->type])) >= 0)
+                exists = true;
+
+        do {
+                int flags;
+
+                /* This assumes the path was passed through path_kill_slashes()! */
+                if (!(slash = strrchr(k, '/')))
+                        break;
+
+                /* Trim the path at the last slash. Keep the slash if it's the root dir. */
+                slash[slash == k] = 0;
+
+                flags = IN_MOVE_SELF;
+                if (!exists)
+                        flags |= IN_DELETE_SELF | IN_ATTRIB | IN_CREATE | IN_MOVED_TO;
+
+                if (inotify_add_watch(s->inotify_fd, k, flags) >= 0)
+                        exists = true;
+        } while (slash != k);
+
+        return 0;
+
+fail:
+        free(k);
+
+        pathspec_unwatch(s, u);
+        return r;
 }
 
-static void path_unwatch_one(Path *p, PathSpec *s) {
+void pathspec_unwatch(PathSpec *s, Unit *u) {
 
         if (s->inotify_fd < 0)
                 return;
 
-        unit_unwatch_fd(UNIT(p), &s->watch);
+        unit_unwatch_fd(u, &s->watch);
 
         close_nointr_nofail(s->inotify_fd);
         s->inotify_fd = -1;
 }
 
+int pathspec_fd_event(PathSpec *s, uint32_t events) {
+        uint8_t *buf = NULL;
+        struct inotify_event *e;
+        ssize_t k;
+        int l;
+        int r = 0;
+
+        if (events != EPOLLIN) {
+                log_error("Got Invalid poll event on inotify.");
+                r = -EINVAL;
+                goto out;
+        }
+
+        if (ioctl(s->inotify_fd, FIONREAD, &l) < 0) {
+                log_error("FIONREAD failed: %m");
+                r = -errno;
+                goto out;
+        }
+
+        assert(l > 0);
+
+        if (!(buf = malloc(l))) {
+                log_error("Failed to allocate buffer: %m");
+                r = -errno;
+                goto out;
+        }
+
+        if ((k = read(s->inotify_fd, buf, l)) < 0) {
+                log_error("Failed to read inotify event: %m");
+                r = -errno;
+                goto out;
+        }
+
+        e = (struct inotify_event*) buf;
+
+        while (k > 0) {
+                size_t step;
+
+                if (s->type == PATH_CHANGED && 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;
+        }
+out:
+        free(buf);
+        return r;
+}
+
+static bool pathspec_check_good(PathSpec *s, bool initial) {
+        bool good = false;
+
+        switch (s->type) {
+
+        case PATH_EXISTS:
+                good = access(s->path, F_OK) >= 0;
+                break;
+
+        case PATH_EXISTS_GLOB:
+                good = glob_exists(s->path) > 0;
+                break;
+
+        case PATH_DIRECTORY_NOT_EMPTY: {
+                int k;
+
+                k = dir_is_empty(s->path);
+                good = !(k == -ENOENT || k > 0);
+                break;
+        }
+
+        case PATH_CHANGED: {
+                bool b;
+
+                b = access(s->path, F_OK) >= 0;
+                good = !initial && b != s->previous_exists;
+                s->previous_exists = b;
+                break;
+        }
+
+        default:
+                ;
+        }
+
+        return good;
+}
+
+static bool pathspec_startswith(PathSpec *s, const char *what) {
+        return path_startswith(s->path, what);
+}
+
+static void pathspec_mkdir(PathSpec *s, mode_t mode) {
+        int r;
+
+        if (s->type == PATH_EXISTS || s->type == PATH_EXISTS_GLOB)
+                return;
+
+        if ((r = mkdir_p(s->path, mode)) < 0)
+                log_warning("mkdir(%s) failed: %s", s->path, strerror(-r));
+}
+
+static void pathspec_dump(PathSpec *s, FILE *f, const char *prefix) {
+        fprintf(f,
+                "%s%s: %s\n",
+                prefix,
+                path_type_to_string(s->type),
+                s->path);
+}
+
+void pathspec_done(PathSpec *s) {
+        assert(s->inotify_fd == -1);
+        free(s->path);
+}
+
+static void path_init(Unit *u) {
+        Path *p = PATH(u);
+
+        assert(u);
+        assert(u->meta.load_state == UNIT_STUB);
+
+        p->directory_mode = 0755;
+}
+
 static void path_done(Unit *u) {
         Path *p = PATH(u);
         PathSpec *s;
@@ -66,9 +242,9 @@ static void path_done(Unit *u) {
         assert(p);
 
         while ((s = p->specs)) {
-                path_unwatch_one(p, s);
+                pathspec_unwatch(s, u);
                 LIST_REMOVE(PathSpec, spec, p->specs, s);
-                free(s->path);
+                pathspec_done(s);
                 free(s);
         }
 }
@@ -86,7 +262,7 @@ int path_add_one_mount_link(Path *p, Mount *m) {
 
         LIST_FOREACH(spec, s, p->specs) {
 
-                if (!path_startswith(s->path, m->where))
+                if (!pathspec_startswith(s, m->where))
                         continue;
 
                 if ((r = unit_add_two_dependencies(UNIT(p), UNIT_AFTER, UNIT_REQUIRES, UNIT(m), true)) < 0)
@@ -187,71 +363,7 @@ static void path_dump(Unit *u, FILE *f, const char *prefix) {
                 prefix, p->directory_mode);
 
         LIST_FOREACH(spec, s, p->specs)
-                fprintf(f,
-                        "%s%s: %s\n",
-                        prefix,
-                        path_type_to_string(s->type),
-                        s->path);
-}
-
-static int path_watch_one(Path *p, PathSpec *s) {
-        static const int flags_table[_PATH_TYPE_MAX] = {
-                [PATH_EXISTS] = IN_DELETE_SELF|IN_MOVE_SELF|IN_ATTRIB,
-                [PATH_EXISTS_GLOB] = IN_DELETE_SELF|IN_MOVE_SELF|IN_ATTRIB,
-                [PATH_CHANGED] = IN_DELETE_SELF|IN_MOVE_SELF|IN_ATTRIB|IN_CLOSE_WRITE|IN_CREATE|IN_DELETE|IN_MOVED_FROM|IN_MOVED_TO,
-                [PATH_DIRECTORY_NOT_EMPTY] = IN_DELETE_SELF|IN_MOVE_SELF|IN_ATTRIB|IN_CREATE|IN_MOVED_TO
-        };
-
-        bool exists = false;
-        char *k, *slash;
-        int r;
-
-        assert(p);
-        assert(s);
-
-        path_unwatch_one(p, s);
-
-        if (!(k = strdup(s->path)))
-                return -ENOMEM;
-
-        if ((s->inotify_fd = inotify_init1(IN_NONBLOCK|IN_CLOEXEC)) < 0) {
-                r = -errno;
-                goto fail;
-        }
-
-        if (unit_watch_fd(UNIT(p), s->inotify_fd, EPOLLIN, &s->watch) < 0) {
-                r = -errno;
-                goto fail;
-        }
-
-        if ((s->primary_wd = inotify_add_watch(s->inotify_fd, k, flags_table[s->type])) >= 0)
-                exists = true;
-
-        do {
-                int flags;
-
-                /* This assumes the path was passed through path_kill_slashes()! */
-                if (!(slash = strrchr(k, '/')))
-                        break;
-
-                /* Trim the path at the last slash. Keep the slash if it's the root dir. */
-                slash[slash == k] = 0;
-
-                flags = IN_MOVE_SELF;
-                if (!exists)
-                        flags |= IN_DELETE_SELF | IN_ATTRIB | IN_CREATE | IN_MOVED_TO;
-
-                if (inotify_add_watch(s->inotify_fd, k, flags) >= 0)
-                        exists = true;
-        } while (slash != k);
-
-        return 0;
-
-fail:
-        free(k);
-
-        path_unwatch_one(p, s);
-        return r;
+                pathspec_dump(s, f, prefix);
 }
 
 static void path_unwatch(Path *p) {
@@ -260,7 +372,7 @@ static void path_unwatch(Path *p) {
         assert(p);
 
         LIST_FOREACH(spec, s, p->specs)
-                path_unwatch_one(p, s);
+                pathspec_unwatch(s, UNIT(p));
 }
 
 static int path_watch(Path *p) {
@@ -270,7 +382,7 @@ static int path_watch(Path *p) {
         assert(p);
 
         LIST_FOREACH(spec, s, p->specs)
-                if ((r = path_watch_one(p, s)) < 0)
+                if ((r = pathspec_watch(s, UNIT(p))) < 0)
                         return r;
 
         return 0;
@@ -361,37 +473,7 @@ static bool path_check_good(Path *p, bool initial) {
         assert(p);
 
         LIST_FOREACH(spec, s, p->specs) {
-
-                switch (s->type) {
-
-                case PATH_EXISTS:
-                        good = access(s->path, F_OK) >= 0;
-                        break;
-
-                case PATH_EXISTS_GLOB:
-                        good = glob_exists(s->path) > 0;
-                        break;
-
-                case PATH_DIRECTORY_NOT_EMPTY: {
-                        int k;
-
-                        k = dir_is_empty(s->path);
-                        good = !(k == -ENOENT || k > 0);
-                        break;
-                }
-
-                case PATH_CHANGED: {
-                        bool b;
-
-                        b = access(s->path, F_OK) >= 0;
-                        good = !initial && b != s->previous_exists;
-                        s->previous_exists = b;
-                        break;
-                }
-
-                default:
-                        ;
-                }
+                good = pathspec_check_good(s, initial);
 
                 if (good)
                         break;
@@ -440,15 +522,8 @@ static void path_mkdir(Path *p) {
         if (!p->make_directory)
                 return;
 
-        LIST_FOREACH(spec, s, p->specs) {
-                int r;
-
-                if (s->type == PATH_EXISTS || s->type == PATH_EXISTS_GLOB)
-                        continue;
-
-                if ((r = mkdir_p(s->path, p->directory_mode)) < 0)
-                        log_warning("mkdir(%s) failed: %s", s->path, strerror(-r));
-        }
+        LIST_FOREACH(spec, s, p->specs)
+                pathspec_mkdir(s, p->directory_mode);
 }
 
 static int path_start(Unit *u) {
@@ -525,12 +600,8 @@ static const char *path_sub_state_to_string(Unit *u) {
 
 static void path_fd_event(Unit *u, int fd, uint32_t events, Watch *w) {
         Path *p = PATH(u);
-        int l;
-        ssize_t k;
-        uint8_t *buf = NULL;
-        struct inotify_event *e;
         PathSpec *s;
-        bool changed;
+        int changed;
 
         assert(p);
         assert(fd >= 0);
@@ -541,13 +612,8 @@ static void path_fd_event(Unit *u, int fd, uint32_t events, Watch *w) {
 
         /* log_debug("inotify wakeup on %s.", u->meta.id); */
 
-        if (events != EPOLLIN) {
-                log_error("Got Invalid poll event on inotify.");
-                goto fail;
-        }
-
         LIST_FOREACH(spec, s, p->specs)
-                if (s->inotify_fd == fd)
+                if (pathspec_owns_inotify_fd(s, fd))
                         break;
 
         if (!s) {
@@ -555,55 +621,23 @@ static void path_fd_event(Unit *u, int fd, uint32_t events, Watch *w) {
                 goto fail;
         }
 
-        if (ioctl(fd, FIONREAD, &l) < 0) {
-                log_error("FIONREAD failed: %m");
-                goto fail;
-        }
-
-        assert(l > 0);
-
-        if (!(buf = malloc(l))) {
-                log_error("Failed to allocate buffer: %s", strerror(ENOMEM));
-                goto fail;
-        }
-
-        if ((k = read(fd, buf, l)) < 0) {
-                log_error("Failed to read inotify event: %m");
+        changed = pathspec_fd_event(s, events);
+        if (changed < 0)
                 goto fail;
-        }
 
         /* If we are already running, then remember that one event was
          * dispatched so that we restart the service only if something
          * actually changed on disk */
         p->inotify_triggered = true;
 
-        e = (struct inotify_event*) buf;
-
-        changed = false;
-        while (k > 0) {
-                size_t step;
-
-                if (s->type == PATH_CHANGED && s->primary_wd == e->wd)
-                        changed = true;
-
-                step = sizeof(struct inotify_event) + e->len;
-                assert(step <= (size_t) k);
-
-                e = (struct inotify_event*) ((uint8_t*) e + step);
-                k -= step;
-        }
-
         if (changed)
                 path_enter_running(p);
         else
                 path_enter_waiting(p, false, true);
 
-        free(buf);
-
         return;
 
 fail:
-        free(buf);
         path_enter_dead(p, false);
 }
 
diff --git a/src/path.h b/src/path.h
index 116fc63..4e6ccf5 100644
--- a/src/path.h
+++ b/src/path.h
@@ -60,6 +60,14 @@ typedef struct PathSpec {
 
 } PathSpec;
 
+int  pathspec_watch(PathSpec *s, Unit *u);
+void pathspec_unwatch(PathSpec *s, Unit *u);
+int  pathspec_fd_event(PathSpec *s, uint32_t events);
+void pathspec_done(PathSpec *s);
+static inline bool pathspec_owns_inotify_fd(PathSpec *s, int fd) {
+        return s->inotify_fd == fd;
+}
+
 struct Path {
         Meta meta;
 



More information about the systemd-commits mailing list