On Sun, Feb 3, 2019 at 2:40 PM Eryu Guan <guaneryu@xxxxxxxxx> wrote:
>
> On Sun, Jan 27, 2019 at 09:50:57AM +0200, Amir Goldstein wrote:
> > This test has the number of files/dirs created by xfsrestore hardcoded
> > in golden output.
> >
> > When fsstress is added new ops, the number of files/dirs created with
> > the same random seed changes and this regularly breaks this test,
> > so when new fsstress ops are added they should be either added to the
> > dump test blacklist or golden output of this test needs to be ammended
> > to reflect the change.
> >
> > The golden output includes only the file count reported by xfsrestore
> > and test does not even verify that this is the correct file count.
> > Instead, leave the golden output nuetral and explicitly verify that
> > file count before and after the test are the same.
> >
> > With this change, the test becomes agnostic to fsstress ops and we
> > could also stop blacklisting clone/dedup/copy ops if we want.
> >
> > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
>
> Looks fine to me overall, thanks a lot for the fix! But I'd like an ACK
> from XFS folks :)
>
> > ---
> > common/dump | 7 +++++++
> > tests/xfs/068 | 14 +++++++++++++-
> > tests/xfs/068.out | 2 +-
> > 3 files changed, 21 insertions(+), 2 deletions(-)
> >
> > diff --git a/common/dump b/common/dump
> > index 89fa0391..f112fc37 100644
> > --- a/common/dump
> > +++ b/common/dump
> > @@ -1514,6 +1514,13 @@ _check_quota_file()
> > _check_quota 'xfsdump_quotas' 'xfsdump_quotas_group' 'xfsdump_quotas_proj'
> > }
> >
> > +_count_dumpdir_files()
> > +{
> > + local ndirs=$(find $dump_dir -type d | wc -l)
> > + local nents=$(find $dump_dir | wc -l)
> > +
> > + echo "$ndirs directories and $nents entries"
> > +}
> >
> > # make sure this script returns success
> > /bin/true
> > diff --git a/tests/xfs/068 b/tests/xfs/068
> > index 7f5900fc..264a9e96 100755
> > --- a/tests/xfs/068
> > +++ b/tests/xfs/068
> > @@ -30,12 +30,24 @@ _cleanup()
> > . ./common/rc
> > . ./common/dump
> >
> > +# remove previous $seqres.full before test
> > +rm -f $seqres.full
> > +
> > # real QA test starts here
> > _supported_fs xfs
> > _supported_os Linux
> >
> > _create_dumpdir_stress_num 4096
> > -_do_dump_restore
> > +
> > +echo -n "Before: " >> $seqres.full
> > +_count_dumpdir_files | tee $tmp.before >> $seqres.full
> > +
> > +# filter out the file count, it changes as fsstress adds new operations
> > +_do_dump_restore | sed -e "/entries processed$/s/[0-9][0-9]*/NUM/g"
> > +
> > +echo -n "After: " >> $seqres.full
> > +_count_dumpdir_files | tee $tmp.after >> $seqres.full
>
> The "Before" and "After" both count the file entries in $dump_dir, so
> they should always be the same, we should count $restore_dir at "After"?
>
Certainly. We should count $restore_dir/$dump_sdir.
I there is an ACK to concept, I will send a fix.
Thanks,
Amir.
