Merge pull request #26350 from keszybz/reload-messages

Improve messages emitted when Reload or Reexec is requested

4 files changed, 69 insertions, 13 deletions

diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index 53121fa1a6..c4f205bc42 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -1527,8 +1527,10 @@ static void log_caller(sd_bus_message *message, Manager *manager, const char *me
         (void) sd_bus_creds_get_comm(creds, &comm);
         caller = manager_get_unit_by_pid(manager, pid);
 
-        log_info("%s requested from client PID " PID_FMT " ('%s') (from unit '%s')...",
-                 method, pid, strna(comm), strna(caller ? caller->id : NULL));
+        log_info("%s requested from client PID " PID_FMT "%s%s%s%s%s%s...",
+                 method, pid,
+                 comm ? " ('" : "", strempty(comm), comm ? "')" : "",
+                 caller ? " (unit " : "", caller ? caller->id : NULL, caller ? ")" : "");
 }
 
 static int method_reload(sd_bus_message *message, void *userdata, sd_bus_error *error) {
diff --git a/src/core/service.c b/src/core/service.c
index 9c2fb2ee1c..9ad3c3d995 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -2691,25 +2691,24 @@ _pure_ static bool service_can_reload(Unit *u) {
                 s->type == SERVICE_NOTIFY_RELOAD;
 }
 
-static unsigned service_exec_command_index(Unit *u, ServiceExecCommand id, ExecCommand *current) {
+static unsigned service_exec_command_index(Unit *u, ServiceExecCommand id, const ExecCommand *current) {
         Service *s = SERVICE(u);
         unsigned idx = 0;
-        ExecCommand *first, *c;
 
         assert(s);
         assert(id >= 0);
         assert(id < _SERVICE_EXEC_COMMAND_MAX);
 
-        first = s->exec_command[id];
+        const ExecCommand *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)
+        for (const ExecCommand *c = current; c != first; c = c->command_prev)
                 idx++;
 
         return idx;
 }
 
-static int service_serialize_exec_command(Unit *u, FILE *f, ExecCommand *command) {
+static int service_serialize_exec_command(Unit *u, FILE *f, const ExecCommand *command) {
         _cleanup_free_ char *args = NULL, *p = NULL;
         Service *s = SERVICE(u);
         const char *type, *key;
@@ -2764,7 +2763,16 @@ static int service_serialize_exec_command(Unit *u, FILE *f, ExecCommand *command
                 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;
 }
@@ -2878,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;
@@ -2919,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;
@@ -2966,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.");
 
diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c
index 10796e25c7..f6a5e4aa06 100644
--- a/src/libsystemd/sd-bus/sd-bus.c
+++ b/src/libsystemd/sd-bus/sd-bus.c
@@ -48,7 +48,9 @@
 #define log_debug_bus_message(m)                                         \
         do {                                                             \
                 sd_bus_message *_mm = (m);                               \
-                log_debug("Got message type=%s sender=%s destination=%s path=%s interface=%s member=%s cookie=%" PRIu64 " reply_cookie=%" PRIu64 " signature=%s error-name=%s error-message=%s", \
+                log_debug("Got message type=%s sender=%s destination=%s path=%s interface=%s member=%s " \
+                          " cookie=%" PRIu64 " reply_cookie=%" PRIu64   \
+                          " signature=%s error-name=%s error-message=%s", \
                           strna(bus_message_type_to_string(_mm->header->type)), \
                           strna(sd_bus_message_get_sender(_mm)),         \
                           strna(sd_bus_message_get_destination(_mm)),    \
@@ -1983,7 +1985,9 @@ static int bus_write_message(sd_bus *bus, sd_bus_message *m, size_t *idx) {
                 return r;
 
         if (*idx >= BUS_MESSAGE_SIZE(m))
-                log_debug("Sent message type=%s sender=%s destination=%s path=%s interface=%s member=%s cookie=%" PRIu64 " reply_cookie=%" PRIu64 " signature=%s error-name=%s error-message=%s",
+                log_debug("Sent message type=%s sender=%s destination=%s path=%s interface=%s member=%s"
+                          " cookie=%" PRIu64 " reply_cookie=%" PRIu64
+                          " signature=%s error-name=%s error-message=%s",
                           bus_message_type_to_string(m->header->type),
                           strna(sd_bus_message_get_sender(m)),
                           strna(sd_bus_message_get_destination(m)),
@@ -2412,7 +2416,8 @@ _public_ int sd_bus_call(
                                                 return 1;
                                         }
 
-                                        return sd_bus_error_set(error, SD_BUS_ERROR_INCONSISTENT_MESSAGE, "Reply message contained file descriptors which I couldn't accept. Sorry.");
+                                        return sd_bus_error_set(error, SD_BUS_ERROR_INCONSISTENT_MESSAGE,
+                                                                "Reply message contained file descriptors which I couldn't accept. Sorry.");
 
                                 } else if (incoming->header->type == SD_BUS_MESSAGE_METHOD_ERROR)
                                         return sd_bus_error_copy(error, &incoming->error);
@@ -2924,7 +2929,8 @@ static int process_fd_check(sd_bus *bus, sd_bus_message *m) {
         if (m->header->type != SD_BUS_MESSAGE_METHOD_CALL)
                 return 1; /* just eat it up */
 
-        return sd_bus_reply_method_errorf(m, SD_BUS_ERROR_INCONSISTENT_MESSAGE, "Message contains file descriptors, which I cannot accept. Sorry.");
+        return sd_bus_reply_method_errorf(m, SD_BUS_ERROR_INCONSISTENT_MESSAGE,
+                                          "Message contains file descriptors, which I cannot accept. Sorry.");
 }
 
 static int process_message(sd_bus *bus, sd_bus_message *m) {
diff --git a/src/test/test-parse-util.c b/src/test/test-parse-util.c
index 388d0fe3f7..c83e8fc630 100644
--- a/src/test/test-parse-util.c
+++ b/src/test/test-parse-util.c
@@ -480,6 +480,14 @@ TEST(safe_atou16) {
         assert_se(r == 0);
         assert_se(l == 12345);
 
+        r = safe_atou16("+12345", &l);
+        assert_se(r == 0);
+        assert_se(l == 12345);
+
+        r = safe_atou16("  +12345", &l);
+        assert_se(r == 0);
+        assert_se(l == 12345);
+
         r = safe_atou16("123456", &l);
         assert_se(r == -ERANGE);
 
@@ -514,6 +522,14 @@ TEST(safe_atoi16) {
         assert_se(r == 0);
         assert_se(l == -12345);
 
+        r = safe_atoi16("+12345", &l);
+        assert_se(r == 0);
+        assert_se(l == 12345);
+
+        r = safe_atoi16("  +12345", &l);
+        assert_se(r == 0);
+        assert_se(l == 12345);
+
         r = safe_atoi16("32767", &l);
         assert_se(r == 0);
         assert_se(l == 32767);
@@ -703,6 +719,22 @@ TEST(safe_atoux64) {
         assert_se(r == 0);
         assert_se(l == 11603985);
 
+        r = safe_atoux64("+12345", &l);
+        assert_se(r == 0);
+        assert_se(l == 0x12345);
+
+        r = safe_atoux64("  +12345", &l);
+        assert_se(r == 0);
+        assert_se(l == 0x12345);
+
+        r = safe_atoux64("+0x12345", &l);
+        assert_se(r == 0);
+        assert_se(l == 0x12345);
+
+        r = safe_atoux64("+0b11011", &l);
+        assert_se(r == 0);
+        assert_se(l == 11603985);
+
         r = safe_atoux64("0o11011", &l);
         assert_se(r == -EINVAL);