Re: [PATCH] generic/175, generic/176: cleanup testdir before exit

[Zorro Lang <zlang@redhat.com>]

On Wed, Jul 22, 2020 at 11:11:31AM +0800, Murphy Zhou wrote:
> On Fri, Jul 17, 2020 at 01:58:53PM +0800, Zorro Lang wrote:
> > On Fri, Jul 17, 2020 at 12:40:17PM +0800, Murphy Zhou wrote:
> > > Usually the _mkfs helper will cleanup these directories at the
> > > beginning of testcase. However, when testing on NFS, the cleanup
> > > could be very slow and it is confusing that: We have already
> > 
> > The _scratch_mkfs logic for NFS is:
> >   [ -n "$SCRATCH_MNT" ] || return 1
> >   _scratch_mount
> >   rm -rf $SCRATCH_MNT/*
> >   _scratch_unmount
> > 
> > If you feel NFS is 'very slow', and isn't normal, you can check how
> > NFS is dealing with the reflink file of g/175.
> > 
> > > started to run generic/176 but we get stuck in _mkfs, cleaning
> > > up files left by the previous testcase generic/175.
> > 
> > Hmm... that makes more sense than above reason, although we generally
> > don't cleanup SCRATCH_MNT/* at the end of a case. Due to if you need to
> > cleanup SCRATCH_MNT/* for NFS, why only these 2 cases need to do that?
> > We might need a better reason/comment to explain, e.g. it cover a issue
> > of NFS?
> > 
> > BTW, the cleanup operation can be in _cleanup() function, likes:
> >   [ -d "$testdir" ] && rm -rf $testdir
> > 
> > Thanks,
> > Zorro
> > 
> > > 
> > > To be clear, cleanup testdir before exit. Also, deleting files
> > > should be part of the stress test.
> 
> These 2 lines are the real reasons for adding this.

Yeah, I see. I mean if for this reason, too lots of cases need to do
'rm -rf $SCRATCH_MNT/*' at the end, due to most of them leave something
in $SCRATCH_MNT/ , and will be cleaned by next _scratch_mkfs*, especially
for un-local filesystem(e.g. NFS, CIFS, glusterfs ...). They all make no
sense.

If only cleanup $SCRATCH_MNT/* in g/175 and g/176, the reason is better to be
'cover a known issue' when cleanup $SCRATCH_MNT in g/175 and g/176. That's
my personal review point :)

Thanks,
Zorro



> 
> Thanks!
> 
> > > 
> > > Signed-off-by: Murphy Zhou <jencce.kernel@gmail.com>
> > > ---
> > >  tests/generic/175 | 1 +
> > >  tests/generic/176 | 1 +
> > >  2 files changed, 2 insertions(+)
> > > 
> > > diff --git a/tests/generic/175 b/tests/generic/175
> > > index 79e5b3d6..bd966a28 100755
> > > --- a/tests/generic/175
> > > +++ b/tests/generic/175
> > > @@ -61,6 +61,7 @@ bytes=$((blks * blksz))
> > >  echo "reflinking $blks blocks, $bytes bytes" >> "$seqres.full"
> > >  _reflink_range "$testdir/file1" 0 "$testdir/file2" 0 $bytes >> "$seqres.full"
> > >  
> > > +rm -rf $testdir
> > >  # success, all done
> > >  status=0
> > >  exit
> > > diff --git a/tests/generic/176 b/tests/generic/176
> > > index a084578a..bc83762e 100755
> > > --- a/tests/generic/176
> > > +++ b/tests/generic/176
> > > @@ -73,6 +73,7 @@ bytes=$((blocks_needed * blksz))
> > >  echo "reflinking $((blocks_needed / 2)) blocks, $((bytes / 2)) bytes" >> "$seqres.full"
> > >  _reflink_range "$testdir/file1" 0 "$testdir/file2" 0 $bytes >> "$seqres.full"
> > >  
> > > +rm -rf $testdir
> > >  # success, all done
> > >  status=0
> > >  exit
> > > -- 
> > > 2.20.1
> > > 
> > 
> 
> -- 
> Murphy
>