Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

core/service: store BUSERROR= & VARLINKERROR= received and show them through systemctl status #33430

Merged
merged 5 commits into from
Jun 20, 2024
+182 −49
Merged

core/service: store BUSERROR= & VARLINKERROR= received and show them through systemctl status #33430

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d319cbe
sd-bus/bus-error: inline iterator var, use assert_return
YHNdnzj May 9, 2024
9c02502
core/service: store BUSERROR= & VARLINKERROR= received through notifi…
YHNdnzj May 9, 2024
f488e97
systemctl-show: show Status{Bus,Varlink}Error in status
YHNdnzj May 9, 2024
8ea9186
busctl: send BUSERROR= to caller via sd_notify() protocol
poettering Jun 20, 2024
4b89c55
TEST-80-NOTIFYACCESS: introduce test for ERRNO= + BUS/VARLINKERROR=
YHNdnzj Jun 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions TODO
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
20 changes: 13 additions & 7 deletions man/org.freedesktop.systemd1.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2745,6 +2745,8 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice {
readonly s FileDescriptorStorePreserve = '...';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw, downstreams asked us to always make changes to TODO in separate commits because they often backport patches and the TODO stuff is a major source of conflicts because it changes so often, but not what they care about for backports.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, TODO is mostly about new features, so I don't quite understand why they would backport those commits in the first place... I prefer not to split such changes out if this is only about backporting.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, people backport stuff for many reasons. i personally would be a lot more conservative, but then again i am not a package maintainer.

readonly s StatusText = '...';
readonly i StatusErrno = ...;
readonly s StatusBusError = '...';
readonly s StatusVarlinkError = '...';
readonly s Result = '...';
readonly s ReloadResult = '...';
readonly s CleanResult = '...';
Expand Down Expand Up @@ -3404,8 +3406,6 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice {

<!--property FileDescriptorStorePreserve is not documented!-->

<!--property StatusErrno is not documented!-->

<!--property ReloadResult is not documented!-->

<!--property CleanResult is not documented!-->
Expand Down Expand Up @@ -4026,6 +4026,10 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice {

<variablelist class="dbus-property" generated="True" extra-ref="StatusErrno"/>

<variablelist class="dbus-property" generated="True" extra-ref="StatusBusError"/>

<variablelist class="dbus-property" generated="True" extra-ref="StatusVarlinkError"/>

<variablelist class="dbus-property" generated="True" extra-ref="Result"/>

<variablelist class="dbus-property" generated="True" extra-ref="ReloadResult"/>
Expand Down Expand Up @@ -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.</para>

<para><varname>StatusText</varname> contains the status text passed to the service manager via a call
to
<citerefentry><refentrytitle>sd_notify</refentrytitle><manvolnum>3</manvolnum></citerefentry>.
This may be used by services to inform the service manager about its internal state with a nice
explanatory string.</para>
<para><varname>StatusText</varname>, <varname>StatusErrno</varname>, <varname>StatusBusError</varname>,
and <varname>StatusVarlinkError</varname> contain the status text, the error number,
and the D-Bus/Varlink error name passed to the service manager via
<citerefentry><refentrytitle>sd_notify</refentrytitle><manvolnum>3</manvolnum></citerefentry>,
respectively. They may be used by services to inform the service manager about its internal state.</para>

<para><varname>Result</varname> 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 <literal>failed</literal> state (see
Expand Down Expand Up @@ -12221,6 +12225,8 @@ $ gdbus introspect --system --dest org.freedesktop.systemd1 \
<varname>EffectiveMemoryMax</varname>,
<varname>EffectiveTasksMax</varname>, and
<varname>MemoryZSwapWriteback</varname> were added in version 256.</para>
<para><varname>StatusBusError</varname>
and <varname>StatusVarlinkError</varname> were added in version 257.</para>
</refsect2>
<refsect2>
<title>Job Objects</title>
Expand Down
12 changes: 10 additions & 2 deletions man/sd_notify.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,12 +258,20 @@
<term>BUSERROR=…</term>

<listitem><para>If a service fails, the D-Bus error-style error code. Example:
<literal>BUSERROR=org.freedesktop.DBus.Error.TimedOut</literal>. Note that this assignment is
currently not used by <command>systemd</command>.</para>
<literal>BUSERROR=org.freedesktop.DBus.Error.TimedOut</literal>.</para>

<xi:include href="version-info.xml" xpointer="v233"/></listitem>
</varlistentry>

<varlistentry>
<term>VARLINKERROR=…</term>

<listitem><para>If a service fails, the Varlink error-style error code. Example:
<literal>VARLINKERROR=org.varlink.service.InvalidParameter</literal>.</para>

<xi:include href="version-info.xml" xpointer="v257"/></listitem>
</varlistentry>

<varlistentry>
<term>EXIT_STATUS=…</term>

Expand Down
33 changes: 27 additions & 6 deletions src/busctl/busctl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
}
Expand Down
2 changes: 2 additions & 0 deletions src/core/dbus-service.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
51 changes: 49 additions & 2 deletions src/core/service.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -4479,7 +4501,7 @@ static void service_notify_message(
else {
t = strdup(e);
if (!t)
log_oom();
log_oom_warning();
}
}

Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/core/service.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
35 changes: 15 additions & 20 deletions src/libsystemd/sd-bus/bus-error.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;

Expand Down
Loading
Loading