On Wed, Jan 23, 2019 at 09:22:16AM +1100, Dave Chinner wrote: > On Tue, Jan 22, 2019 at 12:58:54PM +0800, Zorro Lang wrote: > > On Tue, Jan 22, 2019 at 05:33:19AM +0200, Amir Goldstein wrote: > > > On Tue, Jan 22, 2019 at 12:44 AM Dave Chinner <david@xxxxxxxxxxxxx> wrote: > > > > > > > > On Sat, Jan 19, 2019 at 10:42:58AM +0800, Zorro Lang wrote: > > > > > After fsstress has new operation, xfs/068 has different dump output, > > > > > so change the expected number of files and directories. > > > > > > > > > > Signed-off-by: Zorro Lang <zlang@xxxxxxxxxx> > > > > > --- > > > > > tests/xfs/068.out | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/tests/xfs/068.out b/tests/xfs/068.out > > > > > index fa3a5523..f9ec37b8 100644 > > > > > --- a/tests/xfs/068.out > > > > > +++ b/tests/xfs/068.out > > > > > @@ -22,7 +22,7 @@ xfsrestore: session id: ID > > > > > xfsrestore: media ID: ID > > > > > xfsrestore: searching media for directory dump > > > > > xfsrestore: reading directories > > > > > -xfsrestore: 383 directories and 1335 entries processed > > > > > +xfsrestore: 416 directories and 1373 entries processed > > > > > > > > Shouldn't you tell fsstress to avoid this new operation so none of > > > > the tests that rely on the number of files created by a specific > > > > fstress seed break? > > > > > > > > i.e. changing FSSTRESS_AVOID in common/dump as is done already to > > > > turn off dedupe/clone/copy_file_range for xfsdump/restore tests? > > > > > > > > > > IIRC, dedupe/clone/copy_file_range were blacklisted not for not > > > changing golden output, but because they make the golden output > > > dependent on xfs reflink feature (i.e. on mkfs options). > > > > > > This is not the case with the newly added SPLICE operation and > > > updating the golden output rather than blacklisting the new operation > > > is what was decided and done two times in recent history (also patches > > > by Zorro). > > > > Yes, I saw that FSSTRESS_AVOID in common/dump. But I thought (if I'm wrong > > please feel free to correct me): > > 1) dedupe/clone/copyrange might not be compiled in fsstress, they depend > > on "#ifndef ... #endif". That will make different xfsdump/restore output > > when a fs with or without these operations. Then there's not a fixed 068.out. > > So they must be avoided. > > > 2) splice syscall is an old common function, which is first appeared in Linux > > 2.6.17. So I think it's safe to be added. > > What makes you think it worked in 2.6.17? I mean: > > commit 0ff28d9f4674d781e492bcff6f32f0fe48cf0fed > Author: Christophe Leroy <christophe.leroy@xxxxxx> > Date: Wed May 6 17:26:47 2015 +0200 > > splice: sendfile() at once fails for big files > > Using sendfile with below small program to get MD5 sums of some files, > it appear that big files (over 64kbytes with 4k pages system) get a > wrong MD5 sum while small files get the correct sum. > This program uses sendfile() to send a file to an AF_ALG socket > for hashing. > ..... > > i.e. using sendfile to copy large files resulted in corrupt copies > up until 2015. > > Basically, sendfile is a legacy interface that has spend a very > large amount of it's time broken, full of deadlocks (i.e. unusable) > and not used by anyone. It's not something we really want to enable > in xfstests because it will cause all sorts of failures on older > distros... Sorry I didn't learn about that. I just checked "man 2 splice", it said: "The splice() system call first appeared in Linux 2.6.17; library support was added to glibc in version 2.5." > > > 3) Either change common/dump or xfs/068.out doesn't take much time/code, so I > > decided to change 068.out to make x/068 to do operations. > > Did you validate that the file count is actually correct? > > The file count in these dump/restore tests is how we tell we've > broken bulkstat iteration - any mismatch on the file count indicates > bulkstat didn't find something it should have or it counted things > twice. Hence I'm wary making changes to the file set size the > dump/restore tests use. > > And, really, changing the file set size somewhat invalidates the > long regression history we have with these existing tests. i.e. > don't change tests that work correctly unless it's absolutely > necessary. It only adds to the risk of introducing new bugs. Thanks so much for your detailed description :) I'll turn to change FSSTRESS_AVOID in next V4 patch. Thanks, Zorro > > Cheers, > > Dave. > -- > Dave Chinner > david@xxxxxxxxxxxxx
