[systemd-devel] [PATCH v4] bus-util: be more verbose if dbus job fails

[Lennart Poettering]

On Fri, 10.04.15 13:48, Michal Sekletar (msekleta at redhat.com) wrote:

> 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

We're close. Just two more issues I ntoiced:

> +        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;

There's no need to ue the "p" and "r" variables at all, you can just pass
"result" directorly to the call and return the result directly. That
should shorten the function quite a bit.

> +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" }
> +};
> +
> +static 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));

No assert in this case please. We need the freedom to add new result
codes to PID 1 without breaking all systemctl client versions, and
making the abort. Note that people should be able to use older
systemctl with newer systemd just fine.

Lennart

-- 
Lennart Poettering, Red Hat