[PATCH] f2fs: give RO message when recovering superblock

[PATCH] f2fs: give RO message when recovering superblock

[Jaegeuk Kim]

When one of superblocks is missing, f2fs recovers it with the valid one.
But, even if f2fs is mounted as RO, we'd better notify that too.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/super.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 179304f..9c0af99 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1293,6 +1293,9 @@ int f2fs_commit_super(struct f2fs_sb_info *sbi, bool recover)
 	struct buffer_head *bh;
 	int err;
 
+	if (f2fs_readonly(sbi->sb) || bdev_read_only(sbi->sb->s_bdev))
+		return -EROFS;
+
 	/* write back-up superblock first */
 	bh = sb_getblk(sbi->sb, sbi->valid_super_block ? 0: 1);
 	if (!bh)
@@ -1560,7 +1563,7 @@ try_onemore:
 	kfree(options);
 
 	/* recover broken superblock */
-	if (recovery && !f2fs_readonly(sb) && !bdev_read_only(sb->s_bdev)) {
+	if (recovery) {
 		err = f2fs_commit_super(sbi, true);
 		f2fs_msg(sb, KERN_INFO,
 			"Try to recover %dth superblock, ret: %ld",
-- 
2.6.3

Re: [f2fs-dev] [PATCH] f2fs: give RO message when recovering superblock

[Marc Lehmann]

On Wed, Mar 23, 2016 at 01:38:19PM -0700, Jaegeuk Kim <jaegeuk@kernel.org> wrote:
> When one of superblocks is missing, f2fs recovers it with the valid one.
> But, even if f2fs is mounted as RO, we'd better notify that too.

(I have written this in my other mail, but in case you didn't see it, because
it wasn't directly sent to you, I replied directly).

Basically all other filesystems do not treat "ro" as anything but as a vfs
flag - the mounted volume will be readonly, but they will happily write to
the volume for recovery or integrity purposes. This has been extensively
discussed on lkml in the past and it was decided that overloading "ro" to
have two different meanings is bad.

If f2fs wants to suppress writes, it should use the norecovery option to
decide, not the ro option. This is the behaviour that other filesystems
follow (at least extN, xfs).

Unless f2fs has a very good reason (which I don't think it has), it should
behave like the other filesystems, and treat "ro" merely as a vfs flag to
suppress writing.

There is a third reason to not change the meaning: typically, the root fs
is mounted ro first and later rw. Therefore f2fs must make sure to have
full integrity on a ro mount, even if that means writing to the backing
store. It isn't acceptable to make ro mounts fail when rw mounts would
work, for example, when upgrading the kernel and rebooting.

-- 
                The choice of a       Deliantra, the free code+content MORPG
      -----==-     _GNU_              http://www.deliantra.net
      ----==-- _       generation
      ---==---(_)__  __ ____  __      Marc Lehmann
      --==---/ / _ \/ // /\ \/ /      schmorp@schmorp.de
      -=====/_/_//_/\_,_/ /_/\_\