diff --git a/src/shared/bus-wait-for-jobs.c b/src/shared/bus-wait-for-jobs.c index b3280c15df..70f2043347 100644 --- a/src/shared/bus-wait-for-jobs.c +++ b/src/shared/bus-wait-for-jobs.c @@ -1,13 +1,13 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #include "alloc-util.h" -#include "bus-wait-for-jobs.h" -#include "set.h" -#include "bus-util.h" #include "bus-internal.h" -#include "unit-def.h" +#include "bus-util.h" +#include "bus-wait-for-jobs.h" #include "escape.h" +#include "set.h" #include "strv.h" +#include "unit-def.h" typedef struct BusWaitForJobs { sd_bus *bus; @@ -23,43 +23,6 @@ typedef struct BusWaitForJobs { sd_bus_slot *slot_disconnected; } BusWaitForJobs; -static int match_disconnected(sd_bus_message *m, void *userdata, sd_bus_error *error) { - assert(m); - - log_error("Warning! D-Bus connection terminated."); - sd_bus_close(sd_bus_message_get_bus(m)); - - return 0; -} - -static int match_job_removed(sd_bus_message *m, void *userdata, sd_bus_error *error) { - const char *path, *unit, *result; - BusWaitForJobs *d = ASSERT_PTR(userdata); - uint32_t id; - char *found; - int r; - - assert(m); - - r = sd_bus_message_read(m, "uoss", &id, &path, &unit, &result); - if (r < 0) { - bus_log_parse_error(r); - return 0; - } - - found = set_remove(d->jobs, (char*) path); - if (!found) - return 0; - - free(found); - - (void) free_and_strdup(&d->result, empty_to_null(result)); - - (void) free_and_strdup(&d->name, empty_to_null(unit)); - - return 0; -} - BusWaitForJobs* bus_wait_for_jobs_free(BusWaitForJobs *d) { if (!d) return NULL; @@ -77,6 +40,39 @@ BusWaitForJobs* bus_wait_for_jobs_free(BusWaitForJobs *d) { return mfree(d); } +static int match_disconnected(sd_bus_message *m, void *userdata, sd_bus_error *error) { + assert(m); + + log_warning("D-Bus connection terminated while waiting for jobs."); + sd_bus_close(sd_bus_message_get_bus(m)); + + return 0; +} + +static int match_job_removed(sd_bus_message *m, void *userdata, sd_bus_error *error) { + BusWaitForJobs *d = ASSERT_PTR(userdata); + _cleanup_free_ char *job_found = NULL; + const char *path, *unit, *result; + int r; + + assert(m); + + r = sd_bus_message_read(m, "uoss", /* id = */ NULL, &path, &unit, &result); + if (r < 0) { + bus_log_parse_error(r); + return 0; + } + + job_found = set_remove(d->jobs, (char*) path); + if (!job_found) + return 0; + + (void) free_and_strdup(&d->name, empty_to_null(unit)); + (void) free_and_strdup(&d->result, empty_to_null(result)); + + return 0; +} + int bus_wait_for_jobs_new(sd_bus *bus, BusWaitForJobs **ret) { _cleanup_(bus_wait_for_jobs_freep) BusWaitForJobs *d = NULL; int r; @@ -92,9 +88,8 @@ int bus_wait_for_jobs_new(sd_bus *bus, BusWaitForJobs **ret) { .bus = sd_bus_ref(bus), }; - /* When we are a bus client we match by sender. Direct - * connections OTOH have no initialized sender field, and - * hence we ignore the sender then */ + /* When we are a bus client we match by sender. Direct connections OTOH have no initialized sender + * field, and hence we ignore the sender then */ r = sd_bus_match_signal_async( bus, &d->slot_job_removed, @@ -138,12 +133,12 @@ static int bus_process_wait(sd_bus *bus) { } } -static int bus_job_get_service_result(BusWaitForJobs *d, char **result) { +static int bus_job_get_service_result(BusWaitForJobs *d, char **ret) { _cleanup_free_ char *dbus_path = NULL; assert(d); assert(d->name); - assert(result); + assert(ret); if (!endswith(d->name, ".service")) return -EINVAL; @@ -158,67 +153,57 @@ static int bus_job_get_service_result(BusWaitForJobs *d, char **result) { "org.freedesktop.systemd1.Service", "Result", NULL, - result); + ret); } static void log_job_error_with_service_result(const char* service, const char *result, const char* const* extra_args) { - _cleanup_free_ char *service_shell_quoted = NULL; - const char *systemctl = "systemctl", *journalctl = "journalctl"; static const struct { const char *result, *explanation; } explanations[] = { - { "resources", "of unavailable resources or another system error" }, + { "resources", "of unavailable resources or another system error" }, { "protocol", "the service did not take the steps required by its unit configuration" }, - { "timeout", "a timeout was exceeded" }, - { "exit-code", "the control process exited with error code" }, - { "signal", "a fatal signal was delivered to the control process" }, + { "timeout", "a timeout was exceeded" }, + { "exit-code", "the control process exited with error code" }, + { "signal", "a fatal signal was delivered to the control process" }, { "core-dump", "a fatal signal was delivered causing the control process to dump core" }, - { "watchdog", "the service failed to send watchdog ping" }, - { "start-limit", "start of the service was attempted too often" } + { "watchdog", "the service failed to send watchdog ping" }, + { "start-limit", "start of the service was attempted too often" }, }; + _cleanup_free_ char *service_shell_quoted = NULL; + const char *systemctl = "systemctl", *journalctl = "journalctl"; + assert(service); service_shell_quoted = shell_maybe_quote(service, 0); - if (!strv_isempty((char**) extra_args)) { + if (!strv_isempty((char* const*) extra_args)) { _cleanup_free_ char *t = NULL; - t = strv_join((char**) extra_args, " "); + t = strv_join((char* const*) extra_args, " "); systemctl = strjoina("systemctl ", t ?: ""); journalctl = strjoina("journalctl ", t ?: ""); } - if (!isempty(result)) { - size_t i; - - for (i = 0; i < ELEMENTSOF(explanations); ++i) - if (streq(result, explanations[i].result)) - break; - - if (i < ELEMENTSOF(explanations)) { - log_error("Job for %s failed because %s.\n" - "See \"%s status %s\" and \"%s -xeu %s\" for details.\n", - service, - explanations[i].explanation, - systemctl, - service_shell_quoted ?: "", - journalctl, - service_shell_quoted ?: ""); - goto finish; - } - } + if (!isempty(result)) + FOREACH_ARRAY(i, explanations, ELEMENTSOF(explanations)) + if (streq(result, i->result)) { + log_error("Job for %s failed because %s.\n" + "See \"%s status %s\" and \"%s -xeu %s\" for details.\n", + service, i->explanation, + systemctl, service_shell_quoted ?: "", + journalctl, service_shell_quoted ?: ""); + goto extra; + } log_error("Job for %s failed.\n" "See \"%s status %s\" and \"%s -xeu %s\" for details.\n", service, - systemctl, - service_shell_quoted ?: "", - journalctl, - service_shell_quoted ?: ""); + systemctl, service_shell_quoted ?: "", + journalctl, service_shell_quoted ?: ""); -finish: +extra: /* For some results maybe additional explanation is required */ if (streq_ptr(result, "start-limit")) log_info("To force a start use \"%1$s reset-failed %2$s\"\n" @@ -228,41 +213,53 @@ finish: } static int check_wait_response(BusWaitForJobs *d, WaitJobsFlags flags, const char* const* extra_args) { + int r; + assert(d); assert(d->name); assert(d->result); + if (streq(d->result, "done")) { + if (FLAGS_SET(flags, BUS_WAIT_JOBS_LOG_SUCCESS)) + log_info("Job for %s finished.", d->name); + + return 0; + } else if (streq(d->result, "skipped")) { + if (FLAGS_SET(flags, BUS_WAIT_JOBS_LOG_SUCCESS)) + log_info("Job for %s was skipped.", d->name); + + return 0; + } + if (FLAGS_SET(flags, BUS_WAIT_JOBS_LOG_ERROR)) { if (streq(d->result, "canceled")) - log_error("Job for %s canceled.", strna(d->name)); + log_error("Job for %s canceled.", d->name); else if (streq(d->result, "timeout")) - log_error("Job for %s timed out.", strna(d->name)); + log_error("Job for %s timed out.", d->name); else if (streq(d->result, "dependency")) - log_error("A dependency job for %s failed. See 'journalctl -xe' for details.", strna(d->name)); + log_error("A dependency job for %s failed. See 'journalctl -xe' for details.", d->name); else if (streq(d->result, "invalid")) - log_error("%s is not active, cannot reload.", strna(d->name)); + log_error("%s is not active, cannot reload.", d->name); else if (streq(d->result, "assert")) - log_error("Assertion failed on job for %s.", strna(d->name)); + log_error("Assertion failed on job for %s.", d->name); else if (streq(d->result, "unsupported")) - log_error("Operation on or unit type of %s not supported on this system.", strna(d->name)); + log_error("Operation on or unit type of %s not supported on this system.", d->name); else if (streq(d->result, "collected")) - log_error("Queued job for %s was garbage collected.", strna(d->name)); + log_error("Queued job for %s was garbage collected.", d->name); else if (streq(d->result, "once")) - log_error("Unit %s was started already once and can't be started again.", strna(d->name)); - else if (!STR_IN_SET(d->result, "done", "skipped")) { + log_error("Unit %s was started already once and can't be started again.", d->name); + else if (endswith(d->name, ".service")) { + /* Job result is unknown. For services, let's also try Result property. */ + _cleanup_free_ char *result = NULL; - if (d->name && endswith(d->name, ".service")) { - _cleanup_free_ char *result = NULL; - int q; + r = bus_job_get_service_result(d, &result); + if (r < 0) + log_debug_errno(r, "Failed to get Result property of unit %s, ignoring: %m", + d->name); - q = bus_job_get_service_result(d, &result); - if (q < 0) - log_debug_errno(q, "Failed to get Result property of unit %s: %m", d->name); - - log_job_error_with_service_result(d->name, result, extra_args); - } else - log_error("Job failed. See \"journalctl -xe\" for details."); - } + log_job_error_with_service_result(d->name, result, extra_args); + } else /* Otherwise we just show a generic message. */ + log_error("Job failed. See \"journalctl -xe\" for details."); } if (STR_IN_SET(d->result, "canceled", "collected")) @@ -279,18 +276,10 @@ static int check_wait_response(BusWaitForJobs *d, WaitJobsFlags flags, const cha return -EOPNOTSUPP; else if (streq(d->result, "once")) return -ESTALE; - else if (streq(d->result, "done")) { - if (FLAGS_SET(flags, BUS_WAIT_JOBS_LOG_SUCCESS)) - log_info("Job for %s finished.", strna(d->name)); - return 0; - } else if (streq(d->result, "skipped")) { - if (FLAGS_SET(flags, BUS_WAIT_JOBS_LOG_SUCCESS)) - log_info("Job for %s was skipped.", strna(d->name)); - return 0; - } - return log_debug_errno(SYNTHETIC_ERRNO(EIO), - "Unexpected job result, assuming server side newer than us: %s", d->result); + return log_debug_errno(SYNTHETIC_ERRNO(ENOMEDIUM), + "Unexpected job result '%s' for unit '%s', assuming server side newer than us.", + d->result, d->name); } int bus_wait_for_jobs(BusWaitForJobs *d, WaitJobsFlags flags, const char* const* extra_args) { @@ -307,13 +296,11 @@ int bus_wait_for_jobs(BusWaitForJobs *d, WaitJobsFlags flags, const char* const* if (d->name && d->result) { q = check_wait_response(d, flags, extra_args); - /* Return the first error as it is most likely to be - * meaningful. */ - if (q < 0 && r == 0) - r = q; + /* Return the first error as it is most likely to be meaningful. */ + RET_GATHER(r, q); log_full_errno_zerook(LOG_DEBUG, q, - "Got result %s/%m for job %s", d->result, d->name); + "Got result %s/%m for job %s.", d->result, d->name); } d->name = mfree(d->name); diff --git a/src/shared/bus-wait-for-jobs.h b/src/shared/bus-wait-for-jobs.h index 4d56c079ab..2336b1332a 100644 --- a/src/shared/bus-wait-for-jobs.h +++ b/src/shared/bus-wait-for-jobs.h @@ -5,17 +5,17 @@ #include "macro.h" +typedef struct BusWaitForJobs BusWaitForJobs; + typedef enum WaitJobsFlags { BUS_WAIT_JOBS_LOG_ERROR = 1 << 0, BUS_WAIT_JOBS_LOG_SUCCESS = 1 << 1, } WaitJobsFlags; -typedef struct BusWaitForJobs BusWaitForJobs; +BusWaitForJobs* bus_wait_for_jobs_free(BusWaitForJobs *d); +DEFINE_TRIVIAL_CLEANUP_FUNC(BusWaitForJobs*, bus_wait_for_jobs_free); int bus_wait_for_jobs_new(sd_bus *bus, BusWaitForJobs **ret); -BusWaitForJobs* bus_wait_for_jobs_free(BusWaitForJobs *d); int bus_wait_for_jobs_add(BusWaitForJobs *d, const char *path); int bus_wait_for_jobs(BusWaitForJobs *d, WaitJobsFlags flags, const char* const* extra_args); int bus_wait_for_jobs_one(BusWaitForJobs *d, const char *path, WaitJobsFlags flags, const char* const* extra_args); - -DEFINE_TRIVIAL_CLEANUP_FUNC(BusWaitForJobs*, bus_wait_for_jobs_free); diff --git a/src/shared/bus-wait-for-units.c b/src/shared/bus-wait-for-units.c index 0dd2a295dd..49963ffe34 100644 --- a/src/shared/bus-wait-for-units.c +++ b/src/shared/bus-wait-for-units.c @@ -63,7 +63,7 @@ static WaitForItem *wait_for_item_free(WaitForItem *item) { log_debug_errno(r, "Failed to drop reference to unit %s, ignoring: %m", item->bus_path); } - assert_se(hashmap_remove(item->parent->items, item->bus_path) == item); + assert_se(hashmap_remove_value(item->parent->items, item->bus_path, item)); if (item->parent->current == item) item->parent->current = NULL; @@ -109,12 +109,12 @@ static int match_disconnected(sd_bus_message *m, void *userdata, sd_bus_error *e assert(m); - log_error("Warning! D-Bus connection terminated."); + log_warning("D-Bus connection terminated while waiting for unit."); bus_wait_for_units_clear(d); if (d->ready_callback) - d->ready_callback(d, false, d->userdata); + d->ready_callback(d, BUS_WAIT_FAILURE, d->userdata); else /* If no ready callback is specified close the connection so that the event loop exits */ sd_bus_close(sd_bus_message_get_bus(m)); diff --git a/src/shared/bus-wait-for-units.h b/src/shared/bus-wait-for-units.h index 2623e72ced..c87eabcbbc 100644 --- a/src/shared/bus-wait-for-units.h +++ b/src/shared/bus-wait-for-units.h @@ -25,11 +25,11 @@ typedef void (*bus_wait_for_units_ready_callback)(BusWaitForUnits *d, BusWaitFor typedef void (*bus_wait_for_units_unit_callback)(BusWaitForUnits *d, const char *unit_path, bool good, void *userdata); int bus_wait_for_units_new(sd_bus *bus, BusWaitForUnits **ret); + BusWaitForUnits* bus_wait_for_units_free(BusWaitForUnits *d); +DEFINE_TRIVIAL_CLEANUP_FUNC(BusWaitForUnits*, bus_wait_for_units_free); BusWaitForUnitsState bus_wait_for_units_state(BusWaitForUnits *d); void bus_wait_for_units_set_ready_callback(BusWaitForUnits *d, bus_wait_for_units_ready_callback callback, void *userdata); int bus_wait_for_units_add_unit(BusWaitForUnits *d, const char *unit, BusWaitForUnitsFlags flags, bus_wait_for_units_unit_callback callback, void *userdata); int bus_wait_for_units_run(BusWaitForUnits *d); - -DEFINE_TRIVIAL_CLEANUP_FUNC(BusWaitForUnits*, bus_wait_for_units_free);