From: Lennart Poettering Date: Fri, 14 Sep 2012 13:11:07 +0000 (+0200) Subject: systemctl: show unit name when a job fails X-Git-Tag: v190~89 X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?p=elogind.git;a=commitdiff_plain;h=67f3c40265471056d1e532c6d6e36a521b0a780a systemctl: show unit name when a job fails https://bugzilla.redhat.com/show_bug.cgi?id=845028 https://bugzilla.redhat.com/show_bug.cgi?id=846483 --- diff --git a/TODO b/TODO index 7ed215308..887d684ce 100644 --- a/TODO +++ b/TODO @@ -53,6 +53,8 @@ Bugfixes: Features: +* journald: warn if we drop messages we forward to the syslog socket + * does vasprintf advance the struct vaargs? http://pastie.org/pastes/4712773/text * do shutdown audit/utmp msgs inside of PID 1, get rid of systemd-update-utmp-runlevel diff --git a/src/shared/dbus-common.c b/src/shared/dbus-common.c index bcbef77b5..b8229bd66 100644 --- a/src/shared/dbus-common.c +++ b/src/shared/dbus-common.c @@ -1253,14 +1253,16 @@ bool bus_error_is_no_service(const DBusError *error) { return startswith(error->name, "org.freedesktop.DBus.Error.Spawn."); } -int bus_method_call_with_reply(DBusConnection *bus, - const char *destination, - const char *path, - const char *interface, - const char *method, - DBusMessage **return_reply, - DBusError *return_error, - int first_arg_type, ...) { +int bus_method_call_with_reply( + DBusConnection *bus, + const char *destination, + const char *path, + const char *interface, + const char *method, + DBusMessage **return_reply, + DBusError *return_error, + int first_arg_type, ...) { + DBusError error; DBusMessage *m, *reply; va_list ap; @@ -1287,6 +1289,7 @@ int bus_method_call_with_reply(DBusConnection *bus, if (!reply) { if (!return_error) log_error("Failed to issue method call: %s", bus_error_message(&error)); + if (bus_error_is_no_service(&error)) r = -ENOENT; else if (dbus_error_has_name(&error, DBUS_ERROR_ACCESS_DENIED)) diff --git a/src/shared/hashmap.c b/src/shared/hashmap.c index 0a044b85a..eb5c549e4 100644 --- a/src/shared/hashmap.c +++ b/src/shared/hashmap.c @@ -265,6 +265,8 @@ static void remove_entry(Hashmap *h, struct hashmap_entry *e) { void hashmap_free(Hashmap*h) { + /* Free the hashmap, but nothing in it */ + if (!h) return; @@ -277,6 +279,10 @@ void hashmap_free(Hashmap*h) { } void hashmap_free_free(Hashmap *h) { + + /* Free the hashmap and all data objects in it, but not the + * keys */ + if (!h) return; @@ -371,8 +377,8 @@ void* hashmap_get(Hashmap *h, const void *key) { return NULL; hash = h->hash_func(key) % NBUCKETS; - - if (!(e = hash_scan(h, hash, key))) + e = hash_scan(h, hash, key); + if (!e) return NULL; return e->value; diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index efb9ae294..17a8497dc 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -1175,6 +1175,8 @@ finish: typedef struct WaitData { Set *set; + + char *name; char *result; } WaitData; @@ -1213,9 +1215,12 @@ static DBusHandlerResult wait_filter(DBusConnection *connection, DBusMessage *me p = set_remove(d->set, (char*) path); free(p); - if (*result) + if (!isempty(result)) d->result = strdup(result); + if (!isempty(unit)) + d->name = strdup(unit); + goto finish; } #ifndef LEGACY @@ -1297,7 +1302,7 @@ static int enable_wait_for_jobs(DBusConnection *bus) { } static int wait_for_jobs(DBusConnection *bus, Set *s) { - int r; + int r = 0; WaitData d; assert(bus); @@ -1306,41 +1311,42 @@ static int wait_for_jobs(DBusConnection *bus, Set *s) { zero(d); d.set = s; - if (!dbus_connection_add_filter(bus, wait_filter, &d, NULL)) { - log_error("Failed to add filter."); - r = -ENOMEM; - goto finish; - } + if (!dbus_connection_add_filter(bus, wait_filter, &d, NULL)) + return log_oom(); - while (!set_isempty(s) && - dbus_connection_read_write_dispatch(bus, -1)) - ; + while (!set_isempty(s)) { - 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 journal for details."); - else if (!streq(d.result, "done") && !streq(d.result, "skipped")) - log_error("Job failed. See system journal and 'systemctl status' for details."); - } + if (!dbus_connection_read_write_dispatch(bus, -1)) { + log_error("Disconnected from bus."); + return -ECONNREFUSED; + } - if (streq_ptr(d.result, "timeout")) - r = -ETIME; - else if (streq_ptr(d.result, "canceled")) - r = -ECANCELED; - else if (!streq_ptr(d.result, "done") && !streq_ptr(d.result, "skipped")) - r = -EIO; - else - r = 0; + if (!arg_quiet && d.result) { + if (streq(d.result, "timeout")) + log_error("Job for %s timed out.", strna(d.name)); + else if (streq(d.result, "canceled")) + log_error("Job for %s canceled.", strna(d.name)); + else if (streq(d.result, "dependency")) + log_error("A dependency job for %s failed. See 'journalctl' for details.", strna(d.name)); + else if (!streq(d.result, "done") && !streq(d.result, "skipped")) + log_error("Job for %s failed. See 'systemctl status %s' and 'journalctl' for details.", strna(d.name), strna(d.name)); + } - free(d.result); + if (streq_ptr(d.result, "timeout")) + r = -ETIME; + else if (streq_ptr(d.result, "canceled")) + r = -ECANCELED; + else if (!streq_ptr(d.result, "done") && !streq_ptr(d.result, "skipped")) + r = -EIO; -finish: - /* This is slightly dirty, since we don't undo the filter registration. */ + free(d.result); + d.result = NULL; + + free(d.name); + d.name = NULL; + } + /* This is slightly dirty, since we don't undo the filter registration. */ return r; } @@ -1517,16 +1523,18 @@ static int start_unit_one( DBusMessage *reply = NULL; const char *path; int r; - char *n; + _cleanup_free_ char *n, *p = NULL; assert(method); assert(name); assert(mode); assert(error); - assert(arg_no_block || s); n = unit_name_mangle(name); - r = bus_method_call_with_reply ( + if (!n) + return log_oom(); + + r = bus_method_call_with_reply( bus, "org.freedesktop.systemd1", "/org/freedesktop/systemd1", @@ -1534,17 +1542,17 @@ static int start_unit_one( method, &reply, error, - DBUS_TYPE_STRING, n ? (const char **) &n : &name, + DBUS_TYPE_STRING, &n, DBUS_TYPE_STRING, &mode, DBUS_TYPE_INVALID); - free(n); if (r) { - if (r == -ENOENT && arg_action != ACTION_SYSTEMCTL ) + if (r == -ENOENT && arg_action != ACTION_SYSTEMCTL) /* There's always a fallback possible for * legacy actions. */ r = -EADDRNOTAVAIL; else log_error("Failed to issue method call: %s", bus_error_message(error)); + goto finish; } @@ -1556,24 +1564,24 @@ static int start_unit_one( goto finish; } - if (need_daemon_reload(bus, name)) - log_warning("Warning: Unit file of created job changed on disk, 'systemctl %s daemon-reload' recommended.", - arg_scope == UNIT_FILE_SYSTEM ? "--system" : "--user"); - - if (!arg_no_block) { - char *p; + if (need_daemon_reload(bus, n)) + log_warning("Warning: Unit file of %s changed on disk, 'systemctl %s daemon-reload' recommended.", + n, arg_scope == UNIT_FILE_SYSTEM ? "--system" : "--user"); - if (!(p = strdup(path))) { - log_error("Failed to duplicate path."); - r = -ENOMEM; + if (s) { + p = strdup(path); + if (!p) { + r = log_oom(); goto finish; } - if ((r = set_put(s, p)) < 0) { - free(p); + r = set_put(s, p); + if (r < 0) { log_error("Failed to add path to set."); goto finish; } + + p = NULL; } /* When stopping a unit warn if it can still be triggered by @@ -1688,39 +1696,43 @@ static int start_unit(DBusConnection *bus, char **args) { } if (!arg_no_block) { - if ((ret = enable_wait_for_jobs(bus)) < 0) { + ret = enable_wait_for_jobs(bus); + if (ret < 0) { log_error("Could not watch jobs: %s", strerror(-ret)); goto finish; } - if (!(s = set_new(string_hash_func, string_compare_func))) { - log_error("Failed to allocate set."); - ret = -ENOMEM; + s = set_new(string_hash_func, string_compare_func); + if (!s) { + ret = log_oom(); goto finish; } } if (one_name) { - if ((ret = start_unit_one(bus, method, one_name, mode, &error, s)) <= 0) - goto finish; + ret = start_unit_one(bus, method, one_name, mode, &error, s); + if (ret < 0) + ret = translate_bus_error_to_exit_status(ret, &error); } else { - STRV_FOREACH(name, args+1) - if ((r = start_unit_one(bus, method, *name, mode, &error, s)) != 0) { + STRV_FOREACH(name, args+1) { + r = start_unit_one(bus, method, *name, mode, &error, s); + if (r < 0) { ret = translate_bus_error_to_exit_status(r, &error); dbus_error_free(&error); } + } } - if (!arg_no_block) - if ((r = wait_for_jobs(bus, s)) < 0) { + if (!arg_no_block) { + r = wait_for_jobs(bus, s); + if (r < 0) { ret = r; goto finish; } + } finish: - if (s) - set_free_free(s); - + set_free_free(s); dbus_error_free(&error); return ret;