dd65c9
From 5aafbcced90ae2a3b418d6fe26c67e820daa8bad Mon Sep 17 00:00:00 2001
dd65c9
From: Michal Sekletar <msekleta@redhat.com>
dd65c9
Date: Mon, 23 Jan 2017 17:12:35 +0100
dd65c9
Subject: [PATCH] service: serialize information about currently executing
dd65c9
 command
dd65c9
dd65c9
Stored information will help us to resume execution after the
dd65c9
daemon-reload.
dd65c9
dd65c9
This commit implements following scheme,
dd65c9
dd65c9
* On serialization:
dd65c9
  - we count rank of the currently executing command
dd65c9
  - we store command type, its rank and command line arguments
dd65c9
dd65c9
* On deserialization:
dd65c9
  - configuration is parsed and loaded
dd65c9
  - we deserialize stored data, command type, rank and arguments
dd65c9
  - we look at the given rank in the list and if command there has same
dd65c9
    arguments then we restore execution at that point
dd65c9
  - otherwise we search respective command list and we look for command
dd65c9
    that has the same arguments
dd65c9
  - if both methods fail we do not do not resume execution at all
dd65c9
dd65c9
To better illustrate how does above scheme works, please consider
dd65c9
following cases (<<< denotes position where we resume execution after reload)
dd65c9
dd65c9
; Original unit file
dd65c9
[Service]
dd65c9
ExecStart=/bin/true <<<
dd65c9
ExecStart=/bin/false
dd65c9
dd65c9
; Swapped commands
dd65c9
; Second command is not going to be executed
dd65c9
[Service]
dd65c9
ExecStart=/bin/false
dd65c9
ExecStart=/bin/true <<<
dd65c9
dd65c9
; Commands added before
dd65c9
; Same commands are problematic and execution could be restarted at wrong place
dd65c9
[Service]
dd65c9
ExecStart=/bin/foo
dd65c9
ExecStart=/bin/bar
dd65c9
ExecStart=/bin/true <<<
dd65c9
ExecStart=/bin/false
dd65c9
dd65c9
; Commands added after
dd65c9
; Same commands are not an issue in this case
dd65c9
[Service]
dd65c9
ExecStart=/bin/true <<<
dd65c9
ExecStart=/bin/false
dd65c9
ExecStart=/bin/foo
dd65c9
ExecStart=/bin/bar
dd65c9
dd65c9
; New commands interleaved with old commands
dd65c9
; Some new commands will be executed while others won't
dd65c9
ExecStart=/bin/foo
dd65c9
ExecStart=/bin/true <<<
dd65c9
ExecStart=/bin/bar
dd65c9
ExecStart=/bin/false
dd65c9
dd65c9
As you can see, above scheme has some drawbacks. However, in most
dd65c9
cases (we assume that in most common case unit file command list is not
dd65c9
changed while some other command is running for the same unit) it
dd65c9
should cause that systemd does the right thing, which is restoring
dd65c9
execution exactly at the point we were before daemon-reload.
dd65c9
dd65c9
Fixes #518
dd65c9
dd65c9
(cherry picked from commit e266c068b5597e18b2299f9c9d3ee6cf04198c41)
dd65c9
dd65c9
Resolves: #1404657,#1471230
dd65c9
---
23b3cf
 src/core/service.c | 195 +++++++++++++++++++++++++++++++++++++++++----
dd65c9
 1 file changed, 180 insertions(+), 15 deletions(-)
dd65c9
dd65c9
diff --git a/src/core/service.c b/src/core/service.c
dd65c9
index 3bd6c3338..9ad3a0eb0 100644
dd65c9
--- a/src/core/service.c
dd65c9
+++ b/src/core/service.c
dd65c9
@@ -1950,6 +1950,80 @@ _pure_ static bool service_can_reload(Unit *u) {
dd65c9
         return !!s->exec_command[SERVICE_EXEC_RELOAD];
dd65c9
 }
dd65c9
 
