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

From: Andrew Morton
Date: Fri Dec 02 2022 - 15:53:54 EST


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");