[systemd-commits] 13 commits - .gitignore Makefile.am man/systemctl.xml man/systemd.exec.xml man/systemd.service.xml src/core src/journal src/shared src/systemctl src/test test/sched_idle_bad.service test/sched_idle_ok.service test/sched_rr_bad.service test/sched_rr_change.service test/sched_rr_ok.service

Zbigniew Jędrzejewski-Szmek zbyszek at kemper.freedesktop.org
Thu Nov 15 07:59:48 PST 2012


 .gitignore                            |    1 
 Makefile.am                           |   29 ++++++-
 man/systemctl.xml                     |    5 +
 man/systemd.exec.xml                  |   14 ++-
 man/systemd.service.xml               |  107 +++++++++++++-------------
 src/core/dbus-manager.c               |   10 +-
 src/core/load-fragment-gperf.gperf.m4 |    1 
 src/core/load-fragment.c              |   40 +++++++---
 src/core/transaction.c                |    3 
 src/core/unit-printf.c                |   12 ---
 src/journal/journalctl.c              |    2 
 src/shared/unit-name.c                |    4 -
 src/shared/unit-name.h                |    4 -
 src/systemctl/systemctl.c             |   27 ++++++
 src/test/test-sched-prio.c            |   86 +++++++++++++++++++++
 src/test/test-unit-file.c             |  135 +++++++++++++++++++++++++++++++++-
 test/sched_idle_bad.service           |    6 +
 test/sched_idle_ok.service            |    6 +
 test/sched_rr_bad.service             |    8 ++
 test/sched_rr_change.service          |    9 ++
 test/sched_rr_ok.service              |    6 +
 21 files changed, 420 insertions(+), 95 deletions(-)

New commits:
commit f72daa64dcfa73c8427663be53d49393e0cbb343
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Thu Nov 15 16:30:24 2012 +0100

    dbus-manager: modernize style

diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index 3cf3e90..262e5ff 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -740,7 +740,8 @@ static DBusHandlerResult bus_manager_message_handler(DBusConnection *connection,
                 if (r < 0)
                         return bus_send_error_reply(connection, message, &error, r);
 
-                if (!(reply = dbus_message_new_method_return(message)))
+                reply = dbus_message_new_method_return(message);
+                if (!reply)
                         goto oom;
 
         } else if (dbus_message_is_method_call(message, "org.freedesktop.systemd1.Manager", "GetJob")) {
@@ -1358,7 +1359,8 @@ static DBusHandlerResult bus_manager_message_handler(DBusConnection *connection,
                 if (!e)
                         goto oom;
 
-                if (!(reply = dbus_message_new_method_return(message))) {
+                reply = dbus_message_new_method_return(message);
+                if (!reply) {
                         strv_free(e);
                         goto oom;
                 }
@@ -1410,7 +1412,8 @@ static DBusHandlerResult bus_manager_message_handler(DBusConnection *connection,
                 if (!f)
                         goto oom;
 
-                if (!(reply = dbus_message_new_method_return(message))) {
+                reply = dbus_message_new_method_return(message);
+                if (!reply) {
                         strv_free(f);
                         goto oom;
                 }

commit 645a9e5a2bbb06464a3fba1a3501e9d79e5bbad8
Author: Eelco Dolstra <eelco.dolstra at logicblox.com>
Date:   Wed Oct 31 11:53:56 2012 +0100

    dbus-manager: fix a fatal dbus abort in bus_manager_message_handler()
    
    If ListUnitFiles fails, or an OOM occurs, then dbus_message_unref()
    will be called twice on "reply", causing systemd to crash.  So remove
    the call to dbus_message_unref(); it is unnecessary because of
    the cleanup attribute on "reply".
    
    [zj: modified to leave one dbus_message_unref() alone, per Colin
    Walters' comment.]

diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index 2010241..3cf3e90 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -1436,7 +1436,6 @@ static DBusHandlerResult bus_manager_message_handler(DBusConnection *connection,
                 r = unit_file_get_list(m->running_as == SYSTEMD_SYSTEM ? UNIT_FILE_SYSTEM : UNIT_FILE_USER, NULL, h);
                 if (r < 0) {
                         unit_file_list_free(h);
-                        dbus_message_unref(reply);
                         return bus_send_error_reply(connection, message, NULL, r);
                 }
 

commit bb11271068ff34434f5b8cefd0c2c0bae5ed7fd1
Author: Holger Hans Peter Freyther <holger at moiji-mobile.com>
Date:   Thu Nov 1 18:48:11 2012 +0100

    sched: Only setting CPUSchedulingPriority=rr doesn't work
    
    A service that only sets the scheduling policy to round-robin
    fails to be started. This is because the cpu_sched_priority is
    initialized to 0 and is not adjusted when the policy is changed.
    
    Clamp the cpu_sched_priority when the scheduler policy is set. Use
    the current policy to validate the new priority.
    
    Change the manual page to state that the given range only applies
    to the real-time scheduling policies.
    
    Add a testcase that verifies this change:
    
    $ make test-sched-prio; ./test-sched-prio
    [test/sched_idle_bad.service:6] CPU scheduling priority is out of range, ignoring: 1
    [test/sched_rr_bad.service:7] CPU scheduling priority is out of range, ignoring: 0
    [test/sched_rr_bad.service:8] CPU scheduling priority is out of range, ignoring: 100

diff --git a/.gitignore b/.gitignore
index 18e4e23..3fbd83e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -90,6 +90,7 @@
 /systemd
 /test-engine
 /test-job-type
+/test-sched-prio
 /systemctl
 /systemadm
 .dirstamp
diff --git a/Makefile.am b/Makefile.am
index da3885d..42fed59 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1185,7 +1185,8 @@ noinst_PROGRAMS += \
 	test-unit-file \
 	test-date \
 	test-sleep \
-	test-replace-var
+	test-replace-var \
+	test-sched-prio
 
 TESTS += \
 	test-job-type \
@@ -1195,7 +1196,15 @@ TESTS += \
 	test-unit-file \
 	test-date \
 	test-sleep \
-	test-replace-var
+	test-replace-var \
+	test-sched-prio
+
+EXTRA_DIST += \
+	test/sched_idle_bad.service \
+	test/sched_idle_ok.service \
+	test/sched_rr_bad.service \
+	test/sched_rr_ok.service \
+	test/sched_rr_change.service
 
 test_engine_SOURCES = \
 	src/test/test-engine.c
@@ -1323,6 +1332,18 @@ test_watchdog_SOURCES = \
 test_watchdog_LDADD = \
 	libsystemd-shared.la
 
+test_sched_prio_SOURCES = \
+	src/test/test-sched-prio.c
+
+test_sched_prio_CFLAGS = \
+	$(AM_CFLAGS) \
+	$(DBUS_CFLAGS) \
+	-D"STR(s)=\#s" -D"TEST_DIR=STR($(abs_top_srcdir)/test/)"
+
+test_sched_prio_LDADD = \
+	libsystemd-core.la \
+	libsystemd-daemon.la
+
 # ------------------------------------------------------------------------------
 systemd_initctl_SOURCES = \
 	src/initctl/initctl.c
diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml
index 7b65143..b684bfb 100644
--- a/man/systemd.exec.xml
+++ b/man/systemd.exec.xml
@@ -214,13 +214,15 @@
 
                                 <listitem><para>Sets the CPU
                                 scheduling priority for executed
-                                processes. Takes an integer between 1
-                                (lowest priority) and 99 (highest
-                                priority). The available priority
+                                processes. The available priority
                                 range depends on the selected CPU
-                                scheduling policy (see above). See
-                                <citerefentry><refentrytitle>sched_setscheduler</refentrytitle><manvolnum>2</manvolnum></citerefentry>
-                                for details.</para></listitem>
+                                scheduling policy (see above). For
+                                real-time scheduling policies an
+                                integer between 1 (lowest priority)
+                                and 99 (highest priority) can be used.
+                                See <citerefentry><refentrytitle>sched_setscheduler</refentrytitle><manvolnum>2</manvolnum></citerefentry>
+                                for details.
+                                </para></listitem>
                         </varlistentry>
 
                         <varlistentry>
diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index b99e70e..6759255 100644
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -4,6 +4,7 @@
   This file is part of systemd.
 
   Copyright 2010 Lennart Poettering
+  Copyright 2012 Holger Hans Peter Freyther
 
   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
@@ -689,6 +690,8 @@ int config_parse_exec_cpu_sched_policy(
         }
 
         c->cpu_sched_policy = x;
+        /* Moving to or from real-time policy? We need to adjust the priority */
+        c->cpu_sched_priority = CLAMP(c->cpu_sched_priority, sched_get_priority_min(x), sched_get_priority_max(x));
         c->cpu_sched_set = true;
 
         return 0;
@@ -705,19 +708,28 @@ int config_parse_exec_cpu_sched_prio(
                 void *userdata) {
 
         ExecContext *c = data;
-        int i;
+        int i, min, max;
 
         assert(filename);
         assert(lvalue);
         assert(rvalue);
         assert(data);
 
-        /* On Linux RR/FIFO have the same range */
-        if (safe_atoi(rvalue, &i) < 0 || i < sched_get_priority_min(SCHED_RR) || i > sched_get_priority_max(SCHED_RR)) {
+        if (safe_atoi(rvalue, &i) < 0) {
                 log_error("[%s:%u] Failed to parse CPU scheduling priority, ignoring: %s", filename, line, rvalue);
                 return 0;
         }
 
+
+        /* On Linux RR/FIFO range from 1 to 99 and OTHER/BATCH may only be 0 */
+        min = sched_get_priority_min(c->cpu_sched_policy);
+        max = sched_get_priority_max(c->cpu_sched_policy);
+
+        if (i < min || i > max) {
+                log_error("[%s:%u] CPU scheduling priority is out of range, ignoring: %s", filename, line, rvalue);
+                return 0;
+        }
+
         c->cpu_sched_priority = i;
         c->cpu_sched_set = true;
 
diff --git a/src/test/test-sched-prio.c b/src/test/test-sched-prio.c
new file mode 100644
index 0000000..29235e8
--- /dev/null
+++ b/src/test/test-sched-prio.c
@@ -0,0 +1,86 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2012 Holger Hans Peter Freyther
+
+  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 <sched.h>
+
+#include "manager.h"
+
+
+int main(int argc, char *argv[]) {
+        Manager *m;
+        Unit *idle_ok, *idle_bad, *rr_ok, *rr_bad, *rr_sched;
+        Service *ser;
+        FILE *serial = NULL;
+        FDSet *fdset = NULL;
+
+        /* prepare the test */
+        assert_se(set_unit_path(TEST_DIR) >= 0);
+        assert_se(manager_new(SYSTEMD_SYSTEM, &m) >= 0);
+        assert_se(manager_startup(m, serial, fdset) >= 0);
+
+        /* load idle ok */
+        assert_se(manager_load_unit(m, "sched_idle_ok.service", NULL, NULL, &idle_ok) >= 0);
+        assert_se(idle_ok->load_state == UNIT_LOADED);
+        ser = SERVICE(idle_ok);
+        assert_se(ser->exec_context.cpu_sched_policy == SCHED_OTHER);
+        assert_se(ser->exec_context.cpu_sched_priority == 0);
+
+        /*
+         * load idle bad. This should print a warning but we have no way to look at it.
+         */
+        assert_se(manager_load_unit(m, "sched_idle_bad.service", NULL, NULL, &idle_bad) >= 0);
+        assert_se(idle_bad->load_state == UNIT_LOADED);
+        ser = SERVICE(idle_ok);
+        assert_se(ser->exec_context.cpu_sched_policy == SCHED_OTHER);
+        assert_se(ser->exec_context.cpu_sched_priority == 0);
+
+        /*
+         * load rr ok.
+         * Test that the default priority is moving from 0 to 1.
+         */
+        assert_se(manager_load_unit(m, "sched_rr_ok.service", NULL, NULL, &rr_ok) >= 0);
+        assert_se(rr_ok->load_state == UNIT_LOADED);
+        ser = SERVICE(rr_ok);
+        assert_se(ser->exec_context.cpu_sched_policy == SCHED_RR);
+        assert_se(ser->exec_context.cpu_sched_priority == 1);
+
+        /*
+         * load rr bad.
+         * Test that the value of 0 and 100 is ignored.
+         */
+        assert_se(manager_load_unit(m, "sched_rr_bad.service", NULL, NULL, &rr_bad) >= 0);
+        assert_se(rr_bad->load_state == UNIT_LOADED);
+        ser = SERVICE(rr_bad);
+        assert_se(ser->exec_context.cpu_sched_policy == SCHED_RR);
+        assert_se(ser->exec_context.cpu_sched_priority == 1);
+
+        /*
+         * load rr change.
+         * Test that anything between 1 and 99 can be set.
+         */
+        assert_se(manager_load_unit(m, "sched_rr_change.service", NULL, NULL, &rr_sched) >= 0);
+        assert_se(rr_sched->load_state == UNIT_LOADED);
+        ser = SERVICE(rr_sched);
+        assert_se(ser->exec_context.cpu_sched_policy == SCHED_RR);
+        assert_se(ser->exec_context.cpu_sched_priority == 99);
+
+        return 0;
+}
diff --git a/test/sched_idle_bad.service b/test/sched_idle_bad.service
new file mode 100644
index 0000000..589a87c
--- /dev/null
+++ b/test/sched_idle_bad.service
@@ -0,0 +1,6 @@
+[Unit]
+Description=Bad sched priority for Idle
+
+[Service]
+ExecStart=/bin/true
+CPUSchedulingPriority=1
diff --git a/test/sched_idle_ok.service b/test/sched_idle_ok.service
new file mode 100644
index 0000000..262ef3e
--- /dev/null
+++ b/test/sched_idle_ok.service
@@ -0,0 +1,6 @@
+[Unit]
+Description=Sched idle with prio 0
+
+[Service]
+ExecStart=/bin/true
+CPUSchedulingPriority=0
diff --git a/test/sched_rr_bad.service b/test/sched_rr_bad.service
new file mode 100644
index 0000000..0be534a
--- /dev/null
+++ b/test/sched_rr_bad.service
@@ -0,0 +1,8 @@
+[Unit]
+Description=Bad sched priority for RR
+
+[Service]
+ExecStart=/bin/true
+CPUSchedulingPolicy=rr
+CPUSchedulingPriority=0
+CPUSchedulingPriority=100
diff --git a/test/sched_rr_change.service b/test/sched_rr_change.service
new file mode 100644
index 0000000..b3e3a00
--- /dev/null
+++ b/test/sched_rr_change.service
@@ -0,0 +1,9 @@
+[Unit]
+Description=Change prio
+
+[Service]
+ExecStart=/bin/true
+CPUSchedulingPolicy=rr
+CPUSchedulingPriority=1
+CPUSchedulingPriority=2
+CPUSchedulingPriority=99
diff --git a/test/sched_rr_ok.service b/test/sched_rr_ok.service
new file mode 100644
index 0000000..b88adc5
--- /dev/null
+++ b/test/sched_rr_ok.service
@@ -0,0 +1,6 @@
+[Unit]
+Description=Default prio for RR
+
+[Service]
+ExecStart=/bin/true
+CPUSchedulingPolicy=rr

commit 71c015969233c21ea38b1e63993d02fe171df672
Author: Lekensteyn <lekensteyn at gmail.com>
Date:   Thu Nov 15 12:17:03 2012 +0100

    journalctl: require argument for --priority
    
    This fixes a segfault due to a missing value for --priority. -p is
    unaffected because it is specified in the getopt_long parameter list.

diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
index cccd8a7..011a11b 100644
--- a/src/journal/journalctl.c
+++ b/src/journal/journalctl.c
@@ -157,7 +157,7 @@ static int parse_argv(int argc, char *argv[]) {
                 { "this-boot",    no_argument,       NULL, 'b'              },
                 { "directory",    required_argument, NULL, 'D'              },
                 { "header",       no_argument,       NULL, ARG_HEADER       },
-                { "priority",     no_argument,       NULL, 'p'              },
+                { "priority",     required_argument, NULL, 'p'              },
                 { "setup-keys",   no_argument,       NULL, ARG_SETUP_KEYS   },
                 { "interval",     required_argument, NULL, ARG_INTERVAL     },
                 { "verify",       no_argument,       NULL, ARG_VERIFY       },

commit 2480f0c6774daa062106f9c209d255f59c6a6c58
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Thu Nov 15 15:54:17 2012 +0100

    man: update description of ExecStart and friends
    
    Semicolon separated lines are supported for all those commands,
    and semicolons can now be escaped.

diff --git a/man/systemd.service.xml b/man/systemd.service.xml
index 8c1dfe1..ddb065e 100644
--- a/man/systemd.service.xml
+++ b/man/systemd.service.xml
@@ -296,45 +296,58 @@
 
                         <varlistentry>
                                 <term><varname>ExecStart=</varname></term>
-                                <listitem><para>Takes a command line
-                                that is executed when this service
-                                shall be started up. The first token
-                                of the command line must be an
-                                absolute file name, then followed by
-                                arguments for the process. It is
-                                mandatory to set this option for all
-                                services. This option may not be
-                                specified more than once, except when
+                                <listitem><para>Commands
+                                that are executed when this service is started.
+                                </para>
+
+                                <para>When
                                 <varname>Type=oneshot</varname> is
-                                used in which case more than one
-                                <varname>ExecStart=</varname> line is
-                                accepted which are then invoked one by
-                                one, sequentially in the order they
-                                appear in the unit file.</para>
+                                used, more than one command may be
+                                specified. Multiple command lines may
+                                be concatenated in a single directive,
+                                by separating them with semicolons
+                                (these semicolons must be passed as
+                                separate words). Alternatively, this
+                                directive may be specified more than
+                                once with the same effect. However,
+                                the latter syntax is not recommended
+                                for compatibility with parsers
+                                suitable for XDG
+                                <filename>.desktop</filename> files.
+                                The commands are invoked one by
+                                one sequentially in the order they
+                                appear in the unit file.
+                                When <varname>Type</varname> is
+                                not <option>oneshot</option>, only one
+                                command may be given. Lone semicolons
+                                may be escaped as
+                                '<literal>\;</literal>'.</para>
+
+                                <para>Unless
+                                <varname>Type=forking</varname> is
+                                set, the process started via this
+                                command line will be considered the
+                                main process of the daemon. The
+                                command line accepts '<literal>%</literal>'
+                                specifiers as described in
+                                <citerefentry><refentrytitle>systemd.unit</refentrytitle><manvolnum>5</manvolnum></citerefentry>.</para>
 
                                 <para>Optionally, if the absolute file
                                 name is prefixed with
-                                <literal>@</literal>, the second token
+                                '<literal>@</literal>', the second token
                                 will be passed as
                                 <literal>argv[0]</literal> to the
                                 executed process, followed by the
                                 further arguments specified. If the
-                                first token is prefixed with
-                                <literal>-</literal> an exit code of
+                                absolute file name is prefixed with
+                                '<literal>-</literal>' an exit code of
                                 the command normally considered a
                                 failure (i.e. non-zero exit status or
                                 abnormal exit due to signal) is ignored
                                 and considered success. If both
-                                <literal>-</literal> and
-                                <literal>@</literal> are used they
-                                can appear in either order. Unless
-                                <varname>Type=forking</varname> is
-                                set, the process started via this
-                                command line will be considered the
-                                main process of the daemon. The
-                                command line accepts % specifiers as
-                                described in
-                                <citerefentry><refentrytitle>systemd.unit</refentrytitle><manvolnum>5</manvolnum></citerefentry>.</para>
+                                '<literal>-</literal>' and
+                                '<literal>@</literal>' are used they
+                                can appear in either order.</para>
 
                                 <para>On top of that basic environment
                                 variable substitution is
@@ -380,24 +393,13 @@
                                 <listitem><para>Additional commands
                                 that are executed before or after
                                 the command in
-                                <varname>ExecStart=</varname>, respectively. Multiple
-                                command lines may be concatenated in a
-                                single directive, by separating them
-                                by semicolons (these semicolons must
-                                be passed as separate words). In that
-                                case, the commands are executed one
-                                after the other,
-                                serially. Alternatively, these
-                                directives may be specified more than
-                                once with the same effect. However,
-                                the latter syntax is not recommended
-                                for compatibility with parsers
-                                suitable for XDG
-                                <filename>.desktop</filename> files.
-                                Use of these settings is
-                                optional. Specifier and environment
-                                variable substitution is
-                                supported.</para></listitem>
+                                <varname>ExecStart=</varname>, respectively.
+                                Syntax is the same as for
+                                <varname>ExecStart=</varname>, except
+                                that multiple command lines are allowed
+                                and the commands are executed one
+                                after the other, serially.</para>
+                                </listitem>
                         </varlistentry>
 
                         <varlistentry>
@@ -406,8 +408,8 @@
                                 trigger a configuration reload in the
                                 service. This argument takes multiple
                                 command lines, following the same
-                                scheme as pointed out for
-                                <varname>ExecStartPre=</varname>
+                                scheme as described for
+                                <varname>ExecStart=</varname>
                                 above. Use of this setting is
                                 optional. Specifier and environment
                                 variable substitution is supported
@@ -428,9 +430,8 @@
                                 stop the service started via
                                 <varname>ExecStart=</varname>. This
                                 argument takes multiple command lines,
-                                following the same scheme as pointed
-                                out for
-                                <varname>ExecStartPre=</varname>
+                                following the same scheme as described
+                                for <varname>ExecStart=</varname>
                                 above. Use of this setting is
                                 optional. All processes remaining for
                                 a service after the commands
@@ -456,9 +457,8 @@
                                 configured in
                                 <varname>ExecStop=</varname>. This
                                 argument takes multiple command lines,
-                                following the same scheme as pointed
-                                out for
-                                <varname>ExecStartPre</varname>. Use
+                                following the same scheme as described
+                                for <varname>ExecStart</varname>. Use
                                 of these settings is
                                 optional. Specifier and environment
                                 variable substitution is

commit 0f67f1efae74e6d129338f1b63ede902b3d7e5ae
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Thu Nov 15 15:25:05 2012 +0100

    core: lift restriction on order of - and @ in ExecStart

diff --git a/man/systemd.service.xml b/man/systemd.service.xml
index 00a6398..8c1dfe1 100644
--- a/man/systemd.service.xml
+++ b/man/systemd.service.xml
@@ -326,9 +326,8 @@
                                 abnormal exit due to signal) is ignored
                                 and considered success. If both
                                 <literal>-</literal> and
-                                <literal>@</literal> are used for the
-                                same command the former must precede
-                                the latter. Unless
+                                <literal>@</literal> are used they
+                                can appear in either order. Unless
                                 <varname>Type=forking</varname> is
                                 set, the process started via this
                                 command line will be considered the
diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index 4dc5c52..b99e70e 100644
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -438,6 +438,7 @@ int config_parse_exec(
         e += ltype;
 
         for (;;) {
+                int i;
                 char *w;
                 size_t l;
                 char *state;
@@ -452,18 +453,21 @@ int config_parse_exec(
                 if (rvalue[0] == 0)
                         break;
 
-                if (rvalue[0] == '-') {
-                        ignore = true;
-                        rvalue ++;
-                }
+                for (i = 0; i < 2; i++) {
+                        if (rvalue[0] == '-' && !ignore) {
+                                ignore = true;
+                                rvalue ++;
+                        }
 
-                if (rvalue[0] == '@') {
-                        honour_argv0 = true;
-                        rvalue ++;
+                        if (rvalue[0] == '@' && !honour_argv0) {
+                                honour_argv0 = true;
+                                rvalue ++;
+                        }
                 }
 
                 if (*rvalue != '/') {
-                        log_error("[%s:%u] Invalid executable path in command line, ignoring: %s", filename, line, rvalue);
+                        log_error("[%s:%u] Executable path is not absolute, ignoring: %s",
+                                  filename, line, rvalue);
                         return 0;
                 }
 
diff --git a/src/test/test-unit-file.c b/src/test/test-unit-file.c
index bca8a69..6636b94 100644
--- a/src/test/test-unit-file.c
+++ b/src/test/test-unit-file.c
@@ -111,6 +111,29 @@ static void test_config_parse_exec(void) {
         check_execcommand(c1,
                           "/RValue/slashes3", "argv0a", "r1", true);
 
+        /* ignore && honour_argv0 */
+        r = config_parse_exec("fake", 4, "section",
+                              "LValue", 0, "@-/RValue///slashes4/// argv0b r1",
+                              &c, NULL);
+        assert_se(r >= 0);
+        c1 = c1->command_next;
+        check_execcommand(c1,
+                          "/RValue/slashes4", "argv0b", "r1", true);
+
+        /* ignore && ignore */
+        r = config_parse_exec("fake", 4, "section",
+                              "LValue", 0, "--/RValue argv0 r1",
+                              &c, NULL);
+        assert_se(r == 0);
+        assert_se(c1->command_next == NULL);
+
+        /* ignore && ignore */
+        r = config_parse_exec("fake", 4, "section",
+                              "LValue", 0, "- at -/RValue argv0 r1",
+                              &c, NULL);
+        assert_se(r == 0);
+        assert_se(c1->command_next == NULL);
+
         /* semicolon */
         r = config_parse_exec("fake", 5, "section",
                               "LValue", 0,

commit 7e1a84f55244ca78093b1dabc58683bc0e7f4304
Author: Oleksii Shevchuk <alxchk at gmail.com>
Date:   Sat Nov 3 21:52:02 2012 +0200

    core: interpret \; token in ExecStart as escaped ;
    
    Some commands (like 'find') take a semicolon as separate arg. With
    current parser implementation there is no way to pass one.
    
    Patch adds token \;

diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index 5803044..4dc5c52 100644
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -483,6 +483,8 @@ int config_parse_exec(
                 FOREACH_WORD_QUOTED(w, l, rvalue, state) {
                         if (strncmp(w, ";", MAX(l, 1U)) == 0)
                                 break;
+                        else if (strncmp(w, "\\;", MAX(l, 1U)) == 0)
+                                w ++;
 
                         if (honour_argv0 && w == rvalue) {
                                 assert(!path);
diff --git a/src/test/test-unit-file.c b/src/test/test-unit-file.c
index bb5cbdf..bca8a69 100644
--- a/src/test/test-unit-file.c
+++ b/src/test/test-unit-file.c
@@ -138,6 +138,16 @@ static void test_config_parse_exec(void) {
 
         assert_se(c1->command_next == NULL);
 
+        /* escaped semicolon */
+        r = config_parse_exec("fake", 5, "section",
+                              "LValue", 0,
+                              "/usr/bin/find \\;",
+                              &c, NULL);
+        assert_se(r >= 0);
+        c1 = c1->command_next;
+        check_execcommand(c1,
+                          "/usr/bin/find", "/usr/bin/find", ";", false);
+
         exec_command_free_list(c);
 }
 

commit 2c5417ade0d137f811e4fcc1b099f963e6d5a820
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Thu Nov 15 14:48:12 2012 +0100

    tests: add tests for config_parse_exec

diff --git a/Makefile.am b/Makefile.am
index e3b629f..da3885d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1248,6 +1248,10 @@ test_unit_name_LDADD = \
 test_unit_file_SOURCES = \
 	src/test/test-unit-file.c
 
+test_unit_file_CFLAGS = \
+	$(AM_CFLAGS) \
+	$(DBUS_CFLAGS)
+
 test_unit_file_LDADD = \
 	libsystemd-core.la
 
diff --git a/src/test/test-unit-file.c b/src/test/test-unit-file.c
index 95e2b68..bb5cbdf 100644
--- a/src/test/test-unit-file.c
+++ b/src/test/test-unit-file.c
@@ -28,9 +28,9 @@
 #include "util.h"
 #include "macro.h"
 #include "hashmap.h"
+#include "load-fragment.h"
 
-int main(int argc, char *argv[]) {
-
+static void test_unit_file_get_set(void) {
         int r;
         Hashmap *h;
         Iterator i;
@@ -47,6 +47,104 @@ int main(int argc, char *argv[]) {
                 printf("%s = %s\n", p->path, unit_file_state_to_string(p->state));
 
         unit_file_list_free(h);
+}
+
+static void check_execcommand(ExecCommand *c,
+                              const char* path,
+                              const char* argv0,
+                              const char* argv1,
+                              bool ignore) {
+        assert_se(c);
+        log_info("%s %s %s %s",
+                 c->path, c->argv[0], c->argv[1], c->argv[2]);
+        assert_se(streq(c->path, path));
+        assert_se(streq(c->argv[0], argv0));
+        assert_se(streq(c->argv[1], argv1));
+        assert_se(c->argv[2] == NULL);
+        assert_se(c->ignore == ignore);
+}
+
+static void test_config_parse_exec(void) {
+        /* int config_parse_exec( */
+        /*         const char *filename, */
+        /*         unsigned line, */
+        /*         const char *section, */
+        /*         const char *lvalue, */
+        /*         int ltype, */
+        /*         const char *rvalue, */
+        /*         void *data, */
+        /*         void *userdata) */
+        int r;
+
+        ExecCommand *c = NULL, *c1;
+
+        /* basic test */
+        r = config_parse_exec("fake", 1, "section",
+                              "LValue", 0, "/RValue r1",
+                              &c, NULL);
+        assert_se(r >= 0);
+        check_execcommand(c, "/RValue", "/RValue", "r1", false);
+
+        r = config_parse_exec("fake", 2, "section",
+                              "LValue", 0, "/RValue///slashes/// r1",
+                              &c, NULL);
+       /* test slashes */
+        assert_se(r >= 0);
+        c1 = c->command_next;
+        check_execcommand(c1, "/RValue/slashes", "/RValue///slashes///",
+                          "r1", false);
+
+        /* honour_argv0 */
+        r = config_parse_exec("fake", 3, "section",
+                              "LValue", 0, "@/RValue///slashes2/// argv0 r1",
+                              &c, NULL);
+        assert_se(r >= 0);
+        c1 = c1->command_next;
+        check_execcommand(c1, "/RValue/slashes2", "argv0", "r1", false);
+
+        /* ignore && honour_argv0 */
+        r = config_parse_exec("fake", 4, "section",
+                              "LValue", 0, "-@/RValue///slashes3/// argv0a r1",
+                              &c, NULL);
+        assert_se(r >= 0);
+        c1 = c1->command_next;
+        check_execcommand(c1,
+                          "/RValue/slashes3", "argv0a", "r1", true);
+
+        /* semicolon */
+        r = config_parse_exec("fake", 5, "section",
+                              "LValue", 0,
+                              "-@/RValue argv0 r1 ; "
+                              "/goo/goo boo",
+                              &c, NULL);
+        assert_se(r >= 0);
+        c1 = c1->command_next;
+        check_execcommand(c1,
+                          "/RValue", "argv0", "r1", true);
+
+        c1 = c1->command_next;
+        check_execcommand(c1,
+                          "/goo/goo", "/goo/goo", "boo", false);
+
+        /* trailing semicolon */
+        r = config_parse_exec("fake", 5, "section",
+                              "LValue", 0,
+                              "-@/RValue argv0 r1 ; ",
+                              &c, NULL);
+        assert_se(r >= 0);
+        c1 = c1->command_next;
+        check_execcommand(c1,
+                          "/RValue", "argv0", "r1", true);
+
+        assert_se(c1->command_next == NULL);
+
+        exec_command_free_list(c);
+}
+
+int main(int argc, char *argv[]) {
+
+        test_unit_file_get_set();
+        test_config_parse_exec();
 
         return 0;
 }

commit a66f3bea8b9978fa9e232f213dd6d762254c0f0a
Author: Oleksii Shevchuk <alxchk at gmail.com>
Date:   Sat Nov 3 21:52:00 2012 +0200

    core/load-fragment-gperf: add missing CONDITION_FILE_NOT_EMPTY
    
    Unit files couldn't be properly parsed, because of
    absent ConditionFileNotEmpty in load-fragment table.

diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4
index 0c5cceb..be16ba9 100644
--- a/src/core/load-fragment-gperf.gperf.m4
+++ b/src/core/load-fragment-gperf.gperf.m4
@@ -130,6 +130,7 @@ Unit.ConditionPathIsSymbolicLink,config_parse_unit_condition_path,   CONDITION_P
 Unit.ConditionPathIsMountPoint,  config_parse_unit_condition_path,   CONDITION_PATH_IS_MOUNT_POINT, 0
 Unit.ConditionPathIsReadWrite,   config_parse_unit_condition_path,   CONDITION_PATH_IS_READ_WRITE,  0
 Unit.ConditionDirectoryNotEmpty, config_parse_unit_condition_path,   CONDITION_DIRECTORY_NOT_EMPTY, 0
+Unit.ConditionFileNotEmpty,      config_parse_unit_condition_path,   CONDITION_FILE_NOT_EMPTY,      0
 Unit.ConditionFileIsExecutable,  config_parse_unit_condition_path,   CONDITION_FILE_IS_EXECUTABLE,  0
 Unit.ConditionKernelCommandLine, config_parse_unit_condition_string, CONDITION_KERNEL_COMMAND_LINE, 0
 Unit.ConditionVirtualization,    config_parse_unit_condition_string, CONDITION_VIRTUALIZATION,      0

commit 774de5a97fe69da822fde77b88af8d970ab5d0c6
Author: Oleksii Shevchuk <alxchk at gmail.com>
Date:   Sat Nov 3 21:51:59 2012 +0200

    core: fix %h, %s, %p handling in templates in user session

diff --git a/src/core/unit-printf.c b/src/core/unit-printf.c
index cbae45d..a8eb60a 100644
--- a/src/core/unit-printf.c
+++ b/src/core/unit-printf.c
@@ -127,11 +127,9 @@ static char *specifier_user_name(char specifier, void *data, void *userdata) {
         assert(u);
 
         c = unit_get_exec_context(u);
-        if (!c)
-                return NULL;
 
         /* get USER env from our own env if set */
-        if (!c->user)
+        if (!c || !c->user)
                 return getusername_malloc();
 
         /* fish username from passwd */
@@ -152,11 +150,9 @@ static char *specifier_user_home(char specifier, void *data, void *userdata) {
         assert(u);
 
         c = unit_get_exec_context(u);
-        if (!c)
-                return NULL;
 
         /* return HOME if set, otherwise from passwd */
-        if (!c->user) {
+        if (!c || !c->user) {
                 char *h;
 
                 r = get_home_dir(&h);
@@ -183,11 +179,9 @@ static char *specifier_user_shell(char specifier, void *data, void *userdata) {
         assert(u);
 
         c = unit_get_exec_context(u);
-        if (!c)
-                return NULL;
 
         /* return HOME if set, otherwise from passwd */
-        if (!c->user) {
+        if (!c || !c->user) {
                 char *sh;
 
                 r = get_shell(&sh);

commit f09a7d25545b5e3a2dd3dfc1ff7ebc8560a3354c
Author: Olivier Brunel <i.am.jack.mail at gmail.com>
Date:   Mon Nov 5 00:28:45 2012 +0100

    systemd: highlight ordering cycle deletions
    
    Having unit(s) removed/not started, even if it solved the issue and allowed
    to boot successfully, should still be considered an error, as something
    clearly isn't right.
    
    This patch elevates the log message from warning to error, and adds a status
    message to make things more obvious.

diff --git a/src/core/transaction.c b/src/core/transaction.c
index 4bce942..ee6992a 100644
--- a/src/core/transaction.c
+++ b/src/core/transaction.c
@@ -374,7 +374,8 @@ static int transaction_verify_order_one(Transaction *tr, Job *j, Job *from, unsi
 
 
                 if (delete) {
-                        log_warning("Breaking ordering cycle by deleting job %s/%s", delete->unit->id, job_type_to_string(delete->type));
+                        log_error("Breaking ordering cycle by deleting job %s/%s", delete->unit->id, job_type_to_string(delete->type));
+                        status_printf(ANSI_HIGHLIGHT_RED_ON " SKIP " ANSI_HIGHLIGHT_OFF, true, "Ordering cycle found, skip %s", unit_description(delete->unit));
                         transaction_delete_unit(tr, delete->unit);
                         return -EAGAIN;
                 }

commit 45c0c61df3c63cb1f20505c8d292385d5e300578
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Thu Nov 15 11:54:57 2012 +0100

    systemctl: add help for --type/-t
    
    The list of types and load states if lengthy, so a little reminder
    can be sometimes useful.

diff --git a/man/systemctl.xml b/man/systemctl.xml
index 31f3b1a..62446d7 100644
--- a/man/systemctl.xml
+++ b/man/systemctl.xml
@@ -106,6 +106,11 @@
                                 certain unit types. If not specified
                                 units of in all load states will be
                                 shown.</para>
+
+                                <para>As a special case, if the argument
+                                is <option>help</option>, a list of
+                                allowed values will be printed and the
+                                program will exit.</para>
                                 </listitem>
                         </varlistentry>
 
diff --git a/src/shared/unit-name.c b/src/shared/unit-name.c
index 50031e6..88ca0b8 100644
--- a/src/shared/unit-name.c
+++ b/src/shared/unit-name.c
@@ -33,7 +33,7 @@
         "ABCDEFGHIJKLMNOPQRSTUVWXYZ"            \
         ":-_.\\"
 
-static const char* const unit_type_table[_UNIT_TYPE_MAX] = {
+const char* const unit_type_table[_UNIT_TYPE_MAX] = {
         [UNIT_SERVICE] = "service",
         [UNIT_SOCKET] = "socket",
         [UNIT_TARGET] = "target",
@@ -48,7 +48,7 @@ static const char* const unit_type_table[_UNIT_TYPE_MAX] = {
 
 DEFINE_STRING_TABLE_LOOKUP(unit_type, UnitType);
 
-static const char* const unit_load_state_table[_UNIT_LOAD_STATE_MAX] = {
+const char* const unit_load_state_table[_UNIT_LOAD_STATE_MAX] = {
         [UNIT_STUB] = "stub",
         [UNIT_LOADED] = "loaded",
         [UNIT_ERROR] = "error",
diff --git a/src/shared/unit-name.h b/src/shared/unit-name.h
index 7be3465..d7528a3 100644
--- a/src/shared/unit-name.h
+++ b/src/shared/unit-name.h
@@ -44,7 +44,7 @@ enum UnitType {
 };
 
 enum UnitLoadState {
-        UNIT_STUB,
+        UNIT_STUB = 0,
         UNIT_LOADED,
         UNIT_ERROR,
         UNIT_MERGED,
@@ -53,9 +53,11 @@ enum UnitLoadState {
         _UNIT_LOAD_STATE_INVALID = -1
 };
 
+extern const char* const unit_type_table[];
 const char *unit_type_to_string(UnitType i);
 UnitType unit_type_from_string(const char *s);
 
+extern const char* const unit_load_state_table[];
 const char *unit_load_state_to_string(UnitLoadState i);
 UnitLoadState unit_load_state_from_string(const char *s);
 
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 39f4e6c..f6eed1c 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -4058,6 +4058,22 @@ static int runlevel_help(void) {
         return 0;
 }
 
+static int help_types(void) {
+        int i;
+
+        puts("Available unit types:");
+        for(i = UNIT_SERVICE; i < _UNIT_TYPE_MAX; i++)
+                if (unit_type_table[i])
+                        puts(unit_type_table[i]);
+
+        puts("\nAvailable unit load states: ");
+        for(i = UNIT_STUB; i < _UNIT_LOAD_STATE_MAX; i++)
+                if (unit_type_table[i])
+                        puts(unit_load_state_table[i]);
+
+        return 0;
+}
+
 static int systemctl_parse_argv(int argc, char *argv[]) {
 
         enum {
@@ -4137,6 +4153,11 @@ static int systemctl_parse_argv(int argc, char *argv[]) {
                         return 0;
 
                 case 't':
+                        if (streq(optarg, "help")) {
+                                help_types();
+                                return 0;
+                        }
+
                         if (unit_type_from_string(optarg) >= 0) {
                                 arg_type = optarg;
                                 break;
@@ -4147,6 +4168,7 @@ static int systemctl_parse_argv(int argc, char *argv[]) {
                         }
                         log_error("Unkown unit type or load state '%s'.",
                                   optarg);
+                        log_info("Use -t help to see a list of allowed values.");
                         return -EINVAL;
                 case 'p': {
                         char **l;

commit 48c2826b4ea686cd69e489f9c6fe2d9ddebb6a69
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Thu Nov 15 11:39:23 2012 +0100

    systemctl: remove empty line in case of no units

diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index b82c794..39f4e6c 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -426,6 +426,7 @@ static void output_units_list(const struct unit_info *unit_infos, unsigned c) {
                                "SUB    = The low-level unit activation state, values depend on unit type.\n");
                         if (job_count)
                                 printf("JOB    = Pending job for the unit.\n");
+                        puts("");
                         on = ansi_highlight(true);
                         off = ansi_highlight(false);
                 } else {
@@ -434,11 +435,11 @@ static void output_units_list(const struct unit_info *unit_infos, unsigned c) {
                 }
 
                 if (arg_all)
-                        printf("\n%s%u loaded units listed.%s\n"
+                        printf("%s%u loaded units listed.%s\n"
                                "To show all installed unit files use 'systemctl list-unit-files'.\n",
                                on, n_shown, off);
                 else
-                        printf("\n%s%u loaded units listed.%s Pass --all to see loaded but inactive units, too.\n"
+                        printf("%s%u loaded units listed.%s Pass --all to see loaded but inactive units, too.\n"
                                "To show all installed unit files use 'systemctl list-unit-files'.\n",
                                on, n_shown, off);
         }



More information about the systemd-commits mailing list