shutdown: Don't limit unmount attempts prematurely (#8469)
authorJan Janssen <medhefgo@web.de>
Mon, 19 Mar 2018 09:27:49 +0000 (10:27 +0100)
committerYu Watanabe <watanabe.yu+github@gmail.com>
Mon, 19 Mar 2018 09:27:49 +0000 (18:27 +0900)
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

index fec9c27..1da6e59 100644 (file)
@@ -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();