| Message ID | 20190412052418.22206-1-anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] fstests: generic/077: fix populate fs use _fill_fs() | expand |
On 2019/4/12 下午1:24, Anand Jain wrote: > Test case generic/077 uses files under /lib or /usr to fill SCRATCH_MNT. > If /usr or /lib is below 256mb then test fails to run, or if these dirs > are too large it takes a long time for the cp to finish. On my machine > it takes 645sec. Wait for a minute, ths fs is only 256M sized, if it takes you over 10 minutes, there must be something else wrong. > > This patch propose to use the common/populate function _fill_fs() to > write files into the target directory instead. However I am not too > sure about the motivation of this test case in the first place, and > why does it wanted to cp /usr or /lib, To my eyes, it's using /usr or /lib just for it's mostly full/contains a lot of files for most systems. > and why fs should become full? Maybe to hit certain ENOSPC corner case for ACL/xattr, just my guess. > Any idea? Thanks. Use populate_fs is definitely the right move. However I have some concern below. [snip] > echo "*** set default ACL" > setfacl -R -dm u:fsgqa:rwx,g::rwx,o::r-x,m::rwx $SCRATCH_MNT/subdir > > -echo "*** populate filesystem, pass #1" | tee -a $seqres.full > -cp -rf $filler $SCRATCH_MNT/subdir >$seqres.full 2>&1 > - > -echo "*** populate filesystem, pass #2" | tee -a $seqres.full > -cp -rf $filler $SCRATCH_MNT/subdir >$seqres.full 2>&1 > +blksz="$(_get_block_size $SCRATCH_MNT/subdir)" > +echo "*** populate filesystem" | tee -a $seqres.full > +echo "*** fill_fs $fs_size $SCRATCH_MNT/subdir $blksz 0" >> $seqres.full > +_fill_fs $fs_size $SCRATCH_MNT/subdir $blksz 0 >> $seqres.full 2>&1 Unlike the original behavior, which do 2 passes and the 2nd pass will overwrite previous files. While after your modification, it's no longer the case. At least we should try to replay the 1st run to mimic the original behavior. Thanks, Qu > > _check_scratch_fs > > diff --git a/tests/generic/077.out b/tests/generic/077.out > index eae7226ab29c..9c143c902a2c 100644 > --- a/tests/generic/077.out > +++ b/tests/generic/077.out > @@ -1,7 +1,6 @@ > QA output created by 077 > *** create filesystem > *** set default ACL > -*** populate filesystem, pass #1 > -*** populate filesystem, pass #2 > +*** populate filesystem > *** all done > *** unmount >
On 12.04.19 г. 10:15 ч., Qu Wenruo wrote: > > > On 2019/4/12 下午1:24, Anand Jain wrote: >> Test case generic/077 uses files under /lib or /usr to fill SCRATCH_MNT. >> If /usr or /lib is below 256mb then test fails to run, or if these dirs >> are too large it takes a long time for the cp to finish. On my machine >> it takes 645sec. > > Wait for a minute, ths fs is only 256M sized, if it takes you over 10 > minutes, there must be something else wrong. I've had the same issue, the problem is that once the fs is full cp will continue happily trying to copy stuff and will just be failing with ENOSPC > >> >> This patch propose to use the common/populate function _fill_fs() to >> write files into the target directory instead. However I am not too >> sure about the motivation of this test case in the first place, and >> why does it wanted to cp /usr or /lib, > > To my eyes, it's using /usr or /lib just for it's mostly full/contains a > lot of files for most systems. > >> and why fs should become full? > > Maybe to hit certain ENOSPC corner case for ACL/xattr, just my guess. nod, I've used that test to test certain patches (latest this test was useful is when testing Anad's xattr/acl patches) > >> Any idea? Thanks. > > Use populate_fs is definitely the right move. > > However I have some concern below. > > [snip] >> echo "*** set default ACL" >> setfacl -R -dm u:fsgqa:rwx,g::rwx,o::r-x,m::rwx $SCRATCH_MNT/subdir >> >> -echo "*** populate filesystem, pass #1" | tee -a $seqres.full >> -cp -rf $filler $SCRATCH_MNT/subdir >$seqres.full 2>&1 >> - >> -echo "*** populate filesystem, pass #2" | tee -a $seqres.full >> -cp -rf $filler $SCRATCH_MNT/subdir >$seqres.full 2>&1 >> +blksz="$(_get_block_size $SCRATCH_MNT/subdir)" >> +echo "*** populate filesystem" | tee -a $seqres.full >> +echo "*** fill_fs $fs_size $SCRATCH_MNT/subdir $blksz 0" >> $seqres.full >> +_fill_fs $fs_size $SCRATCH_MNT/subdir $blksz 0 >> $seqres.full 2>&1 > > Unlike the original behavior, which do 2 passes and the 2nd pass will > overwrite previous files. > > While after your modification, it's no longer the case. > > At least we should try to replay the 1st run to mimic the original behavior. Looking at the test shouldn't the "set default acl" be done after the fs is completely full? > > Thanks, > Qu > >> >> _check_scratch_fs >> >> diff --git a/tests/generic/077.out b/tests/generic/077.out >> index eae7226ab29c..9c143c902a2c 100644 >> --- a/tests/generic/077.out >> +++ b/tests/generic/077.out >> @@ -1,7 +1,6 @@ >> QA output created by 077 >> *** create filesystem >> *** set default ACL >> -*** populate filesystem, pass #1 >> -*** populate filesystem, pass #2 >> +*** populate filesystem >> *** all done >> *** unmount >> >
On 12/4/19 3:15 PM, Qu Wenruo wrote: > > > On 2019/4/12 下午1:24, Anand Jain wrote: >> Test case generic/077 uses files under /lib or /usr to fill SCRATCH_MNT. >> If /usr or /lib is below 256mb then test fails to run, or if these dirs >> are too large it takes a long time for the cp to finish. On my machine >> it takes 645sec. > > Wait for a minute, ths fs is only 256M sized, if it takes you over 10 > minutes, there must be something else wrong. What's happening is cp -r fails (ENOSPC) for each file, so the cp -r runs for a long time. >> >> This patch propose to use the common/populate function _fill_fs() to >> write files into the target directory instead. However I am not too >> sure about the motivation of this test case in the first place, and >> why does it wanted to cp /usr or /lib, > > To my eyes, it's using /usr or /lib just for it's mostly full/contains a > lot of files for most systems. A lot of files. At the same time its not consistent across systems. >> and why fs should become full? > > Maybe to hit certain ENOSPC corner case for ACL/xattr, just my guess. ok. >> Any idea? Thanks. > > Use populate_fs is definitely the right move. > > However I have some concern below. > > [snip] >> echo "*** set default ACL" >> setfacl -R -dm u:fsgqa:rwx,g::rwx,o::r-x,m::rwx $SCRATCH_MNT/subdir >> >> -echo "*** populate filesystem, pass #1" | tee -a $seqres.full >> -cp -rf $filler $SCRATCH_MNT/subdir >$seqres.full 2>&1 >> - >> -echo "*** populate filesystem, pass #2" | tee -a $seqres.full >> -cp -rf $filler $SCRATCH_MNT/subdir >$seqres.full 2>&1 >> +blksz="$(_get_block_size $SCRATCH_MNT/subdir)" >> +echo "*** populate filesystem" | tee -a $seqres.full >> +echo "*** fill_fs $fs_size $SCRATCH_MNT/subdir $blksz 0" >> $seqres.full >> +_fill_fs $fs_size $SCRATCH_MNT/subdir $blksz 0 >> $seqres.full 2>&1 > > Unlike the original behavior, which do 2 passes and the 2nd pass will > overwrite previous files. > > While after your modification, it's no longer the case. > > At least we should try to replay the 1st run to mimic the original behavior. yep. Thanks, Anand > Thanks, > Qu > >> >> _check_scratch_fs >> >> diff --git a/tests/generic/077.out b/tests/generic/077.out >> index eae7226ab29c..9c143c902a2c 100644 >> --- a/tests/generic/077.out >> +++ b/tests/generic/077.out >> @@ -1,7 +1,6 @@ >> QA output created by 077 >> *** create filesystem >> *** set default ACL >> -*** populate filesystem, pass #1 >> -*** populate filesystem, pass #2 >> +*** populate filesystem >> *** all done >> *** unmount >> >
On 12/4/19 3:27 PM, Nikolay Borisov wrote: > > > On 12.04.19 г. 10:15 ч., Qu Wenruo wrote: >> >> >> On 2019/4/12 下午1:24, Anand Jain wrote: >>> Test case generic/077 uses files under /lib or /usr to fill SCRATCH_MNT. >>> If /usr or /lib is below 256mb then test fails to run, or if these dirs >>> are too large it takes a long time for the cp to finish. On my machine >>> it takes 645sec. >> >> Wait for a minute, ths fs is only 256M sized, if it takes you over 10 >> minutes, there must be something else wrong. > > I've had the same issue, the problem is that once the fs is full cp will > continue happily trying to copy stuff and will just be failing with ENOSPC > >> >>> >>> This patch propose to use the common/populate function _fill_fs() to >>> write files into the target directory instead. However I am not too >>> sure about the motivation of this test case in the first place, and >>> why does it wanted to cp /usr or /lib, >> >> To my eyes, it's using /usr or /lib just for it's mostly full/contains a >> lot of files for most systems. >> >>> and why fs should become full? >> >> Maybe to hit certain ENOSPC corner case for ACL/xattr, just my guess. > > nod, I've used that test to test certain patches (latest this test was > useful is when testing Anad's xattr/acl patches) > >> >>> Any idea? Thanks. >> >> Use populate_fs is definitely the right move. >> >> However I have some concern below. >> >> [snip] >>> echo "*** set default ACL" >>> setfacl -R -dm u:fsgqa:rwx,g::rwx,o::r-x,m::rwx $SCRATCH_MNT/subdir >>> >>> -echo "*** populate filesystem, pass #1" | tee -a $seqres.full >>> -cp -rf $filler $SCRATCH_MNT/subdir >$seqres.full 2>&1 >>> - >>> -echo "*** populate filesystem, pass #2" | tee -a $seqres.full >>> -cp -rf $filler $SCRATCH_MNT/subdir >$seqres.full 2>&1 >>> +blksz="$(_get_block_size $SCRATCH_MNT/subdir)" >>> +echo "*** populate filesystem" | tee -a $seqres.full >>> +echo "*** fill_fs $fs_size $SCRATCH_MNT/subdir $blksz 0" >> $seqres.full >>> +_fill_fs $fs_size $SCRATCH_MNT/subdir $blksz 0 >> $seqres.full 2>&1 >> >> Unlike the original behavior, which do 2 passes and the 2nd pass will >> overwrite previous files. >> >> While after your modification, it's no longer the case. >> >> At least we should try to replay the 1st run to mimic the original behavior. > > Looking at the test shouldn't the "set default acl" be done after the fs > is completely full? Even by using a large /usr, the cp (2 iterations) was never been able to make the metadata full, (same with __full_fs() as well). After cp -r /usr <256mb fs> reported ENOSPC. stat -f -c "%f" /btrfs 16488 Thanks, Anand >> >> Thanks, >> Qu >> >>> >>> _check_scratch_fs >>> >>> diff --git a/tests/generic/077.out b/tests/generic/077.out >>> index eae7226ab29c..9c143c902a2c 100644 >>> --- a/tests/generic/077.out >>> +++ b/tests/generic/077.out >>> @@ -1,7 +1,6 @@ >>> QA output created by 077 >>> *** create filesystem >>> *** set default ACL >>> -*** populate filesystem, pass #1 >>> -*** populate filesystem, pass #2 >>> +*** populate filesystem >>> *** all done >>> *** unmount >>> >>
diff --git a/tests/generic/077 b/tests/generic/077 index d11b49c6ff15..d8e5551f1925 100755 --- a/tests/generic/077 +++ b/tests/generic/077 @@ -14,18 +14,6 @@ here=`pwd` tmp=/tmp/$$ status=1 -# Something w/ enough data to fill 256M of fs... -filler="" -[ -d /lib/modules ] && \ - [ $(( $(du -h -m /lib/modules | tail -1| cut -f1) * 2 )) -ge 256 ] && \ - filler=/lib/modules - -# fall back in case /lib/modules doesn't exist or smaller -[[ -z $filler ]] && \ - [ -d /usr ] && \ - [ $(( $(du -h -m /usr | tail -1| cut -f1) * 2 )) -ge 256 ] && \ - filler=/usr - _cleanup() { cd / @@ -38,13 +26,12 @@ trap "_cleanup; rm -f $tmp.*; exit \$status" 0 1 2 3 15 . ./common/rc . ./common/filter . ./common/attr +. ./common/populate # real QA test starts here _supported_fs generic _supported_os Linux -[ ! -d $filler ] && _notrun "No directory at least 256MB to source files from" - _require_scratch _require_attrs _require_acls @@ -64,11 +51,10 @@ mkdir $SCRATCH_MNT/subdir echo "*** set default ACL" setfacl -R -dm u:fsgqa:rwx,g::rwx,o::r-x,m::rwx $SCRATCH_MNT/subdir -echo "*** populate filesystem, pass #1" | tee -a $seqres.full -cp -rf $filler $SCRATCH_MNT/subdir >$seqres.full 2>&1 - -echo "*** populate filesystem, pass #2" | tee -a $seqres.full -cp -rf $filler $SCRATCH_MNT/subdir >$seqres.full 2>&1 +blksz="$(_get_block_size $SCRATCH_MNT/subdir)" +echo "*** populate filesystem" | tee -a $seqres.full +echo "*** fill_fs $fs_size $SCRATCH_MNT/subdir $blksz 0" >> $seqres.full +_fill_fs $fs_size $SCRATCH_MNT/subdir $blksz 0 >> $seqres.full 2>&1 _check_scratch_fs diff --git a/tests/generic/077.out b/tests/generic/077.out index eae7226ab29c..9c143c902a2c 100644 --- a/tests/generic/077.out +++ b/tests/generic/077.out @@ -1,7 +1,6 @@ QA output created by 077 *** create filesystem *** set default ACL -*** populate filesystem, pass #1 -*** populate filesystem, pass #2 +*** populate filesystem *** all done *** unmount
Test case generic/077 uses files under /lib or /usr to fill SCRATCH_MNT. If /usr or /lib is below 256mb then test fails to run, or if these dirs are too large it takes a long time for the cp to finish. On my machine it takes 645sec. This patch propose to use the common/populate function _fill_fs() to write files into the target directory instead. However I am not too sure about the motivation of this test case in the first place, and why does it wanted to cp /usr or /lib, and why fs should become full? Any idea? Thanks. Signed-off-by: Anand Jain <anand.jain@oracle.com> --- tests/generic/077 | 24 +++++------------------- tests/generic/077.out | 3 +-- 2 files changed, 6 insertions(+), 21 deletions(-)