dd65c9
+static unsigned service_exec_command_index(Unit *u, ServiceExecCommand id, ExecCommand *current) {
dd65c9
+        Service *s = SERVICE(u);
dd65c9
+        unsigned idx = 0;
dd65c9
+        ExecCommand *first, *c;
dd65c9
+
dd65c9
+        assert(s);
dd65c9
+
dd65c9
+        first = s->exec_command[id];
dd65c9
+
dd65c9
+        /* Figure out where we are in the list by walking back to the beginning */
dd65c9
+        for (c = current; c != first; c = c->command_prev)
dd65c9
+                idx++;
dd65c9
+
dd65c9
+        return idx;
dd65c9
+}
dd65c9
+
dd65c9
+static int service_serialize_exec_command(Unit *u, FILE *f, ExecCommand *command) {
dd65c9
+        Service *s = SERVICE(u);
dd65c9
+        ServiceExecCommand id;
dd65c9
+        unsigned idx;
dd65c9
+        const char *type;
dd65c9
+        char **arg;
dd65c9
+        _cleanup_strv_free_ char **escaped_args = NULL;
dd65c9
+        _cleanup_free_ char *args = NULL, *p = NULL;
dd65c9
+        size_t allocated = 0, length = 0;
dd65c9
+
dd65c9
+        assert(s);
dd65c9
+        assert(f);
dd65c9
+
dd65c9
+        if (!command)
dd65c9
+                return 0;
dd65c9
+
dd65c9
+        if (command == s->control_command) {
dd65c9
+                type = "control";
dd65c9
+                id = s->control_command_id;
dd65c9
+        } else {
dd65c9
+                type = "main";
dd65c9
+                id = SERVICE_EXEC_START;
dd65c9
+        }
dd65c9
+
dd65c9
+        idx = service_exec_command_index(u, id, command);
dd65c9
+
dd65c9
+        STRV_FOREACH(arg, command->argv) {
dd65c9
+                size_t n;
dd65c9
+                _cleanup_free_ char *e = NULL;
dd65c9
+
dd65c9
+                e = xescape(*arg, WHITESPACE);
dd65c9
+                if (!e)
dd65c9
+                        return -ENOMEM;
dd65c9
+
dd65c9
+                n = strlen(e);
dd65c9
+                if (!GREEDY_REALLOC(args, allocated, length + 1 + n + 1))
dd65c9
+                        return -ENOMEM;
dd65c9
+
dd65c9
+                if (length > 0)
dd65c9
+                        args[length++] = ' ';
dd65c9
+
dd65c9
+                memcpy(args + length, e, n);
dd65c9
+                length += n;
dd65c9
+        }
dd65c9
+
dd65c9
+        if (!GREEDY_REALLOC(args, allocated, length + 1))
dd65c9
+                return -ENOMEM;
dd65c9
+        args[length++] = 0;
dd65c9
+
dd65c9
+        p = xescape(command->path, WHITESPACE);
dd65c9
+        if (!p)
dd65c9
+                return -ENOMEM;
dd65c9
+
dd65c9
+        fprintf(f, "%s-command=%s %u %s %s\n", type, service_exec_command_to_string(id), idx, p, args);
dd65c9
+
dd65c9
+        return 0;
dd65c9
+}
dd65c9
+
dd65c9
 static int service_serialize(Unit *u, FILE *f, FDSet *fds) {
dd65c9
         Service *s = SERVICE(u);
dd65c9
         ServiceFDStore *fs;
dd65c9
@@ -1974,12 +2048,8 @@ static int service_serialize(Unit *u, FILE *f, FDSet *fds) {
dd65c9
         if (s->status_text)
dd65c9
                 unit_serialize_item(u, f, "status-text", s->status_text);
dd65c9
 
dd65c9
-        /* FIXME: There's a minor uncleanliness here: if there are
dd65c9
-         * multiple commands attached here, we will start from the
dd65c9
-         * first one again */
dd65c9
-        if (s->control_command_id >= 0)
dd65c9
-                unit_serialize_item(u, f, "control-command",
dd65c9
-                                    service_exec_command_to_string(s->control_command_id));
dd65c9
+        service_serialize_exec_command(u, f, s->control_command);
dd65c9
+        service_serialize_exec_command(u, f, s->main_command);
dd65c9
 
dd65c9
         if (s->socket_fd >= 0) {
dd65c9
                 int copy;
dd65c9
@@ -2035,6 +2105,106 @@ static int service_serialize(Unit *u, FILE *f, FDSet *fds) {
dd65c9
         return 0;
dd65c9
 }
dd65c9
 
dd65c9
+static int service_deserialize_exec_command(Unit *u, const char *key, const char *value) {
dd65c9
+        Service *s = SERVICE(u);
dd65c9
+        int r;
dd65c9
+        unsigned idx = 0, i;
dd65c9
+        bool control, found = false;
dd65c9
+        ServiceExecCommand id = _SERVICE_EXEC_COMMAND_INVALID;
dd65c9
+        ExecCommand *command = NULL;
dd65c9
+        _cleanup_free_ char *args = NULL, *path = NULL;
dd65c9
+        _cleanup_strv_free_ char **argv = NULL;
dd65c9
+
dd65c9
+        enum ExecCommandState {
dd65c9
+                STATE_EXEC_COMMAND_TYPE,
dd65c9
+                STATE_EXEC_COMMAND_INDEX,
dd65c9
+                STATE_EXEC_COMMAND_PATH,
dd65c9
+                STATE_EXEC_COMMAND_ARGS,
dd65c9
+                _STATE_EXEC_COMMAND_MAX,
dd65c9
+                _STATE_EXEC_COMMAND_INVALID = -1,
dd65c9
+        } state;
dd65c9
+
dd65c9
+        assert(s);
dd65c9
+        assert(key);
dd65c9
+        assert(value);
dd65c9
+
dd65c9
+        control = streq(key, "control-command");
dd65c9
+
dd65c9
+        state = STATE_EXEC_COMMAND_TYPE;
dd65c9
+
dd65c9
+        for (;;) {
dd65c9
+                _cleanup_free_ char *arg = NULL;
dd65c9
+
dd65c9
+                r = extract_first_word(&value, &arg, NULL, EXTRACT_CUNESCAPE);
dd65c9
+                if (r == 0)
dd65c9
+                        break;
dd65c9
+                else if (r < 0)
dd65c9
+                        return r;
dd65c9
+
dd65c9
+                switch (state) {
dd65c9
+                case STATE_EXEC_COMMAND_TYPE:
dd65c9
+                        id = service_exec_command_from_string(arg);
dd65c9
+                        if (id < 0)
dd65c9
+                                return -EINVAL;
dd65c9
+
dd65c9
+                        state = STATE_EXEC_COMMAND_INDEX;
dd65c9
+                        break;
dd65c9
+                case STATE_EXEC_COMMAND_INDEX:
dd65c9
+                        r = safe_atou(arg, &idx);
dd65c9
+                        if (r < 0)
dd65c9
+                                return -EINVAL;
dd65c9
+
dd65c9
+                        state = STATE_EXEC_COMMAND_PATH;
dd65c9
+                        break;
dd65c9
+                case STATE_EXEC_COMMAND_PATH:
dd65c9
+                        path = arg;
dd65c9
+                        arg = NULL;
dd65c9
+                        state = STATE_EXEC_COMMAND_ARGS;
dd65c9
+
dd65c9
+                        if (!path_is_absolute(path))
dd65c9
+                                return -EINVAL;
dd65c9
+                        break;
dd65c9
+                case STATE_EXEC_COMMAND_ARGS:
dd65c9
+                        r = strv_extend(&argv, arg);
dd65c9
+                        if (r < 0)
dd65c9
+                                return -ENOMEM;
dd65c9
+                        break;
dd65c9
+                default:
dd65c9
+                        assert_not_reached("Unknown error at deserialization of exec command");
dd65c9
+                        break;
dd65c9
+                }
dd65c9
+        }
dd65c9
+
dd65c9
+        if (state != STATE_EXEC_COMMAND_ARGS)
dd65c9
+                return -EINVAL;
dd65c9
+
dd65c9
+        /* Let's check whether exec command on given offset matches data that we just deserialized */
dd65c9
+        for (command = s->exec_command[id], i = 0; command; command = command->command_next, i++) {
dd65c9
+                if (i != idx)
dd65c9
+                        continue;
dd65c9
+
dd65c9
+                found = strv_equal(argv, command->argv) && streq(command->path, path);
dd65c9
+                break;
dd65c9
+        }
dd65c9
+
dd65c9
+        if (!found) {
dd65c9
+                /* Command at the index we serialized is different, let's look for command that exactly
dd65c9
+                 * matches but is on different index. If there is no such command we will not resume execution. */
dd65c9
+                for (command = s->exec_command[id]; command; command = command->command_next)
dd65c9
+                        if (strv_equal(command->argv, argv) && streq(command->path, path))
dd65c9
+                                break;
dd65c9
+        }
dd65c9
+
dd65c9
+        if (command && control)
dd65c9
+                s->control_command = command;
dd65c9
+        else if (command)
dd65c9
+                s->main_command = command;
dd65c9
+        else
dd65c9
+                log_unit_warning(u->id, "Current command vanished from the unit file, execution of the command list won't be resumed.");
dd65c9
+
dd65c9
+        return 0;
dd65c9
+}
dd65c9
+
dd65c9
 static int service_deserialize_item(Unit *u, const char *key, const char *value, FDSet *fds) {
dd65c9
         Service *s = SERVICE(u);
dd65c9
         int r;
dd65c9
@@ -2105,16 +2275,11 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value,
dd65c9
                         s->status_text = t;
dd65c9
                 }
dd65c9
 
dd65c9
-        } else if (streq(key, "control-command")) {
dd65c9
-                ServiceExecCommand id;
dd65c9
+        } else if (STR_IN_SET(key, "main-command", "control-command")) {
dd65c9
+                r = service_deserialize_exec_command(u, key, value);
dd65c9
+                if (r < 0)
dd65c9
+                        log_unit_debug_errno(u->id, r, "Failed to parse serialized command \"%s\": %m", value);
dd65c9
 
dd65c9
-                id = service_exec_command_from_string(value);
dd65c9
-                if (id < 0)
dd65c9
-                        log_unit_debug(u->id, "Failed to parse exec-command value %s", value);
dd65c9
-                else {
dd65c9
-                        s->control_command_id = id;
dd65c9
-                        s->control_command = s->exec_command[id];
dd65c9
-                }
dd65c9
         } else if (streq(key, "socket-fd")) {
dd65c9
                 int fd;
dd65c9