[PATCH 0/4] btrfs-progs: add support ext4 unwritten file extent

[PATCH 0/4] btrfs-progs: add support ext4 unwritten file extent

[Anand Jain]

These patches add support for the ext4 file data unwritten/preallocated
extents. Patches 1-3 are preparatory patches, and patch 4 adds the
missing feature.

Patch 4 is marked as RFC because this patch is tested with limited
variants of the file extents with unwritten flag.

Anand Jain (4):
  btrfs-progs: convert: refactor ext2_create_file_extents add argument
    ext2_inode
  btrfs-progs: convert: struct blk_iterate_data, add ext2-specific file
    inode pointers
  btrfs-progs: convert: refactor __btrfs_record_file_extent to add a
    prealloc flag
  btrfs-progs: convert: support ext2 unwritten file data extents

 common/extent-tree-utils.c | 11 +++++---
 common/extent-tree-utils.h |  2 +-
 convert/main.c             |  9 ++++---
 convert/source-ext2.c      |  8 +++++-
 convert/source-fs.c        | 52 ++++++++++++++++++++++++++++++++++++--
 convert/source-fs.h        |  6 +++++
 convert/source-reiserfs.c  |  2 +-
 mkfs/rootdir.c             |  3 ++-
 8 files changed, 79 insertions(+), 14 deletions(-)

-- 
2.39.3

[PATCH 1/4] btrfs-progs: convert: refactor ext2_create_file_extents add argument ext2_inode

[Anand Jain]

