Skip to content

!WIP! mkfs: copy verity metadata from the --rootdir #971

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: devel
Choose a base branch
from

Conversation

allisonkarlitskaya
Copy link
Contributor

@allisonkarlitskaya allisonkarlitskaya commented Mar 17, 2025

Add a new --fs-verity option. If it's specified, then check if the regular files in the --rootdir have fs-verity enabled, and if they do, copy the metadata into the created filesystem.

We could have done this implicitly on creation of all filesystems, but I believe the new --fs-verity option is justified (over simply always checking) for a number of reasons:

  • (most importantly) if querying the fs-verity metadata on a file fails, we want to hear about it, loudly. Querying fs-verity metadata on btrfs itself is only supported on kernel 6.14 and later, and on ext4 fs-verity isn't enabled by default. We want to catch those cases and treat them as errors instead of silently proceeding. If we were to always query fs-verity metadata then we'd have to ignore the unsupported cases.
  • tools and scripts invoking mkfs.btrfs and expecting the created filesystem to actually have fs-verity enabled on the relevant files need to be sure of this. We want to fail if the new --fs-verity option isn't present.
  • most people are probably not interested in fs-verity data, and querying every single file (when no file has fs-verity enabled) would slow down filesystem creation
  • finally, doing this without an option would be a change in existing behaviour

Issue: #929

So far this works, but not for zero-size or inline files.

I'd love some feedback here on the implementation. In particular, the way this fits in with the extents copying code is really awkward. I'd like to share the fd that we open() but we don't open the file if it's empty. I'd also maybe like to share the buffer.

My first thought would be to declare a separate buffer as a global and use it from both data copying and fs-verity copying. This code is not multithreaded and already has a bunch of global variables, so it's not thread-safe anyway. This would save the continuous malloc()/free(). I'd also be happy to use a stack for this sort of thing: it's a large allocation (1MB) but it's in non-recursive application code, so it ought to be OK.

There's also a question if the fs-verity copy should happen inside of the extent-copying function or not. That's where the fd is opened and I'd like to avoid opening the same file twice. I think I might like to create a function which opens the fd and then calls the extent copying code and then the fs-verity copying code separately. It's all a matter of taste and I'd like some input there.

asj and others added 2 commits March 15, 2025 00:30
The disk max size cannot be 256MiB because Btrfs does not allow creating
a filesystem on disks smaller than 256MiB.

Remove the incorrect warning for the 'max' option.`

$ btrfs fi resize max /btrfs
Resize device id 1 (/dev/sda) from 3.00GiB to max
WARNING: the new size 0 (0.00B) is < 256MiB, this may be rejected by kernel

Fixes: 158a25a ("btrfs-progs: fi resize: warn if new size is < 256M")
Signed-off-by: Anand Jain <[email protected]>
Signed-off-by: David Sterba <[email protected]>
Add a new --fs-verity option.  If it's specified, then check if the regular
files in the --rootdir have fs-verity enabled, and if they do, copy the
metadata into the created filesystem.

We could have done this implicitly on creation of all filesystems, but I
believe the new --fs-verity option is justified (over simply always
checking) for a number of reasons:
 - (most importantly) if querying the fs-verity metadata on a file
   fails, we want to hear about it, loudly.  Querying fs-verity metadata
   on btrfs itself is only supported on kernel 6.14 and later, and on
   ext4 fs-verity isn't enabled by default.  We want to catch those
   cases and treat them as errors instead of silently proceeding.  If we
   were to always query fs-verity metadata then we'd have to ignore the
   unsupported cases.
 - tools and scripts invoking mkfs.btrfs and expecting the created
   filesystem to actually have fs-verity enabled on the relevant files
   need to be sure of this.  We want to fail if the new --fs-verity
   option isn't present.
 - most people are probably not interested in fs-verity data, and
   querying every single file (when no file has fs-verity enabled) would
   slow down filesystem creation
 - finally, doing this without an option would be a change in existing
   behaviour

Issue: kdave#929
Signed-off-by: Allison Karlitskaya <[email protected]>
@kdave kdave force-pushed the devel branch 3 times, most recently from c9ae825 to 5ad147c Compare March 26, 2025 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants