[systemd-commits] 4 commits - TODO src/cgls src/core src/shared src/test

Lennart Poettering lennart at kemper.freedesktop.org
Thu Sep 26 11:20:40 PDT 2013


 TODO                                  |    2 
 src/cgls/cgls.c                       |    4 
 src/core/automount.c                  |   33 ----
 src/core/automount.h                  |    2 
 src/core/load-fragment-gperf.gperf.m4 |    2 
 src/core/load-fragment.c              |   47 ++++--
 src/core/manager.c                    |   44 +++++
 src/core/manager.h                    |   12 +
 src/core/mount.c                      |  255 ++++++++--------------------------
 src/core/path.c                       |   32 ----
 src/core/path.h                       |    4 
 src/core/socket.c                     |   49 +-----
 src/core/socket.h                     |    4 
 src/core/swap.c                       |   40 -----
 src/core/swap.h                       |    2 
 src/core/unit.c                       |  162 +++++++++++++++++----
 src/core/unit.h                       |    6 
 src/shared/cgroup-util.c              |   33 ++--
 src/shared/path-util.h                |    9 +
 src/shared/socket-util.c              |    8 -
 src/shared/socket-util.h              |    2 
 src/shared/unit-name.c                |    2 
 src/test/test-path-util.c             |   34 ++++
 23 files changed, 374 insertions(+), 414 deletions(-)

New commits:
commit a57f7e2c828b852eb32fd810dcea041bb2975501
Author: Lennart Poettering <lennart at poettering.net>
Date:   Thu Sep 26 20:14:24 2013 +0200

    core: rework how we match mount units against each other
    
    Previously to automatically create dependencies between mount units we
    matched every mount unit agains all others resulting in O(n^2)
    complexity. On setups with large amounts of mount units this might make
    things slow.
    
    This change replaces the matching code to use a hashtable that is keyed
    by a path prefix, and points to a set of units that require that path to
    be around. When a new mount unit is installed it is hence sufficient to
    simply look up this set of units via its own file system paths to know
    which units to order after itself.
    
    This patch also changes all unit types to only create automatic mount
    dependencies via the RequiresMountsFor= logic, and this is exposed to
    the outside to make things more transparent.
    
    With this change we still have some O(n) complexities in place when
    handling mounts, but that's currently unavoidable due to kernel APIs,
    and still substantially better than O(n^2) as before.
    
    https://bugs.freedesktop.org/show_bug.cgi?id=69740

diff --git a/src/core/automount.c b/src/core/automount.c
index 6762392..d1379e0 100644
--- a/src/core/automount.c
+++ b/src/core/automount.c
@@ -117,42 +117,17 @@ static void automount_done(Unit *u) {
         a->tokens = NULL;
 }
 
