Thu, Feb 04, 2021 at 10:04:07PM -0800, Eric Biggers wrote: > Thanks for writing a test for this! > > On Thu, Feb 04, 2021 at 03:24:26PM -0800, Boris Burkov wrote: > > There are some btrfs specific fsverity scenarios that don't map > > neatly onto the tests in generic/574, like holes, inline extents, > > and preallocated extents. Cover those in a btrfs specific test. > > > > That test relies on assumptions about how the Merkle tree is stored > > by ext4/f2fs which don't apply to btrfs, so we also test Merkle tree > > corruption here. This could be merged by some generic abstraction. > > The only part of generic/574 that cares where the Merkle tree is stored is > _fsv_scratch_corrupt_merkle_tree(). Couldn't that be updated to handle btrfs? >
I agree that would be an easy enough fix, I'll make it in this patch. Until I get 574 fully passing, I think I ought to leave the Merkle corruption here as well, though, right? > > Finally, that test relies extensively on fiemap, which is currently > > broken on btrfs for offsets and sizes that don't align to PAGE_SIZE, > > so put a simple regular file case in this test for now, while we fix > > fiemap or generalize extent lookup. > > fiemap is only used by _fsv_scratch_corrupt_bytes(). It just wants to know > the > list of extents that intersect the requested byte range. Does that really not > work on btrfs if the range isn't page-aligned? > Unfortunately, fiemap is in fact broken in btrfs in that case, and prints silly stuff like [K..K-1]. I wrote up a fix for it, but am still figuring out how to test it, and decided to get the verity stuff out ahead of it. However, even if I did get that fix in, it would still not be quite right. Btrfs fiemap is in terms of logical block addresses, so an additional translation with btrfs-map-logical is needed, and I couldn't figure out how to elegantly inject that into _fsv_scratch_corrupt_bytes. I do hope to get btrfs to pass generic/574 soon, though. > - Eric Thanks for the review, Boris
