btrfs-progs: refactor around btrfs_insert_file_extent() #919
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The function btrfs_record_file_extent() has extra handling that's specific to convert, like allowing the range to be split by block group boundary and image file extent boundary. All of these split can only lead to corruption for non-converted fs. As the only caller out of btrfs-convert is rootdir, which expects the file extent item insert to respect the reserved data extent, and never to be split. Thankfully this is not going to cause huge problem, as btrfs_record_file_extent() has extra checks if the data extent overlaps with any existing one, and if it doesn't the handling will be the same as the kernel. But to avoid abuse, change btrfs_record_file_extent() by: - Rename it to btrfs_convert_file_extent() And add extra comments on that it is specific to btrfs-convert. - Move it to convert/common.[ch] - Introduce a helper insert_reserved_file_extent() for rootdir.c Signed-off-by: Qu Wenruo <[email protected]>
…le extent item Just like insert_reserved_file_extent() from the kernel, we can make btrfs_insert_file_extent() to accept an on-stack file extent item directly. This makes btrfs_insert_file_extent() more flex, and it can now handle the converted file extent where it has an non-zero offset. And this makes it much easier to expand for future compressed file extent generation. Signed-off-by: Qu Wenruo <[email protected]>
|
These changes look sensible to me. They certainly make my compression code a lot simpler. There's a couple of existing issues still with insert_reserved_file_extent (it confuses num_bytes and disk_num_bytes, and sets ins_key but doesn't use it), but I'll fix them with my patches. English nitpick too in the patch title: no "to" in "btrfs-progs: make btrfs_insert_file_extent() to accept an on-stack file extent item". I know this makes no sense: it's "order/force/tell/advise X to Y", but "make X Y". |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Although btrfs_insert_file_extent() is a btrfs-progs specific function,
it's pretty close (although not yet similiar enough to cross-port) to
insert_reserved_file_extent() from kernel.
That kernel function uses an on-stack btrfs_file_extent_item to pass all
the needed parameters, supporting both compressed and uncompressed data
extents.
That is way more flex than btrfs_insert_file_extent() from btrfs-progs,
which can not support:
To prepare for the incoming support of compressed data extents
generation for mkfs --rootdir, we need a more generic way to insert file
extents.
This patch improve the situation by:
Move btrfs_record_file_extent() to be a convert specific function
The extra handling are all for converted btrfs, and can split extents
where regular btrfs doesn't want.
For mkfs/rootdir.c, the only caller out of convert, introduce a
helper, insert_reserved_file_extent() to handle the case.
Make btrfs_insert_file_extent() to accept an on-stack btrfs_file_extent_item
Just like insert_reserved_file_extent() from kernel.
Allowing us to customize ever member of the btrfs_file_extent_item.
Now this makes btrfs_insert_file_extent() flex enough for converted
fs, and the incoming compressed file extents.