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