From 5d44db4a905c62d6cf0dfabbf61df49621efec22 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 24 Feb 2011 02:36:34 +0100 Subject: [PATCH] dbus: pass along information why a job failed when it failed (dbus api change!) --- src/dbus-job.c | 8 +++-- src/dbus-job.h | 4 ++- src/dbus-manager.c | 2 +- src/job.c | 47 ++++++++++++++++++--------- src/job.h | 19 +++++++++-- src/systemadm.vala | 2 +- src/systemctl.c | 65 ++++++++++++++++++++++++++++--------- src/systemd-interfaces.vala | 2 +- src/unit.c | 12 +++---- 9 files changed, 115 insertions(+), 46 deletions(-) diff --git a/src/dbus-job.c b/src/dbus-job.c index 18da72d6..16aa8d07 100644 --- a/src/dbus-job.c +++ b/src/dbus-job.c @@ -295,10 +295,10 @@ oom: log_error("Failed to allocate job change signal."); } -void bus_job_send_removed_signal(Job *j, bool success) { +void bus_job_send_removed_signal(Job *j) { char *p = NULL; DBusMessage *m = NULL; - dbus_bool_t b = success; + const char *r; assert(j); @@ -314,10 +314,12 @@ void bus_job_send_removed_signal(Job *j, bool success) { if (!(m = dbus_message_new_signal("/org/freedesktop/systemd1", "org.freedesktop.systemd1.Manager", "JobRemoved"))) goto oom; + r = job_result_to_string(j->result); + if (!dbus_message_append_args(m, DBUS_TYPE_UINT32, &j->id, DBUS_TYPE_OBJECT_PATH, &p, - DBUS_TYPE_BOOLEAN, &b, + DBUS_TYPE_STRING, &r, DBUS_TYPE_INVALID)) goto oom; diff --git a/src/dbus-job.h b/src/dbus-job.h index 801e3e63..103c2ff3 100644 --- a/src/dbus-job.h +++ b/src/dbus-job.h @@ -24,8 +24,10 @@ #include +#include "job.h" + void bus_job_send_change_signal(Job *j); -void bus_job_send_removed_signal(Job *j, bool success); +void bus_job_send_removed_signal(Job *j); extern const DBusObjectPathVTable bus_job_vtable; diff --git a/src/dbus-manager.c b/src/dbus-manager.c index 6f98aa73..1b3bddc3 100644 --- a/src/dbus-manager.c +++ b/src/dbus-manager.c @@ -143,7 +143,7 @@ " \n" \ " \n" \ " \n" \ - " \n" \ + " \n" \ " " diff --git a/src/job.c b/src/job.c index ec57144c..53c47d4a 100644 --- a/src/job.c +++ b/src/job.c @@ -60,7 +60,7 @@ void job_free(Job *j) { /* Detach from next 'bigger' objects */ if (j->installed) { - bus_job_send_removed_signal(j, !j->failed); + bus_job_send_removed_signal(j); if (j->unit->meta.job == j) { j->unit->meta.job = NULL; @@ -376,7 +376,6 @@ int job_run_and_invalidate(Job *j) { j->state = JOB_RUNNING; job_add_to_dbus_queue(j); - job_start_timer(j); /* While we execute this operation the job might go away (for * example: because it is replaced by a new, conflicting @@ -457,17 +456,19 @@ int job_run_and_invalidate(Job *j) { if ((j = manager_get_job(m, id))) { if (r == -EALREADY) - r = job_finish_and_invalidate(j, true); + r = job_finish_and_invalidate(j, JOB_DONE); else if (r == -EAGAIN) j->state = JOB_WAITING; else if (r < 0) - r = job_finish_and_invalidate(j, false); + r = job_finish_and_invalidate(j, JOB_FAILED); + else + job_start_timer(j); } return r; } -int job_finish_and_invalidate(Job *j, bool success) { +int job_finish_and_invalidate(Job *j, JobResult result) { Unit *u; Unit *other; JobType t; @@ -479,7 +480,7 @@ int job_finish_and_invalidate(Job *j, bool success) { job_add_to_dbus_queue(j); /* Patch restart jobs so that they become normal start jobs */ - if (success && (j->type == JOB_RESTART || j->type == JOB_TRY_RESTART)) { + if (result == JOB_DONE && (j->type == JOB_RESTART || j->type == JOB_TRY_RESTART)) { log_debug("Converting job %s/%s -> %s/%s", j->unit->meta.id, job_type_to_string(j->type), @@ -492,22 +493,26 @@ int job_finish_and_invalidate(Job *j, bool success) { return 0; } - j->failed = !success; + j->result = result; - log_debug("Job %s/%s finished, success=%s", j->unit->meta.id, job_type_to_string(j->type), yes_no(success)); + log_debug("Job %s/%s finished, result=%s", j->unit->meta.id, job_type_to_string(j->type), job_result_to_string(result)); - if (j->failed) + if (result == JOB_FAILED) j->manager->n_failed_jobs ++; u = j->unit; t = j->type; job_free(j); - if (!success && j->type == JOB_START) + if (result == JOB_FAILED && j->type == JOB_START) unit_status_printf(u, "Starting %s " ANSI_HIGHLIGHT_ON "failed" ANSI_HIGHLIGHT_OFF ", see 'systemctl status %s' for details.\n", unit_description(u), u->meta.id); + else if (result == JOB_TIMEOUT && j->type == JOB_START) + unit_status_printf(u, "Starting %s " ANSI_HIGHLIGHT_ON "timed out" ANSI_HIGHLIGHT_OFF ".\n", unit_description(u), u->meta.id); + else if (result == JOB_TIMEOUT && j->type == JOB_STOP) + unit_status_printf(u, "Stopping %s " ANSI_HIGHLIGHT_ON "timed out" ANSI_HIGHLIGHT_OFF ".\n", unit_description(u), u->meta.id); /* Fail depending jobs on failure */ - if (!success) { + if (result != JOB_DONE) { if (t == JOB_START || t == JOB_VERIFY_ACTIVE || @@ -518,14 +523,14 @@ int job_finish_and_invalidate(Job *j, bool success) { (other->meta.job->type == JOB_START || other->meta.job->type == JOB_VERIFY_ACTIVE || other->meta.job->type == JOB_RELOAD_OR_START)) - job_finish_and_invalidate(other->meta.job, false); + job_finish_and_invalidate(other->meta.job, JOB_DEPENDENCY); SET_FOREACH(other, u->meta.dependencies[UNIT_BOUND_BY], i) if (other->meta.job && (other->meta.job->type == JOB_START || other->meta.job->type == JOB_VERIFY_ACTIVE || other->meta.job->type == JOB_RELOAD_OR_START)) - job_finish_and_invalidate(other->meta.job, false); + job_finish_and_invalidate(other->meta.job, JOB_DEPENDENCY); SET_FOREACH(other, u->meta.dependencies[UNIT_REQUIRED_BY_OVERRIDABLE], i) if (other->meta.job && @@ -533,7 +538,7 @@ int job_finish_and_invalidate(Job *j, bool success) { (other->meta.job->type == JOB_START || other->meta.job->type == JOB_VERIFY_ACTIVE || other->meta.job->type == JOB_RELOAD_OR_START)) - job_finish_and_invalidate(other->meta.job, false); + job_finish_and_invalidate(other->meta.job, JOB_DEPENDENCY); } else if (t == JOB_STOP) { @@ -542,7 +547,7 @@ int job_finish_and_invalidate(Job *j, bool success) { (other->meta.job->type == JOB_START || other->meta.job->type == JOB_VERIFY_ACTIVE || other->meta.job->type == JOB_RELOAD_OR_START)) - job_finish_and_invalidate(other->meta.job, false); + job_finish_and_invalidate(other->meta.job, JOB_DEPENDENCY); } } @@ -648,7 +653,7 @@ void job_timer_event(Job *j, uint64_t n_elapsed, Watch *w) { assert(w == &j->timer_watch); log_warning("Job %s/%s timed out.", j->unit->meta.id, job_type_to_string(j->type)); - job_finish_and_invalidate(j, false); + job_finish_and_invalidate(j, JOB_TIMEOUT); } static const char* const job_state_table[_JOB_STATE_MAX] = { @@ -678,3 +683,13 @@ static const char* const job_mode_table[_JOB_MODE_MAX] = { }; DEFINE_STRING_TABLE_LOOKUP(job_mode, JobMode); + +static const char* const job_result_table[_JOB_RESULT_MAX] = { + [JOB_DONE] = "done", + [JOB_CANCELED] = "canceled", + [JOB_TIMEOUT] = "timeout", + [JOB_FAILED] = "failed", + [JOB_DEPENDENCY] = "dependency" +}; + +DEFINE_STRING_TABLE_LOOKUP(job_result, JobResult); diff --git a/src/job.h b/src/job.h index 2499e649..de0c1d23 100644 --- a/src/job.h +++ b/src/job.h @@ -30,6 +30,7 @@ typedef struct JobDependency JobDependency; typedef enum JobType JobType; typedef enum JobState JobState; typedef enum JobMode JobMode; +typedef enum JobResult JobResult; #include "manager.h" #include "unit.h" @@ -71,6 +72,16 @@ enum JobMode { _JOB_MODE_INVALID = -1 }; +enum JobResult { + JOB_DONE, + JOB_CANCELED, + JOB_TIMEOUT, + JOB_FAILED, + JOB_DEPENDENCY, + _JOB_RESULT_MAX, + _JOB_RESULT_INVALID = -1 +}; + struct JobDependency { /* Encodes that the 'subject' job needs the 'object' job in * some way. This structure is used only while building a transaction. */ @@ -110,13 +121,14 @@ struct Job { DBusConnection *bus; char *bus_client; + JobResult result; + bool installed:1; bool in_run_queue:1; bool matters_to_anchor:1; bool override:1; bool in_dbus_queue:1; bool sent_dbus_new_signal:1; - bool failed:1; bool ignore_deps:1; }; @@ -146,7 +158,7 @@ int job_start_timer(Job *j); void job_timer_event(Job *j, uint64_t n_elapsed, Watch *w); int job_run_and_invalidate(Job *j); -int job_finish_and_invalidate(Job *j, bool success); +int job_finish_and_invalidate(Job *j, JobResult result); char *job_dbus_path(Job *j); @@ -159,4 +171,7 @@ JobState job_state_from_string(const char *s); const char* job_mode_to_string(JobMode t); JobMode job_mode_from_string(const char *s); +const char* job_result_to_string(JobResult t); +JobResult job_result_from_string(const char *s); + #endif diff --git a/src/systemadm.vala b/src/systemadm.vala index 33777b6b..c262794c 100644 --- a/src/systemadm.vala +++ b/src/systemadm.vala @@ -762,7 +762,7 @@ public class MainWindow : Window { } while (unit_model.iter_next(ref iter)); } - public void on_job_removed(uint32 id, ObjectPath path, bool success) { + public void on_job_removed(uint32 id, ObjectPath path, string res) { TreeIter iter; if (!(job_model.get_iter_first(out iter))) return; diff --git a/src/systemctl.c b/src/systemctl.c index eacc63e9..82741bc6 100644 --- a/src/systemctl.c +++ b/src/systemctl.c @@ -1120,7 +1120,7 @@ finish: typedef struct WaitData { Set *set; - bool failed; + char *result; } WaitData; static DBusHandlerResult wait_filter(DBusConnection *connection, DBusMessage *message, void *data) { @@ -1144,26 +1144,52 @@ static DBusHandlerResult wait_filter(DBusConnection *connection, DBusMessage *me } else if (dbus_message_is_signal(message, "org.freedesktop.systemd1.Manager", "JobRemoved")) { uint32_t id; - const char *path; + const char *path, *result; dbus_bool_t success = true; - if (!dbus_message_get_args(message, &error, - DBUS_TYPE_UINT32, &id, - DBUS_TYPE_OBJECT_PATH, &path, - DBUS_TYPE_BOOLEAN, &success, - DBUS_TYPE_INVALID)) - log_error("Failed to parse message: %s", bus_error_message(&error)); - else { + if (dbus_message_get_args(message, &error, + DBUS_TYPE_UINT32, &id, + DBUS_TYPE_OBJECT_PATH, &path, + DBUS_TYPE_STRING, &result, + DBUS_TYPE_INVALID)) { + char *p; + + if ((p = set_remove(d->set, (char*) path))) + free(p); + + if (*result) + d->result = strdup(result); + + goto finish; + } +#ifndef LEGACY + dbus_error_free(&error); + + if (dbus_message_get_args(message, &error, + DBUS_TYPE_UINT32, &id, + DBUS_TYPE_OBJECT_PATH, &path, + DBUS_TYPE_BOOLEAN, &success, + DBUS_TYPE_INVALID)) { char *p; + /* Compatibility with older systemd versions < + * 19 during upgrades. This should be dropped + * one day */ + if ((p = set_remove(d->set, (char*) path))) free(p); if (!success) - d->failed = true; + d->result = strdup("failed"); + + goto finish; } +#endif + + log_error("Failed to parse message: %s", bus_error_message(&error)); } +finish: dbus_error_free(&error); return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; } @@ -1204,7 +1230,6 @@ static int wait_for_jobs(DBusConnection *bus, Set *s) { zero(d); d.set = s; - d.failed = false; if (!dbus_connection_add_filter(bus, wait_filter, &d, NULL)) { log_error("Failed to add filter."); @@ -1216,10 +1241,19 @@ static int wait_for_jobs(DBusConnection *bus, Set *s) { dbus_connection_read_write_dispatch(bus, -1)) ; - if (!arg_quiet && d.failed) - log_error("Job failed. See system logs and 'systemctl status' for details."); + if (!arg_quiet && d.result) { + if (streq(d.result, "timeout")) + log_error("Job timed out."); + else if (streq(d.result, "canceled")) + log_error("Job canceled."); + else if (streq(d.result, "dependency")) + log_error("A dependency job failed. See system logs for details."); + else + log_error("Job failed. See system logs and 'systemctl status' for details."); + } - r = d.failed ? -EIO : 0; + r = d.result ? -EIO : 0; + free(d.result); finish: /* This is slightly dirty, since we don't undo the filter registration. */ @@ -2750,11 +2784,12 @@ static DBusHandlerResult monitor_filter(DBusConnection *connection, DBusMessage } else if (dbus_message_is_signal(message, "org.freedesktop.systemd1.Manager", "JobNew") || dbus_message_is_signal(message, "org.freedesktop.systemd1.Manager", "JobRemoved")) { uint32_t id; - const char *path; + const char *path, *result; if (!dbus_message_get_args(message, &error, DBUS_TYPE_UINT32, &id, DBUS_TYPE_OBJECT_PATH, &path, + DBUS_TYPE_STRING, &result, DBUS_TYPE_INVALID)) log_error("Failed to parse message: %s", bus_error_message(&error)); else if (streq(dbus_message_get_member(message), "JobNew")) diff --git a/src/systemd-interfaces.vala b/src/systemd-interfaces.vala index e3b79412..a380f797 100644 --- a/src/systemd-interfaces.vala +++ b/src/systemd-interfaces.vala @@ -85,7 +85,7 @@ public interface Manager : DBusProxy { public abstract signal void unit_new(string id, ObjectPath path); public abstract signal void unit_removed(string id, ObjectPath path); public abstract signal void job_new(uint32 id, ObjectPath path); - public abstract signal void job_removed(uint32 id, ObjectPath path, bool success); + public abstract signal void job_removed(uint32 id, ObjectPath path, string res); } [DBus (name = "org.freedesktop.systemd1.Unit")] diff --git a/src/unit.c b/src/unit.c index 0d531237..1ca8e82a 100644 --- a/src/unit.c +++ b/src/unit.c @@ -1123,12 +1123,12 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, bool reload_su case JOB_VERIFY_ACTIVE: if (UNIT_IS_ACTIVE_OR_RELOADING(ns)) - job_finish_and_invalidate(u->meta.job, true); + job_finish_and_invalidate(u->meta.job, JOB_DONE); else if (u->meta.job->state == JOB_RUNNING && ns != UNIT_ACTIVATING) { unexpected = true; if (UNIT_IS_INACTIVE_OR_FAILED(ns)) - job_finish_and_invalidate(u->meta.job, ns != UNIT_FAILED); + job_finish_and_invalidate(u->meta.job, ns == UNIT_FAILED ? JOB_FAILED : JOB_DONE); } break; @@ -1138,12 +1138,12 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, bool reload_su if (u->meta.job->state == JOB_RUNNING) { if (ns == UNIT_ACTIVE) - job_finish_and_invalidate(u->meta.job, reload_success); + job_finish_and_invalidate(u->meta.job, reload_success ? JOB_DONE : JOB_FAILED); else if (ns != UNIT_ACTIVATING && ns != UNIT_RELOADING) { unexpected = true; if (UNIT_IS_INACTIVE_OR_FAILED(ns)) - job_finish_and_invalidate(u->meta.job, ns != UNIT_FAILED); + job_finish_and_invalidate(u->meta.job, ns == UNIT_FAILED ? JOB_FAILED : JOB_DONE); } } @@ -1154,10 +1154,10 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, bool reload_su case JOB_TRY_RESTART: if (UNIT_IS_INACTIVE_OR_FAILED(ns)) - job_finish_and_invalidate(u->meta.job, true); + job_finish_and_invalidate(u->meta.job, JOB_DONE); else if (u->meta.job->state == JOB_RUNNING && ns != UNIT_DEACTIVATING) { unexpected = true; - job_finish_and_invalidate(u->meta.job, false); + job_finish_and_invalidate(u->meta.job, JOB_FAILED); } break; -- 2.39.5