On Sat, Oct 5, 2024 at 12:16 AM Jan Kara <jack@xxxxxxx> wrote:
>
> When we remount filesystem with 'abort' mount option while changing
> other mount options as well (as is LTP test doing), we can return error
> from the system call after commit d3476f3dad4a ("ext4: don't set
> SB_RDONLY after filesystem errors") because the application of mount
> option changes detects shutdown filesystem and refuses to do anything.
> The behavior of application of other mount options in presence of
> 'abort' mount option is currently rather arbitary as some mount option
> changes are handled before 'abort' and some after it.
>
> Move aborting of the filesystem to the end of remount handling so all
> requested changes are properly applied before the filesystem is shutdown
> to have a reasonably consistent behavior.
>
> Fixes: d3476f3dad4a ("ext4: don't set SB_RDONLY after filesystem errors")
> Reported-by: Jan Stancek <jstancek@xxxxxxxxxx>
> Link: https://lore.kernel.org/all/Zvp6L+oFnfASaoHl@t14s
> Signed-off-by: Jan Kara <jack@xxxxxxx>
In case the mount option isn't getting deprecated right away:
Tested-by: Jan Stancek <jstancek@xxxxxxxxxx>
> ---
> fs/ext4/super.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 16a4ce704460..4645f1629673 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -6518,9 +6518,6 @@ static int __ext4_remount(struct fs_context *fc, struct super_block *sb)
> goto restore_opts;
> }
>
> - if (test_opt2(sb, ABORT))
> - ext4_abort(sb, ESHUTDOWN, "Abort forced by user");
> -
> sb->s_flags = (sb->s_flags & ~SB_POSIXACL) |
> (test_opt(sb, POSIX_ACL) ? SB_POSIXACL : 0);
>
> @@ -6689,6 +6686,14 @@ static int __ext4_remount(struct fs_context *fc, struct super_block *sb)
> if (!ext4_has_feature_mmp(sb) || sb_rdonly(sb))
> ext4_stop_mmpd(sbi);
>
> + /*
> + * Handle aborting the filesystem as the last thing during remount to
> + * avoid obsure errors during remount when some option changes fail to
> + * apply due to shutdown filesystem.
> + */
> + if (test_opt2(sb, ABORT))
> + ext4_abort(sb, ESHUTDOWN, "Abort forced by user");
> +
> return 0;
>
> restore_opts:
> --
> 2.35.3
>
