core: introduce a new load state "bad-setting"

Since bb28e68477 parsing failures of
certain unit file settings will result in load failures of units. This
introduces a new load state "bad-setting" that is entered in precisely
this case.

With this addition error messages on bad settings should be a lot more
explicit, as we don't have to show some generic "errno" error in that
case, but can explicitly say that a bad setting is at fault.

Internally this unit load state is entered as soon as any configuration
loader call returns ENOEXEC. Hence: config parser calls should return
ENOEXEC now for such essential unit file settings. Turns out, they
generally already do.

Fixes: #9107
This commit is contained in:
Lennart Poettering 2018-06-01 17:46:01 +02:00
parent 443dee9d2e
commit c4555ad8f6
9 changed files with 38 additions and 22 deletions

View file

@ -702,13 +702,14 @@ To show all installed unit files use 'systemctl list-unit-files'.
were masked, not found, or otherwise failed.</para>
<para>The LOAD column shows the load state, one of <constant>loaded</constant>,
<constant>not-found</constant>, <constant>error</constant>, <constant>masked</constant>. The ACTIVE columns
shows the general unit state, one of <constant>active</constant>, <constant>reloading</constant>,
<constant>inactive</constant>, <constant>failed</constant>, <constant>activating</constant>,
<constant>deactivating</constant>. The SUB column shows the unit-type-specific detailed state of the unit,
possible values vary by unit type. The list of possible LOAD, ACTIVE, and SUB states is not constant and
new systemd releases may both add and remove values. <programlisting>systemctl --state=help</programlisting> command maybe be
used to display the current set of possible values.</para>
<constant>not-found</constant>, <constant>bad-setting</constant>, <constant>error</constant>,
<constant>masked</constant>. The ACTIVE columns shows the general unit state, one of
<constant>active</constant>, <constant>reloading</constant>, <constant>inactive</constant>,
<constant>failed</constant>, <constant>activating</constant>, <constant>deactivating</constant>. The SUB
column shows the unit-type-specific detailed state of the unit, possible values vary by unit type. The list
of possible LOAD, ACTIVE, and SUB states is not constant and new systemd releases may both add and remove
values. <programlisting>systemctl --state=help</programlisting> command maybe be used to display the
current set of possible values.</para>
<para>This is the default command.</para>
</listitem>
@ -970,10 +971,12 @@ Jan 12 10:46:45 example.com bluetoothd[8900]: gatt-time-server: Input/output err
<para>The "Loaded:" line in the output will show <literal>loaded</literal> if the unit has been loaded into
memory. Other possible values for "Loaded:" include: <literal>error</literal> if there was a problem
loading it, <literal>not-found</literal>, and <literal>masked</literal>. Along with showing the path to
the unit file, this line will also show the enablement state. Enabled commands start at boot. See the
full table of possible enablement states — including the definition of <literal>masked</literal> — in the
documentation for the <command>is-enabled</command> command.
loading it, <literal>not-found</literal> if not unit file was found for this unit,
<literal>bad-setting</literal> if an essential unit file setting could not be parsed and
<literal>masked</literal> if the unit file has been masked. Along with showing the path to the unit file,
this line will also show the enablement state. Enabled commands start at boot. See the full table of
possible enablement states — including the definition of <literal>masked</literal> — in the documentation
for the <command>is-enabled</command> command.
</para>
<para>The "Active:" line shows active state. The value is usually <literal>active</literal> or

View file

