diff mbox series

btrfs-progs: convert: fix inline extent size for symbol link

Message ID 08ff5d66302e6b84481fda9d50d53bb50a519fd3.1725329062.git.wqu@suse.com (mailing list archive)
State New
Headers show
Series btrfs-progs: convert: fix inline extent size for symbol link | expand

Commit Message

Qu Wenruo Sept. 3, 2024, 2:04 a.m. UTC
[BUG]
For btrfs converted from ext* or reiserfs, the inlined data extent is
always one byte larger than the inode size:

	item 10 key (267 INODE_ITEM 0) itemoff 15543 itemsize 160
		generation 1 transid 1 size 4 nbytes 5
		block group 0 mode 120777 links 1 uid 0 gid 0 rdev 0
		sequence 0 flags 0x0(none)
	item 11 key (267 INODE_REF 256) itemoff 15529 itemsize 14
		index 3 namelen 4 name: path
	item 12 key (267 EXTENT_DATA 0) itemoff 15503 itemsize 26
		generation 4 type 0 (inline)
		inline extent data size 5 ram_bytes 5 compression 0 (none)

[CAUSE]
Inside the symbol link creation path for each fs, they all create the
inline data extent with a tailing NUL ('\0').

This is different from what btrfs kernel module does:

	item 4 key (257 INODE_ITEM 0) itemoff 15883 itemsize 160
		generation 9 transid 9 size 4 nbytes 4
		block group 0 mode 120777 links 1 uid 0 gid 0 rdev 0
		sequence 0 flags 0x0(none)
	item 5 key (257 INODE_REF 256) itemoff 15869 itemsize 14
		index 2 namelen 4 name: path
	item 6 key (257 EXTENT_DATA 0) itemoff 15844 itemsize 25
		generation 9 type 0 (inline)
		inline extent data size 4 ram_bytes 4 compression 0 (none)

[FIX]
Thankfully this is not a big deal, kernel properly reads the content and
use inode size to determine the proper link target.

Just align the btrfs-progs convert behavior to the kernel one.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 convert/source-ext2.c     | 4 ++--
 convert/source-reiserfs.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

Comments

Qu Wenruo Sept. 3, 2024, 7:20 a.m. UTC | #1
Please drop this one.

There is a bigger problem which can cause kernel rejecting to read a 
symbol link, exposed by random btrfs/125 failure.

So this will be updated to be included in a larger series instead, with 
better error description.

Thanks,
Qu

在 2024/9/3 11:34, Qu Wenruo 写道:
> [BUG]
> For btrfs converted from ext* or reiserfs, the inlined data extent is
> always one byte larger than the inode size:
> 
> 	item 10 key (267 INODE_ITEM 0) itemoff 15543 itemsize 160
> 		generation 1 transid 1 size 4 nbytes 5
> 		block group 0 mode 120777 links 1 uid 0 gid 0 rdev 0
> 		sequence 0 flags 0x0(none)
> 	item 11 key (267 INODE_REF 256) itemoff 15529 itemsize 14
> 		index 3 namelen 4 name: path
> 	item 12 key (267 EXTENT_DATA 0) itemoff 15503 itemsize 26
> 		generation 4 type 0 (inline)
> 		inline extent data size 5 ram_bytes 5 compression 0 (none)
> 
> [CAUSE]
> Inside the symbol link creation path for each fs, they all create the
> inline data extent with a tailing NUL ('\0').
> 
> This is different from what btrfs kernel module does:
> 
> 	item 4 key (257 INODE_ITEM 0) itemoff 15883 itemsize 160
> 		generation 9 transid 9 size 4 nbytes 4
> 		block group 0 mode 120777 links 1 uid 0 gid 0 rdev 0
> 		sequence 0 flags 0x0(none)
> 	item 5 key (257 INODE_REF 256) itemoff 15869 itemsize 14
> 		index 2 namelen 4 name: path
> 	item 6 key (257 EXTENT_DATA 0) itemoff 15844 itemsize 25
> 		generation 9 type 0 (inline)
> 		inline extent data size 4 ram_bytes 4 compression 0 (none)
> 
> [FIX]
> Thankfully this is not a big deal, kernel properly reads the content and
> use inode size to determine the proper link target.
> 
> Just align the btrfs-progs convert behavior to the kernel one.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>   convert/source-ext2.c     | 4 ++--
>   convert/source-reiserfs.c | 4 ++--
>   2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/convert/source-ext2.c b/convert/source-ext2.c
> index acba5db7ee81..d5d6b165a62d 100644
> --- a/convert/source-ext2.c
> +++ b/convert/source-ext2.c
> @@ -489,8 +489,8 @@ static int ext2_create_symlink(struct btrfs_trans_handle *trans,
>   	pathname = (char *)&(ext2_inode->i_block[0]);
>   	BUG_ON(pathname[inode_size] != 0);
>   	ret = btrfs_insert_inline_extent(trans, root, objectid, 0,
> -					 pathname, inode_size + 1);
> -	btrfs_set_stack_inode_nbytes(btrfs_inode, inode_size + 1);
> +					 pathname, inode_size);
> +	btrfs_set_stack_inode_nbytes(btrfs_inode, inode_size);
>   	return ret;
>   }
>   
> diff --git a/convert/source-reiserfs.c b/convert/source-reiserfs.c
> index 3edc72ed08a5..e2f07bfda188 100644
> --- a/convert/source-reiserfs.c
> +++ b/convert/source-reiserfs.c
> @@ -538,8 +538,8 @@ static int reiserfs_copy_symlink(struct btrfs_trans_handle *trans,
>   	len = get_ih_item_len(tp_item_head(&path));
>   
>   	ret = btrfs_insert_inline_extent(trans, root, objectid, 0,
> -					 symlink, len + 1);
> -	btrfs_set_stack_inode_nbytes(btrfs_inode, len + 1);
> +					 symlink, len);
> +	btrfs_set_stack_inode_nbytes(btrfs_inode, len);
>   fail:
>   	pathrelse(&path);
>   	return ret;
diff mbox series

Patch

diff --git a/convert/source-ext2.c b/convert/source-ext2.c
index acba5db7ee81..d5d6b165a62d 100644
--- a/convert/source-ext2.c
+++ b/convert/source-ext2.c
@@ -489,8 +489,8 @@  static int ext2_create_symlink(struct btrfs_trans_handle *trans,
 	pathname = (char *)&(ext2_inode->i_block[0]);
 	BUG_ON(pathname[inode_size] != 0);
 	ret = btrfs_insert_inline_extent(trans, root, objectid, 0,
-					 pathname, inode_size + 1);
-	btrfs_set_stack_inode_nbytes(btrfs_inode, inode_size + 1);
+					 pathname, inode_size);
+	btrfs_set_stack_inode_nbytes(btrfs_inode, inode_size);
 	return ret;
 }
 
diff --git a/convert/source-reiserfs.c b/convert/source-reiserfs.c
index 3edc72ed08a5..e2f07bfda188 100644
--- a/convert/source-reiserfs.c
+++ b/convert/source-reiserfs.c
@@ -538,8 +538,8 @@  static int reiserfs_copy_symlink(struct btrfs_trans_handle *trans,
 	len = get_ih_item_len(tp_item_head(&path));
 
 	ret = btrfs_insert_inline_extent(trans, root, objectid, 0,
-					 symlink, len + 1);
-	btrfs_set_stack_inode_nbytes(btrfs_inode, len + 1);
+					 symlink, len);
+	btrfs_set_stack_inode_nbytes(btrfs_inode, len);
 fail:
 	pathrelse(&path);
 	return ret;