system/systemd
system/systemd
1
0
Fork 0
mirror of https://github.com/systemd/systemd synced 2024-07-08 20:15:55 +00:00
Code Issues Projects Releases Wiki Activity

manager: do not ignore the return value from the main loop

Browse Source 
If manager_loop() fails, we would print an error message, but then actually
ignore the error in main(), and potentially execute the shutdown binary.
I'm not sure how likely this is to happen in practice, but it seems sloppy.
So let's do the cleanup, but actually freeze() if manager_loop() returned
an error.

invoke_main_loop() is refactored to return the manager objective. This way
we don't need to pass a separate parameter to specify whether we are
reexecuting. Subsequent patch will make further use of the returned objective.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2022-01-27 14:14:24 +01:00
parent b87209f933
commit 5409c6fcc5
1 changed files with 39 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

75
src/core/main.c
View File

@ -1955,7 +1955,6 @@ static int invoke_main_loop(
Manager *m,
const struct rlimit *saved_rlimit_nofile,
const struct rlimit *saved_rlimit_memlock,
bool *ret_reexecute,
int *ret_retval, /* Return parameters relevant for shutting down */
const char **ret_shutdown_verb, /* … */
FDSet **ret_fds, /* Return parameters for reexecuting */
@ -1968,7 +1967,6 @@ static int invoke_main_loop(
assert(m);
assert(saved_rlimit_nofile);
assert(saved_rlimit_memlock);
assert(ret_reexecute);
assert(ret_retval);
assert(ret_shutdown_verb);
assert(ret_fds);
@ -1977,13 +1975,13 @@ static int invoke_main_loop(
assert(ret_error_message);
for (;;) {
r = manager_loop(m);
if (r < 0) {
int objective = manager_loop(m);
if (objective < 0) {
*ret_error_message = "Failed to run main loop";
return log_emergency_errno(r, "Failed to run main loop: %m");
return log_emergency_errno(objective, "Failed to run main loop: %m");
}
switch ((ManagerObjective) r) {
switch (objective) {
case MANAGER_RELOAD: {
LogTarget saved_log_target;
@ -2010,17 +2008,15 @@ static int invoke_main_loop(
if (saved_log_target >= 0)
manager_override_log_target(m, saved_log_target);
r = manager_reload(m);
if (r < 0)
if (manager_reload(m) < 0)
/* Reloading failed before the point of no return.
* Let's continue running as if nothing happened. */
m->objective = MANAGER_OK;
break;
continue;
}
case MANAGER_REEXECUTE:
r = prepare_reexecute(m, &arg_serialization, ret_fds, false);
if (r < 0) {
*ret_error_message = "Failed to prepare for reexecution";
@ -2029,12 +2025,11 @@ static int invoke_main_loop(
log_notice("Reexecuting.");
*ret_reexecute = true;
*ret_retval = EXIT_SUCCESS;
*ret_shutdown_verb = NULL;
*ret_switch_root_dir = *ret_switch_root_init = NULL;
return 0;
return objective;
case MANAGER_SWITCH_ROOT:
if (!m->switch_root_init) {
@ -2048,7 +2043,6 @@ static int invoke_main_loop(
log_notice("Switching root.");
*ret_reexecute = true;
*ret_retval = EXIT_SUCCESS;
*ret_shutdown_verb = NULL;
@ -2056,20 +2050,18 @@ static int invoke_main_loop(
*ret_switch_root_dir = TAKE_PTR(m->switch_root);
*ret_switch_root_init = TAKE_PTR(m->switch_root_init);
return 0;
return objective;
case MANAGER_EXIT:
if (MANAGER_IS_USER(m)) {
log_debug("Exit.");
*ret_reexecute = false;
*ret_retval = m->return_value;
*ret_shutdown_verb = NULL;
*ret_fds = NULL;
*ret_switch_root_dir = *ret_switch_root_init = NULL;
return 0;
return objective;
}
_fallthrough_;
@ -2077,7 +2069,7 @@ static int invoke_main_loop(
case MANAGER_POWEROFF:
case MANAGER_HALT:
case MANAGER_KEXEC: {
static const char * const table[_MANAGER_OBJECTIVE_MAX] = {
static const char* const table[_MANAGER_OBJECTIVE_MAX] = {
[MANAGER_EXIT] = "exit",
[MANAGER_REBOOT] = "reboot",
[MANAGER_POWEROFF] = "poweroff",
@ -2087,13 +2079,12 @@ static int invoke_main_loop(
log_notice("Shutting down.");
*ret_reexecute = false;
*ret_retval = m->return_value;
assert_se(*ret_shutdown_verb = table[m->objective]);
*ret_fds = NULL;
*ret_switch_root_dir = *ret_switch_root_init = NULL;
return 0;
return objective;
}
default:
@ -2709,15 +2700,18 @@ static int save_env(void) {
}
int main(int argc, char *argv[]) {
dual_timestamp initrd_timestamp = DUAL_TIMESTAMP_NULL, userspace_timestamp = DUAL_TIMESTAMP_NULL, kernel_timestamp = DUAL_TIMESTAMP_NULL,
security_start_timestamp = DUAL_TIMESTAMP_NULL, security_finish_timestamp = DUAL_TIMESTAMP_NULL;
dual_timestamp
initrd_timestamp = DUAL_TIMESTAMP_NULL,
userspace_timestamp = DUAL_TIMESTAMP_NULL,
kernel_timestamp = DUAL_TIMESTAMP_NULL,
security_start_timestamp = DUAL_TIMESTAMP_NULL,
security_finish_timestamp = DUAL_TIMESTAMP_NULL;
struct rlimit saved_rlimit_nofile = RLIMIT_MAKE_CONST(0),
saved_rlimit_memlock = RLIMIT_MAKE_CONST(RLIM_INFINITY); /* The original rlimits we passed
* in. Note we use different values
* for the two that indicate whether
* these fields are initialized! */
bool skip_setup, loaded_policy = false, queue_default_job = false, first_boot = false, reexecute = false;
bool skip_setup, loaded_policy = false, queue_default_job = false, first_boot = false;
char *switch_root_dir = NULL, *switch_root_init = NULL;
usec_t before_startup, after_startup;
static char systemd[] = "systemd";
@ -3021,16 +3015,23 @@ int main(int argc, char *argv[]) {
goto finish;
}
(void) invoke_main_loop(m,
&saved_rlimit_nofile,
&saved_rlimit_memlock,
&reexecute,
&retval,
&shutdown_verb,
&fds,
&switch_root_dir,
&switch_root_init,
&error_message);
r = invoke_main_loop(m,
&saved_rlimit_nofile,
&saved_rlimit_memlock,
&retval,
&shutdown_verb,
&fds,
&switch_root_dir,
&switch_root_init,
&error_message);
assert(r < 0 || IN_SET(r, MANAGER_EXIT, /* MANAGER_OK is not expected here. */
MANAGER_RELOAD,
MANAGER_REEXECUTE,
MANAGER_REBOOT,
MANAGER_POWEROFF,
MANAGER_HALT,
MANAGER_KEXEC,
MANAGER_SWITCH_ROOT));
finish:
pager_close();
@ -3043,7 +3044,7 @@ finish:
mac_selinux_finish();
if (reexecute)
if (IN_SET(r, MANAGER_REEXECUTE, MANAGER_SWITCH_ROOT))
do_reexecute(argc, argv,
&saved_rlimit_nofile,
&saved_rlimit_memlock,
@ -3076,7 +3077,9 @@ finish:
__lsan_do_leak_check();
#endif
if (shutdown_verb) {
/* Try to invoke the shutdown binary unless we already failed.
* If we failed above, we want to freeze after finishing cleanup. */
if (r >= 0 && shutdown_verb) {
r = become_shutdown(shutdown_verb, retval);
log_error_errno(r, "Failed to execute shutdown binary, %s: %m", getpid_cached() == 1 ? "freezing" : "quitting");
error_message = "Failed to execute shutdown binary";