diff --git a/src/core/mount.c b/src/core/mount.c index 2827f32d64..b90b76d3ac 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -55,7 +55,7 @@ static const UnitActiveState state_translation_table[_MOUNT_STATE_MAX] = { static int mount_dispatch_timer(sd_event_source *source, usec_t usec, void *userdata); static int mount_dispatch_io(sd_event_source *source, int fd, uint32_t revents, void *userdata); -static void mount_enter_dead(Mount *m, MountResult f); +static void mount_enter_dead(Mount *m, MountResult f, bool flush_result); static void mount_enter_mounted(Mount *m, MountResult f); static void mount_cycle_clear(Mount *m); static int mount_process_proc_self_mountinfo(Manager *m); @@ -800,7 +800,7 @@ static void mount_catchup(Unit *u) { break; case MOUNT_MOUNTED: assert(!pidref_is_set(&m->control_pid)); - mount_enter_dead(m, MOUNT_SUCCESS); + mount_enter_dead(m, MOUNT_SUCCESS, /* flush_result = */ false); break; default: break; @@ -899,10 +899,10 @@ static int mount_spawn(Mount *m, ExecCommand *c, PidRef *ret_pid) { return 0; } -static void mount_enter_dead(Mount *m, MountResult f) { +static void mount_enter_dead(Mount *m, MountResult f, bool flush_result) { assert(m); - if (m->result == MOUNT_SUCCESS) + if (m->result == MOUNT_SUCCESS || flush_result) m->result = f; unit_log_result(UNIT(m), m->result == MOUNT_SUCCESS, mount_result_to_string(m->result)); @@ -930,17 +930,20 @@ static void mount_enter_mounted(Mount *m, MountResult f) { mount_set_state(m, MOUNT_MOUNTED); } -static void mount_enter_dead_or_mounted(Mount *m, MountResult f) { +static void mount_enter_dead_or_mounted(Mount *m, MountResult f, bool flush_result) { assert(m); - /* Enter DEAD or MOUNTED state, depending on what the kernel currently says about the mount point. We use this - * whenever we executed an operation, so that our internal state reflects what the kernel says again, after all - * ultimately we just mirror the kernel's internal state on this. */ + /* Enter DEAD or MOUNTED state, depending on what the kernel currently says about the mount point. + * We use this whenever we executed an operation, so that our internal state reflects what + * the kernel says again, after all ultimately we just mirror the kernel's internal state on this. + * + * Note that flush_result only applies to mount_enter_dead(), since that's when the result gets + * turned into unit end state. */ if (m->from_proc_self_mountinfo) mount_enter_mounted(m, f); else - mount_enter_dead(m, f); + mount_enter_dead(m, f, flush_result); } static int state_to_kill_operation(MountState state) { @@ -990,12 +993,12 @@ static void mount_enter_signal(Mount *m, MountState state, MountResult f) { else if (state == MOUNT_UNMOUNTING_SIGTERM && m->kill_context.send_sigkill) mount_enter_signal(m, MOUNT_UNMOUNTING_SIGKILL, MOUNT_SUCCESS); else - mount_enter_dead_or_mounted(m, MOUNT_SUCCESS); + mount_enter_dead_or_mounted(m, MOUNT_SUCCESS, /* flush_result = */ false); return; fail: - mount_enter_dead_or_mounted(m, MOUNT_FAILURE_RESOURCES); + mount_enter_dead_or_mounted(m, MOUNT_FAILURE_RESOURCES, /* flush_result = */ false); } static int mount_set_umount_command(Mount *m, ExecCommand *c) { @@ -1057,7 +1060,7 @@ static void mount_enter_unmounting(Mount *m) { return; fail: - mount_enter_dead_or_mounted(m, MOUNT_FAILURE_RESOURCES); + mount_enter_dead_or_mounted(m, MOUNT_FAILURE_RESOURCES, /* flush_result = */ false); } static int mount_set_mount_command(Mount *m, ExecCommand *c, const MountParameters *p) { @@ -1201,7 +1204,7 @@ static void mount_enter_mounting(Mount *m) { return; fail: - mount_enter_dead_or_mounted(m, MOUNT_FAILURE_RESOURCES); + mount_enter_dead_or_mounted(m, MOUNT_FAILURE_RESOURCES, /* flush_result = */ false); } static void mount_set_reload_result(Mount *m, MountResult result) { @@ -1267,7 +1270,7 @@ static void mount_enter_remounting(Mount *m) { fail: mount_set_reload_result(m, MOUNT_FAILURE_RESOURCES); - mount_enter_dead_or_mounted(m, MOUNT_SUCCESS); + mount_enter_dead_or_mounted(m, MOUNT_SUCCESS, /* flush_result = */ false); } static void mount_cycle_clear(Mount *m) { @@ -1547,7 +1550,7 @@ static void mount_sigchld_event(Unit *u, pid_t pid, int code, int status) { log_unit_warning(UNIT(m), "Mount process finished, but there is no mount."); f = MOUNT_FAILURE_PROTOCOL; } - mount_enter_dead(m, f); + mount_enter_dead(m, f, /* flush_result = */ false); break; case MOUNT_MOUNTING_DONE: @@ -1557,7 +1560,7 @@ static void mount_sigchld_event(Unit *u, pid_t pid, int code, int status) { case MOUNT_REMOUNTING: case MOUNT_REMOUNTING_SIGTERM: case MOUNT_REMOUNTING_SIGKILL: - mount_enter_dead_or_mounted(m, MOUNT_SUCCESS); + mount_enter_dead_or_mounted(m, MOUNT_SUCCESS, /* flush_result = */ false); break; case MOUNT_UNMOUNTING: @@ -1578,22 +1581,27 @@ static void mount_sigchld_event(Unit *u, pid_t pid, int code, int status) { /* Hmm, umount process spawned by us failed, but the mount disappeared anyway? * Maybe someone else is trying to unmount at the same time. */ log_unit_notice(u, "Mount disappeared even though umount process failed, continuing."); - mount_enter_dead(m, MOUNT_SUCCESS); + mount_enter_dead(m, MOUNT_SUCCESS, /* flush_result = */ true); } else - mount_enter_dead_or_mounted(m, f); + /* At this point, either the unmount succeeded or unexpected error occurred. We usually + * remember the first error in 'result', but here let's update that forcibly, since + * there could previous failed attempts yet we only care about the most recent + * attempt. IOW, if we eventually managed to unmount the stuff, don't enter failed + * end state. */ + mount_enter_dead_or_mounted(m, f, /* flush_result = */ true); break; case MOUNT_UNMOUNTING_SIGTERM: case MOUNT_UNMOUNTING_SIGKILL: - mount_enter_dead_or_mounted(m, f); + mount_enter_dead_or_mounted(m, f, /* flush_result = */ false); break; case MOUNT_CLEANING: if (m->clean_result == MOUNT_SUCCESS) m->clean_result = f; - mount_enter_dead(m, MOUNT_SUCCESS); + mount_enter_dead(m, MOUNT_SUCCESS, /* flush_result = */ false); break; default: @@ -1631,7 +1639,7 @@ static int mount_dispatch_timer(sd_event_source *source, usec_t usec, void *user mount_enter_signal(m, MOUNT_REMOUNTING_SIGKILL, MOUNT_SUCCESS); } else { log_unit_warning(UNIT(m), "Remounting timed out. Skipping SIGKILL. Ignoring."); - mount_enter_dead_or_mounted(m, MOUNT_SUCCESS); + mount_enter_dead_or_mounted(m, MOUNT_SUCCESS, /* flush_result = */ false); } break; @@ -1639,7 +1647,7 @@ static int mount_dispatch_timer(sd_event_source *source, usec_t usec, void *user mount_set_reload_result(m, MOUNT_FAILURE_TIMEOUT); log_unit_warning(UNIT(m), "Mount process still around after SIGKILL. Ignoring."); - mount_enter_dead_or_mounted(m, MOUNT_SUCCESS); + mount_enter_dead_or_mounted(m, MOUNT_SUCCESS, /* flush_result = */ false); break; case MOUNT_UNMOUNTING: @@ -1653,13 +1661,13 @@ static int mount_dispatch_timer(sd_event_source *source, usec_t usec, void *user mount_enter_signal(m, MOUNT_UNMOUNTING_SIGKILL, MOUNT_FAILURE_TIMEOUT); } else { log_unit_warning(UNIT(m), "Mount process timed out. Skipping SIGKILL. Ignoring."); - mount_enter_dead_or_mounted(m, MOUNT_FAILURE_TIMEOUT); + mount_enter_dead_or_mounted(m, MOUNT_FAILURE_TIMEOUT, /* flush_result = */ false); } break; case MOUNT_UNMOUNTING_SIGKILL: log_unit_warning(UNIT(m), "Mount process still around after SIGKILL. Ignoring."); - mount_enter_dead_or_mounted(m, MOUNT_FAILURE_TIMEOUT); + mount_enter_dead_or_mounted(m, MOUNT_FAILURE_TIMEOUT, /* flush_result = */ false); break; case MOUNT_CLEANING: @@ -2121,8 +2129,11 @@ static int mount_process_proc_self_mountinfo(Manager *m) { switch (mount->state) { case MOUNT_MOUNTED: - /* This has just been unmounted by somebody else, follow the state change. */ - mount_enter_dead(mount, MOUNT_SUCCESS); + /* This has just been unmounted by somebody else, follow the state change. + * Also explicitly override the result (see the comment in mount_sigchld_event()), + * but more aggressively here since the state change is extrinsic. */ + mount_cycle_clear(mount); + mount_enter_dead(mount, MOUNT_SUCCESS, /* flush_result = */ true); break; case MOUNT_MOUNTING_DONE: @@ -2290,7 +2301,7 @@ static int mount_can_start(Unit *u) { r = unit_test_start_limit(u); if (r < 0) { - mount_enter_dead(m, MOUNT_FAILURE_START_LIMIT_HIT); + mount_enter_dead(m, MOUNT_FAILURE_START_LIMIT_HIT, /* flush_result = */ false); return r; }