-int automount_add_one_mount_link(Automount *a, Mount *m) {
+static int automount_add_mount_links(Automount *a) {
+        _cleanup_free_ char *parent = NULL;
         int r;
 
         assert(a);
-        assert(m);
-
-        if (UNIT(a)->load_state != UNIT_LOADED ||
-            UNIT(m)->load_state != UNIT_LOADED)
-                return 0;
-
-        if (!path_startswith(a->where, m->where))
-                return 0;
 
-        if (path_equal(a->where, m->where))
-                return 0;
-
-        r = unit_add_two_dependencies(UNIT(a), UNIT_AFTER, UNIT_REQUIRES, UNIT(m), true);
+        r = path_get_parent(a->where, &parent);
         if (r < 0)
                 return r;
 
-        return 0;
-}
-
-static int automount_add_mount_links(Automount *a) {
-        Unit *other;
-        int r;
-
-        assert(a);
-
-        LIST_FOREACH(units_by_type, other, UNIT(a)->manager->units_by_type[UNIT_MOUNT]) {
-                r = automount_add_one_mount_link(a, MOUNT(other));
-                if (r < 0)
-                        return r;
-        }
-
-        return 0;
+        return unit_require_mounts_for(UNIT(a), parent);
 }
 
 static int automount_add_default_dependencies(Automount *a) {
diff --git a/src/core/automount.h b/src/core/automount.h
index 0c6b8a7..a7a25d3 100644
--- a/src/core/automount.h
+++ b/src/core/automount.h
@@ -62,8 +62,6 @@ extern const UnitVTable automount_vtable;
 
 int automount_send_ready(Automount *a, int status);
 
-int automount_add_one_mount_link(Automount *a, Mount *m);
-
 const char* automount_state_to_string(AutomountState i) _const_;
 AutomountState automount_state_from_string(const char *s) _pure_;
 
diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4
index 25bd3aa..31fb7bc 100644
--- a/src/core/load-fragment-gperf.gperf.m4
+++ b/src/core/load-fragment-gperf.gperf.m4
@@ -116,7 +116,7 @@ Unit.PropagateReloadTo,          config_parse_unit_deps,             UNIT_PROPAG
 Unit.ReloadPropagatedFrom,       config_parse_unit_deps,             UNIT_RELOAD_PROPAGATED_FROM,   0
 Unit.PropagateReloadFrom,        config_parse_unit_deps,             UNIT_RELOAD_PROPAGATED_FROM,   0
 Unit.PartOf,                     config_parse_unit_deps,             UNIT_PART_OF,                  0
-Unit.RequiresMountsFor,          config_parse_unit_requires_mounts_for, 0,                          offsetof(Unit, requires_mounts_for)
+Unit.RequiresMountsFor,          config_parse_unit_requires_mounts_for, 0,                          0
 Unit.StopWhenUnneeded,           config_parse_bool,                  0,                             offsetof(Unit, stop_when_unneeded)
 Unit.RefuseManualStart,          config_parse_bool,                  0,                             offsetof(Unit, refuse_manual_start)
 Unit.RefuseManualStop,           config_parse_bool,                  0,                             offsetof(Unit, refuse_manual_stop)
diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index 74454ab..70ea13a 100644
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -1773,33 +1773,48 @@ int config_parse_unit_condition_null(const char *unit,
 DEFINE_CONFIG_PARSE_ENUM(config_parse_notify_access, notify_access, NotifyAccess, "Failed to parse notify access specifier");
 DEFINE_CONFIG_PARSE_ENUM(config_parse_start_limit_action, start_limit_action, StartLimitAction, "Failed to parse start limit action specifier");
 
-int config_parse_unit_requires_mounts_for(const char *unit,
-                                          const char *filename,
-                                          unsigned line,
-                                          const char *section,
-                                          const char *lvalue,
-                                          int ltype,
-                                          const char *rvalue,
-                                          void *data,
-                                          void *userdata) {
+int config_parse_unit_requires_mounts_for(
+                const char *unit,
+                const char *filename,
+                unsigned line,
+                const char *section,
+                const char *lvalue,
+                int ltype,
+                const char *rvalue,
+                void *data,
+                void *userdata) {
 
         Unit *u = userdata;
+        char *state;
+        size_t l;
+        char *w;
         int r;
-        bool empty_before;
 
         assert(filename);
         assert(lvalue);
         assert(rvalue);
         assert(data);
 
-        empty_before = !u->requires_mounts_for;
+        FOREACH_WORD_QUOTED(w, l, rvalue, state) {
+                _cleanup_free_ char *n;
+
+                n = strndup(w, l);
+                if (!n)
+                        return log_oom();
 
-        r = config_parse_path_strv(unit, filename, line, section, lvalue, ltype,
-                                   rvalue, data, userdata);
+                if (!utf8_is_valid(n)) {
+                        log_syntax(unit, LOG_ERR, filename, line, EINVAL,
+                                   "Path is not UTF-8 clean, ignoring assignment: %s", rvalue);
+                        continue;
+                }
 
-        /* Make it easy to find units with requires_mounts set */
-        if (empty_before && u->requires_mounts_for)
-                LIST_PREPEND(Unit, has_requires_mounts_for, u->manager->has_requires_mounts_for, u);
+                r = unit_require_mounts_for(u, n);
+                if (r < 0) {
+                        log_syntax(unit, LOG_ERR, filename, line, r,
+                                   "Failed to add required mount for, ignoring: %s", rvalue);
+                        continue;
+                }
+        }
 
         return r;
 }
diff --git a/src/core/manager.c b/src/core/manager.c
index f70ff03..bc57e4a 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -770,6 +770,9 @@ void manager_free(Manager *m) {
         for (i = 0; i < RLIMIT_NLIMITS; i++)
                 free(m->rlimit[i]);
 
+        assert(hashmap_isempty(m->units_requiring_mounts_for));
+        hashmap_free(m->units_requiring_mounts_for);
+
         free(m);
 }
 
@@ -782,9 +785,11 @@ int manager_enumerate(Manager *m) {
         /* Let's ask every type to load all units from disk/kernel
          * that it might know */
         for (c = 0; c < _UNIT_TYPE_MAX; c++)
-                if (unit_vtable[c]->enumerate)
-                        if ((q = unit_vtable[c]->enumerate(m)) < 0)
+                if (unit_vtable[c]->enumerate) {
+                        q = unit_vtable[c]->enumerate(m);
+                        if (q < 0)
                                 r = q;
+                }
 
         manager_dispatch_load_queue(m);
         return r;
@@ -2764,6 +2769,41 @@ void manager_status_printf(Manager *m, bool ephemeral, const char *status, const
         va_end(ap);
 }
 
+int manager_get_unit_by_path(Manager *m, const char *path, const char *suffix, Unit **_found) {
+        _cleanup_free_ char *p = NULL;
+        Unit *found;
+
+        assert(m);
+        assert(path);
+        assert(suffix);
+        assert(_found);
+
+        p = unit_name_from_path(path, suffix);
+        if (!p)
+                return -ENOMEM;
+
+        found = manager_get_unit(m, p);
+        if (!found) {
+                *_found = NULL;
+                return 0;
+        }
+
+        *_found = found;
+        return 1;
+}
+
+Set *manager_get_units_requiring_mounts_for(Manager *m, const char *path) {
+        char p[strlen(path)+1];
+
+        assert(m);
+        assert(path);
+
+        strcpy(p, path);
+        path_kill_slashes(p);
+
+        return hashmap_get(m->units_requiring_mounts_for, streq(p, "/") ? "" : p);
+}
+
 void watch_init(Watch *w) {
         assert(w);
 
diff --git a/src/core/manager.h b/src/core/manager.h
index 3969553..a3049b5 100644
--- a/src/core/manager.h
+++ b/src/core/manager.h
@@ -103,9 +103,6 @@ struct Manager {
          * type we maintain a per type linked list */
         LIST_HEAD(Unit, units_by_type[_UNIT_TYPE_MAX]);
 
-        /* To optimize iteration of units that have requires_mounts_for set */
-        LIST_HEAD(Unit, has_requires_mounts_for);
-
         /* Units that need to be loaded */
         LIST_HEAD(Unit, load_queue); /* this is actually more a stack than a queue, but uh. */
 
@@ -251,6 +248,11 @@ struct Manager {
 
         char *switch_root;
         char *switch_root_init;
+
+        /* This maps all possible path prefixes to the units needing
+         * them. It's a hashmap with a path string as key and a Set as
+         * value where Unit objects are contained. */
+        Hashmap *units_requiring_mounts_for;
 };
 
 int manager_new(SystemdRunningAs running_as, bool reexecuting, Manager **m);
@@ -263,6 +265,8 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds);
 Job *manager_get_job(Manager *m, uint32_t id);
 Unit *manager_get_unit(Manager *m, const char *name);
 
+int manager_get_unit_by_path(Manager *m, const char *path, const char *suffix, Unit **_found);
+
 int manager_get_job_from_dbus_path(Manager *m, const char *s, Job **_j);
 
 int manager_load_unit_prepare(Manager *m, const char *name, const char *path, DBusError *e, Unit **_ret);
@@ -316,4 +320,6 @@ void manager_recheck_journal(Manager *m);
 void manager_set_show_status(Manager *m, bool b);
 void manager_status_printf(Manager *m, bool ephemeral, const char *status, const char *format, ...) _printf_attr_(4,5);
 
+Set *manager_get_units_requiring_mounts_for(Manager *m, const char *path);
+
 void watch_init(Watch *w);
diff --git a/src/core/mount.c b/src/core/mount.c
index 78c5c1e..3d46557 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -157,138 +157,58 @@ _pure_ static MountParameters* get_mount_parameters(Mount *m) {
 }
 
 static int mount_add_mount_links(Mount *m) {
-        Unit *other;
-        int r;
+        _cleanup_free_ char *parent = NULL;
         MountParameters *pm;
-
-        assert(m);
-
-        pm = get_mount_parameters_fragment(m);
-
-        /* Adds in links to other mount points that might lie below or
-         * above us in the hierarchy */
-
-        LIST_FOREACH(units_by_type, other, UNIT(m)->manager->units_by_type[UNIT_MOUNT]) {
-                Mount *n = MOUNT(other);
-                MountParameters *pn;
-
-                if (n == m)
-                        continue;
-
-                if (UNIT(n)->load_state != UNIT_LOADED)
-                        continue;
-
-                pn = get_mount_parameters_fragment(n);
-
-                if (path_startswith(m->where, n->where)) {
-
-                        if ((r = unit_add_dependency(UNIT(m), UNIT_AFTER, UNIT(n), true)) < 0)
-                                return r;
-
-                        if (pn)
-                                if ((r = unit_add_dependency(UNIT(m), UNIT_REQUIRES, UNIT(n), true)) < 0)
-                                        return r;
-
-                } else if (path_startswith(n->where, m->where)) {
-
-                        if ((r = unit_add_dependency(UNIT(n), UNIT_AFTER, UNIT(m), true)) < 0)
-                                return r;
-
-                        if (pm)
-                                if ((r = unit_add_dependency(UNIT(n), UNIT_REQUIRES, UNIT(m), true)) < 0)
-                                        return r;
-
-                } else if (pm && pm->what && path_startswith(pm->what, n->where)) {
-
-                        if ((r = unit_add_dependency(UNIT(m), UNIT_AFTER, UNIT(n), true)) < 0)
-                                return r;
-
-                        if ((r = unit_add_dependency(UNIT(m), UNIT_REQUIRES, UNIT(n), true)) < 0)
-                                return r;
-
-                } else if (pn && pn->what && path_startswith(pn->what, m->where)) {
-
-                        if ((r = unit_add_dependency(UNIT(n), UNIT_AFTER, UNIT(m), true)) < 0)
-                                return r;
-
-                        if ((r = unit_add_dependency(UNIT(n), UNIT_REQUIRES, UNIT(m), true)) < 0)
-                                return r;
-                }
-        }
-
-        return 0;
-}
-
-static int mount_add_swap_links(Mount *m) {
         Unit *other;
+        Iterator i;
+        Set *s;
         int r;
 
         assert(m);
 
-        LIST_FOREACH(units_by_type, other, UNIT(m)->manager->units_by_type[UNIT_SWAP]) {
-                r = swap_add_one_mount_link(SWAP(other), m);
+        if (!path_equal(m->where, "/")) {
+                /* Adds in links to other mount points that might lie further
+                 * up in the hierarchy */
+                r = path_get_parent(m->where, &parent);
                 if (r < 0)
                         return r;
-        }
 
-        return 0;
-}
-
-static int mount_add_path_links(Mount *m) {
-        Unit *other;
-        int r;
-
-        assert(m);
-
-        LIST_FOREACH(units_by_type, other, UNIT(m)->manager->units_by_type[UNIT_PATH]) {
-                r = path_add_one_mount_link(PATH(other), m);
+                r = unit_require_mounts_for(UNIT(m), parent);
                 if (r < 0)
                         return r;
         }
 
-        return 0;
-}
-
-static int mount_add_automount_links(Mount *m) {
-        Unit *other;
-        int r;
-
-        assert(m);
-
-        LIST_FOREACH(units_by_type, other, UNIT(m)->manager->units_by_type[UNIT_AUTOMOUNT]) {
-                r = automount_add_one_mount_link(AUTOMOUNT(other), m);
+        /* Adds in links to other mount points that might be needed
+         * for the source path (if this is a bind mount) to be
+         * available. */
+        pm = get_mount_parameters_fragment(m);
+        if (pm && path_is_absolute(pm->what)) {
+                r = unit_require_mounts_for(UNIT(m), pm->what);
                 if (r < 0)
                         return r;
         }
 
-        return 0;
-}
+        /* Adds in links to other units that use this path or paths
+         * further down in the hierarchy */
+        s = manager_get_units_requiring_mounts_for(UNIT(m)->manager, m->where);
+        SET_FOREACH(other, s, i) {
 
-static int mount_add_socket_links(Mount *m) {
-        Unit *other;
-        int r;
+                if (other->load_state != UNIT_LOADED)
+                        continue;
 
-        assert(m);
+                if (other == UNIT(m))
+                        continue;
 
-        LIST_FOREACH(units_by_type, other, UNIT(m)->manager->units_by_type[UNIT_SOCKET]) {
-                r = socket_add_one_mount_link(SOCKET(other), m);
+                r = unit_add_dependency(other, UNIT_AFTER, UNIT(m), true);
                 if (r < 0)
                         return r;
-        }
 
-        return 0;
-}
-
-static int mount_add_requires_mounts_links(Mount *m) {
-        Unit *other;
-        int r;
-
-        assert(m);
-
-        LIST_FOREACH(has_requires_mounts_for, other, UNIT(m)->manager->has_requires_mounts_for) {
-                r = unit_add_one_mount_link(other, m);
-                if (r < 0)
-                        return r;
+                if (UNIT(m)->fragment_path) {
+                        /* If we have fragment configuration, then make this dependency required */
+                        r = unit_add_dependency(other, UNIT_REQUIRES, UNIT(m), true);
+                        if (r < 0)
+                                return r;
+                }
         }
 
         return 0;
@@ -567,8 +487,9 @@ static int mount_fix_timeouts(Mount *m) {
 }
 
 static int mount_verify(Mount *m) {
+        _cleanup_free_ char *e = NULL;
         bool b;
-        char *e;
+
         assert(m);
 
         if (UNIT(m)->load_state != UNIT_LOADED)
@@ -577,12 +498,11 @@ static int mount_verify(Mount *m) {
         if (!m->from_fragment && !m->from_proc_self_mountinfo)
                 return -ENOENT;
 
-        if (!(e = unit_name_from_path(m->where, ".mount")))
+        e = unit_name_from_path(m->where, ".mount");
+        if (!e)
                 return -ENOMEM;
 
         b = unit_has_name(UNIT(m), e);
-        free(e);
-
         if (!b) {
                 log_error_unit(UNIT(m)->id,
                                "%s's Where setting doesn't match unit name. Refusing.",
@@ -646,26 +566,6 @@ static int mount_add_extras(Mount *m) {
         if (r < 0)
                 return r;
 
-        r = mount_add_socket_links(m);
-        if (r < 0)
-                return r;
-
-        r = mount_add_swap_links(m);
-        if (r < 0)
-                return r;
-
-        r = mount_add_path_links(m);
-        if (r < 0)
-                return r;
-
-        r = mount_add_requires_mounts_links(m);
-        if (r < 0)
-                return r;
-
-        r = mount_add_automount_links(m);
-        if (r < 0)
-                return r;
-
         r = mount_add_quota_links(m);
         if (r < 0)
                 return r;
@@ -1650,79 +1550,56 @@ fail:
 static int mount_load_proc_self_mountinfo(Manager *m, bool set_flags) {
         int r = 0;
         unsigned i;
-        char *device, *path, *options, *options2, *fstype, *d, *p, *o;
 
         assert(m);
 
         rewind(m->proc_self_mountinfo);
 
         for (i = 1;; i++) {
+                _cleanup_free_ char *device = NULL, *path = NULL, *options = NULL, *options2 = NULL, *fstype = NULL, *d = NULL, *p = NULL, *o = NULL;
                 int k;
 
-                device = path = options = options2 = fstype = d = p = o = NULL;
-
-                if ((k = fscanf(m->proc_self_mountinfo,
-                                "%*s "       /* (1) mount id */
-                                "%*s "       /* (2) parent id */
-                                "%*s "       /* (3) major:minor */
-                                "%*s "       /* (4) root */
-                                "%ms "       /* (5) mount point */
-                                "%ms"        /* (6) mount options */
-                                "%*[^-]"     /* (7) optional fields */
-                                "- "         /* (8) separator */
-                                "%ms "       /* (9) file system type */
-                                "%ms"        /* (10) mount source */
-                                "%ms"        /* (11) mount options 2 */
-                                "%*[^\n]",   /* some rubbish at the end */
-                                &path,
-                                &options,
-                                &fstype,
-                                &device,
-                                &options2)) != 5) {
-
-                        if (k == EOF)
-                                break;
-
+                k = fscanf(m->proc_self_mountinfo,
+                           "%*s "       /* (1) mount id */
+                           "%*s "       /* (2) parent id */
+                           "%*s "       /* (3) major:minor */
+                           "%*s "       /* (4) root */
+                           "%ms "       /* (5) mount point */
+                           "%ms"        /* (6) mount options */
+                           "%*[^-]"     /* (7) optional fields */
+                           "- "         /* (8) separator */
+                           "%ms "       /* (9) file system type */
+                           "%ms"        /* (10) mount source */
+                           "%ms"        /* (11) mount options 2 */
+                           "%*[^\n]",   /* some rubbish at the end */
+                           &path,
+                           &options,
+                           &fstype,
+                           &device,
+                           &options2);
+
+                if (k == EOF)
+                        break;
+
+                if (k != 5) {
                         log_warning("Failed to parse /proc/self/mountinfo:%u.", i);
-                        goto clean_up;
+                        continue;
                 }
 
                 o = strjoin(options, ",", options2, NULL);
-                if (!o) {
-                        r = -ENOMEM;
-                        goto finish;
-                }
+                if (!o)
+                        return log_oom();
 
-                if (!(d = cunescape(device)) ||
-                    !(p = cunescape(path))) {
-                        r = -ENOMEM;
-                        goto finish;
-                }
+                d = cunescape(device);
+                p = cunescape(path);
+                if (!d || !p)
+                        return log_oom();
 
-                if ((k = mount_add_one(m, d, p, o, fstype, 0, set_flags)) < 0)
+                k = mount_add_one(m, d, p, o, fstype, 0, set_flags);
+                if (k < 0)
                         r = k;
-
-clean_up:
-                free(device);
-                free(path);
-                free(options);
-                free(options2);
-                free(fstype);
-                free(d);
-                free(p);
-                free(o);
         }
 
-finish:
-        free(device);
-        free(path);
-        free(options);
-        free(options2);
-        free(fstype);
-        free(d);
-        free(p);
-        free(o);
-
         return r;
 }
 
diff --git a/src/core/path.c b/src/core/path.c
index 8a09deb..99e2fed 100644
--- a/src/core/path.c
+++ b/src/core/path.c
@@ -241,10 +241,6 @@ static bool path_spec_check_good(PathSpec *s, bool initial) {
         return good;
 }
 
-static bool path_spec_startswith(PathSpec *s, const char *what) {
-        return path_startswith(s->path, what);
-}
-
 static void path_spec_mkdir(PathSpec *s, mode_t mode) {
         int r;
 
@@ -301,38 +297,14 @@ static void path_done(Unit *u) {
         path_free_specs(p);
 }
 
-int path_add_one_mount_link(Path *p, Mount *m) {
+static int path_add_mount_links(Path *p) {
         PathSpec *s;
         int r;
 
         assert(p);
-        assert(m);
-
-        if (UNIT(p)->load_state != UNIT_LOADED ||
-            UNIT(m)->load_state != UNIT_LOADED)
-                return 0;
 
         LIST_FOREACH(spec, s, p->specs) {
-                if (!path_spec_startswith(s, m->where))
-                        continue;
-
-                r = unit_add_two_dependencies(UNIT(p), UNIT_AFTER, UNIT_REQUIRES,
-                                              UNIT(m), true);
-                if (r < 0)
-                        return r;
-        }
-
-        return 0;
-}
-
-static int path_add_mount_links(Path *p) {
-        Unit *other;
-        int r;
-
-        assert(p);
-
-        LIST_FOREACH(units_by_type, other, UNIT(p)->manager->units_by_type[UNIT_MOUNT]) {
-                r = path_add_one_mount_link(p, MOUNT(other));
+                r = unit_require_mounts_for(UNIT(p), s->path);
                 if (r < 0)
                         return r;
         }
diff --git a/src/core/path.h b/src/core/path.h
index 6adab58..dec3df7 100644
--- a/src/core/path.h
+++ b/src/core/path.h
@@ -90,10 +90,6 @@ struct Path {
         PathResult result;
 };
 
-/* Called from the mount code figure out if a mount is a dependency of
- * any of the paths of this path object */
-int path_add_one_mount_link(Path *p, Mount *m);
-
 void path_free_specs(Path *p);
 
 extern const UnitVTable path_vtable;
diff --git a/src/core/socket.c b/src/core/socket.c
index 190b36c..6c0ac1a 100644
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -258,53 +258,24 @@ static int socket_verify(Socket *s) {
         return 0;
 }
 
-static bool socket_needs_mount(Socket *s, const char *prefix) {
+static int socket_add_mount_links(Socket *s) {
         SocketPort *p;
-
-        assert(s);
-
-        LIST_FOREACH(port, p, s->ports) {
-
-                if (p->type == SOCKET_SOCKET) {
-                        if (socket_address_needs_mount(&p->address, prefix))
-                                return true;
-                } else if (p->type == SOCKET_FIFO || p->type == SOCKET_SPECIAL) {
-                        if (path_startswith(p->path, prefix))
-                                return true;
-                }
-        }
-
-        return false;
-}
-
-int socket_add_one_mount_link(Socket *s, Mount *m) {
         int r;
 
         assert(s);
-        assert(m);
 
-        if (UNIT(s)->load_state != UNIT_LOADED ||
-            UNIT(m)->load_state != UNIT_LOADED)
-                return 0;
-
-        if (!socket_needs_mount(s, m->where))
-                return 0;
-
-        r = unit_add_two_dependencies(UNIT(s), UNIT_AFTER, UNIT_REQUIRES, UNIT(m), true);
-        if (r < 0)
-                return r;
-
-        return 0;
-}
+        LIST_FOREACH(port, p, s->ports) {
+                const char *path = NULL;
 
-static int socket_add_mount_links(Socket *s) {
-        Unit *other;
-        int r;
+                if (p->type == SOCKET_SOCKET)
+                        path = socket_address_get_path(&p->address);
+                else if (p->type == SOCKET_FIFO || p->type == SOCKET_SPECIAL)
+                        path = p->path;
 
-        assert(s);
+                if (!path)
+                        continue;
 
-        LIST_FOREACH(units_by_type, other, UNIT(s)->manager->units_by_type[UNIT_MOUNT]) {
-                r = socket_add_one_mount_link(s, MOUNT(other));
+                r = unit_require_mounts_for(UNIT(s), path);
                 if (r < 0)
                         return r;
         }
diff --git a/src/core/socket.h b/src/core/socket.h
index 5733322..3d7eadc 100644
--- a/src/core/socket.h
+++ b/src/core/socket.h
@@ -156,10 +156,6 @@ struct Socket {
 /* Called from the service code when collecting fds */
 int socket_collect_fds(Socket *s, int **fds, unsigned *n_fds);
 
-/* Called from the mount code figure out if a mount is a dependency of
- * any of the sockets of this socket */
-int socket_add_one_mount_link(Socket *s, Mount *m);
-
 /* Called from the service code when a per-connection service ended */
 void socket_connection_unref(Socket *s);
 
diff --git a/src/core/swap.c b/src/core/swap.c
index dc6731a..a68ab7c 100644
--- a/src/core/swap.c
+++ b/src/core/swap.c
@@ -137,42 +137,6 @@ static void swap_done(Unit *u) {
         unit_unwatch_timer(u, &s->timer_watch);
 }
 
-int swap_add_one_mount_link(Swap *s, Mount *m) {
-         int r;
-
-        assert(s);
-        assert(m);
-
-        if (UNIT(s)->load_state != UNIT_LOADED ||
-            UNIT(m)->load_state != UNIT_LOADED)
-                return 0;
-
-        if (is_device_path(s->what))
-                return 0;
-
-        if (!path_startswith(s->what, m->where))
-                return 0;
-
-        r = unit_add_two_dependencies(UNIT(s), UNIT_AFTER, UNIT_REQUIRES, UNIT(m), true);
-        if (r < 0)
-                return r;
-
-        return 0;
-}
-
-static int swap_add_mount_links(Swap *s) {
-        Unit *other;
-        int r;
-
-        assert(s);
-
-        LIST_FOREACH(units_by_type, other, UNIT(s)->manager->units_by_type[UNIT_MOUNT])
-                if ((r = swap_add_one_mount_link(s, MOUNT(other))) < 0)
-                        return r;
-
-        return 0;
-}
-
 static int swap_add_device_links(Swap *s) {
         SwapParameters *p;
 
@@ -300,11 +264,11 @@ static int swap_load(Unit *u) {
                         if ((r = unit_set_description(u, s->what)) < 0)
                                 return r;
 
-                r = swap_add_device_links(s);
+                r = unit_require_mounts_for(UNIT(s), s->what);
                 if (r < 0)
                         return r;
 
-                r = swap_add_mount_links(s);
+                r = swap_add_device_links(s);
                 if (r < 0)
                         return r;
 
diff --git a/src/core/swap.h b/src/core/swap.h
index 7e48c0e..dd89535 100644
--- a/src/core/swap.h
+++ b/src/core/swap.h
@@ -107,8 +107,6 @@ struct Swap {
 
 extern const UnitVTable swap_vtable;
 
-int swap_add_one_mount_link(Swap *s, Mount *m);
-
 int swap_dispatch_reload(Manager *m);
 int swap_fd_event(Manager *m, int events);
 
diff --git a/src/core/unit.c b/src/core/unit.c
index ab313b9..4b97710 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -374,6 +374,34 @@ static void unit_remove_transient(Unit *u) {
         }
 }
 
+static void unit_free_requires_mounts_for(Unit *u) {
+        char **j;
+
+        STRV_FOREACH(j, u->requires_mounts_for) {
+                char s[strlen(*j) + 1];
+
+                PATH_FOREACH_PREFIX_MORE(s, *j) {
+                        char *y;
+                        Set *x;
+
+                        x = hashmap_get2(u->manager->units_requiring_mounts_for, s, (void**) &y);
+                        if (!x)
+                                continue;
+
+                        set_remove(x, u);
+
+                        if (set_isempty(x)) {
+                                hashmap_remove(u->manager->units_requiring_mounts_for, y);
+                                free(y);
+                                set_free(x);
+                        }
+                }
+        }
+
+        strv_free(u->requires_mounts_for);
+        u->requires_mounts_for = NULL;
+}
+
 void unit_free(Unit *u) {
         UnitDependency d;
         Iterator i;
@@ -390,6 +418,8 @@ void unit_free(Unit *u) {
                 if (UNIT_VTABLE(u)->done)
                         UNIT_VTABLE(u)->done(u);
 
+        unit_free_requires_mounts_for(u);
+
         SET_FOREACH(t, u->names, i)
                 hashmap_remove_value(u->manager->units, t, u);
 
@@ -408,11 +438,6 @@ void unit_free(Unit *u) {
         for (d = 0; d < _UNIT_DEPENDENCY_MAX; d++)
                 bidi_set_free(u, u->dependencies[d]);
 
-        if (u->requires_mounts_for) {
-                LIST_REMOVE(Unit, has_requires_mounts_for, u->manager->has_requires_mounts_for, u);
-                strv_free(u->requires_mounts_for);
-        }
-
         if (u->type != _UNIT_TYPE_INVALID)
                 LIST_REMOVE(Unit, units_by_type, u->manager->units_by_type[u->type], u);
 
@@ -2659,40 +2684,39 @@ void unit_ref_unset(UnitRef *ref) {
         ref->unit = NULL;
 }
 
-int unit_add_one_mount_link(Unit *u, Mount *m) {
+int unit_add_mount_links(Unit *u) {
         char **i;
+        int r;
 
         assert(u);
-        assert(m);
-
-        if (u->load_state != UNIT_LOADED ||
-            UNIT(m)->load_state != UNIT_LOADED)
-                return 0;
 
         STRV_FOREACH(i, u->requires_mounts_for) {
+                char prefix[strlen(*i) + 1];
 
-                if (UNIT(m) == u)
-                        continue;
+                PATH_FOREACH_PREFIX_MORE(prefix, *i) {
+                        Unit *m;
 
-                if (!path_startswith(*i, m->where))
-                        continue;
-
-                return unit_add_two_dependencies(u, UNIT_AFTER, UNIT_REQUIRES, UNIT(m), true);
-        }
-
-        return 0;
-}
+                        r = manager_get_unit_by_path(u->manager, prefix, ".mount", &m);
+                        if (r < 0)
+                                return r;
+                        if (r == 0)
+                                continue;
+                        if (m == u)
+                                continue;
 
-int unit_add_mount_links(Unit *u) {
-        Unit *other;
-        int r;
+                        if (m->load_state != UNIT_LOADED)
+                                continue;
 
-        assert(u);
+                        r = unit_add_dependency(u, UNIT_AFTER, m, true);
+                        if (r < 0)
+                                return r;
 
-        LIST_FOREACH(units_by_type, other, u->manager->units_by_type[UNIT_MOUNT]) {
-                r = unit_add_one_mount_link(u, MOUNT(other));
-                if (r < 0)
-                        return r;
+                        if (m->fragment_path) {
+                                r = unit_add_dependency(u, UNIT_REQUIRES, m, true);
+                                if (r < 0)
+                                        return r;
+                        }
+                }
         }
 
         return 0;
@@ -3012,6 +3036,86 @@ int unit_kill_context(
         return wait_for_exit;
 }
 
+int unit_require_mounts_for(Unit *u, const char *path) {
+        char prefix[strlen(path) + 1], *p;
+        int r;
+
+        assert(u);
+        assert(path);
+
+        /* Registers a unit for requiring a certain path and all its
+         * prefixes. We keep a simple array of these paths in the
+         * unit, since its usually short. However, we build a prefix
+         * table for all possible prefixes so that new appearing mount
+         * units can easily determine which units to make themselves a
+         * dependency of. */
+
+        p = strdup(path);
+        if (!p)
+                return -ENOMEM;
+
+        path_kill_slashes(p);
+
+        if (!path_is_absolute(p)) {
+                free(p);
+                return -EINVAL;
+        }
+
+        if (!path_is_safe(p)) {
+                free(p);
+                return -EPERM;
+        }
+
+        if (strv_contains(u->requires_mounts_for, p)) {
+                free(p);
+                return 0;
+        }
+
+        r = strv_push(&u->requires_mounts_for, p);
+        if (r < 0) {
+                free(p);
+                return r;
+        }
+
+        PATH_FOREACH_PREFIX_MORE(prefix, p) {
+                Set *x;
+
+                x = hashmap_get(u->manager->units_requiring_mounts_for, prefix);
+                if (!x) {
+                        char *q;
+
+                        if (!u->manager->units_requiring_mounts_for) {
+                                u->manager->units_requiring_mounts_for = hashmap_new(string_hash_func, string_compare_func);
+                                if (!u->manager->units_requiring_mounts_for)
+                                        return -ENOMEM;
+                        }
+
+                        q = strdup(prefix);
+                        if (!q)
+                                return -ENOMEM;
+
+                        x = set_new(NULL, NULL);
+                        if (!x) {
+                                free(q);
+                                return -ENOMEM;
+                        }
+
+                        r = hashmap_put(u->manager->units_requiring_mounts_for, q, x);
+                        if (r < 0) {
+                                free(q);
+                                set_free(x);
+                                return r;
+                        }
+                }
+
+                r = set_put(x, u);
+                if (r < 0)
+                        return r;
+        }
+
+        return 0;
+}
+
 static const char* const unit_active_state_table[_UNIT_ACTIVE_STATE_MAX] = {
         [UNIT_ACTIVE] = "active",
         [UNIT_RELOADING] = "reloading",
diff --git a/src/core/unit.h b/src/core/unit.h
index 0caea18..6dd750f 100644
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -173,7 +173,6 @@ struct Unit {
 
         /* Counterparts in the cgroup filesystem */
         char *cgroup_path;
-        bool cgroup_realized;
         CGroupControllerMask cgroup_mask;
 
         UnitRef slice;
@@ -255,6 +254,8 @@ struct Unit {
         bool no_gc:1;
 
         bool in_audit:1;
+
+        bool cgroup_realized:1;
 };
 
 struct UnitStatusMessageFormats {
@@ -589,7 +590,6 @@ void unit_ref_unset(UnitRef *ref);
 #define UNIT_DEREF(ref) ((ref).unit)
 #define UNIT_ISSET(ref) (!!(ref).unit)
 
-int unit_add_one_mount_link(Unit *u, Mount *m);
 int unit_add_mount_links(Unit *u);
 
 int unit_exec_context_defaults(Unit *u, ExecContext *c);
@@ -609,6 +609,8 @@ int unit_kill_context(Unit *u, KillContext *c, bool sigkill, pid_t main_pid, pid
 
 int unit_make_transient(Unit *u);
 
+int unit_require_mounts_for(Unit *u, const char *path);
+
 const char *unit_active_state_to_string(UnitActiveState i) _const_;
 UnitActiveState unit_active_state_from_string(const char *s) _pure_;
 
diff --git a/src/shared/socket-util.c b/src/shared/socket-util.c
index c583d3d..9224208 100644
--- a/src/shared/socket-util.c
+++ b/src/shared/socket-util.c
@@ -486,16 +486,16 @@ bool socket_address_is_netlink(const SocketAddress *a, const char *s) {
         return socket_address_equal(a, &b);
 }
 
-bool socket_address_needs_mount(const SocketAddress *a, const char *prefix) {
+const char* socket_address_get_path(const SocketAddress *a) {
         assert(a);
 
         if (socket_address_family(a) != AF_UNIX)
-                return false;
+                return NULL;
 
         if (a->sockaddr.un.sun_path[0] == 0)
-                return false;
+                return NULL;
 
-        return path_startswith(a->sockaddr.un.sun_path, prefix);
+        return a->sockaddr.un.sun_path;
 }
 
 bool socket_ipv6_is_supported(void) {
diff --git a/src/shared/socket-util.h b/src/shared/socket-util.h
index 7829a33..e0b85ad 100644
--- a/src/shared/socket-util.h
+++ b/src/shared/socket-util.h
@@ -92,7 +92,7 @@ int make_socket_fd(const char* address, int flags);
 
 bool socket_address_equal(const SocketAddress *a, const SocketAddress *b) _pure_;
 
-bool socket_address_needs_mount(const SocketAddress *a, const char *prefix);
+const char* socket_address_get_path(const SocketAddress *a);
 
 const char* socket_address_bind_ipv6_only_to_string(SocketAddressBindIPv6Only b) _const_;
 SocketAddressBindIPv6Only socket_address_bind_ipv6_only_from_string(const char *s) _pure_;

commit 6270c1bd8f83e9985458c63688f452be7626766f
Author: Lennart Poettering <lennart at poettering.net>
Date:   Thu Sep 26 20:03:20 2013 +0200

    unit-name: when escaping a path consider the empty path identical to the root dir

diff --git a/src/shared/unit-name.c b/src/shared/unit-name.c
index 8f6c28e..bc8094d 100644
--- a/src/shared/unit-name.c
+++ b/src/shared/unit-name.c
@@ -301,7 +301,7 @@ char *unit_name_path_escape(const char *f) {
 
         path_kill_slashes(p);
 
-        if (streq(p, "/")) {
+        if (streq(p, "/") || streq(p, "")) {
                 free(p);
                 return strdup("-");
         }

commit e203f7c3ad6a95dfa6179e30904ce0432ea0de91
Author: Lennart Poettering <lennart at poettering.net>
Date:   Thu Sep 26 19:58:33 2013 +0200

    util: properly handle the root dir in PATH_FOREACH_PREFIX
    
    Also add PATH_FOREACH_PREFIX_MORE which includes the specified dir
    itself in the iteration

diff --git a/src/shared/path-util.h b/src/shared/path-util.h
index 03f2cf2..0a42de7 100644
--- a/src/shared/path-util.h
+++ b/src/shared/path-util.h
@@ -52,5 +52,12 @@ int path_is_os_tree(const char *path);
 
 int find_binary(const char *name, char **filename);
 
+/* Iterates through the path prefixes of the specified path, going up
+ * the tree, to root. Also returns "" (and not "/"!) for the root
+ * directory. Excludes the specified directory itself */
 #define PATH_FOREACH_PREFIX(prefix, path) \
-        for (char *_slash = strrchr(path_kill_slashes(strcpy(prefix, path)), '/'); _slash && !(*_slash = 0); _slash = strrchr((prefix), '/'))
+        for (char *_slash = ({ path_kill_slashes(strcpy(prefix, path)); streq(prefix, "/") ? NULL : strrchr(prefix, '/'); }); _slash && !(*_slash = 0); _slash = strrchr((prefix), '/'))
+
+/* Same as PATH_FOREACH_PREFIX but also includes the specified path itself */
+#define PATH_FOREACH_PREFIX_MORE(prefix, path) \
+        for (char *_slash = ({ path_kill_slashes(strcpy(prefix, path)); if (streq(prefix, "/")) prefix[0] = 0; strrchr(prefix, 0); }); _slash && !(*_slash = 0); _slash = strrchr((prefix), '/'))
diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c
index e303e48..ed3b315 100644
--- a/src/test/test-path-util.c
+++ b/src/test/test-path-util.c
@@ -107,29 +107,55 @@ static void test_find_binary(void) {
 }
 
 static void test_prefixes(void) {
-        static const char* values[] = { "/a/b/c", "/a/b", "/a", "", NULL};
-        unsigned i = 0;
+        static const char* values[] = { "/a/b/c/d", "/a/b/c", "/a/b", "/a", "", NULL};
+        unsigned i;
         char s[PATH_MAX];
+        bool b;
 
-        PATH_FOREACH_PREFIX(s, "/a/b/c/d") {
+        i = 0;
+        PATH_FOREACH_PREFIX_MORE(s, "/a/b/c/d") {
                 log_error("---%s---", s);
                 assert_se(streq(s, values[i++]));
         }
+        assert_se(values[i] == NULL);
 
+        i = 1;
+        PATH_FOREACH_PREFIX(s, "/a/b/c/d") {
+                log_error("---%s---", s);
+                assert_se(streq(s, values[i++]));
+        }
         assert_se(values[i] == NULL);
 
         i = 0;
-        PATH_FOREACH_PREFIX(s, "////a////b////c///d///////")
+        PATH_FOREACH_PREFIX_MORE(s, "////a////b////c///d///////")
                 assert_se(streq(s, values[i++]));
+        assert_se(values[i] == NULL);
 
+        i = 1;
+        PATH_FOREACH_PREFIX(s, "////a////b////c///d///////")
+                assert_se(streq(s, values[i++]));
         assert_se(values[i] == NULL);
 
         PATH_FOREACH_PREFIX(s, "////")
+                assert_not_reached("Wut?");
+
+        b = false;
+        PATH_FOREACH_PREFIX_MORE(s, "////") {
+                assert_se(!b);
                 assert_se(streq(s, ""));
+                b = true;
+        }
+        assert_se(b);
 
         PATH_FOREACH_PREFIX(s, "")
                 assert_not_reached("wut?");
 
+        b = false;
+        PATH_FOREACH_PREFIX_MORE(s, "") {
+                assert(!b);
+                assert(streq(s, ""));
+                b = true;
+        }
 }
 
 int main(void) {

commit baa89da40a1d42242c9c62603501ada7e9e52613
Author: Lennart Poettering <lennart at poettering.net>
Date:   Thu Sep 26 19:57:58 2013 +0200

    cgroup: when referencing cgroup controller trees allow omission of the path

diff --git a/TODO b/TODO
index 0f2398a..4745321 100644
--- a/TODO
+++ b/TODO
@@ -56,6 +56,8 @@ CGroup Rework Completion:
 
 Features:
 
+* move config_parse_path_strv() out of conf-parser.c
+
 * libdsystemd-bus should expose utf8 validation calls
 
 * When using "systemd status" on a slice unit also show all messages
diff --git a/src/cgls/cgls.c b/src/cgls/cgls.c
index c3229ad..c689b5c 100644
--- a/src/cgls/cgls.c
+++ b/src/cgls/cgls.c
@@ -156,7 +156,9 @@ int main(int argc, char *argv[]) {
 
                 for (i = optind; i < argc; i++) {
                         int q;
-                        printf("%s:\n", argv[i]);
+
+                        fprintf(stdout, "%s:\n", argv[i]);
+                        fflush(stdout);
 
                         if (arg_machine)
                                 root = strjoin("machine/", arg_machine, "/", argv[i], NULL);
diff --git a/src/shared/cgroup-util.c b/src/shared/cgroup-util.c
index dc0fe85..f57f2b2 100644
--- a/src/shared/cgroup-util.c
+++ b/src/shared/cgroup-util.c
@@ -1003,19 +1003,28 @@ int cg_split_spec(const char *spec, char **controller, char **path) {
                 return -EINVAL;
         }
 
-        u = strdup(e+1);
-        if (!u) {
-                free(t);
-                return -ENOMEM;
-        }
-        if (!path_is_safe(u) ||
-            !path_is_absolute(u)) {
-                free(t);
-                free(u);
-                return -EINVAL;
-        }
+        if (streq(e+1, "")) {
+                u = strdup("/");
+                if (!u) {
+                        free(t);
+                        return -ENOMEM;
+                }
+        } else {
+                u = strdup(e+1);
+                if (!u) {
+                        free(t);
+                        return -ENOMEM;
+                }
 
-        path_kill_slashes(u);
+                if (!path_is_safe(u) ||
+                    !path_is_absolute(u)) {
+                        free(t);
+                        free(u);
+                        return -EINVAL;
+                }
+
+                path_kill_slashes(u);
+        }
 
         if (controller)
                 *controller = t;



More information about the systemd-commits mailing list