Re: [PATCH 1/3] Initial bcachefs support

Eryu Guan
2021-05-24T04:22:56Z

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