[PATCH v2 0/6] btrfs-progs: check: Repair invalid inode mode in subvolume trees

[PATCH v2 0/6] btrfs-progs: check: Repair invalid inode mode in subvolume trees

[Qu Wenruo]

Before this patch, btrfs check can only repair bad free space cache
inode mode (as it was the first case detected by tree-checker and reported)

But Murphy is always right, what may happen will finally happen, we have
users reporting bad inode mode in subvolume trees.
According to the creation time, it looks like some older kernel around
2014 is causing the problem.

Although the reported get the fs fixed by removing the offending old
files, it's still a bad thing that "btrfs check" can't fix it.

This patch will bring the repair functionality to all inodes, along with
needed test image.

The core complexity is in how to determine the correct imode.
This patch will use the following methods to determine the correct
imode:
- INODE_REF
  Do a DIR_INDEX/ITEM search to find a valid filetype then convert it to
  imode. If it works, this should be the most reliable method.

- DIR_INDEX/DIR_ITEM belong to this inode
  Then this inode must be a directory.

- EXTENT_DATA
  This inode can be a regular file or soft link.
  We default to regular file so user can inspect the content to do
  further correction.

- rdev of INODE_ITEM
  If all above fails, and the INODE_ITEM has non-zero rdev, this inode
  must be either BLK or CHR. We default to BLK for this case.

- Error out if nothing matches
  This is to be 100% sure that we won't further corrupt the fs.

Changelog:
v2:
- Implement INODE_REF based imode lookup functionality
- Instead of defaulting to REG, error out if no imode can be found
  To avoid corrupting the fs.

v3:
- Fix two missing "found = true" in two branches
- Update commit message to show the repair flow and its limitation
- Fix coding style on (ret > 0) handling, to make it more explicit
- Make original mode repair more friendly to repair_inode_nlinks()
  This involves the repair timing change and release path from
  reset_imode()
- Update test case image to contain more complex corruption
  Not only regular imode corruption, but also combined with missing
  INODE_REF/DIR_ITEM/DIR_INDEX to test extra imode detection methods.

Qu Wenruo (6):
  btrfs-progs: check: Export btrfs_type_to_imode
  btrfs-progs: check/common: Introduce a function to find imode using
    info from INODE_REF item
  btrfs-progs: check/common: Make repair_imode_common() to handle inodes
    in subvolume trees
  btrfs-progs: check/lowmem: Repair bad imode early
  btrfs-progs: check/original: Fix inode mode in subvolume trees
  btrfs-progs: tests/fsck: Add new images for inode mode repair
    functionality

 check/main.c                                  |  56 ++--
 check/mode-common.c                           | 262 +++++++++++++++++-
 check/mode-common.h                           |  17 ++
 check/mode-lowmem.c                           |  39 +++
 .../039-bad-inode-mode/.lowmem_repairable     |   0
 .../bad_free_space_cache_imode.raw.xz}        | Bin
 .../bad_imodes_in_subvolume_tree.img.xz       | Bin 0 -> 2956 bytes
 7 files changed, 335 insertions(+), 39 deletions(-)
 create mode 100644 tests/fsck-tests/039-bad-inode-mode/.lowmem_repairable
 rename tests/fsck-tests/{039-bad-free-space-cache-inode-mode/test.raw.xz => 039-bad-inode-mode/bad_free_space_cache_imode.raw.xz} (100%)
 create mode 100644 tests/fsck-tests/039-bad-inode-mode/bad_imodes_in_subvolume_tree.img.xz

-- 
2.23.0

[PATCH v3 1/6] btrfs-progs: check: Export btrfs_type_to_imode

[Qu Wenruo]

This function will be later used by common mode code, so export it.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/main.c        | 15 ---------------
 check/mode-common.h | 15 +++++++++++++++
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/check/main.c b/check/main.c
index 2e16b4e6f05b..902279740589 100644
--- a/check/main.c
+++ b/check/main.c
@@ -2448,21 +2448,6 @@ out:
 	return ret;
 }
 
-static u32 btrfs_type_to_imode(u8 type)
-{
-	static u32 imode_by_btrfs_type[] = {
-		[BTRFS_FT_REG_FILE]	= S_IFREG,
-		[BTRFS_FT_DIR]		= S_IFDIR,
-		[BTRFS_FT_CHRDEV]	= S_IFCHR,
-		[BTRFS_FT_BLKDEV]	= S_IFBLK,
-		[BTRFS_FT_FIFO]		= S_IFIFO,
-		[BTRFS_FT_SOCK]		= S_IFSOCK,
-		[BTRFS_FT_SYMLINK]	= S_IFLNK,
-	};
-
-	return imode_by_btrfs_type[(type)];
-}
-
 static int repair_inode_no_item(struct btrfs_trans_handle *trans,
 				struct btrfs_root *root,
 				struct btrfs_path *path,
diff --git a/check/mode-common.h b/check/mode-common.h
index 161b84a8deb0..6c8d6d7578a6 100644
--- a/check/mode-common.h
+++ b/check/mode-common.h
@@ -156,4 +156,19 @@ static inline bool is_valid_imode(u32 imode)
 }
 
 int recow_extent_buffer(struct btrfs_root *root, struct extent_buffer *eb);
+
+static inline u32 btrfs_type_to_imode(u8 type)
+{
+	static u32 imode_by_btrfs_type[] = {
+		[BTRFS_FT_REG_FILE]	= S_IFREG,
+		[BTRFS_FT_DIR]		= S_IFDIR,
+		[BTRFS_FT_CHRDEV]	= S_IFCHR,
+		[BTRFS_FT_BLKDEV]	= S_IFBLK,
+		[BTRFS_FT_FIFO]		= S_IFIFO,
+		[BTRFS_FT_SOCK]		= S_IFSOCK,
+		[BTRFS_FT_SYMLINK]	= S_IFLNK,
+	};
+
+	return imode_by_btrfs_type[(type)];
+}
 #endif
