From e402f694260f364d90e096a0df45bdb8772f7256 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Wed, 16 Jun 2021 18:18:27 +1000 Subject: [PATCH] tests: functional: switch to rsync for directory diffs While "diff -r" is the most straightforward way of comparing directory trees for differences, it has two major issues: * File metadata is not compared, which means that subtle bugs may be missed even if a test is written that exercises the buggy behaviour. * diff(1) doesn't know how to compare special files -- it assumes they are always different, which means that a test using diff(1) on special files will always fail (resulting in such tests not being added). rsync can be used in a very similar manner to diff (with the -ni flags), but has the additional benefit of being able to detect and resolve many more differences between directory trees. In addition, rsync has a standard set of features and flags while diffs feature set depends on whether you're using GNU or BSD binutils. Note that for several of the test cases we expect that file timestamps will not match. For example, the ctime for a file creation or modify event is stored in the intent log but not the mtime. Thus when replaying the log the correct ctime is set but the current mtime is used. This is the expected behavior, so to prevent these tests from failing, there's a replay_directory_diff function which ignores those kinds of changes. This fix was suggested by Brian Behlendorf. Signed-off-by: Aleksa Sarai --- .github/workflows/build-dependencies.txt | 1 + tests/zfs-tests/include/commands.cfg | 1 + tests/zfs-tests/include/libtest.shlib | 73 +++++++++++++++++++ .../zfs_receive/zfs_receive_010_pos.ksh | 4 +- .../cli_root/zfs_send/zfs_send_007_pos.ksh | 2 +- .../zfs_snapshot/zfs_snapshot_009_pos.ksh | 2 +- .../zpool_import/zpool_import_errata4.ksh | 2 +- .../large_dnode/large_dnode_005_pos.ksh | 2 +- .../redacted_send/redacted_incrementals.ksh | 14 ++-- .../redacted_send/redacted_largeblocks.ksh | 2 +- .../tests/functional/rsend/recv_dedup.ksh | 3 +- .../tests/functional/rsend/rsend.kshlib | 6 +- .../functional/slog/slog_replay_fs_001.ksh | 6 +- .../functional/slog/slog_replay_fs_002.ksh | 6 +- .../functional/snapshot/snapshot_002_pos.ksh | 2 +- .../functional/snapshot/snapshot_006_pos.ksh | 2 +- 16 files changed, 102 insertions(+), 26 deletions(-) diff --git a/.github/workflows/build-dependencies.txt b/.github/workflows/build-dependencies.txt index 1dc745d5f642..e591399001f5 100644 --- a/.github/workflows/build-dependencies.txt +++ b/.github/workflows/build-dependencies.txt @@ -38,6 +38,7 @@ python3-dev python3-packaging python3-setuptools rng-tools +rsync samba sysstat uuid-dev diff --git a/tests/zfs-tests/include/commands.cfg b/tests/zfs-tests/include/commands.cfg index 51052e801e87..fa3d12d669f0 100644 --- a/tests/zfs-tests/include/commands.cfg +++ b/tests/zfs-tests/include/commands.cfg @@ -77,6 +77,7 @@ export SYSTEM_FILES_COMMON='arp readlink rm rmdir + rsync scp script sed diff --git a/tests/zfs-tests/include/libtest.shlib b/tests/zfs-tests/include/libtest.shlib index b229a161518b..d3f30f11a358 100644 --- a/tests/zfs-tests/include/libtest.shlib +++ b/tests/zfs-tests/include/libtest.shlib @@ -4303,3 +4303,76 @@ function wait_for_children #children done return $rv } + +# +# Compare two directory trees recursively in a manner similar to diff(1), but +# using rsync. If there are any discrepancies, a summary of the differences are +# output and a non-zero error is returned. +# +# If you're comparing a directory after a ZIL replay, you should set +# LIBTEST_DIFF_ZIL_REPLAY=1 or use replay_directory_diff which will cause +# directory_diff to ignore mtime changes (the ZIL replay won't fix up mtime +# information). +# +function directory_diff # dir_a dir_b +{ + dir_a="$1" + dir_b="$2" + zil_replay="${LIBTEST_DIFF_ZIL_REPLAY:-0}" + + # Run rsync with --dry-run --itemize-changes to get something akin to diff + # output, but rsync is far more thorough in detecting differences (diff + # doesn't compare file metadata, and cannot handle special files). + args=("-nic" "-aAHX" "--devices" "--delete") + + # NOTE: Quite a few rsync builds do not support --crtimes which would be + # necessary to verify that creation times are being maintained properly. So + # Unfortunately because of this we cannot use it unconditionally but we can + # check if this rsync build supports it and use it then. This check is + # based on the same check in the rsync test suite (testsuite/crtimes.test). + # + # We check ctimes even with zil_replay=1 because the ZIL does store + # creation times and we should make sure they match (if the creation times + # do not match there is a "c" entry in one of the columns). + if ( rsync --version | grep -q "[, ] crtimes" >/dev/null ); then + args+=("--crtimes") + else + echo "NOTE: This rsync package does not support --crtimes (-N)." + fi + + # If we are testing a ZIL replay, we need to ignore timestamp changes. + # Unfortunately --no-times doesn't do what we want -- it will still tell if + # you the timestamps don't match but rsync will set the timestamps to the + # current time (leading to an itemised change entry). It's simpler to just + # filter out those lines. + if [ "$zil_replay" -eq 0 ]; then + filter=("cat") + else + # Different rsync versions have different numbers of columns. So just + # require that aside from the first two, all other columns must be + # blank (literal ".") or a timestamp field ("[tT]"). + filter=("grep" "-v" '^\..[.Tt]\+ ') + fi + + diff="$(rsync "${args[@]}" "$dir_a/" "$dir_b/" | "${filter[@]}")" + rv=0 + if [ -n "$diff" ]; then + echo "$diff" + rv=1 + fi + return $rv +} + +# +# Compare two directory trees recursively, without checking whether the mtimes +# match (creation times will be checked if the available rsync binary supports +# it). This is necessary for ZIL replay checks (because the ZIL does not +# contain mtimes and thus after a ZIL replay, mtimes won't match). +# +# This is shorthand for LIBTEST_DIFF_ZIL_REPLAY=1 directory_diff <...>. +# +function replay_directory_diff # dir_a dir_b +{ + LIBTEST_DIFF_ZIL_REPLAY=1 directory_diff "$@" + return $? +} diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_receive/zfs_receive_010_pos.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_receive/zfs_receive_010_pos.ksh index 84485977aa23..47e23e6ebca7 100755 --- a/tests/zfs-tests/tests/functional/cli_root/zfs_receive/zfs_receive_010_pos.ksh +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_receive/zfs_receive_010_pos.ksh @@ -166,12 +166,12 @@ log_must zfs destroy -fr $rfs cat $TESTDIR/zr010p | log_must zfs receive -o origin=$fs2@s1 $rfs mntpnt_old=$(get_prop mountpoint $fs) mntpnt_new=$(get_prop mountpoint $rfs) -log_must diff -r $mntpnt_old $mntpnt_new +log_must directory_diff $mntpnt_old $mntpnt_new log_must zfs destroy -r $rfs cat $TESTDIR/zr010p2 | log_must zfs receive -o origin=$fs@s1 $rfs mntpnt_old=$(get_prop mountpoint $fs2) mntpnt_new=$(get_prop mountpoint $rfs) -log_must diff -r $mntpnt_old $mntpnt_new +log_must directory_diff $mntpnt_old $mntpnt_new log_pass "zfs receive of full send as clone works" diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_send/zfs_send_007_pos.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_send/zfs_send_007_pos.ksh index 675afa72f5af..306fabc8cef4 100755 --- a/tests/zfs-tests/tests/functional/cli_root/zfs_send/zfs_send_007_pos.ksh +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_send/zfs_send_007_pos.ksh @@ -83,7 +83,7 @@ test_pool () cat $streamfile | log_must zfs receive $POOL/recvfs recv_mntpnt=$(get_prop mountpoint "$POOL/recvfs") - log_must diff -r $mntpnt $recv_mntpnt + log_must directory_diff $mntpnt $recv_mntpnt log_must zfs destroy -rf $POOL/fs log_must zfs destroy -rf $POOL/recvfs } diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_snapshot/zfs_snapshot_009_pos.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_snapshot/zfs_snapshot_009_pos.ksh index 6fedba9e5b27..a29cc92509ee 100755 --- a/tests/zfs-tests/tests/functional/cli_root/zfs_snapshot/zfs_snapshot_009_pos.ksh +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_snapshot/zfs_snapshot_009_pos.ksh @@ -93,7 +93,7 @@ for i in 1 2 3; do done log_note "verify snapshot contents" for ds in $datasets; do - diff -q -r /$ds /$ds/.zfs/snapshot/snap > /dev/null 2>&1 + directory_diff /$ds /$ds/.zfs/snapshot/snap if [[ $? -eq 1 ]]; then log_fail "snapshot contents are different from" \ "the filesystem" diff --git a/tests/zfs-tests/tests/functional/cli_root/zpool_import/zpool_import_errata4.ksh b/tests/zfs-tests/tests/functional/cli_root/zpool_import/zpool_import_errata4.ksh index a0f063a8dc83..a199c2a7fb9d 100755 --- a/tests/zfs-tests/tests/functional/cli_root/zpool_import/zpool_import_errata4.ksh +++ b/tests/zfs-tests/tests/functional/cli_root/zpool_import/zpool_import_errata4.ksh @@ -122,7 +122,7 @@ block_device_wait old_mntpnt=$(get_prop mountpoint $POOL_NAME/testfs) new_mntpnt=$(get_prop mountpoint $POOL_NAME/fixed/testfs) -log_must diff -r "$old_mntpnt" "$new_mntpnt" +log_must directory_diff "$old_mntpnt" "$new_mntpnt" log_must diff /dev/zvol/$POOL_NAME/testvol /dev/zvol/$POOL_NAME/fixed/testvol log_must has_ivset_guid $POOL_NAME/fixed/testfs@snap1 diff --git a/tests/zfs-tests/tests/functional/features/large_dnode/large_dnode_005_pos.ksh b/tests/zfs-tests/tests/functional/features/large_dnode/large_dnode_005_pos.ksh index 2be98942634f..03e2db4b8082 100755 --- a/tests/zfs-tests/tests/functional/features/large_dnode/large_dnode_005_pos.ksh +++ b/tests/zfs-tests/tests/functional/features/large_dnode/large_dnode_005_pos.ksh @@ -68,7 +68,7 @@ if [[ "$dnsize" != "1K" ]]; then fi log_must eval "zfs recv -F $TEST_RECV_FS < $TEST_STREAMINCR" -log_must diff -r /$TEST_SEND_FS /$TEST_RECV_FS +log_must directory_diff /$TEST_SEND_FS /$TEST_RECV_FS log_must zfs umount $TEST_SEND_FS log_must zfs umount $TEST_RECV_FS diff --git a/tests/zfs-tests/tests/functional/redacted_send/redacted_incrementals.ksh b/tests/zfs-tests/tests/functional/redacted_send/redacted_incrementals.ksh index 1d2ed3a687be..18ea5d68fcf8 100755 --- a/tests/zfs-tests/tests/functional/redacted_send/redacted_incrementals.ksh +++ b/tests/zfs-tests/tests/functional/redacted_send/redacted_incrementals.ksh @@ -51,10 +51,10 @@ log_must eval "zfs receive $POOL2/rfs <$stream" # Verify receipt of normal incrementals to redaction list members. log_must eval "zfs send -i $sendfs@snap0 $POOL/stride3@snap >$stream" log_must eval "zfs recv $POOL2/rstride3 <$stream" -log_must diff -r /$POOL/stride3 /$POOL2/rstride3 +log_must directory_diff /$POOL/stride3 /$POOL2/rstride3 log_must eval "zfs send -i $sendfs@snap0 $POOL/stride5@snap >$stream" log_must eval "zfs recv $POOL2/rstride5 <$stream" -log_must diff -r /$POOL/stride5 /$POOL2/rstride5 +log_must directory_diff /$POOL/stride5 /$POOL2/rstride5 # But not a normal child that we weren't redacted with respect to. log_must eval "zfs send -i $sendfs@snap0 $POOL/hole@snap >$stream" @@ -73,7 +73,7 @@ log_must mount_redacted -f $POOL2/rint # Verify we can receive grandchildren on the child. log_must eval "zfs send -i $POOL/int@snap $POOL/rm@snap >$stream" log_must eval "zfs receive $POOL2/rrm <$stream" -log_must diff -r /$POOL/rm /$POOL2/rrm +log_must directory_diff /$POOL/rm /$POOL2/rrm # But not a grandchild that the received child wasn't redacted with respect to. log_must eval "zfs send -i $POOL/int@snap $POOL/write@snap >$stream" @@ -92,13 +92,13 @@ log_mustnot zfs redact $POOL/int@snap book6 $POOL/hole@snap # Verify we can receive a full clone of the grandchild on the child. log_must eval "zfs send $POOL/write@snap >$stream" log_must eval "zfs recv -o origin=$POOL2/rint@snap $POOL2/rwrite <$stream" -log_must diff -r /$POOL/write /$POOL2/rwrite +log_must directory_diff /$POOL/write /$POOL2/rwrite # Along with other origins. log_must eval "zfs recv -o origin=$POOL2/rfs@snap0 $POOL2/rwrite1 <$stream" -log_must diff -r /$POOL/write /$POOL2/rwrite1 +log_must directory_diff /$POOL/write /$POOL2/rwrite1 log_must eval "zfs recv -o origin=$POOL2@init $POOL2/rwrite2 <$stream" -log_must diff -r /$POOL/write /$POOL2/rwrite2 +log_must directory_diff /$POOL/write /$POOL2/rwrite2 log_must zfs destroy -R $POOL2/rwrite2 log_must zfs destroy -R $POOL2/rfs @@ -140,7 +140,7 @@ unmount_redacted $POOL2/rfs # sending from the bookmark. log_must eval "zfs send -i $sendfs#book7 $POOL/hole1@snap >$stream" log_must eval "zfs recv $POOL2/rhole1 <$stream" -log_must diff -r /$POOL/hole1 /$POOL2/rhole1 +log_must directory_diff /$POOL/hole1 /$POOL2/rhole1 # Verify we can receive an intermediate clone redacted with respect to a # non-subset if we send from the bookmark. diff --git a/tests/zfs-tests/tests/functional/redacted_send/redacted_largeblocks.ksh b/tests/zfs-tests/tests/functional/redacted_send/redacted_largeblocks.ksh index caccdd360061..ef85bb31dcea 100755 --- a/tests/zfs-tests/tests/functional/redacted_send/redacted_largeblocks.ksh +++ b/tests/zfs-tests/tests/functional/redacted_send/redacted_largeblocks.ksh @@ -58,6 +58,6 @@ unmount_redacted $recvfs log_must eval "zfs send -L -i $sendfs@snap $clone@snap1 >$stream" log_must stream_has_features $stream large_blocks log_must eval "zfs recv $recvfs/new <$stream" -log_must diff -r $clone_mnt $recv_mnt/new +log_must directory_diff $clone_mnt $recv_mnt/new log_pass "Large blocks and redacted send work correctly together." diff --git a/tests/zfs-tests/tests/functional/rsend/recv_dedup.ksh b/tests/zfs-tests/tests/functional/rsend/recv_dedup.ksh index e6e282a1c6f8..ba23c4f6aa3c 100755 --- a/tests/zfs-tests/tests/functional/rsend/recv_dedup.ksh +++ b/tests/zfs-tests/tests/functional/rsend/recv_dedup.ksh @@ -48,6 +48,7 @@ log_must eval "zstream redup $sendfile | zfs recv -d $TESTPOOL/recv" log_must mkdir /$TESTPOOL/tar log_must tar --directory /$TESTPOOL/tar -xzf $tarfile -log_must diff -r /$TESTPOOL/tar /$TESTPOOL/recv +# The recv'd filesystem is called "/fs", so only compare that subdirectory. +log_must directory_diff /$TESTPOOL/tar/fs /$TESTPOOL/recv/fs log_pass "zfs can receive dedup send streams with 'zstream redup'" diff --git a/tests/zfs-tests/tests/functional/rsend/rsend.kshlib b/tests/zfs-tests/tests/functional/rsend/rsend.kshlib index 516d41263294..2c8085f226b0 100644 --- a/tests/zfs-tests/tests/functional/rsend/rsend.kshlib +++ b/tests/zfs-tests/tests/functional/rsend/rsend.kshlib @@ -212,7 +212,7 @@ function cmp_ds_cont srcdir=$(get_prop mountpoint $src_fs) dstdir=$(get_prop mountpoint $dst_fs) - diff -r $srcdir $dstdir > /dev/null 2>&1 + replay_directory_diff $srcdir $dstdir return $? } @@ -627,12 +627,12 @@ function file_check if [[ -d /$recvfs/.zfs/snapshot/a && -d \ /$sendfs/.zfs/snapshot/a ]]; then - diff -r /$recvfs/.zfs/snapshot/a /$sendfs/.zfs/snapshot/a + directory_diff /$recvfs/.zfs/snapshot/a /$sendfs/.zfs/snapshot/a [[ $? -eq 0 ]] || log_fail "Differences found in snap a" fi if [[ -d /$recvfs/.zfs/snapshot/b && -d \ /$sendfs/.zfs/snapshot/b ]]; then - diff -r /$recvfs/.zfs/snapshot/b /$sendfs/.zfs/snapshot/b + directory_diff /$recvfs/.zfs/snapshot/b /$sendfs/.zfs/snapshot/b [[ $? -eq 0 ]] || log_fail "Differences found in snap b" fi } diff --git a/tests/zfs-tests/tests/functional/slog/slog_replay_fs_001.ksh b/tests/zfs-tests/tests/functional/slog/slog_replay_fs_001.ksh index 0b78a099f0b9..0b78a9645a60 100755 --- a/tests/zfs-tests/tests/functional/slog/slog_replay_fs_001.ksh +++ b/tests/zfs-tests/tests/functional/slog/slog_replay_fs_001.ksh @@ -178,8 +178,8 @@ log_must rm /$TESTPOOL/$TESTFS/link_and_unlink.link # # 4. Copy TESTFS to temporary location (TESTDIR/copy) # -log_must mkdir -p $TESTDIR/copy -log_must cp -a /$TESTPOOL/$TESTFS/* $TESTDIR/copy/ +log_must mkdir -p $TESTDIR +log_must rsync -aHAX /$TESTPOOL/$TESTFS/ $TESTDIR/copy # # 5. Unmount filesystem and export the pool @@ -213,7 +213,7 @@ log_must ls_xattr /$TESTPOOL/$TESTFS/xattr.dir log_must ls_xattr /$TESTPOOL/$TESTFS/xattr.file log_note "Verify working set diff:" -log_must diff -r /$TESTPOOL/$TESTFS $TESTDIR/copy +log_must replay_directory_diff $TESTDIR/copy /$TESTPOOL/$TESTFS log_note "Verify file checksum:" typeset checksum1=$(sha256digest /$TESTPOOL/$TESTFS/payload) diff --git a/tests/zfs-tests/tests/functional/slog/slog_replay_fs_002.ksh b/tests/zfs-tests/tests/functional/slog/slog_replay_fs_002.ksh index 3c3ccdf4ad23..50ccc6089322 100755 --- a/tests/zfs-tests/tests/functional/slog/slog_replay_fs_002.ksh +++ b/tests/zfs-tests/tests/functional/slog/slog_replay_fs_002.ksh @@ -98,8 +98,8 @@ log_must eval 'for i in $(seq $NFILES); do zfs set dnodesize=${dnsize[$RANDOM % # # 4. Copy TESTFS to temporary location (TESTDIR/copy) # -log_must mkdir -p $TESTDIR/copy -log_must cp -a /$TESTPOOL/$TESTFS/* $TESTDIR/copy/ +log_must mkdir -p $TESTDIR +log_must rsync -aHAX /$TESTPOOL/$TESTFS/ $TESTDIR/copy # # 5. Unmount filesystem and export the pool @@ -132,6 +132,6 @@ log_note "Verify number of files" log_must test "$(ls /$TESTPOOL/$TESTFS/dir0 | wc -l)" -eq $NFILES log_note "Verify working set diff:" -log_must diff -r /$TESTPOOL/$TESTFS $TESTDIR/copy +log_must replay_directory_diff $TESTDIR/copy /$TESTPOOL/$TESTFS log_pass "Replay of intent log succeeds." diff --git a/tests/zfs-tests/tests/functional/snapshot/snapshot_002_pos.ksh b/tests/zfs-tests/tests/functional/snapshot/snapshot_002_pos.ksh index 124a7db9c6e6..6c9f33cc1d7c 100755 --- a/tests/zfs-tests/tests/functional/snapshot/snapshot_002_pos.ksh +++ b/tests/zfs-tests/tests/functional/snapshot/snapshot_002_pos.ksh @@ -121,7 +121,7 @@ log_must tar xf $TESTDIR/tarball.snapshot.tar cd $CWD || log_fail "Could not cd $CWD" -diff -q -r $TESTDIR/original $TESTDIR/snapshot > /dev/null 2>&1 +directory_diff $TESTDIR/original $TESTDIR/snapshot if [[ $? -eq 1 ]]; then log_fail "Directory structures differ." fi diff --git a/tests/zfs-tests/tests/functional/snapshot/snapshot_006_pos.ksh b/tests/zfs-tests/tests/functional/snapshot/snapshot_006_pos.ksh index 68a616c02a6c..892a38bc8ef6 100755 --- a/tests/zfs-tests/tests/functional/snapshot/snapshot_006_pos.ksh +++ b/tests/zfs-tests/tests/functional/snapshot/snapshot_006_pos.ksh @@ -119,7 +119,7 @@ log_must tar xf $TESTDIR1/tarball.snapshot.tar cd $CWD || log_fail "Could not cd $CWD" -diff -q -r $TESTDIR1/original $TESTDIR1/snapshot > /dev/null 2>&1 +directory_diff $TESTDIR1/original $TESTDIR1/snapshot if [[ $? -eq 1 ]]; then log_fail "Directory structures differ." fi