Merge pull request #9157 from poettering/unit-config-load-error

introduce a new "bad-setting" unit load state in order to improve "systemctl status" output when bad settings are used

21 files changed, 93 insertions, 74 deletions

diff --git a/src/basic/unit-def.c b/src/basic/unit-def.c
index 7361acb297..fd42e0a241 100644
--- a/src/basic/unit-def.c
+++ b/src/basic/unit-def.c
@@ -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"
diff --git a/src/basic/unit-def.h b/src/basic/unit-def.h
index 503867668b..3f13a1c8f3 100644
--- a/src/basic/unit-def.h
+++ b/src/basic/unit-def.h
@@ -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,
diff --git a/src/core/automount.c b/src/core/automount.c
index 0aba87b778..3251e8a5a5 100644
--- a/src/core/automount.c
+++ b/src/core/automount.c
@@ -172,7 +172,7 @@ static int automount_verify(Automount *a) {
 
         if (path_equal(a->where, "/")) {
                 log_unit_error(UNIT(a), "Cannot have an automount unit for the root directory. Refusing.");
-                return -EINVAL;
+                return -ENOEXEC;
         }
 
         r = unit_name_from_path(a->where, ".automount", &e);
@@ -181,7 +181,7 @@ static int automount_verify(Automount *a) {
 
         if (!unit_has_name(UNIT(a), e)) {
                 log_unit_error(UNIT(a), "Where= setting doesn't match unit name. Refusing.");
-                return -EINVAL;
+                return -ENOEXEC;
         }
 
         return 0;
diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index f7535e38fc..09daf268b7 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -610,7 +610,7 @@ static int method_set_unit_properties(sd_bus_message *message, void *userdata, s
         if (r < 0)
                 return r;
 
-        r = bus_unit_check_load_state(u, error);
+        r = bus_unit_validate_load_state(u, error);
         if (r < 0)
                 return r;
 
@@ -634,7 +634,7 @@ static int method_ref_unit(sd_bus_message *message, void *userdata, sd_bus_error
         if (r < 0)
                 return r;
 
-        r = bus_unit_check_load_state(u, error);
+        r = bus_unit_validate_load_state(u, error);
         if (r < 0)
                 return r;
 
@@ -658,7 +658,7 @@ static int method_unref_unit(sd_bus_message *message, void *userdata, sd_bus_err
         if (r < 0)
                 return r;
 
-        r = bus_unit_check_load_state(u, error);
+        r = bus_unit_validate_load_state(u, error);
         if (r < 0)
                 return r;
 
diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c
index 150ccc024c..d0910e9dc3 100644
--- a/src/core/dbus-unit.c
+++ b/src/core/dbus-unit.c
@@ -271,16 +271,18 @@ static int property_get_load_error(
                 sd_bus_error *error) {
 
         _cleanup_(sd_bus_error_free) sd_bus_error e = SD_BUS_ERROR_NULL;
-        int *n = userdata;
+        Unit *u = userdata;
+        int r;
 
         assert(bus);
         assert(reply);
-        assert(n);
+        assert(u);
 
-        if (*n != 0)
-                sd_bus_error_set_errno(&e, *n);
+        r = bus_unit_validate_load_state(u, &e);
+        if (r < 0)
+                return sd_bus_message_append(reply, "(ss)", e.name, e.message);
 
-        return sd_bus_message_append(reply, "(ss)", e.name, e.message);
+        return sd_bus_message_append(reply, "(ss)", NULL, NULL);
 }
 
 static int bus_verify_manage_units_async_full(
@@ -629,7 +631,7 @@ const sd_bus_vtable bus_unit_vtable[] = {
         BUS_PROPERTY_DUAL_TIMESTAMP("AssertTimestamp", offsetof(Unit, assert_timestamp), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
         SD_BUS_PROPERTY("Conditions", "a(sbbsi)", property_get_conditions, offsetof(Unit, conditions), 0),
         SD_BUS_PROPERTY("Asserts", "a(sbbsi)", property_get_conditions, offsetof(Unit, asserts), 0),
-        SD_BUS_PROPERTY("LoadError", "(ss)", property_get_load_error, offsetof(Unit, load_error), SD_BUS_VTABLE_PROPERTY_CONST),
+        SD_BUS_PROPERTY("LoadError", "(ss)", property_get_load_error, 0, SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("Transient", "b", bus_property_get_bool, offsetof(Unit, transient), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("Perpetual", "b", bus_property_get_bool, offsetof(Unit, perpetual), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("StartLimitIntervalUSec", "t", bus_property_get_usec, offsetof(Unit, start_limit.interval), SD_BUS_VTABLE_PROPERTY_CONST),
@@ -1240,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);
 
@@ -1712,22 +1714,33 @@ int bus_unit_set_properties(
         return n;
 }
 
-int bus_unit_check_load_state(Unit *u, sd_bus_error *error) {
+int bus_unit_validate_load_state(Unit *u, sd_bus_error *error) {
         assert(u);
 
-        if (u->load_state == UNIT_LOADED)
-                return 0;
+        /* Generates a pretty error if a unit isn't properly loaded. */
 
-        /* Give a better description of the unit error when
-         * possible. Note that in the case of UNIT_MASKED, load_error
-         * is not set. */
-        if (u->load_state == UNIT_MASKED)
-                return sd_bus_error_setf(error, BUS_ERROR_UNIT_MASKED, "Unit %s is masked.", u->id);
+        switch (u->load_state) {
 
-        if (u->load_state == UNIT_NOT_FOUND)
+        case UNIT_LOADED:
+                return 0;
+
+        case UNIT_NOT_FOUND:
                 return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_UNIT, "Unit %s not found.", u->id);
 
-        return sd_bus_error_set_errnof(error, u->load_error, "Unit %s is not loaded properly: %m.", 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);
+
+        case UNIT_MASKED:
+                return sd_bus_error_setf(error, BUS_ERROR_UNIT_MASKED, "Unit %s is masked.", u->id);
+
+        case UNIT_STUB:
+        case UNIT_MERGED:
+        default:
+                return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_UNIT, "Unexpected load state of unit %s", u->id);
+        }
 }
 
 static int bus_unit_track_handler(sd_bus_track *t, void *userdata) {
diff --git a/src/core/dbus-unit.h b/src/core/dbus-unit.h
index 8c55d7dfdf..0c1d44511d 100644
--- a/src/core/dbus-unit.h
+++ b/src/core/dbus-unit.h
@@ -31,7 +31,7 @@ int bus_unit_method_ref(sd_bus_message *message, void *userdata, sd_bus_error *e
 int bus_unit_method_unref(sd_bus_message *message, void *userdata, sd_bus_error *error);
 
 int bus_unit_queue_job(sd_bus_message *message, Unit *u, JobType type, JobMode mode, bool reload_if_possible, sd_bus_error *error);
-int bus_unit_check_load_state(Unit *u, sd_bus_error *error);
+int bus_unit_validate_load_state(Unit *u, sd_bus_error *error);
 
 int bus_unit_track_add_name(Unit *u, const char *name);
 int bus_unit_track_add_sender(Unit *u, sd_bus_message *m);
diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index 0953355dce..fb5b742fcf 100644
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -820,7 +820,7 @@ int config_parse_exec_input(
                         resolved = mfree(resolved);
                 else if (!fdname_is_valid(resolved)) {
                         log_syntax(unit, LOG_ERR, filename, line, 0, "Invalid file descriptor name: %s", resolved);
-                        return -EINVAL;
+                        return -ENOEXEC;
                 }
 
                 free_and_replace(c->stdio_fdname[STDIN_FILENO], resolved);
@@ -836,7 +836,7 @@ int config_parse_exec_input(
 
                 r = path_simplify_and_warn(resolved, PATH_CHECK_ABSOLUTE | PATH_CHECK_FATAL, unit, filename, line, lvalue);
                 if (r < 0)
-                        return -EINVAL;
+                        return -ENOEXEC;
 
                 free_and_replace(c->stdio_file[STDIN_FILENO], resolved);
 
@@ -1001,7 +1001,7 @@ int config_parse_exec_output(
                         resolved = mfree(resolved);
                 else if (!fdname_is_valid(resolved)) {
                         log_syntax(unit, LOG_ERR, filename, line, 0, "Invalid file descriptor name: %s", resolved);
-                        return -EINVAL;
+                        return -ENOEXEC;
                 }
 
                 eo = EXEC_OUTPUT_NAMED_FD;
@@ -1014,7 +1014,7 @@ int config_parse_exec_output(
 
                 r = path_simplify_and_warn(resolved, PATH_CHECK_ABSOLUTE | PATH_CHECK_FATAL, unit, filename, line, lvalue);
                 if (r < 0)
-                        return -EINVAL;
+                        return -ENOEXEC;
 
                 eo = EXEC_OUTPUT_FILE;
 
diff --git a/src/core/manager.c b/src/core/manager.c
index 04e0c6fe3b..e1ce9229a3 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1954,16 +1954,12 @@ int manager_load_startable_unit_or_warn(
         r = manager_load_unit(m, name, path, &error, &unit);
         if (r < 0)
                 return log_error_errno(r, "Failed to load %s %s: %s",
-                                       name ? "unit" : "file", name ?: path,
+                                       name ? "unit" : "unit file", name ?: path,
                                        bus_error_message(&error, r));
-        else if (IN_SET(unit->load_state, UNIT_ERROR, UNIT_NOT_FOUND))
-                return log_error_errno(unit->load_error, "Failed to load %s %s: %m",
-                                       name ? "unit" : "file", name ?: path);
-        else if (unit->load_state == UNIT_MASKED) {
-                log_error("%s %s is masked.",
-                          name ? "Unit" : "File", name ?: path);
-                return -ERFKILL;
-        }
+
+        r = bus_unit_validate_load_state(unit, &error);
+        if (r < 0)
+                return log_error_errno(r, "%s", bus_error_message(&error, r));
 
         *ret = unit;
         return 0;
diff --git a/src/core/mount.c b/src/core/mount.c
index dcc44657b2..c588ef92d7 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -517,23 +517,23 @@ static int mount_verify(Mount *m) {
 
         if (!unit_has_name(UNIT(m), e)) {
                 log_unit_error(UNIT(m), "Where= setting doesn't match unit name. Refusing.");
-                return -EINVAL;
+                return -ENOEXEC;
         }
 
         if (mount_point_is_api(m->where) || mount_point_ignore(m->where)) {
                 log_unit_error(UNIT(m), "Cannot create mount unit for API file system %s. Refusing.", m->where);
-                return -EINVAL;
+                return -ENOEXEC;
         }
 
         p = get_mount_parameters_fragment(m);
         if (p && !p->what) {
                 log_unit_error(UNIT(m), "What= setting is missing. Refusing.");
-                return -EBADMSG;
+                return -ENOEXEC;
         }
 
         if (m->exec_context.pam_name && m->kill_context.kill_mode != KILL_CONTROL_GROUP) {
                 log_unit_error(UNIT(m), "Unit has PAM enabled. Kill mode must be set to control-group'. Refusing.");
-                return -EINVAL;
+                return -ENOEXEC;
         }
 
         return 0;
diff --git a/src/core/path.c b/src/core/path.c
index 6eacd95a16..e97da44700 100644
--- a/src/core/path.c
+++ b/src/core/path.c
@@ -289,7 +289,7 @@ static int path_verify(Path *p) {
 
         if (!p->specs) {
                 log_unit_error(UNIT(p), "Path unit lacks path setting. Refusing.");
-                return -EINVAL;
+                return -ENOEXEC;
         }
 
         return 0;
diff --git a/src/core/scope.c b/src/core/scope.c
index 27ff545313..f30d135007 100644
--- a/src/core/scope.c
+++ b/src/core/scope.c
@@ -133,7 +133,7 @@ static int scope_verify(Scope *s) {
             !MANAGER_IS_RELOADING(UNIT(s)->manager) &&
             !unit_has_name(UNIT(s), SPECIAL_INIT_SCOPE)) {
                 log_unit_error(UNIT(s), "Scope has no PIDs. Refusing.");
-                return -EINVAL;
+                return -ENOENT;
         }
 
         return 0;
diff --git a/src/core/service.c b/src/core/service.c
index 2d43779cfb..33fd36f8a7 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -543,37 +543,37 @@ static int service_verify(Service *s) {
 
         if (!s->exec_command[SERVICE_EXEC_START] && !s->exec_command[SERVICE_EXEC_STOP]) {
                 log_unit_error(UNIT(s), "Service lacks both ExecStart= and ExecStop= setting. Refusing.");
-                return -EINVAL;
+                return -ENOEXEC;
         }
 
         if (s->type != SERVICE_ONESHOT && !s->exec_command[SERVICE_EXEC_START]) {
                 log_unit_error(UNIT(s), "Service has no ExecStart= setting, which is only allowed for Type=oneshot services. Refusing.");
-                return -EINVAL;
+                return -ENOEXEC;
         }
 
         if (!s->remain_after_exit && !s->exec_command[SERVICE_EXEC_START]) {
                 log_unit_error(UNIT(s), "Service has no ExecStart= setting, which is only allowed for RemainAfterExit=yes services. Refusing.");
-                return -EINVAL;
+                return -ENOEXEC;
         }
 
         if (s->type != SERVICE_ONESHOT && s->exec_command[SERVICE_EXEC_START]->command_next) {
                 log_unit_error(UNIT(s), "Service has more than one ExecStart= setting, which is only allowed for Type=oneshot services. Refusing.");
-                return -EINVAL;
+                return -ENOEXEC;
         }
 
         if (s->type == SERVICE_ONESHOT && s->restart != SERVICE_RESTART_NO) {
                 log_unit_error(UNIT(s), "Service has Restart= setting other than no, which isn't allowed for Type=oneshot services. Refusing.");
-                return -EINVAL;
+                return -ENOEXEC;
         }
 
         if (s->type == SERVICE_ONESHOT && !exit_status_set_is_empty(&s->restart_force_status)) {
                 log_unit_error(UNIT(s), "Service has RestartForceStatus= set, which isn't allowed for Type=oneshot services. Refusing.");
-                return -EINVAL;
+                return -ENOEXEC;
         }
 
         if (s->type == SERVICE_DBUS && !s->bus_name) {
                 log_unit_error(UNIT(s), "Service is of type D-Bus but no D-Bus service name has been specified. Refusing.");
-                return -EINVAL;
+                return -ENOEXEC;
         }
 
         if (s->bus_name && s->type != SERVICE_DBUS)
@@ -581,7 +581,7 @@ static int service_verify(Service *s) {
 
         if (s->exec_context.pam_name && !IN_SET(s->kill_context.kill_mode, KILL_CONTROL_GROUP, KILL_MIXED)) {
                 log_unit_error(UNIT(s), "Service has PAM enabled. Kill mode must be set to 'control-group' or 'mixed'. Refusing.");
-                return -EINVAL;
+                return -ENOEXEC;
         }
 
         if (s->usb_function_descriptors && !s->usb_function_strings)
diff --git a/src/core/slice.c b/src/core/slice.c
index e8a7c9a585..f903d51f4a 100644
--- a/src/core/slice.c
+++ b/src/core/slice.c
@@ -97,7 +97,7 @@ static int slice_verify(Slice *s) {
 
         if (!slice_name_is_valid(UNIT(s)->id)) {
                 log_unit_error(UNIT(s), "Slice name %s is not valid. Refusing.", UNIT(s)->id);
-                return -EINVAL;
+                return -ENOEXEC;
         }
 
         r = slice_build_parent_slice(UNIT(s)->id, &parent);
@@ -106,7 +106,7 @@ static int slice_verify(Slice *s) {
 
         if (parent ? !unit_has_name(UNIT_DEREF(UNIT(s)->slice), parent) : UNIT_ISSET(UNIT(s)->slice)) {
                 log_unit_error(UNIT(s), "Located outside of parent slice. Refusing.");
-                return -EINVAL;
+                return -ENOEXEC;
         }
 
         return 0;
diff --git a/src/core/socket.c b/src/core/socket.c
index ee7428a126..0a74076f05 100644
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -441,32 +441,32 @@ static int socket_verify(Socket *s) {
 
         if (!s->ports) {
                 log_unit_error(UNIT(s), "Unit has no Listen setting (ListenStream=, ListenDatagram=, ListenFIFO=, ...). Refusing.");
-                return -EINVAL;
+                return -ENOEXEC;
         }
 
         if (s->accept && have_non_accept_socket(s)) {
                 log_unit_error(UNIT(s), "Unit configured for accepting sockets, but sockets are non-accepting. Refusing.");
-                return -EINVAL;
+                return -ENOEXEC;
         }
 
         if (s->accept && s->max_connections <= 0) {
                 log_unit_error(UNIT(s), "MaxConnection= setting too small. Refusing.");
-                return -EINVAL;
+                return -ENOEXEC;
         }
 
         if (s->accept && UNIT_DEREF(s->service)) {
                 log_unit_error(UNIT(s), "Explicit service configuration for accepting socket units not supported. Refusing.");
-                return -EINVAL;
+                return -ENOEXEC;
         }
 
         if (s->exec_context.pam_name && s->kill_context.kill_mode != KILL_CONTROL_GROUP) {
                 log_unit_error(UNIT(s), "Unit has PAM enabled. Kill mode must be set to 'control-group'. Refusing.");
-                return -EINVAL;
+                return -ENOEXEC;
         }
 
         if (!strv_isempty(s->symlinks) && !socket_find_symlink_target(s)) {
                 log_unit_error(UNIT(s), "Unit has symlinks set but none or more than one node in the file system. Refusing.");
-                return -EINVAL;
+                return -ENOEXEC;
         }
 
         return 0;
diff --git a/src/core/swap.c b/src/core/swap.c
index 398eb02aef..b61eab3e7e 100644
--- a/src/core/swap.c
+++ b/src/core/swap.c
@@ -240,12 +240,12 @@ static int swap_verify(Swap *s) {
 
         if (!unit_has_name(UNIT(s), e)) {
                 log_unit_error(UNIT(s), "Value of What= and unit name do not match, not loading.");
-                return -EINVAL;
+                return -ENOEXEC;
         }
 
         if (s->exec_context.pam_name && s->kill_context.kill_mode != KILL_CONTROL_GROUP) {
                 log_unit_error(UNIT(s), "Unit has PAM enabled. Kill mode must be set to 'control-group'. Refusing to load.");
-                return -EINVAL;
+                return -ENOEXEC;
         }
 
         return 0;
diff --git a/src/core/timer.c b/src/core/timer.c
index 5babb6d463..5b65e6e370 100644
--- a/src/core/timer.c
+++ b/src/core/timer.c
@@ -78,7 +78,7 @@ static int timer_verify(Timer *t) {
 
         if (!t->values) {
                 log_unit_error(UNIT(t), "Timer unit lacks value setting. Refusing.");
-                return -EINVAL;
+                return -ENOEXEC;
         }
 
         return 0;
diff --git a/src/core/transaction.c b/src/core/transaction.c
index 2886a05989..3dfc99819b 100644
--- a/src/core/transaction.c
+++ b/src/core/transaction.c
@@ -906,11 +906,13 @@ 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) {
-                r = bus_unit_check_load_state(unit, e);
+                r = bus_unit_validate_load_state(unit, e);
                 if (r < 0)
                         return r;
         }
diff --git a/src/core/unit.c b/src/core/unit.c
index a82e5fd9eb..0d9a6b32c6 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -1510,7 +1510,7 @@ int unit_load(Unit *u) {
 
                 if (u->on_failure_job_mode == JOB_ISOLATE && hashmap_size(u->dependencies[UNIT_ON_FAILURE]) > 1) {
                         log_unit_error(u, "More than one OnFailure= dependencies specified but OnFailureJobMode=isolate set. Refusing.");
-                        r = -EINVAL;
+                        r = -ENOEXEC;
                         goto fail;
                 }
 
@@ -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)) {
diff --git a/src/libsystemd/sd-bus/bus-common-errors.c b/src/libsystemd/sd-bus/bus-common-errors.c
index 9d9765e568..b815f2f2eb 100644
--- a/src/libsystemd/sd-bus/bus-common-errors.c
+++ b/src/libsystemd/sd-bus/bus-common-errors.c
@@ -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),
diff --git a/src/libsystemd/sd-bus/bus-common-errors.h b/src/libsystemd/sd-bus/bus-common-errors.h
index c67015c2c9..e34aedc528 100644
--- a/src/libsystemd/sd-bus/bus-common-errors.h
+++ b/src/libsystemd/sd-bus/bus-common-errors.h
@@ -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"
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 9e6d13a400..c9d2912e13 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -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;
@@ -3979,7 +3979,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
@@ -3989,7 +3989,7 @@ static void print_status_info(
         if (path && terminal_urlify_path(path, NULL, &formatted_path) >= 0)
                 path = formatted_path;
 
-        if (i->load_error != 0)
+        if (!isempty(i->load_error))
                 printf("   Loaded: %s%s%s (Reason: %s)\n",
                        on, strna(i->load_state), off, i->load_error);
         else if (path && !isempty(i->unit_file_state) && !isempty(i->unit_file_preset) &&