-- 
2.23.0

[PATCH v3 2/6] btrfs-progs: check/common: Introduce a function to find imode using info from INODE_REF item

[Qu Wenruo]

Introduce a function, find_file_type(), to find filetype using
info from INODE_REF, including dir_id from key index/name from
inode_ref_item.

This function will:
- Search DIR_INDEX first
  DIR_INDEX is easier since there is only one item in it.

- Validate the DIR_INDEX item
  If the DIR_INDEX is valid, use the filetype and call it a day.

- Search DIR_ITEM then
  It needs extra iteration since it's possible to have hash collision.

- Validate the DIR_ITEM
  If valid, call it a day. Or return -ENOENT;

This would be used as the primary method to determine the imode in later
imode repair code.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/mode-common.c | 129 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 129 insertions(+)

diff --git a/check/mode-common.c b/check/mode-common.c
index 195b6efaa7aa..9ccde5cdc2e5 100644
--- a/check/mode-common.c
+++ b/check/mode-common.c
@@ -16,6 +16,7 @@
 
 #include <time.h>
 #include "ctree.h"
+#include "hash.h"
 #include "common/internal.h"
 #include "common/messages.h"
 #include "transaction.h"
@@ -836,6 +837,134 @@ int reset_imode(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 	return ret;
 }
 
