[systemd-commits] 4 commits - .gitignore Makefile.am man/sd_journal_print.xml man/systemd.xml src/core src/journal src/shared src/test

Zbigniew Jędrzejewski-Szmek zbyszek at kemper.freedesktop.org
Mon Jan 27 20:17:35 PST 2014


 .gitignore                   |    3 +
 Makefile.am                  |    7 ++++
 man/sd_journal_print.xml     |   31 ++++++++++++++++++--
 man/systemd.xml              |   14 ++++++---
 src/core/job.c               |    4 ++
 src/core/main.c              |   36 +++++++++++++----------
 src/core/manager.c           |   46 ++++++++++++++++-------------
 src/core/manager.h           |    6 ++-
 src/journal/journal-send.c   |   22 +++++---------
 src/journal/journal-verify.c |   12 +------
 src/shared/conf-parser.c     |   29 ++++++++++++++++++
 src/shared/conf-parser.h     |    1 
 src/shared/def.h             |    1 
 src/shared/exit-status.c     |   17 +++++++++++
 src/shared/exit-status.h     |   12 +++++++
 src/shared/missing.h         |   16 ++++++----
 src/shared/util.c            |   66 +++++++++++++++++++++++++++++++++++++++++++
 src/shared/util.h            |    5 +++
 src/test/test-tmpfiles.c     |   51 +++++++++++++++++++++++++++++++++
 src/test/test-util.c         |   26 ++++++++++++++++
 20 files changed, 329 insertions(+), 76 deletions(-)

New commits:
commit cb8ccb2271727fc114ca43104d3333ee4635cc79
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Mon Jan 27 22:48:18 2014 -0500

    manager: also turn on output on unit failure

