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 +# +# 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 . + +# 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()