[systemd-commits] 5 commits - src/core src/journal src/login src/nspawn src/shared src/test

Lennart Poettering lennart at kemper.freedesktop.org
Mon Apr 22 19:14:30 PDT 2013


 src/core/main.c               |    2 
 src/core/unit.c               |   26 +++++-
 src/journal/journald-server.c |  162 +++++++++++++++++-------------------------
 src/login/logind-session.c    |   13 +++
 src/login/logind-user.c       |   12 ++-
 src/login/sd-login.c          |   35 ---------
 src/nspawn/nspawn.c           |   34 +++++---
 src/shared/cgroup-util.c      |  119 +++++++++++++++++++++++++++---
 src/shared/cgroup-util.h      |    5 +
 src/test/test-cgroup-util.c   |   78 ++++++++++++++++++--
 10 files changed, 319 insertions(+), 167 deletions(-)

New commits:
commit ae018d9bc900d6355dea4af05119b49c67945184
Author: Lennart Poettering <lennart at poettering.net>
Date:   Mon Apr 22 23:10:13 2013 -0300

    cgroup: make sure all our cgroup objects have a suffix and are properly escaped
    
    Session objects will now get the .session suffix, user objects the .user
    suffix, nspawn containers the .nspawn suffix.
    
    This also changes the user cgroups to be named after the numeric UID
    rather than the username, since this allows us the parse these paths
    standalone without requiring access to the cgroup file system.
    
    This also changes the mapping of instanced units to cgroups. Instead of
    mapping foo at bar.service to the cgroup path /user/foo at .service/bar we
    will now map it to /user/foo at .service/foo at bar.service, in order to
    ensure that all our objects are properly suffixed in the tree.

diff --git a/TODO b/TODO
index b53c2a5..9adec5e 100644
--- a/TODO
+++ b/TODO
@@ -32,8 +32,6 @@ Fedora 19:
 
 Features:
 
-* suffix/escape all our objects in the cgroup tree
-
 * see if we can fix https://bugs.freedesktop.org/show_bug.cgi?id=63672
   without dropping the location cache entirely.
 