This is a preparatory patch adds an argument '%ext2_inode' for the
function __btrfs_record_file_extent(); to be used in the following patches.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 convert/source-ext2.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/convert/source-ext2.c b/convert/source-ext2.c
index 2186b2526e38..a3f61bb01171 100644
--- a/convert/source-ext2.c
+++ b/convert/source-ext2.c
@@ -310,6 +310,7 @@ static int ext2_create_file_extents(struct btrfs_trans_handle *trans,
 			       struct btrfs_root *root, u64 objectid,
 			       struct btrfs_inode_item *btrfs_inode,
 			       ext2_filsys ext2_fs, ext2_ino_t ext2_ino,
+			       struct ext2_inode *ext2_inode,
 			       u32 convert_flags)
 {
 	int ret;
@@ -384,6 +385,7 @@ static int ext2_create_symlink(struct btrfs_trans_handle *trans,
 		btrfs_set_stack_inode_size(btrfs_inode, inode_size + 1);
 		ret = ext2_create_file_extents(trans, root, objectid,
 				btrfs_inode, ext2_fs, ext2_ino,
+				ext2_inode,
 				CONVERT_FLAG_DATACSUM |
 				CONVERT_FLAG_INLINE_DATA);
 		btrfs_set_stack_inode_size(btrfs_inode, inode_size);
@@ -903,7 +905,7 @@ static int ext2_copy_single_inode(struct btrfs_trans_handle *trans,
 	switch (ext2_inode->i_mode & S_IFMT) {
 	case S_IFREG:
 		ret = ext2_create_file_extents(trans, root, objectid,
-			&btrfs_inode, ext2_fs, ext2_ino, convert_flags);
+			&btrfs_inode, ext2_fs, ext2_ino, ext2_inode, convert_flags);
 		break;
 	case S_IFDIR:
 		ret = ext2_create_dir_entries(trans, root, objectid,
-- 
2.39.3

[PATCH 2/4] btrfs-progs: convert: struct blk_iterate_data, add ext2-specific file inode pointers

[Anand Jain]

To obtain the file data extent flags, we require the use of ext2 helper
functions, pass these pointer in the 'struct blk_iterate_data'. However,
this struct is a common function across both 'reiserfs' and 'ext4'
filesystems. Since there is no further development on 'convert-reiserfs',
this patch avoids creating a mess which won't be used.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 convert/source-ext2.c | 4 ++++
 convert/source-fs.h   | 5 +++++
 2 files changed, 9 insertions(+)

diff --git a/convert/source-ext2.c b/convert/source-ext2.c
index a3f61bb01171..625387e95857 100644
--- a/convert/source-ext2.c
+++ b/convert/source-ext2.c
@@ -324,6 +324,10 @@ static int ext2_create_file_extents(struct btrfs_trans_handle *trans,
 	init_blk_iterate_data(&data, trans, root, btrfs_inode, objectid,
 			convert_flags & CONVERT_FLAG_DATACSUM);
 
+	data.ext2_fs = ext2_fs;
+	data.ext2_ino = ext2_ino;
+	data.ext2_inode = ext2_inode;
+
 	err = ext2fs_block_iterate2(ext2_fs, ext2_ino, BLOCK_FLAG_DATA_ONLY,
 				    NULL, ext2_block_iterate_proc, &data);
 	if (err)
diff --git a/convert/source-fs.h b/convert/source-fs.h
index b26e1842941d..0e71e79eddcc 100644
--- a/convert/source-fs.h
+++ b/convert/source-fs.h
@@ -20,6 +20,7 @@
 #include "kerncompat.h"
 #include <sys/types.h>
 #include <pthread.h>
+#include <ext2fs/ext2fs.h>
 #include "kernel-shared/uapi/btrfs_tree.h"
 #include "convert/common.h"
 
@@ -118,6 +119,10 @@ struct btrfs_convert_operations {
 };
 
 struct blk_iterate_data {
+	ext2_filsys ext2_fs;
+	struct ext2_inode *ext2_inode;
+	ext2_ino_t ext2_ino;
+
 	struct btrfs_trans_handle *trans;
 	struct btrfs_root *root;
 	struct btrfs_root *convert_root;
-- 
2.39.3

[PATCH 3/4] btrfs-progs: convert: refactor __btrfs_record_file_extent to add a prealloc flag

[Anand Jain]

This preparatory patch adds an argument '%prealloc' to the function
__btrfs_record_file_extent(), to be used in the following patches.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 common/extent-tree-utils.c | 11 +++++++----
 common/extent-tree-utils.h |  2 +-
 convert/main.c             |  9 +++++----
 convert/source-fs.c        |  5 +++--
 convert/source-reiserfs.c  |  2 +-
 mkfs/rootdir.c             |  3 ++-
 6 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/common/extent-tree-utils.c b/common/extent-tree-utils.c
index 34c7e5095160..2ccac6b44cea 100644
--- a/common/extent-tree-utils.c
+++ b/common/extent-tree-utils.c
@@ -122,7 +122,7 @@ static int __btrfs_record_file_extent(struct btrfs_trans_handle *trans,
 				      struct btrfs_root *root, u64 objectid,
 				      struct btrfs_inode_item *inode,
 				      u64 file_pos, u64 disk_bytenr,
-				      u64 *ret_num_bytes)
+				      u64 *ret_num_bytes, bool prealloc)
 {
 	int ret;
 	struct btrfs_fs_info *info = root->fs_info;
@@ -229,7 +229,10 @@ static int __btrfs_record_file_extent(struct btrfs_trans_handle *trans,
 	leaf = path->nodes[0];
 	fi = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_file_extent_item);
 	btrfs_set_file_extent_generation(leaf, fi, trans->transid);
-	btrfs_set_file_extent_type(leaf, fi, BTRFS_FILE_EXTENT_REG);
+	if (prealloc)
+		btrfs_set_file_extent_type(leaf, fi, BTRFS_FILE_EXTENT_PREALLOC);
+	else
+		btrfs_set_file_extent_type(leaf, fi, BTRFS_FILE_EXTENT_REG);
 	btrfs_set_file_extent_disk_bytenr(leaf, fi, extent_bytenr);
 	btrfs_set_file_extent_disk_num_bytes(leaf, fi, extent_num_bytes);
 	btrfs_set_file_extent_offset(leaf, fi, extent_offset);
@@ -265,7 +268,7 @@ int btrfs_record_file_extent(struct btrfs_trans_handle *trans,
 			     struct btrfs_root *root, u64 objectid,
 			     struct btrfs_inode_item *inode,
 			     u64 file_pos, u64 disk_bytenr,
-			     u64 num_bytes)
+			     u64 num_bytes, bool prealloc)
 {
 	u64 cur_disk_bytenr = disk_bytenr;
 	u64 cur_file_pos = file_pos;
@@ -276,7 +279,7 @@ int btrfs_record_file_extent(struct btrfs_trans_handle *trans,
 		ret = __btrfs_record_file_extent(trans, root, objectid,
 						 inode, cur_file_pos,
 						 cur_disk_bytenr,
-						 &cur_num_bytes);
+						 &cur_num_bytes, prealloc);
 		if (ret < 0)
 			break;
 		cur_disk_bytenr += cur_num_bytes;
diff --git a/common/extent-tree-utils.h b/common/extent-tree-utils.h
index f03d9c438375..7abd0337ea0b 100644
--- a/common/extent-tree-utils.h
+++ b/common/extent-tree-utils.h
@@ -31,6 +31,6 @@ int btrfs_record_file_extent(struct btrfs_trans_handle *trans,
 			     struct btrfs_root *root, u64 objectid,
 			     struct btrfs_inode_item *inode,
 			     u64 file_pos, u64 disk_bytenr,
-			     u64 num_bytes);
+			     u64 num_bytes, bool prealloc);
 
 #endif
diff --git a/convert/main.c b/convert/main.c
index f18fab4a236c..d67c4e8eac25 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -337,7 +337,7 @@ static int create_image_file_range(struct btrfs_trans_handle *trans,
 		return -EINVAL;
 	}
 	ret = btrfs_record_file_extent(trans, root, ino, inode, bytenr,
-				       disk_bytenr, len);
+				       disk_bytenr, len, false);
 	if (ret < 0)
 		return ret;
 
@@ -426,7 +426,7 @@ static int migrate_one_reserved_range(struct btrfs_trans_handle *trans,
 
 		/* Now handle extent item and file extent things */
 		ret = btrfs_record_file_extent(trans, root, ino, inode, cur_off,
-					       key.objectid, key.offset);
+					       key.objectid, key.offset, false);
 		if (ret < 0)
 			break;
 		/* Finally, insert csum items */
@@ -438,7 +438,7 @@ static int migrate_one_reserved_range(struct btrfs_trans_handle *trans,
 		hole_len = cur_off - hole_start;
 		if (hole_len) {
 			ret = btrfs_record_file_extent(trans, root, ino, inode,
-					hole_start, 0, hole_len);
+					hole_start, 0, hole_len, false);
 			if (ret < 0)
 				break;
 		}
@@ -455,7 +455,8 @@ static int migrate_one_reserved_range(struct btrfs_trans_handle *trans,
 	 */
 	if (range_end(range) - hole_start > 0)
 		ret = btrfs_record_file_extent(trans, root, ino, inode,
-				hole_start, 0, range_end(range) - hole_start);
+				hole_start, 0, range_end(range) - hole_start,
+				false);
 	return ret;
 }
 
diff --git a/convert/source-fs.c b/convert/source-fs.c
index 66561438866e..9039b0e66758 100644
--- a/convert/source-fs.c
+++ b/convert/source-fs.c
@@ -262,7 +262,7 @@ int record_file_blocks(struct blk_iterate_data *data,
 	if (old_disk_bytenr == 0)
 		return btrfs_record_file_extent(data->trans, root,
 				data->objectid, data->inode, file_pos, 0,
-				num_bytes);
+				num_bytes, false);
 
 	/*
 	 * Search real disk bytenr from convert root
@@ -316,7 +316,8 @@ int record_file_blocks(struct blk_iterate_data *data,
 			      old_disk_bytenr + num_bytes) - cur_off;
 		ret = btrfs_record_file_extent(data->trans, data->root,
 					data->objectid, data->inode, file_pos,
-					real_disk_bytenr, cur_len);
+					real_disk_bytenr, cur_len,
+					false);
 		if (ret < 0)
 			break;
 		cur_off += cur_len;
diff --git a/convert/source-reiserfs.c b/convert/source-reiserfs.c
index 3edc72ed08a5..c67ade9b4c90 100644
--- a/convert/source-reiserfs.c
+++ b/convert/source-reiserfs.c
@@ -365,7 +365,7 @@ static int convert_direct(struct btrfs_trans_handle *trans,
 		return ret;
 
 	return btrfs_record_file_extent(trans, root, objectid, inode, offset,
-					key.objectid, sectorsize);
+					key.objectid, sectorsize, false);
 }
 
 static int reiserfs_convert_tail(struct btrfs_trans_handle *trans,
diff --git a/mkfs/rootdir.c b/mkfs/rootdir.c
index 4ae9f435a7b7..cb6659319b7d 100644
--- a/mkfs/rootdir.c
+++ b/mkfs/rootdir.c
@@ -411,7 +411,8 @@ again:
 
 	if (bytes_read) {
 		ret = btrfs_record_file_extent(trans, root, objectid,
-				btrfs_inode, file_pos, first_block, cur_bytes);
+				btrfs_inode, file_pos, first_block, cur_bytes,
+				false);
 		if (ret)
 			goto end;
 
-- 
2.39.3

[PATCH RFC 4/4] btrfs-progs: convert: support ext2 unwritten file data extents

[Anand Jain]

This patch, along with the dependent patches below, adds support for
ext4 unwritten file extents as preallocated file extent in btrfs.

 btrfs-progs: convert: refactor ext2_create_file_extents add argument ext2_inode
 btrfs-progs: convert: struct blk_iterate_data, add ext2-specific file inode pointers
 btrfs-progs: convert: refactor __btrfs_record_file_extent to add a prealloc flag

The patch is developed with POV of portability with the current
e2fsprogs library.

Testcase:

     $ dd if=/dev/urandom of=/mnt/test/foo bs=4K count=1 conv=fsync status=none
     $ dd if=/dev/urandom of=/mnt/test/foo bs=4K count=2 conv=fsync seek=1 status=none
     $ xfs_io -f -c 'falloc -k 12K 12K' /mnt/test/foo
     $ dd if=/dev/zero of=/mnt/test/foo bs=4K count=1 conv=fsync seek=6 status=none

     $ filefrag -v /mnt/test/foo
     Filesystem type is: ef53
     File size of /mnt/test/foo is 28672 (7 blocks of 4096 bytes)
	 ext:     logical_offset:        physical_offset: length:   expected: flags:
	   0:        0..       0:      33280..     33280:      1:
	   1:        1..       2:      33792..     33793:      2:      33281:
	   2:        3..       5:      33281..     33283:      3:      33794: unwritten
	   3:        6..       6:      33794..     33794:      1:      33284: last,eof

     $ sha256sum /mnt/test/foo
     18619b678a5c207a971a0aa931604f48162e307c57ecdec450d5f095fe9f32c7  /mnt/test/foo

   Convert and compare the checksum

   Before:

     $ filefrag -v /mnt/test/foo
     Filesystem type is: 9123683e
     File size of /mnt/test/foo is 28672 (7 blocks of 4096 bytes)
      ext:     logical_offset:        physical_offset: length:   expected: flags:
      0:        0..       0:      33280..     33280:      1:             shared
      1:        1..       2:      33792..     33793:      2:      33281: shared
      2:        3..       6:      33281..     33284:      4:      33794: last,shared,eof
     /mnt/test/foo: 3 extents found

     $ sha256sum /mnt/test/foo
     6874a1733e5785682210d69c07f256f684cf5433c7235ed29848b4a4d52030e0  /mnt/test/foo

   After:

     $ filefrag -v /mnt/test/foo
     Filesystem type is: 9123683e
     File size of /mnt/test/foo is 28672 (7 blocks of 4096 bytes)
	 ext:     logical_offset:        physical_offset: length:   expected: flags:
	   0:        0..       0:      33280..     33280:      1:             shared
	   1:        1..       2:      33792..     33793:      2:      33281: shared
	   2:        3..       5:      33281..     33283:      3:      33794: unwritten,shared
	   3:        6..       6:      33794..     33794:      1:      33284: last,shared,eof
     /mnt/test/foo: 4 extents found

     $ sha256sum /mnt/test/foo
     18619b678a5c207a971a0aa931604f48162e307c57ecdec450d5f095fe9f32c7  /mnt/test/foo

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
RFC: Limited tested. Is there a ready file or test case available to
exercise the feature?

 convert/source-fs.c | 49 ++++++++++++++++++++++++++++++++++++++++++++-
 convert/source-fs.h |  1 +
 2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/convert/source-fs.c b/convert/source-fs.c
index 9039b0e66758..647ea1f29060 100644
--- a/convert/source-fs.c
+++ b/convert/source-fs.c
@@ -239,6 +239,45 @@ fail:
 	return ret;
 }
 
+int find_prealloc(struct blk_iterate_data *data, int index, bool *prealloc)
+{
+	ext2_extent_handle_t handle;
+	struct ext2fs_extent extent;
+
+	if (ext2fs_extent_open2(data->ext2_fs, data->ext2_ino,
+				data->ext2_inode, &handle)) {
+		error("ext2fs_extent_open2 failed, inode %d", data->ext2_ino);
+		return -EINVAL;
+	}
+
+	if (ext2fs_extent_goto2(handle, 0, index)) {
+		error("ext2fs_extent_goto2 failed, inode %d num_blocks %llu",
+		       data->ext2_ino, data->num_blocks);
+		ext2fs_extent_free(handle);
+		return -EINVAL;
+	}
+
+	memset(&extent, 0, sizeof(struct ext2fs_extent));
+	if (ext2fs_extent_get(handle, EXT2_EXTENT_CURRENT, &extent)) {
+		error("ext2fs_extent_get EXT2_EXTENT_CURRENT failed inode %d",
+		       data->ext2_ino);
+		ext2fs_extent_free(handle);
+		return -EINVAL;
+	}
+
+	if (extent.e_pblk != data->disk_block) {
+	error("inode %d index %d found wrong extent e_pblk %llu wanted disk_block %llu",
+		       data->ext2_ino, index, extent.e_pblk, data->disk_block);
+		ext2fs_extent_free(handle);
+		return -EINVAL;
+	}
+
+	if (extent.e_flags & EXT2_EXTENT_FLAGS_UNINIT)
+		*prealloc = true;
+
+	return 0;
+}
+
 /*
  * Record a file extent in original filesystem into btrfs one.
  * The special point is, old disk_block can point to a reserved range.
@@ -257,6 +296,7 @@ int record_file_blocks(struct blk_iterate_data *data,
 	u64 old_disk_bytenr = disk_block * sectorsize;
 	u64 num_bytes = num_blocks * sectorsize;
 	u64 cur_off = old_disk_bytenr;
+	int index = data->first_block;
 
 	/* Hole, pass it to record_file_extent directly */
 	if (old_disk_bytenr == 0)
@@ -276,6 +316,12 @@ int record_file_blocks(struct blk_iterate_data *data,
 		u64 extent_num_bytes;
 		u64 real_disk_bytenr;
 		u64 cur_len;
+		bool prealloc = false;
+
+		if (find_prealloc(data, index, &prealloc)) {
+			data->errcode = -1;
+			return -EINVAL;
+		}
 
 		key.objectid = data->convert_ino;
 		key.type = BTRFS_EXTENT_DATA_KEY;
@@ -317,11 +363,12 @@ int record_file_blocks(struct blk_iterate_data *data,
 		ret = btrfs_record_file_extent(data->trans, data->root,
 					data->objectid, data->inode, file_pos,
 					real_disk_bytenr, cur_len,
-					false);
+					prealloc);
 		if (ret < 0)
 			break;
 		cur_off += cur_len;
 		file_pos += cur_len;
+		index++;
 
 		/*
 		 * No need to care about csum
diff --git a/convert/source-fs.h b/convert/source-fs.h
index 0e71e79eddcc..3922c444de10 100644
--- a/convert/source-fs.h
+++ b/convert/source-fs.h
@@ -156,5 +156,6 @@ int read_disk_extent(struct btrfs_root *root, u64 bytenr,
 		            u32 num_bytes, char *buffer);
 int record_file_blocks(struct blk_iterate_data *data,
 			      u64 file_block, u64 disk_block, u64 num_blocks);
+int find_prealloc(struct blk_iterate_data *data, int index, bool *prealloc);
 
 #endif
-- 
2.39.3

Re: [PATCH RFC 4/4] btrfs-progs: convert: support ext2 unwritten file data extents

[Qu Wenruo]

在 2024/5/3 18:38, Anand Jain 写道:
> This patch, along with the dependent patches below, adds support for
> ext4 unwritten file extents as preallocated file extent in btrfs.
> 
>   btrfs-progs: convert: refactor ext2_create_file_extents add argument ext2_inode
>   btrfs-progs: convert: struct blk_iterate_data, add ext2-specific file inode pointers
>   btrfs-progs: convert: refactor __btrfs_record_file_extent to add a prealloc flag
> 
> The patch is developed with POV of portability with the current
> e2fsprogs library.
> 
> Testcase:
> 
>       $ dd if=/dev/urandom of=/mnt/test/foo bs=4K count=1 conv=fsync status=none
>       $ dd if=/dev/urandom of=/mnt/test/foo bs=4K count=2 conv=fsync seek=1 status=none
>       $ xfs_io -f -c 'falloc -k 12K 12K' /mnt/test/foo
>       $ dd if=/dev/zero of=/mnt/test/foo bs=4K count=1 conv=fsync seek=6 status=none
> 
>       $ filefrag -v /mnt/test/foo
>       Filesystem type is: ef53
>       File size of /mnt/test/foo is 28672 (7 blocks of 4096 bytes)
> 	 ext:     logical_offset:        physical_offset: length:   expected: flags:
> 	   0:        0..       0:      33280..     33280:      1:
> 	   1:        1..       2:      33792..     33793:      2:      33281:
> 	   2:        3..       5:      33281..     33283:      3:      33794: unwritten
> 	   3:        6..       6:      33794..     33794:      1:      33284: last,eof
> 
>       $ sha256sum /mnt/test/foo
>       18619b678a5c207a971a0aa931604f48162e307c57ecdec450d5f095fe9f32c7  /mnt/test/foo
> 
>     Convert and compare the checksum
> 
>     Before:
> 
>       $ filefrag -v /mnt/test/foo
>       Filesystem type is: 9123683e
>       File size of /mnt/test/foo is 28672 (7 blocks of 4096 bytes)
>        ext:     logical_offset:        physical_offset: length:   expected: flags:
>        0:        0..       0:      33280..     33280:      1:             shared
>        1:        1..       2:      33792..     33793:      2:      33281: shared
>        2:        3..       6:      33281..     33284:      4:      33794: last,shared,eof
>       /mnt/test/foo: 3 extents found
> 
>       $ sha256sum /mnt/test/foo
>       6874a1733e5785682210d69c07f256f684cf5433c7235ed29848b4a4d52030e0  /mnt/test/foo
> 
>     After:
> 
>       $ filefrag -v /mnt/test/foo
>       Filesystem type is: 9123683e
>       File size of /mnt/test/foo is 28672 (7 blocks of 4096 bytes)
> 	 ext:     logical_offset:        physical_offset: length:   expected: flags:
> 	   0:        0..       0:      33280..     33280:      1:             shared
> 	   1:        1..       2:      33792..     33793:      2:      33281: shared
> 	   2:        3..       5:      33281..     33283:      3:      33794: unwritten,shared
> 	   3:        6..       6:      33794..     33794:      1:      33284: last,shared,eof
>       /mnt/test/foo: 4 extents found
> 
>       $ sha256sum /mnt/test/foo
>       18619b678a5c207a971a0aa931604f48162e307c57ecdec450d5f095fe9f32c7  /mnt/test/foo
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> RFC: Limited tested. Is there a ready file or test case available to
> exercise the feature?
> 
>   convert/source-fs.c | 49 ++++++++++++++++++++++++++++++++++++++++++++-
>   convert/source-fs.h |  1 +
>   2 files changed, 49 insertions(+), 1 deletion(-)
> 
> diff --git a/convert/source-fs.c b/convert/source-fs.c
> index 9039b0e66758..647ea1f29060 100644
> --- a/convert/source-fs.c
> +++ b/convert/source-fs.c
> @@ -239,6 +239,45 @@ fail:
>   	return ret;
>   }
>   
> +int find_prealloc(struct blk_iterate_data *data, int index, bool *prealloc)

This function is called for every file extent we're going to create.
I'm not a big fan of doing so many lookup.

My question is, is this the only way to determine the flag of the data 
extent?

My instinct says there should be a straight forward way to determine if 
a file extent is preallocated or not, just like what we do in our file 
extent items.
Thus during the ext2fs_block_iterate2(), there should be some way to 
tell if a block is preallocated or not.

Thus adding ext4 ML to get some feedback.

Thanks,
Qu

> +{
> +	ext2_extent_handle_t handle;
> +	struct ext2fs_extent extent;
> +
> +	if (ext2fs_extent_open2(data->ext2_fs, data->ext2_ino,
> +				data->ext2_inode, &handle)) {
> +		error("ext2fs_extent_open2 failed, inode %d", data->ext2_ino);
> +		return -EINVAL;
> +	}
> +
> +	if (ext2fs_extent_goto2(handle, 0, index)) {
> +		error("ext2fs_extent_goto2 failed, inode %d num_blocks %llu",
> +		       data->ext2_ino, data->num_blocks);
> +		ext2fs_extent_free(handle);
> +		return -EINVAL;
> +	}
> +
> +	memset(&extent, 0, sizeof(struct ext2fs_extent));
> +	if (ext2fs_extent_get(handle, EXT2_EXTENT_CURRENT, &extent)) {
> +		error("ext2fs_extent_get EXT2_EXTENT_CURRENT failed inode %d",
> +		       data->ext2_ino);
> +		ext2fs_extent_free(handle);
> +		return -EINVAL;
> +	}
> +
> +	if (extent.e_pblk != data->disk_block) {
> +	error("inode %d index %d found wrong extent e_pblk %llu wanted disk_block %llu",
> +		       data->ext2_ino, index, extent.e_pblk, data->disk_block);
> +		ext2fs_extent_free(handle);
> +		return -EINVAL;
> +	}
> +
> +	if (extent.e_flags & EXT2_EXTENT_FLAGS_UNINIT)
> +		*prealloc = true;
> +
> +	return 0;
> +}
> +
>   /*
>    * Record a file extent in original filesystem into btrfs one.
>    * The special point is, old disk_block can point to a reserved range.
> @@ -257,6 +296,7 @@ int record_file_blocks(struct blk_iterate_data *data,
>   	u64 old_disk_bytenr = disk_block * sectorsize;
>   	u64 num_bytes = num_blocks * sectorsize;
>   	u64 cur_off = old_disk_bytenr;
> +	int index = data->first_block;
>   
>   	/* Hole, pass it to record_file_extent directly */
>   	if (old_disk_bytenr == 0)
> @@ -276,6 +316,12 @@ int record_file_blocks(struct blk_iterate_data *data,
>   		u64 extent_num_bytes;
>   		u64 real_disk_bytenr;
>   		u64 cur_len;
> +		bool prealloc = false;
> +
> +		if (find_prealloc(data, index, &prealloc)) {
> +			data->errcode = -1;
> +			return -EINVAL;
> +		}
>   
>   		key.objectid = data->convert_ino;
>   		key.type = BTRFS_EXTENT_DATA_KEY;
> @@ -317,11 +363,12 @@ int record_file_blocks(struct blk_iterate_data *data,
>   		ret = btrfs_record_file_extent(data->trans, data->root,
>   					data->objectid, data->inode, file_pos,
>   					real_disk_bytenr, cur_len,
> -					false);
> +					prealloc);
>   		if (ret < 0)
>   			break;
>   		cur_off += cur_len;
>   		file_pos += cur_len;
> +		index++;
>   
>   		/*
>   		 * No need to care about csum
> diff --git a/convert/source-fs.h b/convert/source-fs.h
> index 0e71e79eddcc..3922c444de10 100644
> --- a/convert/source-fs.h
> +++ b/convert/source-fs.h
> @@ -156,5 +156,6 @@ int read_disk_extent(struct btrfs_root *root, u64 bytenr,
>   		            u32 num_bytes, char *buffer);
>   int record_file_blocks(struct blk_iterate_data *data,
>   			      u64 file_block, u64 disk_block, u64 num_blocks);
> +int find_prealloc(struct blk_iterate_data *data, int index, bool *prealloc);
>   
>   #endif

Re: [PATCH 2/4] btrfs-progs: convert: struct blk_iterate_data, add ext2-specific file inode pointers

[David Sterba]

On Fri, May 03, 2024 at 05:08:53PM +0800, Anand Jain wrote:
> To obtain the file data extent flags, we require the use of ext2 helper
> functions, pass these pointer in the 'struct blk_iterate_data'. However,
> this struct is a common function across both 'reiserfs' and 'ext4'
> filesystems. Since there is no further development on 'convert-reiserfs',
> this patch avoids creating a mess which won't be used.

Even though there will be no more reiserfs development you should not
clutter the generic API for filesystems with ext2-specific members.

> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  convert/source-ext2.c | 4 ++++
>  convert/source-fs.h   | 5 +++++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/convert/source-ext2.c b/convert/source-ext2.c
> index a3f61bb01171..625387e95857 100644
> --- a/convert/source-ext2.c
> +++ b/convert/source-ext2.c
> @@ -324,6 +324,10 @@ static int ext2_create_file_extents(struct btrfs_trans_handle *trans,
>  	init_blk_iterate_data(&data, trans, root, btrfs_inode, objectid,
>  			convert_flags & CONVERT_FLAG_DATACSUM);
>  
> +	data.ext2_fs = ext2_fs;
> +	data.ext2_ino = ext2_ino;
> +	data.ext2_inode = ext2_inode;
> +
>  	err = ext2fs_block_iterate2(ext2_fs, ext2_ino, BLOCK_FLAG_DATA_ONLY,
>  				    NULL, ext2_block_iterate_proc, &data);
>  	if (err)
> diff --git a/convert/source-fs.h b/convert/source-fs.h
> index b26e1842941d..0e71e79eddcc 100644
> --- a/convert/source-fs.h
> +++ b/convert/source-fs.h
> @@ -20,6 +20,7 @@
>  #include "kerncompat.h"
>  #include <sys/types.h>
>  #include <pthread.h>
> +#include <ext2fs/ext2fs.h>
>  #include "kernel-shared/uapi/btrfs_tree.h"
>  #include "convert/common.h"
>  
> @@ -118,6 +119,10 @@ struct btrfs_convert_operations {
>  };
>  
>  struct blk_iterate_data {
> +	ext2_filsys ext2_fs;
> +	struct ext2_inode *ext2_inode;
> +	ext2_ino_t ext2_ino;

This should be a void pointer filled by the target filesystem
implementation that fills it with anything it needs.

> +
>  	struct btrfs_trans_handle *trans;
>  	struct btrfs_root *root;
>  	struct btrfs_root *convert_root;
> -- 
> 2.39.3
> 

Re: [PATCH 3/4] btrfs-progs: convert: refactor __btrfs_record_file_extent to add a prealloc flag

[David Sterba]

On Fri, May 03, 2024 at 05:08:54PM +0800, Anand Jain wrote:
> This preparatory patch adds an argument '%prealloc' to the function
> __btrfs_record_file_extent(), to be used in the following patches.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  common/extent-tree-utils.c | 11 +++++++----
>  common/extent-tree-utils.h |  2 +-
>  convert/main.c             |  9 +++++----
>  convert/source-fs.c        |  5 +++--
>  convert/source-reiserfs.c  |  2 +-
>  mkfs/rootdir.c             |  3 ++-
>  6 files changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/common/extent-tree-utils.c b/common/extent-tree-utils.c
> index 34c7e5095160..2ccac6b44cea 100644
> --- a/common/extent-tree-utils.c
> +++ b/common/extent-tree-utils.c
> @@ -122,7 +122,7 @@ static int __btrfs_record_file_extent(struct btrfs_trans_handle *trans,
>  				      struct btrfs_root *root, u64 objectid,
>  				      struct btrfs_inode_item *inode,
>  				      u64 file_pos, u64 disk_bytenr,
> -				      u64 *ret_num_bytes)
> +				      u64 *ret_num_bytes, bool prealloc)
>  {
>  	int ret;
>  	struct btrfs_fs_info *info = root->fs_info;
> @@ -229,7 +229,10 @@ static int __btrfs_record_file_extent(struct btrfs_trans_handle *trans,
>  	leaf = path->nodes[0];
>  	fi = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_file_extent_item);
>  	btrfs_set_file_extent_generation(leaf, fi, trans->transid);
> -	btrfs_set_file_extent_type(leaf, fi, BTRFS_FILE_EXTENT_REG);
> +	if (prealloc)
> +		btrfs_set_file_extent_type(leaf, fi, BTRFS_FILE_EXTENT_PREALLOC);
> +	else
> +		btrfs_set_file_extent_type(leaf, fi, BTRFS_FILE_EXTENT_REG);

The bool parameter makes it less clear what it means in all the callers,
as it is supposed to select the type of extent you could pass the
BTRFS_FILE_ExTENT_ constant directly.

Re: [PATCH RFC 4/4] btrfs-progs: convert: support ext2 unwritten file data extents

[Anand Jain]

On 5/3/24 17:37, Qu Wenruo wrote:
> 
> 
> 在 2024/5/3 18:38, Anand Jain 写道:
>> This patch, along with the dependent patches below, adds support for
>> ext4 unwritten file extents as preallocated file extent in btrfs.
>>
>>   btrfs-progs: convert: refactor ext2_create_file_extents add argument 
>> ext2_inode
>>   btrfs-progs: convert: struct blk_iterate_data, add ext2-specific 
>> file inode pointers
>>   btrfs-progs: convert: refactor __btrfs_record_file_extent to add a 
>> prealloc flag
>>
>> The patch is developed with POV of portability with the current
>> e2fsprogs library.
>>
>> Testcase:
>>
>>       $ dd if=/dev/urandom of=/mnt/test/foo bs=4K count=1 conv=fsync 
>> status=none
>>       $ dd if=/dev/urandom of=/mnt/test/foo bs=4K count=2 conv=fsync 
>> seek=1 status=none
>>       $ xfs_io -f -c 'falloc -k 12K 12K' /mnt/test/foo
>>       $ dd if=/dev/zero of=/mnt/test/foo bs=4K count=1 conv=fsync 
>> seek=6 status=none
>>
>>       $ filefrag -v /mnt/test/foo
>>       Filesystem type is: ef53
>>       File size of /mnt/test/foo is 28672 (7 blocks of 4096 bytes)
>>      ext:     logical_offset:        physical_offset: length:   
>> expected: flags:
>>        0:        0..       0:      33280..     33280:      1:
>>        1:        1..       2:      33792..     33793:      2:      33281:
>>        2:        3..       5:      33281..     33283:      3:      
>> 33794: unwritten
>>        3:        6..       6:      33794..     33794:      1:      
>> 33284: last,eof
>>
>>       $ sha256sum /mnt/test/foo
>>       
>> 18619b678a5c207a971a0aa931604f48162e307c57ecdec450d5f095fe9f32c7  
>> /mnt/test/foo
>>
>>     Convert and compare the checksum
>>
>>     Before:
>>
>>       $ filefrag -v /mnt/test/foo
>>       Filesystem type is: 9123683e
>>       File size of /mnt/test/foo is 28672 (7 blocks of 4096 bytes)
>>        ext:     logical_offset:        physical_offset: length:   
>> expected: flags:
>>        0:        0..       0:      33280..     33280:      
>> 1:             shared
>>        1:        1..       2:      33792..     33793:      2:      
>> 33281: shared
>>        2:        3..       6:      33281..     33284:      4:      
>> 33794: last,shared,eof
>>       /mnt/test/foo: 3 extents found
>>
>>       $ sha256sum /mnt/test/foo
>>       
>> 6874a1733e5785682210d69c07f256f684cf5433c7235ed29848b4a4d52030e0  
>> /mnt/test/foo
>>
>>     After:
>>
>>       $ filefrag -v /mnt/test/foo
>>       Filesystem type is: 9123683e
>>       File size of /mnt/test/foo is 28672 (7 blocks of 4096 bytes)
>>      ext:     logical_offset:        physical_offset: length:   
>> expected: flags:
>>        0:        0..       0:      33280..     33280:      
>> 1:             shared
>>        1:        1..       2:      33792..     33793:      2:      
>> 33281: shared
>>        2:        3..       5:      33281..     33283:      3:      
>> 33794: unwritten,shared
>>        3:        6..       6:      33794..     33794:      1:      
>> 33284: last,shared,eof
>>       /mnt/test/foo: 4 extents found
>>
>>       $ sha256sum /mnt/test/foo
>>       
>> 18619b678a5c207a971a0aa931604f48162e307c57ecdec450d5f095fe9f32c7  
>> /mnt/test/foo
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>> RFC: Limited tested. Is there a ready file or test case available to
>> exercise the feature?
>>
>>   convert/source-fs.c | 49 ++++++++++++++++++++++++++++++++++++++++++++-
>>   convert/source-fs.h |  1 +
>>   2 files changed, 49 insertions(+), 1 deletion(-)
>>
>> diff --git a/convert/source-fs.c b/convert/source-fs.c
>> index 9039b0e66758..647ea1f29060 100644
>> --- a/convert/source-fs.c
>> +++ b/convert/source-fs.c
>> @@ -239,6 +239,45 @@ fail:
>>       return ret;
>>   }
>> +int find_prealloc(struct blk_iterate_data *data, int index, bool 
>> *prealloc)
> 
> This function is called for every file extent we're going to create.
> I'm not a big fan of doing so many lookup.
> 
> My question is, is this the only way to determine the flag of the data 
> extent?
> 
> My instinct says there should be a straight forward way to determine if 
> a file extent is preallocated or not, just like what we do in our file 
> extent items.


> Thus during the ext2fs_block_iterate2(), there should be some way to 
> tell if a block is preallocated or not.

Unfortunately, the callback doesn't provide the extent flags. Unless,  I 
miss something?

Thx,  Anand


> 
> Thus adding ext4 ML to get some feedback.
> 
> Thanks,
> Qu
> 
>> +{
>> +    ext2_extent_handle_t handle;
>> +    struct ext2fs_extent extent;
>> +
>> +    if (ext2fs_extent_open2(data->ext2_fs, data->ext2_ino,
>> +                data->ext2_inode, &handle)) {
>> +        error("ext2fs_extent_open2 failed, inode %d", data->ext2_ino);
>> +        return -EINVAL;
>> +    }
>> +
>> +    if (ext2fs_extent_goto2(handle, 0, index)) {
>> +        error("ext2fs_extent_goto2 failed, inode %d num_blocks %llu",
>> +               data->ext2_ino, data->num_blocks);
>> +        ext2fs_extent_free(handle);
>> +        return -EINVAL;
>> +    }
>> +
>> +    memset(&extent, 0, sizeof(struct ext2fs_extent));
>> +    if (ext2fs_extent_get(handle, EXT2_EXTENT_CURRENT, &extent)) {
>> +        error("ext2fs_extent_get EXT2_EXTENT_CURRENT failed inode %d",
>> +               data->ext2_ino);
>> +        ext2fs_extent_free(handle);
>> +        return -EINVAL;
>> +    }
>> +
>> +    if (extent.e_pblk != data->disk_block) {
>> +    error("inode %d index %d found wrong extent e_pblk %llu wanted 
>> disk_block %llu",
>> +               data->ext2_ino, index, extent.e_pblk, data->disk_block);
>> +        ext2fs_extent_free(handle);
>> +        return -EINVAL;
>> +    }
>> +
>> +    if (extent.e_flags & EXT2_EXTENT_FLAGS_UNINIT)
>> +        *prealloc = true;
>> +
>> +    return 0;
>> +}
>> +
>>   /*
>>    * Record a file extent in original filesystem into btrfs one.
>>    * The special point is, old disk_block can point to a reserved range.
>> @@ -257,6 +296,7 @@ int record_file_blocks(struct blk_iterate_data *data,
>>       u64 old_disk_bytenr = disk_block * sectorsize;
>>       u64 num_bytes = num_blocks * sectorsize;
>>       u64 cur_off = old_disk_bytenr;
>> +    int index = data->first_block;
>>       /* Hole, pass it to record_file_extent directly */
>>       if (old_disk_bytenr == 0)
>> @@ -276,6 +316,12 @@ int record_file_blocks(struct blk_iterate_data 
>> *data,
>>           u64 extent_num_bytes;
>>           u64 real_disk_bytenr;
>>           u64 cur_len;
>> +        bool prealloc = false;
>> +
>> +        if (find_prealloc(data, index, &prealloc)) {
>> +            data->errcode = -1;
>> +            return -EINVAL;
>> +        }
>>           key.objectid = data->convert_ino;
>>           key.type = BTRFS_EXTENT_DATA_KEY;
>> @@ -317,11 +363,12 @@ int record_file_blocks(struct blk_iterate_data 
>> *data,
>>           ret = btrfs_record_file_extent(data->trans, data->root,
>>                       data->objectid, data->inode, file_pos,
>>                       real_disk_bytenr, cur_len,
>> -                    false);
>> +                    prealloc);
>>           if (ret < 0)
>>               break;
>>           cur_off += cur_len;
>>           file_pos += cur_len;
>> +        index++;
>>           /*
>>            * No need to care about csum
>> diff --git a/convert/source-fs.h b/convert/source-fs.h
>> index 0e71e79eddcc..3922c444de10 100644
>> --- a/convert/source-fs.h
>> +++ b/convert/source-fs.h
>> @@ -156,5 +156,6 @@ int read_disk_extent(struct btrfs_root *root, u64 
>> bytenr,
>>                       u32 num_bytes, char *buffer);
>>   int record_file_blocks(struct blk_iterate_data *data,
>>                     u64 file_block, u64 disk_block, u64 num_blocks);
>> +int find_prealloc(struct blk_iterate_data *data, int index, bool 
>> *prealloc);
>>   #endif

Re: [PATCH 2/4] btrfs-progs: convert: struct blk_iterate_data, add ext2-specific file inode pointers

[Anand Jain]

On 5/3/24 19:49, David Sterba wrote:
> On Fri, May 03, 2024 at 05:08:53PM +0800, Anand Jain wrote:
>> To obtain the file data extent flags, we require the use of ext2 helper
>> functions, pass these pointer in the 'struct blk_iterate_data'. However,
>> this struct is a common function across both 'reiserfs' and 'ext4'
>> filesystems. Since there is no further development on 'convert-reiserfs',
>> this patch avoids creating a mess which won't be used.
> 
> Even though there will be no more reiserfs development you should not
> clutter the generic API for filesystems with ext2-specific members.
> 
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>>   convert/source-ext2.c | 4 ++++
>>   convert/source-fs.h   | 5 +++++
>>   2 files changed, 9 insertions(+)
>>
>> diff --git a/convert/source-ext2.c b/convert/source-ext2.c
>> index a3f61bb01171..625387e95857 100644
>> --- a/convert/source-ext2.c
>> +++ b/convert/source-ext2.c
>> @@ -324,6 +324,10 @@ static int ext2_create_file_extents(struct btrfs_trans_handle *trans,
>>   	init_blk_iterate_data(&data, trans, root, btrfs_inode, objectid,
>>   			convert_flags & CONVERT_FLAG_DATACSUM);
>>   
>> +	data.ext2_fs = ext2_fs;
>> +	data.ext2_ino = ext2_ino;
>> +	data.ext2_inode = ext2_inode;
>> +
>>   	err = ext2fs_block_iterate2(ext2_fs, ext2_ino, BLOCK_FLAG_DATA_ONLY,
>>   				    NULL, ext2_block_iterate_proc, &data);
>>   	if (err)
>> diff --git a/convert/source-fs.h b/convert/source-fs.h
>> index b26e1842941d..0e71e79eddcc 100644
>> --- a/convert/source-fs.h
>> +++ b/convert/source-fs.h
>> @@ -20,6 +20,7 @@
>>   #include "kerncompat.h"
>>   #include <sys/types.h>
>>   #include <pthread.h>
>> +#include <ext2fs/ext2fs.h>
>>   #include "kernel-shared/uapi/btrfs_tree.h"
>>   #include "convert/common.h"
>>   
>> @@ -118,6 +119,10 @@ struct btrfs_convert_operations {
>>   };
>>   
>>   struct blk_iterate_data {
>> +	ext2_filsys ext2_fs;
>> +	struct ext2_inode *ext2_inode;
>> +	ext2_ino_t ext2_ino;
> 
> This should be a void pointer filled by the target filesystem
> implementation that fills it with anything it needs.
> 

Thanks for the suggestions.

I hope the following will be better.

struct blk_iterate_data {

+ void *source_fs_data;

::
}


struct ext2_source_fs_data {
	ext2_filsys ext2_fs;
	struct ext2_inode *ext2_inode;
	ext2_ino_t ext2_ino;
}

do malloc() and free() in ext2_create_file_extents().


Thx
Anand

>> +
>>   	struct btrfs_trans_handle *trans;
>>   	struct btrfs_root *root;
>>   	struct btrfs_root *convert_root;
>> -- 
>> 2.39.3
>>

Re: [PATCH 3/4] btrfs-progs: convert: refactor __btrfs_record_file_extent to add a prealloc flag

[Anand Jain]

On 5/3/24 19:50, David Sterba wrote:
> On Fri, May 03, 2024 at 05:08:54PM +0800, Anand Jain wrote:
>> This preparatory patch adds an argument '%prealloc' to the function
>> __btrfs_record_file_extent(), to be used in the following patches.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>>   common/extent-tree-utils.c | 11 +++++++----
>>   common/extent-tree-utils.h |  2 +-
>>   convert/main.c             |  9 +++++----
>>   convert/source-fs.c        |  5 +++--
>>   convert/source-reiserfs.c  |  2 +-
>>   mkfs/rootdir.c             |  3 ++-
>>   6 files changed, 19 insertions(+), 13 deletions(-)
>>
>> diff --git a/common/extent-tree-utils.c b/common/extent-tree-utils.c
>> index 34c7e5095160..2ccac6b44cea 100644
>> --- a/common/extent-tree-utils.c
>> +++ b/common/extent-tree-utils.c
>> @@ -122,7 +122,7 @@ static int __btrfs_record_file_extent(struct btrfs_trans_handle *trans,
>>   				      struct btrfs_root *root, u64 objectid,
>>   				      struct btrfs_inode_item *inode,
>>   				      u64 file_pos, u64 disk_bytenr,
>> -				      u64 *ret_num_bytes)
>> +				      u64 *ret_num_bytes, bool prealloc)
>>   {
>>   	int ret;
>>   	struct btrfs_fs_info *info = root->fs_info;
>> @@ -229,7 +229,10 @@ static int __btrfs_record_file_extent(struct btrfs_trans_handle *trans,
>>   	leaf = path->nodes[0];
>>   	fi = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_file_extent_item);
>>   	btrfs_set_file_extent_generation(leaf, fi, trans->transid);
>> -	btrfs_set_file_extent_type(leaf, fi, BTRFS_FILE_EXTENT_REG);
>> +	if (prealloc)
>> +		btrfs_set_file_extent_type(leaf, fi, BTRFS_FILE_EXTENT_PREALLOC);
>> +	else
>> +		btrfs_set_file_extent_type(leaf, fi, BTRFS_FILE_EXTENT_REG);
> 
> The bool parameter makes it less clear what it means in all the callers,
> as it is supposed to select the type of extent you could pass the
> BTRFS_FILE_ExTENT_ constant directly.


oh. ok. It can be done.

Thx, Anand

Re: [PATCH RFC 4/4] btrfs-progs: convert: support ext2 unwritten file data extents

[Qu Wenruo]

在 2024/5/3 21:55, Anand Jain 写道:
[...]
>>> +int find_prealloc(struct blk_iterate_data *data, int index, bool 
>>> *prealloc)
>>
>> This function is called for every file extent we're going to create.
>> I'm not a big fan of doing so many lookup.
>>
>> My question is, is this the only way to determine the flag of the data 
>> extent?
>>
>> My instinct says there should be a straight forward way to determine 
>> if a file extent is preallocated or not, just like what we do in our 
>> file extent items.
> 
> 
>> Thus during the ext2fs_block_iterate2(), there should be some way to 
>> tell if a block is preallocated or not.
> 
> Unfortunately, the callback doesn't provide the extent flags. Unless,  I 
> miss something?

You're right, the iterator interface does not provide any extra info.

And I also checked the kernel implementation, they have extra 
ext4_map_blocks() to do the resolve, and then ext4_es_lookup_extent() to 
determine if it's unwritten.

So I'm afraid we have to go this solution.


Meanwhile related to the implementation, can we put the prealloc flat 
into blk_iterate_data?
So that we can handle different fses' preallocated extents in a more 
common way.

Thanks,
Qu

Re: [PATCH RFC 4/4] btrfs-progs: convert: support ext2 unwritten file data extents

[Anand Jain]

On 5/4/24 06:23, Qu Wenruo wrote:
> 
> 
> 在 2024/5/3 21:55, Anand Jain 写道:
> [...]
>>>> +int find_prealloc(struct blk_iterate_data *data, int index, bool 
>>>> *prealloc)
>>>
>>> This function is called for every file extent we're going to create.
>>> I'm not a big fan of doing so many lookup.
>>>
>>> My question is, is this the only way to determine the flag of the 
>>> data extent?
>>>
>>> My instinct says there should be a straight forward way to determine 
>>> if a file extent is preallocated or not, just like what we do in our 
>>> file extent items.
>>
>>
>>> Thus during the ext2fs_block_iterate2(), there should be some way to 
>>> tell if a block is preallocated or not.
>>
>> Unfortunately, the callback doesn't provide the extent flags. Unless,  
>> I miss something?
> 
> You're right, the iterator interface does not provide any extra info.
> 
> And I also checked the kernel implementation, they have extra 
> ext4_map_blocks() to do the resolve, and then ext4_es_lookup_extent() to 
> determine if it's unwritten.
> 
> So I'm afraid we have to go this solution.
> 
> 
> Meanwhile related to the implementation, can we put the prealloc flat 
> into blk_iterate_data?
> So that we can handle different fses' preallocated extents in a more 
> common way.
> 

I initially thought so, but is blk_iterate_data::num_blocks always
equal to extent::e_len in all file data extent situations mixed
with hole and unwritten combinations? If not, then the flag might
not be appropriate there, as it doesn't apply to the entirety of
blk_iterate_data::num_blocks.

Thanks, Anand

> Thanks,
> Qu

Re: [PATCH RFC 4/4] btrfs-progs: convert: support ext2 unwritten file data extents

[Qu Wenruo]

在 2024/5/4 08:57, Anand Jain 写道:
> 
> 
> On 5/4/24 06:23, Qu Wenruo wrote:
>>
>>
>> 在 2024/5/3 21:55, Anand Jain 写道:
>> [...]
>>>>> +int find_prealloc(struct blk_iterate_data *data, int index, bool 
>>>>> *prealloc)
>>>>
>>>> This function is called for every file extent we're going to create.
>>>> I'm not a big fan of doing so many lookup.
>>>>
>>>> My question is, is this the only way to determine the flag of the 
>>>> data extent?
>>>>
>>>> My instinct says there should be a straight forward way to determine 
>>>> if a file extent is preallocated or not, just like what we do in our 
>>>> file extent items.
>>>
>>>
>>>> Thus during the ext2fs_block_iterate2(), there should be some way to 
>>>> tell if a block is preallocated or not.
>>>
>>> Unfortunately, the callback doesn't provide the extent flags. Unless, 
>>> I miss something?
>>
>> You're right, the iterator interface does not provide any extra info.
>>
>> And I also checked the kernel implementation, they have extra 
>> ext4_map_blocks() to do the resolve, and then ext4_es_lookup_extent() 
>> to determine if it's unwritten.
>>
>> So I'm afraid we have to go this solution.
>>
>>
>> Meanwhile related to the implementation, can we put the prealloc flat 
>> into blk_iterate_data?
>> So that we can handle different fses' preallocated extents in a more 
>> common way.
>>
> 
> I initially thought so, but is blk_iterate_data::num_blocks always
> equal to extent::e_len in all file data extent situations mixed
> with hole and unwritten combinations? If not, then the flag might
> not be appropriate there, as it doesn't apply to the entirety of
> blk_iterate_data::num_blocks.

I do not think we need @num_blocks to match extent_len.

We're already doing some kind of merge inside block_iterate_proc(), and 
if we find previous extent flag doesn't match the current one, we just 
need to submit the previous one.

Although I also believe we need some better abstraction for the common code.
The current one doesn't explain everything well for things parameters 
like disk_block/file_block.

Thanks,
Qu


> 
> Thanks, Anand
> 
>> Thanks,
>> Qu