From 4e7d8674ebd496c7dfb50cbb90a94949f0c58976 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. Signed-off-by: Aleksa Sarai --- .github/workflows/zfs-tests-functional.yml | 2 +- .github/workflows/zfs-tests-sanity.yml | 2 +- tests/zfs-tests/include/commands.cfg | 1 + tests/zfs-tests/include/libtest.shlib | 19 +++++++++++++++++++ .../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 | 2 +- .../tests/functional/rsend/rsend.kshlib | 6 +++--- .../functional/slog/slog_replay_fs_001.ksh | 2 +- .../functional/slog/slog_replay_fs_002.ksh | 2 +- .../functional/snapshot/snapshot_002_pos.ksh | 2 +- .../functional/snapshot/snapshot_006_pos.ksh | 2 +- 17 files changed, 44 insertions(+), 24 deletions(-) diff --git a/.github/workflows/zfs-tests-functional.yml b/.github/workflows/zfs-tests-functional.yml index eacc95ae1617..c673d5f9594e 100644 --- a/.github/workflows/zfs-tests-functional.yml +++ b/.github/workflows/zfs-tests-functional.yml @@ -19,7 +19,7 @@ jobs: run: | sudo apt-get update sudo apt-get install --yes -qq build-essential autoconf libtool gdb lcov \ - git alien fakeroot wget curl bc fio acl \ + git alien fakeroot wget curl bc fio acl rsync \ sysstat mdadm lsscsi parted gdebi attr dbench watchdog ksh \ nfs-kernel-server samba rng-tools xz-utils \ zlib1g-dev uuid-dev libblkid-dev libselinux-dev \ diff --git a/.github/workflows/zfs-tests-sanity.yml b/.github/workflows/zfs-tests-sanity.yml index 40a7f8ba511c..bc06d5432803 100644 --- a/.github/workflows/zfs-tests-sanity.yml +++ b/.github/workflows/zfs-tests-sanity.yml @@ -15,7 +15,7 @@ jobs: run: | sudo apt-get update sudo apt-get install --yes -qq build-essential autoconf libtool gdb lcov \ - git alien fakeroot wget curl bc fio acl \ + git alien fakeroot wget curl bc fio acl rsync \ sysstat mdadm lsscsi parted gdebi attr dbench watchdog ksh \ nfs-kernel-server samba rng-tools xz-utils \ zlib1g-dev uuid-dev libblkid-dev libselinux-dev \ diff --git a/tests/zfs-tests/include/commands.cfg b/tests/zfs-tests/include/commands.cfg index 0db9724eead0..e0d661a2a638 100644 --- a/tests/zfs-tests/include/commands.cfg +++ b/tests/zfs-tests/include/commands.cfg @@ -78,6 +78,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 5a360bd5e705..280ae4bfa285 100644 --- a/tests/zfs-tests/include/libtest.shlib +++ b/tests/zfs-tests/include/libtest.shlib @@ -4224,3 +4224,22 @@ 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. +# +function directory_diff # dir_a dir_b +{ + # 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). + diff="$(rsync -ni -acAHX --delete "$1/" "$2/")" + rv=0 + if [ -z "$diff" ]; then + echo "$diff" + rv=1 + fi + return $rv +} 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 da0aebe6b581..bbbd6f1f993e 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 a20fcc4ce224..5b497fb48c13 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 @@ -94,7 +94,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 > /dev/null 2>&1 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 a2d92673b180..29932618d3cf 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 @@ -73,7 +73,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..62d7d194ff80 100755 --- a/tests/zfs-tests/tests/functional/rsend/recv_dedup.ksh +++ b/tests/zfs-tests/tests/functional/rsend/recv_dedup.ksh @@ -48,6 +48,6 @@ 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 +log_must directory_diff /$TESTPOOL/tar /$TESTPOOL/recv 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 26755e87d0a5..60d277d4cfbc 100644 --- a/tests/zfs-tests/tests/functional/rsend/rsend.kshlib +++ b/tests/zfs-tests/tests/functional/rsend/rsend.kshlib @@ -204,7 +204,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 + directory_diff $srcdir $dstdir > /dev/null 2>&1 return $? } @@ -619,12 +619,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..7e5d48ee522c 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 @@ -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 directory_diff /$TESTPOOL/$TESTFS $TESTDIR/copy 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..121b627115d1 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 @@ -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 directory_diff /$TESTPOOL/$TESTFS $TESTDIR/copy 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..91e0e13d255b 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 > /dev/null 2>&1 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..e7df9130dfb0 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 > /dev/null 2>&1 if [[ $? -eq 1 ]]; then log_fail "Directory structures differ." fi