@ -93,6 +93,7 @@ static const char* const unit_load_state_table[_UNIT_LOAD_STATE_MAX] = {
[UNIT_STUB] = "stub",
[UNIT_LOADED] = "loaded",
[UNIT_NOT_FOUND] = "not-found",
[UNIT_BAD_SETTING] = "bad-setting",
[UNIT_ERROR] = "error",
[UNIT_MERGED] = "merged",
[UNIT_MASKED] = "masked"

View file

@ -30,8 +30,9 @@ typedef enum UnitType {
typedef enum UnitLoadState {
UNIT_STUB = 0,
UNIT_LOADED,
UNIT_NOT_FOUND,
UNIT_ERROR,
UNIT_NOT_FOUND, /* error condition #1: unit file not found */
UNIT_BAD_SETTING, /* error condition #2: we couldn't parse some essential unit file setting */
UNIT_ERROR, /* error condition #3: other "system" error, catchall for the rest */
UNIT_MERGED,
UNIT_MASKED,
_UNIT_LOAD_STATE_MAX,

View file

@ -1242,7 +1242,7 @@ int bus_unit_queue_job(
}
if (type == JOB_STOP &&
(IN_SET(u->load_state, UNIT_NOT_FOUND, UNIT_ERROR)) &&
IN_SET(u->load_state, UNIT_NOT_FOUND, UNIT_ERROR, UNIT_BAD_SETTING) &&
unit_active_state(u) == UNIT_INACTIVE)
return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_UNIT, "Unit %s not loaded.", u->id);
@ -1727,6 +1727,9 @@ int bus_unit_validate_load_state(Unit *u, sd_bus_error *error) {
case UNIT_NOT_FOUND:
return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_UNIT, "Unit %s not found.", u->id);
case UNIT_BAD_SETTING:
return sd_bus_error_setf(error, BUS_ERROR_BAD_UNIT_SETTING, "Unit %s has a bad unit file setting.", u->id);
case UNIT_ERROR: /* Only show .load_error in UNIT_ERROR state */
return sd_bus_error_set_errnof(error, u->load_error, "Unit %s failed to loaded properly: %m.", u->id);

View file

@ -906,7 +906,9 @@ int transaction_add_job_and_dependencies(
/* by ? by->unit->id : "NA", */
/* by ? job_type_to_string(by->type) : "NA"); */
if (!IN_SET(unit->load_state, UNIT_LOADED, UNIT_ERROR, UNIT_NOT_FOUND, UNIT_MASKED))
/* Safety check that the unit is a valid state, i.e. not in UNIT_STUB or UNIT_MERGED which should only be set
* temporarily. */
if (!IN_SET(unit->load_state, UNIT_LOADED, UNIT_ERROR, UNIT_NOT_FOUND, UNIT_BAD_SETTING, UNIT_MASKED))
return sd_bus_error_setf(e, BUS_ERROR_LOAD_FAILED, "Unit %s is not loaded properly.", unit->id);
if (type != JOB_STOP) {

View file

@ -1528,14 +1528,18 @@ int unit_load(Unit *u) {
return 0;
fail:
u->load_state = u->load_state == UNIT_STUB ? UNIT_NOT_FOUND : UNIT_ERROR;
/* We convert ENOEXEC errors to the UNIT_BAD_SETTING load state here. Configuration parsing code should hence
* return ENOEXEC to ensure units are placed in this state after loading */
u->load_state = u->load_state == UNIT_STUB ? UNIT_NOT_FOUND :
r == -ENOEXEC ? UNIT_BAD_SETTING :
UNIT_ERROR;
u->load_error = r;
unit_add_to_dbus_queue(u);
unit_add_to_gc_queue(u);
log_unit_debug_errno(u, r, "Failed to load configuration: %m");
return r;
return log_unit_debug_errno(u, r, "Failed to load configuration: %m");
}
static bool unit_condition_test_list(Unit *u, Condition *first, const char *(*to_string)(ConditionType t)) {

View file

@ -18,6 +18,7 @@ BUS_ERROR_MAP_ELF_REGISTER const sd_bus_error_map bus_common_errors[] = {
SD_BUS_ERROR_MAP(BUS_ERROR_NO_UNIT_FOR_INVOCATION_ID, ENOENT),
SD_BUS_ERROR_MAP(BUS_ERROR_UNIT_EXISTS, EEXIST),
SD_BUS_ERROR_MAP(BUS_ERROR_LOAD_FAILED, EIO),
SD_BUS_ERROR_MAP(BUS_ERROR_BAD_UNIT_SETTING, ENOEXEC),
SD_BUS_ERROR_MAP(BUS_ERROR_JOB_FAILED, EREMOTEIO),
SD_BUS_ERROR_MAP(BUS_ERROR_NO_SUCH_JOB, ENOENT),
SD_BUS_ERROR_MAP(BUS_ERROR_NOT_SUBSCRIBED, EINVAL),

View file

@ -14,6 +14,7 @@
#define BUS_ERROR_NO_UNIT_FOR_INVOCATION_ID "org.freedesktop.systemd1.NoUnitForInvocationID"
#define BUS_ERROR_UNIT_EXISTS "org.freedesktop.systemd1.UnitExists"
#define BUS_ERROR_LOAD_FAILED "org.freedesktop.systemd1.LoadFailed"
#define BUS_ERROR_BAD_UNIT_SETTING "org.freedesktop.systemd1.BadUnitSetting"
#define BUS_ERROR_JOB_FAILED "org.freedesktop.systemd1.JobFailed"
#define BUS_ERROR_NO_SUCH_JOB "org.freedesktop.systemd1.NoSuchJob"
#define BUS_ERROR_NOT_SUBSCRIBED "org.freedesktop.systemd1.NotSubscribed"

View file

@ -416,7 +416,7 @@ static int output_units_list(const UnitInfo *unit_infos, unsigned c) {
if (!arg_no_legend &&
(streq(u->active_state, "failed") ||
STR_IN_SET(u->load_state, "error", "not-found", "masked")))
STR_IN_SET(u->load_state, "error", "not-found", "bad-setting", "masked")))
circle_len = 2;
}
@ -493,7 +493,7 @@ static int output_units_list(const UnitInfo *unit_infos, unsigned c) {
underline = true;
}
if (STR_IN_SET(u->load_state, "error", "not-found", "masked") && !arg_plain) {
if (STR_IN_SET(u->load_state, "error", "not-found", "bad-setting", "masked") && !arg_plain) {
on_circle = ansi_highlight_yellow();
off_circle = ansi_normal();
circle = true;
@ -3978,7 +3978,7 @@ static void print_status_info(
if (i->following)
printf(" Follow: unit currently follows state of %s\n", i->following);
if (streq_ptr(i->load_state, "error")) {
if (STRPTR_IN_SET(i->load_state, "error", "not-found", "bad-setting")) {
on = ansi_highlight_red();
off = ansi_normal();
} else