diff --git a/src/core/job.c b/src/core/job.c
index fb6709e..93fa44a 100644
--- a/src/core/job.c
+++ b/src/core/job.c
@@ -632,15 +632,18 @@ static void job_print_status_message(Unit *u, JobType t, JobResult result) {
                         break;
 
                 case JOB_FAILED:
+                        manager_flip_auto_status(u->manager, true);
                         unit_status_printf(u, ANSI_HIGHLIGHT_RED_ON "FAILED" ANSI_HIGHLIGHT_OFF, format);
                         manager_status_printf(u->manager, false, NULL, "See 'systemctl status %s' for details.", u->id);
                         break;
 
                 case JOB_DEPENDENCY:
+                        manager_flip_auto_status(u->manager, true);
                         unit_status_printf(u, ANSI_HIGHLIGHT_YELLOW_ON "DEPEND" ANSI_HIGHLIGHT_OFF, format);
                         break;
 
                 case JOB_TIMEOUT:
+                        manager_flip_auto_status(u->manager, true);
                         unit_status_printf(u, ANSI_HIGHLIGHT_RED_ON " TIME " ANSI_HIGHLIGHT_OFF, format);
                         break;
 
@@ -657,6 +660,7 @@ static void job_print_status_message(Unit *u, JobType t, JobResult result) {
                 switch (result) {
 
                 case JOB_TIMEOUT:
+                        manager_flip_auto_status(u->manager, true);
                         unit_status_printf(u, ANSI_HIGHLIGHT_RED_ON " TIME " ANSI_HIGHLIGHT_OFF, format);
                         break;
 
diff --git a/src/core/manager.c b/src/core/manager.c
index 9f615e6..edde109 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -136,6 +136,16 @@ static void draw_cylon(char buffer[], size_t buflen, unsigned width, unsigned po
         }
 }
 
+void manager_flip_auto_status(Manager *m, bool enable) {
+        if (enable) {
+                if (m->show_status == SHOW_STATUS_AUTO)
+                        manager_set_show_status(m, SHOW_STATUS_TEMPORARY);
+        } else {
+                if (m->show_status == SHOW_STATUS_TEMPORARY)
+                        manager_set_show_status(m, SHOW_STATUS_AUTO);
+        }
+}
+
 static void manager_print_jobs_in_progress(Manager *m) {
         _cleanup_free_ char *job_of_n = NULL;
         Iterator i;
@@ -148,8 +158,7 @@ static void manager_print_jobs_in_progress(Manager *m) {
 
         assert(m);
 
-        if (m->show_status == SHOW_STATUS_AUTO)
-                manager_set_show_status(m, SHOW_STATUS_TEMPORARY);
+        manager_flip_auto_status(m, true);
 
         print_nr = (m->jobs_in_progress_iteration / JOBS_IN_PROGRESS_PERIOD_DIVISOR) % m->n_running_jobs;
 
@@ -2459,8 +2468,7 @@ void manager_check_finished(Manager *m) {
                 return;
         }
 
-        if (m->show_status == SHOW_STATUS_TEMPORARY)
-                manager_set_show_status(m, SHOW_STATUS_AUTO);
+        manager_flip_auto_status(m, false);
 
         /* Notify Type=idle units that we are done now */
         m->idle_pipe_event_source = sd_event_source_unref(m->idle_pipe_event_source);
diff --git a/src/core/manager.h b/src/core/manager.h
index 3065822..358aba7 100644
--- a/src/core/manager.h
+++ b/src/core/manager.h
@@ -299,5 +299,6 @@ void manager_recheck_journal(Manager *m);
 
 void manager_set_show_status(Manager *m, ShowStatus mode);
 void manager_status_printf(Manager *m, bool ephemeral, const char *status, const char *format, ...) _printf_(4,5);
+void manager_flip_auto_status(Manager *m, bool enable);
 
 Set *manager_get_units_requiring_mounts_for(Manager *m, const char *path);

commit d450b6f2a9dd8a7fb14e9f8f771ddd70de7afc5e
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Mon Jan 27 22:27:07 2014 -0500

    manager: add systemd.show_status=auto mode
    
    When set to auto, status will shown when the first ephemeral message
    is shown (a job has been running for five seconds). Then until the
    boot or shutdown ends, status messages will be shown.
    
    No indication about the switch is done: I think it should be clear
    for the user that first the cylon eye and the ephemeral messages appear,
    and afterwards messages are displayed.
    
    The initial arming of the event source was still wrong, but now should
    really be fixed.

diff --git a/man/systemd.xml b/man/systemd.xml
index acba820..4e35c96 100644
--- a/man/systemd.xml
+++ b/man/systemd.xml
@@ -1061,14 +1061,20 @@
                                 <term><varname>systemd.show_status=</varname></term>
 
                                 <listitem><para>Takes a boolean
-                                argument. If <option>true</option>,
-                                shows terse service status updates on
-                                the console during bootup. Defaults to
+                                argument or the constant
+                                <constant>auto</constant>. If
+                                <option>true</option>, shows terse
+                                service status updates on the console
+                                during bootup.
+                                <constant>auto</constant> behaves like
+                                <option>false</option> until a service
+                                fails or there is a significant delay
+                                in boot. Defaults to
                                 <option>true</option>, unless
                                 <option>quiet</option> is passed as
                                 kernel command line option in which
                                 case it defaults to
-                                <option>false</option>.</para></listitem>
+                                <constant>auto</constant>.</para></listitem>
                         </varlistentry>
 
                         <varlistentry>
diff --git a/src/core/main.c b/src/core/main.c
index fb34e4d..404fee7 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -90,7 +90,7 @@ static bool arg_dump_core = true;
 static bool arg_crash_shell = false;
 static int arg_crash_chvt = -1;
 static bool arg_confirm_spawn = false;
-static bool arg_show_status = true;
+static ShowStatus arg_show_status = SHOW_STATUS_UNSET;
 static bool arg_switched_root = false;
 static char ***arg_join_controllers = NULL;
 static ExecOutput arg_default_std_output = EXEC_OUTPUT_JOURNAL;
@@ -338,10 +338,9 @@ static int parse_proc_cmdline_word(const char *word) {
         } else if (startswith(word, "systemd.show_status=")) {
                 int r;
 
-                if ((r = parse_boolean(word + 20)) < 0)
+                r = parse_show_status(word + 20, &arg_show_status);
+                if (r < 0)
                         log_warning("Failed to parse show status switch %s. Ignoring.", word + 20);
-                else
-                        arg_show_status = r;
         } else if (startswith(word, "systemd.default_standard_output=")) {
                 int r;
 
@@ -396,7 +395,7 @@ static int parse_proc_cmdline_word(const char *word) {
                                  "systemd.crash_shell=0|1                  Run shell on crash\n"
                                  "systemd.crash_chvt=N                     Change to VT #N on crash\n"
                                  "systemd.confirm_spawn=0|1                Confirm every process spawn\n"
-                                 "systemd.show_status=0|1                  Show status updates on the console during bootup\n"
+                                 "systemd.show_status=0|1|auto             Show status updates on the console during bootup\n"
                                  "systemd.log_target=console|kmsg|journal|journal-or-kmsg|syslog|syslog-or-kmsg|null\n"
                                  "                                         Log target\n"
                                  "systemd.log_level=LEVEL                  Log level\n"
@@ -409,9 +408,10 @@ static int parse_proc_cmdline_word(const char *word) {
                                  "systemd.setenv=ASSIGNMENT                Set an environment variable for all spawned processes\n");
                 }
 
-        } else if (streq(word, "quiet"))
-                arg_show_status = false;
-        else if (streq(word, "debug")) {
+        } else if (streq(word, "quiet")) {
+                if (arg_show_status == SHOW_STATUS_UNSET)
+                        arg_show_status = SHOW_STATUS_AUTO;
+        } else if (streq(word, "debug")) {
                 /* Log to kmsg, the journal socket will fill up before the
                  * journal is started and tools running during that time
                  * will block with every log message for for 60 seconds,
@@ -638,7 +638,7 @@ static int parse_config_file(void) {
                 { "Manager", "LogLocation",           config_parse_location,     0, NULL                     },
                 { "Manager", "DumpCore",              config_parse_bool,         0, &arg_dump_core           },
                 { "Manager", "CrashShell",            config_parse_bool,         0, &arg_crash_shell         },
-                { "Manager", "ShowStatus",            config_parse_bool,         0, &arg_show_status         },
+                { "Manager", "ShowStatus",            config_parse_show_status,  0, &arg_show_status         },
                 { "Manager", "CrashChVT",             config_parse_int,          0, &arg_crash_chvt          },
                 { "Manager", "CPUAffinity",           config_parse_cpu_affinity2, 0, NULL                    },
                 { "Manager", "DefaultStandardOutput", config_parse_output,       0, &arg_default_std_output  },
@@ -897,12 +897,14 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 case ARG_SHOW_STATUS:
-                        r = optarg ? parse_boolean(optarg) : 1;
-                        if (r < 0) {
-                                log_error("Failed to parse show status boolean %s.", optarg);
-                                return r;
-                        }
-                        arg_show_status = r;
+                        if (optarg) {
+                                r = parse_show_status(optarg, &arg_show_status);
+                                if (r < 0) {
+                                        log_error("Failed to parse show status boolean %s.", optarg);
+                                        return r;
+                                }
+                        } else
+                                arg_show_status = SHOW_STATUS_YES;
                         break;
 
                 case ARG_DESERIALIZE: {
@@ -1482,7 +1484,7 @@ int main(int argc, char *argv[]) {
         }
 
         if (arg_running_as == SYSTEMD_SYSTEM && !skip_setup) {
-                if (arg_show_status || plymouth_running())
+                if (arg_show_status > 0 || plymouth_running())
                         status_welcome();
 
 #ifdef HAVE_KMOD
@@ -1557,6 +1559,8 @@ int main(int argc, char *argv[]) {
         if (arg_default_environment)
                 manager_environment_add(m, NULL, arg_default_environment);
 
+        if (arg_show_status == SHOW_STATUS_UNSET)
+                arg_show_status = SHOW_STATUS_YES;
         manager_set_show_status(m, arg_show_status);
 
         /* Remember whether we should queue the default job */
diff --git a/src/core/manager.c b/src/core/manager.c
index 9ed8023..9f615e6 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -102,7 +102,7 @@ static int manager_watch_jobs_in_progress(Manager *m) {
         if (m->jobs_in_progress_event_source)
                 return 0;
 
-        return sd_event_add_monotonic(m->event, JOBS_IN_PROGRESS_WAIT_USEC, 0, manager_dispatch_jobs_in_progress, m, &m->jobs_in_progress_event_source);
+        return sd_event_add_monotonic(m->event, now(CLOCK_MONOTONIC) + JOBS_IN_PROGRESS_WAIT_USEC, 0, manager_dispatch_jobs_in_progress, m, &m->jobs_in_progress_event_source);
 }
 
 #define CYLON_BUFFER_EXTRA (2*(sizeof(ANSI_RED_ON)-1) + sizeof(ANSI_HIGHLIGHT_RED_ON)-1 + 2*(sizeof(ANSI_HIGHLIGHT_OFF)-1))
@@ -148,6 +148,9 @@ static void manager_print_jobs_in_progress(Manager *m) {
 
         assert(m);
 
+        if (m->show_status == SHOW_STATUS_AUTO)
+                manager_set_show_status(m, SHOW_STATUS_TEMPORARY);
+
         print_nr = (m->jobs_in_progress_iteration / JOBS_IN_PROGRESS_PERIOD_DIVISOR) % m->n_running_jobs;
 
         HASHMAP_FOREACH(j, m->jobs, i)
@@ -1637,12 +1640,12 @@ static int manager_dispatch_signal_fd(sd_event_source *source, int fd, uint32_t
 
                         case 20:
                                 log_debug("Enabling showing of status.");
-                                manager_set_show_status(m, true);
+                                manager_set_show_status(m, SHOW_STATUS_YES);
                                 break;
 
                         case 21:
                                 log_debug("Disabling showing of status.");
-                                manager_set_show_status(m, false);
+                                manager_set_show_status(m, SHOW_STATUS_NO);
                                 break;
 
                         case 22:
@@ -2456,6 +2459,9 @@ void manager_check_finished(Manager *m) {
                 return;
         }
 
+        if (m->show_status == SHOW_STATUS_TEMPORARY)
+                manager_set_show_status(m, SHOW_STATUS_AUTO);
+
         /* Notify Type=idle units that we are done now */
         m->idle_pipe_event_source = sd_event_source_unref(m->idle_pipe_event_source);
         manager_close_idle_pipe(m);
@@ -2754,15 +2760,16 @@ void manager_recheck_journal(Manager *m) {
         log_open();
 }
 
-void manager_set_show_status(Manager *m, bool b) {
+void manager_set_show_status(Manager *m, ShowStatus mode) {
         assert(m);
+        assert(IN_SET(mode, SHOW_STATUS_AUTO, SHOW_STATUS_NO, SHOW_STATUS_YES, SHOW_STATUS_TEMPORARY));
 
         if (m->running_as != SYSTEMD_SYSTEM)
                 return;
 
-        m->show_status = b;
+        m->show_status = mode;
 
-        if (b)
+        if (mode > 0)
                 touch("/run/systemd/show-status");
         else
                 unlink("/run/systemd/show-status");
@@ -2777,7 +2784,7 @@ static bool manager_get_show_status(Manager *m) {
         if (m->no_console_output)
                 return false;
 
-        if (m->show_status)
+        if (m->show_status > 0)
                 return true;
 
         /* If Plymouth is running make sure we show the status, so
diff --git a/src/core/manager.h b/src/core/manager.h
index fdf60ae..3065822 100644
--- a/src/core/manager.h
+++ b/src/core/manager.h
@@ -57,6 +57,7 @@ typedef enum ManagerExitCode {
 #include "path-lookup.h"
 #include "execute.h"
 #include "unit-name.h"
+#include "exit-status.h"
 
 struct Manager {
         /* Note that the set of units we know of is allowed to be
@@ -198,7 +199,7 @@ struct Manager {
 
         bool taint_usr:1;
 
-        bool show_status;
+        ShowStatus show_status;
         bool confirm_spawn;
         bool no_console_output;
 
@@ -296,7 +297,7 @@ void manager_undo_generators(Manager *m);
 
 void manager_recheck_journal(Manager *m);
 
-void manager_set_show_status(Manager *m, bool b);
+void manager_set_show_status(Manager *m, ShowStatus mode);
 void manager_status_printf(Manager *m, bool ephemeral, const char *status, const char *format, ...) _printf_(4,5);
 
 Set *manager_get_units_requiring_mounts_for(Manager *m, const char *path);
diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c
index 1e3cee5..df4e961 100644
--- a/src/shared/conf-parser.c
+++ b/src/shared/conf-parser.c
@@ -528,6 +528,35 @@ int config_parse_bool(const char* unit,
         return 0;
 }
 
+int config_parse_show_status(const char* unit,
+                             const char *filename,
+                             unsigned line,
+                             const char *section,
+                             unsigned section_line,
+                             const char *lvalue,
+                             int ltype,
+                             const char *rvalue,
+                             void *data,
+                             void *userdata) {
+
+        int k;
+        ShowStatus *b = data;
+
+        assert(filename);
+        assert(lvalue);
+        assert(rvalue);
+        assert(data);
+
+        k = parse_show_status(rvalue, b);
+        if (k < 0) {
+                log_syntax(unit, LOG_ERR, filename, line, -k,
+                           "Failed to parse show status setting, ignoring: %s", rvalue);
+                return 0;
+        }
+
+        return 0;
+}
+
 int config_parse_string(const char *unit,
                         const char *filename,
                         unsigned line,
diff --git a/src/shared/conf-parser.h b/src/shared/conf-parser.h
index 2d5aa31..ebbcaa5 100644
--- a/src/shared/conf-parser.h
+++ b/src/shared/conf-parser.h
@@ -100,6 +100,7 @@ int config_parse_double(const char *unit, const char *filename, unsigned line, c
 int config_parse_bytes_size(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
 int config_parse_bytes_off(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
 int config_parse_bool(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
+int config_parse_show_status(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
 int config_parse_string(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
 int config_parse_path(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
 int config_parse_strv(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
diff --git a/src/shared/exit-status.c b/src/shared/exit-status.c
index 45131f2..ef2d63f 100644
--- a/src/shared/exit-status.c
+++ b/src/shared/exit-status.c
@@ -190,3 +190,20 @@ bool is_clean_exit_lsb(int code, int status, ExitStatusSet *success_status) {
                 code == CLD_EXITED &&
                 (status == EXIT_NOTINSTALLED || status == EXIT_NOTCONFIGURED);
 }
+
+int parse_show_status(const char *v, ShowStatus *ret) {
+        int r;
+
+        assert(v);
+        assert(ret);
+
+        if (streq(v, "auto")) {
+                *ret = SHOW_STATUS_AUTO;
+                return 0;
+        }
+        r = parse_boolean(v);
+        if (r < 0)
+                return r;
+        *ret = r ? SHOW_STATUS_YES : SHOW_STATUS_NO;
+        return 0;
+}
diff --git a/src/shared/exit-status.h b/src/shared/exit-status.h
index 1f035a3..1ecf9d5 100644
--- a/src/shared/exit-status.h
+++ b/src/shared/exit-status.h
@@ -86,3 +86,15 @@ const char* exit_status_to_string(ExitStatus status, ExitStatusLevel level) _con
 
 bool is_clean_exit(int code, int status, ExitStatusSet *success_status);
 bool is_clean_exit_lsb(int code, int status, ExitStatusSet *success_status);
+
+/* Manager status */
+
+typedef enum ShowStatus {
+        SHOW_STATUS_UNSET = -2,
+        SHOW_STATUS_AUTO = -1,
+        SHOW_STATUS_NO = 0,
+        SHOW_STATUS_YES = 1,
+        SHOW_STATUS_TEMPORARY = 2,
+} ShowStatus;
+
+int parse_show_status(const char *v, ShowStatus *ret);

commit 65b3903ff576488eaabb51d3c4fbf9c73d867d7c
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Sat Jan 25 23:35:28 2014 -0500

    journal: guarantee async-signal-safety in sd_journald_sendv
    
    signal(7) provides a list of functions which may be called from a
    signal handler. Other functions, which only call those functions and
    don't access global memory and are reentrant are also safe.
    sd_j_sendv was mostly OK, but would call mkostemp and writev in a
    fallback path, which are unsafe.
    
    Being able to call sd_j_sendv in a async-signal-safe way is important
    because it allows it be used in signal handlers.
    
    Safety is achieved by replacing mkostemp with open(O_TMPFILE) and an
    open-coded writev replacement which uses write. Unfortunately,
    O_TMPFILE is only available on kernels >= 3.11. When O_TMPFILE is
    unavailable, an open-coded mkostemp is used.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=722889

diff --git a/.gitignore b/.gitignore
index 36b91b4..271f225 100644
--- a/.gitignore
+++ b/.gitignore
@@ -33,7 +33,7 @@
 /hostnamectl
 /install-tree
 /journalctl
-/libsystemd-login.c
+/libsystemd-*.c
 /libtool
 /localectl
 /loginctl
@@ -184,6 +184,7 @@
 /test-strxcpyx
 /test-tables
 /test-time
+/test-tmpfiles
 /test-udev
 /test-unit-file
 /test-unit-name
diff --git a/Makefile.am b/Makefile.am
index 23f7d2f..49ac55f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1126,6 +1126,7 @@ tests += \
 	test-utf8 \
 	test-ellipsize \
 	test-util \
+	test-tmpfiles \
 	test-namespace \
 	test-date \
 	test-sleep \
@@ -1232,6 +1233,12 @@ test_util_SOURCES = \
 test_util_LDADD = \
 	libsystemd-core.la
 
+test_tmpfiles_SOURCES = \
+	src/test/test-tmpfiles.c
+
+test_tmpfiles_LDADD = \
+	libsystemd-shared.la
+
 test_namespace_SOURCES = \
 	src/test/test-namespace.c
 
diff --git a/man/sd_journal_print.xml b/man/sd_journal_print.xml
index a716cc3..57d908f 100644
--- a/man/sd_journal_print.xml
+++ b/man/sd_journal_print.xml
@@ -153,8 +153,8 @@
                 for details) instead of the format string. Each
                 structure should reference one field of the entry to
                 submit. The second argument specifies the number of
-                structures in the
-                array. <function>sd_journal_sendv()</function> is
+                structures in the array.
+                <function>sd_journal_sendv()</function> is
                 particularly useful to submit binary objects to the
                 journal where that is necessary.</para>
 
@@ -221,6 +221,19 @@ sd_journal_send("MESSAGE=Hello World, this is PID %lu!", (unsigned long) getpid(
         </refsect1>
 
         <refsect1>
+                <title>Async signal safety</title>
+                <para><function>sd_journal_sendv()</function> is "async signal
+                safe" in the meaning of <citerefentry><refentrytitle>signal</refentrytitle><manvolnum>7</manvolnum></citerefentry>.
+                </para>
+
+                <para><function>sd_journal_print</function>,
+                <function>sd_journal_printv</function>,
+                <function>sd_journal_send</function>, and
+                <function>sd_journal_perror</function> are
+                not async signal safe.</para>
+        </refsect1>
+
+        <refsect1>
                 <title>Notes</title>
 
                 <para>The <function>sd_journal_print()</function>,
@@ -234,6 +247,16 @@ sd_journal_send("MESSAGE=Hello World, this is PID %lu!", (unsigned long) getpid(
         </refsect1>
 
         <refsect1>
+                <title>History</title>
+
+                <para><function>sd_journal_sendv()</function> was
+                modified to guarantee async-signal-safety in
+                systemd-209. Before that, it would behave safely only
+                when entry size was small enough to fit in one (large)
+                datagram.</para>
+        </refsect1>
+
+        <refsect1>
                 <title>See Also</title>
 
                 <para>
@@ -243,7 +266,9 @@ sd_journal_send("MESSAGE=Hello World, this is PID %lu!", (unsigned long) getpid(
                         <citerefentry><refentrytitle>syslog</refentrytitle><manvolnum>3</manvolnum></citerefentry>,
                         <citerefentry><refentrytitle>perror</refentrytitle><manvolnum>3</manvolnum></citerefentry>,
                         <citerefentry><refentrytitle>errno</refentrytitle><manvolnum>3</manvolnum></citerefentry>,
-                        <citerefentry><refentrytitle>systemd.journal-fields</refentrytitle><manvolnum>7</manvolnum></citerefentry>
+                        <citerefentry><refentrytitle>systemd.journal-fields</refentrytitle><manvolnum>7</manvolnum></citerefentry>,
+                        <citerefentry><refentrytitle>signal</refentrytitle><manvolnum>7</manvolnum></citerefentry>,
+                        <citerefentry><refentrytitle>socket</refentrytitle><manvolnum>7</manvolnum></citerefentry>
                 </para>
         </refsect1>
 
diff --git a/src/journal/journal-send.c b/src/journal/journal-send.c
index ca9199f..960c577 100644
--- a/src/journal/journal-send.c
+++ b/src/journal/journal-send.c
@@ -314,7 +314,7 @@ _public_ int sd_journal_sendv(const struct iovec *iov, int n) {
         if (buffer_fd < 0)
                 return buffer_fd;
 
-        n = writev(buffer_fd, w, j);
+        n = writev_safe(buffer_fd, w, j);
         if (n < 0) {
                 close_nointr_nofail(buffer_fd);
                 return -errno;
diff --git a/src/shared/def.h b/src/shared/def.h
index a2304fd..7777756 100644
--- a/src/shared/def.h
+++ b/src/shared/def.h
@@ -44,6 +44,7 @@
 #define LOWERCASE_LETTERS "abcdefghijklmnopqrstuvwxyz"
 #define UPPERCASE_LETTERS "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
 #define LETTERS LOWERCASE_LETTERS UPPERCASE_LETTERS
+#define ALPHANUMERICAL LETTERS DIGITS
 
 #define REBOOT_PARAM_FILE "/run/systemd/reboot-param"
 
diff --git a/src/shared/missing.h b/src/shared/missing.h
index 6c038d9..4e62100 100644
--- a/src/shared/missing.h
+++ b/src/shared/missing.h
@@ -301,25 +301,29 @@ static inline int name_to_handle_at(int fd, const char *name, struct file_handle
 #endif
 
 #ifndef CIFS_MAGIC_NUMBER
-#define CIFS_MAGIC_NUMBER 0xFF534D42
+#  define CIFS_MAGIC_NUMBER 0xFF534D42
 #endif
 
 #ifndef TFD_TIMER_CANCEL_ON_SET
-#define TFD_TIMER_CANCEL_ON_SET (1 << 1)
+#  define TFD_TIMER_CANCEL_ON_SET (1 << 1)
 #endif
 
 #ifndef SO_REUSEPORT
-#define SO_REUSEPORT 15
+#  define SO_REUSEPORT 15
 #endif
 
 #ifndef EVIOCREVOKE
-#define EVIOCREVOKE _IOW('E', 0x91, int)
+#  define EVIOCREVOKE _IOW('E', 0x91, int)
 #endif
 
 #ifndef DRM_IOCTL_SET_MASTER
-#define DRM_IOCTL_SET_MASTER _IO('d', 0x1e)
+#  define DRM_IOCTL_SET_MASTER _IO('d', 0x1e)
 #endif
 
 #ifndef DRM_IOCTL_DROP_MASTER
-#define DRM_IOCTL_DROP_MASTER _IO('d', 0x1f)
+#  define DRM_IOCTL_DROP_MASTER _IO('d', 0x1f)
+#endif
+
+#ifndef TMP_MAX
+# define TMP_MAX 238328
 #endif
diff --git a/src/shared/util.c b/src/shared/util.c
index 27fc959..a437d9b 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -6109,6 +6109,53 @@ int getpeersec(int fd, char **ret) {
         return 0;
 }
 
+int writev_safe(int fd, const struct iovec *w, int j) {
+        for (int i = 0; i < j; i++) {
+                size_t written = 0;
+
+                while (written < w[i].iov_len) {
+                        ssize_t r;
+
+                        r = write(fd, (char*) w[i].iov_base + written, w[i].iov_len - written);
+                        if (r < 0 && errno != -EINTR)
+                                return -errno;
+
+                        written += r;
+                }
+        }
+
+        return 0;
+}
+
+int mkostemp_safe(char *pattern, int flags) {
+        char *s = pattern + strlen(pattern) - 6;
+        uint64_t tries = TMP_MAX;
+        int randfd, fd, i;
+
+        assert(streq(s, "XXXXXX"));
+
+        randfd = open("/dev/urandom", O_RDONLY);
+        if (randfd < 0)
+                return -ENOSYS;
+
+        while (tries--) {
+                fd = read(randfd, s, 6);
+                if (fd == 0)
+                        return -ENOSYS;
+
+                for (i = 0; i < 6; i++)
+                        s[i] = ALPHANUMERICAL[(unsigned) s[i] % strlen(ALPHANUMERICAL)];
+
+                fd = open(pattern, flags|O_EXCL|O_CREAT, S_IRUSR|S_IWUSR);
+                if (fd >= 0)
+                        return fd;
+                if (!IN_SET(errno, EEXIST, EINTR))
+                        return -errno;
+        }
+
+        return -EEXIST;
+}
+
 int open_tmpfile(const char *path, int flags) {
         int fd;
         char *p;
@@ -6120,12 +6167,9 @@ int open_tmpfile(const char *path, int flags) {
 #endif
         p = strappenda(path, "/systemd-tmp-XXXXXX");
 
-        RUN_WITH_UMASK(0077) {
-                fd = mkostemp(p, O_RDWR|O_CLOEXEC);
-        }
-
+        fd = mkostemp_safe(p, O_RDWR|O_CLOEXEC);
         if (fd < 0)
-                return -errno;
+                return fd;
 
         unlink(p);
         return fd;
diff --git a/src/shared/util.h b/src/shared/util.h
index 630137a..1169864 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -850,4 +850,7 @@ bool pid_valid(pid_t pid);
 int getpeercred(int fd, struct ucred *ucred);
 int getpeersec(int fd, char **ret);
 
+int writev_safe(int fd, const struct iovec *w, int j);
+
+int mkostemp_safe(char *pattern, int flags);
 int open_tmpfile(const char *path, int flags);
diff --git a/src/test/test-tmpfiles.c b/src/test/test-tmpfiles.c
new file mode 100644
index 0000000..f25a0dc
--- /dev/null
+++ b/src/test/test-tmpfiles.c
@@ -0,0 +1,51 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2014 Zbigniew Jędrzejewski-Szmek
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 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
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdio.h>
+
+#include "util.h"
+
+int main(int argc, char** argv) {
+        const char *p = argv[1] ?: "/tmp";
+        char *pattern = strappenda(p, "/systemd-test-XXXXXX");
+        _cleanup_close_ int fd, fd2;
+        _cleanup_free_ char *cmd, *cmd2;
+
+        fd = open_tmpfile(p, O_RDWR);
+        assert(fd >= 0);
+
+        assert_se(asprintf(&cmd, "ls -l /proc/"PID_FMT"/fd/%d", getpid(), fd) > 0);
+        system(cmd);
+
+        fd2 = mkostemp_safe(pattern, O_RDWR);
+        assert(fd >= 0);
+        assert_se(unlink(pattern) == 0);
+
+        assert_se(asprintf(&cmd2, "ls -l /proc/"PID_FMT"/fd/%d", getpid(), fd2) > 0);
+        system(cmd2);
+
+        return 0;
+}
diff --git a/src/test/test-util.c b/src/test/test-util.c
index 05b2294..f819589 100644
--- a/src/test/test-util.c
+++ b/src/test/test-util.c
@@ -28,6 +28,8 @@
 
 #include "util.h"
 #include "strv.h"
+#include "def.h"
+#include "fileio.h"
 
 static void test_streq_ptr(void) {
         assert_se(streq_ptr(NULL, NULL));
@@ -578,6 +580,29 @@ static void test_in_set(void) {
         assert_se(!IN_SET(0, 1, 2, 3, 4));
 }
 
+static void test_writev_safe(void) {
+        char name[] = "/tmp/test-writev_safe.XXXXXX";
+        _cleanup_free_ char *contents;
+        size_t size;
+        int fd, r;
+
+        struct iovec iov[3];
+        IOVEC_SET_STRING(iov[0], "abc\n");
+        IOVEC_SET_STRING(iov[1], ALPHANUMERICAL "\n");
+        IOVEC_SET_STRING(iov[2], "");
+
+        fd = mkstemp(name);
+        printf("test_writev_safe: %s", name);
+
+        r = writev_safe(fd, iov, 3);
+        assert(r == 0);
+
+        r = read_full_file(name, &contents, &size);
+        assert(r == 0);
+        printf("contents: %s", contents);
+        assert(streq(contents, "abc\n" ALPHANUMERICAL "\n"));
+}
+
 int main(int argc, char *argv[]) {
         test_streq_ptr();
         test_first_word();
@@ -615,6 +640,7 @@ int main(int argc, char *argv[]) {
         test_fstab_node_to_udev_node();
         test_get_files_in_directory();
         test_in_set();
+        test_writev_safe();
 
         return 0;
 }

commit 8e33886ec582336564ae11b80023abe93d7599c0
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Sat Jan 25 20:48:01 2014 -0500

    Replace mkostemp+unlink with open(O_TMPFILE)
    
    This will only work on Linux >= 3.11, and probably not on all
    filesystems. Fallback code is provided.

diff --git a/src/core/manager.c b/src/core/manager.c
index 9d43a1c..9ed8023 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -2005,28 +2005,17 @@ void manager_dispatch_bus_name_owner_changed(
 }
 
 int manager_open_serialization(Manager *m, FILE **_f) {
-        _cleanup_free_ char *path = NULL;
+        const char *path;
         int fd = -1;
         FILE *f;
 
         assert(_f);
 
-        if (m->running_as == SYSTEMD_SYSTEM)
-                asprintf(&path, "/run/systemd/dump-"PID_FMT"-XXXXXX", getpid());
-        else
-                asprintf(&path, "/tmp/systemd-dump-"PID_FMT"-XXXXXX", getpid());
-
-        if (!path)
-                return -ENOMEM;
-
-        RUN_WITH_UMASK(0077) {
-                fd = mkostemp(path, O_RDWR|O_CLOEXEC);
-        }
-
+        path = m->running_as == SYSTEMD_SYSTEM ? "/run/systemd" : "/tmp";
+        fd = open_tmpfile(path, O_RDWR|O_CLOEXEC);
         if (fd < 0)
                 return -errno;
 
-        unlink(path);
         log_debug("Serializing state to %s", path);
 
         f = fdopen(fd, "w+");
diff --git a/src/journal/journal-send.c b/src/journal/journal-send.c
index 281e154..ca9199f 100644
--- a/src/journal/journal-send.c
+++ b/src/journal/journal-send.c
@@ -216,10 +216,6 @@ _public_ int sd_journal_sendv(const struct iovec *iov, int n) {
                 uint8_t buf[CMSG_SPACE(sizeof(int))];
         } control;
         struct cmsghdr *cmsg;
-        /* We use /dev/shm instead of /tmp here, since we want this to
-         * be a tmpfs, and one that is available from early boot on
-         * and where unprivileged users can create files. */
-        char path[] = "/dev/shm/journal.XXXXXX";
         bool have_syslog_identifier = false;
 
         assert_return(iov, -EINVAL);
@@ -309,16 +305,14 @@ _public_ int sd_journal_sendv(const struct iovec *iov, int n) {
 
         /* Message doesn't fit... Let's dump the data in a temporary
          * file and just pass a file descriptor of it to the other
-         * side */
-
-        buffer_fd = mkostemp(path, O_CLOEXEC|O_RDWR);
+         * side.
+         *
+         * We use /dev/shm instead of /tmp here, since we want this to
+         * be a tmpfs, and one that is available from early boot on
+         * and where unprivileged users can create files. */
+        buffer_fd = open_tmpfile("/dev/shm", O_RDWR | O_CLOEXEC);
         if (buffer_fd < 0)
-                return -errno;
-
-        if (unlink(path) < 0) {
-                close_nointr_nofail(buffer_fd);
-                return -errno;
-        }
+                return buffer_fd;
 
         n = writev(buffer_fd, w, j);
         if (n < 0) {
diff --git a/src/journal/journal-verify.c b/src/journal/journal-verify.c
index f2422ff..9434cc9 100644
--- a/src/journal/journal-verify.c
+++ b/src/journal/journal-verify.c
@@ -785,9 +785,6 @@ int journal_file_verify(
         uint64_t n_weird = 0, n_objects = 0, n_entries = 0, n_data = 0, n_fields = 0, n_data_hash_tables = 0, n_field_hash_tables = 0, n_entry_arrays = 0, n_tags = 0;
         usec_t last_usec = 0;
         int data_fd = -1, entry_fd = -1, entry_array_fd = -1;
-        char data_path[] = "/var/tmp/journal-data-XXXXXX",
-                entry_path[] = "/var/tmp/journal-entry-XXXXXX",
-                entry_array_path[] = "/var/tmp/journal-entry-array-XXXXXX";
         unsigned i;
         bool found_last;
 #ifdef HAVE_GCRYPT
@@ -808,29 +805,26 @@ int journal_file_verify(
         } else if (f->seal)
                 return -ENOKEY;
 
-        data_fd = mkostemp(data_path, O_CLOEXEC);
+        data_fd = open_tmpfile("/var/tmp", O_RDWR | O_CLOEXEC);
         if (data_fd < 0) {
                 log_error("Failed to create data file: %m");
                 r = -errno;
                 goto fail;
         }
-        unlink(data_path);
 
-        entry_fd = mkostemp(entry_path, O_CLOEXEC);
+        entry_fd = open_tmpfile("/var/tmp", O_RDWR | O_CLOEXEC);
         if (entry_fd < 0) {
                 log_error("Failed to create entry file: %m");
                 r = -errno;
                 goto fail;
         }
-        unlink(entry_path);
 
-        entry_array_fd = mkostemp(entry_array_path, O_CLOEXEC);
+        entry_array_fd = open_tmpfile("/var/tmp", O_RDWR | O_CLOEXEC);
         if (entry_array_fd < 0) {
                 log_error("Failed to create entry array file: %m");
                 r = -errno;
                 goto fail;
         }
-        unlink(entry_array_path);
 
 #ifdef HAVE_GCRYPT
         if ((le32toh(f->header->compatible_flags) & ~HEADER_COMPATIBLE_SEALED) != 0)
diff --git a/src/shared/util.c b/src/shared/util.c
index cdd9a48..27fc959 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -6108,3 +6108,25 @@ int getpeersec(int fd, char **ret) {
         *ret = s;
         return 0;
 }
+
+int open_tmpfile(const char *path, int flags) {
+        int fd;
+        char *p;
+
+#ifdef O_TMPFILE
+        fd = open(path, flags|O_TMPFILE, S_IRUSR|S_IWUSR);
+        if (fd >= 0)
+                return fd;
+#endif
+        p = strappenda(path, "/systemd-tmp-XXXXXX");
+
+        RUN_WITH_UMASK(0077) {
+                fd = mkostemp(p, O_RDWR|O_CLOEXEC);
+        }
+
+        if (fd < 0)
+                return -errno;
+
+        unlink(p);
+        return fd;
+}
diff --git a/src/shared/util.h b/src/shared/util.h
index d6d746b..630137a 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -849,3 +849,5 @@ bool pid_valid(pid_t pid);
 
 int getpeercred(int fd, struct ucred *ucred);
 int getpeersec(int fd, char **ret);
+
+int open_tmpfile(const char *path, int flags);



More information about the systemd-commits mailing list