[systemd-commits] 4 commits - man/journald.conf.xml src/core src/cryptsetup src/fsck src/fstab-generator src/journal src/login src/modules-load src/quotacheck src/shared

Zbigniew Jędrzejewski-Szmek zbyszek at kemper.freedesktop.org
Mon Feb 17 08:31:30 CET 2014


 man/journald.conf.xml                 |    5 
 src/core/main.c                       |  117 +++++++--------
 src/core/shutdown.c                   |  132 +++++++++++++----
 src/cryptsetup/cryptsetup-generator.c |  260 ++++++++++++++++------------------
 src/fsck/fsck.c                       |   48 ++----
 src/fstab-generator/fstab-generator.c |   49 ++----
 src/journal/journald-console.c        |    7 
 src/login/logind-session.c            |    3 
 src/modules-load/modules-load.c       |   38 +---
 src/quotacheck/quotacheck.c           |   43 +----
 src/shared/log.c                      |    8 +
 src/shared/log.h                      |    2 
 src/shared/util.c                     |   29 +++
 src/shared/util.h                     |    1 
 14 files changed, 397 insertions(+), 345 deletions(-)

New commits:
commit b1e90ec515408aec2702522f6f68c4920b56375b
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Sat Feb 15 18:13:46 2014 -0500

    Pass log config from systemd to systemd-shutdown
    
    If PID 1 debug logging is enabled, it is nice to keep those settings
    when switching to systemd-shutdown binary, independently of whether
    this was done through /proc/cmdline options, or through runtime
    manipulations.

