manager: "downgrade" message about command vanishing from the unit file

We would print "Current command vanished from the unit file, execution of
the command list won't be resumed." as a warning, but most of the time there
is nothing to resume, because a unit has just one command. So let's detect
the case where the command that was active is the last command in the sequence
and skip the warning.

I was considering how to store the information that the command is last. An
important consideration is not to use a format that would confuse older versions
of systemd. (It wouldn't be a big problem if older systemd just refused the
new serialization, since we require systemd to be newer, but we should avoid
the case where the deserialization is "successful", but actually incorrect.)
Similarly, the deserialization from the old systemd must not confuse new systemd.
For this command, we have a list of arguments at the end, so just adding a
new field either in the middle or at the end is problematic because it's hard
to ensure that we don't mix up the positional and variable arguments.

We actually need to store just one bit of information, so '+' is prefixed on
the index of the last command and used by new systemd to skip the warning.
When deserializing from older systemd, '+' is not present, so we detect all
commands as "not last", and still emit the warning, so we err on the side of
caution. If the user were to deserialize from newer to older systemd, nothing
untoward would happen, because the '+' is ignored. (Users shouldn't do this,
but we know that this occasionally happens with initrds or exitrds and package
downgrades.)

diff --git a/src/core/service.c b/src/core/service.c
index 7513972fb37bcea24bf8454e7f18486446c8dd6a..9ad3c3d99536f54d0bfc5fc2e6dfb4ff7d89e9ac 100644 (file)
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -2763,7 +2763,16 @@ static int service_serialize_exec_command(Unit *u, FILE *f, const ExecCommand *c
                 return log_oom();
 
         key = strjoina(type, "-command");
-        (void) serialize_item_format(f, key, "%s %u %s %s", service_exec_command_to_string(id), idx, p, args);
+
+        /* We use '+1234' instead of '1234' to mark the last command in a sequence.
+         * This is used in service_deserialize_exec_command(). */
+        (void) serialize_item_format(
+                        f, key,
+                        "%s %s%u %s %s",
+                        service_exec_command_to_string(id),
+                        command->command_next ? "" : "+",
+                        idx,
+                        p, args);
 
         return 0;
 }
@@ -2877,7 +2886,7 @@ int service_deserialize_exec_command(
         Service *s = SERVICE(u);
         int r;
         unsigned idx = 0, i;
-        bool control, found = false;
+        bool control, found = false, last = false;
         ServiceExecCommand id = _SERVICE_EXEC_COMMAND_INVALID;
         ExecCommand *command = NULL;
         _cleanup_free_ char *path = NULL;
@@ -2918,9 +2927,15 @@ int service_deserialize_exec_command(
                         state = STATE_EXEC_COMMAND_INDEX;
                         break;
                 case STATE_EXEC_COMMAND_INDEX:
+                        /* PID 1234 is serialized as either '1234' or '+1234'. The second form is used to
+                         * mark the last command in a sequence. We warn if the deserialized command doesn't
+                         * match what we have loaded from the unit, but we don't need to warn if that is the
+                         * last command. */
+
                         r = safe_atou(arg, &idx);
                         if (r < 0)
                                 return r;
+                        last = arg[0] == '+';
 
                         state = STATE_EXEC_COMMAND_PATH;
                         break;
@@ -2965,6 +2980,8 @@ int service_deserialize_exec_command(
                 s->control_command_id = id;
         } else if (command)
                 s->main_command = command;
+        else if (last)
+                log_unit_debug(u, "Current command vanished from the unit file.");
         else
                 log_unit_warning(u, "Current command vanished from the unit file, execution of the command list won't be resumed.");