101 lines
4.5 KiB
Diff
101 lines
4.5 KiB
Diff
From 207b45b46c41b84cbd2be29331c105308d66b3bd Mon Sep 17 00:00:00 2001
|
|
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
|
|
Date: Thu, 11 May 2017 12:12:41 -0400
|
|
Subject: [PATCH] pid1: improve logging when failing to remount / ro (#5940)
|
|
|
|
https://bugzilla.redhat.com/show_bug.cgi?id=1227736#c49
|
|
|
|
We counted how many filesystems could not be unmounted, but only for those
|
|
filesystems which we tried to unmount. Since we only remount / ro, without
|
|
attempting to unmount, we would emit a confusing error message:
|
|
|
|
Remounting '/' read-only with options 'seclabel,space_cache,subvolid=5,subvol=/'.
|
|
Remounting '/' read-only with options 'seclabel,space_cache,subvolid=5,subvol=/'.
|
|
Remounting '/' read-only with options 'seclabel,space_cache,subvolid=5,subvol=/'.
|
|
All filesystems unmounted.
|
|
|
|
Warn when remount-ro fails, and for filesystems which we won't try to unmount,
|
|
include the failure to remount-ro in n_failed.
|
|
|
|
A few minor cleanups:
|
|
- remove unecessary goto which jumps to the next line anyway
|
|
- always calculate n_failed, even if log_error is false. This causes no change
|
|
in behaviour, but I think the code is easier to follow, since the log setting
|
|
cannot influence other logic.
|
|
(cherry picked from commit c826cd3f7cfd950c8a86d57dfa6303f70de3e207)
|
|
---
|
|
src/core/umount.c | 33 ++++++++++++++++++++-------------
|
|
1 file changed, 20 insertions(+), 13 deletions(-)
|
|
|
|
diff --git a/src/core/umount.c b/src/core/umount.c
|
|
index 2f4b12bdb9..77b5bd9556 100644
|
|
--- a/src/core/umount.c
|
|
+++ b/src/core/umount.c
|
|
@@ -369,6 +369,14 @@ static int delete_dm(dev_t devnum) {
|
|
return 0;
|
|
}
|
|
|
|
+static bool nonunmountable_path(const char *path) {
|
|
+ return path_equal(path, "/")
|
|
+#ifndef HAVE_SPLIT_USR
|
|
+ || path_equal(path, "/usr")
|
|
+#endif
|
|
+ || path_startswith(path, "/run/initramfs");
|
|
+}
|
|
+
|
|
static int mount_points_list_umount(MountPoint **head, bool *changed, bool log_error) {
|
|
MountPoint *m, *n;
|
|
int n_failed = 0;
|
|
@@ -404,21 +412,21 @@ static int mount_points_list_umount(MountPoint **head, bool *changed, bool log_e
|
|
* somehwere else via a bind mount. If we
|
|
* explicitly remount the super block of that
|
|
* alias read-only we hence should be
|
|
- * relatively safe regarding keeping the fs we
|
|
- * can otherwise not see dirty. */
|
|
+ * relatively safe regarding keeping dirty an fs
|
|
+ * we cannot otherwise see. */
|
|
log_info("Remounting '%s' read-only with options '%s'.", m->path, options);
|
|
- (void) mount(NULL, m->path, NULL, MS_REMOUNT|MS_RDONLY, options);
|
|
+ if (mount(NULL, m->path, NULL, MS_REMOUNT|MS_RDONLY, options) < 0) {
|
|
+ if (log_error)
|
|
+ log_notice_errno(errno, "Failed to remount '%s' read-only: %m", m->path);
|
|
+ if (nonunmountable_path(m->path))
|
|
+ n_failed++;
|
|
+ }
|
|
}
|
|
|
|
/* Skip / and /usr since we cannot unmount that
|
|
* anyway, since we are running from it. They have
|
|
* already been remounted ro. */
|
|
- if (path_equal(m->path, "/")
|
|
-#ifndef HAVE_SPLIT_USR
|
|
- || path_equal(m->path, "/usr")
|
|
-#endif
|
|
- || path_startswith(m->path, "/run/initramfs")
|
|
- )
|
|
+ if (nonunmountable_path(m->path))
|
|
continue;
|
|
|
|
/* Trying to umount. We don't force here since we rely
|
|
@@ -430,8 +438,9 @@ static int mount_points_list_umount(MountPoint **head, bool *changed, bool log_e
|
|
*changed = true;
|
|
|
|
mount_point_free(head, m);
|
|
- } else if (log_error) {
|
|
- log_warning_errno(errno, "Could not unmount %s: %m", m->path);
|
|
+ } else {
|
|
+ if (log_error)
|
|
+ log_warning_errno(errno, "Could not unmount %s: %m", m->path);
|
|
n_failed++;
|
|
}
|
|
}
|
|
@@ -555,8 +564,6 @@ int umount_all(bool *changed) {
|
|
|
|
/* umount one more time with logging enabled */
|
|
r = mount_points_list_umount(&mp_list_head, &umount_changed, true);
|
|
- if (r <= 0)
|
|
- goto end;
|
|
|
|
end:
|
|
mount_points_list_free(&mp_list_head);
|