diff --git a/src/core/main.c b/src/core/main.c
index dd67d08..ed64dd1 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -1881,14 +1881,42 @@ finish:
 #endif
 
         if (shutdown_verb) {
-                const char * command_line[] = {
+                char log_level[DECIMAL_STR_MAX(int) + 1];
+                const char* command_line[9] = {
                         SYSTEMD_SHUTDOWN_BINARY_PATH,
                         shutdown_verb,
-                        NULL
+                        "--log-level", log_level,
+                        "--log-target",
                 };
+                unsigned pos = 5;
+                assert(command_line[pos] == NULL);
+
                 _cleanup_strv_free_ char **env_block = NULL;
                 env_block = strv_copy(environ);
 
+                snprintf(log_level, sizeof(log_level), "%d", log_get_max_level());
+
+                switch (log_get_target()) {
+                case LOG_TARGET_KMSG:
+                case LOG_TARGET_JOURNAL_OR_KMSG:
+                case LOG_TARGET_SYSLOG_OR_KMSG:
+                        command_line[pos++] = "kmsg";
+                        break;
+
+                case LOG_TARGET_CONSOLE:
+                default:
+                        command_line[pos++] = "console";
+                        break;
+                };
+
+                if (log_get_show_color())
+                        command_line[pos++] = "--log-color";
+
+                if (log_get_show_location())
+                        command_line[pos++] = "--log-location";
+
+                assert(pos + 1 < ELEMENTSOF(command_line));
+
                 if (arm_reboot_watchdog && arg_shutdown_watchdog > 0) {
                         char *e;
 
@@ -1911,7 +1939,8 @@ finish:
                         cg_uninstall_release_agent(SYSTEMD_CGROUP_CONTROLLER);
 
                 execve(SYSTEMD_SHUTDOWN_BINARY_PATH, (char **) command_line, env_block);
-                log_error("Failed to execute shutdown binary, freezing: %m");
+                log_error("Failed to execute shutdown binary, %s: %m",
+                          getpid() == 1 ? "freezing" : "quitting");
         }
 
         if (getpid() == 1)
diff --git a/src/core/shutdown.c b/src/core/shutdown.c
index 8420a67..9189cfb 100644
--- a/src/core/shutdown.c
+++ b/src/core/shutdown.c
@@ -35,6 +35,7 @@
 #include <stdbool.h>
 #include <stdlib.h>
 #include <string.h>
+#include <getopt.h>
 
 #include "missing.h"
 #include "log.h"
@@ -50,6 +51,93 @@
 
 #define FINALIZE_ATTEMPTS 50
 
+static char* arg_verb;
+
+static int parse_argv(int argc, char *argv[]) {
+        enum {
+                ARG_LOG_LEVEL = 0x100,
+                ARG_LOG_TARGET,
+                ARG_LOG_COLOR,
+                ARG_LOG_LOCATION,
+        };
+
+        static const struct option options[] = {
+                { "log-level",     required_argument, NULL, ARG_LOG_LEVEL    },
+                { "log-target",    required_argument, NULL, ARG_LOG_TARGET   },
+                { "log-color",     optional_argument, NULL, ARG_LOG_COLOR    },
+                { "log-location",  optional_argument, NULL, ARG_LOG_LOCATION },
+                {}
+        };
+
+        int c, r;
+
+        assert(argc >= 1);
+        assert(argv);
+
+        opterr = 0;
+
+        while ((c = getopt_long(argc, argv, ":", options, NULL)) >= 0)
+                switch (c) {
+
+                case ARG_LOG_LEVEL:
+                        r = log_set_max_level_from_string(optarg);
+                        if (r < 0)
+                                log_error("Failed to parse log level %s, ignoring.", optarg);
+
+                        break;
+
+                case ARG_LOG_TARGET:
+                        r = log_set_target_from_string(optarg);
+                        if (r < 0)
+                                log_error("Failed to parse log target %s, ignoring", optarg);
+
+                        break;
+
+                case ARG_LOG_COLOR:
+
+                        if (optarg) {
+                                r = log_show_color_from_string(optarg);
+                                if (r < 0)
+                                        log_error("Failed to parse log color setting %s, ignoring", optarg);
+                        } else
+                                log_show_color(true);
+
+                        break;
+
+                case ARG_LOG_LOCATION:
+                        if (optarg) {
+                                r = log_show_location_from_string(optarg);
+                                if (r < 0)
+                                        log_error("Failed to parse log location setting %s, ignoring", optarg);
+                        } else
+                                log_show_location(true);
+
+                        break;
+
+                case '?':
+                        log_error("Unknown option %s.", argv[optind-1]);
+                        return -EINVAL;
+
+                case ':':
+                        log_error("Missing argument to %s.", argv[optind-1]);
+                        return -EINVAL;
+
+                default:
+                        assert_not_reached("Unhandled option code.");
+                }
+
+        if (optind >= argc) {
+                log_error("Verb argument missing.");
+                return -EINVAL;
+        }
+
+        arg_verb = argv[optind];
+
+        if (optind + 1 < argc)
+                log_error("Excess arguments, ignoring");
+        return 0;
+}
+
 static int prepare_new_root(void) {
         static const char dirs[] =
                 "/run/initramfs/oldroot\0"
@@ -139,52 +227,38 @@ int main(int argc, char *argv[]) {
         unsigned retries;
         int cmd, r;
 
-        /* suppress shutdown status output if 'quiet' is used  */
-        r = proc_cmdline(&line);
-        if (r > 0) {
-                char *w, *state;
-                size_t l;
+        log_parse_environment();
+        r = parse_argv(argc, argv);
+        if (r < 0)
+                goto error;
 
-                FOREACH_WORD_QUOTED(w, l, line, state) {
-                        if (l == 5 && memcmp(w, "quiet", 5) == 0) {
-                                log_set_max_level(LOG_WARNING);
-                                break;
-                        }
-                }
-        }
+        /* journald will die if not gone yet. The log target defaults
+         * to console, but may have been changed by commandline options. */
 
-        log_parse_environment();
-        log_set_target(LOG_TARGET_CONSOLE); /* syslog will die if not gone yet */
         log_close_console(); /* force reopen of /dev/console */
         log_open();
 
         umask(0022);
 
         if (getpid() != 1) {
-                log_error("Not executed by init (pid 1).");
+                log_error("Not executed by init (PID 1).");
                 r = -EPERM;
                 goto error;
         }
 
-        if (argc != 2) {
-                log_error("Invalid number of arguments.");
-                r = -EINVAL;
-                goto error;
-        }
-
         in_container = detect_container(NULL) > 0;
 
-        if (streq(argv[1], "reboot"))
+        if (streq(arg_verb, "reboot"))
                 cmd = RB_AUTOBOOT;
-        else if (streq(argv[1], "poweroff"))
+        else if (streq(arg_verb, "poweroff"))
                 cmd = RB_POWER_OFF;
-        else if (streq(argv[1], "halt"))
+        else if (streq(arg_verb, "halt"))
                 cmd = RB_HALT_SYSTEM;
-        else if (streq(argv[1], "kexec"))
+        else if (streq(arg_verb, "kexec"))
                 cmd = LINUX_REBOOT_CMD_KEXEC;
         else {
-                log_error("Unknown action '%s'.", argv[1]);
                 r = -EINVAL;
+                log_error("Unknown action '%s'.", arg_verb);
                 goto error;
         }
 
@@ -292,7 +366,7 @@ int main(int argc, char *argv[]) {
                 log_info("Storage is finalized.");
 
         arguments[0] = NULL;
-        arguments[1] = argv[1];
+        arguments[1] = arg_verb;
         arguments[2] = NULL;
         execute_directory(SYSTEM_SHUTDOWN_PATH, NULL, arguments);
 
@@ -301,10 +375,11 @@ int main(int argc, char *argv[]) {
 
                 if (prepare_new_root() >= 0 &&
                     pivot_to_new_root() >= 0) {
+                        arguments[0] = (char*) "/shutdown";
 
                         log_info("Returning to initrd...");
 
-                        execv("/shutdown", argv);
+                        execv("/shutdown", arguments);
                         log_error("Failed to execute shutdown binary: %m");
                 }
         }
@@ -389,5 +464,4 @@ int main(int argc, char *argv[]) {
         log_error("Critical error while doing system shutdown: %s", strerror(-r));
 
         freeze();
-        return EXIT_FAILURE;
 }
diff --git a/src/shared/log.c b/src/shared/log.c
index ee20921..3e48b3c 100644
--- a/src/shared/log.c
+++ b/src/shared/log.c
@@ -927,10 +927,18 @@ void log_show_color(bool b) {
         show_color = b;
 }
 
+bool log_get_show_color(void) {
+        return show_color;
+}
+
 void log_show_location(bool b) {
         show_location = b;
 }
 
+bool log_get_show_location(void) {
+        return show_location;
+}
+
 int log_show_color_from_string(const char *e) {
         int t;
 
diff --git a/src/shared/log.h b/src/shared/log.h
index 6a0f673..794af7b 100644
--- a/src/shared/log.h
+++ b/src/shared/log.h
@@ -52,7 +52,9 @@ int log_set_target_from_string(const char *e);
 int log_set_max_level_from_string(const char *e);
 
 void log_show_color(bool b);
+bool log_get_show_color(void) _pure_;
 void log_show_location(bool b);
+bool log_get_show_location(void) _pure_;
 
 int log_show_color_from_string(const char *e);
 int log_show_location_from_string(const char *e);

commit fb4729006a7174472e8a435b0887e532cd6217fc
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Sat Feb 15 18:10:36 2014 -0500

    Some modernizations

diff --git a/man/journald.conf.xml b/man/journald.conf.xml
index e0796e1..03bcd0c 100644
--- a/man/journald.conf.xml
+++ b/man/journald.conf.xml
@@ -399,7 +399,10 @@
                                 <literal>systemd.journald.forward_to_kmsg=</literal>
                                 and
                                 <literal>systemd.journald.forward_to_console=</literal>.
-                                </para></listitem>
+                                When forwarding to the console, the
+                                TTY to log to log to can be changed
+                                with <varname>TTYPath=</varname>,
+                                described below.</para></listitem>
                         </varlistentry>
 
                         <varlistentry>
diff --git a/src/core/main.c b/src/core/main.c
index fc85eed..dd67d08 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -303,7 +303,8 @@ static int parse_proc_cmdline_word(const char *word) {
         } else if (startswith(word, "systemd.dump_core=")) {
                 int r;
 
-                if ((r = parse_boolean(word + 18)) < 0)
+                r = parse_boolean(word + 18);
+                if (r < 0)
                         log_warning("Failed to parse dump core switch %s. Ignoring.", word + 18);
                 else
                         arg_dump_core = r;
@@ -311,7 +312,8 @@ static int parse_proc_cmdline_word(const char *word) {
         } else if (startswith(word, "systemd.crash_shell=")) {
                 int r;
 
-                if ((r = parse_boolean(word + 20)) < 0)
+                r = parse_boolean(word + 20);
+                if (r < 0)
                         log_warning("Failed to parse crash shell switch %s. Ignoring.", word + 20);
                 else
                         arg_crash_shell = r;
@@ -319,7 +321,8 @@ static int parse_proc_cmdline_word(const char *word) {
         } else if (startswith(word, "systemd.confirm_spawn=")) {
                 int r;
 
-                if ((r = parse_boolean(word + 22)) < 0)
+                r = parse_boolean(word + 22);
+                if (r < 0)
                         log_warning("Failed to parse confirm spawn switch %s. Ignoring.", word + 22);
                 else
                         arg_confirm_spawn = r;
@@ -341,23 +344,21 @@ static int parse_proc_cmdline_word(const char *word) {
         } else if (startswith(word, "systemd.default_standard_output=")) {
                 int r;
 
-                if ((r = exec_output_from_string(word + 32)) < 0)
+                r = exec_output_from_string(word + 32);
+                if (r < 0)
                         log_warning("Failed to parse default standard output switch %s. Ignoring.", word + 32);
                 else
                         arg_default_std_output = r;
         } else if (startswith(word, "systemd.default_standard_error=")) {
                 int r;
 
-                if ((r = exec_output_from_string(word + 31)) < 0)
+                r = exec_output_from_string(word + 31);
+                if (r < 0)
                         log_warning("Failed to parse default standard error switch %s. Ignoring.", word + 31);
                 else
                         arg_default_std_error = r;
         } else if (startswith(word, "systemd.setenv=")) {
-                _cleanup_free_ char *cenv = NULL;
-
-                cenv = strdup(word + 15);
-                if (!cenv)
-                        return -ENOMEM;
+                const char *cenv = word + 15;
 
                 if (env_assignment_is_valid(cenv)) {
                         char **env;
@@ -366,7 +367,8 @@ static int parse_proc_cmdline_word(const char *word) {
                         if (env)
                                 arg_default_environment = env;
                         else
-                                log_warning("Setting environment variable '%s' failed, ignoring: %m", cenv);
+                                log_warning("Setting environment variable '%s' failed, ignoring: %s",
+                                            cenv, strerror(ENOMEM));
                 } else
                         log_warning("Environment variable name '%s' is not valid. Ignoring.", cenv);
 
@@ -737,7 +739,7 @@ static int parse_argv(int argc, char *argv[]) {
                 { "switched-root",            no_argument,       NULL, ARG_SWITCHED_ROOT            },
                 { "default-standard-output",  required_argument, NULL, ARG_DEFAULT_STD_OUTPUT,      },
                 { "default-standard-error",   required_argument, NULL, ARG_DEFAULT_STD_ERROR,       },
-                { NULL,                       0,                 NULL, 0                            }
+                {}
         };
 
         int c, r;
@@ -753,7 +755,8 @@ static int parse_argv(int argc, char *argv[]) {
                 switch (c) {
 
                 case ARG_LOG_LEVEL:
-                        if ((r = log_set_max_level_from_string(optarg)) < 0) {
+                        r = log_set_max_level_from_string(optarg);
+                        if (r < 0) {
                                 log_error("Failed to parse log level %s.", optarg);
                                 return r;
                         }
@@ -761,8 +764,8 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 case ARG_LOG_TARGET:
-
-                        if ((r = log_set_target_from_string(optarg)) < 0) {
+                        r = log_set_target_from_string(optarg);
+                        if (r < 0) {
                                 log_error("Failed to parse log target %s.", optarg);
                                 return r;
                         }
@@ -772,7 +775,8 @@ static int parse_argv(int argc, char *argv[]) {
                 case ARG_LOG_COLOR:
 
                         if (optarg) {
-                                if ((r = log_show_color_from_string(optarg)) < 0) {
+                                r = log_show_color_from_string(optarg);
+                                if (r < 0) {
                                         log_error("Failed to parse log color setting %s.", optarg);
                                         return r;
                                 }
@@ -782,9 +786,9 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 case ARG_LOG_LOCATION:
-
                         if (optarg) {
-                                if ((r = log_show_location_from_string(optarg)) < 0) {
+                                r = log_show_location_from_string(optarg);
+                                if (r < 0) {
                                         log_error("Failed to parse log location setting %s.", optarg);
                                         return r;
                                 }
@@ -794,8 +798,8 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 case ARG_DEFAULT_STD_OUTPUT:
-
-                        if ((r = exec_output_from_string(optarg)) < 0) {
+                        r = exec_output_from_string(optarg);
+                        if (r < 0) {
                                 log_error("Failed to parse default standard output setting %s.", optarg);
                                 return r;
                         } else
@@ -803,8 +807,8 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 case ARG_DEFAULT_STD_ERROR:
-
-                        if ((r = exec_output_from_string(optarg)) < 0) {
+                        r = exec_output_from_string(optarg);
+                        if (r < 0) {
                                 log_error("Failed to parse default standard error output setting %s.", optarg);
                                 return r;
                         } else
@@ -813,7 +817,8 @@ static int parse_argv(int argc, char *argv[]) {
 
                 case ARG_UNIT:
 
-                        if ((r = set_default_unit(optarg)) < 0) {
+                        r = set_default_unit(optarg);
+                        if (r < 0) {
                                 log_error("Failed to set default unit %s: %s", optarg, strerror(-r));
                                 return r;
                         }
diff --git a/src/journal/journald-console.c b/src/journal/journald-console.c
index 04c4424..35da52a 100644
--- a/src/journal/journald-console.c
+++ b/src/journal/journald-console.c
@@ -55,7 +55,7 @@ void server_forward_console(
         struct timespec ts;
         char tbuf[4 + DECIMAL_STR_MAX(ts.tv_sec) + DECIMAL_STR_MAX(ts.tv_nsec)-3 + 1];
         int n = 0, fd;
-        char *ident_buf = NULL;
+        _cleanup_free_ char *ident_buf = NULL;
         const char *tty;
 
         assert(s);
@@ -101,14 +101,11 @@ void server_forward_console(
         fd = open_terminal(tty, O_WRONLY|O_NOCTTY|O_CLOEXEC);
         if (fd < 0) {
                 log_debug("Failed to open %s for logging: %m", tty);
-                goto finish;
+                return;
         }
 
         if (writev(fd, iovec, n) < 0)
                 log_debug("Failed to write to %s for logging: %m", tty);
 
         close_nointr_nofail(fd);
-
-finish:
-        free(ident_buf);
 }

commit 141a79f491fd4bf5ea0d66039065c9f9649bfc0e
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Sat Feb 15 18:08:59 2014 -0500

    Extract looping over /proc/cmdline into a shared function
    
    In cryptsetup-generator automatic cleanup had to be replaced
    with manual cleanup, and the code gets a bit longer. But existing
    code had the issue that it returned negative values from main(),
    which was wrong, so should be reworked anyway.

diff --git a/src/core/main.c b/src/core/main.c
index 14e21d6..fc85eed 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -694,35 +694,6 @@ static int parse_config_file(void) {
         return 0;
 }
 
-static int parse_proc_cmdline(void) {
-        _cleanup_free_ char *line = NULL;
-        char *w, *state;
-        size_t l;
-        int r;
-
-        r = proc_cmdline(&line);
-        if (r < 0)
-                log_warning("Failed to read /proc/cmdline, ignoring: %s", strerror(-r));
-        if (r <= 0)
-                return 0;
-
-        FOREACH_WORD_QUOTED(w, l, line, state) {
-                _cleanup_free_ char *word;
-
-                word = strndup(w, l);
-                if (!word)
-                        return log_oom();
-
-                r = parse_proc_cmdline_word(word);
-                if (r < 0) {
-                        log_error("Failed on cmdline argument %s: %s", word, strerror(-r));
-                        return r;
-                }
-        }
-
-        return 0;
-}
-
 static int parse_argv(int argc, char *argv[]) {
 
         enum {
@@ -1408,7 +1379,7 @@ int main(int argc, char *argv[]) {
                 goto finish;
 
         if (arg_running_as == SYSTEMD_SYSTEM)
-                if (parse_proc_cmdline() < 0)
+                if (parse_proc_cmdline(parse_proc_cmdline_word) < 0)
                         goto finish;
 
         log_parse_environment();
diff --git a/src/cryptsetup/cryptsetup-generator.c b/src/cryptsetup/cryptsetup-generator.c
index 46ad9b8..38c746b 100644
--- a/src/cryptsetup/cryptsetup-generator.c
+++ b/src/cryptsetup/cryptsetup-generator.c
@@ -34,6 +34,11 @@ static const char *arg_dest = "/tmp";
 static bool arg_enabled = true;
 static bool arg_read_crypttab = true;
 
+static char **arg_disks;
+static char **arg_options;
+static char *arg_keyfile;
+
+
 static bool has_option(const char *haystack, const char *needle) {
         const char *f = haystack;
         size_t l;
@@ -250,122 +255,94 @@ static int create_disk(
         return 0;
 }
 
-static int parse_proc_cmdline(
-                char ***arg_proc_cmdline_disks,
-                char ***arg_proc_cmdline_options,
-                char **arg_proc_cmdline_keyfile) {
-
-        _cleanup_free_ char *line = NULL;
-        char *w = NULL, *state = NULL;
-        size_t l;
+static int parse_proc_cmdline_word(const char *word) {
         int r;
 
-        r = proc_cmdline(&line);
-        if (r < 0)
-                log_warning("Failed to read /proc/cmdline, ignoring: %s", strerror(-r));
-        if (r <= 0)
-                return 0;
-
-        FOREACH_WORD_QUOTED(w, l, line, state) {
-                _cleanup_free_ char *word = NULL;
+        if (startswith(word, "luks=")) {
+                r = parse_boolean(word + 5);
+                if (r < 0)
+                        log_warning("Failed to parse luks switch %s. Ignoring.", word + 5);
+                else
+                        arg_enabled = r;
 
-                word = strndup(w, l);
-                if (!word)
-                        return log_oom();
+        } else if (startswith(word, "rd.luks=")) {
 
-                if (startswith(word, "luks=")) {
-                        r = parse_boolean(word + 5);
+                if (in_initrd()) {
+                        r = parse_boolean(word + 8);
                         if (r < 0)
-                                log_warning("Failed to parse luks switch %s. Ignoring.", word + 5);
+                                log_warning("Failed to parse luks switch %s. Ignoring.", word + 8);
                         else
                                 arg_enabled = r;
+                }
 
-                } else if (startswith(word, "rd.luks=")) {
+        } else if (startswith(word, "luks.crypttab=")) {
+                r = parse_boolean(word + 14);
+                if (r < 0)
+                        log_warning("Failed to parse luks crypttab switch %s. Ignoring.", word + 14);
+                else
+                        arg_read_crypttab = r;
 
-                        if (in_initrd()) {
-                                r = parse_boolean(word + 8);
-                                if (r < 0)
-                                        log_warning("Failed to parse luks switch %s. Ignoring.", word + 8);
-                                else
-                                        arg_enabled = r;
-                        }
+        } else if (startswith(word, "rd.luks.crypttab=")) {
 
-                } else if (startswith(word, "luks.crypttab=")) {
-                        r = parse_boolean(word + 14);
+                if (in_initrd()) {
+                        r = parse_boolean(word + 17);
                         if (r < 0)
-                                log_warning("Failed to parse luks crypttab switch %s. Ignoring.", word + 14);
+                                log_warning("Failed to parse luks crypttab switch %s. Ignoring.", word + 17);
                         else
                                 arg_read_crypttab = r;
+                }
 
-                } else if (startswith(word, "rd.luks.crypttab=")) {
+        } else if (startswith(word, "luks.uuid=")) {
+                if (strv_extend(&arg_disks, word + 10) < 0)
+                        return log_oom();
 
-                        if (in_initrd()) {
-                                r = parse_boolean(word + 17);
-                                if (r < 0)
-                                        log_warning("Failed to parse luks crypttab switch %s. Ignoring.", word + 17);
-                                else
-                                        arg_read_crypttab = r;
-                        }
+        } else if (startswith(word, "rd.luks.uuid=")) {
 
-                } else if (startswith(word, "luks.uuid=")) {
-                        if (strv_extend(arg_proc_cmdline_disks, word + 10) < 0)
+                if (in_initrd()) {
+                        if (strv_extend(&arg_disks, word + 13) < 0)
                                 return log_oom();
+                }
 
-                } else if (startswith(word, "rd.luks.uuid=")) {
+        } else if (startswith(word, "luks.options=")) {
+                if (strv_extend(&arg_options, word + 13) < 0)
+                        return log_oom();
 
-                        if (in_initrd()) {
-                                if (strv_extend(arg_proc_cmdline_disks, word + 13) < 0)
-                                        return log_oom();
-                        }
+        } else if (startswith(word, "rd.luks.options=")) {
 
-                } else if (startswith(word, "luks.options=")) {
-                        if (strv_extend(arg_proc_cmdline_options, word + 13) < 0)
+                if (in_initrd()) {
+                        if (strv_extend(&arg_options, word + 16) < 0)
                                 return log_oom();
+                }
 
-                } else if (startswith(word, "rd.luks.options=")) {
+        } else if (startswith(word, "luks.key=")) {
+                free(arg_keyfile);
+                arg_keyfile = strdup(word + 9);
+                if (!arg_keyfile)
+                        return log_oom();
 
-                        if (in_initrd()) {
-                                if (strv_extend(arg_proc_cmdline_options, word + 16) < 0)
-                                        return log_oom();
-                        }
+        } else if (startswith(word, "rd.luks.key=")) {
 
-                } else if (startswith(word, "luks.key=")) {
-                        if (*arg_proc_cmdline_keyfile)
-                                free(*arg_proc_cmdline_keyfile);
-                        *arg_proc_cmdline_keyfile = strdup(word + 9);
-                        if (!*arg_proc_cmdline_keyfile)
+                if (in_initrd()) {
+                        free(arg_keyfile);
+                        arg_keyfile = strdup(word + 12);
+                        if (!arg_keyfile)
                                 return log_oom();
+                }
 
-                } else if (startswith(word, "rd.luks.key=")) {
-
-                        if (in_initrd()) {
-                                if (*arg_proc_cmdline_keyfile)
-                                        free(*arg_proc_cmdline_keyfile);
-                                *arg_proc_cmdline_keyfile = strdup(word + 12);
-                                if (!*arg_proc_cmdline_keyfile)
-                                        return log_oom();
-                        }
-
-                } else if (startswith(word, "luks.") ||
-                           (in_initrd() && startswith(word, "rd.luks."))) {
+        } else if (startswith(word, "luks.") ||
+                   (in_initrd() && startswith(word, "rd.luks."))) {
 
-                        log_warning("Unknown kernel switch %s. Ignoring.", word);
-                }
+                log_warning("Unknown kernel switch %s. Ignoring.", word);
         }
 
-        strv_uniq(*arg_proc_cmdline_disks);
-
         return 0;
 }
 
 int main(int argc, char *argv[]) {
-        _cleanup_strv_free_ char **arg_proc_cmdline_disks_done = NULL;
-        _cleanup_strv_free_ char **arg_proc_cmdline_disks = NULL;
-        _cleanup_strv_free_ char **arg_proc_cmdline_options = NULL;
-        _cleanup_free_ char *arg_proc_cmdline_keyfile = NULL;
+        _cleanup_strv_free_ char **disks_done = NULL;
         _cleanup_fclose_ FILE *f = NULL;
         unsigned n = 0;
-        int r = EXIT_SUCCESS;
+        int r = EXIT_FAILURE, r2 = EXIT_FAILURE;
         char **i;
 
         if (argc > 1 && argc != 4) {
@@ -382,11 +359,15 @@ int main(int argc, char *argv[]) {
 
         umask(0022);
 
-        if (parse_proc_cmdline(&arg_proc_cmdline_disks, &arg_proc_cmdline_options, &arg_proc_cmdline_keyfile) < 0)
-                return EXIT_FAILURE;
+        if (parse_proc_cmdline(parse_proc_cmdline_word) < 0)
+                goto cleanup;
 
-        if (!arg_enabled)
-                return EXIT_SUCCESS;
+        if (!arg_enabled) {
+                r = r2 = EXIT_SUCCESS;
+                goto cleanup;
+        }
+
+        strv_uniq(arg_disks);
 
         if (arg_read_crypttab) {
                 struct stat st;
@@ -395,17 +376,14 @@ int main(int argc, char *argv[]) {
                 if (!f) {
                         if (errno == ENOENT)
                                 r = EXIT_SUCCESS;
-                        else {
-                                r = EXIT_FAILURE;
+                        else
                                 log_error("Failed to open /etc/crypttab: %m");
-                        }
 
                         goto next;
                 }
 
                 if (fstat(fileno(f), &st) < 0) {
                         log_error("Failed to stat /etc/crypttab: %m");
-                        r = EXIT_FAILURE;
                         goto next;
                 }
 
@@ -433,36 +411,34 @@ int main(int argc, char *argv[]) {
                         k = sscanf(l, "%ms %ms %ms %ms", &name, &device, &password, &options);
                         if (k < 2 || k > 4) {
                                 log_error("Failed to parse /etc/crypttab:%u, ignoring.", n);
-                                r = EXIT_FAILURE;
                                 continue;
                         }
 
-                        if (arg_proc_cmdline_options) {
-                                /*
-                                  If options are specified on the kernel commandline, let them override
-                                  the ones from crypttab.
-                                */
-                                STRV_FOREACH(i, arg_proc_cmdline_options) {
-                                        _cleanup_free_ char *proc_uuid = NULL, *proc_options = NULL;
-                                        const char *p = *i;
-
-                                        k = sscanf(p, "%m[0-9a-fA-F-]=%ms", &proc_uuid, &proc_options);
-                                        if (k == 2 && streq(proc_uuid, device + 5)) {
-                                                if (options)
-                                                        free(options);
-                                                options = strdup(p);
-                                                if (!proc_options)
-                                                        return log_oom();
+                        /*
+                          If options are specified on the kernel commandline, let them override
+                          the ones from crypttab.
+                        */
+                        STRV_FOREACH(i, arg_options) {
+                                _cleanup_free_ char *proc_uuid = NULL, *proc_options = NULL;
+                                const char *p = *i;
+
+                                k = sscanf(p, "%m[0-9a-fA-F-]=%ms", &proc_uuid, &proc_options);
+                                if (k == 2 && streq(proc_uuid, device + 5)) {
+                                        free(options);
+                                        options = strdup(p);
+                                        if (!proc_options) {
+                                                log_oom();
+                                                goto cleanup;
                                         }
                                 }
                         }
 
-                        if (arg_proc_cmdline_disks) {
+                        if (arg_disks) {
                                 /*
                                   If luks UUIDs are specified on the kernel command line, use them as a filter
                                   for /etc/crypttab and only generate units for those.
                                 */
-                                STRV_FOREACH(i, arg_proc_cmdline_disks) {
+                                STRV_FOREACH(i, arg_disks) {
                                         _cleanup_free_ char *proc_device = NULL, *proc_name = NULL;
                                         const char *p = *i;
 
@@ -472,26 +448,31 @@ int main(int argc, char *argv[]) {
                                         proc_name = strappend("luks-", p);
                                         proc_device = strappend("UUID=", p);
 
-                                        if (!proc_name || !proc_device)
-                                                return log_oom();
+                                        if (!proc_name || !proc_device) {
+                                                log_oom();
+                                                goto cleanup;
+                                        }
 
                                         if (streq(proc_device, device) || streq(proc_name, name)) {
                                                 if (create_disk(name, device, password, options) < 0)
-                                                        r = EXIT_FAILURE;
+                                                        goto cleanup;
 
-                                                if (strv_extend(&arg_proc_cmdline_disks_done, p) < 0)
-                                                        return log_oom();
+                                                if (strv_extend(&disks_done, p) < 0) {
+                                                        log_oom();
+                                                        goto cleanup;
+                                                }
                                         }
                                 }
-                        } else {
-                                if (create_disk(name, device, password, options) < 0)
-                                        r = EXIT_FAILURE;
-                        }
+                        } else if (create_disk(name, device, password, options) < 0)
+                                goto cleanup;
+
                 }
         }
 
+        r = EXIT_SUCCESS;
+
 next:
-        STRV_FOREACH(i, arg_proc_cmdline_disks) {
+        STRV_FOREACH(i, arg_disks) {
                 /*
                   Generate units for those UUIDs, which were specified
                   on the kernel command line and not yet written.
@@ -503,22 +484,24 @@ next:
                 if (startswith(p, "luks-"))
                         p += 5;
 
-                if (strv_contains(arg_proc_cmdline_disks_done, p))
+                if (strv_contains(disks_done, p))
                         continue;
 
                 name = strappend("luks-", p);
                 device = strappend("UUID=", p);
 
-                if (!name || !device)
-                        return log_oom();
+                if (!name || !device) {
+                        log_oom();
+                        goto cleanup;
+                }
 
-                if (arg_proc_cmdline_options) {
+                if (arg_options) {
                         /*
                           If options are specified on the kernel commandline, use them.
                         */
                         char **j;
 
-                        STRV_FOREACH(j, arg_proc_cmdline_options) {
+                        STRV_FOREACH(j, arg_options) {
                                 _cleanup_free_ char *proc_uuid = NULL, *proc_options = NULL;
                                 const char *s = *j;
                                 int k;
@@ -529,29 +512,42 @@ next:
                                                 if (options)
                                                         free(options);
                                                 options = strdup(proc_options);
-                                                if (!options)
-                                                        return log_oom();
+                                                if (!options) {
+                                                        log_oom();
+                                                        goto cleanup;
+                                                }
                                         }
                                 } else if (!options) {
                                         /*
                                           Fall back to options without a specified UUID
                                         */
                                         options = strdup(s);
-                                        if (!options)
-                                                return log_oom();
+                                        if (!options) {
+                                                log_oom();
+                                                goto cleanup;
+                                        };
                                 }
                         }
                 }
 
                 if (!options) {
                         options = strdup("timeout=0");
-                        if (!options)
-                                return log_oom();
+                        if (!options) {
+                                log_oom();
+                                goto cleanup;
+                        }
                 }
 
-                if (create_disk(name, device, arg_proc_cmdline_keyfile, options) < 0)
-                        r = EXIT_FAILURE;
+                if (create_disk(name, device, arg_keyfile, options) < 0)
+                        goto cleanup;
         }
 
-        return r;
+        r2 = EXIT_SUCCESS;
+
+cleanup:
+        strv_free(arg_disks);
+        strv_free(arg_options);
+        free(arg_keyfile);
+
+        return r != EXIT_SUCCESS ? r : r2;
 }
diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c
index 8facc88..4a5f6b1 100644
--- a/src/fsck/fsck.c
+++ b/src/fsck/fsck.c
@@ -72,38 +72,24 @@ static void start_target(const char *target) {
                 log_error("Failed to start unit: %s", bus_error_message(&error, -r));
 }
 
-static int parse_proc_cmdline(void) {
-        _cleanup_free_ char *line = NULL;
-        char *w, *state;
-        size_t l;
-        int r;
-
-        r = proc_cmdline(&line);
-        if (r < 0)
-                log_warning("Failed to read /proc/cmdline, ignoring: %s", strerror(-r));
-        if (r <= 0)
-                return 0;
-
-        FOREACH_WORD_QUOTED(w, l, line, state) {
-
-                if (strneq(w, "fsck.mode=auto", l))
-                        arg_force = arg_skip = false;
-                else if (strneq(w, "fsck.mode=force", l))
-                        arg_force = true;
-                else if (strneq(w, "fsck.mode=skip", l))
-                        arg_skip = true;
-                else if (startswith(w, "fsck"))
-                        log_warning("Invalid fsck parameter. Ignoring.");
+static int parse_proc_cmdline_word(const char *w) {
+        if (streq(w, "fsck.mode=auto"))
+                arg_force = arg_skip = false;
+        else if (streq(w, "fsck.mode=force"))
+                arg_force = true;
+        else if (streq(w, "fsck.mode=skip"))
+                arg_skip = true;
+        else if (startswith(w, "fsck"))
+                log_warning("Invalid fsck parameter. Ignoring.");
 #ifdef HAVE_SYSV_COMPAT
-                else if (strneq(w, "fastboot", l)) {
-                        log_error("Please pass 'fsck.mode=skip' rather than 'fastboot' on the kernel command line.");
-                        arg_skip = true;
-                } else if (strneq(w, "forcefsck", l)) {
-                        log_error("Please pass 'fsck.mode=force' rather than 'forcefsck' on the kernel command line.");
-                        arg_force = true;
-                }
-#endif
+        else if (streq(w, "fastboot")) {
+                log_error("Please pass 'fsck.mode=skip' rather than 'fastboot' on the kernel command line.");
+                arg_skip = true;
+        } else if (streq(w, "forcefsck")) {
+                log_error("Please pass 'fsck.mode=force' rather than 'forcefsck' on the kernel command line.");
+                arg_force = true;
         }
+#endif
 
         return 0;
 }
@@ -229,7 +215,7 @@ int main(int argc, char *argv[]) {
 
         umask(0022);
 
-        parse_proc_cmdline();
+        parse_proc_cmdline(parse_proc_cmdline_word);
         test_files();
 
         if (!arg_force && arg_skip)
diff --git a/src/fstab-generator/fstab-generator.c b/src/fstab-generator/fstab-generator.c
index 0336888..5521a86 100644
--- a/src/fstab-generator/fstab-generator.c
+++ b/src/fstab-generator/fstab-generator.c
@@ -501,47 +501,30 @@ static int parse_new_root_from_proc_cmdline(void) {
         return (r < 0) ? r : 0;
 }
 
-static int parse_proc_cmdline(void) {
-        _cleanup_free_ char *line = NULL;
-        char *w, *state;
-        size_t l;
+static int parse_proc_cmdline_word(const char *word) {
         int r;
 
-        r = proc_cmdline(&line);
-        if (r < 0)
-                log_warning("Failed to read /proc/cmdline, ignoring: %s", strerror(-r));
-        if (r <= 0)
-                return 0;
-
-        FOREACH_WORD_QUOTED(w, l, line, state) {
-                _cleanup_free_ char *word = NULL;
+        if (startswith(word, "fstab=")) {
+                r = parse_boolean(word + 6);
+                if (r < 0)
+                        log_warning("Failed to parse fstab switch %s. Ignoring.", word + 6);
+                else
+                        arg_enabled = r;
 
-                word = strndup(w, l);
-                if (!word)
-                        return log_oom();
+        } else if (startswith(word, "rd.fstab=")) {
 
-                if (startswith(word, "fstab=")) {
-                        r = parse_boolean(word + 6);
+                if (in_initrd()) {
+                        r = parse_boolean(word + 9);
                         if (r < 0)
-                                log_warning("Failed to parse fstab switch %s. Ignoring.", word + 6);
+                                log_warning("Failed to parse fstab switch %s. Ignoring.", word + 9);
                         else
                                 arg_enabled = r;
+                }
 
-                } else if (startswith(word, "rd.fstab=")) {
-
-                        if (in_initrd()) {
-                                r = parse_boolean(word + 9);
-                                if (r < 0)
-                                        log_warning("Failed to parse fstab switch %s. Ignoring.", word + 9);
-                                else
-                                        arg_enabled = r;
-                        }
-
-                } else if (startswith(word, "fstab.") ||
-                           (in_initrd() && startswith(word, "rd.fstab."))) {
+        } else if (startswith(word, "fstab.") ||
+                   (in_initrd() && startswith(word, "rd.fstab."))) {
 
-                        log_warning("Unknown kernel switch %s. Ignoring.", word);
-                }
+                log_warning("Unknown kernel switch %s. Ignoring.", word);
         }
 
         return 0;
@@ -564,7 +547,7 @@ int main(int argc, char *argv[]) {
 
         umask(0022);
 
-        if (parse_proc_cmdline() < 0)
+        if (parse_proc_cmdline(parse_proc_cmdline_word) < 0)
                 return EXIT_FAILURE;
 
         if (in_initrd())
diff --git a/src/modules-load/modules-load.c b/src/modules-load/modules-load.c
index 3ac25fa..1a32d26 100644
--- a/src/modules-load/modules-load.c
+++ b/src/modules-load/modules-load.c
@@ -69,39 +69,19 @@ static int add_modules(const char *p) {
         return 0;
 }
 
-static int parse_proc_cmdline(void) {
-        _cleanup_free_ char *line = NULL;
-        char *w, *state;
-        size_t l;
+static int parse_proc_cmdline_word(const char *word) {
         int r;
 
-        r = proc_cmdline(&line);
-        if (r < 0)
-                log_warning("Failed to read /proc/cmdline, ignoring: %s", strerror(-r));
-        if (r <= 0)
-                return 0;
-
-        FOREACH_WORD_QUOTED(w, l, line, state) {
-                _cleanup_free_ char *word;
-
-                word = strndup(w, l);
-                if (!word)
-                        return log_oom();
-
-                if (startswith(word, "modules-load=")) {
+        if (startswith(word, "modules-load=")) {
+                r = add_modules(word + 13);
+                if (r < 0)
+                        return r;
 
-                        r = add_modules(word + 13);
+        } else if (startswith(word, "rd.modules-load=")) {
+                if (in_initrd()) {
+                        r = add_modules(word + 16);
                         if (r < 0)
                                 return r;
-
-                } else if (startswith(word, "rd.modules-load=")) {
-
-                        if (in_initrd()) {
-                                r = add_modules(word + 16);
-                                if (r < 0)
-                                        return r;
-                        }
-
                 }
         }
 
@@ -273,7 +253,7 @@ int main(int argc, char *argv[]) {
 
         umask(0022);
 
-        if (parse_proc_cmdline() < 0)
+        if (parse_proc_cmdline(parse_proc_cmdline_word) < 0)
                 return EXIT_FAILURE;
 
         ctx = kmod_new(NULL, NULL);
diff --git a/src/quotacheck/quotacheck.c b/src/quotacheck/quotacheck.c
index 622952a..8ff0f7f 100644
--- a/src/quotacheck/quotacheck.c
+++ b/src/quotacheck/quotacheck.c
@@ -31,35 +31,22 @@
 static bool arg_skip = false;
 static bool arg_force = false;
 
-static int parse_proc_cmdline(void) {
-        _cleanup_free_ char *line = NULL;
-        char *w, *state;
-        size_t l;
-        int r;
-
-        r = proc_cmdline(&line);
-        if (r < 0)
-                log_warning("Failed to read /proc/cmdline, ignoring: %s", strerror(-r));
-        if (r <= 0)
-                return 0;
-
-        FOREACH_WORD_QUOTED(w, l, line, state) {
-
-                if (strneq(w, "quotacheck.mode=auto", l))
-                        arg_force = arg_skip = false;
-                else if (strneq(w, "quotacheck.mode=force", l))
-                        arg_force = true;
-                else if (strneq(w, "quotacheck.mode=skip", l))
-                        arg_skip = true;
-                else if (startswith(w, "quotacheck"))
-                        log_warning("Invalid quotacheck parameter. Ignoring.");
+static int parse_proc_cmdline_word(const char *w) {
+        if (streq(w, "quotacheck.mode=auto"))
+                arg_force = arg_skip = false;
+        else if (streq(w, "quotacheck.mode=force"))
+                arg_force = true;
+        else if (streq(w, "quotacheck.mode=skip"))
+                arg_skip = true;
+        else if (startswith(w, "quotacheck"))
+                log_warning("Invalid quotacheck parameter. Ignoring.");
 #ifdef HAVE_SYSV_COMPAT
-                else if (strneq(w, "forcequotacheck", l)) {
-                        log_error("Please use 'quotacheck.mode=force' rather than 'forcequotacheck' on the kernel command line.");
-                        arg_force = true;
-                }
-#endif
+        else if (streq(w, "forcequotacheck")) {
+                log_error("Please use 'quotacheck.mode=force' rather than 'forcequotacheck' on the kernel command line.");
+                arg_force = true;
         }
+#endif
+
         return 0;
 }
 
@@ -93,7 +80,7 @@ int main(int argc, char *argv[]) {
 
         umask(0022);
 
-        parse_proc_cmdline();
+        parse_proc_cmdline(parse_proc_cmdline_word);
         test_files();
 
         if (!arg_force) {
diff --git a/src/shared/util.c b/src/shared/util.c
index b1a9db1..d95a4b4 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -5943,6 +5943,35 @@ int proc_cmdline(char **ret) {
         return 1;
 }
 
+int parse_proc_cmdline(int (*parse_word)(const char *word)) {
+        _cleanup_free_ char *line = NULL;
+        char *w, *state;
+        size_t l;
+        int r;
+
+        r = proc_cmdline(&line);
+        if (r < 0)
+                log_warning("Failed to read /proc/cmdline, ignoring: %s", strerror(-r));
+        if (r <= 0)
+                return 0;
+
+        FOREACH_WORD_QUOTED(w, l, line, state) {
+                _cleanup_free_ char *word;
+
+                word = strndup(w, l);
+                if (!word)
+                        return log_oom();
+
+                r = parse_word(word);
+                if (r < 0) {
+                        log_error("Failed on cmdline argument %s: %s", word, strerror(-r));
+                        return r;
+                }
+        }
+
+        return 0;
+}
+
 int container_get_leader(const char *machine, pid_t *pid) {
         _cleanup_free_ char *s = NULL, *class = NULL;
         const char *p;
diff --git a/src/shared/util.h b/src/shared/util.h
index fcb45b5..4bed5b4 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -852,6 +852,7 @@ static inline void qsort_safe(void *base, size_t nmemb, size_t size,
 }
 
 int proc_cmdline(char **ret);
+int parse_proc_cmdline(int (*parse_word)(const char *word));
 
 int container_get_leader(const char *machine, pid_t *pid);
 

commit 8fe63cd4f16e1e7cdf528ff053f8eb4da7848455
Author: Djalal Harouni <tixxdz at opendz.org>
Date:   Thu Feb 13 23:03:23 2014 +0100

    logind: close race on session state during logins
    
    At login there is a small race window where session_get_state() will
    return SESSION_ACTIVE instead of SESSION_OPENING. This must be fixed
    since during that time there are calls to session_save() to save
    session states and we want to write the correct state.
    
    When we queue the start scope and service jobs, we wait for both of them
    to finish before calling and continue processing in:
    "session_jobs_reply() => session_send_create_reply()"
    to create the session fifo and notify clients.
    
    However, in the match_job_removed() D-Bus signal, we may hit situations
    where the scope job has successfully finished and we are still waiting
    for the user service job to finish. During that time the
    "session->scope_job" will be freed and set to NULL, this makes
    session_get_state() return SESSION_ACTIVE before it is really active, it
    should return SESSION_OPENING since we are still waiting for the service
    job to finish in order to create the session fifo.
    
    To fix this, we also check if the session fifo fd was created, if so then
    the session has entered the SESSION_ACTIVE state, if not then it is still
    in the SESSION_OPENING state and it is waiting for the scope and service
    jobs to finish.

diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index d4742e1..d7c074b 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -915,10 +915,11 @@ void session_add_to_gc_queue(Session *s) {
 SessionState session_get_state(Session *s) {
         assert(s);
 
+        /* always check closing first */
         if (s->stopping || s->timer_event_source)
                 return SESSION_CLOSING;
 
-        if (s->scope_job)
+        if (s->scope_job || s->fifo_fd < 0)
                 return SESSION_OPENING;
 
         if (session_is_active(s))



More information about the systemd-commits mailing list