Merge pull request #5354 from msekletar/issue-518

service: serialize information about currently executing command

3 files changed, 375 insertions, 16 deletions

diff --git a/Makefile.am b/Makefile.am
index 4ba230569a..46a842c6c3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -5946,7 +5946,8 @@ EXTRA_DIST += \
 	src/network/systemd-networkd.pkla \
 	units/systemd-networkd.service.m4.in \
 	units/systemd-networkd-wait-online.service.in \
-	test/networkd-test.py
+	test/networkd-test.py \
+	test/test-exec-deserialization.py
 
 # ------------------------------------------------------------------------------
 if ENABLE_LOGIND
diff --git a/src/core/service.c b/src/core/service.c
index 185173a542..a63c6d8bc3 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -45,6 +45,7 @@
 #include "service.h"
 #include "signal-util.h"
 #include "special.h"
+#include "stdio-util.h"
 #include "string-table.h"
 #include "string-util.h"
 #include "strv.h"
@@ -2140,6 +2141,80 @@ _pure_ static bool service_can_reload(Unit *u) {
         return !!s->exec_command[SERVICE_EXEC_RELOAD];
 }
 
+static unsigned service_exec_command_index(Unit *u, ServiceExecCommand id, ExecCommand *current) {
+        Service *s = SERVICE(u);
+        unsigned idx = 0;
+        ExecCommand *first, *c;
+
+        assert(s);
+
+        first = s->exec_command[id];
+
+        /* Figure out where we are in the list by walking back to the beginning */
+        for (c = current; c != first; c = c->command_prev)
+                idx++;
+
+        return idx;
+}
+
+static int service_serialize_exec_command(Unit *u, FILE *f, ExecCommand *command) {
+        Service *s = SERVICE(u);
+        ServiceExecCommand id;
+        unsigned idx;
+        const char *type;
+        char **arg;
+        _cleanup_strv_free_ char **escaped_args = NULL;
+        _cleanup_free_ char *args = NULL, *p = NULL;
+        size_t allocated = 0, length = 0;
+
+        assert(s);
+        assert(f);
+
+        if (!command)
+                return 0;
+
+        if (command == s->control_command) {
+                type = "control";
+                id = s->control_command_id;
+        } else {
+                type = "main";
+                id = SERVICE_EXEC_START;
+        }
+
+        idx = service_exec_command_index(u, id, command);
+
+        STRV_FOREACH(arg, command->argv) {
+                size_t n;
+                _cleanup_free_ char *e = NULL;
+
+                e = xescape(*arg, WHITESPACE);
+                if (!e)
+                        return -ENOMEM;
+
+                n = strlen(e);
+                if (!GREEDY_REALLOC(args, allocated, length + 1 + n + 1))
+                        return -ENOMEM;
+
+                if (length > 0)
+                        args[length++] = ' ';
+
+                memcpy(args + length, e, n);
+                length += n;
+        }
+
+        if (!GREEDY_REALLOC(args, allocated, length + 1))
+                return -ENOMEM;
+        args[length++] = 0;
+
+        p = xescape(command->path, WHITESPACE);
+        if (!p)
+                return -ENOMEM;
+
+        fprintf(f, "%s-command=%s %u %s %s\n", type, service_exec_command_to_string(id), idx, p, args);
+
+        return 0;
+}
+
 static int service_serialize(Unit *u, FILE *f, FDSet *fds) {
         Service *s = SERVICE(u);
         ServiceFDStore *fs;
@@ -2167,11 +2242,8 @@ static int service_serialize(Unit *u, FILE *f, FDSet *fds) {
         if (r < 0)
                 return r;
 
-        /* FIXME: There's a minor uncleanliness here: if there are
-         * multiple commands attached here, we will start from the
-         * first one again */
-        if (s->control_command_id >= 0)
-                unit_serialize_item(u, f, "control-command", service_exec_command_to_string(s->control_command_id));
+        service_serialize_exec_command(u, f, s->control_command);
+        service_serialize_exec_command(u, f, s->main_command);
 
         r = unit_serialize_item_fd(u, f, fds, "stdin-fd", s->stdin_fd);
         if (r < 0)
@@ -2227,6 +2299,106 @@ static int service_serialize(Unit *u, FILE *f, FDSet *fds) {
         return 0;
 }
 
+static int service_deserialize_exec_command(Unit *u, const char *key, const char *value) {
+        Service *s = SERVICE(u);
+        int r;
+        unsigned idx = 0, i;
+        bool control, found = false;
+        ServiceExecCommand id = _SERVICE_EXEC_COMMAND_INVALID;
+        ExecCommand *command = NULL;
+        _cleanup_free_ char *args = NULL, *path = NULL;
+        _cleanup_strv_free_ char **argv = NULL;
+
+        enum ExecCommandState {
+                STATE_EXEC_COMMAND_TYPE,
+                STATE_EXEC_COMMAND_INDEX,
+                STATE_EXEC_COMMAND_PATH,
+                STATE_EXEC_COMMAND_ARGS,
+                _STATE_EXEC_COMMAND_MAX,
+                _STATE_EXEC_COMMAND_INVALID = -1,
+        } state;
+
+        assert(s);
+        assert(key);
+        assert(value);
+
+        control = streq(key, "control-command");
+
+        state = STATE_EXEC_COMMAND_TYPE;
+
+        for (;;) {
+                _cleanup_free_ char *arg = NULL;
+
+                r = extract_first_word(&value, &arg, NULL, EXTRACT_CUNESCAPE);
+                if (r == 0)
+                        break;
+                else if (r < 0)
+                        return r;
+
+                switch (state) {
+                case STATE_EXEC_COMMAND_TYPE:
+                        id = service_exec_command_from_string(arg);
+                        if (id < 0)
+                                return -EINVAL;
+
+                        state = STATE_EXEC_COMMAND_INDEX;
+                        break;
+                case STATE_EXEC_COMMAND_INDEX:
+                        r = safe_atou(arg, &idx);
+                        if (r < 0)
+                                return -EINVAL;
+
+                        state = STATE_EXEC_COMMAND_PATH;
+                        break;
+                case STATE_EXEC_COMMAND_PATH:
+                        path = arg;
+                        arg = NULL;
+                        state = STATE_EXEC_COMMAND_ARGS;
+
+                        if (!path_is_absolute(path))
+                                return -EINVAL;
+                        break;
+                case STATE_EXEC_COMMAND_ARGS:
+                        r = strv_extend(&argv, arg);
+                        if (r < 0)
+                                return -ENOMEM;
+                        break;
+                default:
+                        assert_not_reached("Unknown error at deserialization of exec command");
+                        break;
+                }
+        }
+
+        if (state != STATE_EXEC_COMMAND_ARGS)
+                return -EINVAL;
+
+        /* Let's check whether exec command on given offset matches data that we just deserialized */
+        for (command = s->exec_command[id], i = 0; command; command = command->command_next, i++) {
+                if (i != idx)
+                        continue;
+
+                found = strv_equal(argv, command->argv) && streq(command->path, path);
+                break;
+        }
+
+        if (!found) {
+                /* Command at the index we serialized is different, let's look for command that exactly
+                 * matches but is on different index. If there is no such command we will not resume execution. */
+                for (command = s->exec_command[id]; command; command = command->command_next)
+                        if (strv_equal(command->argv, argv) && streq(command->path, path))
+                                break;
+        }
+
+        if (command && control)
+                s->control_command = command;
+        else if (command)
+                s->main_command = command;
+        else
+                log_unit_warning(u, "Current command vanished from the unit file, execution of the command list won't be resumed.");
+
+        return 0;
+}
+
 static int service_deserialize_item(Unit *u, const char *key, const char *value, FDSet *fds) {
         Service *s = SERVICE(u);
         int r;
@@ -2309,16 +2481,6 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value,
                         s->status_text = t;
                 }
 
-        } else if (streq(key, "control-command")) {
-                ServiceExecCommand id;
-
-                id = service_exec_command_from_string(value);
-                if (id < 0)
-                        log_unit_debug(u, "Failed to parse exec-command value: %s", value);
-                else {
-                        s->control_command_id = id;
-                        s->control_command = s->exec_command[id];
-                }
         } else if (streq(key, "accept-socket")) {
                 Unit *socket;
 
@@ -2437,6 +2599,10 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value,
                         s->watchdog_override_enable = true;
                         s->watchdog_override_usec = watchdog_override_usec;
                 }
+        } else if (STR_IN_SET(key, "main-command", "control-command")) {
+                r = service_deserialize_exec_command(u, key, value);
+                if (r < 0)
+                        log_unit_debug_errno(u, r, "Failed to parse serialized command \"%s\": %m", value);
         } else
                 log_unit_debug(u, "Unknown serialization key: %s", key);
 
