diff --git a/TODO b/TODO index 534463cd73..92169dea0b 100644 --- a/TODO +++ b/TODO @@ -478,10 +478,6 @@ Features: - kernel-install - systemd-mount (with PK so that desktop environments could use it to mount disks) -* in the service manager, pick up ERRNO= + BUSERROR= + VARLINKERROR= error - identifiers, and store them along with the exit status of a server and report - via "systemctl status". - * enumerate virtiofs devices during boot-up in a generator, and synthesize mounts for rootfs, /usr/, /home/, /srv/ and some others from it, depending on the "tag". (waits for: https://gitlab.com/virtio-fs/virtiofsd/-/issues/128) diff --git a/man/org.freedesktop.systemd1.xml b/man/org.freedesktop.systemd1.xml index b0b45097e3..7ab560a654 100644 --- a/man/org.freedesktop.systemd1.xml +++ b/man/org.freedesktop.systemd1.xml @@ -2745,6 +2745,8 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { readonly s FileDescriptorStorePreserve = '...'; readonly s StatusText = '...'; readonly i StatusErrno = ...; + readonly s StatusBusError = '...'; + readonly s StatusVarlinkError = '...'; readonly s Result = '...'; readonly s ReloadResult = '...'; readonly s CleanResult = '...'; @@ -3404,8 +3406,6 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { - - @@ -4026,6 +4026,10 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { + + + + @@ -4732,11 +4736,11 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { process is currently running while the latter possible contains information collected from the last run even if the process is no longer around. - StatusText contains the status text passed to the service manager via a call - to - sd_notify3. - This may be used by services to inform the service manager about its internal state with a nice - explanatory string. + StatusText, StatusErrno, StatusBusError, + and StatusVarlinkError contain the status text, the error number, + and the D-Bus/Varlink error name passed to the service manager via + sd_notify3, + respectively. They may be used by services to inform the service manager about its internal state. Result encodes the execution result of the last run of the service. It is useful to determine the reason a service failed if it is in the failed state (see @@ -12221,6 +12225,8 @@ $ gdbus introspect --system --dest org.freedesktop.systemd1 \ EffectiveMemoryMax, EffectiveTasksMax, and MemoryZSwapWriteback were added in version 256. + StatusBusError + and StatusVarlinkError were added in version 257. Job Objects diff --git a/man/sd_notify.xml b/man/sd_notify.xml index 35c450b128..f04251bd19 100644 --- a/man/sd_notify.xml +++ b/man/sd_notify.xml @@ -258,12 +258,20 @@ BUSERROR=… If a service fails, the D-Bus error-style error code. Example: - BUSERROR=org.freedesktop.DBus.Error.TimedOut. Note that this assignment is - currently not used by systemd. + BUSERROR=org.freedesktop.DBus.Error.TimedOut. + + VARLINKERROR=… + + If a service fails, the Varlink error-style error code. Example: + VARLINKERROR=org.varlink.service.InvalidParameter. + + + + EXIT_STATUS=… diff --git a/src/busctl/busctl.c b/src/busctl/busctl.c index 5061afe6c9..4a5fac7606 100644 --- a/src/busctl/busctl.c +++ b/src/busctl/busctl.c @@ -159,6 +159,14 @@ static int acquire_bus(bool set_monitor, sd_bus **ret) { return 0; } +static void notify_bus_error(const sd_bus_error *error) { + + if (!sd_bus_error_is_set(error)) + return; + + (void) sd_notifyf(/* unset_environment= */ false, "BUSERROR=%s", error->name); +} + static int list_bus_names(int argc, char **argv, void *userdata) { _cleanup_strv_free_ char **acquired = NULL, **activatable = NULL; _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; @@ -459,6 +467,7 @@ static int find_nodes(sd_bus *bus, const char *service, const char *path, Set *p "org.freedesktop.DBus.Introspectable", "Introspect", &error, &reply, NULL); if (r < 0) { + notify_bus_error(&error); printf("%sFailed to introspect object %s of service %s: %s%s\n", ansi_highlight_red(), path, service, bus_error_message(&error, r), @@ -996,9 +1005,11 @@ static int introspect(int argc, char **argv, void *userdata) { r = sd_bus_call_method(bus, argv[1], argv[2], "org.freedesktop.DBus.Introspectable", "Introspect", &error, &reply_xml, NULL); - if (r < 0) + if (r < 0) { + notify_bus_error(&error); return log_error_errno(r, "Failed to introspect object %s of service %s: %s", argv[2], argv[1], bus_error_message(&error, r)); + } r = sd_bus_message_read(reply_xml, "s", &xml); if (r < 0) @@ -1032,9 +1043,11 @@ static int introspect(int argc, char **argv, void *userdata) { r = sd_bus_call_method(bus, argv[1], argv[2], "org.freedesktop.DBus.Properties", "GetAll", &error, &reply, "s", m->interface); - if (r < 0) + if (r < 0) { + notify_bus_error(&error); return log_error_errno(r, "Failed to get all properties on interface %s: %s", m->interface, bus_error_message(&error, r)); + } r = sd_bus_message_enter_container(reply, 'a', "{sv}"); if (r < 0) @@ -1305,9 +1318,11 @@ static int monitor(int argc, char **argv, int (*dump)(sd_bus_message *m, FILE *f return bus_log_create_error(r); r = sd_bus_call(bus, message, arg_timeout, &error, NULL); - if (r < 0) + if (r < 0) { + notify_bus_error(&error); return log_error_errno(r, "Call to org.freedesktop.DBus.Monitoring.BecomeMonitor failed: %s", bus_error_message(&error, r)); + } r = sd_bus_get_unique_name(bus, &unique_name); if (r < 0) @@ -2076,8 +2091,10 @@ static int call(int argc, char **argv, void *userdata) { } r = sd_bus_call(bus, m, arg_timeout, &error, &reply); - if (r < 0) + if (r < 0) { + notify_bus_error(&error); return log_error_errno(r, "Call failed: %s", bus_error_message(&error, r)); + } r = sd_bus_message_is_empty(reply); if (r < 0) @@ -2180,10 +2197,12 @@ static int get_property(int argc, char **argv, void *userdata) { r = sd_bus_call_method(bus, argv[1], argv[2], "org.freedesktop.DBus.Properties", "Get", &error, &reply, "ss", argv[3], *i); - if (r < 0) + if (r < 0) { + notify_bus_error(&error); return log_error_errno(r, "Failed to get property %s on interface %s: %s", *i, argv[3], bus_error_message(&error, r)); + } r = sd_bus_message_peek_type(reply, &type, &contents); if (r < 0) @@ -2267,10 +2286,12 @@ static int set_property(int argc, char **argv, void *userdata) { return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Too many parameters for signature."); r = sd_bus_call(bus, m, arg_timeout, &error, NULL); - if (r < 0) + if (r < 0) { + notify_bus_error(&error); return log_error_errno(r, "Failed to set property %s on interface %s: %s", argv[4], argv[3], bus_error_message(&error, r)); + } return 0; } diff --git a/src/core/dbus-service.c b/src/core/dbus-service.c index ff970df957..cf7e2fe0eb 100644 --- a/src/core/dbus-service.c +++ b/src/core/dbus-service.c @@ -351,6 +351,8 @@ const sd_bus_vtable bus_service_vtable[] = { SD_BUS_PROPERTY("FileDescriptorStorePreserve", "s", bus_property_get_exec_preserve_mode, offsetof(Service, fd_store_preserve_mode), 0), SD_BUS_PROPERTY("StatusText", "s", NULL, offsetof(Service, status_text), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("StatusErrno", "i", bus_property_get_int, offsetof(Service, status_errno), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), + SD_BUS_PROPERTY("StatusBusError", "s", NULL, offsetof(Service, status_bus_error), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), + SD_BUS_PROPERTY("StatusVarlinkError", "s", NULL, offsetof(Service, status_varlink_error), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("Result", "s", property_get_result, offsetof(Service, result), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("ReloadResult", "s", property_get_result, offsetof(Service, reload_result), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("CleanResult", "s", property_get_result, offsetof(Service, clean_result), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), diff --git a/src/core/service.c b/src/core/service.c index f37a941a6d..937729cd06 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -468,6 +468,8 @@ static void service_done(Unit *u) { s->pid_file = mfree(s->pid_file); s->status_text = mfree(s->status_text); + s->status_bus_error = mfree(s->status_bus_error); + s->status_varlink_error = mfree(s->status_varlink_error); s->exec_runtime = exec_runtime_free(s->exec_runtime); @@ -1045,6 +1047,14 @@ static void service_dump(Unit *u, FILE *f, const char *prefix) { fprintf(f, "%sStatus Errno: %s\n", prefix, STRERROR(s->status_errno)); + if (s->status_bus_error) + fprintf(f, "%sStatus Bus Error: %s\n", + prefix, s->status_bus_error); + + if (s->status_varlink_error) + fprintf(f, "%sStatus Varlink Error: %s\n", + prefix, s->status_varlink_error); + if (s->n_fd_store_max > 0) fprintf(f, "%sFile Descriptor Store Max: %u\n" @@ -2765,6 +2775,8 @@ static int service_start(Unit *u) { s->status_text = mfree(s->status_text); s->status_errno = 0; + s->status_bus_error = mfree(s->status_bus_error); + s->status_varlink_error = mfree(s->status_varlink_error); s->notify_access_override = _NOTIFY_ACCESS_INVALID; s->notify_state = NOTIFY_UNKNOWN; @@ -3036,6 +3048,8 @@ static int service_serialize(Unit *u, FILE *f, FDSet *fds) { return r; (void) serialize_item_format(f, "status-errno", "%d", s->status_errno); + (void) serialize_item(f, "status-bus-error", s->status_bus_error); + (void) serialize_item(f, "status-varlink-error", s->status_varlink_error); (void) serialize_dual_timestamp(f, "watchdog-timestamp", &s->watchdog_timestamp); @@ -3370,6 +3384,14 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value, else s->status_errno = i; + } else if (streq(key, "status-bus-error")) { + if (free_and_strdup(&s->status_bus_error, value) < 0) + log_oom_debug(); + + } else if (streq(key, "status-varlink-error")) { + if (free_and_strdup(&s->status_varlink_error, value) < 0) + log_oom_debug(); + } else if (streq(key, "watchdog-timestamp")) (void) deserialize_dual_timestamp(value, &s->watchdog_timestamp); else if (streq(key, "watchdog-original-usec")) @@ -4353,7 +4375,7 @@ static void service_notify_message( if (DEBUG_LOGGING) { _cleanup_free_ char *cc = strv_join(tags, ", "); - log_unit_debug(u, "Got notification message from PID "PID_FMT" (%s)", ucred->pid, empty_to_na(cc)); + log_unit_debug(u, "Got notification message from PID "PID_FMT": %s", ucred->pid, empty_to_na(cc)); } usec_t monotonic_usec = USEC_INFINITY; @@ -4479,7 +4501,7 @@ static void service_notify_message( else { t = strdup(e); if (!t) - log_oom(); + log_oom_warning(); } } @@ -4523,10 +4545,35 @@ static void service_notify_message( } } + static const struct { + const char *tag; + size_t status_offset; + } status_errors[] = { + { "BUSERROR=", offsetof(Service, status_bus_error) }, + { "VARLINKERROR=", offsetof(Service, status_varlink_error) }, + }; + + FOREACH_ELEMENT(i, status_errors) { + e = strv_find_startswith(tags, i->tag); + if (!e) + continue; + + char **status_error = (char**) ((uint8_t*) s + i->status_offset); + + e = empty_to_null(e); + + if (e && !string_is_safe_ascii(e)) { + _cleanup_free_ char *escaped = cescape(e); + log_unit_warning(u, "Got invalid %s string, ignoring: %s", i->tag, strna(escaped)); + } else if (free_and_strdup_warn(status_error, e) > 0) + notify_dbus = true; + } + /* Interpret EXTEND_TIMEOUT= */ e = strv_find_startswith(tags, "EXTEND_TIMEOUT_USEC="); if (e) { usec_t extend_timeout_usec; + if (safe_atou64(e, &extend_timeout_usec) < 0) log_unit_warning(u, "Failed to parse EXTEND_TIMEOUT_USEC=%s", e); else diff --git a/src/core/service.h b/src/core/service.h index 55ea413f40..1d67d13fda 100644 --- a/src/core/service.h +++ b/src/core/service.h @@ -198,6 +198,8 @@ struct Service { char *bus_name; char *status_text; + char *status_bus_error; + char *status_varlink_error; int status_errno; sd_event_source *timer_event_source; diff --git a/src/libsystemd/sd-bus/bus-error.c b/src/libsystemd/sd-bus/bus-error.c index f415797700..a5146fd6e6 100644 --- a/src/libsystemd/sd-bus/bus-error.c +++ b/src/libsystemd/sd-bus/bus-error.c @@ -62,12 +62,10 @@ extern const sd_bus_error_map __stop_SYSTEMD_BUS_ERROR_MAP[]; static const sd_bus_error_map **additional_error_maps = NULL; static int bus_error_name_to_errno(const char *name) { - const sd_bus_error_map **map, *m; const char *p; int r; - if (!name) - return EINVAL; + assert_return(name, EINVAL); p = startswith(name, "System.Error."); if (p) { @@ -79,8 +77,8 @@ static int bus_error_name_to_errno(const char *name) { } if (additional_error_maps) - for (map = additional_error_maps; *map; map++) - for (m = *map;; m++) { + for (const sd_bus_error_map **map = additional_error_maps; *map; map++) + for (const sd_bus_error_map *m = *map;; m++) { /* For additional error maps the end marker is actually the end marker */ if (m->code == BUS_ERROR_MAP_END_MARKER) break; @@ -91,25 +89,22 @@ static int bus_error_name_to_errno(const char *name) { } } - m = ALIGN_PTR(__start_SYSTEMD_BUS_ERROR_MAP); - while (m < __stop_SYSTEMD_BUS_ERROR_MAP) { - /* For magic ELF error maps, the end marker might - * appear in the middle of things, since multiple maps - * might appear in the same section. Hence, let's skip - * over it, but realign the pointer to the next 8 byte - * boundary, which is the selected alignment for the - * arrays. */ - if (m->code == BUS_ERROR_MAP_END_MARKER) { - m = ALIGN_PTR(m + 1); + const sd_bus_error_map *elf_map = ALIGN_PTR(__start_SYSTEMD_BUS_ERROR_MAP); + while (elf_map < __stop_SYSTEMD_BUS_ERROR_MAP) { + /* For magic ELF error maps, the end marker might appear in the middle of things, since + * multiple maps might appear in the same section. Hence, let's skip over it, but realign + * the pointer to the next 8 byte boundary, which is the selected alignment for the arrays. */ + if (elf_map->code == BUS_ERROR_MAP_END_MARKER) { + elf_map = ALIGN_PTR(elf_map + 1); continue; } - if (streq(m->name, name)) { - assert(m->code > 0); - return m->code; + if (streq(elf_map->name, name)) { + assert(elf_map->code > 0); + return elf_map->code; } - m++; + elf_map++; } return EIO; @@ -389,7 +384,7 @@ _public_ int sd_bus_error_has_names_sentinel(const sd_bus_error *e, ...) { return !!p; } -_public_ int sd_bus_error_get_errno(const sd_bus_error* e) { +_public_ int sd_bus_error_get_errno(const sd_bus_error *e) { if (!e || !e->name) return 0; diff --git a/src/systemctl/systemctl-show.c b/src/systemctl/systemctl-show.c index 2fdf321886..50f30d8565 100644 --- a/src/systemctl/systemctl-show.c +++ b/src/systemctl/systemctl-show.c @@ -202,11 +202,13 @@ typedef struct UnitStatusInfo { bool transient; /* Service */ + bool running; pid_t main_pid; pid_t control_pid; - const char *status_text; const char *pid_file; - bool running; + const char *status_text; + const char *status_bus_error; + const char *status_varlink_error; int status_errno; uint32_t fd_store_max; @@ -681,9 +683,26 @@ static void print_status_info( if (i->status_text) printf(" Status: \"%s%s%s\"\n", ansi_highlight_cyan(), i->status_text, ansi_normal()); - if (i->status_errno > 0) { - errno = i->status_errno; - printf(" Error: %i (%m)\n", i->status_errno); + + if (i->status_errno > 0 || i->status_bus_error || i->status_varlink_error) { + const char *prefix = " "; + + printf(" Error:"); + + if (i->status_errno > 0) { + printf("%scode: %i (%s)", prefix, i->status_errno, STRERROR(i->status_errno)); + prefix = "; "; + } + if (i->status_bus_error) { + printf("%sD-Bus: %s", prefix, i->status_bus_error); + prefix = "; "; + } + if (i->status_varlink_error) { + printf("%sVarlink: %s", prefix, i->status_varlink_error); + prefix = "; "; + } + + putchar('\n'); } if (i->ip_ingress_bytes != UINT64_MAX && i->ip_egress_bytes != UINT64_MAX) @@ -2041,9 +2060,11 @@ static int show_one( { "ExecMainPID", "u", NULL, offsetof(UnitStatusInfo, main_pid) }, { "MainPID", "u", map_main_pid, 0 }, { "ControlPID", "u", NULL, offsetof(UnitStatusInfo, control_pid) }, - { "StatusText", "s", NULL, offsetof(UnitStatusInfo, status_text) }, { "PIDFile", "s", NULL, offsetof(UnitStatusInfo, pid_file) }, + { "StatusText", "s", NULL, offsetof(UnitStatusInfo, status_text) }, { "StatusErrno", "i", NULL, offsetof(UnitStatusInfo, status_errno) }, + { "StatusBusError", "s", NULL, offsetof(UnitStatusInfo, status_bus_error) }, + { "StatusVarlinkError", "s", NULL, offsetof(UnitStatusInfo, status_varlink_error) }, { "FileDescriptorStoreMax", "u", NULL, offsetof(UnitStatusInfo, fd_store_max) }, { "NFileDescriptorStore", "u", NULL, offsetof(UnitStatusInfo, n_fd_store) }, { "ExecMainStartTimestamp", "t", NULL, offsetof(UnitStatusInfo, start_timestamp) }, diff --git a/test/TEST-80-NOTIFYACCESS/TEST-80-NOTIFYACCESS.units/test.sh b/test/TEST-80-NOTIFYACCESS/TEST-80-NOTIFYACCESS.units/test.sh index 565ed8d35a..47333ef0de 100755 --- a/test/TEST-80-NOTIFYACCESS/TEST-80-NOTIFYACCESS.units/test.sh +++ b/test/TEST-80-NOTIFYACCESS/TEST-80-NOTIFYACCESS.units/test.sh @@ -54,10 +54,21 @@ sync_in b echo "toplevel again: $BASHPID" -systemd-notify --ready --status="OK" +systemd-notify --ready +systemd-notify "ERRNO=1" "BUSERROR=org.freedesktop.DBus.Error.InvalidArgs" "VARLINKERROR=org.varlink.service.InvalidParameter" + +sync_out e +sync_in f + +systemd-notify "ERRNO=bogus" "BUSERROR=草wwww" "VARLINKERROR=systemköttel" + +sync_out g +sync_in h + +systemd-notify --status="OK" systemd-notify "NOTIFYACCESS=none" systemd-notify --status="BOGUS3" -sync_out e +sync_out i exec sleep infinity diff --git a/test/units/TEST-80-NOTIFYACCESS.sh b/test/units/TEST-80-NOTIFYACCESS.sh index 97b222a9e8..355f6e7e30 100755 --- a/test/units/TEST-80-NOTIFYACCESS.sh +++ b/test/units/TEST-80-NOTIFYACCESS.sh @@ -20,6 +20,8 @@ sync_out() { export SYSTEMD_LOG_LEVEL=debug +# Test NotifyAccess= override through sd_notify() + systemctl --no-block start notify.service sync_in a @@ -38,6 +40,22 @@ sync_out d sync_in e systemctl --quiet is-active notify.service +[[ "$(systemctl show notify.service -P StatusText)" != BOGUS* ]] + +assert_eq "$(systemctl show notify.service -P StatusErrno)" "1" +assert_eq "$(systemctl show notify.service -P StatusBusError)" "org.freedesktop.DBus.Error.InvalidArgs" +assert_eq "$(systemctl show notify.service -P StatusVarlinkError)" "org.varlink.service.InvalidParameter" + +sync_out f +sync_in g + +assert_eq "$(systemctl show notify.service -P StatusErrno)" "1" +assert_eq "$(systemctl show notify.service -P StatusBusError)" "org.freedesktop.DBus.Error.InvalidArgs" +assert_eq "$(systemctl show notify.service -P StatusVarlinkError)" "org.varlink.service.InvalidParameter" + +sync_out h +sync_in i + assert_eq "$(systemctl show notify.service -p StatusText --value)" "OK" assert_eq "$(systemctl show notify.service -p NotifyAccess --value)" "none" @@ -46,6 +64,12 @@ assert_eq "$(systemctl show notify.service -p NotifyAccess --value)" "all" rm /tmp/syncfifo1 /tmp/syncfifo2 +# Explicitly test busctl's BUSERROR= reporting and systemctl status should show it + +(! systemd-run --wait --unit="TEST-80-BUSERROR.service" -p NotifyAccess=main busctl introspect org.freedesktop.systemd1 /bogus/001) +assert_eq "$(systemctl show TEST-80-BUSERROR.service -P StatusBusError)" "org.freedesktop.DBus.Error.UnknownObject" +assert_in "D-Bus: org.freedesktop.DBus.Error.UnknownObject" "$(systemctl status TEST-80-BUSERROR.service)" + # Now test basic fdstore behaviour MYSCRIPT="/tmp/myscript$RANDOM.sh"