shutdown: loop only as long as we manage to unmount/detach devices, give up immediately when we are stuck

3 files changed, 179 insertions, 115 deletions

diff --git a/src/shutdown.c b/src/shutdown.c
index 2c14371df2..bd4b69bbe3 100644
--- a/src/shutdown.c
+++ b/src/shutdown.c
@@ -38,7 +38,6 @@
 
 #define TIMEOUT_USEC (5 * USEC_PER_SEC)
 #define FINALIZE_ATTEMPTS 50
-#define FINALIZE_CRITICAL_ATTEMPTS 10
 
 static bool ignore_proc(pid_t pid) {
         if (pid == 1)
@@ -69,7 +68,7 @@ static bool is_kernel_thread(pid_t pid)
 static int killall(int sign) {
         DIR *dir;
         struct dirent *d;
-        unsigned int processes = 0;
+        unsigned int n_processes = 0;
 
         if ((dir = opendir("/proc")) == NULL)
                 return -errno;
@@ -87,20 +86,20 @@ static int killall(int sign) {
                         continue;
 
                 if (kill(pid, sign) == 0)
-                        processes++;
+                        n_processes++;
                 else
                         log_warning("Could not kill %d: %m", pid);
         }
 
         closedir(dir);
 
-        return processes;
+        return n_processes;
 }
 
 static int send_signal(int sign) {
         sigset_t mask, oldmask;
         usec_t until;
-        int processes;
+        int n_processes;
         struct timespec ts;
 
         assert_se(sigemptyset(&mask) == 0);
@@ -111,12 +110,12 @@ static int send_signal(int sign) {
         if (kill(-1, SIGSTOP) < 0)
                 log_warning("Failed kill(-1, SIGSTOP): %m");
 
-        processes = killall(sign);
+        n_processes = killall(sign);
 
         if (kill(-1, SIGCONT) < 0)
                 log_warning("Failed kill(-1, SIGCONT): %m");
 
-        if (processes <= 0)
+        if (n_processes <= 0)
                 goto finish;
 
         until = now(CLOCK_MONOTONIC) + TIMEOUT_USEC;
@@ -124,14 +123,15 @@ static int send_signal(int sign) {
                 usec_t n = now(CLOCK_MONOTONIC);
                 for (;;) {
                         pid_t pid = waitpid(-1, NULL, WNOHANG);
+
                         if (pid == 0)
                                 break;
                         else if (pid < 0 && errno == ECHILD) {
-                                processes = 0;
+                                n_processes = 0;
                                 goto finish;
                         }
 
-                        if (--processes == 0)
+                        if (--n_processes == 0)
                                 goto finish;
                 }
 
@@ -146,7 +146,7 @@ static int send_signal(int sign) {
 finish:
         sigprocmask(SIG_SETMASK, &oldmask, NULL);
 
-        return processes;
+        return n_processes;
 }
 
 static int rescue_send_signal(int sign) {
@@ -199,8 +199,10 @@ finish:
 }
 
 int main(int argc, char *argv[]) {
-        int cmd, r, retries;
+        int cmd, r;
+        unsigned retries;
         bool need_umount = true, need_swapoff = true, need_loop_detach = true, need_dm_detach = true;
+        bool killed_everbody = false;
 
         log_parse_environment();
         log_set_target(LOG_TARGET_CONSOLE); /* syslog will die if not gone yet */
@@ -247,11 +249,12 @@ int main(int argc, char *argv[]) {
                 log_warning("Cannot send SIGKILL to all process: %s", strerror(r));
 
         /* Unmount all mountpoints, swaps, and loopback devices */
-        retries = FINALIZE_ATTEMPTS;
-        for (;;) {
+        for (retries = 0; retries < FINALIZE_ATTEMPTS; retries++) {
+                bool changed = false;
+
                 if (need_umount) {
                         log_info("Unmounting filesystems.");
-                        r = umount_all();
+                        r = umount_all(&changed);
                         if (r == 0)
                                 need_umount = false;
                         else if (r > 0)
@@ -262,7 +265,7 @@ int main(int argc, char *argv[]) {
 
                 if (need_swapoff) {
                         log_info("Disabling swaps.");
-                        r = swapoff_all();
+                        r = swapoff_all(&changed);
                         if (r == 0)
                                 need_swapoff = false;
                         else if (r > 0)
@@ -273,7 +276,7 @@ int main(int argc, char *argv[]) {
 
                 if (need_loop_detach) {
                         log_info("Detaching loop devices.");
-                        r = loopback_detach_all();
+                        r = loopback_detach_all(&changed);
                         if (r == 0)
                                 need_loop_detach = false;
                         else if (r > 0)
@@ -284,7 +287,7 @@ int main(int argc, char *argv[]) {
 
                 if (need_dm_detach) {
                         log_info("Detaching DM devices.");
-                        r = dm_detach_all();
+                        r = dm_detach_all(&changed);
                         if (r == 0)
                                 need_dm_detach = false;
                         else if (r > 0)
@@ -293,31 +296,40 @@ int main(int argc, char *argv[]) {
                                 log_error("Error detaching dm devices: %s", strerror(-r));
                 }
 
-                if (need_umount || need_swapoff || need_loop_detach || need_dm_detach) {
-                        retries--;
+                if (!need_umount && !need_swapoff && !need_loop_detach && !need_dm_detach)
+                        /* Yay, done */
+                        break;
 
-                        if (retries == FINALIZE_CRITICAL_ATTEMPTS) {
-                                log_warning("Approaching critical level to finalize filesystem and devices, try to kill all processes.");
-                                rescue_send_signal(SIGTERM);
-                                rescue_send_signal(SIGKILL);
-                        }
+                /* If in this iteration we didn't manage to
+                 * unmount/deactivate anything, we either kill more
+                 * processes, or simply give up */
+                if (!changed) {
 
-                        if (retries > 0)
-                                log_info("Action still required, %d tries left.", retries);
-                        else {
-                                log_error("Giving up. Actions left: Umount=%s, Swap off=%s, Loop detach=%s, dm detach=%s",
-                                          yes_no(need_umount), yes_no(need_swapoff), yes_no(need_loop_detach), yes_no(need_dm_detach));
+                        if (killed_everbody) {
+                                /* Hmm, we already killed everybody,
+                                 * let's just give up */
+                                log_error("Cannot finalize all filesystems and devices, giving up.");
                                 break;
                         }
-                } else
-                        break;
+
+                        log_warning("Cannot finalize filesystems and devices, trying to kill remaining processes.");
+                        rescue_send_signal(SIGTERM);
+                        rescue_send_signal(SIGKILL);
+                        killed_everbody = true;
+                }
+
+                log_debug("Couldn't finalize filesystems and devices after %u retries, trying again.", retries+1);
         }
 
+        if (retries >= FINALIZE_ATTEMPTS)
+                log_error("Too many interations, giving up.");
+
         sync();
 
         if (cmd == LINUX_REBOOT_CMD_KEXEC) {
-                /* we cheat and exec kexec to avoid doing all its work */
+                /* We cheat and exec kexec to avoid doing all its work */
                 pid_t pid = fork();
+
                 if (pid < 0)
                         log_error("Could not fork: %m. Falling back to normal reboot.");
                 else if (pid > 0) {
diff --git a/src/umount.c b/src/umount.c
index c95f72760b..c7f62081d2 100644
--- a/src/umount.c
+++ b/src/umount.c
@@ -40,30 +40,37 @@ typedef struct MountPoint {
         LIST_FIELDS (struct MountPoint, mount_point);
 } MountPoint;
 
-static void mount_point_remove_and_free(MountPoint *mount_point, MountPoint **mount_point_list_head) {
-        LIST_REMOVE(MountPoint, mount_point, *mount_point_list_head, mount_point);
+static void mount_point_free(MountPoint **head, MountPoint *m) {
+        assert(head);
+        assert(m);
 
-        free(mount_point->path);
-        free(mount_point);
+        LIST_REMOVE(MountPoint, mount_point, *head, m);
+
+        free(m->path);
+        free(m);
 }
 
-static void mount_points_list_free(MountPoint **mount_point_list_head) {
-        while (*mount_point_list_head)
-                mount_point_remove_and_free(*mount_point_list_head, mount_point_list_head);
+static void mount_points_list_free(MountPoint **head) {
+        assert(head);
+
+        while (*head)
+                mount_point_free(head, *head);
 }
 
-static int mount_points_list_get(MountPoint **mount_point_list_head) {
+static int mount_points_list_get(MountPoint **head) {
         FILE *proc_self_mountinfo;
         char *path, *p;
         unsigned int i;
         int r;
 
+        assert(head);
+
         if (!(proc_self_mountinfo = fopen("/proc/self/mountinfo", "re")))
                 return -errno;
 
         for (i = 1;; i++) {
                 int k;
-                MountPoint *mp;
+                MountPoint *m;
 
                 path = p = NULL;
 
@@ -103,14 +110,14 @@ static int mount_points_list_get(MountPoint **mount_point_list_head) {
                         continue;
                 }
 
-                if (!(mp = new0(MountPoint, 1))) {
+                if (!(m = new0(MountPoint, 1))) {
                         free(p);
                         r = -ENOMEM;
                         goto finish;
                 }
 
-                mp->path = p;
-                LIST_PREPEND(MountPoint, mount_point, *mount_point_list_head, mp);
+                m->path = p;
+                LIST_PREPEND(MountPoint, mount_point, *head, m);
         }
 
         r = 0;
@@ -121,11 +128,13 @@ finish:
         return r;
 }
 
-static int swap_list_get(MountPoint **swap_list_head) {
+static int swap_list_get(MountPoint **head) {
         FILE *proc_swaps;
         unsigned int i;
         int r;
 
+        assert(head);
+
         if (!(proc_swaps = fopen("/proc/swaps", "re")))
                 return -errno;
 
@@ -173,7 +182,7 @@ static int swap_list_get(MountPoint **swap_list_head) {
                 }
 
                 swap->path = d;
-                LIST_PREPEND(MountPoint, mount_point, *swap_list_head, swap);
+                LIST_PREPEND(MountPoint, mount_point, *head, swap);
         }
 
         r = 0;
@@ -184,12 +193,14 @@ finish:
         return r;
 }
 
-static int loopback_list_get(MountPoint **loopback_list_head) {
+static int loopback_list_get(MountPoint **head) {
         int r;
         struct udev *udev;
         struct udev_enumerate *e = NULL;
         struct udev_list_entry *item = NULL, *first = NULL;
 
+        assert(head);
+
         if (!(udev = udev_new())) {
                 r = -ENOMEM;
                 goto finish;
@@ -244,7 +255,7 @@ static int loopback_list_get(MountPoint **loopback_list_head) {
                 }
 
                 lb->path = loop;
-                LIST_PREPEND(MountPoint, mount_point, *loopback_list_head, lb);
+                LIST_PREPEND(MountPoint, mount_point, *head, lb);
         }
 
         r = 0;
@@ -259,12 +270,14 @@ finish:
         return r;
 }
 
-static int dm_list_get(MountPoint **dm_list_head) {
+static int dm_list_get(MountPoint **head) {
         int r;
         struct udev *udev;
         struct udev_enumerate *e = NULL;
         struct udev_list_entry *item = NULL, *first = NULL;
 
+        assert(head);
+
         if (!(udev = udev_new())) {
                 r = -ENOMEM;
                 goto finish;
@@ -324,7 +337,7 @@ static int dm_list_get(MountPoint **dm_list_head) {
 
                 m->path = node;
                 m->devnum = devnum;
-                LIST_PREPEND(MountPoint, mount_point, *dm_list_head, m);
+                LIST_PREPEND(MountPoint, mount_point, *head, m);
         }
 
         r = 0;
@@ -348,8 +361,14 @@ static int delete_loopback(const char *device) {
         r = ioctl(fd, LOOP_CLR_FD, 0);
         close_nointr_nofail(fd);
 
+        if (r >= 0)
+                return 1;
+
         /* ENXIO: not bound, so no error */
-        return (r >= 0 || errno == ENXIO) ? 0 : -errno;
+        if (errno == ENXIO)
+                return 0;
+
+        return -errno;
 }
 
 static int delete_dm(dev_t devnum) {
@@ -375,92 +394,125 @@ static int delete_dm(dev_t devnum) {
         return r >= 0 ? 0 : -errno;
 }
 
-static int mount_points_list_umount(MountPoint **mount_point_list_head) {
-        MountPoint *mp, *mp_next;
-        int failed = 0;
+static int mount_points_list_umount(MountPoint **head, bool *changed) {
+        MountPoint *m, *n;
+        int n_failed = 0;
 
-        LIST_FOREACH_SAFE(mount_point, mp, mp_next, *mount_point_list_head) {
-                if (streq(mp->path, "/"))
+        assert(head);
+
+        LIST_FOREACH_SAFE(mount_point, m, n, *head) {
+                if (streq(m->path, "/"))
                         continue;
 
                 /* Trying to umount. Forcing to umount if busy (only for NFS mounts) */
-                if (umount2(mp->path, MNT_FORCE) == 0)
-                        mount_point_remove_and_free(mp, mount_point_list_head);
-                else {
-                        log_warning("Could not unmount %s: %m", mp->path);
-                        failed++;
+                if (umount2(m->path, MNT_FORCE) == 0) {
+
+                        if (changed)
+                                *changed = true;
+
+                        mount_point_free(head, m);
+                } else {
+                        log_warning("Could not unmount %s: %m", m->path);
+                        n_failed++;
                 }
         }
 
-        return failed;
+        return n_failed;
 }
 
-static int mount_points_list_remount_read_only(MountPoint **mount_point_list_head) {
-        MountPoint *mp, *mp_next;
-        int failed = 0;
+static int mount_points_list_remount_read_only(MountPoint **head, bool *changed) {
+        MountPoint *m, *n;
+        int n_failed = 0;
+
+        assert(head);
+
+        LIST_FOREACH_SAFE(mount_point, m, n, *head) {
 
-        LIST_FOREACH_SAFE(mount_point, mp, mp_next, *mount_point_list_head) {
                 /* Trying to remount read-only */
-                if (mount(NULL, mp->path, NULL, MS_MGC_VAL|MS_REMOUNT|MS_RDONLY, NULL) == 0)
-                        mount_point_remove_and_free(mp, mount_point_list_head);
-                else {
-                        log_warning("Could not remount as read-only %s: %m", mp->path);
-                        failed++;
+                if (mount(NULL, m->path, NULL, MS_MGC_VAL|MS_REMOUNT|MS_RDONLY, NULL) == 0) {
+                        if (changed)
+                                *changed = true;
+
+                        mount_point_free(head, m);
+                } else {
+                        log_warning("Could not remount as read-only %s: %m", m->path);
+                        n_failed++;
                 }
         }
 
-        return failed;
+        return n_failed;
 }
 
-static int swap_points_list_off(MountPoint **swap_list_head) {
-        MountPoint *swap, *swap_next;
-        int failed = 0;
+static int swap_points_list_off(MountPoint **head, bool *changed) {
+        MountPoint *m, *n;
+        int n_failed = 0;
+
+        assert(head);
 
-        LIST_FOREACH_SAFE(mount_point, swap, swap_next, *swap_list_head) {
-                if (swapoff(swap->path) == 0)
-                        mount_point_remove_and_free(swap, swap_list_head);
-                else {
-                        log_warning("Could not deactivate swap %s: %m", swap->path);
-                        failed++;
+        LIST_FOREACH_SAFE(mount_point, m, n, *head) {
+                if (swapoff(m->path) == 0) {
+                        if (changed)
+                                *changed = true;
+
+                        mount_point_free(head, m);
+                } else {
+                        log_warning("Could not deactivate swap %s: %m", m->path);
+                        n_failed++;
                 }
         }
 
-        return failed;
+        return n_failed;
 }
 
-static int loopback_points_list_detach(MountPoint **loopback_list_head) {
-        MountPoint *loopback, *loopback_next;
-        int failed = 0;
+static int loopback_points_list_detach(MountPoint **head, bool *changed) {
+        MountPoint *m, *n;
+        int n_failed = 0;
+
+        assert(head);
+
+        LIST_FOREACH_SAFE(mount_point, m, n, *head) {
+                int r;
 
-        LIST_FOREACH_SAFE(mount_point, loopback, loopback_next, *loopback_list_head) {
-                if (delete_loopback(loopback->path) == 0)
-                        mount_point_remove_and_free(loopback, loopback_list_head);
-                else {
-                        log_warning("Could not delete loopback %s: %m", loopback->path);
-                        failed++;
+                if ((r = delete_loopback(m->path)) >= 0) {
+
+                        if (r > 0 && changed)
+                                *changed = true;
+
+                        mount_point_free(head, m);
+                } else {
+                        log_warning("Could not delete loopback %s: %m", m->path);
+                        n_failed++;
                 }
         }
 
-        return failed;
+        return n_failed;
 }
 
-static int dm_points_list_detach(MountPoint **dm_list_head) {
-        MountPoint *dm, *dm_next;
-        int failed = 0;
+static int dm_points_list_detach(MountPoint **head, bool *changed) {
+        MountPoint *m, *n;
+        int n_failed = 0;
+
+        assert(head);
+
+        LIST_FOREACH_SAFE(mount_point, m, n, *head) {
+                int r;
+
+                if ((r = delete_dm(m->devnum)) >= 0) {
+
+                        if (r > 0 && changed)
+                                *changed = true;
 
-        LIST_FOREACH_SAFE(mount_point, dm, dm_next, *dm_list_head) {
-                if (delete_dm(dm->devnum) == 0)
-                        mount_point_remove_and_free(dm, dm_list_head);
-                else {
-                        log_warning("Could not delete dm %s: %m", dm->path);
-                        failed++;
+                        mount_point_free(head, m);
+                } else {
+                        log_warning("Could not delete dm %s: %m", m->path);
+                        n_failed++;
                 }
         }
 
-        return failed;
+        return n_failed;
 }
 
-int umount_all(void) {
+int umount_all(bool *changed) {
         int r;
         LIST_HEAD(MountPoint, mp_list_head);
 
@@ -470,11 +522,11 @@ int umount_all(void) {
         if (r < 0)
                 goto end;
 
-        r = mount_points_list_umount(&mp_list_head);
+        r = mount_points_list_umount(&mp_list_head, changed);
         if (r <= 0)
                 goto end;
 
-        r = mount_points_list_remount_read_only(&mp_list_head);
+        r = mount_points_list_remount_read_only(&mp_list_head, changed);
 
   end:
         mount_points_list_free(&mp_list_head);
@@ -482,7 +534,7 @@ int umount_all(void) {
         return r;
 }
 
-int swapoff_all(void) {
+int swapoff_all(bool *changed) {
         int r;
         LIST_HEAD(MountPoint, swap_list_head);
 
@@ -492,7 +544,7 @@ int swapoff_all(void) {
         if (r < 0)
                 goto end;
 
-        r = swap_points_list_off(&swap_list_head);
+        r = swap_points_list_off(&swap_list_head, changed);
 
   end:
         mount_points_list_free(&swap_list_head);
@@ -500,7 +552,7 @@ int swapoff_all(void) {
         return r;
 }
 
-int loopback_detach_all(void) {
+int loopback_detach_all(bool *changed) {
         int r;
         LIST_HEAD(MountPoint, loopback_list_head);
 
@@ -510,7 +562,7 @@ int loopback_detach_all(void) {
         if (r < 0)
                 goto end;
 
-        r = loopback_points_list_detach(&loopback_list_head);
+        r = loopback_points_list_detach(&loopback_list_head, changed);
 
   end:
         mount_points_list_free(&loopback_list_head);
@@ -518,7 +570,7 @@ int loopback_detach_all(void) {
         return r;
 }
 
-int dm_detach_all(void) {
+int dm_detach_all(bool *changed) {
         int r;
         LIST_HEAD(MountPoint, dm_list_head);
 
@@ -528,7 +580,7 @@ int dm_detach_all(void) {
         if (r < 0)
                 goto end;
 
-        r = dm_points_list_detach(&dm_list_head);
+        r = dm_points_list_detach(&dm_list_head, changed);
 
   end:
         mount_points_list_free(&dm_list_head);
diff --git a/src/umount.h b/src/umount.h
index c06ca867a9..acdf09acf1 100644
--- a/src/umount.h
+++ b/src/umount.h
@@ -22,12 +22,12 @@
   along with systemd; If not, see <http://www.gnu.org/licenses/>.
 ***/
 
-int umount_all(void);
+int umount_all(bool *changed);
 
-int swapoff_all(void);
+int swapoff_all(bool *changed);
 
-int loopback_detach_all(void);
+int loopback_detach_all(bool *changed);
 
-int dm_detach_all(void);
+int dm_detach_all(bool *changed);
 
 #endif