diff --git a/test/test-exec-deserialization.py b/test/test-exec-deserialization.py
new file mode 100755
index 0000000000..b974b1c133
--- /dev/null
+++ b/test/test-exec-deserialization.py
@@ -0,0 +1,192 @@
+#!/usr/bin/python3
+
+#
+#  Copyright 2017 Michal Sekletar <msekleta@redhat.com>
+#
+#  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/>.
+
+# ATTENTION: This uses the *installed* systemd, not the one from the built
+# source tree.
+
+import unittest
+import time
+import os
+import tempfile
+import subprocess
+
+from enum import Enum
+
+class UnitFileChange(Enum):
+    NO_CHANGE = 0
+    LINES_SWAPPED = 1
+    COMMAND_ADDED_BEFORE = 2
+    COMMAND_ADDED_AFTER = 3
+    COMMAND_INTERLEAVED = 4
+    REMOVAL = 5
+
+class ExecutionResumeTest(unittest.TestCase):
+    def setUp(self):
+        self.unit = 'test-issue-518.service'
+        self.unitfile_path = '/run/systemd/system/{0}'.format(self.unit)
+        self.output_file = tempfile.mktemp()
+        self.unit_files = {}
+
+        unit_file_content = '''
+        [Service]
+        Type=oneshot
+        ExecStart=/bin/sleep 2
+        ExecStart=/bin/bash -c "echo foo >> {0}"
+        '''.format(self.output_file)
+        self.unit_files[UnitFileChange.NO_CHANGE] = unit_file_content
+
+        unit_file_content = '''
+        [Service]
+        Type=oneshot
+        ExecStart=/bin/bash -c "echo foo >> {0}"
+        ExecStart=/bin/sleep 2
+        '''.format(self.output_file)
+        self.unit_files[UnitFileChange.LINES_SWAPPED] = unit_file_content
+
+        unit_file_content = '''
+        [Service]
+        Type=oneshot
+        ExecStart=/bin/bash -c "echo bar >> {0}"
+        ExecStart=/bin/sleep 2
+        ExecStart=/bin/bash -c "echo foo >> {0}"
+        '''.format(self.output_file)
+        self.unit_files[UnitFileChange.COMMAND_ADDED_BEFORE] = unit_file_content
+
+        unit_file_content = '''
+        [Service]
+        Type=oneshot
+        ExecStart=/bin/sleep 2
+        ExecStart=/bin/bash -c "echo foo >> {0}"
+        ExecStart=/bin/bash -c "echo bar >> {0}"
+        '''.format(self.output_file)
+        self.unit_files[UnitFileChange.COMMAND_ADDED_AFTER] = unit_file_content
+
+        unit_file_content = '''
+        [Service]
+        Type=oneshot
+        ExecStart=/bin/bash -c "echo baz >> {0}"
+        ExecStart=/bin/sleep 2
+        ExecStart=/bin/bash -c "echo foo >> {0}"
+        ExecStart=/bin/bash -c "echo bar >> {0}"
+        '''.format(self.output_file)
+        self.unit_files[UnitFileChange.COMMAND_INTERLEAVED] = unit_file_content
+
+        unit_file_content = '''
+        [Service]
+        Type=oneshot
+        ExecStart=/bin/bash -c "echo bar >> {0}"
+        ExecStart=/bin/bash -c "echo baz >> {0}"
+        '''.format(self.output_file)
+        self.unit_files[UnitFileChange.REMOVAL] = unit_file_content
+
+    def reload(self):
+        subprocess.check_call(['systemctl', 'daemon-reload'])
+
+    def write_unit_file(self, unit_file_change):
+        if not isinstance(unit_file_change, UnitFileChange):
+            raise ValueError('Unknown unit file change')
+
+        content = self.unit_files[unit_file_change]
+
+        with open(self.unitfile_path, 'w') as f:
+            f.write(content)
+
+        self.reload()
+
+    def check_output(self, expected_output):
+        try:
+            with open(self.output_file, 'r') as log:
+                output = log.read()
+        except IOError:
+            self.fail()
+
+        self.assertEqual(output, expected_output)
+
+    def setup_unit(self):
+        self.write_unit_file(UnitFileChange.NO_CHANGE)
+        subprocess.check_call(['systemctl', '--job-mode=replace', '--no-block', 'start', self.unit])
+
+    def test_no_change(self):
+        expected_output = 'foo\n'
+
+        self.setup_unit()
+        self.reload()
+        time.sleep(4)
+
+        self.check_output(expected_output)
+
+    def test_swapped(self):
+        expected_output = ''
+
+        self.setup_unit()
+        self.write_unit_file(UnitFileChange.LINES_SWAPPED)
+        self.reload()
+        time.sleep(4)
+
+        self.assertTrue(not os.path.exists(self.output_file))
+
+    def test_added_before(self):
+        expected_output = 'foo\n'
+
+        self.setup_unit()
+        self.write_unit_file(UnitFileChange.COMMAND_ADDED_BEFORE)
+        self.reload()
+        time.sleep(4)
+
+        self.check_output(expected_output)
+
+    def test_added_after(self):
+        expected_output = 'foo\nbar\n'
+
+        self.setup_unit()
+        self.write_unit_file(UnitFileChange.COMMAND_ADDED_AFTER)
+        self.reload()
+        time.sleep(4)
+
+        self.check_output(expected_output)
+
+    def test_interleaved(self):
+        expected_output = 'foo\nbar\n'
+
+        self.setup_unit()
+        self.write_unit_file(UnitFileChange.COMMAND_INTERLEAVED)
+        self.reload()
+        time.sleep(4)
+
+        self.check_output(expected_output)
+
+    def test_removal(self):
+        self.setup_unit()
+        self.write_unit_file(UnitFileChange.REMOVAL)
+        self.reload()
+        time.sleep(4)
+
+        self.assertTrue(not os.path.exists(self.output_file))
+
+    def tearDown(self):
+        for f in [self.output_file, self.unitfile_path]:
+            try:
+                os.remove(f)
+            except OSError:
+                # ignore error if log file doesn't exist
+                pass
+
+        self.reload()
+
+if __name__ == '__main__':
+    unittest.main()