Re: [PATCH v2 3/4] generic/{455,457,482}: make dmlogwrites tests work on bcachefs

[Kent Overstreet <kent.overstreet@gmail.com>]

On Sun, May 30, 2021 at 09:18:49PM +0800, Eryu Guan wrote:
> On Tue, May 25, 2021 at 06:19:54PM -0400, Kent Overstreet wrote:
> > bcachefs has log structured btree nodes, in addition to a regular
> > journal, which means that unless we replay to markers in the log in the
> > same order that they happened and are careful to avoid writing in
> > between replaying to different events - we need to wipe and start fresh
> > each time.
> > 
> > Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com>
> > ---
> >  tests/generic/455 | 14 ++++++++++++++
> >  tests/generic/457 | 14 ++++++++++++++
> >  tests/generic/482 | 27 ++++++++++++++++++++-------
> >  3 files changed, 48 insertions(+), 7 deletions(-)
> > 
> > diff --git a/tests/generic/455 b/tests/generic/455
> > index 5b4b242e74..6dc46c3c72 100755
> > --- a/tests/generic/455
> > +++ b/tests/generic/455
> > @@ -35,6 +35,17 @@ _require_dm_target thin-pool
> >  
> >  rm -f $seqres.full
> >  
> > +_reset_dmthin()
> > +{
> > +    # With bcachefs, we need to wipe and start fresh every time we replay to a
> > +    # different point in time - if we see metadata from a future point in time,
> > +    # or an unrelated mount, bcachefs will get confused:
> > +    if [ "$FSTYP" = "bcachefs" ]; then
> > +	_dmthin_cleanup
> > +	_dmthin_init $devsize $devsize $csize $lowspace
> > +    fi
> > +}
> 
> I think we probably could make it a common helper, and currently only
> bcachefs needs reset, and more log structured filesystems may be
> supported in the future.

I think it might be better to wait until we have more dmlogwrites tests or
another filesystem that needs this - I don't think this would be a good common
helper as is, it's too coupled to what the tests are doing - factoring out
helpers just because you spot identical code is an anti pattern when there isn't
a good notion of what you're abstracting

Right now 455 and 457 are basically identical anyways, factoring out a single
helper and ignoring the rest doesn't make much sense to me.