[systemd-commits] 4 commits - fixme Makefile.am man/systemd.service.xml src/cgroup-util.c src/load-fragment.c src/log.c src/pam-module.c src/test-env-replace.c src/util.c units/emergency.service units/fedora units/.gitignore

Lennart Poettering lennart at kemper.freedesktop.org
Tue Jul 20 19:33:33 PDT 2010


 Makefile.am                 |   12 ++++
 fixme                       |    4 -
 man/systemd.service.xml     |   12 +++-
 src/cgroup-util.c           |   20 ++++++--
 src/load-fragment.c         |  109 ++++++++++++++++++++++----------------------
 src/log.c                   |    2 
 src/pam-module.c            |   34 ++++++++++---
 src/test-env-replace.c      |   59 +++++++++++++++++++++++
 src/util.c                  |   51 +++++++++++++++++---
 units/.gitignore            |    1 
 units/emergency.service     |    2 
 units/fedora/single.service |    2 
 12 files changed, 224 insertions(+), 84 deletions(-)

New commits:
commit 672c48cc065251ddbdc3926221e8a7718241cccd
Author: Lennart Poettering <lennart at poettering.net>
Date:   Wed Jul 21 04:32:44 2010 +0200

    pam: remove only sessions we ourselves created in the first place

diff --git a/src/cgroup-util.c b/src/cgroup-util.c
index 828b304..2f3fcb5 100644
--- a/src/cgroup-util.c
+++ b/src/cgroup-util.c
@@ -25,6 +25,8 @@
 #include <string.h>
 #include <stdlib.h>
 #include <dirent.h>
+#include <sys/stat.h>
+#include <sys/types.h>
 
 #include "cgroup-util.h"
 #include "log.h"