diff --git a/src/core/unit.c b/src/core/unit.c
index 2525f49..5834009 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -1810,14 +1810,13 @@ static const char *resolve_template(Unit *u, const char *name, const char*path,
         if (u->instance)
                 s = unit_name_replace_instance(name, u->instance);
         else {
-                char *i;
+                _cleanup_free_ char *i = NULL;
 
                 i = unit_name_to_prefix(u->id);
                 if (!i)
                         return NULL;
 
                 s = unit_name_replace_instance(name, i);
-                free(i);
         }
 
         if (!s)
@@ -1972,15 +1971,30 @@ char *unit_default_cgroup_path(Unit *u) {
         assert(u);
 
         if (u->instance) {
-                _cleanup_free_ char *t = NULL;
+                _cleanup_free_ char *t = NULL, *escaped_template = NULL, *escaped_instance = NULL;
 
                 t = unit_name_template(u->id);
                 if (!t)
                         return NULL;
 
-                return strjoin(u->manager->cgroup_hierarchy, "/", t, "/", u->instance, NULL);
-        } else
-                return strjoin(u->manager->cgroup_hierarchy, "/", u->id, NULL);
+                escaped_template = cg_escape(t);
+                if (!escaped_template)
+                        return NULL;
+
+                escaped_instance = cg_escape(u->id);
+                if (!escaped_instance)
+                        return NULL;
+
+                return strjoin(u->manager->cgroup_hierarchy, "/", escaped_template, "/", escaped_instance, NULL);
+        } else {
+                _cleanup_free_ char *escaped = NULL;
+
+                escaped = cg_escape(u->id);
+                if (!escaped)
+                        return NULL;
+
+                return strjoin(u->manager->cgroup_hierarchy, "/", escaped, NULL);
+        }
 }
 
 int unit_add_cgroup_from_text(Unit *u, const char *name, bool overwrite, CGroupBonding **ret) {
diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c
index 7e7b379..4134e9f 100644
--- a/src/journal/journald-server.c
+++ b/src/journal/journald-server.c
@@ -31,10 +31,6 @@
 #include <systemd/sd-messages.h>
 #include <systemd/sd-daemon.h>
 
-#ifdef HAVE_LOGIND
-#include <systemd/sd-login.h>
-#endif
-
 #include "fileio.h"
 #include "mkdir.h"
 #include "hashmap.h"
@@ -505,26 +501,26 @@ static void dispatch_message_real(
                 const char *label, size_t label_len,
                 const char *unit_id) {
 
-        char pid[sizeof("_PID=") + DECIMAL_STR_MAX(ucred->pid)],
-                uid[sizeof("_UID=") + DECIMAL_STR_MAX(ucred->uid)],
-                gid[sizeof("_GID=") + DECIMAL_STR_MAX(ucred->gid)],
+        char pid[sizeof("_PID=") + DECIMAL_STR_MAX(pid_t)],
+                uid[sizeof("_UID=") + DECIMAL_STR_MAX(uid_t)],
+                gid[sizeof("_GID=") + DECIMAL_STR_MAX(gid_t)],
+                owner_uid[sizeof("_SYSTEMD_OWNER_UID=") + DECIMAL_STR_MAX(uid_t)],
                 source_time[sizeof("_SOURCE_REALTIME_TIMESTAMP=") + DECIMAL_STR_MAX(usec_t)],
                 boot_id[sizeof("_BOOT_ID=") + 32] = "_BOOT_ID=",
                 machine_id[sizeof("_MACHINE_ID=") + 32] = "_MACHINE_ID=";
-
-        _cleanup_free_ char *comm = NULL, *cmdline = NULL, *hostname = NULL,
-                *exe = NULL, *cgroup = NULL, *session = NULL,
-                *owner_uid = NULL, *unit = NULL, *selinux_context = NULL;
-
-#ifdef HAVE_AUDIT
-        _cleanup_free_ char *audit_session = NULL, *audit_loginuid = NULL;
-#endif
-
+        char *comm, *exe, *cmdline, *cgroup, *session, *unit, *selinux_context, *hostname;
         sd_id128_t id;
         int r;
-        char *t;
+        char *t, *c;
         uid_t realuid = 0, owner = 0, journal_uid;
         bool owner_valid = false;
+#ifdef HAVE_AUDIT
+        char audit_session[sizeof("_AUDIT_SESSION=") + DECIMAL_STR_MAX(uint32_t)],
+                audit_loginuid[sizeof("_AUDIT_LOGINUID=") + DECIMAL_STR_MAX(uid_t)];
+
+        uint32_t audit;
+        uid_t loginuid;
+#endif
 
         assert(s);
         assert(iovec);
@@ -532,130 +528,111 @@ static void dispatch_message_real(
         assert(n + N_IOVEC_META_FIELDS <= m);
 
         if (ucred) {
-#ifdef HAVE_AUDIT
-                uint32_t audit;
-                uid_t loginuid;
-#endif
-
                 realuid = ucred->uid;
 
-                snprintf(pid, sizeof(pid) - 1, "_PID=%lu", (unsigned long) ucred->pid);
-                char_array_0(pid);
+                sprintf(pid, "_PID=%lu", (unsigned long) ucred->pid);
                 IOVEC_SET_STRING(iovec[n++], pid);
 
-                snprintf(uid, sizeof(uid) - 1, "_UID=%lu", (unsigned long) ucred->uid);
-                char_array_0(uid);
+                sprintf(uid, "_UID=%lu", (unsigned long) ucred->uid);
                 IOVEC_SET_STRING(iovec[n++], uid);
 
-                snprintf(gid, sizeof(gid) - 1, "_GID=%lu", (unsigned long) ucred->gid);
-                char_array_0(gid);
+                sprintf(gid, "_GID=%lu", (unsigned long) ucred->gid);
                 IOVEC_SET_STRING(iovec[n++], gid);
 
                 r = get_process_comm(ucred->pid, &t);
                 if (r >= 0) {
-                        comm = strappend("_COMM=", t);
+                        comm = strappenda("_COMM=", t);
                         free(t);
-
-                        if (comm)
-                                IOVEC_SET_STRING(iovec[n++], comm);
+                        IOVEC_SET_STRING(iovec[n++], comm);
                 }
 
                 r = get_process_exe(ucred->pid, &t);
                 if (r >= 0) {
-                        exe = strappend("_EXE=", t);
+                        exe = strappenda("_EXE=", t);
                         free(t);
-
-                        if (exe)
-                                IOVEC_SET_STRING(iovec[n++], exe);
+                        IOVEC_SET_STRING(iovec[n++], exe);
                 }
 
                 r = get_process_cmdline(ucred->pid, 0, false, &t);
                 if (r >= 0) {
-                        cmdline = strappend("_CMDLINE=", t);
+                        cmdline = strappenda("_CMDLINE=", t);
                         free(t);
-
-                        if (cmdline)
-                                IOVEC_SET_STRING(iovec[n++], cmdline);
+                        IOVEC_SET_STRING(iovec[n++], cmdline);
                 }
 
 #ifdef HAVE_AUDIT
                 r = audit_session_from_pid(ucred->pid, &audit);
-                if (r >= 0)
-                        if (asprintf(&audit_session, "_AUDIT_SESSION=%lu", (unsigned long) audit) >= 0)
-                                IOVEC_SET_STRING(iovec[n++], audit_session);
+                if (r >= 0) {
+                        sprintf(audit_session, "_AUDIT_SESSION=%lu", (unsigned long) audit);
+                        IOVEC_SET_STRING(iovec[n++], audit_session);
+                }
 
                 r = audit_loginuid_from_pid(ucred->pid, &loginuid);
-                if (r >= 0)
-                        if (asprintf(&audit_loginuid, "_AUDIT_LOGINUID=%lu", (unsigned long) loginuid) >= 0)
-                                IOVEC_SET_STRING(iovec[n++], audit_loginuid);
-#endif
-
-                r = cg_pid_get_path(NULL, ucred->pid, &t);
                 if (r >= 0) {
-                        cgroup = strappend("_SYSTEMD_CGROUP=", t);
-                        free(t);
-
-                        if (cgroup)
-                                IOVEC_SET_STRING(iovec[n++], cgroup);
+                        sprintf(audit_loginuid, "_AUDIT_LOGINUID=%lu", (unsigned long) loginuid);
+                        IOVEC_SET_STRING(iovec[n++], audit_loginuid);
                 }
+#endif
 
-#ifdef HAVE_LOGIND
-                r = cg_pid_get_session(ucred->pid, &t);
+                r = cg_pid_get_path_shifted(ucred->pid, NULL, &c);
                 if (r >= 0) {
-                        session = strappend("_SYSTEMD_SESSION=", t);
-                        free(t);
+                        cgroup = strappenda("_SYSTEMD_CGROUP=", c);
+                        IOVEC_SET_STRING(iovec[n++], cgroup);
 
-                        if (session)
+                        r = cg_path_get_session(c, &t);
+                        if (r >= 0) {
+                                session = strappenda("_SYSTEMD_SESSION=", t);
+                                free(t);
                                 IOVEC_SET_STRING(iovec[n++], session);
-                }
+                        }
 
-                if (sd_pid_get_owner_uid(ucred->pid, &owner) >= 0) {
-                        owner_valid = true;
-                        if (asprintf(&owner_uid, "_SYSTEMD_OWNER_UID=%lu", (unsigned long) owner) >= 0)
+                        if (cg_path_get_owner_uid(c, &owner) >= 0) {
+                                owner_valid = true;
+
+                                sprintf(owner_uid, "_SYSTEMD_OWNER_UID=%lu", (unsigned long) owner);
                                 IOVEC_SET_STRING(iovec[n++], owner_uid);
-                }
-#endif
+                        }
 
-                if (cg_pid_get_unit(ucred->pid, &t) >= 0) {
-                        unit = strappend("_SYSTEMD_UNIT=", t);
-                        free(t);
-                } else if (cg_pid_get_user_unit(ucred->pid, &t) >= 0) {
-                        unit = strappend("_SYSTEMD_USER_UNIT=", t);
-                        free(t);
-                } else if (unit_id) {
-                        if (session)
-                                unit = strappend("_SYSTEMD_USER_UNIT=", unit_id);
-                        else
-                                unit = strappend("_SYSTEMD_UNIT=", unit_id);
+                        if (cg_path_get_unit(c, &t) >= 0) {
+                                unit = strappenda("_SYSTEMD_UNIT=", t);
+                                free(t);
+                        } else if (cg_path_get_user_unit(c, &t) >= 0) {
+                                unit = strappenda("_SYSTEMD_USER_UNIT=", t);
+                                free(t);
+                        } else if (unit_id) {
+                                if (session)
+                                        unit = strappenda("_SYSTEMD_USER_UNIT=", unit_id);
+                                else
+                                        unit = strappenda("_SYSTEMD_UNIT=", unit_id);
+                        } else
+                                unit = NULL;
+
+                        if (unit)
+                                IOVEC_SET_STRING(iovec[n++], unit);
+
+                        free(c);
                 }
 
-                if (unit)
-                        IOVEC_SET_STRING(iovec[n++], unit);
-
 #ifdef HAVE_SELINUX
                 if (label) {
-                        selinux_context = malloc(sizeof("_SELINUX_CONTEXT=") + label_len);
-                        if (selinux_context) {
-                                *((char*) mempcpy(stpcpy(selinux_context, "_SELINUX_CONTEXT="), label, label_len)) = 0;
-                                IOVEC_SET_STRING(iovec[n++], selinux_context);
-                        }
+                        selinux_context = alloca(sizeof("_SELINUX_CONTEXT=") + label_len);
+
+                        *((char*) mempcpy(stpcpy(selinux_context, "_SELINUX_CONTEXT="), label, label_len)) = 0;
+                        IOVEC_SET_STRING(iovec[n++], selinux_context);
                 } else {
                         security_context_t con;
 
                         if (getpidcon(ucred->pid, &con) >= 0) {
-                                selinux_context = strappend("_SELINUX_CONTEXT=", con);
-                                if (selinux_context)
-                                        IOVEC_SET_STRING(iovec[n++], selinux_context);
+                                selinux_context = strappenda("_SELINUX_CONTEXT=", con);
                                 freecon(con);
+                                IOVEC_SET_STRING(iovec[n++], selinux_context);
                         }
                 }
 #endif
         }
 
         if (tv) {
-                snprintf(source_time, sizeof(source_time) - 1, "_SOURCE_REALTIME_TIMESTAMP=%llu",
-                         (unsigned long long) timeval_load(tv));
-                char_array_0(source_time);
+                sprintf(source_time, "_SOURCE_REALTIME_TIMESTAMP=%llu", (unsigned long long) timeval_load(tv));
                 IOVEC_SET_STRING(iovec[n++], source_time);
         }
 
@@ -676,10 +653,9 @@ static void dispatch_message_real(
 
         t = gethostname_malloc();
         if (t) {
-                hostname = strappend("_HOSTNAME=", t);
+                hostname = strappenda("_HOSTNAME=", t);
                 free(t);
-                if (hostname)
-                        IOVEC_SET_STRING(iovec[n++], hostname);
+                IOVEC_SET_STRING(iovec[n++], hostname);
         }
 
         assert(n <= m);
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 2f7ab34..662273b 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -465,7 +465,18 @@ static int session_create_cgroup(Session *s) {
         assert(s->user->cgroup_path);
 
         if (!s->cgroup_path) {
-                if (asprintf(&p, "%s/%s", s->user->cgroup_path, s->id) < 0)
+                _cleanup_free_ char *name = NULL, *escaped = NULL;
+
+                name = strappend(s->id, ".session");
+                if (!name)
+                        return log_oom();
+
+                escaped = cg_escape(name);
+                if (!escaped)
+                        return log_oom();
+
+                p = strjoin(s->user->cgroup_path, "/", escaped, NULL);
+                if (!p)
                         return log_oom();
         } else
                 p = s->cgroup_path;
diff --git a/src/login/logind-user.c b/src/login/logind-user.c
index 4b0ac5e..9e2cbf6 100644
--- a/src/login/logind-user.c
+++ b/src/login/logind-user.c
@@ -315,7 +315,17 @@ static int user_create_cgroup(User *u) {
         assert(u);
 
         if (!u->cgroup_path) {
-                if (asprintf(&p, "%s/%s", u->manager->cgroup_path, u->name) < 0)
+                _cleanup_free_ char *name = NULL, *escaped = NULL;
+
+                if (asprintf(&name, "%lu.user", (unsigned long) u->uid) < 0)
+                        return log_oom();
+
+                escaped = cg_escape(name);
+                if (!escaped)
+                        return log_oom();
+
+                p = strjoin(u->manager->cgroup_path, "/", escaped, NULL);
+                if (!p)
                         return log_oom();
         } else
                 p = u->cgroup_path;
diff --git a/src/login/sd-login.c b/src/login/sd-login.c
index 30e07a9..157b7e0 100644
--- a/src/login/sd-login.c
+++ b/src/login/sd-login.c
@@ -73,10 +73,6 @@ _public_ int sd_pid_get_machine_name(pid_t pid, char **name) {
 }
 
 _public_ int sd_pid_get_owner_uid(pid_t pid, uid_t *uid) {
-        int r;
-        _cleanup_free_ char *root = NULL, *cgroup = NULL, *cc = NULL;
-        char *p;
-        struct stat st;
 
         if (pid < 0)
                 return -EINVAL;
@@ -84,36 +80,7 @@ _public_ int sd_pid_get_owner_uid(pid_t pid, uid_t *uid) {
         if (!uid)
                 return -EINVAL;
 
-        r = cg_pid_get_path_shifted(pid, &root, &cgroup);
-        if (r < 0)
-                return r;
-
-        if (!startswith(cgroup, "/user/"))
-                return -ENOENT;
-
-        p = strchr(cgroup + 6, '/');
-        if (!p)
-                return -ENOENT;
-
-        p++;
-        p += strcspn(p, "/");
-        *p = 0;
-
-        r = cg_get_path(SYSTEMD_CGROUP_CONTROLLER, root, cgroup, &cc);
-
-        if (r < 0)
-                return -ENOMEM;
-
-        r = lstat(cc, &st);
-
-        if (r < 0)
-                return -errno;
-
-        if (!S_ISDIR(st.st_mode))
-                return -ENOTDIR;
-
-        *uid = st.st_uid;
-        return 0;
+        return cg_pid_get_owner_uid(pid, uid);
 }
 
 _public_ int sd_uid_get_state(uid_t uid, char**state) {
diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 1e7df38..7b41571 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -1212,7 +1212,7 @@ finish:
 int main(int argc, char *argv[]) {
         pid_t pid = 0;
         int r = EXIT_FAILURE, k;
-        _cleanup_free_ char *machine_root = NULL, *newcg = NULL;
+        _cleanup_free_ char *machine_root = NULL, *name = NULL, *escaped = NULL, *newcg = NULL;
         _cleanup_close_ int master = -1;
         int n_fd_passed;
         const char *console = NULL;
@@ -1298,9 +1298,21 @@ int main(int argc, char *argv[]) {
                 goto finish;
         }
 
-        newcg = strjoin(machine_root, "/", arg_machine, ".nspawn", NULL);
+        name = strappend(arg_machine, ".nspawn");
+        if (!name) {
+                log_oom();
+                goto finish;
+        }
+
+        escaped = cg_escape(name);
+        if (!escaped) {
+                log_oom();
+                goto finish;
+        }
+
+        newcg = strjoin(machine_root, "/", escaped, NULL);
         if (!newcg) {
-                log_error("Failed to allocate cgroup path.");
+                log_oom();
                 goto finish;
         }
 
diff --git a/src/shared/cgroup-util.c b/src/shared/cgroup-util.c
index e54b946..9ec4f40 100644
--- a/src/shared/cgroup-util.c
+++ b/src/shared/cgroup-util.c
@@ -1197,8 +1197,8 @@ int cg_pid_get_path_shifted(pid_t pid, char **root, char **cgroup) {
         return 0;
 }
 
-/* non-static only for testing purposes */
 int cg_path_decode_unit(const char *cgroup, char **unit){
+        _cleanup_free_ char *unescaped = NULL;
         char *p, *e, *c, *s, *k;
 
         assert(cgroup);
@@ -1206,6 +1206,7 @@ int cg_path_decode_unit(const char *cgroup, char **unit){
 
         e = strchrnul(cgroup, '/');
         c = strndupa(cgroup, e - cgroup);
+        c = cg_unescape(c);
 
         /* Could this be a valid unit name? */
         if (!unit_name_is_valid(c, true))
@@ -1218,15 +1219,15 @@ int cg_path_decode_unit(const char *cgroup, char **unit){
                         return -EINVAL;
 
                 e += strspn(e, "/");
+
                 p = strchrnul(e, '/');
+                k = strndupa(e, p - e);
+                k = cg_unescape(k);
 
-                /* Don't allow empty instance strings */
-                if (p == e)
+                if (!unit_name_is_valid(k, false))
                         return -EINVAL;
 
-                k = strndupa(e, p - e);
-
-                s = unit_name_replace_instance(c, k);
+                s = strdup(k);
         }
 
         if (!s)
@@ -1320,7 +1321,7 @@ int cg_pid_get_user_unit(pid_t pid, char **unit) {
 
 int cg_path_get_machine_name(const char *path, char **machine) {
         const char *e, *n;
-        char *s, *dot;
+        char *s, *r;
 
         assert(path);
         assert(machine);
@@ -1333,15 +1334,13 @@ int cg_path_get_machine_name(const char *path, char **machine) {
         if (e == n)
                 return -ENOENT;
 
-        s = strndup(e, n - e);
-        if (!s)
-                return -ENOMEM;
+        s = strndupa(e, n - e);
 
-        dot = strrchr(s, '.');
-        if (dot)
-                *dot = 0;
+        r = strdup(cg_unescape(s));
+        if (!r)
+                return -ENOMEM;
 
-        *machine = s;
+        *machine = r;
         return 0;
 }
 
@@ -1375,13 +1374,12 @@ int cg_path_get_session(const char *path, char **session) {
                 return -ENOENT;
 
         n = strchrnul(e, '/');
-        if (e == n)
+        if (n - e < 8)
                 return -ENOENT;
-
-        if (n - e == 6 && memcmp(e, "shared", 6) == 0)
+        if (memcmp(n - 8, ".session", 8) != 0)
                 return -ENOENT;
 
-        s = strndup(e, n - e);
+        s = strndup(e, n - e - 8);
         if (!s)
                 return -ENOMEM;
 
@@ -1402,6 +1400,43 @@ int cg_pid_get_session(pid_t pid, char **session) {
         return cg_path_get_session(cgroup, session);
 }
 
+int cg_path_get_owner_uid(const char *path, uid_t *uid) {
+        const char *e, *n;
+        char *s;
+
+        assert(path);
+        assert(uid);
+
+        e = path_startswith(path, "/user/");
+        if (!e)
+                return -ENOENT;
+
+        n = strchrnul(e, '/');
+        if (n - e < 5)
+                return -ENOENT;
+        if (memcmp(n - 5, ".user", 5) != 0)
+                return -ENOENT;
+
+        s = strndupa(e, n - e - 5);
+        if (!s)
+                return -ENOMEM;
+
+        return parse_uid(s, uid);
+}
+
+int cg_pid_get_owner_uid(pid_t pid, uid_t *uid) {
+        _cleanup_free_ char *cgroup = NULL;
+        int r;
+
+        assert(uid);
+
+        r = cg_pid_get_path_shifted(pid, NULL, &cgroup);
+        if (r < 0)
+                return r;
+
+        return cg_path_get_owner_uid(cgroup, uid);
+}
+
 int cg_controller_from_attr(const char *attr, char **controller) {
         const char *dot;
         char *c;
@@ -1430,3 +1465,55 @@ int cg_controller_from_attr(const char *attr, char **controller) {
         *controller = c;
         return 1;
 }
+
+char *cg_escape(const char *p) {
+        bool need_prefix = false;
+
+        /* This implements very minimal escaping for names to be used
+         * as file names in the cgroup tree: any name which might
+         * conflict with a kernel name or is prefixed with '_' is
+         * prefixed with a '_'. That way, when reading cgroup names it
+         * is sufficient to remove a single prefixing underscore if
+         * there is one. */
+
+        /* The return value of this function (unlike cg_unescape())
+         * needs free()! */
+
+        if (p[0] == '_' || streq(p, "notify_on_release") || streq(p, "release_agent") || streq(p, "tasks"))
+                need_prefix = true;
+        else {
+                const char *dot;
+
+                dot = strrchr(p, '.');
+                if (dot) {
+
+                        if (dot - p == 6 && memcmp(p, "cgroup", 6) == 0)
+                                need_prefix = true;
+                        else {
+                                char *n;
+
+                                n = strndupa(p, dot - p);
+
+                                if (check_hierarchy(n) >= 0)
+                                        need_prefix = true;
+                        }
+                }
+        }
+
+        if (need_prefix)
+                return strappend("_", p);
+        else
+                return strdup(p);
+}
+
+char *cg_unescape(const char *p) {
+        assert(p);
+
+        /* The return value of this function (unlike cg_escape())
+         * doesn't need free()! */
+
+        if (p[0] == '_')
+                return (char*) p+1;
+
+        return (char*) p;
+}
diff --git a/src/shared/cgroup-util.h b/src/shared/cgroup-util.h
index 5457d1b..2099f93 100644
--- a/src/shared/cgroup-util.h
+++ b/src/shared/cgroup-util.h
@@ -75,6 +75,7 @@ int cg_get_user_path(char **path);
 int cg_get_machine_path(char **path);
 
 int cg_path_get_session(const char *path, char **session);
+int cg_path_get_owner_uid(const char *path, uid_t *uid);
 int cg_path_get_unit(const char *path, char **unit);
 int cg_path_get_user_unit(const char *path, char **unit);
 int cg_path_get_machine_name(const char *path, char **machine);
@@ -82,6 +83,7 @@ int cg_path_get_machine_name(const char *path, char **machine);
 int cg_pid_get_path_shifted(pid_t pid, char **root, char **cgroup);
 
 int cg_pid_get_session(pid_t pid, char **session);
+int cg_pid_get_owner_uid(pid_t pid, uid_t *uid);
 int cg_pid_get_unit(pid_t pid, char **unit);
 int cg_pid_get_user_unit(pid_t pid, char **unit);
 int cg_pid_get_machine_name(pid_t pid, char **machine);
@@ -91,3 +93,6 @@ int cg_path_decode_unit(const char *cgroup, char **unit);
 char **cg_shorten_controllers(char **controllers);
 
 int cg_controller_from_attr(const char *attr, char **controller);
+
+char *cg_escape(const char *p);
+char *cg_unescape(const char *p);
diff --git a/src/test/test-cgroup-util.c b/src/test/test-cgroup-util.c
index f632df1..95cede7 100644
--- a/src/test/test-cgroup-util.c
+++ b/src/test/test-cgroup-util.c
@@ -32,8 +32,8 @@ static void check_p_d_u(const char *path, int code, const char *result) {
 }
 
 static void test_path_decode_unit(void) {
-        check_p_d_u("getty at .service/tty2", 0, "getty at tty2.service");
-        check_p_d_u("getty at .service/tty2/xxx", 0, "getty at tty2.service");
+        check_p_d_u("getty at .service/getty at tty2.service", 0, "getty at tty2.service");
+        check_p_d_u("getty at .service/getty at tty2.service/xxx", 0, "getty at tty2.service");
         check_p_d_u("getty at .service/", -EINVAL, NULL);
         check_p_d_u("getty at .service", -EINVAL, NULL);
         check_p_d_u("getty.service", 0, "getty.service");
@@ -56,9 +56,9 @@ static void check_p_g_u_u(const char *path, int code, const char *result) {
 
 static void test_path_get_unit(void) {
         check_p_g_u("/system/foobar.service/sdfdsaf", 0, "foobar.service");
-        check_p_g_u("/system/getty at .service/tty5", 0, "getty at tty5.service");
-        check_p_g_u("/system/getty at .service/tty5/aaa/bbb", 0, "getty at tty5.service");
-        check_p_g_u("/system/getty at .service/tty5/", 0, "getty at tty5.service");
+        check_p_g_u("/system/getty at .service/getty at tty5.service", 0, "getty at tty5.service");
+        check_p_g_u("/system/getty at .service/getty at tty5.service/aaa/bbb", 0, "getty at tty5.service");
+        check_p_g_u("/system/getty at .service/getty at tty5.service/", 0, "getty at tty5.service");
         check_p_g_u("/system/getty at tty6.service/tty5", 0, "getty at tty6.service");
         check_p_g_u("sadfdsafsda", -ENOENT, NULL);
         check_p_g_u("/system/getty####@tty6.service/tty5", -EINVAL, NULL);
@@ -70,7 +70,7 @@ static void test_path_get_user_unit(void) {
         check_p_g_u_u("/user/lennart/2/systemd-21548/foobar.service/waldo/uuuux", 0, "foobar.service");
         check_p_g_u_u("/user/lennart/2/systemd-21548/waldo/waldo/uuuux", -EINVAL, NULL);
         check_p_g_u_u("/user/lennart/2/foobar.service", -ENOENT, NULL);
-        check_p_g_u_u("/user/lennart/2/systemd-21548/foobar at .service/pie/pa/po", 0, "foobar at pie.service");
+        check_p_g_u_u("/user/lennart/2/systemd-21548/foobar at .service/foobar at pie.service/pa/po", 0, "foobar at pie.service");
 }
 
 static void test_get_paths(void) {
@@ -100,6 +100,7 @@ static void test_proc(void) {
         FOREACH_DIRENT(de, d, break) {
                 _cleanup_free_ char *path = NULL, *path_shifted = NULL, *session = NULL, *unit = NULL, *user_unit = NULL, *machine = NULL, *prefix = NULL;
                 pid_t pid;
+                uid_t uid = (uid_t) -1;
 
                 if (de->d_type != DT_DIR &&
                     de->d_type != DT_UNKNOWN)
@@ -109,18 +110,23 @@ static void test_proc(void) {
                 if (r < 0)
                         continue;
 
+                if (is_kernel_thread(pid))
+                        continue;
+
                 cg_pid_get_path(SYSTEMD_CGROUP_CONTROLLER, pid, &path);
                 cg_pid_get_path_shifted(pid, &prefix, &path_shifted);
+                cg_pid_get_owner_uid(pid, &uid);
                 cg_pid_get_session(pid, &session);
                 cg_pid_get_unit(pid, &unit);
                 cg_pid_get_user_unit(pid, &user_unit);
                 cg_pid_get_machine_name(pid, &machine);
 
-                printf("%lu\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n",
+                printf("%lu\t%s\t%s\t%s\t%lu\t%s\t%s\t%s\t%s\n",
                        (unsigned long) pid,
                        path,
                        prefix,
                        path_shifted,
+                       (unsigned long) uid,
                        session,
                        unit,
                        user_unit,
@@ -128,12 +134,32 @@ static void test_proc(void) {
         }
 }
 
+static void test_escape_one(const char *s, const char *r) {
+        _cleanup_free_ char *b;
+
+        b = cg_escape(s);
+        assert_se(b);
+        assert_se(streq(b, r));
+
+        assert_se(streq(cg_unescape(b), s));
+}
+
+static void test_escape(void) {
+        test_escape_one("foobar", "foobar");
+        test_escape_one("foobar.service", "foobar.service");
+        test_escape_one("cgroup.service", "_cgroup.service");
+        test_escape_one("cpu.service", "_cpu.service");
+        test_escape_one("tasks", "_tasks");
+        test_escape_one("_foobar", "__foobar");
+}
+
 int main(void) {
         test_path_decode_unit();
         test_path_get_unit();
         test_path_get_user_unit();
         test_get_paths();
         test_proc();
+        test_escape();
 
         return 0;
 }

commit cd8f53ab1601513cc3407447bfe3359ee7139676
Author: Lennart Poettering <lennart at poettering.net>
Date:   Mon Apr 22 23:09:02 2013 -0300

    core: there's no point to complain so loudly about non-isolatable boot targets

diff --git a/src/core/main.c b/src/core/main.c
index ce8ec99..6a981a0 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -1683,7 +1683,7 @@ int main(int argc, char *argv[]) {
 
                 r = manager_add_job(m, JOB_START, target, JOB_ISOLATE, false, &error, &default_unit_job);
                 if (r == -EPERM) {
-                        log_error("Default target could not be isolated, starting instead: %s", bus_error(&error, r));
+                        log_debug("Default target could not be isolated, starting instead: %s", bus_error(&error, r));
                         dbus_error_free(&error);
 
                         r = manager_add_job(m, JOB_START, target, JOB_REPLACE, false, &error, &default_unit_job);

commit aff38e74bd776471f15ba54b305a24b0251eb865
Author: Lennart Poettering <lennart at poettering.net>
Date:   Mon Apr 22 17:26:06 2013 -0300

    nspawn: suffix the nspawn cgroups with ".nspawn"
    
    As discussed with Dan Berrange it's a good idea to suffix all objects in
    the cgroup tree with ".something", so that when the system is
    partitioned using a resource management tool we can drop objects of
    different types into the same partition directory without generate
    namespace conflicts.
    
    We'l add this to the Pax Control Group document as soon as write access
    to the fdo wiki is restored.

diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 0a5a151..1e7df38 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -1298,7 +1298,7 @@ int main(int argc, char *argv[]) {
                 goto finish;
         }
 
-        newcg = strjoin(machine_root, "/", arg_machine, NULL);
+        newcg = strjoin(machine_root, "/", arg_machine, ".nspawn", NULL);
         if (!newcg) {
                 log_error("Failed to allocate cgroup path.");
                 goto finish;
diff --git a/src/shared/cgroup-util.c b/src/shared/cgroup-util.c
index 5d44342..e54b946 100644
--- a/src/shared/cgroup-util.c
+++ b/src/shared/cgroup-util.c
@@ -1320,7 +1320,7 @@ int cg_pid_get_user_unit(pid_t pid, char **unit) {
 
 int cg_path_get_machine_name(const char *path, char **machine) {
         const char *e, *n;
-        char *s;
+        char *s, *dot;
 
         assert(path);
         assert(machine);
@@ -1337,6 +1337,10 @@ int cg_path_get_machine_name(const char *path, char **machine) {
         if (!s)
                 return -ENOMEM;
 
+        dot = strrchr(s, '.');
+        if (dot)
+                *dot = 0;
+
         *machine = s;
         return 0;
 }
diff --git a/src/test/test-cgroup-util.c b/src/test/test-cgroup-util.c
index 5eaa129..f632df1 100644
--- a/src/test/test-cgroup-util.c
+++ b/src/test/test-cgroup-util.c
@@ -89,11 +89,51 @@ static void test_get_paths(void) {
         log_info("Machine = %s", d);
 }
 
+static void test_proc(void) {
+        _cleanup_closedir_ DIR *d = NULL;
+        struct dirent *de;
+        int r;
+
+        d = opendir("/proc");
+        assert_se(d);
+
+        FOREACH_DIRENT(de, d, break) {
+                _cleanup_free_ char *path = NULL, *path_shifted = NULL, *session = NULL, *unit = NULL, *user_unit = NULL, *machine = NULL, *prefix = NULL;
+                pid_t pid;
+
+                if (de->d_type != DT_DIR &&
+                    de->d_type != DT_UNKNOWN)
+                        continue;
+
+                r = parse_pid(de->d_name, &pid);
+                if (r < 0)
+                        continue;
+
+                cg_pid_get_path(SYSTEMD_CGROUP_CONTROLLER, pid, &path);
+                cg_pid_get_path_shifted(pid, &prefix, &path_shifted);
+                cg_pid_get_session(pid, &session);
+                cg_pid_get_unit(pid, &unit);
+                cg_pid_get_user_unit(pid, &user_unit);
+                cg_pid_get_machine_name(pid, &machine);
+
+                printf("%lu\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n",
+                       (unsigned long) pid,
+                       path,
+                       prefix,
+                       path_shifted,
+                       session,
+                       unit,
+                       user_unit,
+                       machine);
+        }
+}
+
 int main(void) {
         test_path_decode_unit();
         test_path_get_unit();
         test_path_get_user_unit();
         test_get_paths();
+        test_proc();
 
         return 0;
 }

commit dc2c75602dc9f2529e6ba6db02fa53d057ce0f8c
Author: Lennart Poettering <lennart at poettering.net>
Date:   Mon Apr 22 17:11:05 2013 -0300

    nspawn: always use cg_get_path() to determine fs path for a cgroup

diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index b59b267..0a5a151 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -923,7 +923,8 @@ static int setup_cgroup(const char *path) {
 }
 
 static int save_attributes(const char *cgroup, pid_t pid, const char *uuid, const char *directory) {
-        char buf[DECIMAL_STR_MAX(pid_t)], path[PATH_MAX];
+        _cleanup_free_ char *path = NULL;
+        char buf[DECIMAL_STR_MAX(pid_t)];
         int r = 0, k;
 
         assert(cgroup);
@@ -933,10 +934,10 @@ static int save_attributes(const char *cgroup, pid_t pid, const char *uuid, cons
 #ifdef HAVE_XATTR
         assert_se(snprintf(buf, sizeof(buf), "%lu", (unsigned long) pid) < (int) sizeof(buf));
 
-        r = snprintf(path, sizeof(path), "/sys/fs/cgroup/systemd/%s", cgroup);
-        if (r >= (int) sizeof(path)) {
-                log_error("cgroup name too long");
-                return -EINVAL;
+        r = cg_get_path(SYSTEMD_CGROUP_CONTROLLER, cgroup, NULL, &path);
+        if (r < 0) {
+                log_error("Failed to get path: %s", strerror(-r));
+                return r;
         }
 
         r = setxattr(path, "trusted.init_pid", buf, strlen(buf), XATTR_CREATE);
@@ -954,7 +955,7 @@ static int save_attributes(const char *cgroup, pid_t pid, const char *uuid, cons
 
         k = setxattr(path, "trusted.root_directory", directory, strlen(directory), XATTR_CREATE);
         if (k < 0) {
-                log_warning("Failed to set %s attribute on %s: %m", "trusted.machine_id", path);
+                log_warning("Failed to set %s attribute on %s: %m", "trusted.root_directory", path);
                 if (r == 0)
                         r = k;
         }
@@ -1457,8 +1458,7 @@ int main(int argc, char *argv[]) {
                         if (setup_cgroup(newcg) < 0)
                                 goto child_fail;
 
-                        close_nointr_nofail(pipefd2[1]);
-                        close_nointr_nofail(pipefd2[0]);
+                        close_pipe(pipefd2);
 
                         /* Mark everything as slave, so that we still
                          * receive mounts from the real root, but don't

commit 8fc66914c068e6fa0e1c610b9cdbef429e469a88
Author: Lennart Poettering <lennart at poettering.net>
Date:   Mon Apr 22 17:03:59 2013 -0300

    update TODO

diff --git a/TODO b/TODO
index 9adec5e..b53c2a5 100644
--- a/TODO
+++ b/TODO
@@ -32,6 +32,8 @@ Fedora 19:
 
 Features:
 
+* suffix/escape all our objects in the cgroup tree
+
 * see if we can fix https://bugs.freedesktop.org/show_bug.cgi?id=63672
   without dropping the location cache entirely.
 



More information about the systemd-commits mailing list