[systemd-devel] [PATCH v3] log: be more verbose if dbus job fails

[Michal Sekletar]

Users might have hard time figuring out why exactly their systemctl request
failed. If dbus job fails try to figure out more details about failure by
examining Result property of the service.

https://bugzilla.redhat.com/show_bug.cgi?id=1016680
---

Changes in v3:
        * service result explanation is determined by linear search in explanations array
        * reworked logging in order not to use memstream

 src/libsystemd/sd-bus/bus-util.c | 45 +++++++++++++++++++++++++++++++++-------
 src/shared/log.c                 | 42 +++++++++++++++++++++++++++++++++++++
 src/shared/log.h                 |  2 ++
 3 files changed, 82 insertions(+), 7 deletions(-)

diff --git a/src/libsystemd/sd-bus/bus-util.c b/src/libsystemd/sd-bus/bus-util.c
index ea2379d..890d681 100644
--- a/src/libsystemd/sd-bus/bus-util.c
+++ b/src/libsystemd/sd-bus/bus-util.c
@@ -30,6 +30,7 @@
 #include "path-util.h"
 #include "missing.h"
 #include "set.h"
+#include "unit-name.h"
 
 #include "sd-bus.h"
 #include "bus-error.h"
@@ -1708,6 +1709,34 @@ static int bus_process_wait(sd_bus *bus) {
         }
 }
 
+static int bus_job_get_service_result(BusWaitForJobs *d, char **result) {
+        int r = 0;
+        _cleanup_free_ char *dbus_path = NULL, *p = NULL;
+
+        assert(d);
+        assert(d->name);
+        assert(result);
+
+        dbus_path = unit_dbus_path_from_name(d->name);
+        if (!dbus_path)
+                return -ENOMEM;
+
+        r = sd_bus_get_property_string(d->bus,
+                                       "org.freedesktop.systemd1",
+                                       dbus_path,
+                                       "org.freedesktop.systemd1.Service",
+                                       "Result",
+                                       NULL,
+                                       &p);
+        if (r < 0)
+                return r;
+
+        *result = p;
+        p = NULL;
+
+        return 0;
+}
+
 static int check_wait_response(BusWaitForJobs *d, bool quiet) {
         int r = 0;
 
@@ -1727,15 +1756,17 @@ static int check_wait_response(BusWaitForJobs *d, bool quiet) {
                 else if (streq(d->result, "unsupported"))
                         log_error("Operation on or unit type of %s not supported on this system.", strna(d->name));
                 else if (!streq(d->result, "done") && !streq(d->result, "skipped")) {
-                        _cleanup_free_ char *quoted = NULL;
+                        if (d->name) {
+                                int q;
+                                _cleanup_free_ char *result = NULL;
 
-                        if (d->name)
-                                quoted = shell_maybe_quote(d->name);
+                                q = bus_job_get_service_result(d, &result);
+                                if (q < 0)
+                                        log_debug_errno(q, "Failed to get Result property of service %s: %m", d->name);
 
-                        if (quoted)
-                                log_error("Job for %s failed. See 'systemctl status %s' and 'journalctl -xe' for details.", d->name, quoted);
-                        else
-                                log_error("Job failed. See 'journalctl -xe' for details.");
+                                log_job_error_with_service_result(d->name, result);
+                        } else
+                                log_error("Job failed. See \"journalctl -xe\" for details.");
                 }
         }
 
diff --git a/src/shared/log.c b/src/shared/log.c
index 646a1d6..839051a 100644
--- a/src/shared/log.c
+++ b/src/shared/log.c
@@ -1061,3 +1061,45 @@ void log_received_signal(int level, const struct signalfd_siginfo *si) {
 void log_set_upgrade_syslog_to_journal(bool b) {
         upgrade_syslog_to_journal = b;
 }
+
+static const struct {
+        const char *result, *explanation;
+} explanations [] = {
+        { "resources", "configured resource limit was exceeded" },
+        { "timeout", "timeout was exceeded" },
+        { "exit-code", "control process exited with error code" },
+        { "signal", "fatal signal was delivered to the control process" },
+        { "core-dump", "fatal signal was delivered to the control process. Core dumped" },
+        { "watchdog", "service failed to sent watchdog ping" },
+        { "start-limit", "start of the service was attempted too often too quickly" }
+};
+
+/* Note that if this function is ever called from context where log target is journal, then code
+   should be reworked in a way that only one log entry is produced, containing both error
+   message and hint */
+void log_job_error_with_service_result(const char* service, const char *result) {
+        unsigned i;
+        _cleanup_free_ char *service_shell_quoted = NULL;
+
+        assert(service);
+        assert(result);
+
+        service_shell_quoted = shell_maybe_quote(service);
+
+        for (i = 0; i < ELEMENTSOF(explanations); ++i)
+                if (streq(result, explanations[i].result))
+                        break;
+
+        assert(i < ELEMENTSOF(explanations));
+
+        log_error("Job for %s failed because %s. See \"systemctl status %s\" and \"journalctl -xe\" for details.\n",
+                  service,
+                  explanations[i].explanation,
+                  service_shell_quoted);
+
+        /* For some results maybe additional explanation is required */
+        if (streq_ptr(result, "start-limit"))
+                log_info("To force a start please invoke \"systemctl reset-failed %s\" followed by \"systemctl start %s\" again.",
+                         service_shell_quoted,
+                         service_shell_quoted);
+}
diff --git a/src/shared/log.h b/src/shared/log.h
index d6061c0..1627a1c 100644
--- a/src/shared/log.h
+++ b/src/shared/log.h
@@ -209,3 +209,5 @@ LogTarget log_target_from_string(const char *s) _pure_;
 void log_received_signal(int level, const struct signalfd_siginfo *si);
 
 void log_set_upgrade_syslog_to_journal(bool b);
+
+void log_job_error_with_service_result(const char* service, const char *result);
-- 
2.3.4