+static int find_file_type_dir_index(struct btrfs_root *root, u64 ino, u64 dirid,
+				    u64 index, const char *name, u32 name_len,
+				    u32 *imode_ret)
+{
+	struct btrfs_path path;
+	struct btrfs_key key;
+	struct btrfs_key location;
+	struct btrfs_dir_item *di;
+	char namebuf[BTRFS_NAME_LEN] = {0};
+	bool found = false;
+	u8 filetype;
+	u32 len;
+	int ret;
+
+	btrfs_init_path(&path);
+	key.objectid = dirid;
+	key.offset = index;
+	key.type = BTRFS_DIR_INDEX_KEY;
+
+	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
+	if (ret > 0) {
+		ret = -ENOENT;
+		goto out;
+	}
+	if (ret < 0)
+		goto out;
+	di = btrfs_item_ptr(path.nodes[0], path.slots[0],
+			    struct btrfs_dir_item);
+	btrfs_dir_item_key_to_cpu(path.nodes[0], di, &location);
+
+	/* Various basic check */
+	if (location.objectid != ino || location.type != BTRFS_INODE_ITEM_KEY ||
+	    location.offset != 0)
+		goto out;
+	filetype = btrfs_dir_type(path.nodes[0], di);
+	if (filetype >= BTRFS_FT_MAX || filetype == BTRFS_FT_UNKNOWN)
+		goto out;
+	len = min_t(u32, BTRFS_NAME_LEN,
+		btrfs_item_size_nr(path.nodes[0], path.slots[0]) - sizeof(*di));
+	len = min_t(u32, len, btrfs_dir_name_len(path.nodes[0], di));
+	read_extent_buffer(path.nodes[0], namebuf, (unsigned long)(di + 1), len);
+	if (name_len != len || memcmp(namebuf, name, len))
+		goto out;
+	found = true;
+	*imode_ret = btrfs_type_to_imode(filetype);
+out:
+	btrfs_release_path(&path);
+	if (!found && !ret)
+		ret = -ENOENT;
+	return ret;
+}
+
+static int find_file_type_dir_item(struct btrfs_root *root, u64 ino, u64 dirid,
+				   const char *name, u32 name_len,
+				   u32 *imode_ret)
+{
+	struct btrfs_path path;
+	struct btrfs_key key;
+	struct btrfs_key location;
+	struct btrfs_dir_item *di;
+	char namebuf[BTRFS_NAME_LEN] = {0};
+	bool found = false;
+	unsigned long cur;
+	unsigned long end;
+	u8 filetype;
+	u32 len;
+	int ret;
+
+	btrfs_init_path(&path);
+	key.objectid = dirid;
+	key.offset = btrfs_name_hash(name, name_len);
+	key.type = BTRFS_DIR_INDEX_KEY;
+
+	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
+	if (ret > 0) {
+		ret = -ENOENT;
+		goto out;
+	}
+	if (ret < 0)
+		goto out;
+
+	cur = btrfs_item_ptr_offset(path.nodes[0], path.slots[0]);
+	end = cur + btrfs_item_size_nr(path.nodes[0], path.slots[0]);
+	while (cur < end) {
+		di = (struct btrfs_dir_item *)cur;
+		cur += btrfs_dir_name_len(path.nodes[0], di) + sizeof(*di);
+
+		btrfs_dir_item_key_to_cpu(path.nodes[0], di, &location);
+		/* Various basic check */
+		if (location.objectid != ino ||
+		    location.type != BTRFS_INODE_ITEM_KEY ||
+		    location.offset != 0)
+			continue;
+		filetype = btrfs_dir_type(path.nodes[0], di);
+		if (filetype >= BTRFS_FT_MAX || filetype == BTRFS_FT_UNKNOWN)
+			continue;
+		len = min_t(u32, BTRFS_NAME_LEN,
+			    btrfs_item_size_nr(path.nodes[0], path.slots[0]) -
+			    sizeof(*di));
+		len = min_t(u32, len, btrfs_dir_name_len(path.nodes[0], di));
+		read_extent_buffer(path.nodes[0], namebuf,
+				   (unsigned long)(di + 1), len);
+		if (name_len != len || memcmp(namebuf, name, len))
+			continue;
+		*imode_ret = btrfs_type_to_imode(filetype);
+		found = true;
+		goto out;
+	}
+out:
+	btrfs_release_path(&path);
+	if (!found && !ret)
+		ret = -ENOENT;
+	return ret;
+}
+
+static int find_file_type(struct btrfs_root *root, u64 ino, u64 dirid,
+			  u64 index, const char *name, u32 name_len,
+			  u32 *imode_ret)
+{
+	int ret;
+	ret = find_file_type_dir_index(root, ino, dirid, index, name, name_len,
+				       imode_ret);
+	if (ret == 0)
+		return ret;
+	return find_file_type_dir_item(root, ino, dirid, name, name_len,
+				       imode_ret);
+}
+
 /*
  * Reset the inode mode of the inode specified by @path.
  *
-- 
2.23.0

[PATCH v3 3/6] btrfs-progs: check/common: Make repair_imode_common() to handle inodes in subvolume trees

[Qu Wenruo]

[[PROBLEM]]
Before this patch, repair_imode_common() can only handle two types of
inodes:
- Free space cache inodes
- ROOT DIR inodes

For inodes in subvolume trees, the core complexity is how to determine the
correct imode, thus it was not implemented.

However there are more reports of incorrect imode in subvolume trees, we
need to support such fix.

[[ENHANCEMENT]]
So this patch adds a new function, detect_imode(), to detect imode for
inodes in subvolume trees.
The policy here is, try our best to find a valid imode to recovery.
If no convicing info can be found, fail out.

That function will determine imode by:
1) Search for INODE_REF of the inode
   If we have INODE_REF, we will then try to find DIR_ITEM/DIR_INDEX.
   As long as one valid DIR_ITEM or DIR_INDEX can be found, we convert
   the BTRFS_FT_* to imode, then call it a day.
   This should be the most accurate way.

2) Search for DIR_INDEX/DIR_ITEM belongs to this inode
   If above search fails, we falls back to locate the DIR_INDEX/DIR_ITEM
   just after the INODE_ITEM.
   Thus this only works for non-empty directory.
   If any can be found, it's definitely a directory.

3) Search for EXTENT_DATA belongs to this inode
   If EXTENT_DATA can be found, it's either REG or LNK.
   Thus this only works for non-empty file or soft link.
   For this case, we default to REG, as user can inspect the file to
   determine if it's a file or just a path.

4) Use rdev to detect BLK/CHR
   If all above fails, but INODE_ITEM has non-zero rdev, then it's either
   a BLK or CHR file. Then we default to BLK.

5) Fail out if none of above methods succeeded
   No educated guess to make things worse.

[[SHORTCOMING]]
The above search is not perfect, there are cases where we can't repair:
E.g. orphan empty regular inode.
Since it's already orphan, it has no INODE_REF. And it's regular empty
file, it has no DIR_INDEX nor EXTENT_DATA nor rdev. Thus we can't recover.
Although for this case, it really doesn't matter as it's already orphan
and will be deleted anyway.

Furthermore, due to the DIR_ITEM/DIR_INDEX/INODE_REF repair code which
can happen before imode repair, it's possible that DIR_ITEM search code
may not be executed.
If there is only DIR_ITEM remaining, repair code will remove the
DIR_ITEM completely and move the inode to lost+found, leaving us no
info to rebuild imode.
If there is DIR_INDEX missing, repair code will re-insert the DIR_INDEX,
then imode repair code will go DIR_INDEX directly.

But overall, the repair code should handle the invalid imode caused by
older kernels without problem.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/mode-common.c | 133 +++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 120 insertions(+), 13 deletions(-)

diff --git a/check/mode-common.c b/check/mode-common.c
index 9ccde5cdc2e5..bc566e4aa03e 100644
--- a/check/mode-common.c
+++ b/check/mode-common.c
@@ -965,6 +965,116 @@ static int find_file_type(struct btrfs_root *root, u64 ino, u64 dirid,
 				       imode_ret);
 }
 
+static int detect_imode(struct btrfs_root *root, struct btrfs_path *path,
+			u32 *imode_ret)
+{
+	struct btrfs_key key;
+	struct btrfs_inode_item iitem;
+	bool found = false;
+	u64 ino;
+	u32 imode;
+	int ret = 0;
+
+	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
+	ino = key.objectid;
+	read_extent_buffer(path->nodes[0], &iitem,
+			btrfs_item_ptr_offset(path->nodes[0], path->slots[0]),
+			sizeof(iitem));
+	/* root inode */
+	if (ino == BTRFS_FIRST_FREE_OBJECTID) {
+		imode = S_IFDIR;
+		found = true;
+		goto out;
+	}
+
+	while (1) {
+		struct btrfs_inode_ref *iref;
+		struct extent_buffer *leaf;
+		unsigned long cur;
+		unsigned long end;
+		char namebuf[BTRFS_NAME_LEN] = {0};
+		u64 index;
+		u32 namelen;
+		int slot;
+
+		ret = btrfs_next_item(root, path);
+		if (ret > 0) {
+			/* falls back to rdev check */
+			ret = 0;
+			goto out;
+		}
+		if (ret < 0)
+			goto out;
+		leaf = path->nodes[0];
+		slot = path->slots[0];
+		btrfs_item_key_to_cpu(leaf, &key, slot);
+		if (key.objectid != ino)
+			goto out;
+
+		/*
+		 * We ignore some types to make life easier:
+		 * - XATTR
+		 *   Both REG and DIR can have xattr, so not useful
+		 */
+		switch (key.type) {
+		case BTRFS_INODE_REF_KEY:
+			/* The most accurate way to determine filetype */
+			cur = btrfs_item_ptr_offset(leaf, slot);
+			end = cur + btrfs_item_size_nr(leaf, slot);
+			while (cur < end) {
+				iref = (struct btrfs_inode_ref *)cur;
+				namelen = min_t(u32, end - cur - sizeof(&iref),
+					btrfs_inode_ref_name_len(leaf, iref));
+				index = btrfs_inode_ref_index(leaf, iref);
+				read_extent_buffer(leaf, namebuf,
+					(unsigned long)(iref + 1), namelen);
+				ret = find_file_type(root, ino, key.offset,
+						index, namebuf, namelen,
+						&imode);
+				if (ret == 0) {
+					found = true;
+					goto out;
+				}
+				cur += sizeof(*iref) + namelen;
+			}
+			break;
+		case BTRFS_DIR_ITEM_KEY:
+		case BTRFS_DIR_INDEX_KEY:
+			imode = S_IFDIR;
+			found = true;
+			goto out;
+		case BTRFS_EXTENT_DATA_KEY:
+			/*
+			 * Both REG and LINK could have EXTENT_DATA.
+			 * We just fall back to REG as user can inspect the
+			 * content.
+			 */
+			imode = S_IFREG;
+			found = true;
+			goto out;
+		}
+	}
+
+out:
+	/*
+	 * Both CHR and BLK uses rdev, no way to distinguish them, so fall back
+	 * to BLK. But either way it doesn't really matter, as CHR/BLK on btrfs
+	 * should be pretty rare, and no real data will be lost.
+	 */
+	if (!found && btrfs_stack_inode_rdev(&iitem) != 0) {
+		imode = S_IFBLK;
+		found = true;
+	}
+
+	if (found) {
+		ret = 0;
+		*imode_ret = (imode | 0700);
+	} else {
+		ret = -ENOENT;
+	}
+	return ret;
+}
+
 /*
  * Reset the inode mode of the inode specified by @path.
  *
@@ -981,22 +1091,19 @@ int repair_imode_common(struct btrfs_root *root, struct btrfs_path *path)
 	u32 imode;
 	int ret;
 
-	if (root->root_key.objectid != BTRFS_ROOT_TREE_OBJECTID) {
-		error(
-		"repair inode mode outside of root tree is not supported yet");
-		return -ENOTTY;
-	}
 	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
 	ASSERT(key.type == BTRFS_INODE_ITEM_KEY);
-	if (key.objectid != BTRFS_ROOT_TREE_DIR_OBJECTID &&
-	    !is_fstree(key.objectid)) {
-		error("unsupported ino %llu", key.objectid);
-		return -ENOTTY;
+	if (root->objectid == BTRFS_ROOT_TREE_OBJECTID) {
+		/* In root tree we only have two possible imode */
+		if (key.objectid == BTRFS_ROOT_TREE_OBJECTID)
+			imode = S_IFDIR | 0755;
+		else
+			imode = S_IFREG | 0600;
+	} else {
+		ret = detect_imode(root, path, &imode);
+		if (ret < 0)
+			return ret;
 	}
