From: Aditya Garg <[email protected]>
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.
Signed-off-by: Aditya Garg <[email protected]>
---
fs/hfsplus/super.c | 46 ++++++++++++++++++++++++++++++++++++----------
1 file changed, 36 insertions(+), 10 deletions(-)
diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index 122ed89eb..2367a2407 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -24,6 +24,16 @@ static void hfsplus_free_inode(struct inode *inode);
#include "hfsplus_fs.h"
#include "xattr.h"
+static unsigned int force_journaled_rw;
+module_param(force_journaled_rw, uint, 0644);
+MODULE_PARM_DESC(force_journaled_rw, "Force enable writes on Journaled HFS+ volumes. "
+ "([0] = disabled, 1 = enabled)");
+
+static unsigned int force_locked_rw;
+module_param(force_locked_rw, uint, 0644);
+MODULE_PARM_DESC(force_locked_rw, "Force enable writes on locked HFS+ volumes. "
+ "([0] = disabled, 1 = enabled)");
+
static int hfsplus_system_read_inode(struct inode *inode)
{
struct hfsplus_vh *vhdr = HFSPLUS_SB(inode->i_sb)->s_vhdr;
@@ -346,14 +356,22 @@ static int hfsplus_remount(struct super_block *sb, int *flags, char *data)
/* nothing */
} else if (vhdr->attributes &
cpu_to_be32(HFSPLUS_VOL_SOFTLOCK)) {
- pr_warn("filesystem is marked locked, leaving read-only.\n");
- sb->s_flags |= SB_RDONLY;
- *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, leaving read-only.\n");
+ sb->s_flags |= SB_RDONLY;
+ *flags |= SB_RDONLY;
+ }
} else if (vhdr->attributes &
cpu_to_be32(HFSPLUS_VOL_JOURNALED)) {
- pr_warn("filesystem is marked journaled, leaving read-only.\n");
- sb->s_flags |= SB_RDONLY;
- *flags |= SB_RDONLY;
+ if (force_journaled_rw) {
+ pr_warn("filesystem is marked journaled, but writes have been force enabled.\n");
+ } else {
+ pr_warn("filesystem is marked journaled, leaving read-only.\n");
+ sb->s_flags |= SB_RDONLY;
+ *flags |= SB_RDONLY;
+ }
}
}
return 0;
@@ -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;
+ }
}
err = -EINVAL;
--
2.38.1
> On Dec 1, 2022, at 10:01 PM, Aditya Garg <[email protected]> wrote:
>
> From: Aditya Garg <[email protected]>
>
> 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.
>
If you created HFS+ volume under Linux, then you never have journal
and problem of journal replay (even if you created journaled volume).
So, I see the only one case when you have journal with transactions.
You are using HFS+ volume in Linux and Mac OS X. It means that
Mac OS X can create transactions in the journal and Linux needs
to manage it somehow.
Even if you don’t see any corruptions after such short testing, then
it doesn’t mean that you are safe. The key trouble that you can
silently lose the data because some metadata state could sit
in the journal and no replay operation has happened. Yes, you can
ignore the transactions in the journal and continue to store data and
modify metadata. But if journal still contain valid transactions, then
mount operation under Mac OS X will replay journal. And it sounds
that journal replay under Mac OS X can corrupt metadata and data
state that was modified/created under Linux.
So, I believe that your suggestion is slightly dangerous because
people loves to make mistakes and really hates to lose data.
Thanks,
Slava.
> Signed-off-by: Aditya Garg <[email protected]>
> ---
> fs/hfsplus/super.c | 46 ++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 36 insertions(+), 10 deletions(-)
>
> diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
> index 122ed89eb..2367a2407 100644
> --- a/fs/hfsplus/super.c
> +++ b/fs/hfsplus/super.c
> @@ -24,6 +24,16 @@ static void hfsplus_free_inode(struct inode *inode);
> #include "hfsplus_fs.h"
> #include "xattr.h"
>
> +static unsigned int force_journaled_rw;
> +module_param(force_journaled_rw, uint, 0644);
> +MODULE_PARM_DESC(force_journaled_rw, "Force enable writes on Journaled HFS+ volumes. "
> + "([0] = disabled, 1 = enabled)");
> +
> +static unsigned int force_locked_rw;
> +module_param(force_locked_rw, uint, 0644);
> +MODULE_PARM_DESC(force_locked_rw, "Force enable writes on locked HFS+ volumes. "
> + "([0] = disabled, 1 = enabled)");
> +
> static int hfsplus_system_read_inode(struct inode *inode)
> {
> struct hfsplus_vh *vhdr = HFSPLUS_SB(inode->i_sb)->s_vhdr;
> @@ -346,14 +356,22 @@ static int hfsplus_remount(struct super_block *sb, int *flags, char *data)
> /* nothing */
> } else if (vhdr->attributes &
> cpu_to_be32(HFSPLUS_VOL_SOFTLOCK)) {
> - pr_warn("filesystem is marked locked, leaving read-only.\n");
> - sb->s_flags |= SB_RDONLY;
> - *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, leaving read-only.\n");
> + sb->s_flags |= SB_RDONLY;
> + *flags |= SB_RDONLY;
> + }
> } else if (vhdr->attributes &
> cpu_to_be32(HFSPLUS_VOL_JOURNALED)) {
> - pr_warn("filesystem is marked journaled, leaving read-only.\n");
> - sb->s_flags |= SB_RDONLY;
> - *flags |= SB_RDONLY;
> + if (force_journaled_rw) {
> + pr_warn("filesystem is marked journaled, but writes have been force enabled.\n");
> + } else {
> + pr_warn("filesystem is marked journaled, leaving read-only.\n");
> + sb->s_flags |= SB_RDONLY;
> + *flags |= SB_RDONLY;
> + }
> }
> }
> return 0;
> @@ -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;
> + }
> }
>
> err = -EINVAL;
> --
> 2.38.1
>
On Fri, 2 Dec 2022 06:01:16 +0000 Aditya Garg <[email protected]> wrote:
> From: Aditya Garg <[email protected]>
>
> 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");
On Fri, Dec 02, 2022 at 12:53:44PM -0800, Andrew Morton wrote:
> > + 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");
Linus has asked us to not do that because it makes it hard to grep.
On Fri, 2 Dec 2022 21:01:44 +0000 Matthew Wilcox <[email protected]> wrote:
> On Fri, Dec 02, 2022 at 12:53:44PM -0800, Andrew Morton wrote:
> > > + 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");
>
> Linus has asked us to not do that because it makes it hard to grep.
Yup. But as with everything, there are tradeoffs. These messages are
so messy to read and reading code is more common than grepping for
error messages. Just grep the first 20-30 characters...
> 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.
I’ll add it to the v2
> 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");
I’ll fix this, but you'll find a lot of eyesore lines in this driver, which I guess someone would have to fix then.
From: Aditya Garg <[email protected]>
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.
Signed-off-by: Aditya Garg <[email protected]>
---
v2 :- Add Documentation and split long lines across multiple lines.
Documentation/filesystems/hfsplus.rst | 15 +++++++-
fs/hfsplus/super.c | 49 +++++++++++++++++++++------
2 files changed, 53 insertions(+), 11 deletions(-)
diff --git a/Documentation/filesystems/hfsplus.rst b/Documentation/filesystems/hfsplus.rst
index f02f4f5fc..85feca0b0 100644
--- a/Documentation/filesystems/hfsplus.rst
+++ b/Documentation/filesystems/hfsplus.rst
@@ -46,13 +46,26 @@ When mounting an HFSPlus filesystem, the following options are accepted:
Do not decompose file name characters.
force
- Used to force write access to volumes that are marked as journalled
+ Used to force write access to volumes that are marked as journaled
or locked. Use at your own risk.
nls=cccc
Encoding to use when presenting file names.
+Module Parameters
+=================
+
+The HFSPlus module supports the following module parameters:
+
+ force_journaled_rw=n, force_locked_rw=n
+ Used to force enable writes on volumes marked as journaled/locked.
+ Its risky to use them as they may cause data corruption.
+ Accepted values:
+ 0 (default): Disables writes
+ 1: Enables writes
+
+
References
==========
diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index 122ed89eb..91812c4c3 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -24,6 +24,16 @@ static void hfsplus_free_inode(struct inode *inode);
#include "hfsplus_fs.h"
#include "xattr.h"
+static unsigned int force_journaled_rw;
+module_param(force_journaled_rw, uint, 0644);
+MODULE_PARM_DESC(force_journaled_rw, "Force enable writes on Journaled HFS+ volumes. "
+ "([0] = disabled, 1 = enabled)");
+
+static unsigned int force_locked_rw;
+module_param(force_locked_rw, uint, 0644);
+MODULE_PARM_DESC(force_locked_rw, "Force enable writes on locked HFS+ volumes. "
+ "([0] = disabled, 1 = enabled)");
+
static int hfsplus_system_read_inode(struct inode *inode)
{
struct hfsplus_vh *vhdr = HFSPLUS_SB(inode->i_sb)->s_vhdr;
@@ -346,14 +356,22 @@ static int hfsplus_remount(struct super_block *sb, int *flags, char *data)
/* nothing */
} else if (vhdr->attributes &
cpu_to_be32(HFSPLUS_VOL_SOFTLOCK)) {
- pr_warn("filesystem is marked locked, leaving read-only.\n");
- sb->s_flags |= SB_RDONLY;
- *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, leaving read-only.\n");
+ sb->s_flags |= SB_RDONLY;
+ *flags |= SB_RDONLY;
+ }
} else if (vhdr->attributes &
cpu_to_be32(HFSPLUS_VOL_JOURNALED)) {
- pr_warn("filesystem is marked journaled, leaving read-only.\n");
- sb->s_flags |= SB_RDONLY;
- *flags |= SB_RDONLY;
+ if (force_journaled_rw) {
+ pr_warn("filesystem is marked journaled, but writes have been force enabled.\n");
+ } else {
+ pr_warn("filesystem is marked journaled, leaving read-only.\n");
+ sb->s_flags |= SB_RDONLY;
+ *flags |= SB_RDONLY;
+ }
}
}
return 0;
@@ -459,12 +477,23 @@ 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;
+ }
}
err = -EINVAL;
--
2.34.1
Hi Aditya,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on akpm-mm/mm-everything]
[also build test WARNING on linus/master v6.1-rc7 next-20221202]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Aditya-Garg/hfsplus-Add-module-parameter-to-enable-force-writes/20221203-151143
base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link: https://lore.kernel.org/r/79C58B25-8ECF-432B-AE90-2194921DB797%40live.com
patch subject: [PATCH v2] hfsplus: Add module parameter to enable force writes
reproduce:
# https://github.com/intel-lab-lkp/linux/commit/dfb483e3c16e562d768f9bddc63252f1cccb0275
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Aditya-Garg/hfsplus-Add-module-parameter-to-enable-force-writes/20221203-151143
git checkout dfb483e3c16e562d768f9bddc63252f1cccb0275
make menuconfig
# enable CONFIG_COMPILE_TEST, CONFIG_WARN_MISSING_DOCUMENTS, CONFIG_WARN_ABI_ERRORS
make htmldocs
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
>> Documentation/filesystems/hfsplus.rst:65: WARNING: Unexpected indentation.
vim +65 Documentation/filesystems/hfsplus.rst
60
61 force_journaled_rw=n, force_locked_rw=n
62 Used to force enable writes on volumes marked as journaled/locked.
63 Its risky to use them as they may cause data corruption.
64 Accepted values:
> 65 0 (default): Disables writes
66 1: Enables writes
67
68
--
0-DAY CI Kernel Test Service
https://01.org/lkp
> On 04-Dec-2022, at 6:35 AM, kernel test robot <[email protected]> wrote:
>
> Hi Aditya,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on akpm-mm/mm-everything]
> [also build test WARNING on linus/master v6.1-rc7 next-20221202]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Aditya-Garg/hfsplus-Add-module-parameter-to-enable-force-writes/20221203-151143
> base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
> patch link: https://lore.kernel.org/r/79C58B25-8ECF-432B-AE90-2194921DB797%40live.com
> patch subject: [PATCH v2] hfsplus: Add module parameter to enable force writes
> reproduce:
> # https://github.com/intel-lab-lkp/linux/commit/dfb483e3c16e562d768f9bddc63252f1cccb0275
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review Aditya-Garg/hfsplus-Add-module-parameter-to-enable-force-writes/20221203-151143
> git checkout dfb483e3c16e562d768f9bddc63252f1cccb0275
> make menuconfig
> # enable CONFIG_COMPILE_TEST, CONFIG_WARN_MISSING_DOCUMENTS, CONFIG_WARN_ABI_ERRORS
> make htmldocs
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <[email protected]>
>
> All warnings (new ones prefixed by >>):
>
>>> Documentation/filesystems/hfsplus.rst:65: WARNING: Unexpected indentation.
>
Do I need to fix this or should I consider it as a false positive?
NAK. Global module options overriding mount options don't make sense.
If you mount so much write yourself a script, or add the option to the
configuration of the automounting tool of your choice.
> On 04-Dec-2022, at 1:37 PM, Christoph Hellwig <[email protected]> wrote:
>
> NAK. Global module options overriding mount options don't make sense.
> If you mount so much write yourself a script, or add the option to the
> configuration of the automounting tool of your choice.
Hi Christoph
My bad I wasn't aware udisks2 now supports custom configuration. I just came to know about this and thus this patch of mine indeed does not make any sense now.
Although, if you think its worth it, the following improvements can be made :-
1. There is no logging showing that writes have been force enabled. We could add that.
2. We could have separate mount options for journaled and locked volumes (although I dunno in what case we get locked volumes).
On Sun, Dec 04, 2022 at 11:01:49AM +0000, Aditya Garg wrote:
> Although, if you think its worth it, the following improvements can be made :-
>
> 1. There is no logging showing that writes have been force enabled. We could add that.
I think this would be very useful.
> 2. We could have separate mount options for journaled and locked volumes (although I dunno in what case we get locked volumes).
We can't really retire the existing option, but if for your use case
you'd prefer to only allow one of them and want to not write to the
other case feel free to submit a patch to add that option.
On Sun, Dec 04, 2022 at 06:43:43AM +0000, Aditya Garg wrote:
>
>
> > On 04-Dec-2022, at 6:35 AM, kernel test robot <[email protected]> wrote:
> >
> > Hi Aditya,
> >
> > Thank you for the patch! Perhaps something to improve:
> >
> > [auto build test WARNING on akpm-mm/mm-everything]
> > [also build test WARNING on linus/master v6.1-rc7 next-20221202]
> > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > And when submitting patch, we suggest to use '--base' as documented in
> > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> >
> > url: https://github.com/intel-lab-lkp/linux/commits/Aditya-Garg/hfsplus-Add-module-parameter-to-enable-force-writes/20221203-151143
> > base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
> > patch link: https://lore.kernel.org/r/79C58B25-8ECF-432B-AE90-2194921DB797%40live.com
> > patch subject: [PATCH v2] hfsplus: Add module parameter to enable force writes
> > reproduce:
> > # https://github.com/intel-lab-lkp/linux/commit/dfb483e3c16e562d768f9bddc63252f1cccb0275
> > git remote add linux-review https://github.com/intel-lab-lkp/linux
> > git fetch --no-tags linux-review Aditya-Garg/hfsplus-Add-module-parameter-to-enable-force-writes/20221203-151143
> > git checkout dfb483e3c16e562d768f9bddc63252f1cccb0275
> > make menuconfig
> > # enable CONFIG_COMPILE_TEST, CONFIG_WARN_MISSING_DOCUMENTS, CONFIG_WARN_ABI_ERRORS
> > make htmldocs
> >
> > If you fix the issue, kindly add following tag where applicable
> > | Reported-by: kernel test robot <[email protected]>
> >
> > All warnings (new ones prefixed by >>):
> >
> >>> Documentation/filesystems/hfsplus.rst:65: WARNING: Unexpected indentation.
> >
>
> Do I need to fix this or should I consider it as a false positive?
Your patch creates the warning, so yes you need to fix it.