@@ -546,7 +548,17 @@ int cg_create(const char *controller, const char *path) {
         if ((r = cg_get_path(controller, path, NULL, &fs)) < 0)
                 return r;
 
-        r = mkdir_p(fs, 0755);
+        r = mkdir_parents(fs, 0755);
+
+        if (r >= 0) {
+                if (mkdir(fs, 0755) >= 0)
+                        r = 1;
+                else if (errno == EEXIST)
+                        r = 0;
+                else
+                        r = -errno;
+        }
+
         free(fs);
 
         return r;
@@ -577,7 +589,7 @@ int cg_attach(const char *controller, const char *path, pid_t pid) {
 }
 
 int cg_create_and_attach(const char *controller, const char *path, pid_t pid) {
-        int r;
+        int r, q;
 
         assert(controller);
         assert(path);
@@ -586,8 +598,8 @@ int cg_create_and_attach(const char *controller, const char *path, pid_t pid) {
         if ((r = cg_create(controller, path)) < 0)
                 return r;
 
-        if ((r = cg_attach(controller, path, pid)) < 0)
-                return r;
+        if ((q = cg_attach(controller, path, pid)) < 0)
+                return q;
 
         /* This does not remove the cgroup on failure */
 
diff --git a/src/log.c b/src/log.c
index e67a5b3..28bfb9a 100644
--- a/src/log.c
+++ b/src/log.c
@@ -279,7 +279,7 @@ static int write_to_syslog(
         msghdr.msg_iov = iovec;
         msghdr.msg_iovlen = ELEMENTSOF(iovec);
 
-        if (sendmsg(syslog_fd, &msghdr, 0) < 0)
+        if (sendmsg(syslog_fd, &msghdr, MSG_NOSIGNAL) < 0)
                 return -errno;
 
         return 1;
diff --git a/src/pam-module.c b/src/pam-module.c
index e5652ac..f668c3b 100644
--- a/src/pam-module.c
+++ b/src/pam-module.c
@@ -208,7 +208,7 @@ static int get_user_data(
         return PAM_SUCCESS;
 }
 
-static int create_user_group(pam_handle_t *handle, const char *group, struct passwd *pw, bool attach) {
+static int create_user_group(pam_handle_t *handle, const char *group, struct passwd *pw, bool attach, bool remember) {
         int r;
 
         assert(handle);
@@ -224,6 +224,17 @@ static int create_user_group(pam_handle_t *handle, const char *group, struct pas
                 return PAM_SESSION_ERR;
         }
 
+        if (r > 0 && remember) {
+                /* Remember that it was us who created this group, and
+                 * that hence we need to remove it too. This is a
+                 * protection against removing the cgroup when run
+                 * recursively. */
+                if ((r = pam_set_data(handle, "systemd.created", INT_TO_PTR(1), NULL)) != PAM_SUCCESS) {
+                        pam_syslog(handle, LOG_ERR, "Failed to install created variable.");
+                        return r;
+                }
+        }
+
         if ((r = cg_set_task_access(SYSTEMD_CGROUP_CONTROLLER, group, 0755, pw->pw_uid, pw->pw_gid)) < 0 ||
             (r = cg_set_group_access(SYSTEMD_CGROUP_CONTROLLER, group, 0755, pw->pw_uid, pw->pw_gid)) < 0) {
                 pam_syslog(handle, LOG_ERR, "Failed to change access modes: %s", strerror(-r));
@@ -247,7 +258,7 @@ _public_ PAM_EXTERN int pam_sm_open_session(
 
         assert(handle);
 
-        pam_syslog(handle, LOG_INFO, "pam-systemd initializing");
+        /* pam_syslog(handle, LOG_DEBUG, "pam-systemd initializing"); */
 
         if (parse_argv(handle, argc, argv, &create_session, NULL, NULL) < 0)
                 return PAM_SESSION_ERR;
@@ -331,7 +342,9 @@ _public_ PAM_EXTERN int pam_sm_open_session(
                 goto finish;
         }
 
-        if ((r = create_user_group(handle, buf, pw, true)) != PAM_SUCCESS)
+        pam_syslog(handle, LOG_INFO, "Moving new user session for %s into control group %s.", username, buf);
+
+        if ((r = create_user_group(handle, buf, pw, true, true)) != PAM_SUCCESS)
                 goto finish;
 
         r = PAM_SUCCESS;
@@ -383,6 +396,7 @@ _public_ PAM_EXTERN int pam_sm_close_session(
         char *session_path = NULL, *nosession_path = NULL, *user_path = NULL;
         const char *id;
         struct passwd *pw;
+        const void *created = NULL;
 
         assert(handle);
 
@@ -411,7 +425,9 @@ _public_ PAM_EXTERN int pam_sm_close_session(
                 goto finish;
         }
 
-        if ((id = pam_getenv(handle, "XDG_SESSION_ID"))) {
+        pam_get_data(handle, "systemd.created", &created);
+
+        if ((id = pam_getenv(handle, "XDG_SESSION_ID")) && created) {
 
                 if (asprintf(&session_path, "/user/%s/%s", username, id) < 0 ||
                     asprintf(&nosession_path, "/user/%s/no-session", username) < 0) {
@@ -420,15 +436,19 @@ _public_ PAM_EXTERN int pam_sm_close_session(
                 }
 
                 if (kill_session)  {
+                        pam_syslog(handle, LOG_INFO, "Killing remaining processes of user session %s of %s.", id, username);
+
                         /* Kill processes in session cgroup, and delete it */
                         if ((r = cg_kill_recursive_and_wait(SYSTEMD_CGROUP_CONTROLLER, session_path, true)) < 0)
                                 pam_syslog(handle, LOG_ERR, "Failed to kill session cgroup: %s", strerror(-r));
                 } else {
+                        pam_syslog(handle, LOG_INFO, "Moving remaining processes of user session %s of %s into control group %s.", id, username, nosession_path);
+
                         /* Migrate processes from session to
                          * no-session cgroup. First, try to create the
                          * no-session group in case it doesn't exist
                          * yet. Also, delete the session group. */
-                        create_user_group(handle, nosession_path, pw, 0);
+                        create_user_group(handle, nosession_path, pw, false, false);
 
                         if ((r = cg_migrate_recursive(SYSTEMD_CGROUP_CONTROLLER, session_path, nosession_path, false, true)) < 0)
                                 pam_syslog(handle, LOG_ERR, "Failed to migrate session cgroup: %s", strerror(-r));
@@ -465,13 +485,13 @@ _public_ PAM_EXTERN int pam_sm_close_session(
         if (r >= 0) {
                 const char *runtime_dir;
 
-                /* This will migrate us to the /user cgroup. */
-
                 if ((runtime_dir = pam_getenv(handle, "XDG_RUNTIME_DIR")))
                         if ((r = rm_rf(runtime_dir, false, true)) < 0)
                                 pam_syslog(handle, LOG_ERR, "Failed to remove runtime directory: %s", strerror(-r));
         }
 
+        /* pam_syslog(handle, LOG_DEBUG, "pam-systemd done"); */
+
         r = PAM_SUCCESS;
 
 finish:
commit 294d81f124568983323a53f40bfbaa371f0da077
Author: Lennart Poettering <lennart at poettering.net>
Date:   Wed Jul 21 03:28:10 2010 +0200

    load: make sure that unit files in /etc/ always take precedence, even over link targets, to make them easily overrdiable

diff --git a/fixme b/fixme
index ddb4a6b..a0d2db2 100644
--- a/fixme
+++ b/fixme
@@ -37,8 +37,6 @@
 
 * place /etc/inittab with explaining blurb.
 
-* /etc must always take precedence even if we follow symlinks!
-
 * vielleicht implizit immer auf syslog dependen?
 
 * debian deadlock when partition auf noauto is.
diff --git a/src/load-fragment.c b/src/load-fragment.c
index cd66b2d..98f16f9 100644
--- a/src/load-fragment.c
+++ b/src/load-fragment.c
@@ -1780,68 +1780,69 @@ finish:
 
 int unit_load_fragment(Unit *u) {
         int r;
+        Iterator i;
+        const char *t;
 
         assert(u);
+        assert(u->meta.load_state == UNIT_STUB);
+        assert(u->meta.id);
 
-        if (u->meta.fragment_path) {
+        /* First, try to find the unit under its id. We always look
+         * for unit files in the default directories, to make it easy
+         * to override things by placing things in /etc/systemd/system */
+        if ((r = load_from_path(u, u->meta.id)) < 0)
+                return r;
+
+        /* Try to find an alias we can load this with */
+        if (u->meta.load_state == UNIT_STUB)
+                SET_FOREACH(t, u->meta.names, i) {
+
+                        if (t == u->meta.id)
+                                continue;
+
+                        if ((r = load_from_path(u, t)) < 0)
+                                return r;
+
+                        if (u->meta.load_state != UNIT_STUB)
+                                break;
+                }
 
+        /* And now, try looking for it under the suggested (originally linked) path */
+        if (u->meta.load_state == UNIT_STUB && u->meta.fragment_path)
                 if ((r = load_from_path(u, u->meta.fragment_path)) < 0)
                         return r;
 
-        } else {
-                Iterator i;
-                const char *t;
+        /* Look for a template */
+        if (u->meta.load_state == UNIT_STUB && u->meta.instance) {
+                char *k;
+
+                if (!(k = unit_name_template(u->meta.id)))
+                        return -ENOMEM;
+
+                r = load_from_path(u, k);
+                free(k);
 
-                /* Try to find the unit under its id */
-                if ((r = load_from_path(u, u->meta.id)) < 0)
+                if (r < 0)
                         return r;
 
-                /* Try to find an alias we can load this with */
                 if (u->meta.load_state == UNIT_STUB)
                         SET_FOREACH(t, u->meta.names, i) {
 
                                 if (t == u->meta.id)
                                         continue;
 
-                                if ((r = load_from_path(u, t)) < 0)
+                                if (!(k = unit_name_template(t)))
+                                        return -ENOMEM;
+
+                                r = load_from_path(u, k);
+                                free(k);
+
+                                if (r < 0)
                                         return r;
 
                                 if (u->meta.load_state != UNIT_STUB)
                                         break;
                         }
-
-                /* Now, follow the same logic, but look for a template */
-                if (u->meta.load_state == UNIT_STUB && u->meta.instance) {
-                        char *k;
-
-                        if (!(k = unit_name_template(u->meta.id)))
-                                return -ENOMEM;
-
-                        r = load_from_path(u, k);
-                        free(k);
-
-                        if (r < 0)
-                                return r;
-
-                        if (u->meta.load_state == UNIT_STUB)
-                                SET_FOREACH(t, u->meta.names, i) {
-
-                                        if (t == u->meta.id)
-                                                continue;
-
-                                        if (!(k = unit_name_template(t)))
-                                                return -ENOMEM;
-
-                                        r = load_from_path(u, k);
-                                        free(k);
-
-                                        if (r < 0)
-                                                return r;
-
-                                        if (u->meta.load_state != UNIT_STUB)
-                                                break;
-                                }
-                }
         }
 
         return 0;
commit 8f05424d50045ffd85912e4df36618e8aabd8988
Author: Lennart Poettering <lennart at poettering.net>
Date:   Wed Jul 21 03:13:15 2010 +0200

    unit: allow symlinking unit files to /dev/null

diff --git a/fixme b/fixme
index 29a6db4..ddb4a6b 100644
--- a/fixme
+++ b/fixme
@@ -35,8 +35,6 @@
 
 * systemctl status $PID, systemctl stop $PID!
 
-/dev/null symlinks supporten
-
 * place /etc/inittab with explaining blurb.
 
 * /etc must always take precedence even if we follow symlinks!
diff --git a/src/load-fragment.c b/src/load-fragment.c
index 5000810..cd66b2d 100644
--- a/src/load-fragment.c
+++ b/src/load-fragment.c
@@ -1312,16 +1312,20 @@ static int open_follow(char **filename, FILE **_f, Set *names, char **_final) {
                 path_kill_slashes(*filename);
 
                 /* Add the file name we are currently looking at to
-                 * the names of this unit */
+                 * the names of this unit, but only if it is a valid
+                 * unit name. */
                 name = file_name_from_path(*filename);
-                if (!(id = set_get(names, name))) {
 
-                        if (!(id = strdup(name)))
-                                return -ENOMEM;
+                if (unit_name_is_valid(name)) {
+                        if (!(id = set_get(names, name))) {
 
-                        if ((r = set_put(names, id)) < 0) {
-                                free(id);
-                                return r;
+                                if (!(id = strdup(name)))
+                                        return -ENOMEM;
+
+                                if ((r = set_put(names, id)) < 0) {
+                                        free(id);
+                                        return r;
+                                }
                         }
                 }
 
@@ -1340,7 +1344,7 @@ static int open_follow(char **filename, FILE **_f, Set *names, char **_final) {
                 *filename = target;
         }
 
-        if (!(f = fdopen(fd, "r"))) {
+        if (!(f = fdopen(fd, "re"))) {
                 r = -errno;
                 close_nointr_nofail(fd);
                 return r;
@@ -1730,6 +1734,7 @@ static int load_from_path(Unit *u, const char *path) {
         }
 
         if (!filename) {
+                /* Hmm, no suitable file found? */
                 r = 0;
                 goto finish;
         }
@@ -1750,11 +1755,6 @@ static int load_from_path(Unit *u, const char *path) {
                 goto finish;
         }
 
-        if (!S_ISREG(st.st_mode)) {
-                r = -ENOENT;
-                goto finish;
-        }
-
         /* Now, parse the file contents */
         if ((r = config_parse(filename, f, sections, items, false, u)) < 0)
                 goto finish;
commit c24eb49e6aecd6de2ad450083e826d4c9d9c75b6
Author: Lennart Poettering <lennart at poettering.net>
Date:   Wed Jul 21 02:57:35 2010 +0200

    exec: extend variable substitution to support splitting variable values into seperate arguments

diff --git a/Makefile.am b/Makefile.am
index 94ae6af..4dcecc5 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -77,7 +77,8 @@ noinst_PROGRAMS = \
 	test-ns \
 	test-loopback \
 	test-daemon \
-	test-cgroup
+	test-cgroup \
+	test-env-replace
 
 if HAVE_PAM
 pamlib_LTLIBRARIES = \
@@ -446,6 +447,15 @@ test_cgroup_CFLAGS = \
 test_cgroup_LDADD = \
 	libsystemd-basic.la
 
+test_env_replace_SOURCES = \
+	src/test-env-replace.c
+
+test_env_replace_CFLAGS = \
+	$(AM_CFLAGS)
+
+test_env_replace_LDADD = \
+	libsystemd-basic.la
+
 systemd_logger_SOURCES = \
 	src/logger.c \
 	src/sd-daemon.c \
diff --git a/fixme b/fixme
index 5737863..29a6db4 100644
--- a/fixme
+++ b/fixme
@@ -35,9 +35,9 @@
 
 * systemctl status $PID, systemctl stop $PID!
 
-* place /etc/inittab with explaining blurb.
+/dev/null symlinks supporten
 
-* In command lines, support both "$FOO" and $FOO
+* place /etc/inittab with explaining blurb.
 
 * /etc must always take precedence even if we follow symlinks!
 
diff --git a/man/systemd.service.xml b/man/systemd.service.xml
index 9623883..94a21d3 100644
--- a/man/systemd.service.xml
+++ b/man/systemd.service.xml
@@ -274,10 +274,14 @@
                                 <citerefentry><refentrytitle>systemd.unit</refentrytitle><manvolnum>5</manvolnum></citerefentry>. On
                                 top of that basic environment variable
                                 substitution is supported, where
-                                <literal>$(FOO)</literal> is replaced
-                                by the value of the environment
-                                variable of the same
-                                name.</para></listitem>
+                                <literal>${FOO}</literal> is replaced
+                                by the string value of the environment
+                                variable of the same name. Also
+                                <literal>$FOO</literal> may appear as
+                                seperate word on the command line in
+                                which case the variable is replaced by
+                                its value split at
+                                whitespaces.</para></listitem>
                         </varlistentry>
 
                         <varlistentry>
diff --git a/src/test-env-replace.c b/src/test-env-replace.c
new file mode 100644
index 0000000..df101fd
--- /dev/null
+++ b/src/test-env-replace.c
@@ -0,0 +1,59 @@
+/*-*- Mode: C; c-basic-offset: 8 -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2010 Lennart Poettering
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU General Public License as published by
+  the Free Software Foundation; either version 2 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  General Public License for more details.
+
+  You should have received a copy of the GNU General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include <unistd.h>
+#include <string.h>
+
+#include "util.h"
+#include "log.h"
+#include "strv.h"
+
+int main(int argc, char *argv[]) {
+
+        const char *env[] = {
+                "FOO=BAR BAR",
+                "BAR=waldo",
+                NULL
+        };
+
+        const char *line[] = {
+                "FOO$FOO",
+                "FOO$FOOFOO",
+                "FOO${FOO}$FOO",
+                "FOO${FOO}",
+                "${FOO}",
+                "$FOO",
+                "$FOO$FOO",
+                "${FOO}${BAR}",
+                "${FOO",
+                NULL
+        };
+
+        char **i, **r;
+
+        r = replace_env_argv((char**) line, (char**) env);
+
+        STRV_FOREACH(i, r)
+                printf("%s\n", *i);
+
+        strv_free(r);
+
+}
diff --git a/src/util.c b/src/util.c
index 45de609..da8a6c3 100644
--- a/src/util.c
+++ b/src/util.c
@@ -2778,7 +2778,7 @@ void status_welcome(void) {
 char *replace_env(const char *format, char **env) {
         enum {
                 WORD,
-                DOLLAR,
+                CURLY,
                 VARIABLE
         } state = WORD;
 
@@ -2793,11 +2793,11 @@ char *replace_env(const char *format, char **env) {
 
                 case WORD:
                         if (*e == '$')
-                                state = DOLLAR;
+                                state = CURLY;
                         break;
 
-                case DOLLAR:
-                        if (*e == '(') {
+                case CURLY:
+                        if (*e == '{') {
                                 if (!(k = strnappend(r, word, e-word-1)))
                                         goto fail;
 
@@ -2821,7 +2821,7 @@ char *replace_env(const char *format, char **env) {
                         break;
 
                 case VARIABLE:
-                        if (*e == ')') {
+                        if (*e == '}') {
                                 char *t;
 
                                 if ((t = strv_env_get_with_length(env, word+2, e-word-2))) {
@@ -2853,12 +2853,49 @@ fail:
 
 char **replace_env_argv(char **argv, char **env) {
         char **r, **i;
-        unsigned k = 0;
+        unsigned k = 0, l = 0;
+
+        l = strv_length(argv);
 
-        if (!(r = new(char*, strv_length(argv)+1)))
+        if (!(r = new(char*, l+1)))
                 return NULL;
 
         STRV_FOREACH(i, argv) {
+
+                /* If $FOO appears as single word, replace it by the split up variable */
+                if ((*i)[0] == '$') {
+                        char *e = strv_env_get(env, *i+1);
+
+                        if (e) {
+                                char **w, **m;
+                                unsigned q;
+
+                                if (!(m = strv_split_quoted(e))) {
+                                        r[k] = NULL;
+                                        strv_free(r);
+                                        return NULL;
+                                }
+
+                                q = strv_length(m);
+                                l = l + q - 1;
+
+                                if (!(w = realloc(r, sizeof(char*) * (l+1)))) {
+                                        r[k] = NULL;
+                                        strv_free(r);
+                                        strv_free(m);
+                                        return NULL;
+                                }
+
+                                r = w;
+                                memcpy(r + k, m, q * sizeof(char*));
+                                free(m);
+
+                                k += q;
+                                continue;
+                        }
+                }
+
+                /* If ${FOO} appears as part of a word, replace it by the variable as-is */
                 if (!(r[k++] = replace_env(*i, env))) {
                         strv_free(r);
                         return NULL;
diff --git a/units/.gitignore b/units/.gitignore
index ea85dc0..5de1b5c 100644
--- a/units/.gitignore
+++ b/units/.gitignore
@@ -6,3 +6,4 @@ graphical.target
 multi-user.target
 getty at .service
 remote-fs.target
+test-env-replace
diff --git a/units/emergency.service b/units/emergency.service
index dd4a070..7a1f81a 100644
--- a/units/emergency.service
+++ b/units/emergency.service
@@ -22,4 +22,4 @@ KillMode=process-group
 
 # Bash ignores SIGTERM, so we send SIGHUP first, to ensure that bash
 # terminates cleanly.
-ExecStop=-/bin/kill -HUP $(MAINPID)
+ExecStop=-/bin/kill -HUP ${MAINPID}
diff --git a/units/fedora/single.service b/units/fedora/single.service
index f4af539..93a70cf 100644
--- a/units/fedora/single.service
+++ b/units/fedora/single.service
@@ -25,4 +25,4 @@ KillMode=process-group
 
 # Bash ignores SIGTERM, so we send SIGHUP first, to ensure that bash
 # terminates cleanly.
-ExecStop=-/bin/kill -HUP $(MAINPID)
+ExecStop=-/bin/kill -HUP ${MAINPID}


More information about the systemd-commits mailing list