-	if (key.objectid == BTRFS_ROOT_TREE_DIR_OBJECTID)
-		imode = 040755;
-	else
-		imode = 0100600;
 
 	trans = btrfs_start_transaction(root, 1);
 	if (IS_ERR(trans)) {
-- 
2.23.0

[PATCH v3 4/6] btrfs-progs: check/lowmem: Repair bad imode early

[Qu Wenruo]

For lowmem mode, if we hit a bad inode mode, normally it is reported
when we checking the DIR_INDEX/DIR_ITEM of the parent inode.

If we didn't repair at that timing, the error will be recorded even we
fixed it later.

So this patch will check for INODE_ITEM_MISMATCH error type, and if it's
really caused by invalid imode, repair it and clear the error.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/mode-lowmem.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 5f7f101daab1..5d0c520217fa 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -1550,6 +1550,35 @@ static int lowmem_delete_corrupted_dir_item(struct btrfs_root *root,
 	return ret;
 }
 
+static int try_repair_imode(struct btrfs_root *root, u64 ino)
+{
+	struct btrfs_inode_item *iitem;
+	struct btrfs_path path;
+	struct btrfs_key key;
+	int ret;
+
+	key.objectid = ino;
+	key.type = BTRFS_INODE_ITEM_KEY;
+	key.offset = 0;
+	btrfs_init_path(&path);
+
+	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
+	if (ret > 0)
+		ret = -ENOENT;
+	if (ret < 0)
+		goto out;
+	iitem = btrfs_item_ptr(path.nodes[0], path.slots[0],
+			       struct btrfs_inode_item);
+	if (!is_valid_imode(btrfs_inode_mode(path.nodes[0], iitem))) {
+		ret = repair_imode_common(root, &path);
+	} else {
+		ret = -ENOTTY;
+	}
+out:
+	btrfs_release_path(&path);
+	return ret;
+}
+
 /*
  * Call repair_inode_item_missing and repair_ternary_lowmem to repair
  *
@@ -1574,6 +1603,16 @@ static int repair_dir_item(struct btrfs_root *root, struct btrfs_key *di_key,
 			err &= ~(INODE_ITEM_MISMATCH | INODE_ITEM_MISSING);
 	}
 
+	if (err & INODE_ITEM_MISMATCH) {
+		/*
+		 * INODE_ITEM mismatch can be caused by bad imode,
+		 * so check if it's a bad imode, then repair if possible.
+		 */
+		ret = try_repair_imode(root, ino);
+		if (!ret)
+			err &= ~INODE_ITEM_MISMATCH;
+	}
+
 	if (err & ~(INODE_ITEM_MISMATCH | INODE_ITEM_MISSING)) {
 		ret = repair_ternary_lowmem(root, dirid, ino, index, namebuf,
 					    name_len, filetype, err);
-- 
2.23.0

[PATCH v3 5/6] btrfs-progs: check/original: Fix inode mode in subvolume trees

[Qu Wenruo]

To make original mode to repair imode error in subvolume trees, this
patch will do:
- Remove the show-stopper checks for root->objectid.
  Now repair_imode_original() will accept inodes in subvolume trees.

- Export detect_imode() for original mode
  Due to the call requirement, original mode must use an existing trans
  handler to do the repair, thus we need to re-implement most of the
  work done in repair_imode_common().

- Make repair_imode_original() to use detect_imode().

- Free the path after reset_imode()
  reset_imode() keeps the path, as lowmem mode uses path to locate its
  current check position.
  But for original mode, the unreleased path can cause later repair to
  report warning, so we need to manually release the path.

- Update rec->imode after imode reset
  So later repair depending on rec->imode can get correct value.

- Move the repair before repair_inode_nlinks()
  repair_inode_nlinks() needs correct imode to add DIR_INDEX/DIR_ITEM.
  So moving the repair before repair_inode_nlinks() makes the latter
  repair happier.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/main.c        | 41 ++++++++++++++++++++++++++++++-----------
 check/mode-common.c |  2 +-
 check/mode-common.h |  2 ++
 3 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/check/main.c b/check/main.c
index 902279740589..8bdc9c69fa46 100644
--- a/check/main.c
+++ b/check/main.c
@@ -2756,22 +2756,40 @@ static int repair_imode_original(struct btrfs_trans_handle *trans,
 				 struct btrfs_path *path,
 				 struct inode_record *rec)
 {
+	struct btrfs_key key;
 	int ret;
 	u32 imode;
 
-	if (root->root_key.objectid != BTRFS_ROOT_TREE_OBJECTID)
-		return -ENOTTY;
-	if (rec->ino != BTRFS_ROOT_TREE_DIR_OBJECTID || !is_fstree(rec->ino))
-		return -ENOTTY;
+	key.objectid = rec->ino;
+	key.type = BTRFS_INODE_ITEM_KEY;
+	key.offset = 0;
 
-	if (rec->ino == BTRFS_ROOT_TREE_DIR_OBJECTID)
-		imode = 040755;
-	else
-		imode = 0100600;
+	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
+	if (ret > 0)
+		ret = -ENOENT;
+	if (ret < 0)
+		return ret;
+
+	if (root->objectid == BTRFS_ROOT_TREE_OBJECTID) {
+		/* In root tree we only have two possible imode */
+		if (rec->ino == BTRFS_ROOT_TREE_OBJECTID)
+			imode = S_IFDIR | 0755;
+		else
+			imode = S_IFREG | 0600;
+	} else {
+		ret = detect_imode(root, path, &imode);
+		if (ret < 0) {
+			btrfs_release_path(path);
+			return ret;
+		}
+	}
+	btrfs_release_path(path);
 	ret = reset_imode(trans, root, path, rec->ino, imode);
+	btrfs_release_path(path);
 	if (ret < 0)
 		return ret;
 	rec->errors &= ~I_ERR_INVALID_IMODE;
+	rec->imode = imode;
 	return ret;
 }
 
@@ -2795,7 +2813,8 @@ static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
 			     I_ERR_FILE_NBYTES_WRONG |
 			     I_ERR_INLINE_RAM_BYTES_WRONG |
 			     I_ERR_MISMATCH_DIR_HASH |
-			     I_ERR_UNALIGNED_EXTENT_REC)))
+			     I_ERR_UNALIGNED_EXTENT_REC |
+			     I_ERR_INVALID_IMODE)))
 		return rec->errors;
 
 	/*
@@ -2812,6 +2831,8 @@ static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
 	btrfs_init_path(&path);
 	if (!ret && rec->errors & I_ERR_MISMATCH_DIR_HASH)
 		ret = repair_mismatch_dir_hash(trans, root, rec);
+	if (!ret && rec->errors & I_ERR_INVALID_IMODE)
+		ret = repair_imode_original(trans, root, &path, rec);
 	if (rec->errors & I_ERR_NO_INODE_ITEM)
 		ret = repair_inode_no_item(trans, root, &path, rec);
 	if (!ret && rec->errors & I_ERR_FILE_EXTENT_DISCOUNT)
@@ -2828,8 +2849,6 @@ static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
 		ret = repair_inline_ram_bytes(trans, root, &path, rec);
 	if (!ret && rec->errors & I_ERR_UNALIGNED_EXTENT_REC)
 		ret = repair_unaligned_extent_recs(trans, root, &path, rec);
-	if (!ret && rec->errors & I_ERR_INVALID_IMODE)
-		ret = repair_imode_original(trans, root, &path, rec);
 	btrfs_commit_transaction(trans, root);
 	btrfs_release_path(&path);
 	return ret;
diff --git a/check/mode-common.c b/check/mode-common.c
index bc566e4aa03e..10ad6d228a03 100644
--- a/check/mode-common.c
+++ b/check/mode-common.c
@@ -965,7 +965,7 @@ static int find_file_type(struct btrfs_root *root, u64 ino, u64 dirid,
 				       imode_ret);
 }
 
-static int detect_imode(struct btrfs_root *root, struct btrfs_path *path,
+int detect_imode(struct btrfs_root *root, struct btrfs_path *path,
 			u32 *imode_ret)
 {
 	struct btrfs_key key;
diff --git a/check/mode-common.h b/check/mode-common.h
index 6c8d6d7578a6..edf9257edaf0 100644
--- a/check/mode-common.h
+++ b/check/mode-common.h
@@ -126,6 +126,8 @@ int delete_corrupted_dir_item(struct btrfs_trans_handle *trans,
 			      struct btrfs_root *root,
 			      struct btrfs_key *di_key, char *namebuf,
 			      u32 namelen);
+int detect_imode(struct btrfs_root *root, struct btrfs_path *path,
+		 u32 *imode_ret);
 int reset_imode(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 		struct btrfs_path *path, u64 ino, u32 mode);
 int repair_imode_common(struct btrfs_root *root, struct btrfs_path *path);
-- 
2.23.0

[PATCH v3 6/6] btrfs-progs: tests/fsck: Add new images for inode mode repair functionality

[Qu Wenruo]

Add new test image for imode repair in subvolume trees.

The new test cases including the following cases:
- Regular file with bad imode
  It still has the valid INODE_REF and parent dir has correct DIR_INDEX
  and DIR_ITEM.
  In this case, no matter if the file is empty or not, it should be
  repaired using the info from DIR_INDEX of parent dir.

- Non-empty regular file with bad imode, and without INODE_REF
  The file should be mostly an orphan, so no INODE_REF for imode lookup.
  But it has EXTENT_DATA which should be enough for imode repair.
  The repair also involves moving the orphan to lost+found dir.

- Non-empty dir with bad imode, and without INODE_REF
  Pretty much the same case, but now a directory.
  The repair also involves moving the orphan to lost+found dir.

Also rename the existing test case 039-bad-free-space-cache-inode-mode
to 039-bad-inode-mode, since now we can fix all bad imode.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 .../039-bad-inode-mode/.lowmem_repairable        |   0
 .../bad_free_space_cache_imode.raw.xz}           | Bin
 .../bad_imodes_in_subvolume_tree.img.xz          | Bin 0 -> 2956 bytes
 3 files changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 tests/fsck-tests/039-bad-inode-mode/.lowmem_repairable
 rename tests/fsck-tests/{039-bad-free-space-cache-inode-mode/test.raw.xz => 039-bad-inode-mode/bad_free_space_cache_imode.raw.xz} (100%)
 create mode 100644 tests/fsck-tests/039-bad-inode-mode/bad_imodes_in_subvolume_tree.img.xz

diff --git a/tests/fsck-tests/039-bad-inode-mode/.lowmem_repairable b/tests/fsck-tests/039-bad-inode-mode/.lowmem_repairable
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/tests/fsck-tests/039-bad-free-space-cache-inode-mode/test.raw.xz b/tests/fsck-tests/039-bad-inode-mode/bad_free_space_cache_imode.raw.xz
similarity index 100%
rename from tests/fsck-tests/039-bad-free-space-cache-inode-mode/test.raw.xz
rename to tests/fsck-tests/039-bad-inode-mode/bad_free_space_cache_imode.raw.xz
diff --git a/tests/fsck-tests/039-bad-inode-mode/bad_imodes_in_subvolume_tree.img.xz b/tests/fsck-tests/039-bad-inode-mode/bad_imodes_in_subvolume_tree.img.xz
new file mode 100644
index 0000000000000000000000000000000000000000..8c51ceb7703de8ab36ca8cdd95117c0e13174a04
GIT binary patch
literal 2956
zcmV;73v=}SH+ooF000E$*0e?f03iV!0000G&sfah5C02HT>wRyj;C3^v%$$4d1wdA
zhA1@4m*CN4=JF(pu<{lZu#3rJ{x`%OunHJR_+hWlU+8THA!o)hf&JdREq$B_jm%57
z$k$@1L{^~@)eO=~d}4!Pg}&?=WZA7?y^i8Ts&#ea0yyltsLU^Gf(TifcWf2aWO^hn
zT;ywyIWc#F)mQ1BE<ww59WC_2)q`g%{jaCvsAU(w{9KiI5nq?F(AVl-*<IG=vXsR0
z@B9cC5NQexE<r*z-aql!o{j!S%C9WYgYq%KPIL?GZE__!gGE+ELYwPMF^DHXEb47+
zYg@=6)ZlNllJyms!~1s7P;N&lqrxbLHQF#%!xrv)Lphp;j(kkY627rutWI~OSVqO#
zFSIv8$)WjBA~QQch(y;yR;#I^-UtikAooqVPA{8I<*wb&cK*jbwW_jynjrTQa@yTl
zT513}k%5Xx$bmnvetk1m!8Ird2mdFfU>%<<K!8?xT5?EtNo_6Q4J3n+*KnF1o^le3
zYg}oqZ@Def1=DeVQ}vfhe{nrAF2A5Fy!OvAOU6qD6q;$yzLT<}C9DxfQ|E+fL)<k8
zelyWYjaS0lPLCoas`y%8%Q>IyvimyUvJNB2|Mo)Fslt`UpOQyH!=GhdIIJ&nXNptS
z5>-zoXN+mR|Kd!b>C`pZDeSL|dDc0q=cne=*q|$_u;Mtdv?NB<d^16(H@1QZkf0cd
zI=RalR+vfE__spuhqBAN56ipsaUxu<`6(Nz%_V!neFg}Lebk?d^wXaxN5Uk!HhsA9
zONE8)Bsr#<S%^(LwO`)geoM4i#9rsckwd)DCnDM8;<mm!QjiFM)HMxV@%zWCSBiS0
zWB6I2RV&jzyKcuq*LE8>&jsm4$!H89N@6MB_s-=sZq}74cMD0lH(sU%^^o(ot<{|r
ztKcf34B*6NDzb{6fNLhf#qlKT$4j{W?U3x@9tTUgT-SQV!UupoQ0*1oJ%yJ2AJpTq
z)t*CBp6+TJ<_{;}@x`nd|M|6rNs7b8o3*)M4V)W!cBsPyse$nadTV-@mRqa*8RJ6T
zN;J^~^kAFH)?Yp!@{r0RfVvKLB648Rw>FfV&>>sPR@3sL-8$vOW*Xp*pk6KqU-Xu=
z{c;{BM3?BGNh`Zo7f3aGos=%*7AcwH>)0H*&cn?Z)eV9<ze6H%f`Lv8LqEAJagh3F
z!m?Y`pZ8JuG_`jq(9Ip%$v+li{FV2LfMC0Ur<fmId7j5*>%X3UUy2=&{9w;hyR#n;
z^vqkBZJT!$=Si<zvd=FC3X(`BX~N6JW_!RSZz7i*^kaIy4JjsCEhVM6_KddYPEqT2
zLuC6#Z^rMfBouFHr1f+$q32dwHqTOj;g<b)^TGOMoGM0Gtse6D|5UPC5*#A%b)>(K
z<B>XmuumG*qGca*78hB`%Qr-9e=$wk*5W#FT@FDgT$zyoq`&e5c&Z|myJLIen*rd~
z?U9nHI<L&#Wg~@EEh{bHnL%8pIWNwwHn7HsWW|ZLv{WVD79zP<9L~Vm(IekC^OJ29
z%uyRNGG1)7JyG5e5WSV|erE~S27JRW(T%D-{h_nuz<DaEL5Y*cgmg`axDQU=aa>iX
zHr2jRZOlBcw~4T}&bOi^*fRD&d4`;@WoO7WfZ85%*TAh7Sh)BI>CT_J4@qLDUKcAi
zI2J_;@x;{-?iKAO1|G|c)&sC#l5|FJYxTQ0l<a!5WjGQ`DH0Qh8$?i;h#n)F*{i*z
zdv!!&x#Z22fy>+-iFC5D@A1Ss+Khw)wjEgp-t5)utZPx2rs6;DKf|h;Ye#hdOjHXI
zxGp7R2=d_&J`%<#g@`UXKZ%%jf>fXypwGrw85h@<yE*q{*jeWfjJ3{;CtggNGKW85
zo4OEi2{-7lZud|Md}e~uvlyDf&<oQvv^e^Cob&X4;6ex50G(!~6&S9rKvhDLN1&gM
zlsuYZYfh{Yu>0ladmG#P-s7Ei3_9%QecKvFzSc+kW;(Aj5DtBhUzL7I7YzYMI*7d4
zdei!p`B@H-9yX_6wMTYlEZB;yZqU*l_S;nb^f6g(n+x1ba_syY@HSA!S{;ll4T#&6
zIkR17^8szM34JM2{pf0{$dDapIl_fkt@fzGC#Q<G9%o=^nRXf{?)K`h{2CX9TSHsx
znC(504c4N=E<l6z1I4-vU4by1xI?u2rY!(&a5B?NkIJNNzsE20(6-?xH2)3;RE3uH
zyQV+YkLE1K2qg`A=?XIE@S*G6iT_KPHf9XsN3+&H2nxdD5k$9k1~COubuXK{oJ$0C
zShU2S7SzVzdxaa~5}wB8EdHZ(tMW^SE9kiXgcxTIbjduHAj=j4XQlc@*DZNivPY8|
zRNF<+1Exn2PEm3a7D`h@2Mo&&oRbZs8#M_vY#3#+=cXfy(yAz2j)qnhyviuJ1ZL<$
z&IWQjGUxXp-26;lnUIb;HN?}PnYR|arp_4vGMNJdgsLGDr-9~yj9xLSecSt%L?TN}
z{{rPNzfAu5m~SfR&Wh369jKS=7_06ur6gofJ7rno!i{-Y{#QpHLZ8D_%0wjW0(bhz
zwN<+u$oF4&=|0oa;MzWf==AllEB#Z2H|_9MM_ER%Q`+WD|7|~mAE#!ynf*;=Noi7;
zU9%CtrQ|gf0JnPUj+gUDP|I&mXu`Z0*mV0VK+3VKALqE1rpKqHxY3z0pGj(cC5*KG
z>)({dl7L(m?Tx&ugkV_;Appdrr-40fb{*^&9rrN1IZfjhLzPjf-wW7Z2o#Il6cjwY
zS3^##ZhCq;5tpKbpj%H-BfPnozDvE8_U(fP;c@lOY0x4N!rpz^unWR@jz*#w=Pqld
z#w)YGT9Q2teu~KV`#mwUnnNWlf!dZ8-?X1sWP_GGzaX!=XH1Q54g}X-)4SAes5F=t
zYb|<3M_0O9NM4zVoy>Ssu@|9EtiedrJ*D(#%sVbV0nNO*Opf$5dig?|#laB%X2O2l
zD)Y)25AT~8!!-tN55(F5jWxFY4qhVf{vE>op~l;)i21M7{#*Fx)t>cN`6{;7a|_}d
zV325VM4fuWc~-mI*cdsVV0{-WOo4AT`W^;t8lpYI3gP(-OWK!Ggokqfaig=mYN-$V
z#oLJtita+7wE=Zzr=P@>EBC&5-P0<C-mf<EH)e<2J9iNDg=FaTQb7#Xw;#M%WXA*L
z(Zc#DQ2+2Xh+FnZOw%;#ab>qH_38{tp9*QXMOb9}dNQr&LL^j1l3b|kyL_wL{SckE
zbuUQ%1a1XYgkmr{d7xC~=~7qs?1CXN;@fYZ^Sb@1ej&O3y!iiMG3KHG)ie%y=LTr}
zShAxfD<rz{8D5_53lo%6hn%H1;%K>EoWO$D(Way2no^hku8Pz1pWbZuL+;U>H23;O
zPL7o4aJ|KzTvxL1NzwZd=+$Xo;laWSMA%^oj{=QiiKM?8bI?5yetUrl7zxj*2&-y`
zJt7RLK}Ra%xm&Dx*esCUbgs2^?_I4?KU8~fk1IIWMYu$@krs~4uE9z)2;2m*6-n>v
zv}GC4O-`b51PG|-ihyj=15#l?)S~T4chV>Rxjq}2LL3R9Q+vt1jU<5-NDyd|gtWWB
zPWvZ=#`d1IP(ZcMGGP0+oz$w|%XxC{Sqb)?*^*&;8Lh#)ml)@g&Vyq$@PaJZ=X<MB
z9AzY3msfb2w<3F#1)TtYWk&WfhcNTNpd{A<+11etiTF<jl*L_mz*+_MY@~}6s5%H8
zPy!k3@u-*Dt92l#9GG;*G-})K6Z`4U<f@(+6crqiQbkY$oIQyHR)ds$DP%}_P#$w(
z6-9zYiL&5lzR=Wj4Qu7Dd4K?%uV+$>fi3g^0q7QhAOHZ%-}VEs#Ao{g000001X)^D
C1kMfs

literal 0
HcmV?d00001

-- 
2.23.0

Re: [PATCH v3 1/6] btrfs-progs: check: Export btrfs_type_to_imode

[Nikolay Borisov]

On 12.09.19 г. 6:11 ч., Qu Wenruo wrote:
> This function will be later used by common mode code, so export it.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com> but see one nit below.

> ---
>  check/main.c        | 15 ---------------
>  check/mode-common.h | 15 +++++++++++++++
>  2 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/check/main.c b/check/main.c
> index 2e16b4e6f05b..902279740589 100644
> --- a/check/main.c
> +++ b/check/main.c
> @@ -2448,21 +2448,6 @@ out:
>  	return ret;
>  }
>  
> -static u32 btrfs_type_to_imode(u8 type)
> -{
> -	static u32 imode_by_btrfs_type[] = {
> -		[BTRFS_FT_REG_FILE]	= S_IFREG,
> -		[BTRFS_FT_DIR]		= S_IFDIR,
> -		[BTRFS_FT_CHRDEV]	= S_IFCHR,
> -		[BTRFS_FT_BLKDEV]	= S_IFBLK,
> -		[BTRFS_FT_FIFO]		= S_IFIFO,
> -		[BTRFS_FT_SOCK]		= S_IFSOCK,
> -		[BTRFS_FT_SYMLINK]	= S_IFLNK,
> -	};
> -
> -	return imode_by_btrfs_type[(type)];
> -}
> -
>  static int repair_inode_no_item(struct btrfs_trans_handle *trans,
>  				struct btrfs_root *root,
>  				struct btrfs_path *path,
> diff --git a/check/mode-common.h b/check/mode-common.h
> index 161b84a8deb0..6c8d6d7578a6 100644
> --- a/check/mode-common.h
> +++ b/check/mode-common.h
> @@ -156,4 +156,19 @@ static inline bool is_valid_imode(u32 imode)
>  }
>  
>  int recow_extent_buffer(struct btrfs_root *root, struct extent_buffer *eb);
> +
> +static inline u32 btrfs_type_to_imode(u8 type)
> +{
> +	static u32 imode_by_btrfs_type[] = {
> +		[BTRFS_FT_REG_FILE]	= S_IFREG,
> +		[BTRFS_FT_DIR]		= S_IFDIR,
> +		[BTRFS_FT_CHRDEV]	= S_IFCHR,
> +		[BTRFS_FT_BLKDEV]	= S_IFBLK,
> +		[BTRFS_FT_FIFO]		= S_IFIFO,
> +		[BTRFS_FT_SOCK]		= S_IFSOCK,
> +		[BTRFS_FT_SYMLINK]	= S_IFLNK,
> +	};

nit: If the array is defined in a function in a header this means it
will be copied to every object file this header is included so it will
result in a minor bloat of size. It might better to have it defined in
check/main.c and have it declared extern in mode-common.h

> +
> +	return imode_by_btrfs_type[(type)];
> +}
>  #endif
> 

Re: [PATCH v3 1/6] btrfs-progs: check: Export btrfs_type_to_imode

[Qu WenRuo]

On 2019/9/24 下午4:41, Nikolay Borisov wrote:
> 
> 
> On 12.09.19 г. 6:11 ч., Qu Wenruo wrote:
>> This function will be later used by common mode code, so export it.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
> 
> Reviewed-by: Nikolay Borisov <nborisov@suse.com> but see one nit below.
> 
>> ---
>>  check/main.c        | 15 ---------------
>>  check/mode-common.h | 15 +++++++++++++++
>>  2 files changed, 15 insertions(+), 15 deletions(-)
>>
>> diff --git a/check/main.c b/check/main.c
>> index 2e16b4e6f05b..902279740589 100644
>> --- a/check/main.c
>> +++ b/check/main.c
>> @@ -2448,21 +2448,6 @@ out:
>>  	return ret;
>>  }
>>  
>> -static u32 btrfs_type_to_imode(u8 type)
>> -{
>> -	static u32 imode_by_btrfs_type[] = {
>> -		[BTRFS_FT_REG_FILE]	= S_IFREG,
>> -		[BTRFS_FT_DIR]		= S_IFDIR,
>> -		[BTRFS_FT_CHRDEV]	= S_IFCHR,
>> -		[BTRFS_FT_BLKDEV]	= S_IFBLK,
>> -		[BTRFS_FT_FIFO]		= S_IFIFO,
>> -		[BTRFS_FT_SOCK]		= S_IFSOCK,
>> -		[BTRFS_FT_SYMLINK]	= S_IFLNK,
>> -	};
>> -
>> -	return imode_by_btrfs_type[(type)];
>> -}
>> -
>>  static int repair_inode_no_item(struct btrfs_trans_handle *trans,
>>  				struct btrfs_root *root,
>>  				struct btrfs_path *path,
>> diff --git a/check/mode-common.h b/check/mode-common.h
>> index 161b84a8deb0..6c8d6d7578a6 100644
>> --- a/check/mode-common.h
>> +++ b/check/mode-common.h
>> @@ -156,4 +156,19 @@ static inline bool is_valid_imode(u32 imode)
>>  }
>>  
>>  int recow_extent_buffer(struct btrfs_root *root, struct extent_buffer *eb);
>> +
>> +static inline u32 btrfs_type_to_imode(u8 type)
>> +{
>> +	static u32 imode_by_btrfs_type[] = {
>> +		[BTRFS_FT_REG_FILE]	= S_IFREG,
>> +		[BTRFS_FT_DIR]		= S_IFDIR,
>> +		[BTRFS_FT_CHRDEV]	= S_IFCHR,
>> +		[BTRFS_FT_BLKDEV]	= S_IFBLK,
>> +		[BTRFS_FT_FIFO]		= S_IFIFO,
>> +		[BTRFS_FT_SOCK]		= S_IFSOCK,
>> +		[BTRFS_FT_SYMLINK]	= S_IFLNK,
>> +	};
> 
> nit: If the array is defined in a function in a header this means it
> will be copied to every object file this header is included so it will
> result in a minor bloat of size. It might better to have it defined in
> check/main.c and have it declared extern in mode-common.h

I'm wondering what happens in the final binary.

IIRC there should be only one copy of the static const array in .data
segment.

So that should be mostly OK I guess?

Thanks,
Qu

> 
>> +
>> +	return imode_by_btrfs_type[(type)];
>> +}
>>  #endif
>>