From 0faacd470dfbd24f4c6504da6f04213aa05f9d19 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 12 Dec 2014 21:05:32 +0100 Subject: [PATCH] unit: handle nicely of certain unit types are not supported on specific systems Containers do not really support .device, .automount or .swap units; Systems compiled without support for swap do not support .swap units; Systems without kdbus do not support .busname units. With this change attempts to start a unsupported unit types will result in an immediate "unsupported" job result, which is a lot more descriptive then before. Also, attempts to start device units in containers will now immediately fail instead of causing jobs to be enqueued that never go away. --- src/core/automount.c | 12 ++++++++++++ src/core/busname.c | 12 ++++++++++++ src/core/device.c | 14 ++++++++++++++ src/core/job.c | 24 ++++++++++++++++++------ src/core/job.h | 3 ++- src/core/manager.c | 21 +++++++++++++++------ src/core/mount.c | 1 - src/core/swap.c | 16 ++++++++++++++++ src/core/unit.c | 15 ++++++++------- src/core/unit.h | 4 ++++ src/systemctl/systemctl.c | 24 ++++++++++++++++++------ 11 files changed, 119 insertions(+), 27 deletions(-) diff --git a/src/core/automount.c b/src/core/automount.c index f79548713..90beb4daa 100644 --- a/src/core/automount.c +++ b/src/core/automount.c @@ -807,6 +807,17 @@ static void automount_reset_failed(Unit *u) { a->result = AUTOMOUNT_SUCCESS; } +static bool automount_supported(Manager *m) { + static int supported = -1; + + assert(m); + + if (supported < 0) + supported = access("/dev/autofs", F_OK) >= 0; + + return supported; +} + static const char* const automount_state_table[_AUTOMOUNT_STATE_MAX] = { [AUTOMOUNT_DEAD] = "dead", [AUTOMOUNT_WAITING] = "waiting", @@ -859,6 +870,7 @@ const UnitVTable automount_vtable = { .bus_vtable = bus_automount_vtable, .shutdown = automount_shutdown, + .supported = automount_supported, .status_message_formats = { .finished_start_job = { diff --git a/src/core/busname.c b/src/core/busname.c index d4aa46339..838171fe2 100644 --- a/src/core/busname.c +++ b/src/core/busname.c @@ -971,6 +971,16 @@ static int busname_get_timeout(Unit *u, uint64_t *timeout) { return 1; } +static bool busname_supported(Manager *m) { + int supported = -1; + assert(m); + + if (supported < 0) + supported = access("/sys/fs/kdbus", F_OK) >= 0; + + return supported; +} + static const char* const busname_state_table[_BUSNAME_STATE_MAX] = { [BUSNAME_DEAD] = "dead", [BUSNAME_MAKING] = "making", @@ -1032,6 +1042,8 @@ const UnitVTable busname_vtable = { .reset_failed = busname_reset_failed, + .supported = busname_supported, + .bus_interface = "org.freedesktop.systemd1.BusName", .bus_vtable = bus_busname_vtable, diff --git a/src/core/device.c b/src/core/device.c index c298cd95f..d3deac393 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -673,6 +673,19 @@ static int device_dispatch_io(sd_event_source *source, int fd, uint32_t revents, return 0; } +static bool device_supported(Manager *m) { + static int read_only = -1; + assert(m); + + /* If /sys is read-only we don't support device units, and any + * attempts to start one should fail immediately. */ + + if (read_only < 0) + read_only = path_is_read_only_fs("/sys"); + + return read_only <= 0; +} + static const char* const device_state_table[_DEVICE_STATE_MAX] = { [DEVICE_DEAD] = "dead", [DEVICE_PLUGGED] = "plugged" @@ -708,6 +721,7 @@ const UnitVTable device_vtable = { .enumerate = device_enumerate, .shutdown = device_shutdown, + .supported = device_supported, .status_message_formats = { .starting_stopping = { diff --git a/src/core/job.c b/src/core/job.c index 78bc1083d..a3ba7cf70 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -547,6 +547,8 @@ int job_run_and_invalidate(Job *j) { r = job_finish_and_invalidate(j, JOB_INVALID, true); else if (r == -EPROTO) r = job_finish_and_invalidate(j, JOB_ASSERT, true); + else if (r == -ENOTSUP) + r = job_finish_and_invalidate(j, JOB_UNSUPPORTED, true); else if (r == -EAGAIN) { j->state = JOB_WAITING; m->n_running_jobs--; @@ -591,12 +593,16 @@ _pure_ static const char *job_get_status_message_format_try_harder(Unit *u, JobT if (t == JOB_START) { if (result == JOB_DONE) return "Started %s."; + else if (result == JOB_TIMEOUT) + return "Timed out starting %s."; else if (result == JOB_FAILED) return "Failed to start %s."; else if (result == JOB_DEPENDENCY) return "Dependency failed for %s."; - else if (result == JOB_TIMEOUT) - return "Timed out starting %s."; + else if (result == JOB_ASSERT) + return "Assertion failed for %s."; + else if (result == JOB_UNSUPPORTED) + return "Starting of %s not supported."; } else if (t == JOB_STOP || t == JOB_RESTART) { if (result == JOB_DONE) return "Stopped %s."; @@ -637,6 +643,11 @@ static void job_print_status_message(Unit *u, JobType t, JobResult result) { unit_status_printf(u, ANSI_GREEN_ON " OK " ANSI_HIGHLIGHT_OFF, format); break; + case JOB_TIMEOUT: + manager_flip_auto_status(u->manager, true); + unit_status_printf(u, ANSI_HIGHLIGHT_RED_ON " TIME " ANSI_HIGHLIGHT_OFF, format); + break; + case JOB_FAILED: { bool quotes; @@ -655,14 +666,14 @@ static void job_print_status_message(Unit *u, JobType t, JobResult result) { unit_status_printf(u, ANSI_HIGHLIGHT_YELLOW_ON "DEPEND" ANSI_HIGHLIGHT_OFF, format); break; - case JOB_TIMEOUT: + case JOB_ASSERT: manager_flip_auto_status(u->manager, true); - unit_status_printf(u, ANSI_HIGHLIGHT_RED_ON " TIME " ANSI_HIGHLIGHT_OFF, format); + unit_status_printf(u, ANSI_HIGHLIGHT_YELLOW_ON "ASSERT" ANSI_HIGHLIGHT_OFF, format); break; - case JOB_ASSERT: + case JOB_UNSUPPORTED: manager_flip_auto_status(u->manager, true); - unit_status_printf(u, ANSI_HIGHLIGHT_YELLOW_ON "ASSERT" ANSI_HIGHLIGHT_OFF, format); + unit_status_printf(u, ANSI_HIGHLIGHT_YELLOW_ON "UNSUPP" ANSI_HIGHLIGHT_OFF, format); break; default: @@ -1200,6 +1211,7 @@ static const char* const job_result_table[_JOB_RESULT_MAX] = { [JOB_SKIPPED] = "skipped", [JOB_INVALID] = "invalid", [JOB_ASSERT] = "assert", + [JOB_UNSUPPORTED] = "unsupported", }; DEFINE_STRING_TABLE_LOOKUP(job_result, JobResult); diff --git a/src/core/job.h b/src/core/job.h index 223ff9cba..d967b68a3 100644 --- a/src/core/job.h +++ b/src/core/job.h @@ -96,12 +96,13 @@ enum JobMode { enum JobResult { JOB_DONE, /* Job completed successfully */ JOB_CANCELED, /* Job canceled by a conflicting job installation or by explicit cancel request */ - JOB_TIMEOUT, /* JobTimeout elapsed */ + JOB_TIMEOUT, /* Job timeout elapsed */ JOB_FAILED, /* Job failed */ JOB_DEPENDENCY, /* A required dependency job did not result in JOB_DONE */ JOB_SKIPPED, /* Negative result of JOB_VERIFY_ACTIVE */ JOB_INVALID, /* JOB_RELOAD of inactive unit */ JOB_ASSERT, /* Couldn't start a unit, because an assert didn't hold */ + JOB_UNSUPPORTED, /* Couldn't start a unit, because the unit type is not supported on the system */ _JOB_RESULT_MAX, _JOB_RESULT_INVALID = -1 }; diff --git a/src/core/manager.c b/src/core/manager.c index 7653850cd..afd911d89 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -946,20 +946,29 @@ Manager* manager_free(Manager *m) { } int manager_enumerate(Manager *m) { - int r = 0, q; + int r = 0; UnitType c; assert(m); /* Let's ask every type to load all units from disk/kernel * that it might know */ - for (c = 0; c < _UNIT_TYPE_MAX; c++) - if (unit_vtable[c]->enumerate) { - q = unit_vtable[c]->enumerate(m); - if (q < 0) - r = q; + for (c = 0; c < _UNIT_TYPE_MAX; c++) { + int q; + + if (unit_vtable[c]->supported && !unit_vtable[c]->supported(m)) { + log_info("Unit type .%s is not supported on this system.", unit_type_to_string(c)); + continue; } + if (!unit_vtable[c]->enumerate) + continue; + + q = unit_vtable[c]->enumerate(m); + if (q < 0) + r = q; + } + manager_dispatch_load_queue(m); return r; } diff --git a/src/core/mount.c b/src/core/mount.c index 3029cfd4d..f8731bb8b 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -1447,7 +1447,6 @@ static int mount_add_one( goto fail; } - if (m->running_as == SYSTEMD_SYSTEM) { const char* target; diff --git a/src/core/swap.c b/src/core/swap.c index a6a23554c..cb19d0f12 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -1453,6 +1453,21 @@ static int swap_get_timeout(Unit *u, uint64_t *timeout) { return 1; } +static bool swap_supported(Manager *m) { + static int supported = -1; + + /* If swap support is not available in the kernel, or we are + * running in a container we don't support swap units, and any + * attempts to starting one should fail immediately. */ + + if (supported < 0) + supported = + access("/proc/swaps", F_OK) >= 0 && + detect_container(NULL) <= 0; + + return supported; +} + static const char* const swap_state_table[_SWAP_STATE_MAX] = { [SWAP_DEAD] = "dead", [SWAP_ACTIVATING] = "activating", @@ -1539,6 +1554,7 @@ const UnitVTable swap_vtable = { .enumerate = swap_enumerate, .shutdown = swap_shutdown, + .supported = swap_supported, .status_message_formats = { .starting_stopping = { diff --git a/src/core/unit.c b/src/core/unit.c index fe0dfb208..8cec0e7e7 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -1453,6 +1453,9 @@ int unit_start(Unit *u) { unit_status_log_starting_stopping_reloading(u, JOB_START); unit_status_print_starting_stopping(u, JOB_START); + if (UNIT_VTABLE(u)->supported && !UNIT_VTABLE(u)->supported(u->manager)) + return -ENOTSUP; + /* If it is stopped, but we cannot start it, then fail */ if (!UNIT_VTABLE(u)->start) return -EBADR; @@ -1496,9 +1499,9 @@ int unit_stop(Unit *u) { if (UNIT_IS_INACTIVE_OR_FAILED(state)) return -EALREADY; - if ((following = unit_following(u))) { - log_unit_debug(u->id, "Redirecting stop request from %s to %s.", - u->id, following->id); + following = unit_following(u); + if (following) { + log_unit_debug(u->id, "Redirecting stop request from %s to %s.", u->id, following->id); return unit_stop(following); } @@ -1535,15 +1538,13 @@ int unit_reload(Unit *u) { return -EALREADY; if (state != UNIT_ACTIVE) { - log_unit_warning(u->id, "Unit %s cannot be reloaded because it is inactive.", - u->id); + log_unit_warning(u->id, "Unit %s cannot be reloaded because it is inactive.", u->id); return -ENOEXEC; } following = unit_following(u); if (following) { - log_unit_debug(u->id, "Redirecting reload request from %s to %s.", - u->id, following->id); + log_unit_debug(u->id, "Redirecting reload request from %s to %s.", u->id, following->id); return unit_reload(following); } diff --git a/src/core/unit.h b/src/core/unit.h index f6587ba5f..19fa2f058 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -396,6 +396,10 @@ struct UnitVTable { /* Type specific cleanups. */ void (*shutdown)(Manager *m); + /* If this function is set and return false all jobs for units + * of this type will immediately fail. */ + bool (*supported)(Manager *m); + /* The interface name */ const char *bus_interface; diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 8400bc8cd..8a1b481fd 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -2380,12 +2380,18 @@ static int check_wait_response(WaitData *d) { assert(d->result); if (!arg_quiet) { - if (streq(d->result, "timeout")) - log_error("Job for %s timed out.", strna(d->name)); - else if (streq(d->result, "canceled")) + if (streq(d->result, "canceled")) log_error("Job for %s canceled.", strna(d->name)); + else if (streq(d->result, "timeout")) + log_error("Job for %s timed out.", strna(d->name)); else if (streq(d->result, "dependency")) log_error("A dependency job for %s failed. See 'journalctl -xe' for details.", strna(d->name)); + else if (streq(d->result, "invalid")) + log_error("Job for %s invalid.", strna(d->name)); + else if (streq(d->result, "assert")) + log_error("Assertion failed on job for %s.", strna(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)); else if (!streq(d->result, "done") && !streq(d->result, "skipped")) { if (d->name) { bool quotes; @@ -2400,12 +2406,18 @@ static int check_wait_response(WaitData *d) { } } - if (streq(d->result, "timeout")) - r = -ETIME; - else if (streq(d->result, "canceled")) + if (streq(d->result, "canceled")) r = -ECANCELED; + else if (streq(d->result, "timeout")) + r = -ETIME; else if (streq(d->result, "dependency")) r = -EIO; + else if (streq(d->result, "invalid")) + r = -ENOEXEC; + else if (streq(d->result, "assert")) + r = -EPROTO; + else if (streq(d->result, "unsupported")) + r = -ENOTSUP; else if (!streq(d->result, "done") && !streq(d->result, "skipped")) r = -EIO; -- 2.30.2