Skip to content

Commit

Permalink
tests: functional: switch to rsync for directory diffs
Browse files Browse the repository at this point in the history
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 <cyphar@cyphar.com>
  • Loading branch information
cyphar committed Jun 17, 2021
1 parent 841cee9 commit 4e7d867
Show file tree
Hide file tree
Showing 17 changed files with 44 additions and 24 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/zfs-tests-functional.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/zfs-tests-sanity.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
1 change: 1 addition & 0 deletions tests/zfs-tests/include/commands.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ export SYSTEM_FILES_COMMON='arp
readlink
rm
rmdir
rsync
scp
script
sed
Expand Down
19 changes: 19 additions & 0 deletions tests/zfs-tests/include/libtest.shlib
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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."
2 changes: 1 addition & 1 deletion tests/zfs-tests/tests/functional/rsend/recv_dedup.ksh
Original file line number Diff line number Diff line change
Expand Up @@ -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'"
6 changes: 3 additions & 3 deletions tests/zfs-tests/tests/functional/rsend/rsend.kshlib
Original file line number Diff line number Diff line change
Expand Up @@ -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 $?
}

Expand Down Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 4e7d867

Please sign in to comment.