Re: [PATCH v2 3/8] ext4: reject the 'data_err=abort' option in nojournal mode

[Jan Kara]

On Tue 21-01-25 15:10:45, libaokun@xxxxxxxxxxxxxxx wrote:
> From: Baokun Li <libaokun1@xxxxxxxxxx>
> 
> data_err=abort aborts the journal on I/O errors. However, this option is
> meaningless if journal is disabled, so it is rejected in nojournal mode
> to reduce unnecessary checks. Also, this option is ignored upon remount.
> 
> Signed-off-by: Baokun Li <libaokun1@xxxxxxxxxx>
> Reviewed-by: Zhang Yi <yi.zhang@xxxxxxxxxx>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

> ---
> Or maybe we should make mount and remount consistent?
> Rejecting it in both would make things a lot easier.

The reason why it is ignored on remount is that we cannot modify
JBD2_ABORT_ON_SYNCDATA_ERR on remount. Now that we don't really depend on
JBD2_ABORT_ON_SYNCDATA_ERR, we could indeed make mount and remount
consistent and probably just drop JBD2_ABORT_ON_SYNCDATA_ERR altogether
(and only warn in jbd2 as it used to be long ago).

								Honza
> 
>  fs/ext4/super.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index a50e5c31b937..34a7b6523f8b 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -2785,6 +2785,13 @@ static int ext4_check_opt_consistency(struct fs_context *fc,
>  	}
>  
>  	if (is_remount) {
> +		if (!sbi->s_journal &&
> +		    ctx_test_mount_opt(ctx, EXT4_MOUNT_DATA_ERR_ABORT)) {
> +			ext4_msg(NULL, KERN_WARNING,
> +				 "Remounting fs w/o journal so ignoring data_err option");
> +			ctx_clear_mount_opt(ctx, EXT4_MOUNT_DATA_ERR_ABORT);
> +		}
> +
>  		if (ctx_test_mount_opt(ctx, EXT4_MOUNT_DAX_ALWAYS) &&
>  		    (test_opt(sb, DATA_FLAGS) == EXT4_MOUNT_JOURNAL_DATA)) {
>  			ext4_msg(NULL, KERN_ERR, "can't mount with "
> @@ -5428,6 +5435,11 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
>  				 "data=, fs mounted w/o journal");
>  			goto failed_mount3a;
>  		}
> +		if (test_opt(sb, DATA_ERR_ABORT)) {
> +			ext4_msg(sb, KERN_ERR,
> +				 "can't mount with data_err=abort, fs mounted w/o journal");
> +			goto failed_mount3a;
> +		}
>  		sbi->s_def_mount_opt &= ~EXT4_MOUNT_JOURNAL_CHECKSUM;
>  		clear_opt(sb, JOURNAL_CHECKSUM);
>  		clear_opt(sb, DATA_FLAGS);
> -- 
> 2.39.2
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR