From ac9cea5ba30acbf17fd431a4a4092c4dbee23593 Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Mon, 19 Mar 2018 10:27:49 +0100 Subject: [PATCH] shutdown: Don't limit unmount attempts prematurely (#8469) Once upon a time shutdown.c didn't have the logic to check whether any unmount attempts succeeded or not. So instead it kept looping for a fixed amount and hoped all was right. Nowadays, we do know if we changed anything during a iteration and also stop looping then, but we still limit ourselves to FINALIZE_ATTEMPTS. But, theoretically, we could have such a complicated and nested setup that would survive that limit, leaving stuff around we might actually be able to unmount. And we could also end up in a situation where the extra loop with raised unmount error level could be skipped too. So let's just drop the retries logic and rely fully on the changed flag. --- src/core/shutdown.c | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/src/core/shutdown.c b/src/core/shutdown.c index fec9c27..1da6e59 100644 --- a/src/core/shutdown.c +++ b/src/core/shutdown.c @@ -52,8 +52,6 @@ #include "virt.h" #include "watchdog.h" -#define FINALIZE_ATTEMPTS 50 - #define SYNC_PROGRESS_ATTEMPTS 3 #define SYNC_TIMEOUT_USEC (10*USEC_PER_SEC) @@ -272,7 +270,6 @@ int main(int argc, char *argv[]) { bool in_container, use_watchdog = false, can_initrd; _cleanup_free_ char *cgroup = NULL; char *arguments[3]; - unsigned retries; int cmd, r, umount_log_level = LOG_INFO; static const char* const dirs[] = {SYSTEM_SHUTDOWN_PATH, NULL}; char *watchdog_device; @@ -352,7 +349,7 @@ int main(int argc, char *argv[]) { can_initrd = !in_container && !in_initrd() && access("/run/initramfs/shutdown", X_OK) == 0; /* Unmount all mountpoints, swaps, and loopback devices */ - for (retries = 0; retries < FINALIZE_ATTEMPTS; retries++) { + for (;;) { bool changed = false; if (use_watchdog) @@ -414,10 +411,9 @@ int main(int argc, char *argv[]) { } if (!need_umount && !need_swapoff && !need_loop_detach && !need_dm_detach) { - if (retries > 0) - log_info("All filesystems, swaps, loop devices, DM devices detached."); + log_info("All filesystems, swaps, loop devices and DM devices detached."); /* Yay, done */ - goto initrd_jump; + break; } if (!changed && umount_log_level == LOG_INFO && !can_initrd) { @@ -438,21 +434,16 @@ int main(int argc, char *argv[]) { need_swapoff ? " swap devices," : "", need_loop_detach ? " loop devices," : "", need_dm_detach ? " DM devices," : ""); - goto initrd_jump; + break; } - log_debug("After %u retries, couldn't finalize remaining %s%s%s%s trying again.", - retries + 1, + log_debug("Couldn't finalize remaining %s%s%s%s trying again.", need_umount ? " file systems," : "", need_swapoff ? " swap devices," : "", need_loop_detach ? " loop devices," : "", need_dm_detach ? " DM devices," : ""); } - log_error("Too many iterations, giving up."); - - initrd_jump: - /* We're done with the watchdog. */ watchdog_free_device(); -- 2.7.4