On Mon, May 24, 2021 at 12:04:32AM -0400, Kent Overstreet wrote: > On Mon, May 24, 2021 at 11:56:04AM +0800, Eryu Guan wrote: > > On Sun, May 23, 2021 at 06:51:49PM -0400, Kent Overstreet wrote: > > [snip] > > > > > > > ;; > > > > > @@ -1179,6 +1197,19 @@ _repair_scratch_fs() > > > > > fi > > > > > return $res > > > > > ;; > > > > > + bcachefs) > > > > > + fsck -t $FSTYP -n $SCRATCH_DEV 2>&1 > > > > > > > > _repair_scratch_fs() is supposed to actually fix the errors, does > > > > "fsck -n" fix errors for bcachefs? > > > > > > No - but with bcachefs fsck finding errors _always_ indicates a bug, so for the > > > purposes of these tests I think this is the right thing to do - I don't want the > > > tests to pass if fsck is finding and fixing errors. > > > > Then _check_scratch_fs() should be used instead, which will fail the > > test if any fsck finds any corruptions. _repair_scratch_fs() is meant to > > fix errors, and only report failure when there's unfixable errors. > > I see no reason to make such a change to generic tests that were written for > other filesystems, when this gets me exactly what I want. Oh, I noticed that "with bcachefs fsck finding errors _always_ indicates a bug" now.. Then I think using fsck -n is fine here, but better with comments to describe why this is ok for bcachefs. Also, we could just use _check_scratch_fs here instead of the open-coded fsck command. As _check_scratch_fs calls _check_generic_filesystem() which calls fsck -n internally, and handles mount/umount device correctly and prints pretty log if there's any errors. Thanks, Eryu
