[WIP] mke2fs: enable copying of fs-verity metadata #203
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
Right now we jump to the end as soon as we've found a method that works. This is a reasonable approach because it's the last operation in the function, but soon it won't be. Switch to a logically-equivalent alternative approach: keep trying until we find the approach that works, dropping the `goto out`. Now we can add code after this. Signed-off-by: Allison Karlitskaya <[email protected]>
|
@tytso This PR is working but there are some things I don't like about it. Can you please give advice here about how I can fix those ugly corners up? If not, I can try to clean things up on my own... |
When writing data to an inode (with mke2fs -d) we need to do the typical loop to handle partial writes to make sure all of the data gets written. Move that code to its own function. This function also takes an offset parameter, which makes it feel a bit like pwrite() (except that it does modify the file offset). Signed-off-by: Allison Karlitskaya <[email protected]>
When creating a filesystem with `mke2fs -O verity` and populating content via `-d`, check if that content is fs-verity enabled, and if it is, copy the fs-verity metadata from the host-native filesystem into the created filesystem. This currently doesn't work for reading from btrfs, which fails to implement the required ioctl(), but it will work between two ext4 filesystems. Closes: tytso#201 Signed-off-by: Allison Karlitskaya <[email protected]>
allisonkarlitskaya
force-pushed
the
verity
branch
from
4d047ec to
08a14a0
Compare
3 tasks
cgwalters
reviewed
Dec 4, 2024
|
|
||
| e2_offset += size; | ||
| *written += size; | ||
| } while (size != 0); |
There was a problem hiding this comment.
I don't understand the loop here. I think we only need retry around the ioctl, since write_all handles the data loop.
We could just use glibc's TEMP_FAILURE_RETRY around the ioctl (any usage of that elsewhere in this code?)
There was a problem hiding this comment.
This loop is indeed about repeating the ioctl(). It's a pread-style interface and we might have to call it more than once to get all of the data. For large files, the merkle tree data, in particular, can easily be larger than our read size.
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.
When creating a filesystem with
mke2fs -O verityand populating content via-d, check if that content is fs-verity enabled, and if it is, copy the fs-verity metadata from the host-native filesystem into the created filesystem.This currently doesn't work for reading from btrfs, which fails to implement the required ioctl(), but it will work between two ext4 filesystems.
Closes #201
This is very much WIP. It has quite some issues to work out. Input greatly appreciated!
EXT4_VERITY_FLflag after the fact. Most of the other flags are set ahead of time. We could check the file flags forFS_VERITY_FLwithFS_IOC_GETFLAGSto help with that, but it's a matter of taste. I sort of prefer the current approachfsverity enableon and looking for differences vs. the filesystem created bymke2fs -d.