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:

quoted

On Sun, May 23, 2021 at 06:51:49PM -0400, Kent Overstreet wrote:
[snip]

quoted

quoted

quoted

 	;;

@@ -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