fsck.f2fs: fix to repair ro mounted device w/ -f

As Hagbard Celine reported:

"
Referring to the output from the fsck running against a "ro"
filesystem, especially this line:
Info: Check FS only due to RO

As far as i can tell this says that opposed to other filesystems
running fsck against a "ro" mounted f2fs partition will never fix any
errors.
So I tried running fsck against the same partition mounted "rw":
- mount -o remount,rw /mnt/f2fstest/
- fsck.f2fs  -f /dev/nvme0n1p7
Info: Force to fix corruption
Info: Mounted device!
        Error: Not available on mounted device!

I might be misunderstanding something, but all this tells me that
unless one make a custom initramfs that runs fsck before root is
mounted (something no distributions has, as far as I know), fsck will
never fix an f2fs formatted root partition during boot.
If this is by design and not a bug/unintended behavior, it should be
documented somewhere least more people will experience system crashes
like mine.

All tests above done with kernel 5.0.5 and f2fs-tools 1.12.0 with
"fsck.f2fs: allow to fsck readonly image w/ -f option"-patch by Chao
Yu.
"

We try to make our fsck behavior keeping line with e2fsprogs, but w/
-f option, we can just check a RO mounted device rather repair it, so
let's fix this.

Reported-and-Tested-by: Hagbard Celine <hagbardcelin@gmail.com>
Signed-off-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

diff --git a/fsck/fsck.c b/fsck/fsck.c
index a17555c..3b54878 100644 (file)
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -199,7 +199,7 @@ static int is_valid_ssa_node_blk(struct f2fs_sb_info *sbi, u32 nid,
                        need_fix = 1;
                }
        }
-       if (need_fix && !c.ro) {
+       if (need_fix && f2fs_dev_is_writable()) {
                u64 ssa_blk;
                int ret2;
 
@@ -325,7 +325,7 @@ static int is_valid_ssa_data_blk(struct f2fs_sb_info *sbi, u32 blk_addr,
                        need_fix = 1;
                }
        }
-       if (need_fix && !c.ro) {
+       if (need_fix && f2fs_dev_is_writable()) {
                u64 ssa_blk;
                int ret2;
 
@@ -1015,7 +1015,7 @@ skip_blkcnt_fix:
        }
 
        /* drop extent information to avoid potential wrong access */
-       if (need_fix && !c.ro)
+       if (need_fix && f2fs_dev_is_writable())
                node_blk->i.i_ext.len = 0;
 
        if ((c.feature & cpu_to_le32(F2FS_FEATURE_INODE_CHKSUM)) &&
@@ -1038,7 +1038,7 @@ skip_blkcnt_fix:
                }
        }
 
-       if (need_fix && !c.ro) {
+       if (need_fix && f2fs_dev_is_writable()) {
                ret = dev_write_block(node_blk, ni->blk_addr);
                ASSERT(ret >= 0);
        }
@@ -1073,7 +1073,7 @@ int fsck_chk_dnode_blk(struct f2fs_sb_info *sbi, struct f2fs_inode *inode,
                        FIX_MSG("[0x%x] dn.addr[%d] = 0", nid, idx);
                }
        }
-       if (need_fix && !c.ro) {
+       if (need_fix && f2fs_dev_is_writable()) {
                ret = dev_write_block(node_blk, ni->blk_addr);
                ASSERT(ret >= 0);
        }
@@ -1110,7 +1110,7 @@ skip:
                }
        }
 
-       if (need_fix && !c.ro) {
+       if (need_fix && f2fs_dev_is_writable()) {
                struct node_info ni;
                nid_t nid = le32_to_cpu(node_blk->footer.nid);
 
@@ -1152,7 +1152,7 @@ skip:
                }
        }
 
-       if (need_fix && !c.ro) {
+       if (need_fix && f2fs_dev_is_writable()) {
                struct node_info ni;
                nid_t nid = le32_to_cpu(node_blk->footer.nid);
 
@@ -1595,7 +1595,7 @@ int fsck_chk_dentry_blk(struct f2fs_sb_info *sbi, u32 blk_addr,
                        de_blk->dentry, de_blk->filename,
                        NR_DENTRY_IN_BLOCK, last_blk, enc_name);
 
-       if (dentries < 0 && !c.ro) {
+       if (dentries < 0 && f2fs_dev_is_writable()) {
                ret = dev_write_block(de_blk, blk_addr);
                ASSERT(ret >= 0);
                DBG(1, "[%3d] Dentry Block [0x%x] Fixed hash_codes\n\n",
@@ -1708,7 +1708,7 @@ int fsck_chk_orphan_node(struct f2fs_sb_info *sbi)
                        else if (ret)
                                ASSERT_MSG("[0x%x] wrong orphan inode", ino);
                }
-               if (!c.ro && c.fix_on &&
+               if (f2fs_dev_is_writable() && c.fix_on &&
                                entry_count != new_entry_count) {
                        new_blk->entry_count = cpu_to_le32(new_entry_count);
                        ret = dev_write_block(new_blk, start_blk + i);
@@ -2702,7 +2702,7 @@ int fsck_verify(struct f2fs_sb_info *sbi)
        }
 #endif
        /* fix global metadata */
-       if (force || (c.fix_on && !c.ro)) {
+       if (force || (c.fix_on && f2fs_dev_is_writable())) {
                struct f2fs_checkpoint *cp = F2FS_CKPT(sbi);
 
                if (force || c.bug_on || c.bug_nat_bits) {

diff --git a/fsck/main.c b/fsck/main.c
index d3f0c0d..03076d9 100644 (file)
--- a/fsck/main.c
+++ b/fsck/main.c
@@ -757,9 +757,13 @@ int main(int argc, char **argv)
                }
 
                /* allow ro-mounted partition */
-               MSG(0, "Info: Check FS only due to RO\n");
-               c.fix_on = 0;
-               c.auto_fix = 0;
+               if (c.force) {
+                       MSG(0, "Info: Force to check/repair FS on RO mounted device\n");
+               } else {
+                       MSG(0, "Info: Check FS only on RO mounted device\n");
+                       c.fix_on = 0;
+                       c.auto_fix = 0;
+               }
        }
 
        /* Get device */

diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index e84cccc..b4f992f 100644 (file)
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -1149,6 +1149,7 @@ extern int f2fs_crc_valid(u_int32_t blk_crc, void *buf, int len);
 
 extern void f2fs_init_configuration(void);
 extern int f2fs_devs_are_umounted(void);
+extern int f2fs_dev_is_writable(void);
 extern int f2fs_dev_is_umounted(char *);
 extern int f2fs_get_device_info(void);
 extern int get_device_info(int);

diff --git a/lib/libf2fs.c b/lib/libf2fs.c
index 60b84e0..0f831c6 100644 (file)
--- a/lib/libf2fs.c
+++ b/lib/libf2fs.c
@@ -630,6 +630,11 @@ void f2fs_init_configuration(void)
        c.root_gid = getgid();
 }
 
+int f2fs_dev_is_writable(void)
+{
+       return !c.ro || c.force;
+}
+
 #ifdef HAVE_SETMNTENT
 static int is_mounted(const char *mpt, const char *device)
 {