shaba/systemd-stable
1
1
Fork 0
mirror of https://github.com/systemd/systemd-stable.git synced 2025-01-25 06:03:40 +03:00
Code

core: systemd-shutdown: avoid confusingly redundant messages

Browse Source 
After previous output from systemd-shutdown indicated a bug, my attention
was drawn to redundant output lines.  Did they indicate an anomaly?

It turns out to be an expected, harmless result of the current code.  But
we don't have much justification to run such redundant operations.  Let's
remove the confusing redundant message.

We can stop trying to remount a directory read-only once its mount entry
has successfully been changed to "ro".  We can simply let the kernel keep
track of this for us.  I don't bother to try and avoid re-parsing the
mountinfo.  I appreciate snappy shutdowns, but this code is already
intricate and buggy enough (see issue 7131).

(Disclaimer: At least for the moment, you can't _rely_ on always seeing
suspicious output from systemd-shutdown.  By default, you can expect the
kernel to truncate the log output of systemd-shutdown.  Ick ick ick!
Because /dev/kmsg is rate-limited by default.  Normally it prints a message
"X lines supressed", but we tend to shut down before the timer expires
in this case).

Before:

systemd-shutdown[1]: Remounting '/' read-only with options 'seclabel...
EXT4-fs (vda3): re-mounted. Opts: data=ordered
systemd-shutdown[1]: Remounting '/' read-only with options 'seclabel, ...
EXT4-fs (vda3): re-mounted. Opts: data=ordered

After:

systemd-shutdown[1]: Remounting '/' read-only with options 'seclabel, ...
EXT4-fs (vda3): re-mounted. Opts: data=ordered

I also tested with `systemctl reboot --force`, plus a loopback mount to
cause one of the umounts to fail initially.  In this case another 2 lines
of output are removed (out of a larger number of lines).
This commit is contained in:
Alan Jenkins 2017-10-19 15:02:12 +01:00
parent 066b753dc5
commit 116e6d9643
1 changed files with 25 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
src/core/umount.c
View File

@ -381,13 +381,19 @@ static bool nonunmountable_path(const char *path) {
|| path_startswith(path, "/run/initramfs");
}
/* This includes remounting readonly, which changes the kernel mount options.
* Therefore the list passed to this function is invalidated, and should not be reused. */
static int mount_points_list_umount(MountPoint **head, bool *changed, bool log_error) {
MountPoint *m, *n;
MountPoint *m;
int n_failed = 0;
assert(head);
LIST_FOREACH_SAFE(mount_point, m, n, *head) {
LIST_FOREACH(mount_point, m, *head) {
bool mount_is_readonly;
mount_is_readonly = fstab_test_yes_no_option(m->options, "ro\0rw\0");
/* If we are in a container, don't attempt to
read-only mount anything as that brings no real
@ -397,7 +403,8 @@ static int mount_points_list_umount(MountPoint **head, bool *changed, bool log_e
remount. It brings no value (we cannot leave
a "dirty fs") and could hang if the network is down. */
if (detect_container() <= 0 &&
!fstype_is_network(m->type)) {
!fstype_is_network(m->type) &&
!mount_is_readonly) {
_cleanup_free_ char *options = NULL;
/* MS_REMOUNT requires that the data parameter
* should be the same from the original mount
@ -444,8 +451,6 @@ static int mount_points_list_umount(MountPoint **head, bool *changed, bool log_e
if (umount2(m->path, 0) == 0) {
if (changed)
*changed = true;
mount_point_free(head, m);
} else {
if (log_error)
log_warning_errno(errno, "Could not unmount %s: %m", m->path);
@ -550,9 +555,8 @@ static int dm_points_list_detach(MountPoint **head, bool *changed) {
return n_failed;
}
int umount_all(bool *changed) {
static int umount_all_once(bool *changed, bool log_error) {
int r;
bool umount_changed;
LIST_HEAD(MountPoint, mp_list_head);
LIST_HEAD_INIT(mp_list_head);
@ -560,24 +564,32 @@ int umount_all(bool *changed) {
if (r < 0)
goto end;
r = mount_points_list_umount(&mp_list_head, changed, log_error);
end:
mount_points_list_free(&mp_list_head);
return r;
}
int umount_all(bool *changed) {
bool umount_changed;
int r;
/* retry umount, until nothing can be umounted anymore */
do {
umount_changed = false;
mount_points_list_umount(&mp_list_head, &umount_changed, false);
umount_all_once(&umount_changed, false);
if (umount_changed)
*changed = true;
} while (umount_changed);
/* umount one more time with logging enabled */
r = mount_points_list_umount(&mp_list_head, &umount_changed, true);
r = umount_all_once(&umount_changed, true);
if (umount_changed)
*changed = true;
end:
mount_points_list_free(&mp_list_head);
return r;
}