Re: [PATCH] hfsplus: Add module parameter to enable force writes

[Andrew Morton]

On Fri, 2 Dec 2022 06:01:16 +0000 Aditya Garg <gargaditya08@xxxxxxxx> wrote:

> From: Aditya Garg <gargaditya08@xxxxxxxx>
> 
> This patch enables users to permanently enable writes of HFS+ locked
> and/or journaled volumes using a module parameter.
> 
> Why module parameter?
> Reason being, its not convenient to manually mount the volume with force
> everytime. There are use cases which are fine with force enabling writes
> on journaled volumes. I've seen many on various online forums and I am one
> of them as well.
> 
> Isn't it risky?
> Yes obviously it is, as the driver itself warns users for the same. But
> any user using the parameter obviously shall be well aware of the risks
> involved. To be honest, I've been writing on a 100Gb journaled volume for
> a few days, including both large and small files, and haven't faced any
> corruption yet.
> 

Presumably anyone who enables this knows the risk, and if it's a
convenience, why not.

Documentation/filesystems/hfsplus.rst would be a good place to document
this module parameter please.

> --- a/fs/hfsplus/super.c
> +++ b/fs/hfsplus/super.c
> @@ -459,12 +477,20 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
>  	} else if (test_and_clear_bit(HFSPLUS_SB_FORCE, &sbi->flags)) {
>  		/* nothing */
>  	} else if (vhdr->attributes & cpu_to_be32(HFSPLUS_VOL_SOFTLOCK)) {
> -		pr_warn("Filesystem is marked locked, mounting read-only.\n");
> -		sb->s_flags |= SB_RDONLY;
> +		if (force_locked_rw) {
> +			pr_warn("Filesystem is marked locked, but writes have been force enabled.\n");
> +		} else {
> +			pr_warn("Filesystem is marked locked, mounting read-only.\n");
> +			sb->s_flags |= SB_RDONLY;
> +		}
>  	} else if ((vhdr->attributes & cpu_to_be32(HFSPLUS_VOL_JOURNALED)) &&
>  			!sb_rdonly(sb)) {
> -		pr_warn("write access to a journaled filesystem is not supported, use the force option at your own risk, mounting read-only.\n");
> -		sb->s_flags |= SB_RDONLY;
> +		if (force_journaled_rw) {
> +			pr_warn("write access to a journaled filesystem is not supported, but has been force enabled.\n");
> +		} else {
> +			pr_warn("write access to a journaled filesystem is not supported, use the force option at your own risk, mounting read-only.\n");
> +			sb->s_flags |= SB_RDONLY;
> +		}

All these super long lines are an eyesore.  How about

			pr_warn("write access to a journaled filesystem is "
				"not supported, but has been force enabled.\n");